fix(tools): strip heredoc bodies before background-'&' detection
_strip_quotes documented that it stripped heredoc bodies but only handled single/double/backtick quotes. As a result _foreground_background_guidance scanned heredoc body text for a backgrounding '&' and wrongly rejected valid foreground commands whose heredoc body contained a spaced ampersand — e.g. AppleScript string concat (osascript <<'EOF' ... "a" & b ... EOF), Python bitwise-and, or literal UI text like 'FaceTime & Privacy'. Add a _strip_heredocs pass (runs before quote-stripping, since a heredoc delimiter may itself be quoted) covering <<EOF, <<-EOF, <<'EOF', <<"EOF". The same-line tail after the opener (redirects/args) is preserved and the opener token is blanked so a real backgrounding '&' after the heredoc is still detected. Adds tests/tools/test_terminal_heredoc_background_guard.py.
This commit is contained in:
parent
17688f994e
commit
2bfdd8cd34
|
|
@ -0,0 +1,97 @@
|
|||
"""Regression tests for heredoc-aware background-'&' detection.
|
||||
|
||||
Context: ``_foreground_background_guidance`` blocks a foreground command that
|
||||
looks like it backgrounds a process with ``&`` (so the agent is nudged toward
|
||||
``terminal(background=true)``). Before scanning, it calls ``_strip_quotes`` to
|
||||
blank out quoted content so an ``&`` *inside a string* isn't mistaken for the
|
||||
shell background operator.
|
||||
|
||||
Bug: ``_strip_quotes`` documented that it strips "heredoc-style inline text"
|
||||
but only stripped single/double/backtick quotes — it had no heredoc handling.
|
||||
So a foreground command carrying a heredoc whose BODY contains a spaced ``&``
|
||||
was wrongly rejected. Real-world triggers:
|
||||
|
||||
- ``osascript <<'EOF' ... set x to "a" & b ... EOF`` (AppleScript concat)
|
||||
- ``python3 <<'EOF' ... z = a & b ... EOF`` (Python bitwise-and)
|
||||
- a heredoc body containing literal UI text like ``FaceTime & Privacy``
|
||||
|
||||
The fix strips heredoc bodies (``<<EOF``, ``<<-EOF``, ``<<'EOF'``, ``<<"EOF"``)
|
||||
before the ``&`` scan, so payload ampersands are ignored while a *real*
|
||||
backgrounding ``&`` at the shell level is still caught.
|
||||
"""
|
||||
|
||||
from tools.terminal_tool import (
|
||||
_foreground_background_guidance as guidance,
|
||||
_strip_quotes,
|
||||
)
|
||||
|
||||
# Build commands without a literal '&' in this source where convenient, so the
|
||||
# test file itself never trips a naive scanner. AMP is just an ampersand.
|
||||
AMP = chr(38)
|
||||
NL = chr(10)
|
||||
|
||||
|
||||
class TestHeredocBodyAmpersandAllowed:
|
||||
"""A spaced '&' inside a heredoc body is payload, not backgrounding."""
|
||||
|
||||
def test_applescript_string_concat(self):
|
||||
cmd = (
|
||||
"osascript <<'EOF'" + NL
|
||||
+ 'set out to "count " ' + AMP + " (count of items)" + NL
|
||||
+ "EOF"
|
||||
)
|
||||
assert guidance(cmd) is None
|
||||
|
||||
def test_python_bitwise_and(self):
|
||||
cmd = "python3 <<'EOF'" + NL + "z = a " + AMP + " b" + NL + "print(z)" + NL + "EOF"
|
||||
assert guidance(cmd) is None
|
||||
|
||||
def test_unquoted_delimiter(self):
|
||||
cmd = "cat <<EOF" + NL + "foo " + AMP + " bar" + NL + "EOF"
|
||||
assert guidance(cmd) is None
|
||||
|
||||
def test_double_quoted_delimiter(self):
|
||||
cmd = 'cat <<"EOF"' + NL + "foo " + AMP + " bar" + NL + "EOF"
|
||||
assert guidance(cmd) is None
|
||||
|
||||
def test_dash_delimiter_indented_close(self):
|
||||
cmd = "cat <<-EOF" + NL + "\tfoo " + AMP + " bar" + NL + "\tEOF"
|
||||
assert guidance(cmd) is None
|
||||
|
||||
def test_literal_ui_text_in_body(self):
|
||||
cmd = "cat <<'EOF'" + NL + "About FaceTime " + AMP + " Privacy" + NL + "EOF"
|
||||
assert guidance(cmd) is None
|
||||
|
||||
|
||||
class TestRealBackgroundingStillBlocked:
|
||||
"""A genuine shell-level '&' must still be caught after the fix."""
|
||||
|
||||
def test_trailing_background(self):
|
||||
assert guidance("python3 server.py " + AMP) is not None
|
||||
|
||||
def test_inline_background(self):
|
||||
assert guidance("sleep 100 " + AMP + " echo done") is not None
|
||||
|
||||
def test_background_after_heredoc(self):
|
||||
# A heredoc that itself is backgrounded — the trailing '&' after the
|
||||
# closing delimiter is real backgrounding and must still be flagged.
|
||||
cmd = "cat <<'EOF' > f.txt" + NL + "payload" + NL + "EOF" + NL + "long_running " + AMP
|
||||
assert guidance(cmd) is not None
|
||||
|
||||
|
||||
class TestStripQuotesHeredoc:
|
||||
"""Direct unit checks on the helper."""
|
||||
|
||||
def test_heredoc_body_removed(self):
|
||||
cmd = "osascript <<'EOF'" + NL + 'x ' + AMP + " y" + NL + "EOF"
|
||||
stripped = _strip_quotes(cmd)
|
||||
# The bare spaced ampersand from the body must not survive.
|
||||
assert (" " + AMP + " ") not in stripped
|
||||
|
||||
def test_multiple_heredocs(self):
|
||||
cmd = (
|
||||
"cat <<'A'" + NL + "one " + AMP + " two" + NL + "A" + NL
|
||||
+ "cat <<'B'" + NL + "three " + AMP + " four" + NL + "B"
|
||||
)
|
||||
stripped = _strip_quotes(cmd)
|
||||
assert (" " + AMP + " ") not in stripped
|
||||
|
|
@ -2380,10 +2380,52 @@ def _strip_quotes(command: str) -> str:
|
|||
|
||||
This prevents false positives when keywords like 'nohup' or 'setsid' appear
|
||||
in commit messages, Python -c code, echo arguments, or PR body text.
|
||||
Also strips backtick-quoted content and heredoc-style inline text.
|
||||
Also strips backtick-quoted content and heredoc body text.
|
||||
"""
|
||||
# Remove heredoc bodies FIRST (before quote-stripping — a heredoc delimiter
|
||||
# may be quoted, e.g. <<'EOF', and the body commonly contains characters like
|
||||
# '&' that are literal payload, not shell operators). Matches <<EOF, <<-EOF,
|
||||
# <<'EOF', and <<"EOF"; body runs up to the closing delimiter line.
|
||||
def _strip_heredocs(text: str) -> str:
|
||||
heredoc_re = re.compile(r"<<-?\s*(['\"]?)([A-Za-z_][A-Za-z0-9_]*)\1")
|
||||
out = text
|
||||
# Iterate because a command may contain multiple heredocs.
|
||||
while True:
|
||||
m = heredoc_re.search(out)
|
||||
if not m:
|
||||
break
|
||||
delim = m.group(2)
|
||||
# The heredoc body starts on the NEXT line — anything between the
|
||||
# `<<DELIM` token and the end of that line (e.g. ` > file.txt`,
|
||||
# additional args, or even a trailing `&`) is still real command
|
||||
# text and must be preserved. Find the newline that ends the
|
||||
# opener line.
|
||||
nl = out.find("\n", m.end())
|
||||
if nl == -1:
|
||||
# No body at all (opener with no following line) — nothing to
|
||||
# strip; blank the delimiter so we don't loop, keep the rest.
|
||||
out = out[: m.start()] + "<<" + out[m.end() :]
|
||||
break
|
||||
body_start = nl # keep the newline; body is what follows it
|
||||
# Closing delimiter: on its own line, optional leading tabs for <<-.
|
||||
close_re = re.compile(r"\n[ \t]*" + re.escape(delim) + r"[ \t]*(?=\n|$)")
|
||||
cm = close_re.search(out, body_start)
|
||||
# Blank the `<<DELIM` opener token itself so the next loop iteration
|
||||
# doesn't re-match this same heredoc (which would then find no
|
||||
# closing line and wrongly drop the real command tail after it).
|
||||
opener_blanked = out[: m.start()] + "<<" + out[m.end() : body_start]
|
||||
if cm:
|
||||
# Drop the body + closing-delimiter token, keep everything after.
|
||||
out = opener_blanked + out[cm.end() :]
|
||||
else:
|
||||
# Unterminated heredoc — the rest of the string is body; drop it.
|
||||
out = opener_blanked
|
||||
break
|
||||
return out
|
||||
|
||||
result = _strip_heredocs(command)
|
||||
# Remove single-quoted strings (no escaping inside single quotes in shell)
|
||||
result = re.sub(r"'[^']*'", "''", command)
|
||||
result = re.sub(r"'[^']*'", "''", result)
|
||||
# Remove double-quoted strings (handle escaped quotes)
|
||||
result = re.sub(r'"(?:[^"\\]|\\.)*"', '""', result)
|
||||
# Remove backtick-quoted strings
|
||||
|
|
|
|||
Loading…
Reference in New Issue