security: fix full audit findings (v0.32.0)
Full list of the audit's confirmed findings and their fixes: - Stored XSS via unescaped JSON-LD on the public recipe page (app/r/[id]/page.tsx) — escape < before injecting. - CSP allowed unsafe-eval in production — now dev-only (Next prod never eval()s; only its HMR does). - avatarUrl accepted any URL with no ownership check — now takes an avatarKey issued by avatar-presign, validated server-side, same pattern as recipe/review photos. - No session revocation on password change/reset — both now revoke other sessions (revokeOtherSessions: true, revokeSessionsOnPasswordReset). - Rate-limit bypass via spoofable X-Forwarded-For — take the last (proxy-appended) hop instead of the first (client-supplied) one, matching the single-Traefik-hop topology. - Webhook signing secrets stored plaintext — now AES-256-GCM encrypted like every other secret in this app, with a legacy- plaintext fallback for pre-existing rows (bare hex has no ":", our ciphertext format always does). - Better Auth's own rate limiter defaulted to in-memory storage, ineffective across replicas — now backed by the same Redis as lib/rate-limit.ts (secondaryStorage), with storeSessionInDatabase explicit so session storage itself doesn't move as a side effect. - Presigned upload URLs didn't bind the declared file size to the actual upload, letting a client under-declare size (and quota charge) then PUT an arbitrarily large object — switched to S3 presigned POST with a signed content-length-range condition, enforced by the storage server itself. - generateMetadata() on the recipe page skipped the visibility filter the page body uses, leaking a private recipe's title via <title> to any signed-in user with the id. - Block/unblock had no rate limit, unlike follow/unfollow. - AI quota was charged even when a user's own BYOK key was used (their own credentials/billing) — added an isByok flag through the config-resolution chain and skip the charge when set. Also wired BYOK into generate/generate-from-idea/translate/import-url, which never looked it up at all before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Arnaud
@@ -47,26 +47,33 @@ type QuotaResult<T> = { ok: true; data: T } | { ok: false; response: NextRespons
|
||||
* throws — so a provider outage never silently burns a user's quota.
|
||||
* Centralizes the TierLimitError → 403 and AI-failure → clean-JSON mapping
|
||||
* that every AI route needs instead of repeating try/catch per route.
|
||||
*
|
||||
* Pass `skipQuota: true` when the resolved AiConfig used the caller's own
|
||||
* BYOK key (config.isByok) — it's their own credentials/billing, so it
|
||||
* shouldn't count against the platform's monthly AI-call limit.
|
||||
*/
|
||||
export async function withAiQuota<T>(
|
||||
userId: string,
|
||||
tier: "free" | "pro",
|
||||
fn: () => Promise<T>
|
||||
fn: () => Promise<T>,
|
||||
opts?: { skipQuota?: boolean }
|
||||
): Promise<QuotaResult<T>> {
|
||||
try {
|
||||
await checkAndIncrementTierLimit(userId, tier, "aiCall");
|
||||
} catch (err) {
|
||||
if (err instanceof TierLimitError) {
|
||||
return { ok: false, response: NextResponse.json({ error: "AI call limit reached for your tier" }, { status: 403 }) };
|
||||
if (!opts?.skipQuota) {
|
||||
try {
|
||||
await checkAndIncrementTierLimit(userId, tier, "aiCall");
|
||||
} catch (err) {
|
||||
if (err instanceof TierLimitError) {
|
||||
return { ok: false, response: NextResponse.json({ error: "AI call limit reached for your tier" }, { status: 403 }) };
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
try {
|
||||
const data = await fn();
|
||||
return { ok: true, data };
|
||||
} catch (err) {
|
||||
await refundAiCall(userId);
|
||||
if (!opts?.skipQuota) await refundAiCall(userId);
|
||||
return { ok: false, response: aiErrorResponse(err) };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,9 @@ export type AiConfig = {
|
||||
provider?: AiProvider;
|
||||
model?: string;
|
||||
apiKey?: string;
|
||||
/** True when apiKey is the caller's own BYOK key — their own credentials
|
||||
* and billing, so platform AI-call quota shouldn't be charged for it. */
|
||||
isByok?: boolean;
|
||||
};
|
||||
|
||||
export function resolveModel(config: AiConfig = {}): LanguageModel {
|
||||
|
||||
@@ -44,7 +44,7 @@ export async function withUserKey(userId: string, config: AiConfig): Promise<AiC
|
||||
|
||||
try {
|
||||
const apiKey = decrypt(row.encryptedKey);
|
||||
return { ...config, apiKey };
|
||||
return { ...config, apiKey, isByok: true };
|
||||
} catch {
|
||||
throw new ByokDecryptError(provider);
|
||||
}
|
||||
@@ -81,7 +81,7 @@ export async function getDefaultProviderWithKey(userId: string, useCase?: ModelU
|
||||
const row = keys.find((k) => k.provider === p);
|
||||
if (row) {
|
||||
try {
|
||||
return { provider: p, apiKey: decrypt(row.encryptedKey) };
|
||||
return { provider: p, apiKey: decrypt(row.encryptedKey), isByok: true };
|
||||
} catch {
|
||||
throw new ByokDecryptError(p);
|
||||
}
|
||||
|
||||
@@ -6,10 +6,32 @@ import { sendEmail, verifyEmailHtml, resetPasswordHtml, welcomeHtml } from "@/li
|
||||
import { isSignupsDisabled } from "@/lib/site-settings";
|
||||
import { findValidInvite, consumeInvite, INVITE_COOKIE } from "@/lib/invites";
|
||||
import { generateUniqueUsername } from "@/lib/username";
|
||||
import { getRedis } from "@/lib/redis";
|
||||
|
||||
// Without this, better-auth's own rate limiter (sign-in/sign-up/2FA/password
|
||||
// reset throttles below) defaults to an in-process Map — fine for a single
|
||||
// instance, but each replica gets its own independent counter the moment
|
||||
// this app is ever scaled horizontally, multiplying the effective limit by
|
||||
// the instance count. Backing it with the same Redis used elsewhere
|
||||
// (lib/rate-limit.ts) makes the limit actually shared and enforceable.
|
||||
const redisSecondaryStorage = {
|
||||
async get(key: string) {
|
||||
return getRedis().get(key);
|
||||
},
|
||||
async set(key: string, value: string, ttl?: number) {
|
||||
if (ttl) await getRedis().set(key, value, "EX", ttl);
|
||||
else await getRedis().set(key, value);
|
||||
},
|
||||
async delete(key: string) {
|
||||
await getRedis().del(key);
|
||||
},
|
||||
};
|
||||
|
||||
export const auth = betterAuth({
|
||||
trustedOrigins: [process.env["BETTER_AUTH_URL"] ?? "http://localhost:3000"],
|
||||
|
||||
secondaryStorage: redisSecondaryStorage,
|
||||
|
||||
// Explicit rather than relying on the isProduction default so dev/staging
|
||||
// are protected too. Sign-in/sign-up/change-password/change-email get a
|
||||
// strict built-in 3-req/10s-per-IP rule (better-auth's default special
|
||||
@@ -34,6 +56,9 @@ export const auth = betterAuth({
|
||||
emailAndPassword: {
|
||||
enabled: true,
|
||||
requireEmailVerification: true,
|
||||
// A stolen session should not survive the account owner noticing and
|
||||
// resetting their password.
|
||||
revokeSessionsOnPasswordReset: true,
|
||||
sendResetPassword: async ({ user, url }) => {
|
||||
await sendEmail({
|
||||
to: user.email,
|
||||
@@ -100,6 +125,12 @@ export const auth = betterAuth({
|
||||
enabled: true,
|
||||
maxAge: 60 * 5,
|
||||
},
|
||||
// secondaryStorage above is only meant to back the rate limiter — without
|
||||
// this, better-auth would also move session storage itself into Redis
|
||||
// (see internal-adapter.mjs's `storeInDb` checks), which is an unrelated,
|
||||
// much bigger behavior change (session data no longer queryable from the
|
||||
// `sessions` table at all) that nothing else in this app expects.
|
||||
storeSessionInDatabase: true,
|
||||
},
|
||||
|
||||
databaseHooks: {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// Mirrors CHANGELOG.md at the repo root — update both together.
|
||||
export const APP_VERSION = "0.31.0";
|
||||
export const APP_VERSION = "0.32.0";
|
||||
|
||||
export type ChangelogEntry = {
|
||||
version: string;
|
||||
@@ -11,6 +11,22 @@ export type ChangelogEntry = {
|
||||
};
|
||||
|
||||
export const CHANGELOG: ChangelogEntry[] = [
|
||||
{
|
||||
version: "0.32.0",
|
||||
date: "2026-07-14 17:10",
|
||||
security: [
|
||||
"Fixed a stored-XSS hole in public recipe pages' embedded structured data, and tightened the production Content-Security-Policy so script injection like it can't execute even if a similar bug slips in again.",
|
||||
"Avatar photos now require proof you actually own the upload — previously any URL was accepted, including another user's uploaded photo key.",
|
||||
"Changing your password now signs out every other active session, and so does resetting a forgotten password.",
|
||||
"Fixed a rate-limit bypass on public share-link throttling caused by trusting a spoofable header.",
|
||||
"Webhook signing secrets are now encrypted at rest, matching how API keys and other secrets are already stored.",
|
||||
"Login/2FA/password-reset rate limiting now shares Redis instead of quietly resetting per server instance.",
|
||||
"Photo/avatar uploads are now size-capped by the storage server itself, not just a number the client could lie about.",
|
||||
"A private recipe's title could leak via its page's browser-tab title to any signed-in user who had the link — fixed to respect the same visibility rules as the page itself.",
|
||||
"Blocking/unblocking a user is now rate-limited, matching follow/unfollow.",
|
||||
"AI calls made with your own API key (Settings → AI Keys) no longer count against your monthly AI-call limit.",
|
||||
],
|
||||
},
|
||||
{
|
||||
version: "0.31.0",
|
||||
date: "2026-07-14 16:35",
|
||||
|
||||
@@ -395,7 +395,7 @@ export function generateOpenApiSpec(): object {
|
||||
name: z.string().min(1).max(100).optional(), locale: z.string().max(10).optional(),
|
||||
bio: z.string().max(500).nullable().optional(), privateBio: z.string().max(2000).nullable().optional(),
|
||||
isPrivate: z.boolean().optional(), username: z.string().regex(USERNAME_PATTERN, "3-20 characters, lowercase letters, numbers, and underscores only").optional(),
|
||||
avatarUrl: z.string().url().max(2048).nullable().optional(), useGravatar: z.boolean().optional(),
|
||||
avatarKey: z.string().max(500).nullable().optional().describe("Storage key returned by POST /upload/avatar-presign, not a URL — validated server-side against the caller's own uploads."), useGravatar: z.boolean().optional(),
|
||||
}));
|
||||
const ModelPrefsRef = registry.register("ModelPrefs", z.object({
|
||||
id: z.string(), userId: z.string(),
|
||||
@@ -450,7 +450,7 @@ export function generateOpenApiSpec(): object {
|
||||
recipeCount: z.number().int(), isFollowing: z.boolean(),
|
||||
}));
|
||||
|
||||
registry.registerPath({ method: "patch", path: "/api/v1/users/me", summary: "Update your profile", description: "Toggling useGravatar or clearing avatarUrl re-derives the effective avatar (Gravatar or initials fallback).", security, request: { body: { content: { "application/json": { schema: UpdateMeRef } }, required: true } }, responses: { 200: { description: "Updated", content: { "application/json": { schema: z.object({ ok: z.boolean(), avatarUrl: z.string().nullable() }) } } }, 400: { description: "Validation error", content: { "application/json": { schema: ApiErrorRef } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } }, 409: { description: "Username already taken", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "patch", path: "/api/v1/users/me", summary: "Update your profile", description: "Toggling useGravatar or clearing avatarKey re-derives the effective avatar (Gravatar or initials fallback).", security, request: { body: { content: { "application/json": { schema: UpdateMeRef } }, required: true } }, responses: { 200: { description: "Updated", content: { "application/json": { schema: z.object({ ok: z.boolean(), avatarUrl: z.string().nullable() }) } } }, 400: { description: "Validation error, or avatarKey wasn't issued to you by avatar-presign", content: { "application/json": { schema: ApiErrorRef } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } }, 409: { description: "Username already taken", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "get", path: "/api/v1/users/me/export", summary: "Export all of your account data as a JSON file", description: "Rate-limited: 3 req/hour. Includes profile, recipes, social graph, collections, meal plans, pantry, shopping lists, webhooks, API keys, messages, and more.", security, responses: { 200: { description: "Downloadable JSON export", content: { "application/json": { schema: z.record(z.string(), z.unknown()) } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } }, 429: { description: "Rate limited", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "get", path: "/api/v1/users/me/model-prefs", summary: "Get your AI model provider/model preferences", security, responses: { 200: { description: "Preferences or null", content: { "application/json": { schema: ModelPrefsRef.nullable() } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "put", path: "/api/v1/users/me/model-prefs", summary: "Set your AI model provider/model preferences", security, request: { body: { content: { "application/json": { schema: UpdateModelPrefsRef } }, required: true } }, responses: { 200: { description: "Saved", content: { "application/json": { schema: z.object({ ok: z.boolean() }) } } }, 400: { description: "Invalid request", content: { "application/json": { schema: ApiErrorRef } } }, 401: { description: "Unauthorized", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
@@ -620,10 +620,14 @@ export function generateOpenApiSpec(): object {
|
||||
contentType: z.enum(["image/jpeg", "image/png", "image/webp", "image/avif"]),
|
||||
fileSize: z.number().int().positive().max(5 * 1024 * 1024, "File exceeds 5MB limit"),
|
||||
}));
|
||||
const PresignResponseRef = registry.register("PresignResponse", z.object({ url: z.string(), key: z.string() }));
|
||||
const PresignResponseRef = registry.register("PresignResponse", z.object({
|
||||
url: z.string().describe("POST this URL as multipart/form-data — include every field below, then the file itself last."),
|
||||
fields: z.record(z.string(), z.string()).describe("Form fields to include in the upload POST, including a signed content-length-range condition that the storage server enforces (the declared fileSize can't be lied about)."),
|
||||
key: z.string(),
|
||||
}));
|
||||
|
||||
registry.registerPath({ method: "post", path: "/api/v1/upload/presign", summary: "Get a presigned S3 URL to upload a recipe or review photo", description: "Recipe photos: recipeId must be your own recipe. Review photos: recipe must be visible to you. Charges your tier's storage quota.", security, request: { body: { content: { "application/json": { schema: PresignUploadRef } }, required: true } }, responses: { 200: { description: "Presigned URL and object key", content: { "application/json": { schema: PresignResponseRef } } }, 400: { description: "Validation error", content: { "application/json": { schema: ApiErrorRef } } }, 403: { description: "Storage limit reached for your tier", content: { "application/json": { schema: ApiErrorRef } } }, 404: { description: "Not found", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "post", path: "/api/v1/upload/avatar-presign", summary: "Get a presigned S3 URL to upload your avatar photo", description: "Charges your tier's storage quota.", security, request: { body: { content: { "application/json": { schema: PresignAvatarUploadRef } }, required: true } }, responses: { 200: { description: "Presigned URL and object key", content: { "application/json": { schema: PresignResponseRef } } }, 400: { description: "Validation error", content: { "application/json": { schema: ApiErrorRef } } }, 403: { description: "Storage limit reached for your tier", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "post", path: "/api/v1/upload/presign", summary: "Get a presigned S3 upload POST (URL + form fields) for a recipe or review photo", description: "Recipe photos: recipeId must be your own recipe. Review photos: recipe must be visible to you. Charges your tier's storage quota.", security, request: { body: { content: { "application/json": { schema: PresignUploadRef } }, required: true } }, responses: { 200: { description: "Presigned POST", content: { "application/json": { schema: PresignResponseRef } } }, 400: { description: "Validation error", content: { "application/json": { schema: ApiErrorRef } } }, 403: { description: "Storage limit reached for your tier", content: { "application/json": { schema: ApiErrorRef } } }, 404: { description: "Not found", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
registry.registerPath({ method: "post", path: "/api/v1/upload/avatar-presign", summary: "Get a presigned S3 upload POST (URL + form fields) for your avatar photo", description: "Charges your tier's storage quota.", security, request: { body: { content: { "application/json": { schema: PresignAvatarUploadRef } }, required: true } }, responses: { 200: { description: "Presigned POST", content: { "application/json": { schema: PresignResponseRef } } }, 400: { description: "Validation error", content: { "application/json": { schema: ApiErrorRef } } }, 403: { description: "Storage limit reached for your tier", content: { "application/json": { schema: ApiErrorRef } } } } });
|
||||
|
||||
// --- Push notifications ---
|
||||
const PushSubscribeRef = registry.register("PushSubscribe", z.object({
|
||||
|
||||
+32
-5
@@ -1,5 +1,5 @@
|
||||
import { S3Client, PutObjectCommand, DeleteObjectCommand } from "@aws-sdk/client-s3";
|
||||
import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
|
||||
import { S3Client, DeleteObjectCommand } from "@aws-sdk/client-s3";
|
||||
import { createPresignedPost } from "@aws-sdk/s3-presigned-post";
|
||||
|
||||
const bucket = process.env["STORAGE_BUCKET"] || "epicure-uploads";
|
||||
const endpoint = process.env["STORAGE_ENDPOINT"] || "http://localhost:9000";
|
||||
@@ -32,9 +32,31 @@ const presignS3 = new S3Client({
|
||||
credentials,
|
||||
});
|
||||
|
||||
export async function createPresignedUploadUrl(key: string, contentType: string): Promise<string> {
|
||||
const command = new PutObjectCommand({ Bucket: bucket, Key: key, ContentType: contentType });
|
||||
return getSignedUrl(presignS3, command, { expiresIn: 300 });
|
||||
/**
|
||||
* A presigned PUT URL only signs Bucket/Key/ContentType — nothing binds the
|
||||
* actual request body size to what the caller declared when it asked for the
|
||||
* URL (and was charged storage-tier quota for), so a client could declare
|
||||
* 1 byte and then PUT an arbitrarily large object. S3/MinIO's POST-policy
|
||||
* upload flow is the fix: `content-length-range` is a *signed* condition,
|
||||
* enforced by the storage server itself when it receives the upload — a
|
||||
* mismatched Content-Length is rejected before the bytes are ever accepted,
|
||||
* not just checked against something the client also controls.
|
||||
*/
|
||||
export async function createPresignedUploadPost(
|
||||
key: string,
|
||||
contentType: string,
|
||||
maxBytes: number
|
||||
): Promise<{ url: string; fields: Record<string, string> }> {
|
||||
return createPresignedPost(presignS3, {
|
||||
Bucket: bucket,
|
||||
Key: key,
|
||||
Conditions: [
|
||||
["content-length-range", 0, maxBytes],
|
||||
{ "Content-Type": contentType },
|
||||
],
|
||||
Fields: { "Content-Type": contentType },
|
||||
Expires: 300,
|
||||
});
|
||||
}
|
||||
|
||||
export async function deleteObject(key: string): Promise<void> {
|
||||
@@ -60,3 +82,8 @@ export function isOwnedRecipePhotoKey(key: string, recipeId: string, userId: str
|
||||
export function isOwnedReviewPhotoKey(key: string, recipeId: string, userId: string): boolean {
|
||||
return key.startsWith(`recipes/${recipeId}/reviews/${userId}-`);
|
||||
}
|
||||
|
||||
/** Same rationale as isOwnedRecipePhotoKey, for the avatar-presign key prefix. */
|
||||
export function isOwnedAvatarKey(key: string, userId: string): boolean {
|
||||
return key.startsWith(`user-avatars/${userId}/`);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
/** Uploads a file to an S3/MinIO presigned POST URL (see lib/storage.ts's
|
||||
* createPresignedUploadPost) — unlike a presigned PUT, the storage server
|
||||
* itself rejects a mismatched size via the signed content-length-range
|
||||
* condition, so this can't be used to upload past the declared limit. */
|
||||
export async function uploadToPresignedPost(
|
||||
url: string,
|
||||
fields: Record<string, string>,
|
||||
file: File
|
||||
): Promise<boolean> {
|
||||
const formData = new FormData();
|
||||
for (const [key, value] of Object.entries(fields)) formData.append(key, value);
|
||||
formData.append("file", file);
|
||||
const res = await fetch(url, { method: "POST", body: formData });
|
||||
return res.ok;
|
||||
}
|
||||
@@ -3,6 +3,16 @@ import { db } from "@epicure/db";
|
||||
import { webhooks, webhookDeliveries } from "@epicure/db";
|
||||
import { eq, and } from "@epicure/db";
|
||||
import { safeFetch } from "@/lib/validate-webhook-url";
|
||||
import { decrypt } from "@/lib/encrypt";
|
||||
|
||||
// Secrets created before encryption-at-rest was added are a bare 64-char hex
|
||||
// string (crypto.randomBytes(32).toString("hex")) with no ":" separators —
|
||||
// encrypt()'s output is always "iv:authTag:ciphertext". Fall back to treating
|
||||
// the value as already-plaintext rather than a hard migration, since this is
|
||||
// only reached with a value this app itself generated (never user input).
|
||||
function decryptWebhookSecret(stored: string): string {
|
||||
return stored.split(":").length === 3 ? decrypt(stored) : stored;
|
||||
}
|
||||
|
||||
export const WEBHOOK_EVENTS = [
|
||||
"recipe.created",
|
||||
@@ -27,7 +37,7 @@ export async function dispatchWebhook(userId: string, event: WebhookEvent, paylo
|
||||
await Promise.allSettled(
|
||||
filtered.map(async (hook) => {
|
||||
const body = JSON.stringify({ event, payload, timestamp: new Date().toISOString() });
|
||||
const sig = crypto.createHmac("sha256", hook.secret).update(body).digest("hex");
|
||||
const sig = crypto.createHmac("sha256", decryptWebhookSecret(hook.secret)).update(body).digest("hex");
|
||||
let statusCode = 0;
|
||||
let success = false;
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user