From 1fe6baba7b61b37ab4fac22f19417cd2f21c7da6 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sat, 11 Jul 2026 08:07:17 -0700 Subject: [PATCH] fix(selfhost): remove Claude reviewer tools --- src/selfhost/ai.ts | 27 ++++++++++++++++++++------- test/unit/selfhost-ai.test.ts | 6 +++++- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 634ac3ee1..eda2fd7a3 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -882,13 +882,26 @@ export function createClaudeCodeAi(parentEnv: Record const prompt = toCliPrompt(options, systemAppend); const spawn = spawnImpl ?? (await defaultSpawn()); 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"]; + // Keep bypassPermissions (not "plan") only to avoid a headless approval prompt; the actual boundary is + // tool removal. --tools "" removes every built-in tool, --strict-mcp-config prevents user/home MCP config + // from loading, and mcp__* is a defense-in-depth deny for CLIs that still have MCP tools available. This + // avoids a brittle mutating-tool denylist while preserving the one-shot JSON review flow. + const args = [ + "--print", + "--output-format", + "json", + "--model", + claudeModel, + "--permission-mode", + "bypassPermissions", + "--effort", + effort, + "--tools", + "", + "--strict-mcp-config", + "--disallowedTools", + "mcp__*", + ]; // 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. diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 0ab6516be..617cbed8f 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -1316,7 +1316,7 @@ describe("subscription CLI helpers + fail-safe", () => { 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 () => { + it("Claude Code disables all built-in and MCP tools while keeping bypassPermissions headless (#observability-plan-mode-injection-lookalike)", async () => { let seen: string[] = []; const cap: StubSpawn = async (_c, a) => { seen = a; @@ -1325,6 +1325,10 @@ describe("subscription CLI helpers + fail-safe", () => { 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"); + expect(seen[seen.indexOf("--tools") + 1]).toBe(""); + expect(seen).toContain("--strict-mcp-config"); + expect(seen[seen.indexOf("--disallowedTools") + 1]).toBe("mcp__*"); + expect(seen[seen.indexOf("--disallowedTools") + 1]).not.toContain("Bash"); }); it("chat-only CLIs reject embeds so the chain routes embeddings to an embed-capable provider (Claude review + ollama embed)", async () => {