Repository navigation
fix(agents): Agent-Teams-3 closure — DEK wrap on approve, trusted-author card gate - #409
Merged
Merged
Conversation
…hor card gate
Slice 2's security review passed with two items documented rather than fixed.
Both are closed here, plus a third gap verified.
(a) The approve path wrote `dekId: null`, so a channel-approved object had no
per-entity DEK and no member wrap — it could not be wrapped for sharing
until the next vault open's retro-wrap pass. Fail-closed, but second-class.
The DEK mint + wrap + rollback + zeroing sequence is now ONE shared seam
(`entities/create-with-dek.ts`) used by both `entities.create` and the
approve path, rather than a second implementation of a security-critical
ordering. The approve path's Pending re-check rides the seam's `guard` hook
so it stays atomic with the create. An explicit `DekWrapFailurePolicy`
picks the honest answer per caller: RollbackRow for `entities.create`
(nobody has seen the row), KeepRow for approve (the user has been told the
object exists; the row still carries a real DEK, so the wire path never
sees an ambiguous null-DEK row — the posture retro-wrap already takes).
No DEK store ⇒ Unavailable, never a second-class row.
(b) Chat rendered an approve/discard card from ANY message carrying the
`agentProposal` property, so a card synced in from another vault showed an
Approve button that main could only ever refuse — a UX lie and a phishing
surface. Card rendering is now gated on the message's HOST-written author
(which no app can set) resolving to a SHELL-authored `Agent/v1` record —
the app-side mirror of main's `readTrustedAgentDef` notion. The parser
stays fail-closed, and so does the default: omit the trust context and no
card is ever produced.
(c) Verified, not fixed: row / database / code-file proposals are structurally
unreachable from a channel (those verbs are Agent-app-local and are not in
`PROPOSE_DESCRIPTORS` at all, and the runner filters on
`isChannelProposeVerb` besides). Pinned by a retained probe that grants the
agent every propose dispatch and scripts the model to call each scoped-out
verb by name.
Failing-test-first for (a) and (b); (c) passed on the first run and is recorded
as confirmed-structural. 14 new adversarial probes land as pentest fence 5 —
card forgery/settling through the real entities service, type substitution
including non-canonical spellings, provenance forgery, every foreign-author
stamp, key reuse + AAD binding + zeroing of the live buffer, a 5-way approve
race, capability scarcity, and non-canonical fingerprint spellings against both
halves of the new Chat gate. All refused; no new blocking finding. One hazard
recorded as a NOTE probe, not fixed (outside this diff, no reachable exploit):
live-sync's installWrap stamps `created_by` from the entity TYPE PREFIX, so a
member can cause a row attributed to the reserved `shell` principal — harmless
only while every consumer conjoins the type, which both agent gates do.
`<gates: pentest, security-review>`
Full suite 17,005 passed / 0 failed; typecheck (packages + 20 apps) and lint
(biome + all 10 ratchets, no baseline moved) clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
th3-br41n
added a commit
that referenced
this pull request
Aug 1, 2026
#409 landed while this branch was in flight. The only conflict was in `mention-runner.test.ts`, where both sides appended a test at the same point: #409's CLOSURE (c) probe (the scoped-out propose verbs are structurally unreachable) and this branch's two delegation probes. Both are kept — they assert disjoint properties of the same runner. `mention-runner.ts` and `index.ts` auto-merged: #409 touched the reply-writing tail and the proposal-service wiring; this branch touched the tool set, the dispatch, and the assignment subscriber. Verified after the merge: agents + chat suites 277 passed, typecheck clean, lint clean (biome + all 10 ratchets). Co-Authored-By: Claude Fable 5 <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
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.
Closes the two items slice 2's security review left documented rather than fixed, plus a third gap verified.
(a) The DEK gap — closed
The approve path wrote
dekId: null, so a channel-approved object had no per-entity DEK and no member wrap. It could not be wrapped for sharing until the next vault open's retro-wrap pass — fail-closed (no plaintext leak), but second-class.The mint + wrap + rollback + zeroing sequence is now one shared seam,
packages/shell/src/main/entities/create-with-dek.ts, used by bothentities.createand the approve path — rather than a second implementation of a security-critical ordering. The approve path's Pending re-check rides the seam'sguardhook so it stays atomic with the create (the terminal-decision property is unchanged).An explicit
DekWrapFailurePolicyenum picks the honest answer per caller:RollbackRowforentities.create— nobody has seen the row, so un-create it and let the throw reach the caller (byte-identical to today).KeepRowfor approve — the user has already been told the object exists; deleting it because a Y.Doc round-trip failed would be a worse lie than a missing wrap, and the row still carries a real DEK so the sync wire path never sees an ambiguous null-DEK row. This is the postureretro-wrap-deks.tsalready takes for the identical case.No DEK store ⇒
Unavailable. There is no path to an approval without a key.(b) The card phishing surface — closed
Chat rendered an approve/discard card from any message carrying the
agentProposalproperty, so a card synced in from another vault showed an Approve button that main could only ever refuse.Card rendering is now gated on the message's host-written author (
ownerAppId= the row'screated_by, which no app can set) resolving to a shell-authoredAgent/v1record — the app-side mirror of main'sreadTrustedAgentDefnotion, minus the crypto half main re-checks anyway. A foreign or non-agent-authored card renders as an ordinary message, whose plain body already reads honestly on its own. The parser stays fail-closed, and so does the default: omit the trust context and no card is ever produced.(c) The third gap — verified structural, not a bug
Row / database / code-file proposals are genuinely unreachable from a channel, for two independent reasons: those verbs are Agent-app-local and are not in
PROPOSE_DESCRIPTORSat all, and the runner filters its offered set onisChannelProposeVerbbesides. Pinned by a retained probe that grants the agent every grantable propose dispatch (includingintents.dispatch:propose-row, which the starter agents really hold) and scripts the model to call each scoped-out verb by name before settling on a task: the system prompt never names them, and the run's only card is the task.Method
Failing test first for (a) and (b) — both watched red, then fixed. (c) passed on the first run and is recorded as confirmed-structural rather than claimed as a fix. Every probe is retained.
Gates
Both run by me on this diff; the run records go in the harness ledger.
pentest — pass (first run of the closure probes failed as expected on (a) and (b); that is the recorded shape). 14 new adversarial probes land as fence 5 in
agent-teams-pentest.test.ts, all against real tmpdir vaults with the real repo / entities service / ledger / DEK store / crypto /createAgent:agentProposalis stripped from create and update, so an app can neither plant, settle, nor re-open a card)Agent/v1, an unhonourable kind, and non-canonical"Task"/"task "— only the descriptor's type is ever writtenentity_deksrow:security-review — pass, two nits found and fixed in the pass:
toMessage's new parameter was positional, so.map(toMessage)passed the array index as the trust context (a live crash — it fired inwidget-data.ts); it is now an options bag so the same mistake degrades to the fail-closed default. And the Chat module re-declaredAGENT_TYPElocally; it now imports the shared constant from@brainstorm-os/sdk-types.Still open (recorded, not fixed)
created_byfrom the type prefix. Live-sync'sinstallWrapmaterialises an inbound row withcreatedBy = type.slice(0, type.indexOf("/")), and the type rides a member-sealed wrap — so a remote member can cause a local row stamped with the reservedshellprincipal by sealing a type likeshell/Anything/v1. Not an agent forgery: both trusted-agent gates conjoincreated_by === "shell"with the exactbrainstorm/Agent/v1type, and those are mutually exclusive on that path. Outside this diff; pinned by a NOTE probe so a change to either half turns red.installEntityWrapseam, not through the real ydoc worker.Verification
bun run typecheck(packages + all 20 apps) clean ·bun run lintclean (biome + all 10 ratchets, no baseline moved) · fullbun --bun vitest run: 17,005 passed, 3 skipped, 1 todo, 0 failed.🤖 Generated with Claude Code