chore: block direct pushes to protected branches - #31
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a shared Git pre-push hook that blocks direct pushes to ChangesLocal Git Hooks
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
codeant-ai
Bot
added
the
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@scripts/setup-git-hooks.sh`:
- Around line 9-12: The existing-hook workflow is contradictory and risks
discarding unrelated hooks. In scripts/setup-git-hooks.sh lines 9-12, update the
refusal message to remove the instruction to manually chain and rerun the setup
script; in README.md line 152, distinguish an already-installed shared hook from
an unrelated existing pre-push hook and instruct users to preserve the unrelated
hook rather than discard it.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ce83df3-0249-4f2f-817c-e36894fa70f9
📒 Files selected for processing (3)
.githooks/pre-pushREADME.mdscripts/setup-git-hooks.sh
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| while read -r local_ref local_oid remote_ref remote_oid; do | ||
| case "$remote_ref" in |
There was a problem hiding this comment.
Suggestion: This guard reads Git's pre-push ref records directly from standard input. If it is chained after an existing pre-push hook that also consumes that input, the loop receives EOF, checks no refs, and exits successfully, allowing a push to main or master. The chaining mechanism must preserve or buffer the ref stream, or ensure this guard runs before any consumer. [security]
Severity Level: Critical 🚨
- ❌ Manually chained hooks can bypass main/master protection.
- ⚠️ Direct pushes rely solely on server-side branch protection.Steps of Reproduction ✅
1. Run `./scripts/setup-git-hooks.sh` from a checkout with an existing pre-push hook;
`scripts/setup-git-hooks.sh:9-12` refuses replacement and instructs the developer to chain
`.githooks/pre-push` manually.
2. Configure the existing hook to invoke the repository hook after reading its pre-push
stdin, which is a valid manual chaining arrangement for hooks that inspect ref updates.
3. Push to `refs/heads/main`; Git supplies the ref records on stdin, but the existing hook
consumes them before invoking `.githooks/pre-push`.
4. The loop at `.githooks/pre-push:5-12` receives EOF, never evaluates the
protected-branch case at line 7, and exits successfully at line 14, allowing the push to
proceed locally.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .githooks/pre-push
**Line:** 5:6
**Comment:**
*Security: This guard reads Git's pre-push ref records directly from standard input. If it is chained after an existing pre-push hook that also consumes that input, the loop receives EOF, checks no refs, and exits successfully, allowing a push to `main` or `master`. The chaining mechanism must preserve or buffer the ref stream, or ensure this guard runs before any consumer.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| hooks_dir=$(git -C "$repo_root" rev-parse --git-path hooks) | ||
| target="$hooks_dir/pre-push" |
There was a problem hiding this comment.
Suggestion: The --git-path hooks result can be relative, but git -C only changes Git's working directory; the shell still resolves target from the caller's current directory. Running this script from a repository subdirectory can therefore copy the hook into a nonexistent or incorrect path instead of the active hooks directory. Convert the hooks path to an absolute path or perform the copy from repo_root. [api mismatch]
Severity Level: Major ⚠️
- ❌ Setup from repository subdirectories may install incorrectly.
- ⚠️ Protected-branch checks remain absent after setup failure.Steps of Reproduction ✅
1. Clone the repository and change into a repository subdirectory rather than the
repository root.
2. Invoke the documented setup entry point `./scripts/setup-git-hooks.sh` from that
subdirectory; the script computes the root at `scripts/setup-git-hooks.sh:5`.
3. `git -C "$repo_root" rev-parse --git-path hooks` at line 6 may return a relative Git
path, while the shell resolves `target` at line 7 relative to the caller's current
directory.
4. The `cp` at `scripts/setup-git-hooks.sh:15` then targets a path under the subdirectory
instead of the active hooks directory, causing installation to fail or placing the hook
incorrectly.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/setup-git-hooks.sh
**Line:** 6:7
**Comment:**
*Api Mismatch: The `--git-path hooks` result can be relative, but `git -C` only changes Git's working directory; the shell still resolves `target` from the caller's current directory. Running this script from a repository subdirectory can therefore copy the hook into a nonexistent or incorrect path instead of the active hooks directory. Convert the hooks path to an absolute path or perform the copy from `repo_root`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
|
||
| if [ -e "$target" ] || [ -L "$target" ]; then | ||
| printf 'Refusing to replace existing hook: %s\n' "$target" >&2 | ||
| printf 'Chain %s into that hook manually, then rerun this setup script.\n' "$repo_root/.githooks/pre-push" >&2 |
There was a problem hiding this comment.
Suggestion: The reported recovery procedure cannot succeed as written: manually chaining the repository hook into the existing target leaves target present, so rerunning this script immediately hits the same refusal branch. The setup instructions must either install the guard as part of the existing hook without requiring a rerun, or explicitly tell the user to remove or replace the target after chaining. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Existing-hook users cannot complete documented setup.
- ⚠️ Developers must edit or remove hooks manually.Steps of Reproduction ✅
1. Start with an existing active pre-push hook in the hooks directory resolved at
`scripts/setup-git-hooks.sh:6-7`.
2. Run `scripts/setup-git-hooks.sh`; the existence check at lines 9-13 prints the chaining
instruction at line 11 and exits.
3. Manually add a call to `.githooks/pre-push` inside the existing hook, as instructed;
the original target file remains present.
4. Rerun `scripts/setup-git-hooks.sh`; lines 9-12 detect that same target and exit again,
so the documented “then rerun” recovery procedure cannot install anything.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/setup-git-hooks.sh
**Line:** 11:11
**Comment:**
*Incomplete Implementation: The reported recovery procedure cannot succeed as written: manually chaining the repository hook into the existing target leaves `target` present, so rerunning this script immediately hits the same refusal branch. The setup instructions must either install the guard as part of the existing hook without requiring a rerun, or explicitly tell the user to remove or replace the target after chaining.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
1 participant
Why this exists
The previous ship-cycle failure exposed that this repository had no local guard against pushing directly to
main. The workflow relied on process instructions alone, so reviewed work was accidentally published to the protected branch before a pull request could be opened.Design decisions
Decision: Enforce the destination branch in the pre-push hook
Why: The unsafe operation is a push targeting
refs/heads/mainorrefs/heads/master, regardless of which local branch supplies the commit.Alternatives considered: Blocking only when
mainis checked out was rejected because explicit pushes from another local branch could still update the protected branch.Tradeoff: Developers can still push a feature branch while checked out on
main; server-side branch protection remains the final enforcement layer.Decision: Preserve existing hooks by refusing conflicts
Why: Installing a guard must not silently disable another tool's pre-push hook.
Alternatives considered: Replacing the hook path or overwriting an existing hook was rejected because it can remove unrelated checks.
Tradeoff: Repositories with an existing pre-push hook require manual chaining.
Decision: Keep the hook source version-controlled and install a local copy
Why: The repository needs a reviewable source of truth while Git executes hooks from the local checkout's active hooks directory.
Tradeoff: A fresh checkout requires the one-time setup command, and updates require reinstalling the copied hook.
What this enables
Future commits can be reviewed and pushed through feature branches and pull requests without relying on memory to prevent direct updates to
mainormaster.Risks and safeguards
Risk: A developer bypasses local hooks with
--no-verify.Mitigation: Configure GitHub branch protection to require pull requests and prevent direct updates server-side.
Risk: An existing local pre-push hook is disabled during setup.
Mitigation: Setup refuses to replace any existing hook and documents manual chaining.
Validation
Automated:
npm testnpm run type-check(not defined in this repository)npm run lint(not defined in this repository)npm run test:e2e(not applicable)Evidence:
mainandmasterare rejected locally.CodeAnt-AI Description
Prevent accidental direct pushes to protected branches
What Changed
mainormasterare rejected locally with instructions to use a feature branch and pull requestImpact
✅ Fewer accidental direct updates to main or master✅ Safer pull-request workflow after cloning✅ Existing developer hooks remain intact💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
mainormasterbranches.Documentation