From adf4f1e3f4b68038991ae8e9a397a03fb899fdaf Mon Sep 17 00:00:00 2001 From: Dan Parizher <105245560+danparizher@users.noreply.github.com> Date: Wed, 26 Nov 2025 04:21:50 -0500 Subject: [PATCH] [`flake8-bandit`] Handle string literal bindings in suspicious-url-open-usage (`S310`) (#21469) Co-authored-by: Micha Reiser --- .../test/fixtures/flake8_bandit/S310.py | 19 ++++ crates/ruff_linter/src/preview.rs | 7 ++ .../rules/suspicious_function_call.rs | 56 ++++++++--- ...s__flake8_bandit__tests__S310_S310.py.snap | 81 ++++++++++++++++ ..._bandit__tests__preview__S310_S310.py.snap | 95 ++++++++++++++++++- 5 files changed, 245 insertions(+), 13 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py index 62ec7108fa..1844678316 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py +++ b/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) diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index c49af75fab..52be730545 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/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() +} diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs index 423b7287bb..2eb9fa4351 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/suspicious_function_call.rs +++ b/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 + 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 - .find_argument_value("url", 0) - .and_then(leading_chars) - .is_some_and(has_http_prefix) + if let Some(url_expr) = arguments.find_argument_value("url", 0) + && expression_starts_with_http_prefix( + url_expr, + checker.semantic(), + checker.settings(), + ) { return; } @@ -1186,19 +1212,25 @@ fn suspicious_function( name.segments() == ["urllib", "request", "Request"] }) { - if arguments - .find_argument_value("url", 0) - .and_then(leading_chars) - .is_some_and(has_http_prefix) + if let Some(url_expr) = arguments.find_argument_value("url", 0) + && expression_starts_with_http_prefix( + url_expr, + 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; } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S310_S310.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S310_S310.py.snap index 4a4b61876e..7c73532b23 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S310_S310.py.snap +++ b/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) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S310_S310.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S310_S310.py.snap index ab8e823ec7..081c3afb49 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__preview__S310_S310.py.snap +++ b/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 |