Repository navigation
Conversation
Closes #3294 The smoke stub comment and its copy in docs/reusable-workflows.md said every enable_* input defaults true. enable_release_asset defaults false, so both now say every enable_* input except enable_release_asset defaults true. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## Summary Closes seven allow gaps in the write guard where it read git config or an alias other than the way git does. Each fix adds a denial or fails closed. None relaxes an existing denial. - **#3046.** Some commands may rewrite a push's remote config, and the guard reads that config before the command runs. They are a `git config` or `git remote` that does not only read, a git subcommand that may be an alias, and a `gh` command. A wrapper shell or a substitution running one counts too, and so does a shell whose command string the guard does not read. A push after one of them now reads its remote's `mirror` key and its checkout's origin as unresolved. A checkout or a switch changes neither, and neither does a `--get`, a `--list`, or a `remote -v`. - **#3047.** `_config_alias` reads through `_git_read` and answers `_UNRESOLVED` when the read times out or cannot run. Both alias walks fail closed on it where the name may be an alias. Exit 1 (not set) and other failures still read as no alias. - **#3055.** A `core.hooksPath` override is denied on any git command, whether set by `-c`, by `--config-env`, or in the environment. A `/dev/null` value holds no hook and is allowed, except on a commit, a push, or a subcommand that may be an alias, where it skips the hooks. An empty value is denied, since git then looks for hooks in the filesystem root (traced with strace on git 2.47). The `config --list` control row's earlier decision no longer stands, so it now expects a deny. A `/dev/null` control row replaces it. - **#3057.** A `HOME` or `XDG_CONFIG_HOME` other than `/dev/null` that reaches a git command is read as an unread config directory, as `GIT_CONFIG_GLOBAL` already is. That covers a prefix, an export, and a wrapper shell's prefix. The repository's own scripts and docs hold no shell-level `HOME=... git` shape. The tests that set `HOME` do it in a Python subprocess environment, which the hook never sees. - **#3072.** `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` are paired per git command, from the exports before it and its own prefix. The old pairing across the whole reading is kept too, so no value develop read is lost. - **#3078.** Rule 6 resolves a persisted alias from the repository git runs in, which a `--work-tree` does not move, and still from the work tree. A `GIT_DIR` or `GIT_WORK_TREE` on a wrapper shell's prefix, or exported before it, is carried into the invocations read from its body. - **#3081.** Alias expansions are read past their leading global options (`-p`, `-c`) in both walks, and a `-c alias.<name>=<value>` among them is followed. An include or a `--config-env` among them is denied. So is a `core.hooksPath` read as on a command line, and a command key whose program is unreadable or runs git. The local review raised three sibling gaps that develop shares. They are filed rather than widened into this change: #3303 (persisted config written earlier in the same command), #3304 (a direct git command reads fewer `GIT_DIR` forms than a wrapped one), and #3305 (an alias carrying `--no-verify`). The Copilot round's over-denial from applying one wrapper's `GIT_DIR` to every wrapper body is filed as #3309. ## Local Review Findings and Their Outcomes Each push's two-round local review budget was spent, so these went to this pull request's loop. Every one is `introduced` and now has an outcome. Listed at the first push (pass 3 on 69e26fe): 1. Backticks or `$(` inside single quotes read as a live substitution. **Fixed in 45f1d00.** 2. A one-argument `git config <key>` counted as a write. **Fixed in 45f1d00.** 3. `bash -c "$CMD"` before a push was not counted. **Fixed in 45f1d00** (and in bodies, 03f0e87). 4. The bodiless-shell check keyed on the shell word. **Fixed in 45f1d00 and d7bae25.** A script file or `make` run directly is the documented "a write inside a script file is unseen" limit. 5. The command-position check covered `gh` but not `git`. **Fixed in d7bae25.** 6. A README `So is` bound to the wrong clause. **Fixed in 45f1d00.** 7. The spec READMEs lacked the new shell and substitution forms. **Fixed in 45f1d00 and 03f0e87.** Raised by the passes on 45f1d00 and d7bae25, each **fixed in d7bae25 or 03f0e87**: - A shell given only options still reads its input. - Launchers such as `flock` or `watch` run their git word. - `gh` groups are now read past a repository flag with `_gh_path`. - Config write actions are read cut short. - An argument holding a run-time word, such as `mkdir -p "$D"`, runs nothing. - A substitution after a backgrounded, piped, or looped push is read. - A `gh` read such as `gh pr view` or `gh repo view` no longer counts. Raised by the pass on 03f0e87, and the design points from every pass: - A run-time word behind a launcher or `while`, an `eval` inside a body or substitution, a bodiless `xargs ... bash -c`, and the deny message naming the wrong cause. **Deferred to #3307.** Develop allows each of these, so none relaxes a develop denial. - One walker for `_config_may_change` and `_head_may_move`, one command-position helper, and one `core.hooksPath` predicate. **Deferred to #3308.** The config-change scan for #3046 drew new findings in every local pass and in the Copilot round. That recurring pattern is escalated to the maintainer with a recommendation in the drive's report, rather than answered by another narrowing round. Closes #3046 Closes #3047 Closes #3055 Closes #3057 Closes #3072 Closes #3078 Closes #3081 ## Verification - **Differential, develop versus branch.** Both guards ran over 4,681 commands. Those are every selftest table's commands plus constructed cases. Each ran in ten modes: origin, open rules, foreign origin, no origin, persisted aliases, primary checkout with aliases, alias reads timing out (plain and primary), and a real scratch repository with code-style and open rules. That is 46,810 verdicts at 03f0e87. **Deny to allow: 0. Allow to deny: 14,950. Errors: 0.** Every allow-to-deny flip falls into a class one of the seven fixes names. They are HOME/XDG, hooksPath, timed-out alias reads, per-command env pairs, alias expansions past options, rule-6 alias directories, and pushes after a config rewrite. - **Revert proof.** With the code hunks reverted and the new tests kept, all 56 new deny rows and the 3 timed-out-read checks fail, and no allow row does. Three of the four rows feeding the `_UNRESOLVED` sentinel through the `config_lookup` seam raise `AttributeError` on develop's walk, which takes only a string. The fourth is a control that never reads the alias. Every new control still passes. - **Gates.** The selftest passes. So do ruff check, ruff format, mypy, the agent-safety pytest suite, `tests/test_cli_usage.py`, `scripts/prose_lint.py --diff` (with and without `--check sentence-length`), and `spec/validate.py`. Pyright reports the same 69 errors as develop, message for message. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3310 +/- ##
=======================================
Coverage 65.49% 65.49%
=======================================
Files 17 17
Lines 10011 10011
=======================================
Hits 6557 6557
Misses 3454 3454
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:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate guard-command association issues remain unresolved.
1 open issue
What changed in this PR
Promotes write-guard hardening and smoke workflow documentation fixes to main.
Changes:
- Adds Git config, alias, hook-path, environment, and wrapper protections.
- Expands guard self-test coverage.
- Corrects
enable_release_assetdefault documentation.
| File | Summary |
|---|---|
host-setup/agent-safety/README.md |
Documents guard behavior. |
host-setup/agent-safety/claude/README.md |
Summarizes guard updates. |
host-setup/agent-safety/claude/gh-write-guard.py |
Implements protections and tests. |
docs/reusable-workflows.md |
Corrects smoke input documentation. |
catalog/snippets/workflows/test-pull-request-smoke.yml |
Corrects the smoke stub comment. |
Review effort: Lite
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.
Summary
Promotes these pull requests from develop.
enable_*default claim, sinceenable_release_assetdefaults false.Closes #3046
Closes #3047
Closes #3055
Closes #3057
Closes #3072
Closes #3078
Closes #3081
Closes #3294
🤖 Generated with Claude Code