[flake8-bandit] Also check keyword arguments (S602, S603, S607, S609)

crates/ruff_linter/resources/mdtest/flake8-bandit/rules.md

@@ -0,0 +1,77 @@
+# `flake8-bandit`
+
+Regression tests for <https://github.com/astral-sh/ruff/issues/27631>. Keyword command arguments are
+checked, as well as positional arguments. `*args` is also treated as untrusted.
+
+## `subprocess-popen-with-shell-equals-true` (`S602`)
+
+```toml
+[lint]
+select = ["S602"]
+```
+
+```py
+from subprocess import Popen, call, check_call, check_output, run
+
+Popen(args="true", shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+call(args="true", shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+check_call(args="true", shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+check_output(args="true", shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+run(args="true", shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+
+var_string = "true"
+Popen(args=var_string, shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+
+cmd = input()
+Popen(*cmd, shell=True)  # error: [subprocess-popen-with-shell-equals-true]
+```
+
+## `subprocess-without-shell-equals-true` (`S603`)
+
+```toml
+[lint]
+select = ["S603"]
+```
+
+```py
+from subprocess import Popen, call, check_call, check_output, run
+
+a = input()
+
+Popen(args=a, shell=False)  # error: [subprocess-without-shell-equals-true]
+call(args=a, shell=False)  # error: [subprocess-without-shell-equals-true]
+check_call(args=a, shell=False)  # error: [subprocess-without-shell-equals-true]
+check_output(args=a, shell=False)  # error: [subprocess-without-shell-equals-true]
+run(args=a, shell=False)  # error: [subprocess-without-shell-equals-true]
+check_output(args=[a], shell=False)  # error: [subprocess-without-shell-equals-true]
+run(*a)  # error: [subprocess-without-shell-equals-true]
+run(args=["true"])
+```
+
+## `start-process-with-partial-path` (`S607`)
+
+```toml
+[lint]
+select = ["S607"]
+```
+
+```py
+import os
+import subprocess
+
+os.spawnv(mode=os.P_WAIT, file="/bin/ls", args=["ls"])
+subprocess.run(args="git status")  # error: [start-process-with-partial-path]
+```
+
+## `unix-command-wildcard-injection` (`S609`)
+
+```toml
+[lint]
+select = ["S609"]
+```
+
+```py
+import subprocess
+
+subprocess.Popen(args="chmod -R 777 *", shell=True)  # error: [unix-command-wildcard-injection]
+```

crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs

@@ -334,9 +334,14 @@ fn is_trusted_input(arg: &Expr, semantic: &SemanticModel) -> bool {
 pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
     let call_kind = get_call_kind(&call.func, checker.semantic());
     let shell_keyword = find_shell_keyword(&call.arguments, checker.semantic());
+    let command_argument = match call_kind {
+        Some(CallKind::Subprocess) => find_subprocess_argument(&call.arguments),
+        Some(CallKind::Shell | CallKind::NoShell) => call.arguments.args.first(),
+        None => None,
+    };
 
     if matches!(call_kind, Some(CallKind::Subprocess)) {
-        if let Some(arg) = call.arguments.args.first() {
+        if let Some(arg) = command_argument {
             match shell_keyword {
                 // S602
                 Some(ShellKeyword {
@@ -397,11 +402,9 @@ pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
 
     // S607
     if checker.is_rule_enabled(Rule::StartProcessWithPartialPath) {
-        if call_kind.is_some() {
-            if let Some(arg) = call.arguments.args.first() {
-                if is_partial_path(arg) {
-                    checker.report_diagnostic(StartProcessWithPartialPath, arg.range());
-                }
+        if let Some(arg) = command_argument {
+            if is_partial_path(arg) {
+                checker.report_diagnostic(StartProcessWithPartialPath, arg.range());
             }
         }
     }
@@ -419,7 +422,7 @@ pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
                 )
             )
         {
-            if let Some(arg) = call.arguments.args.first() {
+            if let Some(arg) = command_argument {
                 if is_wildcard_command(arg) {
                     checker.report_diagnostic(UnixCommandWildcardInjection, arg.range());
                 }
@@ -428,6 +431,18 @@ pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
     }
 }
 
+/// Return the command argument to a `subprocess` call.
+fn find_subprocess_argument(arguments: &Arguments) -> Option<&Expr> {
+    arguments.find_argument_value("args", 0).or_else(|| {
+        // A starred first argument (`subprocess.run(*cmd)`) prevents us from locating the
+        // command with `find_argument_value`; treat it as untrusted input.
+        arguments
+            .args
+            .first()
+            .filter(|argument| argument.is_starred_expr())
+    })
+}
+
 #[derive(Copy, Clone, Debug)]
 enum CallKind {
     Subprocess,