From b45d8bef03e1dfe93416d858d6bfe6f8fa2d182e Mon Sep 17 00:00:00 2001 From: Cristina Poncela Cubeiro <140309543+cristinaponcela@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:41:59 +0100 Subject: [PATCH] fix: ensure return url matches source --- .../coding-agent/src/cli/auth-continue.ts | 44 ++++++++++++++- .../test/credential-print.test.ts | 55 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/cli/auth-continue.ts b/packages/coding-agent/src/cli/auth-continue.ts index ed805d8e9..75ba4155d 100644 --- a/packages/coding-agent/src/cli/auth-continue.ts +++ b/packages/coding-agent/src/cli/auth-continue.ts @@ -100,16 +100,56 @@ function decodeBase64Json(inputValue: string): unknown { return JSON.parse(Buffer.from(padded, "base64").toString("utf8")); } +function parseHttpUrl(value: string, field: string): URL { + let url: URL; + try { + url = new URL(value); + } catch { + throw new AuthContinueError(`${field} must be a URL`); + } + if (url.protocol !== "https:" && url.protocol !== "http:") { + throw new AuthContinueError(`${field} must be an http(s) URL`); + } + return url; +} + +function isLocalhost(hostname: string): boolean { + return hostname === "localhost" || hostname.endsWith(".localhost"); +} + +/** + * Same origin, except that `localhost` and `*.localhost` count as one host. A local service may be + * reached at `localhost` (Node cannot resolve `*.localhost`) while its pages live on a `*.localhost` + * name; both always resolve to this machine. + */ +function isSameServiceOrigin(a: URL, b: URL): boolean { + if (a.origin === b.origin) return true; + return a.protocol === b.protocol && a.port === b.port && isLocalhost(a.hostname) && isLocalhost(b.hostname); +} + +/** + * The return URL is opened in the browser and used as the callback redirect, so it must belong to the + * continuation service; otherwise a payload could open local files and apps or redirect to another site. + */ +function requiredReturnUrl(value: unknown, serviceUrl: string): string { + const returnUrl = parseHttpUrl(requiredString(value, "returnUrl"), "returnUrl"); + if (!isSameServiceOrigin(returnUrl, parseHttpUrl(serviceUrl, "serviceUrl"))) { + throw new AuthContinueError("returnUrl must have the same origin as serviceUrl"); + } + return returnUrl.href; +} + function parseAuthContinuationObject(raw: unknown): AuthContinuationPayload { const object = asObject(raw, "Continuation payload"); if (object.version !== 1) throw new AuthContinueError("version must be 1"); + const serviceUrl = requiredString(object.serviceUrl, "serviceUrl"); const base: AuthContinuationBase = { version: 1, - serviceUrl: requiredString(object.serviceUrl, "serviceUrl"), + serviceUrl, providerId: requiredString(object.providerId, "providerId"), continuationId: requiredString(object.continuationId, "continuationId"), secret: requiredString(object.secret, "secret"), - returnUrl: requiredString(object.returnUrl, "returnUrl"), + returnUrl: requiredReturnUrl(object.returnUrl, serviceUrl), }; const mcpRaw = asObject(object.mcp, "mcp"); const mcpBase: McpContinuationBase = { diff --git a/packages/coding-agent/test/credential-print.test.ts b/packages/coding-agent/test/credential-print.test.ts index b68d531ce..47cc12ee0 100644 --- a/packages/coding-agent/test/credential-print.test.ts +++ b/packages/coding-agent/test/credential-print.test.ts @@ -169,4 +169,59 @@ describe("credential print commands", () => { expect(decodeAuthContinuationPayload(encoded)).toMatchObject(payload); expect(decodeAuthContinuationPayload(`pi auth --continue ${encoded}`)).toMatchObject(payload); }); + + test("accepts a *.localhost returnUrl for a localhost serviceUrl", () => { + const payload = { + version: 1, + serviceUrl: "http://localhost:8787", + providerId: "radius-dev", + continuationId: "cont_123", + secret: "one-time-secret", + returnUrl: "http://radius.localhost:8787/connections?status=oauth-connected", + mcp: { serverUrl: "https://mcp.example/mcp", serverName: "example", authType: "none" }, + }; + const encoded = Buffer.from(JSON.stringify(payload)).toString("base64url"); + expect(decodeAuthContinuationPayload(encoded)).toMatchObject(payload); + }); + + test.each([ + ["localhost on another port", "http://localhost:8787", "http://radius.localhost:9999/x"], + ["localhost with another scheme", "http://localhost:8787", "https://radius.localhost:8787/x"], + ["a non-localhost service", "https://auth.example", "https://radius.localhost/x"], + ["a lookalike localhost host", "http://localhost:8787", "http://evillocalhost:8787/x"], + ])("rejects a localhost returnUrl with %s", (_name, serviceUrl, returnUrl) => { + const encoded = Buffer.from( + JSON.stringify({ + version: 1, + serviceUrl, + providerId: "radius-dev", + continuationId: "cont_123", + secret: "one-time-secret", + returnUrl, + mcp: { serverUrl: "https://mcp.example/mcp", serverName: "example", authType: "none" }, + }), + ).toString("base64url"); + expect(() => decodeAuthContinuationPayload(encoded)).toThrow("same origin as serviceUrl"); + }); + + test.each([ + ["another origin", "https://evil.example/connections", "same origin as serviceUrl"], + ["another port", "https://auth.example:8443/connections", "same origin as serviceUrl"], + ["a file URL", "file:///Applications/Calculator.app", "http(s) URL"], + ["a custom scheme", "vscode://open?url=x", "http(s) URL"], + ["a non-URL", "not a url", "must be a URL"], + ])("rejects a returnUrl with %s", (_name, returnUrl, message) => { + const encoded = Buffer.from( + JSON.stringify({ + version: 1, + serviceUrl: "https://auth.example", + providerId: "auth-service", + continuationId: "cont_123", + secret: "one-time-secret", + returnUrl, + mcp: { serverUrl: "https://mcp.example/mcp", serverName: "example", authType: "none" }, + }), + ).toString("base64url"); + expect(() => decodeAuthContinuationPayload(encoded)).toThrow(message); + }); });