Repository navigation
fix(orders): allow authenticated local bursts - #1145
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
@greptileai review |
[High impact] The PR appears safe to merge; no actionable new issues remain.
|
|
@greptileai review |
…iting` on the test auth middleware in `tests/order-write-rate-limits.test.ts` lines 151-166) by registering an app-level `expressRateLimit({ windowMs: 60 * 1000, limit: 1000 })` middleware before the test auth handler in `tests/order-write-rate-limits.test.ts`, matching the convention in `tests/cash-sessions.test.ts` and `tests/cash-closures.test.ts` while leaving route-level `orderWriteRateLimit` and `orderItemCancelRateLimit` test assertions intact. Verified with `npm run test:orders-authz`, `npm run test:script-coverage`, `npm run lint`, `npm run build`, `npm run docs:check`, and `git diff --check`
f6e1f78 to
115f582
Compare
|
@greptileai review |
|
@greptileai review |
Intent
PR 5 - Correct local order-write throttling (Fixes #1073)
Branch:
codex/order-write-limitsTitle:
fix(orders): allow authenticated local burstsRequirements:
FLO_ORDER_WRITE_RATE_LIMIT_MAXfollowing existing configuration parsing: a valid positive configured limit; otherwise the existing default of 60/minute.Acceptance: 61 legitimate authenticated local writes succeed; public traffic remains limited; PIN guessing remains limited; unrelated limiter buckets remain independent.
What Changed
isAllowedPrivateIp(req.socket.remoteAddress)) fromorderWriteRateLimitandorderItemCancelRateLimitinmain/routes/orders.ts, while keeping the two buckets separate, returning{ error: 'Too many requests, please try again later.' }on429, and continuing to rate-limit unauthenticated or public-IP traffic without trusting caller-supplied forwarding headers.resolveOrderWriteRateLimitMaxto support configuring the per-minute ceiling viaFLO_ORDER_WRITE_RATE_LIMIT_MAXwhen set to a positive integer (defaulting to60), and documented the behavior indocs/reference/api.md.tests/order-write-rate-limits.test.ts(wired intonpm run test:orders-authz) for authenticated local write bursts, public-interface and spoofed-header rate limiting, bucket isolation, andFLO_ORDER_WRITE_RATE_LIMIT_MAXparsing.Risk Assessment
✅ Low: The change cleanly scopes the private-IP rate-limit exemption on order-write and item-cancel buckets to authenticated socket connections, preserves public and PIN rate limits, returns the standard JSON error envelope on 429, and adds thorough behavioral test coverage.
Testing
Ran
npm run test:orders-authz(tests/orders-authz.test.tsandtests/order-write-rate-limits.test.ts),npm run test:script-coverage,npm run docs:check, andnpm run build, and drove a live Main API (startServer()) and Server App (startServerApp()) instance in an isolated temp environment to verify 65 local order writes, 62 local item cancellations, public-socket rate limiting withFLO_ORDER_WRITE_RATE_LIMIT_MAX, independentorderWriteRateLimit/orderItemCancelRateLimit/orderReadRateLimitbuckets, spoofed forwarding header and unauthenticated loopback rejection, Server App proxy protection, and manager PIN brute-force rate limiting (backend API/docs change only; no UI surface modified).npm run test:orders-authztests/order-write-rate-limits.test.ts)tests/order-write-rate-limits.test.ts)Evidence: Live Main API and Server App order-write rate-limit verification summary
{ "localOrderWriteBurst": { "count": 65, "all201": true, "sample61stStatus": 201, "sample65thStatus": 201, "lastOrderId": 65 }, "localItemCancelBurst": { "count": 62, "all200": true, "sample61stStatus": 200, "sample62ndStatus": 200 }, "serverAppProxy": { "unauthenticatedStatus": 401, "authenticatedOrderCreateStatus": 201 }, "pinBruteForceProtection": { "firstFiveWrongPinStatuses": [403, 403, 403, 403, 403], "sixthAttemptStatus": 429, "sixthAttemptBody": { "error": "Too many PIN attempts. Try again in 15 minutes." } }, "unrelatedBuckets": { "orderReadStatusAfterPinLockoutAndWriteBurst": 200, "rateLimitLimitHeaderOnOrderRead": "120" } }Evidence: Order authorization and order-write rate-limit test suite output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
npm run test:orders-authztests/order-write-rate-limits.test.ts)tests/order-write-rate-limits.test.ts)npm run test:orders-authznpm run test:script-coveragenpm run docs:checknpm run buildELECTRON_RUN_AS_NODE=1 ./node_modules/.bin/electron ./node_modules/ts-node/dist/bin.js --transpile-only -P tests/tsconfig.json -e "<live startServer() + startServerApp() burst, proxy, PIN lockout, and read-bucket independence verification>"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.