From a7aa814c421fb0e5a24971690bb5e2c3a8ad28df Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 24 Aug 2026 00:18:52 -0700 Subject: [PATCH] fix(tools): widen the command-position anchor to the whole hardline class #93392 was not just one pattern: every hardline rule with a bare \b anchor fired on its token anywhere in the command line, including inside quoted prose handed to echo, git commit -m, or gh --body. Anchor the command-name-token rules and quote-mask the positionless ones: - dd-to-block-device and kill -1 get the same _CMDPOS anchor as the format/rm/shutdown families, keeping their argument tails. - redirect-to-block-device and the fork bomb have no command-name token to anchor (`>` appears mid-command; the bomb is a function definition), so they now match a quote-masked variant (_mask_quoted_prose) where quoted string content is blanked. $() and backtick spans inside double quotes stay raw (the shell executes them), and any command whose command-position words include a shell carrier (sh/bash/zsh/ksh/dash -c, eval, source, .) is scanned unmasked -- quoting is not a bypass. bash/sh -c payloads also still surface as raw detection variants via _execution_flag_findings. Regression tests cover both directions for every touched pattern: quoted prose passes, and every true-positive shape (bare, ; && | separators, sudo/env prefix, $(), backticks, sh -c/bash -c/eval payloads) stays on the unconditional floor. --- tests/tools/test_hardline_blocklist.py | 129 ++++++++++++++++++++ tools/approval.py | 155 +++++++++++++++++++++++-- 2 files changed, 274 insertions(+), 10 deletions(-) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 7f0c392cf9..5057c2272e 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -373,6 +373,135 @@ def test_root_wipe_at_command_position_is_hardline(command): assert desc +# ------------------------------------------------------------------------- +# #93392 class regression: unanchored hardline patterns vs quoted prose +# ------------------------------------------------------------------------- +# Every hardline rule that used a bare \b anchor (mkfs, dd, kill -1) fired on +# the token ANYWHERE in the command — including inside quoted prose handed to +# echo / git commit -m / gh --body — and the positionless rules (redirect to +# a block device, fork bomb) fired on quoted mentions too. Quoted prose must +# pass; every true-positive shape (bare, separators, sudo/env prefix, $(), +# backticks, sh -c/bash -c/eval payloads) must stay unconditionally blocked. + +_QUOTED_PROSE_ALLOW_93392 = [ + # mkfs (the reported symptom) + 'echo "does this workflow use mkfs anywhere?"', + 'git commit -m "add mkfs.ext4 warning to the runbook"', + 'gh pr create --body "this PR anchors the mkfs pattern"', + "grep 'mkfs' docs/runbook.md", + # dd to block device + 'git commit -m "never run dd of=/dev/sda in prod"', + 'echo "dd if=/dev/zero of=/dev/sda wipes the disk"', + "grep 'dd if=/dev/zero of=/dev/sda' notes.md", + # kill -1 + 'echo "kill -1 sends SIGHUP to every process"', + 'gh issue comment 7 --body "the agent must never run kill -1"', + # redirect to block device (positionless rule -> quote-masked) + 'echo "cat file > /dev/sda destroys the disk"', + "echo 'redirect > /dev/sdb1 is fatal'", + 'git commit -m "block > /dev/sda redirects"', + 'gh pr create --body "guards the > /dev/nvme0n1 redirect"', + # fork bomb (positionless rule -> quote-masked) + 'git commit -m "document the fork bomb :(){ :|:& };: pattern"', + 'echo "classic fork bomb: :(){ :|:& };:"', + "echo ':(){ :|:& };: is a fork bomb'", +] + + +@pytest.mark.parametrize("command", _QUOTED_PROSE_ALLOW_93392) +def test_quoted_prose_mentions_are_not_hardline(command): + """Quoted prose mentioning a hardline trigger is data, not a command.""" + is_hl, desc = detect_hardline_command(command) + assert not is_hl, ( + f"quoted prose false-positived the hardline floor: {command!r} " + f"(got: {desc})" + ) + + +_TRUE_POSITIVES_93392 = [ + # mkfs at every command position + "mkfs.ext4 /dev/sda1", + "mkfs /dev/sdb", + "sudo mkfs.xfs /dev/nvme0n1", + "true && mkfs.ext4 /dev/sda1", + "ls; mkfs /dev/sdb", + "env FOO=1 mkfs.ext4 /dev/sda1", + "$(mkfs.ext4 /dev/sda1)", + "`mkfs /dev/sdb`", + 'bash -c "mkfs.ext4 /dev/sda1"', + # dd to raw block device + "dd if=/dev/zero of=/dev/sda bs=1M", + "sudo dd if=/dev/urandom of=/dev/nvme0n1", + "echo start && dd if=/dev/zero of=/dev/sdb", + "ls; dd if=x of=/dev/mmcblk0", + "env X=1 dd if=/dev/zero of=/dev/sda", + "$(dd if=/dev/zero of=/dev/sda)", + "`dd if=/dev/zero of=/dev/sda`", + 'sh -c "dd if=/dev/zero of=/dev/sda"', + # redirect to raw block device (unquoted / carrier / substitution) + "cat file > /dev/sda", + "echo junk > /dev/sdb", + "true && cat f > /dev/nvme0n1", + 'sh -c "cat f > /dev/sda"', + 'bash -c "echo x > /dev/sdb"', + 'eval "cat f > /dev/sda"', + 'echo "$(cat f > /dev/sda)"', + 'echo "`cat f > /dev/sdb`"', + # kill -1 + "kill -1", + "kill -9 -1", + "sudo kill -1", + "ls; kill -1", + "true && kill -HUP -1", + "$(kill -1)", + 'bash -c "kill -1"', + # fork bomb + ":(){ :|:& };:", + "true && :(){ :|:& };:", + 'sh -c ":(){ :|:& };:"', + "eval ':(){ :|:& };:'", +] + + +@pytest.mark.parametrize("command", _TRUE_POSITIVES_93392) +def test_true_positive_shapes_stay_hardline_blocked(command): + """Every real destructive shape stays on the unconditional floor.""" + is_hl, desc = detect_hardline_command(command) + assert is_hl, f"true positive leaked past the hardline floor: {command!r}" + assert desc + + +# DANGEROUS-tier duplicates of the mkfs/dd rules must be anchored the same +# way: quoted prose must not even require approval, while real invocations +# stay flagged (yolo can still bypass this tier — that's what yolo is for). +_DANGEROUS_TIER_PROSE_ALLOW = [ + 'echo "mkfs is a formatting tool"', + 'git commit -m "explain dd if=/dev/zero usage"', +] + +_DANGEROUS_TIER_STILL_FLAGGED = [ + ("mkfs /dev/sdb1", "format filesystem"), + ("sudo mkfs -t vfat /dev/sdc1", "format filesystem"), + ("dd if=backup.img of=restore.img", "disk copy"), + ("true && dd if=a.img of=b.img", "disk copy"), +] + + +@pytest.mark.parametrize("command", _DANGEROUS_TIER_PROSE_ALLOW) +def test_dangerous_tier_prose_not_flagged(command): + is_dangerous, _, desc = detect_dangerous_command(command) + assert not is_dangerous, ( + f"quoted prose tripped the dangerous tier: {command!r} (got: {desc})" + ) + + +@pytest.mark.parametrize("command,expected", _DANGEROUS_TIER_STILL_FLAGGED) +def test_dangerous_tier_real_commands_still_flagged(command, expected): + is_dangerous, _, desc = detect_dangerous_command(command) + assert is_dangerous, f"real command no longer dangerous-flagged: {command!r}" + assert desc == expected + + # ------------------------------------------------------------------------- # Shell line-continuation bypass # ------------------------------------------------------------------------- diff --git a/tools/approval.py b/tools/approval.py index b8b44e7293..775aa85972 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -536,13 +536,27 @@ HARDLINE_PATTERNS = [ # hardline entry so quoted prose ("echo \"does this workflow use mkfs # anywhere?\"") does not trip the unconditional floor (#93392). (_CMDPOS + r'mkfs(\.[a-z0-9]+)?\b', "format filesystem (mkfs)"), - # Raw block device overwrites (dd + redirection) - (r'\bdd\b[^\n]*\bof=/dev/(sd|nvme|hd|mmcblk|vd|xvd)[a-z0-9]*', "dd to raw block device"), + # Raw block device overwrites (dd + redirection). `dd` is a command-name + # token, so anchor it to command position like mkfs/rm/shutdown (#93392): + # quoted prose such as `git commit -m "never dd of=/dev/sda"` is an + # argument, not a command. The argument tail ([^\n]*of=/dev/...) is kept + # so flag order doesn't matter. + (_CMDPOS + r'dd\b[^\n]*\bof=/dev/(sd|nvme|hd|mmcblk|vd|xvd)[a-z0-9]*', "dd to raw block device"), + # The redirect rule has no command-name token to anchor (`>` appears + # mid-command: `cat f > /dev/sda`), so command-position anchoring is the + # wrong tool. It is instead matched against a QUOTE-MASKED variant of the + # command (see _QUOTE_MASKED_HARDLINE / _mask_quoted_strings) so quoted + # prose (`echo "cat f > /dev/sda"`) cannot trip it, while shell-carrying + # wrappers (sh -c / bash -c / eval) still surface their payload as a raw + # detection variant — quoting is not a bypass (#93392). (r'>\s*/dev/(sd|nvme|hd|mmcblk|vd|xvd)[a-z0-9]*\b', "redirect to raw block device"), - # Fork bomb (classic shell form) + # Fork bomb (classic shell form). Also positionless (the trigger is the + # function definition itself, valid anywhere in a command line), so it is + # quote-masked like the redirect rule above rather than _CMDPOS-anchored. (r':\(\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;\s*:', "fork bomb"), - # Kill every process on the system - (r'\bkill\s+(-[^\s]+\s+)*-1\b', "kill all processes"), + # Kill every process on the system — anchor the command-name token so + # `echo "kill -1 sends SIGHUP to everything"` doesn't trip (#93392). + (_CMDPOS + r'kill\s+(-[^\s]+\s+)*-1\b', "kill all processes"), # System shutdown / reboot — anchor to command position (start of line, # after a command separator, or after sudo/env wrappers) so we don't # false-positive on "echo reboot" or "grep 'shutdown' logs". @@ -560,12 +574,113 @@ HARDLINE_PATTERNS = [ # regex work elsewhere in the agent). DANGEROUS_PATTERNS_COMPILED is built # at the end of this module after DANGEROUS_PATTERNS is defined. _RE_FLAGS = re.IGNORECASE | re.DOTALL + +# Hardline rules whose trigger has no command-name token to anchor (the +# redirect target / fork-bomb definition are valid anywhere in a command +# line). These are matched against QUOTE-MASKED variants of the command so +# quoted prose (`echo "cat f > /dev/sda"`, `git commit -m "fork bomb +# :(){ :|:& };:"`) cannot trip the unconditional floor, while the raw +# payloads of shell-carrying wrappers (sh -c, bash -c, eval) are still +# scanned unmasked — quoting is not a bypass (#93392). +_QUOTE_MASKED_HARDLINE_DESCRIPTIONS = frozenset({ + "redirect to raw block device", + "fork bomb", +}) + HARDLINE_PATTERNS_COMPILED = [ - (re.compile(pattern, _RE_FLAGS), description) + ( + re.compile(pattern, _RE_FLAGS), + description, + description in _QUOTE_MASKED_HARDLINE_DESCRIPTIONS, + ) for pattern, description in HARDLINE_PATTERNS ] +# Command names that hand a quoted argument to another shell/parser to +# EXECUTE. For these, quoted text is code, not prose, so the quote-masked +# hardline rules must scan the raw string (see detect_hardline_command). +_SHELL_CARRIER_NAMES = frozenset({ + "eval", "sh", "bash", "zsh", "ksh", "dash", "source", ".", +}) + + +def _contains_shell_carrier(command: str) -> bool: + """Return whether any command-position word is a shell-carrying command.""" + for _, _, word in _iter_shell_command_word_spans(command): + name = os.path.basename( + _deobfuscate_shell_word_for_detection(word) + ).lower() + if name in _SHELL_CARRIER_NAMES: + return True + return False + + +def _mask_quoted_prose(command: str) -> str: + """Blank out quoted string CONTENT for positionless hardline matching. + + Detection-only rewrite used by the quote-masked hardline rules + (redirect-to-block-device, fork bomb): text inside single or double + quotes is data the shell passes as an argument, so `echo "cat f > + /dev/sda"` must not trip the unconditional floor (#93392). Structure is + preserved: the quote characters themselves stay, and inside double + quotes `$(...)` command substitutions and backtick spans are kept RAW + because the shell really executes them (`echo "$(cat f > /dev/sda)"` + remains a true positive). Unquoted text is untouched. Quote tracking + mirrors _mask_quoted_newlines; an unclosed quote masks to end-of-string, + which cannot hide a runnable command (the shell would not run it + either). + """ + out: list[str] = [] + quote: str | None = None + i = 0 + n = len(command) + while i < n: + ch = command[i] + if quote == "'": + if ch == "'": + quote = None + out.append(ch) + else: + out.append(" ") + i += 1 + continue + if quote == '"': + if ch == "\\" and i + 1 < n: + out.append(" ") + i += 2 + continue + if ch == '"': + quote = None + out.append(ch) + i += 1 + continue + if ch == "$" and i + 1 < n and command[i + 1] == "(": + end = _scan_dollar_paren_end(command, i) + if end is not None: + out.append(command[i:end]) + i = end + continue + if ch == "`": + close = command.find("`", i + 1) + if close != -1: + out.append(command[i:close + 1]) + i = close + 1 + continue + out.append(" ") + i += 1 + continue + if ch == "\\" and i + 1 < n: + out.append(command[i:i + 2]) + i += 2 + continue + if ch in ("'", '"'): + quote = ch + out.append(ch) + i += 1 + return "".join(out) + + # ========================================================================= # Sudo stdin guard — block password guessing via "sudo -S" # ========================================================================= @@ -616,8 +731,26 @@ def detect_hardline_command(command: str) -> tuple: return (True, _MALFORMED_EXEC_DESCRIPTION) for command_variant in _command_detection_variants(command): variant_lower = command_variant.lower() - for pattern_re, description in HARDLINE_PATTERNS_COMPILED: - if pattern_re.search(variant_lower): + masked_lower: str | None = None + for pattern_re, description, quote_masked in HARDLINE_PATTERNS_COMPILED: + if quote_masked: + # Positionless rules (redirect-to-block-device, fork bomb) + # match a quote-masked variant so quoted prose in echo / + # git commit -m / gh --body arguments is DATA (#93392). + # Shell-carrying commands (sh/bash -c, eval, source) hand + # their quoted argument to another parser, so those scan + # the raw variant — quoting is not a bypass. bash/sh -c + # payloads additionally surface as their own raw variants + # via _execution_flag_findings. + if masked_lower is None: + if _contains_shell_carrier(command_variant): + masked_lower = variant_lower + else: + masked_lower = _mask_quoted_prose(command_variant).lower() + haystack = masked_lower + else: + haystack = variant_lower + if pattern_re.search(haystack): return (True, description) return (False, None) @@ -860,8 +993,10 @@ DANGEROUS_PATTERNS = [ (r'\bchmod\s+--recursive\b.*(777|666|o\+[rwx]*w|a\+[rwx]*w)', "recursive world/other-writable (long flag)"), (r'\bchown\s+(-[^\s]*)?R\s+root', "recursive chown to root"), (r'\bchown\s+--recur[a-z]*\b.*root', "recursive chown to root (long flag)"), - (r'\bmkfs\b', "format filesystem"), - (r'\bdd\s+.*if=', "disk copy"), + # Anchored to command position like the hardline twins (#93392): + # quoted prose mentioning mkfs/dd must not require approval to echo. + (_CMDPOS + r'mkfs\b', "format filesystem"), + (_CMDPOS + r'dd\s+.*if=', "disk copy"), (r'>\s*/dev/sd', "write to block device"), (r'\bDROP\s+(TABLE|DATABASE)\b', "SQL DROP"), # Use [^\n]* instead of .* so DOTALL mode does not cause a WHERE clause on the