Skip to content

fix(security/channels): enforce cross-chat dispatch guard through the /mcp bridge (#6117)#6125

Merged
houko merged 3 commits into
mainfrom
fix/channel-send-cross-chat-leak
Jun 16, 2026
Merged

fix(security/channels): enforce cross-chat dispatch guard through the /mcp bridge (#6117)#6125
houko merged 3 commits into
mainfrom
fix/channel-send-cross-chat-leak

Conversation

@houko

@houko houko commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #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 2026-05-19: audio voice notes delivered to a stranger's WhatsApp chat).

The leak vector is the subprocess-driver /mcp bridge. routes/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 was not viable — the prerequisite peer-scope plumbing was absent on main (confirmed against the issue's feasibility note). This PR lands the full propagation chain plus the guard, reimplemented on current main (the prior attempt #5282 was closed and ~1 month stale).

The propagation chain

  • 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 in write_mcp_config.
  • mcp_http reads those headers back into ToolExecContext.
  • tool_channel_send enforces the guard.

The guard

Rejects an explicitly-targeted recipient that does not match the current conversation on the same channel the turn arrived on.

  • The comparison key is the conversation id (sender_chat_id), falling back to sender_id for DMs — so a legitimate group reply (recipient = group jid ≠ individual speaker) passes.
  • An auto-filled recipient (reply to the inbound peer) is never scrutinised — only an explicit recipient can be the leak.
  • A different-channel dispatch (e.g. emailing while replying to a WhatsApp peer) stays allowed; only intra-channel re-targeting is the leak.
  • Out-of-band callers with no peer scope run unguarded, preserving existing behaviour. To legitimately reach a different contact, the agent uses notify_owner or waits for that contact's inbound message.

Construction-site ripple

The new CompletionRequest fields default via the struct's existing Default derive, so the ~35 construction sites that carry no peer scope use ..Default::default() rather than each spelling out three Nones. 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_codewrite_mcp_config emits 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 /mcp tools/call for channel_send: blocked with peer headers, unguarded without.

Verification

  • cargo check --workspace --lib — clean
  • cargo clippy --all-targets on the changed crates — zero warnings
  • cargo 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 /mcp test), -p librefang-kernel -p librefang-llm-driver -p librefang-testing --lib (1238 passed, 0 failed)

Evan and others added 2 commits June 16, 2026 12:58
… /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.
@github-actions github-actions Bot added area/runtime Agent loop, LLM drivers, WASM sandbox area/kernel Core kernel (scheduling, RBAC, workflows) size/L 250-999 lines changed labels Jun 16, 2026
…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
houko merged commit 2d5a445 into main Jun 16, 2026
32 checks passed
@houko
houko deleted the fix/channel-send-cross-chat-leak branch June 16, 2026 06:59
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kernel Core kernel (scheduling, RBAC, workflows) area/runtime Agent loop, LLM drivers, WASM sandbox size/L 250-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(security/channels): channel_send can dispatch to a different recipient on the same channel (cross-chat message leak)

2 participants