BF-09 — propose → approve → commit governance (agent cannot approve; denial audited) - #35
Merged
Conversation
…it-insert sgrule + contract reconcile Operator prep for the propose->approve->commit governance slice, all OFF the maker floor by design: - drizzle/0012_governance.sql: event-sourced, append-only governance substrate (governance_proposal / governance_event / governance_signed_note). Verbatim BF-01 RLS template + FORCE on all three; BP-019 tenant-scoped keys (every key leads with practice_id; row ids server-generated); BP-018 append-only posture (S/I grant + belt REVOKE U/D) — committed-note immutability is a privilege-layer property, not app logic. UNIQUE (practice, proposal, action) pins at most one approve + one commit event per proposal; signed-note UNIQUE pins one note per proposal; 'reject' deliberately absent from the action CHECK (rejected exists only in the pure state machine in v0). All probed live on :55432: app S/I ok, UPDATE/DELETE 42501, cross-tenant + no-GUC reads 0 rows, double-approve/double-note 23505, reject CHECK violation, FK holds, cross-practice insert blocked by WITH CHECK. - packages/core/src/db/fhir-rls.test.ts: BP-018 posture matrix += the three governance tables (append-only in both directions). - sgrules/no-direct-audit-insert.yml (+ sgrule-tests): acceptance-#5 structural guard — `insert into audit_log` outside packages/core/src/audit fails the gate (product surface only; the loop bf05 chain oracle stays independent by design). Inversion-proven: injected violation exits 1, clean tree exits 0. Ratchet candidate. - loop/src/contracts/tasks.ts: dead-path reconcile (BF-08 precedent) — the eval corpus loader reads loop/evals/*.jsonl flat, so the contracted loop/evals/governance/** subdir could never load; bf09 eval rows will live at loop/evals/bf09.jsonl like every other slice. - loop/memory/state.jsonl: BF-09 active with the full de-risk record (2 Opus adversarial design reviews found 2 blocking seam flaws pre-code: B1 phantom-allow audit rows under withTenant's err-value-commits semantics; B2 practice-mismatch denials tripping the mis-attribution throw instead of auditing — both designed out before the maker runs). Local verification on the PG17 fallback stack (:55432): turbo gate + core 250 tests green EXCEPT the single repo-own "Postgres major version is 18+" catalog guard (documented PG17-host artifact; the strict PG18 proof is CI's); loop tests green; ast-grep 14/14 rule tests; eval corpus 29/0 on main-parity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…57 tests bonfire-maker output, operator-committed. packages/core/src/governance/**: - types.ts: GOVERNANCE_ROLES = [...abac ROLES, "agent"], state/action unions derived from exported const arrays, GovernanceError codes, signedNoteSchema (64-hex commit audit hash), ProposalRecord. - policy.ts: transition() — default-free exhaustive switch, exactly three legal edges (proposed+approve, approved+commit, proposed+reject); decideGovernance() — ONE total function, whole body incl. safeParse inside try/catch (a throwing-getter actor yields a deny receipt, never a throw), single exhaustive switch(action), receipt.resourceType = "Governance."+action, bound practice is a parameter fed from the GUC (never caller input). - store.ts: proposeRecord/approveProposal/commitProposal on one advance() seam. The audit contract (designed under review, spot-checked post-build): exactly ZERO or ONE appendAuditRowTx call per attempt — deny appends then returns an err VALUE (commits, zero mutations); allow appends immediately before its mutation inserts (same tx, rollback removes both); ZERO rows on invalid input, NOT_FOUND, illegal transitions by authorized actors, and write-layer err values. Per-practice governance advisory lock BEFORE the proposal load; lock ordering pinned (governance -> audit, never reverse). Signed note copies the approver from the approve EVENT row (server truth); commit_audit_hash = the commit allow row's chain hash. - policy.test.ts (45) + store.test.ts (12, live DB, fresh randomUUID practices): 12-combo transition grid; deny grid for every non-clinician role x approve/commit/reject; hostile throwing-getter actor; honest-staging oracle (fhir_resources UNCHANGED at propose); denial audited + chain-linked (verifyAuditChainTx + row-hash JOIN); deny-commits vs DB-fault-rolls-back double-Result split; immutability (chain row_hashes literally unchanged, commit-event count pinned at 1). Operator finalization: dropped the one dead export the post-build spot-check flagged (governanceActorSchema from index.ts — the store boundary takes actor: unknown and parses internally, so no external consumer can need it). Verified in-worktree: governance 57/57; core 307/308 (the single red is the documented "Postgres major version is 18+" catalog guard on this PG17 host); mcp 15, api 18, sdk 18, sql-on-fhir 29, loop 199 all green; loop gate stages 0+1 green; bunx eslint . 0 errors; biome/ast-grep/jscpd/knip/depcruise clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ce now stages, never writes packages/sdk/** is off the BF-09 maker floor; this is the disclosed operator edit the slice depends on (BF-04/05/13 precedent for off-floor waves). - ops.ts opProposeResource: writeScribeResource (live write) -> core proposeRecord with actor = the session subject verbatim (id = audited actor, role/practiceId = membership row) — the same U2 session-last control as every other op, so a caller can never name its own role to the governance authority check. Result type: ProposalRecord with GovernanceError | WriteError | SdkErrorCode. - ir.ts doc + regenerated client.gen.ts: the generated TSDoc no longer claims the write is live (it stages a proposal requiring clinician approve+commit). - etc/sdk.api.md regenerated (api-extractor local) for the changed surface. LOCKSTEP NOTE (disclosed): this commit intentionally leaves packages/mcp red at src/tools.ts:204 (the propose handler still destructures the old live-write result). The immediately following maker phase-B commit adapts the MCP surface (honest staging render + description + inverted tests); the pair lands adjacently by design. sdk tests: 18/18 green at this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lockstep pair with the preceding SDK re-point commit (tree green again here).
packages/mcp (extend-never-widen; ALLOWLIST untouched at exactly 3 tools):
- propose_resource handler adapted to the staged ProposalRecord result;
renderProposeText JSON-encodes every derived field and says what is now
true: proposal id, state "proposed", clinician approval + commit required
before anything reaches the canonical FHIR store. Tool description
rewritten off the old "write is live" claim. ERROR_TEXT gains static
entries for GOVERNANCE_FORBIDDEN / GOVERNANCE_INVALID_TRANSITION /
GOVERNANCE_NOT_FOUND (no error.message ever forwarded).
- server.test.ts: the old "propose writes NOW" oracle INVERTED to honest
staging (fhir_resources unchanged + target id absent; governance_proposal
+1; audit +1; structuredContent = {proposalId, state} with NO
versionId/record fields); pin test name-regex widened to
/approve|commit|reject|sign|sql|shell|fhirpath|exec|file/i on tool NAMES
only (the honest description legitimately contains "approval").
- render.test.ts: direct-drive injection canary on renderProposeText
(hostile newline / forged hit line / forged receipt line / forged second
confirmation line) — e2e cannot carry a hostile char (uuid + literal), the
BF-08 decorative-guard lesson. Exported from tools.ts only; public api.md
surface unchanged.
- support.test.ts tenantFootprint += governance_proposal, so every existing
zero-side-effect oracle now also pins governance rows.
apps/api/src/routes/governance (new surface):
- governanceRoutes(deps) plugin: exactly three POST routes (proposals,
:id/approve, :id/commit), ONE handler factory; the actor is built ONLY
from ctx.identity (iss#sub) + ctx.membership (role, practiceId) — request
body/params are data, never authority; replies carry the typed governance
Result as the body (runAuthenticated owns HTTP codes).
- routes.test.ts: local JWKS + owner-seeded memberships; clinician full flow
(body parses signedNoteSchema); biller approve denied + exactly one
hash-chained deny row (Governance.approve) + zero events; commit-before-
approve typed error; smuggled role/practiceId body inert; no-Bearer 401
with zero governance rows.
Verified: typecheck green; mcp 15/15; api 24/24; turbo gate 32/33 (sole red
= the documented PG18 catalog guard on this PG17 host); loop gate stages
individually green (biome/eslint 0 errors/ast-grep/depcruise/semgrep/
gitleaks/knip/jscpd); read-only surface spot-check PASS on all 8 invariants.
Disclosed: (1) INVALID_SCRIBE_INPUT is unreachable through the live MCP
transport (the SDK validates args pre-handler) — its static-text oracle
drives the allowlisted tool's run() directly with a typed stub client.
(2) Pre-existing BF-08 behavior, ratchet candidate: the MCP SDK's own
pre-handler validation error reflects caller-argument zod detail (reflected
input, not stored data). (3) apps/api imports jose via hoisting without
declaring it (pre-existing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… === state)
Adversarial ownership pass over the four security units (default-deny
authority, illegal-transition enforcement, committed-note immutability,
denial-audit wiring). Three units audited CLEAN against unicode/case role
tricks, throwing-getter parse, actor.id receipt-forgery, TOCTOU under the
per-practice advisory lock, cross-referenced audit hashes, and clock
injection. One defense-in-depth gap found and closed:
The store took `actor: unknown` and parsed it TWICE — once inside
decideGovernance (whose receipt.actorId lands in the AUDIT chain) and once in
a re-parse (whose GovernanceActor lands in the governance_event / proposal /
signed-note STATE). A value-shifting `get id()` returning a fresh value per
read could attribute the allowed decision to one identity in the audit row
and record a different proposer in state — a forged split across the exact
`governance_event.audit_row_hash = audit_log.row_hash` JOIN the whole
tamper-evidence story rests on. The old "fail loud" re-parse only caught a
parse FAILURE, not a value DIVERGENCE.
Fix (store.ts): parse the actor ONCE into a frozen snapshot; feed that
snapshot to decideGovernance AND record the same snapshot in state. Invariant:
audit-actor === state-actor, read once. Byte-identical behavior for every
plain-object input and for malformed/throwing-getter inputs (still a deny row
via decideGovernance's own catch). Only the allow-path value-divergence class
is closed, by construction.
Two pinning tests (store.test.ts): (1) a get-id() actor returning
`agent-${reads}` asserts audit_log.actor_id === governance_proposal
.proposer_actor_id — proven load-bearing (RED when the double-parse is
reintroduced, GREEN on the single-read); (2) a duplicate-resource-id second
commit rolls back wholesale (23505) leaving the first signed note + the audit
chain intact.
No shipped caller (SDK/API/MCP) can deliver a getter-bearing actor — the SDK
derives it from the session subject, the API from ctx.identity/membership,
MCP handlers hold only the session-bound client — so this is defense-in-depth
at the core boundary, not a reachable production hole.
Verified: governance 59/59 (was 57); core 309/310 (sole red = PG18 catalog
guard on the PG17 host); mcp+api 39/39; ast-grep 0 findings; jscpd 0 clones;
biome clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rator wave) Off the maker floor (loop/** + scripts/** are the harness side of the firewall). The Stage-2 governance evals compose the product across the loop<->product boundary by SPAWNING these demos, never importing product code. - scripts/governance-demo/govern.ts: argv-driven propose/approve/commit driver — one withTenant transaction per step (mirroring the real request lifecycle), prints the flattened per-step Result as one JSON line. The governance actor is passed in and parsed by the store, so the propose->approve->commit boundary is exercised without a token/JWKS (governance authority is decideGovernance, not a membership lookup). - scripts/governance-demo/mcp-allowlist.ts: prints the frozen MCP tool ALLOWLIST names across the firewall. Imports ALLOWLIST from the tool module directly (core + zod only — the MCP SDK lives only under packages/mcp and is unresolvable from a script); the complementary in-package pin test proves the live tools/list a connected client sees equals these names, so this eval guards the Stage-2 firewall angle (an ALLOWLIST widened with an approve tool fails here even before a transport exists). - scripts/governance-demo/tsconfig.json: references core + mcp, noEmit. - loop/src/evals/bf09-governance-util.ts: govern()/mcpToolNames() spawners + owner/app postgres clients + expectOk/expectErr + a valid scribe Patient factory, shared by the six bf09-* evals. - knip.json: scripts/governance-demo/** added to the "." entry + project globs. Both demos smoke-tested live on :55432 (agent propose ok, biller approve FORBIDDEN, clinician approve->commit->signed note; ALLOWLIST = the 3 frozen tools). knip exit 0, jscpd 0 clones, depcruise clean, loop typecheck green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… inversion-designed Off the maker floor (loop/**). Six bf09-* execution evals + the flat loop/evals/bf09.jsonl corpus rows (the contract's loop/evals/governance/** path was reconciled to flat in prep — the corpus loader is non-recursive). Each drives a real product build across the harness<->product firewall via the governance-demo scripts and asserts with its own owner/app postgres reads; each is written so a plausible product regression reddens it (inversion stated in each docstring). Fable authored the 3 behavioral format evals; Opus authored the 3 security evals. - bf09-no-approve-commit-mcp-tool (accept #6, propose-only-broken): the MCP ALLOWLIST across the firewall is exactly the 3 frozen tools and no name matches /approve|commit|reject|sign/i. Reddens if a 4th tool is added. - bf09-propose-allowed-approve-denied (accept #4/#2, propose-only-broken; the BF-08 carry-over): biller AND agent each PROPOSE ok for their own practice and are each FORBIDDEN approving it (audited deny, zero events); a clinician approves both — only approve/commit are gated. Also proves governance RLS scopes proposals to the bound practice. - bf09-clinician-only-approval (accept #3, fail-open-authz): approve denied for biller/operations/researcher/agent AND a malformed actor; clinician approve allowed from proposed with exactly one allow row whose row_hash the approve event carries. - bf09-agent-cannot-approve (accept #2, propose-only-broken): agent approve AND commit both FORBIDDEN, state unchanged (zero events, no fhir_resources row for the proposed id), a later clinician approve still succeeds. - bf09-governance-denial-audited (accept #5, audit-bypass): a blocked approval writes exactly one deny row (agent id, Governance.approve, nonempty reason); the whole practice chain re-verifies from the runtime-recomputed genesis; an allow event's audit_row_hash JOINs an allow audit row (the hash column is load-bearing). - bf09-committed-note-immutable (accept #7/#8, immutability): after commit, re-approve/re-commit both GOVERNANCE_INVALID_TRANSITION and UNAUDITED (chain length + signed note unchanged, exactly one commit event); the app RLS role cannot UPDATE/DELETE governance_signed_note (42501). - bf09-stage.ts: a non-eval shared stage-a-proposal helper (jscpd-motivated, no jsonl row). bf09-governance-util MRN slice length named (eslint). Verified: loop eval --slice BF-09 6/6; full corpus 35/35 (was 29); eslint 0 errors on the evals; jscpd 0 clones; loop typecheck green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st-grep gate) The adversarial inversion pass caught that bf09-committed-note-immutable drove its owner/app UPDATE/DELETE probe through `tx.unsafe(statement)` with a loop-variable statement — which trips the repo-wide no-raw-sql-unsafe (BP-020) ast-grep rule (it has no path scope, so it lints loop/** too), making `bunx ast-grep scan` exit 1 (a blocking Stage-1 gate CI would fail on). Rewrite the forbidden-write probe to run bare tagged-template mutations (update/delete governance_signed_note) via a closure, so no statement text is ever passed to sql.unsafe. Same assertion — the app role's missing UPDATE/DELETE grant still raises 42501 — with the query shape now inside the raw-SQL ratchet. ast-grep scan exit 0; eval still PASS; loop typecheck + eslint + jscpd clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…se gate) The CI-trap sweep caught that mcp-allowlist.ts imported the deep internal path packages/mcp/src/tools.ts, tripping depcruise's consumers-use-package-entry- points rule. The deep import was a workaround for the MCP SDK being unresolvable from a script — but bun resolves a bare specifier relative to the importing SOURCE file (server.ts), not the script, so importing the mcp INDEX entry point resolves the SDK against packages/mcp's own node_modules and runs standalone. Switch to the entry point; depcruise clean, eval still PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… format check) The strict slice gate's biome ci flagged govern.ts + the eval util + the immutability eval (long Result/type signatures wrapped differently than biome's formatter). Applied biome check --write; no logic change. Slice eval 6/6, loop typecheck green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… clone) Biome's line-wrapping of the .trim().split().filter().pop() chain turned the govern()/mcpToolNames() output-parsing blocks into a jscpd clone under the strict gate. eval-util already exports lastJsonLine (leading-notice tolerant, fails loud on a non-JSON tail) — reuse it in both, dropping the duplicated block and the redundant try/catch. jscpd 0 clones; slice eval 6/6; biome ci clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fense-in-depth)
The 5-agent review panel (verifier + Opus security-auditor + 3 danger-class
refuters) converged on ZERO blocking/HIGH/MED defects — all three danger
classes independently refuted, two by live adversarial drives on :55432
(agent/biller approve+commit all FORBIDDEN with fhir_resources unchanged; 16
malformed/proto/getter actors all fail closed; concurrent double-commit
serialized to exactly one commit with the loser leaving zero audit rows).
Acting on the one LOW finding worth a code change: loadProposal scoped its
proposal + event reads to the tenant via FORCE RLS alone. Added the explicit
`practice_id = (select safe_uuid(current_setting('app.current_practice_id',
true)))` predicate to both reads — redundant with RLS today, but belt over the
suspenders so a future RLS misconfiguration cannot widen the read to another
tenant. Behavior unchanged (RLS already filtered): governance 59/59, slice
eval 6/6, ast-grep + biome clean.
The remaining panel findings are disclosed residuals (an authorized commit
that fails at the write layer, and a denied approve/commit on a missing/foreign
proposal id, are both unaudited but fail-closed with no state change or leak)
and future-proofing ratchet candidates (an sgrule banning writeScribeResource
imports outside the write path) — carried to the close-out, not this PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BF-09 — propose → approve → commit governance (agent cannot approve; denial audited)
Closes the core AI-safety boundary: an agent actor can only propose; any approve/commit by a non-clinician is default-deny, the denial is written to the BF-05 tamper-evident audit chain, a clinician approves, and an approved proposal commits to an immutable signed-note record. This makes BF-08's
propose_resourcehonest — that MCP write tool was an ungoverned live commit; it now stages a proposal that reaches the canonical FHIR store only through clinician approve + commit.What landed
drizzle/0012_governance.sql):governance_proposal/governance_event/governance_signed_note, all FORCE-RLS + BP-019 tenant-scoped keys + BP-018 append-only (S/I grant, belt REVOKE U/D). Immutability is a privilege-layer property — there is no UPDATE path at all — not app logic.UNIQUE(practice, proposal, action)pins at most one approve + one commit event; the signed-noteUNIQUE(practice, proposal)pins one note.packages/core/src/governance/): a default-free exhaustivetransition()state machine (proposed→approved→committed + rejected; illegal edges are typed errors); a total, fail-closeddecideGovernance()(whole body incl.safeParseintry/catch; a throwing-getter actor yields a deny receipt, never a throw); astorelayer whose audit contract is exactly zero-or-one chain row per attempt, appended only once the outcome is known and immediately before its mutation (same tx), with the bound practice sourced from the GUC (never caller input).proposeResourcere-pointed to stage a proposal (never a live write); the MCPpropose_resourcetool re-skinned to honest staging with the frozen 3-tool ALLOWLIST unchanged (no approve/commit/reject tool); three API routes (POST /governance/proposals,/:id/approve,/:id/commit) whose actor is derived only from the authenticated identity + membership.loop/evals/bf09.jsonl) driving real product builds across the harness↔product firewall viascripts/governance-demo/.How the three named danger classes are structurally closed
decideGovernancedefault-denies approve/commit for every non-clinician. Honest staging is pinned (a propose leaves zerofhir_resourcesrows).errvalue (it commits its audit row), never a swallowed throw.no-direct-audit-insertast-grep guard, inversion-proven); a denied approval writes a hash-chain-linked deny row (actor +Governance.approve+ reason); an allow event'saudit_row_hashJOINs its allow row.Adversarial verification
allowrows underwithTenant's err-value-commit semantics; practice-mismatch denials tripping the audit mis-attribution throw) + one fail-open (throwing-getter parse) — all designed out pre-implementation.audit_row_hashJOIN; fixed by a single-read frozen snapshot (audit-actor === state-actor), with a load-bearing pinning test.insert into audit_log, and a liveGRANT UPDATE/DELETEon the signed note each went RED and GREEN on restore. The pass also surfaced a real CI-blocking gate gap (an eval'ssql.unsafe(variable)tripped the repo-wide raw-SQL ratchet) — fixed.GOVERNANCE_FORBIDDENwithfhir_resourcesunchanged; 16 malformed/__proto__/value-shifting-getter actors all fail closed; a concurrent double-commit serialized to exactly one commit with the loser leaving zero audit rows). One LOW acted on (the explicit tenant predicate below); the rest are the disclosed residuals + close-out ratchet candidates listed here.Verification
loop eval --slice BF-096/6; full eval corpus 35/35.Disclosed off-floor operator waves (BF-04/05/13 precedent)
The maker floor is
packages/core/src/governance/**+packages/mcp/src/**+apps/api/src/routes/governance/**+ tests +drizzle/**+ evals. These edits are outside it, by design, and disclosed:packages/sdk/**(ops/ir/generated/api.md) — theproposeResourcere-point (one identity boundary, not two).packages/core/src/db/fhir-rls.test.ts— posture matrix += the 3 governance tables.sgrules/no-direct-audit-insert.yml(+ test) — acceptance-H2 — single-source agent generators (.claude + .codex) with drift test #5 structural guard (ratchet candidate).loop/**+scripts/governance-demo/**+knip.json— the eval corpus + demo substrate (harness side of the firewall).loop/src/contracts/tasks.ts— dead-path reconcile (loop/evals/governance/**→ flatloop/evals/bf09.jsonl, the corpus loader is non-recursive).Disclosed residuals (v0)
errvalue, or an audit-append fault) rolls back its own audit trace — fail-closed both ways (nofhir_resourcesrow, no state advance); the asymmetry vs "every decision audited" is benign because state-without-audit is what a bypass needs and this leaves neither.TENANT_TX_FAILED(23505 rollback), not a typed governance code.membershiphas noagentrole yet, so an agent actor is constructible only at the core boundary; the MCP + API surfaces structurally cannot approve/commit regardless.Close-out ratchet candidates (follow-up PR, not this one)
writeScribeResourceimports outside the write path + governance store (nothing reaches it from an agent surface today; the guard stops a future SDK op/route from re-opening the live-write path).🤖 Generated with Claude Code