Read Each Refspec's Push Operation, Include Overrides, and Fused Separators in the Write Guard

[ptr727]

Summary

Closes the three write-guard ALLOW gaps #2951 names, in host-setup/agent-safety/claude/gh-write-guard.py, with each of the issue's commands added as a constructed self-test case.

Changes

• Per-refspec push operations. _push_targets no longer keeps one aggregate force and delete flag per push. Each refspec carries its own operation set: +src forces only its own branch, :dst deletes only its own, and --force/--delete apply to every refspec. _check_push_bypass now checks every operation, a force and a delete included, against pull_request, after the force and delete checks. A lone : and a branch pattern (a destination holding * that can name a branch) are read as writing every protected default and each branch named beside them with their own operations. A pattern is also still read as the branch its literal destination spells, so a ruleset pattern such as release/* is still consulted. A tag pattern writes no branch.
• Include overrides. A git command under an include.path or includeIf.<condition>.path override, given by -c or --config-env, whose subcommand is not in git --list-cmds=builtins,main,others, is an unreadable push and denies, since the included file may define it as a push alias. The classify pre-filter gains _GIT_INCLUDE so a command holding no push word reaches the check. _config_key is shared with the hooks-path check.
• A separator fused with a redirection. The tokenizer fuses ;<, &<, &&<, |<, newline+< and similar into one token, which was read as a redirection, so the next command's words read as the push's own refspecs. A push holding such a token is now read twice in _push_invocations: once as before, and once ending at the separator (_collect_arglist(..., split_fused=True)). Both readings are checked, and in the split one a number before the fused token is a word rather than a file descriptor. Every other caller of _collect_arglist keeps develop's reading, because no quote mask the guard has can tell a fused separator from a quoted word or one inside an expansion. _forks_out_of_reach reads the separator part of such a token (_leading_separator) for a background &.
• Self-test. New cases in _GIT_CASES, _PUSH_DIR_CASES, and the wait table, and the _GIT_COMMAND_NAMES pin now covers the _GIT_CASES loop so the include cases do not depend on the host's git.
• READMEs. Requirement 4 in host-setup/agent-safety/README.md and the guard summary in host-setup/agent-safety/claude/README.md describe the per-refspec reading and the include deny.

Differential Against Develop's Guard

As for #2945 and #2952, develop's guard (the merge base) and this branch's guard were loaded side by side and run over the same calls.

• Real-repository mode: 12,855 constructed commands, each in a develop checkout and a feature worktree of a constructed repository, under five rule sets, with no current-branch seam (128,550 calls). The corpus crosses push heads with separators, fused separator-redirection tokens, quoted, backslash-escaped, backtick, $'...', ${...}, $(...), $(<...), process-substitution and comment spellings, branch and tag patterns, include overrides, and gh and wait shapes. A pipeline continued onto the next line and a separator fused with a redirection are both in it.
• Seam mode: every classify call the self-test makes (815) plus 10,245 extra shapes under 20 current-branch and rule seams (205,715 calls).
• Rule 6: 3,042 primary-checkout calls. No verdict differs.

Every allow flip (develop denies, this branch allows) is in the intended per-refspec class, and none occurs under a rule set holding pull_request. In each, another refspec's + or :dst no longer forces or deletes a branch it does not name:

• git push origin +feature/x develop and git push origin :feature/z develop, with develop protected only by non_fast_forward or deletion.
• git push origin : +feature/y and git push origin '+refs/tags/*' develop no longer force develop.
• git push origin +develop feature/y, with rules on feature/y only, no longer forces feature/y.

The separator fix flips nothing toward allow by construction, since develop's reading is still checked. Every other flip is toward deny. Those are the issue's fixes, a force or delete checked against pull_request, the include denies, and the over-denials below.

Known New Over-Denials

• A push whose words hold a fused separator is checked under both readings. A feature-checkout push such as git push ;</dev/null origin develop, where origin develop is really a separate command, denies.
• A branch pattern with a fixed prefix, such as refs/heads/feature/*:refs/heads/feature/*, is read as writing the protected defaults too, so it denies where a default holds pull_request.
• A fused token holding a background &, such as &<, now reads as a background job for the unbounded-wait rule.
• Every includeIf. key reads as an include, one git ignores included, since a --config-env condition holding an = cuts the key the guard reads short of its .path.
• A branch pattern's operations also reach a branch the push names explicitly, which git leaves to that branch's own refspec. That and the fixed-prefix case above are Scope a Branch Pattern's Push Operations to the Branches It Can Match in the Write Guard #2964.

Local Review

Seven frontier passes ran. The first six each found an allow flip that an earlier fix had introduced, and each is fixed:

• A quoted operator word (';<') ending the argument list.
• Fused tokens inside a backtick, $'...', ${...} or $(<...) that the quote mask marked unquoted. The mask was then dropped for the two-reading design above.
• The split reading displacing a push only the whole-command scan finds.
• A forced branch pattern beside a lone :.
• A pattern's literal destination no longer being looked up.

This ran past the two-round edit budget local-strict-review sets, deliberately. Each remaining finding was a new allow in a safety hook, and stopping would have shipped it. The seventh pass, recorded on the pushed head, found no unintended allow flip. Its outcomes:

• Declined (style): sentence-length findings on the new docstrings and README sentences. That check is not a gate here, and the claude README bullet is one physical line, so the older sentences on it are flagged too.
• Accepted over-deny: the fixed-prefix branch pattern above, filed as Scope a Branch Pattern's Push Operations to the Branches It Can Match in the Write Guard #2964.
• Pre-existing, filed: refs/*-style patterns and a pattern matching a non-default protected branch, added to Close the Write Guard's Remaining Push Detection Gaps Found Reviewing the #2936 Fix #2943.

Other pre-existing gaps the passes found, each equal on develop:

• Read a Clustered Short Option's Force or Delete in the Write Guard's Push Check #2957: clustered and abbreviated push flags.
• Deny an Include Override on a Commit as a Hooks-Path Bypass in the Write Guard #2958: include overrides setting the hooks path or a command-running key.
• Read Words the Shell Rewrites Before a Push in the Write Guard #2959: quote-split, backslash-newline-split and run-time words, and uppercase GIT.
• Deny an Include Override on a Git Command That Runs Other Git Commands in the Write Guard #2960: include overrides reaching rebase --exec, submodule foreach and eval.
• Fail Closed on a Comment or a Substitution in a Push or Commit's Own Words in the Write Guard #2961: comment and backtick words.
• Read a Local Branch Name Whole and an Exec-Path Beside an Include Override in the Write Guard #2962: a local branch name losing its first segment, and --exec-path beside an include.

Copilot Round

Copilot's first round raised two threads and two body-only findings, each answered on the pull request. A number before a fused separator is now a word in the split reading, and the pull_request deny message says the push writes the branch. Narrowing the include check to .path keys, as one finding asked, opened the --config-env shape above, so a ninth local pass caught it and the narrowing is reverted. The pattern over-denial is #2964. Two local passes ran on the fix commits, and the last found no new allow.

Verification

• gh-write-guard.py --selftest: 858 cases, SELFTEST PASS. Each new deny case fails against develop's guard, and the split-bookkeeping case also fails against the commit before its fix.
• ruff check, ruff format --check, mypy, python3 -m unittest discover -s tests, test_install.py, build_dist.py --check, repo_gate.py (default, eol, eol-coverage), prose_lint.py on the changed files and --diff origin/develop, spec/validate.py, and docker_lint.py all pass.

Closes on promotion: #2951

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Push checks now evaluate each refspec independently, correctly distinguishing branch updates, force-pushes, and deletions.
  • Writes to branches that require pull requests are denied, including force-pushes and deletions.
  • Pushes with targets the safety check cannot inspect are denied when Git configuration may redirect the command through an alias.
  • Command checks more reliably recognize shell separators, redirections, wrapped commands, and explicit Git directory settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: adbc8938-8ec7-4be0-bf79-813f7276797e

📥 Commits

Reviewing files that changed from the base of the PR and between ee8531c and f31487e.

📒 Files selected for processing: 3

• host-setup/agent-safety/README.md
• host-setup/agent-safety/claude/README.md
• host-setup/agent-safety/claude/gh-write-guard.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The git write guard now parses additional shell separator and redirection forms, detects config includes that may define push aliases, and classifies push operations per refspec. It also denies force-pushes and deletions when a branch requires a pull request.

Changes: Git push guard

Layer / File(s)	Summary
Shell invocation parsing host-setup/agent-safety/claude/gh-write-guard.py	Argument collection handles fused shell separators and redirections. Push checks examine both the original and alternate argument readings. Tests cover fused-token parsing and wait-loop behavior.
Included push alias detection host-setup/agent-safety/claude/gh-write-guard.py, host-setup/agent-safety/README.md	The guard detects include.path and includeIf overrides. It treats commands that may load included push aliases as unreadable targets.
Per-refspec operations and branch rules host-setup/agent-safety/claude/gh-write-guard.py, host-setup/agent-safety/claude/README.md, host-setup/agent-safety/README.md	The guard assigns update, force, and delete operations to individual refspec targets. It denies all three operation types on branches with a pull_request rule. Tests cover refspec handling and denials.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to f3148

The change makes the write guard stricter about per-refspec force and delete operations, include overrides, and fused separators. No concrete merge-blocking risk was found; some over-denials are expected and documented by the author.

Pre-merge checks: 5

Passed checks: 5 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately summarizes the main changes: per-refspec push handling, include overrides, and fused separator parsing. It is specific and concise.
Docstring Coverage		Docstring coverage is 93.75% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 1 files. (2 skipped: 2 …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ptr727]

A recorded local strict-review pass covers head b32876847357b4b05e7088446d18211509fa4067, the content this pull request carries at that commit against develop, and it recorded 4 findings.

[Copilot]

🟡 Changes recommended

Unresolved guard correctness and bypass issues remain, including a critical fused-separator parsing issue.

2 open findings

• Detect fused separators before numeric file descriptors · New
• Scope wildcard operations to matching refspecs · New

What changed in this PR

This PR hardens the Git write guard against refspec, include-config, and fused-separator bypasses.

Changes:

• Tracks force/delete operations per refspec.
• Denies unreadable include-based aliases.
• Adds separator handling, documentation, and self-tests.

Review findings:

• Critical (1 vote): Numeric file-descriptor parsing can miss a fused separator and allow a protected bare push.
• Moderate (3 votes): Wildcard operations are incorrectly applied to unrelated refspecs.
• Moderate (1 vote): Non-.path includeIf keys are incorrectly treated as file includes.
• Nit (1 vote): Force/delete denial messages are inaccurate.

File	Summary
host-setup/​agent-safety/​README.md	Documents refspec and include protections.
host-setup/​agent-safety/​claude/​README.md	Updates the guard behavior summary.
host-setup/​agent-safety/​claude/​gh-write-guard.py	Implements guard logic and self-tests; contains the review findings above.

🧠 Review effort: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.02%. Comparing base (ac78916) to head (f31487e).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2963      +/-   ##
===========================================
+ Coverage    59.96%   60.02%   +0.06%     
===========================================
  Files           17       17              
  Lines         8778     8784       +6     
===========================================
+ Hits          5264     5273       +9     
+ Misses        3514     3511       -3     

Flag	Coverage Δ
python-3.13	60.02% <ø> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ptr727]

A recorded local strict-review pass covers head f31487e41476d7bfa759bbb726503cb7352b3270, the content this pull request carries at that commit against develop, and it recorded 3 findings.

[ptr727]

Answers to the four entries under Review findings: in Copilot's round on b3287684, which the digest counted as (4) findings with no thread of their own.

1. Critical, host-setup/agent-safety/claude/gh-write-guard.py:900, "Numeric file-descriptor parsing can miss a fused separator and allow a protected bare push." Fixed in 3ba4859. This is the threaded finding answered in its thread: under the split reading a digit before a fused separator is now a word, and a self-test case pins it.
2. Moderate, gh-write-guard.py:2452, "Wildcard operations are incorrectly applied to unrelated refspecs." Deferred to Scope a Branch Pattern's Push Operations to the Branches It Can Match in the Write Guard #2964. This is the threaded finding answered in its thread, an over-denial whose narrowing needs its own differential.
3. Moderate, _includes_config_file, "Non-.path includeIf keys are incorrectly treated as file includes." No change needed. 3ba4859 narrowed the check to keys ending .path, and a local review then showed --config-env='includeIf.gitdir:[!=]**/.path=VAR' passing, since git splits --config-env at its last = while the guard reads the key up to the first, so the condition's own = cut the key short of .path. f31487e reverts to reading every includeIf. key as an include, adds a self-test case for that shape, and says why in the docstring. The cost is a deny on an includeIf key git ignores, on a subcommand git does not list.
4. Nit, _check_push_bypass, "Force/delete denial messages are inaccurate." Fixed in 3ba4859. The pull_request deny now reads "This push writes ''", since it fires for a force and a delete as well as an update.

The pull request body's own "Local Review" list, answered the same way:

• Sentence-length findings on new docstrings and README sentences: No change needed. sentence-length is not a gate here (OPERATIONS.md "Local Verification" runs no invocation of it), and the claude README bullet is one physical line, so the older sentences on it are flagged too.
• A fixed-prefix branch pattern, such as refs/heads/feature/*:refs/heads/feature/*, read as writing the protected defaults: Deferred to Scope a Branch Pattern's Push Operations to the Branches It Can Match in the Write Guard #2964, the same over-denial class as item 2.

[ptr727]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.