From acee821b82b7980a54b95372ac9ce8dfacfd11b3 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Fri, 2 Oct 2026 19:31:48 -0500 Subject: [PATCH 1/2] fix(guard): judge find -exec commands and literal expanded program names (#455) The hard rules now judge the words after -exec/-execdir/-ok/-okdir, up to \; ';' or {} +, as a command, so `find . -exec sudo id \;` is denied. The split reuses the stage walk's -exec cut. An expansion that is a whole word and prints a bare program name ($(printf NAME), $(echo NAME), `echo NAME`, ${X:-NAME} and friends) is replaced by that name before judging, as is eval of a literal echo/printf. The name counts only in command position, so `echo $(printf sudo)` and `grep ${X:-sudo} f` stay allowed. Expansions that need evaluating ($X, $(cat f), format strings, echo -n) are out of scope and pinned by tests. Bumps to 10.2.26. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011Zj1PCfsSSJgzLxi32jvde --- BACKLOG.md | 11 +++-- CHANGELOG.md | 23 +++++++++++ pyproject.toml | 2 +- src/omind/__init__.py | 2 +- src/omind/guard.py | 96 +++++++++++++++++++++++++++++++++++++++++++ tests/test_guard.py | 81 ++++++++++++++++++++++++++++++++++++ uv.lock | 2 +- 7 files changed, 211 insertions(+), 6 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 7bc72c0..cc01161 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -11,9 +11,6 @@ _Mirrors the [GitHub Issues tab](https://github.com/CryptoJones/omind/issues). All open items are sequenced by priority (Priority 1 to 7) so future agents know the exact order of operations._ -- [ ] **guard hard rules: `find -exec sudo`, `$(…)`/`${…}` program names and `eval "$(…)"` are not judged** ([#455](https://github.com/CryptoJones/omind/issues/455)) — _bug_ — - deferred from #444: each needs either `find -exec` unwrapping or the output of a command - substitution, which the guard does not evaluate. - [ ] **rules: unique-prefix push options (`--al`, `--mirr`) and glob refspecs reach main from a feature branch** ([#443](https://github.com/CryptoJones/omind/issues/443)) — _bug_ — follow-up from the #433 review: git takes any unique prefix of `--all` / `--mirror`, and `'refs/heads/*:refs/heads/*'` sends `main`. - [ ] **README: link Flatline Roundtable and conveyor in the first lines** ([#436](https://github.com/CryptoJones/omind/issues/436)) — _docs_ — @@ -82,6 +79,14 @@ _Closed as not planned or rejected. Kept for the record; nothing here is pending ## Done +### 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_ — + the words after `-exec`/`-execdir`/`-ok`/`-okdir` up to `\;`, `';'` or `{} +` are judged as a + command, and an expansion that prints a literal program name (`$(printf sudo) id`, `${X:-sudo} id`, + `` `echo sudo` id ``, `eval "$(echo sudo id)"`) is judged with that name in its place. Expansions the + guard would have to evaluate (`$X`, `$(cat f)`, format strings) are documented as out of scope. + ### Shipped — 2026-10-02 (v10.2.25) - [x] **guard hard rules: bundled wrapper option clusters (`env -iC dir sudo …`) are not read getopt-style** ([#451](https://github.com/CryptoJones/omind/issues/451)) — **v10.2.25** — _bug_ — diff --git a/CHANGELOG.md b/CHANGELOG.md index 6af9038..2830832 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.2.26] - 2026-10-02 + +### Fixed +- **The hard rules judge the command a `find` action runs, and an expansion + that prints a literal program name + ([#455](https://github.com/CryptoJones/omind/issues/455)).** `find . -exec + sudo id \;` was allowed: `-exec` is not a separator or a wrapper, so `sudo` + never reached command position. `$(printf sudo) id`, `${X:-sudo} id` and + `eval "$(echo sudo id)"` were allowed too. + - The words after `-exec`, `-execdir`, `-ok` and `-okdir`, up to `\;`, + `';'` or `{} +`, are judged as a command. The split comes from the same + stage walk the repo-work classifier reads. `find . -name sudo`, `find . + -exec grep sudo {} +` and `find . -exec echo sudo \;` stay allowed. + - An expansion that is a whole word and prints a bare program name is + replaced by that name before the command is judged: `$(echo|printf + NAME)`, `` `echo NAME` ``, `${X:-NAME}` (`-`, `=`, `:=` too), optionally + double-quoted. So is `eval` of a literal `echo`/`printf`. The name counts + only in command position, so `echo $(printf sudo)` and `grep ${X:-sudo} f` + stay allowed. + - Out of scope, and pinned as allowed by tests: an expansion whose output + the guard would have to evaluate (`X=sudo; $X id`, `$(cat f) id`, + `$(printf '%s' sudo) id`, `$(echo -n sudo) id`, `eval "$(cat f)"`). + ## [10.2.25] - 2026-10-02 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 2a921d8..c680abc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "10.2.25" +version = "10.2.26" 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 128ae93..5863fb1 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.25" +__version__ = "10.2.26" diff --git a/src/omind/guard.py b/src/omind/guard.py index 453ca60..622b590 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -3962,6 +3962,93 @@ def _with_positionals(body: str, positional: list[str]) -> str | None: return _ALL_POSITIONALS_RE.sub(lambda _m: joined, body) +#: A ``find`` action word whose following words are a command (#455). +_EXEC_ACTION_RE = re.compile(r"(? str | None: + """The commands ``find -exec``/``-execdir``/``-ok``/``-okdir`` run in + ``text``, one per line, each from the word after the action to the last + word before its ``\\;``/``';'``/``{} +`` (#455): ``find . -exec sudo id + \\;`` runs ``sudo id``. ``None`` when there is none. + + ``stages`` are :func:`_program_stages` of ``masked`` (its + :func:`policy.shell_code_text`), which already cuts a segment at each + action, so a stage that shares its segment's end with the stage before + it is an ``-exec`` command. Its text starts at the action, not at the + program, so wrappers (``-exec env sudo id``) and an opt-in assignment + are judged as the hard rules judge any command. Each action's window is + scanned once, so a long chain stays linear (#445).""" + commands: list[str] = [] + prev_start = prev_end = -1 + for _program, args, start, end in stages: + if end == prev_end: + action: re.Match[str] | None = None + for action in _EXEC_ACTION_RE.finditer(masked, prev_start, start): # noqa: B007 + pass + if action is not None: + stop = start + for k, word in enumerate(_WORD_RE.finditer(masked, start, end)): + stop = word.end() + if k >= len(args): + break + commands.append(text[action.end() : stop]) + prev_start, prev_end = start, end + return "\n".join(commands) if commands else None + + +def _literal_name(tag: str) -> str: + """A literal program name, optionally quoted, as an expansion prints it; + its groups are suffixed with ``tag``.""" + return rf"""(?P['"]?)(?P[\w./+-]+)(?P=q{tag})""" + + +#: An expansion that is a whole word and prints a bare program name (#455): +#: ``$(printf sudo)``, ``$(echo 'sudo')``, `` `echo sudo` ``, ``${X:-sudo}`` +#: (``-``, ``:-``, ``=``, ``:=``), optionally double-quoted as one word. Only +#: these shapes: anything else (a variable, ``$(cat f)``, ``printf '%s'``, +#: ``echo -n``) needs the shell to evaluate it, which the guard does not. +_EXPANDED_NAME_RE = re.compile( + r'(?"?)(?:' + r"\$\([ \t]*(?:echo|printf)[ \t]+" + _literal_name("a") + r"[ \t]*\)" + r"|`[ \t]*(?:echo|printf)[ \t]+" + _literal_name("b") + r"[ \t]*`" + r"|\$\{\w+:?[-=]" + _literal_name("c") + r"\}" + r")(?P=dq)(?=[\s;&|)]|\Z)" +) +#: ``eval`` of an ``echo``/``printf`` of literal words (#455): ``eval "$(echo +#: sudo id)"`` runs ``sudo id``. Literal means no ``$``, quote, backtick, +#: ``%`` or backslash in the printed words. +_EVAL_LITERAL_RE = re.compile( + r"(?\"?)\$\([ \t]*(?:echo|printf)[ \t]+" + r"""(?P['"]?)(?P[\w./+=,: \t-]+)(?P=q)[ \t]*\)(?P=dq)(?=[\s;&|)]|\Z)""" +) + + +def _expanded_command_text(text: str) -> str | None: + """``text`` with each expansion that prints a literal program name + replaced by that name, and each ``eval`` of literal ``echo``/``printf`` + words replaced by those words (#455), so the hard rules judge what runs: + ``$(printf sudo) id`` and ``${X:-sudo} id`` run ``sudo id``. The result + is judged as the command is, so the name counts only in command + position: ``echo $(printf sudo)`` and ``grep ${X:-sudo} f`` stay data, + and a single-quoted expansion stays a string. ``None`` when nothing was + replaced. Out of scope: anything whose output needs evaluating (a + variable, ``$(cat f)``, a format string, ``$(printf sudo)`` glued to + other text).""" + if "$" not in text and "`" not in text: + return None + + def name(match: re.Match[str]) -> str: + return match.group("namea") or match.group("nameb") or match.group("namec") + + expanded = _EVAL_LITERAL_RE.sub(lambda m: m.group("words"), text) + expanded = _EXPANDED_NAME_RE.sub(name, expanded) + return expanded if expanded != text else None + + def _local_shell_subjects( text: str, code: list[str], words: list[str], runs: list[str] | None = None ) -> None: @@ -3981,6 +4068,10 @@ def _local_shell_subjects( # Each stage stops at the next stage's start too, so a `find -exec sh x # {} +` chain is not rescanned to the end per shell (#432 review). ends = _stage_ends(masked, sorted(stage[2] for stage in stages)) + # `find . -exec sudo id \;` runs `sudo id` (#455). + exec_commands = _find_exec_commands(text, masked, stages) + if exec_commands is not None: + code.append(exec_commands) floor = 0 for program, _args, pos, _end in stages: if program not in _LOCAL_SHELLS and program not in ("source", "."): @@ -4112,6 +4203,11 @@ def _hard_rule_subjects(command: str) -> _HardSubjects: runs: list[str] = [] try: for text in (command, *bodies): + # `$(printf sudo) id`, `${X:-sudo} id`, `eval "$(echo sudo id)"` + # (#455): the literal the expansion prints, in its place. + expanded = _expanded_command_text(text) + if expanded is not None: + code.append(expanded) _local_shell_subjects(text, code, words, runs) except Exception: pass # keep what was already found: more text judged, never less diff --git a/tests/test_guard.py b/tests/test_guard.py index effd640..0d98338 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -3533,6 +3533,87 @@ def test_wrapper_clusters_leave_benign_commands_alone(command: str) -> None: assert guard._hard_policy_verdict(command) is None +#: #455: the command a `find` action runs, and an expansion that prints a +#: literal program name in command position. +FIND_EXEC_AND_EXPANSION_HARD_COMMANDS = ( + ("find . -exec sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -execdir sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -ok sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -okdir sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -exec sudo id ';'", "sudo-use-fleet-sudo"), + ('find . -exec sudo id ";"', "sudo-use-fleet-sudo"), + ("find . -exec sudo id {} +", "sudo-use-fleet-sudo"), + ("find . -exec env sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -exec /usr/bin/sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -name x -exec grep sudo {} + -exec sudo id \\;", "sudo-use-fleet-sudo"), + ("cd x && find . -exec sudo id \\;", "sudo-use-fleet-sudo"), + ("bash -c 'find . -exec sudo id \\;'", "sudo-use-fleet-sudo"), + ("find . -exec doas id \\;", "privesc-alternatives"), + ("find . -exec gh repo delete o/r --yes \\;", "gh-repo-delete"), + ("$(printf sudo) id", "sudo-use-fleet-sudo"), + ("$(echo 'sudo') id", "sudo-use-fleet-sudo"), + ('"$(printf sudo)" id', "sudo-use-fleet-sudo"), + ("`echo sudo` id", "sudo-use-fleet-sudo"), + ("$(printf /usr/bin/sudo) id", "sudo-use-fleet-sudo"), + ("${X:-sudo} id", "sudo-use-fleet-sudo"), + ("${X-sudo} id", "sudo-use-fleet-sudo"), + ("${X:=sudo} id", "sudo-use-fleet-sudo"), + ("true && $(printf sudo) id", "sudo-use-fleet-sudo"), + ("env $(printf sudo) id", "sudo-use-fleet-sudo"), + ("X=1 ${X:-sudo} id", "sudo-use-fleet-sudo"), + ('eval "$(echo sudo id)"', "sudo-use-fleet-sudo"), + ("eval $(echo sudo id)", "sudo-use-fleet-sudo"), + ("eval \"$(printf 'sudo id')\"", "sudo-use-fleet-sudo"), +) + + +@pytest.mark.parametrize("tokenizing", ["posix", "windows"]) +@pytest.mark.parametrize(("command", "rule_id"), FIND_EXEC_AND_EXPANSION_HARD_COMMANDS) +def test_hard_rules_judge_find_exec_and_expanded_programs( + command: str, rule_id: str, tokenizing: str, request: pytest.FixtureRequest +) -> None: + if tokenizing == "windows": + request.getfixturevalue("windows_tokens") + verdict = guard._hard_policy_verdict(command) + assert verdict is not None and verdict.rule_id == rule_id + + +@pytest.mark.parametrize( + "command", + [ + # `sudo` is an argument of find, or of the command -exec runs. + "find . -name sudo", + "find . -exec grep sudo {} +", + "find . -exec echo sudo \\;", + "find . -name '*.py' -exec sed -i s/sudo/x/ {} +", + "find . -path ./sudo -prune -o -print", + "find . -exec ls {} \\; -name sudo", + "find . -exec env OMI_SUDO_OK=1 sudo id \\;", + # The printed name is not in command position, or is a string. + "echo $(printf sudo)", + "grep ${X:-sudo} f", + "echo '$(printf sudo) id'", + 'echo "$(printf sudo) id"', + "OMI_SUDO_OK=1 $(printf sudo) id", + "${EDITOR:-vi} file", + "$(which python3) -V", + 'cd "${TMPDIR:-/tmp}"', + 'eval "$(echo hi)"', + 'eval "$(ssh-agent -s)"', + 'eval "$(pyenv init -)"', + # Out of scope (#455): the guard does not evaluate an expansion whose + # output is not a literal it can read, so these are not judged. + "X=sudo; $X id", + "$(cat prog.txt) id", + "$(printf '%s' sudo) id", + "$(echo -n sudo) id", + 'eval "$(cat script.sh)"', + ], +) +def test_hard_rules_leave_find_args_and_unread_expansions_alone(command: str) -> None: + assert guard._hard_policy_verdict(command) is None + + @pytest.mark.parametrize("tokenizing", ["posix", "windows"]) def test_unclosed_process_substitution_is_still_judged( tokenizing: str, request: pytest.FixtureRequest diff --git a/uv.lock b/uv.lock index aa5dbd7..0e11ad1 100644 --- a/uv.lock +++ b/uv.lock @@ -2364,7 +2364,7 @@ wheels = [ [[package]] name = "omind" -version = "10.2.25" +version = "10.2.26" source = { editable = "." } dependencies = [ { name = "cryptography" }, From eb2156e2b7114c99d00df9644822476ceb8e3a8f Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Fri, 2 Oct 2026 19:54:42 -0500 Subject: [PATCH 2/2] fix(guard): address #455 review on find -exec and literal expansions - _EVAL_LITERAL_RE matches each blank run one way only (words are blank-separated non-blank runs; the in-quote blanks match only when quoted), so `eval "$(echo ` plus a long blank run is linear, not cubic. - Each find action's command is its own subject, so one command's opt-in never covers another's sudo. - The expanded text is searched again, one level deep: $(printf bash) -c, ${X:-sh} -c and $(printf find) . -exec are judged. - ${X:+NAME} and ${X+NAME}; eval of backticks, of "\$(...)" and with blanks inside the quotes; echo -n/-e/-E, printf --, a path or command/builtin as the printer. - find stages only: every -exec/-execdir/-ok/-okdir word, quoted or not, starts a candidate command, so a primary's value or a quoted action word cannot hide one. mytool -ok sudo x stays an argument. - Stdin-script words get the literal expansion too. - Tests: deny table under POSIX and Windows tokenizing, the benign list under both, per-command opt-in, fail-open when either new search raises, and linear-time pins at 10,000 and 100,000. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011Zj1PCfsSSJgzLxi32jvde --- CHANGELOG.md | 28 ++++--- src/omind/guard.py | 178 ++++++++++++++++++++++++++++++++------------ tests/test_guard.py | 106 +++++++++++++++++++++++++- 3 files changed, 252 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2830832..10b7cbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,19 +16,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 sudo id \;` was allowed: `-exec` is not a separator or a wrapper, so `sudo` never reached command position. `$(printf sudo) id`, `${X:-sudo} id` and `eval "$(echo sudo id)"` were allowed too. - - The words after `-exec`, `-execdir`, `-ok` and `-okdir`, up to `\;`, - `';'` or `{} +`, are judged as a command. The split comes from the same - stage walk the repo-work classifier reads. `find . -name sudo`, `find . - -exec grep sudo {} +` and `find . -exec echo sudo \;` stay allowed. + - In a `find` stage, the words after every `-exec`, `-execdir`, `-ok` and + `-okdir`, quoted or not, up to `\;`, `';'`, `{} +` or the next action + word, are judged as a command of their own. Each action is its own + subject, so one command's opt-in never covers another's `sudo`. Even an + action word that is a primary's value (`-name -exec -exec sudo id \;`) + starts a candidate: over-judging is the safe direction. `find . -name + sudo`, `find . -exec grep sudo {} +`, `find . -exec echo sudo \;` and + `mytool -ok sudo x` stay allowed. - An expansion that is a whole word and prints a bare program name is replaced by that name before the command is judged: `$(echo|printf - NAME)`, `` `echo NAME` ``, `${X:-NAME}` (`-`, `=`, `:=` too), optionally - double-quoted. So is `eval` of a literal `echo`/`printf`. The name counts - only in command position, so `echo $(printf sudo)` and `grep ${X:-sudo} f` - stay allowed. + NAME)`, `` `echo NAME` ``, `${X:-NAME}` (`-`, `=`, `+`, each with or + without `:`), optionally double-quoted. The printer may carry `echo + -n/-e/-E`, `printf --`, a path, or `command`/`builtin`. So is `eval` of + a literal `echo`/`printf`, by `$(…)`, `\$(…)` or backticks. The expanded + command is searched again, one level deep, so `$(printf bash) -c 'sudo + id'` and `$(printf find) . -exec sudo id \;` are judged, and so are the + words a stdin script runs (`echo '$(printf sudo) id' | bash`). The name + counts only in command position, so `echo $(printf sudo)` and `grep + ${X:-sudo} f` stay allowed. Both expansion regexes match each blank run + one way only, so they stay linear (pinned at 100,000). - Out of scope, and pinned as allowed by tests: an expansion whose output the guard would have to evaluate (`X=sudo; $X id`, `$(cat f) id`, - `$(printf '%s' sudo) id`, `$(echo -n sudo) id`, `eval "$(cat f)"`). + `$(printf '%s' sudo) id`, `eval "$(cat f)"`). ## [10.2.25] - 2026-10-02 diff --git a/src/omind/guard.py b/src/omind/guard.py index 622b590..7e0c29e 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -3962,42 +3962,73 @@ def _with_positionals(body: str, positional: list[str]) -> str | None: return _ALL_POSITIONALS_RE.sub(lambda _m: joined, body) -#: A ``find`` action word whose following words are a command (#455). -_EXEC_ACTION_RE = re.compile(r"(? str | None: +) -> list[str]: """The commands ``find -exec``/``-execdir``/``-ok``/``-okdir`` run in - ``text``, one per line, each from the word after the action to the last - word before its ``\\;``/``';'``/``{} +`` (#455): ``find . -exec sudo id - \\;`` runs ``sudo id``. ``None`` when there is none. + ``text``, one subject each (#455): ``find . -exec sudo id \\;`` runs + ``sudo id``. Each is re-quoted (:func:`shlex.join`), so a quoted argument + stays one word and an opt-in counts for its own command only. + + Every action word, quoted or not, starts a candidate (``find . '-exec' + sudo id \\;``), even one that is really a primary's value (``-name + -exec -exec sudo id \\;``): over-judging is the safe direction. A + candidate runs to its ``\\;``/``';'``/``{} +``, or to the next action + word, so each word joins at most one candidate. ``stages`` are :func:`_program_stages` of ``masked`` (its - :func:`policy.shell_code_text`), which already cuts a segment at each - action, so a stage that shares its segment's end with the stage before - it is an ``-exec`` command. Its text starts at the action, not at the - program, so wrappers (``-exec env sudo id``) and an opt-in assignment - are judged as the hard rules judge any command. Each action's window is - scanned once, so a long chain stays linear (#445).""" + :func:`policy.shell_code_text`). Only ``find`` stages count, and each + segment is tokenized once from its first ``find`` (a heredoc body the + stage owns is not), so a long or nested chain stays linear (#445).""" commands: list[str] = [] - prev_start = prev_end = -1 - for _program, args, start, end in stages: - if end == prev_end: - action: re.Match[str] | None = None - for action in _EXEC_ACTION_RE.finditer(masked, prev_start, start): # noqa: B007 - pass - if action is not None: - stop = start - for k, word in enumerate(_WORD_RE.finditer(masked, start, end)): - stop = word.end() - if k >= len(args): - break - commands.append(text[action.end() : stop]) - prev_start, prev_end = start, end - return "\n".join(commands) if commands else None + covered = -1 # the segment end already tokenized + for program, _args, start, end in stages: + if program not in _FIND_PROGRAMS or end == covered: + continue + covered = end + part = text[start : start + len(masked[start:end].rstrip())] + if "-exec" not in part and "-ok" not in part: + continue + try: + tokens = _shell_tokens(part) + except ValueError: + tokens = [word.strip("'\"") for word in part.split()] + current: list[str] | None = None + for token in tokens: + if token in _EXEC_ACTIONS: + if current: + commands.append(shlex.join(current)) + current = [] + elif current is None: + continue + elif token in _FIND_EXEC_ENDS or (token == "+" and current[-1:] == ["{}"]): + if current: + commands.append(shlex.join(current)) + current = None + else: + current.append(token) + if current: + commands.append(shlex.join(current)) + return commands + + +#: The program an expansion runs to print a literal (#455): ``echo``, with +#: any ``-n``/``-e``/``-E`` switches, or ``printf``, with an optional +#: ``--``; by path or behind ``command``/``builtin`` too. The literal holds +#: no ``\\`` or ``%``, so none of these changes what is printed. +_PRINTER = ( + r"(?:(?:command|builtin)[ \t]+)?(?:[\w./-]*/)?" + r"(?:echo(?:[ \t]+-[neE]+)*|printf(?:[ \t]+--)?)[ \t]+" +) def _literal_name(tag: str) -> str: @@ -4008,22 +4039,32 @@ def _literal_name(tag: str) -> str: #: An expansion that is a whole word and prints a bare program name (#455): #: ``$(printf sudo)``, ``$(echo 'sudo')``, `` `echo sudo` ``, ``${X:-sudo}`` -#: (``-``, ``:-``, ``=``, ``:=``), optionally double-quoted as one word. Only -#: these shapes: anything else (a variable, ``$(cat f)``, ``printf '%s'``, -#: ``echo -n``) needs the shell to evaluate it, which the guard does not. +#: (``-``, ``=``, ``+``, each with or without ``:``), optionally +#: double-quoted as one word. Only these shapes: anything else (a variable, +#: ``$(cat f)``, ``printf '%s'``) needs the shell to evaluate it, which the +#: guard does not. No two adjacent parts match the same blank, so a long +#: blank run cannot make the match backtrack super-linearly (#445). _EXPANDED_NAME_RE = re.compile( r'(?"?)(?:' - r"\$\([ \t]*(?:echo|printf)[ \t]+" + _literal_name("a") + r"[ \t]*\)" - r"|`[ \t]*(?:echo|printf)[ \t]+" + _literal_name("b") + r"[ \t]*`" - r"|\$\{\w+:?[-=]" + _literal_name("c") + r"\}" + r"\$\([ \t]*" + _PRINTER + _literal_name("a") + r"[ \t]*\)" + r"|`[ \t]*" + _PRINTER + _literal_name("b") + r"[ \t]*`" + r"|\$\{\w+:?[-=+]" + _literal_name("c") + r"\}" r")(?P=dq)(?=[\s;&|)]|\Z)" ) +#: One literal word an ``eval``'d ``echo``/``printf`` prints: no blank, +#: ``$``, quote, backtick, ``%`` or backslash. +_EVAL_WORD = r"[\w./+=,:-]+" #: ``eval`` of an ``echo``/``printf`` of literal words (#455): ``eval "$(echo -#: sudo id)"`` runs ``sudo id``. Literal means no ``$``, quote, backtick, -#: ``%`` or backslash in the printed words. +#: sudo id)"``, ``eval `echo sudo id```, ``eval " $(echo sudo id)"``, and +#: ``eval "\\$(echo sudo id)"``, whose substitution eval itself runs. The +#: words are blank-separated runs of :data:`_EVAL_WORD`, and the blanks +#: inside the double quotes are matched only when there are quotes, so each +#: blank run has one way to match: linear time on any input (#455 review). _EVAL_LITERAL_RE = re.compile( - r"(?\"?)\$\([ \t]*(?:echo|printf)[ \t]+" - r"""(?P['"]?)(?P[\w./+=,: \t-]+)(?P=q)[ \t]*\)(?P=dq)(?=[\s;&|)]|\Z)""" + r"(?\")[ \t]*)?" + r"(?:\\?\$\(|(?P`))[ \t]*" + _PRINTER + r"""(?P['"]?)""" + r"(?P" + _EVAL_WORD + r"(?:[ \t]+" + _EVAL_WORD + r")*)" + r"(?P=q)[ \t]*(?(bt)`|\))(?(dq)[ \t]*\")(?=[\s;&|)]|\Z)" ) @@ -4049,6 +4090,18 @@ def name(match: re.Match[str]) -> str: return expanded if expanded != text else None +def _add_words(words: list[str], text: str) -> None: + """Append ``text`` (a :func:`_words_in_command_position` result) to + ``words``, and its literal expansions too (#455 review): ``echo + '$(printf sudo) id' | bash`` runs ``sudo id``. The expansion never + raises into the caller: the words themselves are already kept.""" + words.append(text) + with contextlib.suppress(Exception): + expanded = _expanded_command_text(text) + if expanded is not None: + words.append(expanded) + + def _local_shell_subjects( text: str, code: list[str], words: list[str], runs: list[str] | None = None ) -> None: @@ -4069,9 +4122,10 @@ def _local_shell_subjects( # {} +` chain is not rescanned to the end per shell (#432 review). ends = _stage_ends(masked, sorted(stage[2] for stage in stages)) # `find . -exec sudo id \;` runs `sudo id` (#455). - exec_commands = _find_exec_commands(text, masked, stages) - if exec_commands is not None: - code.append(exec_commands) + # One subject per command, so one command's opt-in never covers another's + # match (#455 review). A failure here leaves the rest of the search intact. + with contextlib.suppress(Exception): + code.extend(_find_exec_commands(text, masked, stages)) floor = 0 for program, _args, pos, _end in stages: if program not in _LOCAL_SHELLS and program not in ("source", "."): @@ -4086,7 +4140,7 @@ def _local_shell_subjects( # `bash <(echo sudo id)`, `source <(…)`, `bash < <(…)`: what the # substitution prints is the code the shell runs (#444). if producer.strip(): - words.append(_words_in_command_position(producer)) + _add_words(words, _words_in_command_position(producer)) if runs is not None: runs.append(_word_runs(producer)) continue @@ -4136,7 +4190,7 @@ def _local_shell_subjects( producer_start = _pipeline_head(masked, producer_end, outer_floor) if text[producer_start:producer_end].strip(): script.append(text[producer_start:producer_end]) - words.append(_words_in_command_position(script[-1])) + _add_words(words, _words_in_command_position(script[-1])) if runs is not None: runs.append(_word_runs(script[-1])) # A producer's heredoc body starts on the next line (`cat < _HardSubjects: Pure and never raises: on any walk error only the command itself is judged, which is the pre-#430 behaviour. When the subject search fails - partway, what it already found is kept (#449 review).""" + partway, what it already found is kept (#449 review). + + A command or body holding an expansion that prints a literal program name + (``$(printf bash) -c 'sudo id'``, #455) is searched again with that name + in place, one level deep: :func:`_expanded_command_text` never yields a + new expansion, so this terminates. Its subjects join ``code``, ``words`` + and ``quoted``; ``runs`` keeps the classifiers' view unchanged.""" + return _subjects_of(command, expand=True) + + +def _subjects_of(command: str, *, expand: bool) -> _HardSubjects: + """:func:`_hard_rule_subjects`, re-searching literal expansions when + ``expand``. Never raises.""" try: sites, _cwd, _local, bodies = _shell_walk(command) except Exception: @@ -4201,13 +4267,20 @@ def _hard_rule_subjects(command: str) -> _HardSubjects: code: list[str] = [command, *bodies] words: list[str] = [] runs: list[str] = [] + expansions: list[str] = [] + if expand: + try: + for text in (command, *bodies): + # `$(printf sudo) id`, `${X:-sudo} id`, `eval "$(echo sudo + # id)"` (#455): the literal the expansion prints, in its place. + expanded = _expanded_command_text(text) + if expanded is not None: + expansions.append(expanded) + code.append(expanded) + except Exception: + pass # keep what was already found: more text judged, never less try: for text in (command, *bodies): - # `$(printf sudo) id`, `${X:-sudo} id`, `eval "$(echo sudo id)"` - # (#455): the literal the expansion prints, in its place. - expanded = _expanded_command_text(text) - if expanded is not None: - code.append(expanded) _local_shell_subjects(text, code, words, runs) except Exception: pass # keep what was already found: more text judged, never less @@ -4228,6 +4301,13 @@ def _hard_rule_subjects(command: str) -> _HardSubjects: quoted.append(re.sub(r"['\"]|\\n", "\n", site.text)) except Exception: pass # likewise: keep the sites already read + for expanded in expansions: + # `$(printf bash) -c 'sudo id'`, `$(printf find) . -exec sudo id \;` + # (#455 review): what the expanded command runs, one level only. + more = _subjects_of(expanded, expand=False) + code.extend(more.code[1:]) # more.code[0] is `expanded`, already judged + words.extend(more.words) + quoted.extend(more.quoted) return _HardSubjects(tuple(code), tuple(words), tuple(quoted), tuple(runs)) diff --git a/tests/test_guard.py b/tests/test_guard.py index 0d98338..81ce18e 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -3564,6 +3564,32 @@ def test_wrapper_clusters_leave_benign_commands_alone(command: str) -> None: ('eval "$(echo sudo id)"', "sudo-use-fleet-sudo"), ("eval $(echo sudo id)", "sudo-use-fleet-sudo"), ("eval \"$(printf 'sudo id')\"", "sudo-use-fleet-sudo"), + # #455 EM review: the expanded text is searched again (item 3). + ("$(printf bash) -c 'sudo id'", "sudo-use-fleet-sudo"), + ("${X:-sh} -c 'sudo id'", "sudo-use-fleet-sudo"), + ("$(printf find) . -exec sudo id \\;", "sudo-use-fleet-sudo"), + # More expansion operators (item 4). + ("${X:+sudo} id", "sudo-use-fleet-sudo"), + ("${X+sudo} id", "sudo-use-fleet-sudo"), + # One -exec command's opt-in never covers another's match (item 2). + ("find . -exec env OMI_SUDO_OK=1 true \\; -exec sudo id \\;", "sudo-use-fleet-sudo"), + ("find . -exec sudo id \\; -exec env OMI_SUDO_OK=1 true \\;", "sudo-use-fleet-sudo"), + # More eval shapes (item 5). + ("eval `echo sudo id`", "sudo-use-fleet-sudo"), + ('eval " $(echo sudo id)"', "sudo-use-fleet-sudo"), + ('eval "\\$(echo sudo id)"', "sudo-use-fleet-sudo"), + # find argument traps (item 6). + ("find . -name -exec -exec sudo id \\;", "sudo-use-fleet-sudo"), + ("find . '-exec' sudo id \\;", "sudo-use-fleet-sudo"), + ('find . "-exec" sudo id \\;', "sudo-use-fleet-sudo"), + # Stdin-script words (should fix). + ("echo '$(printf sudo) id' | bash", "sudo-use-fleet-sudo"), + # Printers by another spelling, once pinned as out of scope. + ("$(command printf sudo) id", "sudo-use-fleet-sudo"), + ("$(printf -- sudo) id", "sudo-use-fleet-sudo"), + ("$(/usr/bin/printf sudo) id", "sudo-use-fleet-sudo"), + ("$(echo -e sudo) id", "sudo-use-fleet-sudo"), + ("$(echo -n sudo) id", "sudo-use-fleet-sudo"), ) @@ -3601,19 +3627,95 @@ def test_hard_rules_judge_find_exec_and_expanded_programs( 'eval "$(echo hi)"', 'eval "$(ssh-agent -s)"', 'eval "$(pyenv init -)"', + # Only find's action words start a command (#455 review). + "mytool -ok sudo x", + "mytool -exec sudo x", + "find . -name '-exec sudo id'", + "find . -exec grep -e 'a; sudo id' {} +", + "find . -exec env OMI_SUDO_OK=1 sudo id \\; -exec ls \\;", # Out of scope (#455): the guard does not evaluate an expansion whose # output is not a literal it can read, so these are not judged. "X=sudo; $X id", "$(cat prog.txt) id", "$(printf '%s' sudo) id", - "$(echo -n sudo) id", 'eval "$(cat script.sh)"', ], ) -def test_hard_rules_leave_find_args_and_unread_expansions_alone(command: str) -> None: +@pytest.mark.parametrize("tokenizing", ["posix", "windows"]) +def test_hard_rules_leave_find_args_and_unread_expansions_alone( + command: str, tokenizing: str, request: pytest.FixtureRequest +) -> None: + if tokenizing == "windows": + request.getfixturevalue("windows_tokens") assert guard._hard_policy_verdict(command) is None +def test_find_exec_commands_are_one_subject_each() -> None: + """#455 review: each action's command is its own subject, so an opt-in + counts for its own command only.""" + command = "find . -exec env OMI_SUDO_OK=1 true \\; -exec sudo id \\;" + subjects = guard._hard_rule_subjects(command) + assert "env OMI_SUDO_OK=1 true" in subjects.code + assert "sudo id" in subjects.code + assert not any("\n" in text for text in subjects.code[1:]) + + +@pytest.mark.parametrize( + ("broken", "command"), + [ + ("_expanded_command_text", "find . -exec sudo id \\;"), + ("_find_exec_commands", "$(printf sudo) id"), + ("_find_exec_commands", "echo sudo id | bash"), + ], +) +def test_find_exec_and_expansion_failures_keep_other_subjects( + broken: str, command: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """#455 review: either new search raising leaves the subject search to + finish with what the other parts find (fail open, never a crash).""" + + def boom(*_args: object, **_kwargs: object) -> object: + raise RuntimeError("boom") + + monkeypatch.setattr(guard, broken, boom) + cold_shell_caches() + try: + guard._hard_rule_subjects(command) # never raises + verdict = guard._hard_policy_verdict(command) + finally: + cold_shell_caches() + assert verdict is not None and verdict.rule_id == "sudo-use-fleet-sudo" + + +#: #455 review: (prefix, repeated part). The first is the measured shape. +_LONG_EXPANSIONS_455 = { + "eval-blanks": ('sudo_x # eval "$(echo ', "\t"), + "eval-words": ('eval "$(echo ', "a "), + "eval-backtick": ("eval `echo ", " "), + "printf-blanks": ("$(printf ", "\t"), + "default-blanks": ("${X:-", " "), + "find-actions": ("find . ", "-exec "), +} + + +@pytest.mark.parametrize("count", [10_000, 100_000]) +@pytest.mark.parametrize( + "shape", list(_LONG_EXPANSIONS_455.values()), ids=list(_LONG_EXPANSIONS_455) +) +def test_find_exec_and_expansion_searches_stay_linear(shape: tuple[str, str], count: int) -> None: + """#455 review: `eval "$(echo ` then a long blank run was cubic in + `_EVAL_LITERAL_RE` (0.27 s at 800, about 17 s at 3,200). Both expansion + regexes, and the find action split, stay linear at 10,000 and 100,000.""" + prefix, repeated = shape + command = prefix + repeated * count + with _hard_time_limit(traced_bound(1.0)): + guard._EVAL_LITERAL_RE.sub("", command) + guard._EXPANDED_NAME_RE.sub("", command) + cold_shell_caches() + with _hard_time_limit(traced_bound(1.0)): + guard._hard_policy_verdict(command) + + @pytest.mark.parametrize("tokenizing", ["posix", "windows"]) def test_unclosed_process_substitution_is_still_judged( tokenizing: str, request: pytest.FixtureRequest