From 1440e0669e56b36f037271e2b73d23ca82cbab67 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sat, 27 Jun 2026 10:36:24 -0700 Subject: [PATCH] fix(security): avoid logging raw AI CLI stderr --- src/selfhost/ai.ts | 17 ++++++++++++++-- test/unit/selfhost-ai.test.ts | 38 ++++++++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index bba446bf46..d887776466 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -275,6 +275,19 @@ export function redactSecrets(text: string, knownSecrets: readonly string[] = [] return out; } +/** Turn untrusted CLI stderr into a bounded, allowlisted hint. Stderr can echo auth files, proxy URLs, prompts, or + * config values in formats we do not know how to redact, and exhausted-provider errors flow to Sentry. Keep the + * operational signal that made exit codes diagnosable (#26) without copying arbitrary stderr into logs. */ +export function summarizeCliStderr(stderr: string | undefined, knownSecrets: readonly string[] = []): string { + const safe = redactSecrets(stderr ?? "", knownSecrets).toLowerCase(); + if (/\bauth(?:entication)?\b|api key|oauth|token|credential|unauthorized|forbidden/.test(safe)) return "auth_error"; + if (/rate.?limit|too many requests|\b429\b/.test(safe)) return "rate_limit"; + if (/model.*(?:not supported|unsupported|unknown|not found|invalid)|(?:not supported|unsupported).*model/.test(safe)) return "model_not_supported"; + if (/timeout|timed out|deadline/.test(safe)) return "timeout"; + if (/permission denied|eacces|eperm/.test(safe)) return "permission_denied"; + return safe.trim() ? "stderr_captured" : "no_stderr"; +} + /** Claude Code subscription (CLAUDE_CODE_OAUTH_TOKEN via `claude setup-token`). Headless, read-only, JSON. */ export function createClaudeCodeAi(parentEnv: Record, spawnImpl?: SpawnFn): SelfHostAi { return { @@ -295,7 +308,7 @@ export function createClaudeCodeAi(parentEnv: Record ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "plan", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"], { env, input: prompt, timeoutMs: resolveCliTimeoutMs(parentEnv), cwd: await isolatedCliCwd() }, ); - if (code !== 0) throw new Error(`claude_code_exit_${code ?? "null"}: ${redactSecrets(stderr ?? "", [token]).slice(0, 500)}`); + if (code !== 0) throw new Error(`claude_code_exit_${code ?? "null"}: ${summarizeCliStderr(stderr, [token])}`); const errStatus = claudeErrorStatus(stdout); if (errStatus) throw new Error(`claude_code_error_${errStatus}`); const text = extractCliText(stdout); @@ -329,7 +342,7 @@ export function createCodexAi(parentEnv: Record, spa timeoutMs: resolveCliTimeoutMs(parentEnv), cwd: await isolatedCliCwd(), }); - if (code !== 0) throw new Error(`codex_exit_${code ?? "null"}: ${redactSecrets(stderr ?? "").slice(0, 500)}`); + if (code !== 0) throw new Error(`codex_exit_${code ?? "null"}: ${summarizeCliStderr(stderr)}`); const text = extractCliText(stdout); if (!text) throw new Error("codex_empty_output"); return { response: text }; diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 3781675a9a..b347a71003 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, resolveCliTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, redactSecrets, routeProviders, subscriptionCliEnv } from "../../src/selfhost/ai"; +import { buildProvider, claudeErrorStatus, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, resolveAiReviewerPlan, resolveCliTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, redactSecrets, routeProviders, summarizeCliStderr, 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"; @@ -456,16 +456,16 @@ describe("subscription CLI helpers + fail-safe", () => { await expect(createCodexAi({}, empty).run("gpt-5", { prompt: "x" })).rejects.toThrow(/codex_empty_output/); }); - it("surfaces the CLI's stderr in the non-zero-exit error (diagnosable failures, #26)", async () => { - // Without stderr in the message, a `claude_code_exit_1` / `codex_exit_1` is an opaque dead-end; with it the real - // cause (auth, rate limit, model-not-supported) reaches the logs + Sentry. (stderr-present branch of `?? ""`.) + it("surfaces a safe stderr summary in the non-zero-exit error (diagnosable failures, #26)", async () => { + // Raw stderr can contain prompt/config/auth material; the error keeps the useful class of failure without + // copying the untrusted text into logs + Sentry. const claudeErr: StubSpawn = async () => ({ stdout: "", code: 1, stderr: "Invalid API key · auth_error" }); await expect( createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, claudeErr).run("m", { prompt: "x" }), - ).rejects.toThrow(/claude_code_exit_1: Invalid API key/); + ).rejects.toThrow(/claude_code_exit_1: auth_error/); const codexErr: StubSpawn = async () => ({ stdout: "", code: 1, stderr: "stream error: rate limit reached" }); await expect(createCodexAi({}, codexErr).run("m", { prompt: "x" })).rejects.toThrow( - /codex_exit_1: stream error: rate limit reached/, + /codex_exit_1: rate_limit/, ); }); @@ -477,12 +477,12 @@ describe("subscription CLI helpers + fail-safe", () => { expect(err).toContain("claude_code_exit_1:"); expect(err).not.toContain(token); expect(err).not.toContain("sk-ant-api03"); - expect(err).toContain("[redacted]"); + expect(err).toContain("auth_error"); }); it("redacts key-shaped tokens from codex stderr (no env token to key off) (#1605 sec)", async () => { const leaky: StubSpawn = async () => ({ stdout: "", code: 1, stderr: "auth failed: ghp_ABCDEFGHIJ0123456789KLMNOPQRSTUV" }); - await expect(createCodexAi({}, leaky).run("m", { prompt: "x" })).rejects.toThrow(/codex_exit_1: auth failed: \[redacted\]/); + await expect(createCodexAi({}, leaky).run("m", { prompt: "x" })).rejects.toThrow(/codex_exit_1: auth_error/); }); it("defaultSpawn captures a failing CLI's stderr and surfaces it on the exit error (#26)", async () => { @@ -497,7 +497,7 @@ describe("subscription CLI helpers + fail-safe", () => { try { await expect( createClaudeCodeAi({ ...process.env, CLAUDE_CODE_OAUTH_TOKEN: "t" }).run("sonnet", { prompt: "x" }), - ).rejects.toThrow(/claude_code_exit_1: BOOM: auth failed/); + ).rejects.toThrow(/claude_code_exit_1: auth_error/); } finally { process.env.PATH = origPath; } @@ -540,3 +540,23 @@ describe("redactSecrets — strip credentials from untrusted CLI stderr before i expect(redactSecrets("disk-usage-report-2024-summary failed")).toBe("disk-usage-report-2024-summary failed"); }); }); + +describe("summarizeCliStderr — classify untrusted CLI stderr without logging raw text", () => { + it("classifies common operational failures using only allowlisted summaries", () => { + expect(summarizeCliStderr("Invalid API key · auth_error")).toBe("auth_error"); + expect(summarizeCliStderr("stream error: rate limit reached")).toBe("rate_limit"); + expect(summarizeCliStderr("model gpt-x is not supported on this account")).toBe("model_not_supported"); + expect(summarizeCliStderr("request timed out at deadline")).toBe("timeout"); + expect(summarizeCliStderr("EACCES: permission denied")).toBe("permission_denied"); + }); + + it("withholds arbitrary stderr, including proxy credentials and prompt text", () => { + const stderr = "proxy http://user:p@ssw0rd@example.test failed while handling prompt: private repo text"; + expect(summarizeCliStderr(stderr)).toBe("stderr_captured"); + }); + + it("returns no_stderr for absent or empty stderr", () => { + expect(summarizeCliStderr(undefined)).toBe("no_stderr"); + expect(summarizeCliStderr(" ")).toBe("no_stderr"); + }); +});