Skip to content
Merged
39 changes: 39 additions & 0 deletions src/rules/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ pub fn shannon_entropy(s: &str) -> f32 {
entropy
}

/// Base regex pattern shared by all `*/no-hardcoded-secret` rules.
///
/// Each language rule file should use this constant (or
/// [`CSHARP_HARDCODED_SECRET_PATTERN`] for C#) instead of inlining its
/// own copy of the pattern.
pub const HARDCODED_SECRET_PATTERN: &str =
r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)";

/// Extended variant for C# that adds `connection_?string` /
/// `connectionstring` to the base keyword set.
pub const CSHARP_HARDCODED_SECRET_PATTERN: &str = r"(?i)(password|secret|api_?key|token|auth|credential|private_?key|connection_?string|connectionstring)";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant connectionstring alternative

connection_?string (with _? making the underscore optional) already matches connectionstring, so the explicit connectionstring alternative at the end is redundant. This was present in the original csharp.rs inline pattern too, but now that it's being lifted into a shared constant it's worth cleaning up.

Suggested change
pub const CSHARP_HARDCODED_SECRET_PATTERN: &str = r"(?i)(password|secret|api_?key|token|auth|credential|private_?key|connection_?string|connectionstring)";
pub const CSHARP_HARDCODED_SECRET_PATTERN: &str = r"(?i)(password|secret|api_?key|token|auth|credential|private_?key|connection_?string)";


/// Default minimum length for strings flagged as a hardcoded secret.
///
/// Historically this was hardcoded as `>= 4` across every language-specific
Expand Down Expand Up @@ -338,4 +350,31 @@ mod tests {
assert_eq!(confidence_for_hops(3), 0.6);
assert_eq!(confidence_for_hops(10), 0.6);
}

#[test]
fn csharp_pattern_is_superset_of_base() {
let base = regex::Regex::new(HARDCODED_SECRET_PATTERN).unwrap();
let extended = regex::Regex::new(CSHARP_HARDCODED_SECRET_PATTERN).unwrap();

// Every keyword the base pattern matches must also match the C# pattern.
for kw in &[
"password",
"secret",
"api_key",
"apikey",
"token",
"auth",
"credential",
"private_key",
"privatekey",
] {
assert!(base.is_match(kw), "base should match {kw}");
assert!(extended.is_match(kw), "csharp should match {kw}");
}
// C#-specific extras
for kw in &["connection_string", "connectionstring"] {
assert!(!base.is_match(kw), "base should NOT match {kw}");
assert!(extended.is_match(kw), "csharp should match {kw}");
}
}
}
9 changes: 4 additions & 5 deletions src/rules/csharp.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{is_secret_value_long_enough, make_finding, walk_tree};
use crate::rules::common::{
is_secret_value_long_enough, make_finding, walk_tree, CSHARP_HARDCODED_SECRET_PATTERN,
};
use crate::{Language, Severity};
use regex::Regex;

Expand Down Expand Up @@ -443,10 +445,7 @@ impl_rule! {
fn check(_self, source, tree) {

let mut findings = Vec::new();
let secret_pattern = Regex::new(
r"(?i)(password|secret|api_?key|apikey|token|credential|private_?key|connection_?string|connectionstring)",
)
.unwrap();
let secret_pattern = Regex::new(CSHARP_HARDCODED_SECRET_PATTERN).unwrap();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 C# rule silently gains auth keyword

The old inline C# pattern was password|secret|api_?key|apikey|token|credential|private_?key|connection_?string|connectionstringauth was intentionally absent. CSHARP_HARDCODED_SECRET_PATTERN inherits auth from the shared base, so this rule will now fire on C# variables like authToken, authKey, authHeader, etc. that it never flagged before. The PR description only mentions PHP gaining keywords; this C# expansion is undocumented. If it's intentional (makes sense given the normalisation goal), a brief note in the description or a changelog entry would help reviewers and users understand the wider diff in findings.


walk_tree(tree.root_node(), source, &mut |node, src| {
// variable_declarator: string password = "hardcoded"
Expand Down
4 changes: 2 additions & 2 deletions src/rules/go.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::impl_rule;
use crate::rules::common::AliasTable;
use crate::rules::common::{
get_source_line, is_secret_value_long_enough, make_finding, make_finding_from_offsets,
walk_tree,
walk_tree, HARDCODED_SECRET_PATTERN,
};
use crate::rules::go_taint::{
self, go_aliases_from_tree, go_taint_sources, NodeMatcher as GoNodeMatcher,
Expand Down Expand Up @@ -149,7 +149,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
3 changes: 2 additions & 1 deletion src/rules/java.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{
is_secret_value_long_enough, make_finding, make_finding_from_offsets, walk_tree,
HARDCODED_SECRET_PATTERN,
};
use crate::{Language, Severity};
use regex::Regex;
Expand Down Expand Up @@ -604,7 +605,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|apiKey|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
6 changes: 4 additions & 2 deletions src/rules/javascript.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{get_source_line, is_secret_value_long_enough, make_finding, walk_tree};
use crate::rules::common::{
get_source_line, is_secret_value_long_enough, make_finding, walk_tree, HARDCODED_SECRET_PATTERN,
};
use crate::rules::FileContext;
use crate::{Finding, Language, Severity};
use regex::Regex;
Expand Down Expand Up @@ -56,7 +58,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
3 changes: 2 additions & 1 deletion src/rules/kotlin.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{
is_secret_value_long_enough, make_finding, make_finding_from_offsets, walk_tree,
HARDCODED_SECRET_PATTERN,
};
use crate::{Finding, Language, Severity};
use regex::Regex;
Expand Down Expand Up @@ -521,7 +522,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|apiKey|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
19 changes: 15 additions & 4 deletions src/rules/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,14 @@ impl Rule for CargoLockPqCrypto {
continue;
};
if let Some(entry) = seed_map.get(neighbor_name) {
reached_seeds.entry(entry.name).or_insert(entry);
reached_seeds
.entry(entry.name)
.and_modify(|existing| {
if entry.confidence > existing.confidence {
*existing = entry;
}
})
.or_insert(entry);
} else {
queue.push_back(neighbor);
}
Expand All @@ -319,9 +326,13 @@ impl Rule for CargoLockPqCrypto {
}

// Pick the highest-confidence seed
let best = reached_seeds
.values()
.max_by(|a, b| a.confidence.total_cmp(&b.confidence))
let (_, best) = reached_seeds
.iter()
.max_by(|(k1, v1), (k2, v2)| {
v1.confidence
.total_cmp(&v2.confidence)
.then_with(|| k1.cmp(k2))
})
.unwrap();

// Find byte offset of this package entry.
Expand Down
6 changes: 4 additions & 2 deletions src/rules/php.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{is_secret_value_long_enough, make_finding, walk_tree};
use crate::rules::common::{
is_secret_value_long_enough, make_finding, walk_tree, HARDCODED_SECRET_PATTERN,
};
use crate::{Language, Severity};
use regex::Regex;

Expand Down Expand Up @@ -359,7 +361,7 @@ impl_rule! {
fn check(_self, source, tree) {

let mut findings = Vec::new();
let secret_pattern = Regex::new(r"(?i)(password|secret|api_?key|token)").unwrap();
let secret_pattern = Regex::new(HARDCODED_SECRET_PATTERN).unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
// Detect: $password = "hardcoded";
Expand Down
6 changes: 4 additions & 2 deletions src/rules/python.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{get_source_line, is_secret_value_long_enough, make_finding, walk_tree};
use crate::rules::common::{
get_source_line, is_secret_value_long_enough, make_finding, walk_tree, HARDCODED_SECRET_PATTERN,
};
use crate::rules::FileContext;
use crate::{Finding, Language, Severity};
use regex::Regex;
Expand Down Expand Up @@ -75,7 +77,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
6 changes: 4 additions & 2 deletions src/rules/ruby.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{is_secret_value_long_enough, make_finding, walk_tree};
use crate::rules::common::{
is_secret_value_long_enough, make_finding, walk_tree, HARDCODED_SECRET_PATTERN,
};
use crate::{Language, Severity};
use regex::Regex;

Expand Down Expand Up @@ -484,7 +486,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
6 changes: 4 additions & 2 deletions src/rules/rust_lang.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{is_secret_value_long_enough, make_finding, walk_tree};
use crate::rules::common::{
is_secret_value_long_enough, make_finding, walk_tree, HARDCODED_SECRET_PATTERN,
};
use crate::{Language, Severity};
use regex::Regex;

Expand Down Expand Up @@ -335,7 +337,7 @@ impl_rule! {

let mut findings = Vec::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down
3 changes: 2 additions & 1 deletion src/rules/swift.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::impl_rule;
use crate::rules::common::{
is_secret_value_long_enough, make_finding, make_finding_from_offsets, walk_tree,
HARDCODED_SECRET_PATTERN,
};
use crate::{Language, Severity};
use regex::Regex;
Expand All @@ -21,7 +22,7 @@ impl_rule! {
let mut findings = Vec::new();
let mut reported_lines = std::collections::HashSet::new();
let secret_pattern =
Regex::new(r"(?i)(password|secret|api_?key|token|auth|credential|private_?key)")
Regex::new(HARDCODED_SECRET_PATTERN)
.unwrap();

walk_tree(tree.root_node(), source, &mut |node, src| {
Expand Down