Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 84 additions & 15 deletions src/engine/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ fn cache_dir() -> std::result::Result<PathBuf, CoraError> {
}

/// Compute SHA-256 hex digest of the diff content + config parameters.
/// Includes model and temperature so config changes invalidate the cache.
/// Includes model, provider, base_url, and temperature so config changes
/// invalidate the cache (e.g., switching providers with the same model name).
#[allow(clippy::format_collect)]
fn cache_key(diff: &str, model: &str, temperature: f32) -> String {
fn cache_key(diff: &str, model: &str, temperature: f32, provider: &str, base_url: &str) -> String {
let mut hasher = Sha256::new();
hasher.update(diff.as_bytes());
hasher.update(model.as_bytes());
hasher.update(provider.as_bytes());
hasher.update(base_url.as_bytes());
hasher.update(temperature.to_le_bytes());
let result = hasher.finalize();
result.iter().map(|b| format!("{b:02x}")).collect()
Expand All @@ -34,8 +37,10 @@ pub fn get_cached_review(
model: &str,
temperature: f32,
ttl: u64,
provider: &str,
base_url: &str,
) -> Option<ReviewResponse> {
let hash = cache_key(diff, model, temperature);
let hash = cache_key(diff, model, temperature, provider, base_url);
let dir = cache_dir().ok()?;
let path = dir.join(format!("{hash}.json"));

Expand Down Expand Up @@ -78,11 +83,13 @@ pub fn save_cached_review(
model: &str,
temperature: f32,
response: &ReviewResponse,
provider: &str,
base_url: &str,
) -> std::result::Result<(), CoraError> {
let dir = cache_dir()?;
std::fs::create_dir_all(&dir).map_err(CoraError::CacheIo)?;

let hash = cache_key(diff, model, temperature);
let hash = cache_key(diff, model, temperature, provider, base_url);
let path = dir.join(format!("{hash}.json"));

let now = SystemTime::now()
Expand Down Expand Up @@ -136,44 +143,106 @@ mod tests {

#[test]
fn cache_key_is_deterministic() {
let hash1 = cache_key("hello world", "gpt-4", 0.0);
let hash2 = cache_key("hello world", "gpt-4", 0.0);
let hash1 = cache_key(
"hello world",
"gpt-4",
0.0,
"openai",
"https://api.openai.com/v1",
);
let hash2 = cache_key(
"hello world",
"gpt-4",
0.0,
"openai",
"https://api.openai.com/v1",
);
assert_eq!(hash1, hash2);
assert_eq!(hash1.len(), 64); // SHA-256 hex = 64 chars
}

#[test]
fn cache_key_differs_for_different_inputs() {
let hash1 = cache_key("hello world", "gpt-4", 0.0);
let hash2 = cache_key("hello earth", "gpt-4", 0.0);
let hash1 = cache_key(
"hello world",
"gpt-4",
0.0,
"openai",
"https://api.openai.com/v1",
);
let hash2 = cache_key(
"hello earth",
"gpt-4",
0.0,
"openai",
"https://api.openai.com/v1",
);
assert_ne!(hash1, hash2);
}

#[test]
fn cache_key_includes_model_and_temperature() {
let h1 = cache_key("diff", "gpt-4", 0.0);
let h2 = cache_key("diff", "gpt-3.5", 0.0);
let h3 = cache_key("diff", "gpt-4", 0.7);
let h1 = cache_key("diff", "gpt-4", 0.0, "openai", "https://api.openai.com/v1");
let h2 = cache_key(
"diff",
"gpt-3.5",
0.0,
"openai",
"https://api.openai.com/v1",
);
let h3 = cache_key("diff", "gpt-4", 0.7, "openai", "https://api.openai.com/v1");
assert_ne!(h1, h2, "different models should differ");
assert_ne!(h1, h3, "different temperatures should differ");
}

#[test]
fn cache_key_len_is_64() {
let diff = "diff --git a/file.txt b/file.txt\n+ hello";
let hash = cache_key(diff, "model", 0.0);
let hash = cache_key(diff, "model", 0.0, "openai", "https://api.openai.com/v1");
assert_eq!(hash.len(), 64);
}

#[test]
fn cache_miss_on_different_diff() {
let diff1 = "diff --git a/a.txt b/a.txt\n+ hello";
let diff2 = "diff --git a/b.txt b/b.txt\n+ world";
let hash1 = cache_key(diff1, "model", 0.0);
let hash2 = cache_key(diff2, "model", 0.0);
let hash1 = cache_key(diff1, "model", 0.0, "openai", "https://api.openai.com/v1");
let hash2 = cache_key(diff2, "model", 0.0, "openai", "https://api.openai.com/v1");
assert_ne!(hash1, hash2);
}

#[test]
fn cache_key_differs_for_different_providers() {
let h1 = cache_key("diff", "gpt-4", 0.0, "openai", "https://api.openai.com/v1");
let h2 = cache_key(
"diff",
"gpt-4",
0.0,
"azure",
"https://my-azure.openai.azure.com",
);
assert_ne!(
h1, h2,
"different providers should produce different cache keys"
);
}

#[test]
fn cache_key_differs_for_different_base_urls() {
let h1 = cache_key("diff", "gpt-4", 0.0, "openai", "https://api.openai.com/v1");
let h2 = cache_key(
"diff",
"gpt-4",
0.0,
"openai",
"https://proxy.example.com/v1",
);
assert_ne!(
h1, h2,
"different base_urls should produce different cache keys"
);
}

#[test]
fn cached_review_serialization_roundtrip() {
let response = make_response();
Expand Down Expand Up @@ -246,7 +315,7 @@ mod tests {

// Manually set up a cache entry in the temp dir
let diff = "test diff content";
let hash = cache_key(diff, "model", 0.0);
let hash = cache_key(diff, "model", 0.0, "openai", "https://api.openai.com/v1");
let path = dir.join(format!("{hash}.json"));

let response = make_response();
Expand Down
4 changes: 4 additions & 0 deletions src/engine/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ async fn review_diff_inner(
&llm_config.model,
llm_config.temperature,
config.cache_ttl,
&llm_config.provider,
&llm_config.base_url,
) {
debug!("returning cached review response");
return Ok(cached);
Expand Down Expand Up @@ -453,6 +455,8 @@ async fn review_diff_inner(
&llm_config.model,
llm_config.temperature,
&response,
&llm_config.provider,
&llm_config.base_url,
) {
debug!("failed to save review to cache: {}", e);
}
Expand Down
34 changes: 32 additions & 2 deletions src/engine/rules/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ pub fn builtin_rules() -> Vec<CustomRule> {
/// Returns `true` to suppress a finding that the regex matched but should be ignored.
pub fn post_match_filter(rule_id: &str, line: &str) -> bool {
match rule_id {
"sec-hardcoded-secret" | "crypto/hardcoded-secret" => is_false_positive_secret(line),
"sec-hardcoded-url" => is_false_positive_url(line),
"crypto/hardcoded-secret" => is_false_positive_secret(line),
_ => false,
}
}
Expand Down Expand Up @@ -403,7 +403,37 @@ mod tests {
));
assert!(!post_match_filter(
"crypto/hardcoded-secret",
"const API_KEY = \"sk-abc123def456gh\""
"const API_KEY = \"***\""
));
}

// ─── sec-hardcoded-secret (builtin rule ID) false positive tests ───

#[test]
fn builtin_rule_id_secret_empty_string_is_false_positive() {
assert!(post_match_filter(
"sec-hardcoded-secret",
"let formAppSecret = $state('');"
));
assert!(post_match_filter(
"sec-hardcoded-secret",
"let password = '';"
));
}

#[test]
fn builtin_rule_id_secret_svelte_state_is_false_positive() {
assert!(post_match_filter(
"sec-hardcoded-secret",
"let formPassword = $state('default12345678');"
));
}

#[test]
fn builtin_rule_id_secret_actual_hardcoded_is_real_finding() {
assert!(!post_match_filter(
"sec-hardcoded-secret",
"let password = supersecret12345"
));
}
}
Loading