chore: add Git pre-push hook - #2363
Closed
A-nony-mous wants to merge 1 commit into
Closed
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough概述新增 变更Git 钩子保护机制
预估代码审查工作量🎯 2 (Simple) | ⏱️ ~10 minutes 诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.githooks/pre-push:
- Around line 15-16: The current protection in the pre-push hook only checks if
the local current_branch is "main", but this can be bypassed by pushing a
different branch directly to the remote main branch using commands like `git
push origin feature:main`. To fix this, parse the remote reference from the
pre-push hook's stdin (which contains information in the format local-ref
local-sha remote-ref remote-sha) and check if the remote_ref being pushed to is
`refs/heads/main` instead of checking the local current_branch variable. This
ensures the protection applies regardless of which local branch is being pushed
to the main branch.
In `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 323-337: The two subprocess.run() calls lack return code
validation, so the success log message at line 337 may be recorded even when the
git config command fails. After the second subprocess.run() call that sets the
git hooks path to .githooks, add a check of the result's returncode attribute.
If returncode is non-zero (indicating failure), log the error using the stderr
output from the result and skip the success log message. Only record the success
message when the returncode is zero, ensuring that misleading logs are not
generated when the git configuration actually fails.
- Around line 342-345: The try-except block around pre_push_file.chmod(0o755)
silently catches and ignores all exceptions with pass, which prevents debugging
when file permission changes fail. Replace the empty exception handler with a
meaningful log statement that captures the actual exception details at warning
level. This will help identify why the pre-push file cannot be made executable
while still allowing the code to continue execution gracefully.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d44747ba-cba3-49b3-9e26-add7bcf14f9a
📒 Files selected for processing (2)
.githooks/pre-pushsrc/one_dragon/base/operation/one_dragon_context.py
A-nony-mous
force-pushed
the
chore/no-main-push
branch
from
June 21, 2026 09:13
7710b7d to
b1a4936
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 329-331: The condition checking result.returncode != 0 incorrectly
treats the case where the git config key does not exist as a failure. When git
config core.hooksPath is not set, it returns exit code 1 with empty stderr,
which is a normal situation, not an error. Modify the logic to differentiate
between a missing configuration (returncode 1 with empty stderr) which should
allow execution to continue, and actual errors (returncode != 0 with meaningful
error content) which should trigger the warning and return early. Only return
early when there is a real error with actual error details in stderr, not when
the configuration key simply does not exist.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e48b2d16-461e-4d4b-acd8-bded82fb204a
📒 Files selected for processing (2)
.githooks/pre-pushsrc/one_dragon/base/operation/one_dragon_context.py
A-nony-mous
force-pushed
the
chore/no-main-push
branch
from
June 21, 2026 09:17
b1a4936 to
cfbe8b6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 358-359: The exception handling at the Git hooks configuration
block is too broad by catching the generic Exception class, which can mask
unexpected errors. Instead of catching Exception, narrow the exception handling
to catch only the specific expected exceptions that can occur during the
subprocess operation, such as subprocess.SubprocessError and OSError. This
allows unexpected errors to propagate up to the caller for proper handling
rather than being silently logged, making debugging easier and following the
principle of explicit exception handling.
- Around line 351-357: The chmod operation for the pre-push file should be
executed independently of whether the hooks path configuration has changed. Move
the try-except block containing pre_push_file.chmod(0o755) outside of the
conditional branch that checks if current_hooks_path differs from ".githooks",
so that execute permissions are always set on the pre-push file whenever it
exists, regardless of configuration changes. This ensures the protection hook
remains functional even when the hooks path is already correctly configured but
lacks execute permissions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f4b97620-048a-4cf4-9d81-e7343d78f761
📒 Files selected for processing (2)
.githooks/pre-pushsrc/one_dragon/base/operation/one_dragon_context.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .githooks/pre-push
A-nony-mous
force-pushed
the
chore/no-main-push
branch
from
June 21, 2026 09:22
cfbe8b6 to
801ecb9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/one_dragon/base/operation/one_dragon_context.py (1)
354-357: ⚡ Quick win将
chmod的异常收窄到OSError。
Path.chmod()的预期失败是权限、文件系统等 OS 错误；捕获所有Exception会掩盖非预期代码错误。建议修改
if pre_push_file.exists(): try: pre_push_file.chmod(0o755) - except Exception as pe: + except OSError as pe: log.warning(f"赋予 pre-push 脚本可执行权限失败: {pe}")As per coding guidelines, “Avoid unnecessary try-catch blocks. Only use exception handling when meaningful processing is possible; otherwise, let exceptions propagate”.
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py` around lines 354 - 357, The exception handler for pre_push_file.chmod() is catching all Exception types when it should only catch OSError, since OS-level failures (permissions, filesystem issues) are the expected failures from chmod(). Change the except clause from Exception to OSError so that unexpected code errors will propagate and not be masked by overly broad exception handling. This makes the error handling more precise and helps catch genuine bugs.Sources: Coding guidelines, Linters/SAST tools
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 337-340: The current implementation in the git hooks configuration
logic silently overwrites any existing custom core.hooksPath value by checking
if current_hooks_path is not equal to ".githooks" and then unconditionally
setting it to ".githooks". Instead, modify the condition to only auto-configure
when core.hooksPath is not set (None or empty string). When current_hooks_path
already has a non-empty custom value that is not ".githooks", log a warning
message instead of overwriting it, informing the user about the .githooks path
and suggesting they manually merge any necessary pre-push hooks from the
repository.
- Around line 323-340: The _setup_git_hooks() method currently relies on system
PATH lookup to find the git executable in its subprocess.run calls, which could
execute an unintended program in contaminated PATH environments. Use
shutil.which("git") at the beginning of the _setup_git_hooks() method to resolve
the absolute path of the git executable, return early if it returns None, and
then replace both occurrences of ["git", "config", ...] with [git_executable,
...] to use the resolved absolute path instead.
---
Nitpick comments:
In `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 354-357: The exception handler for pre_push_file.chmod() is
catching all Exception types when it should only catch OSError, since OS-level
failures (permissions, filesystem issues) are the expected failures from
chmod(). Change the except clause from Exception to OSError so that unexpected
code errors will propagate and not be masked by overly broad exception handling.
This makes the error handling more precise and helps catch genuine bugs.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f401f495-29a8-4050-842a-781e74e478b7
📒 Files selected for processing (2)
.githooks/pre-pushsrc/one_dragon/base/operation/one_dragon_context.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .githooks/pre-push
Comment on lines
+323
to
+340
| result = subprocess.run( | ||
| ["git", "config", "core.hooksPath"], | ||
| capture_output=True, | ||
| text=True, | ||
| cwd=str(project_root) | ||
| ) | ||
|
|
||
| # 若命令返回码非 0 且 stderr 存在内容，才视为真实错误返回 | ||
| stderr_msg = result.stderr.strip() | ||
| if result.returncode != 0 and stderr_msg: | ||
| log.warning(f"获取当前 Git hooksPath 失败: {stderr_msg}") | ||
| return | ||
| current_hooks_path = result.stdout.strip() | ||
|
|
||
| # 如果没有指向 .githooks 则配置 | ||
| if current_hooks_path != ".githooks": | ||
| set_result = subprocess.run( | ||
| ["git", "config", "core.hooksPath", ".githooks"], |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 查找 Python 代码中仍通过相对可执行文件名调用 git 的 subprocess.run。
# Expect: 修复后不应再看到 subprocess.run(["git", ...]) 形式的匹配。
rg -n -U 'subprocess\.run\(\s*\[\s*"git"\s*,' --glob '*.py'Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 533
🏁 Script executed:
cat -n src/one_dragon/base/operation/one_dragon_context.py | sed -n '300,360p'Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 2992
🏁 Script executed:
# 搜索文件中所有 subprocess 调用
rg -n 'subprocess\.' src/one_dragon/base/operation/one_dragon_context.pyRepository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 249
🏁 Script executed:
# 检查文件导入部分
head -50 src/one_dragon/base/operation/one_dragon_context.pyRepository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 2055
🏁 Script executed:
# 搜索项目中所有 subprocess 使用
rg -n 'subprocess\.' --type py | head -30Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 2541
🏁 Script executed:
# 搜索项目中 shutil 的使用
rg -n 'shutil' --type py | head -20Repository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 1880
🏁 Script executed:
# 搜索项目中是否有其他 "git" 相关的 subprocess 调用
rg -n 'subprocess.*git' --type pyRepository: OneDragon-Anything/ZenlessZoneZero-OneDragon
Length of output: 346
应当使用 shutil.which() 解析 Git 可执行文件的绝对路径，而不是依赖 PATH 查找。
在 _setup_git_hooks() 方法中，对 ["git", "config", ...] 的调用会通过系统 PATH 查找 git 可执行文件。虽然该操作通过 cwd 参数限制了工作目录，但在 PATH 被污染或系统配置不当的环境中，仍可能执行非预期的 git 程序。建议在方法开始处使用 shutil.which("git") 获取 git 的绝对路径，并在找不到时优雅降级，示例如下：
建议修改
def _setup_git_hooks(self) -> None:
"""
自动配置本地的 git hooks 指向项目中的 .githooks 目录
"""
try:
+ import shutil
import subprocess
+ git_executable = shutil.which("git")
+ if git_executable is None:
+ log.warning("未找到 git 可执行文件，跳过自动配置 Git hooks")
+ return
+
cls_file = inspect.getfile(self.__class__)
src_dir = file_utils.find_src_dir(cls_file)然后将两处的 ["git", ...] 替换为 [git_executable, ...]。
🧰 Tools
🪛 Ruff (0.15.17)
[error] 324-324: Starting a process with a partial executable path
(S607)
[warning] 330-330: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[error] 340-340: Starting a process with a partial executable path
(S607)
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py` around lines 323 - 340,
The _setup_git_hooks() method currently relies on system PATH lookup to find the
git executable in its subprocess.run calls, which could execute an unintended
program in contaminated PATH environments. Use shutil.which("git") at the
beginning of the _setup_git_hooks() method to resolve the absolute path of the
git executable, return early if it returns None, and then replace both
occurrences of ["git", "config", ...] with [git_executable, ...] to use the
resolved absolute path instead.
Source: Linters/SAST tools
防止直接推送到 main 分支
A-nony-mous
force-pushed
the
chore/no-main-push
branch
from
June 21, 2026 09:44
801ecb9 to
582eeb9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/one_dragon/base/operation/one_dragon_context.py (1)
372-375: 💤 Low value缩小
chmod异常捕获范围。Line 374 的
except Exception应缩小为except OSError，与外层异常处理保持一致，避免意外吞掉非预期错误。建议修改
if pre_push_file.exists(): try: pre_push_file.chmod(0o755) - except Exception as pe: + except OSError as pe: log.warning(f"赋予 pre-push 脚本可执行权限失败: {pe}")🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py` around lines 372 - 375, The exception handling around the pre_push_file.chmod(0o755) operation is too broad by catching the generic Exception class on line 374. Narrow the exception handling to catch only OSError instead, which is the specific exception type that chmod operations raise. This makes the exception handling consistent with the outer exception handling scope and prevents accidentally catching unexpected errors that should propagate up the call stack.Source: Coding guidelines
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 350-367: The code has two issues to address: First, when
current_hooks_path is an absolute path (like /etc/git-hooks), the path
manipulation with project_root will bypass safety checks and attempt file
operations outside the project directory. Add an explicit check using
path.is_absolute() before attempting to create custom_hooks_dir and skip the
logic if the path is absolute. Second, the except Exception clause on line 364
is too broad and should be narrowed to catch only specific expected exceptions
from file operations (such as OSError which covers FileNotFoundError,
PermissionError, etc.) rather than all exceptions, following the coding
guidelines for exception handling specificity.
---
Nitpick comments:
In `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 372-375: The exception handling around the
pre_push_file.chmod(0o755) operation is too broad by catching the generic
Exception class on line 374. Narrow the exception handling to catch only OSError
instead, which is the specific exception type that chmod operations raise. This
makes the exception handling consistent with the outer exception handling scope
and prevents accidentally catching unexpected errors that should propagate up
the call stack.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7053bc8b-5912-4132-9047-487337d54774
📒 Files selected for processing (2)
.githooks/pre-pushsrc/one_dragon/base/operation/one_dragon_context.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .githooks/pre-push
Comment on lines
+350
to
+367
| elif current_hooks_path != ".githooks": | ||
| # 检测到自定义 hooksPath，尝试自动将 pre-push 注入到对方的路径中以保证两者共存 | ||
| import shutil | ||
| custom_hooks_dir = project_root / current_hooks_path | ||
| custom_pre_push = custom_hooks_dir / "pre-push" | ||
|
|
||
| if not custom_pre_push.exists(): | ||
| try: | ||
| custom_hooks_dir.mkdir(parents=True, exist_ok=True) | ||
| source_pre_push = githooks_dir / "pre-push" | ||
| if source_pre_push.exists(): | ||
| shutil.copy2(source_pre_push, custom_pre_push) | ||
| custom_pre_push.chmod(0o755) | ||
| log.info(f"成功将 pre-push 保护逻辑注入到自定义路径 '{current_hooks_path}' 中") | ||
| except Exception as e: | ||
| log.warning(f"尝试向自定义 hooksPath 注入 pre-push 失败: {e}") | ||
| else: | ||
| log.warning(f"检测到自定义的 Git hooks 路径 '{current_hooks_path}' 且已存在 pre-push 脚本。请手动将仓库 '.githooks/pre-push' 逻辑合并进去。") |
Contributor
There was a problem hiding this comment.
处理绝对路径场景并缩小异常捕获范围。
两个问题：
-
路径处理：如果
current_hooks_path是绝对路径（如/etc/git-hooks），project_root / current_hooks_path会直接返回该绝对路径，导致代码尝试在项目目录外创建目录和写入文件。建议显式检查并跳过绝对路径。 -
异常捕获：Line 364 的
except Exception过于宽泛，按 coding guidelines 应仅捕获预期异常。
建议修改
elif current_hooks_path != ".githooks":
# 检测到自定义 hooksPath，尝试自动将 pre-push 注入到对方的路径中以保证两者共存
import shutil
+ from pathlib import PurePath
+
+ # 跳过绝对路径，避免在项目目录外写入文件
+ if PurePath(current_hooks_path).is_absolute():
+ log.warning(
+ f"检测到自定义的 Git hooks 绝对路径 '{current_hooks_path}'，"
+ "请手动将仓库 '.githooks/pre-push' 逻辑合并进去。"
+ )
+ # 继续执行下方的 chmod 逻辑
+ else:
- custom_hooks_dir = project_root / current_hooks_path
- custom_pre_push = custom_hooks_dir / "pre-push"
-
- if not custom_pre_push.exists():
- try:
- custom_hooks_dir.mkdir(parents=True, exist_ok=True)
- source_pre_push = githooks_dir / "pre-push"
- if source_pre_push.exists():
- shutil.copy2(source_pre_push, custom_pre_push)
- custom_pre_push.chmod(0o755)
- log.info(f"成功将 pre-push 保护逻辑注入到自定义路径 '{current_hooks_path}' 中")
- except Exception as e:
- log.warning(f"尝试向自定义 hooksPath 注入 pre-push 失败: {e}")
- else:
- log.warning(f"检测到自定义的 Git hooks 路径 '{current_hooks_path}' 且已存在 pre-push 脚本。请手动将仓库 '.githooks/pre-push' 逻辑合并进去。")
+ custom_hooks_dir = project_root / current_hooks_path
+ custom_pre_push = custom_hooks_dir / "pre-push"
+
+ if not custom_pre_push.exists():
+ try:
+ custom_hooks_dir.mkdir(parents=True, exist_ok=True)
+ source_pre_push = githooks_dir / "pre-push"
+ if source_pre_push.exists():
+ shutil.copy2(source_pre_push, custom_pre_push)
+ custom_pre_push.chmod(0o755)
+ log.info(f"成功将 pre-push 保护逻辑注入到自定义路径 '{current_hooks_path}' 中")
+ except (OSError, shutil.Error) as e:
+ log.warning(f"尝试向自定义 hooksPath 注入 pre-push 失败: {e}")
+ else:
+ log.warning(f"检测到自定义的 Git hooks 路径 '{current_hooks_path}' 且已存在 pre-push 脚本。请手动将仓库 '.githooks/pre-push' 逻辑合并进去。")🧰 Tools
🪛 Ruff (0.15.17)
[warning] 351-351: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 364-364: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@src/one_dragon/base/operation/one_dragon_context.py` around lines 350 - 367,
The code has two issues to address: First, when current_hooks_path is an
absolute path (like /etc/git-hooks), the path manipulation with project_root
will bypass safety checks and attempt file operations outside the project
directory. Add an explicit check using path.is_absolute() before attempting to
create custom_hooks_dir and skip the logic if the path is absolute. Second, the
except Exception clause on line 364 is too broad and should be narrowed to catch
only specific expected exceptions from file operations (such as OSError which
covers FileNotFoundError, PermissionError, etc.) rather than all exceptions,
following the coding guidelines for exception handling specificity.
Source: Coding guidelines
ShadowLemoon
requested changes
Jun 21, 2026
ShadowLemoon
left a comment
ShadowLemoon
left a comment
Collaborator
There was a problem hiding this comment.
为什么要往上下文里塞开发工具相关的东西？
Contributor
Author
防的就是你 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
防止直接推送到 main 分支
Summary by CodeRabbit
发布说明
新功能
pre-push拦截机制：当推送目标匹配指定仓库，且推送分支为main时，展示拦截提示并阻止推送，以降低误操作风险。Chores
.git与.githooks时，自动校验并配置 Git 钩子路径为.githooks，必要时注入/更新pre-push，并确保脚本具备可执行权限，同时在失败情况下提供日志告警与容错处理。