(SP: 3) [Security] Harden admin CSRF gate + add DB-backed rate limiting for checkout and Stripe webhooks #154
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,11 @@ import { | |
| appendRefundToMeta, | ||
| } from '@/lib/services/orders/psp-metadata/refunds'; | ||
| import { markStripeAttemptFinal } from '@/lib/services/orders/payment-attempts'; | ||
| import { | ||
| enforceRateLimit, | ||
| getClientIp, | ||
| rateLimitResponse, | ||
| } from '@/lib/security/rate-limit'; | ||
|
|
||
| const REFUND_FULLNESS_UNDETERMINED = 'REFUND_FULLNESS_UNDETERMINED' as const; | ||
|
|
||
|
|
@@ -312,6 +317,22 @@ export async function POST(request: NextRequest) { | |
|
|
||
| const signature = request.headers.get('stripe-signature'); | ||
| if (!signature) { | ||
| const ip = getClientIp(request) ?? 'anon'; | ||
| const decision = await enforceRateLimit({ | ||
| key: `stripe_webhook:missing_sig:${ip}`, | ||
|
Comment on lines
+320
to
+322
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: # First, find the implementation of getClientIp
find . -type f -name "*.ts" -o -name "*.tsx" -o -name "*.js" | head -20Repository: DevLoversTeam/devlovers.net Length of output: 934 🏁 Script executed: # Search for getClientIp implementation
rg "function getClientIp|const getClientIp|export.*getClientIp" --type ts --type tsx --type jsRepository: DevLoversTeam/devlovers.net Length of output: 98 🏁 Script executed: # Look at the Stripe webhook route file
wc -l frontend/app/api/shop/webhooks/stripe/route.tsRepository: DevLoversTeam/devlovers.net Length of output: 122 🏁 Script executed: # Read the specific lines mentioned in the review
sed -n '310,330p' frontend/app/api/shop/webhooks/stripe/route.tsRepository: DevLoversTeam/devlovers.net Length of output: 746 🏁 Script executed: # Also check lines 356-358
sed -n '350,365p' frontend/app/api/shop/webhooks/stripe/route.tsRepository: DevLoversTeam/devlovers.net Length of output: 551 🏁 Script executed: # Search for where enforceRateLimit and getClientIp are imported from
head -50 frontend/app/api/shop/webhooks/stripe/route.tsRepository: DevLoversTeam/devlovers.net Length of output: 1748 🏁 Script executed: # Find the rate-limit utility file
find . -path "*/lib/security/rate-limit*" -type fRepository: DevLoversTeam/devlovers.net Length of output: 108 🏁 Script executed: # Search for the rate-limit file more broadly
fd "rate-limit" --type fRepository: DevLoversTeam/devlovers.net Length of output: 106 🏁 Script executed: # Search for the lib/security directory
ls -la frontend/lib/security/ 2>/dev/null || echo "Path not found, searching..."Repository: DevLoversTeam/devlovers.net Length of output: 370 🏁 Script executed: # Search more broadly for getClientIp implementation
rg "getClientIp" --type ts --type js -A 5 -B 2Repository: DevLoversTeam/devlovers.net Length of output: 3896 🏁 Script executed: # Read the full rate-limit.ts file to see the complete getClientIp implementation
cat -n frontend/lib/security/rate-limit.tsRepository: DevLoversTeam/devlovers.net Length of output: 5200 IP-spoofing risk requires deployment boundary verification.
Action: Document your deployment boundary (e.g., "deployed behind Cloudflare" or "reverse proxy at Also applies to: 356-358 🤖 Prompt for AI Agents |
||
| limit: Number(process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_MAX ?? 30), | ||
| windowSeconds: Number( | ||
| process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_WINDOW_SECONDS ?? 60 | ||
| ), | ||
|
Comment on lines
+323
to
+326
Contributor
There was a problem hiding this comment. Guard against NaN in env‑driven limits.
🛠️ Proposed fix+function readIntEnv(name: string, fallback: number) {
+ const raw = process.env[name];
+ const parsed = Number.parseInt(raw ?? '', 10);
+ return Number.isFinite(parsed) ? parsed : fallback;
+}
@@
- const decision = await enforceRateLimit({
- key: `stripe_webhook:missing_sig:${ip}`,
- limit: Number(process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_MAX ?? 30),
- windowSeconds: Number(
- process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_WINDOW_SECONDS ?? 60
- ),
- });
+ const decision = await enforceRateLimit({
+ key: `stripe_webhook:missing_sig:${ip}`,
+ limit: readIntEnv('STRIPE_WEBHOOK_INVALID_SIG_RL_MAX', 30),
+ windowSeconds: readIntEnv(
+ 'STRIPE_WEBHOOK_INVALID_SIG_RL_WINDOW_SECONDS',
+ 60
+ ),
+ });
@@
- const decision = await enforceRateLimit({
- key: `stripe_webhook:invalid_sig:${ip}`,
- limit: Number(process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_MAX ?? 30),
- windowSeconds: Number(
- process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_WINDOW_SECONDS ?? 60
- ),
- });
+ const decision = await enforceRateLimit({
+ key: `stripe_webhook:invalid_sig:${ip}`,
+ limit: readIntEnv('STRIPE_WEBHOOK_INVALID_SIG_RL_MAX', 30),
+ windowSeconds: readIntEnv(
+ 'STRIPE_WEBHOOK_INVALID_SIG_RL_WINDOW_SECONDS',
+ 60
+ ),
+ });Also applies to: 359-362 🤖 Prompt for AI Agents |
||
| }); | ||
|
|
||
| if (!decision.ok) { | ||
| return rateLimitResponse({ | ||
| retryAfterSeconds: decision.retryAfterSeconds, | ||
| details: { scope: 'stripe_webhook', reason: 'missing_signature' }, | ||
| }); | ||
| } | ||
|
|
||
| logError( | ||
| 'Stripe webhook missing signature header', | ||
| new Error('MISSING_STRIPE_SIGNATURE') | ||
|
|
@@ -332,6 +353,22 @@ export async function POST(request: NextRequest) { | |
| error instanceof Error && | ||
| error.message === 'STRIPE_INVALID_SIGNATURE' | ||
| ) { | ||
| const ip = getClientIp(request) ?? 'anon'; | ||
| const decision = await enforceRateLimit({ | ||
| key: `stripe_webhook:invalid_sig:${ip}`, | ||
| limit: Number(process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_MAX ?? 30), | ||
| windowSeconds: Number( | ||
| process.env.STRIPE_WEBHOOK_INVALID_SIG_RL_WINDOW_SECONDS ?? 60 | ||
| ), | ||
| }); | ||
|
|
||
| if (!decision.ok) { | ||
| return rateLimitResponse({ | ||
| retryAfterSeconds: decision.retryAfterSeconds, | ||
| details: { scope: 'stripe_webhook', reason: 'invalid_signature' }, | ||
| }); | ||
| } | ||
|
|
||
| logError('Stripe webhook signature verification failed', error); | ||
| return NextResponse.json({ code: 'INVALID_SIGNATURE' }, { status: 400 }); | ||
| } | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,51 +5,68 @@ import { useId, useState } from 'react'; | |
| interface AdminProductStatusToggleProps { | ||
| id: string; | ||
| initialIsActive: boolean; | ||
| csrfToken: string; | ||
| } | ||
|
|
||
| export function AdminProductStatusToggle({ | ||
| id, | ||
| initialIsActive, | ||
| csrfToken, | ||
| }: AdminProductStatusToggleProps) { | ||
| const [isActive, setIsActive] = useState(initialIsActive); | ||
| const [isLoading, setIsLoading] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| const errorId = useId(); | ||
|
|
||
| const toggleStatus = async () => { | ||
| setIsLoading(true); | ||
| setError(null); | ||
|
|
||
| if (!csrfToken) { | ||
| setError('Security token missing. Refresh the page.'); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const response = await fetch(`/api/shop/admin/products/${id}/status`, { | ||
| method: 'PATCH', | ||
| headers: { | ||
| 'x-csrf-token': csrfToken, | ||
| }, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| let code: string | undefined; | ||
| try { | ||
| const body = await response.json(); | ||
| code = typeof body?.code === 'string' ? body.code : undefined; | ||
| } catch { | ||
| // ignore | ||
| } | ||
|
|
||
| if (response.status === 403 && (code === 'CSRF_MISSING' || code === 'CSRF_INVALID')) { | ||
| setError('Security token expired. Refresh the page and retry.'); | ||
| return; | ||
| } | ||
|
|
||
| setError('Failed to update status'); | ||
| return; | ||
| } | ||
|
|
||
| const data: { product?: { isActive?: boolean } } = await response.json(); | ||
| if (typeof data.product?.isActive === 'boolean') { | ||
| setIsActive(data.product.isActive); | ||
| } | ||
| } catch { | ||
| setError('Failed to update status'); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const buttonLabel = isLoading ? 'Updating' : isActive ? 'Deactivate' : 'Activate'; | ||
|
|
||
| return ( | ||
| <div className="flex min-w-0 flex-col gap-1"> | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refund endpoint now hard-requires
requireAdminCsrf, but the only caller (frontend/app/[locale]/shop/admin/orders/[id]/RefundButton.tsx) still posts withoutx-csrf-tokenorcsrfTokenform field. That means every admin refund will now return403 CSRF_MISSINGeven for valid admins, effectively breaking refunds in the UI. Either pass an issued token intoRefundButtonand set the header, or allow this specific endpoint to accept the existing request format.Useful? React with 👍 / 👎.