Repository navigation
Read a Bare Push's Destination From Branch Config, a Persisted Mirror, and Its Own Checkout in the Write Guard - #3049
Conversation
…ing in the Write Guard A bare push now also reads its destination from push.default and branch.<name>.merge, reads a persisted remote.<name>.mirror, and is unresolved when a branch or push-target read times out or cannot run. A dash-named word after the first -- or --end-of-options is also read as a positional. A push that -C or a leading cd runs in another checkout is also checked against that checkout's origin, and one whose checkout the guard cannot name counts as having undetermined rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…out Reading A push's checkout is now read only from a directory-moving word before it, so a timeout prefix, a run-time word elsewhere, or a later cd no longer marks its repository unnamed. Each push's targets are checked against its own checkout's origin rather than every checkout in the command. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The moved-checkout reading now covers an eval, source, or . after a shell keyword, a cd a loop or function may run before the push, a git directory variable anywhere before it, and git run by a command not known to keep its directory. The origin URL read keeps develop's strict decoding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
|
A recorded local strict-review pass covers head |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3049 +/- ##
==========================================
Coverage ? 61.21%
==========================================
Files ? 17
Lines ? 9062
Branches ? 0
==========================================
Hits ? 5547
Misses ? 3515
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Fix repository-move detection and quoting handling, and correct the contradictory timeout documentation.
2 open findings
What changed in this PR
This PR hardens the Git push write guard by detecting configured destinations, persisted mirrors, failed reads, option-boundary refspecs, and alternate checkouts.
Changes:
- Expands push destination and mirror detection.
- Fails closed on unresolved metadata and checkout paths.
- Updates documentation and self-tests.
| File | Summary |
|---|---|
host-setup/agent-safety/README.md |
Documents expanded push detection behavior. |
host-setup/agent-safety/claude/README.md |
Updates Claude hook behavior documentation. |
host-setup/agent-safety/claude/gh-write-guard.py |
Implements push analysis, checkout resolution, and tests. |
🧠 Review effort: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ncher Options A cd written after a push now moves its checkout only where a loop or a function may run it first, so a pipe or a background job no longer does. The launcher test skips options, numbers, and redirections, and keeps setsid, gtimeout, time, flock, and winpty as launchers that stay put. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A later cd now moves a push's checkout only where a loop or a function definition may run it first, so a subshell or a command substitution after the push no longer counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A definition written with its brace on the next line, or with a subshell body, lexes its () fused with the next operator, so that token now counts as a definition too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
… Narrative Findings, and Bootstrap Handoff Tests to Main (#3051) - #3042 brings the agent-safety READMEs up to the write guard's #3023 and #3037 readings: `--prune` beside a pattern, `:`, `--all`, or a bare push, a `--mirror` push with no refspec, a `heads/` destination outside `refs/`, the full `@{push}` read, and a bare push denied when `git remote` cannot list the remotes - #3041 has `backlog-burndown` keep an abandoned group's claim comment until cleanup decides its release, say stopped on each path that sends a group to the maintainer, and name a branch no pull request has used when a claim comment first names it - #3044 tests `bootstrap.sh`'s `--report` handoff to the agent tools step, tests the `--agents` handoff on Linux and through `Invoke-AgentsInstall` under `pwsh`, and moves the unprivileged-root probe into `tests/host_capability.py` - #3045 makes `pr_review.py` read an `Open findings remain:` list in a Copilot review and count each entry no thread of the round accounts for, pairing by severity tier and file, so the count can only rise - #3049 makes `gh-write-guard` read a bare push's destination from `push.default` and `branch.<name>.merge`, read a persisted `remote.<name>.mirror`, deny when the `@{push}` read times out or cannot run, give a dash-named word after `--` or `--end-of-options` a reading as a remote or refspec, and judge a push moved into another checkout against that checkout's origin as well. A differential against the previous guard found no deny-to-allow flip. It addresses one item of #2943, which stays open - #3054 makes `gh-write-guard` answer unresolved, and so deny, when its `push.default` read times out or cannot run, a promotion fix for this pull request's first review - #3062 brings both agent-safety READMEs up to #3054: an unset `push.default` is read as `simple`, both destinations are read for a value git does not define or a read exiting with any other error, and a read that times out or cannot run is denied - #3059 bumps `actions/create-github-app-token` from 3.2.0 to 3.3.0 After this merges, the host-safety installer payload changes, so the maintainer runs the installer after promotion. Fixes #3021 Fixes #3026 Fixes #3027 Fixes #3039 Fixes #3033 Fixes #3038 Fixes #3034 Fixes #3031 Fixes #3019 Fixes #3020 Fixes #3029 Fixes #3036 Fixes #3052 🤖 Generated with [Claude Code](https://claude.com/claude-code)


Summary
Closes the write guard's bare-push and
@{push}allow gaps, plus one #2943 item. Every changed verdict goes from allow to deny._current_push_branchkeeps the@{push}read and adds_configured_push_branches. That function readspush.defaultandbranch.<name>.merge.currentandsimplegive the branch's own name, andupstreamandtrackinggive each merge branch. Apush.defaultthat cannot be read, or holds a value git does not define, gives both. The two readings are combined, so a fetch refspec that maps underrefs/remotes/origin/mirror/*, or a missing remote-tracking ref, no longer hidesdevelop._remote_mirrorsreadsremote.<name>.mirrorfor a push with no refspec and no--mirror, in the directory the push runs in. It reads the named remote's key, or every remote's key when the push names none. A true value, a value that is not a boolean, or a read that cannot answer adds amirrorop on each protected default. That op denies where the rules forbid a force, a delete, or a direct write._git_readreturns the exit status beside the output, and_git_stdoutnow wraps it. In_read_branchand the branch config reads, a read that times out or cannot run is unresolved, which denies. Only a non-zero exit still means "not there".--or--end-of-optionsis read a second time, with every word after that boundary taken as the remote or a refspec. The first reading is unchanged, so a word there that develop read as an option still counts._push_targetsnow returns(op, branch, repo). The repo is the origin of the checkout a-Cor leadingcdmoves the push to, from the new_push_repo._check_push_bypasschecks each target against the session origin, as before, and also against its own push's checkout. A checkout or origin the guard cannot name counts as having undetermined rules, which denies onlymain,master, anddevelop. That covers a missing directory,sudo -D,env -C, a wrapper shell, a git directory variable, a latercda loop or a function may run first, and git run by a command not in_STAYING_LAUNCHERS. That reading is_repo_may_move. A command that may moveHEAD, such as a checkout, does not count, since it changes the branch and not the repository. Only that Close the Write Guard's Remaining Push Detection Gaps Found Reviewing the #2936 Fix #2943 item is addressed. Every other Close the Write Guard's Remaining Push Detection Gaps Found Reviewing the #2936 Fix #2943 item stays open, and so does Close the Write Guard's Remaining Push Detection Gaps Found Reviewing the #2936 Fix #2943.The READMEs describe each new reading.
Verification
mktemp -dwithgit push --dry-run --porcelain(git 2.47.3), and develop'sclassifyallowed every one. The probes were the mirror-namespace fetch refspec and the single-branch clone (git wrotedevelopboth times), a persisted mirror (git deleted a remote-only branch, and with--tagsit deleteddevelop), a timed-out@{push}read,--x:refs/heads/developafter--and after--end-of-options, and-C,cd,sudo -D, and wrapped pushes into a checkout whose origin requires a pull request.classifyran over the same inputs: every develop_GIT_CASESrow (301), 99,603 generated push shapes over seven constructed repositories and three rule maps, the 133_PUSH_DIR_CASES, and 189 runs of extra shapes. 0 deny -> allow, 13,007 allow -> deny. By mechanism, those are 5,138 pushes into a missing directory, 2,569 each undersudo -D,env -C, and a wrapper shell, 146 in a checkout with a persisted mirror, and 16 dash-named refspecs after the boundary. A plain push, atimeoutprefix, a run-time word elsewhere, a latercd,git tag/git commitbefore a push, a glob refspec, and a checkout trackingorigin/developunderpush.defaultunset orcurrenteach kept develop's verdict._check_push_bypass,_push_targets,_current_push_branch,_read_branch, and_git_stdoutswapped into the branch module, all 19 new deny cases in the selftest fail and every control passes.gh-write-guard.py --selftestpasses, and passes too with an emptyPATH, as the installer's no-git test runs it.test_install.pypasses (161 tests), and so does the roottests/suite (2,813 tests). Ruff format and check, mypy,prose_lint.py, the eol and eol-coverage gates,spec/validate.py, and markdownlint, editorconfig-checker, and cspell in Docker pass too.Local Review
Frontier
local-strict-reviewpasses ran before every push. These findings were fixed:timeoutprefix, or a latercdhad marked a push's checkout unnamed. That over-denied, so_repo_may_movereplaced it.The opening push listed four
introducedfindings from its final pass. Three are fixed on later pushes, and the fourth is filed:cdnow moves a push's checkout only where a loop or a function definition may run it first, so a pipe, a background job, or a subshell after the push no longer counts.setsid,gtimeout,time,flock, andwinptykeep their directory._repo_may_movedocstring namesuvas its moving example.Filed rather than widened here:
git configorgit remotecommand before the push can rewrite the config the guard reads.originis judged against the origin's rules.remote.<name>.pushandpush.default=matchingare already Close the Write Guard's Remaining Push Detection Gaps Found Reviewing the #2936 Fix #2943 items.Partially addressed: #2943, its item on the directory a push runs in only.
Closes on promotion: #3019, #3020, #3029, #3036
🤖 Generated with Claude Code