[flake8-bandit] Mark str and list[str] literals as trusted input (S603)

crates/ruff_linter/resources/test/fixtures/flake8_bandit/S603.py

@@ -1,20 +1,41 @@
 from subprocess import Popen, call, check_call, check_output, run
 
 # Different Popen wrappers are checked.
+a = input()
+Popen(a, shell=False)
+call(a, shell=False)
+check_call(a, shell=False)
+check_output(a, shell=False)
+run(a, shell=False)
+
+# Falsey values are treated as false.
+Popen(a, shell=0)
+Popen(a, shell=[])
+Popen(a, shell={})
+Popen(a, shell=None)
+
+# Unknown values are treated as falsey.
+Popen(a, shell=True if True else False)
+
+# No value is also caught.
+Popen(a)
+
+# Literals are fine, they're trusted.
+run("true")
+Popen(["true"])
 Popen("true", shell=False)
 call("true", shell=False)
 check_call("true", shell=False)
 check_output("true", shell=False)
 run("true", shell=False)
 
-# Values that falsey values are treated as false.
-Popen("true", shell=0)
-Popen("true", shell=[])
-Popen("true", shell={})
-Popen("true", shell=None)
+# Not through assignments though.
+cmd = ["true"]
+run(cmd)
 
-# Unknown values are treated as falsey.
-Popen("true", shell=True if True else False)
+# Instant named expressions are fine.
+run(c := "true")
 
-# No value is also caught.
-Popen("true")
+# But non-instant are not.
+(e := "echo")
+run(e)

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

@@ -104,6 +104,7 @@ mod tests {
     #[test_case(Rule::SuspiciousURLOpenUsage, Path::new("S310.py"))]
     #[test_case(Rule::SuspiciousNonCryptographicRandomUsage, Path::new("S311.py"))]
     #[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
+    #[test_case(Rule::SubprocessWithoutShellEqualsTrue, Path::new("S603.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",

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

@@ -287,6 +287,19 @@ impl Violation for UnixCommandWildcardInjection {
     }
 }
 
+/// Check if an expression is a trusted input for subprocess.run.
+/// We assume that any str or list[str] literal can be trusted.
+fn is_trusted_input(arg: &Expr) -> bool {
+    match arg {
+        Expr::StringLiteral(_) => true,
+        Expr::List(ast::ExprList { elts, .. }) => {
+            elts.iter().all(|elt| matches!(elt, Expr::StringLiteral(_)))
+        }
+        Expr::Named(named) => is_trusted_input(&named.value),
+        _ => false,
+    }
+}
+
 /// S602, S603, S604, S605, S606, S607, S609
 pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
     let call_kind = get_call_kind(&call.func, checker.semantic());
@@ -310,24 +323,14 @@ pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
                     }
                 }
                 // S603
-                Some(ShellKeyword {
-                    truthiness:
-                        Truthiness::False | Truthiness::Falsey | Truthiness::None | Truthiness::Unknown,
-                }) => {
-                    if checker.enabled(Rule::SubprocessWithoutShellEqualsTrue) {
-                        checker.report_diagnostic(Diagnostic::new(
-                            SubprocessWithoutShellEqualsTrue,
-                            call.func.range(),
-                        ));
-                    }
-                }
-                // S603
-                None => {
-                    if checker.enabled(Rule::SubprocessWithoutShellEqualsTrue) {
-                        checker.report_diagnostic(Diagnostic::new(
-                            SubprocessWithoutShellEqualsTrue,
-                            call.func.range(),
-                        ));
+                _ => {
+                    if !is_trusted_input(arg) || checker.settings.preview.is_disabled() {
+                        if checker.enabled(Rule::SubprocessWithoutShellEqualsTrue) {
+                            checker.report_diagnostic(Diagnostic::new(
+                                SubprocessWithoutShellEqualsTrue,
+                                call.func.range(),
+                            ));
+                        }
                     }
                 }
             }

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S603_S603.py.snap

@@ -1,104 +1,202 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
 ---
-S603.py:4:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:5:1: S603 `subprocess` call: check for execution of untrusted input
   |
 3 | # Different Popen wrappers are checked.
-4 | Popen("true", shell=False)
+4 | a = input()
+5 | Popen(a, shell=False)
   | ^^^^^ S603
-5 | call("true", shell=False)
-6 | check_call("true", shell=False)
+6 | call(a, shell=False)
+7 | check_call(a, shell=False)
   |
 
-S603.py:5:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:6:1: S603 `subprocess` call: check for execution of untrusted input
   |
-3 | # Different Popen wrappers are checked.
-4 | Popen("true", shell=False)
-5 | call("true", shell=False)
+4 | a = input()
+5 | Popen(a, shell=False)
+6 | call(a, shell=False)
   | ^^^^ S603
-6 | check_call("true", shell=False)
-7 | check_output("true", shell=False)
+7 | check_call(a, shell=False)
+8 | check_output(a, shell=False)
   |
 
-S603.py:6:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:7:1: S603 `subprocess` call: check for execution of untrusted input
   |
-4 | Popen("true", shell=False)
-5 | call("true", shell=False)
-6 | check_call("true", shell=False)
+5 | Popen(a, shell=False)
+6 | call(a, shell=False)
+7 | check_call(a, shell=False)
   | ^^^^^^^^^^ S603
-7 | check_output("true", shell=False)
-8 | run("true", shell=False)
+8 | check_output(a, shell=False)
+9 | run(a, shell=False)
   |
 
-S603.py:7:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:8:1: S603 `subprocess` call: check for execution of untrusted input
   |
-5 | call("true", shell=False)
-6 | check_call("true", shell=False)
-7 | check_output("true", shell=False)
+6 | call(a, shell=False)
+7 | check_call(a, shell=False)
+8 | check_output(a, shell=False)
   | ^^^^^^^^^^^^ S603
-8 | run("true", shell=False)
+9 | run(a, shell=False)
   |
 
-S603.py:8:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:9:1: S603 `subprocess` call: check for execution of untrusted input
    |
- 6 | check_call("true", shell=False)
- 7 | check_output("true", shell=False)
- 8 | run("true", shell=False)
+ 7 | check_call(a, shell=False)
+ 8 | check_output(a, shell=False)
+ 9 | run(a, shell=False)
    | ^^^ S603
- 9 |
-10 | # Values that falsey values are treated as false.
+10 |
+11 | # Falsey values are treated as false.
    |
 
-S603.py:11:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:12:1: S603 `subprocess` call: check for execution of untrusted input
    |
-10 | # Values that falsey values are treated as false.
-11 | Popen("true", shell=0)
+11 | # Falsey values are treated as false.
+12 | Popen(a, shell=0)
    | ^^^^^ S603
-12 | Popen("true", shell=[])
-13 | Popen("true", shell={})
+13 | Popen(a, shell=[])
+14 | Popen(a, shell={})
    |
 
-S603.py:12:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:13:1: S603 `subprocess` call: check for execution of untrusted input
    |
-10 | # Values that falsey values are treated as false.
-11 | Popen("true", shell=0)
-12 | Popen("true", shell=[])
+11 | # Falsey values are treated as false.
+12 | Popen(a, shell=0)
+13 | Popen(a, shell=[])
    | ^^^^^ S603
-13 | Popen("true", shell={})
-14 | Popen("true", shell=None)
+14 | Popen(a, shell={})
+15 | Popen(a, shell=None)
    |
 
-S603.py:13:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:14:1: S603 `subprocess` call: check for execution of untrusted input
    |
-11 | Popen("true", shell=0)
-12 | Popen("true", shell=[])
-13 | Popen("true", shell={})
+12 | Popen(a, shell=0)
+13 | Popen(a, shell=[])
+14 | Popen(a, shell={})
    | ^^^^^ S603
-14 | Popen("true", shell=None)
+15 | Popen(a, shell=None)
    |
 
-S603.py:14:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:15:1: S603 `subprocess` call: check for execution of untrusted input
    |
-12 | Popen("true", shell=[])
-13 | Popen("true", shell={})
-14 | Popen("true", shell=None)
+13 | Popen(a, shell=[])
+14 | Popen(a, shell={})
+15 | Popen(a, shell=None)
    | ^^^^^ S603
-15 |
-16 | # Unknown values are treated as falsey.
+16 |
+17 | # Unknown values are treated as falsey.
+   |
+
+S603.py:18:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+17 | # Unknown values are treated as falsey.
+18 | Popen(a, shell=True if True else False)
+   | ^^^^^ S603
+19 |
+20 | # No value is also caught.
+   |
+
+S603.py:21:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+20 | # No value is also caught.
+21 | Popen(a)
+   | ^^^^^ S603
+22 |
+23 | # Literals are fine, they're trusted.
+   |
+
+S603.py:24:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+23 | # Literals are fine, they're trusted.
+24 | run("true")
+   | ^^^ S603
+25 | Popen(["true"])
+26 | Popen("true", shell=False)
    |
 
-S603.py:17:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:25:1: S603 `subprocess` call: check for execution of untrusted input
    |
-16 | # Unknown values are treated as falsey.
-17 | Popen("true", shell=True if True else False)
+23 | # Literals are fine, they're trusted.
+24 | run("true")
+25 | Popen(["true"])
    | ^^^^^ S603
-18 |
-19 | # No value is also caught.
+26 | Popen("true", shell=False)
+27 | call("true", shell=False)
    |
 
-S603.py:20:1: S603 `subprocess` call: check for execution of untrusted input
+S603.py:26:1: S603 `subprocess` call: check for execution of untrusted input
    |
-19 | # No value is also caught.
-20 | Popen("true")
+24 | run("true")
+25 | Popen(["true"])
+26 | Popen("true", shell=False)
    | ^^^^^ S603
+27 | call("true", shell=False)
+28 | check_call("true", shell=False)
+   |
+
+S603.py:27:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+25 | Popen(["true"])
+26 | Popen("true", shell=False)
+27 | call("true", shell=False)
+   | ^^^^ S603
+28 | check_call("true", shell=False)
+29 | check_output("true", shell=False)
+   |
+
+S603.py:28:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+26 | Popen("true", shell=False)
+27 | call("true", shell=False)
+28 | check_call("true", shell=False)
+   | ^^^^^^^^^^ S603
+29 | check_output("true", shell=False)
+30 | run("true", shell=False)
+   |
+
+S603.py:29:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+27 | call("true", shell=False)
+28 | check_call("true", shell=False)
+29 | check_output("true", shell=False)
+   | ^^^^^^^^^^^^ S603
+30 | run("true", shell=False)
+   |
+
+S603.py:30:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+28 | check_call("true", shell=False)
+29 | check_output("true", shell=False)
+30 | run("true", shell=False)
+   | ^^^ S603
+31 |
+32 | # Not through assignments though.
+   |
+
+S603.py:34:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+32 | # Not through assignments though.
+33 | cmd = ["true"]
+34 | run(cmd)
+   | ^^^ S603
+35 |
+36 | # Instant named expressions are fine.
+   |
+
+S603.py:37:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+36 | # Instant named expressions are fine.
+37 | run(c := "true")
+   | ^^^ S603
+38 |
+39 | # But non-instant are not.
+   |
+
+S603.py:41:1: S603 `subprocess` call: check for execution of untrusted input
+   |
+39 | # But non-instant are not.
+40 | (e := "echo")
+41 | run(e)
+   | ^^^ S603
    |