Fix : Backend security issue for payment - #276
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughFirebase Admin initialization now receives an explicit project ID across estimate, quote, and Razorpay APIs, with a local-environment guard for Razorpay. Payment order creation now aborts requests exceeding 15 seconds. ChangesPayment infrastructure updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Visit the preview URL for this PR (updated for commit aab5aa5): https://servio-0--pr276-fix-backend-security-cok0r05q.web.app (expires Sat, 18 Jul 2026 13:03:48 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 15915abb5951eb298a844eda460b24f444d93a69 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/razorpay.ts (1)
22-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
initAdminto a shared module.The
initAdminfunction is duplicated across three API files with diverging implementations — different error log messages and onlyrazorpay.tshaving the local environment guard. Extract to a shared module to ensure consistent behavior and simplify future maintenance.
api/razorpay.ts#L22-L28: MoveinitAdmin(including the local guard) to a shared Firebase Admin init module and import from there.api/estimate.ts#L28-L30: Replace the localinitAdmincopy with an import from the shared module.api/quote.ts#L30-L32: Replace the localinitAdmincopy with an import from the shared module.🤖 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 22 - 28, The initAdmin implementation is duplicated and inconsistent across the API handlers. Extract initAdmin, including the local environment guard, into a shared Firebase Admin initialization module, then import and use it from api/razorpay.ts lines 22-28, api/estimate.ts lines 28-30, and api/quote.ts lines 30-32; remove each local copy so all handlers share the same behavior and error messaging.
🤖 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 22-28: The Firestore initialization in initAdmin() for
api/estimate.ts and api/quote.ts lacks the local credential guard. Add the same
non-production check used by razorpay.ts for missing FIREBASE_SERVICE_ACCOUNT
and GOOGLE_APPLICATION_CREDENTIALS, returning false before initializeApp;
alternatively, reuse a shared helper if one exists.
In `@src/dashboard/pages/PaymentManagement.tsx`:
- Around line 248-261: Update the payment request error handling around the
create-order fetch to detect AbortError and show a user-friendly timeout message
instead of passing the raw abort text to toast.error. Apply the same
AbortController and 15-second timeout pattern to the verifyPayment fetch,
ensuring each timeout is cleared after completion and its AbortError receives
the same timeout-specific handling.
---
Nitpick comments:
In `@api/razorpay.ts`:
- Around line 22-28: The initAdmin implementation is duplicated and inconsistent
across the API handlers. Extract initAdmin, including the local environment
guard, into a shared Firebase Admin initialization module, then import and use
it from api/razorpay.ts lines 22-28, api/estimate.ts lines 28-30, and
api/quote.ts lines 30-32; remove each local copy so all handlers share the same
behavior and error messaging.
🪄 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: 7108c240-6119-4053-86e1-de097c580a33
📒 Files selected for processing (4)
api/estimate.tsapi/quote.tsapi/razorpay.tssrc/dashboard/pages/PaymentManagement.tsx
| if (process.env.NODE_ENV !== "production" && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { | ||
| console.error("Missing FIREBASE_SERVICE_ACCOUNT in local environment. Cannot connect to Firestore."); | ||
| return false; | ||
| } | ||
| initializeApp({ | ||
| projectId: process.env.VITE_FIREBASE_PROJECT_ID || process.env.FIREBASE_PROJECT_ID, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if estimate and quote endpoints are called from the frontend in local development
rg -n "api/estimate\|api/quote" src/ --type=ts -g '!**/node_modules/**' -C2Repository: hrx01-dev/Servio
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== repo files ==\n'
git ls-files | rg '(^|/)(api/(razorpay|estimate|quote)\.ts|src/|pages/|app/|routes/|server/)' || true
printf '\n== locate relevant symbols ==\n'
rg -n "initAdmin|initializeApp|GOOGLE_APPLICATION_CREDENTIALS|FIREBASE_SERVICE_ACCOUNT|estimate|quote|razorpay" api src . -g '!**/node_modules/**' || true
printf '\n== file outlines ==\n'
for f in api/razorpay.ts api/estimate.ts api/quote.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
sed -n '1,260p' "$f" | cat -n | sed -n '1,260p'
fi
doneRepository: hrx01-dev/Servio
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in api/estimate.ts api/quote.ts api/razorpay.ts src/dashboard/services/estimationService.ts src/app/lib/submitQuote.ts; do
if [ -f "$f" ]; then
echo "===== $f ====="
nl -ba "$f" | sed -n '1,120p'
echo
fi
doneRepository: hrx01-dev/Servio
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in api/estimate.ts api/quote.ts api/razorpay.ts; do
echo "===== $f ====="
sed -n '1,120p' "$f" | cat -n
echo
done
echo "===== api/estimate.ts (around init/use) ====="
sed -n '240,270p' api/estimate.ts | cat -n
echo
echo "===== api/quote.ts (around init/use) ====="
sed -n '55,85p' api/quote.ts | cat -n
echo
echo "===== api/razorpay.ts (around init/use) ====="
sed -n '110,190p' api/razorpay.ts | cat -nRepository: hrx01-dev/Servio
Length of output: 23392
🌐 Web query:
Firebase Admin SDK initializeApp projectId getFirestore without GOOGLE_APPLICATION_CREDENTIALS local development hang application default credentials
💡 Result:
The Firebase Admin SDK requires valid credentials to communicate with Google Cloud services. When initializing the SDK in a local development environment without the GOOGLE_APPLICATION_CREDENTIALS environment variable or explicit service account credentials, the SDK attempts to use Application Default Credentials (ADC) [1][2]. If ADC are not configured (e.g., via gcloud auth application-default login), the SDK may attempt to reach the Google metadata server (metadata.google.internal) to fetch credentials [3][4]. If the environment is not a Google-managed infrastructure (like Cloud Functions or Compute Engine), this attempt fails because the metadata server is unreachable, leading to errors or long-running requests that may hang while awaiting a response [3][5]. To resolve this during local development, use one of the following approaches: 1. Service Account Key: Download a service account JSON key from your Firebase Project Settings and set the GOOGLE_APPLICATION_CREDENTIALS environment variable to the path of this file [6][2][7]. 2. gcloud CLI: Run gcloud auth application-default login to set up local ADC credentials [8]. Note that for some services like Firebase Authentication, these end-user credentials may have limitations; in such cases, providing a Project ID is required, and sometimes additional configuration is necessary [8]. 3. Emulators: If you are connecting to local Firebase Emulators, ensure you have set the relevant emulator host environment variables (e.g., FIRESTORE_EMULATOR_HOST) [9][10][11]. Even when using emulators, the Admin SDK typically requires a Project ID to be defined, which can be provided via the projectId option in initializeApp or the GCLOUD_PROJECT environment variable [9][10][12]. If your application hangs specifically when calling getFirestore, ensure you have properly configured the SDK with valid credentials or, if working against the emulator, that your environment variables (like FIRESTORE_EMULATOR_HOST) are correctly set before the Firestore client is instantiated [9][10]. Moving client initialization out of the global scope can also help prevent issues where the SDK attempts to initialize before your environment configuration is fully loaded [13].
Citations:
- 1: https://firebase.google.com/docs/reference/admin/node/firebase-admin.app
- 2: Initializing with environment variables only? firebase/firebase-admin-node#2043
- 3: Cannot
initializeAppon local machine: ENOTFOUND metadata.google.internal firebase/firebase-admin-node#745 - 4: "Error: Could not load the default credentials" after upgrading to 8.0.0+ firebase/firebase-admin-node#738
- 5: When no credentials are available, firestore call never returns. googleapis/nodejs-firestore#1418
- 6: https://firebase.google.com/docs/admin/setup
- 7: https://stackoverflow.com/questions/57140878/how-can-i-admin-initializeapp-no-arguments-in-local
- 8: https://stackoverflow.com/questions/78867047/firebase-authentication-not-working-with-adc-application-default-credentials
- 9: Firestore SDK still requires credentials even if the target is the emulator googleapis/nodejs-firestore#998
- 10: [Firestore] Can not connect to both production and local emulator database firebase/firebase-admin-node#1839
- 11: https://stackoverflow.com/questions/71268856/how-to-connect-firebase-admin-to-emulator-auth
- 12: https://stackoverflow.com/questions/79336721/where-does-firebase-admin-sdk-firestore-pick-up-its-project-id-from-if-not-sp
- 13: Error: Failed to initialize Google Cloud Firestore client with the available credentials. Must initialize the SDK with a certificate credential or application default credentials to use Cloud Firestore API firebase/firebase-admin-node#763
Add the local Firestore guard to api/estimate.ts and api/quote.ts initAdmin() in both handlers still falls back to ADC/projectId-only init, so local requests without FIREBASE_SERVICE_ACCOUNT or GOOGLE_APPLICATION_CREDENTIALS can still stall when they hit Firestore. Mirror the api/razorpay.ts guard here or extract it into a shared helper.
🤖 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 22 - 28, The Firestore initialization in
initAdmin() for api/estimate.ts and api/quote.ts lacks the local credential
guard. Add the same non-production check used by razorpay.ts for missing
FIREBASE_SERVICE_ACCOUNT and GOOGLE_APPLICATION_CREDENTIALS, returning false
before initializeApp; alternatively, reuse a shared helper if one exists.
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), 15000); | ||
|
|
||
| const orderRes = await fetch(`${baseUrl}/api/razorpay?action=createOrder`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| amount: amountToPay, | ||
| clientEmail: auth.currentUser.email, | ||
| }), | ||
| signal: controller.signal, | ||
| }); | ||
| clearTimeout(timeoutId); | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Handle AbortError with a user-friendly timeout message.
When the 15-second timeout fires, fetch throws an AbortError with the raw message "The operation was aborted." — the catch block at line 308 passes this directly to toast.error, which is not user-friendly. Check for AbortError and show a timeout-specific message.
Consider also applying the same timeout pattern to the verifyPayment fetch (lines 276-287), which currently has no abort or timeout handling and could hang indefinitely.
🔧 Proposed fix for AbortError handling
} catch (error: unknown) {
const err = error as Error;
- toast.error(err.message || "Failed to initiate payment");
+ if (err.name === "AbortError") {
+ toast.error("Request timed out. Please try again.");
+ } else {
+ toast.error(err.message || "Failed to initiate payment");
+ }
} finally {
setIsProcessing(false);
}🤖 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/dashboard/pages/PaymentManagement.tsx` around lines 248 - 261, Update the
payment request error handling around the create-order fetch to detect
AbortError and show a user-friendly timeout message instead of passing the raw
abort text to toast.error. Apply the same AbortController and 15-second timeout
pattern to the verifyPayment fetch, ensuring each timeout is cleared after
completion and its AbortError receives the same timeout-specific handling.
description
Summary by CodeRabbit