Use target name in hardcoded-password diagnostics (astral-sh#4365)

squiddy · May 11, 2023 · 572adf7 · 572adf7
1 parent 3b26bf8
commit 572adf7
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 69 deletions.
diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_default.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_default.rs
@@ -7,26 +7,29 @@ use super::super::helpers::{matches_password_name, string_literal};
 
 #[violation]
 pub struct HardcodedPasswordDefault {
-    string: String,
+    name: String,
 }
 
 impl Violation for HardcodedPasswordDefault {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let HardcodedPasswordDefault { string } = self;
-        format!("Possible hardcoded password: \"{}\"", string.escape_debug())
+        let HardcodedPasswordDefault { name } = self;
+        format!(
+            "Possible hardcoded password assigned to function default: \"{}\"",
+            name.escape_debug()
+        )
     }
 }
 
 fn check_password_kwarg(arg: &Arg, default: &Expr) -> Option<Diagnostic> {
-    let string = string_literal(default).filter(|string| !string.is_empty())?;
+    string_literal(default).filter(|string| !string.is_empty())?;
     let kwarg_name = &arg.node.arg;
     if !matches_password_name(kwarg_name) {
         return None;
     }
     Some(Diagnostic::new(
         HardcodedPasswordDefault {
-            string: string.to_string(),
+            name: kwarg_name.to_string(),
         },
         default.range(),
     ))

diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_func_arg.rs
@@ -7,14 +7,17 @@ use super::super::helpers::{matches_password_name, string_literal};
 
 #[violation]
 pub struct HardcodedPasswordFuncArg {
-    string: String,
+    name: String,
 }
 
 impl Violation for HardcodedPasswordFuncArg {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let HardcodedPasswordFuncArg { string } = self;
-        format!("Possible hardcoded password: \"{}\"", string.escape_debug())
+        let HardcodedPasswordFuncArg { name } = self;
+        format!(
+            "Possible hardcoded password assigned to argument: \"{}\"",
+            name.escape_debug()
+        )
     }
 }
 
@@ -23,14 +26,14 @@ pub fn hardcoded_password_func_arg(keywords: &[Keyword]) -> Vec<Diagnostic> {
     keywords
         .iter()
         .filter_map(|keyword| {
-            let string = string_literal(&keyword.node.value).filter(|string| !string.is_empty())?;
+            string_literal(&keyword.node.value).filter(|string| !string.is_empty())?;
             let arg = keyword.node.arg.as_ref()?;
             if !matches_password_name(arg) {
                 return None;
             }
             Some(Diagnostic::new(
                 HardcodedPasswordFuncArg {
-                    string: string.to_string(),
+                    name: arg.to_string(),
                 },
                 keyword.range(),
             ))

diff --git a/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_string.rs b/crates/ruff/src/rules/flake8_bandit/rules/hardcoded_password_string.rs
@@ -7,18 +7,21 @@ use super::super::helpers::{matches_password_name, string_literal};
 
 #[violation]
 pub struct HardcodedPasswordString {
-    string: String,
+    name: String,
 }
 
 impl Violation for HardcodedPasswordString {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let HardcodedPasswordString { string } = self;
-        format!("Possible hardcoded password: \"{}\"", string.escape_debug())
+        let HardcodedPasswordString { name } = self;
+        format!(
+            "Possible hardcoded password assigned to: \"{}\"",
+            name.escape_debug()
+        )
     }
 }
 
-fn is_password_target(target: &Expr) -> bool {
+fn password_target(target: &Expr) -> Option<&str> {
     let target_name = match &target.node {
         // variable = "s3cr3t"
         ExprKind::Name { id, .. } => id,
@@ -28,28 +31,32 @@ fn is_password_target(target: &Expr) -> bool {
                 value: Constant::Str(string),
                 ..
             } => string,
-            _ => return false,
+            _ => return None,
         },
         // obj.password = "s3cr3t"
         ExprKind::Attribute { attr, .. } => attr,
-        _ => return false,
+        _ => return None,
     };
 
-    matches_password_name(target_name)
+    if matches_password_name(target_name) {
+        Some(target_name)
+    } else {
+        None
+    }
 }
 
 /// S105
 pub fn compare_to_hardcoded_password_string(left: &Expr, comparators: &[Expr]) -> Vec<Diagnostic> {
     comparators
         .iter()
         .filter_map(|comp| {
-            let string = string_literal(comp).filter(|string| !string.is_empty())?;
-            if !is_password_target(left) {
+            string_literal(comp).filter(|string| !string.is_empty())?;
+            let Some(name) = password_target(left) else {
                 return None;
-            }
+            };
             Some(Diagnostic::new(
                 HardcodedPasswordString {
-                    string: string.to_string(),
+                    name: name.to_string(),
                 },
                 comp.range(),
             ))
@@ -59,12 +66,15 @@ pub fn compare_to_hardcoded_password_string(left: &Expr, comparators: &[Expr]) -
 
 /// S105
 pub fn assign_hardcoded_password_string(value: &Expr, targets: &[Expr]) -> Option<Diagnostic> {
-    if let Some(string) = string_literal(value).filter(|string| !string.is_empty()) {
+    if string_literal(value)
+        .filter(|string| !string.is_empty())
+        .is_some()
+    {
         for target in targets {
-            if is_password_target(target) {
+            if let Some(name) = password_target(target) {
                 return Some(Diagnostic::new(
                     HardcodedPasswordString {
-                        string: string.to_string(),
+                        name: name.to_string(),
                     },
                     value.range(),
                 ));