fix: Quality Gates を無効化する抜け道を塞ぐ(core.hooksPath ほか) - #979
Conversation
block_git_no_verify.py は --no-verify / -n / HUSKY=0 の3つしか見ておらず、 core.hooksPath の差し替えが素通りしていた。 2026-07-15、Claude が実際に git -c core.hooksPath=/dev/null commit でこのフックを回避しようとし、素通りした(止めたのは Claude Code 側の 分類器であってこのフックではない)。よりによって「Quality Gates が 効いていない穴を塞ぐ」コミットでの出来事だった。 塞いだ抜け道: - git -c core.hooksPath=... その場でフックパスを差し替え - git -ccore.hooksPath=... 値密着形式 - git config core.hooksPath ... 永続的に無効化(以降の全コミットが素通し) - git --config-env=core.hooksPath=... 環境変数経由 - GIT_CONFIG_KEY_n=core.hooksPath 環境変数経由 - GIT_CONFIG_GLOBAL/SYSTEM=/dev/null 設定ファイルごと無効化 - git commit -nm "msg" -n を含む結合ショートフラグ git の設定キーは大文字小文字を区別しないため CORE.HOOKSPATH も検知する。 push の -n は --dry-run で無害なため commit のみを対象にする。 テストは実際にフックを起動する挙動テストとして追加した(結合フラグの 扱いはソースの文字列検査では正しさを証明できないため)。旧実装に対して 15件が失敗することを確認済み。 hooks-lifecycle.test.js の3件は実装の文字列("-n" リテラル・sys.exit(2))を 見ていたため、実装非依存の形に更新した。振る舞いの検証は新しいテストが担う。
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe Git hook now blocks additional verification-bypass techniques, including combined commit flags and ChangesGit bypass protection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BashHook
participant evaluate
participant GitCommand
BashHook->>evaluate: command string
evaluate->>GitCommand: inspect bypass flags and hooksPath overrides
GitCommand-->>evaluate: detected bypass arguments
evaluate-->>BashHook: exit status and sanitized command
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/hooks-block-git-no-verify.test.jsOops! Something went wrong! :( ESLint: 10.7.0 ReferenceError: describe is not defined test/hooks-lifecycle.test.jsOops! Something went wrong! :( ESLint: 10.7.0 ReferenceError: describe is not defined 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/hooks/block_git_no_verify.py:
- Line 147: Update the command reconstruction in the hook’s sanitization flow to
avoid using shlex.join for compound shell commands, since it converts operators
into literal arguments. Preserve the original shell structure while removing
matched spans, or leave compound commands unreconstructed; keep the existing
sanitized output behavior for simple commands.
- Around line 83-99: Reset the Git subcommand flags tracked by the command
sanitizer after each simple shell command, including boundaries marked by &&,
||, ;, and pipeline operators. Update the state handling around seen_git,
seen_commit, and seen_config so a later command such as a push dry-run is
evaluated independently, while preserving detection within each individual Git
command.
- Around line 36-38: Update _is_hooks_path to match only the exact normalized
HOOKS_PATH_KEY or that key followed by “=”, rather than accepting arbitrary
prefixes. Preserve whitespace trimming and case-insensitive matching, while
rejecting keys such as core.hooksPathBackup.
- Around line 28-29: Update GIT_CONFIG_FILE_RE in the git environment-variable
blocking logic to also match GIT_CONFIG_NOSYSTEM, while preserving the existing
matches for GIT_CONFIG, GIT_CONFIG_GLOBAL, and GIT_CONFIG_SYSTEM.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32ca5e82-95fe-45ea-b43b-53ea3facdc00
📒 Files selected for processing (3)
.claude/hooks/block_git_no_verify.pytest/hooks-block-git-no-verify.test.jstest/hooks-lifecycle.test.js
| # GIT_CONFIG_GLOBAL=/dev/null / GIT_CONFIG_SYSTEM=/dev/null(設定ごと無効化) | ||
| GIT_CONFIG_FILE_RE = re.compile(r"^GIT_CONFIG(_GLOBAL|_SYSTEM)?=", re.IGNORECASE) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
curl -fsSL https://git-scm.com/docs/git |
grep -A6 -B2 'GIT_CONFIG_NOSYSTEM' | head -20Repository: keito4/config
Length of output: 975
🏁 Script executed:
sed -n '1,220p' .claude/hooks/block_git_no_verify.pyRepository: keito4/config
Length of output: 5167
Block GIT_CONFIG_NOSYSTEM as well.
It skips system config too, so a system-scoped core.hooksPath can still be bypassed.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 28-28: Comment contains ambiguous ( (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 28-28: Comment contains ambiguous ) (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/hooks/block_git_no_verify.py around lines 28 - 29, Update
GIT_CONFIG_FILE_RE in the git environment-variable blocking logic to also match
GIT_CONFIG_NOSYSTEM, while preserving the existing matches for GIT_CONFIG,
GIT_CONFIG_GLOBAL, and GIT_CONFIG_SYSTEM.
| def _is_hooks_path(value: str) -> bool: | ||
| """core.hooksPath への言及か(git の設定キーは大文字小文字を区別しない)""" | ||
| return value.strip().lower().startswith(HOOKS_PATH_KEY) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the configuration key boundary, not every prefix.
startswith("core.hookspath") also blocks unrelated keys such as core.hooksPathBackup. Accept only the exact key or the core.hooksPath= form.
Proposed fix
def _is_hooks_path(value: str) -> bool:
- return value.strip().lower().startswith(HOOKS_PATH_KEY)
+ normalized = value.strip().lower()
+ return normalized == HOOKS_PATH_KEY or normalized.startswith(
+ f"{HOOKS_PATH_KEY}="
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _is_hooks_path(value: str) -> bool: | |
| """core.hooksPath への言及か(git の設定キーは大文字小文字を区別しない)""" | |
| return value.strip().lower().startswith(HOOKS_PATH_KEY) | |
| def _is_hooks_path(value: str) -> bool: | |
| """core.hooksPath への言及か(git の設定キーは大文字小文字を区別しない)""" | |
| normalized = value.strip().lower() | |
| return normalized == HOOKS_PATH_KEY or normalized.startswith( | |
| f"{HOOKS_PATH_KEY}=" | |
| ) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 37-37: Docstring contains ambiguous ( (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF002)
[warning] 37-37: Docstring contains ambiguous ) (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF002)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/hooks/block_git_no_verify.py around lines 36 - 38, Update
_is_hooks_path to match only the exact normalized HOOKS_PATH_KEY or that key
followed by “=”, rather than accepting arbitrary prefixes. Preserve whitespace
trimming and case-insensitive matching, while rejecting keys such as
core.hooksPathBackup.
| if t == "git": | ||
| seen_git = True | ||
| sanitized.append(t) | ||
| i += 1 | ||
| continue | ||
|
|
||
| if seen_git and t == "commit": | ||
| seen_commit = True | ||
| sanitized.append(t) | ||
| i += 1 | ||
| continue | ||
|
|
||
| if seen_git and t == "config": | ||
| seen_config = True | ||
| sanitized.append(t) | ||
| i += 1 | ||
| continue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Reset Git subcommand state between shell commands.
After seeing git commit, seen_commit stays true indefinitely. Consequently, git commit -m msg && git push -n is blocked even though the PR explicitly permits push dry-runs. Scope the state to each simple command and reset it at &&, ||, ;, and pipelines.
Also applies to: 132-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/hooks/block_git_no_verify.py around lines 83 - 99, Reset the Git
subcommand flags tracked by the command sanitizer after each simple shell
command, including boundaries marked by &&, ||, ;, and pipeline operators.
Update the state handling around seen_git, seen_commit, and seen_config so a
later command such as a push dry-run is evaluated independently, while
preserving detection within each individual Git command.
| if t == "--no-verify": | ||
| block = True | ||
| continue | ||
| return block, (shlex.join(sanitized) if sanitized else "") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not reconstruct compound shell commands with shlex.join.
For example, git commit --no-verify && echo done becomes git commit '&&' echo done, turning the operator into a Git argument. Preserve the original shell structure while removing matched spans, or omit the replacement suggestion for compound commands.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/hooks/block_git_no_verify.py at line 147, Update the command
reconstruction in the hook’s sanitization flow to avoid using shlex.join for
compound shell commands, since it converts operators into literal arguments.
Preserve the original shell structure while removing matched spans, or leave
compound commands unreconstructed; keep the existing sanitized output behavior
for simple commands.
|
🎉 This PR is included in version 1.123.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Closes #978
背景
2026-07-15、Claude(私)が config リポジトリへのコミットで
git -c core.hooksPath=/dev/null commit -m "..."を実行しようとし、
block_git_no_verify.pyは素通しした。止めたのは Claude Code 側の permission 分類器であってこのフックではない。よりによって「private セッションで Quality Gates が効いていない穴を塞ぐ」コミット(#977)での出来事だった。このフックは
--no-verify/-n/HUSKY=0の3つしか見ていなかった。塞いだ抜け道
git -c core.hooksPath=/dev/null commitgit -ccore.hooksPath=/dev/null commitgit config core.hooksPath /dev/nullgit --config-env=core.hooksPath=EVIL commitGIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath …GIT_CONFIG_GLOBAL=/dev/null git commitgit commit -nm "msg"-nmはt == "-n"に一致しなかったgit の設定キーは大文字小文字を区別しないため
CORE.HOOKSPATHも検知する。誤検知させないための線引き
git push -nは通す(--dry-runであって検証スキップではない)。-nの結合フラグ判定は commit の文脈のみgit config user.nameやgit -c color.ui=always logなど通常の設定操作は通すGIT_CONFIG_KEY_0=user.nameのような無関係なキーは通すテスト
test/hooks-block-git-no-verify.test.jsを新規追加。実際にフックを python3 で起動して終了コードを検証する挙動テストにした(-nmの扱いはソースの文字列検査では原理的に証明できないため)。hooks-lifecycle.test.js の3件を更新した理由
既存の3件は実装の文字列(
"-n"リテラル、sys.exit(2))を見ており、リファクタで落ちた。実際の振る舞いは新しい挙動テストが厳密に検証するため、実装非依存の形(sys.exit(main())の伝播、core.hooksPathの検知)に更新した。レビュー観点
GIT_CONFIG_GLOBAL=/dev/nullを一律ブロックしているが、正当な用途(CI等での意図的な設定分離)とぶつからないかSummary by CodeRabbit
New Features
Bug Fixes
Tests