fix(security/channels): enforce cross-chat dispatch guard through the /mcp bridge (#6117)#6125
Merged
Merged
Conversation
… /mcp bridge (#6117) An agent's `channel_send` could dispatch a message to a different recipient than the current turn's inbound peer on the same channel, with no validation — a cross-chat confidentiality leak observed live (audio voice notes delivered to a stranger's WhatsApp chat, 2026-05-19). The leak vector is the subprocess-driver `/mcp` bridge: `network.rs::mcp_http` hardcoded sender_id / channel / chat_id to `None`, so `tool_channel_send` had no peer scope to validate against. A guard alone could not fire — the prerequisite peer-scope plumbing was absent on `main`. This lands the full peer-scope propagation chain and the guard: - `CompletionRequest` gains `sender_user_id` / `sender_channel` / `sender_chat_id`, populated from the turn's existing manifest-metadata bindings in the agent loop (streaming + non-streaming). Out-of-band callers (cron, triggers, compaction, routing probes) leave them `None`. - The `claude-code` driver forwards them on the bridge connection as `X-LibreFang-Current-Peer-Jid` / `-Channel` / `-Chat-Id`, mirroring the existing `X-LibreFang-Agent-Id` pattern. - `mcp_http` reads those headers back into `ToolExecContext`. - `tool_channel_send` rejects an explicitly-targeted recipient that does not match the current conversation on the same channel. The comparison key is the conversation id (`sender_chat_id`, falling back to `sender_id` for DMs) so legitimate group replies pass. A different-channel dispatch stays allowed; an auto-filled recipient (reply to the inbound peer) is never scrutinised. Out-of-band callers with no peer scope run unguarded, preserving existing behaviour. The new `CompletionRequest` fields default via the struct's existing `Default` derive, so the ~35 construction sites that do not carry peer scope use `..Default::default()` rather than each spelling out three `None`s. Tests: - `tool_runner::tests` — guard unit tests: same-channel mismatch DENIED, same-channel match passes, group reply to chat_id passes, cross-channel passes, out-of-band (no peer scope) passes. - `claude_code` — `write_mcp_config` emits / omits the peer headers. - `api_integration_test` — end-to-end `/mcp` `tools/call` for `channel_send` proving the headers reach the guard (blocked with headers, unguarded without). Verification: `cargo check --workspace --lib`; `cargo clippy` (zero warnings) on the changed crates; scoped `cargo test` for librefang-runtime / llm-drivers / api / kernel.
…sentence-boundary line breaks The three peer-scope fields added for #6117 (`sender_user_id`, `sender_channel`, `sender_chat_id`) and the MCP_CHANNEL_MANIFEST doc comment were wrapped at a column boundary mid-sentence. Per the repo's universal prose rule, line breaks inside a paragraph must fall only at sentence boundaries; editors soft-wrap, viewers reflow.
…pient match + empty chat_id fallback (#6117) Self-review hardening of the #6117 guard: - Compare the explicit recipient case-SENSITIVELY. `send_channel_*` lookups are case-sensitive (#6078), so the earlier `eq_ignore_ascii_case` match would let `recipient = "OWNER-JID"` pass the guard while the send routed to a distinct case-sensitive chat — a narrow bypass. The channel match stays case-insensitive so the guard still fires when the model varies the channel casing. - Filter an empty `sender_chat_id` before the `sender_id` DM fallback. The in-process path stamps the raw metadata value (the `/mcp` bridge already drops empty headers), so `Some("")` previously masked the fallback and silently disabled the guard. Adds regression tests for both: a case-variant recipient is blocked, and an empty chat_id still enforces via the sender_id fallback.
houko
added a commit
that referenced
this pull request
Jul 15, 2026
…ugh the /mcp bridge (#6443) (#6455) #6449 fixed the cross-account (cross-tenant) channel_send guard on the in-process agent-loop path and flagged the /mcp bridge (subprocess claude-code driver) as an unchanged parallel surface pending a maintainer call. This lands that parity, mirroring how #6125 threaded the #6117 peer scope through the same chain: - CompletionRequest gains sender_account_id — the bot account / tenant the inbound turn arrived on. Out-of-band callers (cron, triggers, compaction, routing probes) leave it None via the struct's existing Default. - Both agent loops (run_agent_loop and the streaming variant) populate it from the manifest metadata stamp the kernel already writes (SENDER_ACCOUNT_ID_METADATA_KEY, #6449). - The claude-code driver's write_mcp_config forwards it on the bridge connection as X-LibreFang-Current-Account-Id, next to the existing X-LibreFang-Current-Peer-Jid / -Channel / -Chat-Id headers; empty or absent values emit no header. - mcp_http reads the header back and routes tool execution through execute_tool_with_sender_account, so tool_channel_send's existing #6443 guard rejects an explicit account_id that differs from the turn's account on the same channel. Clients that omit the header run unguarded exactly as before. Tests: - claude_code::tests — the peer-scope header tests now cover the account header (emitted when present, omitted when absent). - api_integration_test::test_mcp_http_channel_send_cross_account_guard_uses_account_header — end-to-end /mcp tools/call: mismatched account blocked, matching account passes, missing header no-ops. Verified: cargo check --workspace --lib, cargo clippy --workspace --all-targets -D warnings (clean), cargo fmt, scoped cargo test (llm-drivers mcp_config: 5 passed; api channel_send guards: 2 passed; runtime channel_send: 23 passed). Co-authored-by: Evan <[email protected]>
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.
Summary
Fixes #6117. An agent's
channel_sendcould dispatch a message to a different recipient than the current turn's inbound peer on the same channel, with no validation — a cross-chat confidentiality leak (observed live 2026-05-19: audio voice notes delivered to a stranger's WhatsApp chat).The leak vector is the subprocess-driver
/mcpbridge.routes/network.rs::mcp_httphardcodedsender_id/channel/chat_idtoNone, sotool_channel_sendhad no peer scope to validate against. A guard alone was not viable — the prerequisite peer-scope plumbing was absent onmain(confirmed against the issue's feasibility note). This PR lands the full propagation chain plus the guard, reimplemented on currentmain(the prior attempt #5282 was closed and ~1 month stale).The propagation chain
CompletionRequestgainssender_user_id/sender_channel/sender_chat_id, populated from the turn's existing manifest-metadata bindings in the agent loop (streaming + non-streaming). Out-of-band callers (cron, triggers, compaction, routing probes) leave themNone.claude-codedriver forwards them on the bridge connection asX-LibreFang-Current-Peer-Jid/-Channel/-Chat-Id, mirroring the existingX-LibreFang-Agent-Idpattern inwrite_mcp_config.mcp_httpreads those headers back intoToolExecContext.tool_channel_sendenforces the guard.The guard
Rejects an explicitly-targeted recipient that does not match the current conversation on the same channel the turn arrived on.
sender_chat_id), falling back tosender_idfor DMs — so a legitimate group reply (recipient = group jid ≠ individual speaker) passes.recipientcan be the leak.notify_owneror waits for that contact's inbound message.Construction-site ripple
The new
CompletionRequestfields default via the struct's existingDefaultderive, so the ~35 construction sites that carry no peer scope use..Default::default()rather than each spelling out threeNones. Only the two agent-loop sites set real values.Tests
tool_runner::tests— guard unit tests: same-channel mismatch DENIED; same-channel match passes; group reply to chat_id passes; cross-channel passes; out-of-band (no peer scope) passes.claude_code—write_mcp_configemits the peer headers when present, omits them when absent.api_integration_test::test_mcp_http_channel_send_cross_chat_guard_uses_peer_headers— end-to-end/mcptools/callforchannel_send: blocked with peer headers, unguarded without.Verification
cargo check --workspace --lib— cleancargo clippy --all-targetson the changed crates — zero warningscargo test -p librefang-runtime(tool_runner: 242 incl. guard tests),-p librefang-llm-drivers(driver header tests),-p librefang-api --test api_integration_test(the/mcptest),-p librefang-kernel -p librefang-llm-driver -p librefang-testing --lib(1238 passed, 0 failed)