From dd15c691810dafa260248f120a5115bd560e07a7 Mon Sep 17 00:00:00 2001 From: Samuel Cormier-Iijima Date: Fri, 27 Jan 2023 18:52:55 -0500 Subject: [PATCH] [`flake8-bandit`] Add Rule S110 (try/except/pass) (#2197) --- README.md | 19 ++++++++ resources/test/fixtures/flake8_bandit/S110.py | 14 ++++++ ruff.schema.json | 8 ++++ src/checkers/ast.rs | 9 ++++ src/registry.rs | 1 + src/rules/flake8_bandit/mod.rs | 18 ++++++++ src/rules/flake8_bandit/rules/mod.rs | 2 + .../flake8_bandit/rules/try_except_pass.rs | 43 +++++++++++++++++++ src/rules/flake8_bandit/settings.rs | 12 ++++++ ...s__flake8_bandit__tests__S110_S110.py.snap | 25 +++++++++++ ...les__flake8_bandit__tests__S110_typed.snap | 35 +++++++++++++++ 11 files changed, 186 insertions(+) create mode 100644 resources/test/fixtures/flake8_bandit/S110.py create mode 100644 src/rules/flake8_bandit/rules/try_except_pass.rs create mode 100644 src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_S110.py.snap create mode 100644 src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_typed.snap diff --git a/README.md b/README.md index b99f8ec54654f..81f80c5ce13d2 100644 --- a/README.md +++ b/README.md @@ -826,6 +826,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/) on PyPI. | S106 | hardcoded-password-func-arg | Possible hardcoded password: "{}" | | | S107 | hardcoded-password-default | Possible hardcoded password: "{}" | | | S108 | hardcoded-temp-file | Probable insecure usage of temporary file or directory: "{}" | | +| S110 | try-except-pass | `try`-`except`-`pass` detected, consider logging the exception | | | S113 | request-without-timeout | Probable use of requests call with timeout set to `{value}` | | | S324 | hashlib-insecure-hash-function | Probable use of insecure hash functions in `hashlib`: "{}" | | | S501 | request-with-no-cert-validation | Probable use of `{string}` call with `verify=False` disabling SSL certificate checks | | @@ -2682,6 +2683,24 @@ suppress-none-returning = true ### `flake8-bandit` +#### [`check-typed-exception`](#check-typed-exception) + +Whether to disallow `try`-`except`-`pass` (`S110`) for specific exception types. By default, +`try`-`except`-`pass` is only disallowed for `Exception` and `BaseException`. + +**Default value**: `false` + +**Type**: `bool` + +**Example usage**: + +```toml +[tool.ruff.flake8-bandit] +check-typed-exception = true +``` + +--- + #### [`hardcoded-tmp-directory`](#hardcoded-tmp-directory) A list of directories to consider temporary. diff --git a/resources/test/fixtures/flake8_bandit/S110.py b/resources/test/fixtures/flake8_bandit/S110.py new file mode 100644 index 0000000000000..7f034956dd88c --- /dev/null +++ b/resources/test/fixtures/flake8_bandit/S110.py @@ -0,0 +1,14 @@ +try: + pass +except Exception: + pass + +try: + pass +except: + pass + +try: + pass +except ValueError: + pass diff --git a/ruff.schema.json b/ruff.schema.json index 48ffe864da116..6d3c91bcb1f2d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -578,6 +578,13 @@ "Flake8BanditOptions": { "type": "object", "properties": { + "check-typed-exception": { + "description": "Whether to disallow `try`-`except`-`pass` (`S110`) for specific exception types. By default, `try`-`except`-`pass` is only disallowed for `Exception` and `BaseException`.", + "type": [ + "boolean", + "null" + ] + }, "hardcoded-tmp-directory": { "description": "A list of directories to consider temporary.", "type": [ @@ -1753,6 +1760,7 @@ "S107", "S108", "S11", + "S110", "S113", "S3", "S32", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 285c39c2aa82b..a2891e5e5b1ba 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -3447,6 +3447,15 @@ where body, ); } + if self.settings.rules.enabled(&Rule::TryExceptPass) { + flake8_bandit::rules::try_except_pass( + self, + type_.as_deref(), + name.as_deref(), + body, + self.settings.flake8_bandit.check_typed_exception, + ); + } if self.settings.rules.enabled(&Rule::ReraiseNoCause) { tryceratops::rules::reraise_no_cause(self, body); } diff --git a/src/registry.rs b/src/registry.rs index ada0f1b02099a..d4b6dd1dad542 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -333,6 +333,7 @@ ruff_macros::define_rule_mapping!( S106 => violations::HardcodedPasswordFuncArg, S107 => violations::HardcodedPasswordDefault, S108 => violations::HardcodedTempFile, + S110 => rules::flake8_bandit::rules::TryExceptPass, S113 => violations::RequestWithoutTimeout, S324 => violations::HashlibInsecureHashFunction, S501 => violations::RequestWithNoCertValidation, diff --git a/src/rules/flake8_bandit/mod.rs b/src/rules/flake8_bandit/mod.rs index d3e86f3914f4d..b8aff46c41541 100644 --- a/src/rules/flake8_bandit/mod.rs +++ b/src/rules/flake8_bandit/mod.rs @@ -31,6 +31,7 @@ mod tests { #[test_case(Rule::SnmpWeakCryptography, Path::new("S509.py"); "S509")] #[test_case(Rule::LoggingConfigInsecureListen, Path::new("S612.py"); "S612")] #[test_case(Rule::Jinja2AutoescapeFalse, Path::new("S701.py"); "S701")] + #[test_case(Rule::TryExceptPass, Path::new("S110.py"); "S110")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy()); let diagnostics = test_path( @@ -55,6 +56,7 @@ mod tests { "/dev/shm".to_string(), "/foo".to_string(), ], + check_typed_exception: false, }, ..Settings::for_rule(Rule::HardcodedTempFile) }, @@ -62,4 +64,20 @@ mod tests { assert_yaml_snapshot!("S108_extend", diagnostics); Ok(()) } + + #[test] + fn check_typed_exception() -> Result<()> { + let diagnostics = test_path( + Path::new("./resources/test/fixtures/flake8_bandit/S110.py"), + &Settings { + flake8_bandit: super::settings::Settings { + check_typed_exception: true, + ..Default::default() + }, + ..Settings::for_rule(Rule::TryExceptPass) + }, + )?; + assert_yaml_snapshot!("S110_typed", diagnostics); + Ok(()) + } } diff --git a/src/rules/flake8_bandit/rules/mod.rs b/src/rules/flake8_bandit/rules/mod.rs index ea6d6544693e1..6baa4dd9ab562 100644 --- a/src/rules/flake8_bandit/rules/mod.rs +++ b/src/rules/flake8_bandit/rules/mod.rs @@ -17,6 +17,7 @@ pub use request_with_no_cert_validation::request_with_no_cert_validation; pub use request_without_timeout::request_without_timeout; pub use snmp_insecure_version::snmp_insecure_version; pub use snmp_weak_cryptography::snmp_weak_cryptography; +pub use try_except_pass::{try_except_pass, TryExceptPass}; pub use unsafe_yaml_load::unsafe_yaml_load; mod assert_used; @@ -34,4 +35,5 @@ mod request_with_no_cert_validation; mod request_without_timeout; mod snmp_insecure_version; mod snmp_weak_cryptography; +mod try_except_pass; mod unsafe_yaml_load; diff --git a/src/rules/flake8_bandit/rules/try_except_pass.rs b/src/rules/flake8_bandit/rules/try_except_pass.rs new file mode 100644 index 0000000000000..173d811974911 --- /dev/null +++ b/src/rules/flake8_bandit/rules/try_except_pass.rs @@ -0,0 +1,43 @@ +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, Stmt, StmtKind}; + +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::define_violation; +use crate::registry::Diagnostic; +use crate::violation::Violation; + +define_violation!( + pub struct TryExceptPass; +); +impl Violation for TryExceptPass { + #[derive_message_formats] + fn message(&self) -> String { + format!("`try`-`except`-`pass` detected, consider logging the exception") + } +} + +/// S110 +pub fn try_except_pass( + checker: &mut Checker, + type_: Option<&Expr>, + _name: Option<&str>, + body: &[Stmt], + check_typed_exception: bool, +) { + if body.len() == 1 + && body[0].node == StmtKind::Pass + && (check_typed_exception + || type_.map_or(true, |type_| { + checker.resolve_call_path(type_).map_or(true, |call_path| { + call_path.as_slice() == ["", "Exception"] + || call_path.as_slice() == ["", "BaseException"] + }) + })) + { + checker.diagnostics.push(Diagnostic::new( + TryExceptPass, + Range::from_located(&body[0]), + )); + } +} diff --git a/src/rules/flake8_bandit/settings.rs b/src/rules/flake8_bandit/settings.rs index eb0a76d7794fd..43242b6d7160a 100644 --- a/src/rules/flake8_bandit/settings.rs +++ b/src/rules/flake8_bandit/settings.rs @@ -34,11 +34,20 @@ pub struct Options { /// A list of directories to consider temporary, in addition to those /// specified by `hardcoded-tmp-directory`. pub hardcoded_tmp_directory_extend: Option>, + #[option( + default = "false", + value_type = "bool", + example = "check-typed-exception = true" + )] + /// Whether to disallow `try`-`except`-`pass` (`S110`) for specific exception types. By default, + /// `try`-`except`-`pass` is only disallowed for `Exception` and `BaseException`. + pub check_typed_exception: Option, } #[derive(Debug, Hash)] pub struct Settings { pub hardcoded_tmp_directory: Vec, + pub check_typed_exception: bool, } impl From for Settings { @@ -55,6 +64,7 @@ impl From for Settings { .into_iter(), ) .collect(), + check_typed_exception: options.check_typed_exception.unwrap_or(false), } } } @@ -64,6 +74,7 @@ impl From for Options { Self { hardcoded_tmp_directory: Some(settings.hardcoded_tmp_directory), hardcoded_tmp_directory_extend: None, + check_typed_exception: Some(settings.check_typed_exception), } } } @@ -72,6 +83,7 @@ impl Default for Settings { fn default() -> Self { Self { hardcoded_tmp_directory: default_tmp_dirs(), + check_typed_exception: false, } } } diff --git a/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_S110.py.snap b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_S110.py.snap new file mode 100644 index 0000000000000..4c601ec6c4d38 --- /dev/null +++ b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_S110.py.snap @@ -0,0 +1,25 @@ +--- +source: src/rules/flake8_bandit/mod.rs +expression: diagnostics +--- +- kind: + TryExceptPass: ~ + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 8 + fix: ~ + parent: ~ +- kind: + TryExceptPass: ~ + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 8 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_typed.snap b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_typed.snap new file mode 100644 index 0000000000000..2ae7e68a95ed3 --- /dev/null +++ b/src/rules/flake8_bandit/snapshots/ruff__rules__flake8_bandit__tests__S110_typed.snap @@ -0,0 +1,35 @@ +--- +source: src/rules/flake8_bandit/mod.rs +expression: diagnostics +--- +- kind: + TryExceptPass: ~ + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 8 + fix: ~ + parent: ~ +- kind: + TryExceptPass: ~ + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 8 + fix: ~ + parent: ~ +- kind: + TryExceptPass: ~ + location: + row: 14 + column: 4 + end_location: + row: 14 + column: 8 + fix: ~ + parent: ~ +