diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 77ca3550d3a..7ec2d5868f1 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -965,3 +965,140 @@ class TestFailClosedUnderPromptToolkit: assert result == "once" finally: ptc.get_app_or_none = orig + + +class TestDetectSudoStdin: + """Sudo with stdin / askpass / shell / list-privileges flags (#17873 cat 4). + + An LLM-driven agent has no TTY, so the sudo invocations that succeed + without human interaction are those reading the password from stdin + (-S / --stdin) or via an askpass helper (-A / --askpass). The + shell-launch (-s) and list-privileges (-a) flags are also gated since + they are privilege-relevant invocations the agent can chain after + acquiring the password. + + `_normalize_command_for_detection` lowercases input before pattern + matching, so -S/-s and -A/-a are indistinguishable at the regex + layer; both letter-pairs are gated. + """ + + # Positive cases (must match) + + def test_canonical_pipe_to_sudo_S_detected(self): + is_dangerous, _, desc = detect_dangerous_command( + "echo pwd | sudo -S whoami" + ) + assert is_dangerous is True + assert "sudo" in desc.lower() + + def test_long_flag_stdin_detected(self): + is_dangerous, _, _ = detect_dangerous_command("sudo --stdin id") + assert is_dangerous is True + + def test_non_interactive_plus_stdin_detected(self): + is_dangerous, _, _ = detect_dangerous_command("sudo -n -S id") + assert is_dangerous is True + + def test_user_then_stdin_detected(self): + # Codex audit caught that the original "leading flags only" regex + # missed this form because `-u root` has a flag-argument (`root`) + # that broke the (?:\s+-[^\s]+)* loop. The lazy [^;|&\n]*? class + # consumes flag-args without spanning command separators. + is_dangerous, _, _ = detect_dangerous_command( + "sudo -u root -S whoami" + ) + assert is_dangerous is True + + def test_long_non_interactive_plus_stdin_detected(self): + is_dangerous, _, _ = detect_dangerous_command( + "sudo --non-interactive -S whoami" + ) + assert is_dangerous is True + + def test_long_user_equals_stdin_detected(self): + is_dangerous, _, _ = detect_dangerous_command( + "sudo --user=root -S id" + ) + assert is_dangerous is True + + def test_herestring_input_detected(self): + is_dangerous, _, _ = detect_dangerous_command( + "sudo -S id <<< 'mypwd'" + ) + assert is_dangerous is True + + def test_combined_short_flags_nS_detected(self): + # `-nS` packs `-n` and `-S` into one arg; second pattern catches. + is_dangerous, _, _ = detect_dangerous_command("sudo -nS id") + assert is_dangerous is True + + def test_printf_form_detected(self): + is_dangerous, _, _ = detect_dangerous_command( + 'printf "%s\\n" "$PW" | sudo -S id' + ) + assert is_dangerous is True + + def test_askpass_short_flag_detected(self): + is_dangerous, _, _ = detect_dangerous_command("sudo -A id") + assert is_dangerous is True + + def test_askpass_long_flag_detected(self): + is_dangerous, _, _ = detect_dangerous_command("sudo --askpass id") + assert is_dangerous is True + + def test_two_sudo_invocations_second_caught(self): + # The first sudo here is benign (no -S); the second has -S. + # Lazy [^;|&\n]*? does NOT span past `;`, so re.search anchors + # on the second sudo invocation independently. + is_dangerous, _, _ = detect_dangerous_command( + "sudo whoami; sudo -S id" + ) + assert is_dangerous is True + + # Negative cases (must NOT match) + + def test_plain_sudo_safe(self): + is_dangerous, _, _ = detect_dangerous_command("sudo whoami") + assert is_dangerous is False + + def test_sudo_interactive_shell_safe(self): + is_dangerous, _, _ = detect_dangerous_command("sudo -i") + assert is_dangerous is False + + def test_sudo_with_user_no_stdin_flag_safe(self): + is_dangerous, _, _ = detect_dangerous_command("sudo -u root -i") + assert is_dangerous is False + + def test_man_sudo_safe(self): + is_dangerous, _, _ = detect_dangerous_command("man sudo") + assert is_dangerous is False + + def test_which_sudo_safe(self): + is_dangerous, _, _ = detect_dangerous_command("which sudo") + assert is_dangerous is False + + def test_sudo_user_env_reference_safe(self): + is_dangerous, _, _ = detect_dangerous_command( + "echo SUDO_USER=$SUDO_USER" + ) + assert is_dangerous is False + + def test_apt_install_sudo_safe(self): + is_dangerous, _, _ = detect_dangerous_command("apt install sudo") + assert is_dangerous is False + + def test_ls_etc_sudoers_safe(self): + is_dangerous, _, _ = detect_dangerous_command("ls /etc/sudoers") + assert is_dangerous is False + + def test_pseudosudo_safe_word_boundary(self): + # `\bsudo\b` requires a word boundary; `pseudosudo` has none + # before `sudo`, so should not trigger. + is_dangerous, _, _ = detect_dangerous_command("pseudosudo -S id") + assert is_dangerous is False + + def test_unrelated_redirection_safe(self): + is_dangerous, _, _ = detect_dangerous_command( + "make 2>&1 | tee build.log" + ) + assert is_dangerous is False diff --git a/tools/approval.py b/tools/approval.py index 9f7c1989a84..bd3fb504b33 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -368,6 +368,25 @@ DANGEROUS_PATTERNS = [ # a script is first made executable then immediately run. The script # content may contain dangerous commands that individual patterns miss. (r'\bchmod\s+\+x\b.*[;&|]+\s*\./', "chmod +x followed by immediate execution"), + # Sudo with stdin / askpass / shell / list-privs flags. An LLM-driven + # agent has no TTY, so sudo invocations that succeed without human + # interaction are those reading the password from stdin (-S/--stdin) + # or via an askpass helper (-A/--askpass). The shell-launch (-s) and + # list-privileges (-a) flags are also gated since they are + # privilege-relevant invocations the agent can chain after acquiring + # the password (e.g. read SUDO_PASSWORD from .env -> sudo -S -s -> + # root shell). Plain `sudo cmd` (no flag) is TTY-bound and excluded. + # `_normalize_command_for_detection` lowercases input before pattern + # matching, so case variants of S/s and A/a collapse — both forms + # are gated below. Lazy `[^;|&\n]*?` allows flag arguments (e.g. + # `sudo -u root -S whoami`) without spanning command separators. See + # #17873 category 4. + (r'\bsudo\b[^;|&\n]*?\s+(?:-s\b|--stdin\b|-a\b|--askpass\b)', + "sudo with privilege flag (stdin/askpass/shell/list)"), + # Combined short-flag form: -nS, -ns, -sa, -las — sudo flags packed + # into a single -X token. Catches the same threat class. + (r'\bsudo\b[^;|&\n]*?\s+-[a-z]*[sa][a-z]*\b', + "sudo with combined-flag privilege escalation"), ]