From 17a33a851abca11f530f94d89d3d687dc9f8b3a3 Mon Sep 17 00:00:00 2001 From: Lavindeep Dhillon Date: Fri, 10 Jul 2026 06:41:37 -0400 Subject: [PATCH 1/4] Harden LOW command classification against execution and boundary bypasses. Require capability-bearing LOW tools to prove they cannot execute helpers, write files, or read outside the workspace, with an adversarial regression corpus for the previously auto-run cases. --- shellpilot/policy/command_policy.py | 394 +++++++++++++++++++++++++++- tests/test_command_policy.py | 47 +++- 2 files changed, 432 insertions(+), 9 deletions(-) diff --git a/shellpilot/policy/command_policy.py b/shellpilot/policy/command_policy.py index 39d1060..e66c769 100644 --- a/shellpilot/policy/command_policy.py +++ b/shellpilot/policy/command_policy.py @@ -2,6 +2,24 @@ Policy is deterministic first: no model call ever decides risk, and the model can never downgrade what this module returns (section 14.4). + +LOW invariant +------------- +A command may be classified LOW only when the argv form cannot: + +- execute arbitrary code or configured external helpers +- write or truncate files +- perform network I/O +- read content outside the workspace + +Capability-bearing LOW tools (searchers, ``tree``, ``ps``, ``ls``, readers, and +read-only git verbs) must prove that invariant via explicit checks. Unknown +long options on those tools escalate to MEDIUM — LOW is earned, not assumed +from the executable basename alone. + +Accepted residual: classification still keys off the basename (PATH substitution +of a LOW name remains LOW by design of the argv executor); path-qualified +executables already escalate out of LOW. """ from __future__ import annotations @@ -39,9 +57,17 @@ "ps", } ) +# Inert tools take no filesystem/process payload that can violate the LOW +# invariant under shell=False argv execution. +INERT_LOW_EXECUTABLES: Final = frozenset( + {"pwd", "true", "false", "uname", "date", "whoami", "which", "echo", "df"} +) READER_EXECUTABLES: Final = frozenset( {"cat", "head", "tail", "grep", "egrep", "fgrep", "rg", "wc", "file", "stat", "du"} ) +SEARCHER_EXECUTABLES: Final = frozenset({"grep", "egrep", "fgrep", "rg"}) +# Path-bearing LOW tools that are not already covered by READER_EXECUTABLES. +PATH_CHECKED_LOW_EXECUTABLES: Final = frozenset({"ls", "tree"}) GIT_READONLY_VERBS: Final = frozenset( { "status", @@ -71,6 +97,8 @@ "--work-tree", } ) +# Diff/show helpers that can execute configured external programs. +GIT_EXTERNAL_HELPER_OPTIONS: Final = frozenset({"--ext-diff", "--textconv"}) SHELLS: Final = frozenset({"sh", "bash", "zsh", "fish", "dash", "ksh"}) PACKAGE_MANAGERS: Final = frozenset( { @@ -122,6 +150,170 @@ "secrets", ) +# Long options that are safe for LOW auto-run on searchers. Anything else +# unknown escalates — LOW must be proven, not assumed from the basename. +SEARCHER_SAFE_LONG_OPTIONS: Final = frozenset( + { + "--help", + "--version", + "--color", + "--colour", + "--include", + "--exclude", + "--exclude-dir", + "--exclude-from", + "--file", + "--regexp", + "--ignore-case", + "--invert-match", + "--word-regexp", + "--line-regexp", + "--fixed-strings", + "--basic-regexp", + "--extended-regexp", + "--perl-regexp", + "--count", + "--files-with-matches", + "--files-without-match", + "--only-matching", + "--no-filename", + "--with-filename", + "--line-number", + "--no-messages", + "--quiet", + "--silent", + "--max-count", + "--byte-offset", + "--binary-files", + "--text", + "--directories", + "--devices", + "--recursive", + "--dereference-recursive", + "--no-ignore-case", + "--heading", + "--break", + "--context", + "--after-context", + "--before-context", + "--column", + "--vimgrep", + "--json", + "--debug", + "--trace", + "--hidden", + "--no-hidden", + "--no-ignore", + "--no-ignore-vcs", + "--no-ignore-parent", + "--no-ignore-global", + "--ignore-file", + "--ignore-file-case", + "--glob", + "--iglob", + "--type", + "--type-not", + "--type-add", + "--type-clear", + "--type-list", + "--files", + "--sort", + "--sortr", + "--max-depth", + "--max-filesize", + "--max-columns", + "--max-columns-preview", + "--line-buffered", + "--block-buffered", + "--mmap", + "--no-mmap", + "--search-zip", + "--follow", + "--one-file-system", + "--no-unicode", + "--engine", + "--regexp-size-limit", + "--dfa-size-limit", + "--stop-on-nonmatch", + "--passthru", + "--null", + "--null-data", + "--field-match-separator", + "--field-context-separator", + "--path-separator", + "--hyperlink-format", + "--stats", + "--crlf", + "--no-crlf", + } +) +SEARCHER_EXECUTION_OPTIONS: Final = frozenset({"--pre", "--pre-glob"}) +TREE_SAFE_LONG_OPTIONS: Final = frozenset( + { + "--help", + "--version", + "--noreport", + "--charset", + "--filelimit", + "--si", + "--du", + "--inodes", + "--device", + "--dirsfirst", + "--matchdirs", + "--prune", + "--ignore", + "--gitignore", + "--gitfile", + "--match", + "--fromfile", + "--fflinks", + "--nolinks", + "--timefmt", + } +) +TREE_OUTPUT_OPTIONS: Final = frozenset({"-o", "--output"}) +LS_SAFE_LONG_OPTIONS: Final = frozenset( + { + "--help", + "--version", + "--color", + "--colour", + "--group-directories-first", + "--time-style", + "--format", + "--indicator-style", + "--quoting-style", + "--block-size", + "--hide", + "--ignore", + "--ignore-backups", + "--classify", + "--file-type", + "--hyperlink", + "--si", + "--human-readable", + "--inode", + "--size", + "--recursive", + "--reverse", + "--almost-all", + "--all", + "--author", + "--context", + "--directory", + "--dired", + "--full-time", + "--literal", + "--numeric-uid-gid", + "--no-group", + "--tabsize", + "--width", + "--sort", + "--time", + } +) + @dataclass(frozen=True) class CommandRisk: @@ -195,6 +387,52 @@ def _path_arg_outside_workspace(argv: list[str], workspace: Path) -> str | None: return None +def _long_option_name(token: str) -> str | None: + if not token.startswith("--") or token == "--": + return None + return token.partition("=")[0] + + +def _unknown_long_option(argv: list[str], allowed: frozenset[str]) -> str | None: + for token in argv[1:]: + name = _long_option_name(token) + if name is not None and name not in allowed: + return f"unrecognized option {name}" + return None + + +def _option_present(argv: list[str], names: frozenset[str]) -> str | None: + """Return the matching option name when present (``--opt`` or ``--opt=``).""" + for token in argv[1:]: + if token in names: + return token + name = _long_option_name(token) + if name is not None and name in names: + return name + return None + + +def _split_option_value(argv: list[str], names: frozenset[str]) -> str | None: + """Value of a space-separated or ``=``-attached option, else None.""" + index = 1 + while index < len(argv): + token = argv[index] + if token in names: + return argv[index + 1] if index + 1 < len(argv) else None + name = _long_option_name(token) + if name is not None and name in names and "=" in token: + return token.split("=", 1)[1] + # Glued short form: -oFILE + if not token.startswith("--"): + for name in names: + if name.startswith("-") and not name.startswith("--") and token.startswith(name): + glued = token[len(name) :] + if glued: + return glued + index += 1 + return None + + def _scan_git_verb(argv: list[str]) -> tuple[str, list[str], bool]: conservative_global = False index = 1 @@ -263,6 +501,14 @@ def _git_output_path(tokens: list[str]) -> str | None: return None +def _git_external_helper(flags: list[str]) -> str | None: + for flag in flags: + name = flag.partition("=")[0] + if name in GIT_EXTERNAL_HELPER_OPTIONS: + return f"{name} can execute configured external helpers" + return None + + def _classify_git(argv: list[str], workspace: Path) -> CommandRisk: verb, verb_args, conservative_global = _scan_git_verb(argv) flags = [token for token in argv[1:] if token.startswith("-")] @@ -300,9 +546,20 @@ def _classify_git(argv: list[str], workspace: Path) -> CommandRisk: return CommandRisk(RiskLevel.MEDIUM, (f"git {verb} changes repository state",)) if conservative_global: return CommandRisk(RiskLevel.MEDIUM, ("git uses a non-benign global option",)) - if verb in GIT_READONLY_VERBS: - return CommandRisk(RiskLevel.LOW, ()) - if verb == "stash" and verb_args and verb_args[0] in ("list", "show"): + helper = _git_external_helper(flags) + if helper: + return CommandRisk(RiskLevel.MEDIUM, (helper,)) + if verb in GIT_READONLY_VERBS or ( + verb == "stash" and verb_args and verb_args[0] in ("list", "show") + ): + # Read-only git still has to prove the LOW invariant: no out-of-workspace + # path payloads (e.g. `git diff --no-index /etc/a /etc/b`). + outside = _path_arg_outside_workspace(argv, workspace) + if outside: + return CommandRisk( + RiskLevel.HIGH, + (f"git {verb or '?'} reads outside the workspace boundary: {outside}",), + ) return CommandRisk(RiskLevel.LOW, ()) return CommandRisk(RiskLevel.MEDIUM, (f"git {verb or '?'} changes repository state",)) @@ -319,6 +576,102 @@ def _classify_rm(argv: list[str], workspace: Path) -> CommandRisk: return CommandRisk(RiskLevel.MEDIUM, ("deletes a file",)) +def _classify_searcher(argv: list[str], workspace: Path) -> CommandRisk | None: + """Extra LOW-invariant checks for grep/rg family. None = fall through.""" + hook = _option_present(argv, SEARCHER_EXECUTION_OPTIONS) + if hook: + return CommandRisk( + RiskLevel.HIGH, + (f"{hook} executes a preprocessor over matched files",), + ) + unknown = _unknown_long_option(argv, SEARCHER_SAFE_LONG_OPTIONS) + if unknown: + return CommandRisk(RiskLevel.MEDIUM, (unknown,)) + outside = _path_arg_outside_workspace(argv, workspace) + if outside: + return CommandRisk(RiskLevel.HIGH, (f"reads outside the workspace boundary: {outside}",)) + return None + + +def _classify_tree(argv: list[str], workspace: Path) -> CommandRisk | None: + output = _split_option_value(argv, TREE_OUTPUT_OPTIONS) + if output is not None or _option_present(argv, TREE_OUTPUT_OPTIONS): + target = output or "" + if target and _path_arg_outside_workspace(["tree", target], workspace): + return CommandRisk( + RiskLevel.HIGH, + ("tree output path is outside the workspace boundary",), + ) + return CommandRisk(RiskLevel.MEDIUM, ("tree writes output to a file",)) + unknown = _unknown_long_option(argv, TREE_SAFE_LONG_OPTIONS) + if unknown: + return CommandRisk(RiskLevel.MEDIUM, (unknown,)) + outside = _path_arg_outside_workspace(argv, workspace) + if outside: + return CommandRisk(RiskLevel.HIGH, (f"reads outside the workspace boundary: {outside}",)) + return None + + +def _ps_exposes_environment(argv: list[str]) -> bool: + """True when argv likely requests process environment display. + + BSD ``ps e`` / ``ps auxe`` expose environments. macOS ``-E`` does too. + SysV ``-e`` means "every process" and is left alone (listing only). + """ + for token in argv[1:]: + if token in {"e", "E"}: + return True + name = _long_option_name(token) + if name is not None and "env" in name.lower(): + return True + if not token.startswith("-") and token.isalpha() and "e" in token.lower(): + # BSD clustered flags without a leading dash: auxe, ue, ... + return True + if token.startswith("-") and not token.startswith("--") and "E" in token[1:]: + return True + return False + + +def _classify_ps(argv: list[str]) -> CommandRisk | None: + if _ps_exposes_environment(argv): + return CommandRisk(RiskLevel.MEDIUM, ("ps can expose process environments",)) + # Unknown long options fail the LOW proof. + unknown = _unknown_long_option( + argv, + frozenset( + { + "--help", + "--version", + "--pid", + "--ppid", + "--user", + "--sort", + "--format", + "--forest", + "--cols", + "--columns", + "--width", + "--headers", + "--no-headers", + "--deselect", + } + ), + ) + if unknown: + return CommandRisk(RiskLevel.MEDIUM, (unknown,)) + return None + + +def _classify_ls(argv: list[str], workspace: Path) -> CommandRisk | None: + unknown = _unknown_long_option(argv, LS_SAFE_LONG_OPTIONS) + if unknown: + return CommandRisk(RiskLevel.MEDIUM, (unknown,)) + outside = _path_arg_outside_workspace(argv, workspace) + if outside: + return CommandRisk(RiskLevel.HIGH, (f"reads outside the workspace boundary: {outside}",)) + return None + + def classify_command(argv: list[str], *, workspace: Path) -> CommandRisk: """Classify an argv command (shell=False) by deterministic rules.""" if not argv or not argv[0].strip(): @@ -374,6 +727,28 @@ def classify_command(argv: list[str], *, workspace: Path) -> CommandRisk: if argv[1:] == ["--version"]: return CommandRisk(RiskLevel.LOW, ()) return CommandRisk(RiskLevel.MEDIUM, ("runs arbitrary python code",)) + + # Capability-bearing LOW candidates: prove the invariant before AUTO. + if executable in SEARCHER_EXECUTABLES: + special = _classify_searcher(argv, workspace) + if special is not None: + return special + return CommandRisk(RiskLevel.LOW, ()) + if executable == "tree": + special = _classify_tree(argv, workspace) + if special is not None: + return special + return CommandRisk(RiskLevel.LOW, ()) + if executable == "ps": + special = _classify_ps(argv) + if special is not None: + return special + return CommandRisk(RiskLevel.LOW, ()) + if executable == "ls": + special = _classify_ls(argv, workspace) + if special is not None: + return special + return CommandRisk(RiskLevel.LOW, ()) if executable in READER_EXECUTABLES: # Unlike read_file (which honors allow_sensitive_reads via decide()), # classify_command sees only a RiskLevel and run_command is @@ -386,8 +761,17 @@ def classify_command(argv: list[str], *, workspace: Path) -> CommandRisk: return CommandRisk( RiskLevel.HIGH, (f"reads outside the workspace boundary: {outside}",) ) - # in-workspace readers fall through to the LOW return below - if executable in LOW_EXECUTABLES: + return CommandRisk(RiskLevel.LOW, ()) + if executable in INERT_LOW_EXECUTABLES or executable in LOW_EXECUTABLES: + # Remaining LOW allowlist entries (including inert tools). Path-bearing + # leftovers still get a boundary check so the invariant holds. + if executable in PATH_CHECKED_LOW_EXECUTABLES or executable not in INERT_LOW_EXECUTABLES: + outside = _path_arg_outside_workspace(argv, workspace) + if outside: + return CommandRisk( + RiskLevel.HIGH, + (f"reads outside the workspace boundary: {outside}",), + ) return CommandRisk(RiskLevel.LOW, ()) return CommandRisk( diff --git a/tests/test_command_policy.py b/tests/test_command_policy.py index 1a2f979..db8ccb5 100644 --- a/tests/test_command_policy.py +++ b/tests/test_command_policy.py @@ -254,10 +254,11 @@ def test_flags_not_treated_as_paths() -> None: assert result.risk == RiskLevel.MEDIUM -def test_ls_with_relative_escape_unchanged() -> None: - # read-only LOW commands never hit the write-boundary path +def test_ls_with_relative_escape_is_high() -> None: + # LOW must prove no out-of-workspace reads — ls is path-checked. result = classify_command(["ls", "../"], workspace=WS) - assert result.risk == RiskLevel.LOW + assert result.risk == RiskLevel.HIGH + assert any("outside the workspace" in reason for reason in result.reasons) # -- end relative-path boundary tests ------------------------------------------ @@ -306,7 +307,6 @@ def test_wc_relative_inside_stays_low() -> None: def test_bare_ls_no_path_stays_low() -> None: - # ls is not a reader executable; no path arg, no boundary check result = classify_command(["ls"], workspace=WS) assert result.risk == RiskLevel.LOW @@ -450,6 +450,45 @@ def test_empty_argv_is_blocked() -> None: assert result.risk == RiskLevel.BLOCKED +# -- LOW invariant: no execution / write / out-of-workspace auto-run ---------- + + +@pytest.mark.parametrize( + ("argv", "expected"), + [ + # Verified bypasses that previously classified LOW under balanced AUTO. + (["rg", "--pre", "sh -c evil", "x"], RiskLevel.HIGH), + (["rg", "--pre=./preprocessor", "x"], RiskLevel.HIGH), + (["rg", "--pre-glob", "*.txt", "x"], RiskLevel.HIGH), + (["git", "diff", "--ext-diff"], RiskLevel.MEDIUM), + (["git", "show", "--textconv"], RiskLevel.MEDIUM), + (["git", "diff", "--no-index", "/etc/passwd", "/etc/hosts"], RiskLevel.HIGH), + (["tree", "-o", "/tmp/shellpilot-out", "."], RiskLevel.HIGH), + (["tree", "--output=out.txt", "."], RiskLevel.MEDIUM), + (["tree", "-o", "out.txt", "."], RiskLevel.MEDIUM), + (["ps", "e"], RiskLevel.MEDIUM), + (["ps", "auxe"], RiskLevel.MEDIUM), + (["ps", "-E"], RiskLevel.MEDIUM), + # Unknown long options fail the LOW proof on capability tools. + (["rg", "--totally-unknown-flag", "x"], RiskLevel.MEDIUM), + (["ls", "--totally-unknown-flag"], RiskLevel.MEDIUM), + (["tree", "--totally-unknown-flag"], RiskLevel.MEDIUM), + # Benign forms must keep auto-running. + (["rg", "-n", "TODO", "."], RiskLevel.LOW), + (["rg", "--hidden", "-n", "TODO", "."], RiskLevel.LOW), + (["git", "diff", "--stat"], RiskLevel.LOW), + (["tree", "-L", "2", "."], RiskLevel.LOW), + (["ps", "aux"], RiskLevel.LOW), + (["ps", "-ef"], RiskLevel.LOW), + (["ls", "-la", "."], RiskLevel.LOW), + ], + ids=lambda case: str(case), +) +def test_low_invariant_adversarial_corpus(argv: list[str], expected: RiskLevel) -> None: + result = classify_command(argv, workspace=WS) + assert result.risk == expected, f"{argv}: {result.reasons}" + + # -- sensitive_path_reason (component-exact, not substring) -------------------- SENSITIVE_PATHS: list[str] = [ From a97ebe0813be299f41d192e5fc6761e143119eb4 Mon Sep 17 00:00:00 2001 From: Lavindeep Dhillon Date: Fri, 10 Jul 2026 06:45:06 -0400 Subject: [PATCH 2/4] Close remaining LOW bypasses found in review. Escalate tree -R and clustered -o writes, path-check date file reads, and treat ps format env/environ columns as environment exposure. --- shellpilot/policy/command_policy.py | 121 ++++++++++++++++++++++++---- tests/test_command_policy.py | 15 ++++ 2 files changed, 122 insertions(+), 14 deletions(-) diff --git a/shellpilot/policy/command_policy.py b/shellpilot/policy/command_policy.py index e66c769..533e810 100644 --- a/shellpilot/policy/command_policy.py +++ b/shellpilot/policy/command_policy.py @@ -12,10 +12,12 @@ - perform network I/O - read content outside the workspace -Capability-bearing LOW tools (searchers, ``tree``, ``ps``, ``ls``, readers, and -read-only git verbs) must prove that invariant via explicit checks. Unknown -long options on those tools escalate to MEDIUM — LOW is earned, not assumed -from the executable basename alone. +Capability-bearing LOW tools (searchers, ``tree``, ``ps``, ``ls``, and other +path-bearing allowlisted tools) must prove that invariant via explicit checks. +Unknown long options on searchers / ``tree`` / ``ps`` / ``ls`` escalate to +MEDIUM — LOW is earned, not assumed from the executable basename alone. +Readers and read-only git verbs prove the path/helper parts of the invariant +without a full long-option allowlist. Accepted residual: classification still keys off the basename (PATH substitution of a LOW name remains LOW by design of the argv executor); path-qualified @@ -59,15 +61,14 @@ ) # Inert tools take no filesystem/process payload that can violate the LOW # invariant under shell=False argv execution. +# Truly argv-inert under shell=False: no filesystem payload options we honor. INERT_LOW_EXECUTABLES: Final = frozenset( - {"pwd", "true", "false", "uname", "date", "whoami", "which", "echo", "df"} + {"pwd", "true", "false", "uname", "whoami", "which", "echo", "df"} ) READER_EXECUTABLES: Final = frozenset( {"cat", "head", "tail", "grep", "egrep", "fgrep", "rg", "wc", "file", "stat", "du"} ) SEARCHER_EXECUTABLES: Final = frozenset({"grep", "egrep", "fgrep", "rg"}) -# Path-bearing LOW tools that are not already covered by READER_EXECUTABLES. -PATH_CHECKED_LOW_EXECUTABLES: Final = frozenset({"ls", "tree"}) GIT_READONLY_VERBS: Final = frozenset( { "status", @@ -245,6 +246,16 @@ "--stats", "--crlf", "--no-crlf", + # Common agent-facing ripgrep options (short forms already stay LOW). + "--smart-case", + "--case-sensitive", + "--multiline", + "--multiline-dotall", + "--pcre2", + "--encoding", + "--threads", + "--pretty", + "--no-config", } ) SEARCHER_EXECUTION_OPTIONS: Final = frozenset({"--pre", "--pre-glob"}) @@ -412,8 +423,21 @@ def _option_present(argv: list[str], names: frozenset[str]) -> str | None: return None +def _short_option_letters(names: frozenset[str]) -> frozenset[str]: + return frozenset( + name[1:] + for name in names + if name.startswith("-") and not name.startswith("--") and len(name) == 2 + ) + + def _split_option_value(argv: list[str], names: frozenset[str]) -> str | None: - """Value of a space-separated or ``=``-attached option, else None.""" + """Value of a space-separated, ``=``-attached, glued, or clustered option. + + Clustered short options are supported when the value-taking letter is last + in the cluster (``-ao out.txt``) or followed by a glued value (``-aofoo``). + """ + short_letters = _short_option_letters(names) index = 1 while index < len(argv): token = argv[index] @@ -422,17 +446,34 @@ def _split_option_value(argv: list[str], names: frozenset[str]) -> str | None: name = _long_option_name(token) if name is not None and name in names and "=" in token: return token.split("=", 1)[1] - # Glued short form: -oFILE - if not token.startswith("--"): + if token.startswith("-") and not token.startswith("--"): + body = token[1:] + # Exact/glued short form: -o / -oFILE for name in names: if name.startswith("-") and not name.startswith("--") and token.startswith(name): glued = token[len(name) :] if glued: return glued + # Clustered short options: -ao FILE or -aofoo + for offset, letter in enumerate(body): + if letter not in short_letters: + continue + rest = body[offset + 1 :] + if rest: + return rest + return argv[index + 1] if index + 1 < len(argv) else None index += 1 return None +def _short_flag_letter_present(argv: list[str], letter: str) -> bool: + """True when ``letter`` appears in any short-option cluster (e.g. ``-aR``).""" + for token in argv[1:]: + if token.startswith("-") and not token.startswith("--") and letter in token[1:]: + return True + return False + + def _scan_git_verb(argv: list[str]) -> tuple[str, list[str], bool]: conservative_global = False index = 1 @@ -594,8 +635,15 @@ def _classify_searcher(argv: list[str], workspace: Path) -> CommandRisk | None: def _classify_tree(argv: list[str], workspace: Path) -> CommandRisk | None: + # Debian/GNU tree -R writes 00Tree.html at each level (ala -o). + if _short_flag_letter_present(argv, "R"): + return CommandRisk(RiskLevel.MEDIUM, ("tree -R writes HTML files into the tree",)) output = _split_option_value(argv, TREE_OUTPUT_OPTIONS) - if output is not None or _option_present(argv, TREE_OUTPUT_OPTIONS): + if ( + output is not None + or _option_present(argv, TREE_OUTPUT_OPTIONS) + or _short_flag_letter_present(argv, "o") + ): target = output or "" if target and _path_arg_outside_workspace(["tree", target], workspace): return CommandRisk( @@ -612,12 +660,57 @@ def _classify_tree(argv: list[str], workspace: Path) -> CommandRisk | None: return None +def _format_field_list(value: str) -> list[str]: + return [part.strip().lower() for part in value.replace(" ", ",").split(",") if part.strip()] + + +def _ps_format_exposes_environment(argv: list[str]) -> bool: + """True when ``-o``/``-O``/``--format`` selects env/environ columns.""" + format_names = frozenset({"-o", "-O", "--format", "--Format"}) + index = 1 + while index < len(argv): + token = argv[index] + value: str | None = None + if token in format_names: + value = argv[index + 1] if index + 1 < len(argv) else None + index += 2 + elif token.startswith("--format=") or token.startswith("--Format="): + value = token.split("=", 1)[1] + index += 1 + elif token.startswith("-o") and len(token) > 2: + value = token[2:] + index += 1 + elif token.startswith("-O") and len(token) > 2: + value = token[2:] + index += 1 + else: + # Clustered short options with trailing o/O: -ao environ / -aopid,environ + if token.startswith("-") and not token.startswith("--"): + body = token[1:] + for offset, letter in enumerate(body): + if letter not in {"o", "O"}: + continue + rest = body[offset + 1 :] + value = rest if rest else (argv[index + 1] if index + 1 < len(argv) else None) + break + index += 1 + if value is None: + continue + fields = _format_field_list(value) + if any(field in {"env", "environ", "environment"} for field in fields): + return True + return False + + def _ps_exposes_environment(argv: list[str]) -> bool: """True when argv likely requests process environment display. BSD ``ps e`` / ``ps auxe`` expose environments. macOS ``-E`` does too. SysV ``-e`` means "every process" and is left alone (listing only). + Format selectors (``-o environ``, ``--format=pid,env``) are also covered. """ + if _ps_format_exposes_environment(argv): + return True for token in argv[1:]: if token in {"e", "E"}: return True @@ -763,9 +856,9 @@ def classify_command(argv: list[str], *, workspace: Path) -> CommandRisk: ) return CommandRisk(RiskLevel.LOW, ()) if executable in INERT_LOW_EXECUTABLES or executable in LOW_EXECUTABLES: - # Remaining LOW allowlist entries (including inert tools). Path-bearing - # leftovers still get a boundary check so the invariant holds. - if executable in PATH_CHECKED_LOW_EXECUTABLES or executable not in INERT_LOW_EXECUTABLES: + # Remaining LOW allowlist entries. Non-inert leftovers (e.g. date) still + # get a boundary check so out-of-workspace reads cannot auto-run. + if executable not in INERT_LOW_EXECUTABLES: outside = _path_arg_outside_workspace(argv, workspace) if outside: return CommandRisk( diff --git a/tests/test_command_policy.py b/tests/test_command_policy.py index db8ccb5..41bac38 100644 --- a/tests/test_command_policy.py +++ b/tests/test_command_policy.py @@ -469,6 +469,17 @@ def test_empty_argv_is_blocked() -> None: (["ps", "e"], RiskLevel.MEDIUM), (["ps", "auxe"], RiskLevel.MEDIUM), (["ps", "-E"], RiskLevel.MEDIUM), + (["ps", "-o", "pid,environ"], RiskLevel.MEDIUM), + (["ps", "-o", "pid,env"], RiskLevel.MEDIUM), + (["ps", "--format", "pid,environ"], RiskLevel.MEDIUM), + (["ps", "--format=pid,environ"], RiskLevel.MEDIUM), + # tree write forms beyond plain -o + (["tree", "-R", "-L", "2", "."], RiskLevel.MEDIUM), + (["tree", "-ao", "out.txt", "."], RiskLevel.MEDIUM), + (["tree", "-aofoo.txt", "."], RiskLevel.MEDIUM), + # date is not inert: file-backed reads must not auto-run + (["date", "-r", "/etc/passwd"], RiskLevel.HIGH), + (["date", "--file=/etc/shadow"], RiskLevel.HIGH), # Unknown long options fail the LOW proof on capability tools. (["rg", "--totally-unknown-flag", "x"], RiskLevel.MEDIUM), (["ls", "--totally-unknown-flag"], RiskLevel.MEDIUM), @@ -476,11 +487,15 @@ def test_empty_argv_is_blocked() -> None: # Benign forms must keep auto-running. (["rg", "-n", "TODO", "."], RiskLevel.LOW), (["rg", "--hidden", "-n", "TODO", "."], RiskLevel.LOW), + (["rg", "--smart-case", "TODO", "."], RiskLevel.LOW), + (["rg", "--multiline", "TODO", "."], RiskLevel.LOW), (["git", "diff", "--stat"], RiskLevel.LOW), (["tree", "-L", "2", "."], RiskLevel.LOW), (["ps", "aux"], RiskLevel.LOW), (["ps", "-ef"], RiskLevel.LOW), + (["ps", "-o", "pid,comm"], RiskLevel.LOW), (["ls", "-la", "."], RiskLevel.LOW), + (["date"], RiskLevel.LOW), ], ids=lambda case: str(case), ) From 36a8bd4c685dab16d350e07b9875f465f93d6f43 Mon Sep 17 00:00:00 2001 From: Lavindeep Dhillon Date: Fri, 10 Jul 2026 06:46:48 -0400 Subject: [PATCH 3/4] Escalate glued date file options under the LOW invariant. Parse -r/-f path operands in split, glued, and clustered forms so out-of-workspace date reads cannot auto-run. --- shellpilot/policy/command_policy.py | 66 +++++++++++++++++++++++++++++ tests/test_command_policy.py | 3 ++ 2 files changed, 69 insertions(+) diff --git a/shellpilot/policy/command_policy.py b/shellpilot/policy/command_policy.py index 533e810..31ee3d0 100644 --- a/shellpilot/policy/command_policy.py +++ b/shellpilot/policy/command_policy.py @@ -765,6 +765,67 @@ def _classify_ls(argv: list[str], workspace: Path) -> CommandRisk | None: return None +def _date_file_operands(argv: list[str]) -> list[str]: + """Paths that ``date`` may read via ``-r``/``-f``/``--file``/``--reference``.""" + names = frozenset({"-r", "-f", "--file", "--reference"}) + short_letters = frozenset({"r", "f"}) + found: list[str] = [] + index = 1 + while index < len(argv): + token = argv[index] + if token in names: + if index + 1 < len(argv): + found.append(argv[index + 1]) + index += 2 + continue + name = _long_option_name(token) + if name in {"--file", "--reference"} and "=" in token: + found.append(token.split("=", 1)[1]) + index += 1 + continue + if token.startswith("-") and not token.startswith("--"): + body = token[1:] + # Exact/glued: -rPATH / -fPATH + glued = False + for letter in short_letters: + prefix = f"-{letter}" + if token.startswith(prefix) and len(token) > 2: + found.append(token[2:]) + glued = True + break + if not glued: + # Clustered: -ur PATH (value-taking letter last) + for offset, letter in enumerate(body): + if letter not in short_letters: + continue + rest = body[offset + 1 :] + if rest: + found.append(rest) + elif index + 1 < len(argv): + found.append(argv[index + 1]) + break + index += 1 + return found + + +def _classify_date(argv: list[str], workspace: Path) -> CommandRisk | None: + operands = _date_file_operands(argv) + if operands: + outside = _path_arg_outside_workspace(["date", *operands], workspace) + if outside: + return CommandRisk( + RiskLevel.HIGH, + (f"reads outside the workspace boundary: {outside}",), + ) + # In-workspace file-backed date reads are still a filesystem payload — + # ask rather than auto-run under the LOW invariant. + return CommandRisk(RiskLevel.MEDIUM, ("date reads timestamps from a file",)) + outside = _path_arg_outside_workspace(argv, workspace) + if outside: + return CommandRisk(RiskLevel.HIGH, (f"reads outside the workspace boundary: {outside}",)) + return None + + def classify_command(argv: list[str], *, workspace: Path) -> CommandRisk: """Classify an argv command (shell=False) by deterministic rules.""" if not argv or not argv[0].strip(): @@ -842,6 +903,11 @@ def classify_command(argv: list[str], *, workspace: Path) -> CommandRisk: if special is not None: return special return CommandRisk(RiskLevel.LOW, ()) + if executable == "date": + special = _classify_date(argv, workspace) + if special is not None: + return special + return CommandRisk(RiskLevel.LOW, ()) if executable in READER_EXECUTABLES: # Unlike read_file (which honors allow_sensitive_reads via decide()), # classify_command sees only a RiskLevel and run_command is diff --git a/tests/test_command_policy.py b/tests/test_command_policy.py index 41bac38..cb02249 100644 --- a/tests/test_command_policy.py +++ b/tests/test_command_policy.py @@ -479,7 +479,10 @@ def test_empty_argv_is_blocked() -> None: (["tree", "-aofoo.txt", "."], RiskLevel.MEDIUM), # date is not inert: file-backed reads must not auto-run (["date", "-r", "/etc/passwd"], RiskLevel.HIGH), + (["date", "-r/etc/passwd"], RiskLevel.HIGH), + (["date", "-f/etc/passwd"], RiskLevel.HIGH), (["date", "--file=/etc/shadow"], RiskLevel.HIGH), + (["date", "-r", "README.md"], RiskLevel.MEDIUM), # Unknown long options fail the LOW proof on capability tools. (["rg", "--totally-unknown-flag", "x"], RiskLevel.MEDIUM), (["ls", "--totally-unknown-flag"], RiskLevel.MEDIUM), From 71273f3fee24f4121bf9bac134af4056fcbd34aa Mon Sep 17 00:00:00 2001 From: Lavindeep Dhillon Date: Fri, 10 Jul 2026 08:21:46 -0400 Subject: [PATCH 4/4] docs: document the per-tool LOW-invariant verification in DESIGN 36.1 Extend DESIGN.md 36.1 to cover the LOW-invariant hardening: searcher preprocessor and unrecognized-option escalation with the out-of-workspace operand check, read-only git verb external-helper and no-index boundary escalation, tree output-writing forms, ps environment-display forms, the ls boundary check, and date file-backed reads across split, =, glued, and clustered forms. Correct the now-false "Accepted residual" paragraph: the date/tree/ps glued and clustered forms are closed, leaving only the searcher glued pattern-file form (grep -f/etc/passwd) as the documented LOW residual. Add the matching residual note to the command_policy module docstring so the code and spec agree. --- docs/DESIGN.md | 13 ++++++++++++- shellpilot/policy/command_policy.py | 5 ++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 3be4310..91e128a 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -3134,10 +3134,21 @@ The dedicated `read_file`/`search_text` tools enforce the workspace boundary and - **Reader executables now honour the boundary.** A reader executable (`cat`/`head`/`tail`/`grep`/`rg`/`wc`/`file`/`stat`/`du`) whose path argument resolves **outside the workspace**, or names a secret marker, is escalated to `RiskLevel.HIGH` (always-ask) instead of returning LOW/auto-run. Because reader commands carry `SideEffect.VARIABLE` they can never auto-run silently, so the deterministic over-ask (HIGH → ASK) is the conservative match for the file tools' boundary, rather than threading the `allow_sensitive_reads` setting through the command path. - **`git` global options feed classification.** A `git` invocation carrying a non-benign global option before the verb (`--git-dir`/`--work-tree`/`--exec-path`/`-C`/`-c` and the like) is treated as at least MEDIUM (ASK), closing an out-of-workspace read primitive that the previous first-non-dash-token verb derivation skipped past. Only `--no-pager`/`--literal-pathspecs` remain benign. - **Mutating verbs and `--output` under the read-only allowlist no longer auto-run.** The `GIT_READONLY_VERBS` LOW return is read-*only*, but two forms slipped through it under `balanced`. First, `git branch ` / `git remote add|set-url …` (a non-flag positional names a branch/remote to create, rename, add, or repoint — a state mutation) now classify at least MEDIUM (ASK), while bare `git branch`/`git remote`/`git remote -v` and listing forms (`git branch --list `, `-a`/`-r`) stay LOW. Second, `--output=` is a **global diff-formatting option honoured by every diff-emitting verb** (`diff`/`show`/**`log`**/**`stash show`**/…), so `git log --output=/etc/x` was an arbitrary out-of-workspace file-write/truncate primitive at LOW→AUTO just like `git diff --output=`. A single hoisted check now scans the whole invocation for an `--output`/`-O` path value (the `=`, space-separated, and glued forms) and routes it through the same workspace-boundary check as the write/reader paths: outside → HIGH (always-ask), inside → MEDIUM. The check is placed **after** every HIGH determination (`reset`/`clean`, `branch -d/-D`, force `push`) and **before** any read-only LOW return, so it only escalates a would-be-LOW command and never downgrades an already-HIGH one (`git branch -D x --output=in_ws.txt` stays HIGH). `-O` is git's order-file option, not an `--output` short form; it is covered conservatively (an out-of-workspace order-file read is still a boundary crossing worth escalating). The git path previously never consulted the boundary check at all. -- **Option-encoded paths are checked, not skipped.** `_path_arg_outside_workspace` skipped every `-`-prefixed token, so a path hidden in an option value (`grep --file=/etc/passwd .`, `patch --output=/etc/x`) bypassed the boundary entirely and fell through to the LOW reader/write allowlist. For a `-`-token containing `=`, the substring after the first `=` is now run through the same resolve-and-compare check when it looks path-like (contains `/` or starts with `..`); a non-path option value (`--color=auto`, `--include=*.py`) is still ignored, so an in-workspace option path keeps its LOW/MEDIUM classification and only out-of-workspace targets escalate. **Accepted residual:** only `--opt=PATH` and space-separated (`--opt PATH`) forms are boundary-checked; a path glued to a short flag (`grep -f/etc/passwd`, `cp -t/outside`) is still skipped, so the glued short form remains a known gap — a glued out-of-workspace reader path (`grep -f/etc/passwd .`) still classifies LOW and can auto-run under `balanced`. The high-value `--opt=PATH` primitive (the common, model-natural form) is closed; the glued short form is the lower-value remainder and is left as a documented residual rather than parsing every command's short-flag grammar. +- **Option-encoded paths are checked, not skipped.** `_path_arg_outside_workspace` skipped every `-`-prefixed token, so a path hidden in an option value (`grep --file=/etc/passwd .`, `patch --output=/etc/x`) bypassed the boundary entirely and fell through to the LOW reader/write allowlist. For a `-`-token containing `=`, the substring after the first `=` is now run through the same resolve-and-compare check when it looks path-like (contains `/` or starts with `..`); a non-path option value (`--color=auto`, `--include=*.py`) is still ignored, so an in-workspace option path keeps its LOW/MEDIUM classification and only out-of-workspace targets escalate. This generic check does not decode a path glued to a short flag; the per-tool LOW-invariant verification below closes that form for the auto-running tools (`date`/`tree`/`ps`) and narrows the remaining gap to the single searcher pattern-file form documented in the residual note at the end of this section. - **`pytest` requires approval.** `pytest` and `python -m pytest` execute arbitrary project Python (conftest/collected modules) at collection time, so the previous LOW carve-out is removed: they now classify MEDIUM (ASK in `balanced`), symmetric with `python script.py`. - **Path-qualified basenames are distrusted.** When `argv[0]` contains a path separator (`./grep`, `/abs/grep`), the bare-name LOW allowlist no longer applies; a path-qualified executable is classified at least MEDIUM, so a workspace-staged file sharing a trusted command's name cannot auto-run. +A follow-up review found the read-path work above still let a whole allowlist auto-run on the executable **basename** alone: any name in `LOW_EXECUTABLES` returned LOW regardless of what its options did. That is tightened to a per-tool **LOW invariant** — a command may classify LOW only when its argv form provably cannot execute code or a configured helper, write or truncate a file, perform network I/O, or read content outside the workspace. Capability-bearing LOW tools now prove that invariant explicitly from their own argv; the argv-inert names (`INERT_LOW_EXECUTABLES` = `pwd`/`true`/`false`/`uname`/`whoami`/`which`/`echo`/`df`) carry no filesystem or process payload and keep LOW unconditionally. + +- **Searchers (`grep`/`egrep`/`fgrep`/`rg`).** `--pre`/`--pre-glob` run a preprocessor over matched files → HIGH. An unrecognized long option (anything outside the `SEARCHER_SAFE_LONG_OPTIONS` allowlist) fails the proof → MEDIUM; LOW is earned from a known-safe option set, not assumed from the `grep` basename. A path operand resolving outside the workspace → HIGH. +- **Read-only git verbs.** `--ext-diff`/`--textconv` can execute configured external diff/textconv programs → MEDIUM. The `GIT_READONLY_VERBS` LOW return additionally proves the boundary: an out-of-workspace path operand (`git diff --no-index /etc/a /etc/b`) → HIGH. +- **`tree` output-writing forms.** `-o`/`--output` (and `-R`, which writes `00Tree.html` at each visited level) write a file → MEDIUM, or HIGH when the resolved output path is outside the workspace. An unrecognized long option → MEDIUM; an out-of-workspace read operand → HIGH. +- **`ps` environment-display forms.** Selectors that expose process environments — BSD `ps e`/`ps auxe`, macOS `-E`, and `-o environ`/`--format=…env…` in space-separated, `=`, glued, and clustered forms — → MEDIUM. An unrecognized long option → MEDIUM. +- **`ls` boundary.** An unrecognized long option → MEDIUM; a path operand outside the workspace → HIGH. +- **`date` file-backed reads.** `-r`/`-f`/`--file`/`--reference` make `date` read a file; the value is decoded in split (`-r PATH`), `=` (`--file=PATH`), glued (`-rPATH`), and clustered (`-ur PATH`) forms. An out-of-workspace file → HIGH; an in-workspace file → MEDIUM (a filesystem payload, not a pure clock read). + +**Accepted residual (LOW invariant).** The `date`/`tree`/`ps` glued and clustered forms are now decoded and closed. One searcher form remains: a pattern file glued to its short flag — `grep -f/etc/passwd .` — is read as a *pattern list* (its contents are never printed) and the boundary check does not decode the glued short flag, so it still classifies LOW and can auto-run under `balanced`. The long `--file=/etc/passwd` and space-separated `grep -f /etc/passwd` forms already escalate to HIGH; the glued pattern-file remainder is left as a documented residual rather than parsing every tool's short-flag grammar. + ### 36.2 Terminal Output Sanitization The active-cloud indicator (section 15.2) is only trustworthy if the model — or untrusted data it surfaces — cannot repaint the terminal over it. Control characters and ANSI escape sequences are now stripped at **every** output sink via the shared `_sanitize_line`/`_CONTROL_CHARS` helper (`cli/render.py`), not only in the diff panel: streamed model text (`cli/streaming.py`), raw command output (`show_command_output`), tool-call/tool-result summaries (`cli/render.py`), and status/error lines (`cli/terminal.py`). Tab and newline are preserved; ESC and C0/DEL bytes are removed before anything reaches the screen, so neither model output nor auto-run command output can forge a status region or spoof an approval prompt. diff --git a/shellpilot/policy/command_policy.py b/shellpilot/policy/command_policy.py index 31ee3d0..cb2f6f6 100644 --- a/shellpilot/policy/command_policy.py +++ b/shellpilot/policy/command_policy.py @@ -21,7 +21,10 @@ Accepted residual: classification still keys off the basename (PATH substitution of a LOW name remains LOW by design of the argv executor); path-qualified -executables already escalate out of LOW. +executables already escalate out of LOW. A searcher pattern file glued to its +short flag (``grep -f/etc/passwd``) reads a pattern file the boundary check does +not decode and still classifies LOW; the ``--file=PATH`` and space-separated +forms are already escalated. """ from __future__ import annotations