⚡ Bolt: Skip rate limit delay for cached HIBP checks#100
⚡ Bolt: Skip rate limit delay for cached HIBP checks#100SlasshyOverhere wants to merge 1 commit into
Conversation
💡 What: Updated `checkPasswordBreach` and `fetchHIBP` to return whether the result was served from the in-memory cache, and updated `checkMultipleBreaches` and the `shell.ts` bulk breach check to skip the 200ms rate-limiting delay if the network request was skipped. 🎯 Why: When checking many passwords that share the same 5-character SHA-1 prefix (or are exact duplicates), the application was needlessly enforcing a 200ms delay even when serving the result instantly from local memory. 📊 Impact: Significantly speeds up bulk password breach checks (`breach --all`) when prefix collisions or duplicate passwords exist, skipping the artificial delay for cached entries while safely maintaining the delay for actual network requests. 🔬 Measurement: Run `breach --all` on a vault with duplicate passwords or similar SHA-1 prefixes. The progress spinner will advance instantly for cached checks instead of stalling for 200ms each time.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
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 14 minutes and 30 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR optimizes bulk Have I Been Pwned (HIBP) password breach checks by skipping the 200ms rate-limit delay when a result is served from the in-memory SHA-1 prefix cache, improving throughput for duplicate passwords / prefix collisions.
Changes:
- Updated
fetchHIBP/checkPasswordBreachto expose whether the response was served from cache. - Updated bulk breach-check loops (
checkMultipleBreachesand CLIbreach --all) to skip the delay when no network request occurred. - Added a new note entry to
.jules/bolt.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/cli/breachCheck.ts |
Propagates cache-hit metadata and uses it to conditionally apply rate-limit delay in bulk checks. |
src/cli/shell.ts |
Skips the delay in the breach --all loop when the check was served from cache. |
.jules/bolt.md |
Adds an internal note about safely skipping rate-limit delays using the k-anonymity cache. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## 2024-05-30 - Safe Rate-Limit Bypassing via k-anonymity Cache | ||
| **Learning:** When optimizing external API lookups in a password manager (e.g., Have I Been Pwned checks), NEVER cache by plaintext password in memory as it creates a severe security vulnerability (memory scraping/crash dumps). | ||
| **Action:** Instead, leverage the safe k-anonymity SHA-1 prefix cache (`hibpCache`) to detect cache hits and use that boolean status to safely skip artificial rate-limit delays for duplicate inputs. |
There was a problem hiding this comment.
Repo convention appears to be to avoid committing generated/internal .jules/* artifacts in merged PRs (see CHANGELOG.md 0.1.2 entry). This PR adds a new .jules/bolt.md section; please drop .jules/bolt.md from the PR (or move this note into a non-internal doc location if it must be kept).
| ## 2024-05-30 - Safe Rate-Limit Bypassing via k-anonymity Cache | |
| **Learning:** When optimizing external API lookups in a password manager (e.g., Have I Been Pwned checks), NEVER cache by plaintext password in memory as it creates a severe security vulnerability (memory scraping/crash dumps). | |
| **Action:** Instead, leverage the safe k-anonymity SHA-1 prefix cache (`hibpCache`) to detect cache hits and use that boolean status to safely skip artificial rate-limit delays for duplicate inputs. |
| breached: boolean; | ||
| count: number; // Number of times seen in breaches (0 if not breached) | ||
| error?: string; | ||
| networkRequestSkipped?: boolean; // True if the check was skipped due to a cache hit |
There was a problem hiding this comment.
networkRequestSkipped is now used as a control signal for rate limiting, but it is typed as optional. Making it optional encourages callers/mocks to omit it and forces call sites to rely on JS truthiness (!undefined). Consider making this a required boolean (defaulting to false on non-cached paths) so the API contract is explicit and TypeScript can enforce correct usage everywhere.
| networkRequestSkipped?: boolean; // True if the check was skipped due to a cache hit | |
| networkRequestSkipped: boolean; // True if the check was skipped due to a cache hit |
⚡ Bolt: Skip rate limit delay for cached HIBP checks
💡 What: Updated
checkPasswordBreachandfetchHIBPto return whether the result was served from the in-memory cache, and updatedcheckMultipleBreachesand theshell.tsbulk breach check to skip the 200ms rate-limiting delay if the network request was skipped.🎯 Why: When checking many passwords that share the same 5-character SHA-1 prefix (or are exact duplicates), the application was needlessly enforcing a 200ms delay even when serving the result instantly from local memory.
📊 Impact: Significantly speeds up bulk password breach checks (
breach --all) when prefix collisions or duplicate passwords exist, skipping the artificial delay for cached entries while safely maintaining the delay for actual network requests.🔬 Measurement: Run
breach --allon a vault with duplicate passwords or similar SHA-1 prefixes. The progress spinner will advance instantly for cached checks instead of stalling for 200ms each time.PR created automatically by Jules for task 566358373230666463 started by @SlasshyOverhere