diff --git a/.changeset/quick-rams-wave.md b/.changeset/quick-rams-wave.md new file mode 100644 index 000000000..d528cda0f --- /dev/null +++ b/.changeset/quick-rams-wave.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 265 +--- +Workflow guard now respects its opt-in config before blocking forced git-add commands on worktree-agent branches. diff --git a/bin/install.js b/bin/install.js index 0159bdde5..89241a189 100755 --- a/bin/install.js +++ b/bin/install.js @@ -9556,18 +9556,24 @@ function install(isGlobal, runtime = 'claude', options = {}) { // Configure workflow guard hook (opt-in via hooks.workflow_guard: true) // Detects file edits outside GSD workflow context and advises using - // /gsd-quick or /gsd-fast for state-tracked changes. Advisory only. + // /gsd-quick or /gsd-fast for state-tracked changes. Also hard-blocks + // unsafe Bash commands that violate worktree-agent isolation. const workflowGuardCommand = isGlobal ? buildHookCommand(targetDir, 'gsd-workflow-guard.js', hookOpts) : localCmd('gsd-workflow-guard.js'); - const hasWorkflowGuardHook = settings.hooks[preToolEvent].some(entry => + const workflowGuardMatcher = 'Bash|Edit|Write|MultiEdit'; + const workflowGuardHookEntry = settings.hooks[preToolEvent].find(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-workflow-guard')) ); + const hasWorkflowGuardHook = Boolean(workflowGuardHookEntry); const workflowGuardFile = path.join(targetDir, 'hooks', 'gsd-workflow-guard.js'); - if (!hasWorkflowGuardHook && fs.existsSync(workflowGuardFile) && workflowGuardCommand) { + if (hasWorkflowGuardHook && workflowGuardHookEntry.matcher !== workflowGuardMatcher) { + workflowGuardHookEntry.matcher = workflowGuardMatcher; + console.log(` ${green}✓${reset} Updated workflow guard hook matcher`); + } else if (!hasWorkflowGuardHook && fs.existsSync(workflowGuardFile) && workflowGuardCommand) { settings.hooks[preToolEvent].push({ - matcher: 'Write|Edit', + matcher: workflowGuardMatcher, hooks: [ { type: 'command', diff --git a/hooks/gsd-workflow-guard.js b/hooks/gsd-workflow-guard.js index 55ba45cf4..e786c6e8b 100644 --- a/hooks/gsd-workflow-guard.js +++ b/hooks/gsd-workflow-guard.js @@ -13,6 +13,69 @@ const fs = require('fs'); const path = require('path'); +const { spawnSync } = require('child_process'); +const { tokenize } = require('./lib/git-cmd.js'); + +function forceGitAddCwds(command, defaultCwd) { + const tokens = tokenize(command || ''); + const separators = new Set(['&&', '||', ';', '|']); + const cwdList = []; + for (let i = 0; i < tokens.length; i++) { + if (path.basename(tokens[i]) !== 'git') continue; + + let j = i + 1; + let gitCwd = defaultCwd; + while (j < tokens.length) { + const token = tokens[j]; + const flagName = token.includes('=') ? token.slice(0, token.indexOf('=')) : token; + if (token === '-C' && tokens[j + 1]) { + gitCwd = path.resolve(gitCwd, tokens[j + 1]); + j += 2; + continue; + } + if (['-C', '--git-dir', '--work-tree'].includes(flagName) && !token.includes('=')) { + j += 2; + continue; + } + if (['--git-dir', '--work-tree', '--no-pager', '-p', '-P'].includes(flagName)) { + j++; + continue; + } + break; + } + + if (tokens[j] !== 'add') continue; + for (let k = j + 1; k < tokens.length && !separators.has(tokens[k]); k++) { + if (tokens[k] === '--') break; + if (tokens[k] === '--force' || tokens[k] === '-f' || /^-[A-Za-z]*f[A-Za-z]*$/.test(tokens[k])) { + cwdList.push(gitCwd); + break; + } + } + } + return cwdList; +} + +function currentBranch(cwd) { + const result = spawnSync('git', ['branch', '--show-current'], { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }); + if (result.status !== 0) return ''; + return result.stdout.trim(); +} + +function workflowGuardEnabled(cwd) { + const configPath = path.join(cwd, '.planning', 'config.json'); + if (!fs.existsSync(configPath)) return false; + try { + const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); + return Boolean(config.hooks?.workflow_guard); + } catch (e) { + return false; + } +} let input = ''; const stdinTimeout = setTimeout(() => process.exit(0), 3000); @@ -23,9 +86,30 @@ process.stdin.on('end', () => { try { const data = JSON.parse(input); const toolName = data.tool_name; + const cwd = data.cwd || process.cwd(); + const isWorkflowGuardEnabled = workflowGuardEnabled(cwd); - // Only guard Write and Edit tool calls - if (toolName !== 'Write' && toolName !== 'Edit') { + if (toolName === 'Bash') { + if (!isWorkflowGuardEnabled) { + process.exit(0); + } + const command = data.tool_input?.command || ''; + for (const gitCwd of forceGitAddCwds(command, cwd)) { + const branch = currentBranch(gitCwd); + if (branch.startsWith('worktree-agent-')) { + process.stdout.write(JSON.stringify({ + decision: 'block', + code: 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN', + reason: 'worktree-agent branches must not run git add -f or git add --force. Respect the SDK skipped_gitignored/skipped_commit_docs_false contract and leave gitignored files untracked.', + })); + process.exit(2); + } + } + process.exit(0); + } + + // Only guard Write, Edit, and MultiEdit tool calls + if (!['Write', 'Edit', 'MultiEdit'].includes(toolName)) { process.exit(0); } @@ -57,20 +141,8 @@ process.stdin.on('end', () => { process.exit(0); } - // Check if workflow guard is enabled - const cwd = data.cwd || process.cwd(); - const configPath = path.join(cwd, '.planning', 'config.json'); - if (fs.existsSync(configPath)) { - try { - const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); - if (!config.hooks?.workflow_guard) { - process.exit(0); // Guard disabled (default) - } - } catch (e) { - process.exit(0); - } - } else { - process.exit(0); // No GSD project — don't guard + if (!isWorkflowGuardEnabled) { + process.exit(0); // Guard disabled (default) or no GSD project } // If we get here: GSD project, guard enabled, file edit outside .planning/, diff --git a/tests/bug-261-worktree-force-add-guard.test.cjs b/tests/bug-261-worktree-force-add-guard.test.cjs new file mode 100644 index 000000000..75ff70b7a --- /dev/null +++ b/tests/bug-261-worktree-force-add-guard.test.cjs @@ -0,0 +1,160 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execFileSync, spawnSync } = require('node:child_process'); + +const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-workflow-guard.js'); + +function git(cwd, args) { + return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); +} + +function makeRepo(branch) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-bug-261-')); + git(dir, ['init', '-q']); + git(dir, ['config', 'user.email', 'test@example.com']); + git(dir, ['config', 'user.name', 'Test User']); + git(dir, ['config', 'commit.gpgsign', 'false']); + fs.writeFileSync(path.join(dir, 'README.md'), '# test\n'); + git(dir, ['add', 'README.md']); + git(dir, ['commit', '-q', '-m', 'chore: init']); + git(dir, ['checkout', '-q', '-b', branch]); + return dir; +} + +function setWorkflowGuard(dir, enabled) { + const planningDir = path.join(dir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ hooks: { workflow_guard: enabled } }, null, 2) + ); +} + +function runHookInput(cwd, input) { + return spawnSync(process.execPath, [HOOK_PATH], { + cwd, + encoding: 'utf8', + input: JSON.stringify({ cwd, ...input }), + }); +} + +function runBashHook(cwd, command) { + return runHookInput(cwd, { + tool_name: 'Bash', + tool_input: { command }, + }); +} + +describe('bug #261: workflow guard blocks forced git add on worktree-agent branches', () => { + test('blocks git add -f on worktree-agent branch when workflow guard is enabled', () => { + const dir = makeRepo('worktree-agent-a1'); + try { + setWorkflowGuard(dir, true); + const result = runBashHook(dir, 'git add -f .planning/phases/01/01-01-SUMMARY.md'); + assert.strictEqual(result.status, 2); + const envelope = JSON.parse(result.stdout); + assert.strictEqual(envelope.decision, 'block'); + assert.strictEqual(envelope.code, 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('blocks git add --force with git global options on worktree-agent branch', () => { + const dir = makeRepo('worktree-agent-b2'); + try { + setWorkflowGuard(dir, true); + const result = runBashHook(dir, `git -C "${dir}" add --force .planning/SUMMARY.md`); + assert.strictEqual(result.status, 2); + assert.strictEqual(JSON.parse(result.stdout).code, 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows ordinary git add on worktree-agent branch', () => { + const dir = makeRepo('worktree-agent-c3'); + try { + setWorkflowGuard(dir, true); + const result = runBashHook(dir, 'git add .planning/SUMMARY.md'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows pathspecs named like force flags after git add -- terminator', () => { + const dir = makeRepo('worktree-agent-d4'); + try { + setWorkflowGuard(dir, true); + const result = runBashHook(dir, 'git add -- -f'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows git add -f outside worktree-agent branches', () => { + const dir = makeRepo('feature-docs'); + try { + setWorkflowGuard(dir, true); + const result = runBashHook(dir, 'git add -f .planning/SUMMARY.md'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows git add -f on worktree-agent branch when workflow guard is disabled', () => { + const dir = makeRepo('worktree-agent-e5'); + try { + setWorkflowGuard(dir, false); + const result = runBashHook(dir, 'git add -f .planning/SUMMARY.md'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('allows git add -f on worktree-agent branch when no GSD config exists', () => { + const dir = makeRepo('worktree-agent-f6'); + try { + const result = runBashHook(dir, 'git add -f .planning/SUMMARY.md'); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stdout, ''); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + test('applies the advisory path to MultiEdit when workflow guard is enabled', () => { + const dir = makeRepo('feature-multiedit'); + try { + setWorkflowGuard(dir, true); + const result = runHookInput(dir, { + tool_name: 'MultiEdit', + tool_input: { + file_path: path.join(dir, 'src.js'), + edits: [], + }, + }); + assert.strictEqual(result.status, 0); + const envelope = JSON.parse(result.stdout); + assert.match( + envelope.hookSpecificOutput.additionalContext, + /WORKFLOW ADVISORY/ + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/workflow-guard-registration.test.cjs b/tests/workflow-guard-registration.test.cjs index b2b11c02c..d5730e68c 100644 --- a/tests/workflow-guard-registration.test.cjs +++ b/tests/workflow-guard-registration.test.cjs @@ -54,16 +54,24 @@ describe('workflow-guard hook registration (#1767)', () => { test('install.js pushes workflow-guard entry with correct matcher', () => { const content = fs.readFileSync(INSTALL_JS, 'utf-8'); - // Extract the section between "workflow-guard" command construction - // and the next console.log confirmation. The push block should have: - // matcher: 'Write|Edit' and command referencing workflow-guard + // Extract the workflow-guard registration section. It should install the + // Bash-aware matcher and upgrade old edit-only entries on reinstall. const workflowGuardSection = content.match( - /workflowGuardCommand[\s\S]*?console\.log\([^)]*workflow.guard/i + /workflowGuardCommand[\s\S]*?Configure commit validation hook/i ); assert.ok( workflowGuardSection, 'install.js must have a push block for workflow-guard with a console.log confirmation' ); + assert.ok( + workflowGuardSection[0].includes("const workflowGuardMatcher = 'Bash|Edit|Write|MultiEdit'") && + workflowGuardSection[0].includes('matcher: workflowGuardMatcher'), + 'workflow guard must be registered for Bash so worktree-agent git safety checks can run' + ); + assert.ok( + workflowGuardSection[0].includes('workflowGuardHookEntry.matcher = workflowGuardMatcher'), + 'installer must upgrade existing workflow guard hook entries to the Bash-aware matcher' + ); }); });