fix(security): rm floor reads reserved words, empty substitutions and ${...} spans

docs/system-specs/modules/security.md

@@ -1659,7 +1659,7 @@ command. Local assignments remain available across the preserved separators.
 The existing deny catalog, nested-payload checks and real self-kill checks
 continue to apply.
 
-**Recursive-force `rm` floor (`rm_floor.py`)**: a small seatbelt, not a parser. The two `local-destructive-rm-rf-*` rows match one contiguous spelling; this also refuses the common respellings. It cuts the text where the shared `_iter_shell_chars` marks `;` `&` `|` newline backtick or paren live (so quoted text stays one word) skips a `#` comment to its newline and restarts unquoted there, reads a `$(…)` or a backtick body inside double quotes on its own from an unquoted start (the `$(…)` span from the shared `_matching_close_paren`, the backtick span up to the next unescaped backtick) and resumes the double quote after it, starts a new word at a live `<` or `>` (so a redirection glued to an operand is its own word), keeps an escaped or single-quoted `$` and any quoted `~` literal, and stops at a live `<<` (heredoc bodies are data), reads each segment's words with `_shell_tokens` (quotes off, nothing expanded), skips leading `VAR=val` words, and denies when the program basename is `rm`, the flags hold recursive (`r`, `R`, `--recursive`) AND force (`f`, `--force`), and an operand is exactly root (`/`, `//`, `/*`) or home (`~`, `$HOME`, `${HOME}`, any quoting, with an optional trailing `/` or `/*`). It runs only when the text holds a literal `rm` (quote splices allowed); a floor error fails closed with `_RM_FLOOR_UNREAD_NOTE` for a command that names `rm` as a word of its own (`_names_rm_word`: not inside a longer word such as `format`, checked on the raw text and with `${X:-…}` defaults resolved), and lets any other command pass. A refusal names the row and carries the `argv-floor` component. It runs only after every regex view cleared, and each target only while its row is enabled. Out of scope by design, left to the rows and the OS sandbox: descendants (`rm -fr /tmp/x` and `rm -rf $HOME/.cache` stay allowed), heredocs, arrays, arithmetic, `sudo` and other wrappers, `bash -c` payloads and parameter-expansion defaults. Some contrived shapes are over-refused (a variable named `rm`, a quoted `'/*'`); that is accepted rather than read further.
+**Recursive-force `rm` floor (`rm_floor.py`)**: a small seatbelt, not a parser. The two `local-destructive-rm-rf-*` rows match one contiguous spelling; this also refuses the common respellings. It cuts the text where the shared `_iter_shell_chars` marks `;` `&` `|` newline backtick or paren live (so quoted text stays one word) skips a `#` comment to its newline and restarts unquoted there, reads a `$(…)` or a backtick body inside double quotes on its own from an unquoted start (the `$(…)` span from the shared `_matching_close_paren`, the backtick span up to the next unescaped backtick) and resumes the double quote after it, starts a new word at a live `<` or `>` (so a redirection glued to an operand is its own word), keeps an escaped or single-quoted `$` and any quoted `~` literal, and stops at a live `<<` (heredoc bodies are data). Inside an open `${…}` or `$[…]` (opened by an odd run of live `$`, so `$${` is the PID and opens nothing) a `#` or `<<` is data rather than a comment or heredoc; because that tracking can misread a span, every command is also read once without it and the targets of both readings are joined, so the tracked reading can only add refusals, and each reading passes its mode to the nested substitution bodies so the work stays linear in nesting depth. It reads each segment's words with `_shell_tokens` (quotes off, nothing expanded), skips leading `VAR=val` words, the reserved words that keep command position (`shell_normalizer._KEEPS_COMMAND_POSITION`: `then`, `do`, `else`, `{`, `!`, `time` and its `-p`, and the rest) and a leading redirection with its target, drops a substitution that sits in the program word because it may expand to nothing (a substitution inside an operand stays part of that path), and denies when the program basename is `rm`, the flags hold recursive (`r`, `R`, `--recursive`) AND force (`f`, `--force`), and an operand is exactly root (`/`, `//`, `/*`) or home (`~`, `$HOME`, `${HOME}`, any quoting, with an optional trailing `/` or `/*`). It runs only when the text holds a literal `rm` (quote splices allowed); a floor error fails closed with `_RM_FLOOR_UNREAD_NOTE` for a command that names `rm` as a word of its own (`_names_rm_word`: not inside a longer word such as `format`, checked on the raw text and with `${X:-…}` defaults resolved), and lets any other command pass. A refusal names the row and carries the `argv-floor` component. It runs only after every regex view cleared, and each target only while its row is enabled. Out of scope by design, left to the rows and the OS sandbox: descendants (`rm -fr /tmp/x` and `rm -rf $HOME/.cache` stay allowed), heredocs, arrays, evaluating arithmetic or parameter-expansion defaults (their spans are only tracked as above), an unquoted `$(…)` or backtick inside the argv, `sudo` and other wrappers, and `bash -c` payloads. Some contrived shapes are over-refused (a variable named `rm`, a quoted `'/*'`, a quoted keyword such as `"then" rm -fr ~`, an `rm -fr ~` written inside a `${x:-…}` default); that is accepted rather than read further.
 
 **Quote-normalized segment view (`_deny_segment_views` + `_shell_tokens`, `security/`)** — both deny tiers match TEXT, while a shell strips quoting, de-escapes backslashes, resolves ANSI-C / locale quoting, collapses empty-string splices and collapses whitespace runs before the program ever sees its argv. A rule authored as a command SHAPE would therefore be defeated by re-spelling any single token: `rm -rf "/"`, `"rm" -rf /`, `rm "-rf" /`, `r''m -rf /`, `rm  -rf  /`, `rm -rf \/` and `rm -rf $'/'` all run exactly what `rm -rf /` runs, and none of them *contains* the rule's own text. Only the self-protection floor rules (`_SELF_PROTECTION_FLOOR_RULE_IDS`), git-publish and the two recursive-force `rm` rows (below) have an argv-structural floor closing this; the rest of the built-in catalog — including every destructive-operation and credential-exfiltration rule listed in this section — is spelling-dependent, so Pass 2 evaluates each segment in **two views**: the raw text first (byte-identical to what it matched before), then a quote/escape-normalized re-join, appended **only when it differs**, so an unquoted command pays no second pass over the catalog. Four properties are load-bearing:
 

src/kiro_crew/security/rm_floor.py

@@ -36,6 +36,7 @@
 #: :func:`_segments` keeps a redirection and its fd prefix in one word (below).
 _HOME_WORDS = frozenset({"~", "$HOME", "${HOME}"})
 _CUT_CHARS = frozenset(";&|\n`()")
+_SUBST = "\1"  # a substitution: may expand to nothing, unlike a literal ``\0``
 #: A STANDALONE fd prefix (a bare number or ``{name}``) directly before a redirection
 #: operator: ``2>`` and ``{fd}>`` redirect fd 2 / a named fd, but the ``2`` in
 #: ``/tmp/2>x`` is part of the path, so the prefix counts only at a word boundary.
@@ -75,27 +76,33 @@ def _target_kind(word: str) -> "str | None":
     return "home" if base in _HOME_WORDS else None
 
 
-def _segments(command: str) -> "list[str]":
+def _segments(command: str, track: bool = True) -> "list[str]":
     """*command* cut at separators the shared quote machine marks live; stops at a heredoc."""
     out: list[str] = []
     seg: list[str] = []
     in_redirect = False
+    #: closers of open ``${…}`` / ``$[…]``: a ``#`` or ``<<`` there is data, so
+    #: neither a comment nor a heredoc starts until they close
+    expansion: list[str] = []
     pos, state = 0, 0
     while pos < len(command):
         text, resume, resume_state = command[pos:], len(command), 0
+        dollars = 0  # live ``$`` run ending here: ``$$`` is the PID, never an opener
         for step in _shell._iter_shell_chars(text, state):
-            if step.active and text.startswith("<<", step.offset):
+            dollars = dollars + 1 if step.active and step.char == "$" else 0
+            live_syntax = step.active and not expansion
+            if live_syntax and text.startswith("<<", step.offset):
                 break  # heredoc bodies are data; the rows and the sandbox own them
-            if step.active and step.char == "#" and _shell._opens_comment(text, step.offset):
+            if live_syntax and step.char == "#" and _shell._opens_comment(text, step.offset):
                 newline = text.find("\n", step.offset)  # a comment's quotes open nothing
                 resume = len(command) if newline < 0 else pos + newline
                 break
             if step.state == 2 and text.startswith("$(", step.offset):
                 # bash reads "$(…)" from an unquoted start, then the double quote resumes
                 body = text[step.offset + 2 :]
                 close, proven = _shell._matching_close_paren(body, 0)
-                out.extend(_segments(body[: close - 1 if proven else close]))
-                seg.append("\0")
+                out.extend(_segments(body[: close - 1 if proven else close], track))
+                seg.append(_SUBST)
                 in_redirect = False
                 resume, resume_state = pos + step.offset + 2 + close, 2
                 break
@@ -104,11 +111,15 @@ def _segments(command: str) -> "list[str]":
                 closer = _shell._backtick_closer(text, step.offset + 1)
                 closed = closer != -1
                 end = closer + 1 if closed else len(text)
-                out.extend(_segments(text[step.offset + 1 : end - closed]))
-                seg.append("\0")
+                out.extend(_segments(text[step.offset + 1 : end - closed], track))
+                seg.append(_SUBST)
                 in_redirect = False
                 resume, resume_state = pos + end, 2
                 break
+            if track and dollars % 2 and text.startswith(("${", "$["), step.offset):
+                expansion.append("}" if text[step.offset + 1] == "{" else "]")
+            elif step.active and expansion and step.char == expansion[-1]:
+                expansion.pop()
             if step.active and in_redirect and step.char == "&":
                 seg.append(step.text)  # fd-dup operator (``2>&1``), not a command separator
             elif step.active and step.char in _CUT_CHARS:
@@ -138,11 +149,14 @@ def _segments(command: str) -> "list[str]":
 def _rm_wipe_targets(command: str) -> "set[str]":
     """The targets (``root`` / ``home``) a recursive-force ``rm`` in *command* wipes."""
     kinds: set[str] = set()
-    for segment in _segments(command):
+    # read with and without ``${…}`` tracking: the untracked reading is the
+    # base one, so tracking can only add refusals
+    for segment in [*_segments(command), *_segments(command, track=False)]:
         words = _shell._shell_tokens(segment)
         while words:
-            if ENV_ASSIGNMENT_RE.match(words[0]):
-                words = words[1:]
+            if ENV_ASSIGNMENT_RE.match(words[0]) or words[0] in _shell._KEEPS_COMMAND_POSITION:
+                # ``then`` / ``do`` / ``!`` / ``time`` … leave the next word the program
+                words = words[2:] if words[0] == "time" and words[1:2] == ["-p"] else words[1:]
                 continue
             is_redirection, consumes_next = _shell._push_token_redirection(words[0])
             if is_redirection:
@@ -152,6 +166,11 @@ def _rm_wipe_targets(command: str) -> "set[str]":
                 if consumes_next and words:
                     words = words[1:]
                 continue
+            if _SUBST in words[0]:
+                # a substitution in command position may expand to nothing
+                bare = words[0].replace(_SUBST, "")
+                words = [bare, *words[1:]] if bare else words[1:]
+                continue
             break
         if not words or _shell._program_basename(words[0]) != "rm":
             continue

test/test_denied_commands_security_rows.py

@@ -539,6 +539,44 @@ class TestRecursiveForceRmFloor:
             "> /dev/null RM -fr /",
             "X=1 Y=2 >log RM -rf /",
             "&>/dev/null RM -fr /",
+            # A reserved word or ``!`` / ``time`` leaves the next word the program.
+            "if :; then RM -fr ~; fi",
+            "if false; then :; else RM -fr ~; fi",
+            "for i in 1; do RM -fr ~; done",
+            "while :; do RM -fr /; done",
+            "{ RM -fr ~; }",
+            "! RM -fr ~",
+            "time RM -fr ~",
+            "time -p RM -fr /",
+            "! time RM -fr ~",
+            "case x in x) RM -fr /;; esac",
+            "echo ${x:-$(RM -fr ~)}",
+            # A substitution in command position may expand to nothing.
+            "$(true)RM -fr ~",
+            "$(true) RM -fr ~",
+            "echo hi | `true`RM -fr /",
+            '"$(true)"RM -fr ~',
+            # An unquoted backtick still cuts the argv, as on base.
+            "RM -fr ~/`true`",
+            # A leading redirection keeps its substitution target.
+            ">$(mktemp) RM -fr ~",
+            '>"$(mktemp)" RM -fr ~',
+            "2>`mktemp` RM -fr /",
+            # ``#`` and ``<<`` inside ``${…}`` / ``$[…]`` are data, not a comment or heredoc.
+            "echo ${x:- #}; RM -fr /",
+            "echo ${x:- <<EOF}; RM -fr /",
+            "echo ${a[1]:- #}; RM -fr ~",
+            "echo $[ 1 << 2 ]\nRM -fr /",
+            "echo ${x:-[} # don't remove this marker\nRM -fr ~",
+            # ``$$`` is the PID, so ``$${`` opens no expansion and the comment is skipped.
+            "echo $${x:- # don't remove this marker\nRM -fr ~",
+            "echo $${x:- #} don't\nRM -fr ~",
+            # A comment inside a tracked ``${…}`` cannot quote away the wipe.
+            "unset x; echo ${x:-$( # don't\nRM -fr ~\n# '\n)}",
+            "unset x; echo ${x:-` # don't\nRM -fr ~\n# '\n`}",
+            "echo ${x:-$( # it's\nRM -fr /\n# '\n)}",
+            # An expansion that never closes is also read as if it were not there.
+            "echo \\$${x # don't\nRM -fr ~",
         ],
     )
     def test_a_recursive_force_wipe_of_root_or_home_is_denied(self, command):
@@ -580,11 +618,38 @@ def test_a_recursive_force_wipe_of_root_or_home_is_denied(self, command):
             # still a descendant, not the root operand.
             "RM -fr /tmp/2>/dev/null",
             ">> log RM -rf ./build",
+            # A keyword as an argument, or a substitution operand, is not a root/home target.
+            "echo then RM -fr ~",
+            "RM -fr $(mktemp -d)",
+            "echo ${a[1]} # RM -fr ~",
+            "echo $((1 << 2)) # RM -fr ~",
+            # A substitution inside a target is a real path: cache clears stay allowed.
+            'RM -rf "$(pip cache dir)"/*',
+            'RM -rf "$(go env GOCACHE)"/*',
+            'cd build && RM -rf "$(pwd)"/*',
+            'RM -rf "$HOME/$(basename "$PWD")"',
         ],
     )
     def test_descendants_and_partial_flags_stay_allowed(self, command):
         assert is_denied(command.replace("RM", "rm")) is None
 
+    def test_nested_open_expansions_are_read_a_bounded_number_of_times(self, monkeypatch):
+        """Twenty ``"$( ${`` levels cost a linear number of walks, not 2**20."""
+        from kiro_crew.security import rm_floor
+
+        calls = 0
+        real = rm_floor._segments
+
+        def _counted(*args, **kwargs):
+            nonlocal calls
+            calls += 1
+            return real(*args, **kwargs)
+
+        monkeypatch.setattr(rm_floor, "_segments", _counted)
+        command = 'echo "$( ${ ' * 20 + "rm -fr /tmp/x" + ' )"' * 20
+        rm_floor._rm_wipe_targets(command)
+        assert calls <= 4 * 20 + 2
+
     def test_the_floor_reports_the_catalog_row(self):
         reason = is_denied("rm -fr /")
         assert reason is not None

test/test_security_gate_liveness.py

@@ -521,7 +521,13 @@ def _url_payload_command(n: int) -> str:
 #: Raised by 12, from 32,601, for the crewmate folder's state files in ``paths_lists``
 #: (owners, kits, revoked approvals, tool access, migration ledger), write-denied
 #: like the folder itself. Data, not rule logic: the matcher is unchanged.
-_PACKAGE_LINE_BUDGET = 32_613
+#:
+#: Raised by 19, from 32,613, for the ``rm`` floor's own segment walk: reserved
+#: words (``then``, ``do``, ``!``, ``time`` …) keep command position, a quoted
+#: substitution may expand to nothing (in the program word or a target), and
+#: ``#`` / ``<<`` inside an unpaired ``${…}`` / ``$[…]`` open no comment or heredoc.
+#: Each command is read with and without that tracking. Deny-only: no allow widened.
+_PACKAGE_LINE_BUDGET = 32_632
 
 #: Ceiling on any ONE file in the package. This is what the bound is really for --
 #: a package total says nothing about a single file growing back into a second