From d3736d29ca0a448bdf0ea448d87181bb3ac6ff09 Mon Sep 17 00:00:00 2001 From: Tim Chan Date: Fri, 12 Jul 2024 07:43:01 -0700 Subject: [PATCH 1/3] fix: avoid raising S310 on safe f-strings --- .../test/fixtures/flake8_bandit/S310.py | 16 +- .../rules/suspicious_function_call.rs | 34 ++- ...s__flake8_bandit__tests__S310_S310.py.snap | 234 +++++++++++------- 3 files changed, 193 insertions(+), 91 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 14c9ee2690877..734ee185a7e73 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S310.py @@ -1,25 +1,37 @@ import urllib.request urllib.request.urlopen(url='http://www.google.com') +urllib.request.urlopen(url=f'http://www.google.com') urllib.request.urlopen(url='http://www.google.com', **kwargs) +urllib.request.urlopen(url=f'http://www.google.com', **kwargs) urllib.request.urlopen('http://www.google.com') +urllib.request.urlopen(f'http://www.google.com') urllib.request.urlopen('file:///foo/bar/baz') urllib.request.urlopen(url) -urllib.request.Request(url='http://www.google.com', **kwargs) urllib.request.Request(url='http://www.google.com') +urllib.request.Request(url=f'http://www.google.com') +urllib.request.Request(url='http://www.google.com', **kwargs) +urllib.request.Request(url=f'http://www.google.com', **kwargs) urllib.request.Request('http://www.google.com') +urllib.request.Request(f'http://www.google.com') urllib.request.Request('file:///foo/bar/baz') urllib.request.Request(url) -urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) urllib.request.URLopener().open(fullurl='http://www.google.com') +urllib.request.URLopener().open(fullurl=f'http://www.google.com') +urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) +urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs) urllib.request.URLopener().open('http://www.google.com') +urllib.request.URLopener().open(f'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(f'http://www.google.com')) urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs) +urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com'), **kwargs) urllib.request.urlopen(urllib.request.Request('http://www.google.com')) +urllib.request.urlopen(urllib.request.Request(f'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 index 00aa3d1cdf439..c70e165be4463 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 @@ -850,9 +850,9 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { // MarkSafe ["django", "utils", "safestring" | "html", "mark_safe"] => Some(SuspiciousMarkSafeUsage.into()), // URLOpen (`Request`) - ["urllib", "request","Request"] | + ["urllib", "request", "Request"] | ["six", "moves", "urllib", "request","Request"] => { - // If the `url` argument is a string literal, allow `http` and `https` schemes. + // If the `url` argument is a string literal or an f string, 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(); @@ -861,6 +861,15 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { } } + + if let Some(Expr::FString(ast::ExprFString { value, .. })) = &call.arguments.find_argument("url", 0) { + if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + let url = value.trim_start(); + if url.starts_with("http://") || url.starts_with("https://") { + return None; + } + } + } } Some(SuspiciousURLOpenUsage.into()) } @@ -877,16 +886,35 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { } } + // If the `url` argument is an f string, allow `http` and `https` schemes. + if let Expr::FString(ast::ExprFString { value, .. }) = arg { + if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + let url = value.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) { + 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; } } + + if let Some(Expr::FString(ast::ExprFString { value, .. })) = arguments.find_argument("url", 0) { + if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + let url = value.trim_start(); + if url.starts_with("http://") || url.starts_with("https://") { + return None; + } + } + } } } } 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 a58da3774b520..7cb003c7ba9e5 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 @@ -1,150 +1,212 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S310.py:4:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:5:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | 3 | urllib.request.urlopen(url='http://www.google.com') -4 | urllib.request.urlopen(url='http://www.google.com', **kwargs) +4 | urllib.request.urlopen(url=f'http://www.google.com') +5 | urllib.request.urlopen(url='http://www.google.com', **kwargs) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -5 | urllib.request.urlopen('http://www.google.com') -6 | urllib.request.urlopen('file:///foo/bar/baz') +6 | urllib.request.urlopen(url=f'http://www.google.com', **kwargs) +7 | urllib.request.urlopen('http://www.google.com') | S310.py:6:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -4 | urllib.request.urlopen(url='http://www.google.com', **kwargs) -5 | urllib.request.urlopen('http://www.google.com') -6 | urllib.request.urlopen('file:///foo/bar/baz') - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -7 | urllib.request.urlopen(url) - | - -S310.py:7:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. - | -5 | urllib.request.urlopen('http://www.google.com') -6 | urllib.request.urlopen('file:///foo/bar/baz') -7 | urllib.request.urlopen(url) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -8 | -9 | urllib.request.Request(url='http://www.google.com', **kwargs) +4 | urllib.request.urlopen(url=f'http://www.google.com') +5 | urllib.request.urlopen(url='http://www.google.com', **kwargs) +6 | urllib.request.urlopen(url=f'http://www.google.com', **kwargs) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +7 | urllib.request.urlopen('http://www.google.com') +8 | urllib.request.urlopen(f'http://www.google.com') | S310.py:9:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | - 7 | urllib.request.urlopen(url) - 8 | - 9 | urllib.request.Request(url='http://www.google.com', **kwargs) + 7 | urllib.request.urlopen('http://www.google.com') + 8 | urllib.request.urlopen(f'http://www.google.com') + 9 | urllib.request.urlopen('file:///foo/bar/baz') + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +10 | urllib.request.urlopen(url) + | + +S310.py:10:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | + 8 | urllib.request.urlopen(f'http://www.google.com') + 9 | urllib.request.urlopen('file:///foo/bar/baz') +10 | urllib.request.urlopen(url) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +11 | +12 | urllib.request.Request(url='http://www.google.com') + | + +S310.py:14:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +12 | urllib.request.Request(url='http://www.google.com') +13 | urllib.request.Request(url=f'http://www.google.com') +14 | urllib.request.Request(url='http://www.google.com', **kwargs) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -10 | urllib.request.Request(url='http://www.google.com') -11 | urllib.request.Request('http://www.google.com') +15 | urllib.request.Request(url=f'http://www.google.com', **kwargs) +16 | urllib.request.Request('http://www.google.com') | -S310.py:12:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:15:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +13 | urllib.request.Request(url=f'http://www.google.com') +14 | urllib.request.Request(url='http://www.google.com', **kwargs) +15 | urllib.request.Request(url=f'http://www.google.com', **kwargs) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +16 | urllib.request.Request('http://www.google.com') +17 | urllib.request.Request(f'http://www.google.com') + | + +S310.py:18:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -10 | urllib.request.Request(url='http://www.google.com') -11 | urllib.request.Request('http://www.google.com') -12 | urllib.request.Request('file:///foo/bar/baz') +16 | urllib.request.Request('http://www.google.com') +17 | urllib.request.Request(f'http://www.google.com') +18 | urllib.request.Request('file:///foo/bar/baz') | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -13 | urllib.request.Request(url) +19 | urllib.request.Request(url) | -S310.py:13:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:19:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -11 | urllib.request.Request('http://www.google.com') -12 | urllib.request.Request('file:///foo/bar/baz') -13 | urllib.request.Request(url) +17 | urllib.request.Request(f'http://www.google.com') +18 | urllib.request.Request('file:///foo/bar/baz') +19 | urllib.request.Request(url) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -14 | -15 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) +20 | +21 | urllib.request.URLopener().open(fullurl='http://www.google.com') | -S310.py:15:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:21:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -13 | urllib.request.Request(url) -14 | -15 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) +19 | urllib.request.Request(url) +20 | +21 | urllib.request.URLopener().open(fullurl='http://www.google.com') | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -16 | urllib.request.URLopener().open(fullurl='http://www.google.com') -17 | urllib.request.URLopener().open('http://www.google.com') +22 | urllib.request.URLopener().open(fullurl=f'http://www.google.com') +23 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) | -S310.py:16:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:22:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -15 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) -16 | urllib.request.URLopener().open(fullurl='http://www.google.com') +21 | urllib.request.URLopener().open(fullurl='http://www.google.com') +22 | urllib.request.URLopener().open(fullurl=f'http://www.google.com') | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -17 | urllib.request.URLopener().open('http://www.google.com') -18 | urllib.request.URLopener().open('file:///foo/bar/baz') +23 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) +24 | urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs) | -S310.py:17:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:23:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -15 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) -16 | urllib.request.URLopener().open(fullurl='http://www.google.com') -17 | urllib.request.URLopener().open('http://www.google.com') +21 | urllib.request.URLopener().open(fullurl='http://www.google.com') +22 | urllib.request.URLopener().open(fullurl=f'http://www.google.com') +23 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -18 | urllib.request.URLopener().open('file:///foo/bar/baz') -19 | urllib.request.URLopener().open(url) +24 | urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs) +25 | urllib.request.URLopener().open('http://www.google.com') | -S310.py:18:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:24:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -16 | urllib.request.URLopener().open(fullurl='http://www.google.com') -17 | urllib.request.URLopener().open('http://www.google.com') -18 | urllib.request.URLopener().open('file:///foo/bar/baz') +22 | urllib.request.URLopener().open(fullurl=f'http://www.google.com') +23 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) +24 | urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -19 | urllib.request.URLopener().open(url) +25 | urllib.request.URLopener().open('http://www.google.com') +26 | urllib.request.URLopener().open(f'http://www.google.com') | -S310.py:19:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:25:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. | -17 | urllib.request.URLopener().open('http://www.google.com') -18 | urllib.request.URLopener().open('file:///foo/bar/baz') -19 | urllib.request.URLopener().open(url) +23 | urllib.request.URLopener().open(fullurl='http://www.google.com', **kwargs) +24 | urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs) +25 | urllib.request.URLopener().open('http://www.google.com') | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -20 | -21 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com')) +26 | urllib.request.URLopener().open(f'http://www.google.com') +27 | urllib.request.URLopener().open('file:///foo/bar/baz') | -S310.py:22:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:26:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +24 | urllib.request.URLopener().open(fullurl=f'http://www.google.com', **kwargs) +25 | urllib.request.URLopener().open('http://www.google.com') +26 | urllib.request.URLopener().open(f'http://www.google.com') + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +27 | urllib.request.URLopener().open('file:///foo/bar/baz') +28 | urllib.request.URLopener().open(url) + | + +S310.py:27:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +25 | urllib.request.URLopener().open('http://www.google.com') +26 | urllib.request.URLopener().open(f'http://www.google.com') +27 | urllib.request.URLopener().open('file:///foo/bar/baz') + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +28 | urllib.request.URLopener().open(url) | -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.py:28:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +26 | urllib.request.URLopener().open(f'http://www.google.com') +27 | urllib.request.URLopener().open('file:///foo/bar/baz') +28 | urllib.request.URLopener().open(url) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +29 | +30 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com')) + | + +S310.py:32:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +30 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com')) +31 | urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com')) +32 | 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')) +33 | urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com'), **kwargs) +34 | urllib.request.urlopen(urllib.request.Request('http://www.google.com')) | -S310.py:24:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. +S310.py:33:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +31 | urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com')) +32 | urllib.request.urlopen(url=urllib.request.Request('http://www.google.com'), **kwargs) +33 | urllib.request.urlopen(url=urllib.request.Request(f'http://www.google.com'), **kwargs) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 +34 | urllib.request.urlopen(urllib.request.Request('http://www.google.com')) +35 | urllib.request.urlopen(urllib.request.Request(f'http://www.google.com')) | -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.py:36:1: S310 Audit URL open for permitted schemes. Allowing use of `file:` or custom schemes is often unexpected. + | +34 | urllib.request.urlopen(urllib.request.Request('http://www.google.com')) +35 | urllib.request.urlopen(urllib.request.Request(f'http://www.google.com')) +36 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz')) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -25 | urllib.request.urlopen(urllib.request.Request(url)) +37 | 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. +S310.py:36: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')) +34 | urllib.request.urlopen(urllib.request.Request('http://www.google.com')) +35 | urllib.request.urlopen(urllib.request.Request(f'http://www.google.com')) +36 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz')) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 -25 | urllib.request.urlopen(urllib.request.Request(url)) +37 | 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. +S310.py:37: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)) +35 | urllib.request.urlopen(urllib.request.Request(f'http://www.google.com')) +36 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz')) +37 | 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. +S310.py:37: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)) +35 | urllib.request.urlopen(urllib.request.Request(f'http://www.google.com')) +36 | urllib.request.urlopen(urllib.request.Request('file:///foo/bar/baz')) +37 | urllib.request.urlopen(urllib.request.Request(url)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ S310 | From 2cbaf27e428d9653f60b335b91b3b8a8e4cf1fd9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 13 Jul 2024 16:51:46 -0400 Subject: [PATCH 2/3] Use matches --- .../rules/suspicious_function_call.rs | 65 +++++++++++-------- 1 file changed, 37 insertions(+), 28 deletions(-) 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 c70e165be4463..c6ece214df659 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 @@ -854,21 +854,24 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { ["six", "moves", "urllib", "request","Request"] => { // If the `url` argument is a string literal or an f string, 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) { + match call.arguments.find_argument("url", 0) { + // If the `url` argument is a string literal, allow `http` and `https` schemes. + Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => { let url = value.to_str().trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None; } - - } - - if let Some(Expr::FString(ast::ExprFString { value, .. })) = &call.arguments.find_argument("url", 0) { - if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { - let url = value.trim_start(); - if url.starts_with("http://") || url.starts_with("https://") { - return None; + }, + // If the `url` argument is an f-string literal, allow `http` and `https` schemes. + Some(Expr::FString(ast::ExprFString { value, .. })) => { + if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + let url = value.trim_start(); + if url.starts_with("http://") || url.starts_with("https://") { + return None; + } } - } + }, + _ => {} } } Some(SuspiciousURLOpenUsage.into()) @@ -877,46 +880,52 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { ["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) { + match 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 { + Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => { let url = value.to_str().trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None; } - } + }, - // If the `url` argument is an f string, allow `http` and `https` schemes. - if let Expr::FString(ast::ExprFString { value, .. }) = arg { + // If the `url` argument is an f-string literal, allow `http` and `https` schemes. + Some(Expr::FString(ast::ExprFString { value, .. })) => { if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { let url = value.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 { + Some(Expr::Call(ExprCall { func, arguments, .. })) => { 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) { + match arguments.find_argument("url", 0) { + // If the `url` argument is a string literal, allow `http` and `https` schemes. + Some(Expr::StringLiteral(ast::ExprStringLiteral { value, .. })) => { let url = value.to_str().trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None; } - - } - - if let Some(Expr::FString(ast::ExprFString { value, .. })) = arguments.find_argument("url", 0) { - if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { - let url = value.trim_start(); - if url.starts_with("http://") || url.starts_with("https://") { - return None; + }, + + // If the `url` argument is an f-string literal, allow `http` and `https` schemes. + Some(Expr::FString(ast::ExprFString { value, .. })) => { + if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + let url = value.trim_start(); + if url.starts_with("http://") || url.starts_with("https://") { + return None; + } } - } + }, + _ => {} } } - } + }, + + _ => {} } } Some(SuspiciousURLOpenUsage.into()) From 33bc91fb489fad4beb9cf6f77ead6d65650794cf Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 13 Jul 2024 16:52:56 -0400 Subject: [PATCH 3/3] Avoid unwrap --- .../rules/flake8_bandit/rules/suspicious_function_call.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 c6ece214df659..3221aaf4e3ab3 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 @@ -864,7 +864,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { }, // If the `url` argument is an f-string literal, allow `http` and `https` schemes. Some(Expr::FString(ast::ExprFString { value, .. })) => { - if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + if let Some(ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. })) = value.elements().next() { let url = value.trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None; @@ -891,7 +891,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { // If the `url` argument is an f-string literal, allow `http` and `https` schemes. Some(Expr::FString(ast::ExprFString { value, .. })) => { - if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + if let Some(ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. })) = value.elements().next() { let url = value.trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None; @@ -913,7 +913,7 @@ pub(crate) fn suspicious_function_call(checker: &mut Checker, call: &ExprCall) { // If the `url` argument is an f-string literal, allow `http` and `https` schemes. Some(Expr::FString(ast::ExprFString { value, .. })) => { - if let ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. }) = value.elements().next().unwrap() { + if let Some(ast::FStringElement::Literal(ast::FStringLiteralElement { value, .. })) = value.elements().next() { let url = value.trim_start(); if url.starts_with("http://") || url.starts_with("https://") { return None;