[`flake8-bandit`] Handle string literal bindings in suspicious-url-open-usage (`S310`) (#21469)

Co-authored-by: Micha Reiser <micha@reiser.io>

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

@@ -45,3 +45,22 @@ urllib.request.urlopen(urllib.request.Request(url))
 # https://github.com/astral-sh/ruff/issues/15522
 map(urllib.request.urlopen, [])
 foo = urllib.request.urlopen
+
+# https://github.com/astral-sh/ruff/issues/21462
+path = "https://example.com/data.csv"
+urllib.request.urlretrieve(path, "data.csv")
+url = "https://example.com/api"
+urllib.request.Request(url)
+
+# Test resolved f-strings and concatenated string literals
+fstring_url = f"https://example.com/data.csv"
+urllib.request.urlopen(fstring_url)
+urllib.request.Request(fstring_url)
+
+concatenated_url = "https://" + "example.com/data.csv"
+urllib.request.urlopen(concatenated_url)
+urllib.request.Request(concatenated_url)
+
+nested_concatenated = "http://" + "example.com" + "/data.csv"
+urllib.request.urlopen(nested_concatenated)
+urllib.request.Request(nested_concatenated)

crates/ruff_linter/src/preview.rs

@@ -279,3 +279,10 @@ pub(crate) const fn is_extended_snmp_api_path_detection_enabled(settings: &Linte
 pub(crate) const fn is_enumerate_for_loop_int_index_enabled(settings: &LinterSettings) -> bool {
     settings.preview.is_enabled()
 }
+
+// https://github.com/astral-sh/ruff/pull/21469
+pub(crate) const fn is_s310_resolve_string_literal_bindings_enabled(
+    settings: &LinterSettings,
+) -> bool {
+    settings.preview.is_enabled()
+}

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

@@ -4,11 +4,16 @@
 use itertools::Either;
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::{self as ast, Arguments, Decorator, Expr, ExprCall, Operator};
+use ruff_python_semantic::SemanticModel;
+use ruff_python_semantic::analyze::typing::find_binding_value;
 use ruff_text_size::{Ranged, TextRange};
 
 use crate::Violation;
 use crate::checkers::ast::Checker;
-use crate::preview::is_suspicious_function_reference_enabled;
+use crate::preview::{
+    is_s310_resolve_string_literal_bindings_enabled, is_suspicious_function_reference_enabled,
+};
+use crate::settings::LinterSettings;
 
 /// ## What it does
 /// Checks for calls to `pickle` functions or modules that wrap them.
@@ -1016,6 +1021,25 @@ fn suspicious_function(
             || has_prefix(chars.skip_while(|c| c.is_whitespace()), "https://")
     }
 
+    /// Resolves `expr` to its binding and checks if the resolved expression starts with an HTTP or HTTPS prefix.
+    fn expression_starts_with_http_prefix(
+        expr: &Expr,
+        semantic: &SemanticModel,
+        settings: &LinterSettings,
+    ) -> bool {
+        let resolved_expression = if is_s310_resolve_string_literal_bindings_enabled(settings)
+            && let Some(name_expr) = expr.as_name_expr()
+            && let Some(binding_id) = semantic.only_binding(name_expr)
+            && let Some(value) = find_binding_value(semantic.binding(binding_id), semantic)
+        {
+            value
+        } else {
+            expr
+        };
+
+        leading_chars(resolved_expression).is_some_and(has_http_prefix)
+    }
+
     /// Return the leading characters for an expression, if it's a string literal, f-string, or
     /// string concatenation.
     fn leading_chars(expr: &Expr) -> Option<impl Iterator<Item = char> + Clone + '_> {
@@ -1139,17 +1163,19 @@ fn suspicious_function(
         // URLOpen (`Request`)
         ["urllib", "request", "Request"] | ["six", "moves", "urllib", "request", "Request"] => {
             if let Some(arguments) = arguments {
-                // If the `url` argument is a string literal or an f-string, allow `http` and `https` schemes.
+                // If the `url` argument is a string literal (including resolved bindings), allow `http` and `https` schemes.
                 if arguments.args.iter().all(|arg| !arg.is_starred_expr())
                     && arguments
                         .keywords
                         .iter()
                         .all(|keyword| keyword.arg.is_some())
                 {
-                    if arguments
+                    if let Some(url_expr) = arguments.find_argument_value("url", 0)
-                        .find_argument_value("url", 0)
+                        && expression_starts_with_http_prefix(
-                        .and_then(leading_chars)
+                            url_expr,
-                        .is_some_and(has_http_prefix)
+                            checker.semantic(),
+                            checker.settings(),
+                        )
                     {
                         return;
                     }
@@ -1186,19 +1212,25 @@ fn suspicious_function(
                                     name.segments() == ["urllib", "request", "Request"]
                                 })
                             {
-                                if arguments
+                                if let Some(url_expr) = arguments.find_argument_value("url", 0)
-                                    .find_argument_value("url", 0)
+                                    && expression_starts_with_http_prefix(
-                                    .and_then(leading_chars)
+                                        url_expr,
-                                    .is_some_and(has_http_prefix)
+                                        checker.semantic(),
+                                        checker.settings(),
+                                    )
                                 {
                                     return;
                                 }
                             }
                         }
 
-                        // If the `url` argument is a string literal, allow `http` and `https` schemes.
+                        // If the `url` argument is a string literal (including resolved bindings), allow `http` and `https` schemes.
                         Some(expr) => {
-                            if leading_chars(expr).is_some_and(has_http_prefix) {
+                            if expression_starts_with_http_prefix(
+                                expr,
+                                checker.semantic(),
+                                checker.settings(),
+                            ) {
                                 return;
                             }
                         }

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

@@ -254,3 +254,84 @@ S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom sch
 42 | urllib.request.urlopen(urllib.request.Request(url))
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:51:1
+   |
+49 | # https://github.com/astral-sh/ruff/issues/21462
+50 | path = "https://example.com/data.csv"
+51 | urllib.request.urlretrieve(path, "data.csv")
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+52 | url = "https://example.com/api"
+53 | urllib.request.Request(url)
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:53:1
+   |
+51 | urllib.request.urlretrieve(path, "data.csv")
+52 | url = "https://example.com/api"
+53 | urllib.request.Request(url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+54 |
+55 | # Test resolved f-strings and concatenated string literals
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:57:1
+   |
+55 | # Test resolved f-strings and concatenated string literals
+56 | fstring_url = f"https://example.com/data.csv"
+57 | urllib.request.urlopen(fstring_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+58 | urllib.request.Request(fstring_url)
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:58:1
+   |
+56 | fstring_url = f"https://example.com/data.csv"
+57 | urllib.request.urlopen(fstring_url)
+58 | urllib.request.Request(fstring_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+59 |
+60 | concatenated_url = "https://" + "example.com/data.csv"
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:61:1
+   |
+60 | concatenated_url = "https://" + "example.com/data.csv"
+61 | urllib.request.urlopen(concatenated_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+62 | urllib.request.Request(concatenated_url)
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:62:1
+   |
+60 | concatenated_url = "https://" + "example.com/data.csv"
+61 | urllib.request.urlopen(concatenated_url)
+62 | urllib.request.Request(concatenated_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+63 |
+64 | nested_concatenated = "http://" + "example.com" + "/data.csv"
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:65:1
+   |
+64 | nested_concatenated = "http://" + "example.com" + "/data.csv"
+65 | urllib.request.urlopen(nested_concatenated)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+66 | urllib.request.Request(nested_concatenated)
+   |
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:66:1
+   |
+64 | nested_concatenated = "http://" + "example.com" + "/data.csv"
+65 | urllib.request.urlopen(nested_concatenated)
+66 | urllib.request.Request(nested_concatenated)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |

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

@@ -6,9 +6,100 @@ source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
 +linter.preview = enabled
 
 --- Summary ---
-Removed: 0
+Removed: 8
 Added: 2
 
+--- Removed ---
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:51:1
+   |
+49 | # https://github.com/astral-sh/ruff/issues/21462
+50 | path = "https://example.com/data.csv"
+51 | urllib.request.urlretrieve(path, "data.csv")
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+52 | url = "https://example.com/api"
+53 | urllib.request.Request(url)
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:53:1
+   |
+51 | urllib.request.urlretrieve(path, "data.csv")
+52 | url = "https://example.com/api"
+53 | urllib.request.Request(url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+54 |
+55 | # Test resolved f-strings and concatenated string literals
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:57:1
+   |
+55 | # Test resolved f-strings and concatenated string literals
+56 | fstring_url = f"https://example.com/data.csv"
+57 | urllib.request.urlopen(fstring_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+58 | urllib.request.Request(fstring_url)
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:58:1
+   |
+56 | fstring_url = f"https://example.com/data.csv"
+57 | urllib.request.urlopen(fstring_url)
+58 | urllib.request.Request(fstring_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+59 |
+60 | concatenated_url = "https://" + "example.com/data.csv"
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:61:1
+   |
+60 | concatenated_url = "https://" + "example.com/data.csv"
+61 | urllib.request.urlopen(concatenated_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+62 | urllib.request.Request(concatenated_url)
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:62:1
+   |
+60 | concatenated_url = "https://" + "example.com/data.csv"
+61 | urllib.request.urlopen(concatenated_url)
+62 | urllib.request.Request(concatenated_url)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+63 |
+64 | nested_concatenated = "http://" + "example.com" + "/data.csv"
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:65:1
+   |
+64 | nested_concatenated = "http://" + "example.com" + "/data.csv"
+65 | urllib.request.urlopen(nested_concatenated)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+66 | urllib.request.Request(nested_concatenated)
+   |
+
+
+S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+  --> S310.py:66:1
+   |
+64 | nested_concatenated = "http://" + "example.com" + "/data.csv"
+65 | urllib.request.urlopen(nested_concatenated)
+66 | urllib.request.Request(nested_concatenated)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+
+
 --- Added ---
 S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
   --> S310.py:46:5
@@ -27,4 +118,6 @@ S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom sch
 46 | map(urllib.request.urlopen, [])
 47 | foo = urllib.request.urlopen
    |       ^^^^^^^^^^^^^^^^^^^^^^
+48 |
+49 | # https://github.com/astral-sh/ruff/issues/21462
    |