fix(approval): treat # as comment boundary only when whitespace-preceded
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.
This commit is contained in:
parent
7bfdc0bca6
commit
1d8bd73414
2 changed files with 30 additions and 2 deletions
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue