diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 2b5c1efb5c..634ac3ee16 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -505,6 +505,21 @@ async function isolatedCliCwd(): Promise { return mkdtemp(join(tmpdir(), "gittensory-ai-")); } +/** Write `systemAppend` into `cwd` (the SAME per-call isolated temp dir already used for the subprocess's + * cwd, so it shares that directory's lifecycle) and return its path, for `--append-system-prompt-file`. + * Keeps repo review instructions out of argv/`ps aux` (#3951's concern) WITHOUT falling back to smuggling + * them into the stdin prompt body, which claude-code's own safety training can flag as a prompt-injection + * pattern (a labeled "ADDITIONAL SYSTEM INSTRUCTIONS:" block inside otherwise-untrusted content) instead of + * genuine first-party configuration -- confirmed live via ai_review_provider_unparseable_exhausted events + * where the model refused with exactly that reasoning (#observability-plan-mode-injection-lookalike). */ +async function writeClaudeSystemPromptFile(cwd: string, systemAppend: string): Promise { + const { writeFile } = await import("node:fs/promises"); + const { join } = await import("node:path"); + const path = join(cwd, "system-append.txt"); + await writeFile(path, systemAppend, "utf8"); + return path; +} + /** 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(); @@ -864,14 +879,25 @@ export function createClaudeCodeAi(parentEnv: Record OTEL_METRIC_EXPORT_INTERVAL: parentEnv.OTEL_METRIC_EXPORT_INTERVAL, }); const systemAppend = normalizedSystemAppend(options); - const prompt = prependCliSystemAppend(toCliPrompt(options, systemAppend), systemAppend); + const prompt = toCliPrompt(options, systemAppend); const spawn = spawnImpl ?? (await defaultSpawn()); - const args = ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "plan", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"]; + const cwd = await isolatedCliCwd(); + // bypassPermissions (not "plan"): --disallowedTools already forbids every mutating/networked tool, so + // nothing left needs an interactive approval prompt -- and this call has no TTY to answer one anyway. + // "plan" activates Claude Code's full interactive Plan-Mode WORKFLOW (explore, draft a plan, wait for + // ExitPlanMode approval), not just a permission restriction, which confused the model into treating a + // one-shot review request as an interactive planning session instead of returning a JSON verdict + // (#observability-plan-mode-injection-lookalike, live in ai_review_provider_unparseable_exhausted). + const args = ["--print", "--output-format", "json", "--model", claudeModel, "--permission-mode", "bypassPermissions", "--effort", effort, "--disallowedTools", "Bash,Edit,Write,WebFetch,WebSearch"]; + // A dedicated system-prompt-file channel (not textual stdin-prepending) marks repo instructions + // unambiguously SYSTEM rather than author-controlled content -- see writeClaudeSystemPromptFile's doc + // comment for why the textual-prepend approach this replaces was itself the bug. + if (systemAppend) args.push("--append-system-prompt-file", await writeClaudeSystemPromptFile(cwd, systemAppend)); attempted = true; const { stdout, code, stderr, timedOut, stalledNoOutput } = await spawn( "claude", args, - { env, input: prompt, timeoutMs, firstOutputTimeoutMs, cwd: await isolatedCliCwd() }, + { env, input: prompt, timeoutMs, firstOutputTimeoutMs, cwd }, ); stdoutForMetrics = stdout; if (timedOut && stalledNoOutput) { diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index c4beee2605..0ab6516bec 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -1279,7 +1279,7 @@ describe("subscription CLI helpers + fail-safe", () => { expect(seen[seen.indexOf("--effort") + 1]).toBe("medium"); }); - it("Claude Code keeps systemAppend out of argv and supplies it through stdin once (#1471)", async () => { + it("Claude Code passes systemAppend through --append-system-prompt-file, never argv or stdin (#observability-plan-mode-injection-lookalike, was #1471/#3951)", async () => { const systemAppend = "REPOSITORY REVIEW INSTRUCTIONS: Follow async-error conventions."; let seen: string[] = []; let capturedInput = ""; @@ -1295,20 +1295,38 @@ describe("subscription CLI helpers + fail-safe", () => { ], systemAppend, }); - expect(seen).not.toContain("--append-system-prompt"); + // Never the literal value in argv (`ps aux` visibility, #3951) and never smuggled into stdin behind an + // "ADDITIONAL SYSTEM INSTRUCTIONS:" label (the pattern claude-code's own safety training flagged as a + // prompt injection in production, #observability-plan-mode-injection-lookalike) -- only a short file path. expect(seen).not.toContain(systemAppend); + expect(capturedInput).not.toContain("ADDITIONAL SYSTEM INSTRUCTIONS:"); + expect(capturedInput).not.toContain(systemAppend); expect(capturedInput).toContain("Base system."); expect(capturedInput).toContain("Review this diff."); - expect(countOccurrences(capturedInput, systemAppend)).toBe(1); + const flagIndex = seen.indexOf("--append-system-prompt-file"); + expect(flagIndex).toBeGreaterThan(-1); + const filePath = seen[flagIndex + 1] as string; + expect(readFileSync(filePath, "utf8")).toBe(systemAppend); await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, cap).run("", { prompt: "Review this diff.", systemAppend: " ", }); - expect(seen).not.toContain("--append-system-prompt"); + expect(seen).not.toContain("--append-system-prompt-file"); expect(capturedInput).toBe("Review this diff."); }); + it("Claude Code runs with --permission-mode bypassPermissions, not plan (#observability-plan-mode-injection-lookalike): disallowedTools already forbids every mutating tool, and 'plan' activates the interactive Plan-Mode workflow instead of just restricting permissions", async () => { + let seen: string[] = []; + const cap: StubSpawn = async (_c, a) => { + seen = a; + return { stdout: JSON.stringify({ type: "result", result: "ok" }), code: 0 }; + }; + await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t" }, cap).run("", { prompt: "x" }); + expect(seen[seen.indexOf("--permission-mode") + 1]).toBe("bypassPermissions"); + expect(seen).not.toContain("plan"); + }); + it("chat-only CLIs reject embeds so the chain routes embeddings to an embed-capable provider (Claude review + ollama embed)", async () => { const reviewOk: StubSpawn = async () => ({ stdout: JSON.stringify({ type: "result", result: "the review" }), code: 0 }); // A stand-in embed-capable provider (e.g. ollama): returns `data` for an embed request, `response` for chat.