From 1d8bd73414f5e6930f9cb8fb1c8852f449bfba4f Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Wed, 1 Jul 2026 01:14:55 -0700
Subject: [PATCH] fix(approval): treat # as comment boundary only when
 whitespace-preceded
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The salvaged write-target boundary included `#` in its char class, so a
`#` glued to the redirect/tee path (`echo x > .env#backup`) matched as a
comment boundary and flagged the write as dangerous. But the shell writes
to the distinct file `.env#backup`, not `.env` — a false positive, same
class as the config.yaml.bak case the PR already excluded. Drop `#` from
the boundary; a real trailing comment is always whitespace-preceded (\\s).

Adds regression tests for .env#backup, config.yaml#backup, and
tee .env#backup staying out of the deny.
---
 tests/tools/test_approval.py | 22 ++++++++++++++++++++++
 tools/approval.py            | 10 ++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py
index 70057953d..2dc5f8f30 100644
--- a/tests/tools/test_approval.py
+++ b/tests/tools/test_approval.py
@@ -673,6 +673,28 @@ class TestSensitiveRedirectPattern:
         assert key is None
         assert desc is None
 
+    def test_redirect_to_dotenv_hash_glued_filename_is_safe(self):
+        # A `#` glued to the path is part of the filename, not a comment: the
+        # shell writes to `.env#backup` (a different file), so it must stay out
+        # of the deny — same reasoning as config.yaml.bak. The boundary must
+        # NOT treat `#` as a word boundary (a real comment is whitespace-preceded).
+        dangerous, key, desc = detect_dangerous_command("echo x > .env#backup")
+        assert dangerous is False
+        assert key is None
+        assert desc is None
+
+    def test_redirect_to_config_yaml_hash_glued_filename_is_safe(self):
+        dangerous, key, desc = detect_dangerous_command("echo x > config.yaml#backup")
+        assert dangerous is False
+        assert key is None
+        assert desc is None
+
+    def test_tee_to_dotenv_hash_glued_filename_is_safe(self):
+        dangerous, key, desc = detect_dangerous_command("printenv | tee .env#backup")
+        assert dangerous is False
+        assert key is None
+        assert desc is None
+
 
 class TestProjectSensitiveCopyPattern:
     def test_cp_to_local_dotenv_requires_approval(self):
diff --git a/tools/approval.py b/tools/approval.py
index 11c33f9f7..2e074c825 100644
--- a/tools/approval.py
+++ b/tools/approval.py
@@ -266,13 +266,19 @@ _COMMAND_TAIL = r'(?:\s*(?:&&|\|\||;).*)?$'
 # Boundary for stream-write rules (`>`/`>>` redirection and `tee`), where the
 # sensitive path is ALWAYS a write target no matter what follows it. We only
 # need the path token to END at a shell word boundary — whitespace, a quote, a
-# command separator, a redirection operator, a `#` comment, or end-of-line.
+# command separator, a redirection operator, or end-of-line.
 # Using _COMMAND_TAIL here was too strict: it required the rest of the line to
 # be empty or a command separator, so `echo x > .env extra` (extra arg to echo)
 # and `echo x > .env # note` (trailing comment) slipped past the deny even
 # though the shell still overwrites `.env`. Mirrors the looser system-path
 # redirection rule, which never had this restriction.
-_WRITE_TARGET_BOUNDARY = r'(?=[\s;&|<>#"\']|$)'
+#
+# `#` is deliberately NOT a boundary char: a real trailing comment always has
+# whitespace before the `#` (already covered by `\s`), whereas a `#` glued to
+# the path is part of the filename. `echo x > .env#backup` writes to the
+# distinct file `.env#backup`, not `.env`, so it must stay OUT of the deny —
+# the same reasoning that keeps `config.yaml.bak` safe.
+_WRITE_TARGET_BOUNDARY = r'(?=[\s;&|<>"\']|$)'
 
 # =========================================================================
 # Hardline (unconditional) blocklist