From 9b9d833680647376333db74dc7432de9d0dbc93d Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Mon, 25 Sep 2023 23:34:48 +0200 Subject: [PATCH 1/3] fix(rules): detect policy instances in S507 Applying the same fix as in `bandit` (https://github.com/PyCQA/bandit/pull/1064). `paramiko` supports passing both a class and a class instance for the policy in `set_missing_host_key_policy` (https://github.com/paramiko/paramiko/blob/8e389c77660c5cdae3069b478665427d23012853/paramiko/client.py#L171-L191). --- .../test/fixtures/flake8_bandit/S507.py | 1 + .../rules/ssh_no_host_key_verification.rs | 10 +++- ...s__flake8_bandit__tests__S507_S507.py.snap | 54 +++++++++++-------- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py index 9aeb24a1bdc6d..e5cfda95c6d36 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py @@ -12,6 +12,7 @@ # Errors ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) ssh_client.set_missing_host_key_policy(client.WarningPolicy) +ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) ssh_client.set_missing_host_key_policy(AutoAddPolicy) ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs index d653fdf6586e6..48a450b50c7c6 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs @@ -42,6 +42,14 @@ impl Violation for SSHNoHostKeyVerification { } } +fn extract_policy_argument(call: &ExprCall) -> Option<&Expr> { + return match call.arguments.find_argument("policy", 0) { + Some(Expr::Call(ExprCall { func, .. })) => Some(func.as_ref()), + Some(argument) => Some(argument), + _ => None, + }; +} + /// S507 pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCall) { let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else { @@ -52,7 +60,7 @@ pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCal return; } - let Some(policy_argument) = call.arguments.find_argument("policy", 0) else { + let Some(policy_argument) = extract_policy_argument(call) else { return; }; diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap index bd590f67eaad3..c63a3790517cd 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap @@ -7,7 +7,7 @@ S507.py:13:40: S507 Paramiko call with policy set to automatically trust the unk 13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) -15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) | S507.py:14:40: S507 Paramiko call with policy set to automatically trust the unknown host key @@ -16,47 +16,57 @@ S507.py:14:40: S507 Paramiko call with policy set to automatically trust the unk 13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) 14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 -15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) -16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) | S507.py:15:40: S507 Paramiko call with policy set to automatically trust the unknown host key | 13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) 14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) -15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) - | ^^^^^^^^^^^^^ S507 -16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) -17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) + | ^^^^^^^^^^^^^^^^^^^^ S507 +16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) | -S507.py:16:47: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:16:40: S507 Paramiko call with policy set to automatically trust the unknown host key | 14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) -15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) -16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) - | ^^^^^^^^^^^^^^^^^^^^ S507 -17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) -18 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) + | ^^^^^^^^^^^^^ S507 +17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) | S507.py:17:47: S507 Paramiko call with policy set to automatically trust the unknown host key | -15 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) -16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) -17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 -18 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) | S507.py:18:47: S507 Paramiko call with policy set to automatically trust the unknown host key | -16 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) -17 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) -18 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) + | ^^^^^^^^^^^^^^^^^^^^ S507 +19 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) + | + +S507.py:19:47: S507 Paramiko call with policy set to automatically trust the unknown host key + | +17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) | ^^^^^^^^^^^^^ S507 -19 | -20 | # Unrelated +20 | +21 | # Unrelated | From 98d74c0e585e40c42f6e040bd14ef4df289e9956 Mon Sep 17 00:00:00 2001 From: Mathieu Kniewallner Date: Mon, 25 Sep 2023 23:42:56 +0200 Subject: [PATCH 2/3] fix(rules): account for `paramiko.*` imports `AutoAddPolicy`, `WarningPolicy` and `SSHClient` are not only exposed in `paramiko.client`, but also in `paramiko` (https://github.com/paramiko/paramiko/blob/66117732de6de03914308f9a21b05b50a781d13c/paramiko/__init__.py#L121-L164). So we also have to account for that when resolving the import paths. --- .../test/fixtures/flake8_bandit/S507.py | 3 + .../rules/ssh_no_host_key_verification.rs | 6 +- ...s__flake8_bandit__tests__S507_S507.py.snap | 90 ++++++++++--------- 3 files changed, 58 insertions(+), 41 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py index e5cfda95c6d36..e5e94d183965f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S507.py @@ -1,7 +1,9 @@ +import paramiko from paramiko import client from paramiko.client import AutoAddPolicy, WarningPolicy ssh_client = client.SSHClient() +ssh_client_from_paramiko = paramiko.SSHClient() # OK ssh_client.set_missing_host_key_policy(policy=foo) @@ -17,6 +19,7 @@ ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +ssh_client_from_paramiko.set_missing_host_key_policy(paramiko.AutoAddPolicy) # Unrelated set_missing_host_key_policy(client.AutoAddPolicy) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs index 48a450b50c7c6..ac324cc6e5202 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs @@ -71,6 +71,7 @@ pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCal matches!( call_path.as_slice(), ["paramiko", "client", "AutoAddPolicy" | "WarningPolicy"] + | ["paramiko", "AutoAddPolicy" | "WarningPolicy"] ) }) { @@ -78,7 +79,10 @@ pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCal } if typing::resolve_assignment(value, checker.semantic()).is_some_and(|call_path| { - matches!(call_path.as_slice(), ["paramiko", "client", "SSHClient"]) + matches!( + call_path.as_slice(), + ["paramiko", "client", "SSHClient"] | ["paramiko", "SSHClient"] + ) }) { checker.diagnostics.push(Diagnostic::new( SSHNoHostKeyVerification, diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap index c63a3790517cd..863454b563f9b 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap @@ -1,72 +1,82 @@ --- source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs --- -S507.py:13:40: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:15:40: S507 Paramiko call with policy set to automatically trust the unknown host key | -12 | # Errors -13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) +14 | # Errors +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 -14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) -15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +16 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) +17 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) | -S507.py:14:40: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:16:40: S507 Paramiko call with policy set to automatically trust the unknown host key | -12 | # Errors -13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) -14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) +14 | # Errors +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) +16 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 -15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) -16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +17 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +18 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) | -S507.py:15:40: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:17:40: S507 Paramiko call with policy set to automatically trust the unknown host key | -13 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) -14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) -15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) +16 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) +17 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) | ^^^^^^^^^^^^^^^^^^^^ S507 -16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) -17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +18 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) | -S507.py:16:40: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:18:40: S507 Paramiko call with policy set to automatically trust the unknown host key | -14 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) -15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) -16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +16 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) +17 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +18 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) | ^^^^^^^^^^^^^ S507 -17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) -18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +20 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) | -S507.py:17:47: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:19:47: S507 Paramiko call with policy set to automatically trust the unknown host key | -15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) -16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) -17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +17 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) +18 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 -18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) -19 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +20 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +21 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) | -S507.py:18:47: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:20:47: S507 Paramiko call with policy set to automatically trust the unknown host key | -16 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) -17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) -18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +18 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +20 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) | ^^^^^^^^^^^^^^^^^^^^ S507 -19 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +21 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +22 | ssh_client_from_paramiko.set_missing_host_key_policy(paramiko.AutoAddPolicy) | -S507.py:19:47: S507 Paramiko call with policy set to automatically trust the unknown host key +S507.py:21:47: S507 Paramiko call with policy set to automatically trust the unknown host key | -17 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) -18 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) -19 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +19 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) +20 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +21 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) | ^^^^^^^^^^^^^ S507 -20 | -21 | # Unrelated +22 | ssh_client_from_paramiko.set_missing_host_key_policy(paramiko.AutoAddPolicy) + | + +S507.py:22:54: S507 Paramiko call with policy set to automatically trust the unknown host key + | +20 | ssh_client.set_missing_host_key_policy(policy=client.WarningPolicy) +21 | ssh_client.set_missing_host_key_policy(policy=WarningPolicy) +22 | ssh_client_from_paramiko.set_missing_host_key_policy(paramiko.AutoAddPolicy) + | ^^^^^^^^^^^^^^^^^^^^^^ S507 +23 | +24 | # Unrelated | From 27187e89b8a0610f15f9d5825e7121f107238aa4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 28 Sep 2023 17:23:16 -0400 Subject: [PATCH 3/3] Use map_callable --- .../rules/ssh_no_host_key_verification.rs | 14 ++++---------- ..._rules__flake8_bandit__tests__S507_S507.py.snap | 2 +- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs index ac324cc6e5202..ba7cdf6134095 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/ssh_no_host_key_verification.rs @@ -1,5 +1,6 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::map_callable; use ruff_python_ast::{Expr, ExprAttribute, ExprCall}; use ruff_python_semantic::analyze::typing; use ruff_text_size::Ranged; @@ -42,14 +43,6 @@ impl Violation for SSHNoHostKeyVerification { } } -fn extract_policy_argument(call: &ExprCall) -> Option<&Expr> { - return match call.arguments.find_argument("policy", 0) { - Some(Expr::Call(ExprCall { func, .. })) => Some(func.as_ref()), - Some(argument) => Some(argument), - _ => None, - }; -} - /// S507 pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCall) { let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else { @@ -60,13 +53,14 @@ pub(crate) fn ssh_no_host_key_verification(checker: &mut Checker, call: &ExprCal return; } - let Some(policy_argument) = extract_policy_argument(call) else { + let Some(policy_argument) = call.arguments.find_argument("policy", 0) else { return; }; + // Detect either, e.g., `paramiko.client.AutoAddPolicy` or `paramiko.client.AutoAddPolicy()`. if !checker .semantic() - .resolve_call_path(policy_argument) + .resolve_call_path(map_callable(policy_argument)) .is_some_and(|call_path| { matches!( call_path.as_slice(), diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap index 863454b563f9b..6ff5a45021d54 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S507_S507.py.snap @@ -25,7 +25,7 @@ S507.py:17:40: S507 Paramiko call with policy set to automatically trust the unk 15 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy) 16 | ssh_client.set_missing_host_key_policy(client.WarningPolicy) 17 | ssh_client.set_missing_host_key_policy(client.AutoAddPolicy()) - | ^^^^^^^^^^^^^^^^^^^^ S507 + | ^^^^^^^^^^^^^^^^^^^^^^ S507 18 | ssh_client.set_missing_host_key_policy(AutoAddPolicy) 19 | ssh_client.set_missing_host_key_policy(policy=client.AutoAddPolicy) |