From a75fbc7e597ca625ef4c5b68d43a0b0f63109c78 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Thu, 25 Jun 2026 15:39:52 -0700 Subject: [PATCH] fix(selfhost): isolate subscription cli environments --- src/selfhost/ai.ts | 74 ++++++++++++++++++++++++++++------- test/unit/selfhost-ai.test.ts | 46 ++++++++++++++++++---- 2 files changed, 99 insertions(+), 21 deletions(-) diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 9f58ae218..675ebaeee 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -114,17 +114,52 @@ export function createAnthropicAi(opts: { apiKey: string; model?: string | undef } // ── Subscription CLI providers (#979) — locally-authenticated `claude` / `codex` as a subprocess ────────── -// SECURITY: the child env DELETES the billable API keys so a misconfigured CLI cannot silently bill the -// metered API instead of using the subscription OAuth token. The CLI runs read-only / no extra tools. Any -// non-zero exit / empty output / error-envelope THROWS so the caller degrades — never a silent answer. -const BILLABLE_KEY_VARS = ["ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "CODEX_API_KEY", "OPENAI_API_KEY"] as const; +// SECURITY: subscription CLIs get a strict allowlisted env, not the worker env. This keeps runtime +// credentials out of prompt-injectable subprocesses while preserving CLI auth/home/proxy/cert settings. The CLI +// runs read-only / no extra tools, and non-zero exit / empty output / error-envelope THROWS so the caller degrades. +const SUBSCRIPTION_CLI_ENV_ALLOWLIST = [ + "CODEX_HOME", + "HOME", + "HTTPS_PROXY", + "HTTP_PROXY", + "LANG", + "LC_ALL", + "NODE_EXTRA_CA_CERTS", + "NO_PROXY", + "PATH", + "SSL_CERT_DIR", + "SSL_CERT_FILE", + "TERM", + "XDG_CONFIG_HOME", + "XDG_DATA_HOME", + "XDG_STATE_HOME", + "https_proxy", + "http_proxy", + "no_proxy", +] as const; -function scrubBillableKeys(parent: Record): Record { - const child = { ...parent }; - for (const k of BILLABLE_KEY_VARS) delete child[k]; +export function subscriptionCliEnv( + parent: Record, + extra: Record = {}, +): Record { + const child: Record = {}; + for (const key of SUBSCRIPTION_CLI_ENV_ALLOWLIST) { + const value = parent[key]; + if (value !== undefined) child[key] = value; + } + for (const [key, value] of Object.entries(extra)) { + if (value !== undefined) child[key] = value; + } return child; } +async function isolatedCliCwd(): Promise { + const { mkdtemp } = await import("node:fs/promises"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + return mkdtemp(join(tmpdir(), "gittensory-ai-")); +} + /** Pull the assistant's final text out of a CLI's JSON output (Claude Code `{result}` or Codex JSONL). */ export function extractCliText(stdout: string): string { const trimmed = stdout.trim(); @@ -163,14 +198,18 @@ export function claudeErrorStatus(stdout: string): string | null { return null; } -type SpawnFn = (cmd: string, args: string[], opts: { env: Record; input?: string; timeoutMs: number }) => Promise<{ stdout: string; code: number | null }>; +type SpawnFn = ( + cmd: string, + args: string[], + opts: { env: Record; input?: string; timeoutMs: number; cwd?: string }, +) => Promise<{ stdout: string; code: number | null }>; async function defaultSpawn(): Promise { const cp = await import("node:child_process"); return (cmd, args, o) => new Promise((resolve, reject) => { const stdio: ["pipe", "pipe", "pipe"] = ["pipe", "pipe", "pipe"]; - const child = cp.spawn(cmd, args, { env: o.env as NodeJS.ProcessEnv, stdio }); + const child = cp.spawn(cmd, args, { cwd: o.cwd, env: o.env as NodeJS.ProcessEnv, stdio }); let stdout = ""; /* v8 ignore start */ // a 120s subprocess timeout is not unit-testable without a 2-minute wait const timer = setTimeout(() => { @@ -200,13 +239,16 @@ export function createClaudeCodeAi(parentEnv: Record async run(model, options) { const token = parentEnv.CLAUDE_CODE_OAUTH_TOKEN; if (!token) throw new Error("claude_code_no_oauth_token"); - const env = scrubBillableKeys(parentEnv); - env.CLAUDE_CODE_OAUTH_TOKEN = token; + const env = subscriptionCliEnv(parentEnv, { CLAUDE_CODE_OAUTH_TOKEN: token }); const prompt = toMessages(options).map((m) => m.content).join("\n\n"); const spawn = spawnImpl ?? (await defaultSpawn()); const claudeModel = resolveModel(configuredModel(parentEnv), model, "claude-sonnet-4-6"); const effort = resolveEffort(parentEnv.AI_EFFORT); - const { stdout, code } = await spawn("claude", ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "plan", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"], { env, input: prompt, timeoutMs: 120_000 }); + const { stdout, code } = await spawn( + "claude", + ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "plan", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"], + { env, input: prompt, timeoutMs: 120_000, cwd: await isolatedCliCwd() }, + ); if (code !== 0) throw new Error(`claude_code_exit_${code ?? "null"}`); const errStatus = claudeErrorStatus(stdout); if (errStatus) throw new Error(`claude_code_error_${errStatus}`); @@ -223,7 +265,7 @@ export function createClaudeCodeAi(parentEnv: Record export function createCodexAi(parentEnv: Record, spawnImpl?: SpawnFn): SelfHostAi { return { async run(model, options) { - const env = scrubBillableKeys(parentEnv); + const env = subscriptionCliEnv(parentEnv); const prompt = toMessages(options).map((m) => m.content).join("\n\n"); const spawn = spawnImpl ?? (await defaultSpawn()); // codex 0.142+: `exec` is non-interactive — the old `--ask-for-approval` flag was REMOVED (passing it errors). @@ -234,7 +276,11 @@ export function createCodexAi(parentEnv: Record, spa const args = ["exec", "--json", "--skip-git-repo-check", "--sandbox", "read-only"]; if (codexModel) args.push("--model", codexModel); args.push("--", prompt); - const { stdout, code } = await spawn("codex", args, { env, timeoutMs: 120_000 }); + const { stdout, code } = await spawn("codex", args, { + env, + timeoutMs: 120_000, + cwd: await isolatedCliCwd(), + }); if (code !== 0) throw new Error(`codex_exit_${code ?? "null"}`); const text = extractCliText(stdout); if (!text) throw new Error("codex_empty_output"); diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index c793cc562..700d4b79d 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -2,7 +2,7 @@ import { chmodSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, resolveAiReviewerPlan, resolveEffort, resolveModel, resolveProviderNames, routeProviders } from "../../src/selfhost/ai"; +import { buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, resolveAiReviewerPlan, resolveEffort, resolveModel, resolveProviderNames, routeProviders, subscriptionCliEnv } from "../../src/selfhost/ai"; describe("resolveModel (#979 — never leak the Workers-AI default to a self-host backend)", () => { const WORKERS_DEFAULT = "@cf/meta/llama-3.1-8b-instruct-fp8-fast"; @@ -33,7 +33,11 @@ describe("resolveEffort (#selfhost-effort — Claude Code intelligence dial, def afterEach(() => vi.unstubAllGlobals()); type SpawnResult = { stdout: string; code: number | null }; -type StubSpawn = (cmd: string, args: string[], opts: { env: Record; input?: string; timeoutMs: number }) => Promise; +type StubSpawn = ( + cmd: string, + args: string[], + opts: { env: Record; input?: string; timeoutMs: number; cwd?: string }, +) => Promise; describe("createOpenAiCompatibleAi (#979)", () => { it("POSTs to /chat/completions and returns { response }", async () => { @@ -284,6 +288,17 @@ describe("branch coverage — defaults + edge inputs", () => { }); }); +describe("subscriptionCliEnv (allowlist + extra-override arms)", () => { + it("copies only allowlisted parent vars and drops everything else", () => { + const child = subscriptionCliEnv({ PATH: "/bin", HOME: "/root", ANTHROPIC_API_KEY: "sk-bill", WORKER_ONLY_VALUE: "internal" }); + expect(child).toEqual({ PATH: "/bin", HOME: "/root" }); + }); + it("merges a defined extra value but skips an undefined one", () => { + const child = subscriptionCliEnv({ PATH: "/bin" }, { CLAUDE_CODE_OAUTH_TOKEN: "t", UNSET: undefined }); + expect(child).toEqual({ PATH: "/bin", CLAUDE_CODE_OAUTH_TOKEN: "t" }); // UNSET (undefined) skips the extra-loop false arm + }); +}); + describe("subscription CLI helpers + fail-safe", () => { it("extractCliText pulls the result/text field", () => { expect(extractCliText(JSON.stringify({ type: "result", result: "ok" }))).toBe("ok"); @@ -303,9 +318,12 @@ describe("subscription CLI helpers + fail-safe", () => { capturedEnv = o.env; return { stdout: JSON.stringify({ type: "result", result: "review text" }), code: 0 }; }; - const out = await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t", ANTHROPIC_API_KEY: "sk-bill" }, stub).run("sonnet", { prompt: "x" }); + const out = await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t", ANTHROPIC_API_KEY: "sk-bill", WORKER_ONLY_VALUE: "internal" }, stub).run("sonnet", { + prompt: "x", + }); expect(out.response).toBe("review text"); - expect(capturedEnv.ANTHROPIC_API_KEY).toBeUndefined(); // scrubbed + expect(capturedEnv.ANTHROPIC_API_KEY).toBeUndefined(); // allowlisted subprocess env does not inherit metered API keys + expect(capturedEnv.WORKER_ONLY_VALUE).toBeUndefined(); expect(capturedEnv.CLAUDE_CODE_OAUTH_TOKEN).toBe("t"); }); @@ -327,15 +345,29 @@ describe("subscription CLI helpers + fail-safe", () => { it("Codex: 0.142+ exec flags (no --ask-for-approval, has --skip-git-repo-check); --model only when configured", async () => { let seen: string[] = []; - const ok: StubSpawn = async (_cmd, args) => { seen = args; return { stdout: JSON.stringify({ type: "result", result: "codex review" }), code: 0 }; }; + let capturedEnv: Record = {}; + let capturedCwd = ""; + const ok: StubSpawn = async (_cmd, args, opts) => { + seen = args; + capturedEnv = opts.env; + capturedCwd = opts.cwd ?? ""; + return { stdout: JSON.stringify({ type: "result", result: "codex review" }), code: 0 }; + }; // no configured model + the dual-router's empty model id → OMIT --model (codex picks the account default; // forcing e.g. gpt-5 fails on a ChatGPT-account login). And the removed --ask-for-approval must never appear. - expect((await createCodexAi({}, ok).run("", { prompt: "x" })).response).toBe("codex review"); + expect( + (await createCodexAi({ PATH: "/bin", CODEX_HOME: "/tmp/codex", WORKER_ONLY_VALUE: "internal", OPENAI_API_KEY: "sk-bill" }, ok).run("", { + prompt: "x", + })).response, + ).toBe("codex review"); expect(seen).toEqual(["exec", "--json", "--skip-git-repo-check", "--sandbox", "read-only", "--", "x"]); expect(seen).not.toContain("--ask-for-approval"); - // an explicit model (AI_MODEL, or a `codex:` reviewer id) IS passed through + expect(capturedEnv).toEqual({ CODEX_HOME: "/tmp/codex", PATH: "/bin" }); + expect(capturedCwd).toContain("gittensory-ai-"); + // an explicit model (AI_MODEL, or a `codex:` reviewer id) IS passed through but not inherited as env. await createCodexAi({ AI_MODEL: "o4-mini" }, ok).run("", { prompt: "x" }); expect(seen.join(" ")).toContain("--model o4-mini"); + expect(capturedEnv.AI_MODEL).toBeUndefined(); const bad: StubSpawn = async () => ({ stdout: "", code: 1 }); await expect(createCodexAi({}, bad).run("", { prompt: "x" })).rejects.toThrow(/codex_exit_1/); });