From a47f51161543f71983e1b5782872e33f9cde70fc Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:29:36 +0700 Subject: [PATCH 01/12] feat: enhance default system prompts with anti-hallucination constraints - Add CRITICAL CONSTRAINTS section to prevent file path hallucination - Add explicit severity level definitions with examples - Add 'suggestion' as valid issue_type - Emphasize 'no markdown code fences' in both prompts - Reference: old cora TS repo anti-hallucination patterns Closes #95 --- src/engine/llm.rs | 72 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 23 deletions(-) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index a3443f6..5ab1f13 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( From cb1303fbe5344d2ef180d69f93a2f8e0951579e1 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:31:28 +0700 Subject: [PATCH 02/12] feat: inject valid file paths into prompt + post-parse filtering - Add extract_file_paths_from_diff() to parse diff headers - Inject 'Valid files in this diff:' list into review user prompt - Add post-parse filtering of issues with invalid file paths in review.rs - Add is_valid_file_path() with exact + partial match heuristic - Add 7 new tests (file path extraction, prompt injection, dedup, /dev/null) Anti-hallucination: LLM can now see exactly which files are valid, and issues referencing non-existent files are filtered out post-parse. Closes #93 --- src/engine/llm.rs | 85 ++++++++++++++++++++++++++++++++++++++++++++ src/engine/review.rs | 33 +++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 5ab1f13..e1fe668 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -445,10 +445,49 @@ pub async fn scan_files( 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 `--- a/path` or `+++ b/path` or `--- /dev/null` (skip null) + for prefix in &["--- a/", "+++ b/", "--- ", "+++ "] { + if let Some(rest) = trimmed.strip_prefix(prefix) { + // Skip /dev/null (binary files, deletes) + if rest.starts_with("/dev/null") || rest.starts_with("a/") || rest.starts_with("b/") { + continue; + } + // Strip leading a/ or b/ if present after the prefix + let path = rest + .strip_prefix("a/") + .or_else(|| rest.strip_prefix("b/")) + .unwrap_or(rest); + // 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(", "))); } @@ -932,6 +971,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..afa990b 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -53,12 +53,31 @@ async fn review_diff_inner( }); } + // Extract valid file paths for post-parse filtering + let valid_files = llm::extract_file_paths_from_diff(diff); + let mut response = if stream { llm::review_diff_stream(llm_config, diff, &config.focus, &config.rules).await? } else { llm::review_diff(llm_config, diff, &config.focus, &config.rules).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); @@ -157,3 +176,17 @@ 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 or "file contains" heuristic. +fn is_valid_file_path(issue_file: &str, valid_files: &[String]) -> bool { + // Exact match + if valid_files.iter().any(|f| f == issue_file) { + return true; + } + // The issue file might be a partial path — check if any valid file ends with it + if valid_files.iter().any(|f| f.ends_with(issue_file) || issue_file.ends_with(f)) { + return true; + } + false +} From 502ecae9e0fffb5fc85b0b6a3e93f1fe7e5cc391 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:32:40 +0700 Subject: [PATCH 03/12] fix: tighten file path extraction and validation - Refactor extract_file_paths_from_diff: early return pattern, no loop over prefixes (fixes overly broad matching of '--- ' prefix) - Tighten is_valid_file_path: require '.' in suffix match to avoid matching single-char paths like 'a' or 'b' - Addresses cora review findings (major x2) --- src/engine/llm.rs | 49 ++++++++++++++++++++++++++++---------------- src/engine/review.rs | 12 +++++------ 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index e1fe668..68100df 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -451,24 +451,37 @@ 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 `--- a/path` or `+++ b/path` or `--- /dev/null` (skip null) - for prefix in &["--- a/", "+++ b/", "--- ", "+++ "] { - if let Some(rest) = trimmed.strip_prefix(prefix) { - // Skip /dev/null (binary files, deletes) - if rest.starts_with("/dev/null") || rest.starts_with("a/") || rest.starts_with("b/") { - continue; - } - // Strip leading a/ or b/ if present after the prefix - let path = rest - .strip_prefix("a/") - .or_else(|| rest.strip_prefix("b/")) - .unwrap_or(rest); - // 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()); - } - } + // 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() diff --git a/src/engine/review.rs b/src/engine/review.rs index afa990b..bec3fa3 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -178,15 +178,15 @@ fn apply_ignore_rules(mut issues: Vec, ignore_rules: &[String]) -> } /// Check if a file path from an LLM issue matches any of the valid diff file paths. -/// Uses exact match or "file contains" heuristic. +/// Uses exact match or suffix match (e.g., issue reports "main.rs" and valid is "src/main.rs"). fn is_valid_file_path(issue_file: &str, valid_files: &[String]) -> bool { // Exact match if valid_files.iter().any(|f| f == issue_file) { return true; } - // The issue file might be a partial path — check if any valid file ends with it - if valid_files.iter().any(|f| f.ends_with(issue_file) || issue_file.ends_with(f)) { - return true; - } - false + // The issue file might be a shortened path — check suffix match + // e.g., LLM reports "main.rs" but valid file is "src/main.rs" + valid_files + .iter() + .any(|f| f.ends_with(issue_file) && issue_file.contains('.')) } From d557e8ee6ccff8a2bc7c2d20ee281af4731ab412 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:33:20 +0700 Subject: [PATCH 04/12] fix: use basename match instead of suffix match for file path validation - Extract basename from issue file path and compare against valid file basenames - Prevents false matches like '.rs' matching 'main.rs' --- src/engine/review.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/engine/review.rs b/src/engine/review.rs index bec3fa3..e48af80 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -178,15 +178,22 @@ fn apply_ignore_rules(mut issues: Vec, ignore_rules: &[String]) -> } /// Check if a file path from an LLM issue matches any of the valid diff file paths. -/// Uses exact match or suffix match (e.g., issue reports "main.rs" and valid is "src/main.rs"). +/// Uses exact match or filename suffix match (e.g., issue reports "main.rs" and valid is "src/main.rs"). fn is_valid_file_path(issue_file: &str, valid_files: &[String]) -> bool { // Exact match if valid_files.iter().any(|f| f == issue_file) { return true; } - // The issue file might be a shortened path — check suffix match + // Filename-based suffix match: extract basename from issue file and check + // if any valid file ends with that basename (with path separator boundary) // e.g., LLM reports "main.rs" but valid file is "src/main.rs" - valid_files - .iter() - .any(|f| f.ends_with(issue_file) && issue_file.contains('.')) + if let Some(basename) = issue_file.rsplit('/').next() { + if basename.contains('.') && basename == issue_file { + // Issue file has no directory — check if any valid file has this basename + return valid_files + .iter() + .any(|f| f.rsplit('/').next() == Some(basename)); + } + } + false } From 0967b7996e47f5db5ec6b87d088909c36251837e Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:33:54 +0700 Subject: [PATCH 05/12] fix: use exact-only match for file path validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify to exact match — fuzzy matching introduces false positives. The prompt now includes 'Valid files in this diff:' list so the LLM should use those exact paths. --- src/engine/review.rs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/engine/review.rs b/src/engine/review.rs index e48af80..55dfdfa 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -178,22 +178,7 @@ fn apply_ignore_rules(mut issues: Vec, ignore_rules: &[String]) -> } /// Check if a file path from an LLM issue matches any of the valid diff file paths. -/// Uses exact match or filename suffix match (e.g., issue reports "main.rs" and valid is "src/main.rs"). +/// 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 { - // Exact match - if valid_files.iter().any(|f| f == issue_file) { - return true; - } - // Filename-based suffix match: extract basename from issue file and check - // if any valid file ends with that basename (with path separator boundary) - // e.g., LLM reports "main.rs" but valid file is "src/main.rs" - if let Some(basename) = issue_file.rsplit('/').next() { - if basename.contains('.') && basename == issue_file { - // Issue file has no directory — check if any valid file has this basename - return valid_files - .iter() - .any(|f| f.rsplit('/').next() == Some(basename)); - } - } - false + valid_files.iter().any(|f| f == issue_file) } From 2c0b0c714a59b55fcf44e856e1a060afd8f4995b Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:37:30 +0700 Subject: [PATCH 06/12] feat: add response_format and custom system prompt config support - Add ReviewSection and ScanSection to CoraFile schema - Add response_format, review/scan system_prompt fields to Config - Wire response_format to LLM API calls (chat_completion and streaming) - Support custom system_prompt and system_prompt_file for review and scan - Update all callers in review.rs, scan.rs, and llm.rs Refs #92 #94 --- src/commands/scan.rs | 2 ++ src/config/schema.rs | 58 +++++++++++++++++++++++++++++++++++ src/engine/llm.rs | 36 ++++++++++++++++++---- src/engine/review.rs | 72 ++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 159 insertions(+), 9 deletions(-) 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..1739a15 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()); + } + } } } diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 68100df..32eea50 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -122,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(); @@ -148,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"); @@ -210,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?; @@ -246,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)?; @@ -272,10 +283,14 @@ 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)?; @@ -297,6 +312,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; @@ -304,7 +320,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 }, @@ -315,6 +331,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 @@ -420,9 +440,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(", "))); @@ -440,7 +464,7 @@ 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) } diff --git a/src/engine/review.rs b/src/engine/review.rs index 55dfdfa..949ef31 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -5,6 +5,36 @@ 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 or can't be read. +fn load_system_prompt_file(path: &str) -> Option { + match std::fs::read_to_string(path) { + 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, @@ -56,10 +86,32 @@ 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) @@ -128,8 +180,22 @@ pub async fn scan_project( }); } + // 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).await?; + 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); From d48b6ce3ef5aa3b5ccf7ae255d3f61c7b81619c4 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:38:46 +0700 Subject: [PATCH 07/12] test: add tests for response_format and custom system prompt features - Config defaults for new fields (response_format, prompt overrides) - ReviewSection YAML parsing and merge (response_format, system_prompt, file) - ScanSection YAML parsing and merge - Full config file with both review and scan sections - resolve_system_prompt priority logic (inline > file > none) - system_prompt_file resolution from disk - Missing file handling (returns None, logs warning) Refs #92 #94 --- src/config/schema.rs | 158 +++++++++++++++++++++++++++++++++++++++++++ src/engine/review.rs | 34 ++++++++++ 2 files changed, 192 insertions(+) diff --git a/src/config/schema.rs b/src/config/schema.rs index 1739a15..c2ec5da 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -526,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/review.rs b/src/engine/review.rs index 949ef31..b4088f1 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -248,3 +248,37 @@ fn apply_ignore_rules(mut issues: Vec, ignore_rules: &[String]) -> 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() { + let dir = std::env::temp_dir().join("cora_test_prompt"); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("prompt.md"); + std::fs::write(&path, "file prompt content").unwrap(); + let result = resolve_system_prompt(None, Some(path.to_str().unwrap())); + assert_eq!(result.as_deref(), Some("file prompt content")); + let _ = std::fs::remove_dir_all(&dir); + } + + #[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()); + } +} From 301f265bcec67e24502270175e96f8abb4da4d77 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:39:25 +0700 Subject: [PATCH 08/12] style: fix formatting (rustfmt) --- src/engine/llm.rs | 12 ++++++++++-- src/engine/review.rs | 24 ++++++++++-------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 32eea50..82d9bba 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -290,7 +290,8 @@ pub async fn review_diff_stream( 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 raw = + chat_completion_stream(llm_config, system_prompt, &user_prompt, response_format).await?; let (issues, summary, tokens_used) = parse_review_response(&raw)?; @@ -464,7 +465,14 @@ 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, system_prompt, &user_prompt, Some(&spinner), response_format).await?; + let raw = chat_completion( + llm_config, + system_prompt, + &user_prompt, + Some(&spinner), + response_format, + ) + .await?; parse_scan_response(&raw) } diff --git a/src/engine/review.rs b/src/engine/review.rs index b4088f1..5a4bf01 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -22,10 +22,7 @@ fn load_system_prompt_file(path: &str) -> Option { } /// 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 { +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 { @@ -186,16 +183,15 @@ pub async fn scan_project( 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?; + 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); From c0a85d7cd761adf04d980e1b07416a8a1021c110 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:42:21 +0700 Subject: [PATCH 09/12] fix: add path traversal guard for system_prompt_file - Canonicalize path and check it's within project root (CWD) - Block paths outside project root (e.g. /etc/shadow, ../../.env) - Fix test to use project-local file instead of temp dir - Add test for path traversal rejection Addresses cora review finding: MAJOR arbitrary file read via config. Security: prevents secret leakage through malicious .cora.yaml commits. --- src/engine/review.rs | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/src/engine/review.rs b/src/engine/review.rs index 5a4bf01..591707e 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -6,9 +6,21 @@ 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 or can't be read. +/// 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 { - match std::fs::read_to_string(path) { + let canonical = std::fs::canonicalize(path).ok()?; + let project_root = std::env::current_dir().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!( @@ -257,13 +269,12 @@ mod tests { #[test] fn resolve_prompt_file_fallback() { - let dir = std::env::temp_dir().join("cora_test_prompt"); - std::fs::create_dir_all(&dir).unwrap(); - let path = dir.join("prompt.md"); - std::fs::write(&path, "file prompt content").unwrap(); - let result = resolve_system_prompt(None, Some(path.to_str().unwrap())); + // 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_dir_all(&dir); + let _ = std::fs::remove_file(&test_file); } #[test] @@ -277,4 +288,14 @@ mod tests { 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" + ); + } } From d6fb05c3525a24813cb90d94c55339649fb5657e Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:43:21 +0700 Subject: [PATCH 10/12] fix: explicit canonicalize error handling + remove duplicate line - Replace .ok()? with explicit match for canonicalize error - Log debug message when file doesn't exist (intended behavior) - Remove accidentally duplicated project_root line --- src/engine/review.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/engine/review.rs b/src/engine/review.rs index 591707e..7ea41f5 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -9,7 +9,13 @@ use crate::engine::types::{LLMConfig, ReviewIssue, ReviewResponse, ScanResponse} /// 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 = std::fs::canonicalize(path).ok()?; + 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()?; if !canonical.starts_with(&project_root) { From c9eb9ad07bcfa0c42d9eb879ea454c723de2adaf Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:45:50 +0700 Subject: [PATCH 11/12] fix: canonicalize project root to prevent symlink bypass Both the file path and project root are now canonicalized before comparison, preventing potential bypass via symlinked project root components. Addresses cora review finding: symlink bypass in path traversal guard. --- src/engine/review.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/engine/review.rs b/src/engine/review.rs index 7ea41f5..b159825 100644 --- a/src/engine/review.rs +++ b/src/engine/review.rs @@ -17,6 +17,7 @@ fn load_system_prompt_file(path: &str) -> Option { } }; 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!( From 946ba8ac0741e135e8508fc5803426d859d339d1 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 13:46:32 +0700 Subject: [PATCH 12/12] release: v0.1.6 Added: - Custom system prompts via .cora.yaml config (#94) - response_format: json_object opt-in (#92) - File path injection into review prompts (#93) - Post-parse file path filtering (#93) - Enhanced default system prompts (#95) Fixed: - Path traversal in system_prompt_file (#92) - Symlink bypass in path traversal guard --- CHANGELOG.md | 15 +++++++++++++++ Cargo.toml | 2 +- README.md | 8 ++++---- 3 files changed, 20 insertions(+), 5 deletions(-) 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)