fix(auth): harden restore containment checks

Author: ndycode

lib/codex-manager.ts

@@ -60,6 +60,7 @@ import {
 	assertNamedBackupRestorePath,
 	importAccounts,
 	getNamedBackupsDirectoryPath,
+	isNamedBackupContainmentError,
 	listNamedBackups,
 	NAMED_BACKUP_ASSESS_CONCURRENCY,
 	findMatchingAccountIndex,
@@ -3875,7 +3876,8 @@ async function runAuthLogin(): Promise<number> {
 				menuResult.mode === "check" ||
 				menuResult.mode === "deep-check" ||
 				menuResult.mode === "forecast" ||
-				menuResult.mode === "fix";
+				menuResult.mode === "fix" ||
+				menuResult.mode === "restore-backup";
 			if (modeTouchesQuotaCache) {
 				const pendingQuotaRefresh = pendingMenuQuotaRefresh;
 				if (pendingQuotaRefresh) {
@@ -4262,6 +4264,9 @@ async function runBackupRestoreManager(
 				assessments.push(result.value);
 				continue;
 			}
+			if (isNamedBackupContainmentError(result.reason)) {
+				throw result.reason;
+			}
 			const backupName = chunk[resultIndex]?.name ?? "unknown";
 			const reason =
 				result.reason instanceof Error

lib/storage.ts

@@ -1,6 +1,6 @@
 import { AsyncLocalStorage } from "node:async_hooks";
 import { createHash } from "node:crypto";
-import { existsSync, promises as fs, type Dirent } from "node:fs";
+import { existsSync, promises as fs, realpathSync, type Dirent } from "node:fs";
 import { basename, dirname, isAbsolute, join, relative } from "node:path";
 import { ACCOUNT_LIMITS } from "./constants.js";
 import { createLogger } from "./logger.js";
@@ -1737,7 +1737,7 @@ async function retryTransientFilesystemOperation<T>(
 				throw error;
 			}
 			const baseDelayMs = TRANSIENT_FILESYSTEM_BASE_DELAY_MS * 2 ** attempt;
-			const jitterMs = Math.floor(Math.random() * 10);
+			const jitterMs = Math.floor(Math.random() * baseDelayMs);
 			await new Promise((resolve) =>
 				setTimeout(resolve, baseDelayMs + jitterMs),
 			);
@@ -2009,12 +2009,26 @@ async function findExistingNamedBackupPath(
 	return undefined;
 }
 
+function resolvePathForNamedBackupContainment(path: string): string {
+	const resolvedPath = resolvePath(path);
+	if (!existsSync(resolvedPath)) {
+		return resolvedPath;
+	}
+	try {
+		return realpathSync.native(resolvedPath);
+	} catch {
+		return resolvedPath;
+	}
+}
+
 export function assertNamedBackupRestorePath(
 	path: string,
 	backupRoot: string,
 ): string {
 	const resolvedPath = resolvePath(path);
-	const relativePath = relative(resolvePath(backupRoot), resolvedPath);
+	const resolvedBackupRoot = resolvePathForNamedBackupContainment(backupRoot);
+	const containedPath = resolvePathForNamedBackupContainment(resolvedPath);
+	const relativePath = relative(resolvedBackupRoot, containedPath);
 	const firstSegment = relativePath.split(/[\\/]/)[0];
 	if (
 		relativePath.length === 0 ||
@@ -2026,7 +2040,7 @@ export function assertNamedBackupRestorePath(
 	return resolvedPath;
 }
 
-function isNamedBackupContainmentError(error: unknown): boolean {
+export function isNamedBackupContainmentError(error: unknown): boolean {
 	return (
 		error instanceof Error &&
 		/escapes backup directory/i.test(error.message)

test/codex-manager-cli.test.ts

@@ -1,7 +1,10 @@
 import { resolve } from "node:path";
+import { fileURLToPath } from "node:url";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
-const MOCK_BACKUP_DIR = resolve(process.cwd(), ".vitest-mock-backups");
+const MOCK_BACKUP_DIR = fileURLToPath(
+	new URL("./.vitest-mock-backups", import.meta.url),
+);
 const mockBackupPath = (name: string): string =>
 	resolve(MOCK_BACKUP_DIR, `${name}.json`);
 
@@ -2899,6 +2902,55 @@ describe("codex manager cli commands", () => {
 		}
 	});
 
+	it("propagates containment errors from batch backup assessment and returns to the login menu", async () => {
+		setInteractiveTTY(true);
+		loadAccountsMock.mockResolvedValue(null);
+		const now = Date.now();
+		listNamedBackupsMock.mockResolvedValue([
+			{
+				name: "escaped-backup",
+				path: mockBackupPath("escaped-backup"),
+				createdAt: null,
+				updatedAt: now,
+				sizeBytes: 128,
+				version: 3,
+				accountCount: 1,
+				schemaErrors: [],
+				valid: true,
+				loadError: undefined,
+			},
+		]);
+		assessNamedBackupRestoreMock.mockRejectedValueOnce(
+			new Error("Backup path escapes backup directory"),
+		);
+		promptLoginModeMock
+			.mockResolvedValueOnce({ mode: "restore-backup" })
+			.mockResolvedValueOnce({ mode: "cancel" });
+		const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+		const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+		try {
+			const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js");
+			const exitCode = await runCodexMultiAuthCli(["auth", "login"]);
+
+			expect(exitCode).toBe(0);
+			expect(promptLoginModeMock).toHaveBeenCalledTimes(2);
+			expect(selectMock).not.toHaveBeenCalled();
+			expect(importAccountsMock).not.toHaveBeenCalled();
+			expect(errorSpy).toHaveBeenCalledWith(
+				expect.stringContaining(
+					"Restore failed: Backup path escapes backup directory",
+				),
+			);
+			expect(warnSpy).not.toHaveBeenCalledWith(
+				expect.stringContaining('Skipped backup assessment for "escaped-backup"'),
+			);
+		} finally {
+			errorSpy.mockRestore();
+			warnSpy.mockRestore();
+		}
+	});
+
 	it("keeps healthy backups selectable when one assessment fails", async () => {
 		setInteractiveTTY(true);
 		loadAccountsMock.mockResolvedValue(null);
@@ -4845,6 +4897,96 @@ describe("codex manager cli commands", () => {
 		}
 	});
 
+	it("waits for an in-flight menu quota refresh before starting backup restore", async () => {
+		setInteractiveTTY(true);
+		const now = Date.now();
+		loadAccountsMock.mockResolvedValue({
+			version: 3,
+			activeIndex: 0,
+			activeIndexByFamily: { codex: 0 },
+			accounts: [
+				{
+					email: "restore@example.com",
+					accountId: "acc-restore",
+					accessToken: "access-restore",
+					expiresAt: now + 3_600_000,
+					refreshToken: "refresh-restore",
+					addedAt: now,
+					lastUsed: now,
+					enabled: true,
+				},
+			],
+		});
+		loadDashboardDisplaySettingsMock.mockResolvedValue({
+			showPerAccountRows: true,
+			showQuotaDetails: true,
+			showForecastReasons: true,
+			showRecommendations: true,
+			showLiveProbeNotes: true,
+			menuAutoFetchLimits: true,
+			menuShowFetchStatus: true,
+			menuQuotaTtlMs: 60_000,
+			menuSortEnabled: true,
+			menuSortMode: "ready-first",
+			menuSortPinCurrent: true,
+			menuSortQuickSwitchVisibleRow: true,
+		});
+		let currentQuotaCache: {
+			byAccountId: Record<string, unknown>;
+			byEmail: Record<string, unknown>;
+		} = {
+			byAccountId: {},
+			byEmail: {},
+		};
+		loadQuotaCacheMock.mockImplementation(async () =>
+			structuredClone(currentQuotaCache),
+		);
+		saveQuotaCacheMock.mockImplementation(async (value: typeof currentQuotaCache) => {
+			currentQuotaCache = structuredClone(value);
+		});
+		const fetchStarted = createDeferred<void>();
+		const releaseFetch = createDeferred<void>();
+		fetchCodexQuotaSnapshotMock.mockImplementation(async () => {
+			fetchStarted.resolve();
+			await releaseFetch.promise;
+			return {
+				status: 200,
+				model: "gpt-5-codex",
+				primary: {},
+				secondary: {},
+			};
+		});
+		listNamedBackupsMock.mockResolvedValue([]);
+		promptLoginModeMock
+			.mockResolvedValueOnce({ mode: "restore-backup" })
+			.mockResolvedValueOnce({ mode: "cancel" });
+		const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
+
+		try {
+			const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js");
+			const runPromise = runCodexMultiAuthCli(["auth", "login"]);
+
+			await fetchStarted.promise;
+			await Promise.resolve();
+
+			expect(listNamedBackupsMock).not.toHaveBeenCalled();
+
+			releaseFetch.resolve();
+
+			const exitCode = await runPromise;
+
+			expect(exitCode).toBe(0);
+			expect(saveQuotaCacheMock).toHaveBeenCalledTimes(1);
+			expect(listNamedBackupsMock).toHaveBeenCalledTimes(1);
+			expect(saveQuotaCacheMock.mock.invocationCallOrder[0]).toBeLessThan(
+				listNamedBackupsMock.mock.invocationCallOrder[0] ??
+					Number.POSITIVE_INFINITY,
+			);
+		} finally {
+			logSpy.mockRestore();
+		}
+	});
+
 	it("skips a second destructive action while reset is already running", async () => {
 		const now = Date.now();
 		const skipMessage =

test/storage.test.ts

@@ -1,13 +1,14 @@
 import { existsSync, promises as fs } from "node:fs";
 import { tmpdir } from "node:os";
-import { dirname, join } from "node:path";
+import { dirname, join, resolve } from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { ACCOUNT_LIMITS } from "../lib/constants.js";
 import { clearQuotaCache, getQuotaCachePath } from "../lib/quota-cache.js";
 import { getConfigDir, getProjectStorageKey } from "../lib/storage/paths.js";
 import { removeWithRetry } from "./helpers/remove-with-retry.js";
 import {
 	assessNamedBackupRestore,
+	assertNamedBackupRestorePath,
 	buildNamedBackupPath,
 	clearAccounts,
 	clearFlaggedAccounts,
@@ -1858,6 +1859,43 @@ describe("storage", () => {
 			}
 		});
 
+		it("rejects backup paths whose real path escapes the backups directory through symlinked directories", async () => {
+			const backupRoot = join(dirname(testStoragePath), "backups");
+			const outsideRoot = join(testWorkDir, "outside");
+			const linkedRoot = join(backupRoot, "linked");
+			const outsideBackupPath = join(outsideRoot, "escape.json");
+			await fs.mkdir(backupRoot, { recursive: true });
+			await fs.mkdir(outsideRoot, { recursive: true });
+			await fs.writeFile(
+				outsideBackupPath,
+				JSON.stringify({
+					version: 3,
+					activeIndex: 0,
+					accounts: [
+						{
+							accountId: "linked-escape",
+							refreshToken: "ref-linked-escape",
+							addedAt: 1,
+							lastUsed: 1,
+						},
+					],
+				}),
+				"utf-8",
+			);
+			await fs.symlink(
+				resolve(outsideRoot),
+				linkedRoot,
+				process.platform === "win32" ? "junction" : "dir",
+			);
+
+			expect(() =>
+				assertNamedBackupRestorePath(
+					join(linkedRoot, "escape.json"),
+					backupRoot,
+				),
+			).toThrow(/escapes backup directory/i);
+		});
+
 		it("rejects named backup listings whose resolved paths escape the backups directory", async () => {
 			const backupRoot = join(dirname(testStoragePath), "backups");
 			const originalReaddir = fs.readdir.bind(fs);