Repository navigation
fix(security): prevent metadata spoofing of internal job monitor flag - #1195
Conversation
github-actions
Bot
added
scope: agent
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a crucial security enhancement by addressing a vulnerability where external channels could potentially bypass the agent's core processing pipeline by spoofing an internal job monitor flag. The changes introduce a dedicated, internally controlled Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a security fix to prevent metadata spoofing of an internal job monitor flag. It achieves this by adding an is_internal boolean field to IncomingMessage, which can only be set by trusted code via a new as_internal() method. The agent loop now correctly checks this flag to bypass the normal processing pipeline for internal messages. This is a robust solution to the security vulnerability. The implementation is clean, and the inclusion of new tests to verify the fix and prevent regressions is excellent.
There was a problem hiding this comment.
Pull request overview
Closes a security gap where external channel-provided metadata could spoof an “internal job monitor” message and bypass the normal agent pipeline by introducing an explicit internal flag on IncomingMessage and using it in the agent loop.
Changes:
- Adds
IncomingMessage.is_internalplusas_internal()and updates call sites/tests accordingly. - Reworks job monitor injection to use explicit route context (channel/user/thread) and marks injected messages as internal; agent loop now short-circuits on
message.is_internal. - Bumps registry manifest versions to
0.2.1and updatesCargo.lock.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/channels/channel.rs |
Adds is_internal to IncomingMessage and an as_internal() setter. |
src/agent/agent_loop.rs |
Forwards internal messages directly, bypassing LLM/tool loop. |
src/agent/job_monitor.rs |
Adds JobMonitorRoute, injects routed internal messages, updates/extends tests. |
src/tools/builtin/job.rs |
Spawns job monitor with explicit route from JobContext metadata; skips if missing. |
src/agent/dispatcher.rs |
Populates JobContext.metadata with notify route fields for monitoring. |
tests/e2e_routine_heartbeat.rs |
Updates IncomingMessage struct literals to include is_internal. |
registry/tools/web-search.json |
Version bump to 0.2.1. |
registry/tools/github.json |
Version bump to 0.2.1. |
registry/channels/discord.json |
Version bump to 0.2.1. |
Cargo.lock |
Updates uds_windows to 1.2.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| /// Mark this message as internal (bypasses user-input pipeline). | ||
| pub fn as_internal(mut self) -> Self { |
| pub is_internal: bool, | ||
| } | ||
|
|
||
| impl IncomingMessage { |
| // The `is_internal` flag can only be set by trusted in-process code | ||
| // (not via metadata), so external channels cannot spoof it. |
6d5b3b3 to
80a0305
Compare
zmanian
left a comment
zmanian
left a comment
There was a problem hiding this comment.
Review: APPROVE
Well-executed security fix for a real vulnerability. WASM channel plugins could set a magic metadata key (__internal_job_monitor) to bypass the entire agent pipeline — hooks, safety checks, LLM processing.
The fix: Replaces metadata-based check with a typed is_internal: bool field on IncomingMessage that defaults to false and can only be set via as_internal(), called only from trusted in-process code (job_monitor.rs).
Bypass vectors checked — none found:
IncomingMessagedoesn't deriveDeserialize— no wire-level injection- WASM wrapper uses
IncomingMessage::new()which defaults tofalse with_metadata()doesn't touchis_internal- Struct literal sites verified:
thread_ops.rsclones existing message, test sites setfalse
Additional defense in depth: Removes forwarding of full metadata blob to job monitor, replacing with typed JobMonitorRoute struct carrying only routing fields.
Minor suggestions (non-blocking):
is_internalispub— considerpub(crate)or a comment noting it should only be set viaas_internal()- Brief comment on why
notify_userhas a fallback butnotify_channeldoesn't inmonitor_route_from_ctx
CI all green.
There was a problem hiding this comment.
Pull request overview
Hardens internal message handling by replacing metadata-based “internal job monitor” detection with a non-spoofable IncomingMessage::is_internal flag, ensuring external channels/plugins can’t bypass the normal agent pipeline.
Changes:
- Add
is_internaltoIncomingMessageand anas_internal()constructor-style setter for trusted in-process injection. - Update job monitor injection to route notifications back to the originating channel/user/thread and mark them internal.
- Update agent loop to short-circuit internal messages (bypass LLM/tools/hooks pipeline) and adjust tests to use the new constructor APIs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e_routine_heartbeat.rs | Switches test message construction to IncomingMessage::new(...). |
| src/tools/builtin/job.rs | Adds route extraction from JobContext metadata and passes it into the job monitor spawn. |
| src/channels/channel.rs | Introduces IncomingMessage.is_internal and as_internal(); initializes flag in new(). |
| src/agent/job_monitor.rs | Adds JobMonitorRoute, routes monitor output to real channel/user/thread, and marks injected messages internal; adds regression tests. |
| src/agent/dispatcher.rs | Populates JobContext.metadata with notify routing keys for job-monitor/message-tool routing. |
| src/agent/agent_loop.rs | Replaces metadata key check with message.is_internal and forwards internal content directly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "notify_channel": message.channel, | ||
| "notify_user": message.user_id, | ||
| "notify_thread_id": message.thread_id, |
There was a problem hiding this comment.
That Copilot comment is wrong. serde_json::json! doesn't move values — it calls serde_json::to_value()
which works with references via Serialize. Since String and Option implement Serialize, the
macro serializes from &message.channel etc. without moving.
The `__internal_job_monitor` metadata key that bypassed the entire agent pipeline (hooks, safety checks, LLM processing) was spoofable by external channels — WASM channel plugins could inject arbitrary metadata including this key, causing attacker-controlled content to be forwarded directly as assistant responses. Replace the metadata-based check with a dedicated `is_internal` field on `IncomingMessage` that can only be set via `into_internal()` by trusted in-process code. Both the field and setter are `pub(crate)` to prevent external crates from spoofing the flag. Also remove `notify_metadata` forwarding (the monitor only needs channel/user/thread routing) and the unused `__job_monitor_job_id` metadata key. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ddb9942 to
46a588e
Compare
…nearai#1195) The `__internal_job_monitor` metadata key that bypassed the entire agent pipeline (hooks, safety checks, LLM processing) was spoofable by external channels — WASM channel plugins could inject arbitrary metadata including this key, causing attacker-controlled content to be forwarded directly as assistant responses. Replace the metadata-based check with a dedicated `is_internal` field on `IncomingMessage` that can only be set via `into_internal()` by trusted in-process code. Both the field and setter are `pub(crate)` to prevent external crates from spoofing the flag. Also remove `notify_metadata` forwarding (the monitor only needs channel/user/thread routing) and the unused `__job_monitor_job_id` metadata key. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nearai#1195) The `__internal_job_monitor` metadata key that bypassed the entire agent pipeline (hooks, safety checks, LLM processing) was spoofable by external channels — WASM channel plugins could inject arbitrary metadata including this key, causing attacker-controlled content to be forwarded directly as assistant responses. Replace the metadata-based check with a dedicated `is_internal` field on `IncomingMessage` that can only be set via `into_internal()` by trusted in-process code. Both the field and setter are `pub(crate)` to prevent external crates from spoofing the flag. Also remove `notify_metadata` forwarding (the monitor only needs channel/user/thread routing) and the unused `__job_monitor_job_id` metadata key. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
is_internal: boolfield toIncomingMessagethat can only be set viaas_internal()by trusted in-process code — external channels cannot spoof it through metadata__internal_job_monitormetadata key check in the agent loop withmessage.is_internal, closing a security hole where WASM channel plugins could bypass the entire agent pipeline (hooks, safety checks, LLM processing)notify_metadataforwarding from dispatcher (monitor only needs channel/user/thread routing, not the full original metadata blob)__job_monitor_job_idmetadata key frombuild_internal_metadataFixes security issue identified in review of #1173.
Test plan
test_external_metadata_cannot_spoof_internal_flag— verifieswith_metadata({"__internal_job_monitor": true})does NOT setis_internaltest_as_internal_sets_flag— verifiesas_internal()correctly sets the flagtest_monitor_forwards_assistant_messagesupdated to assertis_internalinstead of metadata key🤖 Generated with Claude Code