diff --git a/src/plyngent/tools/command_scan.py b/src/plyngent/tools/command_scan.py index 58d9c70..3ca6565 100644 --- a/src/plyngent/tools/command_scan.py +++ b/src/plyngent/tools/command_scan.py @@ -107,17 +107,20 @@ _DURATION = re.compile(r"[0-9]+(?:\.[0-9]+)?[smhd]?") # Options that consume the next token, so ``sudo -u root python`` still finds # ``python``; jammed forms (``-n10``, ``--with=pkg``) need no entry, and unknown -# options are skipped bare. +# options are skipped bare. List value-taking options only: an entry for a flag +# eats the program behind it, which hid ``rm`` from ``exec -c rm -rf /`` and +# judged ``exec -c git -c … init`` as ``init``. An option with an *optional* +# value (``xargs -e``, ``watch -d``) needs no entry either — its bare form runs +# the program, and the jammed form carries the value in the same token. _VALUE_OPTS: dict[str, frozenset[str]] = { - "command": frozenset({"-p", "-v", "-V"}), "doas": frozenset({"-u", "--user"}), "env": frozenset({"-C", "-S", "-u", "--chdir", "--split-string", "--unset"}), - "exec": frozenset({"-a", "-c", "-l"}), + "exec": frozenset({"-a"}), "ionice": frozenset({"-c", "-n", "-p", "-P", "-u"}), "nice": frozenset({"-n", "--adjustment"}), - "pkexec": frozenset({"--disable-internal-agent", "--user"}), + "pkexec": frozenset({"--user"}), "stdbuf": frozenset({"-e", "-i", "-o"}), - "su": frozenset({"-c", "-g", "-G", "-p", "-s", "-u", "-w", "--command", "--group", "--session-command", "--shell"}), + "su": frozenset({"-c", "-g", "-G", "-s", "-u", "-w", "--command", "--group", "--session-command", "--shell"}), "sudo": frozenset( { "-C", @@ -145,7 +148,7 @@ _VALUE_OPTS: dict[str, frozenset[str]] = { "time": frozenset({"-f", "-o", "--format", "--output"}), "timeout": frozenset({"-k", "-s", "--kill-after", "--signal"}), "watch": frozenset({"-n"}), - "xargs": frozenset({"-a", "-d", "-E", "-e", "-I", "-L", "-n", "-P", "-s"}), + "xargs": frozenset({"-a", "-d", "-E", "-I", "-L", "-n", "-P", "-s"}), } # Launcher options that consume a value (``uv run --with rich python``). diff --git a/tests/test_tools/test_command_scan.py b/tests/test_tools/test_command_scan.py index 8c65843..11c5aad 100644 --- a/tests/test_tools/test_command_scan.py +++ b/tests/test_tools/test_command_scan.py @@ -65,6 +65,33 @@ def test_unwrap_stops_when_no_program_follows() -> None: assert scan.base == argv[0], argv +def test_unwrap_value_free_flags_do_not_swallow_the_program() -> None: + """Only options that consume the next token may be listed as value-taking. + + ``exec -c`` (empty environment) and ``exec -l`` (login), ``command + -p|-v|-V``, ``pkexec --disable-internal-agent`` and ``su -p`` are flags: a + table entry for them ate the program behind them, so ``exec -c rm -rf /`` + hid ``rm`` from the denylist while ``exec -c git -c … init`` resolved to + ``init``. + """ + cases = [ + (["exec", "-c", "rm", "-rf", "/"], "rm"), + (["exec", "-c", "git", "-c", "init.defaultBranch=main", "init"], "git"), + (["exec", "-l", "python", "script.py"], "python"), + (["command", "-v", "rm"], "rm"), + (["command", "-p", "rm", "-rf", "/"], "rm"), + (["pkexec", "--disable-internal-agent", "rm", "-rf", "/"], "rm"), + (["su", "-p", "python", "script.py"], "python"), + # Options that do take a value still consume it. + (["exec", "-a", "name", "rm", "-rf", "/"], "rm"), + (["xargs", "-e", "rm", "-rf", "/"], "rm"), + ] + for argv, base in cases: + scan = unwrap_command(argv) + assert scan.base == base, argv + assert scan.command[0] == base, argv + + def test_unwrap_empty_argv() -> None: scan = unwrap_command([]) assert scan.base == "" diff --git a/tests/test_tools/test_danger.py b/tests/test_tools/test_danger.py index aae3cc5..3cedc69 100644 --- a/tests/test_tools/test_danger.py +++ b/tests/test_tools/test_danger.py @@ -136,6 +136,14 @@ def test_wrappers_reach_the_interpreter() -> None: assert "$(command)" in reason +def test_value_free_flags_do_not_hide_the_interpreter() -> None: + """``exec -c`` takes no value, so the interpreter behind it is still reviewed.""" + reason = classify_danger("run_argv", {"argv": ["exec", "-c", "python", "-c", "print(1)"]}) + assert reason is not None + assert "interpreter 'python'" in reason + assert "print(1)" in reason + + def test_only_the_interpreter_dash_c_becomes_a_placeholder() -> None: reason = classify_danger("run_argv", {"argv": ["ionice", "-c", "2", "python", "-c", "print(1)"]}) assert reason is not None diff --git a/tests/test_tools/test_workspace.py b/tests/test_tools/test_workspace.py index 92bac0e..cc3d1f9 100644 --- a/tests/test_tools/test_workspace.py +++ b/tests/test_tools/test_workspace.py @@ -215,6 +215,30 @@ def test_command_denylist_policy_confirm_timeout_value(workspace: object) -> Non clear_policy_allowed_commands() +def test_command_denylist_sees_the_program_behind_value_free_flags(workspace: object) -> None: + """`exec -c` / `command -p` take no value, so the program stays visible.""" + del workspace + from plyngent.tools.workspace import clear_policy_allowed_commands, set_policy_confirm_hook + + set_policy_confirm_hook(None) + clear_policy_allowed_commands() + set_command_denylist(None) + try: + for argv in ( + ["exec", "-c", "rm", "-rf", "/"], + ["command", "-p", "rm", "-rf", "/"], + ["command", "-v", "rm"], + ): + with pytest.raises(WorkspaceError, match="basename 'rm' is blocked"): + check_command_allowed(list(argv)) + # ``init`` here is a git subcommand, not the denylisted program: the + # command resolves to ``git`` and stays allowed. + check_command_allowed(["exec", "-c", "git", "-c", "init.defaultBranch=main", "init"]) + finally: + clear_policy_allowed_commands() + set_command_denylist(None) + + def test_command_denylist_reaches_through_wrappers(workspace: object) -> None: del workspace from plyngent.tools.workspace import clear_policy_allowed_commands, set_policy_confirm_hook