fix(git-guards): update test fixtures + strip boolean global git opts#257
Open
JacobPEvans wants to merge 8 commits intomainfrom
Open
fix(git-guards): update test fixtures + strip boolean global git opts#257JacobPEvans wants to merge 8 commits intomainfrom
JacobPEvans wants to merge 8 commits intomainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request tightens the security policy by moving force-push commands from a confirmation-required list to a strictly denied list across all branches. It also refines the logic for detecting core.hooksPath bypasses via the -c flag. Review feedback suggests improving the force-push regex to handle flags placed after the remote or branch and using a more robust regex for configuration key matching to avoid manual string manipulation.
There was a problem hiding this comment.
Pull request overview
Updates the git permission guard and its test fixtures to align with the tightened BLOCKED_ON_MAIN/force-push policy so CI stops failing due to stale expectations.
Changes:
- Deny force-pushes via
DENY_GIT_ONLYand remove force-push fromASK_GIT. - Tighten
-c core.hooksPathfallback parsing to inspect only the config key (reducing false positives). - Update
test_permission_guard.pyfixtures to reflect the tightened behavior and avoid main-branch-dependent commands.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
git-guards/scripts/git-permission-guard.py |
Adjusts force-push classification and refines fallback core.hooksPath detection. |
git-guards/scripts/test_permission_guard.py |
Updates expected decisions/commands to match current guard policy and make tests branch-agnostic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Prefix force-push pattern with ^ to prevent false positives from
commit messages containing "push --force"
- Add .* between push and force flag to match deferred-flag forms
like `git push origin main --force`
- Replace split("=")[0] + simple match with anchored regex for
core.hooksPath detection in fallback tokenizer
- Update stale test comment to reflect all-branch deny policy
3 tasks
JacobPEvans
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JacobPEvans
added a commit
that referenced
this pull request
Apr 29, 2026
* chore(git-standards): forbid emoji in commits and PR titles Adds an explicit "Commit & PR Title Style" section to the pr-standards skill (loaded on demand whenever an agent drafts a PR), and a one-line cross-reference bullet in git-workflow-standards. Companion to the soul.md flip in ai-assistant-instructions. Together these eliminate the gap that caused recent automated AI-fix PRs (#263, #257) to land with `🤖` prefixes — soul.md was endorsing gitmoji and neither plugin skill said otherwise. Applies to human, AI-assisted, and bot-authored PRs alike. Conventional- commit prefixes carry the signal. (claude) * fix(git-standards): clarify emoji-ban scope and conventional-commit phrasing Addresses copilot review feedback (threads VOoy, VOpG, VOpT) on PR #266. Changes: - Rename section header from "Commit & PR Title Style" to "Commit, PR Title & PR Description Style" (header now matches body scope) - Split rule into two parts: emoji ban (commits + titles + descriptions + release notes) and conventional-commit prefix requirement (commit subjects + PR titles only) - Replace ambiguous "Use conventional-commit prefixes only" with "are required at the start of commit subjects and PR titles only" — the original "only" was meant to scope where prefixes apply, but read as if the prefix should be the entire message - Add release notes to the ban for parity with the soul.md update in ai-assistant-instructions PR #604 (claude)
…ith --no-pager Add --no-pager, --bare, -p, -P, --paginate, --no-replace-objects to the global option extraction loop so they are stripped before ^-anchored DENY patterns are applied. Previously the loop only stripped -C and -c, so `git --no-pager push --force` left `--no-pager` at the start of subcommand, defeating the `^push` anchor and silently allowing the force-push. Adds regression test: git --no-pager push --force origin feature/my-branch → deny. (claude)
…pping The test expected silent_allow for git --no-pager -c core.hooksPath=/dev/null because the old loop broke on --no-pager and left the -c option unprocessed. Now that --no-pager is stripped by the loop, the following -c core.hooksPath option is extracted directly (no shlex fallback needed), so the correct outcome is deny. Updated the fixture and its comment accordingly. (claude)
JacobPEvans
changed the title
JacobPEvans
changed the title
The comment listed examples that the regex itself enumerates. Reframe to explain WHY (no argument = different parse pattern from -C/-c). (claude)
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #256
Summary
After the March 2026 policy tightening (all force-pushes now denied, not just to main), three categories of test failures needed fixing:
askfor force-push to feature branches — nowdenygit --no-pager push --forcesilently allowed because--no-pagerwas left insubcommandafter the extraction loop broke, defeating the^pushanchorChanges
--no-pager,--bare,-p/-P,--paginate,--no-replace-objects) so^-anchored DENY patterns fire correctly when global options precede the subcommandask→denyfor feature branches; add regression test forgit --no-pager push --force--no-pageris now stripped by loop so-c core.hooksPathis caught directly (deny), not via shlex fallback (silent_allow)^push\s+.*to avoid matching commit messages; hooksPath key extraction anchored to key portion onlyTest Plan
test_permission_guard.py,test_shlex_valueerror.py,test_gh_guard.py