Skip to content

Commit 43fd189

Browse files
trag1cmaxmynter
trag1c
authored and
maxmynter
committed
[flake8-bandit] Mark str and list[str] literals as trusted input (S603) (astral-sh#17136)
## Summary Closes astral-sh#17112. Allows passing in string and list-of-strings literals into `subprocess.run` (and related) calls without marking them as untrusted input: ```py import subprocess subprocess.run("true") # "instant" named expressions are also allowed subprocess.run(c := "ls") ``` ## Test Plan Added test cases covering new behavior, passed with `cargo nextest run`.
1 parent 504c1f9 commit 43fd189

File tree

5 files changed

+332
-84
lines changed

5 files changed

+332
-84
lines changed

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

+30-9
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,41 @@
11
from subprocess import Popen, call, check_call, check_output, run
22

33
# Different Popen wrappers are checked.
4+
a = input()
5+
Popen(a, shell=False)
6+
call(a, shell=False)
7+
check_call(a, shell=False)
8+
check_output(a, shell=False)
9+
run(a, shell=False)
10+
11+
# Falsey values are treated as false.
12+
Popen(a, shell=0)
13+
Popen(a, shell=[])
14+
Popen(a, shell={})
15+
Popen(a, shell=None)
16+
17+
# Unknown values are treated as falsey.
18+
Popen(a, shell=True if True else False)
19+
20+
# No value is also caught.
21+
Popen(a)
22+
23+
# Literals are fine, they're trusted.
24+
run("true")
25+
Popen(["true"])
426
Popen("true", shell=False)
527
call("true", shell=False)
628
check_call("true", shell=False)
729
check_output("true", shell=False)
830
run("true", shell=False)
931

10-
# Values that falsey values are treated as false.
11-
Popen("true", shell=0)
12-
Popen("true", shell=[])
13-
Popen("true", shell={})
14-
Popen("true", shell=None)
32+
# Not through assignments though.
33+
cmd = ["true"]
34+
run(cmd)
1535

16-
# Unknown values are treated as falsey.
17-
Popen("true", shell=True if True else False)
36+
# Instant named expressions are fine.
37+
run(c := "true")
1838

19-
# No value is also caught.
20-
Popen("true")
39+
# But non-instant are not.
40+
(e := "echo")
41+
run(e)

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

+1
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ mod tests {
104104
#[test_case(Rule::SuspiciousURLOpenUsage, Path::new("S310.py"))]
105105
#[test_case(Rule::SuspiciousNonCryptographicRandomUsage, Path::new("S311.py"))]
106106
#[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
107+
#[test_case(Rule::SubprocessWithoutShellEqualsTrue, Path::new("S603.py"))]
107108
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
108109
let snapshot = format!(
109110
"preview__{}_{}",

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

+21-18
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,19 @@ impl Violation for UnixCommandWildcardInjection {
287287
}
288288
}
289289

290+
/// Check if an expression is a trusted input for subprocess.run.
291+
/// We assume that any str or list[str] literal can be trusted.
292+
fn is_trusted_input(arg: &Expr) -> bool {
293+
match arg {
294+
Expr::StringLiteral(_) => true,
295+
Expr::List(ast::ExprList { elts, .. }) => {
296+
elts.iter().all(|elt| matches!(elt, Expr::StringLiteral(_)))
297+
}
298+
Expr::Named(named) => is_trusted_input(&named.value),
299+
_ => false,
300+
}
301+
}
302+
290303
/// S602, S603, S604, S605, S606, S607, S609
291304
pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
292305
let call_kind = get_call_kind(&call.func, checker.semantic());
@@ -310,24 +323,14 @@ pub(crate) fn shell_injection(checker: &Checker, call: &ast::ExprCall) {
310323
}
311324
}
312325
// S603
313-
Some(ShellKeyword {
314-
truthiness:
315-
Truthiness::False | Truthiness::Falsey | Truthiness::None | Truthiness::Unknown,
316-
}) => {
317-
if checker.enabled(Rule::SubprocessWithoutShellEqualsTrue) {
318-
checker.report_diagnostic(Diagnostic::new(
319-
SubprocessWithoutShellEqualsTrue,
320-
call.func.range(),
321-
));
322-
}
323-
}
324-
// S603
325-
None => {
326-
if checker.enabled(Rule::SubprocessWithoutShellEqualsTrue) {
327-
checker.report_diagnostic(Diagnostic::new(
328-
SubprocessWithoutShellEqualsTrue,
329-
call.func.range(),
330-
));
326+
_ => {
327+
if !is_trusted_input(arg) || checker.settings.preview.is_disabled() {
328+
if checker.enabled(Rule::SubprocessWithoutShellEqualsTrue) {
329+
checker.report_diagnostic(Diagnostic::new(
330+
SubprocessWithoutShellEqualsTrue,
331+
call.func.range(),
332+
));
333+
}
331334
}
332335
}
333336
}

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

+155-57
Original file line numberDiff line numberDiff line change
@@ -1,104 +1,202 @@
11
---
22
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
33
---
4-
S603.py:4:1: S603 `subprocess` call: check for execution of untrusted input
4+
S603.py:5:1: S603 `subprocess` call: check for execution of untrusted input
55
|
66
3 | # Different Popen wrappers are checked.
7-
4 | Popen("true", shell=False)
7+
4 | a = input()
8+
5 | Popen(a, shell=False)
89
| ^^^^^ S603
9-
5 | call("true", shell=False)
10-
6 | check_call("true", shell=False)
10+
6 | call(a, shell=False)
11+
7 | check_call(a, shell=False)
1112
|
1213

13-
S603.py:5:1: S603 `subprocess` call: check for execution of untrusted input
14+
S603.py:6:1: S603 `subprocess` call: check for execution of untrusted input
1415
|
15-
3 | # Different Popen wrappers are checked.
16-
4 | Popen("true", shell=False)
17-
5 | call("true", shell=False)
16+
4 | a = input()
17+
5 | Popen(a, shell=False)
18+
6 | call(a, shell=False)
1819
| ^^^^ S603
19-
6 | check_call("true", shell=False)
20-
7 | check_output("true", shell=False)
20+
7 | check_call(a, shell=False)
21+
8 | check_output(a, shell=False)
2122
|
2223

23-
S603.py:6:1: S603 `subprocess` call: check for execution of untrusted input
24+
S603.py:7:1: S603 `subprocess` call: check for execution of untrusted input
2425
|
25-
4 | Popen("true", shell=False)
26-
5 | call("true", shell=False)
27-
6 | check_call("true", shell=False)
26+
5 | Popen(a, shell=False)
27+
6 | call(a, shell=False)
28+
7 | check_call(a, shell=False)
2829
| ^^^^^^^^^^ S603
29-
7 | check_output("true", shell=False)
30-
8 | run("true", shell=False)
30+
8 | check_output(a, shell=False)
31+
9 | run(a, shell=False)
3132
|
3233

33-
S603.py:7:1: S603 `subprocess` call: check for execution of untrusted input
34+
S603.py:8:1: S603 `subprocess` call: check for execution of untrusted input
3435
|
35-
5 | call("true", shell=False)
36-
6 | check_call("true", shell=False)
37-
7 | check_output("true", shell=False)
36+
6 | call(a, shell=False)
37+
7 | check_call(a, shell=False)
38+
8 | check_output(a, shell=False)
3839
| ^^^^^^^^^^^^ S603
39-
8 | run("true", shell=False)
40+
9 | run(a, shell=False)
4041
|
4142

42-
S603.py:8:1: S603 `subprocess` call: check for execution of untrusted input
43+
S603.py:9:1: S603 `subprocess` call: check for execution of untrusted input
4344
|
44-
6 | check_call("true", shell=False)
45-
7 | check_output("true", shell=False)
46-
8 | run("true", shell=False)
45+
7 | check_call(a, shell=False)
46+
8 | check_output(a, shell=False)
47+
9 | run(a, shell=False)
4748
| ^^^ S603
48-
9 |
49-
10 | # Values that falsey values are treated as false.
49+
10 |
50+
11 | # Falsey values are treated as false.
5051
|
5152

52-
S603.py:11:1: S603 `subprocess` call: check for execution of untrusted input
53+
S603.py:12:1: S603 `subprocess` call: check for execution of untrusted input
5354
|
54-
10 | # Values that falsey values are treated as false.
55-
11 | Popen("true", shell=0)
55+
11 | # Falsey values are treated as false.
56+
12 | Popen(a, shell=0)
5657
| ^^^^^ S603
57-
12 | Popen("true", shell=[])
58-
13 | Popen("true", shell={})
58+
13 | Popen(a, shell=[])
59+
14 | Popen(a, shell={})
5960
|
6061

61-
S603.py:12:1: S603 `subprocess` call: check for execution of untrusted input
62+
S603.py:13:1: S603 `subprocess` call: check for execution of untrusted input
6263
|
63-
10 | # Values that falsey values are treated as false.
64-
11 | Popen("true", shell=0)
65-
12 | Popen("true", shell=[])
64+
11 | # Falsey values are treated as false.
65+
12 | Popen(a, shell=0)
66+
13 | Popen(a, shell=[])
6667
| ^^^^^ S603
67-
13 | Popen("true", shell={})
68-
14 | Popen("true", shell=None)
68+
14 | Popen(a, shell={})
69+
15 | Popen(a, shell=None)
6970
|
7071

71-
S603.py:13:1: S603 `subprocess` call: check for execution of untrusted input
72+
S603.py:14:1: S603 `subprocess` call: check for execution of untrusted input
7273
|
73-
11 | Popen("true", shell=0)
74-
12 | Popen("true", shell=[])
75-
13 | Popen("true", shell={})
74+
12 | Popen(a, shell=0)
75+
13 | Popen(a, shell=[])
76+
14 | Popen(a, shell={})
7677
| ^^^^^ S603
77-
14 | Popen("true", shell=None)
78+
15 | Popen(a, shell=None)
7879
|
7980

80-
S603.py:14:1: S603 `subprocess` call: check for execution of untrusted input
81+
S603.py:15:1: S603 `subprocess` call: check for execution of untrusted input
8182
|
82-
12 | Popen("true", shell=[])
83-
13 | Popen("true", shell={})
84-
14 | Popen("true", shell=None)
83+
13 | Popen(a, shell=[])
84+
14 | Popen(a, shell={})
85+
15 | Popen(a, shell=None)
8586
| ^^^^^ S603
86-
15 |
87-
16 | # Unknown values are treated as falsey.
87+
16 |
88+
17 | # Unknown values are treated as falsey.
89+
|
90+
91+
S603.py:18:1: S603 `subprocess` call: check for execution of untrusted input
92+
|
93+
17 | # Unknown values are treated as falsey.
94+
18 | Popen(a, shell=True if True else False)
95+
| ^^^^^ S603
96+
19 |
97+
20 | # No value is also caught.
98+
|
99+
100+
S603.py:21:1: S603 `subprocess` call: check for execution of untrusted input
101+
|
102+
20 | # No value is also caught.
103+
21 | Popen(a)
104+
| ^^^^^ S603
105+
22 |
106+
23 | # Literals are fine, they're trusted.
107+
|
108+
109+
S603.py:24:1: S603 `subprocess` call: check for execution of untrusted input
110+
|
111+
23 | # Literals are fine, they're trusted.
112+
24 | run("true")
113+
| ^^^ S603
114+
25 | Popen(["true"])
115+
26 | Popen("true", shell=False)
88116
|
89117

90-
S603.py:17:1: S603 `subprocess` call: check for execution of untrusted input
118+
S603.py:25:1: S603 `subprocess` call: check for execution of untrusted input
91119
|
92-
16 | # Unknown values are treated as falsey.
93-
17 | Popen("true", shell=True if True else False)
120+
23 | # Literals are fine, they're trusted.
121+
24 | run("true")
122+
25 | Popen(["true"])
94123
| ^^^^^ S603
95-
18 |
96-
19 | # No value is also caught.
124+
26 | Popen("true", shell=False)
125+
27 | call("true", shell=False)
97126
|
98127

99-
S603.py:20:1: S603 `subprocess` call: check for execution of untrusted input
128+
S603.py:26:1: S603 `subprocess` call: check for execution of untrusted input
100129
|
101-
19 | # No value is also caught.
102-
20 | Popen("true")
130+
24 | run("true")
131+
25 | Popen(["true"])
132+
26 | Popen("true", shell=False)
103133
| ^^^^^ S603
134+
27 | call("true", shell=False)
135+
28 | check_call("true", shell=False)
136+
|
137+
138+
S603.py:27:1: S603 `subprocess` call: check for execution of untrusted input
139+
|
140+
25 | Popen(["true"])
141+
26 | Popen("true", shell=False)
142+
27 | call("true", shell=False)
143+
| ^^^^ S603
144+
28 | check_call("true", shell=False)
145+
29 | check_output("true", shell=False)
146+
|
147+
148+
S603.py:28:1: S603 `subprocess` call: check for execution of untrusted input
149+
|
150+
26 | Popen("true", shell=False)
151+
27 | call("true", shell=False)
152+
28 | check_call("true", shell=False)
153+
| ^^^^^^^^^^ S603
154+
29 | check_output("true", shell=False)
155+
30 | run("true", shell=False)
156+
|
157+
158+
S603.py:29:1: S603 `subprocess` call: check for execution of untrusted input
159+
|
160+
27 | call("true", shell=False)
161+
28 | check_call("true", shell=False)
162+
29 | check_output("true", shell=False)
163+
| ^^^^^^^^^^^^ S603
164+
30 | run("true", shell=False)
165+
|
166+
167+
S603.py:30:1: S603 `subprocess` call: check for execution of untrusted input
168+
|
169+
28 | check_call("true", shell=False)
170+
29 | check_output("true", shell=False)
171+
30 | run("true", shell=False)
172+
| ^^^ S603
173+
31 |
174+
32 | # Not through assignments though.
175+
|
176+
177+
S603.py:34:1: S603 `subprocess` call: check for execution of untrusted input
178+
|
179+
32 | # Not through assignments though.
180+
33 | cmd = ["true"]
181+
34 | run(cmd)
182+
| ^^^ S603
183+
35 |
184+
36 | # Instant named expressions are fine.
185+
|
186+
187+
S603.py:37:1: S603 `subprocess` call: check for execution of untrusted input
188+
|
189+
36 | # Instant named expressions are fine.
190+
37 | run(c := "true")
191+
| ^^^ S603
192+
38 |
193+
39 | # But non-instant are not.
194+
|
195+
196+
S603.py:41:1: S603 `subprocess` call: check for execution of untrusted input
197+
|
198+
39 | # But non-instant are not.
199+
40 | (e := "echo")
200+
41 | run(e)
201+
| ^^^ S603
104202
|

0 commit comments

Comments
 (0)