diff --git a/packages/ai/.changes/eng-5345-oauth-private-network.md b/packages/ai/.changes/eng-5345-oauth-private-network.md new file mode 100644 index 0000000000..5ed67529f7 --- /dev/null +++ b/packages/ai/.changes/eng-5345-oauth-private-network.md @@ -0,0 +1 @@ +- Fixed MCP OAuth discovery accepting loopback, private, link-local and local-network hosts (`127.0.0.1`, `10.x`, `localhost`, `*.internal`, ...) for the MCP endpoint and every discovered authorization-server, metadata, registration, authorization and token URL; `allowPrivateNetwork: true` opts a server back in. diff --git a/packages/ai/src/mcp/host-policy.ts b/packages/ai/src/mcp/host-policy.ts new file mode 100644 index 0000000000..1ad27927c1 --- /dev/null +++ b/packages/ai/src/mcp/host-policy.ts @@ -0,0 +1,102 @@ +import { isIPv4, isIPv6 } from "node:net"; + +/** Hostname suffixes that only ever resolve inside a local network. */ +const PRIVATE_NAME_SUFFIXES = [".localhost", ".local", ".internal", ".home.arpa"]; + +const PRIVATE_IPV4_RANGES: Array<[number, number]> = [ + [0x00000000, 8], // 0.0.0.0/8 "this" network + [0x0a000000, 8], // 10.0.0.0/8 + [0x64400000, 10], // 100.64.0.0/10 shared address space (CGNAT) + [0x7f000000, 8], // 127.0.0.0/8 loopback + [0xa9fe0000, 16], // 169.254.0.0/16 link-local + [0xac100000, 12], // 172.16.0.0/12 + [0xc0000000, 24], // 192.0.0.0/24 IETF protocol assignments + [0xc0000200, 24], // 192.0.2.0/24 TEST-NET-1 + [0xc0586300, 24], // 192.88.99.0/24 6to4 relay anycast (deprecated) + [0xc0a80000, 16], // 192.168.0.0/16 + [0xc6120000, 15], // 198.18.0.0/15 benchmarking + [0xc6336400, 24], // 198.51.100.0/24 TEST-NET-2 + [0xcb007100, 24], // 203.0.113.0/24 TEST-NET-3 + [0xe0000000, 4], // 224.0.0.0/4 multicast + [0xf0000000, 4], // 240.0.0.0/4 reserved and broadcast +]; + +function ipv4ToNumber(address: string): number { + return address.split(".").reduce((value, octet) => value * 256 + Number(octet), 0); +} + +function isPublicIPv4(address: string): boolean { + const value = ipv4ToNumber(address); + return !PRIVATE_IPV4_RANGES.some(([base, bits]) => value >>> (32 - bits) === base >>> (32 - bits)); +} + +/** Expand an IPv6 literal into eight 16-bit groups; undefined when malformed. */ +function ipv6Groups(address: string): number[] | undefined { + let text = address; + const zone = text.indexOf("%"); + if (zone !== -1) text = text.slice(0, zone); + // Embedded dotted IPv4 tail (e.g. ::ffff:127.0.0.1) becomes two groups. + const lastColon = text.lastIndexOf(":"); + const tail = text.slice(lastColon + 1); + if (tail.includes(".")) { + if (!isIPv4(tail)) return undefined; + const v4 = ipv4ToNumber(tail); + text = `${text.slice(0, lastColon + 1)}${(v4 >>> 16).toString(16)}:${(v4 & 0xffff).toString(16)}`; + } + const halves = text.split("::"); + if (halves.length > 2) return undefined; + const parse = (part: string): number[] => (part === "" ? [] : part.split(":").map((group) => parseInt(group, 16))); + const head = parse(halves[0] ?? ""); + const rest = halves.length === 2 ? parse(halves[1] ?? "") : []; + const missing = 8 - head.length - rest.length; + if (missing < 0 || (halves.length === 1 && missing !== 0)) return undefined; + const groups = [...head, ...new Array(Math.max(0, missing)).fill(0), ...rest]; + return groups.length === 8 && groups.every((group) => Number.isInteger(group) && group >= 0 && group <= 0xffff) + ? groups + : undefined; +} + +function isPublicIPv6(address: string): boolean { + const groups = ipv6Groups(address); + if (!groups) return false; + const [g0, g1, g2, g3, g4, g5, g6, g7] = groups as [number, number, number, number, number, number, number, number]; + const embeddedV4 = (): string => `${g6 >>> 8}.${g6 & 0xff}.${g7 >>> 8}.${g7 & 0xff}`; + // :: (unspecified) and ::1 (loopback) + if (g0 === 0 && g1 === 0 && g2 === 0 && g3 === 0 && g4 === 0 && g5 === 0 && g6 === 0 && (g7 === 0 || g7 === 1)) { + return false; + } + // ::ffff:a.b.c.d (IPv4-mapped) and ::a.b.c.d (IPv4-compatible, deprecated) + if (g0 === 0 && g1 === 0 && g2 === 0 && g3 === 0 && g4 === 0 && (g5 === 0xffff || g5 === 0)) { + return isPublicIPv4(embeddedV4()); + } + // 64:ff9b::/96 NAT64 well-known prefix + if (g0 === 0x64 && g1 === 0xff9b && g2 === 0 && g3 === 0 && g4 === 0 && g5 === 0) return isPublicIPv4(embeddedV4()); + if ((g0 & 0xfe00) === 0xfc00) return false; // fc00::/7 unique local + if ((g0 & 0xffc0) === 0xfe80) return false; // fe80::/10 link-local + if ((g0 & 0xffc0) === 0xfec0) return false; // fec0::/10 site-local (deprecated) + if ((g0 & 0xff00) === 0xff00) return false; // ff00::/8 multicast + if (g0 === 0x2001 && g1 === 0x0db8) return false; // 2001:db8::/32 documentation + if (g0 === 0x2001 && g1 === 0) return false; // 2001::/32 Teredo (tunnels client addresses) + if (g0 === 0x2002) return isPublicIPv4(`${g1 >>> 8}.${g1 & 0xff}.${g2 >>> 8}.${g2 & 0xff}`); // 2002::/16 6to4 + return true; +} + +/** + * True when a URL hostname (as produced by the WHATWG URL parser) can only name a + * public destination: not a loopback, private, link-local, multicast or otherwise + * reserved literal address and not a name reserved for local networks. + */ +export function isPublicHost(hostname: string): boolean { + const host = hostname.toLowerCase().replace(/\.$/, ""); + if (host === "") return false; + if (host.startsWith("[") && host.endsWith("]")) { + const literal = host.slice(1, -1); + return isIPv6(literal) && isPublicIPv6(literal); + } + if (isIPv4(host)) return isPublicIPv4(host); + if (isIPv6(host)) return isPublicIPv6(host); + if (host === "localhost") return false; + if (PRIVATE_NAME_SUFFIXES.some((suffix) => host.endsWith(suffix))) return false; + // Single-label names resolve through local search domains, never public DNS. + return host.includes("."); +} diff --git a/packages/ai/src/mcp/oauth.ts b/packages/ai/src/mcp/oauth.ts index 525d6b4fb1..3ca1ff2cb0 100644 --- a/packages/ai/src/mcp/oauth.ts +++ b/packages/ai/src/mcp/oauth.ts @@ -2,6 +2,7 @@ import type { Server } from "node:http"; import { oauthErrorHtml, oauthSuccessHtml } from "../utils/oauth/oauth-page.js"; import { generatePKCE } from "../utils/oauth/pkce.js"; import type { OAuthCredentials, OAuthLoginCallbacks, OAuthProviderInterface } from "../utils/oauth/types.js"; +import { isPublicHost } from "./host-policy.js"; const CALLBACK_HOST = process.env.PI_OAUTH_CALLBACK_HOST || "127.0.0.1"; // A range (not one port) so a leaked/concurrent login can't wedge all logins with EADDRINUSE. @@ -33,6 +34,11 @@ interface Discovery { issuer?: string; } +/** Host restrictions applied to every URL the OAuth flow contacts or opens. */ +interface UrlPolicy { + allowPrivateNetwork: boolean; +} + export interface McpOAuthConfig { /** MCP server name; provider id becomes `mcp:`. */ server: string; @@ -44,6 +50,12 @@ export interface McpOAuthConfig { clientId?: string; /** Requested OAuth scopes; defaults to the server's advertised scopes. */ scopes?: string; + /** + * Permit loopback, private, link-local and local-network hostnames (e.g. `127.0.0.1`, + * `10.0.0.7`, `localhost`, `*.internal`) for the MCP endpoint and every OAuth URL discovered + * from it. Off by default so hostile metadata cannot steer the flow at internal services. + */ + allowPrivateNetwork?: boolean; } interface McpCredentials extends OAuthCredentials { @@ -57,7 +69,7 @@ interface McpCredentials extends OAuthCredentials { issuer?: string; } -function validatedHttpsUrl(value: string, name: string): URL { +function validatedHttpsUrl(value: string, name: string, policy: UrlPolicy): URL { let url: URL; try { url = new URL(value); @@ -67,6 +79,12 @@ function validatedHttpsUrl(value: string, name: string): URL { if (url.protocol !== "https:" || url.username || url.password || url.hash) { throw new Error(`${name} must be an absolute HTTPS URL without credentials or a fragment`); } + if (!policy.allowPrivateNetwork && !isPublicHost(url.hostname)) { + throw new Error( + `${name} ${url.origin} points at a private, loopback or local-network host; ` + + `set allowPrivateNetwork: true for this MCP server to allow it`, + ); + } return url; } @@ -75,8 +93,8 @@ function canonicalResource(url: URL): string { return `${url.origin}${url.pathname}${url.search}`; } -function authorizationServerMetadataUrls(issuer: string): string[] { - const url = validatedHttpsUrl(issuer, "Authorization server issuer"); +function authorizationServerMetadataUrls(issuer: string, policy: UrlPolicy): string[] { + const url = validatedHttpsUrl(issuer, "Authorization server issuer", policy); if (url.search) throw new Error("Authorization server issuer must not contain a query string"); const path = url.pathname === "/" ? "" : url.pathname.replace(/\/$/, ""); return [ @@ -97,7 +115,12 @@ async function fetchJson(url: string, init?: RequestInit): Promise { return res.json(); } -function authorizationServerMetadata(value: unknown, issuer: string, requireExactIssuer: boolean): AuthServerMetadata { +function authorizationServerMetadata( + value: unknown, + issuer: string, + requireExactIssuer: boolean, + policy: UrlPolicy, +): AuthServerMetadata { if (!value || typeof value !== "object") throw new Error(`Authorization server metadata for ${issuer} is invalid`); const metadata = value as Partial; if (typeof metadata.issuer !== "string") { @@ -108,7 +131,7 @@ function authorizationServerMetadata(value: unknown, issuer: string, requireExac throw new Error(`Authorization server metadata issuer does not exactly match ${issuer}`); } } else { - const advertisedIssuer = validatedHttpsUrl(metadata.issuer, "Authorization server metadata issuer"); + const advertisedIssuer = validatedHttpsUrl(metadata.issuer, "Authorization server metadata issuer", policy); if (advertisedIssuer.origin !== new URL(issuer).origin || advertisedIssuer.search) { throw new Error(`Origin authorization server metadata issuer must stay on ${new URL(issuer).origin}`); } @@ -116,9 +139,11 @@ function authorizationServerMetadata(value: unknown, issuer: string, requireExac if (typeof metadata.authorization_endpoint !== "string" || typeof metadata.token_endpoint !== "string") { throw new Error(`Authorization server metadata for ${issuer} is missing required endpoints`); } - validatedHttpsUrl(metadata.authorization_endpoint, "Authorization endpoint"); - validatedHttpsUrl(metadata.token_endpoint, "Token endpoint"); - if (metadata.registration_endpoint) validatedHttpsUrl(metadata.registration_endpoint, "Registration endpoint"); + validatedHttpsUrl(metadata.authorization_endpoint, "Authorization endpoint", policy); + validatedHttpsUrl(metadata.token_endpoint, "Token endpoint", policy); + if (metadata.registration_endpoint) { + validatedHttpsUrl(metadata.registration_endpoint, "Registration endpoint", policy); + } return metadata as AuthServerMetadata; } @@ -129,14 +154,23 @@ async function jsonMetadata(response: Response, url: string): Promise { return response.json(); } -async function discoverAuthorizationServer(issuer: string, requireExactIssuer: boolean): Promise { - const candidates = authorizationServerMetadataUrls(issuer); +async function discoverAuthorizationServer( + issuer: string, + requireExactIssuer: boolean, + policy: UrlPolicy, +): Promise { + const candidates = authorizationServerMetadataUrls(issuer, policy); let lastError: unknown; for (const candidate of candidates) { try { const response = await fetchResponse(candidate); if (response.status === 404) continue; - return authorizationServerMetadata(await jsonMetadata(response, candidate), issuer, requireExactIssuer); + return authorizationServerMetadata( + await jsonMetadata(response, candidate), + issuer, + requireExactIssuer, + policy, + ); } catch (error) { lastError = error; } @@ -156,7 +190,7 @@ function randomState(): string { .replace(/=/g, ""); } -function resourceMetadata(value: unknown, resource: string): ProtectedResourceMetadata { +function resourceMetadata(value: unknown, resource: string, policy: UrlPolicy): ProtectedResourceMetadata { if (!value || typeof value !== "object") throw new Error("Protected-resource metadata is invalid"); const metadata = value as Partial; if (metadata.resource !== resource) @@ -167,7 +201,7 @@ function resourceMetadata(value: unknown, resource: string): ProtectedResourceMe for (const issuer of metadata.authorization_servers) { if (typeof issuer !== "string") throw new Error("Protected-resource metadata has an invalid authorization server"); - validatedHttpsUrl(issuer, "Authorization server issuer"); + validatedHttpsUrl(issuer, "Authorization server issuer", policy); } return metadata as ProtectedResourceMetadata; } @@ -183,8 +217,11 @@ function headerResourceMetadata(value: string | null): string | undefined { return match[1].replace(/\\(.)/g, "$1"); } -async function tryProtectedResourceMetadata(url: string): Promise { - const resource = validatedHttpsUrl(url, "MCP endpoint"); +async function tryProtectedResourceMetadata( + url: string, + policy: UrlPolicy, +): Promise { + const resource = validatedHttpsUrl(url, "MCP endpoint", policy); let headerUrl: string | undefined; try { // This probe deliberately has no Authorization header. It must not leak an existing token. @@ -196,30 +233,30 @@ async function tryProtectedResourceMetadata(url: string): Promise { - const protectedResource = await tryProtectedResourceMetadata(url); +async function discover(url: string, policy: UrlPolicy): Promise { + const protectedResource = await tryProtectedResourceMetadata(url, policy); if (protectedResource) { const issuer = protectedResource.authorization_servers[0]; return { - metadata: await discoverAuthorizationServer(issuer, true), + metadata: await discoverAuthorizationServer(issuer, true, policy), resource: protectedResource.resource, issuer, }; } - const issuer = validatedHttpsUrl(url, "MCP endpoint").origin; - return { metadata: await discoverAuthorizationServer(issuer, false) }; + const issuer = validatedHttpsUrl(url, "MCP endpoint", policy).origin; + return { metadata: await discoverAuthorizationServer(issuer, false, policy) }; } -async function registerClient(registrationEndpoint: string, label: string): Promise { - validatedHttpsUrl(registrationEndpoint, "Registration endpoint"); +async function registerClient(registrationEndpoint: string, label: string, policy: UrlPolicy): Promise { + validatedHttpsUrl(registrationEndpoint, "Registration endpoint", policy); const body = { client_name: label, redirect_uris: ALL_REDIRECT_URIS, @@ -346,8 +383,9 @@ function parseRedirectInput(input: string, expectedState: string): { code: strin async function exchangeToken( tokenEndpoint: string, params: Record, + policy: UrlPolicy, ): Promise<{ access_token: string; refresh_token?: string; expires_in?: number }> { - validatedHttpsUrl(tokenEndpoint, "Token endpoint"); + validatedHttpsUrl(tokenEndpoint, "Token endpoint", policy); const res = await fetchResponse(tokenEndpoint, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded" }, @@ -406,9 +444,10 @@ function toCredentials( export function createMcpOAuthProvider(config: McpOAuthConfig): OAuthProviderInterface { const label = config.label ?? config.server; + const policy: UrlPolicy = { allowPrivateNetwork: config.allowPrivateNetwork === true }; async function login(callbacks: OAuthLoginCallbacks): Promise { - const discovery = await discover(config.url); + const discovery = await discover(config.url, policy); const { metadata: meta } = discovery; callbacks.onProgress?.(`Discovered ${discovery.issuer ?? meta.issuer}`); @@ -421,7 +460,7 @@ export function createMcpOAuthProvider(config: McpOAuthConfig): OAuthProviderInt ); } callbacks.onProgress?.("Registering OAuth client…"); - clientId = await registerClient(meta.registration_endpoint, `Prime Agent (${label})`); + clientId = await registerClient(meta.registration_endpoint, `Prime Agent (${label})`, policy); } const { verifier, challenge } = await generatePKCE(); @@ -502,14 +541,18 @@ export function createMcpOAuthProvider(config: McpOAuthConfig): OAuthProviderInt } callbacks.onProgress?.("Exchanging authorization code for tokens…"); - const token = await exchangeToken(meta.token_endpoint, { - grant_type: "authorization_code", - code: result.code, - redirect_uri: cb.redirectUri, - client_id: clientId, - code_verifier: verifier, - ...(discovery.resource ? { resource: discovery.resource } : {}), - }); + const token = await exchangeToken( + meta.token_endpoint, + { + grant_type: "authorization_code", + code: result.code, + redirect_uri: cb.redirectUri, + client_id: clientId, + code_verifier: verifier, + ...(discovery.resource ? { resource: discovery.resource } : {}), + }, + policy, + ); return toCredentials(token, meta.token_endpoint, clientId, config.url, discovery.resource, discovery.issuer); } finally { cb.server.close(); @@ -521,7 +564,7 @@ export function createMcpOAuthProvider(config: McpOAuthConfig): OAuthProviderInt if (creds.endpoint !== config.url) { throw new Error(`Stored OAuth credentials are not bound to ${config.url}; re-run /mcp login ${config.server}`); } - const configuredResource = canonicalResource(validatedHttpsUrl(config.url, "MCP endpoint")); + const configuredResource = canonicalResource(validatedHttpsUrl(config.url, "MCP endpoint", policy)); if (creds.resource !== undefined && creds.resource !== configuredResource) { throw new Error( `Stored OAuth credentials are not bound to ${configuredResource}; re-run /mcp login ${config.server}`, @@ -532,11 +575,11 @@ export function createMcpOAuthProvider(config: McpOAuthConfig): OAuthProviderInt `Stored OAuth credentials for ${label} have incomplete resource binding; re-run /mcp login ${config.server}`, ); } - if (creds.issuer !== undefined) validatedHttpsUrl(creds.issuer, "Stored authorization server issuer"); + if (creds.issuer !== undefined) validatedHttpsUrl(creds.issuer, "Stored authorization server issuer", policy); if (!creds.refresh) { throw new Error(`No refresh token stored for ${label}; re-run /mcp login ${config.server}`); } - const discovery = await discover(config.url); + const discovery = await discover(config.url, policy); if ((creds.resource === undefined) !== (discovery.resource === undefined)) { throw new Error(`OAuth discovery mode changed for ${config.url}; re-run /mcp login ${config.server}`); } @@ -555,12 +598,16 @@ export function createMcpOAuthProvider(config: McpOAuthConfig): OAuthProviderInt } const clientId = creds.clientId ?? config.clientId; if (!tokenEndpoint) throw new Error(`No token endpoint stored for ${label}; re-run /mcp login ${config.server}`); - const token = await exchangeToken(tokenEndpoint, { - grant_type: "refresh_token", - refresh_token: creds.refresh, - ...(clientId ? { client_id: clientId } : {}), - ...(creds.resource ? { resource: creds.resource } : {}), - }); + const token = await exchangeToken( + tokenEndpoint, + { + grant_type: "refresh_token", + refresh_token: creds.refresh, + ...(clientId ? { client_id: clientId } : {}), + ...(creds.resource ? { resource: creds.resource } : {}), + }, + policy, + ); return toCredentials( token, tokenEndpoint, diff --git a/packages/ai/test/mcp-host-policy.test.ts b/packages/ai/test/mcp-host-policy.test.ts new file mode 100644 index 0000000000..8ae2ed51c4 --- /dev/null +++ b/packages/ai/test/mcp-host-policy.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { isPublicHost } from "../src/mcp/host-policy.js"; + +describe("isPublicHost", () => { + it.each([ + "mcp.linear.app", + "srv.test", + "login.example", + "8.8.8.8", + "[2606:4700::1111]", + "[2002:808:808::1]", // 6to4 embedding a public IPv4 + "example.local.example.com", + ])("accepts public host %s", (host) => { + expect(isPublicHost(host)).toBe(true); + }); + + it.each([ + "", + "localhost", + "LOCALHOST", + "api.localhost", + "printer.local", + "vault.internal", + "nas.home.arpa", + "intranet", + "localhost.", + "127.0.0.1", + "127.255.255.254", + "0.0.0.0", + "10.0.0.7", + "100.64.1.1", + "169.254.169.254", + "172.16.0.1", + "172.31.255.255", + "192.0.0.1", + "192.0.2.1", + "192.88.99.1", + "192.168.1.1", + "198.18.0.1", + "198.51.100.1", + "203.0.113.1", + "224.0.0.1", + "255.255.255.255", + "[::]", + "[::1]", + "[::ffff:7f00:1]", + "[::ffff:127.0.0.1]", + "[::ffff:a00:7]", + "[::a00:7]", + "[64:ff9b::a00:7]", + "[fc00::1]", + "[fd12:3456::1]", + "[fe80::1]", + "[fe80::1%25eth0]", + "[fec0::1]", + "[ff02::1]", + "[2001:db8::1]", + "[2001::1]", + "[2002:7f00:1::1]", // 6to4 embedding loopback + ])("rejects non-public host %s", (host) => { + expect(isPublicHost(host)).toBe(false); + }); + + it("matches the hostnames the URL parser produces for obfuscated literals", () => { + for (const raw of ["https://2130706433/", "https://0x7f.1/", "https://0177.0.0.1/", "https://[0:0::1]/"]) { + expect(isPublicHost(new URL(raw).hostname)).toBe(false); + } + }); +}); diff --git a/packages/ai/test/mcp-oauth.test.ts b/packages/ai/test/mcp-oauth.test.ts index e35f2a009d..bf3f6232cb 100644 --- a/packages/ai/test/mcp-oauth.test.ts +++ b/packages/ai/test/mcp-oauth.test.ts @@ -445,3 +445,173 @@ describe.sequential("MCP OAuth provider", () => { ).rejects.toThrow("dynamic client registration"); }); }); + +describe.sequential("MCP OAuth provider private-network policy (ENG-5345)", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const meta = (host: string) => ({ + issuer: host, + authorization_endpoint: `${host}/authorize`, + token_endpoint: `${host}/token`, + registration_endpoint: `${host}/register`, + }); + + function stubRoutes(routes: Record Response>): string[] { + const requested: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (input: unknown): Promise => { + const url = urlOf(input); + requested.push(url); + const route = routes[url]; + return route ? route() : new Response("", { status: 404 }); + }), + ); + return requested; + } + + async function attemptLogin( + config: Parameters[0], + ): Promise<{ authUrl?: string; error?: string }> { + let authUrl: string | undefined; + try { + await createMcpOAuthProvider(config).login({ + onAuth: (info) => { + authUrl = info.url; + }, + onPrompt: async () => "", + onManualCodeInput: async () => { + throw new Error("cancelled by test"); + }, + }); + } catch (error) { + return { authUrl, error: error instanceof Error ? error.message : String(error) }; + } + return { authUrl }; + } + + it.each([ + "https://127.0.0.1/mcp", + "https://[::1]/mcp", + "https://localhost/mcp", + "https://mcp.internal/mcp", + "https://mcp.local/mcp", + "https://10.0.0.7/mcp", + "https://169.254.169.254/mcp", + "https://2130706433/mcp", + "https://intranet/mcp", + ])("rejects the non-public MCP endpoint %s before any request", async (url) => { + const requested = stubRoutes({}); + const result = await attemptLogin({ server: "t", url }); + expect(result.error).toMatch(/MCP endpoint .* private, loopback or local-network host/); + expect(result.authUrl).toBeUndefined(); + expect(requested).toEqual([]); + }); + + it("rejects protected-resource metadata naming a private authorization server", async () => { + const requested = stubRoutes({ + "https://srv.test/mcp": () => new Response("", { status: 401 }), + "https://srv.test/.well-known/oauth-protected-resource/mcp": () => + jsonResponse({ resource: "https://srv.test/mcp", authorization_servers: ["https://10.0.0.7"] }), + "https://10.0.0.7/.well-known/oauth-authorization-server": () => jsonResponse(meta("https://10.0.0.7")), + }); + const result = await attemptLogin({ server: "t", url: "https://srv.test/mcp" }); + expect(result.error).toMatch(/Authorization server issuer https:\/\/10\.0\.0\.7 points at a private/); + expect(requested).not.toContain("https://10.0.0.7/.well-known/oauth-authorization-server"); + }); + + it("rejects a WWW-Authenticate resource_metadata pointer on a private host", async () => { + const requested = stubRoutes({ + "https://srv.test/mcp": () => + new Response("", { + status: 401, + headers: { "WWW-Authenticate": 'Bearer resource_metadata="https://192.168.1.20/prm"' }, + }), + "https://192.168.1.20/prm": () => + jsonResponse({ resource: "https://srv.test/mcp", authorization_servers: ["https://auth.example.org"] }), + }); + const result = await attemptLogin({ server: "t", url: "https://srv.test/mcp" }); + expect(result.error).toMatch(/resource_metadata https:\/\/192\.168\.1\.20 points at a private/); + expect(requested).not.toContain("https://192.168.1.20/prm"); + }); + + it.each(["authorization_endpoint", "token_endpoint", "registration_endpoint"] as const)( + "rejects authorization-server metadata whose %s is on a private host", + async (field) => { + const requested = stubRoutes({ + "https://srv.test/mcp": () => new Response("", { status: 401 }), + "https://srv.test/.well-known/oauth-protected-resource/mcp": () => + jsonResponse({ resource: "https://srv.test/mcp", authorization_servers: ["https://auth.example.org"] }), + "https://auth.example.org/.well-known/oauth-authorization-server": () => + jsonResponse({ ...meta("https://auth.example.org"), [field]: "https://[fd00::1]/private" }), + }); + const result = await attemptLogin({ server: "t", url: "https://srv.test/mcp" }); + expect(result.error).toMatch(/https:\/\/\[fd00::1\] points at a private/); + expect(result.authUrl).toBeUndefined(); + expect(requested).not.toContain("https://[fd00::1]/private"); + }, + ); + + it("rejects an origin-fallback metadata document that moves the endpoints to a private host", async () => { + const requested = stubRoutes({ + "https://srv.test/.well-known/oauth-authorization-server": () => + jsonResponse({ ...meta("https://srv.test"), token_endpoint: "https://127.0.0.1:8443/token" }), + }); + const result = await attemptLogin({ server: "t", url: "https://srv.test/mcp" }); + expect(result.error).toMatch(/Token endpoint https:\/\/127\.0\.0\.1:8443 points at a private/); + expect(requested).not.toContain("https://127.0.0.1:8443/token"); + }); + + it("keeps a legitimate separate authorization server on another public origin", async () => { + const requested = stubRoutes({ + "https://srv.test/mcp": () => new Response("", { status: 401 }), + "https://srv.test/.well-known/oauth-protected-resource/mcp": () => + jsonResponse({ resource: "https://srv.test/mcp", authorization_servers: ["https://auth.example.org"] }), + "https://auth.example.org/.well-known/oauth-authorization-server": () => + jsonResponse(meta("https://auth.example.org")), + "https://auth.example.org/register": () => jsonResponse({ client_id: "client-1" }), + }); + const result = await attemptLogin({ server: "t", url: "https://srv.test/mcp" }); + expect(result.authUrl).toMatch(/^https:\/\/auth\.example\.org\/authorize\?/); + expect(new URL(result.authUrl ?? "").searchParams.get("resource")).toBe("https://srv.test/mcp"); + expect(requested).toContain("https://auth.example.org/register"); + }); + + it("allows a private MCP endpoint and authorization server only with allowPrivateNetwork", async () => { + const routes = { + "https://127.0.0.1/mcp": () => new Response("", { status: 401 }), + "https://127.0.0.1/.well-known/oauth-protected-resource/mcp": () => + jsonResponse({ resource: "https://127.0.0.1/mcp", authorization_servers: ["https://10.0.0.7"] }), + "https://10.0.0.7/.well-known/oauth-authorization-server": () => jsonResponse(meta("https://10.0.0.7")), + "https://10.0.0.7/register": () => jsonResponse({ client_id: "client-2" }), + }; + stubRoutes(routes); + const denied = await attemptLogin({ server: "t", url: "https://127.0.0.1/mcp" }); + expect(denied.authUrl).toBeUndefined(); + expect(denied.error).toMatch(/allowPrivateNetwork/); + + stubRoutes(routes); + const allowed = await attemptLogin({ server: "t", url: "https://127.0.0.1/mcp", allowPrivateNetwork: true }); + expect(allowed.authUrl).toMatch(/^https:\/\/10\.0\.0\.7\/authorize\?/); + expect(new URL(allowed.authUrl ?? "").searchParams.get("resource")).toBe("https://127.0.0.1/mcp"); + }); + + it("refuses to refresh with stored credentials whose issuer is private", async () => { + const requested = stubRoutes({}); + const provider = createMcpOAuthProvider({ server: "t", url: "https://srv.test/mcp" }); + await expect( + provider.refreshToken({ + access: "a", + refresh: "r", + expires: 0, + endpoint: "https://srv.test/mcp", + resource: "https://srv.test/mcp", + issuer: "https://10.0.0.7", + tokenEndpoint: "https://10.0.0.7/token", + } as never), + ).rejects.toThrow(/Stored authorization server issuer https:\/\/10\.0\.0\.7 points at a private/); + expect(requested).toEqual([]); + }); +}); diff --git a/packages/coding-agent/.changes/eng-5345-oauth-discovery-hardening.md b/packages/coding-agent/.changes/eng-5345-oauth-discovery-hardening.md new file mode 100644 index 0000000000..fe2a6d0914 --- /dev/null +++ b/packages/coding-agent/.changes/eng-5345-oauth-discovery-hardening.md @@ -0,0 +1,2 @@ +- Added `allowPrivateNetwork` (`mcp add --allow-private-network`) for OAuth MCP servers on private or loopback addresses, which discovery now rejects by default. +- Fixed the login dialog launching the system browser opener with non-http(s) or control-character sign-in links; such links are now shown as text with a message instead. diff --git a/packages/coding-agent/docs/mcp-integrations.md b/packages/coding-agent/docs/mcp-integrations.md index 4991cecab5..2f4a3512e4 100644 --- a/packages/coding-agent/docs/mcp-integrations.md +++ b/packages/coding-agent/docs/mcp-integrations.md @@ -85,7 +85,13 @@ prime-agent mcp remove remote Use the same forms after `/mcp` in the TUI. Add `--oauth` for the existing OAuth login flow and then use `/mcp login `; use `--force` to replace a complete -existing entry. Static secret values are not accepted: bearer and stdio secrets +existing entry. OAuth discovery refuses loopback, private (RFC 1918), link-local +and local-network destinations (`127.0.0.1`, `10.x`, `localhost`, `*.local`, +`*.internal`, ...) for the MCP endpoint and for every authorization-server, +metadata, registration, authorization and token URL it discovers, so a hostile +server cannot steer the login at internal services. For a genuinely internal +OAuth-protected server, add `--allow-private-network` (settings key +`allowPrivateNetwork: true`) to that one entry. Static secret values are not accepted: bearer and stdio secrets are environment-variable references. Project `.prime/agent/settings.json` MCP entries are ignored for execution, so a repository cannot start a local process or shadow a user server. @@ -132,7 +138,9 @@ result = await mcp.call_tool("remote", "search", {"query": "example"}) ``` HTTP servers may be anonymous, use static `headers`, use a token named by -`bearerTokenEnvVar`, or opt into the existing OAuth login with `oauth: true`. +`bearerTokenEnvVar`, or opt into the existing OAuth login with `oauth: true` +(add `allowPrivateNetwork: true` only for an internal OAuth server whose +endpoints live on private or loopback addresses). For stdio, `command` and `args` are executed directly without a shell. `env` accepts only tagged references to existing environment variables; literal secrets are not supported. The runtime passes a small ambient environment plus diff --git a/packages/coding-agent/src/cli/command-registry.ts b/packages/coding-agent/src/cli/command-registry.ts index e1c37e126c..79faa8bc0c 100644 --- a/packages/coding-agent/src/cli/command-registry.ts +++ b/packages/coding-agent/src/cli/command-registry.ts @@ -99,7 +99,7 @@ export const COMMAND_SPECS: readonly CommandSpec[] = [ }, { path: ["mcp", "add"], - usage: "mcp add --url [--bearer-token-env-var |--oauth] [--force]", + usage: "mcp add --url [--bearer-token-env-var |--oauth [--allow-private-network]] [--force]", summary: "Add an HTTP or stdio MCP server", description: "For stdio, use: mcp add [--cwd ] [--env CHILD=SOURCE] -- [args...]", }, diff --git a/packages/coding-agent/src/core/mcp/mcp-command.ts b/packages/coding-agent/src/core/mcp/mcp-command.ts index 51c2d88e37..b76518f4b2 100644 --- a/packages/coding-agent/src/core/mcp/mcp-command.ts +++ b/packages/coding-agent/src/core/mcp/mcp-command.ts @@ -102,6 +102,7 @@ export function parseMcpAddArgs(args: readonly string[]): { let url: string | undefined; let bearerTokenEnvVar: string | undefined; let oauth = false; + let allowPrivateNetwork = false; let force = false; let cwd: string | undefined; const env: Record = Object.create(null); @@ -111,8 +112,9 @@ export function parseMcpAddArgs(args: readonly string[]): { const option = optionArgs[index]!; if (option !== "--env" && seenOptions.has(option)) throw new Error(`Duplicate MCP add option: ${option}`); seenOptions.add(option); - if (option === "--oauth" || option === "--force") { + if (option === "--oauth" || option === "--force" || option === "--allow-private-network") { if (option === "--oauth") oauth = true; + else if (option === "--allow-private-network") allowPrivateNetwork = true; else force = true; continue; } @@ -137,7 +139,9 @@ export function parseMcpAddArgs(args: readonly string[]): { } if (separator !== -1) { - if (url || bearerTokenEnvVar || oauth) throw new Error("Stdio MCP servers cannot use HTTP options."); + if (url || bearerTokenEnvVar || oauth || allowPrivateNetwork) { + throw new Error("Stdio MCP servers cannot use HTTP options."); + } if (commandArgs.length === 0 || !commandArgs[0]?.trim()) { throw new Error("A command is required after --."); } @@ -158,6 +162,7 @@ export function parseMcpAddArgs(args: readonly string[]): { if (cwd || Object.keys(env).length > 0) throw new Error("--cwd and --env require a stdio command after --."); if (!url) throw new Error("Use --url for HTTP or -- [args...] for stdio."); if (bearerTokenEnvVar && oauth) throw new Error("--oauth and --bearer-token-env-var cannot be combined."); + if (allowPrivateNetwork && !oauth) throw new Error("--allow-private-network requires --oauth."); return { name, force, @@ -166,6 +171,7 @@ export function parseMcpAddArgs(args: readonly string[]): { url: validateHttpUrl(url), ...(bearerTokenEnvVar ? { bearerTokenEnvVar } : {}), ...(oauth ? { oauth: true } : {}), + ...(allowPrivateNetwork ? { allowPrivateNetwork: true } : {}), }, }; } diff --git a/packages/coding-agent/src/core/mcp/mcp-manager.ts b/packages/coding-agent/src/core/mcp/mcp-manager.ts index b924b3c35e..9a694bd90e 100644 --- a/packages/coding-agent/src/core/mcp/mcp-manager.ts +++ b/packages/coding-agent/src/core/mcp/mcp-manager.ts @@ -133,6 +133,7 @@ export class McpManager { server: integration.server, label: integration.label, url: integration.config.url, + allowPrivateNetwork: integration.config.allowPrivateNetwork === true, }), ); } diff --git a/packages/coding-agent/src/core/settings-manager.ts b/packages/coding-agent/src/core/settings-manager.ts index 1fb87cbdb4..540d00384e 100644 --- a/packages/coding-agent/src/core/settings-manager.ts +++ b/packages/coding-agent/src/core/settings-manager.ts @@ -107,6 +107,11 @@ export type McpServerConfig = bearerTokenEnvVar?: string; /** Use the generic OAuth login flow for this server. */ oauth?: boolean; + /** + * Allow the OAuth flow to contact loopback, private (RFC 1918), link-local and + * local-network hosts (`localhost`, `*.local`, `*.internal`). Off by default. + */ + allowPrivateNetwork?: boolean; /** Force-disable even when credentials exist. */ enabled?: boolean; enabledTools?: string[]; diff --git a/packages/coding-agent/src/modes/interactive/components/login-dialog.ts b/packages/coding-agent/src/modes/interactive/components/login-dialog.ts index c3a20aaca8..657c3f8070 100644 --- a/packages/coding-agent/src/modes/interactive/components/login-dialog.ts +++ b/packages/coding-agent/src/modes/interactive/components/login-dialog.ts @@ -1,4 +1,3 @@ -import { win32 } from "node:path"; import { getOAuthProviders } from "@earendil-works/pi-ai/oauth"; import { type Component, @@ -13,7 +12,7 @@ import { visibleWidth, } from "@earendil-works/pi-tui"; import { PRIME_BUTTERFLY_LOGO } from "../../../themes/prime-logo.js"; -import { execFileHidden } from "../../../utils/child-process.js"; +import { openUrlInBrowser, sanitizeUrlForDisplay, validateBrowserUrl } from "../../../utils/browser-url.js"; import { copyToClipboard } from "../../../utils/clipboard.js"; import { theme } from "../theme/theme.js"; import { formatKeyText, keyHint } from "./keybinding-hints.js"; @@ -152,17 +151,31 @@ export class LoginDialogComponent extends Container implements Focusable { } /** - * Called by onAuth callback - show URL and optional instructions + * Called by onAuth callback - show URL and optional instructions. Only an + * http(s) URL is opened or rendered as a hyperlink; anything else is shown as + * sanitized text so a hostile string can never reach the OS opener or terminal. */ showAuth(url: string, instructions?: string): void { this.startContent(); - this.authUrl = url; + const browserUrl = validateBrowserUrl(url); + this.authUrl = browserUrl; this.addSectionTitle("Browser sign-in"); - this.addMutedText("The sign-in page should already be opening. If it did not open, use the link below."); - this.contentContainer.addChild(new Spacer(1)); - this.addLabel("Sign-in link"); - const linkedUrl = getCapabilities().hyperlinks ? `\x1b]8;;${url}\x07${url}\x1b]8;;\x07` : url; - this.contentContainer.addChild(new Text(theme.fg("text", linkedUrl), 0, 0)); + if (browserUrl) { + this.addMutedText("The sign-in page should already be opening. If it did not open, use the link below."); + this.contentContainer.addChild(new Spacer(1)); + this.addLabel("Sign-in link"); + const linkedUrl = getCapabilities().hyperlinks + ? `\x1b]8;;${browserUrl}\x07${browserUrl}\x1b]8;;\x07` + : browserUrl; + this.contentContainer.addChild(new Text(theme.fg("text", linkedUrl), 0, 0)); + } else { + this.addMutedText( + "The sign-in link was not opened because it is not a valid http(s) URL. Check the provider configuration.", + ); + this.contentContainer.addChild(new Spacer(1)); + this.addLabel("Sign-in link (not opened)"); + this.contentContainer.addChild(new Text(theme.fg("text", sanitizeUrlForDisplay(url)), 0, 0)); + } this.authActions = new Text(this.getAuthActionsText(), 0, 0); this.contentContainer.addChild(this.authActions); @@ -171,18 +184,7 @@ export class LoginDialogComponent extends Container implements Focusable { this.addInstructions(instructions); } - // Try to open browser - const [command, ...args] = - process.platform === "darwin" - ? ["open", url] - : process.platform === "win32" - ? [ - win32.join(process.env.SystemRoot ?? "C:\\Windows", "System32", "rundll32.exe"), - "url.dll,FileProtocolHandler", - url, - ] - : ["xdg-open", url]; - execFileHidden(command, args, {}, () => {}); + if (browserUrl) openUrlInBrowser(browserUrl); this.tui.requestRender(); } @@ -349,7 +351,7 @@ export class LoginDialogComponent extends Container implements Focusable { ? configuredCopyKeys.filter((key) => !isTextEntryKeybinding(key)) : configuredCopyKeys.slice(0, 1); const copyHint = - copyKeys.length > 0 + this.authUrl && copyKeys.length > 0 ? theme.fg("dim", formatKeyText(copyKeys.join("/"))) + theme.fg("muted", ` ${status === "failed" ? "retry" : "copy"}`) : undefined; diff --git a/packages/coding-agent/src/utils/browser-url.ts b/packages/coding-agent/src/utils/browser-url.ts new file mode 100644 index 0000000000..a4aedcd666 --- /dev/null +++ b/packages/coding-agent/src/utils/browser-url.ts @@ -0,0 +1,47 @@ +import { win32 } from "node:path"; +import { execFileHidden } from "./child-process.js"; + +const MAX_BROWSER_URL_LENGTH = 8192; +const CONTROL_CHARACTERS = /[\u0000-\u001f\u007f-\u009f]/; + +/** + * Returns the URL unchanged when it is safe to hand to the OS opener: an absolute + * http(s) URL without embedded credentials or control characters, at most 8 KiB. + * Anything else (`file:`, `javascript:`, escape-sequence payloads) yields undefined. + */ +export function validateBrowserUrl(value: string): string | undefined { + if (value.length > MAX_BROWSER_URL_LENGTH || CONTROL_CHARACTERS.test(value)) return undefined; + let url: URL; + try { + url = new URL(value); + } catch { + return undefined; + } + if (url.protocol !== "http:" && url.protocol !== "https:") return undefined; + if (url.username || url.password || !url.hostname) return undefined; + return value; +} + +/** Text-safe rendering of a string that failed validateBrowserUrl: control characters removed, length capped. */ +export function sanitizeUrlForDisplay(value: string): string { + const stripped = value.replace(/[\u0000-\u001f\u007f-\u009f]/g, ""); + return stripped.length > 512 ? `${stripped.slice(0, 512)}…` : stripped; +} + +/** Open a URL with the platform browser opener. Returns false without launching anything when the URL is rejected. */ +export function openUrlInBrowser(value: string): boolean { + const url = validateBrowserUrl(value); + if (!url) return false; + const [command, ...args] = + process.platform === "darwin" + ? ["open", url] + : process.platform === "win32" + ? [ + win32.join(process.env.SystemRoot ?? "C:\\Windows", "System32", "rundll32.exe"), + "url.dll,FileProtocolHandler", + url, + ] + : ["xdg-open", url]; + execFileHidden(command, args, {}, () => {}); + return true; +} diff --git a/packages/coding-agent/test/browser-url.test.ts b/packages/coding-agent/test/browser-url.test.ts new file mode 100644 index 0000000000..9ab8931fba --- /dev/null +++ b/packages/coding-agent/test/browser-url.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ execFile: vi.fn() })); +vi.mock("child_process", () => ({ execFile: mocks.execFile })); + +import { openUrlInBrowser, sanitizeUrlForDisplay, validateBrowserUrl } from "../src/utils/browser-url.js"; + +describe("validateBrowserUrl", () => { + it.each([ + "https://auth.example.org/authorize?x=1&state=abc", + "http://localhost:53700/callback?code=1", + "https://example.com/oauth?state=$(touch /tmp/pwned);whoami&pipe=|id", + ])("accepts the http(s) URL %s unchanged", (url) => { + expect(validateBrowserUrl(url)).toBe(url); + }); + + it.each([ + ["file URL", "file:///etc/passwd"], + ["javascript URL", "javascript:alert(1)"], + ["data URL", "data:text/html,hi"], + ["custom scheme", "ms-settings:windowsupdate"], + ["embedded credentials", "https://user:secret@example.com/"], + ["ESC in path", "https://x.test/\x1b]52;c;QUFB\x07"], + ["newline", "https://x.test/\npath"], + ["C1 control", "https://x.test/\u0085path"], + ["relative path", "/etc/passwd"], + ["not a URL", "open me"], + ["empty", ""], + ["over the length cap", `https://x.test/${"a".repeat(8200)}`], + ])("rejects %s", (_label, url) => { + expect(validateBrowserUrl(url)).toBeUndefined(); + }); +}); + +describe("sanitizeUrlForDisplay", () => { + it("strips control characters and caps the length", () => { + expect(sanitizeUrlForDisplay("https://x.test/\x1b]52;c;QUFB\x07end")).toBe("https://x.test/]52;c;QUFBend"); + expect(sanitizeUrlForDisplay(`https://x.test/${"a".repeat(600)}`)).toHaveLength(513); + }); +}); + +describe("openUrlInBrowser", () => { + it("launches the platform opener only for validated URLs", () => { + mocks.execFile.mockClear(); + expect(openUrlInBrowser("file:///etc/passwd")).toBe(false); + expect(openUrlInBrowser("javascript:alert(1)")).toBe(false); + expect(mocks.execFile).not.toHaveBeenCalled(); + expect(openUrlInBrowser("https://auth.example.org/authorize")).toBe(true); + expect(mocks.execFile).toHaveBeenCalledTimes(1); + expect(mocks.execFile.mock.calls[0]?.[1]).toContain("https://auth.example.org/authorize"); + }); +}); diff --git a/packages/coding-agent/test/login-dialog.test.ts b/packages/coding-agent/test/login-dialog.test.ts index 28a55889ab..c48b8743a0 100644 --- a/packages/coding-agent/test/login-dialog.test.ts +++ b/packages/coding-agent/test/login-dialog.test.ts @@ -119,6 +119,39 @@ describe("LoginDialogComponent", () => { } }); + it.each([ + ["file URL", "file:///etc/passwd"], + ["javascript URL", "javascript:alert(1)"], + ["URL with embedded credentials", "https://user:secret@example.com/oauth"], + ["URL carrying an OSC 52 payload", `https://x.test/${"\x1b"}]52;c;QUFB${"\x07"}`], + ["over-long URL", `https://example.com/${"a".repeat(9000)}`], + ])("does not open or hyperlink a %s and shows it as text instead", (_label, url) => { + setCapabilities({ images: null, trueColor: true, hyperlinks: true }); + const dialog = new LoginDialogComponent(createFakeTui(), "anthropic", () => {}, "Anthropic"); + + dialog.showAuth(url, "Complete login in your browser."); + const rawOutput = dialog.render(120).join("\n"); + const output = stripAnsi(rawOutput); + + expect(mocks.execFile).not.toHaveBeenCalled(); + expect(rawOutput).not.toContain("\x1b]8;;"); + expect(rawOutput).not.toContain("\x1b]52"); + expect(output).toContain("Sign-in link (not opened)"); + expect(output).toContain("not a valid http(s) URL"); + expect(output).not.toContain("should already be opening"); + expect(output).not.toContain("copy"); + + dialog.handleInput("c"); + expect(mocks.copyToClipboard).not.toHaveBeenCalled(); + }); + + it("still opens plain http URLs such as a loopback callback page", () => { + const dialog = new LoginDialogComponent(createFakeTui(), "anthropic", () => {}, "Anthropic"); + dialog.showAuth("http://localhost:53700/start"); + expect(mocks.execFile).toHaveBeenCalledTimes(1); + expect(mocks.execFile.mock.calls[0]?.[1]).toContain("http://localhost:53700/start"); + }); + it("renders sign-in URLs as OSC 8 hyperlinks when supported", () => { setCapabilities({ images: null, trueColor: true, hyperlinks: true }); const dialog = new LoginDialogComponent(createFakeTui(), "anthropic", () => {}, "Anthropic"); diff --git a/packages/coding-agent/test/mcp-command.test.ts b/packages/coding-agent/test/mcp-command.test.ts index 81f19581b5..c17a15a209 100644 --- a/packages/coding-agent/test/mcp-command.test.ts +++ b/packages/coding-agent/test/mcp-command.test.ts @@ -63,6 +63,23 @@ describe("MCP management commands", () => { } }); + it("records --allow-private-network only for OAuth HTTP servers", () => { + expect( + parseMcpAddArgs(["internal", "--url", "https://mcp.internal/mcp", "--oauth", "--allow-private-network"]) + .config, + ).toEqual({ type: "http", url: "https://mcp.internal/mcp", oauth: true, allowPrivateNetwork: true }); + expect(parseMcpAddArgs(["remote", "--url", "https://example.com/mcp", "--oauth"]).config).not.toHaveProperty( + "allowPrivateNetwork", + ); + for (const args of [ + ["remote", "--url", "https://example.com/mcp", "--allow-private-network"], + ["remote", "--url", "https://example.com/mcp", "--bearer-token-env-var", "TOKEN", "--allow-private-network"], + ["local", "--allow-private-network", "--", "node"], + ] as string[][]) { + expect(() => parseMcpAddArgs(args)).toThrow(); + } + }); + it("shows only the server name and transport at the public output boundary", () => { const output = formatMcpServer("remote", { type: "http", diff --git a/packages/coding-agent/test/mcp-manager.test.ts b/packages/coding-agent/test/mcp-manager.test.ts index fdc83c64b9..1a21ccd569 100644 --- a/packages/coding-agent/test/mcp-manager.test.ts +++ b/packages/coding-agent/test/mcp-manager.test.ts @@ -72,6 +72,41 @@ describe("McpManager", () => { expect(getOAuthProvider("mcp:acme")).toBeDefined(); }); + it("passes allowPrivateNetwork through to the user server's OAuth provider", async () => { + const requested: string[] = []; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (input: unknown): Promise => { + requested.push(typeof input === "string" ? input : String(input)); + return new Response("", { status: 404 }); + }) as typeof fetch; + try { + const attempt = async (config: McpServerConfig): Promise => { + resetOAuthProviders(); + new McpManager({ authStorage, getUserServers: () => ({ internal: config }) }); + const provider = getOAuthProvider("mcp:internal"); + if (!provider) throw new Error("provider not registered"); + return provider.login({ onAuth: () => {}, onPrompt: async () => "" }).then( + () => "unexpected success", + (error: Error) => error.message, + ); + }; + const denied = await attempt({ type: "http", url: "https://10.0.0.7/mcp", oauth: true }); + expect(denied).toMatch(/private, loopback or local-network host/); + expect(requested).toEqual([]); + + const allowed = await attempt({ + type: "http", + url: "https://10.0.0.7/mcp", + oauth: true, + allowPrivateNetwork: true, + }); + expect(allowed).toMatch(/Could not discover OAuth metadata/); + expect(requested).toContain("https://10.0.0.7/mcp"); + } finally { + globalThis.fetch = originalFetch; + } + }); + it("exposes only mcp.refresh when no interactive login is wired", async () => { const manager = new McpManager({ authStorage }); const handlers = manager.hostHandlers();