Merge remote-tracking branch 'origin/main' into approvals

packages/ai/src/utils/oauth/github-copilot.ts

@@ -132,10 +132,22 @@ async function startDeviceFlow(domain: string): Promise<DeviceCodeResponse> {
 		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 {
 		device_code: deviceCode,
 		user_code: userCode,
-		verification_uri: verificationUri,
+		verification_uri: parsedUri.href,
 		interval,
 		expires_in: expiresIn,
 	};

packages/ai/src/utils/oauth/openai-codex.ts

@@ -28,7 +28,6 @@ import type {
 	OAuthProviderInterface,
 } from "./types.ts";
 
-const CALLBACK_HOST = process.env.PI_OAUTH_CALLBACK_HOST || "127.0.0.1";
 const CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann";
 const AUTH_BASE_URL = "https://auth.openai.com";
 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 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 = {
 	deviceAuthId: string;
 	userCode: string;
@@ -368,7 +371,7 @@ function startLocalOAuthServer(state: string): Promise<OAuthServerInfo> {
 
 	return new Promise((resolve) => {
 		server
-			.listen(1455, CALLBACK_HOST, () => {
+			.listen(1455, getCallbackHost(), () => {
 				resolve({
 					close: () => server.close(),
 					cancelWait: () => {

packages/ai/test/github-copilot-oauth.test.ts

@@ -84,6 +84,97 @@ describe("GitHub Copilot OAuth device flow", () => {
 		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 () => {
 		vi.useFakeTimers();
 		const startTime = new Date("2026-03-09T00:00:00Z");

packages/coding-agent/CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ### Fixed
 
+- Fixed git package source handling to reject unsafe host/path components and keep managed clone paths inside install roots.
 - Fixed stored XSS in HTML session exports by sanitizing Markdown link and image URLs with a scheme allow-list after stripping control characters.
 - Fixed SDK embedding in bundled Node apps failing with `ENOENT` when `package.json` is not present next to the bundle entrypoint. The package metadata reader now gracefully handles missing `package.json` by using defaults, enabling `createAgentSession()` without requiring package-adjacent files at runtime ([#5226](https://github.com/earendil-works/pi/issues/5226)).
 - Fixed HTTP timeout setting not being respected for non-Codex providers (e.g., llama.cpp via OpenAI-compatible API). The `httpIdleTimeoutMs` setting (set via `/settings` HTTP timeout) now applies as the default SDK request timeout for all providers that support it, not just OpenAI Codex Responses. Disabling the timeout (HTTP timeout = false) now correctly disables SDK timeouts for all supported providers by sending a maximum int32 value (effectively infinite) instead of 0, since SDKs treat timeout=0 as an immediate timeout ([#5294](https://github.com/earendil-works/pi/issues/5294)).

packages/coding-agent/src/modes/interactive/components/login-dialog.ts

@@ -1,6 +1,6 @@
 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 { exec } from "child_process";
+import { openBrowser } from "../../../utils/open-browser.ts";
 import { theme } from "../theme/theme.ts";
 import { DynamicBorder } from "./dynamic-border.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.openUrl(url);
+		openBrowser(url);
 		this.tui.requestRender();
 	}
 
@@ -120,19 +120,10 @@ export class LoginDialogComponent extends Container implements Focusable {
 		this.contentContainer.addChild(new Spacer(1));
 		this.contentContainer.addChild(new Text(theme.fg("warning", `Enter code: ${info.userCode}`), 1, 0));
 
-		this.openUrl(info.verificationUri);
+		openBrowser(info.verificationUri);
 		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)
 	 */

packages/coding-agent/src/utils/git.ts

@@ -73,6 +73,56 @@ function splitRef(url: string): { repo: string; ref?: string } {
 	};
 }
 
+function decodeForValidation(value: string): string | null {
+	try {
+		return decodeURIComponent(value);
+	} catch {
+		return null;
+	}
+}
+
+function hasUnsafeGitInstallPart(value: string, allowSlash: boolean): boolean {
+	const decoded = decodeForValidation(value);
+	if (decoded === null) {
+		return true;
+	}
+	const candidates = [value, decoded];
+	for (const candidate of candidates) {
+		if (candidate.includes("\0") || candidate.includes("\\") || candidate.startsWith("/")) {
+			return true;
+		}
+		if (!allowSlash && candidate.includes("/")) {
+			return true;
+		}
+		if (candidate.split("/").includes("..")) {
+			return true;
+		}
+	}
+	return false;
+}
+
+function buildGitSource(args: { repo: string; host: string; path: string; ref?: string }): GitSource | null {
+	if (args.path.startsWith("/")) {
+		return null;
+	}
+	const normalizedPath = args.path.replace(/\.git$/, "").replace(/^\/+/, "");
+	if (!args.host || !normalizedPath || normalizedPath.split("/").length < 2) {
+		return null;
+	}
+	if (hasUnsafeGitInstallPart(args.host, false) || hasUnsafeGitInstallPart(normalizedPath, true)) {
+		return null;
+	}
+
+	return {
+		type: "git",
+		repo: args.repo,
+		host: args.host,
+		path: normalizedPath,
+		ref: args.ref,
+		pinned: Boolean(args.ref),
+	};
+}
+
 function parseGenericGitUrl(url: string): GitSource | null {
 	const { repo: repoWithoutRef, ref } = splitRef(url);
 	let repo = repoWithoutRef;
@@ -109,19 +159,7 @@ function parseGenericGitUrl(url: string): GitSource | null {
 		repo = `https://${repoWithoutRef}`;
 	}
 
-	const normalizedPath = path.replace(/\.git$/, "").replace(/^\/+/, "");
-	if (!host || !normalizedPath || normalizedPath.split("/").length < 2) {
-		return null;
-	}
-
-	return {
-		type: "git",
-		repo,
-		host,
-		path: normalizedPath,
-		ref,
-		pinned: Boolean(ref),
-	};
+	return buildGitSource({ repo, host, path, ref });
 }
 
 /**
@@ -157,14 +195,12 @@ export function parseGitUrl(source: string): GitSource | null {
 				!split.repo.startsWith("ssh://") &&
 				!split.repo.startsWith("git://") &&
 				!split.repo.startsWith("git@");
-			return {
-				type: "git",
+			return buildGitSource({
 				repo: useHttpsPrefix ? `https://${split.repo}` : split.repo,
 				host: info.domain || "",
-				path: `${info.user}/${info.project}`.replace(/\.git$/, ""),
+				path: `${info.user}/${info.project}`,
 				ref: info.committish || split.ref || undefined,
-				pinned: Boolean(info.committish || split.ref),
-			};
+			});
 		}
 	}
 
@@ -177,14 +213,12 @@ export function parseGitUrl(source: string): GitSource | null {
 			if (split.ref && info.project?.includes("@")) {
 				continue;
 			}
-			return {
-				type: "git",
+			return buildGitSource({
 				repo: `https://${split.repo}`,
 				host: info.domain || "",
-				path: `${info.user}/${info.project}`.replace(/\.git$/, ""),
+				path: `${info.user}/${info.project}`,
 				ref: info.committish || split.ref || undefined,
-				pinned: Boolean(info.committish || split.ref),
-			};
+			});
 		}
 	}
 

packages/coding-agent/src/utils/open-browser.ts

@@ -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();
+}

packages/coding-agent/test/git-ssh-url.test.ts

@@ -62,6 +62,19 @@ describe("Git URL Parsing", () => {
 		});
 	});
 
+	it("should reject unsafe git install path inputs", () => {
+		for (const source of [
+			"git:git@evil.example:../../victim/repo",
+			"https://evil.example/..%2F..%2Fvictim/repo",
+			"https://evil.example/..%2F..%2Fvictim/repo%",
+			"git:git@evil.example:/absolute/repo",
+			"git:git@evil.example:user\\repo/name",
+			"git:git@evil.example:user/repo\0name",
+		]) {
+			expect(parseGitUrl(source)).toBeNull();
+		}
+	});
+
 	describe("unsupported without git: prefix", () => {
 		it("should reject git@host:path without git: prefix", () => {
 			expect(parseGitUrl("git@github.com:user/repo")).toBeNull();

scripts/tool-stats.ts

@@ -3,7 +3,7 @@
 import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs";
 import { homedir, tmpdir } from "node:os";
 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 ImageContent { type: "image"; data: string; mimeType?: string }
@@ -229,4 +229,4 @@ const html = `<!doctype html>
 mkdirSync(resolve(output, ".."), { recursive: true });
 writeFileSync(output, html);
 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);