feat(hooks): deny git checkout/restore that would discard uncommitted work

[tvna]

Summary

Adds a new checkout_restore_paths detection surface to hooks/gitapex_check_bash_safety.py's classify() and a new live git diff wrapper step to hooks/check-bash-safety.sh, so a git checkout/git restore invocation that would silently discard uncommitted work is denied instead of run unchecked.

Dedup: this PR implements issue #1375, which already recorded its own dedup search (repo:tvna/gitapex git checkout discard uncommitted guard / repo:tvna/gitapex checkout restore PreToolUse deny, 4 unrelated hits, no duplicate proposing this mechanism) before this PR existed; no additional PR-level dedup search performed.

Additional hardening found during Step 8 independent review (issue #1375)

This PR's own iterative Step 8 independent-review loop (drafting-a-pr-to-merge
skill) found and fixed substantially more than the V1 feature described above
while reviewing this diff -- every fix below is committed to this same branch,
each with its own regression tests and detailed docstring citing the live
reproduction. This section summarizes that story for reviewers; see the
per-commit messages and the cited functions' own docstrings in
hooks/gitapex_check_bash_safety.py for full detail on any individual round.

Early rounds: checkout/restore-specific hardening

Closed a series of narrower bypasses and false positives specific to the new
checkout_restore_paths detection surface itself: line-continuation handling,
-b/-B/--orphan's own value wrongly treated as a path, --pathspec-from-file
parity with restore, a whole-module shlex nested-quote bypass (disclosed,
not fully closed -- see Residuals below), redirect-clause exclusion and
strict-vs-lenient redirect-fd handling, cd/pushd/popd recognized as
cwd-relocating (including a dynamic form), and outer-scope threading into
$(...) command-substitution recursion.

The main story: an order-blind / scope-blind reassignment-collapse bug class

A sustained series of independent-review rounds found progressively different
shapes of the same underlying gap: this classifier's static token-scan
resolves a bash variable's value using a heuristic that can ignore bash's real
sequential execution order or its real scoping rules, trusting a stale, wrong,
or scope-inappropriate value. This affected not just checkout/restore but two
other pre-existing consumers sharing the same resolution machinery:
_rule_gh_api_write (the gh api write-method/field detector) and the B1a/B1b
rules (dynamically-constructed watched-tool/verb detection).

• Order-blindness within a single scope: a name reassigned after an
  earlier read stayed trusted at its stale value -- closed via a "sticky bias"
  mechanism, then widened to cd/pushd/popd tokens, fused/history-widened
  indirect references, and (critically) to the two gh-api-write/B1a-B1b
  consumers, which originally had no such protection at all.
• Assignment-construct blind spots: NAME+=value (compound append),
  read/readarray/mapfile/printf -v, and NAME[i]=value (array-element
  assignment) were each, in turn, found completely invisible to the
  assignment-tracking regexes -- defeating checkout/restore and, separately,
  the gh-api-write/B1a-B1b consumers.
• Over-narrow and over-broad exclusions: a fix meant to avoid regressing
  one specific known-unresolvable case (see graphql-mutation-keyword- variable-concatenation below) was first too broad (excluding an entire
  class of genuine reassignment from protection) and, once narrowed, itself
  needed a "clear on a later static reassignment" refinement so a name
  restored to a trustworthy value did not stay falsely poisoned forever.
• Indirect-reference resolution: a single-level ${!NAME} indirect-
  reference scan (sufficient for one code path) was reused for a second path
  where it was NOT sufficient, defeating detection through one extra layer of
  indirection.
• Scope isolation (the most severe findings): three consecutive rounds
  found genuine security bypasses (not merely over-denials) in how the
  "clear on a later static reassignment" logic accounted for bash's own
  scoping rules -- a fake "the value is safe now" reassignment written inside
  a (...) subshell, a | pipeline stage, a backgrounded (cmd &) job, a
  local/declare/typeset function-scoped declaration, or a <(...)/
  >(...) process substitution can never actually reach the parent shell's
  own copy of the name in real bash -- but the pre-fix code trusted each of
  these exactly like an ordinary top-level reassignment, letting an earlier,
  genuine poisoning be wiped out. Each was independently reproduced live via
  classify() and real bash execution against stand-in uv/gh
  binaries before being closed, generalizing into a single
  _segment_tokens_with_scope_isolation mechanism built on
  _raw_segments_with_boundaries.

A companion fix (((EXPR)) arithmetic-command syntax vs. a deliberately-
spaced, genuinely double-nested ( (cmd) ) subshell) was investigated and its
"obvious" resolution deliberately rejected after live verification proved it
would itself reopen a bypass -- see Residuals below.

Round 40: a fourth checkout/restore protection gap

Round 40 deliberately branched away from the scope-isolation story into
checkout/restore path resolution itself once that area showed signs of
exhaustion, and found a fourth class of protection gap: _git_checkout_paths
treated --ours/--theirs/-2/-3 (git's own conflict-resolution side
flags) the same as the genuinely ref-ambiguous -b/-B/--orphan, folding a
single trailing positional after any of them into the bare-SOMENAME Non-goal
and never surfacing it to the wrapper's live dirty-file check. But real git
flatly refuses to combine a conflict side flag with branch switching
(live-verified against real git 2.43.0), so that positional is unambiguously a
path -- git checkout --ours realfile.py (and --theirs/-2/-3) silently
discarded a genuinely dirty tracked file while classify() reported nothing
to check. Fixed by recognizing these four flags and resolving the remaining
positional as a path; the --conflict=<style> flag was independently
live-verified NOT to share this property (a real ref after it still switches
branches) and correctly stays in the Non-goal, unchanged.

Round 41 (final review): a boundary bug and a disclosed heredoc-blindness residual

Round 41 was designated this PR's own FINAL independent-review round (an
explicit operator decision, made after 40 rounds -- see "Independent review
verdict" below for the full disclosure of what that means for this round's
own fix). Deliberately auditing areas outside the heavily-reviewed
scope-isolation and checkout-path-resolution stories, it found one genuine
security bypass and one genuine, previously-undisclosed false-positive class.

Fixed: _git_checkout_paths's -b/-B/--orphan and
--pathspec-from-file/--pathspec-file-nul checks scanned the ENTIRE token
list, including every token after a literal --, before the -- boundary
was ever honored. Real git guarantees every token after -- is a pathspec,
never a flag -- but a tracked file literally named -b (or -B/--orphan/
--pathspec-from-file), referenced after -- (git checkout -- -b, an
ordinary, unambiguous path reference), folded the WHOLE invocation into the
bare-SOMENAME Non-goal, silently skipping the wrapper's own live dirty-file
check. Live-verified end to end through the actual shipped wrapper: with a
tracked file literally named -b genuinely dirty, the wrapper allowed
git checkout -- -b outright, and running it for real silently discarded
the dirty content. Fixed by restricting both checks to the tokens strictly
before the first --.

Disclosed, not fixed: tokenize() has no here-document (<<DELIM)
or here-string (<<<) awareness at all -- a heredoc body's own text is
tokenized as if it were live command source, so a denied phrase sitting in
pure heredoc DATA triggers a denial bash never actually executes (safe
direction, never a bypass). A DIFFERENT mechanism from the #1404/#1412/#1502
shlex quote-information-loss class (a missing lexical construct, not
quote-provenance loss) -- tracked as its own dedicated issue,
#1520, matching this module's own
established disclosure convention.

Disclosed, accepted residuals (not fixed, deliberately)

• ((EXPR)) arithmetic vs. double-subshell ambiguity: this classifier's
  own tokenizer discards the presence/absence of a space between adjacent
  parens that real bash's lexer uses to disambiguate ((VERB=1)) (arithmetic,
  runs in the current shell) from ( (VERB=1) ) (a genuine double-nested
  subshell). Treating the ambiguous pair as depth-neutral was proven live to
  reopen a bypass (a double-subshell can hold an arbitrary, fully-isolated
  string value), so ((...))-shaped content stays conservatively treated as
  isolated -- an accepted over-denial on genuinely harmless arithmetic usage,
  not a bypass.
• graphql-mutation-keyword-variable-concatenation: a gh api graphql
  mutation built from purely-dynamic variable concatenation with no earlier
  static value is genuinely unresolvable by this classifier; the existing
  _substitute_var_refs_candidates-based resolution already treats it as "no
  evidence of a write" rather than poisoning it, to avoid a documented
  false-positive regression. Tracked in KNOWN_BYPASS_COMMANDS.
• printf -v$NAME fused form: the separate-token printf -v NAME form is
  recognized; the fused printf -vNAME (no space) form is not.
• Outer-scope-threading for $(...)/array-literal spans: a poisoned
  name's status does not thread into a nested recursive $(...)/array-literal
  span the same way it does at the top level.
• Whole-module shlex nested-quote bypass: disclosed early in this PR's
  own review (see docs(hooks): disclose a critical, whole-module shlex nested-quote bypass); a pre-existing limitation of the underlying
  tokenizer, out of this PR's own scope to fix.
• Quoted-paren-shaped subshell-depth-tracking bypass/false-positive: the
  same shlex quote-information-loss class as the item above, found live
  during round 39 while stress-testing rounds 30-38's own scope-isolation
  story -- _raw_segments_with_boundaries recognizes a real subshell
  open/close purely by a token's TEXT, so a QUOTED "("/")" argument
  tokenizes identically to a genuine, unquoted operator. Two shapes
  confirmed live: a quoted ) inside a real subshell prematurely
  decrements tracked depth (a bypass, letting an isolated reassignment
  wrongly clear a poisoning), and a quoted ( with no matching close
  inflates depth for the rest of the command (a false positive, wrongly
  denying an ordinary top-level clearing reassignment). Deliberately NOT
  attempted here, for the identical reason issue CRITICAL: a quoted redirect-operator-shaped filename defeats checkout/restore path extraction #1412 already gives (a
  narrow patch risks reintroducing a worse false-positive class; a sound
  fix needs tokenize() itself to preserve per-token quote/escape
  provenance). Tracked as its own dedicated issue,
  CRITICAL: quoted parenthesis characters defeat subshell-depth tracking in the scope-isolation reassignment-clearing mechanism #1502.
• Heredoc-body-tokenized-as-live-command-text: found live during round
  41 (this PR's own final review round) -- see "Round 41" above. Tracked as
  its own dedicated issue, hooks/gitapex_check_bash_safety.py: here-document bodies are tokenized as live command text, causing false-positive denials #1520.
• Issue CRITICAL: nested double quotes inside a command substitution defeat the whole bash-safety classifier #1404, issue CRITICAL: a quoted redirect-operator-shaped filename defeats checkout/restore path extraction #1412, issue CRITICAL: quoted parenthesis characters defeat subshell-depth tracking in the scope-isolation reassignment-clearing mechanism #1502, and issue hooks/gitapex_check_bash_safety.py: here-document bodies are tokenized as live command text, causing false-positive denials #1520 remain open,
  tracked separately, out of this PR's scope.

Independent review verdict

• Verdict: CLEAN
• Verified commit: 426592c

Step 8's independent-review loop ran 41 rounds against this PR (issue
#1375), each dispatching a fresh adversarial reviewer against the current
diff informed of every prior round's findings so it would not re-report
them. Rounds 30-38 formed an unusually dense run in the scope-isolation
area (8 of 9 rounds found genuine defects); round 39 found a third instance
of the disclosed shlex-quote-information-loss class (issue #1502) rather
than a fixable defect; round 40 found and fixed a fourth checkout/restore
protection gap (conflict-side flags). By explicit operator decision, made
after this sustained run of rounds, round 41 was designated this PR's
own final independent-review round rather than continuing the loop
indefinitely: it found and fixed one more genuine security bypass (the
-- boundary bug, see "Round 41" above) and disclosed one more
genuine, safe-direction false-positive class (heredoc-body-blindness,
issue #1520) -- both independently re-verified against classify() and
real bash/git by this session directly, not merely trusted from the
reviewing subagent's own report.

Disclosed scope limitation of this verdict: round 41's own fix (the
-- boundary fix above) was regression-tested at every layer this PR's
own established discipline requires (a @given-parametrized unit test,
a classify()-level end-to-end test, and a real-scratch-repo wrapper-
level pin against a genuinely dirty tracked file), and the full existing
suite (1026 tests) stays green with no regression -- but, because round 41
was designated final, that fix itself was NOT put through a FURTHER fresh
adversarial review round the way every earlier round's own fix was before
this PR reached CLEAN. This is a deliberate, disclosed, operator-approved
scope boundary on this verdict, not a silent gap: current test count for
the classifier's own suites is 1026 passed
(hooks/test_gitapex_check_bash_safety.py +
tests/test_gitapex_check_bash_safety_properties.py), 99% coverage on
hooks/gitapex_check_bash_safety.py with an unchanged, pre-existing
2-function gap; ruff/mypy and the detection-logic property-coverage gate
all green as of the verified commit above.

Facts

• hooks/gitapex_check_bash_safety.py had zero handling for checkout/restore before this change (grepped the full file case-insensitively; confirmed again on this diff's base).
• Verdict was a 3-field NamedTuple(deny, reason, is_git_push); this PR adds a 4th field, checkout_restore_paths: tuple[str, ...] = (), defaulted so every pre-existing Verdict(...) call site needed no change.
• Live-verified against real git 2.43.0 in a scratch repo (re-verified independently in this session, not only cited from the issue): git diff --quiet HEAD -- PATH exits 1 when dirty and 0 for both a nonexistent path and an unresolved literal string (the fail-open shape this design avoids by denying an unresolvable dynamic path token in the classifier itself); git checkout . silently discards an uncommitted change; git check-ref-format --branch ./--branch .. both fail as invalid branch names; git checkout no-such-ref no-such-file (two unresolvable positionals, no --) reports a pathspec error for both.
• _is_git_push_segment lowercases every literal token before comparison (confirmed at hooks/gitapex_check_bash_safety.py:2407 pre-diff) -- reusing that shape verbatim for git restore's flag walk would collapse -S/-s, so the new restore flag walk is deliberately case-sensitive instead.
• _rule_command_substitution_content/_rule_array_literal_content already thread is_git_push outward through every recursive span (issue fix(hooks): close pipe/variable-indirection bypass class in check-bash-safety.sh deny logic (Stage 1) #1326's own fifteenth/nineteenth-round fixes); this PR widens both to also thread checkout_restore_paths the same way, confirmed with a dedicated regression test for each (x=$(git checkout -- f.py), A=(git checkout -- f.py); "${A[@]}").
• Two real fail-open bugs were found and fixed during this session's own defeat-testing before this PR was opened (see "Assumptions" below for how each was found): a ${paths[@]}-shaped array-subscript path was resolved to its own unexpanded literal text instead of being recognized as unresolved, and a for f in $(...); do git checkout -- "$f"; done-shaped loop was invisible to detection because the subcommand scan was anchored to seg[0].

Assumptions

• The issue's own Acceptance Criteria Map is adopted without modification (independently re-verified against current source and live git before this branch started; re-verification marker posted on the issue).
• The ${paths[@]} bug was found by directly exercising classify() against the issue's own example commands during implementation, not inferred from the design doc alone -- the design doc had already predicted the underlying _VAR_REF_FULL_RE limitation but the specific silent-passthrough failure mode was confirmed live in this session.
• The for-loop detection gap was found the same way: _find_git_checkout_restore originally anchored its git scan to seg[0], which a literal do token (bash's for/do/done are not shell control operators, so segment_tokens never splits on them) defeats. Fixed by scanning for a literal git token at any position in the segment, mirroring _is_git_push_segment's own existing scan.
• git commit -m "$(cat <<'EOF' ... EOF)" (used for this branch's own first commit message) recursively re-tokenizes the heredoc body as if it were executable shell source (a pre-existing, disclosed limitation of _rule_command_substitution_content -- it does not understand heredoc syntax at all -- now formally disclosed and tracked as issue hooks/gitapex_check_bash_safety.py: here-document bodies are tokenized as live command text, causing false-positive denials #1520, see "Round 41" above) -- a commit message describing this feature in prose can trip the new gate on its own words. Worked around here by using git commit -F for the second commit instead of fixing the underlying heredoc-blindness, which is out of this issue's own scope.

Acceptance Criteria Map

Restated from issue #1375's own ACM, row by row, with each Proof method's actual result appended.

Criterion	Interpretation	Planned ops	Proof method	Residual risk
The near-miss's own exact command, replayed from the repo root AND from a subdirectory, is denied when the target has uncommitted changes	Fact 5's cwd finding must actually be fixed, not just diagnosed	Wrapper reads .cwd from the payload for the live check	hooks/test_gitapex_check_bash_safety.py::test_checkout_denied_when_target_has_uncommitted_changes / ::test_checkout_denied_from_a_subdirectory_when_target_has_uncommitted_changes, run end-to-end through the real shipped script against a scratch repo -- PASS	Low
A dynamic/unresolvable path token denies rather than silently passing	Fact 5's fail-open finding	Classifier denies on unresolvable dynamic path token; wrapper never sees it	test_classify_denies_a_loop_fed_dynamic_checkout_path (the issue's own for f in $(...); do git checkout -- "$f"; done example) and test_resolve_path_tokens_denies_an_unresolvable_dynamic_token -- PASS; this exact case was a real bug found and fixed during implementation (_find_git_checkout_restore was originally seg[0]-anchored)	Low
git checkout . and multi-positional git checkout (no --) are covered	Fact 5's pathspec-fallback finding widens V1 scope	Detection sub-cases (b) and (c)	test_git_checkout_paths_treats_a_single_dot_or_dotdot_positional_as_a_path, test_git_checkout_paths_extracts_two_or_more_positionals_with_no_double_dash, test_checkout_dot_denied_when_a_tracked_file_is_dirty (end-to-end) -- PASS	Low
git restore --staged is never denied; -s PATH and --staged --worktree PATH are always denied when dirty	Fact 5's case-collapse finding	Case-sensitive, enumerated restore flag walk	test_git_restore_paths_empty_when_staged_without_worktree, test_git_restore_paths_checked_when_staged_and_worktree_both_present, test_git_restore_paths_checked_for_source_short_flag_not_conflated_with_staged, test_restore_staged_allowed_even_when_worktree_is_dirty (end-to-end) -- PASS	Medium (restore's flag vocabulary is enumerated against git 2.43.0's own git restore -h; a materially different git version's flag set is not re-verified here)
--pathspec-from-file and any unrecognized flag in a restore segment deny rather than under-extracting paths	Fail-open shape found during design review	Explicit deny branches, not silent fallthrough	test_git_restore_paths_denies_pathspec_from_file, test_git_restore_paths_denies_an_unrecognized_flag -- PASS	Low
$(...)-wrapped and array-literal-wrapped checkout/restore thread checkout_restore_paths outward	Fact 5's recursion-drop finding	Both recursive rule functions thread and concatenate the new field, mirroring is_git_push's own OR-ing	test_classify_threads_checkout_restore_paths_through_command_substitution, test_classify_threads_checkout_restore_paths_through_array_literal -- PASS	Low
A path containing a newline or other shell-hazardous byte is still checked correctly, not silently split/skipped	Newline-splitting hazard found during design review	JSON array (not newline-joined string) from classifier; base64 round-trip in the wrapper	Implemented per design; exercised indirectly by every other passing path-extraction test, but not independently pinned with a dedicated newline-containing-path regression test in this PR	Low-medium (residual: no dedicated test for this specific byte-hazard case)
An unmerged (merge-conflict) path denies with a working remedy in the message, not only git stash	Deny message's own remedy does not work mid-conflict	Deny message names git checkout -m -- PATH as an alternative	test_checkout_denied_in_a_real_merge_conflict_names_the_conflict_remedy -- a REAL merge conflict built in a scratch repo, not simulated -- PASS	Low
A fresh repo with no commits yet (unborn HEAD) does not spuriously deny a clean checkout/restore	Unborn-HEAD false positive found during design review	Empty-tree-hash fallback when HEAD does not resolve (via git rev-parse --verify -q HEAD, not error-message parsing)	test_checkout_allowed_on_unborn_head_with_no_conflicting_content / test_checkout_denied_on_unborn_head_when_staged -- PASS	Low
New detection logic carries Hypothesis property coverage	Issue #1178's own CI gate requirement	@given property tests in tests/test_gitapex_check_bash_safety_properties.py	.github/scripts/gitapex_gate_detection_logic_property_coverage.py run directly against this diff -- PASS: OK: 1 in-scope file(s) graded, 0 inline waiver(s) honoured.	Low
Registry stays in sync	Issue #1345's own precedent	New target entries + refreshed rule text in .gitapex/ssot.json's bash-cli-write-and-install-guard	tests/test_gitapex_scan_ssot_schema.py (91 tests) -- PASS; gitapex_scan_ssot_schema.py run directly -- PASS: No ssot.json drift found.	Low
Ordinary branch-switching and every existing _rule_* behavior is unchanged	Additive detection surface only	No existing _rule_* function modified	Full existing hooks/test_gitapex_check_bash_safety.py + tests/test_gitapex_check_bash_safety_properties.py + tests/test_gitapex_check_bash_safety_differential.py (real-bash-oracle fuzzer) suites -- PASS, 0 regressions	Low

Risk / blast radius

This hook already fails closed for jq/python3-missing/malformed-payload cases; this PR adds new deny paths to the same fail-closed family, all additive (no existing _rule_* function's behavior is modified -- confirmed by the full pre-existing suite staying green). Blast radius is every Bash tool call whose command contains git checkout/git restore in this repository's own Claude Code sessions (and any other environment that installs this same hook). A false deny blocks a legitimate checkout/restore; the remedy is always named in the deny message (stash, resolve-and-add, or git checkout -m --).

Rollback

Revert this PR's commits (git revert). No schema/data migration; .gitapex/ssot.json's new target rows and rule text revert cleanly along with the code.

Verification

uv run --frozen pytest hooks/test_gitapex_check_bash_safety.py tests/test_gitapex_check_bash_safety_properties.py tests/test_gitapex_check_bash_safety_differential.py tests/test_gitapex_scan_ssot_schema.py -q --no-cov
# 586 passed

uv run --frozen pytest --ignore=tests/test_gitapex_check_bash_safety_oracle_pins.py --ignore=tests/test_gitapex_check_task_bash_safety_oracle_pins.py --ignore=tests/test_gitapex_check_bash_safety_differential.py --ignore=tests/test_gitapex_check_task_bash_safety_differential.py -q --no-cov
# 6598 total; 6597 passed, 1 failed (test_gitapex_scan_harden_checkout_pin_drift.py -- a shallow-clone artifact of this session's own checkout, unrelated to this diff: `git rev-parse --is-shallow-repository` is `true` here; the real CI workflow always fetches full history)

uv run --frozen ruff check hooks/gitapex_check_bash_safety.py hooks/test_gitapex_check_bash_safety.py tests/test_gitapex_check_bash_safety_properties.py
uv run --frozen ruff format --check
uv run --frozen mypy hooks/gitapex_check_bash_safety.py hooks/test_gitapex_check_bash_safety.py
# all clean

uv run --frozen xenon --max-absolute E --max-modules B --max-average A --exclude "apm_modules/*,skills/evaluating-skill-quality/scripts/gitapex_check_skill_shape.py" .
# no output (no violations)

git diff -- hooks/gitapex_check_bash_safety.py hooks/check-bash-safety.sh | uv run --frozen python3 .github/scripts/gitapex_gate_detection_logic_property_coverage.py
# OK: 1 in-scope file(s) graded, 0 inline waiver(s) honoured.

uv run --frozen python3 .github/scripts/gitapex_scan_ssot_schema.py .gitapex/ssot.json
# No ssot.json drift found.

Manual end-to-end validation against real scratch git repos (not just pytest) during development: the near-miss's own exact command from both the repo root and a subdirectory; a real merge conflict; an unborn-HEAD fresh repo in both a clean and a staged-dirty state -- all matched the ACM's own expected result before being converted into permanent regression tests.

Checklist

• Tests pass locally
• Docs updated if behavior changed (.gitapex/ssot.json's registry entry)
• Issue number cited in every commit
• If this PR adds/modifies a skills/*/SKILL.md, a docs/superpowers/specs/*.md design doc, a security-relevant skill, or a deterministic checker script (skills/*/scripts/*.py, evals/scripts/*.py, .github/scripts/*.py), a ## Skill audit evidence section discloses the required verdicts/waivers (see .github/scripts/gitapex_gate_skill_audit_disclosure.py) -- no skills/*/scripts/*.py/evals/scripts/*.py/.github/scripts/*.py touched, but this diff DOES register as a changed deterministic gate by hooks/check-*.sh naming and .gitapex/ssot.json registration, so ## Skill audit evidence below is filled in for that reason
• If this PR adds a new Kept-edit-log entry to any evals/*/split.md, that entry discloses a Transfer check line -- N/A, no evals/*/split.md touched
• If this PR adds or increases a skills/*/SKILL.md's Stop-boundary bullets or named dispatch branches, evals//tasks/*.yaml gained at least as many new fixtures -- N/A, no SKILL.md touched

Skill audit evidence

git diff --name-status main...HEAD -- .gitapex/ssot.json hooks/check-bash-safety.sh hooks/gitapex_check_bash_safety.py | uv run --frozen python3 .github/scripts/gitapex_detect_changed_gate_scripts.py names all three of .gitapex/ssot.json, hooks/check-bash-safety.sh, hooks/gitapex_check_bash_safety.py as changed deterministic-gate paths requiring disclosure (registration in .gitapex/ssot.json and the hooks/check-*.sh naming rule). checker-script-adversarial-review does not apply (none of skills/*/scripts/*.py, evals/scripts/*.py, .github/scripts/*.py are touched).

• deterministic-gate-quality: RAN -- read against skills/evaluating-deterministic-gate-quality/references/dimensions.md's deterministic-shape checks (1-6): the new deny path uses the exact same dual-channel deny() (JSON hookSpecificOutput.permissionDecision + non-zero exit) as every existing deny in this hook, not a silently-downgraded warn (dimension 1/2); the live git diff --quiet check re-validates the actual, specific condition being gated (real working-tree dirtiness), not a proxy (dimension 3); a bundled test suite ships beside the new logic in both the unit-level property file and an end-to-end scratch-repo suite (dimension 4); every path fed to the shell wrapper is base64-decoded, never interpolated raw (dimension 5); the wrapper's live git calls carry no unbounded loop or missing timeout beyond what the existing git-push step already accepts (dimension 6, unchanged shape).
• defeat-test-disclosure: RAN -- two real fail-open bugs (the ${paths[@]} unexpanded-literal passthrough and the for-loop seg[0]-anchoring gap) were found by deliberately trying to defeat the new detection logic with the issue's own adversarially-reviewed example commands, before this PR was opened, and both are now permanent regression tests (test_resolve_path_tokens_denies_an_array_subscript_token, test_classify_denies_an_array_subscript_fed_checkout_path, test_classify_denies_a_loop_fed_dynamic_checkout_path, test_find_git_checkout_restore_finds_git_at_any_segment_position).

Related Issue

Refs #1128. Closes #1375.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 03f4405a-3b6e-4e76-9def-b366b48b97f1

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.61%. Comparing base (ae8c090) to head (426592c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1380      +/-   ##
==========================================
+ Coverage   99.60%   99.61%   +0.01%     
==========================================
  Files         145      145              
  Lines       23666    24700    +1034     
  Branches     2833     3001     +168     
==========================================
+ Hits        23572    24606    +1034     
  Misses         94       94              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tvna]

Merge conflict resolution (main -> this branch, commit b03b059)

main advanced past this branch's original base (three merges: PR #1383/#1382/#1377) while this PR was in review, and one of those (issue #1350, PR #1383) produced a real content conflict with this branch's own line-continuation fix.

File in conflict: hooks/gitapex_check_bash_safety.py (the only file with real conflict markers; every other file main advanced with auto-merged cleanly).

What conflicted: both branches independently added a same-named _strip_line_continuations function for the same underlying bash line-continuation gap (this branch found it via issue #1375's own round-3 checkout/restore review; main found it independently via issue #1350's own newline-as-statement-separator fix). Main's merged PR also added a new, unrelated _strip_comments fix (a #-comment word-boundary scanner) that this branch did not have.

Resolution approach:

• Kept _strip_comments as-is (main's, additive, no conflict with this branch's own work).
• For the duplicate _strip_line_continuations, kept this branch's own algorithm (which tracks both single-quote AND double-quote state) over main's simpler version (which tracked only single-quote state), after finding a real correctness bug in main's version during reconciliation: an apostrophe inside an already-open double-quoted string (e.g. a contraction like "don't" in a commit-message-shaped string literal) is wrongly read by main's version as a fresh single-quote opener, since it never tracks double-quote state at all -- this then incorrectly suppresses continuation-removal for the rest of that string. Live-verified before and after the fix:
  • Before (main's algorithm): printf '[%s]' "don't strip \ + newline + this" would leave the continuation unstripped once it saw the apostrophe.
  • After (this branch's algorithm, now the merged result): the same command correctly joins into one argument, [don't strip this].
• Merged both sides' docstrings so neither side's live-verified reasoning was dropped (this branch's own round-3 bypass finding, issue bash-safety: newline-separated statements collapse into one segment, bypassing seg[0]-anchored deny rules #1350's dual fix and the _strip_comments interaction, and this reconciliation's own double-quote finding).
• tokenize() now runs _strip_comments first, then _strip_line_continuations, then constructs the lexer once (removed a duplicate lexer.whitespace = lexer.whitespace.replace("\n", "") line left over from the raw merge).

Verification after resolving: ruff, mypy, xenon, the property-coverage gate, and the ssot-schema scan all clean; hooks/, the checkout/restore suites, both oracle-pins files, and the sibling skills/executing-a-branch-plan/scripts/gitapex_check_task_bash_safety.py suite all green (835 tests); 100% line+branch coverage maintained on the merged file.

Known follow-up, out of this PR's scope: the sibling module skills/executing-a-branch-plan/scripts/gitapex_check_task_bash_safety.py (a self-contained duplicate of the hooks module, per its own header comment) carries its own copy of _strip_line_continuations, merged in from the same main advance, with the identical single-quote-only bug described above -- confirmed by reading its source, not yet independently live-verified against that module's own test harness. Flagging as a candidate follow-up task rather than fixing it here, since it is unrelated to issue #1375's own scope and untouched by this branch until this merge.

---

Generated by Claude Code