fix: audit fixes — tier-quota bypass, webhook SSRF, auth hardening, pagination, a11y
Full audit (bugs/UI-UX/backend/feature-gap) turned up a money-leak AI quota bypass, webhook SSRF, and a long tail of missing pagination/auth/a11y work. Fixes land together since HANDOFF.md tracked them as one backlog. - AI routes charge tier quota before generating; nutrition POST is author-only - Webhook dispatch re-validates URL per delivery (SSRF/DNS-rebinding), treats redirects as failures; recipe.published now actually dispatches - New indexes/unique constraints on recipes, meal-planning, comments FK cascade - Recipe PUT/restore snapshot only inside the transaction, after validation - Recipe DELETE cleans up S3 objects (recipe + review photos) - Optimistic UI (favorite/star/follow/shopping-list) rolls back on failure - Upload presign enforces file size cap + per-tier storage quota - Route-level loading/error/not-found states across (app), admin, and root - middleware.ts guards (app)/admin; requireAdmin checks DB role, not cached session; rate limiting applied to both session and API-key branches, bucketed per key; Stripe webhook dedupes by event id - Pagination added to recipes, feed, profile, comments, pantry, admin tables - Nav shows real avatar + profile link + dark-mode toggle; destructive actions standardized on AlertDialog - Unsaved-changes guard + real ingredient/step validation on recipe form; canonical /recipes/[id] used in-app; next/image migration; aria-labels and alt text across icon buttons, avatars, recipe photos - packages/api-types removed (zero callers, too drifted to safely rewire); openapi.ts and ai-keys error shape drift fixed; BYOK decrypt failures now surface instead of silently falling back to the platform key Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ vi.mock("@epicure/db", () => ({
|
||||
and: vi.fn((...args) => args),
|
||||
}));
|
||||
|
||||
const { getDefaultProviderWithKey, getModelConfigForUseCase, withUserKey } = await import("../resolve-user-key");
|
||||
const { getDefaultProviderWithKey, getModelConfigForUseCase, withUserKey, ByokDecryptError } = await import("../resolve-user-key");
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -74,15 +74,13 @@ describe("getDefaultProviderWithKey", () => {
|
||||
expect(config.apiKey).toBe("sk-from-site-settings");
|
||||
});
|
||||
|
||||
it("skips corrupted BYOK key and tries next provider", async () => {
|
||||
it("throws ByokDecryptError instead of silently falling back on a corrupted BYOK key", async () => {
|
||||
mockUserAiKeysFindMany.mockResolvedValue([
|
||||
{ provider: "openrouter", encryptedKey: "CORRUPT:NOT:VALID" },
|
||||
{ provider: "openai", encryptedKey: encrypt("sk-valid") },
|
||||
]);
|
||||
|
||||
const config = await getDefaultProviderWithKey("user1");
|
||||
expect(config.provider).toBe("openai");
|
||||
expect(config.apiKey).toBe("sk-valid");
|
||||
await expect(getDefaultProviderWithKey("user1")).rejects.toThrow(ByokDecryptError);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { NextResponse } from "next/server";
|
||||
import { APICallError } from "ai";
|
||||
import { checkAndIncrementTierLimit, refundAiCall, TierLimitError } from "@/lib/tiers";
|
||||
import { ByokDecryptError } from "@/lib/ai/resolve-user-key";
|
||||
|
||||
/**
|
||||
* Maps an AI-call failure to a clean JSON response instead of letting the
|
||||
@@ -23,6 +24,22 @@ export function aiErrorResponse(err: unknown): NextResponse {
|
||||
return NextResponse.json({ error: "AI request failed. Please try again." }, { status: 502 });
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a BYOK/model config and converts a decrypt failure into a clean
|
||||
* 400 response instead of letting it throw uncaught (or letting the caller
|
||||
* silently fall back to the platform key/billing).
|
||||
*/
|
||||
export async function resolveAiConfigOrError<T>(resolve: () => Promise<T>): Promise<{ ok: true; data: T } | { ok: false; response: NextResponse }> {
|
||||
try {
|
||||
return { ok: true, data: await resolve() };
|
||||
} catch (err) {
|
||||
if (err instanceof ByokDecryptError) {
|
||||
return { ok: false, response: NextResponse.json({ error: err.message }, { status: 400 }) };
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
type QuotaResult<T> = { ok: true; data: T } | { ok: false; response: NextResponse };
|
||||
|
||||
/**
|
||||
|
||||
@@ -5,6 +5,20 @@ import type { AiConfig, AiProvider } from "./factory";
|
||||
|
||||
export type ModelUseCase = "text" | "vision" | "mealPlan";
|
||||
|
||||
/**
|
||||
* Thrown when a user's saved BYOK key exists but fails to decrypt (e.g. the
|
||||
* encryption secret rotated, or the stored ciphertext is corrupt). Callers
|
||||
* must surface this to the user instead of silently falling back to the
|
||||
* platform key — otherwise the user believes their own key/billing is being
|
||||
* used when it is not.
|
||||
*/
|
||||
export class ByokDecryptError extends Error {
|
||||
constructor(public readonly provider: string) {
|
||||
super(`Failed to decrypt saved API key for provider "${provider}". Please re-enter it in Settings.`);
|
||||
this.name = "ByokDecryptError";
|
||||
}
|
||||
}
|
||||
|
||||
export async function withUserKey(userId: string, config: AiConfig): Promise<AiConfig> {
|
||||
const provider = config.provider;
|
||||
if (!provider) return config;
|
||||
@@ -19,7 +33,7 @@ export async function withUserKey(userId: string, config: AiConfig): Promise<AiC
|
||||
const apiKey = decrypt(row.encryptedKey);
|
||||
return { ...config, apiKey };
|
||||
} catch {
|
||||
return config;
|
||||
throw new ByokDecryptError(provider);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,7 +70,7 @@ export async function getDefaultProviderWithKey(userId: string): Promise<AiConfi
|
||||
try {
|
||||
return { provider: p, apiKey: decrypt(row.encryptedKey) };
|
||||
} catch {
|
||||
continue;
|
||||
throw new ByokDecryptError(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,7 +16,17 @@ export async function requireSession() {
|
||||
export async function requireAdmin() {
|
||||
const { session, response } = await requireSession();
|
||||
if (response) return { session: null, response };
|
||||
if (session!.user.role !== "admin") {
|
||||
|
||||
// Don't trust session.user.role — it comes from a 5-minute cookieCache
|
||||
// (see lib/auth/server.ts), so a just-demoted admin would keep access for
|
||||
// up to 5 minutes. Query the current role directly.
|
||||
const [dbUser] = await db
|
||||
.select({ role: users.role })
|
||||
.from(users)
|
||||
.where(eq(users.id, session!.user.id))
|
||||
.limit(1);
|
||||
|
||||
if (dbUser?.role !== "admin") {
|
||||
return { session: null, response: NextResponse.json({ error: "Forbidden" }, { status: 403 }) };
|
||||
}
|
||||
return { session, response: null };
|
||||
@@ -72,11 +82,12 @@ export async function requireSessionOrApiKey(
|
||||
.limit(1);
|
||||
|
||||
if (user) {
|
||||
// Apply rate limit when requested (API key path only)
|
||||
// Rate limit per API key (not per user — a user's other keys shouldn't
|
||||
// share this bucket).
|
||||
if (opts?.rateLimit) {
|
||||
const { limit, windowSeconds } = opts.rateLimit;
|
||||
const rateLimitResponse = await applyRateLimit(
|
||||
`rl:api:${user.id}`,
|
||||
`rl:api:key:${keyRow.id}`,
|
||||
limit,
|
||||
windowSeconds
|
||||
);
|
||||
@@ -100,5 +111,20 @@ export async function requireSessionOrApiKey(
|
||||
}
|
||||
|
||||
// 2. Fall back to session cookie
|
||||
return requireSession();
|
||||
const result = await requireSession();
|
||||
if (result.response) return result;
|
||||
|
||||
if (opts?.rateLimit) {
|
||||
const { limit, windowSeconds } = opts.rateLimit;
|
||||
const rateLimitResponse = await applyRateLimit(
|
||||
`rl:api:session:${result.session!.user.id}`,
|
||||
limit,
|
||||
windowSeconds
|
||||
);
|
||||
if (rateLimitResponse) {
|
||||
return { session: null, response: rateLimitResponse };
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -104,7 +104,8 @@ export function generateOpenApiSpec(): object {
|
||||
|
||||
const PaginatedRecipes = z.object({
|
||||
data: z.array(RecipeRef),
|
||||
pagination: z.object({ page: z.number(), limit: z.number(), total: z.number(), pages: z.number() }),
|
||||
limit: z.number(),
|
||||
offset: z.number(),
|
||||
});
|
||||
|
||||
const PaginatedCollections = z.object({
|
||||
@@ -118,7 +119,7 @@ export function generateOpenApiSpec(): object {
|
||||
|
||||
const idParam = z.object({ id: z.string() });
|
||||
|
||||
registry.registerPath({ method: "get", path: "/api/v1/recipes", summary: "List recipes", security, request: { query: z.object({ page: z.coerce.number().default(1), limit: z.coerce.number().default(20), visibility: z.enum(["private", "unlisted", "public"]).optional(), q: z.string().optional(), difficulty: z.enum(["easy", "medium", "hard"]).optional() }) }, responses: { 200: { description: "Paginated", content: { "application/json": { schema: PaginatedRecipes } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "get", path: "/api/v1/recipes", summary: "List recipes", security, request: { query: z.object({ limit: z.coerce.number().default(20), offset: z.coerce.number().default(0), visibility: z.enum(["private", "unlisted", "public"]).optional() }) }, responses: { 200: { description: "Paginated", content: { "application/json": { schema: PaginatedRecipes } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "post", path: "/api/v1/recipes", summary: "Create recipe", security, request: { body: { content: { "application/json": { schema: CreateRecipeRef } }, required: true } }, responses: { 201: { description: "Created", content: { "application/json": { schema: RecipeRef } } }, 400: { description: "Bad request", content: { "application/json": { schema: ApiErrorRef } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "get", path: "/api/v1/recipes/{id}", summary: "Get recipe", security, request: { params: idParam }, responses: { 200: { description: "Recipe", content: { "application/json": { schema: RecipeRef } } }, 404: { description: "Not found", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "put", path: "/api/v1/recipes/{id}", summary: "Update recipe", security, request: { params: idParam, body: { content: { "application/json": { schema: CreateRecipeRef } }, required: true } }, responses: { 200: { description: "Updated", content: { "application/json": { schema: RecipeRef } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } }, 404: { description: "Not found", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
|
||||
+18
-3
@@ -31,14 +31,15 @@ export class TierLimitError extends Error {
|
||||
export async function checkAndIncrementTierLimit(
|
||||
userId: string,
|
||||
userTier: "free" | "pro",
|
||||
key: "recipe" | "aiCall"
|
||||
key: "recipe" | "aiCall" | "storage",
|
||||
amount = 1
|
||||
): Promise<void> {
|
||||
const [tierDef] = await db
|
||||
.select()
|
||||
.from(tierDefinitions)
|
||||
.where(eq(tierDefinitions.tier, userTier));
|
||||
|
||||
if (!tierDef) return;
|
||||
if (!tierDef) throw new TierLimitError(key, userTier);
|
||||
|
||||
const month = currentMonth();
|
||||
const id = `${userId}-${month}`;
|
||||
@@ -57,7 +58,7 @@ export async function checkAndIncrementTierLimit(
|
||||
if (result.length === 0) {
|
||||
throw new TierLimitError("aiCall", userTier);
|
||||
}
|
||||
} else {
|
||||
} else if (key === "recipe") {
|
||||
const limit = tierDef.maxRecipes;
|
||||
const cap = limit === UNLIMITED ? sql`true` : sql`user_usage.recipe_count < ${limit}`;
|
||||
const result = await db.execute(sql`
|
||||
@@ -71,6 +72,20 @@ export async function checkAndIncrementTierLimit(
|
||||
if (result.length === 0) {
|
||||
throw new TierLimitError("recipe", userTier);
|
||||
}
|
||||
} else {
|
||||
const limit = tierDef.storageMb;
|
||||
const cap = limit === UNLIMITED ? sql`true` : sql`user_usage.storage_used_mb + ${amount} <= ${limit}`;
|
||||
const result = await db.execute(sql`
|
||||
INSERT INTO user_usage (id, user_id, month, ai_calls_used, recipe_count, storage_used_mb)
|
||||
VALUES (${id}, ${userId}, ${month}, 0, 0, ${amount})
|
||||
ON CONFLICT (user_id, month) DO UPDATE
|
||||
SET storage_used_mb = user_usage.storage_used_mb + ${amount}
|
||||
WHERE ${cap}
|
||||
RETURNING storage_used_mb
|
||||
`);
|
||||
if (result.length === 0) {
|
||||
throw new TierLimitError("storage", userTier);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,15 +2,19 @@ import crypto from "crypto";
|
||||
import { db } from "@epicure/db";
|
||||
import { webhooks, webhookDeliveries } from "@epicure/db";
|
||||
import { eq, and } from "@epicure/db";
|
||||
import { validateWebhookUrl } from "@/lib/validate-webhook-url";
|
||||
|
||||
export type WebhookEvent =
|
||||
| "recipe.created"
|
||||
| "recipe.updated"
|
||||
| "recipe.published"
|
||||
| "recipe.deleted"
|
||||
| "meal_plan.updated"
|
||||
| "shopping_list.completed"
|
||||
| "comment.added";
|
||||
export const WEBHOOK_EVENTS = [
|
||||
"recipe.created",
|
||||
"recipe.updated",
|
||||
"recipe.published",
|
||||
"recipe.deleted",
|
||||
"meal_plan.updated",
|
||||
"shopping_list.completed",
|
||||
"comment.added",
|
||||
] as const;
|
||||
|
||||
export type WebhookEvent = (typeof WEBHOOK_EVENTS)[number];
|
||||
|
||||
export async function dispatchWebhook(userId: string, event: WebhookEvent, payload: object) {
|
||||
const hooks = await db
|
||||
@@ -27,6 +31,11 @@ export async function dispatchWebhook(userId: string, event: WebhookEvent, paylo
|
||||
let statusCode = 0;
|
||||
let success = false;
|
||||
try {
|
||||
// Re-validate at dispatch time to defeat DNS rebinding between create and
|
||||
// dispatch. A small window remains between this lookup and fetch's own
|
||||
// resolution of the hostname — accepted for now.
|
||||
const ssrfError = await validateWebhookUrl(hook.url);
|
||||
if (ssrfError) throw new Error(ssrfError);
|
||||
const res = await fetch(hook.url, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
@@ -35,6 +44,8 @@ export async function dispatchWebhook(userId: string, event: WebhookEvent, paylo
|
||||
"X-Epicure-Event": event,
|
||||
},
|
||||
body,
|
||||
// Redirects could point at internal services, so treat 3xx as failure.
|
||||
redirect: "manual",
|
||||
signal: AbortSignal.timeout(10000),
|
||||
});
|
||||
statusCode = res.status;
|
||||
|
||||
Reference in New Issue
Block a user