Repository navigation
Keep Escaped Backslash Pairs Before a Line Break in the Write Guard - #3368
Conversation
A shell running a kept quoted span reads each escaped backslash pair before a line break as one literal backslash, so the word keeps it. Every reading dropped the whole run, which read a push of a word ending in a backslash as a push of the word without it. The readings now keep the pairs, parted from the line break by a space so no reading holds a backslash-newline to read again. Also correct the README and the _literal_span_kept docstring, which understated the word test and the names test, and read as if every kept span is read as its own scan places it. Refs #3358 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The docstring said no reading holds a backslash-newline, but a joined odd run followed by an empty line leaves its pairs before a newline, which classify reads again and denies as the shell would. State only that the break a reading adds is not read again, and add controls for an escaped pair ending a comment in a shell body. Refs #3358 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
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 |
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified, and the documented behavior and regression coverage are aligned.
No open issues 🎉
What changed in this PR
This pull request updates the write guard to preserve escaped backslash pairs when rereading kept quoted shell text, while aligning documentation and regression coverage.
Changes:
- Preserve escaped backslash pairs across generated line-break readings.
- Add allow and deny self-tests for shell bodies and quoted heredocs.
- Clarify the README requirement and
_literal_span_keptdocumentation.
| File | Description |
|---|---|
host-setup/agent-safety/README.md |
Documents the expanded kept-span, command-name, and escaped-pair rules. |
host-setup/agent-safety/claude/gh-write-guard.py |
Updates line-break readings, documentation, and regression/control test cases. |
Review effort: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3368 +/- ##
==========================================
Coverage ? 81.32%
==========================================
Files ? 18
Lines ? 10047
Branches ? 0
==========================================
Hits ? 8171
Misses ? 1876
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:
|
|
@coderabbitai review |
|
…dows Self-Test Skip, and Subprocess Failure Rule to Main (#3359) ## Summary - Skip the invalid-byte audit self-test probe when Windows Git cannot add the name, so the self-test no longer fails on a host that cannot represent the path (#3350). The skip fires only on that one add, after a control file's add has proven git works (#3360). - Read a backslash-newline in the write guard by its quoting: inside single quotes and quoted heredoc data it is text, and a continuation after a shell operator is no word split (#3357). - Keep each escaped backslash pair before a kept line break in the guard's readings, as the shell does, and state the kept-span word and names tests in full in the README and docstring (#3368). - Add a `CODESTYLE.md` General rule: classify a subprocess failure by which command failed and the status code it reports, never by the wording of its message (#3366). Closes #3338 Closes #3358 Closes #3363 Refs #3344, which stays open because only part of it is fixed here. #3358's Markdown-body finding moves to #3354. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
Fixes the first three findings of #3358 in the write guard's backslash-newline narrowing.
_line_break_readingbuilt dropped the whole run instead, sobash -c 'git push origin main\\<newline>echo hi'was read as a push ofmain, where the inner shell pushesmain\. The readings now keep the pairs as script text, so a shell reading them still sees one literal backslash per pair. A space parts the pairs from the line break, so no reading holds a backslash-newline thatclassifywould read again. The odd-run line-break reading keeps its pairs the same way. Outside a kept span nothing changes, and an escaped backslash before a line break is still denied there._literal_span_keptdocstring. The summary line no longer reads as if every kept span is read as its own scan places it. A span naming no git, gh, or push is kept whatever its scan found, and the word test now names an escaped part and an escaped word break before the word.Out of scope: #3358's fourth finding, a Markdown body naming git or gh across a further quoting level. It goes to #3354's single context walker and is not attempted here.
Verification
python3 host-setup/agent-safety/claude/gh-write-guard.py --selftest:SELFTEST PASS, with new_CASESentries: four allows, plus control denies for the line after a pair and for a pair ending a comment in a shell body._line_break_readingchange reverted and the new cases kept, the self-test fails on exactly the four new allow cases (denywhereallowis wanted). The control denies pass either way.classifyre-reads and denies correctly) and a missing comment-ending test. Both are fixed in the second commit, and pass 2 raised nothing.prose_lint.py --diff(default and--check sentence-length), the eol gate, and markdownlint on the README are clean.Develop-versus-branch differential
Every command classified by develop (
075ec57a) and by this branch with the self-test's pins:The probes are 40 templates, each with a backslash run of 0 to 6 before a newline. They cover even and odd runs, outside quotes, inside a single-quoted
bash -c/sh -cbody, inside a double-quoted body, in a quoted heredoc fed tobash, in a$(...)substitution, ingh pr comment/gh pr createbodies, and next to git, gh, push,cd,-C,--no-verify,--force, and a comment.Every flip is intended. Each has a run of 2 to 6 backslashes, so at least one escaped pair, before the newline in a kept span:
bash -c 'git push origin main<run><newline>echo hi',bash -c 'git push origin main<run><newline>', the same in a quoted heredoc fed tobash, and insideecho "$(...)"(20 flips). The inner shell's refspec ismain\(even run, followed by a line break) ormain\echo(odd run, joined). A ref name cannot hold a backslash, so git refuses either and no push ofmainruns.bash -c 'git push origin HEAD:main<run><newline>'(5 flips). The destination ismain\, which git refuses for the same reason.bash -c 'gh<run><newline>pr merge 5 --admin'(5 flips). The inner shell runs a command namedgh\(even run) orgh\pr(odd run joined), not gh.The single-backslash run (a plain continuation) and the plain newline flip nothing, and the control shapes that must stay denied, such as a later line pushing
main, acdbefore a push,-Csplit frompush, and--no-verifyon the next line, stay denied.Refs #3358
🤖 Generated with Claude Code