fix(security): cap account_ids at 200 per request (input amplification guard)#102
Conversation
… amplification
Closes H9 from the security review: parseAccountIDs accepted a comma-
separated list of any length. Each accepted ID fans out into per-account
DB queries / cloud API calls downstream, so an unbounded list is an
amplification vector — a single request with thousands of IDs can exhaust
connection pools or hit cloud-API rate limits.
Adds MaxAccountIDsPerRequest=200, returning a 400 ("too many account IDs:
max 200 per request") for over-the-limit input. The cap is checked BEFORE
the UUID validation loop, so a 10k-entry input is rejected without
allocating a 10k-element slice or running 10k validations.
Bounded audit per plan §PR3: grepped internal/api/ for `strings.Split(.,",")`
and other comma-fan-out patterns. Only parseAccountIDs hit. Recommendations
list already has its own `maxRecommendations` cap at handler_purchases.go:430.
No other unbounded fan-out lists found in the 30-min audit window.
Tests: 3 new sub-tests cover the at-limit success, just-over-limit 400, and
massively-over-limit fast-rejection cases. Existing parse tests unchanged.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (2)
✨ 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
cristim
added
triaged
|
P1 — closes H9: unbounded account_ids input was an amplification vector (1 request → N DB queries + cloud API calls). Cap of 200 enforced before validation loop. Small diff, well-tested. CI green, MERGEABLE. (triage agent wave2-E) |
1 participant
Summary
Closes H9 from the security review:
parseAccountIDsaccepted a comma-separated list of any length. Each ID fans out into per-account DB queries / cloud API calls downstream, so an unbounded list is an amplification vector — a single request with thousands of IDs can exhaust connection pools or hit cloud-API rate limits.Adds
MaxAccountIDsPerRequest = 200, returning a 400 (too many account IDs: max 200 per request) for over-the-limit input. The cap is checked before the UUID validation loop so a 10k-entry input is rejected without allocating a 10k-element slice or running 10k validations.Bounded audit (per plan §PR3, 30-min time-box)
Grepped
internal/api/forstrings.Split(.,",")and other comma-fan-out patterns:parseAccountIDshit.maxRecommendationscap athandler_purchases.go:430.Test plan
parseAccountIDstests unchanged + still passing.go test -short -race -count=1 ./internal/api/...green.Follow-up PRs
PR4 (IAM wildcards), PR5 (supply chain) — independent of this PR.
🤖 Generated with claude-flow