Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions src/selfhost/ai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,21 @@ async function isolatedCliCwd(): Promise<string> {
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<string> {
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();
Expand Down Expand Up @@ -864,14 +879,25 @@ export function createClaudeCodeAi(parentEnv: Record<string, string | undefined>
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) {
Expand Down
28 changes: 23 additions & 5 deletions test/unit/selfhost-ai.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 = "";
Expand All @@ -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.
Expand Down
Loading