fix(api): prevent payment amount spoofing in createOrder and add replay protection - #274
Merged
hrx01-dev merged 1 commit intoJul 9, 2026
Conversation
…replay protection - createOrder now checks the requested amount against the client's outstanding balance in Firestore (totalCost minus completed payments) so a malicious client cannot create orders for arbitrary amounts. - verifyPayment now rejects duplicate razorpay_payment_id values to prevent replay attacks that could inflate the paid balance. - Added test coverage for both new validations. Closes hrx01-dev#252
|
@Harsh-goswami-103 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. |
📝 WalkthroughWalkthroughThe ChangesRazorpay billing validation and replay protection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant createOrder
participant FirebaseAdmin
participant Firestore
Client->>createOrder: request order with amount
createOrder->>FirebaseAdmin: initAdmin()
FirebaseAdmin-->>createOrder: initialized (or fails -> 500)
createOrder->>Firestore: read projectBilling/<email>
Firestore-->>createOrder: billing doc (or missing -> 404)
createOrder->>createOrder: compute remaining = totalCost - paidSoFar
alt amount exceeds remaining
createOrder-->>Client: 400 amount exceeds balance
else amount within remaining
createOrder-->>Client: 200 order created
end
sequenceDiagram
participant Client
participant verifyPayment
participant Firestore
Client->>verifyPayment: submit razorpay_payment_id
verifyPayment->>Firestore: read existing payments
Firestore-->>verifyPayment: payments list
alt payment_id already recorded
verifyPayment-->>Client: 409 already recorded
else new payment
verifyPayment->>Firestore: update billing record
verifyPayment-->>Client: success response
end
Possibly related issues
Possibly related PRs
🚥 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: 1
🧹 Nitpick comments (1)
api/razorpay.ts (1)
122-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the shared collection constant here
"projectBilling"is defined insrc/admin/lib/collections.tsas the single source of truth, andapi/already imports fromsrc/in this repo. ReuseCOLLECTIONS.projectBillinghere (and at the other call site) to avoid silent drift if the collection name changes.🤖 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 122 - 125, The billing lookup in the Razorpay flow is hardcoding the projectBilling collection name instead of using the shared source of truth. Update the db.collection call in this path, using the existing COLLECTIONS.projectBilling constant from src/admin/lib/collections.ts, and make the same replacement at the other call site so both references stay aligned if the collection name changes.
🤖 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/razorpay.ts`:
- Around line 264-274: The replay protection in verifyPayment is not atomic
because the payments read and the billing update happen separately, so
concurrent requests can both pass the alreadyRecorded check and duplicate the
payment. Move the check and write into a Firestore transaction around
billingRef, or switch to a payment_id-keyed document and use create() so
uniqueness is enforced by the database. Keep the existing duplicate detection
logic centered on razorpay_payment_id and the payments collection update path.
---
Nitpick comments:
In `@api/razorpay.ts`:
- Around line 122-125: The billing lookup in the Razorpay flow is hardcoding the
projectBilling collection name instead of using the shared source of truth.
Update the db.collection call in this path, using the existing
COLLECTIONS.projectBilling constant from src/admin/lib/collections.ts, and make
the same replacement at the other call site so both references stay aligned if
the collection name changes.
🪄 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: 5aa2beb2-bf00-4a31-9228-cf48237816b2
📒 Files selected for processing (2)
api/razorpay.tssrc/test/razorpay-endpoint.test.ts
Comment on lines
+264
to
+274
| // Replay protection: reject if this payment ID was already recorded so a | ||
| // captured payment can't be submitted twice to inflate the paid balance. | ||
| const alreadyRecorded = payments.some( | ||
| (p) => p.reference === razorpay_payment_id || p.id === razorpay_payment_id | ||
| ); | ||
| if (alreadyRecorded) { | ||
| return res | ||
| .status(409) | ||
| .json({ error: "This payment has already been recorded" }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no transaction/runTransaction wraps the verifyPayment read-check-update,
# and that billingRef.get()/update() are the only guards.
ast-grep outline api/razorpay.ts --items all
rg -nP 'runTransaction|\.get\(\)|\.update\(|payments\.some' api/razorpay.tsRepository: hrx01-dev/Servio
Length of output: 814
Make the replay check atomic with the write
billingRef.get() and billingRef.update() are separate operations here, so two concurrent verifyPayment requests with the same razorpay_payment_id can both pass alreadyRecorded and append the payment. Move the read/check/update into a Firestore transaction, or use a payment_id-keyed doc with create() to enforce uniqueness.
🤖 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 264 - 274, The replay protection in
verifyPayment is not atomic because the payments read and the billing update
happen separately, so concurrent requests can both pass the alreadyRecorded
check and duplicate the payment. Move the check and write into a Firestore
transaction around billingRef, or switch to a payment_id-keyed document and use
create() so uniqueness is enforced by the database. Keep the existing duplicate
detection logic centered on razorpay_payment_id and the payments collection
update path.
hrx01-dev
self-requested a review
July 9, 2026 20:26
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.
Problem
Fixes #252
api/razorpay.ts→verifyPaymentalready validates the payment amount against Razorpay's server-side records (viapayments.fetch), which prevents the specific exploit of sending a spoofed largeamountin the request body.However, two related vulnerabilities remained:
1.
createOrderaccepts arbitrary amountsA malicious client could call
createOrderwith a tiny amount (e.g. ₹1), pay it legitimately, and thenverifyPaymentwould record that tiny amount — allowing underpayment to go undetected.2. No replay protection in
verifyPaymentA captured payment could be submitted multiple times, each time appending a new payment record and inflating the paid balance.
Fix
createOrder— Server-side amount validationcreateOrderpathtotalCost - sum(completed payments)verifyPayment— Replay protectionrazorpay_payment_idalready exists in the payments array (byidorreferencefield)Tests Added
createOrder: rejects when billing record is missingcreateOrder: rejects when amount exceeds outstanding balancecreateOrder: allows amount within outstanding balanceverifyPayment: rejects duplicate payment ID (replay protection)All 16 tests pass ✅
Files Changed
api/razorpay.ts— core fixsrc/test/razorpay-endpoint.test.ts— new test coverageSummary by CodeRabbit
Bug Fixes
Tests