fix(oauth): harden browser launch handling
This commit is contained in:
@@ -132,10 +132,22 @@ async function startDeviceFlow(domain: string): Promise<DeviceCodeResponse> {
|
|||||||
throw new Error("Invalid device code response fields");
|
throw new Error("Invalid device code response fields");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The verification URI is opened in the user's browser and to prevent `open` from
|
||||||
|
// opening an executable or similar, we force it to be a URL.
|
||||||
|
let parsedUri: URL;
|
||||||
|
try {
|
||||||
|
parsedUri = new URL(verificationUri);
|
||||||
|
} catch {
|
||||||
|
throw new Error("Untrusted verification_uri in device code response");
|
||||||
|
}
|
||||||
|
if (parsedUri.protocol !== "https:" && parsedUri.protocol !== "http:") {
|
||||||
|
throw new Error("Untrusted verification_uri in device code response");
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
device_code: deviceCode,
|
device_code: deviceCode,
|
||||||
user_code: userCode,
|
user_code: userCode,
|
||||||
verification_uri: verificationUri,
|
verification_uri: parsedUri.href,
|
||||||
interval,
|
interval,
|
||||||
expires_in: expiresIn,
|
expires_in: expiresIn,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -28,7 +28,6 @@ import type {
|
|||||||
OAuthProviderInterface,
|
OAuthProviderInterface,
|
||||||
} from "./types.ts";
|
} from "./types.ts";
|
||||||
|
|
||||||
const CALLBACK_HOST = process.env.PI_OAUTH_CALLBACK_HOST || "127.0.0.1";
|
|
||||||
const CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann";
|
const CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann";
|
||||||
const AUTH_BASE_URL = "https://auth.openai.com";
|
const AUTH_BASE_URL = "https://auth.openai.com";
|
||||||
const AUTHORIZE_URL = `${AUTH_BASE_URL}/oauth/authorize`;
|
const AUTHORIZE_URL = `${AUTH_BASE_URL}/oauth/authorize`;
|
||||||
@@ -47,6 +46,10 @@ const JWT_CLAIM_PATH = "https://api.openai.com/auth";
|
|||||||
type OAuthToken = { access: string; refresh: string; expires: number };
|
type OAuthToken = { access: string; refresh: string; expires: number };
|
||||||
type TokenOperation = "exchange" | "refresh";
|
type TokenOperation = "exchange" | "refresh";
|
||||||
|
|
||||||
|
function getCallbackHost(): string {
|
||||||
|
return typeof process !== "undefined" ? process.env.PI_OAUTH_CALLBACK_HOST || "127.0.0.1" : "127.0.0.1";
|
||||||
|
}
|
||||||
|
|
||||||
type DeviceAuthInfo = {
|
type DeviceAuthInfo = {
|
||||||
deviceAuthId: string;
|
deviceAuthId: string;
|
||||||
userCode: string;
|
userCode: string;
|
||||||
@@ -368,7 +371,7 @@ function startLocalOAuthServer(state: string): Promise<OAuthServerInfo> {
|
|||||||
|
|
||||||
return new Promise((resolve) => {
|
return new Promise((resolve) => {
|
||||||
server
|
server
|
||||||
.listen(1455, CALLBACK_HOST, () => {
|
.listen(1455, getCallbackHost(), () => {
|
||||||
resolve({
|
resolve({
|
||||||
close: () => server.close(),
|
close: () => server.close(),
|
||||||
cancelWait: () => {
|
cancelWait: () => {
|
||||||
|
|||||||
@@ -84,6 +84,97 @@ describe("GitHub Copilot OAuth device flow", () => {
|
|||||||
await loginPromise;
|
await loginPromise;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("rejects a non-http(s) verification_uri before it reaches onDeviceCode", async () => {
|
||||||
|
// A malicious enterprise OAuth server could return a verification_uri that
|
||||||
|
// the browser launcher would otherwise hand to the OS. Ensure such values
|
||||||
|
// are rejected at the deserialization boundary.
|
||||||
|
const fetchMock = vi.fn(async (input: unknown): Promise<Response> => {
|
||||||
|
const url = getUrl(input);
|
||||||
|
if (url.endsWith("/login/device/code")) {
|
||||||
|
return jsonResponse({
|
||||||
|
device_code: "device-code",
|
||||||
|
user_code: "ABCD-EFGH",
|
||||||
|
verification_uri: "$(id>/tmp/pwned)",
|
||||||
|
interval: 1,
|
||||||
|
expires_in: 900,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
throw new Error(`Unexpected fetch URL: ${url}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.stubGlobal("fetch", fetchMock);
|
||||||
|
|
||||||
|
const onDeviceCode = vi.fn();
|
||||||
|
await expect(
|
||||||
|
loginGitHubCopilot({
|
||||||
|
onDeviceCode,
|
||||||
|
onPrompt: async () => "",
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/Untrusted verification_uri/);
|
||||||
|
expect(onDeviceCode).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes verification_uri before it reaches onDeviceCode", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.setSystemTime(new Date("2026-03-09T00:00:00Z"));
|
||||||
|
|
||||||
|
const rawVerificationUri = "https://github.com/login/\x1b]8;;evil";
|
||||||
|
const normalizedVerificationUri = new URL(rawVerificationUri).href;
|
||||||
|
expect(normalizedVerificationUri).not.toBe(rawVerificationUri);
|
||||||
|
|
||||||
|
const fetchMock = vi.fn(async (input: unknown): Promise<Response> => {
|
||||||
|
const url = getUrl(input);
|
||||||
|
|
||||||
|
if (url.endsWith("/login/device/code")) {
|
||||||
|
return jsonResponse({
|
||||||
|
device_code: "device-code",
|
||||||
|
user_code: "ABCD-EFGH",
|
||||||
|
verification_uri: rawVerificationUri,
|
||||||
|
interval: 1,
|
||||||
|
expires_in: 900,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
if (url.endsWith("/login/oauth/access_token")) {
|
||||||
|
return jsonResponse({ access_token: "ghu_refresh_token" });
|
||||||
|
}
|
||||||
|
|
||||||
|
if (url.includes("/copilot_internal/v2/token")) {
|
||||||
|
return jsonResponse({
|
||||||
|
token: "tid=test;exp=9999999999;proxy-ep=proxy.individual.githubcopilot.com;",
|
||||||
|
expires_at: 9999999999,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
if (url.includes("/models/") && url.endsWith("/policy")) {
|
||||||
|
return new Response("", { status: 200 });
|
||||||
|
}
|
||||||
|
|
||||||
|
throw new Error(`Unexpected fetch URL: ${url}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.stubGlobal("fetch", fetchMock);
|
||||||
|
|
||||||
|
const onDeviceCode = vi.fn();
|
||||||
|
const loginPromise = loginGitHubCopilot({
|
||||||
|
onDeviceCode,
|
||||||
|
onPrompt: async () => "",
|
||||||
|
});
|
||||||
|
|
||||||
|
await vi.advanceTimersByTimeAsync(0);
|
||||||
|
|
||||||
|
expect(onDeviceCode).toHaveBeenCalledWith({
|
||||||
|
userCode: "ABCD-EFGH",
|
||||||
|
verificationUri: normalizedVerificationUri,
|
||||||
|
intervalSeconds: 1,
|
||||||
|
expiresInSeconds: 900,
|
||||||
|
});
|
||||||
|
expect(onDeviceCode).not.toHaveBeenCalledWith(expect.objectContaining({ verificationUri: rawVerificationUri }));
|
||||||
|
|
||||||
|
await vi.advanceTimersByTimeAsync(1000);
|
||||||
|
await loginPromise;
|
||||||
|
});
|
||||||
|
|
||||||
it("polls immediately and increases the interval after slow_down", async () => {
|
it("polls immediately and increases the interval after slow_down", async () => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
const startTime = new Date("2026-03-09T00:00:00Z");
|
const startTime = new Date("2026-03-09T00:00:00Z");
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { getOAuthProviders, type OAuthDeviceCodeInfo } from "@earendil-works/pi-ai/oauth";
|
import { getOAuthProviders, type OAuthDeviceCodeInfo } from "@earendil-works/pi-ai/oauth";
|
||||||
import { Container, type Focusable, getKeybindings, Input, Spacer, Text, type TUI } from "@earendil-works/pi-tui";
|
import { Container, type Focusable, getKeybindings, Input, Spacer, Text, type TUI } from "@earendil-works/pi-tui";
|
||||||
import { exec } from "child_process";
|
import { openBrowser } from "../../../utils/open-browser.ts";
|
||||||
import { theme } from "../theme/theme.ts";
|
import { theme } from "../theme/theme.ts";
|
||||||
import { DynamicBorder } from "./dynamic-border.ts";
|
import { DynamicBorder } from "./dynamic-border.ts";
|
||||||
import { keyHint } from "./keybinding-hints.ts";
|
import { keyHint } from "./keybinding-hints.ts";
|
||||||
@@ -101,7 +101,7 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||||||
this.contentContainer.addChild(new Text(theme.fg("warning", instructions), 1, 0));
|
this.contentContainer.addChild(new Text(theme.fg("warning", instructions), 1, 0));
|
||||||
}
|
}
|
||||||
|
|
||||||
this.openUrl(url);
|
openBrowser(url);
|
||||||
this.tui.requestRender();
|
this.tui.requestRender();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -120,19 +120,10 @@ export class LoginDialogComponent extends Container implements Focusable {
|
|||||||
this.contentContainer.addChild(new Spacer(1));
|
this.contentContainer.addChild(new Spacer(1));
|
||||||
this.contentContainer.addChild(new Text(theme.fg("warning", `Enter code: ${info.userCode}`), 1, 0));
|
this.contentContainer.addChild(new Text(theme.fg("warning", `Enter code: ${info.userCode}`), 1, 0));
|
||||||
|
|
||||||
this.openUrl(info.verificationUri);
|
openBrowser(info.verificationUri);
|
||||||
this.tui.requestRender();
|
this.tui.requestRender();
|
||||||
}
|
}
|
||||||
|
|
||||||
private openUrl(url: string): void {
|
|
||||||
const openCmd = process.platform === "darwin" ? "open" : process.platform === "win32" ? "start" : "xdg-open";
|
|
||||||
try {
|
|
||||||
exec(`${openCmd} "${url}"`, () => {});
|
|
||||||
} catch {
|
|
||||||
// Ignore browser launch failures. The URL remains visible for manual opening/copying.
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Show input for manual code/URL entry (for callback server providers)
|
* Show input for manual code/URL entry (for callback server providers)
|
||||||
*/
|
*/
|
||||||
|
|||||||
24
packages/coding-agent/src/utils/open-browser.ts
Normal file
24
packages/coding-agent/src/utils/open-browser.ts
Normal file
@@ -0,0 +1,24 @@
|
|||||||
|
import { spawn } from "node:child_process";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Open a URL or file in the platform browser/default handler.
|
||||||
|
*
|
||||||
|
* This intentionally never invokes a shell. On Windows, do not use
|
||||||
|
* `cmd /c start`: cmd.exe re-parses metacharacters (&, |, ^, ...) before
|
||||||
|
* `start` runs, which would make attacker-controlled URLs injectable.
|
||||||
|
*/
|
||||||
|
export function openBrowser(target: string): void {
|
||||||
|
const [cmd, args]: [string, string[]] =
|
||||||
|
process.platform === "darwin"
|
||||||
|
? ["open", [target]]
|
||||||
|
: process.platform === "win32"
|
||||||
|
? ["rundll32", ["url.dll,FileProtocolHandler", target]]
|
||||||
|
: ["xdg-open", [target]];
|
||||||
|
|
||||||
|
// spawn reports launcher failures (for example, missing xdg-open) via an
|
||||||
|
// error event. Browser launch is best-effort: callers still present the target
|
||||||
|
// to the user, so keep the launcher failure from becoming a process crash.
|
||||||
|
spawn(cmd, args, { stdio: "ignore", detached: true })
|
||||||
|
.on("error", () => {})
|
||||||
|
.unref();
|
||||||
|
}
|
||||||
@@ -3,7 +3,7 @@
|
|||||||
import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs";
|
import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs";
|
||||||
import { homedir, tmpdir } from "node:os";
|
import { homedir, tmpdir } from "node:os";
|
||||||
import { join, resolve } from "node:path";
|
import { join, resolve } from "node:path";
|
||||||
import { spawn } from "node:child_process";
|
import { openBrowser } from "../packages/coding-agent/src/utils/open-browser.ts";
|
||||||
|
|
||||||
interface TextContent { type: "text"; text: string }
|
interface TextContent { type: "text"; text: string }
|
||||||
interface ImageContent { type: "image"; data: string; mimeType?: string }
|
interface ImageContent { type: "image"; data: string; mimeType?: string }
|
||||||
@@ -229,4 +229,4 @@ const html = `<!doctype html>
|
|||||||
mkdirSync(resolve(output, ".."), { recursive: true });
|
mkdirSync(resolve(output, ".."), { recursive: true });
|
||||||
writeFileSync(output, html);
|
writeFileSync(output, html);
|
||||||
console.log(`Wrote ${output}`);
|
console.log(`Wrote ${output}`);
|
||||||
spawn(process.platform === "darwin" ? "open" : process.platform === "win32" ? "cmd" : "xdg-open", process.platform === "win32" ? ["/c", "start", output] : [output], { detached: true, stdio: "ignore" }).unref();
|
openBrowser(output);
|
||||||
|
|||||||
Reference in New Issue
Block a user