From be8c45c28e3274ad807737b0c74093dcd5ea0855 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 23:00:00 +0700 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20v0.1.8=20patch=20batch=20=E2=80=94?= =?UTF-8?q?=208=20quality/security/bug=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #87: unwrap() → expect() in ProgressStyle (llm.rs, scanner.rs) Closes #83: consolidate duplicate Severity impl blocks into one Closes #88: fix AGENT.md config validate → show/set references Closes #77: file_content_hash returns Option instead of empty string on read failure (prevents rescan loop) Closes #76: log permission errors in scanner file walk at debug level Closes #72: warn if auth file has overly permissive permissions (Unix) Closes #81: replace non-deterministic DefaultHasher with sha2 for scan cache (deterministic across Rust versions) Closes #82: validate SARIF file size before upload (GitHub 10MB limit) --- AGENT.md | 4 ++-- src/commands/scan.rs | 28 +++++++++++++++------------- src/commands/upload.rs | 10 ++++++++++ src/config/loader.rs | 16 ++++++++++++++++ src/engine/llm.rs | 2 +- src/engine/scanner.rs | 5 ++++- src/engine/types.rs | 24 +++++++++++------------- 7 files changed, 59 insertions(+), 30 deletions(-) diff --git a/AGENT.md b/AGENT.md index acda24e0..b6e7dea4 100644 --- a/AGENT.md +++ b/AGENT.md @@ -29,7 +29,7 @@ src/ │ ├── mod.rs │ ├── review.rs # cora review (diff-based review) │ ├── scan.rs # cora scan (full-file scan) -│ ├── config_cmd.rs # cora config (show/validate) +│ ├── config_cmd.rs # cora config (show/set) │ ├── auth.rs # cora auth (API key management) │ ├── hook_cmd.rs # cora hook (pre-commit hook install/uninstall) │ ├── init.rs # cora init (project scaffolding) @@ -67,7 +67,7 @@ src/ | File | Purpose | |---|---| -| `commands/config_cmd.rs` | Config subcommand — display, validate, path resolution | +| `commands/config_cmd.rs` | Config subcommand — display, set, path resolution | | `config/loader.rs` | Config loading with full priority chain resolution | | `config/schema.rs` | All config structs, defaults, serde annotations | | `commands/init.rs` | Project scaffolding, `.cora.yaml` generation | diff --git a/src/commands/scan.rs b/src/commands/scan.rs index 95523c1d..40f10f7c 100644 --- a/src/commands/scan.rs +++ b/src/commands/scan.rs @@ -67,7 +67,10 @@ pub async fn execute_scan( let root_abs = root.canonicalize().unwrap_or_else(|_| root.clone()); files.retain(|f| { let abs_path = root_abs.join(&f.path); - let hash = file_content_hash(&abs_path); + let hash = match file_content_hash(&abs_path) { + Some(h) => h, + None => return true, // can't read file, rescan it + }; match cache.get(&root_abs, &f.path) { Some(cached_hash) if cached_hash == hash => { debug!(file = %f.path, "skipping unchanged file (incremental)"); @@ -160,7 +163,10 @@ pub async fn execute_scan( let mut cache = ScanCache::load().unwrap_or_default(); for f in &files { let abs_path = root_abs.join(&f.path); - let hash = file_content_hash(&abs_path); + let hash = match file_content_hash(&abs_path) { + Some(h) => h, + None => continue, // can't read file, skip cache entry + }; cache.set(&root_abs, &f.path, &hash); } cache.save()?; @@ -175,17 +181,13 @@ pub async fn execute_scan( } /// Compute a short SHA256 hash of a file's content for incremental scanning. -fn file_content_hash(path: &std::path::Path) -> String { - match std::fs::read(path) { - Ok(bytes) => { - use std::hash::{Hash, Hasher}; - // Use a fast non-crypto hash — we just need change detection, not security. - let mut hasher = std::collections::hash_map::DefaultHasher::new(); - bytes.hash(&mut hasher); - format!("{:016x}", hasher.finish()) - } - Err(_) => String::new(), - } +/// Returns None if the file cannot be read (caller should rescan it). +fn file_content_hash(path: &std::path::Path) -> Option { + use sha2::Digest; + let bytes = std::fs::read(path).ok()?; + let hash = sha2::Sha256::digest(&bytes); + // Use first 8 bytes as hex — consistent representation, no truncation + Some(hash.iter().take(8).map(|b| format!("{b:02x}")).collect()) } /// Cache of file content hashes for incremental scanning. diff --git a/src/commands/upload.rs b/src/commands/upload.rs index 0d3e2121..902b109c 100644 --- a/src/commands/upload.rs +++ b/src/commands/upload.rs @@ -40,6 +40,16 @@ pub async fn execute_upload(opts: &UploadOptions) -> Result { // Read SARIF content let sarif_content = read_sarif(opts.file.as_deref())?; + // Validate file size (GitHub limits SARIF uploads to 10MB) + const MAX_SARIF_SIZE: usize = 10 * 1024 * 1024; // 10MB + if sarif_content.len() > MAX_SARIF_SIZE { + bail!( + "SARIF file is {} bytes ({}MB), exceeding GitHub's 10MB limit. Consider reviewing a smaller diff.", + sarif_content.len(), + sarif_content.len() / (1024 * 1024) + ); + } + // Validate it's valid JSON (basic sanity check) let parsed: serde_json::Value = serde_json::from_str(&sarif_content).context("SARIF file does not contain valid JSON")?; diff --git a/src/config/loader.rs b/src/config/loader.rs index bd143b59..5e42718c 100644 --- a/src/config/loader.rs +++ b/src/config/loader.rs @@ -375,6 +375,22 @@ pub fn load_api_key_from_auth_file() -> Result> { return Ok(None); } + // Security: warn if auth file has overly permissive permissions (Unix only) + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if let Ok(meta) = std::fs::metadata(&path) { + let mode = meta.permissions().mode(); + if mode & 0o077 != 0 { + tracing::warn!( + "auth file has overly permissive permissions ({:o}). Run: chmod 600 {}", + mode & 0o777, + path.display() + ); + } + } + } + let content = std::fs::read_to_string(&path) .with_context(|| format!("failed to read {}", path.display()))?; diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 9726732a..4e4e1a9e 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -222,7 +222,7 @@ fn create_spinner(message: &str) -> ProgressBar { spinner.enable_steady_tick(std::time::Duration::from_millis(80)); spinner.set_style( ProgressStyle::with_template("{spinner:.cyan} {msg}") - .unwrap() + .expect("valid spinner template") .tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈ "), ); spinner.set_message(message.to_string()); diff --git a/src/engine/scanner.rs b/src/engine/scanner.rs index acb3ebcb..a69f718f 100644 --- a/src/engine/scanner.rs +++ b/src/engine/scanner.rs @@ -128,7 +128,10 @@ pub fn walk_project( // Read file content (skip binary / unreadable) let content = match std::fs::read_to_string(path) { Ok(c) => c, - Err(_) => continue, + Err(e) => { + debug!(file = %relative, error = %e, "skipping unreadable file"); + continue; + } }; // Skip empty files diff --git a/src/engine/types.rs b/src/engine/types.rs index ddf47cfe..ca7e7a71 100644 --- a/src/engine/types.rs +++ b/src/engine/types.rs @@ -21,20 +21,7 @@ impl Severity { _ => Severity::Info, } } -} - -impl fmt::Display for Severity { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Severity::Critical => write!(f, "critical"), - Severity::Major => write!(f, "major"), - Severity::Minor => write!(f, "minor"), - Severity::Info => write!(f, "info"), - } - } -} -impl Severity { /// Get the label text for this severity. pub fn label(&self) -> &'static str { match self { @@ -56,6 +43,17 @@ impl Severity { } } +impl fmt::Display for Severity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Severity::Critical => write!(f, "critical"), + Severity::Major => write!(f, "major"), + Severity::Minor => write!(f, "minor"), + Severity::Info => write!(f, "info"), + } + } +} + /// Issue type categories #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] From 8dd964577edbb040ad1ffab16384af1b571c6098 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 23:10:48 +0700 Subject: [PATCH 2/3] fix: generate checksums-sha256.txt in release workflow Prevents 'No checksums file found' warning in cora-review CI action. Checksums are generated from all release artifacts and uploaded to the GitHub release alongside binaries. --- .github/workflows/release.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2ee5733d..cf9c203f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -125,6 +125,12 @@ jobs: - name: List artifacts run: ls -la release-artifacts/ + - name: Generate SHA256 checksums + run: | + cd release-artifacts + sha256sum * > checksums-sha256.txt + cat checksums-sha256.txt + - name: Extract version from tag id: version run: | From 1f5810c41ad8c2fc768a98ec6e8899b0be86d9e5 Mon Sep 17 00:00:00 2001 From: CTO Hermes Date: Mon, 1 Jun 2026 23:19:54 +0700 Subject: [PATCH 3/3] fix: use float division for accurate MB display in SARIF size error Co-authored-by: CodeCora --- src/commands/upload.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/commands/upload.rs b/src/commands/upload.rs index 902b109c..b9c9b88d 100644 --- a/src/commands/upload.rs +++ b/src/commands/upload.rs @@ -43,10 +43,11 @@ pub async fn execute_upload(opts: &UploadOptions) -> Result { // Validate file size (GitHub limits SARIF uploads to 10MB) const MAX_SARIF_SIZE: usize = 10 * 1024 * 1024; // 10MB if sarif_content.len() > MAX_SARIF_SIZE { + let size_mb = sarif_content.len() as f64 / (1024.0 * 1024.0); bail!( - "SARIF file is {} bytes ({}MB), exceeding GitHub's 10MB limit. Consider reviewing a smaller diff.", + "SARIF file is {} bytes ({:.2}MB), exceeding GitHub's 10MB limit. Consider reviewing a smaller diff.", sarif_content.len(), - sarif_content.len() / (1024 * 1024) + size_mb ); }