Repository navigation
Read a Timed-Out push.default Read as Unresolved in the Write Guard - #3054
Merged
Merged
Conversation
A push.default read that timed out or could not run fell through to the read-as-both branch, so a bare push under a real matching mode could be allowed. It now returns unresolved, which denies, as the docstring states for every other read in the function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
|
A recorded local strict-review pass covers head |
|
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 |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified, and verification passes.
0 open findings
What changed in this PR
Updates the write guard to fail closed when push.default cannot be read.
Changes:
- Returns
_UNRESOLVEDfor timed-out or unavailable reads. - Documents the behavior.
- Adds a timeout regression self-test.
| File | Summary |
|---|---|
host-setup/agent-safety/claude/gh-write-guard.py |
Updates push-branch resolution and timeout coverage. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3054 +/- ##
========================================
Coverage 61.21% 61.21%
========================================
Files 17 17
Lines 9062 9062
========================================
Hits 5547 5547
Misses 3515 3515
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:
|
ptr727
added a commit
that referenced
this pull request
Oct 15, 2026
…READMEs (#3062) A promotion fix for #3051. Since #3054, the write guard answers unresolved, and so denies, when its `push.default` read times out or cannot run. Both agent-safety READMEs still said such a read is read as both destinations. They now say: - an unset `push.default` is read as `simple` - both destinations are read where `push.default` holds a value git does not define, or where its read exits with any other error - a bare push is denied where any read of its branch, its push target, or `push.default` times out or cannot run This answers the two README threads on #3051 and fixes #3052, the issue filed for that drift. The `sentence-length` hits that `prose_lint.py --diff` reports on `host-setup/agent-safety/claude/README.md:15` are sentences that were already there. That bullet is one physical line, so any change to it brings every sentence in it into scope. Neither new sentence exceeds the limit. Two local strict review passes ran at Opus. Pass 1 raised three findings. Two, a timed-out read still covered by the "read as both" sentence and the unset case left unstated, are fixed. The third, the guard folding the value's case, is pre-existing and filed as #3061. Pass 2 raised nothing. Closes on promotion: #3052 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
ptr727
added a commit
that referenced
this pull request
Oct 15, 2026
… 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)
ptr727
added a commit
that referenced
this pull request
Oct 15, 2026
…s in the Write Guard (#3058) ## Summary Closes the write guard's hidden-command allow gaps. Every changed verdict goes from allow to deny. - **#3010, past the wrapper cap.** `_check_bypass_flags` denies a body nested past `_WRAPPER_NESTING_LIMIT` that names a git commit or push. Its flags cannot be read, so a `--no-verify`, a `commit -n`, or a hooks-path override there no longer passes. The same commit two levels deep is still read and judged. - **#2989 and #2999, substitutions the guard skipped.** `_substitution_bodies` reads a `$(...)` or backtick substitution inside a double-quoted `${...}` (`_expansion_substitutions`). It reads one inside a heredoc body whose delimiter is unquoted (`_unquoted_heredoc_bodies`), at top level and inside a substitution's body. A body under a quoted delimiter, such as `<<'EOF'`, stays data. - **#2972, config keys that run a command.** A `-c` value for a key git runs as a command, such as `core.pager`, `core.fsmonitor`, `core.sshCommand`, or a `diff.<driver>.textconv`, is read as the command it runs. A value naming its program by a path or an expansion is denied, since the guard does not read that program. So is a `--config-env` value for such a key. An `include.path`/`includeIf` override is now denied on any git command, the direction the issue named as the simpler one. - **#2943, partly: other shells, stdin, editor and config environment, and aliases.** - A command string run by `pwsh`, `powershell`, `fish`, `script`, `cmd`, `ash`, `csh`, `tcsh`, or `rbash` is read as a wrapper body, only where that shell is its command's command (see `_command_prefix`). That covers `-Command`, `--command=`, `cmd /c` and `cmd //c`, and a Windows PowerShell positional command. - A push in interpreter code that can start a process, such as `python3 -c "...os.system(...)"` or `node -e`, is denied. Code that only mentions a push, such as a `perl -pi -e` substitution, is allowed. - A push in text piped to a shell that reads its script from that pipe is denied. - `GIT_EDITOR`, `GIT_SEQUENCE_EDITOR`, `GIT_PAGER`, `GIT_SSH_COMMAND`, and the other variables git runs as a command are read as commands, with the same path rule. They count where they reach a git command, through `export` or in that command's own prefix. `_command_prefix` reads a prefix forward, past a redirection and a precommand such as `timeout` or `sudo` with its options and their values. - Config set through `GIT_CONFIG_COUNT` and its key and value variables counts as a `-c` override. A `GIT_CONFIG_GLOBAL` or `GIT_CONFIG_SYSTEM` file other than `/dev/null` is read as an include. - A `--config-env` alias for the subcommand it runs is denied. So is a push run by an alias set in the environment, by an inline alias chaining into the persisted config, or by a persisted alias (`_hidden_command`). A persisted alias is read only for a word shaped as an alias name that git does not run ahead of one. The selftest pins persisted aliases to an empty table, as it already pins git's command list, so no row reads the host's own aliases. The READMEs describe each new reading. ## Verification - **Gaps confirmed at develop's tip.** A `classify` probe on constructed commands, against a throwaway repository under `mktemp -d`, showed develop allowing every shape the issues name, each listed in the new selftest rows. - **Develop-versus-branch differential**, against develop at `2b0975ae` (after #3054, #3059, and #3062, whose guard is unchanged from #3054). Both guards' `classify` ran over the same inputs: every develop `_GIT_CASES` row with every new hidden-command row (387), the 7 alias rows, 113,784 generated push shapes over eight constructed repositories and three rule maps, the 140 `_PUSH_DIR_CASES`, and 1,944 runs of 81 extra common and edge shapes over the same repositories and maps, 116,262 inputs in all. **0 deny -> allow, 114 allow -> deny.** Every flip is a new selftest row, one of the two `include` rows this PR relabels from allow to deny for #2972, or one of two extra shapes: `powershell -Command` pushing to `develop` and a push piped to `bash`. The generated push shapes kept every develop verdict. - **Revert proof.** With develop's `_skip_quote_body`, `_substitution_bodies`, `_embedded_wrapper_commands`, `_check_bypass_flags`, and `_unreadable_push` swapped into the branch module and `_hidden_command` disabled, 64 of the 66 new deny rows fail and all 29 controls pass. The other two are regression rows develop already denies, kept to guard the flip a review found. - `gh-write-guard.py --selftest` passes, and passes too with an empty `PATH`. `test_install.py` passes (161 tests), and so does the root `tests/` suite (2,838 tests). Ruff format and check, mypy, `prose_lint.py`, the eol and eol-coverage gates, `spec/validate.py`, and markdownlint and editorconfig-checker in Docker pass. ## Local Review Three frontier `local-strict-review` passes ran before the push. Fixed from them: - A deny-to-allow flip. A word naming `cmd`, `script`, or `fish` made the wrapper scan skip the arguments after it, so a later `sh -c` body went unread. A foreign shell is now read only where it opens a command, and the scan continues past it. - Over-denials: a shell name used as a piped command's argument, a `NAME=value` word in a commit message or heredoc data, and interpreter code that only mentions a push. - An inline alias chaining to a persisted push alias, `fish --command=`, a PowerShell positional command, `|&`, a precommand between the assignments and git, `cmd //c`, the `py` launcher, an empty `GIT_CONFIG_GLOBAL`, and a control row whose label its command did not test. - The selftest read the host's real git aliases. Declined, with evidence: - An alias defined earlier in the same command, `git config alias.pp push && git pp origin develop`, is the #3046 class. - A timed-out persisted alias read reads as no alias, which is #3047. - The pipeline walk in `_heredoc_openers_in` treats `|&` as ending a pipeline. Teaching `_ends_pipeline` otherwise changes the heredoc and wait rules, and no verdict differs. - `_PINNED_ALIASES` follows the existing `_GIT_COMMAND_NAMES` pin. A seam would touch about nineteen selftest loops. - The overflow push check in `_unreadable_push` stays beside the new one, since it also covers include keys and `eval` overflow. The third pass's three `introduced` findings came after the opening push's two-round edit budget. They and both Copilot threads were fixed in the review loop's next push, under three more frontier passes: - An assignment is read only in the prefix of the git command it reaches, so `echo GIT_CONFIG_GLOBAL=/example/x -n git` is allowed. Config set on an earlier `git status` no longer counts on a later push. - A precommand option's separate value, short or long, is passed, as in `timeout -s KILL 60`, `sudo --user root`, or `xargs --delimiter ,`. A leading redirection is passed too. - A foreign shell behind an option-taking launcher, such as `sudo -u root pwsh -Command`, is read, and a shell name a `grep` behind `timeout` searches for is not. That push's last pass found `--eof` among the xargs options taking a separate value, though GNU xargs takes its value attached. The push after it drops `--eof`, and then `--max-lines` for the same reason, and adds sudo's `-a` and `-c`. Its last pass, the eighth, found nothing new and no deny-to-allow flip over 8,000 fuzzed commands. No `introduced` finding is open. Filed rather than widened here: - #3055: a hooks-path override on a git command other than commit or push. - #3056: a PowerShell `-EncodedCommand` body. - #3057: a `HOME` or `XDG_CONFIG_HOME` override on a git command. Partially addressed: #2943, its items on non-POSIX shells, stdin scripts, editor and config environment, and aliases. Its other items stay open, and so does #2943. Closes on promotion: #3010, #2972, #2999, #2989 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Answers the Copilot finding on the develop -> main promotion PR #3051 at
gh-write-guard.py's_configured_push_branches.push.defaultread that times out or cannot run now returns_UNRESOLVED, so a baregit pushdenies with the "Could not tell which branch" reason. An unsetpush.defaultstayssimple, and an unlisted value or another error exit stays the "read as both" reading._push_read_failure_selftestgains apush.defaulttimeout case expecting the unresolved deny. Restoring the old line fails only that case, since the old reading denied there for the protected-branch reason instead.Verification:
gh-write-guard.py --selftestpasses, and passes with an emptyPATH.test_install.pypasses, and so do ruff, mypy, the prose gate, the eol gates, andspec/validate.py. A differential of develop against this branch ranclassifyover_GIT_CASES,_PUSH_DIR_CASES, and generated bare and explicit push shapes under eachpush.defaultmode, each upstream, and each single timed-out read, 2239 cases in all. It found 0 deny-to-allow flips and 84 allow-to-deny flips. All 84 are under a timed-outpush.defaultread.Local review findings, each with an outcome:
introduced: both agent-safety READMEs still say apush.defaultthat cannot be read is read as both, and that only "any other read" timing out denies. Another lane holds those READMEs, so this is filed as State the Timed-Out push.default Deny in the Agent-Safety READMEs #3052 rather than edited here.introduced, declined: the reviewer asked whether the early return is over-hardening, since the old "both" reading never missed a protected branch the defined modes write. The function's docstring already said "A read that times out or cannot run is_UNRESOLVED", and_read_branchtreats every timed-out read the same way. The only cost is a fail-closed deny on a timed-out local config read.pre-existing: a bare push underpush.default=matchingis read as writing only the current branch, filed as Read a Bare Push Under push.default=matching as Updating Every Matching Branch in the Write Guard #3053.Closes on promotion:
🤖 Generated with Claude Code