Skip to content

Commit

Permalink
[refurb] Guard hashlib imports and mark hashlib-digest-hex fix …
Browse files Browse the repository at this point in the history
…as safe (`FURB181`) (#14694)

## Summary

- Check if `hashlib` and `crypt` imports have been seen for `FURB181`
and `S324`
- Mark the fix for `FURB181` as safe: I think it was accidentally marked
as unsafe in the first place. The rule does not support user-defined
classes as the "fix safety" section suggests.
- Removed `hashlib._Hash`, as it's not part of the `hashlib` module.

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

Updated the test snapshots
  • Loading branch information
sbrugman authored Dec 2, 2024
1 parent 289a938 commit 48ec3a8
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::helpers::is_const_false;
use ruff_python_ast::{self as ast, Arguments};
use ruff_python_semantic::Modules;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -64,6 +65,13 @@ impl Violation for HashlibInsecureHashFunction {

/// S324
pub(crate) fn hashlib_insecure_hash_functions(checker: &mut Checker, call: &ast::ExprCall) {
if !checker
.semantic()
.seen_module(Modules::HASHLIB | Modules::CRYPT)
{
return;
}

if let Some(weak_hash_call) = checker
.semantic()
.resolve_qualified_name(&call.func)
Expand Down
13 changes: 6 additions & 7 deletions crates/ruff_linter/src/rules/refurb/rules/hashlib_digest_hex.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{Expr, ExprAttribute, ExprCall};
use ruff_python_semantic::Modules;
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
Expand All @@ -27,11 +28,6 @@ use crate::checkers::ast::Checker;
/// hashed = sha512(b"some data").hexdigest()
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as the target of the `.digest()` call
/// could be a user-defined class that implements a `.hex()` method, rather
/// than a hashlib hash object.
///
/// ## References
/// - [Python documentation: `hashlib`](https://docs.python.org/3/library/hashlib.html)
#[derive(ViolationMetadata)]
Expand All @@ -52,6 +48,10 @@ impl Violation for HashlibDigestHex {

/// FURB181
pub(crate) fn hashlib_digest_hex(checker: &mut Checker, call: &ExprCall) {
if !checker.semantic().seen_module(Modules::HASHLIB) {
return;
}

if !call.arguments.is_empty() {
return;
}
Expand Down Expand Up @@ -105,14 +105,13 @@ pub(crate) fn hashlib_digest_hex(checker: &mut Checker, call: &ExprCall) {
| "sha3_512"
| "shake_128"
| "shake_256"
| "_Hash"
]
)
})
{
let mut diagnostic = Diagnostic::new(HashlibDigestHex, call.range());
if arguments.is_empty() {
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
".hexdigest".to_string(),
TextRange::new(value.end(), call.func.end()),
)));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
snapshot_kind: text
---
FURB181.py:19:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
Expand All @@ -13,7 +12,7 @@ FURB181.py:19:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
16 16 |
17 17 | # these will match
18 18 |
Expand All @@ -33,7 +32,7 @@ FURB181.py:20:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
17 17 | # these will match
18 18 |
19 19 | blake2b().digest().hex()
Expand All @@ -54,7 +53,7 @@ FURB181.py:21:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
18 18 |
19 19 | blake2b().digest().hex()
20 20 | blake2s().digest().hex()
Expand All @@ -75,7 +74,7 @@ FURB181.py:22:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
19 19 | blake2b().digest().hex()
20 20 | blake2s().digest().hex()
21 21 | md5().digest().hex()
Expand All @@ -96,7 +95,7 @@ FURB181.py:23:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
20 20 | blake2s().digest().hex()
21 21 | md5().digest().hex()
22 22 | sha1().digest().hex()
Expand All @@ -117,7 +116,7 @@ FURB181.py:24:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
21 21 | md5().digest().hex()
22 22 | sha1().digest().hex()
23 23 | sha224().digest().hex()
Expand All @@ -138,7 +137,7 @@ FURB181.py:25:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
22 22 | sha1().digest().hex()
23 23 | sha224().digest().hex()
24 24 | sha256().digest().hex()
Expand All @@ -159,7 +158,7 @@ FURB181.py:26:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
23 23 | sha224().digest().hex()
24 24 | sha256().digest().hex()
25 25 | sha384().digest().hex()
Expand All @@ -180,7 +179,7 @@ FURB181.py:27:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
24 24 | sha256().digest().hex()
25 25 | sha384().digest().hex()
26 26 | sha3_224().digest().hex()
Expand All @@ -201,7 +200,7 @@ FURB181.py:28:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
25 25 | sha384().digest().hex()
26 26 | sha3_224().digest().hex()
27 27 | sha3_256().digest().hex()
Expand All @@ -222,7 +221,7 @@ FURB181.py:29:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
26 26 | sha3_224().digest().hex()
27 27 | sha3_256().digest().hex()
28 28 | sha3_384().digest().hex()
Expand All @@ -243,7 +242,7 @@ FURB181.py:30:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
27 27 | sha3_256().digest().hex()
28 28 | sha3_384().digest().hex()
29 29 | sha3_512().digest().hex()
Expand Down Expand Up @@ -285,7 +284,7 @@ FURB181.py:34:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
31 31 | shake_128().digest(10).hex()
32 32 | shake_256().digest(10).hex()
33 33 |
Expand All @@ -306,7 +305,7 @@ FURB181.py:36:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
33 33 |
34 34 | hashlib.sha256().digest().hex()
35 35 |
Expand All @@ -327,7 +326,7 @@ FURB181.py:38:1: FURB181 [*] Use of hashlib's `.digest().hex()`
|
= help: Replace with `.hexdigest()`

Unsafe fix
Safe fix
35 35 |
36 36 | sha256(b"text").digest().hex()
37 37 |
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,8 @@ impl<'a> SemanticModel<'a> {
"typing_extensions" => self.seen.insert(Modules::TYPING_EXTENSIONS),
"attr" | "attrs" => self.seen.insert(Modules::ATTRS),
"airflow" => self.seen.insert(Modules::AIRFLOW),
"hashlib" => self.seen.insert(Modules::HASHLIB),
"crypt" => self.seen.insert(Modules::CRYPT),
_ => {}
}
}
Expand Down Expand Up @@ -2039,6 +2041,8 @@ bitflags! {
const ATTRS = 1 << 25;
const REGEX = 1 << 26;
const AIRFLOW = 1 << 27;
const HASHLIB = 1 << 28;
const CRYPT = 1 << 29;
}
}

Expand Down

0 comments on commit 48ec3a8

Please sign in to comment.