From 07adeb50a028ed40aed5b918894ae83581993f31 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 17 Jun 2026 08:50:28 -0400 Subject: [PATCH] fix(#1342): scope worktree-path-guard to GSD executor runs; fail open for no-repo targets (#1361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#1342): scope worktree-path-guard to GSD executor runs; fail open for no-repo targets The PreToolUse worktree-path-guard fired for any Write/Edit in any linked git worktree, with no check for active GSD work — so Claude Code plan-mode writing ~/.claude/plans/.md from a manually-created worktree was hard-blocked. - Gate enforcement on the GSD isolated-executor branch namespace (^worktree-agent-[A-Za-z0-9._/-]+$, per worktree-branch-check.md #2924); the guard is a no-op in non-GSD linked worktrees. - Fail open when a target resolves to no git repository (e.g. ~/.claude/plans/) instead of blocking — that is not the #260 main-repo vector. A target inside a .git directory still blocks (git rev-parse --is-inside-git-dir). - The #260 different-git-root hard block (escape to the main repo) is preserved. Detached-HEAD executors no-op the gate; this is accepted because they are fail-closed by worktree-branch-check.md (exit 42) before committing. Co-Authored-By: Claude Opus 4.8 * chore(#1342): add changeset for worktree-path-guard scoping fix Co-Authored-By: Claude Opus 4.8 * test(#1342): build dot-dot traversal path portably (Windows drive-letter fix) The traversal test built its file_path by stripping a leading slash from an absolute externalDir and path.join-ing it after a `..` chain. On Windows the drive letter (C:\) is not a leading slash, so it survived and path.resolve produced an invalid doubled-drive path (C:\C:\Users\...), which resolves to no git repo — the hook failed open (exit 0) and the test expected a block (exit 2). Use path.relative(worktreeDir, externalTarget) + string concat so the file_path carries literal `..` segments that resolve to externalTarget on both posix and win32 (no drive doubling). Verified with path.win32/path.posix. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/1342-worktree-guard-gsd-gate.md | 6 + hooks/gsd-worktree-path-guard.js | 52 ++-- tests/bug-260-worktree-path-guard.test.cjs | 261 ++++++++++++++++++--- 3 files changed, 269 insertions(+), 50 deletions(-) create mode 100644 .changeset/1342-worktree-guard-gsd-gate.md diff --git a/.changeset/1342-worktree-guard-gsd-gate.md b/.changeset/1342-worktree-guard-gsd-gate.md new file mode 100644 index 000000000..9e593f101 --- /dev/null +++ b/.changeset/1342-worktree-guard-gsd-gate.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1361 +--- + +**The worktree path guard no longer blocks ordinary writes in non-GSD git worktrees** — the `gsd-worktree-path-guard` PreToolUse hook fired for every `Write`/`Edit` in any linked git worktree, so Claude Code plan-mode writing its plan to `~/.claude/plans/.md` from a manually-created worktree was hard-blocked. The hook now only enforces inside a GSD isolated-executor worktree (branch `worktree-agent-*`) and fails open when a target resolves to no git repository, while still blocking writes that escape to a different git root (the #260 protection) or into a repository's `.git` internals. (#1342) diff --git a/hooks/gsd-worktree-path-guard.js b/hooks/gsd-worktree-path-guard.js index e0160c920..78b329632 100644 --- a/hooks/gsd-worktree-path-guard.js +++ b/hooks/gsd-worktree-path-guard.js @@ -71,6 +71,17 @@ process.stdin.on('end', () => { process.exit(0); // main repo, submodule, or separate-git-dir — no-op } + // #1342: Only enforce inside a GSD-managed isolated executor worktree. Those + // are always on a `worktree-agent-*` branch (the positive allow-list enforced + // by worktree-branch-check.md, #2924). A manually-created linked worktree (plain + // non-GSD work, e.g. Claude Code plan-mode) is on the user's own branch, so the + // guard must be a no-op there. Detached HEAD / error → not GSD-managed → no-op. + const branchResult = git(['symbolic-ref', '--short', 'HEAD'], cwd); + const branch = branchResult.status === 0 && branchResult.stdout ? branchResult.stdout.trim() : ''; + if (!/^worktree-agent-[A-Za-z0-9._/-]+$/.test(branch)) { + process.exit(0); // not a GSD-managed executor worktree — no-op + } + // Get the raw --show-toplevel output for the worktree (cwd). // We keep it raw (not path.resolve'd) to compare directly with the // file's toplevel — same git binary, same format, no normalization needed. @@ -110,15 +121,9 @@ process.stdin.on('end', () => { if (!checkDir) { // Walked to root without finding any directory — path is synthetic. - // Block conservatively. - const output = { - decision: 'block', - reason: - `Worktree path guard: '${filePath}' has no existing ancestor directory — ` + - `cannot verify it is inside the worktree '${wtTopRaw}'. Use a relative path instead.`, - }; - process.stdout.write(JSON.stringify(output)); - process.exit(2); + // A path with no existing ancestor is not the #260 main-repo vector; + // #260 is caught by the different-git-root branch below. Fail open. (#1342) + process.exit(0); } // Ask git for the toplevel of the file's location. @@ -130,15 +135,26 @@ process.stdin.on('end', () => { const fileTopResult = git(['rev-parse', '--show-toplevel'], checkDir); if (fileTopResult.status !== 0 || !fileTopResult.stdout) { - // checkDir is not inside any git repo → cannot be inside the worktree. - const output = { - decision: 'block', - reason: - `Worktree path guard: '${filePath}' is not inside any git repository — ` + - `it cannot be inside the worktree at '${wtTopRaw}'. Use a relative path instead.`, - }; - process.stdout.write(JSON.stringify(output)); - process.exit(2); + // The target's location is not a git work tree. Two sub-cases: + // - Inside a .git directory (e.g. /main-repo/.git/config or .git/hooks/*) + // → an absolute write into a repository's internals; still a #260-class + // escape (and dangerous) → BLOCK. + // - Truly outside all git repositories (e.g. ~/.claude/plans/) → not the + // main-repo vector → fail open. (#1342) + const insideGitDir = git(['rev-parse', '--is-inside-git-dir'], checkDir); + if (insideGitDir.status === 0 && insideGitDir.stdout && insideGitDir.stdout.trim() === 'true') { + const output = { + decision: 'block', + reason: + `Worktree path guard: '${filePath}' is inside a git internal (.git) directory, ` + + `not the active worktree at '${wtTopRaw}'. Writing to repository internals via an ` + + `absolute path is not permitted from an isolated executor worktree. Use a relative path.`, + }; + process.stdout.write(JSON.stringify(output)); + process.exit(2); + } + // Outside all git repositories — fail open (#1342). + process.exit(0); } const fileTopRaw = fileTopResult.stdout.trim(); diff --git a/tests/bug-260-worktree-path-guard.test.cjs b/tests/bug-260-worktree-path-guard.test.cjs index 9aec9beda..93d02f967 100644 --- a/tests/bug-260-worktree-path-guard.test.cjs +++ b/tests/bug-260-worktree-path-guard.test.cjs @@ -66,11 +66,14 @@ function makeMainRepo() { /** * Create a worktree off mainRepo and return its path. * In the worktree, .git is a FILE (the gitdir pointer). + * @param {string} mainRepo - path to the main repo + * @param {string} [branchName] - branch name to use (default: 'worktree-agent-test') */ -function makeWorktree(mainRepo) { +function makeWorktree(mainRepo, branchName) { + const branch = branchName || 'worktree-agent-test'; const wtDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-260-wt-')); fs.rmdirSync(wtDir); // git worktree add creates the dir itself - git(mainRepo, ['worktree', 'add', '-q', '-b', 'wt-test-branch', wtDir]); + git(mainRepo, ['worktree', 'add', '-q', '-b', branch, wtDir]); return wtDir; } @@ -261,25 +264,67 @@ describe('bug #260: gsd-worktree-path-guard.js', () => { }); }); - // 6. Sibling directory path is BLOCKED (validates the '/' boundary check) + // 6. Sibling directory path is BLOCKED (validates the '/' boundary check AND prefix-overlap) describe('sibling path is blocked', () => { test('path that shares prefix with worktree root but is a sibling exits 2', () => { - // e.g. worktreeDir = /tmp/gsd-260-wt-XXXXX - // sibling = /tmp/gsd-260-wt-XXXXXsibling/file.ts - // This would pass a naive startsWith(wtRoot) check without the '/' suffix. - const siblingPath = worktreeDir + '-sibling/file.ts'; - const payload = { - cwd: worktreeDir, - tool_name: 'Edit', - tool_input: { file_path: siblingPath }, - }; - const result = runHook(worktreeDir, payload); - assert.strictEqual(result.status, 2, - `Sibling path "${siblingPath}" must be blocked (exit 2), got ${result.status}. ` + - `This validates the '/' boundary check in startsWith(wtRoot + '/'). stderr: ${result.stderr}` - ); - const parsed = JSON.parse(result.stdout); - assert.strictEqual(parsed.decision, 'block'); + // This test exercises BOTH the prefix-overlap boundary check AND the different-git-root block: + // worktree = /wt + // sibling = /wt-sibling ← shares "wt" prefix with the worktree root + // target = /wt-sibling/file.ts + // + // A naive startsWith(wtRoot) check would wrongly classify "/wt-sibling/..." as inside + // the worktree (it doesn't include the '/' boundary). The hook resolves the sibling's git + // toplevel (a different repo) so the different-git-root block fires regardless. + // (#1342: paths outside all git repos now fail open; only different-git-root blocks.) + const base = realp(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-260-sib-base-'))); + const wtDir = path.join(base, 'wt'); + const siblingRepoDir = path.join(base, 'wt-sibling'); + // We need a genuine linked worktree at /wt and a separate git repo at /wt-sibling. + // Create a fresh main repo to host this worktree (the fixture worktree is already allocated). + const sibMainRepo = realp(makeMainRepo()); + try { + fs.mkdirSync(base, { recursive: true }); + // Create linked worktree at /wt (using sibMainRepo as its host). + git(sibMainRepo, ['worktree', 'add', '-q', '-b', 'worktree-agent-sib-test', wtDir]); + // Create a separate git repo at /wt-sibling (shares "wt" prefix). + fs.mkdirSync(siblingRepoDir, { recursive: true }); + git(siblingRepoDir, ['init', '-q']); + git(siblingRepoDir, ['config', 'user.email', 'test@example.com']); + git(siblingRepoDir, ['config', 'user.name', 'Test User']); + git(siblingRepoDir, ['config', 'commit.gpgsign', 'false']); + fs.writeFileSync(path.join(siblingRepoDir, 'README.md'), '# sibling\n'); + git(siblingRepoDir, ['add', 'README.md']); + git(siblingRepoDir, ['commit', '-q', '-m', 'chore: sibling init']); + + // Confirm prefix-overlap: siblingRepoDir starts with wtDir (without trailing sep). + assert.ok( + siblingRepoDir.startsWith(wtDir), + `Sibling "${siblingRepoDir}" must share a string prefix with worktree "${wtDir}" for this test to be meaningful` + ); + // Confirm they are genuinely distinct (different toplevel). + assert.notStrictEqual( + realp(siblingRepoDir), realp(wtDir), + 'sibling and worktree must be different directories' + ); + + const siblingPath = path.join(realp(siblingRepoDir), 'file.ts'); + const payload = { + cwd: realp(wtDir), + tool_name: 'Edit', + tool_input: { file_path: siblingPath }, + }; + const result = runHook(realp(wtDir), payload); + assert.strictEqual(result.status, 2, + `Path inside a prefix-sibling git repo "${siblingPath}" must be blocked (exit 2), got ${result.status}. ` + + `This validates both the prefix-overlap boundary and the different-git-root block. stderr: ${result.stderr}` + ); + const parsed = JSON.parse(result.stdout); + assert.strictEqual(parsed.decision, 'block'); + } finally { + try { git(sibMainRepo, ['worktree', 'remove', '--force', wtDir]); } catch { /* ignore */ } + cleanup(sibMainRepo); + cleanup(base); + } }); }); @@ -323,14 +368,13 @@ describe('bug #260: gsd-worktree-path-guard.js', () => { // 8. Adversarial: `..` traversal is normalised before the containment check (Codex finding #1) describe('dot-dot traversal is blocked', () => { test('path with .. that escapes the worktree is blocked', () => { - // Construct the traversal target inside a SEPARATE tmpdir that is + // Construct the traversal target inside a SEPARATE git repo that is // guaranteed to be outside the worktree on every platform (no symlink - // ambiguity). We create the directory so that the hook's - // nearestExistingDir() walk finds it and dispatches git --show-toplevel - // on it — which will either fail (not a git repo → block) or return a - // different toplevel (different repo → block). Either path through the - // hook exits 2. - const externalDir = realp(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-260-ext-'))); + // ambiguity). The hook finds the external dir's git toplevel (a different + // repo → different-git-root block). + // (#1342: paths outside all git repos now fail open; only different-git-root blocks, + // so externalDir must be inside a real different git repo to exercise the block.) + const externalDir = realp(makeMainRepo()); try { // Sanity: the external directory must not be inside the worktree. assert.ok( @@ -344,12 +388,15 @@ describe('bug #260: gsd-worktree-path-guard.js', () => { // We compute the number of segments needed to reach the filesystem root // from worktreeDir so the traversal always lands at the right level // regardless of how deep the worktree path is. - const depthFromRoot = worktreeDir.split(path.sep).filter(Boolean).length; - const upSegments = Array(depthFromRoot + 1).fill('..').join(path.sep); - // Strip the leading separator from externalDir so path.join treats it - // as relative segments when appended after the .. chain. - const externalRelative = externalDir.replace(/^[/\\]+/, ''); - const traversalPath = path.join(worktreeDir, upSegments, externalRelative, 'file.ts'); + // Build a file_path containing literal `..` segments that climb out of the + // worktree into externalDir. path.relative() yields a ..-laden relative path + // between two same-drive absolute paths (both live under os.tmpdir()); we + // re-anchor it at worktreeDir via STRING CONCAT (NOT path.join, which would + // normalise the `..` away) so the hook's path.resolve() must collapse it. + // Windows-safe: avoids the drive-letter doubling that + // path.join(worktreeDir, '..', absolutePath) produces on win32 (#1342). + const externalTarget = path.join(externalDir, 'file.ts'); + const traversalPath = worktreeDir + path.sep + path.relative(worktreeDir, externalTarget); // Confirm the resolved path is truly outside the worktree (test integrity guard). const resolved = path.resolve(traversalPath); @@ -410,6 +457,156 @@ describe('bug #260: gsd-worktree-path-guard.js', () => { }); +// --------------------------------------------------------------------------- +// #1342 — GSD-activity gate + fail-open for no-repo targets +// --------------------------------------------------------------------------- + +describe('#1342 — GSD-activity gate + fail-open for no-repo targets', () => { + // Fixtures: one non-agent linked worktree (plain user branch) + one agent worktree + let mainRepo1342; + let nonAgentWorktree; // on branch 'feature-x' — non-GSD + let agentWorktree; // on branch 'worktree-agent-foo' — GSD-managed + + before(() => { + mainRepo1342 = realp(makeMainRepo()); + nonAgentWorktree = realp(makeWorktree(mainRepo1342, 'feature-x')); + agentWorktree = realp(makeWorktree(mainRepo1342, 'worktree-agent-foo')); + }); + + after(() => { + try { git(mainRepo1342, ['worktree', 'remove', '--force', nonAgentWorktree]); } catch { /* ignore */ } + try { git(mainRepo1342, ['worktree', 'remove', '--force', agentWorktree]); } catch { /* ignore */ } + cleanup(mainRepo1342); + cleanup(nonAgentWorktree); + cleanup(agentWorktree); + }); + + // Test 1 — reporter repro: non-agent worktree writing outside all git repos → exit 0 + test('(1) non-agent linked worktree: Write to a path outside all git repos exits 0 (no block)', () => { + // Simulates Claude Code plan-mode writing ~/.claude/plans/.md from a + // manually-created linked worktree that is NOT on a worktree-agent-* branch. + const plansDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1342-plans-')); + try { + const targetPath = path.join(plansDir, 'my-plan.md'); + const payload = { + cwd: nonAgentWorktree, + tool_name: 'Write', + tool_input: { file_path: targetPath }, + }; + const result = runHook(nonAgentWorktree, payload); + assert.strictEqual(result.status, 0, + `Non-agent linked worktree writing outside git repos must exit 0 (reporter repro). ` + + `Got exit ${result.status}. stderr: ${result.stderr}` + ); + assert.strictEqual(result.stdout, '', 'Expected no block output'); + } finally { + cleanup(plansDir); + } + }); + + // Test 2 — non-agent linked worktree: Edit targeting MAIN repo root → exit 0 (gate no-op) + test('(2) non-agent linked worktree: Edit targeting main repo root exits 0 (gate no-op, not #260 block)', () => { + const payload = { + cwd: nonAgentWorktree, + tool_name: 'Edit', + tool_input: { file_path: path.join(mainRepo1342, 'src', 'index.ts') }, + }; + const result = runHook(nonAgentWorktree, payload); + assert.strictEqual(result.status, 0, + `Non-agent linked worktree must exit 0 (GSD-activity gate fires before #260 check). ` + + `Got exit ${result.status}. stderr: ${result.stderr}` + ); + assert.strictEqual(result.stdout, '', 'Expected no block output'); + }); + + // Test 3 — GSD-managed worktree (worktree-agent-foo): Edit targeting main repo root → exit 2 (block) + test('(3) GSD-managed worktree: Edit targeting main repo root exits 2 with block decision', () => { + const payload = { + cwd: agentWorktree, + tool_name: 'Edit', + tool_input: { file_path: path.join(mainRepo1342, 'src', 'index.ts') }, + }; + const result = runHook(agentWorktree, payload); + assert.strictEqual(result.status, 2, + `GSD-managed worktree targeting main repo root must be blocked (exit 2). ` + + `Got exit ${result.status}. stderr: ${result.stderr}` + ); + let parsed; + assert.doesNotThrow(() => { parsed = JSON.parse(result.stdout); }, 'stdout must be valid JSON'); + assert.strictEqual(parsed.decision, 'block', 'Expected decision:"block" in output'); + }); + + // Test 4 — GSD-managed worktree: absolute target INSIDE the active worktree → exit 0 + test('(4) GSD-managed worktree: absolute target inside the active worktree exits 0', () => { + const payload = { + cwd: agentWorktree, + tool_name: 'Edit', + tool_input: { file_path: path.join(agentWorktree, 'src', 'foo.ts') }, + }; + const result = runHook(agentWorktree, payload); + assert.strictEqual(result.status, 0, + `GSD-managed worktree targeting its own subtree must pass. ` + + `Got exit ${result.status}. stderr: ${result.stderr}` + ); + assert.strictEqual(result.stdout, '', 'Expected no block output'); + }); + + // Test 5 — GSD-managed worktree: target OUTSIDE all git repos (tmpdir) → exit 0 (fail open) + test('(5) GSD-managed worktree: target outside all git repos exits 0 (fail open, not #260 vector)', () => { + // Create a temp dir that is NOT a git repository (no .git). + // This is the ~/.claude/plans/ scenario — a path that has a real ancestor + // directory but is outside every git repo. + // IMPORTANT: this dir must NOT be inside any .git directory — it must be a plain tempdir + // so the fail-open path (truly outside all repos) is exercised, not the .git-internals block. + const externalDir = realp(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1342-ext-'))); + try { + const targetPath = path.join(externalDir, 'notes.md'); + const payload = { + cwd: agentWorktree, + tool_name: 'Write', + tool_input: { file_path: targetPath }, + }; + const result = runHook(agentWorktree, payload); + assert.strictEqual(result.status, 0, + `GSD-managed worktree writing to a path outside all git repos must fail open (exit 0). ` + + `Only the different-git-root vector (#260) blocks; no-repo targets are not that vector. ` + + `Got exit ${result.status}. stderr: ${result.stderr}` + ); + assert.strictEqual(result.stdout, '', 'Expected no block output'); + } finally { + cleanup(externalDir); + } + }); + + // Test 6 — GSD-managed worktree: Write to .git/config of the MAIN repo → exit 2 (block) + test('(6) blocks absolute writes into the main repo .git internals from a GSD worktree (#1342)', () => { + // A target like /main-repo/.git/config or /main-repo/.git/hooks/pre-commit causes + // `git rev-parse --show-toplevel` to FAIL (a .git dir is not a work tree), so the + // "file not in any git repo" branch fires. Previously that branch failed open — but + // writing into repository internals via an absolute path is still a #260-class escape + // (and dangerous, e.g. injecting a git hook). The fix checks --is-inside-git-dir and + // blocks when true. + const gitConfigPath = path.join(mainRepo1342, '.git', 'config'); + const payload = { + cwd: agentWorktree, + tool_name: 'Write', + tool_input: { file_path: gitConfigPath }, + }; + const result = runHook(agentWorktree, payload); + assert.strictEqual(result.status, 2, + `GSD-managed worktree targeting .git/config of another repo must be blocked (exit 2). ` + + `Got exit ${result.status}. stderr: ${result.stderr}` + ); + let parsed; + assert.doesNotThrow(() => { parsed = JSON.parse(result.stdout); }, 'stdout must be valid JSON'); + assert.strictEqual(parsed.decision, 'block', 'Expected decision:"block" in output'); + assert.ok( + parsed.reason && parsed.reason.includes('.git'), + `Block reason should mention .git internals. Got: ${parsed.reason}` + ); + }); +}); + // --------------------------------------------------------------------------- // Static analysis: install.js guard // ---------------------------------------------------------------------------