From 4d6da50a7e9ec01eee41bdef75a247bb60c8bc23 Mon Sep 17 00:00:00 2001 From: SP Son <1376128+seungpyoson@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:55:10 +0900 Subject: [PATCH 1/6] fix(sandbox): run no hooks in read-only Claude children Review and adversarial review start Claude with -p in a worktree of the commit under review. Nothing limited setting sources, so that commit's .claude/settings.json hooks ran at startup: shell commands outside the Bash-free review allowlist, with no workspace trust prompt under -p. Set disableAllHooks in the read-only preset, which review, adversarial review, the stop-review gate and read-only rescue share. It applies only to the spawned child and keeps project CLAUDE.md and permissions. Fixes #117 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017ZnqeTJxgWVEKtH4XefpUV --- scripts/lib/claude-cli.mjs | 9 +++++++-- tests/sandbox-modes.test.mjs | 7 ++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 68ffdbb..9677826 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -466,13 +466,18 @@ export const SANDBOX_REVIEW_TOOLS = [ * * read-only: no file writes outside the OS temp dir. Network is allowed so * that `WebFetch`, `WebSearch`, and the Claude CLI's API path keep - * working; the review allowlist excludes Bash entirely, so there - * is no shell surface to exfiltrate or mutate state through. + * working; the review allowlist excludes Bash entirely and hooks + * are disabled, so there is no shell surface to exfiltrate or + * mutate state through. * workspace-write: Bash can write to cwd + OS temp dir only, no network from Bash. * All tools allowed (no allowedTools restriction). */ export const SANDBOX_SETTINGS = { "read-only": { + // Reviews run in a checkout of the code under review, whose + // .claude/settings.json can register hooks. Hooks run outside the tool + // allowlist and `-p` skips workspace trust, so read-only children run none. + disableAllHooks: true, sandbox: { enabled: true, // No Bash in the review allowlist, but keep this flag conservative so that diff --git a/tests/sandbox-modes.test.mjs b/tests/sandbox-modes.test.mjs index ee9c303..4971118 100644 --- a/tests/sandbox-modes.test.mjs +++ b/tests/sandbox-modes.test.mjs @@ -216,10 +216,15 @@ describe("sandbox settings content", () => { assert.deepEqual(s.sandbox.filesystem.allowWrite, [SANDBOX_TEMP_DIR]); // network block intentionally omitted: review/adversarial-review need network // for WebFetch/WebSearch and the Claude CLI's own API access. Mutation - // surfaces are closed off by removing Bash from the allowlist instead. + // surfaces are closed off by removing Bash from the allowlist and disabling + // hooks instead. assert.equal(s.sandbox.network, undefined); }); + it("read-only: disables hooks, including those the checkout under review registers", () => { + assert.equal(SANDBOX_SETTINGS["read-only"].disableAllHooks, true); + }); + it("workspace-write: sandbox enabled, allowWrite cwd+temp dir, no network", () => { const s = SANDBOX_SETTINGS["workspace-write"]; assert.equal(s.sandbox.enabled, true); From 943db2f449e7239be44b559efbffb41b1b531f5a Mon Sep 17 00:00:00 2001 From: SP Son <1376128+seungpyoson@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:34:32 +0900 Subject: [PATCH 2/6] fix(review): load user settings only in review children disableAllHooks closed one path from the code under review into its reviewer and left the rest: with project settings loaded, the checkout's `env` reached the processes the child starts (the git MCP server) and its CLAUDE.md and rules reached the model, and a print-mode child never asks whether to trust them. It also switched off the user's own hooks. runClaudeReview now always passes --setting-sources user, after caller options, so standard review, adversarial review and the stop-review gate load no project or local settings. --settings and managed settings still apply. Rescue keeps project settings. The read-only preset is unchanged from main. Fixes #117 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017ZnqeTJxgWVEKtH4XefpUV --- scripts/lib/claude-cli.mjs | 19 +++++++----- tests/claude-cli.test.mjs | 6 ++++ tests/hooks.test.mjs | 1 + tests/integration/claude-companion.test.mjs | 32 +++++++++++++++++++++ tests/sandbox-modes.test.mjs | 7 +---- 5 files changed, 52 insertions(+), 13 deletions(-) diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 9677826..69b4113 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -466,18 +466,13 @@ export const SANDBOX_REVIEW_TOOLS = [ * * read-only: no file writes outside the OS temp dir. Network is allowed so * that `WebFetch`, `WebSearch`, and the Claude CLI's API path keep - * working; the review allowlist excludes Bash entirely and hooks - * are disabled, so there is no shell surface to exfiltrate or - * mutate state through. + * working; the review allowlist excludes Bash entirely, so there + * is no shell surface to exfiltrate or mutate state through. * workspace-write: Bash can write to cwd + OS temp dir only, no network from Bash. * All tools allowed (no allowedTools restriction). */ export const SANDBOX_SETTINGS = { "read-only": { - // Reviews run in a checkout of the code under review, whose - // .claude/settings.json can register hooks. Hooks run outside the tool - // allowlist and `-p` skips workspace trust, so read-only children run none. - disableAllHooks: true, sandbox: { enabled: true, // No Bash in the review allowlist, but keep this flag conservative so that @@ -751,6 +746,9 @@ export function buildArgs(prompt, options = {}) { if (options.settingsFile) { args.push("--settings", options.settingsFile); } + if (options.settingSources) { + args.push("--setting-sources", options.settingSources); + } if (options.mcpConfigFile) { args.push("--mcp-config", options.mcpConfigFile); } @@ -887,6 +885,12 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { * MCP tool surface). Callers that want to run with an alternative allowlist — * e.g., legacy `SANDBOX_READ_ONLY_TOOLS` for back-compat — can override via * `options.allowedTools`. Bash is intentionally excluded by default. + * + * Reviews load user settings only. The code under review can carry + * `.claude/settings.json` and `CLAUDE.md`, and a print-mode child never asks + * whether to trust them: project hooks would run, project `env` would reach + * the processes the child starts, and project instructions would reach the + * model. `--settings` and managed settings still apply. */ export async function runClaudeReview(cwd, prompt, options = {}) { // Use streaming mode (same as runClaudeTurn) for progress reporting @@ -894,6 +898,7 @@ export async function runClaudeReview(cwd, prompt, options = {}) { noSessionPersistence: true, allowedTools: SANDBOX_REVIEW_TOOLS, ...options, + settingSources: "user", }); return { diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index 5c37212..8c8f97c 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -798,6 +798,12 @@ describe("buildArgs", () => { assert.ok(idx >= 0); assert.equal(args[idx + 1], "/tmp/s.json"); }); + + it("includes --setting-sources only when settingSources is provided", () => { + const args = buildArgs("p", { settingSources: "user" }); + assert.equal(args[args.indexOf("--setting-sources") + 1], "user"); + assert.equal(buildArgs("p", {}).includes("--setting-sources"), false); + }); }); // =========================================================================== diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index a03c819..3a7b180 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -382,6 +382,7 @@ describe("hooks", () => { assert.ok(permissionModeIndex >= 0); assert.equal(claudeArgs[permissionModeIndex + 1], "dontAsk"); assert.ok(claudeArgs.includes("--settings")); + assert.equal(claudeArgs[claudeArgs.indexOf("--setting-sources") + 1], "user"); assert.ok(claudeArgs.includes("--mcp-config")); assert.ok(claudeArgs.includes("--strict-mcp-config")); diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index 5e808ba..fc6c854 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -1059,6 +1059,38 @@ describe("claude-companion integration", () => { } }); + it("loads user settings only in review children and keeps project settings for tasks", () => { + const testEnv = createTestEnvironment(); + + try { + const taskArgsFile = path.join(testEnv.rootDir, "setting-sources-task-args.json"); + runCompanion( + ["task", "--cwd", testEnv.workspaceDir, "--quiet-progress", "setting sources task delay=20"], + { env: { ...testEnv.env, CLAUDE_ARGS_FILE: taskArgsFile } } + ); + const taskArgs = JSON.parse(fs.readFileSync(taskArgsFile, "utf8")); + assert.equal(taskArgs.includes("--setting-sources"), false); + + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + + for (const [command, focusText] of [ + ["review", []], + ["adversarial-review", ["focus on settings"]], + ]) { + const invocationFile = path.join(testEnv.rootDir, `setting-sources-${command}-invocation.json`); + runCompanion( + [command, "--cwd", testEnv.workspaceDir, "--scope", "working-tree", ...focusText], + { env: { ...testEnv.env, CLAUDE_INVOCATION_FILE: invocationFile } } + ); + const { args } = JSON.parse(fs.readFileSync(invocationFile, "utf8")); + assert.equal(args[args.indexOf("--setting-sources") + 1], "user", command); + } + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("uses --resume to continue the latest session and keeps --fresh from injecting a resume id", async () => { const testEnv = createTestEnvironment(); const sessionEnv = { diff --git a/tests/sandbox-modes.test.mjs b/tests/sandbox-modes.test.mjs index 4971118..ee9c303 100644 --- a/tests/sandbox-modes.test.mjs +++ b/tests/sandbox-modes.test.mjs @@ -216,15 +216,10 @@ describe("sandbox settings content", () => { assert.deepEqual(s.sandbox.filesystem.allowWrite, [SANDBOX_TEMP_DIR]); // network block intentionally omitted: review/adversarial-review need network // for WebFetch/WebSearch and the Claude CLI's own API access. Mutation - // surfaces are closed off by removing Bash from the allowlist and disabling - // hooks instead. + // surfaces are closed off by removing Bash from the allowlist instead. assert.equal(s.sandbox.network, undefined); }); - it("read-only: disables hooks, including those the checkout under review registers", () => { - assert.equal(SANDBOX_SETTINGS["read-only"].disableAllHooks, true); - }); - it("workspace-write: sandbox enabled, allowWrite cwd+temp dir, no network", () => { const s = SANDBOX_SETTINGS["workspace-write"]; assert.equal(s.sandbox.enabled, true); From fc2ca15f84ffde38eabe19f5cfcaa3b47c82191c Mon Sep 17 00:00:00 2001 From: SP Son <1376128+seungpyoson@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:17:08 +0900 Subject: [PATCH 3/6] fix(review): load user settings only in the review worktree Forcing --setting-sources user on every review also dropped the user's own project settings where reviews run in their repository: working-tree reviews and the stop-review gate lost project deny rules (for example Read(./.env), with Read in the review allowlist) and the repository's CLAUDE.md. The exposure in #117 is the review worktree: a checkout of the commit under review that the user never trusted, in which a print-mode child loads its settings without asking. createReviewIsolation now returns the setting sources for the directory it chooses: "user" for a worktree, none for the user's repository, where the child loads settings as any Claude session there does. Review and adversarial review pass it through; runClaudeReview no longer forces a value. Fixes #117 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017ZnqeTJxgWVEKtH4XefpUV --- scripts/claude-companion.mjs | 2 ++ scripts/lib/claude-cli.mjs | 7 ------ scripts/lib/review-worktree.mjs | 13 ++++++++--- tests/hooks.test.mjs | 3 ++- tests/integration/claude-companion.test.mjs | 25 ++++++++++++++------- tests/review-worktree.test.mjs | 2 ++ 6 files changed, 33 insertions(+), 19 deletions(-) diff --git a/scripts/claude-companion.mjs b/scripts/claude-companion.mjs index 2a16c9e..f4099e4 100644 --- a/scripts/claude-companion.mjs +++ b/scripts/claude-companion.mjs @@ -805,6 +805,7 @@ async function executeReviewRun(request) { onSpawn: request.onSpawn, permissionMode: "dontAsk", settingsFile: sandboxSettingsFile, + settingSources: isolation.settingSources, mcpConfigFile, strictMcpConfig: true, }); @@ -878,6 +879,7 @@ async function executeReviewRun(request) { onSpawn: request.onSpawn, permissionMode: "dontAsk", settingsFile: sandboxSettingsFile, + settingSources: isolation.settingSources, mcpConfigFile, strictMcpConfig: true, } diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 69b4113..caede13 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -885,12 +885,6 @@ export async function runClaudeTurn(cwd, prompt, options = {}) { * MCP tool surface). Callers that want to run with an alternative allowlist — * e.g., legacy `SANDBOX_READ_ONLY_TOOLS` for back-compat — can override via * `options.allowedTools`. Bash is intentionally excluded by default. - * - * Reviews load user settings only. The code under review can carry - * `.claude/settings.json` and `CLAUDE.md`, and a print-mode child never asks - * whether to trust them: project hooks would run, project `env` would reach - * the processes the child starts, and project instructions would reach the - * model. `--settings` and managed settings still apply. */ export async function runClaudeReview(cwd, prompt, options = {}) { // Use streaming mode (same as runClaudeTurn) for progress reporting @@ -898,7 +892,6 @@ export async function runClaudeReview(cwd, prompt, options = {}) { noSessionPersistence: true, allowedTools: SANDBOX_REVIEW_TOOLS, ...options, - settingSources: "user", }); return { diff --git a/scripts/lib/review-worktree.mjs b/scripts/lib/review-worktree.mjs index b82792e..92b0dfb 100644 --- a/scripts/lib/review-worktree.mjs +++ b/scripts/lib/review-worktree.mjs @@ -137,9 +137,15 @@ function cleanupWorktreeDir(worktreePath) { * would mislead Claude into thinking the repo is unchanged. Instead we run in * the original repo and rely on the Bash-free allowlist for containment. * - * Returns `{ cwd, gitRoot, cleanup }`. `gitRoot` is the path the MCP git server - * should be rooted at; `cwd` is what the Claude CLI should treat as the - * working directory. + * Returns `{ cwd, gitRoot, cleanup, isolated, settingSources }`. `gitRoot` is + * the path the MCP git server should be rooted at; `cwd` is what the Claude + * CLI should treat as the working directory. `settingSources` is the Claude + * `--setting-sources` value for that directory. A review worktree is a + * checkout of the commit under review that the user never trusted, and a + * print-mode child never asks: its `.claude/settings.json` hooks and `env` + * and its `CLAUDE.md` would load. So a worktree run loads user settings only. + * A working-tree review runs in the user's own repository and loads its + * settings as any Claude session there does. */ export function createReviewIsolation(repoRoot, target, { label = "review" } = {}) { if (target?.mode === "working-tree") { @@ -156,6 +162,7 @@ export function createReviewIsolation(repoRoot, target, { label = "review" } = { gitRoot: worktree.path, cleanup: () => worktree.cleanup(), isolated: true, + settingSources: "user", }; } diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 3a7b180..28b75a5 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -382,7 +382,8 @@ describe("hooks", () => { assert.ok(permissionModeIndex >= 0); assert.equal(claudeArgs[permissionModeIndex + 1], "dontAsk"); assert.ok(claudeArgs.includes("--settings")); - assert.equal(claudeArgs[claudeArgs.indexOf("--setting-sources") + 1], "user"); + // The gate runs in the user's own repository and loads its settings. + assert.equal(claudeArgs.includes("--setting-sources"), false); assert.ok(claudeArgs.includes("--mcp-config")); assert.ok(claudeArgs.includes("--strict-mcp-config")); diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index fc6c854..8138c72 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -1059,7 +1059,7 @@ describe("claude-companion integration", () => { } }); - it("loads user settings only in review children and keeps project settings for tasks", () => { + it("loads user settings only for reviews in a review worktree", () => { const testEnv = createTestEnvironment(); try { @@ -1078,13 +1078,22 @@ describe("claude-companion integration", () => { ["review", []], ["adversarial-review", ["focus on settings"]], ]) { - const invocationFile = path.join(testEnv.rootDir, `setting-sources-${command}-invocation.json`); - runCompanion( - [command, "--cwd", testEnv.workspaceDir, "--scope", "working-tree", ...focusText], - { env: { ...testEnv.env, CLAUDE_INVOCATION_FILE: invocationFile } } - ); - const { args } = JSON.parse(fs.readFileSync(invocationFile, "utf8")); - assert.equal(args[args.indexOf("--setting-sources") + 1], "user", command); + for (const [target, expected] of [ + [["--scope", "working-tree"], undefined], + [["--base", "main"], "user"], + ]) { + const invocationFile = path.join( + testEnv.rootDir, + `setting-sources-${command}-${target[0].slice(2)}-invocation.json` + ); + runCompanion( + [command, "--cwd", testEnv.workspaceDir, ...target, ...focusText], + { env: { ...testEnv.env, CLAUDE_INVOCATION_FILE: invocationFile } } + ); + const { args } = JSON.parse(fs.readFileSync(invocationFile, "utf8")); + const index = args.indexOf("--setting-sources"); + assert.equal(index === -1 ? undefined : args[index + 1], expected, `${command} ${target.join(" ")}`); + } } } finally { cleanupTestEnvironment(testEnv); diff --git a/tests/review-worktree.test.mjs b/tests/review-worktree.test.mjs index 19a4fec..4c08407 100644 --- a/tests/review-worktree.test.mjs +++ b/tests/review-worktree.test.mjs @@ -155,6 +155,7 @@ describe("createReviewIsolation", () => { assert.equal(iso.cwd, repoRoot); assert.equal(iso.gitRoot, repoRoot); assert.equal(iso.isolated, false); + assert.equal(iso.settingSources, undefined); } finally { iso.cleanup(); } @@ -185,6 +186,7 @@ describe("createReviewIsolation", () => { assert.notEqual(iso.cwd, repoRoot); assert.equal(iso.gitRoot, iso.cwd); assert.equal(iso.isolated, true); + assert.equal(iso.settingSources, "user"); assert.ok(fs.existsSync(iso.cwd)); } finally { iso.cleanup(); From 4869eb64cbdf5d6a757be880ad63c03f0c2141ba Mon Sep 17 00:00:00 2001 From: SP Son <1376128+seungpyoson@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:09:01 +0900 Subject: [PATCH 4/6] docs(review): say what the worktree setting sources keep and why The comment called the review worktree a checkout the user never trusted. It is a new directory, usually at the user's own HEAD, that Claude has never been asked to trust. It also said a worktree run "loads user settings only", although managed settings and the --settings file still load, and it described the working-tree case as ordinary trust, although that run skips the trust prompt because the plugin treats the user's repository as trusted. The return-value description now says settingSources is set only for a worktree. Refs #117 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017ZnqeTJxgWVEKtH4XefpUV --- scripts/lib/review-worktree.mjs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/scripts/lib/review-worktree.mjs b/scripts/lib/review-worktree.mjs index 92b0dfb..949978b 100644 --- a/scripts/lib/review-worktree.mjs +++ b/scripts/lib/review-worktree.mjs @@ -137,15 +137,19 @@ function cleanupWorktreeDir(worktreePath) { * would mislead Claude into thinking the repo is unchanged. Instead we run in * the original repo and rely on the Bash-free allowlist for containment. * - * Returns `{ cwd, gitRoot, cleanup, isolated, settingSources }`. `gitRoot` is - * the path the MCP git server should be rooted at; `cwd` is what the Claude - * CLI should treat as the working directory. `settingSources` is the Claude - * `--setting-sources` value for that directory. A review worktree is a - * checkout of the commit under review that the user never trusted, and a - * print-mode child never asks: its `.claude/settings.json` hooks and `env` - * and its `CLAUDE.md` would load. So a worktree run loads user settings only. - * A working-tree review runs in the user's own repository and loads its - * settings as any Claude session there does. + * Returns `{ cwd, gitRoot, cleanup, isolated }`, plus `settingSources` for a + * worktree. `gitRoot` is the path the MCP git server should be rooted at; + * `cwd` is what the Claude CLI should treat as the working directory. + * + * `settingSources` is the Claude `--setting-sources` value. The worktree is a + * new directory that Claude has never been asked to trust, and a print-mode + * child does not ask. Its project settings would otherwise load: hooks would + * run, `env` would reach the processes the child starts, and `CLAUDE.md` and + * rules would reach the model. With `user`, project and local settings do not + * load; user settings, managed settings and the `--settings` file still do. A + * working-tree review runs in the user's own repository and loads its project + * settings without a trust prompt: the plugin treats that repository as + * trusted. */ export function createReviewIsolation(repoRoot, target, { label = "review" } = {}) { if (target?.mode === "working-tree") { From 3d80361bded29c274ec1eeadaadb94cdc0d59418 Mon Sep 17 00:00:00 2001 From: SP Son <1376128+seungpyoson@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:02:11 +0900 Subject: [PATCH 5/6] fix(review): refuse worktree reviews on Claude Code older than 2.1.211 Claude Code before 2.1.211 accepts --setting-sources user but still loads a nested .claude/rules file from the checkout once Claude reads a file in that directory (Claude Code changelog, 2.1.211). A branch review on such a CLI completed with the reviewed code's instructions in the model context. runClaudeTurn now reads `claude --version` whenever settingSources is set and throws before starting Claude if the version is older than 2.1.211 or cannot be read. Working-tree reviews, the stop gate, task and rescue do not pass the flag and are not checked. The integration test now covers auto scope on a clean tree, auto scope on a dirty tree and --scope branch as well as --scope working-tree and --base, and a new test runs both review commands against a fake 2.1.210 CLI: they fail with the update message, Claude is never started and no review worktree is left behind. The fake CLI reports 2.1.283 unless a test sets CLAUDE_VERSION_OUTPUT. The createReviewIsolation comment no longer says Claude was never asked to trust the worktree, and no longer calls the Bash-free allowlist containment for working-tree reviews: it limits tools, not what the repository's project settings do. Refs #117 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017ZnqeTJxgWVEKtH4XefpUV --- scripts/lib/claude-cli.mjs | 34 +++++++++ scripts/lib/review-worktree.mjs | 21 +++--- tests/claude-cli.test.mjs | 25 +++++++ tests/integration/claude-companion.test.mjs | 79 +++++++++++++++++---- 4 files changed, 134 insertions(+), 25 deletions(-) diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index caede13..42455bb 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -148,6 +148,33 @@ export function getClaudeAvailability(cwd) { } } +// Older Claude Code accepts `--setting-sources` without applying it to every +// file: until 2.1.211, nested `.claude/rules/*.md` files still loaded when +// setting sources excluded project settings (Claude Code changelog, 2.1.211). +const SETTING_SOURCES_MIN_VERSION = [2, 1, 211]; + +/** + * Throw unless `claude --version` output names Claude Code + * SETTING_SOURCES_MIN_VERSION or later. + */ +/** @visibleForTesting */ +export function assertSettingSourcesSupported(versionOutput) { + const match = /^(\d+)\.(\d+)\.(\d+)\b/.exec(String(versionOutput ?? "").trim()); + if (!match) { + throw new Error( + `Cannot read the Claude Code version from \`claude --version\` output ${JSON.stringify(versionOutput)}.` + ); + } + const [major, minor, patch] = match.slice(1, 4).map(Number); + const [minMajor, minMinor, minPatch] = SETTING_SOURCES_MIN_VERSION; + if ((major - minMajor || minor - minMinor || patch - minPatch) < 0) { + throw new Error( + `Claude Code ${match[0]} loads nested .claude/rules files from the code under review even with --setting-sources. ` + + `Update Claude Code to ${SETTING_SOURCES_MIN_VERSION.join(".")} or later to review a branch.` + ); + } +} + export function getClaudeAuthStatus(cwd) { if (process.env.ANTHROPIC_API_KEY) { return { available: true, loggedIn: true, detail: "API key configured" }; @@ -764,6 +791,13 @@ export function buildArgs(prompt, options = {}) { * Returns { status, sessionId, finalMessage, toolUses, touchedFiles, stderr, pid, pidIdentity } */ export async function runClaudeTurn(cwd, prompt, options = {}) { + if (options.settingSources) { + const claude = getClaudeAvailability(cwd); + if (!claude.available) { + throw new Error(claude.detail); + } + assertSettingSourcesSupported(claude.detail); + } const args = buildArgs(prompt, { outputFormat: "stream-json", ...options, diff --git a/scripts/lib/review-worktree.mjs b/scripts/lib/review-worktree.mjs index 949978b..6d82490 100644 --- a/scripts/lib/review-worktree.mjs +++ b/scripts/lib/review-worktree.mjs @@ -135,21 +135,22 @@ function cleanupWorktreeDir(worktreePath) { * that the reviewer is supposed to inspect — `git status` would report clean, * `git diff` would show nothing, and the MCP server pointed at the worktree * would mislead Claude into thinking the repo is unchanged. Instead we run in - * the original repo and rely on the Bash-free allowlist for containment. + * the original repo with the Bash-free allowlist. That allowlist limits the + * model's tools; it does not stop the repo's project settings from loading. * * Returns `{ cwd, gitRoot, cleanup, isolated }`, plus `settingSources` for a * worktree. `gitRoot` is the path the MCP git server should be rooted at; * `cwd` is what the Claude CLI should treat as the working directory. * - * `settingSources` is the Claude `--setting-sources` value. The worktree is a - * new directory that Claude has never been asked to trust, and a print-mode - * child does not ask. Its project settings would otherwise load: hooks would - * run, `env` would reach the processes the child starts, and `CLAUDE.md` and - * rules would reach the model. With `user`, project and local settings do not - * load; user settings, managed settings and the `--settings` file still do. A - * working-tree review runs in the user's own repository and loads its project - * settings without a trust prompt: the plugin treats that repository as - * trusted. + * `settingSources` is the Claude `--setting-sources` value. The worktree checks + * out the commit under review, which may come from anyone, and a print-mode + * child does not ask for trust. Its project settings would otherwise load: + * hooks would run, `env` would reach the processes the child starts, and + * `CLAUDE.md` and rules would reach the model. With `user`, project and local + * settings do not load; user settings, managed settings and the `--settings` + * file still do. A working-tree review runs in the user's own repository and + * loads its project settings without a trust prompt: the plugin treats that + * repository as trusted. */ export function createReviewIsolation(repoRoot, target, { label = "review" } = {}) { if (target?.mode === "working-tree") { diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index 8c8f97c..ae00c1c 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -14,6 +14,7 @@ import { resolveDefaultModel, resolveClaudeBin, buildArgs, + assertSettingSourcesSupported, EFFORT_ALIASES, VALID_EFFORTS, DEFAULT_MODEL, @@ -806,6 +807,30 @@ describe("buildArgs", () => { }); }); +// =========================================================================== +// assertSettingSourcesSupported +// =========================================================================== + +describe("assertSettingSourcesSupported", () => { + it("accepts Claude Code 2.1.211 and later", () => { + for (const output of ["2.1.211 (Claude Code)", "2.1.283 (Claude Code)", "2.2.0", "3.0.0 (Claude Code)"]) { + assert.doesNotThrow(() => assertSettingSourcesSupported(output), output); + } + }); + + it("rejects versions that still load nested rules under --setting-sources", () => { + for (const output of ["2.1.210 (Claude Code)", "2.1.90 (Claude Code)", "2.0.999", "1.9.300"]) { + assert.throws(() => assertSettingSourcesSupported(output), /2\.1\.211 or later/, output); + } + }); + + it("rejects output that does not start with a version", () => { + for (const output of ["", "Claude Code", "claude CLI not found in PATH", undefined]) { + assert.throws(() => assertSettingSourcesSupported(output), /Cannot read the Claude Code version/); + } + }); +}); + // =========================================================================== // SANDBOX_READ_ONLY_TOOLS constant // =========================================================================== diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index 8138c72..d53f8f6 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -54,7 +54,7 @@ function sanitize(value) { async function main() { if (args[0] === "--version") { - process.stdout.write("2.1.90 (Claude Code)\\n"); + process.stdout.write((process.env.CLAUDE_VERSION_OUTPUT || "2.1.283 (Claude Code)") + "\\n"); return; } @@ -1059,7 +1059,7 @@ describe("claude-companion integration", () => { } }); - it("loads user settings only for reviews in a review worktree", () => { + it("excludes project settings only for reviews in a review worktree", () => { const testEnv = createTestEnvironment(); try { @@ -1072,27 +1072,41 @@ describe("claude-companion integration", () => { assert.equal(taskArgs.includes("--setting-sources"), false); setupGitWorkspace(testEnv.workspaceDir); - seedWorkingTreeDiff(testEnv.workspaceDir); - for (const [command, focusText] of [ + const settingSourcesFor = (command, target, focusText) => { + const invocationFile = path.join(testEnv.rootDir, "setting-sources-invocation.json"); + fs.rmSync(invocationFile, { force: true }); + runCompanion( + [command, "--cwd", testEnv.workspaceDir, ...target, ...focusText], + { env: { ...testEnv.env, CLAUDE_INVOCATION_FILE: invocationFile } } + ); + const { args } = JSON.parse(fs.readFileSync(invocationFile, "utf8")); + const index = args.indexOf("--setting-sources"); + return index === -1 ? undefined : args[index + 1]; + }; + const commands = [ ["review", []], ["adversarial-review", ["focus on settings"]], - ]) { + ]; + + // Auto scope on a clean tree reviews the branch in a worktree. + for (const [command, focusText] of commands) { + assert.equal(settingSourcesFor(command, [], focusText), "user", `${command} auto, clean tree`); + } + + seedWorkingTreeDiff(testEnv.workspaceDir); + for (const [command, focusText] of commands) { for (const [target, expected] of [ + [[], undefined], [["--scope", "working-tree"], undefined], + [["--scope", "branch"], "user"], [["--base", "main"], "user"], ]) { - const invocationFile = path.join( - testEnv.rootDir, - `setting-sources-${command}-${target[0].slice(2)}-invocation.json` - ); - runCompanion( - [command, "--cwd", testEnv.workspaceDir, ...target, ...focusText], - { env: { ...testEnv.env, CLAUDE_INVOCATION_FILE: invocationFile } } + assert.equal( + settingSourcesFor(command, target, focusText), + expected, + `${command} ${target.join(" ") || "auto, dirty tree"}` ); - const { args } = JSON.parse(fs.readFileSync(invocationFile, "utf8")); - const index = args.indexOf("--setting-sources"); - assert.equal(index === -1 ? undefined : args[index + 1], expected, `${command} ${target.join(" ")}`); } } } finally { @@ -1100,6 +1114,41 @@ describe("claude-companion integration", () => { } }); + it("refuses worktree reviews on a Claude CLI older than 2.1.211", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + const invocationFile = path.join(testEnv.rootDir, "old-cli-invocation.json"); + const env = { + ...testEnv.env, + CLAUDE_VERSION_OUTPUT: "2.1.210 (Claude Code)", + CLAUDE_INVOCATION_FILE: invocationFile, + }; + + for (const command of ["review", "adversarial-review"]) { + const result = runCompanionExpectFailure( + [command, "--cwd", testEnv.workspaceDir, "--base", "main"], + { env } + ); + assert.match(result.stderr, /Claude Code 2\.1\.210 .*2\.1\.211 or later/, command); + assert.equal(fs.existsSync(invocationFile), false, `${command} started Claude`); + assert.equal( + runGit(testEnv.workspaceDir, ["worktree", "list", "--porcelain"]).split("\n").filter((line) => line.startsWith("worktree ")).length, + 1, + `${command} left a review worktree behind` + ); + } + + // Working-tree reviews do not pass the flag, so the version does not matter. + runCompanion(["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], { env }); + assert.equal(fs.existsSync(invocationFile), true); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("uses --resume to continue the latest session and keeps --fresh from injecting a resume id", async () => { const testEnv = createTestEnvironment(); const sessionEnv = { From b93712539b3d01fecd2814a0d7c130440f0e9a3d Mon Sep 17 00:00:00 2001 From: SP Son <1376128+seungpyoson@users.noreply.github.com> Date: Mon, 28 Sep 2026 19:01:00 +0900 Subject: [PATCH 6/6] fix(review): require Claude Code 2.1.281 and a release version for worktree reviews The 2.1.211 floor covered only the nested .claude/rules fix. The Claude Code changelog records two later --setting-sources fixes: 2.1.246 made the command sandbox's filesystem configuration respect the flag, and 2.1.281 made spawned sessions (teammates, /bg, claude agents sessions) inherit it. The floor is now 2.1.281, the latest release that fixed the flag letting excluded settings through. The version check matched only a numeric prefix, so 2.1.283-rc.1 or 2.1.283.0 passed as 2.1.283. It now requires plain major.minor.patch followed by whitespace or the end of the output, and rejects anything else as unreadable. README prerequisites now name the version branch reviews need. Refs #117 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017ZnqeTJxgWVEKtH4XefpUV --- README.md | 2 +- scripts/lib/claude-cli.mjs | 21 ++++++++++-------- tests/claude-cli.test.mjs | 24 ++++++++++++++------- tests/integration/claude-companion.test.mjs | 6 +++--- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index e47b098..c3c489e 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ npx cc-plugin-codex install On Windows, prefer the Sendbird marketplace path or the `npx` helper. The shell-script helper below is POSIX-only. Codex CLI's official guidance still treats Windows support as experimental and recommends a WSL workspace for the best Codex experience. Claude Code supports both native Windows and WSL. -> **Prerequisites:** Node.js 18+, Codex with hook support, and `claude` CLI installed and authenticated. +> **Prerequisites:** Node.js 18+, Codex with hook support, and `claude` CLI installed and authenticated. Branch reviews need Claude Code 2.1.281 or later. > If you don't have the Claude CLI yet: > ```bash > npm install -g @anthropic-ai/claude-code && claude auth login diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 42455bb..7668cbf 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -148,28 +148,31 @@ export function getClaudeAvailability(cwd) { } } -// Older Claude Code accepts `--setting-sources` without applying it to every -// file: until 2.1.211, nested `.claude/rules/*.md` files still loaded when -// setting sources excluded project settings (Claude Code changelog, 2.1.211). -const SETTING_SOURCES_MIN_VERSION = [2, 1, 211]; +// The latest Claude Code release that fixed `--setting-sources` letting +// excluded settings through. Older releases accept the flag but still load +// nested `.claude/rules/*.md` files (fixed in 2.1.211), apply the command +// sandbox's filesystem configuration from excluded sources (2.1.246), and +// start spawned sessions without the restriction (2.1.281), per the Claude +// Code changelog. +const SETTING_SOURCES_MIN_VERSION = [2, 1, 281]; /** - * Throw unless `claude --version` output names Claude Code - * SETTING_SOURCES_MIN_VERSION or later. + * Throw unless `claude --version` output starts with a release version, plain + * `major.minor.patch`, at or above SETTING_SOURCES_MIN_VERSION. */ /** @visibleForTesting */ export function assertSettingSourcesSupported(versionOutput) { - const match = /^(\d+)\.(\d+)\.(\d+)\b/.exec(String(versionOutput ?? "").trim()); + const match = /^(\d+)\.(\d+)\.(\d+)(?=\s|$)/.exec(String(versionOutput ?? "").trim()); if (!match) { throw new Error( - `Cannot read the Claude Code version from \`claude --version\` output ${JSON.stringify(versionOutput)}.` + `Cannot read a Claude Code release version from \`claude --version\` output ${JSON.stringify(versionOutput)}.` ); } const [major, minor, patch] = match.slice(1, 4).map(Number); const [minMajor, minMinor, minPatch] = SETTING_SOURCES_MIN_VERSION; if ((major - minMajor || minor - minMinor || patch - minPatch) < 0) { throw new Error( - `Claude Code ${match[0]} loads nested .claude/rules files from the code under review even with --setting-sources. ` + + `Claude Code ${match[0]} can load settings from the code under review even with --setting-sources. ` + `Update Claude Code to ${SETTING_SOURCES_MIN_VERSION.join(".")} or later to review a branch.` ); } diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index ae00c1c..a53c6cd 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -812,21 +812,29 @@ describe("buildArgs", () => { // =========================================================================== describe("assertSettingSourcesSupported", () => { - it("accepts Claude Code 2.1.211 and later", () => { - for (const output of ["2.1.211 (Claude Code)", "2.1.283 (Claude Code)", "2.2.0", "3.0.0 (Claude Code)"]) { + it("accepts Claude Code 2.1.281 and later", () => { + for (const output of ["2.1.281 (Claude Code)", "2.1.283 (Claude Code)\r\n", "2.2.0", "3.0.0 (Claude Code)"]) { assert.doesNotThrow(() => assertSettingSourcesSupported(output), output); } }); - it("rejects versions that still load nested rules under --setting-sources", () => { - for (const output of ["2.1.210 (Claude Code)", "2.1.90 (Claude Code)", "2.0.999", "1.9.300"]) { - assert.throws(() => assertSettingSourcesSupported(output), /2\.1\.211 or later/, output); + it("rejects releases with a known --setting-sources gap", () => { + for (const output of ["2.1.280 (Claude Code)", "2.1.246 (Claude Code)", "2.1.210 (Claude Code)", "2.0.999", "1.9.300"]) { + assert.throws(() => assertSettingSourcesSupported(output), /2\.1\.281 or later/, output); } }); - it("rejects output that does not start with a version", () => { - for (const output of ["", "Claude Code", "claude CLI not found in PATH", undefined]) { - assert.throws(() => assertSettingSourcesSupported(output), /Cannot read the Claude Code version/); + it("rejects output that does not start with a release version", () => { + for (const output of [ + "", + "Claude Code", + "claude CLI not found in PATH", + "v2.1.283", + "2.1.283-rc.1 (Claude Code)", + "2.1.283.0", + undefined, + ]) { + assert.throws(() => assertSettingSourcesSupported(output), /Cannot read a Claude Code release version/, String(output)); } }); }); diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index d53f8f6..6430146 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -1114,7 +1114,7 @@ describe("claude-companion integration", () => { } }); - it("refuses worktree reviews on a Claude CLI older than 2.1.211", () => { + it("refuses worktree reviews on a Claude CLI older than 2.1.281", () => { const testEnv = createTestEnvironment(); try { @@ -1123,7 +1123,7 @@ describe("claude-companion integration", () => { const invocationFile = path.join(testEnv.rootDir, "old-cli-invocation.json"); const env = { ...testEnv.env, - CLAUDE_VERSION_OUTPUT: "2.1.210 (Claude Code)", + CLAUDE_VERSION_OUTPUT: "2.1.280 (Claude Code)", CLAUDE_INVOCATION_FILE: invocationFile, }; @@ -1132,7 +1132,7 @@ describe("claude-companion integration", () => { [command, "--cwd", testEnv.workspaceDir, "--base", "main"], { env } ); - assert.match(result.stderr, /Claude Code 2\.1\.210 .*2\.1\.211 or later/, command); + assert.match(result.stderr, /Claude Code 2\.1\.280 .*2\.1\.281 or later/, command); assert.equal(fs.existsSync(invocationFile), false, `${command} started Claude`); assert.equal( runGit(testEnv.workspaceDir, ["worktree", "list", "--porcelain"]).split("\n").filter((line) => line.startsWith("worktree ")).length,