Skip to content

harden mcp oauth discovery against private hosts and validate browser opener urls - #2172

Draft
kevinjosethomas wants to merge 1 commit into
mainfrom
eng-5345-oauth-discovery-hardening
Draft

harden mcp oauth discovery against private hosts and validate browser opener urls#2172
kevinjosethomas wants to merge 1 commit into
mainfrom
eng-5345-oauth-discovery-hardening

Conversation

@kevinjosethomas

@kevinjosethomas kevinjosethomas commented Sep 9, 2026

Copy link
Copy Markdown
Member

Context

ENG-5345: MCP OAuth discovery accepted private, loopback and link-local HTTPS destinations for every URL the flow contacts or opens (MCP endpoint, resource_metadata pointer, authorization_servers, metadata URLs, authorization/token/registration endpoints), and LoginDialogComponent.showAuth handed any string to open/xdg-open/rundll32.

Root cause: validatedHttpsUrl in packages/ai/src/mcp/oauth.ts only checked scheme, credentials and fragment, so a hostile PRM or metadata document could steer registration, the browser and the token POST at internal hosts. The dialog opener had no validation at all (defence in depth: today it only receives discovery-validated or constant URLs).

Changes

  • packages/ai: new isPublicHost (src/mcp/host-policy.ts) rejects literal IPv4/IPv6 addresses that are loopback, unspecified, private (RFC 1918, CGNAT), link-local, ULA, multicast, documentation/benchmark ranges and IPv4-mapped/compatible/NAT64/6to4 forms, plus localhost, *.localhost, *.local, *.internal, *.home.arpa and single-label names. validatedHttpsUrl applies it to every URL in the flow (endpoint, resource_metadata pointer, issuers, metadata endpoints, stored issuer on refresh); McpOAuthConfig.allowPrivateNetwork is the single escape hatch.
  • packages/coding-agent: mcpServers.<name>.allowPrivateNetwork (also mcp add --allow-private-network, OAuth servers only) is passed to the provider; documented in docs/mcp-integrations.md.
  • packages/coding-agent: validateBrowserUrl (src/utils/browser-url.ts) allows only http(s) URLs without credentials or control characters, capped at 8 KiB. showAuth no longer launches the opener, emits an OSC 8 hyperlink or offers copy for anything else; it shows a sanitized copy with a message instead. The TUI hyperlink opener (packages/tui/src/tui.ts) already checks scheme and control characters and is unchanged.
  • No model-facing surface changed.

Validation

Local (worktree, macOS): npm run check clean; packages/ai: npx tsx ../../node_modules/vitest/dist/cli.js --run test/mcp-oauth.test.ts test/mcp-host-policy.test.ts (82 passed); packages/coding-agent: same runner on test/login-dialog.test.ts test/browser-url.test.ts test/mcp-command.test.ts test/mcp-manager.test.ts test/oauth-selector.test.ts (89 passed).

Prime Sandbox node:24-bookworm, user tester, git archive of origin/main @ 427ea4c with the ENG-5345 validation fixtures (stubbed fetch / mocked execFileHidden):

  • Before (main): https://127.0.0.1/mcp with PRM naming https://10.0.0.7 produced https://10.0.0.7/authorize?... after requests to 127.0.0.1 and 10.0.0.7; file:///etc/passwd, javascript:alert(1) and an ESC/OSC 52 URL were all forwarded to xdg-open.
  • After (git apply of this branch): the same flow fails with MCP endpoint https://127.0.0.1 points at a private, loopback or local-network host; set allowPrivateNetwork: true ... with zero requests; only https://auth.example.org/authorize?x=1 reached xdg-open; the legitimate separate-AS case still yields https://auth.example.org/authorize?... with resource=https://srv.test/mcp. The suites above pass there too (82 + 89).

Linear: ENG-5345 — https://linear.app/primeintellect/issue/ENG-5345

Note

Reject private-network hosts in MCP OAuth discovery and validate browser opener URLs

  • Adds isPublicHost/isPublicIPv4/isPublicIPv6 classifiers in host-policy.ts that reject loopback, private, link-local, local-suffix, single-label, and reserved addresses for both IPv4 and IPv6.
  • Threads a UrlPolicy through every OAuth discovery, registration, token-exchange, and refresh path in oauth.ts so non-public destinations are blocked by default; providers can opt in via allowPrivateNetwork.
  • Adds --allow-private-network to mcp add in mcp-command.ts, stored per HTTP OAuth server and rejected for stdio/bearer-token configs.
  • Adds browser-url.ts validation that only opens absolute HTTP(S) URLs with no credentials or control characters and caps length at 8 KiB; the login dialog sanitizes and displays rejected URLs without launching a process or hyperlink.
  • Risk: existing MCP OAuth servers on private/loopback hosts will be rejected by default unless allowPrivateNetwork is set; check McpServerConfig.allowPrivateNetwork in settings-manager.ts and createMcpOAuthProvider policy threading in oauth.ts.

Macroscope summarized 3aa2113.

…and validate browser opener URLs

MCP OAuth discovery now refuses loopback, private, link-local and
local-network hosts for the MCP endpoint, the resource_metadata pointer,
authorization_servers, metadata URLs and the authorization, token and
registration endpoints unless the server sets allowPrivateNetwork. The
login dialog only launches the OS opener for http(s) URLs without
credentials or control characters and shows anything else as text.

Linear: ENG-5345
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Prime Agent performance — completed

PR 3aa21130 compared with main f9c7e06b.
↓ improved · ↑ regressed · ≈ no clear change · — unavailable

Metric Main This PR Change Change % Result
Cold startup 3,435.0 ms 3,510.9 ms ≈ +75.8 ms +2.21% no clear change
Warm startup 2,098.8 ms 2,085.6 ms ≈ -13.2 ms -0.63% no clear change
Installation 36.44 s 33.99 s ≈ -2.45 s -6.72% no clear change
Compressed release artifacts 11.10 MB 11.11 MB ≈ +0.01 MB +0.12% no clear change
Installed footprint 597.35 MB 597.40 MB ≈ +0.05 MB +0.01% no clear change
Idle memory, summed RSS 1,126.37 MB 1,116.82 MB ≈ -9.54 MB -0.85% no clear change

Python runtime

Metric Main This PR Change Change % Result
Python kernel startup 137.0 ms 143.2 ms ≈ +6.2 ms +4.51% no clear change
Python cell round trip 0.597 ms 0.639 ms ≈ +0.041 ms +6.92% no clear change
Empty bash command 13.3 ms 12.6 ms ≈ -0.7 ms -5.38% no clear change
Bash git status 18.8 ms 19.2 ms ≈ +0.4 ms +2.09% no clear change
Bash 32 KiB output 13.0 ms 13.8 ms ≈ +0.8 ms +5.94% no clear change
35 cells / 9 shell calls 189.9 ms 191.4 ms ≈ +1.5 ms +0.80% no clear change
Python interrupt to done 1.643 ms 1.602 ms ≈ -0.040 ms -2.46% no clear change
Python state snapshot 28.1 ms 29.2 ms ≈ +1.1 ms +3.99% no clear change
Python state restore 397.4 ms 403.8 ms ≈ +6.5 ms +1.63% no clear change
Python idle RSS 33.86 MB 33.96 MB ≈ +0.10 MB +0.30% no clear change
Python RSS after pandas workload 95.94 MB 96.83 MB ≈ +0.89 MB +0.93% no clear change

Sandbox cost: ~$0.1074 — no inference calls.
Run, logs, and downloadable raw results

Methodology and samples

Main resolved at 2026-09-09T23:53:50.137474+00:00. Harness f9c7e06b.
Linux x64, 4 vCPU, 8 GB RAM, 20 GB disk; region us.
Image: node:24-bookworm@sha256:be23f54a88d34e8824c741b19b91064094f92c1c97b194144bfc8b50d67258e2.
Stock tools, skills, daemon, and Python bootstrap enabled; fresh homes and a fixed Git fixture.
Onboarding is dismissed; the editor starts without a selected model or submitted prompt.
Medians shown. Arrows require a 20% timing/memory change plus absolute floors and IQR.
These practical noise floors are not a statistical significance test.
Cold means stopped Prime processes; OS filesystem caches are not flushed.
No model requests or credentials. Installation excludes build/setup time.
Installer tarballs use loopback; npm/Python downloads use the network with fresh caches.
Artifact size counts release tarballs; footprint after first use includes registry packages.
MB is decimal. Summed RSS can double-count shared pages; PSS is recorded when available.
Provisioning, setup, and build durations are recorded separately in the raw results.
Kernel probes use the installed JSONL runtime, outside the TUI/TypeScript host.
Per trial: 50 Python cells, 5 calls per shell case, and one 35-cell mix (9 git status calls).
Cell/shell values are batch means; other runtime timings are single operations.
State fixture: a 10,000-row × 8-column integer DataFrame and a 10,000-integer list.
Restore runs in a fresh kernel, including pandas imports; kernel startup is excluded.
Kernel RSS covers the isolated Python process; loaded RSS follows the pandas workload.
Costs estimate full sandbox lifetimes at configured rates, including setup and build.
Budget target: $1; not a billing cap. Checks are informational.

Metric Main successful/attempted PR successful/attempted Main spread PR spread
Cold startup 10/10 10/10 IQR 322.9 ms IQR 410.0 ms
Warm startup 10/10 10/10 IQR 218.9 ms IQR 112.6 ms
Installation 3/3 3/3 range 3.36 s range 3.04 s
Compressed release artifacts 1/1 1/1
Installed footprint 1/1 1/1
Idle memory, summed RSS 10/10 10/10 IQR 22.95 MB IQR 17.83 MB
Python kernel startup 10/10 10/10 IQR 5.9 ms IQR 7.1 ms
Python cell round trip 10/10 10/10 IQR 0.068 ms IQR 0.128 ms
Empty bash command 10/10 10/10 IQR 0.6 ms IQR 0.8 ms
Bash git status 10/10 10/10 IQR 0.8 ms IQR 1.3 ms
Bash 32 KiB output 10/10 10/10 IQR 0.9 ms IQR 1.1 ms
35 cells / 9 shell calls 10/10 10/10 IQR 12.8 ms IQR 8.9 ms
Python interrupt to done 10/10 10/10 IQR 0.175 ms IQR 0.182 ms
Python state snapshot 10/10 10/10 IQR 4.0 ms IQR 4.2 ms
Python state restore 10/10 10/10 IQR 40.8 ms IQR 21.6 ms
Python idle RSS 10/10 10/10 IQR 0.74 MB IQR 3.27 MB
Python RSS after pandas workload 10/10 10/10 IQR 2.50 MB IQR 3.33 MB

@Abydin

Abydin commented Sep 10, 2026

Copy link
Copy Markdown

Nice, this closes the literal-IP and private-suffix cases cleanly. A scope question on the discovery hardening:

isPublicHost validates the hostname as a string, so it correctly rejects literal private IPs and .internal / .local / single-label names. But a registered public hostname whose A/AAAA record points at a private address (for example oauth.attacker.com resolving to 169.254.169.254) still passes isPublicHost, and the discovery fetch would then connect to the private IP. Even if the host were resolved and checked at validation time, without pinning the resolved address for the actual connection it is re-resolved at socket time, so a DNS-rebinding TOCTOU remains between the check and the connect.

Is DNS-based SSRF in scope for this PR, or intentionally deferred to a follow-up? If in scope, the usual approach is to resolve the host, reject if any returned address is non-public, and pin that address for the connection (a custom undici lookup / dispatcher) so it cannot re-resolve after the check. Happy to help with that layer plus a rebinding regression test if it would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants