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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- **Rules are test-code aware (#663).** Whole test files (`*_test.*`, `*.test.ts`, `spec/`, `fixtures/`, ... not just `tests/`) and Rust `#[cfg(test)]` modules / `#[test]` functions (also a diff hunk inside `mod tests`) are exempt from `bug-unwrap`, `bug-expect`, `bug-println`, `qual-clone`, `qual-error-ignore`, `sec-hardcoded-url` and `sec-hardcoded-secret`; the secret scanners only run their high-confidence patterns there, so a real AWS key or token in a test is still reported (injection, `eval` and other security patterns still apply inside test blocks, and a test region is only trusted when our brace matching agrees with the code's own indentation: `/* { */` comments are ignored, and a region that never closes, or that contains an item starting at or left of its own indentation, is dropped instead of exempting what follows). An attribute that carries its item on the same line (`#[cfg(test)] fn helper() {}`) no longer exempts the NEXT line, and an attribute inside a block comment is not an attribute. `bug-println` is also skipped in `main.rs`, `src/bin/`, `examples/` and `benches/`. New setting `rules_engine.include_tests: true` turns the exemptions off. On `uteke-core` (Rust) the findings drop from 2400 to 235 and the 6 Critical ones disappear; on `uteke-cloud` the 3 Major URL findings are gone. New module `engine::test_context`.

- **`rules_engine.max_findings` now defaults to 50, and Critical/Major findings are never hidden.** The old default of 5 could hide serious findings (a module with 7 critical findings showed 5) and made agents and users misread a capped list as the full picture. The cap now limits only the noise: it keeps every Critical and Major finding and fills the remaining slots with the most severe Minor/Info ones, still reporting how many were not shown. On a real Rust codebase the unfiltered list was up to 2400 findings, 88% `bug-unwrap`, so unlimited is not the default. Set `max_findings: 0` (or `null`) for no cap.

- **Review is now prepare / judge / finalize with the LLM as the seam (#649).** `review::prepare` turns a diff into a `judge::Packet` (judge-visible text, assembled context, system prompt override, `valid_files`, diff chunks, deterministic findings); a `judge::Judge` returns findings (`HttpJudge` = today's LLM call, `NoJudge` = deterministic only); `review::review_with` merges, guards file paths, post-processes and decides `should_block`. Output of `cora review` is unchanged. The review pipeline now has an end-to-end test with a fake judge (no network), and the `llm` module no longer prints: spinners moved to `progress::Spinner` and `llm::run_review_prompts` / `llm::scan_files` take an events observer (the old `llm::review_diff` / `llm::review_diff_stream` are replaced by `run_review_prompts`).
Expand Down
11 changes: 11 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,19 @@ rules_engine:
|-------|------|---------|-------------|
| `enabled` | `bool` | `true` | Enable/disable the rule engine |
| `max_findings` | `int` | `50` | Max deterministic findings shown per run; Critical and Major are never hidden (`0` or `null` = unlimited). See below |
| `include_tests` | `bool` | `false` | Scan test code like any other code (see "Test code" below) |
| `index_skip_files` | `[string]` | *(see below)* | Glob patterns for files to skip during index scanning |

#### Test code

Tests legitimately `unwrap()`, `println!`, clone, use `http://` fixtures and fake credentials, so by default the rules treat test code differently:

- **What counts as test code:** whole files under test paths (`tests/`, `*_test.*`, `*.test.ts`, `spec/`, `__tests__/`, `fixtures/`, `examples/`), Rust `#[cfg(test)]` modules and `#[test]` functions (found by brace matching), and, in a diff, a hunk inside `mod tests`.
- **What is skipped there:** the built-in rules `bug-unwrap`, `bug-expect`, `bug-println`, `qual-clone`, `qual-error-ignore`, `sec-hardcoded-url` and `sec-hardcoded-secret`. The secret scanners only run their high-confidence patterns (AWS keys, GitHub tokens, private keys, ...) on test code, so a real credential committed in a test is still found.
- `bug-println` is also skipped in program entry points (`main.rs`, `src/bin/`, `examples/`, `benches/`). Other CLI modules keep the rule; use `ignore.rules` to silence it there.
- **Not exempt:** custom rules, `bug-todo`, and every other pattern. In particular, injection, `eval`, weak-crypto and similar security patterns still apply inside inline test blocks, so wrapping code in a test-looking block does not hide it. (Whole test *files* by path keep the older behavior: only high-confidence secrets are scanned.)
- Set `rules_engine.include_tests: true` to scan test code like any other code. Detection is conservative: when it cannot tell, the line stays scanned. A `#[cfg(test)]` region is only trusted when brace matching and indentation agree (block comments are skipped; a region that never closes, or has an item starting at or left of its own indentation, is ignored rather than extended over the code after it), and a diff that does not show the `mod tests` line is scanned normally.

#### What `max_findings` caps

`max_findings` caps the **deterministic** findings only (rule engine, secrets, security, and index scanners; the findings that carry a `rule_id`). LLM issues are never capped.
Expand Down
6 changes: 6 additions & 0 deletions src/config/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,10 @@ pub struct RulesSection {
pub custom: Vec<crate::engine::rules::types::CustomRule>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub index_skip_files: Vec<String>,
/// Scan test code like any other code (default `false`, see
/// `RulesConfig::include_tests`, #663).
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
pub include_tests: bool,
}

/// File bundling configuration section for `.cora.yaml`.
Expand Down Expand Up @@ -800,6 +804,7 @@ impl CoraFile {
if let Some(re) = &self.rules_engine {
config.rules_config.enabled = re.enabled;
config.rules_config.max_findings = re.max_findings;
config.rules_config.include_tests = re.include_tests;
if !re.custom.is_empty() {
config.rules_config.custom_rules = re.custom.clone();
}
Expand Down Expand Up @@ -1178,6 +1183,7 @@ provider: zai
max_findings: 10,
custom: Vec::new(),
index_skip_files: vec!["my-generated/**".to_string()],
include_tests: false,
}),
..Default::default()
};
Expand Down
24 changes: 20 additions & 4 deletions src/engine/deterministic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ pub fn skip_patterns(config: &Config) -> Vec<String> {
crate::index::skip_patterns_from_config(Some(config)).unwrap_or_default()
}

/// Switch test awareness off when `rules_engine.include_tests` is set (#663).
fn scan_files_for<'a>(
files: Vec<crate::engine::scan_input::ScanFile<'a>>,
config: &Config,
) -> Vec<crate::engine::scan_input::ScanFile<'a>> {
if config.rules_config.include_tests {
files
.into_iter()
.map(crate::engine::scan_input::ScanFile::ignoring_tests)
.collect()
} else {
files
}
}

/// Run every deterministic check on a parsed diff.
pub fn run(chunks: &[FileChunk], config: &Config, bridge: &IndexBridge) -> DeterministicReport {
// Scanners run uncapped: `rules_engine.max_findings` is enforced by
Expand All @@ -57,11 +72,12 @@ pub fn run(chunks: &[FileChunk], config: &Config, bridge: &IndexBridge) -> Deter
max_findings: 0,
..config.rules_config.clone()
};
let files = scan_files_for(crate::engine::scan_input::from_chunks(chunks), config);

DeterministicReport {
rules: rules::run_rules(chunks, &uncapped_rules),
secrets: secrets_scanner::scan_secrets(chunks, max),
security: security_scanner::scan_security(chunks, max),
rules: rules::run_rules_in(&files, &uncapped_rules),
secrets: secrets_scanner::scan_secrets_in(&files, max),
security: security_scanner::scan_security_in(&files, max),
index_unused: index_scanner::scan_unused_imports(bridge, chunks, max, &skip),
index_dead: index_scanner::scan_dead_code_in_review(bridge, chunks, max, &skip),
index_breaking: index_scanner::scan_breaking_changes(bridge, chunks, max, &skip),
Expand All @@ -78,7 +94,7 @@ pub fn run_files(
files: &[crate::engine::scanner::FileEntry],
config: &Config,
) -> DeterministicReport {
let scan_files = crate::engine::scan_input::from_entries(files);
let scan_files = scan_files_for(crate::engine::scan_input::from_entries(files), config);
let uncapped_rules = crate::engine::rules::types::RulesConfig {
max_findings: 0,
..config.rules_config.clone()
Expand Down
1 change: 1 addition & 0 deletions src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub mod secret_patterns;
pub mod secrets_scanner;
pub mod security_scanner;
pub mod static_analysis;
pub mod test_context;
pub mod types;

// Re-export commonly used types from other modules for convenience
Expand Down
12 changes: 12 additions & 0 deletions src/engine/rules/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ pub fn post_match_filter(rule_id: &str, line: &str) -> bool {
pub fn post_match_filter_for_path(rule_id: &str, line: &str, path: &str) -> bool {
match rule_id {
"sec-hardcoded-secret" | "crypto/hardcoded-secret" => is_false_positive_secret(line, path),
"bug-println" => is_cli_output_path(path),
"sec-hardcoded-url" => is_false_positive_url(line),
"config/cors-wildcard" => is_false_positive_cors(line),
"injection/sql-concat" | "sec-sql-concat" => is_false_positive_sql_concat(line),
Expand All @@ -577,6 +578,17 @@ pub fn post_match_filter_for_path(rule_id: &str, line: &str, path: &str) -> bool
}
}

/// `println!` is the intended output of a program's entry point: `main.rs`,
/// `src/bin/`, `examples/`, `benches/` (#663). Other files keep the rule; a
/// crate's own `commands/` modules can be silenced with `ignore.rules` or
/// `bug-println` in a profile.
fn is_cli_output_path(path: &str) -> bool {
let p = path.replace('\\', "/");
p.rsplit('/').next() == Some("main.rs")
|| p.split('/')
.any(|seg| matches!(seg, "bin" | "examples" | "benches"))
}

/// Check if an `http://` URL match is a false positive.
///
/// Suppresses: XML/SVG namespaces, Docker internal hostnames, comment/docstring lines,
Expand Down
134 changes: 134 additions & 0 deletions src/engine/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ pub fn run_rules(chunks: &[FileChunk], config: &RulesConfig) -> Vec<RuleFinding>

/// Run all rules (built-in + custom) against file lines. The one implementation
/// behind both the diff and the whole-file entry points.
/// Built-in rules that skip test code (files under test paths and inline Rust
/// test blocks) unless `rules_engine.include_tests` is set (#663): tests
/// legitimately unwrap, print, clone, use `http://` fixtures and fake secrets.
/// Real secrets in tests are still found by the high-confidence scanners.
const TEST_EXEMPT_RULES: &[&str] = &[
"bug-unwrap",
"bug-expect",
"bug-println",
"qual-clone",
"qual-error-ignore",
"sec-hardcoded-url",
"sec-hardcoded-secret",
];

pub fn run_rules_in(files: &[ScanFile<'_>], config: &RulesConfig) -> Vec<RuleFinding> {
if !config.enabled {
debug!("rule engine is disabled");
Expand Down Expand Up @@ -67,6 +81,14 @@ pub fn run_rules_in(files: &[ScanFile<'_>], config: &RulesConfig) -> Vec<RuleFin
continue;
}

// Test code is exempt from the noisy built-in rules (#663).
if !config.include_tests
&& file.in_test(line_no)
&& TEST_EXEMPT_RULES.contains(&rule.id.as_str())
{
continue;
}

// Check pattern match
if !matching::match_rule_against_line(rule, content) {
continue;
Expand Down Expand Up @@ -301,6 +323,7 @@ mod tests {
max_findings: 10,
custom_rules: Vec::new(),
index_skip_files: super::types::default_index_skip_files(),
include_tests: false,
}
}

Expand Down Expand Up @@ -700,4 +723,115 @@ mod tests {
let out = merge_rule_findings(vec![strong], vec![finding]);
assert_eq!(out[0].severity, Severity::Critical);
}

// ─── #663: test-code awareness ───

fn file_entry(path: &str, content: &str) -> crate::engine::scanner::FileEntry {
crate::engine::scanner::FileEntry {
path: path.to_string(),
lines: content.lines().count(),
content: content.to_string(),
}
}

fn rule_ids(entry: &crate::engine::scanner::FileEntry, cfg: &RulesConfig) -> Vec<String> {
let files = crate::engine::scan_input::from_entries(std::slice::from_ref(entry));
run_rules_in(&files, cfg)
.into_iter()
.map(|f| format!("{}@{}", f.rule_id, f.line))
.collect()
}

const RUST_WITH_TESTS: &str = "\
fn real() {
let a = x.unwrap();
}
#[cfg(test)]
mod tests {
#[test]
fn t() {
let b = y.unwrap();
println!(\"debug\");
let u = \"http://example.test/x\";
}
}
";

#[test]
fn inline_rust_test_blocks_skip_the_noisy_rules() {
let entry = file_entry("src/lib.rs", RUST_WITH_TESTS);
let ids = rule_ids(&entry, &default_config());
assert!(ids.contains(&"bug-unwrap@2".to_string()), "{ids:?}");
assert!(
!ids.iter()
.any(|i| i.ends_with("@8") || i.ends_with("@9") || i.ends_with("@10")),
"test block lines must be exempt: {ids:?}"
);
}

#[test]
fn include_tests_scans_test_blocks_again() {
let entry = file_entry("src/lib.rs", RUST_WITH_TESTS);
let cfg = RulesConfig {
include_tests: true,
..default_config()
};
let ids = rule_ids(&entry, &cfg);
assert!(ids.contains(&"bug-unwrap@8".to_string()), "{ids:?}");
assert!(ids.contains(&"bug-println@9".to_string()), "{ids:?}");
}

#[test]
fn test_paths_are_exempt_beyond_the_tests_directory() {
// `foo_test.rs` / `app.test.ts` are not under tests/, but are test files.
for path in ["src/foo_test.rs", "src/app.test.ts", "src/__tests__/a.rs"] {
let entry = file_entry(path, "fn a() { let x = y.unwrap(); }\n");
let ids = rule_ids(&entry, &default_config());
assert!(
!ids.iter().any(|i| i.starts_with("bug-unwrap")),
"{path}: {ids:?}"
);
}
let entry = file_entry("src/prod.rs", "fn a() { let x = y.unwrap(); }\n");
assert!(
rule_ids(&entry, &default_config())
.iter()
.any(|i| i.starts_with("bug-unwrap"))
);
}

#[test]
fn rules_that_are_not_noise_still_fire_in_tests() {
// bug-todo is not exempt: a TODO in a test is still a TODO.
let entry = file_entry("src/foo_test.rs", "// TODO: finish\n");
assert!(
rule_ids(&entry, &default_config())
.iter()
.any(|i| i.starts_with("bug-todo"))
);
}

#[test]
fn println_is_fine_in_program_entry_points_only() {
let src = "fn main() { println!(\"hi\"); }\n";
for ok in [
"src/main.rs",
"crates/x/src/bin/tool.rs",
"examples/demo.rs",
] {
let entry = file_entry(ok, src);
assert!(
!rule_ids(&entry, &default_config())
.iter()
.any(|i| i.starts_with("bug-println")),
"{ok}"
);
}
let entry = file_entry("src/lib.rs", "pub fn f() { println!(\"hi\"); }\n");
assert!(
rule_ids(&entry, &default_config())
.iter()
.any(|i| i.starts_with("bug-println"))
);
}
}
8 changes: 8 additions & 0 deletions src/engine/rules/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ pub struct RulesConfig {
/// Matches against the file path relative to the project root.
#[serde(default = "default_index_skip_files")]
pub index_skip_files: Vec<String>,
/// Scan test code like any other code. By default noisy rules
/// (`bug-unwrap`, `bug-expect`, `bug-println`, `qual-clone`,
/// `sec-hardcoded-url`, `sec-hardcoded-secret`) skip test files and inline
/// Rust test blocks, and the secret/security scanners only run their
/// high-confidence patterns on inline test blocks (#663).
#[serde(default)]
pub include_tests: bool,
}

/// Default skip patterns for index scanners — common false-positive sources.
Expand Down Expand Up @@ -67,6 +74,7 @@ impl Default for RulesConfig {
max_findings: 50,
custom_rules: Vec::new(),
index_skip_files: default_index_skip_files(),
include_tests: false,
}
}
}
Expand Down
Loading
Loading