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).
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
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 next2026-08-29 16:54:40 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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
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 throughappendHierarchyEvent(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).companies.visibilitycolumn (migration 0020, private default, CHECK), admin-only auditedvisibility_changewith old+new values, closed-field directory listing, §6.7 no-existence-oracle refusals (unauthorized probe ≡ missing node).Evidence
tsc --noEmitclean on both packagesIndependent 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 heada3446a13— 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:
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:
role === 'admin' → truemakes the §1.1 retirement spec 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.