From 9e9d5b20fb3c1c2f425a659f069cb9a5e946cd42 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 06:22:19 -0400 Subject: [PATCH 01/14] fix(server): reject blank bind hostnames --- src/config.ts | 1 + src/server/index.ts | 3 ++- tests/config.test.ts | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/config.ts b/src/config.ts index 1168e0e4e..2db277d72 100644 --- a/src/config.ts +++ b/src/config.ts @@ -527,6 +527,7 @@ export function modelAdapterRecordConfigError( const configSchema = z.object({ port: z.number().int().min(0).max(65535).default(10100), + hostname: z.string().trim().min(1).optional(), providers: z.record(z.string(), providerConfigSchema), defaultProvider: z.string().min(1).default("openai"), openaiProviderTierVersion: z.union([z.literal(1), z.literal(2)]).optional(), diff --git a/src/server/index.ts b/src/server/index.ts index e12dc32c3..ba973ffc1 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -314,7 +314,8 @@ export function startServer(port?: number) { // resolves localhost→127.0.0.1): on Windows `localhost` resolves ::1-first, but the injected URL // is 127.0.0.1, so binding literal "localhost" would reintroduce the F4 refusal. Wildcards // (0.0.0.0/::) and specific hosts are left untouched so intentional exposure is preserved. - const bindHost = /^localhost$/i.test(config.hostname ?? "") ? "127.0.0.1" : (config.hostname ?? "127.0.0.1"); + const configuredHost = config.hostname?.trim(); + const bindHost = !configuredHost || /^localhost$/i.test(configuredHost) ? "127.0.0.1" : configuredHost; // Codex treats empty / non-JSON 503 bodies as "Unknown error" (#452). Keep Retry-After and // the server_is_overloaded code so clients can back off, but always return a JSON envelope. diff --git a/tests/config.test.ts b/tests/config.test.ts index 54cfa7292..7b1e75b39 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -21,6 +21,7 @@ import { readRuntimePort, removePid, removeRuntimePort, + validateConfigCandidate, writeRuntimePort, writePid, } from "../src/config"; @@ -105,6 +106,23 @@ describe("opencodex config defaults", () => { expect(codexAutoStartEnabled({ codexAutoStart: true })).toBe(true); }); + test("config candidates reject blank server hostnames", () => { + const base = getDefaultConfig(); + + expect(validateConfigCandidate({ ...base, hostname: "" })).toMatchObject({ + ok: false, + error: expect.stringContaining("hostname"), + }); + expect(validateConfigCandidate({ ...base, hostname: " " })).toMatchObject({ + ok: false, + error: expect.stringContaining("hostname"), + }); + expect(validateConfigCandidate({ ...base, hostname: "127.0.0.1" })).toMatchObject({ + ok: true, + config: expect.objectContaining({ hostname: "127.0.0.1" }), + }); + }); + test("Codex shim auto-restore defaults on with config and environment opt-out precedence", () => { expect(getDefaultConfig().codexShimAutoRestore).toBe(true); expect(codexShimAutoRestoreEnabled({}, {})).toBe(true); From 5eafd3626131e2308a58841c32dc236ed3b6451d Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 06:28:22 -0400 Subject: [PATCH 02/14] fix(macos): harden launchctl environment cleanup --- src/server/system-env.ts | 32 +++++++--- tests/system-env.test.ts | 126 +++++++++++++++++++++++++++------------ 2 files changed, 111 insertions(+), 47 deletions(-) diff --git a/src/server/system-env.ts b/src/server/system-env.ts index 123e07db8..99add046c 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; import { readFileSync, writeFileSync, unlinkSync, mkdirSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir } from "../config"; @@ -128,6 +128,19 @@ const SYSTEM_ENV_NAMES = [ "ANTHROPIC_AUTH_TOKEN", ] as const; +const MANAGED_SYSTEM_ENV_NAMES = new Set([ + ...SYSTEM_ENV_NAMES, + "ANTHROPIC_DEFAULT_OPUS_MODEL", + "ANTHROPIC_DEFAULT_SONNET_MODEL", + "ANTHROPIC_DEFAULT_FABLE_MODEL", + "ANTHROPIC_DEFAULT_HAIKU_MODEL", + "ANTHROPIC_SMALL_FAST_MODEL", + "CLAUDE_CODE_MAX_CONTEXT_TOKENS", + "DISABLE_COMPACT", + "CLAUDE_CODE_AUTO_COMPACT_WINDOW", + "CLAUDE_CODE_ALWAYS_ENABLE_EFFORT", +]); + interface SystemEnvTracking { pid: number; port: number; @@ -147,7 +160,7 @@ export function getSystemEnvTrackingPath(): string { export function launchctlGetenv(name: string): string | undefined { if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name)) return undefined; try { - const value = execSync(`launchctl getenv ${name}`, { encoding: "utf8" }).trim(); + const value = execFileSync("/bin/launchctl", ["getenv", name], { encoding: "utf8" }).trim(); return value || undefined; } catch { return undefined; @@ -160,22 +173,23 @@ function readTracking(): SystemEnvTracking | undefined { if (!Number.isInteger(tracking.port) || typeof tracking.pid !== "number" || typeof tracking.injectedAt !== "string") { return undefined; } - return tracking as SystemEnvTracking; + const injectedKeys = Array.isArray(tracking.injectedKeys) + ? [...new Set(tracking.injectedKeys.filter( + (name): name is string => typeof name === "string" && MANAGED_SYSTEM_ENV_NAMES.has(name), + ))] + : undefined; + return { ...tracking, injectedKeys } as SystemEnvTracking; } catch { return undefined; } } -function shellArg(value: string): string { - return /^[A-Za-z0-9_./:-]+$/.test(value) ? value : `'${value.replaceAll("'", `'\\''`)}'`; -} - function setLaunchctlEnv(name: string, value: string): void { - execSync(`launchctl setenv ${name} ${shellArg(value)}`); + execFileSync("/bin/launchctl", ["setenv", name, value]); } function unsetLaunchctlEnv(name: string): void { - execSync(`launchctl unsetenv ${name}`); + execFileSync("/bin/launchctl", ["unsetenv", name]); } function ownedBaseUrl(port: number): string { diff --git a/tests/system-env.test.ts b/tests/system-env.test.ts index 46a684bca..74dc2a005 100644 --- a/tests/system-env.test.ts +++ b/tests/system-env.test.ts @@ -19,12 +19,14 @@ const baseConfig = { } satisfies OcxConfig; let execSpy: ReturnType; +let execFileSpy: ReturnType; let readSpy: ReturnType; let writeSpy: ReturnType; let unlinkSpy: ReturnType; let mkdirSpy: ReturnType; let trackingFile: string | undefined; let launchctlBaseUrl: string | undefined; +let launchctlEnvValues: Record; function setPlatform(platform: NodeJS.Platform): void { Object.defineProperty(process, "platform", { configurable: true, value: platform }); @@ -34,16 +36,28 @@ function tracking(port = 4567): string { return JSON.stringify({ pid: 123, port, injectedAt: "2026-07-11T00:00:00.000Z" }); } +function launchctlCommands(): string[] { + return execFileSpy.mock.calls + .filter(call => call[0] === "/bin/launchctl") + .map(call => `launchctl ${(call[1] as string[]).join(" ")}`); +} + beforeEach(() => { setPlatform("darwin"); trackingFile = undefined; launchctlBaseUrl = undefined; + launchctlEnvValues = {}; globalThis.fetch = mock(async () => new Response("ok")) as unknown as typeof fetch; - execSpy = spyOn(childProcess, "execSync").mockImplementation(((command: string) => { - if (command === "launchctl getenv ANTHROPIC_BASE_URL") return launchctlBaseUrl ?? ""; + execSpy = spyOn(childProcess, "execSync").mockImplementation((() => Buffer.alloc(0)) as typeof childProcess.execSync); + execFileSpy = spyOn(childProcess, "execFileSync").mockImplementation(((file: string, args?: readonly string[]) => { + if (file === "/bin/launchctl" && args?.[0] === "getenv") { + const name = args[1]; + if (name === "ANTHROPIC_BASE_URL") return launchctlBaseUrl ?? ""; + return launchctlEnvValues[name] ?? ""; + } return Buffer.alloc(0); - }) as typeof childProcess.execSync); + }) as typeof childProcess.execFileSync); readSpy = spyOn(fs, "readFileSync").mockImplementation((() => { if (trackingFile === undefined) throw new Error("ENOENT"); return trackingFile; @@ -59,6 +73,7 @@ beforeEach(() => { afterEach(() => { execSpy.mockRestore(); + execFileSpy.mockRestore(); readSpy.mockRestore(); writeSpy.mockRestore(); unlinkSpy.mockRestore(); @@ -71,7 +86,7 @@ describe("system environment injection", () => { test("injectSystemEnv sets the Claude launchctl variables on macOS", async () => { expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: true }); - const commands = execSpy.mock.calls.map(call => call[0]); + const commands = launchctlCommands(); expect(commands).toContain("launchctl setenv ANTHROPIC_BASE_URL http://127.0.0.1:4567"); expect(commands).toContain("launchctl setenv CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY 1"); // Writes include the shell env file and the tracking file (agent-def syncing @@ -82,11 +97,26 @@ describe("system environment injection", () => { expect(JSON.parse(trackingFile!)).toMatchObject({ pid: process.pid, port: 4567 }); }); + test("injectSystemEnv invokes launchctl without a command shell", async () => { + expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: true }); + + expect(execFileSpy).toHaveBeenCalledWith( + "/bin/launchctl", + ["getenv", "ANTHROPIC_BASE_URL"], + { encoding: "utf8" }, + ); + expect(execFileSpy).toHaveBeenCalledWith( + "/bin/launchctl", + ["setenv", "ANTHROPIC_BASE_URL", "http://127.0.0.1:4567"], + ); + expect(execSpy).not.toHaveBeenCalled(); + }); + test("injectSystemEnv is a no-op outside macOS", async () => { setPlatform("linux"); expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: false, reason: "not macOS" }); - expect(execSpy).not.toHaveBeenCalled(); + expect(execFileSpy).not.toHaveBeenCalled(); }); test("injectSystemEnv skips disabled Claude and system environment integration", async () => { @@ -107,7 +137,7 @@ describe("system environment injection", () => { injected: false, reason: "user has custom ANTHROPIC_BASE_URL", }); - expect(execSpy.mock.calls.some(call => String(call[0]).includes("setenv"))).toBe(false); + expect(launchctlCommands().some(command => command.includes("setenv"))).toBe(false); }); test("injectSystemEnv includes the first configured API key", async () => { @@ -117,18 +147,19 @@ describe("system environment injection", () => { }; expect(await injectSystemEnv(4567, config)).toEqual({ injected: true }); - expect(execSpy.mock.calls.map(call => call[0])).toContain("launchctl setenv ANTHROPIC_AUTH_TOKEN secret-token"); + expect(launchctlCommands()).toContain("launchctl setenv ANTHROPIC_AUTH_TOKEN secret-token"); }); - test("injectSystemEnv shell-quotes API keys with special characters", async () => { + test("injectSystemEnv passes API keys with special characters as one argument", async () => { const config: OcxConfig = { ...baseConfig, apiKeys: [{ id: "key-1", name: "Primary", key: "secret token'quoted", createdAt: "2026-07-11T00:00:00.000Z" }], }; expect(await injectSystemEnv(4567, config)).toEqual({ injected: true }); - expect(execSpy.mock.calls.map(call => call[0])).toContain( - "launchctl setenv ANTHROPIC_AUTH_TOKEN 'secret token'\\''quoted'", + expect(execFileSpy).toHaveBeenCalledWith( + "/bin/launchctl", + ["setenv", "ANTHROPIC_AUTH_TOKEN", "secret token'quoted"], ); }); @@ -139,11 +170,7 @@ describe("system environment injection", () => { } function mockAuthTokenGetenv(value: string | undefined): void { - execSpy.mockImplementation(((command: string) => { - if (command === "launchctl getenv ANTHROPIC_BASE_URL") return launchctlBaseUrl ?? ""; - if (command === "launchctl getenv ANTHROPIC_AUTH_TOKEN") return value ?? ""; - return Buffer.alloc(0); - }) as typeof childProcess.execSync); + launchctlEnvValues.ANTHROPIC_AUTH_TOKEN = value; } test("re-inject after switching back to subscription unsets the owned dummy token", async () => { @@ -159,7 +186,7 @@ describe("system environment injection", () => { claudeCode: { systemEnv: true, authMode: "subscription" }, } as unknown as OcxConfig; expect(await injectSystemEnv(4567, subscription)).toEqual({ injected: true }); - expect(execSpy.mock.calls.map(call => call[0])).toContain("launchctl unsetenv ANTHROPIC_AUTH_TOKEN"); + expect(execFileSpy).toHaveBeenCalledWith("/bin/launchctl", ["unsetenv", "ANTHROPIC_AUTH_TOKEN"]); expect(JSON.parse(trackingFile!).injectedKeys).not.toContain("ANTHROPIC_AUTH_TOKEN"); }); @@ -169,7 +196,7 @@ describe("system environment injection", () => { mockAuthTokenGetenv("sk-user-real-token"); expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: true }); - expect(execSpy.mock.calls.map(call => call[0])).not.toContain("launchctl unsetenv ANTHROPIC_AUTH_TOKEN"); + expect(launchctlCommands()).not.toContain("launchctl unsetenv ANTHROPIC_AUTH_TOKEN"); }); test("re-inject preserves an untracked dummy-valued token it does not own", async () => { @@ -180,7 +207,7 @@ describe("system environment injection", () => { mockAuthTokenGetenv("opencodex-proxy"); expect(await injectSystemEnv(4567, baseConfig)).toEqual({ injected: true }); - expect(execSpy.mock.calls.map(call => call[0])).not.toContain("launchctl unsetenv ANTHROPIC_AUTH_TOKEN"); + expect(launchctlCommands()).not.toContain("launchctl unsetenv ANTHROPIC_AUTH_TOKEN"); }); }); @@ -195,7 +222,7 @@ describe("system environment cleanup", () => { "CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY", "ANTHROPIC_AUTH_TOKEN", ]) { - expect(execSpy.mock.calls.map(call => call[0])).toContain(`launchctl unsetenv ${name}`); + expect(execFileSpy).toHaveBeenCalledWith("/bin/launchctl", ["unsetenv", name]); } // Two deletes: shell env file + tracking file expect(unlinkSpy).toHaveBeenCalledTimes(2); @@ -209,6 +236,39 @@ describe("system environment cleanup", () => { expect(unlinkSpy).not.toHaveBeenCalled(); }); + test("revertSystemEnv ignores unrecognized names from a tampered tracking file", () => { + trackingFile = JSON.stringify({ + pid: 123, + port: 4567, + injectedAt: "2026-07-11T00:00:00.000Z", + injectedKeys: [ + "ANTHROPIC_BASE_URL", + "CLAUDE_CODE_MAX_CONTEXT_TOKENS", + "UNRELATED_USER_SETTING", + ], + }); + launchctlBaseUrl = "http://127.0.0.1:4567"; + + expect(revertSystemEnv()).toEqual({ reverted: true }); + const unsetNames = execFileSpy.mock.calls + .filter(call => call[0] === "/bin/launchctl" && (call[1] as string[])[0] === "unsetenv") + .map(call => (call[1] as string[])[1]); + expect(unsetNames).toContain("ANTHROPIC_BASE_URL"); + expect(unsetNames).toContain("CLAUDE_CODE_MAX_CONTEXT_TOKENS"); + expect(unsetNames).not.toContain("UNRELATED_USER_SETTING"); + }); + + test("revertSystemEnv invokes launchctl without a command shell", () => { + trackingFile = tracking(); + launchctlBaseUrl = "http://127.0.0.1:4567"; + + expect(revertSystemEnv()).toEqual({ reverted: true }); + expect(execFileSpy).toHaveBeenCalledWith( + "/bin/launchctl", + ["unsetenv", "ANTHROPIC_BASE_URL"], + ); + }); + test("cleanStaleSystemEnv reverts a dead tracked proxy", async () => { trackingFile = tracking(); launchctlBaseUrl = "http://127.0.0.1:4567"; @@ -238,7 +298,7 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { test("injects lever keys, tracks them, and shell file uses conditional exports", async () => { const writes = capturedWrites(); expect(await injectSystemEnv(4096, leverConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls).toContain("launchctl setenv CLAUDE_CODE_MAX_CONTEXT_TOKENS 1000000"); expect(setCalls).toContain("launchctl setenv DISABLE_COMPACT 1"); expect(setCalls).toContain("launchctl setenv CLAUDE_CODE_ALWAYS_ENABLE_EFFORT 1"); @@ -255,14 +315,9 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { test("user-preset launchctl values are skipped and never tracked (revert cannot delete them)", async () => { const writes = capturedWrites(); - execSpy.mockImplementation(((command: string) => { - if (command === "launchctl getenv ANTHROPIC_BASE_URL") return launchctlBaseUrl ?? ""; - if (command === "launchctl getenv CLAUDE_CODE_MAX_CONTEXT_TOKENS") return "777000"; - if (command.startsWith("launchctl getenv")) return ""; - return Buffer.alloc(0); - }) as typeof childProcess.execSync); + launchctlEnvValues.CLAUDE_CODE_MAX_CONTEXT_TOKENS = "777000"; expect(await injectSystemEnv(4096, leverConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls).not.toContain("launchctl setenv CLAUDE_CODE_MAX_CONTEXT_TOKENS 1000000"); expect(setCalls).toContain("launchctl setenv DISABLE_COMPACT 1"); const trackingWrite = writes.filter(w => w.path.includes("system-env-port")).at(-1); @@ -274,7 +329,7 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { test("levers disabled: no lever keys injected or exported", async () => { const writes = capturedWrites(); expect(await injectSystemEnv(4096, baseConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls.some(c => c.includes("CLAUDE_CODE_MAX_CONTEXT_TOKENS"))).toBe(false); expect(setCalls.some(c => c.includes("CLAUDE_CODE_ALWAYS_ENABLE_EFFORT"))).toBe(false); const shellWrite = writes.find(w => w.path.includes("claude-env.sh")); @@ -284,7 +339,7 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { test("auto-context default lever: AUTO_COMPACT_WINDOW 350000 injected, tracked, conditionally exported (devlog 020)", async () => { const writes = capturedWrites(); expect(await injectSystemEnv(4096, baseConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls).toContain("launchctl setenv CLAUDE_CODE_AUTO_COMPACT_WINDOW 350000"); const trackingWrite = writes.filter(w => w.path.includes("system-env-port")).at(-1); expect(JSON.parse(trackingWrite!.data).injectedKeys).toContain("CLAUDE_CODE_AUTO_COMPACT_WINDOW"); @@ -294,14 +349,9 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { test("auto-context: user-preset launchctl value is respected and untracked (audit 021 #2)", async () => { const writes = capturedWrites(); - execSpy.mockImplementation(((command: string) => { - if (command === "launchctl getenv ANTHROPIC_BASE_URL") return launchctlBaseUrl ?? ""; - if (command === "launchctl getenv CLAUDE_CODE_AUTO_COMPACT_WINDOW") return "500000"; - if (command.startsWith("launchctl getenv")) return ""; - return Buffer.alloc(0); - }) as typeof childProcess.execSync); + launchctlEnvValues.CLAUDE_CODE_AUTO_COMPACT_WINDOW = "500000"; expect(await injectSystemEnv(4096, baseConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls.some(c => c.startsWith("launchctl setenv CLAUDE_CODE_AUTO_COMPACT_WINDOW"))).toBe(false); const trackingWrite = writes.filter(w => w.path.includes("system-env-port")).at(-1); expect(JSON.parse(trackingWrite!.data).injectedKeys).not.toContain("CLAUDE_CODE_AUTO_COMPACT_WINDOW"); @@ -310,7 +360,7 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { test("auto-context stays inert while the maxContextTokens lever is set", async () => { capturedWrites(); expect(await injectSystemEnv(4096, leverConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls.some(c => c.startsWith("launchctl setenv CLAUDE_CODE_AUTO_COMPACT_WINDOW"))).toBe(false); }); @@ -321,7 +371,7 @@ describe("systemEnv lever keys (devlog 136 B6)", () => { claudeCode: { systemEnv: true, tierModels: { opus: "cursor/gpt-5.6-luna", sonnet: "mock/small" } }, } satisfies OcxConfig; expect(await injectSystemEnv(4096, tierConfig)).toEqual({ injected: true }); - const setCalls = execSpy.mock.calls.map(call => String(call[0])); + const setCalls = launchctlCommands(); expect(setCalls.some(c => c.startsWith("launchctl setenv ANTHROPIC_DEFAULT_OPUS_MODEL"))).toBe(true); expect(setCalls.some(c => c.startsWith("launchctl setenv ANTHROPIC_DEFAULT_SONNET_MODEL"))).toBe(true); const trackingWrite = writes.filter(w => w.path.includes("system-env-port")).at(-1); From 0cb977666e5cd138b1ac13bc5dd9abc0315bf0f4 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 06:41:00 -0400 Subject: [PATCH 03/14] fix(windows): prevent update cwd command hijacking --- bin/ocx.mjs | 27 +++++---- src/update/index.ts | 54 ++++++++++++------ src/update/job.ts | 4 +- src/update/npm-invocation.d.mts | 23 ++++++++ src/update/npm-invocation.mjs | 86 +++++++++++++++++++++++++++++ tests/update-job.test.ts | 1 + tests/update-npm-invocation.test.ts | 56 +++++++++++++++++++ tests/update-stop-first.test.ts | 27 +++++++-- 8 files changed, 241 insertions(+), 37 deletions(-) create mode 100644 src/update/npm-invocation.d.mts create mode 100644 src/update/npm-invocation.mjs create mode 100644 tests/update-npm-invocation.test.ts diff --git a/bin/ocx.mjs b/bin/ocx.mjs index 0218832de..eb9e734c4 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -14,6 +14,7 @@ import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { npmInvocation } from "../src/update/npm-invocation.mjs"; import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "../src/update/tray-update-plan.mjs"; const PKG = "@bitkyc08/opencodex"; @@ -29,10 +30,6 @@ function isBunGlobalInstall() { return /[\\/]\.bun[\\/]/.test(here); } -function npmBin() { - return process.platform === "win32" ? "npm.cmd" : "npm"; -} - function currentPackageVersion() { try { return JSON.parse(readFileSync(join(here, "..", "package.json"), "utf8")).version ?? "?"; @@ -115,15 +112,17 @@ function runTrayLifecycle(launcher, action) { function runNpmSelfUpdate() { const current = currentPackageVersion(); const tag = updateTag(current); - const npm = npmBin(); - // Node ≥18.20/20.12 refuses to spawn .cmd/.bat without a shell (CVE-2024-27980 - // hardening) — spawning "npm.cmd" shell-less throws EINVAL on Windows. - const winShell = process.platform === "win32"; - const latestResult = spawnSync(npm, ["view", `${PKG}@${tag}`, "version"], { + const latestInvocation = npmInvocation(["view", `${PKG}@${tag}`, "version"]); + const installInvocation = npmInvocation(["install", "-g", `${PKG}@${tag}`]); + if (!latestInvocation || !installInvocation) { + console.error("opencodex: could not resolve npm from a trusted absolute PATH entry; aborting before stopping the proxy."); + process.exit(1); + } + const latestResult = spawnSync(latestInvocation.file, latestInvocation.args, { encoding: "utf8", timeout: 12000, windowsHide: true, - shell: winShell, + ...latestInvocation.options, }); const latest = latestResult.status === 0 ? latestResult.stdout.trim() : ""; @@ -221,12 +220,12 @@ function runNpmSelfUpdate() { } } - console.log(`Updating${latest ? ` to v${latest}` : ""}...\n$ ${npm} install -g ${PKG}@${tag}`); - const res = spawnSync(npm, ["install", "-g", `${PKG}@${tag}`], { + console.log(`Updating${latest ? ` to v${latest}` : ""}...\n$ npm install -g ${PKG}@${tag}`); + const res = spawnSync(installInvocation.file, installInvocation.args, { stdio: "inherit", timeout: 180000, windowsHide: true, - shell: winShell, + ...installInvocation.options, }); if (res.status === 0) { console.log(`\nUpdated${latest ? ` to v${latest}` : ""}.`); @@ -278,7 +277,7 @@ function runNpmSelfUpdate() { process.exit(0); } if (trayBeforeUpdate.restoreOnFailure) runTrayLifecycle(launcher, "start"); - console.error(`\nUpdate failed (${npm} exit ${res.status ?? "?"}). Try manually: ${npm} install -g ${PKG}@${tag}`); + console.error(`\nUpdate failed (npm exit ${res.status ?? "?"}). Try manually: npm install -g ${PKG}@${tag}`); process.exit(1); } diff --git a/src/update/index.ts b/src/update/index.ts index 7cd453b64..41bc64b1a 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -3,6 +3,7 @@ import { readFileSync, readdirSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; import { getConfigDir, loadConfig, readPid, readRuntimePort } from "../config"; +import { npmInvocation } from "./npm-invocation.mjs"; import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "./tray-update-plan.mjs"; /** @@ -50,19 +51,24 @@ export function updateTag(current: string): Channel { return defaultUpdateTag(current); } -/** - * npm is `npm.cmd` on Windows, and Node/Bun refuse shell-less .cmd spawns - * (CVE-2024-27980 hardening) — route Windows npm invocations through the shell. - */ -function npmSpawnTarget(bin: string): { bin: string; shell: boolean } { - if (process.platform !== "win32" || bin !== "npm") return { bin, shell: false }; - return { bin: "npm.cmd", shell: true }; +function npmSpawnTarget(args: readonly string[]): { bin: string; args: string[]; options: { windowsVerbatimArguments?: boolean } } | null { + const invocation = npmInvocation(args); + if (!invocation) return null; + return { bin: invocation.file, args: invocation.args, options: invocation.options }; +} + +function updateSpawnTarget(bin: string, args: readonly string[]): { bin: string; args: string[]; options: { windowsVerbatimArguments?: boolean } } | null { + if (bin === "npm") return npmSpawnTarget(args); + if (process.platform === "win32" && bin === "bun") { + return { bin: process.execPath, args: [...args], options: {} }; + } + return { bin, args: [...args], options: {} }; } /** * The GUI update worker sets OCX_SERVICE=1 and has stdio ignored — inheriting that for - * `npm.cmd` (shell:true) opens stacked visible consoles on Windows. Pipe instead and - * relay bounded output after the child exits. (Ported from PR #167.) + * Background package-manager children can open stacked visible consoles on Windows. + * Pipe instead and relay bounded output after the child exits. (Ported from PR #167.) */ function updateChildStdio(): "inherit" | "pipe" { if (process.env.OCX_SERVICE === "1") return "pipe"; @@ -79,8 +85,14 @@ function logSpawnOutput(label: string, result: { stdout?: string | Buffer | null /** Latest published version from the registry (best-effort; null if npm isn't available). */ export function latestVersion(tag: string): string | null { - const npm = npmSpawnTarget("npm"); - const r = spawnSync(npm.bin, ["view", `${PKG}@${tag}`, "version"], { encoding: "utf8", timeout: 12000, windowsHide: true, shell: npm.shell }); + const npm = npmSpawnTarget(["view", `${PKG}@${tag}`, "version"]); + if (!npm) return null; + const r = spawnSync(npm.bin, npm.args, { + encoding: "utf8", + timeout: 12000, + windowsHide: true, + ...npm.options, + }); return r.status === 0 ? (r.stdout.trim() || null) : null; } @@ -116,11 +128,12 @@ export function checkUpdatePackageIntegrity( spawn: typeof spawnSync = spawnSync, ): { ok: true; integrity: string } | { ok: false; reason: string } | { ok: "skipped"; reason: string } { if (!version) return { ok: "skipped", reason: "no resolved version (registry unavailable)" }; - const npm = npmSpawnTarget("npm"); + const npm = npmSpawnTarget(["view", `${PKG}@${version}`, "dist.integrity"]); + if (!npm) return { ok: "skipped", reason: "npm executable was not found on a trusted PATH entry" }; const r = spawn( npm.bin, - ["view", `${PKG}@${version}`, "dist.integrity"], - { encoding: "utf8", timeout: 12000, windowsHide: true, shell: npm.shell }, + npm.args, + { encoding: "utf8", timeout: 12000, windowsHide: true, ...npm.options }, ); // status !== 0 covers nonzero exits AND timeouts (status === null). if (r.status !== 0) return { ok: "skipped", reason: `registry integrity query failed (status ${r.status ?? "timeout"})` }; @@ -164,6 +177,13 @@ export async function runUpdate(): Promise { console.log(`Verified ${PKG}@${latest} integrity metadata ${integrity.integrity.slice(0, 24)}…`); } + const { bin, args: cmdArgs } = updateCommand(installer, tag, latest); + const target = updateSpawnTarget(bin, cmdArgs); + if (!target) { + console.error("⚠️ Could not resolve npm from a trusted absolute PATH entry; aborting before stopping the proxy."); + process.exit(1); + } + // Remember whether a background service manages the proxy BEFORE stopping — `ocx stop` // unloads it permanently, so a successful update must reinstall/restart it afterwards. let serviceWasInstalled = false; @@ -240,17 +260,15 @@ export async function runUpdate(): Promise { } } - const { bin, args: cmdArgs } = updateCommand(installer, tag, latest); console.log(`Updating${latest ? ` to v${latest}` : ""}…\n$ ${bin} ${cmdArgs.join(" ")}`); - const target = npmSpawnTarget(bin); const installStdio = updateChildStdio(); - const r = spawnSync(target.bin, cmdArgs, { + const r = spawnSync(target.bin, target.args, { stdio: installStdio, encoding: installStdio === "pipe" ? "utf8" : undefined, timeout: 180000, windowsHide: true, - shell: target.shell, + ...target.options, }); if (installStdio === "pipe") logSpawnOutput("", r); if (r.status === 0) { diff --git a/src/update/job.ts b/src/update/job.ts index 24c82874b..10563011c 100644 --- a/src/update/job.ts +++ b/src/update/job.ts @@ -170,7 +170,9 @@ export function updateExecutionCommand( return { bin, args, display: formatCommand(bin, args) }; } if (installer === "bun") { - const { bin, args } = updateCommand(installer, channel, resolvedVersion); + const command = updateCommand(installer, channel, resolvedVersion); + const bin = process.platform === "win32" ? process.execPath : command.bin; + const { args } = command; return { bin, args, display: updateCommandStr(installer, channel, resolvedVersion) }; } return { bin: "sh", args: ["-lc", manualSourceCommand()], display: manualSourceCommand() }; diff --git a/src/update/npm-invocation.d.mts b/src/update/npm-invocation.d.mts new file mode 100644 index 000000000..f7cf3e3a2 --- /dev/null +++ b/src/update/npm-invocation.d.mts @@ -0,0 +1,23 @@ +export interface NpmInvocationDeps { + cwd?: string; + exists?: (path: string) => boolean; +} + +export interface NpmInvocation { + file: string; + args: string[]; + options: { windowsVerbatimArguments?: boolean }; +} + +export declare function resolveNpmCommand( + platform?: NodeJS.Platform, + env?: Record, + deps?: NpmInvocationDeps, +): string | null; + +export declare function npmInvocation( + args: readonly string[], + platform?: NodeJS.Platform, + env?: Record, + deps?: NpmInvocationDeps, +): NpmInvocation | null; diff --git a/src/update/npm-invocation.mjs b/src/update/npm-invocation.mjs new file mode 100644 index 000000000..437ee1282 --- /dev/null +++ b/src/update/npm-invocation.mjs @@ -0,0 +1,86 @@ +import { existsSync } from "node:fs"; +import { win32 } from "node:path"; + +const CMD_META = /([()%!^"`<>&|;, *?])/g; + +function escapeCmdArg(arg) { + let out = String(arg).replace(/(\\*)"/g, '$1$1\\"').replace(/(\\*)$/, "$1$1"); + return `"${out}"`.replace(CMD_META, "^$1"); +} + +function escapeCmdCommand(command) { + return command.replace(CMD_META, "^$1"); +} + +function isInside(root, candidate) { + const relative = win32.relative(win32.resolve(root), win32.resolve(candidate)); + return relative === "" || ( + relative !== ".." + && !relative.startsWith(`..${win32.sep}`) + && !win32.isAbsolute(relative) + ); +} + +function cleanPathEntry(entry) { + const trimmed = entry.trim(); + if (trimmed.startsWith('"') && trimmed.endsWith('"')) return trimmed.slice(1, -1); + return trimmed; +} + +export function resolveNpmCommand( + platform = process.platform, + env = process.env, + deps = {}, +) { + if (platform !== "win32") return "npm"; + const exists = deps.exists ?? existsSync; + const cwd = deps.cwd ?? process.cwd(); + const extensions = (env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD") + .split(";") + .filter(Boolean); + const pathEntries = (env.PATH ?? env.Path ?? "") + .split(win32.delimiter) + .map(cleanPathEntry) + .filter(Boolean); + + for (const entry of pathEntries) { + if (!win32.isAbsolute(entry)) continue; + for (const extension of extensions) { + const candidate = win32.join(entry, `npm${extension.toLowerCase()}`); + if (isInside(cwd, candidate)) continue; + if (exists(candidate)) return win32.resolve(candidate); + } + } + return null; +} + +function systemCommandProcessor(env) { + const systemRoot = env.SystemRoot ?? env.windir; + if (systemRoot && win32.isAbsolute(systemRoot)) { + return win32.join(systemRoot, "System32", "cmd.exe"); + } + const comSpec = env.ComSpec; + return comSpec && win32.isAbsolute(comSpec) ? win32.resolve(comSpec) : null; +} + +export function npmInvocation( + args, + platform = process.platform, + env = process.env, + deps = {}, +) { + const npm = resolveNpmCommand(platform, env, deps); + if (!npm) return null; + if (platform !== "win32" || !/\.(cmd|bat)$/i.test(npm)) { + return { file: npm, args: [...args], options: {} }; + } + + const commandProcessor = systemCommandProcessor(env); + if (!commandProcessor) return null; + const line = [escapeCmdCommand(npm), ...args.map(escapeCmdArg)].join(" "); + return { + file: commandProcessor, + args: ["/d", "/s", "/c", `"${line}"`], + options: { windowsVerbatimArguments: true }, + }; +} diff --git a/tests/update-job.test.ts b/tests/update-job.test.ts index c41a9d5d9..e84d6cef9 100644 --- a/tests/update-job.test.ts +++ b/tests/update-job.test.ts @@ -798,6 +798,7 @@ describe("immutable update target (WP160)", () => { test("bun worker execution pins the resolved version through updateExecutionCommand", () => { const cmd = updateExecutionCommand("bun", "latest", "/pkg/bin/ocx.mjs", "2.7.24"); + expect(cmd.bin).toBe(process.platform === "win32" ? process.execPath : "bun"); expect(cmd.args).toEqual(["add", "-g", "@bitkyc08/opencodex@2.7.24"]); expect(cmd.display).toContain("@2.7.24"); }); diff --git a/tests/update-npm-invocation.test.ts b/tests/update-npm-invocation.test.ts new file mode 100644 index 000000000..7530b9602 --- /dev/null +++ b/tests/update-npm-invocation.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, test } from "bun:test"; +import { + npmInvocation, + resolveNpmCommand, +} from "../src/update/npm-invocation.mjs"; + +const cwd = "C:\\work\\untrusted-project"; +const trustedNpm = "C:\\Program Files\\nodejs\\npm.cmd"; +const systemCmd = "C:\\Windows\\System32\\cmd.exe"; + +describe("Windows npm update invocation", () => { + test("ignores current-directory candidates and resolves npm from an absolute PATH entry", () => { + const existing = new Set([ + `${cwd}\\npm.cmd`, + trustedNpm, + ]); + const env = { + PATH: `${cwd};.;C:\\Program Files\\nodejs`, + PATHEXT: ".CMD", + SystemRoot: "C:\\Windows", + }; + + expect(resolveNpmCommand("win32", env, { + cwd, + exists: path => existing.has(path), + })).toBe(trustedNpm); + + const invocation = npmInvocation(["view", "pkg@latest", "version"], "win32", env, { + cwd, + exists: path => existing.has(path), + }); + expect(invocation).toMatchObject({ + file: systemCmd, + args: ["/d", "/s", "/c", expect.stringContaining("nodejs\\npm.cmd")], + options: { windowsVerbatimArguments: true }, + }); + expect(String(invocation?.args.at(-1) ?? "").includes(cwd)).toBe(false); + }); + + test("fails closed when npm is available only from the current directory", () => { + const env = { + PATH: `${cwd};.`, + PATHEXT: ".CMD", + SystemRoot: "C:\\Windows", + }; + + expect(resolveNpmCommand("win32", env, { + cwd, + exists: path => path === `${cwd}\\npm.cmd`, + })).toBeNull(); + expect(npmInvocation(["view", "pkg@latest", "version"], "win32", env, { + cwd, + exists: path => path === `${cwd}\\npm.cmd`, + })).toBeNull(); + }); +}); diff --git a/tests/update-stop-first.test.ts b/tests/update-stop-first.test.ts index 32ca3196c..e1c902a08 100644 --- a/tests/update-stop-first.test.ts +++ b/tests/update-stop-first.test.ts @@ -11,8 +11,9 @@ describe("update stops the running proxy before replacing files", () => { test("bun/source update path gates on the pid file and spawns 'stop' before the package manager", () => { expect(updateSource).toContain('spawnSync(process.execPath, [process.argv[1], "stop"]'); const stopAt = updateSource.indexOf('[process.argv[1], "stop"]'); - const updateAt = updateSource.indexOf("const { bin, args: cmdArgs } = updateCommand(installer, tag, latest);"); + const updateAt = updateSource.indexOf("spawnSync(target.bin, target.args"); expect(stopAt).toBeGreaterThan(-1); + expect(updateAt).toBeGreaterThan(-1); expect(stopAt).toBeLessThan(updateAt); expect(updateSource).toContain("if (serviceWasInstalled || readPid() || readRuntimePort())"); }); @@ -30,13 +31,30 @@ describe("update stops the running proxy before replacing files", () => { test("npm launcher update path stops via its own launcher path before npm install", () => { expect(launcherSource).toContain('spawnSync(process.execPath, [launcher, "stop"]'); const stopAt = launcherSource.indexOf('[launcher, "stop"]'); - const installAt = launcherSource.indexOf('spawnSync(npm, ["install", "-g"'); + const installAt = launcherSource.indexOf("spawnSync(installInvocation.file, installInvocation.args"); expect(stopAt).toBeGreaterThan(-1); + expect(installAt).toBeGreaterThan(-1); expect(stopAt).toBeLessThan(installAt); expect(launcherSource).toContain('existsSync(join(configDir(), "ocx.pid"))'); expect(launcherSource).toContain('existsSync(join(configDir(), "runtime-port.json"))'); }); + test("Windows npm paths resolve safely before stop and never use shell:true", () => { + const updateResolveAt = updateSource.indexOf("const target = updateSpawnTarget(bin, cmdArgs);"); + const updateStopAt = updateSource.indexOf('[process.argv[1], "stop"]'); + const launcherResolveAt = launcherSource.indexOf("const installInvocation = npmInvocation("); + const launcherStopAt = launcherSource.indexOf('[launcher, "stop"]'); + + expect(updateResolveAt).toBeGreaterThan(-1); + expect(launcherResolveAt).toBeGreaterThan(-1); + expect(updateResolveAt).toBeLessThan(updateStopAt); + expect(launcherResolveAt).toBeLessThan(launcherStopAt); + expect(updateSource).not.toContain("shell: true"); + expect(launcherSource).not.toContain("shell: true"); + expect(updateSource).not.toContain('"npm.cmd"'); + expect(launcherSource).not.toContain('"npm.cmd"'); + }); + test("both paths abort when the stop fails, and reinstall a managed service after success", () => { expect(updateSource).toContain("aborting the update"); // The update path now uses serviceReinstallArgs() to preserve the chosen backend. @@ -64,8 +82,9 @@ describe("update stops the running proxy before replacing files", () => { expect(launcherSource).toContain('name.startsWith("codex-history-backup-") && name.endsWith(".json")'); expect(launcherSource).toContain("if (historyRestoreIncomplete())"); const warnAt = launcherSource.indexOf("Codex resume history was NOT restored"); - const installAt = launcherSource.indexOf('spawnSync(npm, ["install", "-g"'); + const installAt = launcherSource.indexOf("spawnSync(installInvocation.file, installInvocation.args"); expect(warnAt).toBeGreaterThan(-1); + expect(installAt).toBeGreaterThan(-1); expect(warnAt).toBeLessThan(installAt); }); @@ -75,7 +94,7 @@ describe("update stops the running proxy before replacing files", () => { expect(launcherSource).toContain("stopRes.status !== 0 || stillHasRuntimeState"); }); - test("GUI worker update children use pipe stdio so Windows npm.cmd does not open consoles", () => { + test("GUI worker update children use pipe stdio so background updates do not open consoles", () => { expect(updateSource).toContain("function updateChildStdio()"); expect(updateSource).toContain('process.env.OCX_SERVICE === "1"'); expect(updateSource).toContain('return "pipe"'); From a4adba8c79fd3700bf4de58b048604e478eaf4b7 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 07:00:10 -0400 Subject: [PATCH 04/14] test(macos): cover shell-free launchctl injection --- tests/claude-system-env-auto.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/claude-system-env-auto.test.ts b/tests/claude-system-env-auto.test.ts index 6b9274028..289b73fec 100644 --- a/tests/claude-system-env-auto.test.ts +++ b/tests/claude-system-env-auto.test.ts @@ -58,10 +58,9 @@ beforeEach(() => { process.env.CLAUDE_CONFIG_DIR = empty; process.env.HOME = empty; - execSpy = spyOn(childProcess, "execSync").mockImplementation(((command: string) => { - // No keychain item, no launchctl value: a clean "absent" world. - if (String(command).startsWith("launchctl getenv")) return ""; - if (String(command).startsWith("security ")) throw new Error("no keychain item"); + execSpy = spyOn(childProcess, "execFileSync").mockImplementation(((file: string, args?: string[]) => { + // No launchctl value: a clean "absent" world. + if (file === "/bin/launchctl" && args?.[0] === "getenv") return ""; return ""; }) as never); // 44 = SecKeychainSearchCopyNext "item not found": a REAL absent, not a probe failure. From 9d8689dfc0edceb4ed8d17ce4ae64632e0278926 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 07:04:54 -0400 Subject: [PATCH 05/14] fix(uninstall): remove only manifest-owned config state --- src/adapters/mimo-free.ts | 2 + src/cli/help.ts | 10 +- src/cli/index.ts | 10 +- src/cli/star-prompt.ts | 7 +- src/codex/shim.ts | 5 +- src/config.ts | 2 + src/images/artifacts.ts | 3 + src/lib/config-ownership.ts | 327 +++++++++++++++++++++++ src/lib/crash-guard.ts | 2 + src/lib/winsw.ts | 2 + src/oauth/kimi.ts | 2 + src/oauth/store.ts | 3 +- src/server/system-env.ts | 6 +- src/service.ts | 7 + src/tray/windows.ts | 2 + src/usage/debug.ts | 2 + src/usage/log.ts | 2 + structure/02_config-and-codex-home.md | 11 + tests/config-ownership-uninstall.test.ts | 208 ++++++++++++++ 19 files changed, 605 insertions(+), 8 deletions(-) create mode 100644 src/lib/config-ownership.ts create mode 100644 tests/config-ownership-uninstall.test.ts diff --git a/src/adapters/mimo-free.ts b/src/adapters/mimo-free.ts index 56dac58a3..9c5297264 100644 --- a/src/adapters/mimo-free.ts +++ b/src/adapters/mimo-free.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir } from "../config"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import type { OcxProviderConfig, OcxParsedRequest } from "../types"; import { createOpenAIChatAdapter } from "./openai-chat"; import type { ProviderAdapter, AdapterRequest } from "./base"; @@ -59,6 +60,7 @@ export function getMimoClientId(): string { } catch { /* fall through to regenerate */ } const fresh = randomUUID(); try { + recordOwnedConfigPath(dir, file); if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); writeFileSync(file, `${fresh}\n`, "utf8"); } catch { /* persist best-effort; still usable for this process */ } diff --git a/src/cli/help.ts b/src/cli/help.ts index 95f2d67d6..f42fdb4fa 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -30,12 +30,18 @@ const helpEntries: Record = { uninstall: { usage: "ocx uninstall", summary: "Remove service/shim/config and restore native Codex.", - details: ["Alias: ocx remove"], + details: [ + "Alias: ocx remove", + "Config cleanup requires ownership metadata created by a fresh install; legacy or shared directories are left in place.", + ], }, remove: { usage: "ocx remove", summary: "Remove service/shim/config and restore native Codex.", - details: ["Alias of: ocx uninstall"], + details: [ + "Alias of: ocx uninstall", + "Config cleanup requires ownership metadata created by a fresh install; legacy or shared directories are left in place.", + ], }, service: { usage: "ocx service [install|start|stop|status|uninstall|remove]", diff --git a/src/cli/index.ts b/src/cli/index.ts index a661a2119..db9a05e10 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -1,6 +1,5 @@ #!/usr/bin/env bun import { spawn } from "node:child_process"; -import { rmSync } from "node:fs"; import { currentExternalCodexModelProvider, restoreNativeCodex, shouldInjectApiAuthHeader } from "../codex/inject"; import { stripGrokConfig } from "../grok/inject"; import { restoreLegacyOpenaiHistory } from "../codex/history-provider"; @@ -43,6 +42,7 @@ import { maybeShowUpdatePrompt } from "../update/notify"; import { syncModelsToCodex } from "../codex/sync"; import { normalizeUpdateChannel, runGuiUpdateWorker } from "../update/job"; import { collectOrcaCodexHomeDiagnostic } from "../codex/home"; +import { removeOwnedConfigState } from "../lib/config-ownership"; const args = process.argv.slice(2); const command = args[0]; @@ -608,7 +608,13 @@ async function handleUninstall() { if (failures.length === 0) { await runStep("opencodex config removed", () => { - rmSync(getConfigDir(), { recursive: true, force: true }); + const result = removeOwnedConfigState(getConfigDir()); + if (result.status === "absent") return false; + if (result.status === "removed") return true; + const residual = result.residualPaths.length > 0 + ? ` Residual path(s): ${result.residualPaths.join(", ")}` + : ""; + throw new Error(`${result.status} uninstall: ${result.reason ?? "config state was not removed"}.${residual}`); }); } else { console.error("Leaving opencodex config/backups in place so the failed restore step can be retried."); diff --git a/src/cli/star-prompt.ts b/src/cli/star-prompt.ts index ae38abe0d..7a251fc21 100644 --- a/src/cli/star-prompt.ts +++ b/src/cli/star-prompt.ts @@ -2,6 +2,7 @@ import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { spawnSync } from "node:child_process"; import { getConfigDir } from "../config"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import { isAgentDriven } from "./agent-driven"; import { interactiveConfirm } from "./interactive-confirm"; @@ -85,7 +86,11 @@ export async function maybeShowStarPrompt(): Promise { printAgentDeferral(); return; } - try { mkdirSync(dir, { recursive: true }); writeFileSync(marker, new Date().toISOString()); } catch { /* best-effort */ } + try { + recordOwnedConfigPath(dir, marker); + mkdirSync(dir, { recursive: true }); + writeFileSync(marker, new Date().toISOString()); + } catch { /* best-effort */ } const yes = await interactiveConfirm({ question: "\n \x1b[38;5;141m⭐ Enjoying opencodex? Star it on GitHub (via gh)?\x1b[0m", diff --git a/src/codex/shim.ts b/src/codex/shim.ts index c752e9a5f..52f87b7df 100644 --- a/src/codex/shim.ts +++ b/src/codex/shim.ts @@ -22,6 +22,7 @@ import { getConfigDir } from "../config"; import { durableBunPath } from "../lib/bun-runtime"; import { isProcessAlive } from "../lib/process-control"; import { serviceApiTokenFilePath } from "../lib/service-secrets"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import { windowsEnvIndirectBatchValue } from "../lib/win-paths"; import { isWslRuntime, wslAutomountRoot } from "./home"; @@ -582,8 +583,10 @@ function statePath(): string { } function writeState(state: ShimState): void { + const path = statePath(); + recordOwnedConfigPath(getConfigDir(), path); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true }); - writeFileSync(statePath(), JSON.stringify(state, null, 2) + "\n", "utf8"); + writeFileSync(path, JSON.stringify(state, null, 2) + "\n", "utf8"); } /** Git-Bash accepts `C:/...` but not backslashed paths inside sh scripts. */ diff --git a/src/config.ts b/src/config.ts index 2db277d72..07a5e73d1 100644 --- a/src/config.ts +++ b/src/config.ts @@ -5,6 +5,7 @@ import { join, resolve } from "node:path"; import * as z from "zod/v4"; import { comboConfigIssues } from "./combos/types"; import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl"; +import { recordOwnedConfigPath } from "./lib/config-ownership"; import { providerDestinationConfigError } from "./lib/destination-policy"; import { openRouterRoutingConfigError } from "./providers/openrouter-routing"; import { @@ -90,6 +91,7 @@ export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO truncate: target => truncateSync(target, 0), unlink: unlinkSync, }): void { + recordOwnedConfigPath(resolveConfigDir(), path); const tmp = `${path}.ocx.${process.pid}.${++_atomicSeq}.tmp`; let hardened = false; try { diff --git a/src/images/artifacts.ts b/src/images/artifacts.ts index d6c887e8a..d5ad5e02f 100644 --- a/src/images/artifacts.ts +++ b/src/images/artifacts.ts @@ -6,6 +6,7 @@ import type { RequestOptions } from "node:https"; import { basename, join, resolve, sep } from "node:path"; import { getConfigDir } from "../config"; import { assessUrlDestination, resolvePublicAddresses } from "../lib/destination-policy"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; const MAX_DECODED_BYTES_PER_IMAGE = 50 * 1024 * 1024; const MAX_DECODED_BYTES_PER_RESPONSE = 100 * 1024 * 1024; @@ -243,6 +244,7 @@ export async function materializeInlineImage( budget?: ImageBudget, ): Promise { const dir = getArtifactsDir(); + recordOwnedConfigPath(getConfigDir(), dir); await mkdir(dir, { recursive: true, mode: 0o700 }); const buf = decodeValidatedImageBase64(base64Data); @@ -470,6 +472,7 @@ export async function downloadImageToArtifact( chargeImageBudget(budget, bytes.length); const dir = getArtifactsDir(); + recordOwnedConfigPath(getConfigDir(), dir); await mkdir(dir, { recursive: true, mode: 0o700 }); // Retention is post-batch via pruneArtifacts (see fulfill.ts). diff --git a/src/lib/config-ownership.ts b/src/lib/config-ownership.ts new file mode 100644 index 000000000..6c8b855f5 --- /dev/null +++ b/src/lib/config-ownership.ts @@ -0,0 +1,327 @@ +import { + existsSync, + lstatSync, + mkdirSync, + readFileSync, + readdirSync, + realpathSync, + renameSync, + rmdirSync, + unlinkSync, + writeFileSync, +} from "node:fs"; +import { randomUUID } from "node:crypto"; +import { isAbsolute, join, relative, resolve, sep } from "node:path"; + +export const CONFIG_OWNER_FILE = ".opencodex-owner.json"; +export const CONFIG_UNINSTALL_MANIFEST = ".opencodex-uninstall.json"; + +export type ConfigRemovalResult = { + status: "absent" | "removed" | "partial" | "refused"; + reason?: string; + residualPaths: string[]; +}; + +type ConfigOwner = { + version: 1; + ownerId: string; + root: string; +}; + +type ConfigUninstallManifest = ConfigOwner & { + paths: string[]; +}; + +const METADATA_MAX_BYTES = 64 * 1024; +const MANIFEST_MAX_PATHS = 1024; +const INITIAL_OWNED_PATHS = [ + ".star-prompted", + "artifacts", + "auth.json", + "auth.store.lock", + "catalog-backup.json", + "claude-env.sh", + "codex-accounts.json", + "codex-runtime-clamp.json", + "codex-runtime.json", + "codex-shim.autorestore.lock", + "codex-shim.json", + "config.json", + "crash.log", + "kimi-device-id", + "mimo-client-id", + "ocx.pid", + "opencodex-service-launcher.vbs", + "opencodex-service-task.xml", + "opencodex-service.cmd", + "opencodex-tray-offline.ico", + "opencodex-tray-online.ico", + "opencodex-tray-warning.ico", + "opencodex-tray.ps1", + "responses-state.json", + "runtime-port.json", + "service-api-token", + "service-state.json", + "service.log", + "system-env-port", + "tray-heartbeat.json", + "tray-state.json", + "update-job.json", + "usage-debug.jsonl", + "usage.jsonl", + "version.json", + "winsw", +] as const; +const ownershipCache = new Map(); + +function ownershipCacheKey(configDir: string): string { + const key = resolve(configDir); + return process.platform === "win32" ? key.toLowerCase() : key; +} + +function samePath(left: string, right: string): boolean { + return process.platform === "win32" + ? left.toLowerCase() === right.toLowerCase() + : left === right; +} + +function readBoundedJson(path: string): unknown { + const metadata = lstatSync(path); + if (!metadata.isFile() || metadata.isSymbolicLink()) { + throw new Error("ownership metadata is not a regular file"); + } + if (metadata.size > METADATA_MAX_BYTES) throw new Error("ownership metadata is too large"); + return JSON.parse(readFileSync(path, "utf8")) as unknown; +} + +function isOwner(value: unknown): value is ConfigOwner { + if (!value || typeof value !== "object" || Array.isArray(value)) return false; + const owner = value as Record; + return owner.version === 1 + && typeof owner.ownerId === "string" + && /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(owner.ownerId) + && typeof owner.root === "string"; +} + +function isManifest(value: unknown): value is ConfigUninstallManifest { + if (!isOwner(value)) return false; + const paths = (value as Record).paths; + return Array.isArray(paths) + && paths.length <= MANIFEST_MAX_PATHS + && paths.every(path => typeof path === "string"); +} + +function canonicalRoot(configDir: string): string { + return realpathSync.native(resolve(configDir)); +} + +function isWithinRoot(root: string, candidate: string): boolean { + const rel = relative(root, candidate); + return rel === "" || ( + rel !== ".." + && !rel.startsWith(`..${sep}`) + && !isAbsolute(rel) + ); +} + +function manifestRelativePath(configDir: string, candidatePath: string): string | null { + const root = resolve(configDir); + const candidate = resolve(candidatePath); + const rel = relative(root, candidate); + if ( + !rel + || rel === ".." + || rel.startsWith(`..${sep}`) + || isAbsolute(rel) + ) return null; + const normalized = rel.split(sep).join("/"); + if (normalized.split("/").some(part => !part || part === "." || part === ".." || part.includes("\\"))) { + return null; + } + if (normalized === CONFIG_OWNER_FILE || normalized === CONFIG_UNINSTALL_MANIFEST) return null; + return normalized; +} + +function loadOwnership(configDir: string): { owner: ConfigOwner; manifest: ConfigUninstallManifest } | null { + const ownerPath = join(configDir, CONFIG_OWNER_FILE); + const manifestPath = join(configDir, CONFIG_UNINSTALL_MANIFEST); + if (!existsSync(ownerPath) || !existsSync(manifestPath)) return null; + try { + const owner = readBoundedJson(ownerPath); + const manifest = readBoundedJson(manifestPath); + const root = canonicalRoot(configDir); + if ( + !isOwner(owner) + || !isManifest(manifest) + || owner.ownerId !== manifest.ownerId + || !samePath(owner.root, root) + || !samePath(manifest.root, root) + ) return null; + return { owner, manifest }; + } catch { + return null; + } +} + +function createOwnership(configDir: string): { owner: ConfigOwner; manifest: ConfigUninstallManifest } | null { + const rootStat = lstatSync(configDir); + if (!rootStat.isDirectory() || rootStat.isSymbolicLink() || readdirSync(configDir).length !== 0) return null; + const owner: ConfigOwner = { + version: 1, + ownerId: randomUUID(), + root: canonicalRoot(configDir), + }; + const manifest: ConfigUninstallManifest = { ...owner, paths: [...INITIAL_OWNED_PATHS] }; + writeFileSync(join(configDir, CONFIG_OWNER_FILE), `${JSON.stringify(owner, null, 2)}\n`, { + encoding: "utf8", + flag: "wx", + mode: 0o600, + }); + try { + writeFileSync(join(configDir, CONFIG_UNINSTALL_MANIFEST), `${JSON.stringify(manifest, null, 2)}\n`, { + encoding: "utf8", + flag: "wx", + mode: 0o600, + }); + } catch (error) { + try { unlinkSync(join(configDir, CONFIG_OWNER_FILE)); } catch { /* incomplete metadata fails closed */ } + throw error; + } + return { owner, manifest }; +} + +function writeManifest(configDir: string, manifest: ConfigUninstallManifest): void { + const path = join(configDir, CONFIG_UNINSTALL_MANIFEST); + const temp = `${path}.${process.pid}.${randomUUID()}.tmp`; + writeFileSync(temp, `${JSON.stringify(manifest, null, 2)}\n`, { encoding: "utf8", mode: 0o600 }); + try { + renameSync(temp, path); + } catch (error) { + try { unlinkSync(temp); } catch { /* best effort */ } + throw error; + } +} + +function removeOwnedEntry(root: string, path: string): void { + const entry = lstatSync(path); + if (entry.isSymbolicLink()) { + unlinkSync(path); + return; + } + if (!entry.isDirectory()) { + unlinkSync(path); + return; + } + + const realDirectory = realpathSync.native(path); + if (!isWithinRoot(root, realDirectory)) { + throw new Error(`owned directory resolves outside the config root: ${path}`); + } + for (const name of readdirSync(path)) { + removeOwnedEntry(root, join(path, name)); + } + rmdirSync(path); +} + +export function recordOwnedConfigPath(configDir: string, candidatePath: string): boolean { + const rel = manifestRelativePath(configDir, candidatePath); + if (!rel) return false; + const cacheKey = ownershipCacheKey(configDir); + if (!existsSync(configDir)) { + ownershipCache.delete(cacheKey); + mkdirSync(configDir, { recursive: true, mode: 0o700 }); + } + let ownership = ownershipCache.get(cacheKey); + if (ownership === undefined) { + ownership = loadOwnership(configDir) ?? createOwnership(configDir); + ownershipCache.set(cacheKey, ownership); + } + if (!ownership) return false; + if (ownership.manifest.paths.includes(rel)) return true; + const manifest = { + ...ownership.manifest, + paths: [...ownership.manifest.paths, rel].sort(), + }; + writeManifest(configDir, manifest); + ownershipCache.set(cacheKey, { owner: ownership.owner, manifest }); + return true; +} + +export function removeOwnedConfigState(configDir: string): ConfigRemovalResult { + ownershipCache.delete(ownershipCacheKey(configDir)); + if (!existsSync(configDir)) return { status: "absent", residualPaths: [] }; + const root = lstatSync(configDir); + if (!root.isDirectory() || root.isSymbolicLink()) { + return { + status: "refused", + reason: "config ownership root is not a real directory", + residualPaths: [configDir], + }; + } + const ownership = loadOwnership(configDir); + if (!ownership) { + return { + status: "refused", + reason: "config ownership metadata is missing or invalid", + residualPaths: [configDir], + }; + } + + for (const rel of ownership.manifest.paths) { + const path = manifestRelativePath(configDir, join(configDir, ...rel.split("/"))); + if (path !== rel) { + return { + status: "refused", + reason: "config ownership manifest contains an unsafe path", + residualPaths: [configDir], + }; + } + } + + const rootPath = canonicalRoot(configDir); + for (const rel of ownership.manifest.paths) { + const path = join(configDir, ...rel.split("/")); + if (!existsSync(path)) continue; + try { + removeOwnedEntry(rootPath, path); + } catch (error) { + return { + status: "partial", + reason: `could not remove owned path ${rel}: ${error instanceof Error ? error.message : String(error)}`, + residualPaths: [path], + }; + } + } + + try { + unlinkSync(join(configDir, CONFIG_UNINSTALL_MANIFEST)); + unlinkSync(join(configDir, CONFIG_OWNER_FILE)); + } catch (error) { + return { + status: "partial", + reason: `could not remove ownership metadata: ${error instanceof Error ? error.message : String(error)}`, + residualPaths: readdirSync(configDir).map(name => join(configDir, name)), + }; + } + const residualPaths = readdirSync(configDir).map(name => join(configDir, name)); + if (residualPaths.length > 0) { + return { + status: "partial", + reason: "unowned files remain in the config directory", + residualPaths, + }; + } + try { + rmdirSync(configDir); + } catch (error) { + return { + status: "partial", + reason: `could not remove the empty config directory: ${error instanceof Error ? error.message : String(error)}`, + residualPaths: [configDir], + }; + } + return { status: "removed", residualPaths: [] }; +} diff --git a/src/lib/crash-guard.ts b/src/lib/crash-guard.ts index 6112703ab..10693e37e 100644 --- a/src/lib/crash-guard.ts +++ b/src/lib/crash-guard.ts @@ -1,6 +1,7 @@ import { appendFileSync, mkdirSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir } from "../config"; +import { recordOwnedConfigPath } from "./config-ownership"; import { redactSecretString, redactUrlForLog } from "./redact"; import { sidecarBreadcrumb, activityBreadcrumb } from "./sidecar-tracker"; @@ -27,6 +28,7 @@ let installed = false; function crashLogPath(): string { const dir = getConfigDir(); try { + recordOwnedConfigPath(dir, join(dir, "crash.log")); mkdirSync(dir, { recursive: true }); } catch { /* best-effort: directory usually already exists */ diff --git a/src/lib/winsw.ts b/src/lib/winsw.ts index 5e945315b..5c00e47af 100644 --- a/src/lib/winsw.ts +++ b/src/lib/winsw.ts @@ -19,6 +19,7 @@ import { existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from " import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { expandUserPath, getConfigDir, loadConfig } from "../config"; +import { recordOwnedConfigPath } from "./config-ownership"; import { durableBunPath } from "./bun-runtime"; import { serviceApiTokenFilePath } from "./service-secrets"; @@ -133,6 +134,7 @@ export async function ensureWinswBinary(fetchImpl: typeof fetch = fetch): Promis unlinkSync(exe); console.warn("⚠️ Existing WinSW binary failed hash verification; re-downloading."); } + recordOwnedConfigPath(getConfigDir(), winswDir()); if (!existsSync(winswDir())) mkdirSync(winswDir(), { recursive: true }); let body: ArrayBuffer; try { diff --git a/src/oauth/kimi.ts b/src/oauth/kimi.ts index 45282a2f9..be14640e9 100644 --- a/src/oauth/kimi.ts +++ b/src/oauth/kimi.ts @@ -3,6 +3,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import * as os from "node:os"; import { join } from "node:path"; import { randomUUID } from "node:crypto"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import { getConfigDir } from "../config"; import type { OAuthController, OAuthCredentials } from "./types"; @@ -106,6 +107,7 @@ function getDeviceId(): string { if ((e as { code?: string })?.code !== "ENOENT") throw e; } const id = randomUUID().replace(/-/g, ""); + recordOwnedConfigPath(getConfigDir(), p); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true }); writeFileSync(p, id + "\n", { mode: 0o600 }); deviceIdCache = id; diff --git a/src/oauth/store.ts b/src/oauth/store.ts index ad761d6a0..5cdb8ffac 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -19,6 +19,7 @@ import { createHash, randomUUID } from "node:crypto"; import { chmodSync, closeSync, copyFileSync, existsSync, fstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret } from "../config"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import { validateCopilotApiBaseUrl } from "./github-copilot"; import type { OAuthCredentialSource, OAuthCredentials, ProviderAccount, ProviderAccountSet } from "./types"; @@ -150,7 +151,7 @@ function sameFd(a: LockSnapshot,b: ReturnType): boolean { retu export function createOAuthFileLock(options: OAuthFileLockOptions): { acquire(): Promise } { const wait=options.waitTimeoutMs??5000, stale=options.staleAfterMs??120000, min=options.pollMinMs??25,max=options.pollMaxMs??100,sleep=options.sleep??(ms=>Bun.sleep(ms)),now=options.now??Date.now,random=options.random??Math.random,write=options.writeMetadata??((fd,b)=>writeFileSync(fd,b,"utf8")); if(wait<0||stale<=0||min<0||maxstale){options.beforeStaleUnlink?.();const b=snapshot(options.path);if(sameSnapshot(a,b))unlinkSync(options.path);continue;}}catch(e){if(errorCode(e)==="ENOENT")continue;throw new OAuthFileLockError("Could not inspect OAuth file lock",{cause:e});} const elapsed=now()-started;if(elapsed>=wait)throw new OAuthFileLockError(`Timed out after ${wait}ms waiting for OAuth file lock`);await sleep(Math.min(wait-elapsed,min+Math.floor(random()*(max-min+1)))); } } }; } /** Wait long enough for slow IdP refreshes (e.g. Cursor 15s × 3 attempts) before timing out. */ diff --git a/src/server/system-env.ts b/src/server/system-env.ts index 99add046c..ec76ed165 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -6,6 +6,7 @@ import { resolveAutoContext, type AutoContextMode } from "../claude/context-wind import { PROXY_MARKER, defaultAuthDetectDeps, detectClaudeAuth, ownAdmissionTokens } from "../claude/auth-detect"; import { resolveClaudeAuthMode } from "../claude/auth-mode"; import type { OcxConfig } from "../types"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; /** * Does the opencodex dummy marker belong in the system environment? @@ -72,8 +73,10 @@ function writeShellEnvFile(port: number, config: OcxConfig, modelEnv: Record { + recordOwnedConfigPath(getConfigDir(), serviceStatePath()); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true }); writeServiceApiTokenFile(); let hadScheduler = false; @@ -1321,6 +1327,7 @@ function installSystemd(): void { ensureUserBusEnv(); // reach the user bus over a bare SSH session (F9) const dir = unitDir(); if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); + recordOwnedConfigPath(getConfigDir(), serviceStatePath()); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true }); writeServiceApiTokenFile(); writeFileSync(unitPath(), buildUnit(), "utf8"); diff --git a/src/tray/windows.ts b/src/tray/windows.ts index d7aae3163..78ef5e4db 100644 --- a/src/tray/windows.ts +++ b/src/tray/windows.ts @@ -6,6 +6,7 @@ import { join, resolve } from "node:path"; import { expandUserPath, getConfigDir } from "../config"; import { durableBunPath } from "../lib/bun-runtime"; import { hardenSecretDir, hardenSecretPath } from "../lib/windows-secret-acl"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; const RUN_KEY = "HKCU\\Software\\Microsoft\\Windows\\CurrentVersion\\Run"; const RUN_PARENT_KEY = "HKCU\\Software\\Microsoft\\Windows\\CurrentVersion"; @@ -476,6 +477,7 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { for (const path of [entry.bun, entry.cli, sourceScript, ...iconPairs.map(pair => pair.source)]) { if (!existsSync(path)) throw new Error(`Cannot install the tray because a required file is missing: ${path}`); } + recordOwnedConfigPath(getConfigDir(), trayStatePath()); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true, mode: 0o700 }); const runCommand = buildWindowsTrayRunCommand(entry); const runValue = windowsTrayRunValue(entry.opencodexHome); diff --git a/src/usage/debug.ts b/src/usage/debug.ts index 8ed592807..36539441b 100644 --- a/src/usage/debug.ts +++ b/src/usage/debug.ts @@ -4,6 +4,7 @@ import { appendFileSync, chmodSync, existsSync, mkdirSync, readFileSync, writeFi import { join } from "node:path"; import { getConfigDir } from "../config"; import { DEBUG_ENV } from "../lib/debug-settings"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import type { DebugLogEntry } from "../lib/debug-log-buffer"; import { redactSecretString, redactSecrets } from "../lib/redact"; import type { OcxUsage } from "../types"; @@ -42,6 +43,7 @@ export function truncateForDebug(text: string, max = USAGE_DEBUG_BODY_SAMPLE_BYT function ensureUsageDebugDir(): void { const dir = getConfigDir(); + recordOwnedConfigPath(dir, usageDebugPath()); mkdirSync(dir, { recursive: true, mode: 0o700 }); try { chmodSync(dir, 0o700); } catch { /* best-effort */ } } diff --git a/src/usage/log.ts b/src/usage/log.ts index c98163622..07d1bcbd5 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -1,6 +1,7 @@ import { chmodSync, closeSync, existsSync, fstatSync, mkdirSync, openSync, readFileSync, readSync, appendFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir } from "../config"; +import { recordOwnedConfigPath } from "../lib/config-ownership"; import { usageDisplayTotalTokens } from "./totals"; import type { OcxUsage } from "../types"; @@ -317,6 +318,7 @@ function normalizeUsageEntry(entry: PersistedUsageEntry): PersistedUsageEntry { function ensureUsageLogDir(): void { const dir = getConfigDir(); + recordOwnedConfigPath(dir, usageLogPath()); mkdirSync(dir, { recursive: true, mode: 0o700 }); try { chmodSync(dir, 0o700); } catch { /* best-effort on platforms that ignore chmod */ } } diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 224dc34a3..09432b7cc 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -107,3 +107,14 @@ and `routeModel`, but user config overrides registry defaults per field/key. `ocx stop`, `ocx restore` / `ocx eject`, `ocx service stop`, and `ocx service uninstall` must strip opencodex config and routed catalog entries without damaging native Codex state. + +Full `ocx uninstall` config cleanup is ownership-manifest based. A fresh config directory receives a +root-bound owner marker and an uninstall manifest before its first atomic config write. Uninstall +validates both bounded metadata files, rejects path traversal and a symlink/junction config root, +and removes only normalized manifest entries. Manifest-owned directory links are unlinked without +traversing their targets. Unknown files remain in place and make the command report a partial +uninstall with their exact paths. + +Legacy nonempty config directories are deliberately not retroactively claimed. If either ownership +file is missing, malformed, or bound to another root, uninstall refuses config deletion and reports +the residual directory for manual review; there is no recursive-delete fallback. diff --git a/tests/config-ownership-uninstall.test.ts b/tests/config-ownership-uninstall.test.ts new file mode 100644 index 000000000..dca1ca677 --- /dev/null +++ b/tests/config-ownership-uninstall.test.ts @@ -0,0 +1,208 @@ +import { describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + CONFIG_OWNER_FILE, + CONFIG_UNINSTALL_MANIFEST, + recordOwnedConfigPath, + removeOwnedConfigState, +} from "../src/lib/config-ownership"; +import { getDefaultConfig, saveConfig } from "../src/config"; + +describe("owned config uninstall", () => { + test("first owned write creates a missing config root and its metadata", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-config-first-owned-path-")); + const dir = join(parent, "config"); + + try { + expect(recordOwnedConfigPath(dir, join(dir, "usage.jsonl"))).toBe(true); + expect(existsSync(join(dir, CONFIG_OWNER_FILE))).toBe(true); + expect(existsSync(join(dir, CONFIG_UNINSTALL_MANIFEST))).toBe(true); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("refuses a legacy config directory without ownership metadata", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-uninstall-legacy-")); + const configPath = join(dir, "config.json"); + writeFileSync(configPath, '{"keep":true}\n'); + + try { + const result = removeOwnedConfigState(dir); + expect(result.status).toBe("refused"); + expect(result.reason).toContain("ownership"); + expect(readFileSync(configPath, "utf8")).toBe('{"keep":true}\n'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("removes manifest-owned state and the empty config directory", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-uninstall-owned-")); + const configPath = join(dir, "config.json"); + + try { + expect(recordOwnedConfigPath(dir, configPath)).toBe(true); + writeFileSync(configPath, '{"owned":true}\n'); + + expect(removeOwnedConfigState(dir)).toEqual({ + status: "removed", + residualPaths: [], + }); + expect(existsSync(dir)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("preserves unowned files and reports a partial uninstall", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-uninstall-shared-")); + const ownedPath = join(dir, "config.json"); + const foreignPath = join(dir, "personal.txt"); + + try { + expect(recordOwnedConfigPath(dir, ownedPath)).toBe(true); + writeFileSync(ownedPath, '{"owned":true}\n'); + writeFileSync(foreignPath, "keep me\n"); + + const result = removeOwnedConfigState(dir); + expect(result.status).toBe("partial"); + expect(result.residualPaths).toEqual([foreignPath]); + expect(existsSync(ownedPath)).toBe(false); + expect(readFileSync(foreignPath, "utf8")).toBe("keep me\n"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("recursively removes a manifest-owned state directory", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-uninstall-tree-")); + const artifacts = join(dir, "artifacts"); + + try { + expect(recordOwnedConfigPath(dir, artifacts)).toBe(true); + mkdirSync(join(artifacts, "nested"), { recursive: true }); + writeFileSync(join(artifacts, "nested", "image.bin"), "owned"); + + expect(removeOwnedConfigState(dir)).toEqual({ + status: "removed", + residualPaths: [], + }); + expect(existsSync(dir)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("unlinks an owned directory link without traversing its external target", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-uninstall-link-")); + const dir = join(parent, "config"); + const external = join(parent, "external"); + const linkedArtifacts = join(dir, "artifacts"); + mkdirSync(dir); + mkdirSync(external); + writeFileSync(join(external, "keep.bin"), "external"); + + try { + expect(recordOwnedConfigPath(dir, linkedArtifacts)).toBe(true); + symlinkSync(external, linkedArtifacts, process.platform === "win32" ? "junction" : "dir"); + + expect(removeOwnedConfigState(dir).status).toBe("removed"); + expect(readFileSync(join(external, "keep.bin"), "utf8")).toBe("external"); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("rejects a manifest path that escapes the config directory", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-uninstall-traversal-")); + const dir = join(parent, "config"); + const ownedPath = join(dir, "config.json"); + const external = join(parent, "keep.txt"); + mkdirSync(dir); + writeFileSync(external, "external"); + + try { + expect(recordOwnedConfigPath(dir, ownedPath)).toBe(true); + writeFileSync(ownedPath, "{}\n"); + const manifestPath = join(dir, CONFIG_UNINSTALL_MANIFEST); + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { paths: string[] }; + manifest.paths = ["../keep.txt"]; + writeFileSync(manifestPath, `${JSON.stringify(manifest)}\n`); + + expect(removeOwnedConfigState(dir).status).toBe("refused"); + expect(readFileSync(external, "utf8")).toBe("external"); + expect(readFileSync(ownedPath, "utf8")).toBe("{}\n"); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("rejects linked ownership metadata without deleting owned state", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-uninstall-linked-metadata-")); + const dir = join(parent, "config"); + const external = join(parent, "external"); + const ownedPath = join(dir, "config.json"); + mkdirSync(dir); + mkdirSync(external); + + try { + expect(recordOwnedConfigPath(dir, ownedPath)).toBe(true); + writeFileSync(ownedPath, "{}\n"); + rmSync(join(dir, CONFIG_UNINSTALL_MANIFEST)); + symlinkSync( + external, + join(dir, CONFIG_UNINSTALL_MANIFEST), + process.platform === "win32" ? "junction" : "dir", + ); + + expect(removeOwnedConfigState(dir).status).toBe("refused"); + expect(readFileSync(ownedPath, "utf8")).toBe("{}\n"); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("a fresh config save creates ownership metadata and records config.json", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-config-first-write-")); + const dir = join(parent, "config"); + const previous = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = dir; + + try { + saveConfig(getDefaultConfig()); + expect(existsSync(join(dir, CONFIG_OWNER_FILE))).toBe(true); + const manifest = JSON.parse( + readFileSync(join(dir, CONFIG_UNINSTALL_MANIFEST), "utf8"), + ) as { paths: string[] }; + expect(manifest.paths).toContain("config.json"); + } finally { + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("an existing nonempty config directory is not retroactively claimed", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-config-legacy-write-")); + const dir = join(parent, "config"); + const foreignPath = join(dir, "personal.txt"); + mkdirSync(dir); + writeFileSync(foreignPath, "keep me\n"); + const previous = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = dir; + + try { + saveConfig(getDefaultConfig()); + expect(existsSync(join(dir, CONFIG_OWNER_FILE))).toBe(false); + expect(removeOwnedConfigState(dir).status).toBe("refused"); + expect(readFileSync(foreignPath, "utf8")).toBe("keep me\n"); + } finally { + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + rmSync(parent, { recursive: true, force: true }); + } + }); +}); From b0c92d08b43920af53744e810ddfd619af372504 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 07:09:27 -0400 Subject: [PATCH 06/14] fix(server): deny framing of local responses --- src/server/auth-cors.ts | 8 ++++++ src/server/gui-static.ts | 8 +++--- structure/05_gui-and-management-api.md | 5 ++++ tests/server-clickjacking-headers.test.ts | 34 +++++++++++++++++++++++ 4 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 tests/server-clickjacking-headers.test.ts diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index b067f7130..29b9e61d5 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -88,6 +88,13 @@ export function isAllowedRequestOrigin(req: Request, config: OcxConfig): boolean return !origin || isLoopbackOriginValue(origin) || isSameOriginAsRequest(req, origin) || isExtraAllowedOrigin(origin, config); } +export function browserSecurityHeaders(): Record { + return { + "X-Frame-Options": "DENY", + "Content-Security-Policy": "frame-ancestors 'none'", + }; +} + export function corsHeaders(req?: Request, config?: OcxConfig): Record { const origin = req?.headers.get("Origin"); const allowOrigin = origin && req && config && isAllowedRequestOrigin(req, config) ? origin : _corsOrigin; @@ -99,6 +106,7 @@ export function corsHeaders(req?: Request, config?: OcxConfig): Record { @@ -54,8 +55,7 @@ function isFile(path: string): boolean { } } -export function serveGuiFile(pathname: string): Response | null { - const guiDist = findGuiDist(); +export function serveGuiFile(pathname: string, guiDist = findGuiDist()): Response | null { if (!guiDist) return null; const filePath = resolveGuiFilePath(guiDist, pathname); if (!filePath) return null; @@ -65,7 +65,7 @@ export function serveGuiFile(pathname: string): Response | null { const indexPath = join(guiDist, "index.html"); if (isFile(indexPath)) { return new Response(Bun.file(indexPath), { - headers: { "Content-Type": "text/html" }, + headers: { "Content-Type": "text/html", ...browserSecurityHeaders() }, }); } } @@ -75,7 +75,7 @@ export function serveGuiFile(pathname: string): Response | null { const ext = extname(filePath); const contentType = MIME_TYPES[ext] || "application/octet-stream"; return new Response(Bun.file(filePath), { - headers: { "Content-Type": contentType }, + headers: { "Content-Type": contentType, ...browserSecurityHeaders() }, }); } diff --git a/structure/05_gui-and-management-api.md b/structure/05_gui-and-management-api.md index c464c4361..1a8205202 100644 --- a/structure/05_gui-and-management-api.md +++ b/structure/05_gui-and-management-api.md @@ -5,6 +5,11 @@ The bundled React dashboard is built into `gui/dist` and served by the same Bun proxy. `ocx gui` starts the proxy when needed and opens `http://localhost:`. +All ordinary HTTP responses (excluding successful WebSocket upgrades) include `X-Frame-Options: DENY` and +`Content-Security-Policy: frame-ancestors 'none'`. This prevents another page from framing the local +dashboard or management responses. Embedding the dashboard in an iframe is intentionally +unsupported; deployments that previously relied on such embedding must open it as a top-level page. + ## API ownership `src/server/index.ts` authenticates and routes `/api/*`, then delegates the management surface to diff --git a/tests/server-clickjacking-headers.test.ts b/tests/server-clickjacking-headers.test.ts new file mode 100644 index 000000000..8f4695f51 --- /dev/null +++ b/tests/server-clickjacking-headers.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { browserSecurityHeaders, corsHeaders } from "../src/server/auth-cors"; +import { serveGuiFile } from "../src/server/gui-static"; + +const EXPECTED = { + "X-Frame-Options": "DENY", + "Content-Security-Policy": "frame-ancestors 'none'", +}; + +describe("clickjacking response headers", () => { + test("the shared browser header set denies all framing", () => { + expect(browserSecurityHeaders()).toEqual(EXPECTED); + }); + + test("API and preflight headers include the framing policy", () => { + expect(corsHeaders()).toMatchObject(EXPECTED); + }); + + test("static dashboard responses include the framing policy", () => { + const guiDist = mkdtempSync(join(tmpdir(), "ocx-gui-headers-")); + writeFileSync(join(guiDist, "index.html"), "test"); + try { + const response = serveGuiFile("/", guiDist); + expect(response).not.toBeNull(); + expect(response?.headers.get("X-Frame-Options")).toBe("DENY"); + expect(response?.headers.get("Content-Security-Policy")).toBe("frame-ancestors 'none'"); + } finally { + rmSync(guiDist, { recursive: true, force: true }); + } + }); +}); From 43b4f6513ad4a6791c600e5a7256a19589417ba4 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 07:41:43 -0400 Subject: [PATCH 07/14] test(security): align source invariants with hardening --- tests/ocx-launcher-source.test.ts | 16 ++++++++-------- tests/uninstall.test.ts | 3 ++- tests/windows-deploy-close-regressions.test.ts | 3 ++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/ocx-launcher-source.test.ts b/tests/ocx-launcher-source.test.ts index 5ecdcccb6..654b90805 100644 --- a/tests/ocx-launcher-source.test.ts +++ b/tests/ocx-launcher-source.test.ts @@ -9,16 +9,16 @@ import { join } from "node:path"; const source = readFileSync(join(import.meta.dir, "..", "bin", "ocx.mjs"), "utf8"); describe("ocx.mjs npm launcher (source invariants)", () => { - test("npm spawns go through a shell on Windows (Node ≥18.20 EINVALs shell-less .cmd spawns)", () => { - const spawnSites = source.match(/spawnSync\(npm,[\s\S]*?\}\)/g) ?? []; - expect(spawnSites.length).toBe(2); - for (const site of spawnSites) { - expect(site).toContain("shell: winShell"); - } - expect(source).toContain('const winShell = process.platform === "win32";'); + test("Windows npm spawns use the trusted absolute invocation without shell lookup", () => { + expect(source).toContain("const latestInvocation = npmInvocation("); + expect(source).toContain("const installInvocation = npmInvocation("); + expect(source).toContain("spawnSync(latestInvocation.file, latestInvocation.args"); + expect(source).toContain("spawnSync(installInvocation.file, installInvocation.args"); + expect(source).not.toContain("shell: true"); + expect(source).not.toContain('"npm.cmd"'); }); - test("--tag is allowlisted before reaching shell-joined spawn args", () => { + test("--tag is allowlisted before reaching package-manager arguments", () => { expect(source).toContain('if (explicit === "preview" || explicit === "latest") return explicit;'); expect(source).not.toMatch(/if \(tagIndex !== -1 && process\.argv\[tagIndex \+ 1\]\) return process\.argv/); }); diff --git a/tests/uninstall.test.ts b/tests/uninstall.test.ts index f8925c9ce..5243ecc8e 100644 --- a/tests/uninstall.test.ts +++ b/tests/uninstall.test.ts @@ -15,7 +15,8 @@ describe("full uninstall command", () => { expect(cli).toContain("uninstallServiceIfInstalled"); expect(cli).toContain("uninstallCodexShim"); expect(cli).toContain("restoreNativeCodex"); - expect(cli).toContain("rmSync(getConfigDir()"); + expect(cli).toContain("removeOwnedConfigState(getConfigDir())"); + expect(cli).not.toContain("rmSync(getConfigDir()"); }); test("CLI exposes explicit legacy history recovery command", async () => { diff --git a/tests/windows-deploy-close-regressions.test.ts b/tests/windows-deploy-close-regressions.test.ts index 2d9a89afb..46742f3a5 100644 --- a/tests/windows-deploy-close-regressions.test.ts +++ b/tests/windows-deploy-close-regressions.test.ts @@ -51,7 +51,8 @@ describe("systemd detection tolerates a no-DBUS SSH session (F9)", () => { describe("server bind canonicalizes explicit localhost but preserves wildcards (F4 symmetry)", () => { const src = read("src/server/index.ts"); test("literal localhost binds to 127.0.0.1; 0.0.0.0/:: exposure is untouched", () => { - expect(src).toContain('/^localhost$/i.test(config.hostname ?? "") ? "127.0.0.1"'); + expect(src).toContain("const configuredHost = config.hostname?.trim();"); + expect(src).toContain('!configuredHost || /^localhost$/i.test(configuredHost) ? "127.0.0.1"'); expect(src).toContain("hostname: bindHost,"); // Must not blanket-rewrite the bind host (that would break intentional 0.0.0.0 exposure). expect(src).not.toContain('hostname: "127.0.0.1",'); From 42bdb1f3924353ba3c25bc7eba9ca8114f58790b Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 07:43:54 -0400 Subject: [PATCH 08/14] test(server): exercise real websocket origin rejection --- tests/project-config-warnings.test.ts | 19 ++++++----- tests/server-auth.test.ts | 46 +++++++++++++++++++++------ 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/tests/project-config-warnings.test.ts b/tests/project-config-warnings.test.ts index ed32686ce..6aa74e290 100644 --- a/tests/project-config-warnings.test.ts +++ b/tests/project-config-warnings.test.ts @@ -5,7 +5,6 @@ import { join, posix, win32 } from "node:path"; import { analyzeProjectCodexConfig, collectProjectCodexConfigWarnings, - getCachedProjectConfigDiagnostics, isGlobalOpencodexRoutingActive, invalidateProjectConfigDiagnosticsCache, parseTrustedProjectPathsFromCodexConfig, @@ -227,25 +226,29 @@ name = "anthropic" expect(collectProjectCodexConfigWarnings()).toEqual([]); }); - test("caches diagnostics for repeated API reads", () => { + test("uncached collection reflects project config changes", () => { const projectDir = join(testDir, "proj"); const codexConfigPath = join(process.env.CODEX_HOME!, "config.toml"); + const projectConfigPath = join(projectDir, ".codex", "config.toml"); writeGlobalRoutingConfig(` [projects.'${projectDir}'] trust_level = "trusted" `); mkdirSync(join(projectDir, ".codex"), { recursive: true }); - writeFileSync(join(projectDir, ".codex", "config.toml"), ` + writeFileSync(projectConfigPath, ` model_provider = "anthropic" [model_providers.anthropic] name = "anthropic" `); - // Use collectProjectCodexConfigWarnings with explicit cwd to avoid real-CWD leakage - const first = collectProjectCodexConfigWarnings({ cwd: testDir, codexConfigPath }); + // Parent discovery may legitimately find a real user config above the OS temp + // directory, so scope this assertion to the fixture project. + const first = collectProjectCodexConfigWarnings({ cwd: testDir, codexConfigPath }) + .filter(warning => warning.path === projectConfigPath); expect(first.length).toBe(1); - writeFileSync(join(projectDir, ".codex", "config.toml"), `model_provider = "openai"`); - // Stale call still returns old result (no invalidation) - const second = collectProjectCodexConfigWarnings({ cwd: testDir, codexConfigPath }); + writeFileSync(projectConfigPath, `model_provider = "openai"`); + // Direct collection bypasses the diagnostics cache and sees the new file. + const second = collectProjectCodexConfigWarnings({ cwd: testDir, codexConfigPath }) + .filter(warning => warning.path === projectConfigPath); expect(second.length).toBe(0); }); }); diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 2ad8bb630..af247582b 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { request as httpRequest } from "node:http"; import { join } from "node:path"; import { saveCodexAccountCredential } from "../src/codex/account-store"; import { getTrackedCodexWebSocketCountForAccount } from "../src/codex/websocket-registry"; @@ -681,18 +682,43 @@ describe("server local API auth", () => { const server = startServer(0); try { - const response = await fetch(new URL("/v1/responses", server.url), { - method: "GET", - headers: { - authorization: "Bearer inbound-main-token", - connection: "Upgrade", - upgrade: "websocket", - origin: "https://attacker.test", - "x-opencodex-api-key": "local-secret", - }, + const response = await new Promise<{ status: number; body: string }>((resolve, reject) => { + const req = httpRequest({ + hostname: "127.0.0.1", + port: server.port, + path: "/v1/responses", + method: "GET", + headers: { + authorization: "Bearer inbound-main-token", + connection: "Upgrade", + upgrade: "websocket", + origin: "https://attacker.test", + "sec-websocket-key": "dGhlIHNhbXBsZSBub25jZQ==", + "sec-websocket-version": "13", + "x-opencodex-api-key": "local-secret", + }, + }, incoming => { + let body = ""; + incoming.setEncoding("utf8"); + incoming.on("data", chunk => { + body += chunk; + }); + incoming.on("end", () => { + resolve({ status: incoming.statusCode ?? 0, body }); + }); + }); + req.setTimeout(5_000, () => { + req.destroy(new Error("hostile websocket handshake timed out")); + }); + req.on("upgrade", (incoming, socket) => { + socket.destroy(); + resolve({ status: incoming.statusCode ?? 0, body: "" }); + }); + req.on("error", reject); + req.end(); }); expect(response.status).toBe(403); - expect(await response.json()).toMatchObject({ + expect(JSON.parse(response.body)).toMatchObject({ error: { code: "origin_rejected" }, }); } finally { From c2e314ef04bda98d4307837a458d926145c42301 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 10:57:48 -0400 Subject: [PATCH 09/14] fix(config): preserve config on blank persisted hostname --- src/config.ts | 41 ++++++++++++++++++++++++++++++++++++++++- tests/config.test.ts | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/config.ts b/src/config.ts index 07a5e73d1..ea9daba77 100644 --- a/src/config.ts +++ b/src/config.ts @@ -529,7 +529,13 @@ export function modelAdapterRecordConfigError( const configSchema = z.object({ port: z.number().int().min(0).max(65535).default(10100), - hostname: z.string().trim().min(1).optional(), + // A blank hostname degrades to undefined rather than failing the parse. `getDefaultConfig()` + // carries no `hostname` key, so the backup-and-defaults repair path below cannot merge one + // away — a hand-edited `"hostname": ""` would fail twice and reset providers/apiKeys to + // defaults, which is strictly worse than the bind bug this validation exists for. Degrading + // is safe: startServer() already falls back to 127.0.0.1 for a missing hostname. Write-time + // rejection lives in validateConfigCandidate() so bad values still surface to the caller. + hostname: z.string().trim().min(1).optional().catch(undefined), providers: z.record(z.string(), providerConfigSchema), defaultProvider: z.string().min(1).default("openai"), openaiProviderTierVersion: z.union([z.literal(1), z.literal(2)]).optional(), @@ -809,6 +815,19 @@ function warnDegradedStreamMode(rawParsed: unknown, validated: OcxConfig): void } } +/** + * Companion to {@link warnDegradedStreamMode} for a blank persisted `hostname`. The bind + * falls back to loopback, which is the safe direction but not what the file asked for — + * say so once instead of silently ignoring the field. + */ +function warnDegradedHostname(rawParsed: unknown, validated: OcxConfig): void { + if (!rawParsed || typeof rawParsed !== "object") return; + const raw = (rawParsed as Record).hostname; + if (raw !== undefined && validated.hostname === undefined) { + console.warn(`⚠️ config.json hostname ${JSON.stringify(raw)} is not a usable bind address — falling back to 127.0.0.1`); + } +} + type NativeSubagentPersistedField = "injectionModel" | "injectionEffort" | "syncCodexSubagentDefaults"; function rawConfigRecord(rawParsed: unknown): Record | null { @@ -883,6 +902,7 @@ export function loadConfig(): OcxConfig { if (result.success) { const config = result.data as OcxConfig; warnDegradedStreamMode(parsed, config); + warnDegradedHostname(parsed, config); warnDegradedNativeSubagentConfig(parsed, config); return normalizeNativeSubagentSync(config, parsed); } @@ -899,6 +919,7 @@ export function loadConfig(): OcxConfig { if (retryResult.success) { warnConfigRepaired(configPath, result.error); const config = retryResult.data as OcxConfig; + warnDegradedHostname(parsed, config); warnDegradedNativeSubagentConfig(parsed, config); return normalizeNativeSubagentSync(config, parsed); } @@ -974,8 +995,26 @@ function schemaDiagnosticsError(error: z.ZodError): string { return details.length > 0 ? `schema_invalid: ${details.join("; ")}` : "schema_invalid"; } +/** + * Reject a hostname the schema deliberately degrades on read. Load-time has to keep a + * blank value non-fatal (see the `hostname` field comment), but an incoming write is a + * live caller who can be told the value is wrong — silently rewriting it to loopback + * would look like the bind succeeded on the address they asked for. + */ +function blankHostnameError(value: unknown): string | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const hostname = (value as Record).hostname; + if (hostname === undefined) return null; + if (typeof hostname !== "string" || !hostname.trim()) { + return "schema_invalid: hostname: must be a nonblank bind address"; + } + return null; +} + /** Validate an in-memory config candidate without touching disk. Used by headless CLI import/set. */ export function validateConfigCandidate(value: unknown): { ok: true; config: OcxConfig } | { ok: false; error: string } { + const hostnameError = blankHostnameError(value); + if (hostnameError) return { ok: false, error: hostnameError }; const result = configSchema.safeParse(value); if (result.success) return { ok: true, config: result.data as OcxConfig }; return { ok: false, error: schemaDiagnosticsError(result.error) }; diff --git a/tests/config.test.ts b/tests/config.test.ts index 7b1e75b39..2dd18b174 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -123,6 +123,46 @@ describe("opencodex config defaults", () => { }); }); + test("a blank hostname already on disk degrades without wiping providers or keys", () => { + // Regression: rejecting a blank hostname in the schema made loadConfig fail twice + // (getDefaultConfig() has no hostname key, so the merge-defaults repair cannot fix + // one), which backed the file up and returned defaults — resetting providers and + // apiKeys for exactly the users the blank-hostname hardening was meant to protect. + writeConfig({ + port: 12345, + hostname: "", + defaultProvider: "custom", + providers: { custom: { adapter: "openai-chat", baseUrl: "https://example.test/v1", apiKey: "upstream-secret" } }, + apiKeys: [{ id: "key-1", name: "default", key: "ocx_persisted", createdAt: "2026-07-28T00:00:00.000Z" }], + }); + + const config = loadConfig(); + + expect(config.hostname).toBeUndefined(); + expect(config).toMatchObject({ + port: 12345, + defaultProvider: "custom", + providers: { custom: { baseUrl: "https://example.test/v1", apiKey: "upstream-secret" } }, + apiKeys: [expect.objectContaining({ id: "key-1", key: "ocx_persisted" })], + }); + expect(backupNames()).toEqual([]); + }); + + test("a whitespace hostname on disk is treated the same as a blank one", () => { + writeConfig({ + port: 12345, + hostname: " ", + defaultProvider: "custom", + providers: { custom: { adapter: "openai-chat", baseUrl: "https://example.test/v1" } }, + }); + + const config = loadConfig(); + + expect(config.hostname).toBeUndefined(); + expect(config.providers.custom.baseUrl).toBe("https://example.test/v1"); + expect(backupNames()).toEqual([]); + }); + test("Codex shim auto-restore defaults on with config and environment opt-out precedence", () => { expect(getDefaultConfig().codexShimAutoRestore).toBe(true); expect(codexShimAutoRestoreEnabled({}, {})).toBe(true); From 7ce68d1035daef26c0a64e1a4916e8281f4216b8 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 10:57:55 -0400 Subject: [PATCH 10/14] fix(update): allow npm below trusted cwd ancestors --- src/update/npm-invocation.mjs | 24 ++++++++++++------ tests/update-npm-invocation.test.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/update/npm-invocation.mjs b/src/update/npm-invocation.mjs index 437ee1282..38abb375f 100644 --- a/src/update/npm-invocation.mjs +++ b/src/update/npm-invocation.mjs @@ -12,13 +12,21 @@ function escapeCmdCommand(command) { return command.replace(CMD_META, "^$1"); } -function isInside(root, candidate) { - const relative = win32.relative(win32.resolve(root), win32.resolve(candidate)); - return relative === "" || ( - relative !== ".." - && !relative.startsWith(`..${win32.sep}`) - && !win32.isAbsolute(relative) - ); +/** + * Whether a PATH entry *is* the current directory. The hijack this guards against is + * cmd.exe resolving a bare `npm` out of the directory opencodex was launched from, so + * only that exact directory has to be skipped — every candidate we hand to spawn is an + * absolute path, which is what actually defeats the implicit cwd-first search. + * + * Deliberately not a subtree test: npm's default Windows global prefix is + * `%AppData%\npm` (`C:\Users\x\AppData\Roaming\npm`), so excluding everything under the + * cwd would fail closed for anyone whose shell sits in their home directory — a normal + * setup, not the untrusted-project case this hardening is for. + */ +function isCurrentDirectory(cwd, entry) { + const left = win32.resolve(entry); + const right = win32.resolve(cwd); + return left.toLowerCase() === right.toLowerCase(); } function cleanPathEntry(entry) { @@ -45,9 +53,9 @@ export function resolveNpmCommand( for (const entry of pathEntries) { if (!win32.isAbsolute(entry)) continue; + if (isCurrentDirectory(cwd, entry)) continue; for (const extension of extensions) { const candidate = win32.join(entry, `npm${extension.toLowerCase()}`); - if (isInside(cwd, candidate)) continue; if (exists(candidate)) return win32.resolve(candidate); } } diff --git a/tests/update-npm-invocation.test.ts b/tests/update-npm-invocation.test.ts index 7530b9602..6a0956906 100644 --- a/tests/update-npm-invocation.test.ts +++ b/tests/update-npm-invocation.test.ts @@ -37,6 +37,45 @@ describe("Windows npm update invocation", () => { expect(String(invocation?.args.at(-1) ?? "").includes(cwd)).toBe(false); }); + test("resolves the default global npm prefix when the cwd is its ancestor", () => { + // Regression: excluding the whole cwd subtree (rather than the cwd itself) hid npm's + // default Windows global prefix `%AppData%\npm` from anyone whose shell sits in their + // home directory, silently failing updates closed in a normal setup. + const home = "C:\\Users\\dev"; + const appDataNpm = `${home}\\AppData\\Roaming\\npm\\npm.cmd`; + const env = { + PATH: `${home}\\AppData\\Roaming\\npm`, + PATHEXT: ".CMD", + SystemRoot: "C:\\Windows", + }; + + expect(resolveNpmCommand("win32", env, { + cwd: home, + exists: path => path === appDataNpm, + })).toBe(appDataNpm); + }); + + test("still skips the current directory when it is a PATH entry under the home tree", () => { + // The narrower rule must not lose the actual defense: a PATH entry equal to the + // launch directory stays excluded even though it sits inside the user's home. + const home = "C:\\Users\\dev"; + const project = `${home}\\untrusted`; + const env = { + PATH: `${project};${home}\\AppData\\Roaming\\npm`, + PATHEXT: ".CMD", + SystemRoot: "C:\\Windows", + }; + const existing = new Set([ + `${project}\\npm.cmd`, + `${home}\\AppData\\Roaming\\npm\\npm.cmd`, + ]); + + expect(resolveNpmCommand("win32", env, { + cwd: project, + exists: path => existing.has(path), + })).toBe(`${home}\\AppData\\Roaming\\npm\\npm.cmd`); + }); + test("fails closed when npm is available only from the current directory", () => { const env = { PATH: `${cwd};.`, From c372771dc301303415e708dd5f8c84194b371b9e Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 12:15:38 -0400 Subject: [PATCH 11/14] feat(security): split management and data credentials --- gui/src/api.ts | 33 +- gui/tests/api-auth-memory.test.ts | 7 +- gui/tests/models-empty-provider.test.tsx | 2 +- src/cli/claude.ts | 6 +- src/cli/doctor.ts | 6 +- src/lib/admin-secrets.ts | 25 + src/lib/process-control.ts | 4 +- src/oauth/login-cli.ts | 5 +- src/server/auth-cors.ts | 101 +++- src/server/gui-static.ts | 32 +- src/server/index.ts | 28 +- src/server/management-api.ts | 4 +- src/server/management-auth.ts | 216 +++++++++ src/server/management/oauth-account-routes.ts | 2 +- src/server/management/system-routes.ts | 2 +- structure/05_gui-and-management-api.md | 39 ++ tests/account-pool-management-api.test.ts | 1 + tests/api-debug.test.ts | 1 + tests/api-storage-cleanup.test.ts | 1 + tests/api-storage-policy.test.ts | 1 + tests/api-storage.test.ts | 1 + tests/api-usage.test.ts | 1 + tests/autostart-health.test.ts | 1 + tests/claude-desktop-config-path.test.ts | 1 + tests/claude-management-api.test.ts | 1 + tests/claude-messages-endpoint.test.ts | 1 + tests/claude-native-passthrough.test.ts | 1 + tests/cli-management-auth.test.ts | 97 ++++ tests/codex-catalog.test.ts | 1 + tests/codex-sync-api.test.ts | 3 +- tests/codex-v2-gate.test.ts | 1 + tests/combo-management-api.test.ts | 1 + tests/cursor-hardening.test.ts | 1 + tests/effort-policy.test.ts | 1 + tests/forward-admission-separation.test.ts | 132 ++++++ tests/grok-management-api.test.ts | 1 + tests/gui-management-session.test.ts | 62 +++ tests/helpers/management-auth.ts | 43 ++ tests/injection-model-api.test.ts | 1 + tests/management-api-logs-metrics.test.ts | 1 + tests/management-provider-validation.test.ts | 1 + tests/memory-watchdog.test.ts | 1 + tests/model-visibility-management-api.test.ts | 1 + tests/native-model-toggle.test.ts | 1 + tests/oauth-accounts-api.test.ts | 1 + tests/oauth-login-cli-live-update.test.ts | 1 + tests/oauth-manual-code.test.ts | 1 + tests/oauth-public-surface.test.ts | 1 + tests/oauth-reauth-bind.test.ts | 1 + tests/openai-api-virtual-models.test.ts | 7 +- tests/openai-provider-option-e2e.test.ts | 1 + tests/opencode-cli.test.ts | 1 + tests/process-control-graceful.test.ts | 9 +- tests/provider-api-keys.test.ts | 1 + tests/provider-connection-test.test.ts | 1 + tests/server-403-permission-e2e.test.ts | 1 + tests/server-auth.test.ts | 39 +- tests/server-combo-failover-e2e.test.ts | 1 + tests/server-management-auth.test.ts | 439 ++++++++++++++++++ tests/settings-stream-mode.test.ts | 1 + tests/startup-action-control.test.ts | 1 + tests/storage-mutation-race.test.ts | 1 + tests/storage-policy-job-responsive.test.ts | 1 + tests/storage-restore-job-responsive.test.ts | 1 + tests/subagent-model-fallback-api.test.ts | 1 + tests/system-restart.test.ts | 1 + tests/vision-anthropic.test.ts | 1 + tests/windows-tray.test.ts | 1 + 68 files changed, 1317 insertions(+), 69 deletions(-) create mode 100644 src/lib/admin-secrets.ts create mode 100644 src/server/management-auth.ts create mode 100644 tests/cli-management-auth.test.ts create mode 100644 tests/forward-admission-separation.test.ts create mode 100644 tests/gui-management-session.test.ts create mode 100644 tests/helpers/management-auth.ts create mode 100644 tests/server-management-auth.test.ts diff --git a/gui/src/api.ts b/gui/src/api.ts index fd6a777bb..2c6d05a14 100644 --- a/gui/src/api.ts +++ b/gui/src/api.ts @@ -7,7 +7,7 @@ function needsApiAuth(input: RequestInfo | URL): boolean { const url = new URL(raw, window.location.href); // Absolute cross-origin URLs must never get the local API token or 401 prompt. if (url.origin !== window.location.origin) return false; - return url.pathname.startsWith("/api/") || url.pathname.startsWith("/v1/"); + return url.pathname.startsWith("/api/"); } catch { return false; } @@ -18,6 +18,8 @@ const LEGACY_TOKEN_KEY = "opencodex-api-token"; /** In-memory only — never write tokens to web storage (XSS can read sessionStorage/localStorage). */ let memoryToken: string | null = null; +let memoryCsrfToken: string | null = null; +let memorySessionOrigin: string | null = null; function readToken(): string | null { return memoryToken; @@ -29,6 +31,25 @@ function storeToken(token: string): void { function clearToken(): void { memoryToken = null; + memoryCsrfToken = null; + memorySessionOrigin = null; +} + +function takeMetaContent(name: string): string | null { + const element = document.querySelector(`meta[name="${name}"]`) as HTMLMetaElement | null; + const content = element?.content.trim() || null; + element?.remove(); + return content; +} + +function loadInjectedSession(): void { + const token = takeMetaContent("opencodex-session-token"); + const csrfToken = takeMetaContent("opencodex-session-csrf"); + const origin = takeMetaContent("opencodex-session-origin"); + if (!token?.startsWith("ocx_session_") || !csrfToken || origin !== window.location.origin) return; + memoryToken = token; + memoryCsrfToken = csrfToken; + memorySessionOrigin = origin; } function clearLegacySessionToken(): void { @@ -42,6 +63,13 @@ function clearLegacySessionToken(): void { function withToken(input: RequestInfo | URL, init: RequestInit | undefined, token: string): [RequestInfo | URL, RequestInit | undefined] { const headers = new Headers(init?.headers ?? (input instanceof Request ? input.headers : undefined)); headers.set("X-OpenCodex-API-Key", token); + if (memorySessionOrigin && memoryCsrfToken && token.startsWith("ocx_session_")) { + headers.set("X-OpenCodex-GUI-Origin", memorySessionOrigin); + const method = (init?.method ?? (input instanceof Request ? input.method : "GET")).toUpperCase(); + if (method !== "GET" && method !== "HEAD") { + headers.set("X-OpenCodex-CSRF-Token", memoryCsrfToken); + } + } if (input instanceof Request) return [new Request(input, { headers }), init ? { ...init, headers } : undefined]; return [input, { ...init, headers }]; } @@ -59,6 +87,7 @@ export function installApiAuthFetch(): void { installed = true; // Drop any leftover XSS-readable token; new tokens stay memory-only (no read/migrate). clearLegacySessionToken(); + loadInjectedSession(); const originalFetch = window.fetch.bind(window); window.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { if (!needsApiAuth(input)) return originalFetch(input, init); @@ -84,5 +113,7 @@ export function installApiAuthFetch(): void { export function resetApiAuthFetchForTests(): void { installed = false; memoryToken = null; + memoryCsrfToken = null; + memorySessionOrigin = null; promptInFlight = null; } diff --git a/gui/tests/api-auth-memory.test.ts b/gui/tests/api-auth-memory.test.ts index 4d27d8600..0b44d304b 100644 --- a/gui/tests/api-auth-memory.test.ts +++ b/gui/tests/api-auth-memory.test.ts @@ -113,7 +113,7 @@ test("cross-origin /api/* requests do not receive the API key or token prompt", expect(promptCalls).toBe(beforeCrossPrompts); }); -test("cross-origin /v1/* requests do not receive the API key or token prompt", async () => { +test("data-plane requests never receive the management token or prompt", async () => { let promptCalls = 0; let phase: "seed" | "cross" = "seed"; const seenHeaders: Array = []; @@ -132,8 +132,9 @@ test("cross-origin /v1/* requests do not receive the API key or token prompt", a }; await installMockAuthFetch(stateful); - expect((await fetch("/v1/models")).status).toBe(200); - expect(promptCalls).toBe(1); + expect((await fetch("/v1/models")).status).toBe(401); + expect(seenHeaders).toEqual([null]); + expect(promptCalls).toBe(0); phase = "cross"; const beforeCrossPrompts = promptCalls; diff --git a/gui/tests/models-empty-provider.test.tsx b/gui/tests/models-empty-provider.test.tsx index d7cfdc106..37902595b 100644 --- a/gui/tests/models-empty-provider.test.tsx +++ b/gui/tests/models-empty-provider.test.tsx @@ -52,7 +52,7 @@ async function providerDto( ): Promise> { const requestUrl = new URL("http://127.0.0.1/api/providers"); const response = await handleManagementAPI( - new Request(requestUrl), + new Request(requestUrl, { headers: { Host: requestUrl.host } }), requestUrl, { providers: { diff --git a/src/cli/claude.ts b/src/cli/claude.ts index 14a165f91..e8dc359a1 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -14,6 +14,7 @@ import { refreshGatewayModelCacheFromProxy } from "../claude/gateway-cache"; import { commandInvocation } from "../lib/win-exec"; import { findLiveProxy } from "../server/proxy-liveness"; import type { OcxConfig } from "../types"; +import { configuredAdminToken } from "../lib/admin-secrets"; import { PROXY_MARKER, ownAdmissionTokens, defaultAuthDetectDeps, detectClaudeAuth, type AuthDetectDeps } from "../claude/auth-detect"; import { resolveClaudeAuthMode } from "../claude/auth-mode"; @@ -149,14 +150,13 @@ export function buildClaudeEnv( /** * Context-window map from the RUNNING proxy's management API (warm TTL cache; the - * daemon registers every selector form — audit R3#1). 3s bound + auth header - * (OPENCODEX_API_AUTH_TOKEN first, config key fallback — audit R4#1). Failure → {} + * daemon registers every selector form — audit R3#1). 3s bound + management auth header. * (no [1m] marking, conservative). */ export async function fetchClaudeContextWindows(config: OcxConfig, port: number, timeoutMs = 3_000): Promise> { try { const headers = new Headers(); - const token = process.env.OPENCODEX_API_AUTH_TOKEN || config.apiKeys?.[0]?.key; + const token = configuredAdminToken(); if (token) headers.set("x-opencodex-api-key", token); const res = await fetch(`http://127.0.0.1:${port}/api/claude-code`, { headers, diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index f4a28361a..bfb902119 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -13,7 +13,7 @@ import { dirname, join } from "node:path"; import { getConfigDir, getConfigPath, readConfigDiagnostics, readPid, readRuntimePort, resolveEnvValue } from "../config"; import { gracefulStopHost } from "../lib/process-control"; import { maskAccountId } from "../lib/privacy"; -import { loadServiceTokenFromFile } from "../lib/service-secrets"; +import { configuredAdminToken } from "../lib/admin-secrets"; import { readCodexTokens } from "../codex/auth-collision"; import { collectOrcaCodexHomeDiagnostic, resolveCodexHomeDir as resolveCodexHomeDirImpl, isWslRuntime, listWslWindowsCodexHomes, wslAutomountRoot, type CodexHomeDeps } from "../codex/home"; import { findCodexOnPath, isWindowsInteropDir } from "../codex/shim"; @@ -571,7 +571,7 @@ export function formatServiceMemoryLines(report: ServiceMemoryReport): string[] const lines: string[] = []; lines.push(` -- doctor process Bun ${Bun.version} (this is NOT the service process)`); if (report.status === "unauthorized") { - lines.push(" -- proxy reachable but rejected the request — set OPENCODEX_API_AUTH_TOKEN to match the service"); + lines.push(" -- proxy reachable but rejected the request — set OPENCODEX_ADMIN_AUTH_TOKEN to match the service"); return lines; } if (report.status === "unreachable") { @@ -756,7 +756,7 @@ export async function runDoctor(args: string[] = []): Promise { console.log(` -- doctor process Bun ${Bun.version} (this is NOT the service process)`); console.log(" -- no running ocx proxy found (no live pid/runtime record)"); } else { - const token = process.env.OPENCODEX_API_AUTH_TOKEN ?? loadServiceTokenFromFile(process.env); + const token = configuredAdminToken(); const report = await fetchServiceMemory(gracefulStopHost(runtime.hostname), runtime.port, token); for (const line of formatServiceMemoryLines(report)) console.log(line); } diff --git a/src/lib/admin-secrets.ts b/src/lib/admin-secrets.ts new file mode 100644 index 000000000..e0adbb92d --- /dev/null +++ b/src/lib/admin-secrets.ts @@ -0,0 +1,25 @@ +import { lstatSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { getConfigDir } from "../config"; + +export const ADMIN_TOKEN_FILE = "admin-api-token"; + +export function adminApiTokenFilePath(configDir = getConfigDir()): string { + return join(configDir, ADMIN_TOKEN_FILE); +} + +export function loadAdminTokenFromFile(configDir = getConfigDir()): string | null { + const path = adminApiTokenFilePath(configDir); + try { + const stat = lstatSync(path); + if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 512) return null; + const token = readFileSync(path, "utf8").trim(); + return /^ocx_admin_[A-Za-z0-9_-]{43}$/.test(token) ? token : null; + } catch { + return null; + } +} + +export function configuredAdminToken(configDir = getConfigDir(), env: NodeJS.ProcessEnv = process.env): string | null { + return env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim() || loadAdminTokenFromFile(configDir); +} diff --git a/src/lib/process-control.ts b/src/lib/process-control.ts index 1a1926b1d..f55028460 100644 --- a/src/lib/process-control.ts +++ b/src/lib/process-control.ts @@ -1,5 +1,6 @@ import { execFileSync } from "node:child_process"; import { loadConfig, readRuntimePort } from "../config"; +import { configuredAdminToken } from "./admin-secrets"; export function isProcessAlive(pid: number): boolean { try { @@ -66,8 +67,7 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): if (!runtime?.port) return false; const env = io.env ?? process.env; const headers: Record = {}; - // Non-loopback binds require management auth; loopback ignores the extra header. - const token = env.OPENCODEX_API_AUTH_TOKEN?.trim(); + const token = configuredAdminToken(env.OPENCODEX_HOME?.trim() || undefined, env as NodeJS.ProcessEnv); if (token) headers["x-opencodex-api-key"] = token; const fetchFn = io.fetchFn ?? fetch; try { diff --git a/src/oauth/login-cli.ts b/src/oauth/login-cli.ts index cf225b9ce..127652df6 100644 --- a/src/oauth/login-cli.ts +++ b/src/oauth/login-cli.ts @@ -5,11 +5,12 @@ import { findLiveProxy, probeHostname } from "../server/proxy-liveness"; import { isPublicOAuthProvider, listOAuthProviders, runLogin } from "./index"; import { KEY_LOGIN_PROVIDERS, isKeyLoginProvider, validateApiKey, type KeyLoginProvider } from "./key-providers"; import type { OcxProviderConfig } from "../types"; +import { configuredAdminToken } from "../lib/admin-secrets"; export function runningProxyUpdateHeaders(): Headers { const headers = new Headers({ "Content-Type": "application/json" }); - const apiToken = process.env.OPENCODEX_API_AUTH_TOKEN?.trim(); - if (apiToken) headers.set("X-OpenCodex-API-Key", apiToken); + const adminToken = configuredAdminToken(); + if (adminToken) headers.set("X-OpenCodex-API-Key", adminToken); return headers; } diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 29b9e61d5..90a00f7ff 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -88,6 +88,27 @@ export function isAllowedRequestOrigin(req: Request, config: OcxConfig): boolean return !origin || isLoopbackOriginValue(origin) || isSameOriginAsRequest(req, origin) || isExtraAllowedOrigin(origin, config); } +export function managementRequestOrigin(req: Request, config: OcxConfig): string | null { + const host = req.headers.get("Host"); + const parsedHost = parseHttpHost(host); + if (!host || !parsedHost) return null; + if (!isApiAuthRequired(config) && !isLoopbackHostname(parsedHost.hostname)) return null; + try { + const protocol = new URL(req.url).protocol; + if (protocol !== "http:" && protocol !== "https:") return null; + return new URL(`${protocol}//${host}`).origin; + } catch { + return null; + } +} + +export function isAllowedManagementOrigin(req: Request, config: OcxConfig): boolean { + const requestOrigin = managementRequestOrigin(req, config); + if (!requestOrigin) return false; + const origin = req.headers.get("Origin"); + return !origin || origin === requestOrigin; +} + export function browserSecurityHeaders(): Record { return { "X-Frame-Options": "DENY", @@ -110,6 +131,15 @@ export function corsHeaders(req?: Request, config?: OcxConfig): Record { + const headers = corsHeaders(); + const origin = req?.headers.get("Origin"); + if (origin && req && config && isAllowedManagementOrigin(req, config)) { + headers["Access-Control-Allow-Origin"] = origin; + } + return headers; +} + export function withCors(response: Response, req: Request, config: OcxConfig): Response { const headers = new Headers(response.headers); for (const [name, value] of Object.entries(corsHeaders(req, config))) { @@ -122,6 +152,18 @@ export function withCors(response: Response, req: Request, config: OcxConfig): R }); } +export function withManagementCors(response: Response, req: Request, config: OcxConfig): Response { + const headers = new Headers(response.headers); + for (const [name, value] of Object.entries(managementCorsHeaders(req, config))) { + headers.set(name, value); + } + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + }); +} + export function jsonResponse(data: unknown, status = 200, req?: Request, config?: OcxConfig): Response { return new Response(JSON.stringify(data), { status, @@ -134,6 +176,11 @@ export function configuredApiAuthToken(_config: OcxConfig): string | undefined { return token || undefined; } +export function configuredAdminAuthToken(): string | undefined { + const token = process.env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim(); + return token || undefined; +} + export function isLoopbackHostname(hostname: string | undefined): boolean { // A fully-qualified "localhost." is the same host as "localhost": curl and some clients // send the trailing dot verbatim, and refusing it 403s a legitimate loopback caller. @@ -146,31 +193,48 @@ export function isApiAuthRequired(config: OcxConfig): boolean { } export function assertServerAuthConfig(config: OcxConfig): void { - if (isApiAuthRequired(config) && !configuredApiAuthToken(config)) { - throw new Error("OPENCODEX_API_AUTH_TOKEN is required when binding opencodex to a non-loopback hostname"); + const hasConfiguredDataCredential = !!configuredApiAuthToken(config) + || (config.apiKeys ?? []).some(entry => !!entry.key.trim()); + if (isApiAuthRequired(config) && !hasConfiguredDataCredential) { + throw new Error( + "A data-plane credential (OPENCODEX_API_AUTH_TOKEN or config.apiKeys) is required when binding opencodex to a non-loopback hostname", + ); } } -/** Whether `token` is one of the proxy's own admission secrets (env token or config API keys). */ -export function isProxyAdmissionSecret(token: string, config: OcxConfig): boolean { - const actual = token.trim(); - if (!actual) return false; +function secretEquals(actual: string, expected: string | undefined): boolean { + if (!expected) return false; const enc = new TextEncoder(); const actualBytes = enc.encode(actual); - // Check env-based token - const expected = configuredApiAuthToken(config); - if (expected) { - const expectedBytes = enc.encode(expected); - if (expectedBytes.length === actualBytes.length && timingSafeEqual(actualBytes, expectedBytes)) return true; - } - // Check config-based API keys + const expectedBytes = enc.encode(expected); + return expectedBytes.length === actualBytes.length && timingSafeEqual(actualBytes, expectedBytes); +} + +/** Whether `token` is a data-plane admission secret. */ +export function isDataPlaneAdmissionSecret(token: string, config: OcxConfig): boolean { + const actual = token.trim(); + if (!actual) return false; + if (secretEquals(actual, configuredApiAuthToken(config))) return true; for (const k of config.apiKeys ?? []) { - const keyBytes = enc.encode(k.key); - if (keyBytes.length === actualBytes.length && timingSafeEqual(actualBytes, keyBytes)) return true; + if (secretEquals(actual, k.key)) return true; } return false; } +/** Whether `token` is the environment-provided management secret. */ +export function isManagementAdmissionSecret(token: string): boolean { + const actual = token.trim(); + return !!actual && secretEquals(actual, configuredAdminAuthToken()); +} + +/** Whether `token` is one of the proxy's own admission secrets and must never reach an upstream. */ +export function isProxyAdmissionSecret(token: string, config: OcxConfig): boolean { + const actual = token.trim(); + if (!actual) return false; + if (/^ocx_(?:data|admin|session)_/.test(actual) || /^ocx_[0-9a-f]{40}$/.test(actual)) return true; + return isDataPlaneAdmissionSecret(actual, config) || isManagementAdmissionSecret(actual); +} + export class ForwardAdmissionCredentialError extends Error { constructor() { super("OpenCodex admission credentials cannot be forwarded upstream"); @@ -190,12 +254,11 @@ export function hasValidApiAuth(req: Request, config: OcxConfig): boolean { // Anthropic-SDK clients (Claude Code with ANTHROPIC_API_KEY) authenticate via x-api-key. || req.headers.get("x-api-key")?.trim(); if (!actual) return false; - return isProxyAdmissionSecret(actual, config); + return isDataPlaneAdmissionSecret(actual, config); } -export function requireApiAuth(req: Request, config: OcxConfig, kind: "management" | "data-plane"): Response | null { +export function requireApiAuth(req: Request, config: OcxConfig, _kind: "data-plane"): Response | null { if (hasValidApiAuth(req, config)) return null; - if (kind === "management") return jsonResponse({ error: "opencodex API key required" }, 401); return formatErrorResponse(401, "authentication_error", "opencodex API key required"); } @@ -207,7 +270,7 @@ export function requireApiAuth(req: Request, config: OcxConfig, kind: "managemen export function requireResponsesApiAuth(req: Request, config: OcxConfig): Response | null { if (!isApiAuthRequired(config)) return null; const actual = req.headers.get("x-opencodex-api-key")?.trim(); - if (actual && isProxyAdmissionSecret(actual, config)) return null; + if (actual && isDataPlaneAdmissionSecret(actual, config)) return null; return formatErrorResponse(401, "authentication_error", "opencodex API key required"); } diff --git a/src/server/gui-static.ts b/src/server/gui-static.ts index 4542eaa41..e314c1582 100644 --- a/src/server/gui-static.ts +++ b/src/server/gui-static.ts @@ -1,6 +1,7 @@ import { existsSync, readFileSync, statSync } from "node:fs"; import { extname, isAbsolute, join, relative, resolve } from "node:path"; import { browserSecurityHeaders } from "./auth-cors"; +import type { GuiSessionBootstrap } from "./management-auth"; /** opencodex version, read from the packaged package.json (same source as the server bootstrap). */ const VERSION = (() => { @@ -55,7 +56,31 @@ function isFile(path: string): boolean { } } -export function serveGuiFile(pathname: string, guiDist = findGuiDist()): Response | null { +function htmlResponse(path: string, session?: GuiSessionBootstrap): Response { + let html = readFileSync(path, "utf8"); + if (session) { + const bootstrap = [ + ``, + ``, + ``, + ].join(""); + html = html.includes("") ? html.replace("", `${bootstrap}`) : `${bootstrap}${html}`; + } + return new Response(html, { + headers: { + "Content-Type": "text/html", + "Cache-Control": "no-store", + Pragma: "no-cache", + ...browserSecurityHeaders(), + }, + }); +} + +export function serveGuiFile( + pathname: string, + guiDist = findGuiDist(), + session?: GuiSessionBootstrap, +): Response | null { if (!guiDist) return null; const filePath = resolveGuiFilePath(guiDist, pathname); if (!filePath) return null; @@ -64,9 +89,7 @@ export function serveGuiFile(pathname: string, guiDist = findGuiDist()): Respons if (!extname(pathname)) { const indexPath = join(guiDist, "index.html"); if (isFile(indexPath)) { - return new Response(Bun.file(indexPath), { - headers: { "Content-Type": "text/html", ...browserSecurityHeaders() }, - }); + return htmlResponse(indexPath, session); } } return null; @@ -74,6 +97,7 @@ export function serveGuiFile(pathname: string, guiDist = findGuiDist()): Respons const ext = extname(filePath); const contentType = MIME_TYPES[ext] || "application/octet-stream"; + if (ext === ".html") return htmlResponse(filePath, session); return new Response(Bun.file(filePath), { headers: { "Content-Type": contentType, ...browserSecurityHeaders() }, }); diff --git a/src/server/index.ts b/src/server/index.ts index ba973ffc1..0df808b2a 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -104,8 +104,10 @@ export { import { assertServerAuthConfig, corsHeaders, + managementCorsHeaders, hasValidApiAuth, isAllowedRequestOrigin, + isAllowedManagementOrigin, isApiAuthRequired, isLoopbackHostname, jsonResponse, @@ -114,6 +116,7 @@ import { safeConfigDTO, setCorsOrigin, withCors, + withManagementCors, } from "./auth-cors"; export { assertServerAuthConfig, @@ -135,6 +138,7 @@ import { handleImages } from "./images"; import { handleLive, logLiveSidebandFrame, parseLiveSidebandTarget, resolveLiveSidebandUpgrade } from "./live"; import { handleSearch } from "./search"; import { fetchAllModels, handleManagementAPI, VERSION } from "./management-api"; +import { initializeManagementAuthState, issueGuiSession, requireManagementAuth } from "./management-auth"; const MAX_WS_FRAME_BYTES = 50 * 1024 * 1024; const WEBSOCKET_IDLE_TIMEOUT_SECONDS = 0; @@ -250,6 +254,7 @@ export function startServer(port?: number) { const config = runAlibabaRegionStartupMigration(runOpenAiTierStartupMigration(loadConfig())); applyProxyEnv(config); assertServerAuthConfig(config); + const managementAuth = initializeManagementAuthState(config); // Refresh OAuth provider presets (models/noReasoningModels) from the registry so a proxy update // adding/dropping models reaches existing configs on start — not just fresh installs. reconcileOAuthProviders(config); @@ -338,10 +343,17 @@ export function startServer(port?: number) { markActivity(`${req.method} ${url.pathname}`); if (req.method === "OPTIONS") { - if (!isAllowedRequestOrigin(req, config)) { + const managementPreflight = url.pathname.startsWith("/api/"); + const allowed = managementPreflight + ? isAllowedManagementOrigin(req, config) + : isAllowedRequestOrigin(req, config); + if (!allowed) { return new Response(null, { status: 403, headers: corsHeaders() }); } - return new Response(null, { status: 204, headers: corsHeaders(req, config) }); + return new Response(null, { + status: 204, + headers: managementPreflight ? managementCorsHeaders(req, config) : corsHeaders(req, config), + }); } // Responses WebSocket (phase 120.2). Codex upgrades the same /v1/responses path; auth is @@ -377,10 +389,11 @@ export function startServer(port?: number) { } if (url.pathname.startsWith("/api/")) { - const apiAuthError = requireApiAuth(req, config, "management"); - if (apiAuthError) return withCors(apiAuthError, req, config); + const apiAuthError = requireManagementAuth(req, managementAuth, config); + if (apiAuthError) return withManagementCors(apiAuthError, req, config); const mgmtResponse = await handleManagementAPI(req, url, config); - if (mgmtResponse) return withCors(mgmtResponse, req, config); + if (mgmtResponse) return withManagementCors(mgmtResponse, req, config); + return withManagementCors(formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${url.pathname}`), req, config); } if (url.pathname === "/v1/models" && req.method === "GET") { @@ -724,7 +737,10 @@ export function startServer(port?: number) { return withCors(formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${url.pathname}`), req, config); } - const guiFile = serveGuiFile(url.pathname); + const guiSessionCandidate = req.method === "GET" && (url.pathname === "/" || !url.pathname.includes(".")) + ? issueGuiSession(req, config, managementAuth) + : null; + const guiFile = serveGuiFile(url.pathname, undefined, guiSessionCandidate ?? undefined); if (guiFile) return guiFile; if (url.pathname === "/" && req.method === "GET") { return jsonResponse(rootFallbackPayload()); diff --git a/src/server/management-api.ts b/src/server/management-api.ts index 52e2021b3..ccf9e1908 100644 --- a/src/server/management-api.ts +++ b/src/server/management-api.ts @@ -53,7 +53,7 @@ import { drainAndShutdown } from "./lifecycle"; import { filterRequestLogs, getRequestLogEntries, type RequestLogEntry } from "./request-log"; import { estimateComboCost, estimateRequestCost, normalizeCostTokens, tokensPerSecond } from "../usage/cost"; import type { PersistedUsageAttempt } from "../usage/log"; -import { isAllowedRequestOrigin, jsonResponse, providerManagementConfigError, publicProviderBaseUrl, safeConfigDTO } from "./auth-cors"; +import { isAllowedManagementOrigin, jsonResponse, providerManagementConfigError, publicProviderBaseUrl, safeConfigDTO } from "./auth-cors"; import { applySystemEnvToggle } from "./system-env"; import type { ManagementApiDeps } from "./management/context"; @@ -79,7 +79,7 @@ export const VERSION = (() => { })(); export async function handleManagementAPI(req: Request, url: URL, config: OcxConfig, deps: ManagementApiDeps = {}): Promise { - if (!isAllowedRequestOrigin(req, config)) { + if (!isAllowedManagementOrigin(req, config)) { return jsonResponse({ error: "cross-origin request blocked" }, 403, req, config); } // Management bodies are small JSON (provider names, key ids, settings). Reject oversized diff --git a/src/server/management-auth.ts b/src/server/management-auth.ts new file mode 100644 index 000000000..52fd46401 --- /dev/null +++ b/src/server/management-auth.ts @@ -0,0 +1,216 @@ +import { randomBytes, randomUUID, timingSafeEqual } from "node:crypto"; +import { + chmodSync, + closeSync, + fsyncSync, + linkSync, + lstatSync, + mkdirSync, + openSync, + readFileSync, + unlinkSync, + writeFileSync, +} from "node:fs"; +import { dirname, join } from "node:path"; +import { adminApiTokenFilePath } from "../lib/admin-secrets"; +import { hardenSecretDir, hardenSecretPath } from "../lib/windows-secret-acl"; +import type { OcxConfig } from "../types"; +import { + isAllowedManagementOrigin, + isApiAuthRequired, + isDataPlaneAdmissionSecret, + isLoopbackHostname, + managementRequestOrigin, + parseHttpHost, +} from "./auth-cors"; + +const GUI_SESSION_TTL_MS = 5 * 60_000; +const GUI_SESSION_LIMIT = 128; + +interface GuiSessionRecord { + csrfToken: string; + origin: string; + expiresAt: number; +} + +export interface GuiSessionBootstrap extends GuiSessionRecord { + token: string; +} + +export type ManagementAuthState = + | { + available: true; + token: string; + source: "environment" | "file"; + sessions: Map; + } + | { available: false; reason: string }; + +function fail(reason: string): ManagementAuthState { + return { available: false, reason }; +} + +function assertSafeDirectory(path: string): void { + mkdirSync(path, { recursive: true, mode: 0o700 }); + const stat = lstatSync(path); + if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error("management token directory is not a regular directory"); + chmodSync(path, 0o700); + const hardened = hardenSecretDir(path, { required: true }); + if (!hardened.ok) throw new Error("management token directory ACL hardening did not complete"); +} + +function readExistingToken(path: string): string { + const stat = lstatSync(path); + if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 512) { + throw new Error("management token path is not a regular secret file"); + } + chmodSync(path, 0o600); + const hardened = hardenSecretPath(path, { required: true }); + if (!hardened.ok) throw new Error("management token file ACL hardening did not complete"); + const token = readFileSync(path, "utf8").trim(); + if (!/^ocx_admin_[A-Za-z0-9_-]{43}$/.test(token)) throw new Error("management token file is invalid"); + return token; +} + +function removeBestEffort(path: string): void { + try { unlinkSync(path); } catch { /* fail-closed state is preserved by the caller */ } +} + +function createTokenFile(path: string): string { + const directory = dirname(path); + const token = `ocx_admin_${randomBytes(32).toString("base64url")}`; + const temporary = join(directory, `.${randomUUID()}.admin-token.tmp`); + let linked = false; + let fd: number | null = null; + try { + fd = openSync(temporary, "wx", 0o600); + writeFileSync(fd, `${token}\n`, "utf8"); + fsyncSync(fd); + closeSync(fd); + fd = null; + chmodSync(temporary, 0o600); + const temporaryHardened = hardenSecretPath(temporary, { required: true }); + if (!temporaryHardened.ok) throw new Error("management token temporary ACL hardening did not complete"); + try { + linkSync(temporary, path); + linked = true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") return readExistingToken(path); + throw error; + } + const finalHardened = hardenSecretPath(path, { required: true }); + if (!finalHardened.ok) throw new Error("management token file ACL hardening did not complete"); + return token; + } catch (error) { + if (linked) removeBestEffort(path); + throw error; + } finally { + if (fd !== null) { + try { closeSync(fd); } catch { /* best effort */ } + } + removeBestEffort(temporary); + } +} + +function ready(token: string, source: "environment" | "file", config: OcxConfig): ManagementAuthState { + if (isDataPlaneAdmissionSecret(token, config)) { + return fail("management credential conflicts with a data-plane credential"); + } + return { available: true, token, source, sessions: new Map() }; +} + +export function initializeManagementAuthState(config: OcxConfig): ManagementAuthState { + const environmentToken = process.env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim(); + if (environmentToken) { + return ready(environmentToken, "environment", config); + } + try { + const path = adminApiTokenFilePath(); + assertSafeDirectory(dirname(path)); + let token: string; + try { + token = readExistingToken(path); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + token = createTokenFile(path); + } + return ready(token, "file", config); + } catch (error) { + return fail(error instanceof Error ? error.message : "management token initialization failed"); + } +} + +function equalSecret(actual: string, expected: string): boolean { + const encoder = new TextEncoder(); + const left = encoder.encode(actual); + const right = encoder.encode(expected); + return left.length === right.length && timingSafeEqual(left, right); +} + +function removeExpiredSessions(state: Extract, now = Date.now()): void { + for (const [token, session] of state.sessions) { + if (session.expiresAt <= now) state.sessions.delete(token); + } +} + +function randomSessionSecret(prefix: "ocx_session_"): string { + return `${prefix}${randomBytes(32).toString("base64url")}`; +} + +export function issueGuiSession( + req: Request, + config: OcxConfig, + state: ManagementAuthState, +): GuiSessionBootstrap | null { + if (isApiAuthRequired(config) || !state.available || req.method !== "GET" || !isAllowedManagementOrigin(req, config)) return null; + const host = parseHttpHost(req.headers.get("Host")); + if (!host || !isLoopbackHostname(host.hostname)) return null; + const origin = managementRequestOrigin(req, config); + if (!origin) return null; + const now = Date.now(); + removeExpiredSessions(state, now); + while (state.sessions.size >= GUI_SESSION_LIMIT) { + const oldest = state.sessions.keys().next().value as string | undefined; + if (!oldest) break; + state.sessions.delete(oldest); + } + const token = randomSessionSecret("ocx_session_"); + const session: GuiSessionRecord = { + csrfToken: randomBytes(32).toString("base64url"), + origin, + expiresAt: now + GUI_SESSION_TTL_MS, + }; + state.sessions.set(token, session); + return { token, ...session }; +} + +export function requireManagementAuth( + req: Request, + state: ManagementAuthState, + config?: OcxConfig, +): Response | null { + if (!state.available) { + return Response.json({ error: "management API unavailable" }, { status: 503 }); + } + const actual = req.headers.get("x-opencodex-api-key")?.trim() + || req.headers.get("authorization")?.replace(/^Bearer\s+/i, "").trim(); + if (actual && equalSecret(actual, state.token)) return null; + if (actual && config) { + removeExpiredSessions(state); + const session = state.sessions.get(actual); + if (session) { + const requestOrigin = managementRequestOrigin(req, config); + const claimedOrigin = req.headers.get("x-opencodex-gui-origin"); + const browserOrigin = req.headers.get("Origin"); + const sameOrigin = requestOrigin === session.origin + && claimedOrigin === session.origin + && (!browserOrigin || browserOrigin === session.origin); + const safeMethod = req.method === "GET" || req.method === "HEAD"; + const csrf = req.headers.get("x-opencodex-csrf-token")?.trim(); + if (sameOrigin && (safeMethod || (browserOrigin === session.origin && !!csrf && equalSecret(csrf, session.csrfToken)))) { + return null; + } + } + } + return Response.json({ error: "opencodex admin token required" }, { status: 401 }); +} diff --git a/src/server/management/oauth-account-routes.ts b/src/server/management/oauth-account-routes.ts index 70b8c85a7..9a81665b6 100644 --- a/src/server/management/oauth-account-routes.ts +++ b/src/server/management/oauth-account-routes.ts @@ -443,7 +443,7 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const salt = crypto.randomUUID(); const hashInput = `${providerKeys}|${salt}|${Date.now()}`; const hashBuf = new Bun.CryptoHasher("sha256").update(hashInput).digest(); - const key = "ocx_" + Buffer.from(hashBuf).toString("hex").slice(0, 40); + const key = "ocx_data_" + Buffer.from(hashBuf).toString("hex").slice(0, 40); const entry = { id: crypto.randomUUID(), name, key, createdAt: new Date().toISOString() }; config.apiKeys = [...(config.apiKeys ?? []), entry]; saveConfigPreservingClaudeCode(config); diff --git a/src/server/management/system-routes.ts b/src/server/management/system-routes.ts index 00befe58c..86ee52b69 100644 --- a/src/server/management/system-routes.ts +++ b/src/server/management/system-routes.ts @@ -3,7 +3,7 @@ * and the memory-card drain-and-restart action (#563). * * Rides the standard management gate: every /api/* request already passed - * requireApiAuth("management") + the origin check before dispatch, so these + * the independent management-auth gate + the origin check before dispatch, so these * routes add no auth of their own. NEVER expose this data on the * unauthenticated /healthz surface. * diff --git a/structure/05_gui-and-management-api.md b/structure/05_gui-and-management-api.md index 1a8205202..92aa8f603 100644 --- a/structure/05_gui-and-management-api.md +++ b/structure/05_gui-and-management-api.md @@ -10,6 +10,45 @@ All ordinary HTTP responses (excluding successful WebSocket upgrades) include `X dashboard or management responses. Embedding the dashboard in an iframe is intentionally unsupported; deployments that previously relied on such embedding must open it as a top-level page. +## Authentication boundaries + +OpenCodex uses three mutually exclusive admission credential classes: + +| Credential class | Sources | Allowed surface | +| --- | --- | --- | +| Data plane | `OPENCODEX_API_AUTH_TOKEN`, the `service-api-token` file loaded through `OCX_API_TOKEN_FILE`, and `config.apiKeys` | `/v1/*` HTTP endpoints and new data-plane WebSocket handshakes only | +| Management plane | `OPENCODEX_ADMIN_AUTH_TOKEN` or the independent protected `admin-api-token` file | `/api/*` only | +| GUI session | A short-lived token issued only with a legitimate same-origin local dashboard page | `/api/*` only, bound to the issuing origin | + +The service token file remains a delivery mechanism for the data-plane environment token; it is not +a fourth credential class. A management credential that equals any configured data-plane credential +does not enable management access. The data plane may continue to start, but `/api/*` remains closed. + +Management authentication never has a loopback bypass. If no management credential is available, or +management token creation, validation, or permission hardening fails, every `/api/*` request returns +503 while `/v1/*` and unauthenticated `/healthz` continue to operate. Windows ACL hardening results +must be checked explicitly because an `icacls` timeout is a soft failure in the shared secret helper. + +Local dashboard page entry requires a loopback binding, a valid parseable loopback `Host`, and an +exact request origin. A non-loopback dashboard uses the management token flow instead. The server +issues an in-memory session for five minutes, capped at 128 live sessions. The session is bound to the +exact protocol, host, and port; state-changing requests additionally require the session CSRF token. +The dashboard never attaches its management session to `/v1/*` requests, and pages containing a +session bootstrap are served with `Cache-Control: no-store`. + +Proxy admission credentials must never reach an upstream provider. The forwarding guard rejects the +`ocx_data_`, `ocx_admin_`, and `ocx_session_` prefixes, historical keys matching +`^ocx_[0-9a-f]{40}$`, both environment tokens by constant-time comparison, and manually configured +data keys by constant-time comparison. + +Audit item #16 remains partially deferred. This credential split protects new WebSocket handshakes, +but the following established-connection controls are intentionally outside this batch and must not +be treated as implemented: + +- revoke an already established connection when its data key is deleted; +- enforce an idle timeout; +- reauthenticate subsequent frames after the handshake. + ## API ownership `src/server/index.ts` authenticates and routes `/api/*`, then delegates the management surface to diff --git a/tests/account-pool-management-api.test.ts b/tests/account-pool-management-api.test.ts index 11cb5e5b7..4dbe39926 100644 --- a/tests/account-pool-management-api.test.ts +++ b/tests/account-pool-management-api.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/api-debug.test.ts b/tests/api-debug.test.ts index de32dcced..d2458d656 100644 --- a/tests/api-debug.test.ts +++ b/tests/api-debug.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/api-storage-cleanup.test.ts b/tests/api-storage-cleanup.test.ts index 1559c3dd9..8676f7115 100644 --- a/tests/api-storage-cleanup.test.ts +++ b/tests/api-storage-cleanup.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { Database } from "bun:sqlite"; import { existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; diff --git a/tests/api-storage-policy.test.ts b/tests/api-storage-policy.test.ts index 4dead3680..d60805f8c 100644 --- a/tests/api-storage-policy.test.ts +++ b/tests/api-storage-policy.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { Database } from "bun:sqlite"; import { mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; diff --git a/tests/api-storage.test.ts b/tests/api-storage.test.ts index 4415db818..9b94dd18e 100644 --- a/tests/api-storage.test.ts +++ b/tests/api-storage.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/api-usage.test.ts b/tests/api-usage.test.ts index c0a24f785..4d43a1aac 100644 --- a/tests/api-usage.test.ts +++ b/tests/api-usage.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { appendFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/autostart-health.test.ts b/tests/autostart-health.test.ts index f26ed10e9..9d476a0c3 100644 --- a/tests/autostart-health.test.ts +++ b/tests/autostart-health.test.ts @@ -181,3 +181,4 @@ describe("Codex startup health", () => { } }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/claude-desktop-config-path.test.ts b/tests/claude-desktop-config-path.test.ts index 1da834adc..c97628538 100644 --- a/tests/claude-desktop-config-path.test.ts +++ b/tests/claude-desktop-config-path.test.ts @@ -1,4 +1,5 @@ import { expect, test, describe } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join, posix, win32 } from "node:path"; diff --git a/tests/claude-management-api.test.ts b/tests/claude-management-api.test.ts index 38909396e..7eb7b6232 100644 --- a/tests/claude-management-api.test.ts +++ b/tests/claude-management-api.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, expect, setDefaultTimeout, spyOn, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/claude-messages-endpoint.test.ts b/tests/claude-messages-endpoint.test.ts index 326715eda..41c029106 100644 --- a/tests/claude-messages-endpoint.test.ts +++ b/tests/claude-messages-endpoint.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/claude-native-passthrough.test.ts b/tests/claude-native-passthrough.test.ts index 379afd332..6743ded19 100644 --- a/tests/claude-native-passthrough.test.ts +++ b/tests/claude-native-passthrough.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/cli-management-auth.test.ts b/tests/cli-management-auth.test.ts new file mode 100644 index 000000000..7389b5298 --- /dev/null +++ b/tests/cli-management-auth.test.ts @@ -0,0 +1,97 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { runtimeRequest } from "../src/cli/runtime-api"; +import { stopProxyGracefully } from "../src/lib/process-control"; +import { fetchClaudeContextWindows } from "../src/cli/claude"; +import type { OcxConfig } from "../src/types"; + +const previousHome = process.env.OPENCODEX_HOME; +const previousDataToken = process.env.OPENCODEX_API_AUTH_TOKEN; +const previousAdminToken = process.env.OPENCODEX_ADMIN_AUTH_TOKEN; +const homes: string[] = []; +const originalFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = originalFetch; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (previousDataToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousDataToken; + if (previousAdminToken === undefined) delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + else process.env.OPENCODEX_ADMIN_AUTH_TOKEN = previousAdminToken; + for (const home of homes.splice(0)) rmSync(home, { recursive: true, force: true }); +}); + +async function capturedManagementToken(): Promise { + let token: string | null = null; + await runtimeRequest("/api/config", {}, { + baseUrl: "http://127.0.0.1:10100", + fetchImpl: async (_input, init) => { + token = new Headers(init?.headers).get("x-opencodex-api-key"); + return Response.json({ ok: true }); + }, + }); + return token; +} + +describe("CLI management authentication", () => { + test("the management environment token replaces the data token", async () => { + process.env.OPENCODEX_API_AUTH_TOKEN = "data-secret"; + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "admin-secret"; + expect(await capturedManagementToken()).toBe("admin-secret"); + }); + + test("the protected management token file is used when the environment token is absent", async () => { + const home = mkdtempSync(join(tmpdir(), "ocx-cli-admin-auth-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + process.env.OPENCODEX_API_AUTH_TOKEN = "data-secret"; + delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + writeFileSync(join(home, "admin-api-token"), `ocx_admin_${"a".repeat(43)}\n`, { mode: 0o600 }); + expect(await capturedManagementToken()).toBe(`ocx_admin_${"a".repeat(43)}`); + }); + + test("graceful stop sends the management token instead of the data token", async () => { + let token: string | null = null; + const result = await stopProxyGracefully(1234, { + readRuntime: () => ({ port: 10100, hostname: "127.0.0.1" }), + waitExit: () => true, + env: { + OPENCODEX_API_AUTH_TOKEN: "data-secret", + OPENCODEX_ADMIN_AUTH_TOKEN: "admin-secret", + }, + fetchFn: async (_input, init) => { + token = new Headers(init?.headers).get("x-opencodex-api-key"); + return new Response(null, { status: 200 }); + }, + }); + expect(result).toBe(true); + expect(token).toBe("admin-secret"); + }); + + test("Claude context discovery sends the management token", async () => { + process.env.OPENCODEX_API_AUTH_TOKEN = "data-secret"; + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "admin-secret"; + let token: string | null = null; + globalThis.fetch = (async (_input, init) => { + token = new Headers(init?.headers).get("x-opencodex-api-key"); + return Response.json({ contextWindows: { "gpt-test": 200_000 } }); + }) as typeof fetch; + const config = { + port: 10100, + defaultProvider: "test", + providers: {}, + apiKeys: [{ + id: "configured", + name: "Configured data key", + key: "ocx_data_configured-secret", + createdAt: "2026-07-28T00:00:00.000Z", + }], + } as OcxConfig; + + expect(await fetchClaudeContextWindows(config, 10100)).toEqual({ "gpt-test": 200_000 }); + expect(token).toBe("admin-secret"); + }); +}); diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 179e8a1bc..c3cc89504 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -2769,3 +2769,4 @@ describe("Codex reasoning-effort capability clamp", () => { expect(models).toEqual(before); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/codex-sync-api.test.ts b/tests/codex-sync-api.test.ts index 24f3cd58e..85064b582 100644 --- a/tests/codex-sync-api.test.ts +++ b/tests/codex-sync-api.test.ts @@ -201,7 +201,7 @@ describe("GUI/CLI Codex sync backend", () => { const { handleManagementAPI } = await import("./src/server/management-api.ts"); const config = { port: 10100, defaultProvider: "openai", providers: {} }; const response = await handleManagementAPI( - new Request("http://localhost/api/sync", { method: "POST" }), + new Request("http://localhost/api/sync", { method: "POST", headers: { Host: "localhost" } }), new URL("http://localhost/api/sync"), config, ); @@ -268,3 +268,4 @@ describe("GUI/CLI Codex sync backend", () => { ]); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/codex-v2-gate.test.ts b/tests/codex-v2-gate.test.ts index 035582f19..dae5dd894 100644 --- a/tests/codex-v2-gate.test.ts +++ b/tests/codex-v2-gate.test.ts @@ -588,3 +588,4 @@ describe("3-state multi-agent mode", () => { expect(native.multi_agent_version).toBeUndefined(); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/combo-management-api.test.ts b/tests/combo-management-api.test.ts index 5bbf5a0af..6df30c861 100644 --- a/tests/combo-management-api.test.ts +++ b/tests/combo-management-api.test.ts @@ -822,3 +822,4 @@ describe("supported disabled-provider activation", () => { }); }, 10_000); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/cursor-hardening.test.ts b/tests/cursor-hardening.test.ts index 16388f98c..51d1dc6af 100644 --- a/tests/cursor-hardening.test.ts +++ b/tests/cursor-hardening.test.ts @@ -346,3 +346,4 @@ describe("Cursor live transport unexpected EOF", () => { }); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/effort-policy.test.ts b/tests/effort-policy.test.ts index 8d83b8a43..fbad670ea 100644 --- a/tests/effort-policy.test.ts +++ b/tests/effort-policy.test.ts @@ -467,3 +467,4 @@ describe("/api/effort-caps", () => { expect(config.subagentEffortCap).toBe("low"); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/forward-admission-separation.test.ts b/tests/forward-admission-separation.test.ts new file mode 100644 index 000000000..1d698f797 --- /dev/null +++ b/tests/forward-admission-separation.test.ts @@ -0,0 +1,132 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../src/config"; +import { resolveFirstUsableOpenAiSidecar } from "../src/providers/openai-sidecar"; +import { startServer } from "../src/server"; +import type { OcxConfig } from "../src/types"; + +const originalFetch = globalThis.fetch; +const previousHome = process.env.OPENCODEX_HOME; +const previousDataToken = process.env.OPENCODEX_API_AUTH_TOKEN; +const previousAdminToken = process.env.OPENCODEX_ADMIN_AUTH_TOKEN; +let testHome = ""; +let upstreamAttempts: string[] = []; + +function forwardConfig(): OcxConfig { + return { + port: 0, + hostname: "127.0.0.1", + defaultProvider: "openai", + openaiProviderTierVersion: 2, + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "direct", + }, + }, + } as OcxConfig; +} + +beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), "ocx-forward-admission-")); + process.env.OPENCODEX_HOME = testHome; + delete process.env.OPENCODEX_API_AUTH_TOKEN; + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "admin-secret"; + upstreamAttempts = []; + globalThis.fetch = (async (input, init) => { + const raw = input instanceof Request ? input.url : String(input); + const url = new URL(raw); + if (url.hostname === "chatgpt.com" || url.hostname === "api.openai.com") { + upstreamAttempts.push(url.toString()); + return Response.json({ error: "unexpected upstream request" }, { status: 500 }); + } + return originalFetch(input, init); + }) as typeof fetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (previousDataToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousDataToken; + if (previousAdminToken === undefined) delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + else process.env.OPENCODEX_ADMIN_AUTH_TOKEN = previousAdminToken; + if (testHome) rmSync(testHome, { recursive: true, force: true }); + testHome = ""; +}); + +describe("management credentials never leave data-plane forwarding paths", () => { + test("the shared OpenAI sidecar refuses a management bearer", async () => { + const config = forwardConfig(); + const provider = config.providers.openai!; + const selected = await resolveFirstUsableOpenAiSidecar( + [{ providerName: "openai", provider, accountMode: "direct" }], + new Headers({ authorization: "Bearer admin-secret" }), + config, + ); + expect(selected).toBeUndefined(); + expect(upstreamAttempts).toHaveLength(0); + }); + + for (const request of [ + { + name: "ordinary Responses", + expectedStatus: 401, + path: "/v1/responses", + contentType: "application/json", + body: JSON.stringify({ model: "openai/gpt-test", input: "hello" }), + }, + { + name: "compact Responses", + expectedStatus: 401, + path: "/v1/responses/compact", + contentType: "application/json", + body: JSON.stringify({ model: "openai/gpt-test", input: [] }), + }, + { + name: "Images", + expectedStatus: 400, + path: "/v1/images/generations", + contentType: "application/json", + body: JSON.stringify({ model: "gpt-image-2", prompt: "test" }), + }, + { + name: "Live", + expectedStatus: 401, + path: "/v1/live", + contentType: "application/json", + body: JSON.stringify({ sdp: "v=0", session: { model: "gpt-live" } }), + }, + { + name: "Search", + expectedStatus: 401, + path: "/v1/alpha/search", + contentType: "application/json", + body: JSON.stringify({ id: "search-session", model: "gpt-test" }), + }, + ]) { + test(`${request.name} rejects a management bearer before upstream I/O`, async () => { + saveConfig(forwardConfig()); + const server = startServer(0); + try { + const response = await originalFetch(new URL(request.path, server.url), { + method: "POST", + headers: { + "content-type": request.contentType, + authorization: "Bearer admin-secret", + }, + body: request.body, + }); + expect(response.status).toBe(request.expectedStatus); + expect(upstreamAttempts).toHaveLength(0); + } finally { + await server.stop(true); + } + }); + } +}); diff --git a/tests/grok-management-api.test.ts b/tests/grok-management-api.test.ts index b8446e102..3f3e82460 100644 --- a/tests/grok-management-api.test.ts +++ b/tests/grok-management-api.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, expect, setDefaultTimeout, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/gui-management-session.test.ts b/tests/gui-management-session.test.ts new file mode 100644 index 000000000..44e74cc3d --- /dev/null +++ b/tests/gui-management-session.test.ts @@ -0,0 +1,62 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { installApiAuthFetch, resetApiAuthFetchForTests } from "../gui/src/api"; + +const originalWindow = globalThis.window; +const originalDocument = globalThis.document; +const originalSessionStorage = globalThis.sessionStorage; + +afterEach(() => { + resetApiAuthFetchForTests(); + Object.assign(globalThis, { + window: originalWindow, + document: originalDocument, + sessionStorage: originalSessionStorage, + }); +}); + +describe("GUI management session bootstrap", () => { + test("management requests use the injected session while data requests remain untouched", async () => { + const seen: Array<{ url: string; method: string; headers: Headers }> = []; + const meta = new Map([ + ["opencodex-session-token", "ocx_session_browser-secret"], + ["opencodex-session-csrf", "csrf-browser-secret"], + ["opencodex-session-origin", "http://localhost:10100"], + ]); + const fetchImpl = async (input: RequestInfo | URL, init?: RequestInit): Promise => { + seen.push({ + url: input instanceof Request ? input.url : String(input), + method: init?.method ?? (input instanceof Request ? input.method : "GET"), + headers: new Headers(init?.headers ?? (input instanceof Request ? input.headers : undefined)), + }); + return Response.json({ ok: true }); + }; + Object.assign(globalThis, { + document: { + querySelector(selector: string) { + const match = selector.match(/^meta\[name="([^"]+)"\]$/); + const content = match ? meta.get(match[1] ?? "") : undefined; + return content ? { content, remove() {} } : null; + }, + }, + sessionStorage: { removeItem() {} }, + window: { + location: new URL("http://localhost:10100/"), + fetch: fetchImpl, + prompt: () => null, + }, + }); + + installApiAuthFetch(); + await window.fetch("/api/config"); + await window.fetch("/api/settings", { method: "PUT", body: "{}" }); + await window.fetch("/v1/models"); + + expect(seen[0]?.headers.get("x-opencodex-api-key")).toBe("ocx_session_browser-secret"); + expect(seen[0]?.headers.get("x-opencodex-gui-origin")).toBe("http://localhost:10100"); + expect(seen[0]?.headers.get("x-opencodex-csrf-token")).toBeNull(); + expect(seen[1]?.headers.get("x-opencodex-api-key")).toBe("ocx_session_browser-secret"); + expect(seen[1]?.headers.get("x-opencodex-csrf-token")).toBe("csrf-browser-secret"); + expect(seen[2]?.headers.get("x-opencodex-api-key")).toBeNull(); + expect(seen[2]?.headers.get("x-opencodex-gui-origin")).toBeNull(); + }); +}); diff --git a/tests/helpers/management-auth.ts b/tests/helpers/management-auth.ts new file mode 100644 index 000000000..bd29cbfb8 --- /dev/null +++ b/tests/helpers/management-auth.ts @@ -0,0 +1,43 @@ +import { configuredAdminToken } from "../../src/lib/admin-secrets"; + +function isLocalManagementRequest(input: RequestInfo | URL): boolean { + try { + const raw = input instanceof Request ? input.url : String(input); + const url = new URL(raw, "http://127.0.0.1"); + const hostname = url.hostname.toLowerCase(); + const loopback = hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1" || hostname === "[::1]"; + return loopback && url.pathname.startsWith("/api/"); + } catch { + return false; + } +} + +/** Test transport: authenticate local management requests without changing data/upstream traffic. */ +export function managementFetch(input: RequestInfo | URL, init?: RequestInit): Promise { + if (!isLocalManagementRequest(input)) return globalThis.fetch(input, init); + const headers = managementHeaders(init?.headers ?? (input instanceof Request ? input.headers : undefined)); + if (!headers.has("x-opencodex-api-key")) return globalThis.fetch(input, init); + if (input instanceof Request) { + return globalThis.fetch(new Request(input, { headers }), init ? { ...init, headers } : undefined); + } + return globalThis.fetch(input, { ...init, headers }); +} + +/** Headers for callers that must retain a captured fetch implementation. */ +export function managementHeaders(initial?: HeadersInit): Headers { + const headers = new Headers(initial); + const token = configuredAdminToken(); + if (token && !headers.has("x-opencodex-api-key")) headers.set("x-opencodex-api-key", token); + return headers; +} + +/** Direct-handler test request with the Host header an actual HTTP server would provide. */ +export class ManagementRequest extends globalThis.Request { + constructor(input: RequestInfo | URL, init?: RequestInit) { + const raw = input instanceof globalThis.Request ? input.url : String(input); + const url = new URL(raw, "http://127.0.0.1"); + const headers = new Headers(init?.headers ?? (input instanceof globalThis.Request ? input.headers : undefined)); + if (isLocalManagementRequest(input) && !headers.has("Host")) headers.set("Host", url.host); + super(input, { ...init, headers }); + } +} diff --git a/tests/injection-model-api.test.ts b/tests/injection-model-api.test.ts index e324ea625..8314d0a50 100644 --- a/tests/injection-model-api.test.ts +++ b/tests/injection-model-api.test.ts @@ -412,3 +412,4 @@ describe("/api/injection-model guidance kill switch + partial update", () => { }); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/management-api-logs-metrics.test.ts b/tests/management-api-logs-metrics.test.ts index d1aecfe50..3b4c6c2d5 100644 --- a/tests/management-api-logs-metrics.test.ts +++ b/tests/management-api-logs-metrics.test.ts @@ -140,3 +140,4 @@ describe("GET /api/logs display metrics", () => { expect(dto!.displayMetrics.cost).toEqual({ kind: "unavailable", reason: "invalid_cache_breakdown" }); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index dd11eacb2..ed52a7c69 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, setDefaultTimeout, spyOn, test } from "bun:test"; +import { managementFetch as fetch, ManagementRequest as Request } from "./helpers/management-auth"; import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { saveCodexAccountCredential } from "../src/codex/account-store"; diff --git a/tests/memory-watchdog.test.ts b/tests/memory-watchdog.test.ts index 039f4d3d4..2064888d3 100644 --- a/tests/memory-watchdog.test.ts +++ b/tests/memory-watchdog.test.ts @@ -219,3 +219,4 @@ describe("GET /api/system/memory", () => { expect(body.watchdog).toBeNull(); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/model-visibility-management-api.test.ts b/tests/model-visibility-management-api.test.ts index b36a2016b..2200ac7a4 100644 --- a/tests/model-visibility-management-api.test.ts +++ b/tests/model-visibility-management-api.test.ts @@ -266,3 +266,4 @@ describe("atomic model visibility management", () => { expect(refreshes).toBe(2); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/native-model-toggle.test.ts b/tests/native-model-toggle.test.ts index 68a9fbf9a..1d1066aa0 100644 --- a/tests/native-model-toggle.test.ts +++ b/tests/native-model-toggle.test.ts @@ -130,3 +130,4 @@ describe("native GPT model toggles (bare slugs in disabledModels)", () => { expect(sub.available).toContain("gpt-5.6-terra"); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/oauth-accounts-api.test.ts b/tests/oauth-accounts-api.test.ts index b80095002..7d50b81dc 100644 --- a/tests/oauth-accounts-api.test.ts +++ b/tests/oauth-accounts-api.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/oauth-login-cli-live-update.test.ts b/tests/oauth-login-cli-live-update.test.ts index fca3cdde7..7f8fa8275 100644 --- a/tests/oauth-login-cli-live-update.test.ts +++ b/tests/oauth-login-cli-live-update.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/oauth-manual-code.test.ts b/tests/oauth-manual-code.test.ts index 943f76f05..cf36c6faa 100644 --- a/tests/oauth-manual-code.test.ts +++ b/tests/oauth-manual-code.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { createHash } from "node:crypto"; import { existsSync, mkdirSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; diff --git a/tests/oauth-public-surface.test.ts b/tests/oauth-public-surface.test.ts index aa699e417..edf919a61 100644 --- a/tests/oauth-public-surface.test.ts +++ b/tests/oauth-public-surface.test.ts @@ -100,3 +100,4 @@ describe("legacy ChatGPT OAuth public-surface exclusion", () => { expect(cfg.providers.chatgpt).toBeUndefined(); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/oauth-reauth-bind.test.ts b/tests/oauth-reauth-bind.test.ts index b88f43dbe..8a236a9f0 100644 --- a/tests/oauth-reauth-bind.test.ts +++ b/tests/oauth-reauth-bind.test.ts @@ -200,3 +200,4 @@ describe("OAuth account-scoped reauth", () => { expect(source).toContain("Unknown account for reauth"); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/openai-api-virtual-models.test.ts b/tests/openai-api-virtual-models.test.ts index 4476036bf..3ff38c5e1 100644 --- a/tests/openai-api-virtual-models.test.ts +++ b/tests/openai-api-virtual-models.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { managementHeaders } from "./helpers/management-auth"; import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -229,7 +230,8 @@ describe("OpenAI API compact transport", () => { }; const server = startServer(0); - const readLogs = () => originalFetch(new URL("/api/logs", server.url)).then(response => response.json()) as Promise>>; + const readLogs = () => originalFetch(new URL("/api/logs", server.url), { headers: managementHeaders() }) + .then(response => response.json()) as Promise>>; const readUsage = (): Array> => existsSync(usageLogPath()) ? readFileSync(usageLogPath(), "utf8").trim().split("\n").filter(Boolean).map(line => JSON.parse(line) as Record) : []; @@ -396,7 +398,8 @@ describe("OpenAI API Pro transport identities", () => { const readUsage = (): Array> => existsSync(usageLogPath()) ? readFileSync(usageLogPath(), "utf8").trim().split("\n").filter(Boolean).map(line => JSON.parse(line) as Record) : []; - const readLogs = () => originalFetch(new URL("/api/logs", server.url)).then(response => response.json()) as Promise>>; + const readLogs = () => originalFetch(new URL("/api/logs", server.url), { headers: managementHeaders() }) + .then(response => response.json()) as Promise>>; const expectOnePersisted = async ( beforeLogs: number, beforeUsage: number, diff --git a/tests/openai-provider-option-e2e.test.ts b/tests/openai-provider-option-e2e.test.ts index 573f26cb7..c865f2eb0 100644 --- a/tests/openai-provider-option-e2e.test.ts +++ b/tests/openai-provider-option-e2e.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { createHash } from "node:crypto"; import { chmodSync, diff --git a/tests/opencode-cli.test.ts b/tests/opencode-cli.test.ts index 00dbad5a1..365404ca4 100644 --- a/tests/opencode-cli.test.ts +++ b/tests/opencode-cli.test.ts @@ -568,3 +568,4 @@ describe("ocx opencode not-found hint", () => { expect(opencodeNotFoundHint(0, null, "win32")).toBeNull(); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/process-control-graceful.test.ts b/tests/process-control-graceful.test.ts index ed2065366..54c9ea6b9 100644 --- a/tests/process-control-graceful.test.ts +++ b/tests/process-control-graceful.test.ts @@ -52,7 +52,7 @@ describe("stopProxyGracefully", () => { expect(calls).toEqual([{ url: "http://127.0.0.1:10123/api/stop", method: "POST" }]); }); - test("sends the management auth header when OPENCODEX_API_AUTH_TOKEN is set", async () => { + test("sends the management token instead of the data token", async () => { let headers: Record | undefined; await stopProxyGracefully(1, { readRuntime: () => ({ port: 10100 }), @@ -61,10 +61,13 @@ describe("stopProxyGracefully", () => { return okResponse(); }) as typeof fetch, waitExit: () => true, - env: { OPENCODEX_API_AUTH_TOKEN: "secret-token" }, + env: { + OPENCODEX_API_AUTH_TOKEN: "data-secret", + OPENCODEX_ADMIN_AUTH_TOKEN: "admin-secret", + }, }); - expect(headers?.["x-opencodex-api-key"]).toBe("secret-token"); + expect(headers?.["x-opencodex-api-key"]).toBe("admin-secret"); }); test("returns false when no runtime port is recorded (caller falls back to killProxy)", async () => { diff --git a/tests/provider-api-keys.test.ts b/tests/provider-api-keys.test.ts index 0d5cdf8af..392875f30 100644 --- a/tests/provider-api-keys.test.ts +++ b/tests/provider-api-keys.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/provider-connection-test.test.ts b/tests/provider-connection-test.test.ts index f4bf64775..40b0790e5 100644 --- a/tests/provider-connection-test.test.ts +++ b/tests/provider-connection-test.test.ts @@ -187,3 +187,4 @@ describe("POST /api/oauth/login/cancel (WP040)", () => { expect(ok.body).toEqual({ ok: true, cancelled: false }); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/server-403-permission-e2e.test.ts b/tests/server-403-permission-e2e.test.ts index 16251464c..a4f0ce348 100644 --- a/tests/server-403-permission-e2e.test.ts +++ b/tests/server-403-permission-e2e.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index af247582b..28c08d011 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -31,6 +31,7 @@ import { handleManagementAPI } from "../src/server/management-api"; import type { OcxConfig } from "../src/types"; import { fakeChatGptJwt } from "./helpers/fake-chatgpt-jwt"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +import { configuredAdminToken } from "../src/lib/admin-secrets"; const previousApiToken = process.env.OPENCODEX_API_AUTH_TOKEN; const previousOpencodexHome = process.env.OPENCODEX_HOME; @@ -55,6 +56,14 @@ function config(hostname?: string): OcxConfig { }; } +function managementHeaders(initial?: HeadersInit): Headers { + const token = configuredAdminToken(); + if (!token) throw new Error("management token was not initialized"); + const headers = new Headers(initial); + headers.set("x-opencodex-api-key", token); + return headers; +} + const canonicalDirect = { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", @@ -506,13 +515,13 @@ describe("server local API auth", () => { const server = startServer(0); try { const response = await fetch(`http://127.0.0.1:${server.port}/api/config`, { - headers: { "x-opencodex-api-key": "local-secret", origin: "https://attacker.test" }, + headers: managementHeaders({ origin: "https://attacker.test" }), }); expect(response.status).toBe(403); expect(await response.json()).toMatchObject({ error: "cross-origin request blocked" }); const ok = await fetch(`http://127.0.0.1:${server.port}/api/config`, { - headers: { "x-opencodex-api-key": "local-secret", origin: `http://127.0.0.1:${server.port}` }, + headers: managementHeaders({ origin: `http://127.0.0.1:${server.port}` }), }); expect(ok.status).toBe(200); } finally { @@ -536,7 +545,7 @@ describe("server local API auth", () => { expect(missing.status).toBe(401); const ok = await fetch(`http://127.0.0.1:${server.port}/api/system/memory`, { - headers: { "x-opencodex-api-key": "local-secret" }, + headers: managementHeaders(), }); expect(ok.status).toBe(200); const body = await ok.json() as { rss?: number; bunVersion?: string }; @@ -599,6 +608,7 @@ describe("server local API auth", () => { headers: { host: `attacker.test:${server.port}`, origin: attackerOrigin, + "x-opencodex-api-key": configuredAdminToken() ?? "missing-admin-token", }, }); expect(response.status).toBe(403); @@ -618,14 +628,14 @@ describe("server local API auth", () => { const origin = `http://127.0.0.1:${server.port}`; try { const settings = await fetch(new URL("/api/settings", server.url), { - headers: { origin }, + headers: managementHeaders({ origin }), }); expect(settings.status).toBe(200); expect(settings.headers.get("access-control-allow-origin")).toBe(origin); expect(settings.headers.get("vary")).toContain("Origin"); const active = await fetch(new URL("/api/codex-auth/active", server.url), { - headers: { origin }, + headers: managementHeaders({ origin }), }); expect(active.status).toBe(200); expect(active.headers.get("access-control-allow-origin")).toBe(origin); @@ -656,11 +666,10 @@ describe("server local API auth", () => { expect(missing.status).toBe(401); const ok = await fetch(`http://127.0.0.1:${server.port}/api/settings`, { - headers: { + headers: managementHeaders({ host: `lan.example.test:${server.port}`, origin, - "x-opencodex-api-key": "local-secret", - }, + }), }); expect(ok.status).toBe(200); expect(ok.headers.get("access-control-allow-origin")).toBe(origin); @@ -1249,7 +1258,7 @@ describe("server local API auth", () => { const switched = await fetch(new URL("/api/codex-auth/active", sequential.url), { method: "PUT", - headers: { "content-type": "application/json", "x-opencodex-api-key": "local-secret" }, + headers: managementHeaders({ "content-type": "application/json" }), body: JSON.stringify({ accountId: "pool-b" }), }); expect(switched.status).toBe(200); @@ -1599,7 +1608,7 @@ describe("server local API auth", () => { ws.close(); expect(seenAuth).toEqual(["Bearer old-access-token", "Bearer new-access-token"]); - const logs = await fetch(new URL("/api/logs?tail=2", server.url)).then(r => r.json()) as Array<{ status: number }>; + const logs = await fetch(new URL("/api/logs?tail=2", server.url), { headers: managementHeaders() }).then(r => r.json()) as Array<{ status: number }>; expect(logs.map(entry => entry.status)).toEqual([200, 200]); } finally { Date.now = originalNow; @@ -1671,7 +1680,7 @@ describe("server local API auth", () => { await waitForTerminal(); ws.close(); - const logs = await fetch(new URL("/api/logs?tail=1", server.url)).then(r => r.json()) as Array<{ + const logs = await fetch(new URL("/api/logs?tail=1", server.url), { headers: managementHeaders() }).then(r => r.json()) as Array<{ status: number; terminalStatus?: string; closeReason?: string; @@ -2189,7 +2198,7 @@ describe("server local API auth", () => { consecutiveFailures: 3, lastFailureStatus: 502, }); - const logs = await fetch(new URL("/api/logs?tail=1", server.url)).then(r => r.json()) as Array<{ status: number; errorCode?: string; terminalStatus?: string; closeReason?: string }>; + const logs = await fetch(new URL("/api/logs?tail=1", server.url), { headers: managementHeaders() }).then(r => r.json()) as Array<{ status: number; errorCode?: string; terminalStatus?: string; closeReason?: string }>; expect(logs.at(-1)).toMatchObject({ status: 502, errorCode: "upstream_server_error", @@ -2245,7 +2254,7 @@ describe("server local API auth", () => { expect(response.status).toBe(200); await response.text(); - const logs = await fetch(new URL("/api/logs?tail=1", server.url)).then(r => r.json()) as Array<{ + const logs = await fetch(new URL("/api/logs?tail=1", server.url), { headers: managementHeaders() }).then(r => r.json()) as Array<{ status: number; terminalStatus?: string; closeReason?: string; @@ -2267,7 +2276,7 @@ describe("server local API auth", () => { }, }); - const usage = await fetch(new URL("/api/usage?range=all&surface=codex", server.url)).then(r => r.json()) as { + const usage = await fetch(new URL("/api/usage?range=all&surface=codex", server.url), { headers: managementHeaders() }).then(r => r.json()) as { surface: string; summary: { requests: number; reportedRequests: number; totalTokens: number }; models: Array<{ provider: string; model: string; reportedRequests: number; totalTokens: number }>; @@ -2281,7 +2290,7 @@ describe("server local API auth", () => { totalTokens: 18, }); - const claudeUsage = await fetch(new URL("/api/usage?range=all&surface=claude", server.url)).then(r => r.json()) as { + const claudeUsage = await fetch(new URL("/api/usage?range=all&surface=claude", server.url), { headers: managementHeaders() }).then(r => r.json()) as { surface: string; summary: { requests: number }; }; diff --git a/tests/server-combo-failover-e2e.test.ts b/tests/server-combo-failover-e2e.test.ts index c464ed4a0..366526ec3 100644 --- a/tests/server-combo-failover-e2e.test.ts +++ b/tests/server-combo-failover-e2e.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, mock, setDefaultTimeout, test } from "bun:test"; +import { managementFetch as fetch, ManagementRequest as Request } from "./helpers/management-auth"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; diff --git a/tests/server-management-auth.test.ts b/tests/server-management-auth.test.ts new file mode 100644 index 000000000..1484eb2eb --- /dev/null +++ b/tests/server-management-auth.test.ts @@ -0,0 +1,439 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../src/config"; +import { startServer } from "../src/server"; +import type { OcxConfig } from "../src/types"; +import { serveGuiFile } from "../src/server/gui-static"; +import { isProxyAdmissionSecret } from "../src/server/auth-cors"; +import { + initializeManagementAuthState, + issueGuiSession, + requireManagementAuth, +} from "../src/server/management-auth"; +import { + resetHardenedStateForTests, + setIcaclsRunnerForTests, + setPlatformForTests, +} from "../src/lib/windows-secret-acl"; + +const previousHome = process.env.OPENCODEX_HOME; +const previousDataToken = process.env.OPENCODEX_API_AUTH_TOKEN; +const previousAdminToken = process.env.OPENCODEX_ADMIN_AUTH_TOKEN; +let testHome = ""; + +function remoteConfig(): OcxConfig { + return { + port: 0, + hostname: "0.0.0.0", + defaultProvider: "test", + providers: { + test: { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + disabled: true, + models: ["gpt-test"], + }, + }, + }; +} + +function websocketHandshakeOpens(url: URL, token: string): Promise { + return new Promise(resolve => { + const target = new URL("/v1/responses", url); + target.protocol = target.protocol === "https:" ? "wss:" : "ws:"; + const socket = new WebSocket(target, { + headers: { "X-OpenCodex-API-Key": token }, + } as unknown as string[]); + let settled = false; + const finish = (opened: boolean) => { + if (settled) return; + settled = true; + clearTimeout(timer); + try { socket.close(); } catch { /* already closed */ } + resolve(opened); + }; + socket.addEventListener("open", () => finish(true)); + socket.addEventListener("error", () => finish(false)); + socket.addEventListener("close", () => finish(false)); + const timer = setTimeout(() => finish(false), 5_000); + }); +} + +beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), "ocx-management-auth-")); + process.env.OPENCODEX_HOME = testHome; + process.env.OPENCODEX_API_AUTH_TOKEN = "data-secret"; + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "admin-secret"; +}); + +afterEach(() => { + setIcaclsRunnerForTests(null); + setPlatformForTests(null); + resetHardenedStateForTests(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (previousDataToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousDataToken; + if (previousAdminToken === undefined) delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + else process.env.OPENCODEX_ADMIN_AUTH_TOKEN = previousAdminToken; + if (testHome) rmSync(testHome, { recursive: true, force: true }); + testHome = ""; +}); + +describe("management and data-plane credential separation", () => { + test("data and management environment tokens authorize only their own planes", async () => { + saveConfig(remoteConfig()); + const server = startServer(0); + try { + const managementWithDataToken = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "data-secret" }, + }); + expect(managementWithDataToken.status).toBe(401); + + const managementWithAdminToken = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "admin-secret" }, + }); + expect(managementWithAdminToken.status).toBe(200); + + const dataWithDataToken = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": "data-secret" }, + }); + expect(dataWithDataToken.status).toBe(200); + + const dataWithAdminToken = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": "admin-secret" }, + }); + expect(dataWithAdminToken.status).toBe(401); + } finally { + await server.stop(true); + } + }); + + test("a management token that matches the data environment token closes only the management plane", async () => { + process.env.OPENCODEX_ADMIN_AUTH_TOKEN = "data-secret"; + saveConfig(remoteConfig()); + const server = startServer(0); + try { + const data = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": "data-secret" }, + }); + expect(data.status).toBe(200); + + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "data-secret" }, + }); + expect(management.status).toBe(503); + } finally { + await server.stop(true); + } + }); + + test("a management token that matches a configured data key closes only the management plane", async () => { + delete process.env.OPENCODEX_API_AUTH_TOKEN; + const config = remoteConfig(); + config.apiKeys = [{ + id: "conflict", + name: "Conflicting data key", + key: "admin-secret", + createdAt: "2026-07-28T00:00:00.000Z", + }]; + saveConfig(config); + const server = startServer(0); + try { + const data = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": "admin-secret" }, + }); + expect(data.status).toBe(200); + + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "admin-secret" }, + }); + expect(management.status).toBe(503); + } finally { + await server.stop(true); + } + }); + + test("a protected management token file is generated and remains management-only", async () => { + delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + saveConfig(remoteConfig()); + const server = startServer(0); + try { + const adminToken = readFileSync(join(testHome, "admin-api-token"), "utf8").trim(); + expect(adminToken).toMatch(/^ocx_admin_[A-Za-z0-9_-]{43}$/); + + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": adminToken }, + }); + expect(management.status).toBe(200); + + const data = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": adminToken }, + }); + expect(data.status).toBe(401); + } finally { + await server.stop(true); + } + }); + + test("an icacls timeout keeps the management plane closed without stopping the data plane", async () => { + delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + saveConfig(remoteConfig()); + setPlatformForTests("win32"); + setIcaclsRunnerForTests(args => { + const target = args[0] ?? ""; + if (target.includes(".admin-token.tmp")) { + return { success: false, exitCode: null, timedOut: true, stdout: "" }; + } + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + + const server = startServer(0); + try { + const health = await fetch(new URL("/healthz", server.url)); + expect(health.status).toBe(200); + + const data = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": "data-secret" }, + }); + expect(data.status).toBe(200); + + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "ocx_admin_unhardened" }, + }); + expect(management.status).toBe(503); + expect(await management.json()).toEqual({ error: "management API unavailable" }); + } finally { + await server.stop(true); + } + }); + + test("a configured data key satisfies the remote data-plane startup requirement", async () => { + delete process.env.OPENCODEX_API_AUTH_TOKEN; + const config = remoteConfig(); + config.apiKeys = [{ + id: "configured", + name: "Configured data key", + key: "ocx_data_configured-secret", + createdAt: "2026-07-28T00:00:00.000Z", + }]; + saveConfig(config); + + const server = startServer(0); + try { + const data = await fetch(new URL("/v1/models", server.url), { + headers: { "x-opencodex-api-key": "ocx_data_configured-secret" }, + }); + expect(data.status).toBe(200); + + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "ocx_data_configured-secret" }, + }); + expect(management.status).toBe(401); + } finally { + await server.stop(true); + } + }); + + test("management browser origins must match the request origin exactly", async () => { + const config = remoteConfig(); + config.hostname = "127.0.0.1"; + saveConfig(config); + const server = startServer(0); + try { + const crossPort = await fetch(new URL("/api/config", server.url), { + headers: { + "x-opencodex-api-key": "admin-secret", + origin: "http://127.0.0.1:65534", + }, + }); + expect(crossPort.status).toBe(403); + + const sameOrigin = await fetch(new URL("/api/config", server.url), { + headers: { + "x-opencodex-api-key": "admin-secret", + origin: server.url.origin, + }, + }); + expect(sameOrigin.status).toBe(200); + expect(sameOrigin.headers.get("access-control-allow-origin")).toBe(server.url.origin); + } finally { + await server.stop(true); + } + }); + + test("a local GUI page receives an origin-bound session with CSRF protection", async () => { + const config = remoteConfig(); + config.hostname = "127.0.0.1"; + const state = initializeManagementAuthState(config); + const pageRequest = new Request("http://localhost:10100/", { + headers: { Host: "localhost:10100" }, + }); + const session = issueGuiSession(pageRequest, config, state); + expect(session).not.toBeNull(); + + const guiDist = join(testHome, "gui"); + const { mkdirSync, writeFileSync } = await import("node:fs"); + mkdirSync(guiDist); + writeFileSync(join(guiDist, "index.html"), ""); + const page = serveGuiFile("/", guiDist, session ?? undefined); + expect(page?.headers.get("cache-control")).toBe("no-store"); + const html = await page?.text(); + expect(html).toContain(`name="opencodex-session-token" content="${session?.token}"`); + expect(html).toContain(`name="opencodex-session-csrf" content="${session?.csrfToken}"`); + + const sameOriginRead = new Request("http://localhost:10100/api/config", { + headers: { + Host: "localhost:10100", + "x-opencodex-api-key": session?.token ?? "", + "x-opencodex-gui-origin": "http://localhost:10100", + }, + }); + expect(requireManagementAuth(sameOriginRead, state, config)).toBeNull(); + + const crossPortRead = new Request("http://localhost:10100/api/config", { + headers: { + Host: "localhost:10100", + Origin: "http://localhost:20100", + "x-opencodex-api-key": session?.token ?? "", + "x-opencodex-gui-origin": "http://localhost:20100", + }, + }); + expect(requireManagementAuth(crossPortRead, state, config)?.status).toBe(401); + + const mutationWithoutCsrf = new Request("http://localhost:10100/api/config", { + method: "POST", + headers: { + Host: "localhost:10100", + Origin: "http://localhost:10100", + "x-opencodex-api-key": session?.token ?? "", + "x-opencodex-gui-origin": "http://localhost:10100", + }, + }); + expect(requireManagementAuth(mutationWithoutCsrf, state, config)?.status).toBe(401); + + const mutationWithCsrf = new Request("http://localhost:10100/api/config", { + method: "POST", + headers: { + Host: "localhost:10100", + Origin: "http://localhost:10100", + "x-opencodex-api-key": session?.token ?? "", + "x-opencodex-gui-origin": "http://localhost:10100", + "x-opencodex-csrf-token": session?.csrfToken ?? "", + }, + }); + expect(requireManagementAuth(mutationWithCsrf, state, config)).toBeNull(); + + expect(issueGuiSession(new Request("http://attacker.test/", { + headers: { Host: "attacker.test" }, + }), config, state)).toBeNull(); + expect(issueGuiSession(new Request("http://localhost:10100/"), config, state)).toBeNull(); + }); + + test("a non-loopback binding never issues a GUI session from a forged loopback Host", () => { + const config = remoteConfig(); + const state = initializeManagementAuthState(config); + const request = new Request("http://localhost:10100/", { + headers: { Host: "localhost:10100" }, + }); + expect(issueGuiSession(request, config, state)).toBeNull(); + }); + + test("all local credential shapes are rejected by the upstream-forwarding guard", () => { + const config = remoteConfig(); + config.apiKeys = [ + { + id: "manual", + name: "Manual data key", + key: "manually-configured-data-secret", + createdAt: "2026-07-28T00:00:00.000Z", + }, + { + id: "legacy", + name: "Legacy data key", + key: `ocx_${"a".repeat(40)}`, + createdAt: "2026-07-28T00:00:00.000Z", + }, + ]; + for (const secret of [ + "data-secret", + "admin-secret", + "manually-configured-data-secret", + `ocx_${"a".repeat(40)}`, + "ocx_data_generated", + "ocx_admin_generated", + "ocx_session_generated", + ]) { + expect(isProxyAdmissionSecret(secret, config)).toBe(true); + } + expect(isProxyAdmissionSecret("ocx_provider_upstream", config)).toBe(false); + }); + + test("Responses authentication and WebSocket handshakes accept data credentials only", async () => { + const config = remoteConfig(); + config.websockets = true; + saveConfig(config); + const server = startServer(0); + try { + for (const rejected of ["admin-secret", "ocx_session_browser-secret"]) { + const response = await fetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { + "content-type": "application/json", + "x-opencodex-api-key": rejected, + }, + body: JSON.stringify({ model: "test/gpt-test", input: "hello" }), + }); + expect(response.status).toBe(401); + expect(await websocketHandshakeOpens(server.url, rejected)).toBe(false); + } + expect(await websocketHandshakeOpens(server.url, "data-secret")).toBe(true); + } finally { + await server.stop(true); + } + }); + + test("an invalid existing management token file keeps management unavailable", async () => { + delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + saveConfig(remoteConfig()); + writeFileSync(join(testHome, "admin-api-token"), "corrupt-token\n", { mode: 0o600 }); + const server = startServer(0); + try { + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": "corrupt-token" }, + }); + expect(management.status).toBe(503); + expect(readFileSync(join(testHome, "admin-api-token"), "utf8")).toBe("corrupt-token\n"); + expect((await fetch(new URL("/healthz", server.url))).status).toBe(200); + } finally { + await server.stop(true); + } + }); + + test("an existing management token ACL hardening failure keeps management unavailable", async () => { + delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN; + saveConfig(remoteConfig()); + const adminToken = `ocx_admin_${"b".repeat(43)}`; + writeFileSync(join(testHome, "admin-api-token"), `${adminToken}\n`, { mode: 0o600 }); + setPlatformForTests("win32"); + setIcaclsRunnerForTests(args => { + const target = args[0] ?? ""; + if (target.endsWith("admin-api-token")) { + return { success: false, exitCode: 5, timedOut: false, stdout: "" }; + } + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + const server = startServer(0); + try { + const management = await fetch(new URL("/api/config", server.url), { + headers: { "x-opencodex-api-key": adminToken }, + }); + expect(management.status).toBe(503); + expect((await fetch(new URL("/healthz", server.url))).status).toBe(200); + } finally { + await server.stop(true); + } + }); +}); diff --git a/tests/settings-stream-mode.test.ts b/tests/settings-stream-mode.test.ts index d403f237d..ea5d9f4da 100644 --- a/tests/settings-stream-mode.test.ts +++ b/tests/settings-stream-mode.test.ts @@ -216,3 +216,4 @@ describe("config.json schema resilience", () => { expect(loadConfig().streamMode).toBe("legacy-tee"); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/startup-action-control.test.ts b/tests/startup-action-control.test.ts index 5c36fd331..46ccaab10 100644 --- a/tests/startup-action-control.test.ts +++ b/tests/startup-action-control.test.ts @@ -46,3 +46,4 @@ describe("startup install actions", () => { expect(called).toBe(false); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/storage-mutation-race.test.ts b/tests/storage-mutation-race.test.ts index bb841cd58..820c0736c 100644 --- a/tests/storage-mutation-race.test.ts +++ b/tests/storage-mutation-race.test.ts @@ -2,6 +2,7 @@ * Regression: cleanup and restore must not mutate CODEX_HOME concurrently. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { Database } from "bun:sqlite"; import { existsSync, diff --git a/tests/storage-policy-job-responsive.test.ts b/tests/storage-policy-job-responsive.test.ts index 9128d55f2..bc3ca1af9 100644 --- a/tests/storage-policy-job-responsive.test.ts +++ b/tests/storage-policy-job-responsive.test.ts @@ -3,6 +3,7 @@ * streaming response on the proxy event loop. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { Database } from "bun:sqlite"; import { mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; diff --git a/tests/storage-restore-job-responsive.test.ts b/tests/storage-restore-job-responsive.test.ts index 206ccd94f..be4df504c 100644 --- a/tests/storage-restore-job-responsive.test.ts +++ b/tests/storage-restore-job-responsive.test.ts @@ -3,6 +3,7 @@ * streaming response on the proxy event loop. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { managementFetch as fetch } from "./helpers/management-auth"; import { Database } from "bun:sqlite"; import { existsSync, mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; diff --git a/tests/subagent-model-fallback-api.test.ts b/tests/subagent-model-fallback-api.test.ts index 10fde1547..d93ca16f8 100644 --- a/tests/subagent-model-fallback-api.test.ts +++ b/tests/subagent-model-fallback-api.test.ts @@ -89,3 +89,4 @@ describe("/api/subagent-model-fallback atomic validation", () => { expect(config.subagentModelFallback).toEqual(next); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/system-restart.test.ts b/tests/system-restart.test.ts index e5aff232f..16c585ceb 100644 --- a/tests/system-restart.test.ts +++ b/tests/system-restart.test.ts @@ -233,3 +233,4 @@ describe("POST /api/system/restart", () => { expect(body.message.toLowerCase()).toContain("drain"); }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/vision-anthropic.test.ts b/tests/vision-anthropic.test.ts index b0dab8f33..1df2a0c85 100644 --- a/tests/vision-anthropic.test.ts +++ b/tests/vision-anthropic.test.ts @@ -345,3 +345,4 @@ describe("Anthropic vision planning and management config", () => { } }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; diff --git a/tests/windows-tray.test.ts b/tests/windows-tray.test.ts index 29e245761..36cfe6f9c 100644 --- a/tests/windows-tray.test.ts +++ b/tests/windows-tray.test.ts @@ -275,3 +275,4 @@ describe("Windows tray packaging and command safety", () => { } }); }); +import { ManagementRequest as Request } from "./helpers/management-auth"; From 8e221b5d952a5a4283702d1be97ce8b5153761be Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 13:07:29 -0400 Subject: [PATCH 12/14] test(oauth): relax live-update timeout --- tests/oauth-login-cli-live-update.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/oauth-login-cli-live-update.test.ts b/tests/oauth-login-cli-live-update.test.ts index 7f8fa8275..06e993e52 100644 --- a/tests/oauth-login-cli-live-update.test.ts +++ b/tests/oauth-login-cli-live-update.test.ts @@ -93,5 +93,5 @@ describe("CLI OAuth live-update credential preservation", () => { } finally { await server.stop(true); } - }); + }, 15_000); }); From 64342c9492f9986dc60b2a124bbed7ce4ed70d98 Mon Sep 17 00:00:00 2001 From: YourName Date: Tue, 28 Jul 2026 15:49:45 -0400 Subject: [PATCH 13/14] security: pin provider discovery transports --- .../content/docs/reference/configuration.md | 25 +++ src/codex/catalog/provider-fetch.ts | 30 ++- src/images/artifacts.ts | 130 +---------- src/lib/destination-policy.ts | 55 +++-- src/lib/pinned-http.ts | 151 +++++++++++++ src/lib/provider-outbound.ts | 163 ++++++++++++++ src/lib/provider-url.ts | 14 ++ src/server/management/provider-routes.ts | 14 +- structure/04_transports-and-sidecars.md | 13 ++ tests/codex-catalog.test.ts | 2 + tests/destination-policy-resolved.test.ts | 39 +++- tests/fixtures/provider-outbound-e2e.ts | 135 ++++++++++++ tests/provider-connection-test.test.ts | 27 +++ tests/provider-live-models.test.ts | 81 ++++++- tests/provider-outbound.test.ts | 201 ++++++++++++++++++ 15 files changed, 925 insertions(+), 155 deletions(-) create mode 100644 src/lib/pinned-http.ts create mode 100644 src/lib/provider-outbound.ts create mode 100644 src/lib/provider-url.ts create mode 100644 tests/fixtures/provider-outbound-e2e.ts create mode 100644 tests/provider-outbound.test.ts diff --git a/docs-site/src/content/docs/reference/configuration.md b/docs-site/src/content/docs/reference/configuration.md index 01e2aa6f3..cb96546be 100644 --- a/docs-site/src/content/docs/reference/configuration.md +++ b/docs-site/src/content/docs/reference/configuration.md @@ -75,6 +75,31 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. `max_concurrent_threads_per_session` value under `[features.multi_agent_v2]` in Codex's `$CODEX_HOME/config.toml`; enable v2 first so that table exists. +## Provider diagnostic outbound safety + +The dashboard provider connection test and live model discovery use a bounded GET-only outbound +transport. Without an outbound proxy, opencodex resolves the provider hostname once and connects +only to that validated address. HTTPS keeps the original hostname for Host, SNI, and certificate +verification; certificate verification cannot be disabled by provider config. + +When `HTTP_PROXY` or `HTTPS_PROXY` applies, these two operations keep Bun's native fetch so existing +proxy behavior is not silently bypassed. URL/literal checks still run. Successful local DNS answers +are classified, but a local DNS failure is allowed through because proxy-only networks commonly +delegate name resolution to the proxy. The proxy chooses the final route, DNS answer, and peer, so +opencodex logs that this path cannot pin or verify the proxy-selected peer. This is an explicit +security limitation, not equivalent protection against DNS rebinding. + +Private/local provider destinations require both `allowPrivateNetwork: true` and a matching +`NO_PROXY` entry whenever an outbound proxy is configured. Loopback entries are added to `NO_PROXY` +automatically. A LAN provider such as `192.168.1.50` must be added explicitly; otherwise connection +tests and model discovery reject it with an actionable message instead of sending it to the proxy. +Metadata and link-local destinations remain blocked even when `allowPrivateNetwork` is enabled. + +Both direct and proxied diagnostic paths reject redirects and report a credential-stripped target; +configure the final provider URL directly. Ordinary provider requests, streaming responses, and +retry paths are not migrated in this phase. Their redirect handling and per-hop destination review +remain deferred, so this phase does not close the main-request redirect finding. + ## Combos (`config.combos`) Failover / round-robin aliases live under `combos.` with `targets` (provider + model), optional diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 7ad985eef..de9f220f7 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -39,7 +39,11 @@ import { targetKey, } from "../../combos"; import type { NormalizedComboConfig } from "../../combos/types"; -import { providerDestinationResolvedError } from "../../lib/destination-policy"; +import { + ProviderOutboundPolicyError, + providerOutboundGet, + providerRedirectError, +} from "../../lib/provider-outbound"; import { redactSecretString } from "../../lib/redact"; import upstreamModelsSnapshot from "../data/upstream-models.json"; @@ -322,21 +326,20 @@ export async function fetchProviderModels(name: string, prov: OcxProviderConfig, }; }; try { - const destinationError = await providerDestinationResolvedError(name, { - baseUrl: url, - allowPrivateNetwork: prov.allowPrivateNetwork, + const res = await providerOutboundGet(name, prov, url, { + headers, + signal: AbortSignal.timeout(8000), }); - if (destinationError) { - const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "blocked" }); + const redirectError = await providerRedirectError(res, url); + if (redirectError) { + const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "http", httpStatus: res.status }); if (shouldLog) { console.warn( - `[opencodex] Provider model discovery for "${name}" was blocked by destination policy: ${destinationError} [urlClass=${urlClass}, fallback=${fallback}].`, + `[opencodex] Provider model discovery for "${name}" ${redirectError} [urlClass=${urlClass}, fallback=${fallback}].`, ); } return models; } - - const res = await fetch(url, { headers, signal: AbortSignal.timeout(8000) }); if (!res.ok) { const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "http", httpStatus: res.status }); if (shouldLog) { @@ -421,6 +424,15 @@ export async function fetchProviderModels(name: string, prov: OcxProviderConfig, setCached(name, live); return live; } catch (error) { + if (error instanceof ProviderOutboundPolicyError) { + const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "blocked" }); + if (shouldLog) { + console.warn( + `[opencodex] Provider model discovery for "${name}" was blocked by destination policy: ${error.message} [urlClass=${urlClass}, fallback=${fallback}].`, + ); + } + return models; + } const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "network" }); if (shouldLog) { console.warn( diff --git a/src/images/artifacts.ts b/src/images/artifacts.ts index d5ad5e02f..f18b11847 100644 --- a/src/images/artifacts.ts +++ b/src/images/artifacts.ts @@ -1,12 +1,12 @@ import { readdirSync, readFileSync, statSync, unlinkSync, existsSync } from "node:fs"; import { mkdir, writeFile } from "node:fs/promises"; -import type { IncomingMessage } from "node:http"; -import https from "node:https"; -import type { RequestOptions } from "node:https"; import { basename, join, resolve, sep } from "node:path"; import { getConfigDir } from "../config"; import { assessUrlDestination, resolvePublicAddresses } from "../lib/destination-policy"; import { recordOwnedConfigPath } from "../lib/config-ownership"; +import { pinnedHttpGet, type PinnedAddress } from "../lib/pinned-http"; + +export type { PinnedAddress } from "../lib/pinned-http"; const MAX_DECODED_BYTES_PER_IMAGE = 50 * 1024 * 1024; const MAX_DECODED_BYTES_PER_RESPONSE = 100 * 1024 * 1024; @@ -40,8 +40,6 @@ export interface ImageBudget { spent: number; } -export type PinnedAddress = { address: string; family: number }; - /** Test seam / custom transport: must connect to `pinned`, not re-resolve `url`'s hostname. */ export type PinnedDownloadFn = ( url: string, @@ -282,120 +280,14 @@ export function pinnedHttpsGet( } const maxBytes = options?.maxBytes ?? MAX_DOWNLOAD_BYTES; const idleTimeoutMs = options?.idleTimeoutMs ?? DOWNLOAD_IDLE_TIMEOUT_MS; - - return new Promise((resolve, reject) => { - if (signal?.aborted) { - reject(signal.reason instanceof Error ? signal.reason : new Error("aborted")); - return; - } - - let settled = false; - const fail = (err: unknown) => { - try { req.destroy(); } catch { /* ignore */ } - if (settled) return; - settled = true; - reject(err instanceof Error ? err : new Error(String(err))); - }; - - const optionsHttps: RequestOptions & { servername?: string } = { - protocol: "https:", - hostname: parsed.hostname, - servername: parsed.hostname, - port: parsed.port || 443, - path: `${parsed.pathname}${parsed.search}`, - method: "GET", - headers: { Host: parsed.host }, - rejectUnauthorized: options?.rejectUnauthorized, - lookup(_hostname, lookupOptions, callback) { - const opts = typeof lookupOptions === "function" ? undefined : lookupOptions; - const cb = typeof lookupOptions === "function" ? lookupOptions : callback; - if (!cb) return; - // Pin the validated peer — do not call dns.lookup again. - // Honor `{ all: true }` array shape used by some Node/Bun https paths. - if (opts && typeof opts === "object" && "all" in opts && opts.all) { - (cb as (err: NodeJS.ErrnoException | null, addresses: PinnedAddress[]) => void)( - null, - [{ address: pinned.address, family: pinned.family }], - ); - return; - } - (cb as (err: NodeJS.ErrnoException | null, address: string, family: 4 | 6) => void)( - null, - pinned.address, - pinned.family as 4 | 6, - ); - }, - }; - - const req = https.request(optionsHttps, (res: IncomingMessage) => { - const status = res.statusCode ?? 0; - // Any non-2xx must destroy immediately. Returning a streaming Response for - // 4xx/5xx (or 3xx) lets callers that only check `Response.ok` abandon an - // unread body while the peer keeps sending — a failed-response socket leak. - if (status < 200 || status >= 300) { - try { res.destroy(); } catch { /* ignore */ } - fail(new Error("image download failed: " + status)); - return; - } - const headers = new Headers(); - for (const [key, value] of Object.entries(res.headers)) { - if (value === undefined || value === null) continue; - if (Array.isArray(value)) { - for (const item of value) headers.append(key, String(item)); - } else { - headers.set(key, String(value)); - } - } - - let received = 0; - const stream = new ReadableStream({ - start(controller) { - res.setTimeout(idleTimeoutMs, () => { - fail(new Error("image download stalled")); - try { controller.error(new Error("image download stalled")); } catch { /* closed */ } - }); - res.on("data", (chunk: Buffer | string) => { - const buf = typeof chunk === "string" ? Buffer.from(chunk) : chunk; - received += buf.byteLength; - if (received > maxBytes) { - const err = new Error(`image download exceeds ${maxBytes} byte cap`); - fail(err); - try { controller.error(err); } catch { /* closed */ } - return; - } - try { controller.enqueue(buf); } catch { /* closed */ } - }); - res.on("end", () => { - try { controller.close(); } catch { /* closed */ } - }); - res.on("error", (err: Error) => { - fail(err); - try { controller.error(err); } catch { /* closed */ } - }); - }, - cancel() { - req.destroy(); - }, - }); - - if (settled) return; - settled = true; - resolve(new Response(stream, { status, headers })); - }); - - const onAbort = () => { - fail(signal?.reason instanceof Error ? signal.reason : new Error("aborted")); - }; - signal?.addEventListener("abort", onAbort, { once: true }); - req.setTimeout(idleTimeoutMs, () => { - fail(new Error("image download timed out")); - }); - req.on("error", (err) => { - signal?.removeEventListener("abort", onAbort); - fail(err); - }); - req.on("close", () => signal?.removeEventListener("abort", onAbort)); - req.end(); + return pinnedHttpGet(url, pinned, signal, { + maxBytes, + idleTimeoutMs, + rejectUnauthorized: options?.rejectUnauthorized, + context: "image download", + }).then(response => { + if (!response.ok) throw new Error("image download failed: " + response.status); + return response; }); } diff --git a/src/lib/destination-policy.ts b/src/lib/destination-policy.ts index b2731df68..6d9ed9bda 100644 --- a/src/lib/destination-policy.ts +++ b/src/lib/destination-policy.ts @@ -204,6 +204,10 @@ export interface UrlDestinationAssessment { detail: string; } +export class DestinationDnsResolutionError extends Error { + override readonly name = "DestinationDnsResolutionError"; +} + /** * Synchronous literal URL destination assessment — classifies the hostname * without DNS resolution. Returns null for unparseable URLs. @@ -213,31 +217,44 @@ export function assessUrlDestination(url: string): UrlDestinationAssessment | nu } /** - * Async DNS-resolved URL safety check. Resolves A/AAAA records and rejects - * if any address is loopback, private, link-local, unspecified, or metadata. - * Throws on unsafe destination; returns the validated public addresses on success - * so callers can pin the connect peer and avoid a second, rebindable resolution. - * DNS resolution failures are treated as unsafe (fail-closed). + * Async DNS-resolved URL safety check. By default, rejects every non-public + * address. Provider diagnostics may explicitly admit loopback/private answers; + * metadata, link-local, and unspecified addresses remain unconditionally denied. + * Returns the validated addresses so direct callers can pin the connect peer and + * avoid a second, rebindable resolution. DNS failures remain fail-closed here; + * the provider proxy wrapper alone may recognize that typed failure and degrade. */ -export async function resolvePublicAddresses(url: string): Promise<{ +export async function resolvePublicAddresses( + url: string, + options?: { context?: string; allowPrivateNetwork?: boolean }, +): Promise<{ hostname: string; addresses: { address: string; family: number }[]; + privateNetwork: boolean; }> { + const context = options?.context?.trim() || "image URL"; + const privateNetworkAllowed = options?.allowPrivateNetwork === true; let hostname: string; try { hostname = normalizeHostname(new URL(url.trim()).hostname); } catch { - throw new Error("image URL is not a valid URL"); + throw new Error(`${context} is not a valid URL`); } - if (!hostname) throw new Error("image URL has no hostname"); + if (!hostname) throw new Error(`${context} has no hostname`); const literalAssessment = assessDestination(url); + let privateNetwork = false; if (literalAssessment && literalAssessment.kind !== "public" && literalAssessment.kind !== "hostname") { - throw new Error(`image URL targets ${literalAssessment.detail}`); + const allowedPrivateLiteral = privateNetworkAllowed + && (literalAssessment.kind === "localhost" + || literalAssessment.kind === "loopback" + || literalAssessment.kind === "private"); + if (!allowedPrivateLiteral) throw new Error(`${context} targets ${literalAssessment.detail}`); + privateNetwork = true; } // Literal public IPs: no DNS round-trip; pin the literal itself. const literalKind = isIP(hostname); if (literalKind !== 0) { - return { hostname, addresses: [{ address: hostname, family: literalKind }] }; + return { hostname, addresses: [{ address: hostname, family: literalKind }], privateNetwork }; } let addresses: { address: string; family: number }[]; try { @@ -245,23 +262,29 @@ export async function resolvePublicAddresses(url: string): Promise<{ } catch { // If DNS fails, we can't verify — fail-closed (unlike provider config-time validation, // this is a runtime fetch to an untrusted URL, so be conservative). - throw new Error(`image URL hostname ${hostname} could not be resolved`); + throw new DestinationDnsResolutionError(`${context} hostname ${hostname} could not be resolved`); } if (addresses.length === 0) { - throw new Error(`image URL hostname ${hostname} could not be resolved`); + throw new DestinationDnsResolutionError(`${context} hostname ${hostname} could not be resolved`); } - const publicAddresses: { address: string; family: number }[] = []; + const validatedAddresses: { address: string; family: number }[] = []; for (const { address, family } of addresses) { // Prefer classifying from the address string itself — do not trust a mislabeled // resolver `family` that could skip IPv4/IPv6 private checks. const ipKind = isIP(address) || (family === 4 || family === 6 ? family : 0); const assessment = ipKind === 4 ? classifyIpv4(address) : ipKind === 6 ? classifyIpv6(normalizeHostname(address)) : null; if (!assessment || assessment.kind !== "public") { - throw new Error(`image URL hostname ${hostname} resolves to ${assessment?.detail ?? "an unsafe address"} (${address})`); + const allowedPrivateAddress = privateNetworkAllowed + && assessment + && (assessment.kind === "loopback" || assessment.kind === "private"); + if (!allowedPrivateAddress) { + throw new Error(`${context} hostname ${hostname} resolves to ${assessment?.detail ?? "an unsafe address"} (${address})`); + } + privateNetwork = true; } - publicAddresses.push({ address, family: ipKind === 4 || ipKind === 6 ? ipKind : (family || 4) }); + validatedAddresses.push({ address, family: ipKind === 4 || ipKind === 6 ? ipKind : (family || 4) }); } - return { hostname, addresses: publicAddresses }; + return { hostname, addresses: validatedAddresses, privateNetwork }; } /** diff --git a/src/lib/pinned-http.ts b/src/lib/pinned-http.ts new file mode 100644 index 000000000..0a6ecf291 --- /dev/null +++ b/src/lib/pinned-http.ts @@ -0,0 +1,151 @@ +import http, { type IncomingMessage, type RequestOptions } from "node:http"; +import https from "node:https"; + +export type PinnedAddress = { address: string; family: number }; + +export interface PinnedHttpGetOptions { + headers?: HeadersInit; + maxBytes?: number; + idleTimeoutMs?: number; + rejectUnauthorized?: boolean; + context?: string; +} + +/** + * GET a URL through one previously validated address. The original hostname + * remains authoritative for Host, SNI, and certificate verification. + */ +export function pinnedHttpGet( + url: string, + pinned: PinnedAddress, + signal?: AbortSignal, + options?: PinnedHttpGetOptions, +): Promise { + const parsed = new URL(url); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + throw new Error(`${options?.context ?? "request"} must use HTTP or HTTPS, got ${parsed.protocol}`); + } + const context = options?.context ?? "request"; + const idleTimeoutMs = options?.idleTimeoutMs ?? 60_000; + const maxBytes = options?.maxBytes; + const headers = new Headers(options?.headers); + headers.set("host", parsed.host); + const requestHeaders: Record = {}; + headers.forEach((value, key) => { requestHeaders[key] = value; }); + + return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(signal.reason instanceof Error ? signal.reason : new Error("aborted")); + return; + } + + let settled = false; + const fail = (error: unknown) => { + try { req.destroy(); } catch { /* ignore */ } + if (settled) return; + settled = true; + reject(error instanceof Error ? error : new Error(String(error))); + }; + const requestOptions: RequestOptions & { servername?: string } = { + protocol: parsed.protocol, + hostname: parsed.hostname, + port: parsed.port || (parsed.protocol === "https:" ? 443 : 80), + path: `${parsed.pathname}${parsed.search}`, + method: "GET", + headers: requestHeaders, + ...(parsed.protocol === "https:" + ? { + servername: parsed.hostname, + rejectUnauthorized: options?.rejectUnauthorized ?? true, + } + : {}), + lookup(_hostname, lookupOptions, callback) { + const opts = typeof lookupOptions === "function" ? undefined : lookupOptions; + const cb = typeof lookupOptions === "function" ? lookupOptions : callback; + if (!cb) return; + if (opts && typeof opts === "object" && "all" in opts && opts.all) { + (cb as (error: NodeJS.ErrnoException | null, addresses: PinnedAddress[]) => void)( + null, + [{ address: pinned.address, family: pinned.family }], + ); + return; + } + (cb as (error: NodeJS.ErrnoException | null, address: string, family: 4 | 6) => void)( + null, + pinned.address, + pinned.family as 4 | 6, + ); + }, + }; + + const onResponse = (response: IncomingMessage) => { + const status = response.statusCode ?? 0; + const responseHeaders = new Headers(); + for (const [key, value] of Object.entries(response.headers)) { + if (value === undefined || value === null) continue; + if (Array.isArray(value)) { + for (const item of value) responseHeaders.append(key, String(item)); + } else { + responseHeaders.set(key, String(value)); + } + } + + if (status < 200 || status >= 300) { + try { response.destroy(); } catch { /* ignore */ } + try { req.destroy(); } catch { /* ignore */ } + if (settled) return; + settled = true; + resolve(new Response(null, { status, headers: responseHeaders })); + return; + } + + let received = 0; + const stream = new ReadableStream({ + start(controller) { + response.setTimeout(idleTimeoutMs, () => { + const error = new Error(`${context} stalled`); + fail(error); + try { controller.error(error); } catch { /* closed */ } + }); + response.on("data", (chunk: Buffer | string) => { + const buffer = typeof chunk === "string" ? Buffer.from(chunk) : chunk; + received += buffer.byteLength; + if (maxBytes !== undefined && received > maxBytes) { + const error = new Error(`${context} exceeds ${maxBytes} byte cap`); + fail(error); + try { controller.error(error); } catch { /* closed */ } + return; + } + try { controller.enqueue(buffer); } catch { /* closed */ } + }); + response.on("end", () => { + try { controller.close(); } catch { /* closed */ } + }); + response.on("error", (error: Error) => { + fail(error); + try { controller.error(error); } catch { /* closed */ } + }); + }, + cancel() { + req.destroy(); + }, + }); + + if (settled) return; + settled = true; + resolve(new Response(stream, { status, headers: responseHeaders })); + }; + + const requestFn = parsed.protocol === "https:" ? https.request : http.request; + const req = requestFn(requestOptions, onResponse); + const onAbort = () => fail(signal?.reason instanceof Error ? signal.reason : new Error("aborted")); + signal?.addEventListener("abort", onAbort, { once: true }); + req.setTimeout(idleTimeoutMs, () => fail(new Error(`${context} timed out`))); + req.on("error", error => { + signal?.removeEventListener("abort", onAbort); + fail(error); + }); + req.on("close", () => signal?.removeEventListener("abort", onAbort)); + req.end(); + }); +} diff --git a/src/lib/provider-outbound.ts b/src/lib/provider-outbound.ts new file mode 100644 index 000000000..9603391cc --- /dev/null +++ b/src/lib/provider-outbound.ts @@ -0,0 +1,163 @@ +import type { OcxProviderConfig } from "../types"; +import { + assessUrlDestination, + DestinationDnsResolutionError, + providerDestinationConfigError, + resolvePublicAddresses, +} from "./destination-policy"; +import { pinnedHttpGet } from "./pinned-http"; +import { publicProviderBaseUrl } from "./provider-url"; + +type ProviderGetInit = Omit; +type ProviderOutboundConfig = Pick & { + fetch?: typeof globalThis.fetch; +}; +export interface ProviderOutboundDependencies { + resolveAddresses?: typeof resolvePublicAddresses; + pinnedGet?: typeof pinnedHttpGet; +} + +export class ProviderOutboundPolicyError extends Error { + override readonly name = "ProviderOutboundPolicyError"; +} + +function pickPinnedAddress(addresses: Array<{ address: string; family: number }>): { address: string; family: number } { + return addresses.find(address => address.family === 4) ?? addresses[0]!; +} + +function configuredProxyFor(url: URL): boolean { + const values = url.protocol === "https:" + ? [process.env.HTTPS_PROXY, process.env.https_proxy] + : [process.env.HTTP_PROXY, process.env.http_proxy]; + return values.some(value => Boolean(value?.trim())); +} + +function normalizeProxyHostname(hostname: string): string { + const normalized = hostname.trim().toLowerCase().replace(/\.+$/, ""); + return normalized.startsWith("[") && normalized.endsWith("]") + ? normalized.slice(1, -1) + : normalized; +} + +function noProxyMatches(url: URL): boolean { + const raw = process.env.NO_PROXY ?? process.env.no_proxy ?? ""; + const hostname = normalizeProxyHostname(url.hostname); + const port = url.port || (url.protocol === "https:" ? "443" : "80"); + for (const rawEntry of raw.split(",")) { + let entry = rawEntry.trim().toLowerCase(); + if (!entry) continue; + if (entry === "*") return true; + entry = entry.replace(/^https?:\/\//, "").split("/", 1)[0]!; + + let entryHost = entry; + let entryPort = ""; + const bracketed = /^\[([^\]]+)](?::(\d+))?$/.exec(entry); + if (bracketed) { + entryHost = bracketed[1]!; + entryPort = bracketed[2] ?? ""; + } else if ((entry.match(/:/g)?.length ?? 0) === 1) { + const separator = entry.lastIndexOf(":"); + const possiblePort = entry.slice(separator + 1); + if (/^\d+$/.test(possiblePort)) { + entryHost = entry.slice(0, separator); + entryPort = possiblePort; + } + } + if (entryPort && entryPort !== port) continue; + entryHost = normalizeProxyHostname(entryHost.replace(/^\*?\./, "")); + if (!entryHost) continue; + if (hostname === entryHost || hostname.endsWith(`.${entryHost}`)) return true; + } + return false; +} + +let proxyBoundaryWarned = false; +let proxyDnsDegradationWarned = false; + +function warnProxyBoundaryOnce(): void { + if (proxyBoundaryWarned) return; + proxyBoundaryWarned = true; + console.warn( + "[opencodex] Provider outbound proxy mode preserves Bun proxy/NO_PROXY routing and validates " + + "the URL plus available local DNS results; the final route and peer cannot be pinned locally.", + ); +} + +function warnProxyDnsDegradationOnce(): void { + if (proxyDnsDegradationWarned) return; + proxyDnsDegradationWarned = true; + console.warn( + "[opencodex] Local DNS could not resolve a proxied provider hostname; continuing after URL/literal checks. " + + "The proxy-selected peer cannot be verified or pinned locally.", + ); +} + +export async function providerRedirectError(response: Response, requestUrl: string): Promise { + if (response.status < 300 || response.status >= 400) return null; + try { await response.body?.cancel(); } catch { /* ignore cancellation failures */ } + const location = response.headers.get("location"); + let target = "the final upstream URL"; + if (location) { + try { target = publicProviderBaseUrl(new URL(location, requestUrl).toString()); } catch { /* keep fallback */ } + } + return `provider returned ${response.status} redirect to ${target}; configure the final provider URL directly`; +} + +export async function providerOutboundGet( + name: string, + provider: ProviderOutboundConfig, + url: string, + init: ProviderGetInit = {}, + dependencies: ProviderOutboundDependencies = {}, +): Promise { + if (provider.fetch) { + const assessment = assessUrlDestination(url); + if (assessment?.kind === "metadata" || assessment?.kind === "link-local" || assessment?.kind === "unspecified") { + throw new ProviderOutboundPolicyError(`provider URL targets ${assessment.detail}`); + } + if (!provider.allowPrivateNetwork) { + const destinationError = providerDestinationConfigError(name, { + baseUrl: url, + allowPrivateNetwork: false, + }); + if (destinationError) throw new ProviderOutboundPolicyError(destinationError); + } + return provider.fetch(url, { ...init, method: "GET", redirect: "manual" }); + } + const parsed = new URL(url); + const proxyConfigured = configuredProxyFor(parsed); + const resolveAddresses = dependencies.resolveAddresses ?? resolvePublicAddresses; + const pinnedGet = dependencies.pinnedGet ?? pinnedHttpGet; + let resolved: Awaited>; + try { + resolved = await resolveAddresses(url, { + context: "provider URL", + allowPrivateNetwork: provider.allowPrivateNetwork, + }); + } catch (error) { + const dnsResolutionFailed = error instanceof DestinationDnsResolutionError + || (error instanceof Error && error.name === "DestinationDnsResolutionError"); + if (!dnsResolutionFailed) { + throw new ProviderOutboundPolicyError(error instanceof Error ? error.message : "provider destination was blocked"); + } + if (!proxyConfigured) throw error; + warnProxyBoundaryOnce(); + warnProxyDnsDegradationOnce(); + return globalThis.fetch(url, { ...init, method: "GET", redirect: "manual" }); + } + if (proxyConfigured && !resolved.privateNetwork) { + warnProxyBoundaryOnce(); + return globalThis.fetch(url, { ...init, method: "GET", redirect: "manual" }); + } + if (proxyConfigured && resolved.privateNetwork && !noProxyMatches(parsed)) { + const hostname = normalizeProxyHostname(parsed.hostname); + throw new Error( + `provider URL resolves to a private-network destination; add ${hostname} to NO_PROXY before using allowPrivateNetwork with an outbound proxy`, + ); + } + return pinnedGet(url, pickPinnedAddress(resolved.addresses), init.signal ?? undefined, { + headers: init.headers, + rejectUnauthorized: true, + context: "provider response", + }); +} diff --git a/src/lib/provider-url.ts b/src/lib/provider-url.ts new file mode 100644 index 000000000..36426394a --- /dev/null +++ b/src/lib/provider-url.ts @@ -0,0 +1,14 @@ +/** Strip credentials and non-routing URL components before displaying a provider URL. */ +export function publicProviderBaseUrl(baseUrl: string): string { + try { + const parsed = new URL(baseUrl.trim()); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return "(invalid URL)"; + parsed.username = ""; + parsed.password = ""; + parsed.search = ""; + parsed.hash = ""; + return parsed.toString().replace(/\/$/, baseUrl.endsWith("/") ? "/" : ""); + } catch { + return "(invalid URL)"; + } +} diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 53d17ff10..be77a448c 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -23,6 +23,7 @@ import { } from "../../oauth"; import { removeCredential } from "../../oauth/store"; import { providerDestinationResolvedError } from "../../lib/destination-policy"; +import { providerOutboundGet, providerRedirectError } from "../../lib/provider-outbound"; import { enrichProviderFromCatalog, listKeyLoginProviders } from "../../oauth/key-providers"; import { deriveProviderPresets } from "../../providers/derive"; import { providerCodexAccountMode } from "../../providers/registry"; @@ -334,8 +335,19 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise { baseUrl: "http://198.18.0.1/v1", apiKey: "sk-test", models: ["static-fallback"], + fetch: globalThis.fetch, }, }, }); @@ -1645,6 +1646,7 @@ describe("Codex catalog routed normalization", () => { baseUrl: "http://198.18.0.1/v1", allowPrivateNetwork: true, apiKey: "sk-test", + fetch: globalThis.fetch, }, }, }); diff --git a/tests/destination-policy-resolved.test.ts b/tests/destination-policy-resolved.test.ts index d47df9416..2ae103c13 100644 --- a/tests/destination-policy-resolved.test.ts +++ b/tests/destination-policy-resolved.test.ts @@ -4,7 +4,7 @@ import { describe, expect, mock, test } from "bun:test"; const lookupMock = mock(async (_hostname: string, _opts: unknown): Promise<{ address: string; family: number }[]> => []); mock.module("node:dns/promises", () => ({ lookup: lookupMock })); -const { providerDestinationConfigError, providerDestinationResolvedError } = await import("../src/lib/destination-policy"); +const { providerDestinationConfigError, providerDestinationResolvedError, resolvePublicAddresses } = await import("../src/lib/destination-policy"); const provider = (baseUrl: string, allowPrivateNetwork?: boolean) => ({ baseUrl, allowPrivateNetwork }); @@ -164,3 +164,40 @@ describe("providerDestinationResolvedError — canonical openai Clash fake-IP ex )).toContain("benchmark address (198.18.0.30)"); }); }); + +describe("resolvePublicAddresses — caller-specific diagnostics", () => { + test("provider callers do not receive image-URL DNS errors", async () => { + lookupMock.mockRejectedValueOnce(Object.assign(new Error("ENOTFOUND"), { code: "ENOTFOUND" })); + + await expect(resolvePublicAddresses( + "https://unresolvable.example/v1/models", + { context: "provider URL" }, + )).rejects.toThrow("provider URL hostname unresolvable.example could not be resolved"); + }); + + test("DNS resolution failures have a distinct error type for proxy degradation", async () => { + lookupMock.mockRejectedValueOnce(Object.assign(new Error("ENOTFOUND"), { code: "ENOTFOUND" })); + + let error: unknown; + try { + await resolvePublicAddresses("https://proxy-only.example/v1/models", { context: "provider URL" }); + } catch (caught) { + error = caught; + } + + expect(error).toBeInstanceOf(Error); + expect((error as Error).name).toBe("DestinationDnsResolutionError"); + }); + + test("provider private-network opt-in returns classified private addresses", async () => { + lookupMock.mockResolvedValueOnce([{ address: "192.168.1.50", family: 4 }]); + + const resolved = await resolvePublicAddresses( + "http://ollama.lan:11434/v1/models", + { context: "provider URL", allowPrivateNetwork: true }, + ); + + expect(resolved.privateNetwork).toBe(true); + expect(resolved.addresses).toEqual([{ address: "192.168.1.50", family: 4 }]); + }); +}); diff --git a/tests/fixtures/provider-outbound-e2e.ts b/tests/fixtures/provider-outbound-e2e.ts new file mode 100644 index 000000000..0aea4897c --- /dev/null +++ b/tests/fixtures/provider-outbound-e2e.ts @@ -0,0 +1,135 @@ +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import { saveConfig } from "../../src/config"; +import { fetchProviderModels } from "../../src/codex/catalog/provider-fetch"; +import { providerOutboundGet } from "../../src/lib/provider-outbound"; +import { handleManagementAPI } from "../../src/server/management-api"; +import type { OcxConfig } from "../../src/types"; +import { ManagementRequest as Request } from "../helpers/management-auth"; + +const proxyKeys = [ + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "http_proxy", + "https_proxy", + "all_proxy", +] as const; + +async function listen(server: ReturnType): Promise { + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + return (server.address() as AddressInfo).port; +} + +async function close(server: ReturnType): Promise { + await new Promise(resolve => server.close(() => resolve())); +} + +async function probe(config: OcxConfig, name: string): Promise> { + saveConfig(config); + const request = new Request(`http://127.0.0.1/api/providers/test?name=${name}`, { method: "POST" }); + const response = await handleManagementAPI(request, new URL(request.url), config, {}); + if (!response) throw new Error("handler returned no response"); + return await response.json() as Record; +} + +const proxyRequests: string[] = []; +const providerRequests: string[] = []; +const redirectTarget = new URL("http://final.example/v1/models?token=secret#fragment"); +redirectTarget.username = "user"; +redirectTarget.password = "password"; + +const proxy = createServer((request, response) => { + proxyRequests.push(request.url ?? ""); + if (request.url?.startsWith("http://connection-proxy.invalid/")) { + response.writeHead(302, { location: redirectTarget.toString() }); + response.end(); + return; + } + response.writeHead(200, { "content-type": "application/json" }); + response.end(request.url?.startsWith("http://proxy-models.invalid/") + ? '{"data":[{"id":"proxy-discovered-model"}]}' + : '{"data":[{"id":"proxied-model"}]}'); +}); +const provider = createServer((request, response) => { + providerRequests.push(request.url ?? ""); + response.writeHead(200, { "content-type": "application/json" }); + response.end('{"data":[{"id":"local-model"}]}'); +}); + +try { + const [proxyPort, providerPort] = await Promise.all([listen(proxy), listen(provider)]); + const proxyUrl = `http://127.0.0.1:${proxyPort}`; + process.env.HTTP_PROXY = proxyUrl; + process.env.http_proxy = proxyUrl; + process.env.NO_PROXY = "localhost,127.0.0.1,::1,[::1]"; + process.env.no_proxy = "localhost,127.0.0.1,::1,[::1]"; + + const outboundResponse = await providerOutboundGet( + "proxied", + { baseUrl: "http://proxy-only.invalid/v1", allowPrivateNetwork: false }, + "http://proxy-only.invalid/v1/models", + ); + const outbound = { status: outboundResponse.status, body: await outboundResponse.text() }; + + const managementProxy = await probe({ + port: 0, + hostname: "127.0.0.1", + defaultProvider: "proxied", + providers: { + proxied: { + adapter: "openai-chat", + baseUrl: "http://connection-proxy.invalid/v1", + apiKey: "sk-x", + }, + }, + } as OcxConfig, "proxied"); + + const proxyModels = await fetchProviderModels("proxy-discovery-e2e", { + baseUrl: "http://proxy-models.invalid/v1", + adapter: "openai-chat", + apiKey: "sk-test", + models: [], + }, 0); + + const localConfig = { + port: 0, + hostname: "127.0.0.1", + defaultProvider: "local", + providers: { + local: { + adapter: "openai-chat", + baseUrl: `http://127.0.0.1:${providerPort}/v1`, + apiKey: "sk-x", + allowPrivateNetwork: true, + }, + }, + } as OcxConfig; + const managementNoProxy = await probe(localConfig, "local"); + + for (const key of proxyKeys) delete process.env[key]; + const managementDirect = await probe(localConfig, "local"); + const directModels = await fetchProviderModels("direct-discovery-e2e", { + baseUrl: `http://127.0.0.1:${providerPort}/v1`, + adapter: "openai-chat", + apiKey: "sk-test", + allowPrivateNetwork: true, + models: [], + }, 0); + + console.log(JSON.stringify({ + outbound, + managementProxy, + proxyModels: proxyModels.map(model => model.id), + managementNoProxy, + managementDirect, + directModels: directModels.map(model => model.id), + proxyRequests, + providerRequests, + })); +} finally { + await Promise.all([close(proxy), close(provider)]); +} diff --git a/tests/provider-connection-test.test.ts b/tests/provider-connection-test.test.ts index 40b0790e5..e88c243df 100644 --- a/tests/provider-connection-test.test.ts +++ b/tests/provider-connection-test.test.ts @@ -24,6 +24,11 @@ afterEach(() => { }); function baseConfig(providers: OcxConfig["providers"]): OcxConfig { + if (globalThis.fetch !== originalFetch) { + for (const provider of Object.values(providers)) { + (provider as typeof provider & { fetch?: typeof fetch }).fetch = globalThis.fetch; + } + } const config = { port: 0, hostname: "127.0.0.1", @@ -53,6 +58,28 @@ describe("POST /api/providers/test (WP040 connectivity probe)", () => { expect(typeof body.error).toBe("string"); }); + test("metadata endpoints stay blocked even with private-network opt-in", async () => { + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(JSON.stringify({ data: [{ id: "should-not-load" }] }), { status: 200 }); + }) as typeof fetch; + const config = baseConfig({ + metadata: { + adapter: "openai-chat", + baseUrl: "http://169.254.169.254/latest/meta-data", + apiKey: "sk-x", + allowPrivateNetwork: true, + }, + }); + + const { body } = await probe(config, "metadata"); + + expect(body.ok).toBe(false); + expect(String(body.error)).toContain("blocked metadata endpoint"); + expect(fetches).toBe(0); + }); + test("static catalog cannot masquerade as a live connection", async () => { const config = baseConfig({ staticprov: { diff --git a/tests/provider-live-models.test.ts b/tests/provider-live-models.test.ts index 58228a89d..3b0fdbb61 100644 --- a/tests/provider-live-models.test.ts +++ b/tests/provider-live-models.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, spyOn, test } from "bun:test"; import { gatherRoutedModels } from "../src/codex/catalog"; import { clearModelCache } from "../src/codex/model-cache"; -import type { OcxConfig } from "../src/types"; +import type { OcxConfig, OcxProviderConfig } from "../src/types"; // Phase 2 of devlog/model_update/260709_model_refresh: live /models discovery is the // authoritative lineup; static config lists are the fallback seed. These tests pin the @@ -12,17 +12,24 @@ const HY3_PROVIDER = "opencode-go"; const HY3_CONTROL_PROVIDER = "hy3-control-live-test"; const OPENCODE_FREE_PROVIDER = "opencode-free"; +function withTestFetch(provider: T): T { + if (globalThis.fetch !== originalFetch) { + (provider as T & { fetch?: typeof fetch }).fetch = globalThis.fetch; + } + return provider; +} + function config(): OcxConfig { return { providers: { - [PROVIDER]: { + [PROVIDER]: withTestFetch({ baseUrl: "https://api.x.ai/v1", adapter: "openai-chat", authMode: "key", apiKey: "sk-test", models: ["grok-4.5", "grok-4.3"], modelContextWindows: { "grok-4.5": 500_000 }, - }, + }), }, } as unknown as OcxConfig; } @@ -88,19 +95,19 @@ describe("live provider model discovery (authority + fallback)", () => { const models = await gatherRoutedModels({ providers: { - [HY3_PROVIDER]: { + [HY3_PROVIDER]: withTestFetch({ baseUrl: "https://opencode-go.test/v1", adapter: "openai-chat", authMode: "key", apiKey: "sk-test", models: ["glm-5.2"], - }, - [HY3_CONTROL_PROVIDER]: { + }), + [HY3_CONTROL_PROVIDER]: withTestFetch({ baseUrl: "https://hy3-control.test/v1", adapter: "openai-chat", authMode: "key", apiKey: "sk-test", - }, + }), }, } as unknown as OcxConfig); const slugs = models.map(model => `${model.provider}/${model.id}`); @@ -141,14 +148,14 @@ describe("live provider model discovery (authority + fallback)", () => { const models = await gatherRoutedModels({ providers: { - [OPENCODE_FREE_PROVIDER]: { + [OPENCODE_FREE_PROVIDER]: withTestFetch({ baseUrl: "https://opencode.ai/zen/v1", adapter: "openai-chat", authMode: "key", keyOptional: true, models: ["big-pickle", "deepseek-v4-flash-free", "mimo-v2.5-free", "north-mini-code-free"], liveModels: true, - }, + }), }, } as unknown as OcxConfig); @@ -164,6 +171,62 @@ describe("live provider model discovery (authority + fallback)", () => { expect(ids.sort()).toEqual(["grok-4.3", "grok-4.5"]); }); + test("redirect responses log a credential-safe final-URL hint and fall back", async () => { + const warning = spyOn(console, "warn").mockImplementation(() => {}); + const redirectTarget = new URL("https://final.example/v1/models?token=secret#fragment"); + redirectTarget.username = "user"; + redirectTarget.password = "password"; + globalThis.fetch = (async () => new Response(null, { + status: 302, + headers: { + location: redirectTarget.toString(), + }, + })) as typeof fetch; + + try { + const models = await gatherRoutedModels(config()); + const ids = models.filter(model => model.provider === PROVIDER).map(model => model.id); + const warningText = warning.mock.calls.flat().join(" "); + + expect(ids.sort()).toEqual(["grok-4.3", "grok-4.5"]); + expect(warningText).toContain("returned 302 redirect"); + expect(warningText).toContain("https://final.example/v1/models"); + expect(warningText).not.toContain("user:password"); + expect(warningText).not.toContain("token=secret"); + } finally { + warning.mockRestore(); + } + }); + + test("link-local model discovery stays blocked despite private-network opt-in", async () => { + const providerName = "link-local-live-test"; + let fetches = 0; + globalThis.fetch = (async () => { + fetches += 1; + return new Response(JSON.stringify({ data: [{ id: "should-not-load" }] }), { status: 200 }); + }) as typeof fetch; + + try { + const models = await gatherRoutedModels({ + providers: { + [providerName]: withTestFetch({ + baseUrl: "http://169.254.10.20/v1", + adapter: "openai-chat", + apiKey: "sk-test", + allowPrivateNetwork: true, + models: ["configured-fallback"], + }), + }, + } as unknown as OcxConfig); + + expect(models.filter(model => model.provider === providerName).map(model => model.id)) + .toEqual(["configured-fallback"]); + expect(fetches).toBe(0); + } finally { + clearModelCache(providerName); + } + }); + test("oauth without a usable token still returns the configured static catalog", async () => { // No oauth store / no network: resolveModelsAuthToken yields no key, and the // catalog must not collapse to [] (GUI Models tab / rail counts). diff --git a/tests/provider-outbound.test.ts b/tests/provider-outbound.test.ts new file mode 100644 index 000000000..0578905d7 --- /dev/null +++ b/tests/provider-outbound.test.ts @@ -0,0 +1,201 @@ +import { afterEach, describe, expect, mock, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ProviderOutboundDependencies } from "../src/lib/provider-outbound"; + +const proxyKeys = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"] as const; +const originalProxyEnv = Object.fromEntries(proxyKeys.map(key => [key, process.env[key]])); + +afterEach(() => { + for (const key of proxyKeys) { + const previous = originalProxyEnv[key]; + if (previous === undefined) delete process.env[key]; + else process.env[key] = previous; + } +}); + +function directDependencies( + response: Response, + options?: { privateNetwork?: boolean; address?: string }, +): { + dependencies: ProviderOutboundDependencies; + captured: { address?: string; rejectUnauthorized?: boolean; authorization?: string }; +} { + const captured: { address?: string; rejectUnauthorized?: boolean; authorization?: string } = {}; + const address = options?.address ?? "93.184.216.34"; + return { + captured, + dependencies: { + resolveAddresses: mock(async () => ({ + hostname: "provider.example", + addresses: [{ address, family: 4 }], + privateNetwork: options?.privateNetwork === true, + })), + pinnedGet: mock(async (_url, pinned, _signal, requestOptions) => { + captured.address = pinned.address; + captured.rejectUnauthorized = requestOptions?.rejectUnauthorized; + captured.authorization = new Headers(requestOptions?.headers).get("authorization") ?? undefined; + return response; + }), + }, + }; +} + +describe("provider outbound GET transport", () => { + test("direct HTTPS connects only to the validated address with TLS verification", async () => { + for (const key of proxyKeys) delete process.env[key]; + const { providerOutboundGet } = await import("../src/lib/provider-outbound"); + const { dependencies, captured } = directDependencies(new Response('{"data":[]}', { + status: 200, + headers: { "content-type": "application/json" }, + })); + + const response = await providerOutboundGet( + "custom", + { baseUrl: "https://provider.example/v1" }, + "https://provider.example/v1/models", + { headers: { authorization: "Bearer test-key" } }, + dependencies, + ); + + expect(await response.json()).toEqual({ data: [] }); + expect(captured).toEqual({ + address: "93.184.216.34", + rejectUnauthorized: true, + authorization: "Bearer test-key", + }); + }); + + test("private providers behind a configured proxy require an explicit NO_PROXY match", async () => { + const proxyUrl = "http://127.0.0.1:9"; + process.env.HTTPS_PROXY = proxyUrl; + process.env.https_proxy = proxyUrl; + process.env.NO_PROXY = "localhost,127.0.0.1,::1,[::1]"; + process.env.no_proxy = "localhost,127.0.0.1,::1,[::1]"; + const { providerOutboundGet } = await import("../src/lib/provider-outbound"); + const { dependencies, captured } = directDependencies(new Response(null, { status: 200 }), { + privateNetwork: true, + address: "192.168.1.50", + }); + + await expect(providerOutboundGet( + "ollama-lan", + { baseUrl: "https://ollama.lan:11434/v1", allowPrivateNetwork: true }, + "https://ollama.lan:11434/v1/models", + {}, + dependencies, + )).rejects.toThrow(/add ollama\.lan to NO_PROXY/); + expect(captured.address).toBeUndefined(); + }); + + test("direct redirects return the same credential-safe final-URL guidance", async () => { + for (const key of proxyKeys) delete process.env[key]; + const redirectTarget = new URL("https://final.example/v1/models?token=secret#fragment"); + redirectTarget.username = "user"; + redirectTarget.password = "password"; + const { providerOutboundGet, providerRedirectError } = await import("../src/lib/provider-outbound"); + const { dependencies } = directDependencies(new Response(null, { + status: 302, + headers: { location: redirectTarget.toString() }, + })); + const requestUrl = "https://provider.example/v1/models"; + + const response = await providerOutboundGet( + "custom", + { baseUrl: "https://provider.example/v1" }, + requestUrl, + {}, + dependencies, + ); + const error = await providerRedirectError(response, requestUrl); + + expect(error).toContain("returned 302 redirect"); + expect(error).toContain("https://final.example/v1/models"); + expect(error).not.toContain("user:password"); + expect(error).not.toContain("token=secret"); + }); + + test("a per-provider fetch override remains the transport injection boundary", async () => { + for (const key of proxyKeys) delete process.env[key]; + const override = mock(async (_url: string | URL | Request, init?: RequestInit) => { + expect(init?.redirect).toBe("manual"); + return new Response('{"data":[{"id":"override-model"}]}', { + status: 200, + headers: { "content-type": "application/json" }, + }); + }) as typeof fetch; + const provider = { + baseUrl: "https://override.example/v1", + fetch: override, + } as { baseUrl: string; fetch: typeof fetch }; + const { providerOutboundGet } = await import("../src/lib/provider-outbound"); + + const response = await providerOutboundGet( + "override", + provider, + "https://override.example/v1/models", + ); + + expect(await response.json()).toEqual({ data: [{ id: "override-model" }] }); + expect(override).toHaveBeenCalledTimes(1); + }); + + test("proxy mode reaches one real proxy across outbound, connection-test, and model-discovery paths", async () => { + const childHome = mkdtempSync(join(tmpdir(), "ocx-provider-proxy-e2e-")); + const child = Bun.spawn([ + process.execPath, + "tests/fixtures/provider-outbound-e2e.ts", + ], { + cwd: process.cwd(), + env: { + ...process.env, + OPENCODEX_HOME: childHome, + }, + stdout: "pipe", + stderr: "pipe", + }); + + try { + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + const result = JSON.parse(stdout.trim()) as { + outbound: { status: number; body: string }; + managementProxy: Record; + proxyModels: string[]; + managementNoProxy: Record; + managementDirect: Record; + directModels: string[]; + proxyRequests: string[]; + providerRequests: string[]; + }; + + expect(exitCode).toBe(0); + expect(result.outbound).toEqual({ + status: 200, + body: '{"data":[{"id":"proxied-model"}]}', + }); + expect(result.managementProxy.ok).toBe(false); + expect(String(result.managementProxy.error)).toContain("returned 302 redirect"); + expect(String(result.managementProxy.error)).toContain("http://final.example/v1/models"); + expect(String(result.managementProxy.error)).not.toContain("user:password"); + expect(String(result.managementProxy.error)).not.toContain("token=secret"); + expect(result.proxyModels).toEqual(["proxy-discovered-model"]); + expect(result.managementNoProxy).toMatchObject({ ok: true, models: 1 }); + expect(result.managementDirect).toMatchObject({ ok: true, models: 1 }); + expect(result.directModels).toEqual(["local-model"]); + expect(result.proxyRequests).toEqual([ + "http://proxy-only.invalid/v1/models", + "http://connection-proxy.invalid/v1/models", + "http://proxy-models.invalid/v1/models", + ]); + expect(result.providerRequests).toEqual(["/v1/models", "/v1/models", "/v1/models"]); + expect(stderr).toContain("cannot be pinned locally"); + } finally { + rmSync(childHome, { recursive: true, force: true }); + } + }, 15_000); +}); From 14e1e6ba2e8e9130386bcd3821fca5b1e7fdf3f9 Mon Sep 17 00:00:00 2001 From: YourName Date: Wed, 29 Jul 2026 03:01:07 -0400 Subject: [PATCH 14/14] fix(security): honor all outbound proxy variables --- .../content/docs/reference/configuration.md | 6 ++-- src/cli/doctor.ts | 7 ++--- src/lib/provider-outbound.ts | 13 ++++---- src/lib/proxy-env.ts | 18 +++++++++++ structure/04_transports-and-sidecars.md | 7 +++-- tests/fixtures/provider-outbound-e2e.ts | 22 ++++++++----- tests/provider-outbound.test.ts | 13 ++++++-- ...subagent-fallback-handle-responses.test.ts | 31 ++++++++++++------- 8 files changed, 82 insertions(+), 35 deletions(-) create mode 100644 src/lib/proxy-env.ts diff --git a/docs-site/src/content/docs/reference/configuration.md b/docs-site/src/content/docs/reference/configuration.md index cb96546be..c95526af2 100644 --- a/docs-site/src/content/docs/reference/configuration.md +++ b/docs-site/src/content/docs/reference/configuration.md @@ -82,8 +82,8 @@ transport. Without an outbound proxy, opencodex resolves the provider hostname o only to that validated address. HTTPS keeps the original hostname for Host, SNI, and certificate verification; certificate verification cannot be disabled by provider config. -When `HTTP_PROXY` or `HTTPS_PROXY` applies, these two operations keep Bun's native fetch so existing -proxy behavior is not silently bypassed. URL/literal checks still run. Successful local DNS answers +When `HTTP_PROXY`, `HTTPS_PROXY`, or `ALL_PROXY` applies, these two operations keep Bun's native fetch +so existing proxy behavior is not silently bypassed. URL/literal checks still run. Successful local DNS answers are classified, but a local DNS failure is allowed through because proxy-only networks commonly delegate name resolution to the proxy. The proxy chooses the final route, DNS answer, and peer, so opencodex logs that this path cannot pin or verify the proxy-selected peer. This is an explicit @@ -94,6 +94,8 @@ Private/local provider destinations require both `allowPrivateNetwork: true` and automatically. A LAN provider such as `192.168.1.50` must be added explicitly; otherwise connection tests and model discovery reject it with an actionable message instead of sending it to the proxy. Metadata and link-local destinations remain blocked even when `allowPrivateNetwork` is enabled. +The safety guard accepts exact hosts, domain suffixes, optional ports, bracketed IPv6, and `*` in +`NO_PROXY`; it does not interpret CIDR entries, so list each private provider host or address explicitly. Both direct and proxied diagnostic paths reject redirects and report a credential-stripped target; configure the final provider URL directly. Ordinary provider requests, streaming responses, and diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index bfb902119..b468bbf74 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -13,6 +13,7 @@ import { dirname, join } from "node:path"; import { getConfigDir, getConfigPath, readConfigDiagnostics, readPid, readRuntimePort, resolveEnvValue } from "../config"; import { gracefulStopHost } from "../lib/process-control"; import { maskAccountId } from "../lib/privacy"; +import { PROXY_ENV_KEYS, proxyEnvPresent } from "../lib/proxy-env"; import { configuredAdminToken } from "../lib/admin-secrets"; import { readCodexTokens } from "../codex/auth-collision"; import { collectOrcaCodexHomeDiagnostic, resolveCodexHomeDir as resolveCodexHomeDirImpl, isWslRuntime, listWslWindowsCodexHomes, wslAutomountRoot, type CodexHomeDeps } from "../codex/home"; @@ -285,17 +286,15 @@ export function collectWslDualInstall(deps: WslDualInstallDeps = {}): WslDualIns }; } -const PROXY_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY"] as const; - export type ProxyEnvRow = { key: string; present: boolean }; export type EnvMap = Record; /** Report only presence/absence of proxy env vars - never the value (it may * embed credentials). Checks both upper- and lower-case forms. */ export function collectProxyEnv(env: EnvMap = process.env): ProxyEnvRow[] { - return PROXY_KEYS.map(key => ({ + return PROXY_ENV_KEYS.map(key => ({ key, - present: !!(env[key]?.trim() || env[key.toLowerCase()]?.trim()), + present: proxyEnvPresent(key, env), })); } diff --git a/src/lib/provider-outbound.ts b/src/lib/provider-outbound.ts index 9603391cc..e4f7d0a5c 100644 --- a/src/lib/provider-outbound.ts +++ b/src/lib/provider-outbound.ts @@ -6,6 +6,7 @@ import { resolvePublicAddresses, } from "./destination-policy"; import { pinnedHttpGet } from "./pinned-http"; +import { outboundProxyConfigured } from "./proxy-env"; import { publicProviderBaseUrl } from "./provider-url"; type ProviderGetInit = Omit; @@ -25,11 +26,8 @@ function pickPinnedAddress(addresses: Array<{ address: string; family: number }> return addresses.find(address => address.family === 4) ?? addresses[0]!; } -function configuredProxyFor(url: URL): boolean { - const values = url.protocol === "https:" - ? [process.env.HTTPS_PROXY, process.env.https_proxy] - : [process.env.HTTP_PROXY, process.env.http_proxy]; - return values.some(value => Boolean(value?.trim())); +function configuredProxyFor(): boolean { + return outboundProxyConfigured(); } function normalizeProxyHostname(hostname: string): string { @@ -111,6 +109,9 @@ export async function providerOutboundGet( dependencies: ProviderOutboundDependencies = {}, ): Promise { if (provider.fetch) { + // A caller-owned executor cannot be peer-pinned here. This branch keeps literal/config + // checks and redirect blocking, but does not provide the resolved-address guarantees of + // the built-in transport. Main-request migration must define that executor contract first. const assessment = assessUrlDestination(url); if (assessment?.kind === "metadata" || assessment?.kind === "link-local" || assessment?.kind === "unspecified") { throw new ProviderOutboundPolicyError(`provider URL targets ${assessment.detail}`); @@ -125,7 +126,7 @@ export async function providerOutboundGet( return provider.fetch(url, { ...init, method: "GET", redirect: "manual" }); } const parsed = new URL(url); - const proxyConfigured = configuredProxyFor(parsed); + const proxyConfigured = configuredProxyFor(); const resolveAddresses = dependencies.resolveAddresses ?? resolvePublicAddresses; const pinnedGet = dependencies.pinnedGet ?? pinnedHttpGet; let resolved: Awaited>; diff --git a/src/lib/proxy-env.ts b/src/lib/proxy-env.ts new file mode 100644 index 000000000..d34688ab9 --- /dev/null +++ b/src/lib/proxy-env.ts @@ -0,0 +1,18 @@ +export const OUTBOUND_PROXY_ENV_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY"] as const; +export const PROXY_ENV_KEYS = [...OUTBOUND_PROXY_ENV_KEYS, "NO_PROXY"] as const; + +export type ProxyEnvKey = typeof PROXY_ENV_KEYS[number]; +export type ProxyEnvMap = Record; + +export function proxyEnvPresent( + key: ProxyEnvKey, + env: ProxyEnvMap = process.env, +): boolean { + return Boolean(env[key]?.trim() || env[key.toLowerCase()]?.trim()); +} + +export function outboundProxyConfigured( + env: ProxyEnvMap = process.env, +): boolean { + return OUTBOUND_PROXY_ENV_KEYS.some(key => proxyEnvPresent(key, env)); +} diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index dfb8d18fc..6194d2902 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -4,14 +4,17 @@ Provider connection tests and live model discovery share the GET-only provider outbound wrapper. Direct HTTP(S) resolves once and pins the validated address; HTTPS preserves the original Host/SNI -and always verifies certificates. Proxy-configured requests stay on Bun fetch so HTTP(S)_PROXY and -NO_PROXY semantics remain authoritative. The wrapper classifies successful local DNS answers, but +and always verifies certificates. Proxy-configured requests stay on Bun fetch so HTTP(S)_PROXY, +ALL_PROXY, and NO_PROXY semantics remain authoritative. The wrapper classifies successful local DNS answers, but only a typed DNS-resolution failure degrades to proxy resolution; every literal, metadata, and resolved-address policy error still rejects. Proxy mode logs once that the proxy-selected peer cannot be pinned. Private destinations additionally require allowPrivateNetwork plus NO_PROXY. Both paths reject redirects and expose only credential-stripped final-address guidance. This phase does not cover ordinary requests, streaming, retries, or per-hop redirect review on those paths. +Caller-owned `provider.fetch` executors are also deferred: they receive literal/config checks and +redirect blocking, but cannot inherit DNS classification or peer pinning without a verified-peer +executor contract. Main-request migration must not treat that branch as fixed-transport equivalent. ## Responses HTTP/SSE diff --git a/tests/fixtures/provider-outbound-e2e.ts b/tests/fixtures/provider-outbound-e2e.ts index 0aea4897c..55f9ef2e6 100644 --- a/tests/fixtures/provider-outbound-e2e.ts +++ b/tests/fixtures/provider-outbound-e2e.ts @@ -3,18 +3,12 @@ import type { AddressInfo } from "node:net"; import { saveConfig } from "../../src/config"; import { fetchProviderModels } from "../../src/codex/catalog/provider-fetch"; import { providerOutboundGet } from "../../src/lib/provider-outbound"; +import { PROXY_ENV_KEYS } from "../../src/lib/proxy-env"; import { handleManagementAPI } from "../../src/server/management-api"; import type { OcxConfig } from "../../src/types"; import { ManagementRequest as Request } from "../helpers/management-auth"; -const proxyKeys = [ - "HTTP_PROXY", - "HTTPS_PROXY", - "ALL_PROXY", - "http_proxy", - "https_proxy", - "all_proxy", -] as const; +const proxyKeys = PROXY_ENV_KEYS.flatMap(key => [key, key.toLowerCase()]); async function listen(server: ReturnType): Promise { await new Promise((resolve, reject) => { @@ -95,6 +89,15 @@ try { models: [], }, 0); + for (const key of proxyKeys) delete process.env[key]; + process.env.ALL_PROXY = proxyUrl; + const allProxyResponse = await providerOutboundGet( + "all-proxy", + { baseUrl: "http://all-proxy-only.invalid/v1", allowPrivateNetwork: false }, + "http://all-proxy-only.invalid/v1/models", + ); + const allProxy = { status: allProxyResponse.status, body: await allProxyResponse.text() }; + const localConfig = { port: 0, hostname: "127.0.0.1", @@ -108,6 +111,8 @@ try { }, }, } as OcxConfig; + process.env.NO_PROXY = "localhost,127.0.0.1,::1,[::1]"; + process.env.no_proxy = "localhost,127.0.0.1,::1,[::1]"; const managementNoProxy = await probe(localConfig, "local"); for (const key of proxyKeys) delete process.env[key]; @@ -122,6 +127,7 @@ try { console.log(JSON.stringify({ outbound, + allProxy, managementProxy, proxyModels: proxyModels.map(model => model.id), managementNoProxy, diff --git a/tests/provider-outbound.test.ts b/tests/provider-outbound.test.ts index 0578905d7..de9b78d90 100644 --- a/tests/provider-outbound.test.ts +++ b/tests/provider-outbound.test.ts @@ -3,8 +3,9 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { ProviderOutboundDependencies } from "../src/lib/provider-outbound"; +import { PROXY_ENV_KEYS } from "../src/lib/proxy-env"; -const proxyKeys = ["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"] as const; +const proxyKeys = PROXY_ENV_KEYS.flatMap(key => [key, key.toLowerCase()]); const originalProxyEnv = Object.fromEntries(proxyKeys.map(key => [key, process.env[key]])); afterEach(() => { @@ -162,8 +163,12 @@ describe("provider outbound GET transport", () => { new Response(child.stderr).text(), child.exited, ]); + if (exitCode !== 0) { + throw new Error(`provider outbound fixture exited ${exitCode}: ${stderr.trim()}`); + } const result = JSON.parse(stdout.trim()) as { outbound: { status: number; body: string }; + allProxy: { status: number; body: string }; managementProxy: Record; proxyModels: string[]; managementNoProxy: Record; @@ -173,11 +178,14 @@ describe("provider outbound GET transport", () => { providerRequests: string[]; }; - expect(exitCode).toBe(0); expect(result.outbound).toEqual({ status: 200, body: '{"data":[{"id":"proxied-model"}]}', }); + expect(result.allProxy).toEqual({ + status: 200, + body: '{"data":[{"id":"proxied-model"}]}', + }); expect(result.managementProxy.ok).toBe(false); expect(String(result.managementProxy.error)).toContain("returned 302 redirect"); expect(String(result.managementProxy.error)).toContain("http://final.example/v1/models"); @@ -191,6 +199,7 @@ describe("provider outbound GET transport", () => { "http://proxy-only.invalid/v1/models", "http://connection-proxy.invalid/v1/models", "http://proxy-models.invalid/v1/models", + "http://all-proxy-only.invalid/v1/models", ]); expect(result.providerRequests).toEqual(["/v1/models", "/v1/models", "/v1/models"]); expect(stderr).toContain("cannot be pinned locally"); diff --git a/tests/subagent-fallback-handle-responses.test.ts b/tests/subagent-fallback-handle-responses.test.ts index afaf12ccb..298c9ded4 100644 --- a/tests/subagent-fallback-handle-responses.test.ts +++ b/tests/subagent-fallback-handle-responses.test.ts @@ -296,29 +296,29 @@ describe("subagent fallback without primary auth cooldown failure", () => { expect(response.status).not.toBe(429); }); - test("final-route auth failure does not leave a primary probe lease", async () => { + test("final-route direct auth failure does not acquire a pool probe lease", async () => { const now = 1_800_000_000_000; Date.now = () => now; installPoolCredential("pool-a", "pool_acc", now); const cfg: OcxConfig = { port: 0, - defaultProvider: "openai", + defaultProvider: "xai", activeCodexAccountId: "pool-a", autoSwitchThreshold: 80, - subagentModelFallback: ["openai-direct/gpt-5.5"], + subagentModelFallback: ["gpt-5.5"], providers: { openai: { - adapter: "openai-responses", - baseUrl: "https://chatgpt.com/backend-api/codex", - authMode: "forward", - codexAccountMode: "pool", - }, - "openai-direct": { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", authMode: "forward", codexAccountMode: "direct", }, + xai: { + adapter: "openai-chat", + baseUrl: "https://api.x.ai/v1", + authMode: "key", + apiKey: "xai-test", + }, }, codexAccounts: [ { id: "main", email: "main@example.test", isMain: true }, @@ -328,8 +328,16 @@ describe("subagent fallback without primary auth cooldown failure", () => { updateAccountQuota("pool-a", 95, undefined, 20); const resetAt = Math.floor((now + 4 * 24 * 60 * 60_000) / 1000); recordCodexUpstreamOutcome(cfg, "pool-a", 429, { resetAt, now }); + const { noteSubagentModelFailure } = await import("../src/codex/subagent-model-fallback"); + noteSubagentModelFailure("xai/grok-4.5", "429", cfg); - // Omit authorization so direct-mode final auth fails — primary never leased. + let fetchCalls = 0; + globalThis.fetch = (async () => { + fetchCalls += 1; + throw new Error("must not dispatch"); + }) as typeof fetch; + + // Omit authorization so the canonical Direct final route fails before dispatch. const response = await handleResponses( new Request("http://localhost/v1/responses", { method: "POST", @@ -338,7 +346,7 @@ describe("subagent fallback without primary auth cooldown failure", () => { "x-openai-subagent": "collab_spawn", }, body: JSON.stringify({ - model: "gpt-5.6-sol", + model: "xai/grok-4.5", input: readableAgentInput(), stream: false, }), @@ -348,6 +356,7 @@ describe("subagent fallback without primary auth cooldown failure", () => { ); expect(response.status).toBe(401); + expect(fetchCalls).toBe(0); expect(getCodexUpstreamHealth("pool-a")?.probeLeaseId).toBeUndefined(); }); });