feat(rules): cover more S605 cases

Ref: PyCQA/bandit#1116
mkniewallner · Mar 8, 2024 · bb5d997 · bb5d997
1 parent 1d97f27
commit bb5d997
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 87 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S605.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S605.py
@@ -1,4 +1,5 @@
 import os
+import subprocess
 
 import commands
 import popen2
@@ -16,6 +17,8 @@
 popen2.Popen4("true")
 commands.getoutput("true")
 commands.getstatusoutput("true")
+subprocess.getoutput("true")
+subprocess.getstatusoutput("true")
 
 
 # Check command argument looks unsafe.

diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs
@@ -433,6 +433,7 @@ fn get_call_kind(func: &Expr, semantic: &SemanticModel) -> Option<CallKind> {
                     "Popen" | "call" | "check_call" | "check_output" | "run" => {
                         Some(CallKind::Subprocess)
                     }
+                    "getoutput" | "getstatusoutput" => Some(CallKind::Shell),
                     _ => None,
                 },
                 "popen2" => match submodule {

diff --git a/...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S605_S605.py.snap b/...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S605_S605.py.snap
@@ -1,147 +1,165 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
 ---
-S605.py:7:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
-  |
-6 | # Check all shell functions.
-7 | os.system("true")
-  |           ^^^^^^ S605
-8 | os.popen("true")
-9 | os.popen2("true")
-  |
-
-S605.py:8:10: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
-   |
- 6 | # Check all shell functions.
- 7 | os.system("true")
- 8 | os.popen("true")
-   |          ^^^^^^ S605
- 9 | os.popen2("true")
-10 | os.popen3("true")
+S605.py:8:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+   |
+ 7 | # Check all shell functions.
+ 8 | os.system("true")
+   |           ^^^^^^ S605
+ 9 | os.popen("true")
+10 | os.popen2("true")
    |
 
-S605.py:9:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+S605.py:9:10: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
- 7 | os.system("true")
- 8 | os.popen("true")
- 9 | os.popen2("true")
-   |           ^^^^^^ S605
-10 | os.popen3("true")
-11 | os.popen4("true")
+ 7 | # Check all shell functions.
+ 8 | os.system("true")
+ 9 | os.popen("true")
+   |          ^^^^^^ S605
+10 | os.popen2("true")
+11 | os.popen3("true")
    |
 
 S605.py:10:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
- 8 | os.popen("true")
- 9 | os.popen2("true")
-10 | os.popen3("true")
+ 8 | os.system("true")
+ 9 | os.popen("true")
+10 | os.popen2("true")
    |           ^^^^^^ S605
-11 | os.popen4("true")
-12 | popen2.popen2("true")
+11 | os.popen3("true")
+12 | os.popen4("true")
    |
 
 S605.py:11:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
- 9 | os.popen2("true")
-10 | os.popen3("true")
-11 | os.popen4("true")
+ 9 | os.popen("true")
+10 | os.popen2("true")
+11 | os.popen3("true")
    |           ^^^^^^ S605
-12 | popen2.popen2("true")
-13 | popen2.popen3("true")
+12 | os.popen4("true")
+13 | popen2.popen2("true")
    |
 
-S605.py:12:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+S605.py:12:11: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-10 | os.popen3("true")
-11 | os.popen4("true")
-12 | popen2.popen2("true")
-   |               ^^^^^^ S605
-13 | popen2.popen3("true")
-14 | popen2.popen4("true")
+10 | os.popen2("true")
+11 | os.popen3("true")
+12 | os.popen4("true")
+   |           ^^^^^^ S605
+13 | popen2.popen2("true")
+14 | popen2.popen3("true")
    |
 
 S605.py:13:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-11 | os.popen4("true")
-12 | popen2.popen2("true")
-13 | popen2.popen3("true")
+11 | os.popen3("true")
+12 | os.popen4("true")
+13 | popen2.popen2("true")
    |               ^^^^^^ S605
-14 | popen2.popen4("true")
-15 | popen2.Popen3("true")
+14 | popen2.popen3("true")
+15 | popen2.popen4("true")
    |
 
 S605.py:14:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-12 | popen2.popen2("true")
-13 | popen2.popen3("true")
-14 | popen2.popen4("true")
+12 | os.popen4("true")
+13 | popen2.popen2("true")
+14 | popen2.popen3("true")
    |               ^^^^^^ S605
-15 | popen2.Popen3("true")
-16 | popen2.Popen4("true")
+15 | popen2.popen4("true")
+16 | popen2.Popen3("true")
    |
 
 S605.py:15:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-13 | popen2.popen3("true")
-14 | popen2.popen4("true")
-15 | popen2.Popen3("true")
+13 | popen2.popen2("true")
+14 | popen2.popen3("true")
+15 | popen2.popen4("true")
    |               ^^^^^^ S605
-16 | popen2.Popen4("true")
-17 | commands.getoutput("true")
+16 | popen2.Popen3("true")
+17 | popen2.Popen4("true")
    |
 
 S605.py:16:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-14 | popen2.popen4("true")
-15 | popen2.Popen3("true")
-16 | popen2.Popen4("true")
+14 | popen2.popen3("true")
+15 | popen2.popen4("true")
+16 | popen2.Popen3("true")
    |               ^^^^^^ S605
-17 | commands.getoutput("true")
-18 | commands.getstatusoutput("true")
+17 | popen2.Popen4("true")
+18 | commands.getoutput("true")
    |
 
-S605.py:17:20: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+S605.py:17:15: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-15 | popen2.Popen3("true")
-16 | popen2.Popen4("true")
-17 | commands.getoutput("true")
+15 | popen2.popen4("true")
+16 | popen2.Popen3("true")
+17 | popen2.Popen4("true")
+   |               ^^^^^^ S605
+18 | commands.getoutput("true")
+19 | commands.getstatusoutput("true")
+   |
+
+S605.py:18:20: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+   |
+16 | popen2.Popen3("true")
+17 | popen2.Popen4("true")
+18 | commands.getoutput("true")
    |                    ^^^^^^ S605
-18 | commands.getstatusoutput("true")
+19 | commands.getstatusoutput("true")
+20 | subprocess.getoutput("true")
    |
 
-S605.py:18:26: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+S605.py:19:26: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
    |
-16 | popen2.Popen4("true")
-17 | commands.getoutput("true")
-18 | commands.getstatusoutput("true")
+17 | popen2.Popen4("true")
+18 | commands.getoutput("true")
+19 | commands.getstatusoutput("true")
    |                          ^^^^^^ S605
+20 | subprocess.getoutput("true")
+21 | subprocess.getstatusoutput("true")
    |
 
-S605.py:23:11: S605 Starting a process with a shell, possible injection detected
+S605.py:20:22: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+   |
+18 | commands.getoutput("true")
+19 | commands.getstatusoutput("true")
+20 | subprocess.getoutput("true")
+   |                      ^^^^^^ S605
+21 | subprocess.getstatusoutput("true")
+   |
+
+S605.py:21:28: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+   |
+19 | commands.getstatusoutput("true")
+20 | subprocess.getoutput("true")
+21 | subprocess.getstatusoutput("true")
+   |                            ^^^^^^ S605
    |
-21 | # Check command argument looks unsafe.
-22 | var_string = "true"
-23 | os.system(var_string)
+
+S605.py:26:11: S605 Starting a process with a shell, possible injection detected
+   |
+24 | # Check command argument looks unsafe.
+25 | var_string = "true"
+26 | os.system(var_string)
    |           ^^^^^^^^^^ S605
-24 | os.system([var_string])
-25 | os.system([var_string, ""])
+27 | os.system([var_string])
+28 | os.system([var_string, ""])
    |
 
-S605.py:24:11: S605 Starting a process with a shell, possible injection detected
+S605.py:27:11: S605 Starting a process with a shell, possible injection detected
    |
-22 | var_string = "true"
-23 | os.system(var_string)
-24 | os.system([var_string])
+25 | var_string = "true"
+26 | os.system(var_string)
+27 | os.system([var_string])
    |           ^^^^^^^^^^^^ S605
-25 | os.system([var_string, ""])
+28 | os.system([var_string, ""])
    |
 
-S605.py:25:11: S605 Starting a process with a shell, possible injection detected
+S605.py:28:11: S605 Starting a process with a shell, possible injection detected
    |
-23 | os.system(var_string)
-24 | os.system([var_string])
-25 | os.system([var_string, ""])
+26 | os.system(var_string)
+27 | os.system([var_string])
+28 | os.system([var_string, ""])
    |           ^^^^^^^^^^^^^^^^ S605
    |
-
-