From 2bfdd8cd347d90f3eefff92466e5511c2bfbf464 Mon Sep 17 00:00:00 2001
From: Taylor Mingos <54285+tmingos@users.noreply.github.com>
Date: Mon, 13 Jul 2026 10:32:59 -0400
Subject: [PATCH] fix(tools): strip heredoc bodies before background-'&'
 detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

_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.
---
 .../test_terminal_heredoc_background_guard.py | 97 +++++++++++++++++++
 tools/terminal_tool.py                        | 46 ++++++++-
 2 files changed, 141 insertions(+), 2 deletions(-)
 create mode 100644 tests/tools/test_terminal_heredoc_background_guard.py

diff --git a/tests/tools/test_terminal_heredoc_background_guard.py b/tests/tools/test_terminal_heredoc_background_guard.py
new file mode 100644
index 0000000000000..72d9408d27451
--- /dev/null
+++ b/tests/tools/test_terminal_heredoc_background_guard.py
@@ -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
diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py
index 2f664fbb1336d..222e0693184e2 100644
--- a/tools/terminal_tool.py
+++ b/tools/terminal_tool.py
@@ -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