From 142b627bb82b4467ece0f3d627767e825e30afde Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 31 Jan 2023 16:56:03 -0500 Subject: [PATCH] Avoid Bandit false-positives for empty-string-as-password (#2421) --- resources/test/fixtures/flake8_bandit/S105.py | 1 + resources/test/fixtures/flake8_bandit/S106.py | 1 + resources/test/fixtures/flake8_bandit/S107.py | 4 + .../rules/hardcoded_password_default.rs | 2 +- .../rules/hardcoded_password_func_arg.rs | 2 +- .../rules/hardcoded_password_string.rs | 4 +- ...s__flake8_bandit__tests__S105_S105.py.snap | 172 +++++++++--------- ...s__flake8_bandit__tests__S106_S106.py.snap | 4 +- 8 files changed, 98 insertions(+), 92 deletions(-) diff --git a/resources/test/fixtures/flake8_bandit/S105.py b/resources/test/fixtures/flake8_bandit/S105.py index 30d7060fa7..919fe574ed 100644 --- a/resources/test/fixtures/flake8_bandit/S105.py +++ b/resources/test/fixtures/flake8_bandit/S105.py @@ -4,6 +4,7 @@ d = {} safe = "s3cr3t" password = True password = safe +password = "" password is True password == 1 d["safe"] = "s3cr3t" diff --git a/resources/test/fixtures/flake8_bandit/S106.py b/resources/test/fixtures/flake8_bandit/S106.py index fd83a752e9..f7523999dd 100644 --- a/resources/test/fixtures/flake8_bandit/S106.py +++ b/resources/test/fixtures/flake8_bandit/S106.py @@ -7,6 +7,7 @@ string = "Hello World" # OK func("s3cr3t") func(1, password=string) +func(1, password="") func(pos="s3cr3t", password=string) # Error diff --git a/resources/test/fixtures/flake8_bandit/S107.py b/resources/test/fixtures/flake8_bandit/S107.py index a954e8de0c..c0656f500b 100644 --- a/resources/test/fixtures/flake8_bandit/S107.py +++ b/resources/test/fixtures/flake8_bandit/S107.py @@ -28,3 +28,7 @@ def ok_all(first, /, pos, default="posonly", *, kwonly="kwonly"): def default_all(first, /, pos, secret="posonly", *, password="kwonly"): pass + + +def ok_empty(first, password=""): + pass diff --git a/src/rules/flake8_bandit/rules/hardcoded_password_default.rs b/src/rules/flake8_bandit/rules/hardcoded_password_default.rs index 0912b29ec4..3ac5726ce3 100644 --- a/src/rules/flake8_bandit/rules/hardcoded_password_default.rs +++ b/src/rules/flake8_bandit/rules/hardcoded_password_default.rs @@ -6,7 +6,7 @@ use crate::registry::Diagnostic; use crate::violations; fn check_password_kwarg(arg: &Located, default: &Expr) -> Option { - let string = string_literal(default)?; + let string = string_literal(default).filter(|string| !string.is_empty())?; let kwarg_name = &arg.node.arg; if !matches_password_name(kwarg_name) { return None; diff --git a/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs b/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs index 873bd9abff..b2ce0f95ad 100644 --- a/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs +++ b/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs @@ -10,7 +10,7 @@ pub fn hardcoded_password_func_arg(keywords: &[Keyword]) -> Vec { keywords .iter() .filter_map(|keyword| { - let string = string_literal(&keyword.node.value)?; + let string = string_literal(&keyword.node.value).filter(|string| !string.is_empty())?; let arg = keyword.node.arg.as_ref()?; if !matches_password_name(arg) { return None; diff --git a/src/rules/flake8_bandit/rules/hardcoded_password_string.rs b/src/rules/flake8_bandit/rules/hardcoded_password_string.rs index 95ab903d88..9bed90e327 100644 --- a/src/rules/flake8_bandit/rules/hardcoded_password_string.rs +++ b/src/rules/flake8_bandit/rules/hardcoded_password_string.rs @@ -30,7 +30,7 @@ pub fn compare_to_hardcoded_password_string(left: &Expr, comparators: &[Expr]) - comparators .iter() .filter_map(|comp| { - let string = string_literal(comp)?; + let string = string_literal(comp).filter(|string| !string.is_empty())?; if !is_password_target(left) { return None; } @@ -46,7 +46,7 @@ pub fn compare_to_hardcoded_password_string(left: &Expr, comparators: &[Expr]) - /// S105 pub fn assign_hardcoded_password_string(value: &Expr, targets: &[Expr]) -> Option { - if let Some(string) = string_literal(value) { + if let Some(string) = string_literal(value).filter(|string| !string.is_empty()) { for target in targets { if is_password_target(target) { return Some(Diagnostic::new( diff --git a/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S105_S105.py.snap b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S105_S105.py.snap index 8a61b82768..3a69610352 100644 --- a/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S105_S105.py.snap +++ b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S105_S105.py.snap @@ -6,10 +6,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 12 + row: 13 column: 11 end_location: - row: 12 + row: 13 column: 19 fix: ~ parent: ~ @@ -17,10 +17,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 13 + row: 14 column: 8 end_location: - row: 13 + row: 14 column: 16 fix: ~ parent: ~ @@ -28,10 +28,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 14 + row: 15 column: 9 end_location: - row: 14 + row: 15 column: 17 fix: ~ parent: ~ @@ -39,10 +39,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 15 + row: 16 column: 6 end_location: - row: 15 + row: 16 column: 14 fix: ~ parent: ~ @@ -50,10 +50,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 16 + row: 17 column: 9 end_location: - row: 16 + row: 17 column: 17 fix: ~ parent: ~ @@ -61,10 +61,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 17 + row: 18 column: 8 end_location: - row: 17 + row: 18 column: 16 fix: ~ parent: ~ @@ -72,22 +72,11 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 18 + row: 19 column: 10 end_location: - row: 18 - column: 18 - fix: ~ - parent: ~ -- kind: - HardcodedPasswordString: - string: s3cr3t - location: row: 19 column: 18 - end_location: - row: 19 - column: 26 fix: ~ parent: ~ - kind: @@ -105,10 +94,21 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 22 + row: 21 + column: 18 + end_location: + row: 21 + column: 26 + fix: ~ + parent: ~ +- kind: + HardcodedPasswordString: + string: s3cr3t + location: + row: 23 column: 16 end_location: - row: 22 + row: 23 column: 24 fix: ~ parent: ~ @@ -116,10 +116,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 23 + row: 24 column: 12 end_location: - row: 23 + row: 24 column: 20 fix: ~ parent: ~ @@ -127,10 +127,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 24 + row: 25 column: 14 end_location: - row: 24 + row: 25 column: 22 fix: ~ parent: ~ @@ -138,10 +138,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 25 + row: 26 column: 11 end_location: - row: 25 + row: 26 column: 19 fix: ~ parent: ~ @@ -149,10 +149,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 26 + row: 27 column: 14 end_location: - row: 26 + row: 27 column: 22 fix: ~ parent: ~ @@ -160,10 +160,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 27 + row: 28 column: 13 end_location: - row: 27 + row: 28 column: 21 fix: ~ parent: ~ @@ -171,22 +171,11 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 28 + row: 29 column: 15 end_location: - row: 28 - column: 23 - fix: ~ - parent: ~ -- kind: - HardcodedPasswordString: - string: s3cr3t - location: row: 29 column: 23 - end_location: - row: 29 - column: 31 fix: ~ parent: ~ - kind: @@ -204,10 +193,21 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 34 + row: 31 + column: 23 + end_location: + row: 31 + column: 31 + fix: ~ + parent: ~ +- kind: + HardcodedPasswordString: + string: s3cr3t + location: + row: 35 column: 15 end_location: - row: 34 + row: 35 column: 23 fix: ~ parent: ~ @@ -215,10 +215,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 38 + row: 39 column: 19 end_location: - row: 38 + row: 39 column: 27 fix: ~ parent: ~ @@ -226,10 +226,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 39 + row: 40 column: 16 end_location: - row: 39 + row: 40 column: 24 fix: ~ parent: ~ @@ -237,10 +237,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 40 + row: 41 column: 17 end_location: - row: 40 + row: 41 column: 25 fix: ~ parent: ~ @@ -248,10 +248,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 41 + row: 42 column: 14 end_location: - row: 41 + row: 42 column: 22 fix: ~ parent: ~ @@ -259,10 +259,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 42 + row: 43 column: 17 end_location: - row: 42 + row: 43 column: 25 fix: ~ parent: ~ @@ -270,10 +270,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 43 + row: 44 column: 16 end_location: - row: 43 + row: 44 column: 24 fix: ~ parent: ~ @@ -281,10 +281,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 44 + row: 45 column: 18 end_location: - row: 44 + row: 45 column: 26 fix: ~ parent: ~ @@ -292,10 +292,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 46 + row: 47 column: 12 end_location: - row: 46 + row: 47 column: 20 fix: ~ parent: ~ @@ -303,10 +303,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 47 + row: 48 column: 9 end_location: - row: 47 + row: 48 column: 17 fix: ~ parent: ~ @@ -314,10 +314,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 48 + row: 49 column: 10 end_location: - row: 48 + row: 49 column: 18 fix: ~ parent: ~ @@ -325,10 +325,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 49 + row: 50 column: 7 end_location: - row: 49 + row: 50 column: 15 fix: ~ parent: ~ @@ -336,10 +336,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 50 + row: 51 column: 10 end_location: - row: 50 + row: 51 column: 18 fix: ~ parent: ~ @@ -347,10 +347,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 51 + row: 52 column: 9 end_location: - row: 51 + row: 52 column: 17 fix: ~ parent: ~ @@ -358,10 +358,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 52 + row: 53 column: 11 end_location: - row: 52 + row: 53 column: 19 fix: ~ parent: ~ @@ -369,10 +369,10 @@ expression: diagnostics HardcodedPasswordString: string: s3cr3t location: - row: 53 + row: 54 column: 20 end_location: - row: 53 + row: 54 column: 28 fix: ~ parent: ~ @@ -380,10 +380,10 @@ expression: diagnostics HardcodedPasswordString: string: "1\n2" location: - row: 55 + row: 56 column: 12 end_location: - row: 55 + row: 56 column: 18 fix: ~ parent: ~ @@ -391,10 +391,10 @@ expression: diagnostics HardcodedPasswordString: string: "3\t4" location: - row: 58 + row: 59 column: 12 end_location: - row: 58 + row: 59 column: 18 fix: ~ parent: ~ @@ -402,10 +402,10 @@ expression: diagnostics HardcodedPasswordString: string: "5\r6" location: - row: 61 + row: 62 column: 12 end_location: - row: 61 + row: 62 column: 18 fix: ~ parent: ~ diff --git a/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S106_S106.py.snap b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S106_S106.py.snap index c2ecb76434..2b82892c5b 100644 --- a/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S106_S106.py.snap +++ b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S106_S106.py.snap @@ -6,10 +6,10 @@ expression: diagnostics HardcodedPasswordFuncArg: string: s3cr3t location: - row: 13 + row: 14 column: 8 end_location: - row: 13 + row: 14 column: 25 fix: ~ parent: ~