diff --git a/BACKLOG.md b/BACKLOG.md index cc01161..bc59fe6 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -79,6 +79,15 @@ _Closed as not planned or rejected. Kept for the record; nothing here is pending ## Done +### Shipped — 2026-10-02 (v10.2.27) + +- [x] **guard hard rules: quoted wrapper switch values and unique-prefix long options hide sudo** ([#464](https://github.com/CryptoJones/omind/issues/464)) — **v10.2.27** — _bug_ — + a quoted switch value with blanks (`env -iC "/a b" sudo id`, `xargs -0I '{} x' sudo id`) is one word + to the command-position regex and the stage parser, and a long value-taking switch matches by unique + prefix (`env --chd /x sudo id`) in the regex and `_switch_width`. An ambiguous prefix stays a flag. + Every value-taking wrapper switch has its long twin (`timeout --kill-after`, `xargs --max-args`, …), + and both parsers share one shell-word definition, so `\"` and `a\ b` read the same in each. + ### Shipped — 2026-10-02 (v10.2.26) - [x] **guard hard rules: `find -exec sudo`, `$(…)`/`${…}` program names and `eval "$(…)"` are not judged** ([#455](https://github.com/CryptoJones/omind/issues/455)) — **v10.2.26** — _bug_ — diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b7cbc..f065cd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,54 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.2.27] - 2026-10-02 + +### Fixed +- **The hard rules read a quoted wrapper switch value as one word, and a long + value-taking wrapper switch by unique prefix + ([#464](https://github.com/CryptoJones/omind/issues/464)).** `env --chd /x + sudo id` and `stdbuf --out L sudo id` were allowed: the command-position + regex and `guard._switch_width` treated any `--…` they did not know exactly + as a flag with no value, so the value was read as the program. `env -iC "/a + b" sudo id` and `xargs -0I '{} x' sudo id` were denied only through the + shell walk's opaque-site fallback; the regex itself took a value as `\S+`, + and the stage parser split the quoted value in two. + - A wrapper switch, its value and a leading `VAR=value` assignment may hold + a quoted run with blanks (`"…"`, `'…'`) or an escaped character (`a\ b`). + Every piece of a word starts with a different character, so a word still + matches one way only and long runs cannot backtrack. `timeout`'s duration + may be quoted too. + - A long value-taking switch matches by any prefix that no other + value-taking switch of that wrapper shares, as GNU getopt_long reads it, + in the regex, in `_switch_width` and in `env --chdir` tracking. An + ambiguous prefix stays a plain flag. `env --chd sudo echo hi` is no longer + a false positive. + - The wrapper table lists the long twin of every value-taking switch: + `timeout --signal`/`--kill-after`, `nice --adjustment`, `ionice + --class`/`--classdata`/`--pid`/`--pgid`/`--uid`, `time + --format`/`--output`, `xargs --max-args`/`--max-procs`/`--max-chars`/ + `--delimiter`/`--arg-file`/`--process-slot-var`, and sudo's. GNU xargs' + `--replace`, `--eof` and `--max-lines` take an OPTIONAL value (attached + with `=` only), so the word after them is the command and they stay out. + - The hard rules, the stage parser and the search-cost bound share one + definition of a shell word (`policy.SHELL_WORD_RE`). So an escaped quote + (`env -u \" bash -c 'sudo id' x "y"`) opens no quoted run that would hide + the `bash -c` stage, and an escaped blank (`env -C a\ b sudo id`) joins + its word in both parsers; the parity test covers both. + - `_cmd_position_cost` reads those same words, so a quoted value is one + word and its chain is bounded as tightly as any other (`A="x" true; ` × + 700 is judged, not denied as too complex). Only a separator inside a + quoted run (`"$(a;b)"`) and a quote with no partner are bounded by the + end of their line. A quoted duration after a switch value (`timeout -s K + "5"`) counts as a link, so the bound still only overestimates. + - `command -\v sudo` is a lookup, as `command -v sudo` is. A quoted lookup + switch (`command -"v"`) is still judged: the search reads it blanked and + cannot tell it from `-"p"`, which runs the program. + - Out of scope: a backslash-newline continuation inside a wrapper value + (`env -C a\b sudo id`, missed on main too). No word piece crosses + a newline, which the search-cost bound relies on; an `xfail(strict)` + test pins it. + ## [10.2.26] - 2026-10-02 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index c680abc..f2c02ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "10.2.26" +version = "10.2.27" description = "Reproduce the OMI/Obsidian memory integration for AI agents, plus a local web app to view, edit, and add memory entries." readme = "README.md" requires-python = ">=3.10" diff --git a/src/omind/__init__.py b/src/omind/__init__.py index 5863fb1..6ffbb19 100644 --- a/src/omind/__init__.py +++ b/src/omind/__init__.py @@ -2,4 +2,4 @@ # Copyright 2026 Aaron K. Clark """omind — OMI/Obsidian memory tooling for AI agents.""" -__version__ = "10.2.26" +__version__ = "10.2.27" diff --git a/src/omind/guard.py b/src/omind/guard.py index 7e0c29e..f697a05 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -1800,13 +1800,15 @@ def _env_chdir_value(word: str, after: str) -> str | None: next word), or ``None`` when it changes none. ``-C`` may end or sit inside a short cluster (``-iC ``, ``-iC``). A letter before it that takes a value (``-uC`` unsets ``C``), or ``-S`` (its rest is a command), - ends the cluster first.""" - if word in ("-C", "--chdir"): + ends the cluster first. ``--chdir`` may be any unambiguous prefix of it + (``--chd ``, #464).""" + if word == "-C": return after - if word.startswith("--chdir="): - return word.partition("=")[2] if word.startswith("--"): - return None + name, eq, value = word.partition("=") + if _long_option(name, _long_names(_STAGE_WRAPPERS["env"])) != "--chdir": + return None + return value if eq else after for k in range(1, len(word)): if word[k] == "C": return word[k + 1 :] or after @@ -3269,10 +3271,15 @@ def _switch_width(word: str, takes_arg: frozenset[str]) -> int: switches in ``takes_arg`` that take a value. A short cluster reads like getopt (#432 review): the first value-taking letter takes the rest of the word, or the next word when it ends the cluster (``env -iC ``, - ``sudo -iu bob``), so that next word is never read as the program.""" + ``sudo -iu bob``), so that next word is never read as the program. A long + switch may be any unambiguous prefix of its name (``env --chd ``, + #464), as :func:`_long_option` resolves it; an ambiguous one is a flag.""" if word in takes_arg: return 2 - if word.startswith("--") or len(word) < 3: + if word.startswith("--"): + valued = "=" not in word and _long_option(word, _long_names(takes_arg)) in takes_arg + return 2 if valued else 1 + if len(word) < 3: return 1 for k in range(1, len(word)): if "-" + word[k] in takes_arg: @@ -3280,6 +3287,12 @@ def _switch_width(word: str, takes_arg: frozenset[str]) -> int: return 1 +@functools.lru_cache(maxsize=64) +def _long_names(options: frozenset[str]) -> frozenset[str]: + """The long (``--name``) options among ``options``.""" + return frozenset(opt for opt in options if opt.startswith("--")) + + def _basename(word: str) -> str: base = re.split(r"[/\\]", word)[-1] return base[:-4] if base.lower().endswith(".exe") else base @@ -3298,8 +3311,15 @@ def _program_stages(code: str, raw: str = "") -> list[tuple[str, list[str], int, _ASSIGNMENT_WORD_RE = re.compile(r"[A-Za-z_]\w*=") +#: A word of a stage segment: the hard rules' own word (#464 review), so a +#: quoted value with blanks in it (``env -C "/a b" sudo``, blanked to ``" "`` +#: by ``shell_code_text``) or an escaped blank (``a\ b``) stays one word, and +#: an escaped quote (``\"``) opens no quoted run. +_STAGE_WORD_RE = policy.SHELL_WORD_RE + + def _word_offset(text: str, words: list[str], offsets: list[int], k: int) -> int: - """Where ``words[k]`` (``text.split()``) starts in ``text``. ``offsets`` + """Where ``words[k]`` (:data:`_STAGE_WORD_RE`) starts in ``text``. ``offsets`` caches the words located so far, so a segment's words are found once each and only up to the last one a stage asks for (#445).""" while len(offsets) <= k: @@ -3337,12 +3357,12 @@ def _program_stages_cached( end = seg.end() while end < n and code[end].isspace(): end += 1 - # Words by `str.split` (the same Unicode blanks as `\S+`), offsets + # Words by one regex scan (the same Unicode blanks as `\S+`), offsets # only where a stage needs one, and the next `-exec` by bisection, so # one long segment is not walked word by word in Python (#445). text = seg.group() base_at = seg.start() - words = text.split() + words = _STAGE_WORD_RE.findall(text) offsets: list[int] = [] ntok = len(words) execs = ( diff --git a/src/omind/policy.py b/src/omind/policy.py index f8b09a2..b9cb65d 100644 --- a/src/omind/policy.py +++ b/src/omind/policy.py @@ -86,19 +86,36 @@ def opt_in_satisfied(opt_in: str, command: str) -> bool: STAGE_WRAPPERS: dict[str, frozenset[str]] = { **{name: frozenset() for name in _HEREDOC_OWNER_SKIP}, **{kw: frozenset() for kw in ("do", "then", "else", "elif", "if", "while", "until", "{", "!")}, - "xargs": frozenset({"-I", "-n", "-P", "-L", "-s", "-d", "-E", "-a"}), - "sudo": frozenset({"-u", "-g", "-C", "-h", "-p", "-U", "-r", "-t", "-D"}), + # #464 review: each value-taking short switch with its long twin. Not + # `--replace`, `--eof` or `--max-lines`: their value is OPTIONAL and only + # ever attached (`--eof=X`), so the next word is the command. + "xargs": frozenset( + { + *("-I", "-n", "-P", "-L", "-s", "-d", "-E", "-a"), + *("--max-args", "--max-procs", "--max-chars", "--delimiter", "--arg-file"), + "--process-slot-var", + } + ), + "sudo": frozenset( + { + *("-u", "-g", "-C", "-h", "-p", "-U", "-r", "-t", "-D"), + *("--user", "--group", "--close-from", "--host", "--prompt"), + *("--other-user", "--role", "--type", "--chdir"), + } + ), # Not `-S`: its value is itself a command (`env -S sudo id`), so the word # after it stays in command position (#430 review). "env": frozenset({"-u", "-C", "--unset", "--chdir"}), - "nice": frozenset({"-n"}), - "timeout": frozenset({"-s", "-k"}), - "time": frozenset({"-f", "-o"}), + "nice": frozenset({"-n", "--adjustment"}), + "timeout": frozenset({"-s", "-k", "--signal", "--kill-after"}), + "time": frozenset({"-f", "-o", "--format", "--output"}), # #434: wrappers that also exec their trailing command; #444 review: # `stdbuf --output L sudo id`. "stdbuf": frozenset({"-i", "-o", "-e", "--input", "--output", "--error"}), "caffeinate": frozenset({"-t", "-w"}), - "ionice": frozenset({"-c", "-n", "-p", "-P", "-u"}), + "ionice": frozenset( + {"-c", "-n", "-p", "-P", "-u", "--class", "--classdata", "--pid", "--pgid", "--uid"} + ), "chronic": frozenset(), # #444: execs the next word (`setsid sudo id`). "setsid": frozenset(), @@ -112,6 +129,44 @@ def opt_in_satisfied(opt_in: str, command: str) -> bool: _LOOKUP_WRAPPERS = frozenset({"command", "builtin"}) +def _word_piece(excluded: str = "") -> str: + """One piece of a shell word for the hard rules (#464): a quoted run + (``"/a b"``, ``'{} x'``), a backslash with the character it escapes (``a\\ + b``), or a character that is none of blank, quote or backslash. A letter + in ``excluded`` is no piece, bare or escaped. Each piece starts with a + different character, so a word splits into pieces one way only and cannot + make a search backtrack. No piece crosses a newline, as + ``_cmd_position_cost`` assumes; so a backslash-newline continuation inside + a word (``a\\b``) is not joined (out of scope for #464).""" + return rf"""(?:"(?:[^"\\\n]|\\.)*"|'[^'\n]*'|\\[^\n{excluded}]|[^\s"'\\{excluded}])""" + + +#: A shell word's piece, and a whole word (a switch's or assignment's value). +_WORD_PIECE = _word_piece() +_WORD = _WORD_PIECE + "+" +#: The words of a text: runs of :data:`_WORD_PIECE`, or of a quote or a +#: backslash with no partner, an ordinary character there. The one definition +#: of a shell word that the hard rules, ``guard``'s stage parser and +#: :func:`_cmd_position_cost` share (#464 review), so ``a\ b``, ``"/a b"`` and +#: ``\"`` split the same way in all three. +SHELL_PIECE_RE = re.compile(_WORD_PIECE + r"""|["'\\]""") +SHELL_WORD_RE = re.compile("(?:" + _WORD_PIECE + r"""|["'\\])+""") + + +def _long_prefixes(names: list[str]) -> list[str]: + """Every spelling of the long options ``names`` that GNU getopt_long reads + as one of them (#464): the name, or a prefix of at least ``--`` and one + letter that no other name in ``names`` shares, as ``guard._long_option`` + resolves it. An ambiguous prefix is left out, so it stays a plain flag.""" + spellings = set(names) + for name in names: + for k in range(3, len(name)): + prefix = name[:k] + if sum(other.startswith(prefix) for other in names) == 1: + spellings.add(prefix) + return sorted(spellings, key=lambda spelling: (-len(spelling), spelling)) + + def _wrapper_pattern(name: str, takes_arg: frozenset[str]) -> str: """One :data:`STAGE_WRAPPERS` entry as a regex: an optional path (``/usr/bin/env``), the name, its switches (a switch that takes a separate @@ -130,21 +185,34 @@ def _wrapper_pattern(name: str, takes_arg: frozenset[str]) -> str: next word when it ends the cluster, so ``env -iC /x sudo id`` and ``xargs -0I {} sudo id`` still reach the program. A cluster ending in a value-taking letter holds no other one before it, so it matches only the - value branch.""" - switch = r"-[^\svV]*" if name in _LOOKUP_WRAPPERS else r"-\S*" + value branch. + + A switch or its value may hold a quoted run with blanks in it (``env -iC + "/a b" sudo id``, #464), and a long value-taking switch matches by any + unambiguous prefix (``env --chd /x sudo id``), as ``guard._switch_width`` + reads it.""" + switch = "-" + _WORD_PIECE + "*" + if name in _LOOKUP_WRAPPERS: + # An escaped `\v` is a `v` too (`command -\v sudo`, #464 review). A + # quoted run stays a switch piece: the search reads it blanked, so + # `-"v"` cannot be told from `-"p"`, which runs the program; both are + # judged, failing closed. + switch = "-" + _word_piece("vV") + "*" if takes_arg: - long_names = sorted(opt for opt in takes_arg if len(opt) != 2) + long_names = _long_prefixes(sorted(opt for opt in takes_arg if len(opt) != 2)) letters = "".join(sorted(opt[1] for opt in takes_arg if len(opt) == 2)) valued = [re.escape(opt) for opt in long_names] if letters: letter_class = re.escape(letters) valued.append(rf"-(?!-)[^\s{letter_class}]*[{letter_class}]") names = "|".join(valued) - switch = rf"(?:{names})[ \t]+\S+|(?!(?:{names})(?:\s|$)){switch}" + switch = rf"(?:{names})[ \t]+{_WORD}|(?!(?:{names})(?:\s|$)){switch}" switches = rf"(?:[ \t]+(?:{switch}))*" pattern = r"(?:[./][^\s;&|`()]*/)?" + re.escape(name) + switches if name in WRAPPER_POSITIONAL: - pattern += r"[ \t]+\d[\d.]*[smhd]?" + switches + # The duration may be quoted (`timeout "5" sudo id`, #464); the + # search reads ``shell_code_text``, which blanks a quote's content. + pattern += r"""[ \t]+(?:\d[\d.]*[smhd]?|"[^"\n]*"|'[^'\n]*')""" + switches return pattern @@ -173,7 +241,7 @@ def _wrapper_pattern(name: str, takes_arg: frozenset[str]) -> str: #: assignment-skip never crosses a newline into another command. _CMD_POSITION = ( r"(?:^|[\n;&|`(])[ \t]*" - r"(?:(?:\w+=\S*|" + _CMD_WRAPPERS + r")[ \t]+)*" + r"(?:(?:\w+=" + _WORD_PIECE + "*|" + _CMD_WRAPPERS + r")[ \t]+)*" r"(?:[./][^\s;&|`()]*/)?" ) @@ -194,6 +262,7 @@ def _wrapper_pattern(name: str, takes_arg: frozenset[str]) -> str: ) _CHAIN_GAP_RE = re.compile(r"[ \t]+") _WORD_SPAN_RE = re.compile(r"\S+") +_QUOTE_RE = re.compile(r"[\"'\\]") @functools.lru_cache(maxsize=256) @@ -212,25 +281,61 @@ def _cmd_position_cost(text: str) -> int: skips a chain of assignment and wrapper words, each a blank-free run joined by blanks; so the bound sums, over the starts, the extent of the chain each could skip. A word counts as a link unless it cannot be an - assignment, a wrapper, a switch or a switch's value, so the bound only - overestimates. Linear; a short text returns its trivial bound unscanned.""" + assignment, a wrapper, a switch, a switch's value or a quoted duration, so + the bound only overestimates. + + Words are :data:`SHELL_WORD_RE`'s, the search's own (#464 review), so a + quoted value with blanks in it (``env -C "/a b"``) is one word and its + chain is bounded as tightly as any other. Two places read past a word's + end, and are bounded by the end of its line, since no word piece crosses + a newline: a separator inside a quoted run (``"$(a;b)"``), where a chain + starting after it may pair the quotes another way; and a quote with no + partner, whose run is tried to the end of the line. Linear; a short text + returns its trivial bound unscanned.""" n = len(text) if n * n <= CMD_SEARCH_BUDGET: return n * n - spans = [(m.start(), m.end()) for m in _WORD_SPAN_RE.finditer(text)] + spans = [(m.start(), m.end()) for m in SHELL_WORD_RE.finditer(text)] count = len(spans) # joined[i]: word i + 1 follows word i across blanks only (`[ \t]+`). joined = [ bool(_CHAIN_GAP_RE.fullmatch(text, spans[i][1], spans[i + 1][0])) for i in range(count - 1) ] + [False] + newlines: list[int] = [] + + def line_end(pos: int) -> int: + if not newlines: + newlines.extend(m.start() for m in re.finditer("\n", text)) + newlines.append(n) + return newlines[bisect.bisect_left(newlines, pos)] + + # far[i]: where a search reading word i may stop, past its end for an + # unpartnered quote. inner[i]: its separators inside a quoted run. + far = [end for _start, end in spans] + inner = [0] * count + for i, (start, end) in enumerate(spans): + if not _QUOTE_RE.search(text, start, end): + continue + for piece in SHELL_PIECE_RE.finditer(text, start, end): + if piece.group() in ('"', "'"): + far[i] = line_end(end) + elif piece.group()[0] in "\"'": + inner[i] += sum(piece.group().count(ch) for ch in _SEPARATOR_CHARS) + # link[i]: word i may be a link in a chain. + link = [False] * count + for i, (start, _end) in enumerate(spans): + link[i] = bool(_CHAIN_LINK_RE.match(text, start)) or ( + i > 0 + and ( + text[spans[i - 1][0]] == "-" # a switch's value + # `timeout -s K "5"`: a quoted duration ends a chain of links. + or (text[start] in "\"'" and link[i - 1] and joined[i - 1]) + ) + ) # reach[i]: how far a chain that enters word i can run. reach = [n] * (count + 1) for i in range(count - 1, -1, -1): - start, end = spans[i] - link = bool(_CHAIN_LINK_RE.match(text, start)) or ( - i > 0 and text[spans[i - 1][0]] == "-" # a switch's value - ) - reach[i] = reach[i + 1] if link and joined[i] else end + reach[i] = max(reach[i + 1] if link[i] and joined[i] else spans[i][1], far[i]) cost = n previous_end = 0 for i, (start, end) in enumerate(spans): @@ -239,7 +344,9 @@ def _cmd_position_cost(text: str) -> int: # A separator inside it starts a chain at the rest of it. seps = sum(text.count(ch, start, end) for ch in _SEPARATOR_CHARS if ch != "\n") if seps: - cost += seps * ((reach[i + 1] if joined[i] else end) - start) + rest = max(reach[i + 1] if joined[i] else end, far[i]) + cost += (seps - inner[i]) * (rest - start) + cost += inner[i] * (max(rest, line_end(end)) - start) previous_end = end return cost diff --git a/tests/test_guard.py b/tests/test_guard.py index 81ce18e..537e301 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -3507,6 +3507,44 @@ def test_windows_tokenizing_leaves_benign_stdin_operands_alone(command: str) -> "stdbuf -oL sudo id", "ionice -c3 sudo id", "sudo -Eu root id", + # #464: a switch value with a blank in it, quoted, is one word. + 'env -iC "/a b" sudo id', + "xargs -0I '{} x' sudo id", + 'env -C "/a b" sudo id', + 'env -C"/a b" sudo id', + 'env --chdir="/a b" sudo id', + 'sudo -u "a b" id', + 'X="a b" env -C "/a b" sudo id', + 'timeout "5" sudo id', + # #464: a long value-taking switch by unique prefix takes the next word. + "env --chd /x sudo id", + 'env --chd "/a b" sudo id', + "env --u VAR sudo id", + "stdbuf --out L sudo id", + # #464 review: one word definition for both parsers, so an escaped quote + # opens no quoted run and an escaped blank joins its word. + 'env -u \\" sudo id', + "env -C a\\ b sudo id", + "env -C '/a b' sudo id", + 'env -u \\" -C "/a b" sudo id', + # #464 review: the long twin of every value-taking wrapper switch. + "timeout --kill-after 5 10 sudo id", + "timeout --signal KILL 5 sudo id", + "timeout --sig KILL 5 sudo id", + "xargs --max-args 1 sudo id", + "xargs --max-procs 2 sudo id", + "xargs --max-chars 99 sudo id", + "xargs --arg-file f sudo id", + "xargs --delimiter , sudo id", + "xargs --process-slot-var V sudo id", + "nice --adjustment 5 sudo id", + "ionice --class 3 sudo id", + "ionice --classdata 7 sudo id", + "ionice --pid 1 sudo id", + "time --output f sudo id", + "time --format %e sudo id", + "sudo --user root id", + "sudo --chdir /x id", ) @@ -3527,6 +3565,15 @@ def test_hard_rules_read_wrapper_clusters_getopt_style(command: str) -> None: "timeout -k5 10 echo sudo", "command -v sudo", "tmux new -s sudo-test", + # #464: the prefix takes `sudo` as its value; a quoted value hides it. + "env --chd sudo echo hi", + 'env -C "sudo x" echo hi', + # #464 review: a long twin takes `sudo` as its value; an escaped `v` + # is still a lookup. + "timeout --signal sudo 5 echo hi", + "xargs --max-args sudo echo hi", + "command -\\v sudo", + "command -\\V sudo", ], ) def test_wrapper_clusters_leave_benign_commands_alone(command: str) -> None: @@ -3787,6 +3834,13 @@ def test_windows_shell_tokens_join_mid_word_quotes_and_keep_backslashes( # #451 review: runs of value-taking clusters stay linear too. "env -iC /x " * 400 + "x", "xargs -0I {} " * 400 + "x", + # #464: quoted values and long-option prefixes stay linear too. + 'env -iC "/a b" ' * 400 + "x", + "xargs -0I '{} x' " * 400 + "x", + 'env -C "-a -b" ' * 400 + "x", + "env --chd /x " * 400 + "x", + 'X="a b" ' * 400 + "x", + 'timeout "5" ' * 400 + "x", ], ) def test_wrapper_runs_cannot_backtrack_the_hard_rules(command: str) -> None: @@ -4464,6 +4518,122 @@ def test_long_option_unique_prefix(given: str, expected: str) -> None: assert guard._long_option(given, known) == expected +@pytest.mark.parametrize( + ("word", "width"), + [ + ("--chdir", 2), + ("--chd", 2), + ("--c", 2), + ("--chd=/x", 1), + ("--u", 2), + ("--", 1), + ("--debug", 1), + ], +) +def test_switch_width_reads_long_options_by_unique_prefix(word: str, width: int) -> None: + """#464: a long value-taking wrapper switch may be any unambiguous prefix.""" + assert guard._switch_width(word, policy.STAGE_WRAPPERS["env"]) == width + + +@pytest.mark.parametrize( + "command", + [ + 'env -iC "/a b" sudo id', + "xargs -0I '{} x' sudo id", + 'env --chdir="/a b" sudo id', + 'timeout "5" sudo id', + "env -C a\\ b sudo id", + ], +) +def test_command_position_reads_a_quoted_wrapper_value_as_one_word(command: str) -> None: + """#464: the hard-rule regex itself reaches ``sudo`` past a quoted value, + rather than relying on the walk's opaque-site fallback.""" + rule = next(r for r in policy.SEED_RULES if r.id == "sudo-use-fleet-sudo") + assert rule.judge(command) is True + + +def test_ambiguous_long_prefix_is_a_plain_flag() -> None: + """#464: a prefix two value-taking switches share takes no value, in the + stage parser and in the hard rules alike.""" + takes_arg = frozenset({"--output", "--outfile"}) + assert guard._switch_width("--out", takes_arg) == 1 + assert guard._switch_width("--outp", takes_arg) == 2 + spellings = policy._long_prefixes(sorted(takes_arg)) + assert "--outp" in spellings and "--outf" in spellings + assert not {"--o", "--ou", "--out"} & set(spellings) + + +def test_env_chdir_value_reads_a_long_prefix() -> None: + assert guard._env_chdir_value("--chd", "/x") == "/x" + assert guard._env_chdir_value("--chd=/y", "/x") == "/y" + assert guard._env_chdir_value("--debug", "/x") is None + + +def test_an_escaped_quote_hides_no_stage() -> None: + """#464 review: ``\\"`` is an escaped character, not a quote that opens a + run to the next ``"``, so the ``bash -c`` stage after it is still read and + its body judged (denied on main, allowed by the first cut of #464).""" + command = 'env -u \\" bash -c \'sudo id\' x "y"' + stages = guard._program_stages(policy.shell_code_text(command), command) + assert [program for program, *_ in stages] == ["bash"] + verdict = guard._hard_policy_verdict(command) + assert verdict is not None and verdict.rule_id == "sudo-use-fleet-sudo" + + +@pytest.mark.parametrize( + ("text", "words"), + [ + ('env -u \\" bash -c x "y"', ["env", "-u", '\\"', "bash", "-c", "x", '"y"']), + ("env -C a\\ b sudo id", ["env", "-C", "a\\ b", "sudo", "id"]), + ('env -C "/a b" x', ["env", "-C", '"/a b"', "x"]), + ('a \'b c\' \\" "d\\"e"', ["a", "'b c'", '\\"', '"d\\"e"']), + ], +) +def test_stage_words_are_the_hard_rules_words(text: str, words: list[str]) -> None: + """#464 review: the stage parser splits a segment with the hard rules' own + word definition, and each word is one ``_WORD`` match there.""" + assert guard._STAGE_WORD_RE is policy.SHELL_WORD_RE + assert guard._STAGE_WORD_RE.findall(text) == words + assert all(re.fullmatch(policy._WORD, word) for word in words) + + +@pytest.mark.parametrize( + "command", + ["xargs --max-lines sudo id", "xargs --replace sudo id", "xargs --eof sudo id"], +) +def test_optional_value_long_switches_take_no_next_word(command: str) -> None: + """#464 review: GNU xargs' ``--max-lines``, ``--replace`` and ``--eof`` take + an OPTIONAL value, only ever attached with ``=``, so the next word is the + command they run; they stay out of the value-taking table.""" + verdict = guard._hard_policy_verdict(command) + assert verdict is not None and verdict.rule_id == "sudo-use-fleet-sudo" + + +@pytest.mark.parametrize("command", ['command -"v" sudo id', 'command -"p" sudo id']) +def test_quoted_lookup_switch_fails_closed(command: str) -> None: + """#464 review: the search reads a quoted switch blanked, so ``-"v"`` (a + lookup) cannot be told from ``-"p"`` (which runs sudo). Both are judged: + the hard rule fails closed, as it did on main.""" + verdict = guard._hard_policy_verdict(command) + assert verdict is not None and verdict.rule_id == "sudo-use-fleet-sudo" + + +def test_env_chdir_resolves_a_quoted_directory(tmp_path: Path) -> None: + """#464 review: ``env -C "/a b"`` moves to ``/a b``, not to a relative + directory named with its quotes; the prefix is unquoted with shell rules.""" + target = (tmp_path / "a b").as_posix() + for prefix in (f'env -C "{target}" ', f"env -iC '{target}' ", f'env --chd="{target}" '): + assert guard._env_chdir(prefix + "git status", tmp_path / "x") == Path(target), prefix + + +@pytest.mark.xfail(strict=True, reason="#464 out of scope: no word piece joins a continuation") +def test_line_continuation_inside_a_wrapper_value_is_out_of_scope() -> None: + """``env -C a\\b sudo id`` runs sudo (the shell joins ``a`` and + ``b``). Missed on main too: a word piece never crosses a newline, which + the search-cost bound relies on. Pinned so a fix shows up here.""" + assert guard._hard_policy_verdict("env -C a\\\nb sudo id") is not None + + def test_redirect_with_no_target_repo_is_not_repo_work(tmp_path: Path) -> None: """No target repo, nothing to protect: the redirect check fails open.""" action = {"tool": "Bash", "command": "echo x > x.py", "cwd": tmp_path.as_posix()} @@ -4910,6 +5080,21 @@ def test_command_position_cost_bounds_the_chain_shapes() -> None: assert policy._cmd_position_cost(text) < 4 * len(text), text[:8] for chain in ("x=1;" * 2_000, "a=1 a=1;" * 2_000, "env -x;" * 2_000): assert policy._cmd_position_cost(chain) > policy.CMD_SEARCH_BUDGET, chain[:8] + # #464 review: a quoted word is one word, so a benign quoted assignment + # line stays near its length, and a chain through quoted values or a + # quoted duration after a switch value is still counted in full. + for text in ('A="x" true; ' * 700 + "echo sudo", 'echo "a b"; ' * 2_000, "X='a b' y\n" * 2_000): + assert policy._cmd_position_cost(text) < 4 * len(text), text[:8] + for chain in ('x="a b";' * 2_000, 'env -C "/a b";' * 2_000, 'env -u \\";' * 2_000): + assert policy._cmd_position_cost(chain) > policy.CMD_SEARCH_BUDGET, chain[:8] + durations = ";" * 1_000 + ' timeout -s K "5"' * 1_000 + assert policy._cmd_position_cost(durations) > 1_000 * (len(durations) - 1_000) + + +def test_a_benign_quoted_one_liner_stays_under_the_budget() -> None: + """#464 review: quoted assignments between separators are judged, not + denied as too complex (allowed on main).""" + assert guard._hard_policy_verdict('A="x" true; ' * 700 + "echo sudo") is None def test_split_words_tokenizes_like_shlex() -> None: diff --git a/uv.lock b/uv.lock index 0e11ad1..8bafd05 100644 --- a/uv.lock +++ b/uv.lock @@ -2364,7 +2364,7 @@ wheels = [ [[package]] name = "omind" -version = "10.2.26" +version = "10.2.27" source = { editable = "." } dependencies = [ { name = "cryptography" },