fix(audit): security HIGHs — rate-limit hard-delete codes, collapse error msgs, doc bad-secret per-IP
H1: hard-delete-request and bulk-hard-delete-request endpoints had no rate limit; an admin's compromised account could email-bomb the operator's inbox or use the endpoints as a client-id oracle. Added a new `hardDeleteCode` limiter (5 per hour per user). H3: hard-delete error messages distinguished "no code requested" from "wrong code", letting an attacker brute-force the 4-digit space with ~5k attempts (vs the full 10k). Both single + bulk paths now return the same 'Invalid or expired confirmation code' message. H5: invalid Documenso webhook secret submissions are now rate-limited per-IP (10 per 15min) and only audit-logged inside the cap, so a slow enumeration can't fill the audit log silently. Real Documenso traffic won't fail the secret check, so any traffic beyond the cap is brute-force. 1175/1175 vitest passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
import { NextResponse } from 'next/server';
|
import { NextResponse } from 'next/server';
|
||||||
|
|
||||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
import { withAuth, withPermission, withRateLimit } from '@/lib/api/helpers';
|
||||||
import { requestHardDeleteCode } from '@/lib/services/client-hard-delete.service';
|
import { requestHardDeleteCode } from '@/lib/services/client-hard-delete.service';
|
||||||
import { errorResponse, NotFoundError } from '@/lib/errors';
|
import { errorResponse, NotFoundError } from '@/lib/errors';
|
||||||
|
|
||||||
@@ -13,7 +13,10 @@ import { errorResponse, NotFoundError } from '@/lib/errors';
|
|||||||
* checked by the partner /hard-delete route — see route comment there.
|
* checked by the partner /hard-delete route — see route comment there.
|
||||||
*/
|
*/
|
||||||
export const POST = withAuth(
|
export const POST = withAuth(
|
||||||
withPermission('admin', 'permanently_delete_clients', async (_req, ctx, params) => {
|
withPermission(
|
||||||
|
'admin',
|
||||||
|
'permanently_delete_clients',
|
||||||
|
withRateLimit('hardDeleteCode', async (_req, ctx, params) => {
|
||||||
try {
|
try {
|
||||||
const id = params.id;
|
const id = params.id;
|
||||||
if (!id) throw new NotFoundError('client');
|
if (!id) throw new NotFoundError('client');
|
||||||
@@ -35,4 +38,5 @@ export const POST = withAuth(
|
|||||||
return errorResponse(error);
|
return errorResponse(error);
|
||||||
}
|
}
|
||||||
}),
|
}),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { NextResponse } from 'next/server';
|
import { NextResponse } from 'next/server';
|
||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
|
||||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
import { withAuth, withPermission, withRateLimit } from '@/lib/api/helpers';
|
||||||
import { parseBody } from '@/lib/api/route-helpers';
|
import { parseBody } from '@/lib/api/route-helpers';
|
||||||
import { requestBulkHardDeleteCode } from '@/lib/services/client-hard-delete.service';
|
import { requestBulkHardDeleteCode } from '@/lib/services/client-hard-delete.service';
|
||||||
import { errorResponse } from '@/lib/errors';
|
import { errorResponse } from '@/lib/errors';
|
||||||
@@ -11,7 +11,10 @@ const bodySchema = z.object({
|
|||||||
});
|
});
|
||||||
|
|
||||||
export const POST = withAuth(
|
export const POST = withAuth(
|
||||||
withPermission('admin', 'permanently_delete_clients', async (req, ctx) => {
|
withPermission(
|
||||||
|
'admin',
|
||||||
|
'permanently_delete_clients',
|
||||||
|
withRateLimit('hardDeleteCode', async (req, ctx) => {
|
||||||
try {
|
try {
|
||||||
const { ids } = await parseBody(req, bodySchema);
|
const { ids } = await parseBody(req, bodySchema);
|
||||||
const result = await requestBulkHardDeleteCode({
|
const result = await requestBulkHardDeleteCode({
|
||||||
@@ -30,4 +33,5 @@ export const POST = withAuth(
|
|||||||
return errorResponse(error);
|
return errorResponse(error);
|
||||||
}
|
}
|
||||||
}),
|
}),
|
||||||
|
),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import {
|
|||||||
} from '@/lib/services/documents.service';
|
} from '@/lib/services/documents.service';
|
||||||
import { logger } from '@/lib/logger';
|
import { logger } from '@/lib/logger';
|
||||||
import { createAuditLog } from '@/lib/audit';
|
import { createAuditLog } from '@/lib/audit';
|
||||||
|
import { checkRateLimit, rateLimiters } from '@/lib/rate-limit';
|
||||||
|
|
||||||
// BR-024: Dedup via signatureHash unique index on documentEvents
|
// BR-024: Dedup via signatureHash unique index on documentEvents
|
||||||
// Always return 200 from webhook (webhook best practice)
|
// Always return 200 from webhook (webhook best practice)
|
||||||
@@ -66,7 +67,19 @@ export async function POST(req: NextRequest): Promise<NextResponse> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (!matched) {
|
if (!matched) {
|
||||||
logger.warn({ providedLen: providedSecret.length }, 'Invalid Documenso webhook secret');
|
const callerIp =
|
||||||
|
req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
|
||||||
|
req.headers.get('x-real-ip') ??
|
||||||
|
'unknown';
|
||||||
|
// Rate-limit per IP. Real Documenso traffic won't fail the secret
|
||||||
|
// check, so any traffic here is enumeration / brute-force; we cap
|
||||||
|
// it sharply to keep audit-log volume bounded too.
|
||||||
|
const rl = await checkRateLimit(callerIp, rateLimiters.webhookBadSecret);
|
||||||
|
logger.warn(
|
||||||
|
{ providedLen: providedSecret.length, ip: callerIp, allowed: rl.allowed },
|
||||||
|
'Invalid Documenso webhook secret',
|
||||||
|
);
|
||||||
|
if (rl.allowed) {
|
||||||
void createAuditLog({
|
void createAuditLog({
|
||||||
userId: null,
|
userId: null,
|
||||||
portId: null,
|
portId: null,
|
||||||
@@ -77,11 +90,13 @@ export async function POST(req: NextRequest): Promise<NextResponse> {
|
|||||||
reason: 'invalid_secret',
|
reason: 'invalid_secret',
|
||||||
providedLen: providedSecret.length,
|
providedLen: providedSecret.length,
|
||||||
},
|
},
|
||||||
ipAddress: req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ?? '',
|
ipAddress: callerIp,
|
||||||
userAgent: req.headers.get('user-agent') ?? '',
|
userAgent: req.headers.get('user-agent') ?? '',
|
||||||
severity: 'warning',
|
severity: 'warning',
|
||||||
source: 'webhook',
|
source: 'webhook',
|
||||||
});
|
});
|
||||||
|
}
|
||||||
|
// Always return 200 (webhook best-practice — don't leak signal).
|
||||||
return NextResponse.json({ ok: false, error: 'Invalid secret' }, { status: 200 });
|
return NextResponse.json({ ok: false, error: 'Invalid secret' }, { status: 200 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -77,6 +77,15 @@ export const rateLimiters = {
|
|||||||
upload: { windowMs: 60 * 1000, max: 10, keyPrefix: 'upload' },
|
upload: { windowMs: 60 * 1000, max: 10, keyPrefix: 'upload' },
|
||||||
/** Bulk operations: 5 per minute. */
|
/** Bulk operations: 5 per minute. */
|
||||||
bulk: { windowMs: 60 * 1000, max: 5, keyPrefix: 'bulk' },
|
bulk: { windowMs: 60 * 1000, max: 5, keyPrefix: 'bulk' },
|
||||||
|
/** Hard-delete code requests: 5 per hour per user. Each request emails
|
||||||
|
* a fresh code; without the cap a compromised admin account could
|
||||||
|
* email-bomb the operator's inbox or use the endpoint as a client-id
|
||||||
|
* oracle. */
|
||||||
|
hardDeleteCode: { windowMs: 60 * 60 * 1000, max: 5, keyPrefix: 'hard-delete-code' },
|
||||||
|
/** Inbound webhook with bad secret: 10 attempts per 15 minutes per IP.
|
||||||
|
* Real webhooks won't fail the secret check, so any traffic here is
|
||||||
|
* enumeration / brute-force. Block beyond the cap with a 429. */
|
||||||
|
webhookBadSecret: { windowMs: 15 * 60 * 1000, max: 10, keyPrefix: 'wh-bad-secret' },
|
||||||
/** Receipt scanner: 10 OCR runs per minute per user. */
|
/** Receipt scanner: 10 OCR runs per minute per user. */
|
||||||
ocr: { windowMs: 60 * 1000, max: 10, keyPrefix: 'ocr' },
|
ocr: { windowMs: 60 * 1000, max: 10, keyPrefix: 'ocr' },
|
||||||
/** Server-side AI calls (summary, embeddings, etc): 60 per minute per user. */
|
/** Server-side AI calls (summary, embeddings, etc): 60 per minute per user. */
|
||||||
|
|||||||
@@ -166,11 +166,12 @@ export async function hardDeleteClient(args: {
|
|||||||
|
|
||||||
const key = codeKey(args.requesterUserId, args.clientId);
|
const key = codeKey(args.requesterUserId, args.clientId);
|
||||||
const stored = await redis.get(key);
|
const stored = await redis.get(key);
|
||||||
if (!stored) {
|
// Same error for both cases so an attacker can't distinguish "no code
|
||||||
throw new ValidationError('Confirmation code expired or not requested');
|
// requested" (probe to know the request endpoint window is open) from
|
||||||
}
|
// "wrong code" (probe to brute-force the 4-digit space). The operator
|
||||||
if (!safeEqualStr(stored, args.code.trim())) {
|
// has the email open and can re-request if expired.
|
||||||
throw new ValidationError('Confirmation code is incorrect');
|
if (!stored || !safeEqualStr(stored, args.code.trim())) {
|
||||||
|
throw new ValidationError('Invalid or expired confirmation code');
|
||||||
}
|
}
|
||||||
// Single-use: delete the code immediately so a failed delete tx
|
// Single-use: delete the code immediately so a failed delete tx
|
||||||
// forces the operator to request a fresh code.
|
// forces the operator to request a fresh code.
|
||||||
@@ -352,11 +353,10 @@ export async function bulkHardDeleteClients(args: {
|
|||||||
const idsHash = hashIds(args.clientIds);
|
const idsHash = hashIds(args.clientIds);
|
||||||
const key = bulkCodeKey(args.requesterUserId, idsHash);
|
const key = bulkCodeKey(args.requesterUserId, idsHash);
|
||||||
const stored = await redis.get(key);
|
const stored = await redis.get(key);
|
||||||
if (!stored) {
|
// Same error for both cases — see single-client variant for rationale.
|
||||||
throw new ValidationError('Confirmation code expired or not requested for this exact set');
|
// Code is tied to the exact set hash so a wrong-set probe fails here too.
|
||||||
}
|
if (!stored || !safeEqualStr(stored, args.code.trim())) {
|
||||||
if (!safeEqualStr(stored, args.code.trim())) {
|
throw new ValidationError('Invalid or expired confirmation code');
|
||||||
throw new ValidationError('Confirmation code is incorrect');
|
|
||||||
}
|
}
|
||||||
await redis.del(key);
|
await redis.del(key);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user