fix(approval): judge shell quoting on the raw command, not the escape-stripped one

The hardline floor tracked quote state on text that
_normalize_command_for_detection had already rewritten (`\"` -> `"`).
That flips quote parity and broke both ways:

- false positive: a shell-valid `grep -o "[^\"]*" f` lexed as an
  unterminated quote and hit the unconditional "malformed executable
  payload" block. 118 of the 125 hardline blocks in one week of real
  agent use on this install were this shape, every one a benign grep,
  and the block cannot be bypassed by --yolo or approvals.mode=off.
- bypass: `cat "f\"n.txt"; rm -rf --no-preserve-root /` put the `; rm`
  start "inside" a phantom quote, so no command start was marked and the
  floor let the root wipe through with approved=True.

Fix: the malformed-quoting verdict reads the raw command (only quoted
newlines masked, which keeps quoting intact), and
_command_detection_variants adds a variant whose command starts were
marked on the raw command before normalization. The marker is " \n" so a
preceding literal backslash cannot eat it as a line continuation.
_iter_shell_command_starts no longer treats the `{` of `${...}` as a
brace-group opener, so the new variant does not split `${IFS}` and defeat
the IFS collapse.

Direction from #85922 by @Soju06, re-implemented onto the decomposed
tools/approval_detection.py; the parameter-expansion scanner from that PR
is replaced by the one-character `${` check above.

Co-authored-by: Soju06 <qlskssk@gmail.com>
This commit is contained in:
teknium1
2026-09-14 08:47:59 -07:00
committed by Teknium
parent caa7f21f4c
commit e383c28d2f
3 changed files with 63 additions and 6 deletions

View File

@@ -0,0 +1,42 @@
"""The hardline floor judges shell quoting on the AUTHOR's text, not on normalized text.
``_normalize_command_for_detection`` strips backslash escapes so ``r\\m`` cannot hide ``rm``. That
is right for pattern matching but wrong for quote tracking: ``\\"`` becomes ``"`` and flips quote
parity. Two consumers paid for it in opposite directions:
* false positive — a shell-valid ``grep -o "[^\\"]*" f`` lexed as unterminated and hit the
unconditional block (118 of 125 hardline blocks in one week of real agent use, all benign greps);
* bypass — ``cat "f\\"n.txt"; rm -rf --no-preserve-root /`` put the ``; rm`` start "inside" a
phantom quote, so no command start was marked and the floor let it through.
Direction from #85922 (@Soju06): read quote state from the raw command (only quoted newlines masked).
"""
import pytest
from tools.approval_detection import detect_dangerous_command, detect_hardline_command
@pytest.mark.parametrize("command", [
'grep -o "[^\\"]*" f',
r'grep -n "^from\|^__all__\|^ \"" tools/environments/__init__.py | head -15',
r'egrep -n "alpha\"beta|gamma" input.txt',
r'grep -v "^./tests/\|def \|\"\"\"" x.py | grep -v "read_only=True"',
'grep -n "; reboot" f.txt',
'echo "value is ${HOME}/x"',
])
def test_escaped_quotes_in_a_valid_grep_pattern_are_not_malformed(command):
assert detect_hardline_command(command) == (False, None)
assert detect_dangerous_command(command) == (False, None, None)
@pytest.mark.parametrize(("command", "description"), [
(r'cat "f\"n.txt"; rm -rf --no-preserve-root /', "recursive delete of root filesystem"),
(r'echo "a\"b"; reboot', "system shutdown/reboot"),
(r'echo "a\"b" && rm -rf ~', "recursive delete of home directory"),
(r'echo "a\"b"; rm${IFS}-rf${IFS}/', "recursive delete of root filesystem"),
(r'grep -n "prefix \"quoted\" suffix" input.txt; reboot', "system shutdown/reboot"),
("printf \\\\\nreboot", "system shutdown/reboot"),
("grep 'unterminated", "command parser limit or malformed executable payload"),
])
def test_escaped_quote_before_a_hardline_command_does_not_hide_it(command, description):
assert detect_hardline_command(command) == (True, description)

View File

@@ -175,8 +175,11 @@ def detect_hardline_command(command: str) -> tuple:
"""Check hardline patterns (NEVER bypassable, even in YOLO) -> (is_hardline, description)."""
if _command_parser_limit_exceeded(command):
return (True, _PARSER_LIMIT_DESCRIPTION)
normalized = _normalize_command_for_detection(command)
_, malformed_grep = _grep_safe_detection_variant(normalized)
# The malformed-quoting verdict needs the author's quote state. Normalization strips escapes
# (`\"` -> `"`), so a shell-valid pattern like `grep -o "[^\"]*"` lexed as unterminated and was
# reported as a hardline block (118 of 125 hardline blocks in one week of real use, every one a
# benign grep). Only quoted newlines are masked: they are data, and masking keeps quoting intact.
_, malformed_grep = _grep_safe_detection_variant(_mask_quoted_newlines(command))
if malformed_grep:
return (True, _MALFORMED_EXEC_DESCRIPTION)
for command_variant in _command_detection_variants(command):
@@ -1087,7 +1090,9 @@ def _iter_shell_command_starts(command: str):
starts.append(inner)
scan(inner, end if j is None else j - 1)
elif kind == "char" and quote is None and i != skip:
if command[i] in "({;\n":
# `${` opens a parameter expansion, not a brace group: a start marked inside it would
# split `${IFS}` and defeat the IFS collapse in normalization.
if command[i] in "({;\n" and not (command[i] == "{" and i > 0 and command[i - 1] == "$"):
starts.append(i + 1)
elif command[i] in "&|":
repeated = i + 1 < end and command[i + 1] == command[i]
@@ -1107,14 +1112,14 @@ def _iter_shell_command_starts(command: str):
starts.append(end)
def _mark_command_starts(command: str) -> str:
"""Insert a newline before each real (quote-aware) command start.
def _mark_command_starts(command: str, marker: str = "\n") -> str:
"""Insert *marker* (a newline) before each real (quote-aware) command start.
``\\n`` is already a ``_CMDPOS`` separator, so this exposes subshell ``(cmd)`` and brace-group
``{ cmd; }`` openers — which the flat pattern class omits — to the anchored patterns WITHOUT the
quoted-prose false positives that adding ``(`` / ``{`` to ``_CMDPOS`` would cause: starts inside
quotes are never produced, so ``--title "block (reboot)"`` is left as-is."""
offsets = sorted(o for o in _iter_shell_command_starts(command) if o > 0)
return _splice(command, [(o, o, "\n") for o in offsets]) if offsets else command
return _splice(command, [(o, o, marker) for o in offsets]) if offsets else command
def _mask_quoted_newlines(command: str) -> str:
@@ -1376,6 +1381,14 @@ def _command_detection_variants(command: str):
marked = _mark_command_starts(grep_safe)
if marked != grep_safe and fresh(marked):
yield marked
# Every variant above tracks quotes on NORMALIZED text, where `\"` has already become `"`. That
# flips quote parity, so in `cat "f\"n.txt"; rm -rf /` the `; rm` start sat "inside" a phantom
# quote, no start was marked, and the hardline floor let it through. Mark starts on the RAW
# command (only quoted newlines masked), then normalize; the leading space keeps the marker
# from being eaten as a `\<newline>` continuation when the preceding text ends in a backslash.
faithful = _normalize_command_for_detection(_mark_command_starts(_mask_quoted_newlines(command), marker=" \n"))
if fresh(faithful):
yield faithful
# Quoting/escaping can spell an executable in pieces (r\m, r''m). Keep that deobfuscation scoped
# to command words so arguments don't false-positive.
for word_start, word_end, word in _iter_shell_command_word_spans(normalized):

View File

@@ -140,6 +140,8 @@ The blocklist is the floor below `--yolo`. It trips **before** the approval laye
If you hit the blocklist, the tool call returns an explanatory error to the agent and nothing runs. If a legitimate workflow needs one of these commands (you're the operator of a wipe-and-reinstall pipeline, for example), run it outside the agent.
The floor also fails closed on a command whose shell quoting cannot be parsed (`grep 'unterminated`): the error says `malformed executable payload`. Quoting is judged on the command exactly as written, so shell-valid escapes inside a quoted pattern (`grep -o "[^\"]*" file`) are not malformed, and an escaped quote before a separator (`echo "a\"b"; reboot`) does not hide the command that follows it.
### User-Defined Deny Rules (`approvals.deny`)
The hardline blocklist is fixed and code-shipped. `approvals.deny` is its user-editable counterpart: a list of glob patterns that block matching terminal commands unconditionally — **before** `--yolo`, `/yolo`, and `approvals.mode: off` are consulted. Use it to run yolo-with-exceptions: "let the agent do everything, except these specific things, ever."