diff --git a/features/v3.0.0-beta1/048-rebac-permission-model-grants/design.md b/features/v3.0.0-beta1/048-rebac-permission-model-grants/design.md index 907eed45b2..9757f7b730 100644 --- a/features/v3.0.0-beta1/048-rebac-permission-model-grants/design.md +++ b/features/v3.0.0-beta1/048-rebac-permission-model-grants/design.md @@ -1057,8 +1057,15 @@ higher consistency。Grant/mode/lifecycle command 仍只允许从 `CURRENT` clai `scripts/reconcile_f048_projection_operations.py --tenant-id `；默认 dry-run 必须先核对 ledger、CURRENT Catalog 的 Store/model pin 与 scope fence，只有显式 `--apply` 才逐 operation 调用领域 `reconcile_operation()`。脚本不得直接 UPDATE operation -状态、资源镜像或增删 OpenFGA tuple；`FAILED_CLOSED`、pin/scope 不匹配或 ledger 不完整时 -保持阻断并转人工分析。重复传入 `FINALIZED` operation 只验证并跳过。 +状态、资源镜像或增删 OpenFGA tuple；普通 reconcile 对 `FAILED_CLOSED`、pin/scope 不匹配或 +ledger 不完整继续保持阻断。`FAILED_CLOSED` resource operation 经人工确认其冻结请求仍是目标后， +改用 `recover_f048_failed_closed_projection.py`：先 dry-run 用 higher consistency 读取 exact tuple， +操作者只输入 tenant/resource type/resource ID，脚本从资源 mode row 的 operation fence 解析 ledger； +按 ledger AFTER 集计算单次原子前向修正并输出绑定 live correction 的确认 checksum。apply 必须同时 +确认 Store、model 和该 checksum，任一资源绑定或 tuple 事实漂移即拒绝。领域恢复入口只补缺失 WRITE/多余 +DELETE，不重放已满足的 visible/action tuple；完整 AFTER verify 后才执行原 SQL finalizer，并以 +`FINALIZED + CURRENT(target_version)` 收口。超过 90 个 terminal correction、外部业务 scope、 +scope/version/operation 不匹配继续转人工分析。重复传入 `FINALIZED` operation 只验证并跳过。 #### Mode switch diff --git a/features/v3.0.0-beta1/048-rebac-permission-model-grants/tasks.md b/features/v3.0.0-beta1/048-rebac-permission-model-grants/tasks.md index 7d3678925d..96451e742b 100644 --- a/features/v3.0.0-beta1/048-rebac-permission-model-grants/tasks.md +++ b/features/v3.0.0-beta1/048-rebac-permission-model-grants/tasks.md @@ -1657,6 +1657,32 @@ arch-guard 与 diff-check 通过。 - **依赖**：T195 +--- + +## Wave 16 — FAILED_CLOSED 资源受控前向恢复 + +- [x] **T197：FAILED_CLOSED 前向恢复合同测试** + - **文件**：`src/backend/test/permission/test_f048_projection_service.py`, + `src/backend/test/permission/test_f048_projection_sql_runtime.py`, + `src/backend/test/permission/test_f048_failed_closed_recovery_cli.py` + - **测试**：覆盖 mixed live tuple 仅补 terminal difference、已存在 visible 不重复写、dry-run + checksum 漂移拒绝、FAILED_CLOSED SQL mirror 原子 finalize、CLI 按资源解析 operation、 + Store/model/checksum 二次确认和默认不写。 + - **覆盖 AC**：AC-166, AC-167, AC-170, AC-179 + - **依赖**：T159, T196 + +- [x] **T198：实现通用 resource FAILED_CLOSED 恢复脚本** + - **文件**：`src/backend/bisheng/permission/domain/services/projection_service.py`, + `src/backend/bisheng/permission/application/sql_runtime.py`, + `src/backend/scripts/recover_f048_failed_closed_projection.py`, + `src/backend/scripts/README.md` + - **逻辑**：只接受仍持有 expected version/operation fence 的 resource operation；以 durable + ledger AFTER 集和 higher-consistency exact reads 生成至多 90 条单次原子 correction，dry-run + checksum 绑定 live proposal，apply 后完整 verify 再复用 SQL finalizer 收口。普通 reconcile + 继续拒绝 FAILED_CLOSED，department 等外部业务 scope 不自动恢复。 + - **验收**：T197、F048 permission focused suite、Ruff、arch-guard 与 diff-check 通过。 + - **依赖**：T197 + ## 实际偏差记录 > 只记录一句话指针；设计原因和反直觉事实回写 [design.md](./design.md)。 diff --git a/src/backend/bisheng/permission/application/sql_runtime.py b/src/backend/bisheng/permission/application/sql_runtime.py index d9c53d695a..b77a738e7a 100644 --- a/src/backend/bisheng/permission/application/sql_runtime.py +++ b/src/backend/bisheng/permission/application/sql_runtime.py @@ -9,7 +9,7 @@ from uuid import uuid4 from loguru import logger -from sqlalchemy import func, update +from sqlalchemy import and_, func, or_, update from sqlmodel import select from bisheng.common.errcode.permission import ( @@ -445,6 +445,36 @@ async def is_expected_version( row_id = (await session.execute(statement)).scalar_one_or_none() return row_id is not None + async def is_failed_closed_recovery_scope( + self, + plan: ProjectionPlan, + operation_id: int, + ) -> bool: + if plan.scope_type != "resource": + return False + resource_type, separator, resource_id = plan.scope_key.partition(":") + if not separator: + return False + async with get_async_db_session() as session: + statement = select(ResourcePermissionMode.id).where( + ResourcePermissionMode.tenant_id == plan.tenant_id, + ResourcePermissionMode.resource_type == resource_type, + ResourcePermissionMode.resource_id == resource_id, + ResourcePermissionMode.operation_id == operation_id, + or_( + and_( + ResourcePermissionMode.version == plan.expected_version, + ResourcePermissionMode.projection_state == "FAILED_CLOSED", + ), + and_( + ResourcePermissionMode.version == plan.target_version, + ResourcePermissionMode.projection_state == "CURRENT", + ), + ), + ) + row_id = (await session.execute(statement)).scalar_one_or_none() + return row_id is not None + async def fail_closed( self, plan: ProjectionPlan, @@ -588,9 +618,14 @@ async def finalize( ) if mode_row.version == plan.target_version and mode_row.projection_state == "CURRENT": return - if mode_row.version != plan.expected_version or mode_row.projection_state != "PROJECTING": + if mode_row.version != plan.expected_version or mode_row.projection_state not in { + "PROJECTING", + "FAILED_CLOSED", + }: raise PermissionPublishNotReadyError(msg=("Permission scope changed before projection finalize")) + source_projection_state = mode_row.projection_state + grant_ids = tuple( ( await session.execute( @@ -733,7 +768,7 @@ async def finalize( ResourcePermissionMode.id == mode_row.id, ResourcePermissionMode.operation_id == operation_id, ResourcePermissionMode.version == plan.expected_version, - ResourcePermissionMode.projection_state == "PROJECTING", + ResourcePermissionMode.projection_state == source_projection_state, ) .values(**mode_values) ) diff --git a/src/backend/bisheng/permission/domain/services/projection_service.py b/src/backend/bisheng/permission/domain/services/projection_service.py index ac497aac33..5e1da24d15 100644 --- a/src/backend/bisheng/permission/domain/services/projection_service.py +++ b/src/backend/bisheng/permission/domain/services/projection_service.py @@ -2,7 +2,9 @@ from __future__ import annotations -from dataclasses import replace +import json +from dataclasses import dataclass, replace +from hashlib import sha256 from typing import Protocol from loguru import logger @@ -54,6 +56,12 @@ async def is_expected_version( operation_id: int, ) -> bool: ... + async def is_failed_closed_recovery_scope( + self, + plan: ProjectionPlan, + operation_id: int, + ) -> bool: ... + async def fail_closed(self, plan: ProjectionPlan, reason: str) -> None: ... @@ -99,6 +107,28 @@ async def emit(self, name: str, fields: dict) -> None: return None +@dataclass(frozen=True, slots=True) +class FailedClosedRecoveryPreview: + """Exact terminal-state correction proposed for one fenced resource.""" + + operation_id: int + tenant_id: int + operation_type: str + scope_type: str + scope_key: str + expected_version: int + target_version: int + store_id: str + model_id: str + operation_status: str + request_checksum: str + after_checksum: str + observed_state: str + target_tuple_count: int + correction_deltas: tuple[ProjectionTupleDelta, ...] + confirmation_checksum: str + + def restore_projection_plan( operation: PermissionProjectionOperation, tuple_rows, @@ -269,6 +299,153 @@ async def reconcile_operation( plan = restore_projection_plan(operation, tuple_rows) return await self.reconcile(plan) + async def inspect_failed_closed_recovery( + self, + operation_id: int, + ) -> FailedClosedRecoveryPreview: + """Build a read-only, checksum-bound forward recovery proposal.""" + + operation, plan = await self._load_operation_plan(operation_id) + status = str(operation.status) + if status not in { + ProjectionOperationStatus.FAILED_CLOSED.value, + ProjectionOperationStatus.COMMITTED.value, + }: + raise PermissionPublishNotReadyError( + msg=f"Projection operation is not recoverable from status {status}", + ) + if plan.scope_type != "resource": + raise PermissionPublishNotReadyError( + msg="FAILED_CLOSED forward recovery only supports resource scopes", + ) + + correction = await self._terminal_correction(plan.deltas) + observed_state = "AFTER" if not correction else await self._classify(plan.deltas) + confirmation_checksum = self._recovery_confirmation_checksum( + operation=operation, + correction=correction, + ) + return FailedClosedRecoveryPreview( + operation_id=int(operation.id), + tenant_id=plan.tenant_id, + operation_type=plan.operation_type, + scope_type=plan.scope_type, + scope_key=plan.scope_key, + expected_version=plan.expected_version, + target_version=plan.target_version, + store_id=plan.store_id, + model_id=plan.model_id, + operation_status=status, + request_checksum=operation.request_checksum, + after_checksum=operation.after_checksum, + observed_state=observed_state, + target_tuple_count=len( + projection_state_expectations(plan.deltas, after=True), + ), + correction_deltas=correction, + confirmation_checksum=confirmation_checksum, + ) + + async def recover_failed_closed_operation( + self, + operation_id: int, + *, + confirmation_checksum: str, + ) -> ProjectionOutcome: + """Forward-complete one fenced resource to its frozen AFTER state.""" + + operation, plan = await self._load_operation_plan(operation_id) + request_checksum = projection_request_checksum(plan) + status = str(operation.status) + if status == ProjectionOperationStatus.FINALIZED.value: + return self._outcome( + plan, + operation, + request_checksum=request_checksum, + idempotent=True, + reconciled=True, + ) + + preview = await self.inspect_failed_closed_recovery(operation_id) + if preview.confirmation_checksum != confirmation_checksum: + raise PermissionVersionConflictError( + msg="FAILED_CLOSED recovery confirmation checksum changed", + ) + if len(preview.correction_deltas) > MAX_ATOMIC_TUPLES: + raise PermissionPublishNotReadyError( + msg=( + "FAILED_CLOSED recovery requires more than " + f"{MAX_ATOMIC_TUPLES} atomic tuple corrections" + ), + ) + if not await self._scope_guard.is_failed_closed_recovery_scope( + plan, + int(operation.id), + ): + raise PermissionPublishNotReadyError( + msg="FAILED_CLOSED resource scope no longer owns the operation fence", + ) + + if status == ProjectionOperationStatus.COMMITTED.value: + if preview.correction_deltas: + raise PermissionProjectionFailedError( + msg="Committed recovery operation no longer has its full AFTER state", + ) + return await self._finalize( + plan, + operation, + request_checksum=request_checksum, + reconciled=True, + ) + + if not await self._marker.is_ready(): + raise PermissionPublishNotReadyError( + msg="Permission recent-change marker sentinel is not ready", + ) + try: + await self._marker.arm(plan) + commit_checksum = ( + await self._write(preview.correction_deltas) + if preview.correction_deltas + else operation.after_checksum + ) + except Exception as exc: + if not await self._is_after(plan.deltas): + await self._emit(plan, operation, "FAILED_CLOSED_RECOVERY_FAILED", exc) + raise PermissionProjectionFailedError(exception=exc) from exc + commit_checksum = operation.after_checksum + + if not await self._is_after(plan.deltas): + error = PermissionProjectionFailedError( + msg="FAILED_CLOSED recovery did not reach the frozen AFTER state", + ) + await self._emit(plan, operation, "FAILED_CLOSED_RECOVERY_FAILED", error) + raise error + await self._transition( + operation, + expected=ProjectionOperationStatus.FAILED_CLOSED.value, + target=ProjectionOperationStatus.COMMITTED.value, + commit_checksum=commit_checksum, + ) + return await self._finalize( + plan, + operation, + request_checksum=request_checksum, + reconciled=True, + ) + + async def _load_operation_plan( + self, + operation_id: int, + ) -> tuple[PermissionProjectionOperation, ProjectionPlan]: + operation = await self._repository.aget_operation(operation_id) + if operation is None: + raise PermissionPublishNotReadyError( + msg="Projection operation does not exist", + ) + tuple_rows = await self._repository.aget_operation_tuples(operation_id) + return operation, restore_projection_plan(operation, tuple_rows) + async def _run_prepared( self, plan: ProjectionPlan, @@ -495,6 +672,63 @@ async def _pending_stage_deltas( ) return tuple(delta for delta in deltas if (delta.key in present) != (delta.action == "WRITE")) + async def _terminal_correction( + self, + deltas: tuple[ProjectionTupleDelta, ...], + ) -> tuple[ProjectionTupleDelta, ...]: + """Return one exact mutation for every tuple not at terminal AFTER.""" + + expected = projection_state_expectations(deltas, after=True) + representatives: dict[tuple[str, str, str], ProjectionTupleDelta] = {} + for delta in deltas: + representatives[delta.key] = delta + present = await self._fga.read_present( + tuple(representatives[key] for key in sorted(representatives)), + consistency=HIGHER_CONSISTENCY, + ) + correction: list[ProjectionTupleDelta] = [] + for sequence, (key, should_exist) in enumerate(sorted(expected.items())): + if (key in present) == should_exist: + continue + correction.append( + replace( + representatives[key], + phase="COMMIT", + sequence=sequence, + action="WRITE" if should_exist else "DELETE", + ) + ) + return tuple(correction) + + @staticmethod + def _recovery_confirmation_checksum( + *, + operation: PermissionProjectionOperation, + correction: tuple[ProjectionTupleDelta, ...], + ) -> str: + payload = { + "after_checksum": operation.after_checksum, + "correction": [ + { + "action": delta.action, + "object": delta.object, + "relation": delta.relation, + "user": delta.user, + } + for delta in correction + ], + "operation_id": int(operation.id), + "operation_status": str(operation.status), + "request_checksum": operation.request_checksum, + } + canonical = json.dumps( + payload, + ensure_ascii=True, + separators=(",", ":"), + sort_keys=True, + ) + return sha256(canonical.encode("utf-8")).hexdigest() + async def _classify( self, deltas: tuple[ProjectionTupleDelta, ...], diff --git a/src/backend/scripts/README.md b/src/backend/scripts/README.md index b6ce98123e..3c4b7c4407 100644 --- a/src/backend/scripts/README.md +++ b/src/backend/scripts/README.md @@ -148,6 +148,55 @@ is safe: already `FINALIZED` operations are verified and skipped. Catalog publish should only be retried when the final `remaining_active` report is empty. +### `recover_f048_failed_closed_projection.py` + +Forward-recover one explicitly selected F048 resource operation that has +already entered `FAILED_CLOSED`. This is separate from ordinary reconcile: it +uses the operation ledger's frozen AFTER state, reads the exact live tuples +with higher consistency, and proposes only the missing writes and surplus +deletes. It never derives authorization intent from staged SQL rows and never +updates SQL or OpenFGA directly. + +Run a dry-run first from `src/backend/` with the same `config` as the service: + +```bash +export config=config.yaml +PYTHONPATH=./ .venv/bin/python \ + scripts/recover_f048_failed_closed_projection.py \ + --tenant-id 1 \ + --resource-type knowledge_space \ + --resource-id 4166 +``` + +The dry-run prints the live Store/model pins and a +`recovery_confirmation_checksum` bound to the exact correction proposal. Copy +those three values into the apply command: + +```bash +PYTHONPATH=./ .venv/bin/python \ + scripts/recover_f048_failed_closed_projection.py \ + --tenant-id 1 \ + --resource-type knowledge_space \ + --resource-id 4166 \ + --apply \ + --confirm-store-id '' \ + --confirm-model-id '' \ + --confirm-recovery-checksum '' +``` + +The script resolves the active operation from the resource mode row; operators +do not need to discover or enter an operation ID. The resolved ID remains in +the dry-run output for audit. Apply is refused when tenant, resource scope, +operation ownership, expected +version, CURRENT Catalog Store/model pin, durable ledger checksum, or the live +tuple proposal changed after dry-run. The correction must fit one atomic +OpenFGA write (at most 90 tuples). Only resource scopes are supported; external +business-owned scopes such as department remain manual-analysis cases. A +successful run higher-consistency verifies the full AFTER state, finalizes the +staged SQL rows, advances the resource version, and ends at +`operation=FINALIZED` plus `resource projection_state=CURRENT`. Re-running a +finalized operation only verifies and skips it. + ### `migrate_f048_permission_data.py` Formal, forward-only migration from the legacy relation-model Config and diff --git a/src/backend/scripts/recover_f048_failed_closed_projection.py b/src/backend/scripts/recover_f048_failed_closed_projection.py new file mode 100644 index 0000000000..dd60991d86 --- /dev/null +++ b/src/backend/scripts/recover_f048_failed_closed_projection.py @@ -0,0 +1,357 @@ +#!/usr/bin/env python3 +"""Forward-recover one FAILED_CLOSED F048 resource projection. + +The script treats the durable projection operation as the frozen intent. It +reads the live OpenFGA state with higher consistency, proposes only the exact +differences required to reach the operation's AFTER state, and binds that +proposal to a confirmation checksum. It never updates SQL or OpenFGA directly; +``--apply`` delegates the write, verification, and SQL finalize sequence to the +normal permission projection domain service. + +Run from ``src/backend/`` with the same ``config`` value as the live service: + + export config=config.yaml + PYTHONPATH=./ .venv/bin/python scripts/recover_f048_failed_closed_projection.py \ + --tenant-id 1 --resource-type knowledge_space --resource-id 4166 + +The default is dry-run. Review the output and repeat with ``--apply`` plus the +three exact confirmation values printed by the dry-run. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import os +import sys +import traceback +from collections import Counter +from dataclasses import dataclass +from typing import Any + +_BACKEND_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) +if _BACKEND_ROOT not in sys.path: + sys.path.insert(0, _BACKEND_ROOT) + +from sqlmodel import select # noqa: E402 + +from bisheng.common.services.config_service import settings # noqa: E402 +from bisheng.core.context.manager import ( # noqa: E402 + app_context, + close_app_context, + initialize_app_context, +) +from bisheng.core.context.tenant import ( # noqa: E402 + bypass_tenant_filter, + current_tenant_id, + set_current_tenant_id, +) +from bisheng.core.database import get_async_db_session # noqa: E402 +from bisheng.department.domain.services.department_projection_scope import ( # noqa: E402 + get_department_projection_scope, +) +from bisheng.permission.application.runtime import ( # noqa: E402 + build_f048_permission_runtime, +) +from bisheng.permission.application.sql_runtime import ( # noqa: E402 + SqlCatalogDecisionState, +) +from bisheng.permission.domain.models import ( # noqa: E402 + PermissionProjectionOperation, + ResourcePermissionMode, +) +from bisheng.permission.domain.repositories.projection_repository import ( # noqa: E402 + ProjectionRepository, +) + +EXIT_OK = 0 +EXIT_BLOCKED = 3 +EXIT_RUNTIME_ERROR = 4 + + +class FailedClosedRecoveryBlockedError(RuntimeError): + """A recovery preflight or confirmation invariant was not satisfied.""" + + +@dataclass(frozen=True, slots=True) +class RecoveryRuntime: + client: Any + projection: Any + repository: Any + + +def _positive_int(value: str) -> int: + parsed = int(value) + if parsed <= 0: + raise argparse.ArgumentTypeError("value must be a positive integer") + return parsed + + +def _resource_type(value: str) -> str: + if not value.strip() or ":" in value: + raise argparse.ArgumentTypeError("resource type must be non-empty and must not contain ':'") + return value + + +def _resource_id(value: str) -> str: + if not value.strip(): + raise argparse.ArgumentTypeError("resource ID must be non-empty") + return value + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument("--tenant-id", type=_positive_int, required=True) + parser.add_argument("--resource-type", type=_resource_type, required=True) + parser.add_argument("--resource-id", type=_resource_id, required=True) + parser.add_argument("--apply", action="store_true") + parser.add_argument("--confirm-store-id") + parser.add_argument("--confirm-model-id") + parser.add_argument("--confirm-recovery-checksum") + args = parser.parse_args(argv) + if args.apply: + missing = [ + flag + for flag, value in ( + ("--confirm-store-id", args.confirm_store_id), + ("--confirm-model-id", args.confirm_model_id), + ("--confirm-recovery-checksum", args.confirm_recovery_checksum), + ) + if not value + ] + if missing: + parser.error(f"--apply requires {', '.join(missing)}") + return args + + +def _require(condition: bool, message: str) -> None: + if not condition: + raise FailedClosedRecoveryBlockedError(message) + + +async def _build_runtime() -> RecoveryRuntime: + client = await app_context.async_get_instance("openfga") + await SqlCatalogDecisionState( + expected_store_id=client.store_id, + expected_model_id=client.model_id, + ).ensure_runtime_ready() + components = await build_f048_permission_runtime( + client, + external_scopes={ + "department": get_department_projection_scope(), + }, + ) + return RecoveryRuntime( + client=client, + projection=components.projection, + repository=ProjectionRepository(), + ) + + +async def _load_mode( + *, + tenant_id: int, + resource_type: str, + resource_id: str, +) -> ResourcePermissionMode | None: + with bypass_tenant_filter(): + async with get_async_db_session() as session: + statement = select(ResourcePermissionMode).where( + ResourcePermissionMode.tenant_id == tenant_id, + ResourcePermissionMode.resource_type == resource_type, + ResourcePermissionMode.resource_id == resource_id, + ) + return (await session.execute(statement)).scalars().first() + + +def _mode_payload(mode: ResourcePermissionMode) -> dict[str, Any]: + return { + "mode": mode.mode, + "version": int(mode.version), + "projection_state": mode.projection_state, + "operation_id": int(mode.operation_id) if mode.operation_id is not None else None, + } + + +async def _load_operation( + runtime: RecoveryRuntime, + operation_id: int, +) -> PermissionProjectionOperation | None: + with bypass_tenant_filter(): + return await runtime.repository.aget_operation(operation_id) + + +async def inspect( + runtime: RecoveryRuntime, + args: argparse.Namespace, +) -> tuple[Any | None, dict[str, Any]]: + scope_key = f"{args.resource_type}:{args.resource_id}" + mode = await _load_mode( + tenant_id=args.tenant_id, + resource_type=args.resource_type, + resource_id=args.resource_id, + ) + _require(mode is not None, "resource permission mode row does not exist") + _require(mode.operation_id is not None, "resource has no bound projection operation") + operation_id = int(mode.operation_id) + operation = await _load_operation(runtime, operation_id) + _require(operation is not None, f"bound operation {operation_id} does not exist") + _require( + int(operation.tenant_id) == args.tenant_id, + f"operation belongs to tenant {operation.tenant_id}, not {args.tenant_id}", + ) + _require(operation.scope_type == "resource", "only resource projection scopes are supported") + _require(operation.scope_key == scope_key, "bound operation scope does not match the resource") + _require( + operation.store_id == runtime.client.store_id + and operation.model_id == runtime.client.model_id, + "operation Store/model pin does not match the CURRENT runtime", + ) + + if operation.status == "FINALIZED": + _require( + int(mode.version) >= int(operation.target_version) + and mode.projection_state == "CURRENT", + "FINALIZED operation has an inconsistent resource mirror", + ) + return None, { + "operation_id": operation_id, + "status": "FINALIZED", + "scope_key": scope_key, + "resource_mode": _mode_payload(mode), + } + + _require( + operation.status in {"FAILED_CLOSED", "COMMITTED"}, + f"operation cannot be forward-recovered from status {operation.status}", + ) + valid_mode = ( + int(mode.version) == int(operation.expected_version) + and mode.projection_state == "FAILED_CLOSED" + ) or ( + operation.status == "COMMITTED" + and int(mode.version) == int(operation.target_version) + and mode.projection_state == "CURRENT" + ) + _require(valid_mode, "resource mirror does not match the failed operation fence") + + preview = await runtime.projection.inspect_failed_closed_recovery(operation_id) + with bypass_tenant_filter(): + visible_sources = await runtime.repository.aget_visible_operation_sources( + operation_id, + ) + visible_source_summary = Counter(source.state for source in visible_sources) + if operation.operation_type == "GRANT_MUTATION": + _require(visible_sources, "Grant mutation has no frozen visible source after-state") + if mode.projection_state == "FAILED_CLOSED" and visible_sources: + _require( + set(visible_source_summary) == {"PENDING"}, + "failed operation visible source after-state is incomplete or mixed", + ) + correction_summary = Counter( + f"{delta.action}:{delta.relation}" for delta in preview.correction_deltas + ) + payload = { + "operation_id": preview.operation_id, + "tenant_id": preview.tenant_id, + "operation_type": preview.operation_type, + "status": preview.operation_status, + "scope_key": preview.scope_key, + "expected_version": preview.expected_version, + "target_version": preview.target_version, + "store_id": preview.store_id, + "model_id": preview.model_id, + "request_checksum": preview.request_checksum, + "after_checksum": preview.after_checksum, + "observed_state": preview.observed_state, + "target_tuple_count": preview.target_tuple_count, + "correction_tuple_count": len(preview.correction_deltas), + "correction_summary": dict(sorted(correction_summary.items())), + "visible_source_count": len(visible_sources), + "visible_source_summary": dict(sorted(visible_source_summary.items())), + "recovery_confirmation_checksum": preview.confirmation_checksum, + "resource_mode": _mode_payload(mode), + } + return preview, payload + + +async def execute(args: argparse.Namespace) -> int: + tenant_token = set_current_tenant_id(args.tenant_id) + initialized = False + try: + await initialize_app_context(config=settings) + initialized = True + runtime = await _build_runtime() + preview, payload = await inspect(runtime, args) + print( + json.dumps( + { + "event": "failed_closed_recovery_preflight", + "mode": "apply" if args.apply else "dry-run", + **payload, + }, + ensure_ascii=False, + sort_keys=True, + ) + ) + if preview is None: + print("[skip] operation is already FINALIZED and the resource mirror is CURRENT") + return EXIT_OK + if not args.apply: + print( + "[dry-run] no SQL or OpenFGA mutations were requested; repeat with --apply " + "and the printed Store/model/recovery confirmation values" + ) + return EXIT_OK + + _require(args.confirm_store_id == preview.store_id, "Store confirmation does not match") + _require(args.confirm_model_id == preview.model_id, "model confirmation does not match") + _require( + args.confirm_recovery_checksum == preview.confirmation_checksum, + "recovery confirmation checksum does not match the live proposal", + ) + outcome = await runtime.projection.recover_failed_closed_operation( + preview.operation_id, + confirmation_checksum=args.confirm_recovery_checksum, + ) + verified_preview, verified = await inspect(runtime, args) + _require(verified_preview is None, "operation did not reach FINALIZED") + print( + json.dumps( + { + "event": "failed_closed_recovery_finalized", + "operation_id": preview.operation_id, + "status": outcome.status, + "resource_mode": verified["resource_mode"], + }, + ensure_ascii=False, + sort_keys=True, + ) + ) + return EXIT_OK + finally: + try: + if initialized: + await close_app_context() + finally: + current_tenant_id.reset(tenant_token) + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv) + try: + return asyncio.run(execute(args)) + except FailedClosedRecoveryBlockedError as exc: + print(f"F048 FAILED_CLOSED recovery blocked: {exc}", file=sys.stderr) + return EXIT_BLOCKED + except Exception: + traceback.print_exc() + return EXIT_RUNTIME_ERROR + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/backend/test/permission/test_f048_failed_closed_recovery_cli.py b/src/backend/test/permission/test_f048_failed_closed_recovery_cli.py new file mode 100644 index 0000000000..2dd0e8fd82 --- /dev/null +++ b/src/backend/test/permission/test_f048_failed_closed_recovery_cli.py @@ -0,0 +1,256 @@ +"""CLI safety contract for FAILED_CLOSED resource projection recovery.""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from bisheng.core.context.tenant import get_current_tenant_id +from scripts import recover_f048_failed_closed_projection as cli + + +def _args(*extra: str): + return cli.parse_args( + [ + "--tenant-id", + "1", + "--resource-type", + "knowledge_space", + "--resource-id", + "4166", + *extra, + ] + ) + + +def _operation(**overrides): + values = { + "id": 405, + "tenant_id": 1, + "operation_type": "GRANT_MUTATION", + "scope_type": "resource", + "scope_key": "knowledge_space:4166", + "expected_version": 19, + "target_version": 20, + "store_id": "store-live", + "model_id": "model-live", + "status": "FAILED_CLOSED", + } + values.update(overrides) + return SimpleNamespace(**values) + + +def _mode(**overrides): + values = { + "mode": "CUSTOM", + "version": 19, + "projection_state": "FAILED_CLOSED", + "operation_id": 405, + } + values.update(overrides) + return SimpleNamespace(**values) + + +def _preview(): + return SimpleNamespace( + operation_id=405, + tenant_id=1, + operation_type="GRANT_MUTATION", + operation_status="FAILED_CLOSED", + scope_key="knowledge_space:4166", + expected_version=19, + target_version=20, + store_id="store-live", + model_id="model-live", + request_checksum="r" * 64, + after_checksum="a" * 64, + observed_state="MIXED", + target_tuple_count=7, + correction_deltas=( + SimpleNamespace(action="WRITE", relation="ordinary_assignee"), + SimpleNamespace(action="DELETE", relation="ordinary_assignee"), + ), + confirmation_checksum="c" * 64, + ) + + +class FakeRepository: + def __init__(self, operation) -> None: + self.operation = operation + + async def aget_operation(self, operation_id: int): + return self.operation if operation_id == self.operation.id else None + + async def aget_visible_operation_sources(self, operation_id: int): + assert operation_id == self.operation.id + return [SimpleNamespace(state="PENDING")] + + +class FakeProjection: + def __init__(self, preview) -> None: + self.preview = preview + + async def inspect_failed_closed_recovery(self, operation_id: int): + assert operation_id == 405 + return self.preview + + +def test_parse_args_defaults_to_dry_run_and_apply_requires_confirmations() -> None: + args = _args() + assert args.apply is False + + with pytest.raises(SystemExit) as exc_info: + _args("--apply") + assert exc_info.value.code == 2 + + args = _args( + "--apply", + "--confirm-store-id", + "store-live", + "--confirm-model-id", + "model-live", + "--confirm-recovery-checksum", + "c" * 64, + ) + assert args.apply is True + + +@pytest.mark.asyncio +async def test_inspect_binds_operation_scope_pin_and_live_correction(monkeypatch) -> None: + preview = _preview() + runtime = cli.RecoveryRuntime( + client=SimpleNamespace(store_id="store-live", model_id="model-live"), + projection=FakeProjection(preview), + repository=FakeRepository(_operation()), + ) + + async def load_mode(**kwargs): + assert kwargs == { + "tenant_id": 1, + "resource_type": "knowledge_space", + "resource_id": "4166", + } + return _mode() + + monkeypatch.setattr(cli, "_load_mode", load_mode) + inspected, payload = await cli.inspect(runtime, _args()) + + assert inspected is preview + assert payload["observed_state"] == "MIXED" + assert payload["correction_tuple_count"] == 2 + assert payload["correction_summary"] == { + "DELETE:ordinary_assignee": 1, + "WRITE:ordinary_assignee": 1, + } + assert payload["visible_source_summary"] == {"PENDING": 1} + assert payload["recovery_confirmation_checksum"] == "c" * 64 + + +@pytest.mark.asyncio +async def test_execute_dry_run_never_recovers(monkeypatch, capsys) -> None: + events: list[object] = [] + preview = _preview() + runtime = SimpleNamespace() + + async def initialize_context(*, config): + events.append(("initialize", config)) + + async def close_context(): + events.append("close") + + async def build_runtime(): + events.append(("runtime", get_current_tenant_id())) + return runtime + + async def inspect(runtime_arg, args): + assert runtime_arg is runtime + events.append(("inspect", args.resource_type, args.resource_id)) + return preview, { + "operation_id": 405, + "status": "FAILED_CLOSED", + } + + monkeypatch.setattr(cli, "initialize_app_context", initialize_context) + monkeypatch.setattr(cli, "close_app_context", close_context) + monkeypatch.setattr(cli, "_build_runtime", build_runtime) + monkeypatch.setattr(cli, "inspect", inspect) + + exit_code = await cli.execute(_args()) + + assert exit_code == cli.EXIT_OK + assert events[1:] == [ + ("runtime", 1), + ("inspect", "knowledge_space", "4166"), + "close", + ] + assert get_current_tenant_id() is None + assert "[dry-run] no SQL or OpenFGA mutations" in capsys.readouterr().out + + +@pytest.mark.asyncio +async def test_execute_apply_uses_confirmed_domain_recovery_and_verifies(monkeypatch) -> None: + preview = _preview() + + class ApplyingProjection: + def __init__(self) -> None: + self.calls: list[tuple[int, str]] = [] + + async def recover_failed_closed_operation( + self, + operation_id: int, + *, + confirmation_checksum: str, + ): + self.calls.append((operation_id, confirmation_checksum)) + return SimpleNamespace(status="FINALIZED") + + projection = ApplyingProjection() + runtime = SimpleNamespace(projection=projection) + inspections = iter( + ( + (preview, {"operation_id": 405, "status": "FAILED_CLOSED"}), + ( + None, + { + "operation_id": 405, + "status": "FINALIZED", + "resource_mode": { + "version": 20, + "projection_state": "CURRENT", + }, + }, + ), + ) + ) + + async def no_op_context(**kwargs): + del kwargs + + async def build_runtime(): + return runtime + + async def inspect(*args, **kwargs): + del args, kwargs + return next(inspections) + + monkeypatch.setattr(cli, "initialize_app_context", no_op_context) + monkeypatch.setattr(cli, "close_app_context", no_op_context) + monkeypatch.setattr(cli, "_build_runtime", build_runtime) + monkeypatch.setattr(cli, "inspect", inspect) + + exit_code = await cli.execute( + _args( + "--apply", + "--confirm-store-id", + "store-live", + "--confirm-model-id", + "model-live", + "--confirm-recovery-checksum", + "c" * 64, + ) + ) + + assert exit_code == cli.EXIT_OK + assert projection.calls == [(405, "c" * 64)] + assert get_current_tenant_id() is None diff --git a/src/backend/test/permission/test_f048_projection_service.py b/src/backend/test/permission/test_f048_projection_service.py index 5918324431..449146ee4b 100644 --- a/src/backend/test/permission/test_f048_projection_service.py +++ b/src/backend/test/permission/test_f048_projection_service.py @@ -116,6 +116,7 @@ class FakeScopeGuard: def __init__(self) -> None: self.current = True self.fenced = False + self.recoverable = True self.reserve_error: Exception | None = None self.reservations: list[tuple[str, int]] = [] @@ -136,6 +137,15 @@ async def is_expected_version( assert operation_id > 0 return self.current + async def is_failed_closed_recovery_scope( + self, + plan: ProjectionPlan, + operation_id: int, + ) -> bool: + assert plan.scope_type == "resource" + assert operation_id > 0 + return self.recoverable + async def fail_closed(self, plan: ProjectionPlan, reason: str) -> None: assert reason self.fenced = True @@ -535,6 +545,90 @@ async def test_mixed_commit_result_is_failed_closed() -> None: assert scope.fenced is True +@pytest.mark.asyncio +async def test_failed_closed_recovery_writes_only_terminal_difference() -> None: + new_manager = ProjectionTupleDelta( + phase="COMMIT", + sequence=0, + action="WRITE", + user="user:841", + relation="ordinary_assignee", + object="permission_grant:g-manager", + ) + old_editor = ProjectionTupleDelta( + phase="COMMIT", + sequence=1, + action="DELETE", + user="user:841", + relation="ordinary_assignee", + object="permission_grant:g-editor", + ) + existing_visible = ProjectionTupleDelta( + phase="COMMIT", + sequence=2, + action="WRITE", + user="user:841", + relation="visible", + object="knowledge_space:4166", + ) + plan = _plan( + new_manager, + old_editor, + existing_visible, + idempotency_key="recover-4166", + change_item_count=1, + ) + service, repository, _, _, fga, finalizer, _ = _service(plan) + fga.present.add(_key(existing_visible)) + fga.timeout_mode = "before" + + with pytest.raises(PermissionProjectionFailedError): + await service.execute(plan) + assert repository.operation.status == ProjectionOperationStatus.FAILED_CLOSED + + preview = await service.inspect_failed_closed_recovery(int(repository.operation.id)) + assert preview.scope_key == "workflow:42" + assert preview.observed_state == "MIXED" + assert {_key(row) for row in preview.correction_deltas} == { + _key(new_manager), + _key(old_editor), + } + + fga.reject_existing_writes = True + outcome = await service.recover_failed_closed_operation( + int(repository.operation.id), + confirmation_checksum=preview.confirmation_checksum, + ) + + assert outcome.status == ProjectionOperationStatus.FINALIZED + assert repository.operation.status == ProjectionOperationStatus.FINALIZED + writes, deletes = fga.calls[-1] + assert tuple(_key(row) for row in writes) == (_key(new_manager),) + assert tuple(_key(row) for row in deletes) == (_key(old_editor),) + assert _key(existing_visible) in fga.present + assert finalizer.calls == 1 + + +@pytest.mark.asyncio +async def test_failed_closed_recovery_rejects_stale_confirmation() -> None: + plan = _plan(_delta(1), _delta(2)) + service, repository, _, _, fga, _, _ = _service(plan) + fga.timeout_mode = "mixed" + with pytest.raises(PermissionProjectionFailedError): + await service.execute(plan) + + preview = await service.inspect_failed_closed_recovery(int(repository.operation.id)) + fga.present.add(_key(_delta(2))) + + with pytest.raises(PermissionVersionConflictError, match="confirmation checksum"): + await service.recover_failed_closed_operation( + int(repository.operation.id), + confirmation_checksum=preview.confirmation_checksum, + ) + + assert repository.operation.status == ProjectionOperationStatus.FAILED_CLOSED + + @pytest.mark.asyncio async def test_definite_stage_failure_applies_inverse_compensation() -> None: plan = _plan( diff --git a/src/backend/test/permission/test_f048_projection_sql_runtime.py b/src/backend/test/permission/test_f048_projection_sql_runtime.py index ddb765bda8..9772b3bb15 100644 --- a/src/backend/test/permission/test_f048_projection_sql_runtime.py +++ b/src/backend/test/permission/test_f048_projection_sql_runtime.py @@ -126,7 +126,11 @@ def _plan() -> ProjectionPlan: ) -async def _seed_projecting_state(session_factory) -> int: +async def _seed_projecting_state( + session_factory, + *, + projection_state: str = "PROJECTING", +) -> int: with bypass_tenant_filter(): async with session_factory() as session: async with session.begin(): @@ -154,7 +158,7 @@ async def _seed_projecting_state(session_factory) -> int: resource_id="42", mode="CUSTOM", version=3, - projection_state="PROJECTING", + projection_state=projection_state, operation_id=int(operation.id), ) grant = PermissionGrant( @@ -295,6 +299,31 @@ async def test_resource_finalizer_atomically_converges_and_replays( assert [row.state for row in visible_sources] == ["ACTIVE", "RETIRED"] +@pytest.mark.asyncio +async def test_resource_finalizer_forward_recovers_failed_closed_mirror( + session_factory, +) -> None: + operation_id = await _seed_projecting_state( + session_factory, + projection_state="FAILED_CLOSED", + ) + guard = SqlProjectionScopeGuard() + + assert await guard.is_failed_closed_recovery_scope(_plan(), operation_id) is True + await SqlProjectionFinalizer().finalize(_plan(), operation_id) + + with bypass_tenant_filter(): + async with session_factory() as session: + mode = (await session.execute(select(ResourcePermissionMode))).scalars().one() + grant = (await session.execute(select(PermissionGrant))).scalars().one() + assert (mode.version, mode.projection_state, mode.mode) == ( + 4, + "CURRENT", + "INHERIT", + ) + assert (grant.state, grant.projection_state) == ("ACTIVE", "CURRENT") + + @pytest.mark.asyncio async def test_resource_finalizer_retires_old_model_source_after_assignee_move( session_factory, diff --git a/src/frontend/client/src/components/permission/PermissionDialog.test.tsx b/src/frontend/client/src/components/permission/PermissionDialog.test.tsx index 128275cc37..2c6f24a506 100644 --- a/src/frontend/client/src/components/permission/PermissionDialog.test.tsx +++ b/src/frontend/client/src/components/permission/PermissionDialog.test.tsx @@ -9,12 +9,6 @@ import type { ResourcePermissionContext } from "~/api/permission"; import { ModeHeader } from "./ModeHeader"; import { PermissionDialog } from "./PermissionDialog"; -jest.mock("~/hooks/AuthContext", () => ({ - // Not the creator of anything in these fixtures: the top-tier guard reads the - // viewer from the roster, and none of them carry a CREATOR row. - useAuthContext: () => ({ user: { id: "auth-user" } }), -})); - jest.mock("~/api/permission", () => ({ applyResourcePermissionModeDraft: jest.fn(), createResourcePermissionModeDraft: jest.fn(), diff --git a/src/frontend/client/src/components/permission/PermissionGrantTab.test.tsx b/src/frontend/client/src/components/permission/PermissionGrantTab.test.tsx index 84d8a744cd..9d47a9778c 100644 --- a/src/frontend/client/src/components/permission/PermissionGrantTab.test.tsx +++ b/src/frontend/client/src/components/permission/PermissionGrantTab.test.tsx @@ -9,12 +9,6 @@ import type { } from "~/api/permission"; import { PermissionGrantTab } from "./PermissionGrantTab"; -jest.mock("~/hooks/AuthContext", () => ({ - // Not the creator of anything in these fixtures: the top-tier guard reads the - // viewer from the roster, and none of them carry a CREATOR row. - useAuthContext: () => ({ user: { id: "auth-user" } }), -})); - jest.mock("~/api/permission", () => ({ getGrantablePermissionModels: jest.fn(), mutateResourceGrants: jest.fn(), @@ -114,6 +108,7 @@ describe("F048 Client PermissionGrantTab", () => { mockedGetModels.mockResolvedValue([ { key: "viewer", name: "Viewer", level: 1, active: true }, { key: "editor", name: "Editor", level: 2, active: true }, + { key: "owner", name: "Owner", level: 4, active: true }, { key: "inactive", name: "Inactive", level: 3, active: false }, ]); mockedMutate.mockResolvedValue({ resource_version: 8, items: [] }); @@ -179,6 +174,9 @@ describe("F048 Client PermissionGrantTab", () => { ).toBeInTheDocument(), ); expect(screen.queryByText("Inactive")).not.toBeInTheDocument(); + expect(screen.getByLabelText("f048_permission.grant.add_model")).toHaveTextContent( + "Owner", + ); fireEvent.change( screen.getByLabelText("f048_permission.grant.model.1"), diff --git a/src/frontend/client/src/components/permission/PermissionGrantTab.tsx b/src/frontend/client/src/components/permission/PermissionGrantTab.tsx index 75e7e45ded..30c13c2014 100644 --- a/src/frontend/client/src/components/permission/PermissionGrantTab.tsx +++ b/src/frontend/client/src/components/permission/PermissionGrantTab.tsx @@ -16,11 +16,9 @@ import type { } from "~/api/permission"; import { Button, Checkbox } from "~/components/ui"; import { useLocalize } from "~/hooks"; -import { useAuthContext } from "~/hooks/AuthContext"; import { SubjectSearchDepartment } from "./SubjectSearchDepartment"; import { SubjectSearchUser } from "./SubjectSearchUser"; import { SubjectSearchUserGroup } from "./SubjectSearchUserGroup"; -import { canManageLevel, viewerIsCreator } from "./topTierGuard"; const SUBJECT_TYPES: SubjectType[] = ["user", "department", "user_group"]; @@ -92,12 +90,6 @@ export function PermissionGrantTab({ const handleIncludeChildrenChange = onIncludeChildrenChange ?? setInternalIncludeChildren; - const { user } = useAuthContext(); - const isCreator = useMemo( - () => viewerIsCreator(assignees, user?.id), - [assignees, user?.id], - ); - useEffect(() => { let cancelled = false; setModelsLoading(true); @@ -105,11 +97,7 @@ export function PermissionGrantTab({ void getGrantablePermissionModels(resourceType, resourceId) .then((result) => { if (cancelled) return; - // Hiding the edit control on existing owner rows would be pointless if - // the same viewer could still grant a fresh one here. - const activeModels = result.filter( - (model) => model.active && canManageLevel(model.level, isCreator), - ); + const activeModels = result.filter((model) => model.active); setModels(activeModels); setSelectedModelKey((current) => activeModels.some((model) => model.key === current) @@ -127,7 +115,7 @@ export function PermissionGrantTab({ return () => { cancelled = true; }; - }, [resourceId, resourceType, isCreator]); + }, [resourceId, resourceType]); useEffect(() => { setTargetModels({}); diff --git a/src/frontend/client/src/components/permission/PermissionListTab.test.tsx b/src/frontend/client/src/components/permission/PermissionListTab.test.tsx index 467364fa72..58dbf2f8f7 100644 --- a/src/frontend/client/src/components/permission/PermissionListTab.test.tsx +++ b/src/frontend/client/src/components/permission/PermissionListTab.test.tsx @@ -10,12 +10,6 @@ import type { } from "~/api/permission"; import { PermissionListTab } from "./PermissionListTab"; -jest.mock("~/hooks/AuthContext", () => ({ - // Not the creator of anything in these fixtures: the top-tier guard reads the - // viewer from the roster, and none of them carry a CREATOR row. - useAuthContext: () => ({ user: { id: "auth-user" } }), -})); - jest.mock("~/api/permission", () => ({ getGrantablePermissionModels: jest.fn(), getMyResourcePermissions: jest.fn(), @@ -114,9 +108,7 @@ describe("F048 Client PermissionListTab", () => { ).toBeInTheDocument(); }); - it("locks owner rows for an owner who is not the creator", async () => { - // The creator's own row is already protected by the server; what this hides - // is one ordinary owner editing another — peers of the same trust tier. + it("allows an editable ordinary owner row to be managed", async () => { mockedGetGrants.mockResolvedValueOnce({ data: [ assignee("9", "DIRECT", { @@ -138,7 +130,7 @@ describe("F048 Client PermissionListTab", () => { ); const ownerRow = await screen.findByTestId("permission-assignee-9"); - expect(ownerRow).toHaveAttribute("data-editable", "false"); + expect(ownerRow).toHaveAttribute("data-editable", "true"); }); it("renders protected and inherited grants as read-only and paginates by cursor", async () => { diff --git a/src/frontend/client/src/components/permission/PermissionListTab.tsx b/src/frontend/client/src/components/permission/PermissionListTab.tsx index f052023408..ab2e0aed1f 100644 --- a/src/frontend/client/src/components/permission/PermissionListTab.tsx +++ b/src/frontend/client/src/components/permission/PermissionListTab.tsx @@ -27,10 +27,8 @@ import type { } from "~/api/permission"; import { Button } from "~/components/ui"; import { useLocalize } from "~/hooks"; -import { useAuthContext } from "~/hooks/AuthContext"; import { useConfirm } from "~/Providers"; import { SourceBadge } from "./SourceBadge"; -import { canManageLevel, viewerIsCreator } from "./topTierGuard"; interface PermissionListTabProps { resourceType: ResourceType; @@ -58,16 +56,13 @@ function createMutationIdempotencyKey(): string { function canEditAssignee( assignee: PermissionGrantAssignee, context: ResourcePermissionContext, - isCreator: boolean, ): boolean { return ( context.mode === "CUSTOM" && context.can_manage_permission && assignee.scope === "LOCAL" && assignee.editable && - !assignee.protected && - // Top-tier grants stay with the creator; an owner does not manage owners. - canManageLevel(assignee.model.level, isCreator) + !assignee.protected ); } @@ -101,7 +96,6 @@ interface RosterRowProps { context: ResourcePermissionContext; models: GrantablePermissionModel[]; pending: boolean; - isCreator: boolean; onMove: (assignee: PermissionGrantAssignee, modelKey: string) => void; onRemove: (assignee: PermissionGrantAssignee) => void; } @@ -111,13 +105,12 @@ function RosterRow({ context, models, pending, - isCreator, onMove, onRemove, }: RosterRowProps) { const localize = useLocalize(); const SubjectIcon = SUBJECT_ICONS[assignee.subject.type]; - const editable = canEditAssignee(assignee, context, isCreator); + const editable = canEditAssignee(assignee, context); const displayName = assignee.subject.name || `${assignee.subject.type}:${assignee.subject.id}`; @@ -254,7 +247,6 @@ export function PermissionListTab({ }: PermissionListTabProps) { const localize = useLocalize(); const confirm = useConfirm(); - const { user } = useAuthContext(); const [assignees, setAssignees] = useState([]); const [models, setModels] = useState([]); const [summary, setSummary] = useState(null); @@ -358,11 +350,6 @@ export function PermissionListTab({ resourceType, ]); - const isCreator = useMemo( - () => viewerIsCreator(assignees, user?.id), - [assignees, user?.id], - ); - const visibleAssignees = useMemo(() => { const query = searchQuery.trim().toLowerCase(); return assignees.filter((assignee) => { @@ -496,7 +483,6 @@ export function PermissionListTab({ context={context} models={models} pending={pendingAssigneeId === assignee.assignee_id} - isCreator={isCreator} onMove={(item, modelKey) => void mutateAssignee(item, { op: "MOVE", diff --git a/src/frontend/client/src/components/permission/topTierGuard.test.ts b/src/frontend/client/src/components/permission/topTierGuard.test.ts deleted file mode 100644 index f5a559b7c8..0000000000 --- a/src/frontend/client/src/components/permission/topTierGuard.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { canManageLevel, viewerIsCreator } from "./topTierGuard"; - -/** Twin of the platform app's guard test — the two copies must not drift. */ -function row(overrides: Record = {}) { - return { - assignee_id: "1", - assignee_version: 1, - subject: { type: "user", id: "7", name: "Alice" }, - model: { key: "owner", name: "Owner", level: 4, active: true }, - source: { type: "DIRECT", include_children: false }, - scope: "LOCAL", - inherited_from: null, - protected: false, - editable: true, - ...overrides, - } as never; -} - -describe("top-tier grant guard", () => { - it("recognises the creator from their own roster row", () => { - const roster = [ - row({ - source: { type: "CREATOR", include_children: false }, - subject: { type: "user", id: "7" }, - }), - row({ subject: { type: "user", id: "9" } }), - ]; - expect(viewerIsCreator(roster, 7)).toBe(true); - expect(viewerIsCreator(roster, "7")).toBe(true); - expect(viewerIsCreator(roster, 9)).toBe(false); - }); - - it("does not mistake an ordinary owner for the creator", () => { - // The reported case: granted owner, so allowed to manage the resource — - // but not to edit the other owners sitting beside them. - const roster = [row({ subject: { type: "user", id: "7" } })]; - expect(viewerIsCreator(roster, 7)).toBe(false); - }); - - it("does not mistake a group or department for the viewer", () => { - const roster = [ - row({ - source: { type: "CREATOR", include_children: false }, - subject: { type: "department", id: "7" }, - }), - ]; - expect(viewerIsCreator(roster, 7)).toBe(false); - }); - - it("reads an anonymous viewer as not the creator", () => { - const roster = [ - row({ - source: { type: "CREATOR", include_children: false }, - subject: { type: "user", id: "7" }, - }), - ]; - expect(viewerIsCreator(roster, null)).toBe(false); - expect(viewerIsCreator(roster, undefined)).toBe(false); - }); - - it("locks only the top tier, and only for non-creators", () => { - expect(canManageLevel(4, false)).toBe(false); - expect(canManageLevel(4, true)).toBe(true); - for (const level of [1, 2, 3]) { - expect(canManageLevel(level, false)).toBe(true); - } - // A model with no level is not the top tier. - expect(canManageLevel(null, false)).toBe(true); - expect(canManageLevel(undefined, false)).toBe(true); - }); -}); diff --git a/src/frontend/client/src/components/permission/topTierGuard.ts b/src/frontend/client/src/components/permission/topTierGuard.ts deleted file mode 100644 index 5daa7beb92..0000000000 --- a/src/frontend/client/src/components/permission/topTierGuard.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { PermissionGrantAssignee } from "~/api/permission"; - -/** - * Top-tier (owner) grants may only be managed by the resource creator. - * - * This is a UI guardrail, not an authorization boundary. The catalog still ships - * `owner.allow_same_level = true`, so the server continues to accept an owner - * changing another owner and a direct API call is unaffected. The creator's own - * row is already safe from everyone: it is a protected assignment and the server - * refuses to move or remove it outright, so what this hides is one ordinary - * owner editing another — peers of the same trust tier. - * - * Enforcing it properly means changing what the permission model expresses, and - * that decision is still open. Kept in step with the platform app's copy. - */ - -export const TOP_TIER_LEVEL = 4; - -/** Read the viewer's standing from the roster it already loaded. - * - * Only the creator's own row identifies them, so a roster paged past that row - * reads as "not the creator" — restrictive, which is the safe direction for a - * guardrail. Rosters are served sorted with a default page of 50, so in practice - * the row is on the first page. - */ -export function viewerIsCreator( - assignees: PermissionGrantAssignee[], - currentUserId: string | number | null | undefined, -): boolean { - if (currentUserId === null || currentUserId === undefined) return false; - const userId = String(currentUserId); - return assignees.some( - (assignee) => - assignee.source.type === "CREATOR" && - assignee.subject.type === "user" && - String(assignee.subject.id) === userId, - ); -} - -/** Whether the viewer may act on a grant at this model level. */ -export function canManageLevel( - level: number | null | undefined, - isCreator: boolean, -): boolean { - return level !== TOP_TIER_LEVEL || isCreator; -} diff --git a/src/frontend/platform/src/components/bs-comp/permission/PermissionGrantTab.tsx b/src/frontend/platform/src/components/bs-comp/permission/PermissionGrantTab.tsx index 3c2821cff2..91162ab1e3 100644 --- a/src/frontend/platform/src/components/bs-comp/permission/PermissionGrantTab.tsx +++ b/src/frontend/platform/src/components/bs-comp/permission/PermissionGrantTab.tsx @@ -11,11 +11,9 @@ import { type ResourcePermissionContext, } from "@/controllers/API/permission" import { LockKeyhole } from "lucide-react" -import { userContext } from "@/contexts/userContext" -import { useContext, useEffect, useMemo, useState } from "react" +import { useEffect, useMemo, useState } from "react" import { useTranslation } from "react-i18next" import { SourceBadge } from "./SourceBadge" -import { canManageLevel, viewerIsCreator } from "./topTierGuard" import { SubjectSearchDepartment } from "./SubjectSearchDepartment" import { SubjectSearchUser } from "./SubjectSearchUser" import { SubjectSearchUserGroup } from "./SubjectSearchUserGroup" @@ -164,11 +162,6 @@ export function PermissionGrantTab({ onSuccess, }: PermissionGrantTabProps) { const { t } = useTranslation("permission") - const { user } = useContext(userContext) - const isCreator = useMemo( - () => viewerIsCreator(assignees, user?.user_id), - [assignees, user?.user_id], - ) const [models, setModels] = useState([]) const [subjectType, setSubjectType] = useState( fixedSubjectType ?? "user", @@ -192,11 +185,7 @@ export function PermissionGrantTab({ void getGrantablePermissionModelsApi(resourceType, resourceId) .then((result) => { if (cancelled) return - // Hiding the edit control on existing owner rows would be pointless if - // the same viewer could still grant a fresh one here. - const activeModels = result.filter( - (model) => model.active && canManageLevel(model.level, isCreator), - ) + const activeModels = result.filter((model) => model.active) setModels(activeModels) setSelectedModelKey((current) => activeModels.some((model) => model.key === current) @@ -213,7 +202,7 @@ export function PermissionGrantTab({ return () => { cancelled = true } - }, [resourceId, resourceType, isCreator]) + }, [resourceId, resourceType]) useEffect(() => { if (fixedSubjectType) setSubjectType(fixedSubjectType) diff --git a/src/frontend/platform/src/components/bs-comp/permission/PermissionListTab.tsx b/src/frontend/platform/src/components/bs-comp/permission/PermissionListTab.tsx index bbf20f7dae..baef4824bf 100644 --- a/src/frontend/platform/src/components/bs-comp/permission/PermissionListTab.tsx +++ b/src/frontend/platform/src/components/bs-comp/permission/PermissionListTab.tsx @@ -23,11 +23,9 @@ import { User, Users, } from "lucide-react" -import { userContext } from "@/contexts/userContext" -import { useCallback, useContext, useEffect, useMemo, useState } from "react" +import { useCallback, useEffect, useMemo, useState } from "react" import { useTranslation } from "react-i18next" import { SourceBadge } from "./SourceBadge" -import { canManageLevel, viewerIsCreator } from "./topTierGuard" import type { SubjectType } from "./types" interface PermissionListTabProps { @@ -54,16 +52,13 @@ function createMutationIdempotencyKey(): string { function assigneeEditable( assignee: PermissionGrantAssignee, context: ResourcePermissionContext, - isCreator: boolean, ): boolean { return ( context.mode === "CUSTOM" && context.can_manage_permission && assignee.scope === "LOCAL" && assignee.editable && - !assignee.protected && - // Top-tier grants stay with the creator; an owner does not manage owners. - canManageLevel(assignee.model.level, isCreator) + !assignee.protected ) } @@ -95,7 +90,6 @@ interface RosterRowProps { context: ResourcePermissionContext models: GrantablePermissionModel[] pending: boolean - isCreator: boolean onMove: (assignee: PermissionGrantAssignee, modelKey: string) => void onRemove: (assignee: PermissionGrantAssignee) => void } @@ -105,13 +99,12 @@ function RosterRow({ context, models, pending, - isCreator, onMove, onRemove, }: RosterRowProps) { const { t } = useTranslation("permission") const SubjectIcon = SUBJECT_ICONS[assignee.subject.type] - const editable = assigneeEditable(assignee, context, isCreator) + const editable = assigneeEditable(assignee, context) const displayName = assignee.subject.name || `${assignee.subject.type}:${assignee.subject.id}` @@ -231,7 +224,6 @@ export function PermissionListTab({ onMutationSuccess, }: PermissionListTabProps) { const { t } = useTranslation("permission") - const { user } = useContext(userContext) const [assignees, setAssignees] = useState([]) const [models, setModels] = useState([]) const [summary, setSummary] = useState(null) @@ -333,11 +325,6 @@ export function PermissionListTab({ resourceType, ]) - const isCreator = useMemo( - () => viewerIsCreator(assignees, user?.user_id), - [assignees, user?.user_id], - ) - const visibleAssignees = useMemo(() => { const query = searchQuery.trim().toLowerCase() return assignees.filter((assignee) => { @@ -468,7 +455,6 @@ export function PermissionListTab({ context={context} models={models} pending={pendingAssigneeId === assignee.assignee_id} - isCreator={isCreator} onMove={(item, modelKey) => void mutateAssignee(item, { op: "MOVE", diff --git a/src/frontend/platform/src/components/bs-comp/permission/topTierGuard.ts b/src/frontend/platform/src/components/bs-comp/permission/topTierGuard.ts deleted file mode 100644 index b1bdc8f37c..0000000000 --- a/src/frontend/platform/src/components/bs-comp/permission/topTierGuard.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { PermissionGrantAssignee } from "@/controllers/API/permission" - -/** - * Top-tier (owner) grants may only be managed by the resource creator. - * - * This is a UI guardrail, not an authorization boundary. The catalog still ships - * `owner.allow_same_level = true`, so the server continues to accept an owner - * changing another owner and a direct API call is unaffected. The creator's own - * row is already safe from everyone: it is a protected assignment and the server - * refuses to move or remove it outright, so what this hides is one ordinary - * owner editing another — peers of the same trust tier. - * - * Enforcing it properly means changing what the permission model expresses, and - * that decision is still open. - */ - -export const TOP_TIER_LEVEL = 4 - -/** Read the viewer's standing from the roster it already loaded. - * - * Only the creator's own row identifies them, so a roster paged past that row - * reads as "not the creator" — restrictive, which is the safe direction for a - * guardrail. Rosters are served sorted with a default page of 50, so in practice - * the row is on the first page. - */ -export function viewerIsCreator( - assignees: PermissionGrantAssignee[], - currentUserId: string | number | null | undefined, -): boolean { - if (currentUserId === null || currentUserId === undefined) return false - const userId = String(currentUserId) - return assignees.some( - (assignee) => - assignee.source.type === "CREATOR" && - assignee.subject.type === "user" && - String(assignee.subject.id) === userId, - ) -} - -/** Whether the viewer may act on a grant at this model level. */ -export function canManageLevel( - level: number | null | undefined, - isCreator: boolean, -): boolean { - return level !== TOP_TIER_LEVEL || isCreator -} diff --git a/src/frontend/platform/src/test/f048PermissionGrantTab.test.tsx b/src/frontend/platform/src/test/f048PermissionGrantTab.test.tsx index 57e1f0d875..4cc2323fef 100644 --- a/src/frontend/platform/src/test/f048PermissionGrantTab.test.tsx +++ b/src/frontend/platform/src/test/f048PermissionGrantTab.test.tsx @@ -77,6 +77,7 @@ describe("F048 PermissionGrantTab", () => { vi.mocked(getGrantablePermissionModelsApi).mockResolvedValue([ { key: "viewer", name: "Viewer", level: 1, active: true }, { key: "editor", name: "Editor", level: 2, active: true }, + { key: "owner", name: "Owner", level: 4, active: true }, ]) vi.mocked(mutateResourceGrantsApi).mockResolvedValue({ resource_version: 9, @@ -136,7 +137,7 @@ describe("F048 PermissionGrantTab", () => { ) expect(await screen.findByText("Editor")).toBeInTheDocument() - expect(screen.queryByText("Owner")).toBeNull() + expect(screen.getByText("Owner")).toBeInTheDocument() fireEvent.click(screen.getByRole("button", { name: "select-alice" })) fireEvent.change(screen.getByLabelText("grant.addModel"), { target: { value: "editor" }, diff --git a/src/frontend/platform/src/test/f048TopTierGuard.test.ts b/src/frontend/platform/src/test/f048TopTierGuard.test.ts deleted file mode 100644 index a4db211661..0000000000 --- a/src/frontend/platform/src/test/f048TopTierGuard.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { - canManageLevel, - viewerIsCreator, -} from "@/components/bs-comp/permission/topTierGuard" -import { describe, expect, it } from "vitest" - -function row(overrides: Record = {}) { - return { - assignee_id: "1", - assignee_version: 1, - subject: { type: "user", id: "7", name: "Alice" }, - model: { key: "owner", name: "Owner", level: 4, active: true }, - source: { type: "DIRECT", include_children: false }, - scope: "LOCAL", - inherited_from: null, - protected: false, - editable: true, - ...overrides, - } as never -} - -describe("top-tier grant guard", () => { - it("recognises the creator from their own roster row", () => { - const roster = [ - row({ source: { type: "CREATOR", include_children: false }, subject: { type: "user", id: "7" } }), - row({ subject: { type: "user", id: "9" } }), - ] - expect(viewerIsCreator(roster, 7)).toBe(true) - expect(viewerIsCreator(roster, "7")).toBe(true) - expect(viewerIsCreator(roster, 9)).toBe(false) - }) - - it("does not mistake an ordinary owner for the creator", () => { - const roster = [row({ subject: { type: "user", id: "7" } })] - expect(viewerIsCreator(roster, 7)).toBe(false) - }) - - it("does not mistake a group or department for the viewer", () => { - const roster = [ - row({ - source: { type: "CREATOR", include_children: false }, - subject: { type: "department", id: "7" }, - }), - ] - expect(viewerIsCreator(roster, 7)).toBe(false) - }) - - it("reads as not-the-creator without a signed-in user", () => { - const roster = [ - row({ source: { type: "CREATOR", include_children: false }, subject: { type: "user", id: "7" } }), - ] - expect(viewerIsCreator(roster, null)).toBe(false) - expect(viewerIsCreator(roster, undefined)).toBe(false) - }) - - it("fails closed when the creator row is not on the loaded page", () => { - // Restrictive is the safe direction for a guardrail: the creator loses the - // control until more rows load, rather than an owner gaining it. - expect(viewerIsCreator([row({ subject: { type: "user", id: "7" } })], 7)).toBe(false) - }) - - it("reserves the top tier for the creator", () => { - expect(canManageLevel(4, false)).toBe(false) - expect(canManageLevel(4, true)).toBe(true) - }) - - it("leaves every lower tier alone", () => { - for (const level of [1, 2, 3]) { - expect(canManageLevel(level, false)).toBe(true) - } - }) - - it("treats a level-less model as manageable", () => { - expect(canManageLevel(null, false)).toBe(true) - expect(canManageLevel(undefined, false)).toBe(true) - }) -})