From 7b74e2314be204cd9aaf69c661b3b5e23ed8c4e9 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 2 Jun 2026 13:18:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit):=20M24=20=E2=80=94=20reserve=20'bran?= =?UTF-8?q?ding'/'avatar'=20file=20categories=20from=20the=20upload/update?= =?UTF-8?q?=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The public file-stream gate keys off files.category==='branding'; the API upload/update schemas now reject the reserved categories so a user can't self-set branding to publicly expose their own file. System writers (admin image, avatar) set them via the service directly and are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/services/files.ts | 21 ++++++++++++++++ src/lib/validators/files.ts | 50 ++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/lib/services/files.ts b/src/lib/services/files.ts index 2d8e17af..6e4e5b2e 100644 --- a/src/lib/services/files.ts +++ b/src/lib/services/files.ts @@ -173,6 +173,23 @@ export async function getPreviewUrl(id: string, portId: string) { // ─── Update ─────────────────────────────────────────────────────────────────── +/** + * Categories that gate system surfaces and must never be settable through the + * general (validated) update path. `branding` is the public-stream gate on + * `/api/public/files/[id]`; `avatar` is system-managed. The upload/update Zod + * schemas (`userFileCategorySchema`) already reject these, so this is + * belt-and-suspenders for any future non-HTTP caller (M24). + * + * NB: `uploadFile` is intentionally NOT guarded — the admin branding writer + * (`admin/settings/image` route) and the avatar writer (`me/avatar` route) + * legitimately call the service with `category: 'branding' | 'avatar'`, + * constructing the payload inline and bypassing the Zod schema. Guarding the + * upload service would break those legitimate system writers. `updateFile`, + * by contrast, has a single schema-validated caller and no legitimate + * reserved-category use, so it is safe to harden here. + */ +const RESERVED_FILE_CATEGORIES = new Set(['branding', 'avatar']); + export async function updateFile( id: string, portId: string, @@ -181,6 +198,10 @@ export async function updateFile( ) { const existing = await getFileById(id, portId); + if (data.category !== undefined && RESERVED_FILE_CATEGORIES.has(data.category)) { + throw new ValidationError(`Category '${data.category}' is reserved and cannot be set`); + } + const updates: { filename?: string; category?: string } = {}; if (data.filename !== undefined) updates.filename = sanitizeFilename(data.filename); if (data.category !== undefined) updates.category = data.category; diff --git a/src/lib/validators/files.ts b/src/lib/validators/files.ts index 2002f436..4de599f0 100644 --- a/src/lib/validators/files.ts +++ b/src/lib/validators/files.ts @@ -2,20 +2,51 @@ import { z } from 'zod'; import { baseListQuerySchema } from '@/lib/api/list-query'; +/** + * User-settable file categories (allow-list). + * + * M24: `category` is a free string used to gate the public, unauthenticated + * stream-by-id surface (`/api/public/files/[id]` only serves rows where + * `category === 'branding'`). When the upload/update API accepted an open + * `z.string()`, any user with `files.upload` could self-set + * `category='branding'` on their own file and make it publicly streamable + + * CDN-cached 24h (a confused-deputy self-exposure). This allow-list reserves + * the system-managed categories so the general upload/update path can never + * set them. + * + * RESERVED (deliberately EXCLUDED) — never settable via the validated API: + * - 'branding' → public-stream gate (set only by the admin branding writers: + * `admin/settings/image` route + `logo.service.setPortLogo`, both of which + * bypass this schema) + * - 'avatar' → system-managed user avatar (set only by `me/avatar`, which + * also bypasses this schema) + */ +export const USER_FILE_CATEGORIES = [ + 'eoi', + 'contract', + 'image', + 'receipt', + 'correspondence', + 'misc', + 'other', +] as const; + +export const userFileCategorySchema = z.enum(USER_FILE_CATEGORIES); + export const uploadFileSchema = z.object({ filename: z.string().min(1).max(255), clientId: z.string().optional(), yachtId: z.string().optional(), companyId: z.string().optional(), folderId: z.string().uuid().optional(), - category: z.string().optional(), + category: userFileCategorySchema.optional(), entityType: z.string().optional(), entityId: z.string().optional(), }); export const updateFileSchema = z.object({ filename: z.string().min(1).max(255).optional(), - category: z.string().optional(), + category: userFileCategorySchema.optional(), }); export const listFilesSchema = baseListQuerySchema @@ -49,6 +80,17 @@ export const listFilesSchema = baseListQuerySchema path: ['entityType'], }); -export type UploadFileInput = z.infer; -export type UpdateFileInput = z.infer; +// The API schema rejects the reserved categories ('branding'/'avatar') at +// parse time (M24). But the `uploadFile`/`updateFile` SERVICE is also called +// directly by the system writers (admin branding image, user avatar) that +// legitimately set those reserved categories and bypass the schema — so the +// service-facing input type keeps `category` as a plain string. Validation +// strictness lives in the schema; the type just must not block the system +// callers. +export type UploadFileInput = Omit, 'category'> & { + category?: string; +}; +export type UpdateFileInput = Omit, 'category'> & { + category?: string; +}; export type ListFilesInput = z.infer;