Harden interception and MCP capabilities - #2132
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9f31aef38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ApprovabilityVerdict: Needs human review This PR introduces security-sensitive changes to authentication flows (separating model/state capabilities, adding HMAC signature verification). Additionally, two unresolved review comments identify a potential bug where injected interceptions could cause 401 failures for shared tool state access. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
1dbdc9d to
3c1df80
Compare
Overview
Separates model inference access from rollout state access and hardens the shared MCP state bridge.
Details
/state;/taskremains available only to task-scoped state credentials.Rationale and impact
Previously, one rollout credential crossed model inference, task data, and mutable rollout state boundaries, while shared stateful MCP routes exposed their state credential to the harness. The narrower capability model keeps each component limited to the access it needs while preserving per-rollout state sharing for framework-owned tools.
Note
Medium Risk
Changes authentication boundaries for interception and shared MCP state; internal call sites are updated, but any external code unpacking the old 2-tuple Slot or relying on a single shared bearer would break.
Overview
Hardens interception and the MCP state bridge by giving model inference and rollout state separate bearer tokens, and by stopping shared-tool URLs from exposing per-rollout secrets to the harness.
Each rollout
Slotis now(base_url, model_secret, state_secret). Model routes still authenticate withmodel_secret;/stateand/taskusestate_secretonly, so a harness API key cannot read or mutate trace state or fetch task payloads. Interception pools and per-rollout servers threadstate_service_secretsfrom eval-level shared MCP processes;GET/PUT /statecan also authorize a trusted service bearer plusX-Verifiers-State-Route(trace id) to reach the right session without the per-rollout state token.Shared stateful MCP servers get a private
VF_STATE_SECRETat eval start. Per rollout, the harness URL gainsvf_state_url,vf_state_route, and an HMAC-SHA256vf_state_signatureover url+route instead of putting secrets in query strings. The server verifies signatures withhmac.compare_digestbefore proxying state. Task-scoped servers still receive the rolloutstate_secretvia env for/taskand direct state access.Agent._interception_forskips reusing a long-lived owned interception server when any shared tool carries astate_secret, so rollouts get a server that knows the shared service secrets.Env.servingcollects those secrets when building interception.Reviewed by Cursor Bugbot for commit 1dbdc9d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Harden interception and MCP state access with separate model/state credentials and HMAC-signed routing
Slotfrom a 2-tuple into a 3-tuple(base_url, model_secret, state_secret), giving rollouts distinct bearer tokens for model inference and state access across interception/base.py, pool.py, server.py, and rollout.py.state_service_secretsset; shared tool servers can read/write rollout state using their service secret plus anX-Verifiers-State-Routeheader instead of the rollout's state bearer.vf_state_url,vf_state_route,vf_state_signature) instead of embedding a bearer token in the query string, keeping the service secret private.serve_in_runtimein mcp/launch.py always writesVF_STATE_URLandVF_STATE_SECRETto subprocess environments to prevent inheritance of stale values.Slotas a 2-tuple will break; all internal consumers are updated but any external or third-party code depending on the old tuple shape must be updated.Macroscope summarized 3c1df80.