End the Guard's Stdin Redirect Scan at a Reserved Word

[ptr727]

Summary

_redirects_stdin in host-setup/agent-safety/claude/gh-write-guard.py scanned the tokens after a loop's done up to the first separator only. A redirect on the next command inside an enclosing compound therefore read as the loop's own input bound:

if while read l; do sleep 30; done then echo x < f; fi

The < f binds the echo, and the loop reads the caller's stdin with nothing bounding it, yet the guard allowed it. The else form did the same.

The scan now ends at any bash reserved word, the same way it ends at a separator. That includes the closing words }, fi, done, and esac. A redirect after one of those binds the compound it closes, and a pipe inside that compound can still feed the loop, so { yes | while read l; do sleep 30; done } < f runs forever. Develop allowed it, and this change denies it.

One behavior change is in the safe direction: { while read l; do sleep 30; done } < f is now denied, a false deny of the kind _reads_its_input's docstring already accepts. while read l; do sleep 30; done < f stays allowed.

Verification

• python3 host-setup/agent-safety/claude/gh-write-guard.py --selftest passes. Four new _WAIT_CASES rows pin the then, else, closing-word, and piped-group forms.
• With the reserved-word check disabled, all four new rows fail.
• Each shape was checked in bash first. A reserved word after a redirect target is a bash syntax error, so a closing word can only come directly after done.
• ruff format, ruff check, mypy, the prose gate, and the eol gate are clean.
• Two local-strict-review passes ran. The first found the piped-group false allow described above, which the second commit fixes. The second found a pre-existing false allow from a redirect inside a trailing comment (done # < f). That one is filed as The Guard's Stdin Redirect Scan Credits a Redirect Inside a Trailing Comment #2152 and is out of this change's scope.

Closes on promotion: #2032

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Corrected how shell input redirections are interpreted after control-flow boundaries, preventing later commands from being treated as changing a loop’s input.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 0b2a7dd5-f241-418c-aaff-01d163812f51

📥 Commits

Reviewing files that changed from the base of the PR and between 2a028d0 and 71d285f.

📒 Files selected for processing (1)

• 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

Walkthrough

The loop stdin redirection scan now stops at shell reserved words as well as separators. Added self-tests cover redirects after then, else, and a closing }.

Changes

Loop stdin redirection guard

Layer / File(s)	Summary
Reserved-word boundary and validation host-setup/agent-safety/claude/gh-write-guard.py	The scanner now stops at shell reserved words when checking loop stdin redirection. Self-tests cover redirects after then, else, and a closing }.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Merge Risk: ⚪ Minimal · up to 71d28

The changed boundary prevents redirects in later commands from affecting the loop’s stdin classification. No actionable merge risk was identified in the reviewed change.

Architecture Summary

Architecture risk: 🔵 Low · up to 71d28

The change affects 1 system.

Changed systems: host-setup

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — host-setup (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in host-setup/agent-safety/claude/gh-write-guard.py: Adds _RESERVED_WORDS, a set of shell reserved words used to identify boundaries while scanning tokens after a loop.
• observed — Modified behavior in host-setup/agent-safety/claude/gh-write-guard.py: Adds documentation that a loop command ends at reserved words, including closing words such as }, when determining whether a later input redirection bounds the loop.
• observed — Modified behavior in host-setup/agent-safety/claude/gh-write-guard.py: _redirects_stdin now returns its accumulated result when it encounters a reserved word, in addition to its existing separator boundary. Later redirects therefore do not alter the loop’s stdin-bound determination.
• observed — Modified behavior in host-setup/agent-safety/claude/gh-write-guard.py: Adds self-test cases expecting denial when apparent input redirects follow then, else, or a closing } after a sleeping read loop.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: ending the guard's stdin redirect scan at shell reserved words.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	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

---

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.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@2a028d0). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff             @@
##             develop    #2153   +/-   ##
==========================================
  Coverage           ?   56.47%           
==========================================
  Files              ?       16           
  Lines              ?     7455           
  Branches           ?        0           
==========================================
  Hits               ?     4210           
  Misses             ?     3245           
  Partials           ?        0           

Flag	Coverage Δ
python-3.13	56.47% <ø> (?)

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.

[Copilot]

Copilot review overview

🟢 Approval recommended

The change strictly tightens the guard in the safe direction, is well-reasoned, and is covered by four new selftest rows whose command shapes I verified as valid bash.

Review effort: Balanced
Findings: None

What changed in this PR

This PR fixes a false-allow in the agent-safety write guard's unbounded-wait-loop detector. _redirects_stdin in host-setup/agent-safety/claude/gh-write-guard.py scans the tokens after a loop's done to decide whether descriptor 0 is bound to a source that ends (which would make a read-driven loop terminate). Previously the scan only stopped at a shell separator, so a redirect belonging to a later command inside an enclosing compound (e.g. after then, else, fi, or }) was mis-credited as the loop's own input bound, letting an unbounded loop through. The scan now also ends at any bash reserved word, closing that gap.

Changes:

• Add a complete _RESERVED_WORDS frozenset (the 22 bash reserved words) and end the _redirects_stdin scan when a reserved word is reached, mirroring the existing separator stop.
• Extend the _redirects_stdin docstring to explain the reserved-word boundary, including the closing-word case where a pipe inside the compound can still feed the loop.
• Add four _WAIT_CASES selftest rows pinning the then, else, closing-word, and piped-group shapes.

File	Description
host-setup/​agent-safety/​claude/​gh-write-guard.py	Adds _RESERVED_WORDS, ends the stdin-redirect scan at a reserved word, documents the behavior, and adds four selftest cases covering the newly denied shapes.

I reviewed the logic and confirmed the change only returns early (adding deny conditions) and therefore cannot introduce a new false-allow: bound is only true when a valid loop-binding redirect already appeared immediately after done, before any reserved word. The reserved-word set matches bash's 22 reserved words exactly, and reserved words appearing as redirect targets are consumed as targets before the top-of-loop check, so done < do still binds correctly. I also verified with bash -n that all four new test-case command strings are valid bash. The two sibling defects (process-substitution scan and trailing-comment scan) are correctly scoped out with issues filed.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[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.