diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eddbd5..fe2b879 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.6] - 2026-06-01 + +### Added + +- **Custom system prompts via config** — `review.system_prompt`, `review.system_prompt_file`, `scan.system_prompt`, `scan.system_prompt_file` fields in `.cora.yaml` (#94) +- **`response_format` config** — opt-in `json_object` response format for providers that support it, via `review.response_format: json_object` (#92) +- **File path injection into prompts** — valid diff file paths are injected into the review user prompt to reduce LLM hallucination (#93) +- **Post-parse file path filtering** — issues referencing non-existent files are filtered out after LLM response parsing (#93) +- **Enhanced default system prompts** — both review and scan prompts now include explicit anti-hallucination constraints, severity definitions, and format instructions (#95) + +### Fixed + +- **Path traversal in `system_prompt_file`** — arbitrary file read vulnerability. Now validates file path is within canonicalized project root (#92) +- **Symlink bypass in path traversal guard** — project root is now canonicalized to match resolved file paths + ## [0.1.5] - 2026-06-01 ### Fixed diff --git a/Cargo.toml b/Cargo.toml index 6f15839..c155e29 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cora-cli" -version = "0.1.5" +version = "0.1.6" edition = "2024" description = "CLI-first AI code review — BYOK, diff/scan/branch, pre-commit hooks" license = "MIT" diff --git a/README.md b/README.md index f720c06..2813355 100644 --- a/README.md +++ b/README.md @@ -41,10 +41,10 @@ Download the latest release from [GitHub Releases](https://github.com/ajianaz/co ```bash # Determine your platform tag from the releases page, e.g.: -# cora-aarch64-unknown-linux-gnu-v0.1.5.tar.gz -# cora-x86_64-unknown-linux-gnu-v0.1.5.tar.gz -# cora-aarch64-apple-darwin-v0.1.5.tar.gz -# cora-x86_64-pc-windows-msvc-v0.1.5.zip +# cora-aarch64-unknown-linux-gnu-v0.1.6.tar.gz +# cora-x86_64-unknown-linux-gnu-v0.1.6.tar.gz +# cora-aarch64-apple-darwin-v0.1.6.tar.gz +# cora-x86_64-pc-windows-msvc-v0.1.6.zip # Example: Linux aarch64 VERSION=$(curl -s https://api.github.com/repos/ajianaz/cora-cli/releases/latest | grep tag_name | cut -d'"' -f4) diff --git a/src/commands/scan.rs b/src/commands/scan.rs index 5aa4f74..95523c1 100644 --- a/src/commands/scan.rs +++ b/src/commands/scan.rs @@ -118,6 +118,8 @@ pub async fn execute_scan( &files_content, &effective_focus, &config.rules, + &config.response_format, + None, ) .await?; diff --git a/src/config/schema.rs b/src/config/schema.rs index a3e2db6..c2ec5da 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -18,6 +18,16 @@ pub struct Config { pub hook: HookConfig, /// Output configuration. pub output: OutputConfig, + /// Response format for LLM API calls ("none" or "json_object"). + pub response_format: String, + /// Optional custom system prompt that replaces the default for review. + pub review_system_prompt_override: Option, + /// Optional custom system prompt file path for review. + pub review_system_prompt_file: Option, + /// Optional custom system prompt that replaces the default for scan. + pub scan_system_prompt_override: Option, + /// Optional custom system prompt file path for scan. + pub scan_system_prompt_file: Option, } /// Provider configuration. @@ -83,6 +93,11 @@ impl Default for Config { format: "pretty".to_string(), color: true, }, + response_format: "none".to_string(), + review_system_prompt_override: None, + review_system_prompt_file: None, + scan_system_prompt_override: None, + scan_system_prompt_file: None, } } } @@ -109,6 +124,10 @@ pub struct CoraFile { pub hook: Option, #[serde(skip_serializing_if = "Option::is_none")] pub output: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub review: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub scan: Option, } #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -147,6 +166,26 @@ pub struct OutputSection { pub color: Option, } +/// Review-specific configuration section. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct ReviewSection { + #[serde(skip_serializing_if = "Option::is_none")] + pub response_format: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub system_prompt: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub system_prompt_file: Option, +} + +/// Scan-specific configuration section. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct ScanSection { + #[serde(skip_serializing_if = "Option::is_none")] + pub system_prompt: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub system_prompt_file: Option, +} + impl CoraFile { pub fn from_str(content: &str) -> Result { serde_yaml_ng::from_str(content).context("failed to parse .cora.yaml") @@ -198,6 +237,25 @@ impl CoraFile { config.output.color = v; } } + if let Some(r) = &self.review { + if let Some(v) = &r.response_format { + config.response_format = v.clone(); + } + if let Some(v) = &r.system_prompt { + config.review_system_prompt_override = Some(v.clone()); + } + if let Some(v) = &r.system_prompt_file { + config.review_system_prompt_file = Some(v.clone()); + } + } + if let Some(s) = &self.scan { + if let Some(v) = &s.system_prompt { + config.scan_system_prompt_override = Some(v.clone()); + } + if let Some(v) = &s.system_prompt_file { + config.scan_system_prompt_file = Some(v.clone()); + } + } } } @@ -468,4 +526,162 @@ output: assert_eq!(back.provider.provider, cfg.provider.provider); assert_eq!(back.output.format, cfg.output.format); } + + // ─── Config::default() new fields ─── + + #[test] + fn config_default_response_format_none() { + let cfg = Config::default(); + assert_eq!(cfg.response_format, "none"); + } + + #[test] + fn config_default_system_prompt_overrides_none() { + let cfg = Config::default(); + assert!(cfg.review_system_prompt_override.is_none()); + assert!(cfg.review_system_prompt_file.is_none()); + assert!(cfg.scan_system_prompt_override.is_none()); + assert!(cfg.scan_system_prompt_file.is_none()); + } + + // ─── ReviewSection parsing and merge ─── + + #[test] + fn parse_review_section_with_response_format() { + let yaml = r#" +review: + response_format: json_object +"#; + let cora = CoraFile::from_str(yaml).unwrap(); + assert_eq!( + cora.review.as_ref().unwrap().response_format.as_deref(), + Some("json_object") + ); + } + + #[test] + fn parse_review_section_with_system_prompt() { + let yaml = r#" +review: + system_prompt: | + You are a security-focused reviewer. + system_prompt_file: .cora/prompts/review.md +"#; + let cora = CoraFile::from_str(yaml).unwrap(); + assert_eq!( + cora.review.as_ref().unwrap().system_prompt.as_deref(), + Some("You are a security-focused reviewer.\n") + ); + assert_eq!( + cora.review.as_ref().unwrap().system_prompt_file.as_deref(), + Some(".cora/prompts/review.md") + ); + } + + #[test] + fn merge_review_response_format() { + let mut cfg = Config::default(); + let cora = CoraFile { + review: Some(ReviewSection { + response_format: Some("json_object".to_string()), + system_prompt: None, + system_prompt_file: None, + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!(cfg.response_format, "json_object"); + } + + #[test] + fn merge_review_system_prompt() { + let mut cfg = Config::default(); + let cora = CoraFile { + review: Some(ReviewSection { + response_format: None, + system_prompt: Some("Custom prompt here.".to_string()), + system_prompt_file: None, + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!( + cfg.review_system_prompt_override.as_deref(), + Some("Custom prompt here.") + ); + } + + #[test] + fn merge_review_system_prompt_file() { + let mut cfg = Config::default(); + let cora = CoraFile { + review: Some(ReviewSection { + response_format: None, + system_prompt: None, + system_prompt_file: Some("prompts/review.md".to_string()), + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!( + cfg.review_system_prompt_file.as_deref(), + Some("prompts/review.md") + ); + } + + // ─── ScanSection parsing and merge ─── + + #[test] + fn parse_scan_section_with_system_prompt() { + let yaml = r#" +scan: + system_prompt: | + You are a performance-focused scanner. +"#; + let cora = CoraFile::from_str(yaml).unwrap(); + assert_eq!( + cora.scan.as_ref().unwrap().system_prompt.as_deref(), + Some("You are a performance-focused scanner.\n") + ); + } + + #[test] + fn merge_scan_system_prompt() { + let mut cfg = Config::default(); + let cora = CoraFile { + scan: Some(ScanSection { + system_prompt: Some("Performance only.".to_string()), + system_prompt_file: None, + }), + ..Default::default() + }; + cora.merge_into(&mut cfg); + assert_eq!( + cfg.scan_system_prompt_override.as_deref(), + Some("Performance only.") + ); + } + + // ─── Full .cora.yaml with review and scan sections ─── + + #[test] + fn parse_cora_file_with_review_and_scan() { + let yaml = r#" +review: + response_format: json_object + system_prompt: | + Security only. +scan: + system_prompt: | + Performance only. + system_prompt_file: scan.md +"#; + let cora = CoraFile::from_str(yaml).unwrap(); + let review = cora.review.unwrap(); + assert_eq!(review.response_format.as_deref(), Some("json_object")); + assert_eq!(review.system_prompt.as_deref(), Some("Security only.\n")); + let scan = cora.scan.unwrap(); + assert_eq!(scan.system_prompt.as_deref(), Some("Performance only.\n")); + assert_eq!(scan.system_prompt_file.as_deref(), Some("scan.md")); + } } diff --git a/src/engine/llm.rs b/src/engine/llm.rs index a3443f6..82d9bba 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -44,41 +44,66 @@ struct Usage { } /// System prompt for code review. -const REVIEW_SYSTEM_PROMPT: &str = r#"You are an expert code reviewer. Your job is to analyze code diffs and identify issues. - -Focus on these categories: -- Security vulnerabilities (injections, auth issues, data exposure) -- Performance problems (inefficient algorithms, memory leaks, N+1 queries) -- Bugs (logic errors, off-by-one, null/undefined handling) -- Best practices (idiomatic code, error handling, naming) - -For each issue found, return a JSON array of objects with these fields: -- "file": string — the file path +const REVIEW_SYSTEM_PROMPT: &str = r#"You are an expert code reviewer providing actionable feedback on code diffs. + +CRITICAL CONSTRAINTS: +1. You MUST ONLY comment on files that appear in the diff. Do NOT invent or hallucinate file paths. +2. Each issue MUST have a clear, descriptive title (one brief sentence, max 100 chars). +3. If uncertain whether something is a real issue, omit it rather than guessing. + +SEVERITY LEVELS: +- "critical": Security vulnerabilities, crashes, data loss, breaking bugs +- "major": Bugs that affect functionality, significant problems +- "minor": Style issues, small nitpicks, minor improvements +- "info": Suggestions, optional enhancements + +FOCUS AREAS (in priority order): +1. Security vulnerabilities (SQL injection, XSS, auth issues, data exposure) +2. Bugs and logic errors (off-by-one, null handling, race conditions) +3. Performance problems (inefficient algorithms, memory leaks, N+1 queries) +4. Best practices (idiomatic code, error handling, naming) + +RESPONSE FORMAT: +Return a JSON array of objects with these fields: +- "file": string — the file path (MUST be from the diff) - "line": number or null — the approximate line number - "severity": "critical" | "major" | "minor" | "info" -- "issue_type": string — category (security, performance, bugs, best-practice, style) +- "issue_type": string — category (security, performance, bugs, best_practice, style, suggestion) - "title": string — short description (max 100 chars) - "body": string — detailed explanation - "suggested_fix": string or null — optional fix suggestion -If no issues are found, return an empty array: [] +If no issues are found, return: [] -Return ONLY the JSON array. No markdown, no explanation, just the JSON."#; +Return ONLY the JSON array. No markdown code fences, no explanation, no conversational text. +Start with [ and end with ]."#; /// System prompt for full project scanning. const SCAN_SYSTEM_PROMPT: &str = r#"You are an expert code reviewer performing a full project scan. Analyze the provided code files and identify issues. -Focus on these categories: -- Security vulnerabilities -- Performance problems -- Bugs and logic errors -- Best practices and code quality - -For each issue found, return a JSON array of objects with these fields: -- "file": string — the file path +CRITICAL CONSTRAINTS: +1. You MUST ONLY comment on files that were provided to you. Do NOT invent file paths. +2. Each issue MUST have a clear, descriptive title (one brief sentence, max 100 chars). +3. If uncertain whether something is a real issue, omit it rather than guessing. + +SEVERITY LEVELS: +- "critical": Security vulnerabilities, crashes, data loss, breaking bugs +- "major": Bugs that affect functionality, significant problems +- "minor": Style issues, small nitpicks, minor improvements +- "info": Suggestions, optional enhancements + +FOCUS AREAS (in priority order): +1. Security vulnerabilities (SQL injection, XSS, auth issues, data exposure) +2. Bugs and logic errors (off-by-one, null handling, race conditions) +3. Performance problems (inefficient algorithms, memory leaks, N+1 queries) +4. Best practices (idiomatic code, error handling, naming) + +RESPONSE FORMAT: +Return a JSON array of objects with these fields: +- "file": string — the file path (MUST be from the provided files) - "line": number or null — the approximate line number - "severity": "critical" | "major" | "minor" | "info" -- "issue_type": string — category +- "issue_type": string — category (security, performance, bugs, best_practice, style, suggestion) - "title": string — short description (max 100 chars) - "body": string — detailed explanation - "suggested_fix": string or null — optional fix suggestion @@ -88,7 +113,8 @@ Also include a "summary" string at the end after a "|||" separator: If no issues are found, return: []|||No issues found. -Return ONLY this format. No markdown."#; +Return ONLY this format. No markdown code fences, no conversational text. +Start the JSON array with [ and end with ]."#; /// Send a chat completion request to an OpenAI-compatible API. async fn chat_completion( @@ -96,6 +122,7 @@ async fn chat_completion( system_prompt: &str, user_message: &str, spinner: Option<&ProgressBar>, + response_format: &str, ) -> Result { let client = reqwest::Client::new(); @@ -122,7 +149,11 @@ async fn chat_completion( ], temperature: 0.2, max_tokens: 4096, - response_format: None, + response_format: if response_format == "json_object" { + Some(serde_json::json!({"type": "json_object"})) + } else { + None + }, }; debug!(model = %config.model, url = %url, "sending LLM request"); @@ -184,16 +215,21 @@ pub async fn review_diff( diff: &str, focus: &[String], rules: &[String], + response_format: &str, + system_prompt_override: Option<&str>, ) -> Result { let spinner = create_spinner("Reviewing diff…"); let user_prompt = build_review_prompt(diff, focus, rules); + let system_prompt = system_prompt_override.unwrap_or(REVIEW_SYSTEM_PROMPT); + let raw = chat_completion( llm_config, - REVIEW_SYSTEM_PROMPT, + system_prompt, &user_prompt, Some(&spinner), + response_format, ) .await?; @@ -220,9 +256,10 @@ pub async fn review_diff( ); let retry_raw = chat_completion( llm_config, - REVIEW_SYSTEM_PROMPT, + system_prompt, &strict_prompt, Some(&spinner), + response_format, ) .await?; let (issues, summary, tokens_used) = parse_review_response(&retry_raw)?; @@ -246,10 +283,15 @@ pub async fn review_diff_stream( diff: &str, focus: &[String], rules: &[String], + response_format: &str, + system_prompt_override: Option<&str>, ) -> Result { let user_prompt = build_review_prompt(diff, focus, rules); - let raw = chat_completion_stream(llm_config, REVIEW_SYSTEM_PROMPT, &user_prompt).await?; + let system_prompt = system_prompt_override.unwrap_or(REVIEW_SYSTEM_PROMPT); + + let raw = + chat_completion_stream(llm_config, system_prompt, &user_prompt, response_format).await?; let (issues, summary, tokens_used) = parse_review_response(&raw)?; @@ -271,6 +313,7 @@ async fn chat_completion_stream( config: &LLMConfig, system_prompt: &str, user_message: &str, + response_format: &str, ) -> Result { use futures_util::StreamExt; use std::io::Write; @@ -278,7 +321,7 @@ async fn chat_completion_stream( let client = reqwest::Client::new(); let url = format!("{}/chat/completions", config.base_url.trim_end_matches('/')); - let request_body = serde_json::json!({ + let mut request_body = serde_json::json!({ "model": config.model, "messages": [ { "role": "system", "content": system_prompt }, @@ -289,6 +332,10 @@ async fn chat_completion_stream( "stream": true }); + if response_format == "json_object" { + request_body["response_format"] = serde_json::json!({"type": "json_object"}); + } + debug!(model = %config.model, url = %url, "sending streaming LLM request"); let response = client @@ -394,9 +441,13 @@ pub async fn scan_files( files_content: &str, focus: &[String], rules: &[String], + response_format: &str, + system_prompt_override: Option<&str>, ) -> Result<(Vec, Option, Option)> { let spinner = create_spinner("Scanning files…"); + let system_prompt = system_prompt_override.unwrap_or(SCAN_SYSTEM_PROMPT); + let mut user_prompt = String::new(); if !focus.is_empty() { user_prompt.push_str(&format!("Focus areas: {}\n\n", focus.join(", "))); @@ -414,15 +465,74 @@ pub async fn scan_files( user_prompt.push_str("Files to review:\n\n"); user_prompt.push_str(files_content); - let raw = chat_completion(llm_config, SCAN_SYSTEM_PROMPT, &user_prompt, Some(&spinner)).await?; + let raw = chat_completion( + llm_config, + system_prompt, + &user_prompt, + Some(&spinner), + response_format, + ) + .await?; parse_scan_response(&raw) } +/// Extract file paths from a unified diff string. +/// Matches lines like `--- a/path/file.rs` and `+++ b/path/file.rs`. +pub(crate) fn extract_file_paths_from_diff(diff: &str) -> Vec { + let mut paths = std::collections::HashSet::new(); + for line in diff.lines() { + let trimmed = line.trim_start(); + // Match unified diff headers: `--- a/path` or `+++ b/path` + // Also handles `--- path` without a/ or b/ prefix (some diffs) + let (prefix, strip_ab) = if let Some(rest) = trimmed.strip_prefix("--- a/") { + (rest, true) + } else if let Some(rest) = trimmed.strip_prefix("+++ b/") { + (rest, true) + } else if let Some(rest) = trimmed.strip_prefix("--- ") { + (rest, false) + } else if let Some(rest) = trimmed.strip_prefix("+++ ") { + (rest, false) + } else { + continue; + }; + // Skip /dev/null (binary files, deletes) + if prefix.starts_with("/dev/null") { + continue; + } + let path = if strip_ab { + prefix.to_string() + } else { + // Strip a/ or b/ prefix if present + prefix + .strip_prefix("a/") + .or_else(|| prefix.strip_prefix("b/")) + .unwrap_or(prefix) + .to_string() + }; + // Strip trailing \t (git shows tabs for renamed files) + let path = path.split('\t').next().unwrap_or(&path); + if !path.is_empty() { + paths.insert(path.to_string()); + } + } + paths.into_iter().collect() +} + /// Build the user prompt for diff review. fn build_review_prompt(diff: &str, focus: &[String], rules: &[String]) -> String { let mut prompt = String::new(); + // Inject valid file paths to reduce hallucination + let file_paths = extract_file_paths_from_diff(diff); + if !file_paths.is_empty() { + prompt.push_str("Valid files in this diff:\n"); + for path in &file_paths { + prompt.push_str(&format!("- \"{}\"\n", path)); + } + prompt.push('\n'); + } + if !focus.is_empty() { prompt.push_str(&format!("Focus areas: {}\n\n", focus.join(", "))); } @@ -906,6 +1016,52 @@ mod tests { assert!(prompt.contains("no unwrap")); } + #[test] + fn build_prompt_contains_file_paths() { + let diff = "diff --git a/src/main.rs b/src/main.rs\n--- a/src/main.rs\n+++ b/src/main.rs\n@@ -1 +1 @@\n- old\n+ new"; + let prompt = build_review_prompt(diff, &[], &[]); + assert!(prompt.contains("Valid files in this diff:")); + assert!(prompt.contains("src/main.rs")); + } + + #[test] + fn build_prompt_no_file_paths_for_empty_diff() { + let prompt = build_review_prompt("no diff headers here", &[], &[]); + assert!(!prompt.contains("Valid files in this diff:")); + } + + // ─── extract_file_paths_from_diff ─── + + #[test] + fn extract_paths_single_file() { + let diff = "--- a/src/main.rs\n+++ b/src/main.rs\n@@ -1 +1 @@\n- old\n+ new"; + let paths = extract_file_paths_from_diff(diff); + assert_eq!(paths, vec!["src/main.rs"]); + } + + #[test] + fn extract_paths_multiple_files() { + let diff = "--- a/src/a.rs\n+++ b/src/a.rs\n--- a/src/b.rs\n+++ b/src/b.rs"; + let paths = extract_file_paths_from_diff(diff); + assert!(paths.contains(&"src/a.rs".to_string())); + assert!(paths.contains(&"src/b.rs".to_string())); + } + + #[test] + fn extract_paths_skips_dev_null() { + let diff = "--- /dev/null\n+++ b/src/new.rs\n--- a/src/old.rs\n+++ /dev/null"; + let paths = extract_file_paths_from_diff(diff); + assert!(paths.contains(&"src/new.rs".to_string())); + assert!(paths.contains(&"src/old.rs".to_string())); + } + + #[test] + fn extract_paths_deduplicates() { + let diff = "--- a/src/main.rs\n+++ b/src/main.rs\n--- a/src/main.rs\n+++ b/src/main.rs"; + let paths = extract_file_paths_from_diff(diff); + assert_eq!(paths.len(), 1); + } + // ─── repair_json_string ─── #[test] diff --git a/src/engine/review.rs b/src/engine/review.rs index 9b6368e..b159825 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -5,6 +5,52 @@ use crate::config::schema::Config; use crate::engine::llm; use crate::engine::types::{LLMConfig, ReviewIssue, ReviewResponse, ScanResponse}; +/// Load a custom system prompt from a file path. +/// Returns the file content, or None if the file doesn't exist, can't be read, +/// or is outside the project root (path traversal guard). +fn load_system_prompt_file(path: &str) -> Option { + let canonical = match std::fs::canonicalize(path) { + Ok(p) => p, + Err(_) => { + tracing::debug!(path = path, "system_prompt_file does not exist"); + return None; + } + }; + let project_root = std::env::current_dir().ok()?; + let project_root = std::fs::canonicalize(&project_root).ok()?; + + if !canonical.starts_with(&project_root) { + tracing::warn!( + path = path, + "system_prompt_file is outside project root, ignoring (potential path traversal)" + ); + return None; + } + + match std::fs::read_to_string(&canonical) { + Ok(content) => Some(content), + Err(e) => { + tracing::warn!( + path = path, + error = %e, + "failed to read system_prompt_file, using default prompt" + ); + None + } + } +} + +/// Resolve the effective system prompt: inline override > file override > None (use default). +pub fn resolve_system_prompt(inline: Option<&str>, file_path: Option<&str>) -> Option { + if let Some(prompt) = inline { + Some(prompt.to_string()) + } else if let Some(path) = file_path { + load_system_prompt_file(path) + } else { + None + } +} + /// Run a code review on the given diff string. /// /// Builds the prompt from the diff + config focus/rules, calls the LLM, @@ -53,12 +99,53 @@ async fn review_diff_inner( }); } + // Extract valid file paths for post-parse filtering + let valid_files = llm::extract_file_paths_from_diff(diff); + + // Resolve custom system prompt for review + let review_prompt = resolve_system_prompt( + config.review_system_prompt_override.as_deref(), + config.review_system_prompt_file.as_deref(), + ); + let mut response = if stream { - llm::review_diff_stream(llm_config, diff, &config.focus, &config.rules).await? + llm::review_diff_stream( + llm_config, + diff, + &config.focus, + &config.rules, + &config.response_format, + review_prompt.as_deref(), + ) + .await? } else { - llm::review_diff(llm_config, diff, &config.focus, &config.rules).await? + llm::review_diff( + llm_config, + diff, + &config.focus, + &config.rules, + &config.response_format, + review_prompt.as_deref(), + ) + .await? }; + // Filter out issues with invalid file paths (hallucination guard) + if !valid_files.is_empty() { + let before = response.issues.len(); + response + .issues + .retain(|issue| is_valid_file_path(&issue.file, &valid_files)); + let filtered = before - response.issues.len(); + if filtered > 0 { + debug!( + filtered, + remaining = response.issues.len(), + "filtered issues with invalid file paths" + ); + } + } + // Apply ignore rules: filter out issues matching ignored patterns response.issues = apply_ignore_rules(response.issues, &config.ignore.rules); @@ -109,8 +196,21 @@ pub async fn scan_project( }); } - let (issues, summary, tokens_used) = - llm::scan_files(llm_config, files_content, &config.focus, &config.rules).await?; + // Resolve custom system prompt for scan + let scan_prompt = resolve_system_prompt( + config.scan_system_prompt_override.as_deref(), + config.scan_system_prompt_file.as_deref(), + ); + + let (issues, summary, tokens_used) = llm::scan_files( + llm_config, + files_content, + &config.focus, + &config.rules, + &config.response_format, + scan_prompt.as_deref(), + ) + .await?; // Apply ignore rules let issues = apply_ignore_rules(issues, &config.ignore.rules); @@ -157,3 +257,52 @@ fn apply_ignore_rules(mut issues: Vec, ignore_rules: &[String]) -> issues } + +/// Check if a file path from an LLM issue matches any of the valid diff file paths. +/// Uses exact match only — the LLM should report paths exactly as they appear in the diff. +fn is_valid_file_path(issue_file: &str, valid_files: &[String]) -> bool { + valid_files.iter().any(|f| f == issue_file) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_prompt_inline_takes_priority() { + let result = resolve_system_prompt(Some("inline prompt"), Some("file.md")); + assert_eq!(result.as_deref(), Some("inline prompt")); + } + + #[test] + fn resolve_prompt_file_fallback() { + // Use a file within the project root so the path traversal guard allows it + let test_file = std::path::PathBuf::from(".cora-test-prompt.tmp"); + std::fs::write(&test_file, "file prompt content").unwrap(); + let result = resolve_system_prompt(None, Some(".cora-test-prompt.tmp")); + assert_eq!(result.as_deref(), Some("file prompt content")); + let _ = std::fs::remove_file(&test_file); + } + + #[test] + fn resolve_prompt_none_when_both_missing() { + let result = resolve_system_prompt(None, None); + assert!(result.is_none()); + } + + #[test] + fn resolve_prompt_none_when_file_missing() { + let result = resolve_system_prompt(None, Some("/nonexistent/prompt.md")); + assert!(result.is_none()); + } + + #[test] + fn reject_path_traversal_outside_project() { + // /etc/passwd exists but is outside project root — should be rejected + let result = resolve_system_prompt(None, Some("/etc/passwd")); + assert!( + result.is_none(), + "system_prompt_file outside project root should be rejected" + ); + } +}