[flake8-bandit] Allow urllib.request.urlopen calls with static `R…

…equest` argument (#10964) ## Summary Allows, e.g.: ```python import urllib urllib.request.urlopen(urllib.request.Request("https://example.com/")) ``` ...in [`suspicious-url-open-usage`](https://docs.astral.sh/ruff/rules/suspicious-url-open-usage/). See: #7918 (comment)
astral-sh · Apr 16, 2024 · effd518 · effd518
1 parent 6dccbd2
commit effd518
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 6 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
@@ -17,3 +17,9 @@
 urllib.request.URLopener().open('http://www.google.com')
 urllib.request.URLopener().open('file:///foo/bar/baz')
 urllib.request.URLopener().open(url)
+
+urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'))
+urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs)
+urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
+urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
+urllib.request.urlopen(urllib.request.Request(url))
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
@@ -849,15 +849,45 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) {
             ["" | "builtins", "eval"] => Some(SuspiciousEvalUsage.into()),
             // MarkSafe
             ["django", "utils", "safestring" | "html", "mark_safe"] => Some(SuspiciousMarkSafeUsage.into()),
-            // URLOpen (`urlopen`, `urlretrieve`, `Request`)
-            ["urllib", "request", "urlopen" | "urlretrieve" | "Request"] |
-            ["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" | "Request"] => {
+            // URLOpen (`Request`)
+            ["urllib", "request","Request"] |
+            ["six", "moves", "urllib", "request","Request"] => {
                 // If the `url` argument is a string literal, allow `http` and `https` schemes.
                 if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
                     if let Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) = &call.arguments.find_argument("url", 0) {
-                        let url = value.to_str().trim_start();
-                        if url.starts_with("http://") || url.starts_with("https://") {
-                            return None;
+                            let url = value.to_str().trim_start();
+                            if url.starts_with("http://") || url.starts_with("https://") {
+                                return None;
+                            }
+
+                    }
+                }
+                Some(SuspiciousURLOpenUsage.into())
+            }
+            // URLOpen (`urlopen`, `urlretrieve`)
+            ["urllib", "request", "urlopen" | "urlretrieve" ] |
+            ["six", "moves", "urllib", "request", "urlopen" | "urlretrieve" ] => {
+                if call.arguments.args.iter().all(|arg| !arg.is_starred_expr()) && call.arguments.keywords.iter().all(|keyword| keyword.arg.is_some()) {
+                    if let Some(arg) = &call.arguments.find_argument("url", 0) {
+                        // If the `url` argument is a string literal, allow `http` and `https` schemes.
+                        if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = arg {
+                            let url = value.to_str().trim_start();
+                            if url.starts_with("http://") || url.starts_with("https://") {
+                                return None;
+                            }
+                        }
+
+                        // If the `url` argument is a `urllib.request.Request` object, allow `http` and `https` schemes.
+                        if let Expr::Call(ExprCall { func, arguments, .. }) = arg {
+                            if checker.semantic().resolve_qualified_name(func.as_ref()).is_some_and(|name| name.segments() == ["urllib", "request", "Request"]) {
+                                if let Some( Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) = arguments.find_argument("url", 0) {
+                                        let url = value.to_str().trim_start();
+                                        if url.starts_with("http://") || url.starts_with("https://") {
+                                            return None;
+                                        }
+
+                                }
+                            }
                         }
                     }
                 }

diff --git a/...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S310_S310.py.snap b/...rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S310_S310.py.snap
@@ -102,6 +102,49 @@ S310.py:19:1: S310 Audit URL open for permitted schemes. Allowing use of `file:`
 18 | urllib.request.URLopener().open('file:///foo/bar/baz')
 19 | urllib.request.URLopener().open(url)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
+20 | 
+21 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'))
    |
 
+S310.py:22:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+   |
+21 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'))
+22 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
+23 | urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
+24 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
+   |
 
+S310.py:24:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+   |
+22 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs)
+23 | urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
+24 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
+25 | urllib.request.urlopen(urllib.request.Request(url))
+   |
+
+S310.py:24:24: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+   |
+22 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs)
+23 | urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
+24 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
+25 | urllib.request.urlopen(urllib.request.Request(url))
+   |
+
+S310.py:25:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+   |
+23 | urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
+24 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
+25 | urllib.request.urlopen(urllib.request.Request(url))
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
+   |
+
+S310.py:25:24: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected.
+   |
+23 | urllib.request.urlopen(urllib.request.Request('http://www.google.com'))
+24 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz'))
+25 | urllib.request.urlopen(urllib.request.Request(url))
+   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310
+   |