fix: migrate findings/run_count/total_findings from u32 to u64#605
fix: migrate findings/run_count/total_findings from u32 to u64#605EffortlessSteven wants to merge 2 commits intomainfrom
Conversation
…gration Work item: work-b00574c1 Decision: Migrate findings, run_count, and total_findings from u32 to u64 following the existing files_scanned precedent (commit 2421a91). Introduce schema v2 for the breaking JSON schema change. Alternatives rejected: - TryFrom with explicit error handling (API-breaking return type change) - No schema bump (CHANGELOG doesn't exist, schemas are versioned)
…rom u32 to u64 Work item: work-b00574c1 Closes #577 Changes: - TrendRun.findings: u32 -> u64 - TrendSummary.run_count: u32 -> u64 - TrendSummary.total_findings: u32 -> u64 - Removed .min() guards that were silently truncating values - Added TREND_HISTORY_SCHEMA_V2 for breaking JSON schema change - Created schemas/diffguard.trend-history.v2.schema.json with findings as uint64 Following precedent from commit 2421a91 (files_scanned: u32->u64)
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 55 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Green Test Builder SummaryEdge Cases Tested (16 new tests)Empty/Single-Run Histories:
Large Value Handling (Core u32→u64 Migration):
Delta Calculation with Large Values:
Saturating Arithmetic:
Schema Versioning:
Type Verification (Compile-Time):
Test Resultsrunning 4 tests test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s running 16 tests test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s Known Gaps
|
Code Review: PR #605 — u32→u64 migrations in diffguard-analyticsWhat's good
Concerns1. Breaking change for persisted TrendHistory data 2. The findings: receipt.findings.len().min(u32::MAX as usize) as u32to: findings: receipt.findings.len() as u64This removes the 3. CI failures — Test and xtask ci SummaryGood direction. The schema V2 bump is the right migration path. Run |
Closes #577
Summary
Fixes a usize→u32 truncation vulnerability in diffguard-analytics crate by migrating three fields from u32 to u64:
The fix follows the existing files_scanned:u64 precedent established in commit 2421a91 and introduces schema v2 for the breaking JSON schema change.
ADR
Specs
What Changed
Code Changes
New Files
Test Results (so far)
Tests and linting should be run by CI. Implementation follows the established pattern from files_scanned migration.
Friction Encountered
Notes