fix(gitcheck): block bare mutating git commands when worktrees are active

[frits-v]

Summary

• Block 18 bare mutating git subcommands (add, commit, push, pull, merge, rebase, reset, cherry-pick, revert, stash, clean, rm, mv, restore, apply, am, checkout, switch) when worktrees are active
• Split compound commands on shell separators (&&, ||, ;, |) and check each git invocation independently — correctly handles git -C <wt> fetch && git add .
• Extract git subcommand past global flags (--no-pager, -c key=value) to avoid false positives from commit messages containing words like "merge" or "add"
• Read-only commands (status, log, diff, fetch, branch, remote, etc.) remain allowed bare

Context

Real incident: git add && git commit --amend --no-edit && git push --force-with-lease ran in the main checkout instead of the worktree, amending the wrong commit on main. The old code only blocked bare git checkout/switch — all other bare mutating commands passed through.

Test plan

• 17 new unit tests covering all blocked subcommands, compound commands, -C passthrough, commit message false positives, cd-to-worktree allowance, and subcommand extraction
• All 233 tests pass (205 unit + 5 claude_md + 13 integration + 10 proptest)
• cargo clippy -D warnings clean
• cargo fmt --check clean

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR significantly expands the git worktree enforcement guard in gitcheck.rs: the old single-pattern check for git checkout/switch is replaced with a full per-segment pipeline that blocks 18 mutating subcommands (add, commit, push, pull, merge, rebase, reset, cherry-pick, revert, stash, clean, rm, mv, restore, apply, am, checkout, switch). The change is motivated by a real incident where a compound git command ran against the main checkout instead of the active worktree.

Key additions:

• find_bare_mutating_git: splits compound commands on shell separators and checks each segment independently, correctly handling git -C <wt> fetch && git add .
• extract_git_subcommand: walks tokens past global flags (-C, -c, --git-dir, --work-tree) with quoted-value spanning, and skips git-lfs/git-annex-style extensions via the after_git.starts_with('-') guard
• strip_shell_comment: removes unquoted #-tails to prevent cd or git subcommands in inline comments from causing bypasses or false positives
• 25 new unit tests covering all blocked subcommands, compound commands, and multiple regression cases

Issues found:

• RE_CMD_SEP splits on && unconditionally, including occurrences inside $(...) command substitutions. A command like git add $(cd /tmp && echo .) causes cd /tmp to appear in the outer segment alongside git add, satisfying RE_CD_PATH.is_match and silently skipping the bare-mutating-git check. A test and guard for this bypass are missing.
• The test test_cd_same_segment_allows_git has a contradictory name ("allows") and opening comment ("not separated by &&"), while the command uses ; (a separator) and the assertion expects blocking. The test logic is correct but the naming is misleading to future readers.

Confidence Score: 3/5

• Mostly safe to merge with a known edge-case bypass via command substitution that should be addressed before this guard is relied upon as a hard security boundary.
• The core logic is a clear improvement over the old single-subcommand check, and the 25 new tests cover the expected bypass vectors well. However, the command substitution && splitting issue (e.g. git add $(cd /tmp && echo .)) creates an unguarded bypass path not covered by any test, which matters because this hook is the last line of defense against accidental mutations to the main checkout. The misleading test name/comment is minor. Score 3 reflects: solid design + good test coverage — one real bypass gap remains.
• Pay close attention to hooks/src/gitcheck.rs — specifically the find_bare_mutating_git function's RE_CD_PATH segment-skip logic and how RE_CMD_SEP handles &&/|| inside command substitutions.

Important Files Changed

Filename	Overview
hooks/src/gitcheck.rs	Major logic expansion: replaces the narrow git checkout/switch bare-command guard with a full per-segment analysis that blocks 18 mutating subcommands. Introduces find_bare_mutating_git, extract_git_subcommand, and strip_shell_comment helpers with 25 new unit tests covering most bypass vectors. One new bypass via command substitution && splitting was found; a test-name/comment inconsistency was also noted.
CLAUDE.md	Test count updated from 219 to 260 with a revised breakdown reflecting the new test modules. Documentation-only change; no logic impact.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["check_worktree_enforcement(cmd)"] --> B{worktree_active?}
    B -- No --> Z[Return None - allowed]
    B -- Yes --> C{cmd contains 'git'?}
    C -- No --> Z
    C -- Yes --> D{RE_GIT_WORKTREE matches?}
    D -- Yes --> Z
    D -- No --> E{RE_GIT_C matches path?}
    E -- "Yes, main checkout" --> F[BLOCK: git -C on main checkout]
    E -- "No / worktree path" --> G{RE_CD_PATH matches?}
    G -- "Yes, main checkout" --> H[BLOCK: cd to main checkout]
    G -- "No / worktree path" --> I["find_bare_mutating_git(cmd)"]
    I --> J["Split cmd on RE_CMD_SEP (&&, ||, ;, |, &)"]
    J --> K["For each segment: strip_shell_comment + trim"]
    K --> L{RE_GIT_WORD_BOUNDARY matches?}
    L -- No --> K
    L -- Yes --> M{RE_CD_PATH matches segment?}
    M -- "Yes (any cd present)" --> K
    M -- No --> N["extract_git_subcommand(segment)"]
    N --> O{had_dir_flag -C?}
    O -- Yes --> K
    O -- No --> P{subcommand in MUTATING_GIT_SUBCMDS?}
    P -- Yes --> Q["BLOCK: Bare 'git subcmd'"]
    P -- No --> K
    K -- "All segments done" --> Z

    style M fill:#ffe0b2,stroke:#f57c00
    style J fill:#ffe0b2,stroke:#f57c00

Loading

_{Last reviewed commit: "fix(gitcheck): skip ..."}

[frits-v]

Addressed both P1 findings from Greptile in 72ecfda:

1. cd guard narrowed to per-segment — Removed the whole-command !RE_CD_PATH.is_match(cmd) guard. cd is now checked per-segment inside find_bare_mutating_git: only a cd within the same command segment as git skips detection. cd /tmp && git add . is now correctly blocked.

2. -C detection via flag-aware walker — Removed the contains(" -C ") substring check. extract_git_subcommand now returns a GitSubcommand struct with had_dir_flag, set by the token walker when it encounters -C in GIT_FLAGS_WITH_ARG. Quoted strings like env GIT_SSH_COMMAND="ssh -C" git add . are now correctly blocked.

Added regression tests for both cases (test_cd_tmp_does_not_bypass_bare_check, test_ssh_c_flag_does_not_bypass). All 236 tests pass.

[greptile-apps]

.

[frits-v]

@greptile review

[greptile-apps]

.

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[greptile-apps]

Command substitution && creates a cd-bypass

RE_CMD_SEP splits on &&, ||, ;, |, and & unconditionally — including occurrences inside $(...) subshells. This means cd from inside a command substitution can appear in the same split segment as the outer git command and trigger the RE_CD_PATH exemption on line 458.

Concrete example:

git add $(cd /tmp && echo .)

Split on && → ["git add $(cd /tmp ", " echo .)"]

• Segment 1: "git add $(cd /tmp" — RE_GIT_WORD_BOUNDARY matches, then RE_CD_PATH matches cd /tmp → segment skipped (bypass)
• Segment 2: "echo .)" — no git → skipped

The shell would execute git add . in the main checkout CWD (the subshell cd only changes the subshell's directory; echo . returns the literal string .), but the hook silently allows it.

The fix requires either tracking nesting depth when splitting (count $( / ) pairs) or stripping/replacing command substitutions before calling RE_CMD_SEP.split. A minimal guard is to strip anything inside $(...) before the segment loop:

// Strip command substitutions before splitting to prevent cd inside $(...) from
// matching RE_CD_PATH in the outer segment.
let cmd_stripped = RE_CMD_SUBST.replace_all(cmd, "");
for segment in RE_CMD_SEP.split(&cmd_stripped) {

where RE_CMD_SUBST matches $([^)]*) (simplified; a full solution would need balanced-paren handling).

[greptile-apps]

Misleading test name and contradictory comments

The test name test_cd_same_segment_allows_git and its opening comment claim to test cd and git in the same shell segment, but the command "cd /ws/repo/.worktrees/abc12345; git add ." uses ; — which is listed in RE_CMD_SEP — so the two commands are in different segments. The inline comment on line 1063 even acknowledges this ("; splits into two segments, so git add is still bare → blocked").

Additionally, the function name says "allows" while assert!(reason.is_some(), ...) asserts the command is blocked, making the name and the assertion semantically opposite.

Suggested rename and comment cleanup:

Suggested change

	#[test]
	fn test_cd_same_segment_allows_git() {
	// cd and git in the SAME shell segment (not separated by &&)
	// This is unusual but the per-segment cd check handles it
	let cmd = "cd /ws/repo/.worktrees/abc12345; git add .";
	// `;` splits into two segments, so git add is still bare → blocked
	let reason = check_worktree_enforcement(cmd, true, "abc12345");
	assert!(reason.is_some(), "cd in different segment should block");
	fn test_cd_semicolon_does_not_exempt_bare_git() {
	// `;` is a shell separator — same as `&&` for splitting purposes.
	// cd and git land in *different* segments, so git add has no cd context → blocked.
	let cmd = "cd /ws/repo/.worktrees/abc12345; git add .";
	let reason = check_worktree_enforcement(cmd, true, "abc12345");
	assert!(reason.is_some(), "cd in different segment should block");