Skip to content

Approval gate: scope cd to subshells, read heredocs as the shell does; README timing wording - #298

Merged
yuanhao merged 1 commit into
mainfrom
fix/gate-scope-heredoc
Oct 11, 2026
Merged

yuanhao merged 1 commit into
mainfrom
fix/gate-scope-heredoc

Conversation

@yuanhao

@yuanhao yuanhao commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes from review of #297.

[P1] Subshell directory changes bypassed approval. With an exempt scratch folder, (cd scratch); rm -f owner.txt was allowed: the checker assumed the subshell's cd applied to the rm.

  • The tokenizer now marks ( … ), and the checker restores the directory after one.
  • A cd is followed only where it surely applies: a literal directory that exists, not in a pipeline, the background or before ||.
  • Anywhere else, later paths are unknown and ask.

[P1] Quoted heredoc text hid real commands. echo '<<EOF', then rm -f owner.txt on the next line, then EOF, was allowed: a regex pre-pass took the quoted text for a heredoc and skipped the deletion. Heredocs are now recognized inside the tokenizer:

  • only an unquoted <<WORD, <<-WORD or quoted delimiter starts one;
  • its body is skipped from the next line to the delimiter, as the shell does;
  • a here-string (<<<) is data.

Tests: new cases for both bugs and their variants:

  • a cd in a pipe, in the background, before ||, or to a missing folder;
  • a real heredoc followed by a real command;
  • a redirection on a heredoc's opening line;
  • the legitimate forms, which stay free.

Run against the gate on main, the new tests fail on exactly the two reported bypasses; with the fix, all pass. The pi dialog test and coding_agent's CI check also pass.

[P2] README timing wording. Removed "tools overlap the model's time". The loop awaits each model response and then that response's tools, so they don't overlap. One response's tool calls run concurrently, so their durations can't simply be added.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG

…edocs as the shell does

From review of #297:
- [P1] `(cd scratch); rm -f owner.txt` passed: the subshell's cd was taken
  to apply to the rm. The tokenizer now marks `( … )`, and the checker
  restores the directory after one. A cd is followed only where it surely
  applies: a literal existing directory, not in a pipeline, the background
  or before `||`; otherwise later paths are unknown and ask.
- [P1] `echo '<<EOF'` + a newline + `rm -f owner.txt` passed: a pre-pass
  regex took quoted text for a heredoc and skipped the rm. Heredocs are now
  recognized in the tokenizer (unquoted `<<WORD`, `<<-`, quoted
  delimiters), their bodies skipped from the next line to the delimiter.
- Tests for both and their variants; on the previous gate they fail on
  exactly the two reported bypasses.
- [P2] README: model and tools do not overlap (the loop awaits each); one
  response's tool calls run concurrently, so their durations do not add.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG
@yuanhao
yuanhao merged commit c8411a5 into main Oct 11, 2026
17 checks passed
@yuanhao
yuanhao deleted the fix/gate-scope-heredoc branch October 11, 2026 14:23
yuanhao added a commit that referenced this pull request Oct 11, 2026
…#299)

From review of #298: an unquoted heredoc body is expanded by the shell, so
`cat <<EOF` / `$(rm -f owner.txt)` / `EOF` ran the deletion without
approval. The same held anywhere a substitution runs: `echo $(rm …)`,
`"$(rm …)"`, backquotes, `<( … )` / `>( … )`, an assignment's value,
inside `$(( … ))`, and `eval`.

The tokenizer now collects what each command's substitutions run (also
inside double quotes and unquoted heredoc bodies; a quoted or escaped
delimiter keeps the body literal) and the checker reads it like any
command, in that command's directory. `eval` of literal text is read the
same way; of unknown text it asks. Everyday forms stay free: `$(date)`,
`$(git rev-parse HEAD) > rev.txt`, `$(( 1 + 2 ))`.

Tests: 22 cases; on the previous gate they fail on the reported heredoc.


Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant