[ajianaz]

Summary

8 fixes from issue backlog — quality, security, and bug fixes only. No new features.

Fixes

Issue	Severity	Description
#87	📝 Quality	unwrap() → expect("valid spinner template") in ProgressStyle::with_template()
#83	📝 Quality	Consolidate duplicate impl Severity blocks into one
#88	📝 Docs	Fix AGENT.md referencing non-existent config validate subcommand
#77	🐛 Bug	file_content_hash returns Option<String> instead of empty string on read failure
#76	🐛 Bug	Log permission errors in scanner file walk at debug level (was silently skipping)
#72	🔒 Security	Warn if auth file has overly permissive permissions on read (chmod 600 hint)
#81	🔧 Quality	Replace non-deterministic DefaultHasher with sha2 for scan cache
#82	🔧 Quality	Validate SARIF file size before upload (GitHub 10MB limit)

Validation

• ✅ 205 tests passed (0 failed)
• ✅ cargo clippy clean
• ✅ cargo build --release clean
• ✅ Cora review: no issues found
• ✅ Cora review caught SHA256 truncation bug during commit — fixed before push

Files Changed

• src/engine/llm.rs — 1 line
• src/engine/scanner.rs — 5 lines
• src/engine/types.rs — 24 lines (consolidated impl)
• src/commands/scan.rs — 26 lines (sha2 + Option return)
• src/commands/upload.rs — 10 lines (size check)
• src/config/loader.rs — 16 lines (permission check)
• AGENT.md — 4 lines (doc fix)

Summary by CodeRabbit

• New Features

  • Added 10MB maximum size validation for SARIF uploads with clear error messages.
  • Added permission warnings for insecure auth configuration files.

• Bug Fixes

  • Improved handling of unreadable files during incremental scans to ensure complete coverage.

• Documentation

  • Updated config command description for clarity.

[coderabbitai]

Warning

Review limit reached

@ajianaz, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 32 minutes and 47 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5e7e06fe-b7cc-4bfb-b2aa-0583767cadeb

📥 Commits

Reviewing files that changed from the base of the PR and between be8c45c and 1f5810c.

📒 Files selected for processing (2)

• .github/workflows/release.yml
• src/commands/upload.rs

📝 Walkthrough

Walkthrough

PR adds file permission warnings for auth files, enforces 10MB SARIF upload limits, refactors incremental scan hashing from DefaultHasher to SHA-256 with Option-based unreadable-file handling, improves file-read error logging, hardens error messages, and corrects documentation describing the config command.

Changes

Security Hardening, Hash Refactor, and Robustness Improvements

Layer / File(s)	Summary
Incremental scan hash refactor src/commands/scan.rs	file_content_hash now returns Option<String> with SHA-256 digest (8-byte hex) and None for unreadable files; cache-filtering and cache-save logic updated to treat None as "rescans" and skip cache updates respectively, replacing empty-string-based detection.
Security and size validation guards src/config/loader.rs, src/commands/upload.rs, src/engine/scanner.rs	Auth file loader emits Unix-only warning for overly-permissive ~/.cora/auth.toml modes, upload validates SARIF content ≤10MB before parsing, and project walker logs debug message when file reads fail.
Error handling and code organization src/engine/llm.rs, src/engine/types.rs	Spinner template unwrap changed to explicit expect message; Display impl for Severity moved after its impl block without changing formatting behavior.
Documentation correction AGENT.md	Config subcommand description updated from "display, validate" to "display, set" in both file tree and key files table.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• 🔒 Security: Auth file permission not validated on read #72 — File permission warning for ~/.cora/auth.toml directly implements the Unix-only security check proposed in this issue.
• 🐛 Silent error swallowing in scanner file walk #76 — File-read error logging in src/engine/scanner.rs addresses the request to log permission and read errors before continuing.
• 🔧 No SARIF file size validation before upload #82 — 10MB SARIF size validation in src/commands/upload.rs directly fixes the missing file-size-validation bug.
• 📝 ProgressStyle::with_template().unwrap() in production #87 — Spinner template error handling change from unwrap() to expect() matches the suggested panic-message improvement.
• 🐛 file_content_hash returns empty string on errors (scan cache bug) #77 — Option-based file hash returning None for unreadable files solves the bug where empty strings caused unnecessary rescans of unreadable files.

Poem

🐰 Hashing now with SHA-256's gleam,
Auth permissions blessed with warnings supreme,
Files that can't be read stay in rescans,
SARIF uploads capped by ten-meg plans!
Error messages shine, bugs now all mend—
A robustness quest that comes to an end. 🛡️

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the pull request as a batch of 8 quality/security/bug fixes for version 0.1.8, which aligns with the multiple changes across documentation, bug fixes, and security improvements described in the raw summary.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/v0.1.8-patch-batch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🔍 Cora AI Code Review

✅ No issues found. Code looks good!

---

Review powered by cora-cli · BYOK · MIT

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/commands/scan.rs (1)

189-190: ⚡ Quick win

Clarify the "no truncation" comment.

The comment states "no truncation" but the code is taking only the first 8 bytes of the 32-byte SHA-256 hash. While this might refer to the hex conversion (each byte is fully represented as 2 hex digits), the phrasing could confuse future maintainers.

Consider rephrasing to be more explicit:

-    // Use first 8 bytes as hex — consistent representation, no truncation
+    // Take first 8 bytes of 32-byte hash and format as 16-char hex string
     Some(hash.iter().take(8).map(|b| format!("{b:02x}")).collect())

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/scan.rs` around lines 189 - 190, The comment "no truncation" is
misleading because the code takes only the first 8 bytes of the 32-byte SHA-256
digest; update the comment near the Some(hash.iter().take(8).map(|b|
format!("{b:02x}")).collect()) expression to clearly state that we intentionally
use an 8-byte prefix (16 hex characters) as a short identifier and that each
byte is fully represented as two hex digits (i.e., bytes aren't partially
truncated, but the digest is shortened to a prefix).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/upload.rs`:
- Around line 43-51: The error message uses integer division when computing MB,
causing truncation; update the check around MAX_SARIF_SIZE and the bail! call to
compute the human-readable size as a floating-point MiB value (e.g., let
size_mib = sarif_content.len() as f64 / (1024.0 * 1024.0)) and format it with a
fixed precision (such as two decimals) in the bail! message, and consider
labeling the units as "MiB" to reflect binary units; modify the code that reads
sarif_content.len() and the bail! invocation so it prints the formatted size_mib
instead of the integer division result.

---

Nitpick comments:
In `@src/commands/scan.rs`:
- Around line 189-190: The comment "no truncation" is misleading because the
code takes only the first 8 bytes of the 32-byte SHA-256 digest; update the
comment near the Some(hash.iter().take(8).map(|b| format!("{b:02x}")).collect())
expression to clearly state that we intentionally use an 8-byte prefix (16 hex
characters) as a short identifier and that each byte is fully represented as two
hex digits (i.e., bytes aren't partially truncated, but the digest is shortened
to a prefix).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64557a6c-cc00-4e21-83aa-89123eab60e9

📥 Commits

Reviewing files that changed from the base of the PR and between 4e21db7 and be8c45c.

📒 Files selected for processing (7)

• AGENT.md
• src/commands/scan.rs
• src/commands/upload.rs
• src/config/loader.rs
• src/engine/llm.rs
• src/engine/scanner.rs
• src/engine/types.rs