Fix/issue 239 server rate limiting - #249
Merged
hrx01-dev merged 11 commits intoJul 8, 2026
Merged
Conversation
Return 500 instead of silently falling back to the hardcoded production origin, so a misconfigured deployment is caught immediately rather than leaking the origin allowlist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e status filter useAdminData: add serverOrder param to useCollectionData so paginated queries use a matching Firestore orderBy instead of re-sorting a doc-ID-ordered page client-side (which dropped items across page boundaries). Restore status-aware useMessages that pushes the where+orderBy into Firestore so hasMore reflects the filtered set, not the raw collection. Messages: pass active filter to useMessages, drop client-side visible/counts memos and count badges (counts are meaningless when only one status is loaded). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add paymentErrorMessage() to payments.ts mapping Firestore error codes (permission-denied, unauthenticated, unavailable) to user-friendly strings. Kept Firebase-free for unit-testability. useClientPayments: use paymentErrorMessage instead of raw err.message. PaymentManagement: surface the specific error and add a Try again button. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…loses hrx01-dev#241 permissions.test.ts: covers unauthenticated access (null/undefined role), admin-only routes (super_admin-exclusive permissions), privilege escalation for all three non-super-admin roles, and complete permission-set assertions. loginLockout.test.ts: covers brute-force protection (5-attempt lockout), lockout persistence across page refresh, lockout survival across sign-out, email normalisation, expired-window clean slate, and success-path clear. authError.test.ts: covers unauthenticated/invalid-credential codes, revoked/disabled account (auth/user-disabled), rate-limiting, network errors, and graceful fallback for unknown codes and non-object inputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rx01-dev#239 Move Firestore writes from the client to a new Vercel API endpoint (api/quote.ts) that re-runs field validation and enforces a 3/10-min sliding-window rate limit keyed by hashed caller IP, stored in a Firestore `quoteRateLimit` collection that security rules deny to clients. The client (submitQuote.ts) now POSTs to /api/quote and surfaces a typed RateLimitError on 429 so QuoteForm can show the server's message directly. Client-side localStorage tracking is kept as a UX hint (fast feedback on the common path) but is no longer the enforcement boundary. Pure helpers (buildQuoteSummary, buildMessageData, buildMailData) stay in submitQuote.ts so the server can import them without duplicating logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@kumudranjan6127-debug is attempting to deploy a commit to the hrx01-dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughMoves quote submission to a server-side ChangesServer-side quote flow and endpoint hardening
Admin data, dashboard errors, and regression tests
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/admin/lib/loginLockout.test.ts (1)
31-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit coverage for storage failures.
This suite never simulates
localStorage.getItem,setItem, orremoveItemthrowing, so the graceful-degradation branches insrc/admin/lib/loginLockout.ts:46-123are still untested even though the header says unavailable storage is covered. A small spy-based case for each public helper would lock in that fallback behavior.🤖 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/admin/lib/loginLockout.test.ts` around lines 31 - 167, Add explicit tests in loginLockout.test.ts for storage failure fallbacks by mocking localStorage methods to throw. Cover readLoginLockout, recordLoginFailure, and clearLoginLockout so the graceful-degradation paths in loginLockout.ts are exercised when getItem, setItem, or removeItem fail. Use the existing helper names and EMAIL/T0 setup to verify each public helper returns the safe fallback state and does not throw.src/admin/rbac/permissions.test.ts (1)
217-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact sensitive-permission set.
This still passes if an extra super-admin-only permission is added to
SENSITIVE_PERMISSIONS, so it doesn't fully lock down the PIN-gated contract.Suggested tightening
- it("includes projects:delete, admins:manage, and business:view_sensitive", () => { - expect(SENSITIVE_PERMISSIONS).toContain("projects:delete"); - expect(SENSITIVE_PERMISSIONS).toContain("admins:manage"); - expect(SENSITIVE_PERMISSIONS).toContain("business:view_sensitive"); + it("matches the exact sensitive permission set", () => { + expect([...SENSITIVE_PERMISSIONS].sort()).toEqual( + ["admins:manage", "business:view_sensitive", "projects:delete"].sort(), + ); });🤖 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/admin/rbac/permissions.test.ts` around lines 217 - 221, The test in permissions.test.ts only checks for a few members in SENSITIVE_PERMISSIONS, so it can still pass if extra PIN-gated permissions are added. Tighten the assertion around SENSITIVE_PERMISSIONS to verify the exact set of sensitive permissions rather than only using toContain checks, and keep the existing intent anchored on SENSITIVE_PERMISSIONS and the sensitive-permission contract.
🤖 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 `@api/quote.ts`:
- Around line 42-45: The IP identifier helper in hashIp currently uses an
unsalted SHA-256 hash, which is too easy to reverse for common IPs. Update
hashIp to use a keyed HMAC (or equivalent server-only secret/pepper) while
preserving deterministic output for the same input, and source the secret from a
secure server configuration value rather than hardcoding it. Make sure the new
logic stays localized to hashIp so callers in api/quote.ts continue using the
same interface.
- Around line 19-31: initAdmin() currently catches Firebase Admin initialization
errors but only logs them, allowing handlers to continue into getFirestore() and
crash later; change initAdmin() in quote.ts to signal failure so the request
path can return a controlled 500, and apply the same fail-fast guard pattern in
razorpay.ts. Use the initAdmin() helper and the getFirestore() call sites to
locate the flow, and ensure invalid FIREBASE_SERVICE_ACCOUNT or other init
errors stop execution before any Firestore access.
- Around line 101-124: The rate-limit logic in quote handling is currently
non-atomic and can fail open, so parallel requests can bypass the cap and
Firestore errors disable enforcement. Update the quote flow around
evaluateRateLimit, rateLimitRef, and the lead creation path to perform the
read/check/update inside a Firestore transaction before saving the lead, then
only proceed when the transaction succeeds. If the transaction or rate-limit
access fails, return an enforcement error instead of continuing so the limit is
enforced consistently.
In `@src/admin/hooks/useAdminData.ts`:
- Around line 227-260: The `useAdminData` hook only resets `pageLimit` when
`status` changes, so `state.data` and `loading` can briefly show the previous
filter’s results in `Messages`. Update the `useEffect` that watches `status` to
also reset the hook state immediately (clear data, set loading true, reset
error/hasMore as appropriate) before the Firestore `onSnapshot` in the second
`useEffect` repopulates it, using the existing `setState`, `pageLimit`, and
`status` logic.
- Around line 242-248: The filtered messages query in useAdminData’s messages
listener combines where("status", "==", status) with orderBy("createdAt",
"desc"), so add the matching composite index for the messages collection in
firestore.indexes.json. Update the Firestore index configuration to support that
exact query shape so the onSnapshot(query(messagesCollection, ...constraints))
path does not fail with failed-precondition when status is set.
---
Nitpick comments:
In `@src/admin/lib/loginLockout.test.ts`:
- Around line 31-167: Add explicit tests in loginLockout.test.ts for storage
failure fallbacks by mocking localStorage methods to throw. Cover
readLoginLockout, recordLoginFailure, and clearLoginLockout so the
graceful-degradation paths in loginLockout.ts are exercised when getItem,
setItem, or removeItem fail. Use the existing helper names and EMAIL/T0 setup to
verify each public helper returns the safe fallback state and does not throw.
In `@src/admin/rbac/permissions.test.ts`:
- Around line 217-221: The test in permissions.test.ts only checks for a few
members in SENSITIVE_PERMISSIONS, so it can still pass if extra PIN-gated
permissions are added. Tighten the assertion around SENSITIVE_PERMISSIONS to
verify the exact set of sensitive permissions rather than only using toContain
checks, and keep the existing intent anchored on SENSITIVE_PERMISSIONS and the
sensitive-permission contract.
🪄 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 Plus
Run ID: 989daf97-2ee4-4df2-9eb1-85eb872aac90
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
api/estimate.tsapi/quote.tssrc/admin/hooks/useAdminData.tssrc/admin/lib/authError.test.tssrc/admin/lib/loginLockout.test.tssrc/admin/pages/Messages.tsxsrc/admin/rbac/permissions.test.tssrc/app/components/QuoteForm.tsxsrc/app/lib/submitQuote.test.tssrc/app/lib/submitQuote.tssrc/dashboard/hooks/useClientPayments.tssrc/dashboard/lib/payments.test.tssrc/dashboard/lib/payments.tssrc/dashboard/pages/PaymentManagement.tsx
Comment on lines
+242
to
+248
| const constraints: QueryConstraint[] = []; | ||
| if (status) constraints.push(where("status", "==", status)); | ||
| constraints.push(orderBy("createdAt", "desc")); | ||
| constraints.push(firestoreLimit(pageLimit)); | ||
|
|
||
| const unsubscribe = onSnapshot( | ||
| query(messagesCollection, ...constraints), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'firebase.json' . -x sh -c 'echo "== $1 =="; cat "$1"' sh {}
fd -i 'firestore.indexes*.json' . -x sh -c '
echo "== $1 ==";
rg -n -C3 '"'"'messages|status|createdAt'"'"' "$1" || true
' sh {}Repository: hrx01-dev/Servio
Length of output: 2650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== firestore.indexes.json =="
cat -n firestore.indexes.json
echo
echo "== src/admin/hooks/useAdminData.ts (around the query) =="
sed -n '230,260p' src/admin/hooks/useAdminData.tsRepository: hrx01-dev/Servio
Length of output: 1682
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "messagesCollection|collectionGroup|collection\\(" src/admin/hooks/useAdminData.ts src -g '!**/dist/**'Repository: hrx01-dev/Servio
Length of output: 2641
Add a composite index for the filtered messages query. where("status", "==", status) + orderBy("createdAt", "desc") needs a matching messages index in firestore.indexes.json; without it, every status-filtered load will fail with failed-precondition.
🤖 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/admin/hooks/useAdminData.ts` around lines 242 - 248, The filtered
messages query in useAdminData’s messages listener combines where("status",
"==", status) with orderBy("createdAt", "desc"), so add the matching composite
index for the messages collection in firestore.indexes.json. Update the
Firestore index configuration to support that exact query shape so the
onSnapshot(query(messagesCollection, ...constraints)) path does not fail with
failed-precondition when status is set.
hrx01-dev
self-requested a review
June 29, 2026 18:06
hrx01-dev
requested changes
Jun 29, 2026
hrx01-dev
left a comment
hrx01-dev
left a comment
Owner
There was a problem hiding this comment.
Nice work there !! fix the issues given by @coderabbitai once you do that I'll be happy to merge it .. Also PR without any proof of work mainly screenshot or screen recording will not be approved
api/quote.ts: - initAdmin() now returns a boolean; the handler returns a controlled 500 when Firebase Admin can't initialize instead of falling through to getFirestore() and crashing mid-request. Same guard applied to api/razorpay.ts. - Hash the caller IP with a keyed HMAC (RATE_LIMIT_HASH_SECRET pepper) instead of a bare SHA-256, which is trivially reversible for IPv4 via a dictionary. - Enforce the rate limit inside a single Firestore transaction (read → check → consume slot), so concurrent requests can't each read the same window and slip past the 3-per-10-min cap. Any enforcement failure now fails CLOSED with a 500 rather than silently allowing the write. src/admin/hooks/useAdminData.ts: - Clear useMessages snapshot state on status-filter change so the previous filter's rows don't linger until the next snapshot. .env.example: document RATE_LIMIT_HASH_SECRET and FIREBASE_SERVICE_ACCOUNT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r-rate-limiting # Conflicts: # api/razorpay.ts # src/app/components/QuoteForm.tsx # src/app/lib/submitQuote.test.ts # src/app/lib/submitQuote.ts
…-dev#239) Drives the real api/quote.ts handler against an in-memory Firestore mock, proving the server-side enforcement end to end: - valid submission → 200 and persists lead + email - 4th submission from one IP within the window → 429 with Retry-After - each IP tracked independently - fails closed (500) when RATE_LIMIT_HASH_SECRET / ALLOWED_ORIGIN is unset - invalid body → 400, non-POST → 405 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
Contributor
Author
✅ CodeRabbit review addressed + proof of workAll 5 CodeRabbit actionable comments are resolved (commits
Proof of workAdded
|
✅ Action performedReview finished.
|
hrx01-dev
self-requested a review
July 8, 2026 17:24
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/quote.ts (1)
116-170: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRate limit slot is consumed before the lead write — transient failures waste submission slots.
The transaction at lines 135-140 atomically consumes the rate limit slot, but the lead write at lines 160-164 happens outside the transaction. If the lead write fails (transient Firestore error), the user gets a 500 but their slot has already been consumed. On their 3rd allowed submission, this locks them out for the full 10-minute window despite never having their request saved.
Move the lead write into the same transaction so both operations commit or fail atomically. Note: the test mock's
txobject (insrc/test/quote-endpoint.test.tslines 35-46) would need acreatemethod added if this fix is applied.🔧 Proposed fix: include lead write in the transaction
let rateVerdict: { allowed: boolean; retryAfterMs: number }; try { + const summary = buildQuoteSummary(form); rateVerdict = await db.runTransaction(async (tx) => { const snap = await tx.get(rateLimitRef); const stored = snap.exists ? snap.get("timestamps") : undefined; const history = Array.isArray(stored) ? (stored as unknown[]).filter((t): t is number => typeof t === "number") : []; const result = evaluateRateLimit(history, now, SERVER_RATE_LIMIT); // Consume the slot inside the transaction so it's atomic with the read. if (result.allowed) { tx.set(rateLimitRef, { timestamps: result.nextHistory, updatedAt: FieldValue.serverTimestamp(), }); + // Write the lead inside the same transaction so both succeed or fail together. + tx.create(db.collection("messages").doc(), { + ...buildMessageData(summary), + createdAt: FieldValue.serverTimestamp(), + }); } return { allowed: result.allowed, retryAfterMs: result.retryAfterMs }; }); } catch (err) { console.error("[quote] rate-limit transaction failed:", err); return res .status(500) .json({ error: "Could not process your request right now. Please try again." }); } if (!rateVerdict.allowed) { res.setHeader("Retry-After", String(Math.ceil(rateVerdict.retryAfterMs / 1000))); return res.status(429).json({ error: "Too many submissions. Please try again later.", retryAfterMs: rateVerdict.retryAfterMs, }); } - // 4. Write the lead (bypasses security rules — input is validated above). - const summary = buildQuoteSummary(form); - try { - await db.collection("messages").add({ - ...buildMessageData(summary), - createdAt: FieldValue.serverTimestamp(), - }); - } catch (err) { - console.error("[quote] failed to save lead:", err); - return res.status(500).json({ - error: "Failed to save your request. Please try again.", - }); - } - // 5. Email notification — best-effort; the lead is already saved.🤖 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 `@api/quote.ts` around lines 116 - 170, The rate-limit consumption and lead persistence in the quote flow are split between separate operations, so a failed lead save can still burn a submission slot. Update the quote handler in quote.ts to perform the `messages` write inside the same `db.runTransaction` block that uses `rateLimitRef` so the slot update and lead creation are atomic. Use the existing symbols `runTransaction`, `rateLimitRef`, `buildQuoteSummary`, and `buildMessageData` to keep the logic together, and make sure the transaction commits both actions or fails without consuming the slot; also update the quote endpoint test mock to support the transaction write path.
🧹 Nitpick comments (2)
src/test/quote-endpoint.test.ts (1)
19-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for failure paths to verify fail-closed behavior.
The suite covers the happy path, rate limiting, and config validation well. However, several critical failure paths are untested:
- Transaction failure → 500: Make the
runTransactionmock throw to verify the handler returns 500 instead of proceeding.- Lead write failure → 500: Make the
messages.addmock throw to verify the error response. (If the lead write is moved into the transaction per the suggestion inapi/quote.ts, this test would verify the transaction's 500 path instead.)- Email notification failure → still 200: Make the
mail.addmock throw to verify the handler still returns 200 (best-effort path).- OPTIONS method → 200: No test for the CORS preflight response.
initAdmin()failure → 500: The mock'sgetAppsalways returns[{}], so the init failure path is never exercised. Consider a test that swaps the mock to return[]and makesinitializeAppthrow.Also, if the lead write is moved into the transaction (as suggested in
api/quote.ts), thetxmock (lines 35-46) will need acreatemethod:create: (ref: { __path: string }, data: Record<string, unknown>) => { store.set(ref.__path, data); },Also applies to: 96-178
🤖 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/test/quote-endpoint.test.ts` around lines 19 - 54, The `quote-endpoint.test.ts` suite is missing failure-path coverage for fail-closed behavior in the handler and init flow. Add tests that force `runTransaction` to throw and assert a 500, make the lead write path fail (either `messages.add` or the transaction write path used in `api/quote.ts`) and assert the error response, and make the email notification write fail while still asserting a 200. Also add coverage for the `OPTIONS` preflight response and an `initAdmin()` failure by overriding the `getApps`/`initializeApp` mocks to exercise the error path.api/razorpay.ts (1)
15-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
initAdmin()across API routes.This function is nearly identical to the one in
api/quote.ts(lines 22-36). If the Firebase Admin initialization logic changes, both copies must be updated independently. Consider extracting to a shared module (e.g.,api/_firebase.ts) within theapi/directory to avoid Vercel cross-folder bundling issues noted in theisValidEmailcomment.🤖 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 `@api/razorpay.ts` around lines 15 - 29, The initAdmin() logic is duplicated here and in the other API route, so update the Firebase Admin initialization to use a shared helper instead of keeping two copies. Extract the existing initAdmin() implementation into a common module under api/ (for example, a shared firebase helper) and import it from both razorpay.ts and quote.ts, preserving the current getApps(), initializeApp(), and error handling behavior.
🤖 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.
Outside diff comments:
In `@api/quote.ts`:
- Around line 116-170: The rate-limit consumption and lead persistence in the
quote flow are split between separate operations, so a failed lead save can
still burn a submission slot. Update the quote handler in quote.ts to perform
the `messages` write inside the same `db.runTransaction` block that uses
`rateLimitRef` so the slot update and lead creation are atomic. Use the existing
symbols `runTransaction`, `rateLimitRef`, `buildQuoteSummary`, and
`buildMessageData` to keep the logic together, and make sure the transaction
commits both actions or fails without consuming the slot; also update the quote
endpoint test mock to support the transaction write path.
---
Nitpick comments:
In `@api/razorpay.ts`:
- Around line 15-29: The initAdmin() logic is duplicated here and in the other
API route, so update the Firebase Admin initialization to use a shared helper
instead of keeping two copies. Extract the existing initAdmin() implementation
into a common module under api/ (for example, a shared firebase helper) and
import it from both razorpay.ts and quote.ts, preserving the current getApps(),
initializeApp(), and error handling behavior.
In `@src/test/quote-endpoint.test.ts`:
- Around line 19-54: The `quote-endpoint.test.ts` suite is missing failure-path
coverage for fail-closed behavior in the handler and init flow. Add tests that
force `runTransaction` to throw and assert a 500, make the lead write path fail
(either `messages.add` or the transaction write path used in `api/quote.ts`) and
assert the error response, and make the email notification write fail while
still asserting a 200. Also add coverage for the `OPTIONS` preflight response
and an `initAdmin()` failure by overriding the `getApps`/`initializeApp` mocks
to exercise the error path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5919f9a4-9670-43b3-9cab-899a27c3be99
📒 Files selected for processing (9)
.env.exampleapi/estimate.tsapi/quote.tsapi/razorpay.tssrc/admin/hooks/useAdminData.tssrc/admin/pages/Messages.tsxsrc/app/components/QuoteForm.tsxsrc/dashboard/pages/PaymentManagement.tsxsrc/test/quote-endpoint.test.ts
✅ Files skipped from review due to trivial changes (1)
- .env.example
🚧 Files skipped from review as they are similar to previous changes (5)
- src/dashboard/pages/PaymentManagement.tsx
- src/app/components/QuoteForm.tsx
- api/estimate.ts
- src/admin/pages/Messages.tsx
- src/admin/hooks/useAdminData.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Client-side rate limiting (in
localStorage) can be bypassed by clearing storage, using incognito, or switching browsers/devices. This PR adds authoritative server-side enforcement.api/quote.tsnow owns all Firestore writes. It re-runs full field validation and enforces a 3-submissions-per-10-minutes sliding window keyed by a keyed-HMAC hash of the caller IP, evaluated inside a Firestore transaction so concurrent requests can't slip past the cap. Any enforcement failure fails closed (429/500), never open.submitQuote.tsnow POSTs to/api/quoteinstead of writing to Firestore directly, and throws a typedRateLimitErroron a 429 soQuoteFormcan show the server's message.localStoragetracking is kept as a UX hint only — it is no longer the enforcement boundary. A user who clears storage still hits the server limit.Deployment note
Requires two server env vars (documented in
.env.example), set in Vercel → Settings → Environment Variables:RATE_LIMIT_HASH_SECRET— any long random string (e.g.openssl rand -hex 32)FIREBASE_SERVICE_ACCOUNT— Firebase Admin service-account JSONChanges
api/quote.tsvalidateFields, writes lead + mail via Firebase Admin; fails closed on any init/enforcement errorapi/razorpay.tsinitAdmin()guardsrc/app/lib/submitQuote.ts/api/quote; addsRateLimitError; keeps pure helpers so the server can reuse themsrc/app/lib/submitQuote.test.tsfetch-based tests +RateLimitErrorcasessrc/app/components/QuoteForm.tsxRateLimitErrorTest plan
npx tsc --noEmit— 0 errorsnpx vitest run— 313 / 313 passedlocalStorageand resubmitting is blocked independently by the serverCloses #239
Summary by CodeRabbit
New Features
Bug Fixes