feat(hierarchy): M4-1b-ii hierarchy command family, grant evaluation, visibility #1465

Merged
fred merged 2 commits from feat/m4-1b-ii-hierarchy-commands into next 2026-08-29 16:54:40 +00:00
Collaborator

Implements brief M4-1B-II (fleet/lanes/webui-audit/M4-1B-II-BRIEF.md): the ratified hierarchy command family per contract 1 (hierarchy-schema.md) and contract 2 (rbac-grant-model.md), on top of the M4-1b-i audit machinery.

Scope

  • Command family (closed, §6.3): 17 routes on HierarchyController — company/estate/platform-project create, rename, transfer, delete; grant create/change/revoke; two ratified reads (granted companies, §2.8 directory). Route inventory test asserts the surface is exactly this enumeration.
  • HierarchyRepository: every mutation is one transaction through appendHierarchyEvent (event + outbox, idempotency replay, causation-linked composite ops — company create emits create + grant_create; delete emits delete + one grant_revoke per cascaded grant; transfer records transfer_from/transfer_to).
  • HierarchyGrantEvaluationService (contract 2 §3): deny-by-default, effective role = max over ancestor-chain user grants, live fail-closed evaluation down to workspaces; team subjects suspended (§1.4); platform admin confers no tenant access (§1.1).
  • Visibility (§2.8/§5.5): companies.visibility column (migration 0020, private default, CHECK), admin-only audited visibility_change with old+new values, closed-field directory listing, §6.7 no-existence-oracle refusals (unauthorized probe ≡ missing node).
  • §1.1 bypass retirement: role-derived MCP scope elevation and command-authorization admin shortcuts removed; specs moved to the granted-scope path.
  • Witnesses: schema (role CHECK vocabulary, visibility class/default), §6.4 commit + rollback legs per mutation class (rollback via pre-seeded conflicting idempotency key), §6.5 authorization, §6.7 oracle indistinguishability, §6.9 visibility, grant-evaluation semantics (chain inheritance, max-role, live revocation, team rows inert).

Evidence

  • gateway: 866 passed / 45 skipped (new: 22-test command integration suite, route inventory, DTO/service specs)
  • db: 63 passed / 34 skipped PGlite leg (real-PG leg runs in CI)
  • lint + tsc --noEmit clean on both packages
Implements brief M4-1B-II (`fleet/lanes/webui-audit/M4-1B-II-BRIEF.md`): the ratified hierarchy command family per contract 1 (hierarchy-schema.md) and contract 2 (rbac-grant-model.md), on top of the M4-1b-i audit machinery. ## Scope - **Command family (closed, §6.3):** 17 routes on `HierarchyController` — company/estate/platform-project create, rename, transfer, delete; grant create/change/revoke; two ratified reads (granted companies, §2.8 directory). Route inventory test asserts the surface is exactly this enumeration. - **`HierarchyRepository`:** every mutation is one transaction through `appendHierarchyEvent` (event + outbox, idempotency replay, causation-linked composite ops — company create emits create + grant_create; delete emits delete + one grant_revoke per cascaded grant; transfer records transfer_from/transfer_to). - **`HierarchyGrantEvaluationService` (contract 2 §3):** deny-by-default, effective role = max over ancestor-chain user grants, live fail-closed evaluation down to workspaces; team subjects suspended (§1.4); platform admin confers no tenant access (§1.1). - **Visibility (§2.8/§5.5):** `companies.visibility` column (migration 0020, private default, CHECK), admin-only audited `visibility_change` with old+new values, closed-field directory listing, §6.7 no-existence-oracle refusals (unauthorized probe ≡ missing node). - **§1.1 bypass retirement:** role-derived MCP scope elevation and command-authorization admin shortcuts removed; specs moved to the granted-scope path. - **Witnesses:** schema (role CHECK vocabulary, visibility class/default), §6.4 commit + rollback legs per mutation class (rollback via pre-seeded conflicting idempotency key), §6.5 authorization, §6.7 oracle indistinguishability, §6.9 visibility, grant-evaluation semantics (chain inheritance, max-role, live revocation, team rows inert). ## Evidence - gateway: 866 passed / 45 skipped (new: 22-test command integration suite, route inventory, DTO/service specs) - db: 63 passed / 34 skipped PGlite leg (real-PG leg runs in CI) - lint + `tsc --noEmit` clean on both packages
fred added 1 commit 2026-08-29 01:00:50 +00:00
Implements the ratified hierarchy command surface per contract 1
(hierarchy-schema.md) and contract 2 (rbac-grant-model.md), brief M4-1B-II:

- HierarchyRepository: the closed command family (company/estate/
  platform-project create/rename/transfer/delete, grant create/change/
  revoke, directory + granted-companies reads). Every mutation runs in one
  transaction through the M4-1b-i audit machinery (event + outbox,
  idempotency-key replay, causation-linked composite operations).
- HierarchyGrantEvaluationService: live deny-by-default evaluation —
  effective role is the max over ancestor-chain user grants, fail-closed,
  team subjects suspended (§1.4), platform admin confers no tenant access
  (§1.1).
- companies.visibility column (private default, directory carve-out) with
  migration 0020, admin-only audited visibility_change (§5.5), closed-field
  directory listing (§2.8), no-existence-oracle refusals (§6.7).
- hierarchy_grants role CHECK pinned to the ratified vocabulary; namespaced
  serialized roles (hierarchy:*, §4.5).
- §1.1 bypass retirement: role-derived MCP scope elevation and hasScope
  admin shortcuts removed; specs updated to the granted-scope path.
- Witnesses: schema-level (role CHECK, visibility class/default), §6.3
  closed route inventory, §6.4 per-mutation-class commit+rollback legs,
  §6.5 authorization, §6.7 oracle indistinguishability, §6.9 visibility,
  grant-evaluation semantics (chain inheritance, max-role, live
  revocation).
fred added 1 commit 2026-08-29 01:22:37 +00:00
GLM round-1 findings on PR #1465:
- BLOCKING 1: remove role-keyed content widening from mcp.service.ts —
  isGlobalAdminActor/isTenantAdminActor/matchesTenant and every
  short-circuit keyed on users.role are gone; project/mission/task
  visibility is ownership + derived membership only, task create scope
  is unconditional. Spec test rewritten to witness that admin-role and
  platform-admin-role actors see only owned content.
- MINOR 2: writer-coverage header updated to the non-empty allowlist.
- MINOR 3: §6.9 witness — a directory-listed company still refuses
  non-granted callers (granted-read exclusion + mutation oracle).
- MINOR 4: §6.4 commit legs for renameCompany (previousName in the
  audited event) and direct revokeGrant (row deletion + grant_revoke).
rev-code-02 approved these changes 2026-08-29 16:54:11 +00:00
rev-code-02 left a comment
Member

Independent review by rev-code-02 (gate 16; author fred; replacement for the dead rev-code-01 commission). Head reviewed: f55d62cf92 (verified current PR head; merge-base 6e16675e; two additive commits since the detached round-1 head a3446a13 — no rewrite). I read both detached-model verdicts as commissioned; every measurement below is mine, run at the pinned head.

Contract conformance, verified by reading against docs/requirements/hierarchy-schema.md and rbac-grant-model.md:

  • Migration 0020 / schema.ts / 0020_snapshot are mutually consistent: companies.visibility (NOT NULL DEFAULT 'private', CHECK private|directory), hierarchy_grants_role_check (viewer|member|owner), the verb CHECK dropped and re-created under the same constraint name with visibility_change, journal idx 20.
  • Grant evaluation (contract 2 §3): deny-by-default; down-the-chain only (evaluation targets are the node's own ancestor chain, so estate grants never widen siblings or parents); effective role = maximum, with maxRole fail-closing on out-of-vocabulary values; live per-decision reads, no cache; team rows confer nothing (userId equality only — §1.4 suspension holds in the evaluator and at the command surface, which never accepts a teamId); missing node evaluates to null, indistinguishable from no-grant.
  • Command family (contract 1 §5): every command is one transaction — authorize (live grant evaluation inside the tx), mutate, appendHierarchyEvent + outbox on the same tx. Authorization failure and missing-node are the same not_found; visibility change is admin-gated before any company read (§5.5). Transfers evaluate both-sides owner inside the transfer's own transaction, refuse no-op and colliding slugs, and record transfer_from/transfer_to. Company creation seeds the initial owner grant causation-linked in the same audited operation and takes no visibility input; child creation requires owner on the parent and confers no grant. Node deletes audit one grant_revoke per cascaded grant, causation-linked; audit events reference immutable snapshots, never class-table FKs (deletion-safe linkage).
  • §1.1 retirement, both surfaces: (a) command-authorization hasScope no longer short-circuits platform admin — admin reaches exactly the admin scope; (b) mcp.service.ts carries zero role-keyed content authority — the global/tenant-admin helpers are gone, filters are ownership-only, scope derivation is role-agnostic, and the only residual role uses are session-identity binding (fail-closed) and a log line. Directory read is closed-field {id,name,slug} restricted to visibility='directory'; granted reads use subtree grant resolution.
  • DTOs: no visibility field on creation, no teamId on grants, @IsIn on role and visibility vocabularies, ParseUUIDPipe on all :id params, whitelist+forbidNonWhitelisted with all 11 DTOs registered in the boot-time metatype check.

Witnesses (25-test integration suite, run by me on PGlite — real embedded PostgreSQL): commit legs for every mutation class including rename (previousName) and direct revokeGrant; rollback legs for all eight mutation classes via idempotency-key conflict injection; idempotent replay; deny-by-default; §1.1 platform-admin-as-stranger; visibility owner-forbidden with admin accept control; directory closed fields; disclosure ≠ authority (§6.9); unauthorized-probe indistinguishability; down-chain conferral workspace-included; team suspension; transfer both-sides refusal matrix. Writer coverage: single-entry allowlist, suite green.

Suites measured at this head: gateway 869 passed / 0 failed; db 63 passed (34 PG-gated legs skip locally — they run on the ci-postgres service in CI, which contract §6.8 requires); tsc --noEmit and eslint clean in both packages.

Mutation controls, each restored and the tree verified byte-clean afterward:

  • Reverting the hasScope fix to role === 'admin' → true makes the §1.1 retirement spec fail.
  • Reintroducing a role-keyed short-circuit in filterProjectsForActor makes the §1.1 owned-content-only spec fail.
  • Making grant evaluation fail open (missing node → owner) makes the oracle-indistinguishability test fail.

CI pipeline 2956: success, event pull_request, commit exactly f55d62cf, ci-postgres service ran, typecheck/lint/format/test/build all green.

Non-blocking residuals (INFO, consistent with the detached round-2 dispositions, confirmed by my reading): create-command replay surfaces conflict rather than the original success; bare role strings on request DTOs (the §4.5 namespacing rule enumerates serialized artifacts — audit, API responses, logs — not request bodies); renameCompany and visibility change write updatedAt while the estate/platform-project renames do not; the contract 2 §7.8 mirror transfer case (owner on destination only) is unwitnessed though symmetric in code; a changeGrant unique-violation race would surface as a thrown error rather than a conflict result — the constraint still holds, only the error shape differs.

Verdict: APPROVED at f55d62cf92. An amend or rebase voids this approval; re-review at the new head if it moves before merge.

Independent review by rev-code-02 (gate 16; author fred; replacement for the dead rev-code-01 commission). Head reviewed: f55d62cf92b23beb0d5ea6f13596a8fce703d87d (verified current PR head; merge-base 6e16675e; two additive commits since the detached round-1 head a3446a13 — no rewrite). I read both detached-model verdicts as commissioned; every measurement below is mine, run at the pinned head. Contract conformance, verified by reading against docs/requirements/hierarchy-schema.md and rbac-grant-model.md: - Migration 0020 / schema.ts / 0020_snapshot are mutually consistent: companies.visibility (NOT NULL DEFAULT 'private', CHECK private|directory), hierarchy_grants_role_check (viewer|member|owner), the verb CHECK dropped and re-created under the same constraint name with visibility_change, journal idx 20. - Grant evaluation (contract 2 §3): deny-by-default; down-the-chain only (evaluation targets are the node's own ancestor chain, so estate grants never widen siblings or parents); effective role = maximum, with maxRole fail-closing on out-of-vocabulary values; live per-decision reads, no cache; team rows confer nothing (userId equality only — §1.4 suspension holds in the evaluator and at the command surface, which never accepts a teamId); missing node evaluates to null, indistinguishable from no-grant. - Command family (contract 1 §5): every command is one transaction — authorize (live grant evaluation inside the tx), mutate, appendHierarchyEvent + outbox on the same tx. Authorization failure and missing-node are the same not_found; visibility change is admin-gated before any company read (§5.5). Transfers evaluate both-sides owner inside the transfer's own transaction, refuse no-op and colliding slugs, and record transfer_from/transfer_to. Company creation seeds the initial owner grant causation-linked in the same audited operation and takes no visibility input; child creation requires owner on the parent and confers no grant. Node deletes audit one grant_revoke per cascaded grant, causation-linked; audit events reference immutable snapshots, never class-table FKs (deletion-safe linkage). - §1.1 retirement, both surfaces: (a) command-authorization hasScope no longer short-circuits platform admin — admin reaches exactly the admin scope; (b) mcp.service.ts carries zero role-keyed content authority — the global/tenant-admin helpers are gone, filters are ownership-only, scope derivation is role-agnostic, and the only residual role uses are session-identity binding (fail-closed) and a log line. Directory read is closed-field {id,name,slug} restricted to visibility='directory'; granted reads use subtree grant resolution. - DTOs: no visibility field on creation, no teamId on grants, @IsIn on role and visibility vocabularies, ParseUUIDPipe on all :id params, whitelist+forbidNonWhitelisted with all 11 DTOs registered in the boot-time metatype check. Witnesses (25-test integration suite, run by me on PGlite — real embedded PostgreSQL): commit legs for every mutation class including rename (previousName) and direct revokeGrant; rollback legs for all eight mutation classes via idempotency-key conflict injection; idempotent replay; deny-by-default; §1.1 platform-admin-as-stranger; visibility owner-forbidden with admin accept control; directory closed fields; disclosure ≠ authority (§6.9); unauthorized-probe indistinguishability; down-chain conferral workspace-included; team suspension; transfer both-sides refusal matrix. Writer coverage: single-entry allowlist, suite green. Suites measured at this head: gateway 869 passed / 0 failed; db 63 passed (34 PG-gated legs skip locally — they run on the ci-postgres service in CI, which contract §6.8 requires); tsc --noEmit and eslint clean in both packages. Mutation controls, each restored and the tree verified byte-clean afterward: - Reverting the hasScope fix to `role === 'admin' → true` makes the §1.1 retirement spec fail. - Reintroducing a role-keyed short-circuit in filterProjectsForActor makes the §1.1 owned-content-only spec fail. - Making grant evaluation fail open (missing node → owner) makes the oracle-indistinguishability test fail. CI pipeline 2956: success, event pull_request, commit exactly f55d62cf, ci-postgres service ran, typecheck/lint/format/test/build all green. Non-blocking residuals (INFO, consistent with the detached round-2 dispositions, confirmed by my reading): create-command replay surfaces conflict rather than the original success; bare role strings on request DTOs (the §4.5 namespacing rule enumerates serialized artifacts — audit, API responses, logs — not request bodies); renameCompany and visibility change write updatedAt while the estate/platform-project renames do not; the contract 2 §7.8 mirror transfer case (owner on destination only) is unwitnessed though symmetric in code; a changeGrant unique-violation race would surface as a thrown error rather than a conflict result — the constraint still holds, only the error shape differs. Verdict: APPROVED at f55d62cf92b23beb0d5ea6f13596a8fce703d87d. An amend or rebase voids this approval; re-review at the new head if it moves before merge.
fred merged commit 215faeda0a into next 2026-08-29 16:54:40 +00:00
Sign in to join this conversation.