From 976d8e27ad4f2ba59ba5fc14a0c1e811267712d5 Mon Sep 17 00:00:00 2001 From: fr33d3m0n Date: Thu, 7 May 2026 17:36:39 +0800 Subject: [PATCH] fix(approval): catch sudo with stdin/askpass/shell privilege flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the only #17873 category not covered by the in-flight PRs #17962 (briandevans, reverse shell + download-execute) and #7993 (SHL0MS, credential reads + curl/wget exfiltration): sudo invocations that an LLM-driven agent can drive without TTY interaction. The agent has no TTY, so the sudo forms that succeed without human involvement 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. Two patterns: 1. Direct flag: `\bsudo\b[^;|&\n]*?\s+(?:-s\b|--stdin\b|-a\b|--askpass\b)` The lazy `[^;|&\n]*?` consumes flag-arguments without spanning command separators, so `sudo -u root -S whoami` matches (a textbook offensive form that a strict `(?:\s+-[^\s]+)*` "leading flags only" pattern would have missed because `root` is a flag-value not a flag). 2. Combined short flags: `\bsudo\b[^;|&\n]*?\s+-[a-z]*[sa][a-z]*\b` Catches packed forms like `sudo -nS id` where multiple flags share a single `-X` token. `_normalize_command_for_detection` lowercases input before pattern matching (tools/approval.py:340), so case variants of S/s and A/a collapse — both letter-pairs are gated since each is a privilege- relevant invocation. Tests: 21 new cases in TestDetectSudoStdin (12 positive covering all flag-order permutations including herestring source and printf-piped forms; 9 negative including TTY-bound `sudo whoami`, interactive `sudo -i`, env-var reference `$SUDO_USER`, doc lookup `man sudo`, package install, and the `pseudosudo` word-boundary edge case). Empirical coverage: 11/11 attacks matched, 0/10 false positives. Refs: #17873 category 4. Adjacent: #17962 (reverse shell + download- execute), #7993 (credential reads + curl/wget exfiltration). Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/tools/test_approval.py | 137 +++++++++++++++++++++++++++++++++++ tools/approval.py | 19 +++++ 2 files changed, 156 insertions(+) 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"), ]