fix(guard): block git checkout/restore with quoted dot argument#94
fix(guard): block git checkout/restore with quoted dot argument#94majiayu000 wants to merge 6 commits intomainfrom
Conversation
COMMAND_STRIPPED replaces quoted content with empty strings, so `git checkout "."` becomes `git checkout ""` and bypasses the guard. Fix by feeding both COMMAND_STRIPPED and COMMAND_PATH_SCAN (which strips quotes but retains content) into the grep check, so the quoted form is also intercepted. Adds two regression tests for `git checkout "."` and `git restore '.'`. Signed-off-by: majiayu000 <1835304752@qq.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f38bbb00eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # git checkout . / git restore . (discard all changes) | ||
| # Only matches pure "." endings, excluding legal path operations such as git checkout ./src/file | ||
| if echo "$COMMAND_STRIPPED" | grep -qE 'git\s+(checkout|restore)\s+\.\s*(;|&&|\|\||$)'; then | ||
| if echo "$COMMAND_STRIPPED"$'\n'"$COMMAND_PATH_SCAN" | grep -qE 'git\s+(checkout|restore)\s+\.\s*(;|&&|\|\||$)'; then |
There was a problem hiding this comment.
Avoid scanning quoted text for checkout/restore blocks
Using COMMAND_PATH_SCAN in the checkout/restore guard makes the regex inspect quoted payloads again, so harmless commands that only mention git checkout . (for example echo "git checkout ." or logging/templating commands) are now blocked. This is a regression from the previous behavior where quoted content was removed via COMMAND_STRIPPED, and it can interrupt non-destructive workflows that print or manipulate command text rather than execute it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Hook Latency (P95)
Details
| Benchmark suite | Current: 789c594 | Previous: 9a96f4d | Ratio |
|---|---|---|---|
pre-edit-guard (P95) |
206 ms |
181 ms |
1.14 |
pre-write-guard (P95) |
232 ms |
210 ms |
1.10 |
pre-bash-guard (P95) |
270 ms |
230 ms |
1.17 |
post-edit-guard (100) (P95) |
316 ms |
291 ms |
1.09 |
post-write-guard (100) (P95) |
222 ms |
204 ms |
1.09 |
post-edit-guard (5000) (P95) |
328 ms |
303 ms |
1.08 |
post-write-guard (5000) (P95) |
221 ms |
203 ms |
1.09 |
stop-guard (5000) (P95) |
137 ms |
128 ms |
1.07 |
learn-evaluator (5000) (P95) |
137 ms |
128 ms |
1.07 |
This comment was automatically generated by workflow using github-action-benchmark.
…kout/restore block Two follow-up fixes after PR #88: 1. False-positive: COMMAND_PATH_SCAN stripped quote delimiters but kept content, so `echo "git checkout ."` or `git commit -m "doc git checkout ."` were incorrectly blocked. Fix: use COMMAND_STRIPPED only (content replaced with empty "" / '') for this check. 2. Missing coverage: `git checkout -- "."` and `git restore -- "."` bypassed the guard because the old regex required the pathspec to follow immediately after the verb. Fix: add optional `(--\s+)?` and match `""` / `''` as pathspecs. Adds four new test cases: two negative (false-positive regression) and two positive (-- separator variants). Signed-off-by: majiayu000 <1835304752@qq.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 711e804e74
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if echo "$COMMAND_STRIPPED" | grep -qE 'git\s+(checkout|restore)\s+\.\s*(;|&&|\|\||$)'; then | ||
| # Only matches pure "." pathspec (bare, quoted, or with -- separator), excluding legal paths like git checkout ./src/file | ||
| # Uses COMMAND_STRIPPED only (quoted content replaced with "" or '') to avoid false positives from commit messages or echo strings. | ||
| if echo "$COMMAND_STRIPPED" | grep -qE 'git\s+(checkout|restore)\s+(--\s+)?(\.|""|'"''"')\s*(;|&&|\|\||$)'; then |
There was a problem hiding this comment.
Restrict checkout/restore guard to dot pathspecs only
Matching ""|'' against COMMAND_STRIPPED makes the guard treat any quoted path as dangerous, because the quote-stripping step rewrites both "README.md" and 'src/file' to empty quotes. This causes legitimate commands like git checkout "README.md" or git restore 'src/file' to be blocked, which is a functional regression from the prior dot-only check. The matcher needs to differentiate quoted . from other quoted pathspecs instead of keying on empty-quote placeholders.
Useful? React with 👍 / 👎.
…athspecs COMMAND_STRIPPED collapses both "." and "README.md" to "" making them indistinguishable. Switch to a two-step check: COMMAND_STRIPPED confirms git checkout/restore is the top-level command, then COMMAND verifies the pathspec is exactly ".", '"."', or "'.'" to avoid blocking safe operations like `git restore "README.md"` or `git checkout -- 'src/file.txt'`. Add regression tests for both false-positive cases. Signed-off-by: majiayu000 <1835304752@qq.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b01f24aa97
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Two-step: COMMAND_STRIPPED identifies git checkout/restore as top-level command (filters out content inside | ||
| # -m strings); COMMAND verifies pathspec is exactly "." since COMMAND_STRIPPED collapses all quoted args to "". | ||
| if echo "$COMMAND_STRIPPED" | grep -qE 'git\s+(checkout|restore)\s+'; then | ||
| if echo "$COMMAND" | grep -qE 'git\s+(checkout|restore)\s+(--\s+)?("\."|'"'"'\.'"'"'|\.)\s*(;|&&|\|\||$)'; then |
There was a problem hiding this comment.
Scan checkout/restore pathspecs on heredoc-stripped input
The second-stage matcher now runs on raw COMMAND, so heredoc payload text can trigger a false block whenever the top-level command contains any git checkout/git restore. For example, git checkout main && cat <<'EOF' ... git checkout . ... EOF is blocked even though the only git checkout . is inert heredoc content. Before this commit, checkout/restore detection only used COMMAND_STRIPPED (derived from COMMAND_NO_HEREDOC), so this regression was avoided. Reusing COMMAND_NO_HEREDOC (or similarly stripped input) for the dot-pathspec regex would prevent these false positives.
Useful? React with 👍 / 👎.
|
/gemini review |
…heredoc false positives
Raw COMMAND retains heredoc block content, causing a false positive when
a heredoc contains a line like `git checkout .` (e.g. documentation
examples). COMMAND_NO_HEREDOC strips heredoc blocks before the regex
runs while preserving actual quoted arguments ("." vs "README.md"),
so the guard still blocks the real destructive patterns.
Signed-off-by: majiayu000 <1835304752@qq.com>
|
/gemini review |
…d dot in commit message The two-step check (COMMAND_STRIPPED to detect checkout/restore, then COMMAND_NO_HEREDOC to verify "." pathspec) caused a false positive when a chained command like `git checkout -- README.md && git commit -m 'docs: mention git checkout ".";'` was run. COMMAND_NO_HEREDOC still contains the raw commit message, so the regex matched `git checkout ".";` inside it. Replace with a shell-aware subcommand splitter that splits on &&/||/; only outside of quotes. Each subcommand is checked independently: the stripped version detects git checkout/restore, the original subcommand verifies the dot pathspec — keeping both heredoc safety and quote safety. Signed-off-by: majiayu000 <1835304752@qq.com>
… scanner Issue 1 (line 100): the quoted-dot regex lacked a word-boundary assertion, so `git checkout "."/src/file` (shell argv `./src/file`, a specific-path checkout) was incorrectly blocked. Add `(?=\s|$)` lookahead after the closing quote so only standalone `"."` / `'.'` arguments are flagged. Issue 2 (line 67): the Python scanner was invoked with command data in `_CMD_DATA` env var and inside an `if ...; then` guard that failed-open: any exec failure (Python not found, env too large for big commands) silently allowed the destructive command through. Fix: - Pass the command on stdin via a pipe (`printf '%s' ... | python3 <(...)`) to avoid exec-env size limits. - Capture the exit code in `_SCANNER_EC`; treat any code other than 1 (the explicit "safe" sentinel) as dangerous (fail-closed). Signed-off-by: majiayu000 <1835304752@qq.com>
|
/gemini review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 789c594f97
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if not re.search(r'git\s+(checkout|restore)\s+', s): | ||
| continue | ||
| # quoted dot: "." or '.' as a standalone argument (not part of a larger path) | ||
| m = re.search(r'git\s+(checkout|restore)\s+(?:--\s+)?(["\'])\.(["\'])(?=\s|$)', sub) |
There was a problem hiding this comment.
Strip shell comments before checkout/restore matching
The new scanner runs its git checkout/restore regexes against raw sub text, so tokens that appear only in shell comments can trigger a block. For example, true; # git checkout "." is now blocked even though the commented portion is never executed; the previous implementation allowed this. This is a functional regression that can interrupt harmless commands/scripts that annotate examples inline with # ... comments.
Useful? React with 👍 / 👎.
|
/gemini review |
Summary
git checkout "."bypassed the guard becauseCOMMAND_STRIPPEDreplaces quoted content with"", so the literal.check never matchedCOMMAND_STRIPPEDandCOMMAND_PATH_SCAN(which strips quotes but keeps content) in a singlegrepcallgit checkout "."andgit restore '.'Root cause
COMMAND_PATH_SCANalready existed for therm -rfpath scanner (strips quotes, retains content). The checkout/restore check only usedCOMMAND_STRIPPED. Feeding both into grep closes the bypass.Test plan
bash tests/test_hooks.sh— 96/96 passCloses #88