From ba77f81fe98c78d84345a1381706402fdb6e1918 Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Sun, 26 Jul 2026 22:20:39 +0900 Subject: [PATCH 1/5] fix(approval): stop absolute-path spellings bypassing the hardline floor _CMDPOS anchors every hardline pattern to "separator + optional wrappers", so a binary reached by path never matched: on current main `shutdown -h now` is blocked while `/sbin/shutdown -h now`, `C:\Windows\System32\shutdown.exe /s /t 0`, `"C:\Program Files\Git\usr\bin\rm.exe" -rf /` and `/sbin/init 6` all return (False, None) and sail through the floor that is supposed to hold even under yolo. Path spelling is the natural form on hosts where PATH is unreliable and the default an LLM produces for Windows tools. Fix at the variant layer instead of per pattern: a detection-only projection walks each command position with the same quote-aware word reader the r\m detection uses, recognizes wrappers by their PROJECTED basename (so `/usr/bin/env /usr/bin/sudo /sbin/shutdown` resolves in one pass), skips assignment prefixes and wrapper options, and rewrites the executable word to "\n" - outer quotes removed, both separators handled, case-insensitive .exe/.bat/.cmd stripped. Composed spellings (`'/sbin/'shutdown`, `/sbin/shut\down`) reduce through the same escape collapsing as bare-name detection. Windows-absolute words are split on separators BEFORE that collapsing, which would otherwise dissolve the path; the whole projection runs on the raw command for the same reason. Execution-flag payloads (`bash -c '/sbin/shutdown -h now'`) get the same projection. Every _CMDPOS rule, the user deny-list and the dangerous-pattern layer inherit the coverage in one place. Path-as-data stays untouched: `echo /sbin/shutdown`, `cat /etc/init.d/reboot`, near-miss basenames like `rebooter`, and assignment values (`X=/sbin/shutdown echo ok`) keep returning (False, None), each verified against clean main for parity. Co-Authored-By: Claude Fable 5 --- tests/tools/test_hardline_blocklist.py | 89 ++++++++++++++ tools/approval.py | 155 +++++++++++++++++++++++++ 2 files changed, 244 insertions(+) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 40b1c567555a0..e3133d7f83d69 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -670,3 +670,92 @@ def test_sudo_stdin_guard_container_bypass(clean_session): for cmd in _SUDO_STDIN_BLOCK: result = check_all_command_guards(cmd, env) assert result["approved"] is True, f"container {env} should bypass sudo guard on {cmd!r}" + + +# Absolute-path invocations must not defeat the floor. _CMDPOS only accepted +# "start | separator | subshell opener | sudo/env-style wrappers" before the +# command word, so spelling the binary by path — the natural form on systems +# where PATH is unreliable, and the default an LLM produces for Windows +# tools — returned (False, None) for every hardline pattern. Regression set +# pins both directions: path-spelled commands at command position are +# blocked, the same strings as data arguments stay allowed. +_ABS_PATH_BLOCK = [ + "/sbin/shutdown -h now", + "/usr/sbin/reboot", + "/sbin/halt", + "/sbin/init 6", + "/bin/rm -rf /", + "sudo /sbin/shutdown -h now", + "env LC_ALL=C /sbin/reboot", + "echo done; /sbin/shutdown -h now", + "true && /usr/sbin/poweroff", + r"C:\Windows\System32\shutdown.exe /s /t 0", + "C:/Windows/System32/shutdown.exe /s /t 0", + r'"C:\Program Files\Git\usr\bin\rm.exe" -rf /', + r"C:\Windows\System32\shutdown.EXE /s", + # path-spelled wrapper chains resolve in the single projection pass + "/usr/bin/sudo /sbin/shutdown -h now", + "/usr/bin/env /usr/bin/sudo /sbin/shutdown -h now", + "exec /sbin/reboot", + "( /sbin/shutdown -h now )", + "/sbin/telinit 6", + "./shutdown -h now", + "shutdown.exe /s", + # composed spellings reduce through the same collapsing as r\m detection + "'/sbin/'shutdown -h now", + "/sbin/shut\\down -h now", + # a payload's executable can be path-spelled too + "bash -c '/sbin/shutdown -h now'", + "exec bash -c '/bin/rm -rf /'", + # group/substitution closers stay outside the projected word + "(/sbin/reboot)", + "$(/sbin/reboot)", + "`/sbin/reboot`", + "{ /sbin/reboot; }", + "env -u FOO /sbin/reboot", +] + +_ABS_PATH_ALLOW = [ + "echo /sbin/shutdown", + "ls -la /sbin/shutdown", + "grep 'shutdown' /var/log/syslog", + "stat /usr/sbin/reboot", + r"stat C:\Windows\System32\shutdown.exe", + "cat /bin/rm", + 'echo "/sbin/init 6"', + "md5sum /sbin/halt", + # basename must match the pattern word exactly, not a near-miss + "/usr/local/bin/rebooter --dry-run", + "./deploy.sh shutdown", + "cat /etc/init.d/reboot", + # a bare assignment prefix is data — projecting its value manufactured + # a False->True flip in the first cut of this fix (Sol round 2) + "X=/sbin/shutdown echo ok", + # a wrapper option's operand is data, not the command word + "env --chdir /tmp/reboot /bin/echo ok", + "env --chdir /tmp/reboot echo ok", + "env --argv0 /tmp/reboot /bin/echo ok", + # `}` is not a shell metacharacter: it can end a legitimate word + "/tmp/reboot}", +] + + +@pytest.mark.parametrize("command", _ABS_PATH_BLOCK) +def test_abs_path_invocation_is_hardline_blocked(command): + is_hl, desc = detect_hardline_command(command) + assert is_hl, f"absolute-path spelling bypassed the floor: {command!r}" + assert desc, "hardline match must provide a description" + + +@pytest.mark.parametrize("command", _ABS_PATH_ALLOW) +def test_abs_path_as_data_is_not_hardline(command): + is_hl, desc = detect_hardline_command(command) + assert not is_hl, f"path-as-data false positive: {command!r} (got: {desc})" + + +def test_abs_path_hardline_not_bypassed_by_yolo(clean_session, monkeypatch): + """The floor must hold for path-spelled commands under yolo too.""" + monkeypatch.setenv("HERMES_YOLO_MODE", "1") + result = check_all_command_guards("/sbin/shutdown -h now", "local") + assert result["approved"] is False + assert result.get("hardline") is True diff --git a/tools/approval.py b/tools/approval.py index 50111de8d67b8..5b63c2c788159 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1190,6 +1190,19 @@ _SUDO_OPTIONS_WITH_ARG = { "-p", "--prompt", "-u", "--user", } +# GNU env options whose value arrives as the NEXT word. Their operand is +# data (`env --chdir /tmp/reboot /bin/echo` never executes reboot), so the +# projection walker must not treat it as the command word. `=`-attached +# forms are already handled by the generic option check. `-S/--split-string` +# is listed here only for the walker's word skipping — its payload is +# executable-bearing, but parsing it is a separate fail-closed change +# (same verdict as the design review); skipping keeps parity with main. +_ENV_OPTIONS_WITH_ARG = { + "-a", "--argv0", + "-c", "--chdir", + "-s", "--split-string", + "-u", "--unset", +} _INTERPRETER_EXEC_FLAGS = { "python": {"-c"}, @@ -2091,6 +2104,123 @@ def _iter_shell_command_word_spans(command: str): break +_EXECUTABLE_LAUNCHER_SUFFIXES = (".exe", ".bat", ".cmd") +_PROJECTED_BASENAME_RE = re.compile(r"[A-Za-z0-9_][\w.+-]*\Z") +_WINDOWS_ABSOLUTE_RE = re.compile(r"[A-Za-z]:[\\/]|\\\\") +# `_read_shell_word` stops at whitespace and `;&|`, so a command that ends +# at a substitution/subshell closer keeps that byte in the word: +# `(/sbin/reboot)` reads as `/sbin/reboot)`. Trim the closers before the +# basename check or the stray byte makes the word look like something other +# than a program name and the projection silently declines. Only `)` and +# the backtick qualify: both are shell metacharacters that can never sit +# inside an unquoted word. `}` is NOT one (`echo foo}` prints `foo}`, and +# a brace group's `}` is its own word behind `;`), so trimming it would +# rewrite legitimate names like `/tmp/reboot}`. +_WORD_TAIL_CLOSERS = ")`" + + +def _projected_executable_basename(word: str) -> str | None: + """Reduce one command-position word to the bare program name a shell + would resolve, or None when the word already is one (or can't be one). + + A Windows-absolute word (drive or UNC prefix) is split on both + separators BEFORE any escape collapsing — the deobfuscation pass treats + backslashes as shell escapes and would dissolve the path. Everything + else goes through the same quote/escape collapsing the r\\m detection + uses, so composed spellings (``'/sbin/'shutdown``, ``/sbin/shut\\down``) + reduce like their plain forms.""" + stripped = _strip_optional_shell_quotes(word) + if _WINDOWS_ABSOLUTE_RE.match(stripped): + basename = stripped.replace("\\", "/").rsplit("/", 1)[-1] + else: + collapsed = _deobfuscate_shell_word_for_detection(word) or word + basename = _strip_optional_shell_quotes(collapsed).rsplit("/", 1)[-1] + lowered = basename.lower() + for suffix in _EXECUTABLE_LAUNCHER_SUFFIXES: + if lowered.endswith(suffix): + basename = basename[: -len(suffix)] + break + if basename == word: + return None # already a bare name — nothing to project + # Refuse anything that doesn't reduce to a plain program name (empty + # basename from a trailing slash, expansion debris): projecting those + # could only manufacture command words a shell would not actually run. + if not _PROJECTED_BASENAME_RE.fullmatch(basename): + return None + return basename + + +def _project_path_spelled_executables(command: str) -> str | None: + """Detection-only variant: rewrite command-position executables that are + spelled by path (POSIX or Windows, quoted or bare, with or without a + launcher suffix) to ``\\n`` so the flat ``_CMDPOS``-anchored + patterns see the same command word a bare spelling would produce — + ``/sbin/shutdown``, ``C:\\Windows\\System32\\shutdown.exe`` and + ``"C:\\Program Files\\Git\\usr\\bin\\rm.exe"`` must behave exactly like + ``shutdown`` / ``rm``. Must run on the RAW command: the global + normalization strips backslashes and fuses a Windows path into one word + before the basename could be recovered. + + The walk mirrors ``_iter_shell_command_word_spans`` but recognizes + wrappers by their PROJECTED basename, so a path-spelled wrapper chain + (``/usr/bin/env /usr/bin/sudo /sbin/shutdown``) resolves in this single + pass — no fixpoint, no overlapping re-splice. Assignment-shaped words + (``X=/sbin/shutdown``) are prefix data, never executables. Returns None + when nothing needed rewriting.""" + replacements: dict[int, tuple[int, str]] = {} + for command_start in _iter_shell_command_starts(command): + pos = command_start + wrapper_arg_options: frozenset | set = frozenset() + skip_wrapper_options = False + skip_next_wrapper_arg = False + for _ in range(12): + word_start, raw_end, _raw_word = _read_shell_word(command, pos) + if word_start == raw_end: + break + pos = raw_end + # Trim group/substitution closers the word reader keeps + # (`(/sbin/reboot)` reads as `/sbin/reboot)`); the closer stays + # outside the replacement span so the splice preserves it. + word_end = raw_end + while word_end > word_start and command[word_end - 1] in _WORD_TAIL_CLOSERS: + word_end -= 1 + if word_end == word_start: + continue # bare closer: structure, not a word + word = command[word_start:word_end] + if skip_next_wrapper_arg: + skip_next_wrapper_arg = False + continue + deobfuscated = _deobfuscate_shell_word_for_detection(word) or word + if _ENV_ASSIGNMENT_RE.fullmatch(deobfuscated): + continue # VAR=value prefix: data, keep walking + if skip_wrapper_options and deobfuscated.startswith("-"): + option_name = deobfuscated.lower().split("=", 1)[0] + skip_next_wrapper_arg = ( + "=" not in deobfuscated + and option_name in wrapper_arg_options + ) + continue + basename = _projected_executable_basename(word) + effective = (basename or deobfuscated).lower() + if basename is not None and word_start not in replacements: + replacements[word_start] = (word_end, basename) + if effective in _COMMAND_WRAPPER_WORDS: + skip_wrapper_options = effective in {"sudo", "env"} + if effective == "sudo": + wrapper_arg_options = _SUDO_OPTIONS_WITH_ARG + elif effective == "env": + wrapper_arg_options = _ENV_OPTIONS_WITH_ARG + continue # wrapper (bare or path-spelled): walk to the command + break + if not replacements: + return None + projected = command + for word_start in sorted(replacements, reverse=True): + word_end, basename = replacements[word_start] + projected = projected[:word_start] + "\n" + basename + projected[word_end:] + return projected + + def _command_detection_variants(command: str): # Mask quoted newlines BEFORE normalization: normalization strips # backslash-escapes (\" -> ") and empty-string pairs (""), which would @@ -2121,6 +2251,16 @@ def _command_detection_variants(command: str): if marked_payload != payload and marked_payload not in seen: seen.add(marked_payload) yield marked_payload + # A payload's own executable can be path-spelled too + # (`bash -c '/sbin/shutdown -h now'`), so it needs the same + # basename projection as the outer command. POSIX paths + # survive the normalization the payload came through; + # backslash Windows paths inside payloads stay out of reach + # until normalization preserves separators (#71919). + projected_payload = _project_path_spelled_executables(payload) + if projected_payload is not None and projected_payload not in seen: + seen.add(projected_payload) + yield projected_payload pending.append(payload) # Subshell `(cmd)` and brace-group `{ cmd; }` openers put `cmd` at a real # command position, but the flat `_CMDPOS`-anchored patterns can't see it: @@ -2137,6 +2277,21 @@ def _command_detection_variants(command: str): if marked != grep_safe and marked not in seen: seen.add(marked) yield marked + # Absolute-path spellings bypass every _CMDPOS-anchored rule the same + # way subshell openers did: the anchor class knows wrappers but not + # paths, so `/sbin/shutdown` or `C:\Windows\System32\shutdown.exe` + # never reaches `shutdown\b`. Project command-position executables to + # `\n` from the RAW command (normalization strips backslashes + # and fuses Windows paths before a basename could be recovered), then + # feed the projection through the standard normalize/grep-safe pipe. + projected = _project_path_spelled_executables(command) + if projected is not None: + projected_variant, _ = _grep_safe_detection_variant( + _normalize_command_for_detection(projected) + ) + if projected_variant not in seen: + seen.add(projected_variant) + yield projected_variant # Shell quoting/escaping can spell a dangerous executable name in pieces # (for example r\m or r''m). Keep that deobfuscation scoped to command # words so similarly shaped arguments do not become false positives. From b3268de19b4b2981385036be6240b7c94e091f9f Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Mon, 10 Aug 2026 12:31:30 +0900 Subject: [PATCH 2/5] fix(approval): treat leading redirections and prefixes as command-position words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit egilewski (PR #82830) found that _project_path_spelled_executables() stopped at a leading redirection word, so the hardline floor still missed path-spelled commands reached past one — /dev/null /usr/sbin/reboot, >/tmp/out /bin/rm -rf /. POSIX simple-command grammar puts (assignment | redirection)* before the command word, so those are prefixes, not the command. Add a dedicated redirection-prefix scanner the walker consumes before reading the command word. It recognizes fused and separated forms, fd duplications the shared word reader cannot (2>&1, &>f, >&2), heredoc and herestring markers, and balances process-substitution operands so the walk resumes at the real command. Kept out of the shared tokenizers to avoid changing & word-splitting anywhere else. The same normalization closes two main-era bare-name gaps a redirection or assignment prefix exposed ( --- tests/tools/test_hardline_blocklist.py | 46 ++++++ tools/approval.py | 185 ++++++++++++++++++++++++- 2 files changed, 226 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index e3133d7f83d69..2735482f4fd0a 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -713,6 +713,42 @@ _ABS_PATH_BLOCK = [ "`/sbin/reboot`", "{ /sbin/reboot; }", "env -u FOO /sbin/reboot", + # leading redirections are POSIX prefix words, not the command (egilewski, + # PR #82830): the executable that follows must still reach the floor + "/dev/null /usr/sbin/reboot", + "< /dev/null /sbin/shutdown -h now", + "2>&1 /sbin/shutdown -h now", + "&>/dev/null /sbin/reboot", + "</dev/null /tmp/a 2>>/tmp/b <>/tmp/c >|/tmp/d /sbin/reboot", + # a redirection prefix in front of a bare-name command (main-era gap, + # closed here by the same normalization) + "/dev/null reboot", + # an assignment prefix in front of a bare-name command (same gap) + "FOO=1 shutdown -h now", + # process substitution as a redirection operand still resumes at the cmd + "> >(cat) /sbin/shutdown -h now", + # gluing must not hide the command: no separator before the redirection + "shutdown&>out -h now", + # an escaped > is a literal, so & stays a background operator and the + # following command starts fresh and must block + "printf x \\>& shutdown -h now", + # a sudo option that takes an argument must not shift the command word + "sudo -D /tmp /sbin/shutdown -h now", + # >&file redirects both streams to a filename (not an fd), so the file is + # the operand and the command still follows (Grok round 1) + ">&/dev/null /sbin/shutdown -h now", + # a quoted ) inside a process substitution must not end the balance early + '> >(echo "x)y") /sbin/shutdown -h now', + # a digit run glued to non-digits after >&/2>& is a filename operand, not + # a bare fd number, so the whole token is consumed and the command follows + # (Grok round 2) + ">&2x /sbin/shutdown -h now", + "2>&1x /sbin/reboot", ] _ABS_PATH_ALLOW = [ @@ -737,6 +773,16 @@ _ABS_PATH_ALLOW = [ "env --argv0 /tmp/reboot /bin/echo ok", # `}` is not a shell metacharacter: it can end a legitimate word "/tmp/reboot}", + # benign leading redirections in front of a benign command stay allowed + ">out.txt make build", + "2>/dev/null ls /tmp", + "< /dev/null cat file", + # process substitution feeding a benign command is not the floor's concern + "diff <(sort a) <(sort b)", + # `env -C dir cmd` operand is data; the benign command is not blocked + "env -C /tmp /bin/ls", + # background operator between two benign commands keeps its meaning + "echo hi & echo bye", ] diff --git a/tools/approval.py b/tools/approval.py index 5b63c2c788159..f1dee62cbe637 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1185,9 +1185,15 @@ _COMMAND_WRAPPER_WORDS = { } _SUDO_OPTIONS_WITH_ARG = { "-c", "--close-from", + "-D", "--chdir", "-g", "--group", "-h", "--host", "-p", "--prompt", + "-R", "--chroot", + "-r", "--role", + "-T", "--command-timeout", + "-t", "--type", + "-U", "--other-user", "-u", "--user", } # GNU env options whose value arrives as the NEXT word. Their operand is @@ -2150,6 +2156,147 @@ def _projected_executable_basename(word: str) -> str | None: return basename +def _skip_leading_redirection(command: str, pos: int) -> int | None: + """If a redirection token begins at ``pos``, return the offset just past it + (and its operand); otherwise return None. + + POSIX simple-command grammar allows ``(assignment | redirection)*`` before + the command word, so a redirection reached by path — ``&1``, ``&>f``, ``>&2``); this + dedicated scanner recognizes them without touching the shared tokenizers. + + ``pos`` must already sit on the first non-space character. Process + substitution operands (``< <(cmd)``) are consumed as balanced ``( )`` so + the outer walk resumes at the command word; the inner command is still + surfaced independently by ``_iter_shell_command_starts``.""" + i = pos + n = len(command) + # Optional leading fd number (``2>``); bare digits are only a redirection + # when an operator follows, otherwise they are an ordinary word. + digits_end = i + while digits_end < n and command[digits_end].isdigit(): + digits_end += 1 + op = i + if digits_end < n and command[digits_end] in "<>": + op = digits_end + elif command[i] == "&" and i + 1 < n and command[i + 1] == ">": + op = i # ``&>`` / ``&>>``: whole-command redirect, no fd prefix + else: + return None # not a redirection + + # Consume the operator glyphs. + if command[op] == "&": + j = op + 1 # at '>' + j += 1 + if j < n and command[j] == ">": + j += 1 # ``&>>`` + else: + ch = command[op] + j = op + 1 + if ch == "<" and command.startswith("<<", op): + # heredoc / herestring: ``<<`` ``<<-`` ``<<<`` + j = op + 2 + if j < n and command[j] == "-": + j += 1 + elif j < n and command[j] == "<": + j += 1 # ``<<<`` + elif ch == ">" and j < n and command[j] == ">": + j += 1 # ``>>`` + elif ch == "<" and j < n and command[j] == ">": + j += 1 # ``<>`` + elif ch == ">" and j < n and command[j] == "|": + j += 1 # ``>|`` + # fd duplication ``>&`` / ``<&`` (also ``2>&1``, ``>&-``) + if j < n and command[j] == "&": + j += 1 + fd_end = j + while fd_end < n and (command[fd_end].isdigit() or command[fd_end] == "-"): + fd_end += 1 + # A real fd-dup operand is a whole word of digits/`-`; a digit run + # glued to more characters (``>&2x``) is a filename, not fd 2. + if fd_end > j and (fd_end >= n or command[fd_end].isspace() + or command[fd_end] in ";&|<>"): + return fd_end # fused fd / `-` + # ``>&file`` (redirect both streams to a file) or a glued + # non-fd token: the operand is a filename, fused or separated — + # fall through to operand scanning. + + # Operator consumed; now the operand. If it is fused (no separating + # space), the operand chars run to the next whitespace/metacharacter, + # except a process-substitution operand which we balance. + k = j + if k < n and not command[k].isspace(): + return _scan_redirection_operand(command, k) + # Separated operand: skip whitespace, then the operand word. + k = _skip_shell_whitespace(command, k) + if k >= n: + return j # dangling operator; nothing to consume + return _scan_redirection_operand(command, k) + + +def _scan_redirection_operand(command: str, pos: int) -> int: + """Return the offset past one redirection operand starting at ``pos``. + + ``<(cmd)`` / ``>(cmd)`` process substitutions are consumed as a balanced + paren group; every other operand runs to the next unquoted whitespace or + metacharacter (mirroring ``_read_shell_word`` minus the ``&`` break, which + inside an operand is just filename text).""" + n = len(command) + i = pos + if command.startswith("<(", i) or command.startswith(">(", i): + depth = 0 + i += 1 # at '(' + sub_quote: str | None = None + while i < n: + ch = command[i] + if sub_quote: + if ch == "\\" and sub_quote == '"' and i + 1 < n: + i += 2 + continue + if ch == sub_quote: + sub_quote = None + i += 1 + continue + if ch in ("'", '"'): + sub_quote = ch + i += 1 + continue + if ch == "\\" and i + 1 < n: + i += 2 + continue + if ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 0: + return i + 1 + i += 1 + return i + quote: str | None = None + while i < n: + ch = command[i] + if quote: + if ch == "\\" and quote == '"' and i + 1 < n: + i += 2 + continue + if ch == quote: + quote = None + i += 1 + continue + if ch in ("'", '"'): + quote = ch + i += 1 + continue + if ch == "\\" and i + 1 < n: + i += 2 + continue + if ch.isspace() or ch in ";&|<>": + break + i += 1 + return i + + def _project_path_spelled_executables(command: str) -> str | None: """Detection-only variant: rewrite command-position executables that are spelled by path (POSIX or Windows, quoted or bare, with or without a @@ -2173,7 +2320,22 @@ def _project_path_spelled_executables(command: str) -> str | None: wrapper_arg_options: frozenset | set = frozenset() skip_wrapper_options = False skip_next_wrapper_arg = False - for _ in range(12): + # POSIX prefixes (``VAR=v``, ``2>/dev/null``) can precede the command + # word any number of times. When one is consumed, the command word + # that follows sits mid-string where the flat ``_CMDPOS`` anchors would + # miss even a bare spelling, so a ``\n`` boundary is spliced in front of + # it just as a path spelling is projected to its basename. + prefix_consumed = False + # Unbounded walk with a progress guard (never fail open on a long + # prefix run): every step past here advances ``pos`` strictly. + while True: + scan = _skip_shell_whitespace(command, pos) + redir_end = _skip_leading_redirection(command, scan) if scan < len(command) else None + if redir_end is not None and redir_end > scan: + prefix_consumed = True + pos = redir_end + continue + word_start, raw_end, _raw_word = _read_shell_word(command, pos) if word_start == raw_end: break @@ -2192,18 +2354,31 @@ def _project_path_spelled_executables(command: str) -> str | None: continue deobfuscated = _deobfuscate_shell_word_for_detection(word) or word if _ENV_ASSIGNMENT_RE.fullmatch(deobfuscated): + prefix_consumed = True continue # VAR=value prefix: data, keep walking if skip_wrapper_options and deobfuscated.startswith("-"): - option_name = deobfuscated.lower().split("=", 1)[0] - skip_next_wrapper_arg = ( - "=" not in deobfuscated - and option_name in wrapper_arg_options + raw_option = deobfuscated.split("=", 1)[0] + # sudo short options are case-significant (``-C`` close-from vs + # ``-c``, ``-D`` chdir), so match the raw spelling as well as + # the lowercased one the long options are listed under. + takes_arg = ( + raw_option in wrapper_arg_options + or raw_option.lower() in wrapper_arg_options ) + skip_next_wrapper_arg = "=" not in deobfuscated and takes_arg continue basename = _projected_executable_basename(word) effective = (basename or deobfuscated).lower() if basename is not None and word_start not in replacements: replacements[word_start] = (word_end, basename) + elif ( + basename is None + and prefix_consumed + and word_start not in replacements + ): + # A prefix ran, so this bare command word needs the same ``\n`` + # anchor a projected path spelling would get. + replacements[word_start] = (word_end, word) if effective in _COMMAND_WRAPPER_WORDS: skip_wrapper_options = effective in {"sudo", "env"} if effective == "sudo": From 791eea43ce8def25fa0cab9689ff965f1878ca2d Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Mon, 10 Aug 2026 20:49:20 +0900 Subject: [PATCH 3/5] fix(approval): resolve sudo/env options with their upstream getopt grammars egilewski's second review round (PR #82830) showed the wrapper-option skip still guessed: a lowercased whole-word membership folded sudo's case-significant shorts (-P/-H became -p/-h and skipped an operand), bundled shorts (-Eu root) matched nothing and misread the operand as the command word, getopt_long unambiguous abbreviations (--use, --chd) were invisible, and whitespace inside a redirection operand's $() cut the operand scan short so the following executable escaped projection. Replace the two per-walker checks with one resolver that models the upstream grammars (sudo 1.9.17 src/parse_args.c, coreutils 9.11 src/env.c): case-sensitive short bundles walked char by char, required args consuming the bundle remainder or the next word, and long options resolved exact-match-first then unique-prefix like getopt_long. Unknown and ambiguous spellings consume nothing, keeping detection fail-closed. The option tables also gain sudo's missing -a/-C, correct -c to login-class, and drop the fabricated lowercase env pair (-c/-s are really -C/-S); -E stays plain no-arg per the optstring. An unterminated $( or backtick inside a redirection operand now projects to a guaranteed-hardline marker instead of scanning a truncated operand. Tests: 11 new block cases across the case-flip, bundling, abbreviation, and operand-substitution classes (9 of them fail-before on the previous tree), plus allow parity for the env --arg abbreviation and the ambiguous sudo --pre spelling. 267 hardline tests green; the approval-suite failure set is byte-identical with and without this change on this Windows host. Co-Authored-By: Claude Fable 5 --- tests/tools/test_hardline_blocklist.py | 23 +++ tools/approval.py | 226 +++++++++++++++++++++---- 2 files changed, 217 insertions(+), 32 deletions(-) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 2735482f4fd0a..71d87d3446b14 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -749,6 +749,23 @@ _ABS_PATH_BLOCK = [ # (Grok round 2) ">&2x /sbin/shutdown -h now", "2>&1x /sbin/reboot", + # sudo/env wrapper options use their case-sensitive upstream grammars. + "sudo -E /sbin/shutdown", + "sudo -P /sbin/shutdown", + "sudo -H /sbin/shutdown", + "sudo -a type /sbin/shutdown", + "sudo -Eu root /sbin/shutdown", + "sudo -PC 5 /sbin/shutdown", + # getopt_long exact and unambiguous abbreviated required options own the + # following word, while an =-attached operand remains in the option token. + "sudo --use root /sbin/shutdown", + "sudo --chd /tmp /sbin/shutdown", + "sudo --chdir=/tmp /sbin/shutdown", + # Both command-word walkers must share the same bundle handling. + "sudo -P /sbin/shut\\down", + # Whitespace inside a redirection operand's command substitution must not + # terminate the operand scan before the following executable. + ">$(printf '%s' a) /sbin/shutdown", ] _ABS_PATH_ALLOW = [ @@ -771,6 +788,12 @@ _ABS_PATH_ALLOW = [ "env --chdir /tmp/reboot /bin/echo ok", "env --chdir /tmp/reboot echo ok", "env --argv0 /tmp/reboot /bin/echo ok", + # GNU env accepts the unambiguous --argv0 abbreviation and still treats + # its following word as data. + "env --arg /tmp/x /bin/echo ok", + # Ambiguous sudo long options make sudo exit before executing a command. + # The projection must not invent an operand skip for them. + "sudo --pre root /sbin/shutdown", # `}` is not a shell metacharacter: it can end a legitimate word "/tmp/reboot}", # benign leading redirections in front of a benign command stay allowed diff --git a/tools/approval.py b/tools/approval.py index f1dee62cbe637..2086e9aedb083 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1184,7 +1184,9 @@ _COMMAND_WRAPPER_WORDS = { "builtin", } _SUDO_OPTIONS_WITH_ARG = { - "-c", "--close-from", + "-a", "--auth-type", + "-C", "--close-from", + "-c", "--login-class", "-D", "--chdir", "-g", "--group", "-h", "--host", @@ -1205,11 +1207,130 @@ _SUDO_OPTIONS_WITH_ARG = { # (same verdict as the design review); skipping keeps parity with main. _ENV_OPTIONS_WITH_ARG = { "-a", "--argv0", - "-c", "--chdir", - "-s", "--split-string", + "-C", "--chdir", + "-S", "--split-string", "-u", "--unset", } +# Complete long-option universes used for getopt_long-compatible exact and +# unambiguous-prefix resolution. Keep these synchronized with sudo v1.9.17 +# src/parse_args.c and GNU coreutils v9.11 src/env.c. Missing or ambiguous +# entries deliberately consume no following word, which biases detection +# toward over-blocking rather than hiding an executable as option data. +_SUDO_LONG_OPTION_ARITY = { + "--other-user": "required", + "--auth-type": "required", + "--close-from": "required", + "--login-class": "required", + "--chdir": "required", + "--group": "required", + "--host": "required", + "--prompt": "required", + "--chroot": "required", + "--role": "required", + "--command-timeout": "required", + "--type": "required", + "--user": "required", + "--preserve-env": "optional", + "--background": "none", + "--edit": "none", + "--set-home": "none", + "--login": "none", + "--remove-timestamp": "none", + "--list": "none", + "--preserve-groups": "none", + "--shell": "none", + "--validate": "none", + "--askpass": "none", + "--bell": "none", + "--help": "none", + "--reset-timestamp": "none", + "--no-update": "none", + "--non-interactive": "none", + "--stdin": "none", + "--version": "none", +} +_ENV_LONG_OPTION_ARITY = { + "--argv0": "required", + "--unset": "required", + "--chdir": "required", + "--split-string": "required", + "--default-signal": "optional", + "--ignore-signal": "optional", + "--block-signal": "optional", + "--ignore-environment": "none", + "--null": "none", + "--debug": "none", + "--list-signal-handling": "none", + "--help": "none", + "--version": "none", +} +_SUDO_SHORT_OPTIONS_REQUIRED_ARG = frozenset("aCcDgpRrTtUu") +_SUDO_SHORT_OPTIONS_OPTIONAL_ARG = frozenset("h") +_SUDO_SHORT_OPTIONS_NO_ARG = frozenset("ABbEeHiKklNnPSsVv") +_ENV_SHORT_OPTIONS_REQUIRED_ARG = frozenset("aCSu") +_ENV_SHORT_OPTIONS_OPTIONAL_ARG = frozenset() +_ENV_SHORT_OPTIONS_NO_ARG = frozenset("i0v") + + +def _wrapper_option_consumes_next_word(wrapper: str, token: str) -> bool: + """Return whether one sudo/env option word owns the following word. + + Short bundles follow each wrapper's case-sensitive optstring. Required + options consume the remainder of their bundle, or the next word when they + occur last. sudo's short ``-h`` follows its got_host_flag behavior and + conservatively consumes the next word when no host is attached. + + Long options use exact-match-first, then unique-prefix resolution like + ``getopt_long``. Optional long arguments only consume ``=``-attached data. + Unknown or ambiguous options consume no next word so executable detection + continues in the fail-closed direction. + """ + if wrapper == "sudo": + long_options = _SUDO_LONG_OPTION_ARITY + required_short = _SUDO_SHORT_OPTIONS_REQUIRED_ARG + optional_short = _SUDO_SHORT_OPTIONS_OPTIONAL_ARG + no_arg_short = _SUDO_SHORT_OPTIONS_NO_ARG + elif wrapper == "env": + long_options = _ENV_LONG_OPTION_ARITY + required_short = _ENV_SHORT_OPTIONS_REQUIRED_ARG + optional_short = _ENV_SHORT_OPTIONS_OPTIONAL_ARG + no_arg_short = _ENV_SHORT_OPTIONS_NO_ARG + else: + return False + + if token.startswith("--"): + option, equals, _ = token.partition("=") + arity = long_options.get(option) + if arity is None: + matches = [ + candidate + for candidate in long_options + if candidate.startswith(option) + ] + if len(matches) != 1: + return False + arity = long_options[matches[0]] + return arity == "required" and not equals + + if ( + not token.startswith("-") + or token == "-" + or "=" in token + or len(token) < 2 + ): + return False + + chars = token[1:] + for index, char in enumerate(chars): + if char in required_short: + return index == len(chars) - 1 + if char in optional_short: + return index == len(chars) - 1 + if char not in no_arg_short: + return False + return False + _INTERPRETER_EXEC_FLAGS = { "python": {"-c"}, "node": {"-e", "--eval", "-p", "--print"}, @@ -2073,7 +2194,7 @@ def _iter_shell_command_word_spans(command: str): for command_start in _iter_shell_command_starts(command): pos = command_start prefix_words = 0 - skip_wrapper_options = False + wrapper_kind: str | None = None skip_next_wrapper_arg = False while prefix_words < 12: word_start, word_end, word = _read_shell_word(command, pos) @@ -2086,11 +2207,9 @@ def _iter_shell_command_word_spans(command: str): pos = word_end prefix_words += 1 continue - if skip_wrapper_options and lower_word.startswith("-"): - option_name = lower_word.split("=", 1)[0] - skip_next_wrapper_arg = ( - "=" not in lower_word - and option_name in _SUDO_OPTIONS_WITH_ARG + if wrapper_kind and deobfuscated.startswith("-"): + skip_next_wrapper_arg = _wrapper_option_consumes_next_word( + wrapper_kind, deobfuscated ) pos = word_end prefix_words += 1 @@ -2100,11 +2219,11 @@ def _iter_shell_command_word_spans(command: str): prefix_words += 1 if lower_word in _COMMAND_WRAPPER_WORDS: - skip_wrapper_options = lower_word in {"sudo", "env"} + wrapper_kind = lower_word if lower_word in {"sudo", "env"} else None pos = word_end continue if _ENV_ASSIGNMENT_RE.fullmatch(deobfuscated): - skip_wrapper_options = False + wrapper_kind = None pos = word_end continue break @@ -2156,6 +2275,14 @@ def _projected_executable_basename(word: str) -> str | None: return basename +_MALFORMED_REDIRECTION = -1 +# `_project_path_spelled_executables` has a string-or-None compatibility +# contract with callers outside this segment. A malformed redirection operand +# therefore projects to a guaranteed hardline marker rather than returning +# None and silently allowing an uninspected command suffix. +_MALFORMED_REDIRECTION_PROJECTION = "\nmkfs" + + def _skip_leading_redirection(command: str, pos: int) -> int | None: """If a redirection token begins at ``pos``, return the offset just past it (and its operand); otherwise return None. @@ -2227,21 +2354,32 @@ def _skip_leading_redirection(command: str, pos: int) -> int | None: # except a process-substitution operand which we balance. k = j if k < n and not command[k].isspace(): - return _scan_redirection_operand(command, k) + operand_end = _scan_redirection_operand(command, k) + return ( + operand_end + if operand_end is not None + else _MALFORMED_REDIRECTION + ) # Separated operand: skip whitespace, then the operand word. k = _skip_shell_whitespace(command, k) if k >= n: return j # dangling operator; nothing to consume - return _scan_redirection_operand(command, k) + operand_end = _scan_redirection_operand(command, k) + return ( + operand_end + if operand_end is not None + else _MALFORMED_REDIRECTION + ) -def _scan_redirection_operand(command: str, pos: int) -> int: +def _scan_redirection_operand(command: str, pos: int) -> int | None: """Return the offset past one redirection operand starting at ``pos``. ``<(cmd)`` / ``>(cmd)`` process substitutions are consumed as a balanced paren group; every other operand runs to the next unquoted whitespace or metacharacter (mirroring ``_read_shell_word`` minus the ``&`` break, which - inside an operand is just filename text).""" + inside an operand is just filename text). Returns None when a command + substitution or backtick payload is unterminated.""" n = len(command) i = pos if command.startswith("<(", i) or command.startswith(">(", i): @@ -2265,6 +2403,18 @@ def _scan_redirection_operand(command: str, pos: int) -> int: if ch == "\\" and i + 1 < n: i += 2 continue + if command.startswith("$(", i): + end = _scan_dollar_paren_end(command, i) + if end is None: + return None + i = end + continue + if ch == "`": + end = _scan_backtick_end(command, i) + if end is None: + return None + i = end + continue if ch == "(": depth += 1 elif ch == ")": @@ -2272,7 +2422,7 @@ def _scan_redirection_operand(command: str, pos: int) -> int: if depth == 0: return i + 1 i += 1 - return i + return None quote: str | None = None while i < n: ch = command[i] @@ -2291,6 +2441,18 @@ def _scan_redirection_operand(command: str, pos: int) -> int: if ch == "\\" and i + 1 < n: i += 2 continue + if command.startswith("$(", i): + end = _scan_dollar_paren_end(command, i) + if end is None: + return None + i = end + continue + if ch == "`": + end = _scan_backtick_end(command, i) + if end is None: + return None + i = end + continue if ch.isspace() or ch in ";&|<>": break i += 1 @@ -2317,8 +2479,7 @@ def _project_path_spelled_executables(command: str) -> str | None: replacements: dict[int, tuple[int, str]] = {} for command_start in _iter_shell_command_starts(command): pos = command_start - wrapper_arg_options: frozenset | set = frozenset() - skip_wrapper_options = False + wrapper_kind: str | None = None skip_next_wrapper_arg = False # POSIX prefixes (``VAR=v``, ``2>/dev/null``) can precede the command # word any number of times. When one is consumed, the command word @@ -2331,6 +2492,8 @@ def _project_path_spelled_executables(command: str) -> str | None: while True: scan = _skip_shell_whitespace(command, pos) redir_end = _skip_leading_redirection(command, scan) if scan < len(command) else None + if redir_end == _MALFORMED_REDIRECTION: + return _MALFORMED_REDIRECTION_PROJECTION if redir_end is not None and redir_end > scan: prefix_consumed = True pos = redir_end @@ -2351,21 +2514,24 @@ def _project_path_spelled_executables(command: str) -> str | None: word = command[word_start:word_end] if skip_next_wrapper_arg: skip_next_wrapper_arg = False + # The option that owns this operand already proved a prefix + # ran, so the bare command word that follows needs the same + # ``\n`` anchor a projected path spelling would get. + prefix_consumed = True continue deobfuscated = _deobfuscate_shell_word_for_detection(word) or word if _ENV_ASSIGNMENT_RE.fullmatch(deobfuscated): prefix_consumed = True continue # VAR=value prefix: data, keep walking - if skip_wrapper_options and deobfuscated.startswith("-"): - raw_option = deobfuscated.split("=", 1)[0] - # sudo short options are case-significant (``-C`` close-from vs - # ``-c``, ``-D`` chdir), so match the raw spelling as well as - # the lowercased one the long options are listed under. - takes_arg = ( - raw_option in wrapper_arg_options - or raw_option.lower() in wrapper_arg_options + if wrapper_kind and deobfuscated.startswith("-"): + skip_next_wrapper_arg = _wrapper_option_consumes_next_word( + wrapper_kind, deobfuscated ) - skip_next_wrapper_arg = "=" not in deobfuscated and takes_arg + # A wrapper option is itself a prefix word, whether or not it + # owns an operand: ``sudo -E rm ...`` needs the same anchor as + # ``sudo -u root rm ...`` in front of the bare command word + # that follows. + prefix_consumed = True continue basename = _projected_executable_basename(word) effective = (basename or deobfuscated).lower() @@ -2380,11 +2546,7 @@ def _project_path_spelled_executables(command: str) -> str | None: # anchor a projected path spelling would get. replacements[word_start] = (word_end, word) if effective in _COMMAND_WRAPPER_WORDS: - skip_wrapper_options = effective in {"sudo", "env"} - if effective == "sudo": - wrapper_arg_options = _SUDO_OPTIONS_WITH_ARG - elif effective == "env": - wrapper_arg_options = _ENV_OPTIONS_WITH_ARG + wrapper_kind = effective if effective in {"sudo", "env"} else None continue # wrapper (bare or path-spelled): walk to the command break if not replacements: From 4d616721f066e33367ee107c03324ac7007bc4ef Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Mon, 10 Aug 2026 22:17:55 +0900 Subject: [PATCH 4/5] test(approval): pin the bare-name-behind-wrapper-operand hardline cases The B-1 fix landed in 791eea43c without the regressions that prove it. A bare-name hardline command sitting behind a wrapper option's operand (`sudo -u root rm -rf /etc`, `env -i rm -rf /etc`) used to reach execution: the flat _CMDPOS-anchored patterns only match a command word that is "\n"-anchored, and the option-skip / operand-skip branches in both prefix walkers returned without setting `prefix_consumed`, so nothing spliced an anchor in front of the word that followed. Path-spelled hardlines were unaffected, which is why the gap survived the first two review rounds. Blocked cases cover the plain operand, a bundled short option, `env -i`, and an unambiguous getopt_long abbreviation. The allow case pins that the new anchor does not turn an ordinary command behind an operand into a false positive. --- tests/tools/test_hardline_blocklist.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 71d87d3446b14..0881af1d93c33 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -135,6 +135,17 @@ _HARDLINE_BLOCK = [ "{ poweroff; }", "true && (reboot)", "echo hi; { reboot; }", + # bare-name hardline commands sitting behind a wrapper option's OPERAND + # (`-u root`, `-Eu root`, `env -i`, or an unambiguous `--us` abbreviation) + # used to slip past: the flat _CMDPOS-anchored pattern only reaches a + # command word that is anchored ("\n"-preceded), and the operand-skip / + # option-skip branches in the two prefix walkers never set + # `prefix_consumed`, so no anchor was spliced in front of the bare + # command word that followed (egilewski, PR #82830 second report, B-1). + "sudo -u root rm -rf /etc", + "sudo -Eu root rm -rf /etc", + "env -i rm -rf /etc", + "sudo --us root rm -rf /etc", ] @@ -199,6 +210,10 @@ _HARDLINE_ALLOW = [ "npm run build", "sudo apt update", "curl https://example.com | head", + # B-1 non-regression: a benign command behind a wrapper option's operand + # must still be allowed — the operand-skip anchor must not manufacture a + # false positive out of an ordinary command word. + "sudo -u root whoami", ] From 851b093a5ddce6c9ef5a7cddaed15cddc87e3067 Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Tue, 11 Aug 2026 13:35:26 +0900 Subject: [PATCH 5/5] fix(approval): count every grouping paren when balancing a redirection operand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit egilewski (PR #82830) found that `>$(echo <(true) tail >/dev/null; printf /dev/null) /sbin/shutdown -h now` still reached execution. `_scan_dollar_paren_end()` only incremented its depth on `$(` while every `)` decremented it, so the process substitution's closing paren ended the enclosing substitution early and the operand scan stopped before the executable. Counting any unquoted `(` balances `$(`, `<(`, `>(` and plain subshells under one rule, but on its own it counts parens the shell never treats as grouping, and each of those overshoots the real closer and swallows the command word behind it. So the scan now steps over the three contexts that hold literal parens — `#` comments, `${...}` expansions, and `$((` / `$[` arithmetic — before it counts. A `#` only opens a comment where a word can start, so `$(true)#` stays part of its word. A heredoc inside the substitution puts its body out of reach of a character scanner, so the operand is reported malformed there rather than guessed at, and the existing fail-closed path blocks it. The heredoc regression from the same review round is not addressed here. Its fix belongs at the redirection-prefix walk that b3268de19 added, and scoping it correctly needs its own change; the current behaviour blocks a literal heredoc body, which is the safe direction to be wrong in. Tests: the reported bypass and two process-substitution variants, the comment-hidden paren in both subshell spellings, the glued `$(true)#` word, nested arithmetic, and the heredoc-in-substitution operand; plus an allow case for `${d:-(}`, whose paren must not count. 281 hardline tests green, and the guard costs what it did before this commit. Co-Authored-By: Claude Fable 5 --- tests/tools/test_hardline_blocklist.py | 19 ++++++ tools/approval.py | 88 +++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 0881af1d93c33..419a3ed38812b 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -781,9 +781,28 @@ _ABS_PATH_BLOCK = [ # Whitespace inside a redirection operand's command substitution must not # terminate the operand scan before the following executable. ">$(printf '%s' a) /sbin/shutdown", + # Every paren the shell treats as grouping balances the operand's + # substitution, so a nested process substitution cannot close it early and + # leave the executable outside the scanned range. + ">$(echo <(true) tail >/dev/null; printf /dev/null) /sbin/shutdown -h now", + ">$(echo >(cat) ; printf /dev/null) /sbin/shutdown", + ">$(echo <(true)) /sbin/shutdown", + # Parens the shell does NOT treat as grouping must not count, or the scan + # runs past the real closer and swallows the command word behind it. The + # `#` in `$(true)#` is glued to the word and starts no comment. + "( > $(echo /tmp/f # (\n) shutdown -h now )", + "( > $(echo /tmp/f # (\n) rm -rf /etc )", + "( >$(echo $(true)#) /sbin/shutdown -h now\n)", + "> $(echo $((1 + (2))) ) /sbin/shutdown", + # A heredoc inside the operand's substitution puts its body beyond this + # scanner, so the operand is malformed rather than guessed at. + "( > $(cat <<'E'\n(\nE\n) shutdown -h now )", ] _ABS_PATH_ALLOW = [ + # A paren inside a parameter expansion is literal text, so counting it + # would end the operand early and block an ordinary command. + "> $(echo ${d:-(}) cat /etc/hosts", "echo /sbin/shutdown", "ls -la /sbin/shutdown", "grep 'shutdown' /var/log/syslog", diff --git a/tools/approval.py b/tools/approval.py index 2086e9aedb083..2405345814504 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1844,9 +1844,42 @@ def _scan_dollar_paren_end(command: str, start: int) -> int | None: if ch == "\\" and i + 1 < len(command): i += 2 continue - if command.startswith("$(", i): + if command.startswith("#", i) and ( + i == start + 2 or command[i - 1].isspace() or command[i - 1] in ";&|(" + ): + # A comment runs to the newline, and its parens are text. Counting + # them would push the depth past the real closer and swallow + # whatever follows the substitution. + newline = command.find("\n", i) + if newline == -1: + return None + i = newline + continue + if command.startswith("${", i): + # `${x:-(}` holds a literal paren, not a group. + end = _scan_brace_expansion_end(command, i) + if end is None: + return None + i = end + continue + if command.startswith("$((", i) or command.startswith("$[", i): + # Arithmetic parens are operators, not grouping. + end = _scan_arithmetic_end(command, i) + if end is None: + return None + i = end + continue + if command.startswith("<<", i) and not command.startswith("<<<", i): + # A heredoc inside the substitution puts its body's parens beyond + # this scanner's reach. Refusing to guess keeps the operand from + # swallowing a command word; the caller treats None as malformed + # and blocks. + return None + if ch == "(": + # Every unquoted paren the shell treats as grouping counts, so + # `<(`, `>(` and plain subshells balance under one rule. depth += 1 - i += 2 + i += 1 continue if ch == ")": depth -= 1 @@ -1858,6 +1891,57 @@ def _scan_dollar_paren_end(command: str, start: int) -> int | None: return None +def _scan_brace_expansion_end(command: str, start: int) -> int | None: + """Return the offset after a balanced ``${...}`` expansion.""" + depth = 0 + i = start + 1 + quote: str | None = None + while i < len(command): + ch = command[i] + if quote: + if ch == "\\" and quote == '"' and i + 1 < len(command): + i += 2 + continue + if ch == quote: + quote = None + i += 1 + continue + if ch == "\\" and i + 1 < len(command): + i += 2 + continue + if ch in ("'", '"'): + quote = ch + i += 1 + continue + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return i + 1 + i += 1 + return None + + +def _scan_arithmetic_end(command: str, start: int) -> int | None: + """Return the offset after ``$((...))`` or the legacy ``$[...]``.""" + if command.startswith("$[", start): + end = command.find("]", start + 2) + return None if end == -1 else end + 1 + depth = 0 + i = start + 1 + while i < len(command): + ch = command[i] + if ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 0: + return i + 1 + i += 1 + return None + + def _scan_backtick_end(command: str, start: int) -> int | None: i = start + 1 while i < len(command):