Cross-chat prompt injection: the privacy tier is the only boundary - #137
Merged
Conversation
… a child
The write rule for cross-session tools was `VIS ∧ lineage ∈ {self, child}`, so
an agent could steer a conversation it had spawned and only *read* one it had
not. The product owner's requirement is the opposite: an agent may inject into
ANY conversation, related or not, as long as it does not cross the
private/public boundary. The privacy tier is the only boundary.
- `privacy::visibility::may_write` is `VIS`. `Lineage` and `lineage_of` had one
production caller between them and are deleted rather than left as an unread
argument. The table-driven matrix test keeps its shape and collapses from
nine columns to four; a new property test asserts WRITE == READ over the whole
cross product, so a re-narrowing on any cell fails without anyone having to
remember to add a case.
- The test helper that FABRICATED lineage (rewriting the target's
`parent_session_id` to the caller's before every write-tool call, purely to
satisfy the old rule) is gone. Every tier assertion now drives an unrelated
conversation, which is the case that helper was quietly converting away.
`requires_first_crossing_approval` was documented, correct and WIRED TO NOTHING
— the guard census carried it as "OPERATOR DECISION OUTSTANDING… the state it
needs was never built". Widening the write rule is what settled that decision:
a private chat's reachable public targets are no longer only the ones it
spawned. So the disclosure is built:
- `privacy::crossing` is the (caller, target) ledger the predicate always
needed. Asking never records; only a landed write does, so a denied approval
cannot buy silence for the retry.
- `WorkspaceCrossingInspector` raises a `RequireApproval` carrying the exact
payload, in every permission mode, for `workspace_send_prompt` and
`workspace_set_tools`.
Real-time reflection, which was a genuine gap in all three modes:
- `note` publishes the durable row onto the session bus itself.
- `turn`'s injected prompt is published by `Agent::reply` where it becomes
durable. #66 deliberately never yields a `Message` for a user prompt, on the
premise that the client authored it and already holds it — false for an
injection, and nothing else carries the body.
- `steer` already reached the bus through the target's own drain loop; a second
publish would double-render a row that is not durable yet. Pinned by test
rather than duplicated.
- A publish is a no-op with no subscriber, and **a tab the user opened has no
subscriber**. Every accepted injection now sends an `observe` frame asking
that window to attach one. Deliberately unordered against the publish: an
observer's first frame is a full snapshot from the store, so the message
renders whichever arrives first.
`ensure_spawn_extension` grants `workspace_send_prompt` and `workspace_list`.
The latter needs justifying and gets it in the source: without it the tool's one
required argument is unobtainable, since an agent knows its children's ids and
no others. Still excluded: `workspace_set_tools` and `workspace_open`.
Docs updated rather than worked around: R6 and DR-5 retired, §7's matrix
collapsed, and the execution plan's Task 63 (which planned to REFUSE this write
and delete these predicates) marked superseded.
…ervices lock `FakeServices::install()` writes a process-global override, so two tests that install one cannot run concurrently. Both new tests passed alone and failed in the full parallel run — the failure mode this file's existing `#[serial_test::serial(workspace_services)]` attribute exists for. Also regenerates the OpenAPI schema and TS client for the `parent_session_id` doc comment, which is mirrored into `Session`'s description.
…and let toasts expire Two defects, both met on a FIRST run by a user who had set nothing up. **Six toasts for extensions nobody installed.** The daemon writes its bundled baseline into the catalogue the first time it starts, and every entry arrives as `added` + `enabled` — so `newlyInstalledExtensions` reported `developer`, `computercontroller`, `autovisualiser`, `memory`, `knowledge` and `agent_drafter` as fresh installs and each got "Extension installed. Turn it on for this chat…". The notification is worth having for what it was written for — an install made in another window or terminal that this chat can now opt into — and that case is always third-party. Bundled/builtin/platform entries are excluded; a MISSING config still notifies, because the safe direction is one extra notification rather than a silently swallowed real install. **And they never went away.** `react-toastify` defaults `pauseOnFocusLoss` to true, so every dismissal timer stops while the window is not frontmost — which in a desktop app is most of the time. The toasts were not "sticky" by configuration; their 5s timer simply never ran. `pauseOnHover` stays on, and the pair is the point: hovering pauses because the user is reading, which is a reason to wait; losing focus is a reason to go.
…running turn Found by the GUI drive-through, and invisible to every unit test that existed. `applyMessageEvent` derives `ChatState.Streaming` from any message, which is right on the driver path — there a message IS a turn producing output. On the observer feed with no turn in flight it is false, and it fails in the worst direction: nothing is running, so nothing will ever publish the terminal that would retire the state. `workspace_send_prompt mode:"note"` is exactly that case — it appends a row and starts nothing. Measured in the running app before the fix: the daemon's `/active_work` empty, the target tab showing "Thinking · 29s" with a live stop button, indefinitely. Narrow on purpose. A real observed turn announces itself first (`TurnStarted` sets `activeTurnId`, and the SSE handler sends `TurnState` right after its snapshot), so a running turn's messages still raise the running state — pinned by the second test. A confirmation card still parks the chat, because that one is genuinely waiting on the person.
`test (ubuntu-latest)` and `serve` both start with `apt-get update`, and the hosted runner image ships Microsoft's `azure-cli` and `prod` repository lists preinstalled. When `packages.microsoft.com` answers 403 — which it did on main's last run — `apt-get update` exits 100 with *"is no longer signed"* and the job fails, for an outage on a CDN neither job installs anything from. Both steps now drop those lists first, so the update depends only on the archives the packages actually come from. Defensive rather than urgent: the same job passed at 9m3s on the very next run.
…rvices fake
Two bugs, one shared global. `FakeServices` is installed process-wide via
`workspace_services::set_for_tests`, and `workspace_services::get()` is global —
so a test that never installs anything still USES whatever fake is currently
installed. Guarding installers is therefore not enough; the racer is a consumer.
**1. A fire-and-forget emit consumed a staged reply it could never return.**
`gui_command` popped `gui_answers` BEFORE branching on `wait_result`, so an
emit that answers `{"sent": true}` still took an answer off the queue — directly
contradicting the comment two lines below it. Any `notify_target` toast has been
silently stealing answers since it landed. It surfaces far from the theft:
`panel_secret_guard_rejects_top_level_and_wrapped_locators` stages two
secret-bearing panel replies and consumes them with two reads, so one stolen
answer turns its second read into the `{"ok": true}` default, the SecretGuard
refusal never fires, and it dies asserting "the wrapped locator bypassed
SecretGuard". Pre-existing: reproduced 1/10 on clean main with this branch
stashed.
**2. §3c's new emit made it frequent.** `reflect_in_target_tab` fires on every
accepted `workspace_send_prompt`, `mode:"note"` included, which previously
issued no GUI frame at all. Five tests perform a successful send without
`serial(workspace_services)`, so those frames landed in other tests' fakes.
Captured, pre-fix:
a_public_caller_cannot_close_a_private_conversation (:7297)
a frame about a private conversation reached the GUI:
[{"cmd":"observe","session_id":"20260827_2"}]
That is a SECURITY assertion — it checks no frame names a private conversation —
tripped by a frame about a public one from a concurrent test. The symmetry that
proves it is the shared fake rather than either test: this branch's own
`a_refused_injection_asks_for_no_live_feed` (:9749) was tripped by someone
else's frame.
Measured: 20x `cargo test -p biorouter --lib` after the fix — zero occurrences
of any of the three discriminating failures (:7297, :9749, panel_secret_guard).
One unrelated failure remains in that batch,
`claude_code::dropping_an_unread_stream_reaps_the_child`, a separate child-wait
timeout being fixed elsewhere.
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.
What this does
An agent may now inject a prompt into any conversation it can see — a subagent it spawned, a sibling, or a chat the user opened and it has never touched. The privacy tier is the only boundary; lineage is not.
The write rule in
privacy/visibility.rswasmay_write(c, t, lineage) = visible_to(c, t) && lineage != Other. It is nowmay_write(c, t) = visible_to(c, t), andLineage/lineage_ofare deleted rather than left as an unread argument (they had one production caller between them).The disclosure this made load-bearing
requires_first_crossing_approvalshipped wired to nothing.tests/privacy_guard_wiring.rscarried it asStatus::Unwired("OPERATOR DECISION OUTSTANDING … the state it needs was never built"). Widening the write rule is what settled that decision: a private chat's reachable public targets are no longer only the ones it spawned — it can now write into any public conversation on the machine, one the user opened and is reading.So the disclosure is built:
privacy/crossing.rs— the (caller, target) ledger the predicate always needed. Asking never records; only a landed write does, so a denied approval cannot buy silence for the retry.WorkspaceCrossingInspector— raises aRequireApprovalshowing the exact payload, in every permission mode including Fully Automatic, forworkspace_send_promptandworkspace_set_tools.Real-time reflection (§3c) — a genuine gap in all three modes
notepublishes the durable row onto the session bus itself.turn's injected prompt is published byAgent::replywhere it becomes durable. The MessagesPersisted ordering invariant is a convention and a test net, not a mechanism #66 deliberately never yields aMessagefor a user prompt, on the premise that the client authored it and already holds it — false for an injection, and nothing else carried the body.steeralready reached the bus through the target's own drain loop. A second publish would double-render a row that is not durable yet, so it is pinned by test rather than duplicated.observeframe asking that window to attach one, routed withgui_command_nearso it reaches the window actually holding the conversation. Deliberately unordered against the publish: an observer's first frame is a full snapshot, so the message renders whichever arrives first.ensure_spawn_extensionNow grants
workspace_send_promptandworkspace_list.workspace_listis the entry that needs justifying. For: without it the injection tool's one required argument is unobtainable — an agent knows its children's ids from spawn results and no others — so a grant that advertisessend_promptwithout it produces a model that tries and fails rather than one that does not try.Against, and this is the real cost:
workspace_listis a targeting oracle. A row carries the conversation's LLM-generated name, its working directory, its enabled extensions and its GUI placement, andscope:"all"returns every same-tier conversation on the machine. A user who turned on "delegate to subagents" did not knowingly turn on "enumerate my chats and their tool grants" — and the enumeration is what makes an injection aimable: a chat can find one whose extensions includedeveloperand inject text that runs there, in that conversation's permission context.Three things bound it, and none of them is the lineage rule that was removed:
appears_in_liststill omits private rows rather than redacting them, so the discovery surface is exactly what this capability may already read in full; the delegation tier exists only inBioRouterMode::Auto; and every injection is provenance-stamped and toasted on the target's tab. The trade is recorded at the decision site inagent.rsas well as here.Still excluded:
workspace_set_tools,workspace_open.Defects found by the adversarial review and the drive-through
Four, all fixed with a regression that fails without the fix:
workspace_set_tools { set_knowledge_bases: [] }is an accepted, payload-free change that raised no card — and recorded the crossing, silently consuming the pair's one disclosure. The record half now asks the samecrossing_payloadthe inspector does.execute_codebypassed the disclosure entirely. A script's inner tool calls go straight todispatch_tool_call, which no inspector sees. Worse than one skipped card: the handler then records the pair, so every later inspected write was silent too. Third boundary refusal added, beside the existing global-memory and session-store ones.observeframe went to the wrong window (gui_commandresolves to the focused one), so a conversation open in a background window never got its live feed — exactly the case §3c exists for./active_workempty, target tab at "Thinking · 29s" with a stop button, indefinitely, because no turn existed to publish a terminal.Two UX fixes requested during review
developer,computercontroller,autovisualiser,memory,knowledge,agent_drafter— things the user had never heard of on a screen where they had set nothing up.react-toastifydefaultspauseOnFocusLossto true, so every dismissal timer stopped while the window was not frontmost. They were not sticky by configuration; their 5s timer simply never ran.Verification
cargo test -p biorouter/-p biorouter-server/-p biorouter-mcp/-p biorouter-cli(isolatedHOME),npm run test:run(3737 tests, 366 files),./scripts/clippy-lint.sh,just check-everything— all green. The twotoo_many_linesbaseline hits were fixed by extracting, not by widening the baseline.Every new regression was verified to fail without its fix.
GUI drive-through (isolated root, driven over CDP): two chats side by side, note/turn/steer each rendering live with
navigation.length === 1; the public→private refusal; the private→public approval showing the payload, denied then approved;.br-tab__doton adata-active="true"tab together withdata-working="true"on the composer; and the model's own listing ofworkspace__workspace_send_prompt.CLI drive-through:
session list, a CLI-driven agent injecting into a chat whose GUI tab reflected it live, and the tier refusal reading correctly in a terminal.Docs
R6 and DR-5 are retired, §7's nine-column matrix collapses to four, and the execution plan's Task 63 — which planned to refuse this write and delete these predicates — is marked superseded with the original struck through rather than deleted.
Known limits, stated rather than left to be found
docs/extensions/built-in/workspace.mdnow says so plainly.workspace_open { new: { prompt } }mints a public session and seeds it with caller text, with no crossing disclosure — a session that does not exist yet has no pair to key on. Pre-existing, out of this PR's scope, and now named explicitly inprivacy-tiers.md's open items.workflow_inspectorsregisters neither workspace inspector. Not reachable today; commented so the next person to route the workspace surface through it adds them in the same change.