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

### Fixed

- **Custom rules are validated and the docs match the schema (#684, #678).** `cora config validate` and config load now reject an invalid regex, an empty or duplicate id, an id that shadows a built-in rule, an empty message and an unknown `languages` value, naming the rule. `languages` accepts language names (`"rust"`) as well as extensions (`"rs"`); `languages` and `exclude` are optional. `cora config validate --dry-run [--path DIR]` runs each custom rule and prints how many lines it matched. `docs/configuration.md` now shows the real schema (`rules_engine.custom`); the 0.19.0 entry no longer says `custom_rules`.
- The `Cora Review` workflow now fails with an error naming each empty secret (`CORA_API_KEY`, `CORA_BASE_URL`, `CORA_MODEL`) instead of running without them.
- The `Cora Review` job no longer gets cancelled on larger pull requests: its time limit is 30 minutes (the review action allows up to three LLM attempts of 10 minutes each).

Expand Down Expand Up @@ -66,7 +67,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- **The MCP `cora.review_diff` tool no longer crashes the server (#647).** Every call panicked with "Cannot start a runtime from within a runtime" because the handler blocked on a second runtime from a tokio runtime thread. Handlers now go through `block_in_place`.

- **`cora scan` now runs the rules engine (#645).** Built-in rules (`bug-todo`, `sec-sql-concat`, ...) and custom `rules_engine.custom_rules` were only run by `cora review`; scan silently skipped them. Expect more findings from `cora scan` after upgrading (e.g. `bug-todo` on every TODO), still under the `rules_engine.max_findings` cap (default 5); `rules_engine.enabled: false` turns them off. One set of deterministic families (`deterministic::run_files` for whole files, `deterministic::run` for diffs) now serves both, and the MCP `cora.check_snippet` tool picks up rules too. Scan also has a judge seam (`judge::ScanJudge`, with `HttpScanJudge` and `NoJudge`) so the batch loop is tested without a network, and `DEFAULT_BATCH_TOKEN_BUDGET` is renamed `DEFAULT_BATCH_CHAR_BUDGET` (it was always a character count). `docs/cli-reference.md` now states which review flags scan does not have.
- **`cora scan` now runs the rules engine (#645).** Built-in rules (`bug-todo`, `sec-sql-concat`, ...) and custom `rules_engine.custom` rules were only run by `cora review`; scan silently skipped them. Expect more findings from `cora scan` after upgrading (e.g. `bug-todo` on every TODO), still under the `rules_engine.max_findings` cap (default 5); `rules_engine.enabled: false` turns them off. One set of deterministic families (`deterministic::run_files` for whole files, `deterministic::run` for diffs) now serves both, and the MCP `cora.check_snippet` tool picks up rules too. Scan also has a judge seam (`judge::ScanJudge`, with `HttpScanJudge` and `NoJudge`) so the batch loop is tested without a network, and `DEFAULT_BATCH_TOKEN_BUDGET` is renamed `DEFAULT_BATCH_CHAR_BUDGET` (it was always a character count). `docs/cli-reference.md` now states which review flags scan does not have.

## [0.18.0] - 2026-10-09

Expand Down
42 changes: 27 additions & 15 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -437,25 +437,37 @@ profile: security-first

## Custom Rule Engine

Write your own regex-based rules in `.cora.yaml`:
Write your own regex-based rules under `rules_engine.custom` in `.cora.yaml`:

```yaml
rules:
- id: no-unwrap
pattern: "\\.unwrap\\(\\)"
severity: minor
message: "Avoid unwrap() in production code — use proper error handling"
languages: ["rust"]
exclude: ["tests/**"]

- id: no-console-log
pattern: "console\\.log\\("
severity: minor
message: "Remove console.log before merging"
languages: ["typescript", "javascript"]
rules_engine:
custom:
- id: no-unwrap
pattern: "\\.unwrap\\(\\)"
severity: minor
message: "Avoid unwrap() in production code — use proper error handling"
languages: ["rs"]
exclude: ["tests/**"]

- id: no-console-log
pattern: "console\\.log\\("
severity: minor
message: "Remove console.log before merging"
languages: ["ts", "js"]
```

Rules run during the deterministic pre-scan phase (no LLM needed).
| Field | Required | Description |
|-------|----------|-------------|
| `id` | yes | Unique rule id; must not collide with a built-in rule id (it is what `ignore.rules` and `cora-ignore` name) |
| `pattern` | yes | A regex matched against each line; an invalid regex is a config error |
| `severity` | yes | `critical`, `major`, `minor` or `info` |
| `message` | yes | The finding text |
| `languages` | no (default `["all"]`) | File extensions (`"rs"`, `"py"`, `"ts"`) or language names (`"rust"`, `"typescript"`); `"all"` or `"*"` matches every file. An unknown value is a config error |
| `exclude` | no (default none) | Glob patterns of paths to skip |

The top-level `rules:` key is a different thing: a list of free-text instructions for the LLM reviewer (strings), not rule objects.

Rules run during the deterministic pre-scan phase (no LLM needed). `cora config validate` rejects an invalid pattern, a duplicate id, an unknown language or an empty message with the rule id. `cora config validate --dry-run [--path DIR]` also runs every custom rule over the project and prints how many lines each one matched, so a rule that matches nothing is visible.

### Index Scanner Configuration

Expand Down
51 changes: 51 additions & 0 deletions src/commands/config_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,57 @@ pub fn execute_config_set(key: &str, value: &str, global: bool) -> Result<()> {
Ok(())
}

/// Execute `cora config validate --dry-run`: run every custom rule over the
/// project and print how many lines each one matched (#684). A rule that
/// matches nothing is reported, since that is how a wrong pattern shows up.
pub fn execute_config_dry_run(config_path: Option<&str>, path: Option<&str>) -> Result<i32> {
use crate::engine::rules;
use crate::engine::scan_input;

let config = loader::load_config(config_path, None, None, None, None, false)?;
let custom = &config.rules_config.custom_rules;
if custom.is_empty() {
println!("No custom rules configured (rules_engine.custom).");
return Ok(0);
}
let root = match path {
Some(p) => PathBuf::from(p),
None => std::env::current_dir()?,
};
let files = crate::engine::scanner::walk_project(&root, &[], &config.ignore.files, &[])
.map_err(|e| anyhow::anyhow!("{e}"))?;
let mut only_custom = config.rules_config.clone();
only_custom.include_tests = true;
only_custom.max_findings = 0;
let findings = rules::run_rules_in(&scan_input::from_entries(&files), &only_custom);
println!("Dry run over {} files in {}", files.len(), root.display());
for rule in custom {
let hits: Vec<_> = findings.iter().filter(|f| f.rule_id == rule.id).collect();
let n_files = hits
.iter()
.map(|f| f.file.as_str())
.collect::<std::collections::HashSet<_>>()
.len();
if hits.is_empty() {
println!(
"{} {}: 0 matches. Check the pattern and `languages` ({:?})",
"⚠️".yellow(),
rule.id,
rule.languages
);
} else {
println!(
"{} {}: {} lines in {} files",
"✅".green(),
rule.id,
hits.len(),
n_files
);
}
}
Ok(0)
}

/// Execute `cora config validate` — load config and report validity.
///
/// Returns exit code: 0 if valid, 2 if issues found.
Expand Down
5 changes: 5 additions & 0 deletions src/config/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,11 @@ impl Config {
));
}

// ── custom rules (#684) ──
errs.extend(crate::engine::rules::validate_custom_rules(
&self.rules_config.custom_rules,
));

// ── profile (#81) ──
if let Some(profile) = &self.profile {
if let Err(pe) = profile.validate() {
Expand Down
50 changes: 46 additions & 4 deletions src/engine/rules/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,52 @@ use super::types::CustomRule;

/// Check if a rule applies to the given file language.
pub fn matches_language(rule: &CustomRule, file_lang: &str) -> bool {
rule.languages.iter().any(|lang| {
let lang_lower = lang.to_lowercase();
lang_lower == "all" || lang_lower == file_lang
})
rule.languages
.iter()
.any(|lang| match normalize_language(lang) {
Some(n) => {
// A scan sees the raw extension (`tsx`), a diff the shorthand (`ts`).
n == "all"
|| n == file_lang
|| normalize_language(file_lang).as_deref() == Some(n.as_str())
}
// Unknown value: keep the old literal comparison (config validation reports it).
None => lang.to_lowercase() == file_lang,
})
}

/// Map a configured language (extension or name) to the shorthand used for
/// files (`"rust"` and `"rs"` -> `"rs"`). `"all"` / `"*"` -> `"all"`.
/// `None` when it names no known language.
pub fn normalize_language(lang: &str) -> Option<String> {
let l = lang.trim().trim_start_matches('.').to_lowercase();
if l == "all" || l == "*" {
return Some("all".to_string());
}
let by_name = match l.as_str() {
"rust" => "rs",
"python" => "py",
"typescript" => "ts",
"javascript" | "node" => "js",
"golang" => "go",
"kotlin" => "kt",
"ruby" => "rb",
"shell" | "bash" => "sh",
"markdown" => "md",
"csharp" | "c#" => "cs",
"c++" => "cpp",
"elixir" => "ex",
_ => "",
};
if !by_name.is_empty() {
return Some(by_name.to_string());
}
let detected = crate::engine::diff_parser::detect_language(&format!("x.{l}"));
if detected == "unknown" {
// Languages detect_language does not know but that are real extensions.
return matches!(l.as_str(), "cs" | "vue" | "tf" | "xml" | "ini" | "txt").then_some(l);
}
Some(detected.to_string())
}

/// Check if a file path should be excluded from a rule based on glob patterns.
Expand Down
104 changes: 103 additions & 1 deletion src/engine/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub mod types;
use tracing::debug;

use crate::engine::diff_parser::{FileChunk, parse_diff};
use crate::engine::rules::types::{RuleFinding, RulesConfig};
use crate::engine::rules::types::{CustomRule, RuleFinding, RulesConfig};
use crate::engine::scan_input::{self, ScanFile};
use crate::engine::types::{ReviewIssue, Severity};

Expand Down Expand Up @@ -45,6 +45,51 @@ const TEST_EXEMPT_RULES: &[&str] = &[
"sec-hardcoded-secret",
];

/// Lint custom rules: returns one message per problem (empty = fine).
///
/// Catches what would otherwise fail silently: a pattern that is not a valid
/// regex, an id that is empty, duplicated or shadows a built-in rule, an empty
/// message, and `languages` values that match no file (#684, #678).
pub fn validate_custom_rules(rules: &[CustomRule]) -> Vec<String> {
let builtin_ids: std::collections::HashSet<String> =
builtin::builtin_rules().into_iter().map(|r| r.id).collect();
let mut seen = std::collections::HashSet::new();
let mut errs = Vec::new();
for (i, r) in rules.iter().enumerate() {
let name = if r.id.is_empty() {
format!("rules_engine.custom[{i}]")
} else {
format!("rules_engine.custom[{i}] ({})", r.id)
};
if r.id.trim().is_empty() {
errs.push(format!("{name}: id must not be empty"));
} else if builtin_ids.contains(&r.id) {
errs.push(format!("{name}: id collides with a built-in rule"));
} else if !seen.insert(r.id.clone()) {
errs.push(format!("{name}: duplicate id"));
}
if let Err(e) = regex::Regex::new(&r.pattern) {
errs.push(format!("{name}: pattern is not a valid regex: {e}"));
}
if r.message.trim().is_empty() {
errs.push(format!("{name}: message must not be empty"));
}
if r.languages.is_empty() {
errs.push(format!(
"{name}: languages is empty, so the rule can never match; use [\"all\"] or file extensions like [\"rs\"]"
));
}
for lang in &r.languages {
if matching::normalize_language(lang).is_none() {
errs.push(format!(
"{name}: languages entry \"{lang}\" is not a known extension or language (use e.g. \"rs\", \"py\", \"ts\", or \"all\")"
));
}
}
}
errs
}

pub fn run_rules_in(files: &[ScanFile<'_>], config: &RulesConfig) -> Vec<RuleFinding> {
if !config.enabled {
debug!("rule engine is disabled");
Expand Down Expand Up @@ -317,6 +362,63 @@ mod tests {
use super::*;
use crate::engine::Severity;

fn rule(id: &str, pattern: &str, languages: &[&str]) -> CustomRule {
CustomRule {
id: id.into(),
pattern: pattern.into(),
severity: Severity::Major,
message: "m".into(),
languages: languages.iter().map(|s| s.to_string()).collect(),
exclude: vec![],
compiled_pattern: None,
}
}

#[test]
fn a_valid_custom_rule_has_no_problems() {
assert!(validate_custom_rules(&[rule("mine", "fn a", &["rs"])]).is_empty());
assert!(
validate_custom_rules(&[rule("mine", "x", &["rust", "TypeScript", "*"])]).is_empty()
);
}

#[test]
fn validation_reports_each_silent_failure_with_the_rule_id() {
let errs = validate_custom_rules(&[
rule("bad", "(", &["rs"]),
rule("lang", "x", &["klingon"]),
rule("lang", "x", &[]),
rule("bug-todo", "x", &["rs"]),
]);
let all = errs.join("\n");
assert!(
all.contains("(bad)") && all.contains("not a valid regex"),
"{all}"
);
assert!(all.contains("klingon"), "{all}");
assert!(all.contains("duplicate id"), "{all}");
assert!(all.contains("languages is empty"), "{all}");
assert!(all.contains("collides with a built-in rule"), "{all}");
}

#[test]
fn language_names_match_files_like_extensions() {
let r = rule("mine", "fn a", &["rust"]);
assert!(matching::matches_language(&r, "rs"));
assert!(!matching::matches_language(&r, "py"));
assert!(matching::matches_language(&rule("m", "x", &["tsx"]), "ts"));
}

#[test]
fn exclude_and_languages_are_optional_in_yaml() {
let r: CustomRule = serde_yaml_ng::from_str(
"id: no-console-log\npattern: 'console'\nseverity: minor\nmessage: no\n",
)
.unwrap();
assert!(r.exclude.is_empty());
assert_eq!(r.languages, vec!["all".to_string()]);
}

fn default_config() -> RulesConfig {
RulesConfig {
enabled: true,
Expand Down
11 changes: 9 additions & 2 deletions src/engine/rules/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,22 @@ pub struct CustomRule {
pub severity: Severity,
/// Human-readable description used as the finding body.
pub message: String,
/// Languages this rule applies to. `["all"]` means all languages.
/// Languages this rule applies to: file extensions (`"rs"`) or language
/// names (`"rust"`). `["all"]` (or `"*"`) means all languages; omitted = all.
#[serde(default = "default_languages")]
pub languages: Vec<String>,
/// Glob patterns for file paths to exclude from this rule.
/// Glob patterns for file paths to exclude from this rule (default: none).
#[serde(default)]
pub exclude: Vec<String>,
/// Pre-compiled regex for the pattern (populated after rule assembly).
#[serde(skip, default)]
pub compiled_pattern: Option<regex::Regex>,
}

fn default_languages() -> Vec<String> {
vec!["all".to_string()]
}

impl CustomRule {
/// Compile the rule's pattern into a cached regex, if not already compiled.
/// Returns `true` if compilation succeeded, `false` if the pattern is invalid.
Expand Down
22 changes: 19 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,15 @@ enum ConfigAction {
global: bool,
},
/// Validate the current configuration and report status
Validate,
Validate {
/// Also run each custom rule over the project and report how many
/// lines it matched (a rule with 0 matches is flagged)
#[clap(long)]
dry_run: bool,
/// Directory for `--dry-run` (default: current directory)
#[clap(long, requires = "dry_run")]
path: Option<String>,
},
}

#[derive(Subcommand, Debug)]
Expand Down Expand Up @@ -1331,8 +1339,16 @@ async fn main() -> Result<()> {
config_cmd::execute_config_set(&key, &value, global)?;
0
}
ConfigAction::Validate => {
config_cmd::execute_config_validate(cli.global.config.as_deref())?
ConfigAction::Validate { dry_run, path } => {
let code = config_cmd::execute_config_validate(cli.global.config.as_deref())?;
if dry_run && code == 0 {
config_cmd::execute_config_dry_run(
cli.global.config.as_deref(),
path.as_deref(),
)?
} else {
code
}
}
},
Command::Providers => {
Expand Down
Loading
Loading