You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The engine-v2 auth surfacing path in src/bridge/effect_adapter.rs passes auth_url straight through from the tool_activate/tool_auth tool output to the gate_required SSE event without validating the URL scheme. A malicious or buggy tool can return javascript:, file://, http://, or data: and it will reach the browser.
The engine-v1 path in src/agent/dispatcher.rs gained a sanitize_auth_url helper (https-only) in #2038, but the v2 path was left untouched. This is a pre-existing gap, not a regression — filing as a follow-up from the #2038 post-merge review.
Same pattern in the `tool_install` post-readiness branch (`effect_adapter.rs:865`), the pre-flight `check_action_auth` gate (`effect_adapter.rs:665`), and the pre-flight `check_tool_readiness` gate (`effect_adapter.rs:706`). All of these feed `ResumeKind::Authentication { auth_url, .. }` into the engine gate stream and onto the client.
v1: sanitized (reference implementation)
`src/agent/dispatcher.rs` (`sanitize_auth_url`):
```rust
/// Only allow `https://` URLs for auth/setup links to prevent scheme injection
/// (e.g. `javascript:`, `file://`). Host validation is explicitly out of scope:
/// the URL source is a trusted local extension tool result, and display-side
/// rendering controls where the user is actually navigated.
fn sanitize_auth_url(url: Option<&str>) -> Option {
url.map(str::trim)
.filter(|u| u.starts_with("https://"))
.map(ToOwned::to_owned)
}
```
Applied in `parse_auth_result` to both `auth_url` and `setup_url`. Test coverage: `test_sanitize_auth_url_rejects_non_https_schemes`, `test_parse_auth_result_strips_non_https_urls`.
Threat model
`tool_activate`/`tool_auth` output comes from local extensions (WASM, MCP, shell, HTTP). These are trusted-ish but not bulletproof — a buggy extension, a misconfigured MCP server responding with unexpected JSON, or a registry-installed tool with a compromised capabilities file can produce an `auth_url` that:
navigates the user to an attacker-controlled origin outside the expected OAuth provider
executes `javascript:` in the page's origin (cookie theft, session hijack) if the frontend renders it into an `` without its own defense
opens `file://` or `data:` URIs
The web frontend already opens `auth_url` in a new tab via `window.open(url, '_blank')` (see #2050 — "Open OAuth auth links in a new tab"). Modern browsers will refuse `javascript:` in `window.open` targets in most cases, but this is defense-in-depth we already ship for v1 and there's no reason for v2 to lag.
Desired fix
Extract `sanitize_auth_url` from `src/agent/dispatcher.rs` into a shared location. Natural home: `src/auth/oauth.rs` (where [codex] Stabilize auth readiness and gate flows #2050 moved the rest of the shared OAuth runtime) as `pub(crate) fn sanitize_auth_url(url: Option<&str>) -> Option`.
Update the dispatcher v1 path to import from there instead of keeping its private copy.
Apply it at every v2 site that extracts `auth_url` from tool output:
Any matching sites in `src/bridge/auth_manager.rs` that build `auth_url` from runtime data rather than from hard-coded config.
Regression test on the v2 path per the "Test Through the Caller, Not Just the Helper" rule in `.claude/rules/testing.md`: drive `EffectAdapter::execute_action` with an `OAuthPromptTool` fixture that returns `auth_url: "javascript:alert(1)"` and assert the resulting `ResumeKind::Authentication.auth_url` is `None` (not the original string).
Out of scope
Host validation (e.g. allowlist of OAuth providers). The v1 comment explicitly documents this as out of scope: the source is a local trusted extension, and display-side rendering controls navigation. Keep the invariant symmetric across v1/v2.
Any change to the actual `tool_activate`/`tool_auth` schema — the fix lives purely in the host-side reader.
Why now
Filed as a post-merge follow-up from PR #2038 review after staging was merged into the PR branch. The two auth-surfacing paths (v1 dispatcher and v2 effect adapter) were previously parallel implementations of the same tool-output contract with slightly different invariants. Consolidating the URL sanitizer into one shared helper eliminates the divergence and prevents the gap from recurring the next time one side is refactored.
Summary
The engine-v2 auth surfacing path in
src/bridge/effect_adapter.rspassesauth_urlstraight through from thetool_activate/tool_authtool output to thegate_requiredSSE event without validating the URL scheme. A malicious or buggy tool can returnjavascript:,file://,http://, ordata:and it will reach the browser.The engine-v1 path in
src/agent/dispatcher.rsgained asanitize_auth_urlhelper (https-only) in #2038, but the v2 path was left untouched. This is a pre-existing gap, not a regression — filing as a follow-up from the #2038 post-merge review.Locations
v2: no sanitization (vulnerable)
src/bridge/effect_adapter.rs:147—auth_gate_from_extension_result:Same pattern in the `tool_install` post-readiness branch (`effect_adapter.rs:865`), the pre-flight `check_action_auth` gate (`effect_adapter.rs:665`), and the pre-flight `check_tool_readiness` gate (`effect_adapter.rs:706`). All of these feed `ResumeKind::Authentication { auth_url, .. }` into the engine gate stream and onto the client.
v1: sanitized (reference implementation)
`src/agent/dispatcher.rs` (`sanitize_auth_url`):
```rust
/// Only allow `https://` URLs for auth/setup links to prevent scheme injection
/// (e.g. `javascript:`, `file://`). Host validation is explicitly out of scope:
/// the URL source is a trusted local extension tool result, and display-side
/// rendering controls where the user is actually navigated.
fn sanitize_auth_url(url: Option<&str>) -> Option {
url.map(str::trim)
.filter(|u| u.starts_with("https://"))
.map(ToOwned::to_owned)
}
```
Applied in `parse_auth_result` to both `auth_url` and `setup_url`. Test coverage: `test_sanitize_auth_url_rejects_non_https_schemes`, `test_parse_auth_result_strips_non_https_urls`.
Threat model
`tool_activate`/`tool_auth` output comes from local extensions (WASM, MCP, shell, HTTP). These are trusted-ish but not bulletproof — a buggy extension, a misconfigured MCP server responding with unexpected JSON, or a registry-installed tool with a compromised capabilities file can produce an `auth_url` that:
The web frontend already opens `auth_url` in a new tab via `window.open(url, '_blank')` (see #2050 — "Open OAuth auth links in a new tab"). Modern browsers will refuse `javascript:` in `window.open` targets in most cases, but this is defense-in-depth we already ship for v1 and there's no reason for v2 to lag.
Desired fix
Out of scope
Why now
Filed as a post-merge follow-up from PR #2038 review after staging was merged into the PR branch. The two auth-surfacing paths (v1 dispatcher and v2 effect adapter) were previously parallel implementations of the same tool-output contract with slightly different invariants. Consolidating the URL sanitizer into one shared helper eliminates the divergence and prevents the gap from recurring the next time one side is refactored.