From f9a0ee8e30b975c52a4a8f2e3286633de111e45a Mon Sep 17 00:00:00 2001 From: Sivert Date: Fri, 2 Oct 2026 14:34:03 +0200 Subject: [PATCH 1/3] feat(server): accept MP4 card banners Co-Authored-By: OpenAI Codex --- src/routes/bannerUpload.test.ts | 21 ++++++- src/routes/uploads.ts | 97 +++++++++++++++++++++++++++------ 2 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/routes/bannerUpload.test.ts b/src/routes/bannerUpload.test.ts index c5c98dbc..a1b7f142 100644 --- a/src/routes/bannerUpload.test.ts +++ b/src/routes/bannerUpload.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import type { AddressInfo } from "node:net"; import type { Server } from "node:http"; import { tmpdir } from "node:os"; @@ -55,6 +55,7 @@ async function remove(who: Member) { const png = (width = 1600, height = 900) => sharp({ create: { width, height, channels: 3, background: { r: 200, g: 60, b: 90 } } }).png().toBuffer(); +const mp4 = () => readFileSync(join(__dirname, "../utils/testdata/landscape.mp4")); const bannerOf = async (m: Member) => (await getUserByServerId(m.serverUserId))?.banner_file_id ?? null; const rowOf = async (m: Member) => (await buildMemberList({})).find((r) => r.serverUserId === m.serverUserId); @@ -120,6 +121,24 @@ describe("POST /api/uploads/banner", () => { assert.equal(await bannerOf(bob), null); }); + it("stores a readable MP4 without flattening its animation", async () => { + const player = await member("player"); + const res = await upload(player, mp4(), "video/mp4", "loop.mp4"); + assert.equal(res.status, 201, JSON.stringify(res.body)); + + const file = await getFile(res.body.bannerFileId as string); + assert.equal(file?.mime, "video/mp4"); + assert.equal(file?.width, 160); + assert.equal(file?.height, 90); + }); + + it("refuses bytes merely labelled as MP4", async () => { + const poser = await member("poser"); + const res = await upload(poser, Buffer.from("not a video"), "video/mp4", "fake.mp4"); + assert.equal(res.status, 400); + assert.equal(await bannerOf(poser), null); + }); + it("deletes the old banner when it is replaced", async () => { const carol = await member("carol"); const first = (await upload(carol, await png())).body.bannerFileId as string; diff --git a/src/routes/uploads.ts b/src/routes/uploads.ts index 187f3f77..dd59b791 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -4,7 +4,7 @@ import type { Request, Response, NextFunction } from "express"; import multer from "multer"; import { v4 as uuidv4 } from "uuid"; import mime from "mime-types"; -import { unlink } from "fs/promises"; +import { readFile, unlink } from "fs/promises"; import { putObject, getObject } from "../storage"; import { insertFile, getFile, updateUserAvatar, setUserAvatar, setUserBanner, getServerConfig, getUserByServerId, DEFAULT_AVATAR_MAX_BYTES, DEFAULT_UPLOAD_MAX_BYTES } from "../db"; import { BANNER_BOX } from "../constants/media"; @@ -20,6 +20,7 @@ import { sendStoredBody } from "../utils/sendStoredBody"; import { ensurePermission } from "../middleware/requirePermission"; import { sanitizeSvg } from "../utils/svgSanitize"; import { storeUploadedFile } from "../services/storeUploadedFile"; +import { readVideoDimensionsFromFile } from "../utils/videoDimensions"; /** SVG is on this list only because it has been through sanitizeSvg(), is drawn through ``, and is sandboxed by the CSP below. That header is required. */ @@ -139,15 +140,15 @@ uploadsRouter.post( }, ); -/** One pipeline for every picture. Only an avatar or a banner is put on the +/** One pipeline for avatar-shaped pictures. Only an avatar is put on the uploader's own row; a group picture doing that was GRYT-1182. */ -const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => +const storeAvatarImage = (purpose: "avatar" | "group" | "webhook") => (req: Request, res: Response, next: NextFunction): void => { const file = req.file; if (!file) { res.status(400).json({ error: "file_required", message: "file is required" }); return; } if (!(file.mimetype || "").startsWith("image/")) { res.status(400).json({ error: "invalid_file", message: "Only image files are allowed" }); return; } - const what = { avatar: "Avatar", group: "Group picture", webhook: "Webhook avatar", banner: "Banner" }[purpose]; + const what = { avatar: "Avatar", group: "Group picture", webhook: "Webhook avatar" }[purpose]; const disableS3 = (process.env.DISABLE_S3 || "").toLowerCase() === "true"; if (disableS3) { res.status(503).json({ error: "s3_disabled", message: `S3 is disabled (DISABLE_S3=true). ${what} upload is unavailable.` }); return; } @@ -174,11 +175,6 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => // SVG never reaches sharp. Stored as the sanitised vector; see // svgSanitize.ts for why that is enough. if ((file.mimetype || "").toLowerCase() === "image/svg+xml") { - // A vector is not cut to the 5:2 box, so a banner is always a raster. - if (purpose === "banner") { - res.status(400).json({ error: "invalid_file", message: "A banner can't be an SVG." }); - return; - } const svg = sanitizeSvg(file.buffer); if (!svg.valid) { res.status(400).json({ error: "invalid_file", message: svg.reason }); @@ -217,7 +213,6 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => uploadedBy: serverUserId, maxBytes, what, - ...(purpose === "banner" ? { box: BANNER_BOX, prefix: "banners" } : {}), }); if (!stored.ok) { res.status(stored.status).json({ error: stored.error, message: stored.message }); @@ -227,11 +222,6 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => if (purpose === "avatar") { await updateUserAvatar(serverUserId, stored.fileId); res.status(201).json({ avatarFileId: stored.fileId, processing: stored.processing }); - } else if (purpose === "banner") { - const previous = (await getUserByServerId(serverUserId))?.banner_file_id ?? null; - await setUserBanner(serverUserId, stored.fileId); - if (previous && previous !== stored.fileId) await deleteUnreferencedFiles([previous]); - res.status(201).json({ bannerFileId: stored.fileId, processing: stored.processing }); } else { res.status(201).json({ fileId: stored.fileId, processing: stored.processing }); } @@ -239,6 +229,79 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => .catch(next); }; +const storeBannerMedia = (req: Request, res: Response, next: NextFunction): void => { + const file = req.file; + if (!file) { res.status(400).json({ error: "file_required", message: "file is required" }); return; } + const type = (file.mimetype || "").toLowerCase(); + if (!type.startsWith("image/") && type !== "video/mp4") { + void discardTemp(file); + res.status(400).json({ error: "invalid_file", message: "Only images and MP4 videos are allowed" }); + return; + } + + const bucket = process.env.S3_BUCKET as string; + const serverUserId = req.tokenPayload?.serverUserId; + const disableS3 = (process.env.DISABLE_S3 || "").toLowerCase() === "true"; + if (disableS3) { void discardTemp(file); res.status(503).json({ error: "s3_disabled", message: "S3 is disabled (DISABLE_S3=true). Banner upload is unavailable." }); return; } + if (!bucket) { void discardTemp(file); res.status(500).json({ error: "s3_not_configured", message: "S3_BUCKET not configured" }); return; } + if (!serverUserId) { void discardTemp(file); res.status(401).json({ error: "auth_required" }); return; } + + Promise.resolve() + .then(async () => { + const cfg = await getServerConfig().catch(() => null); + const maxBytes = typeof cfg?.upload_max_bytes === "number" ? cfg.upload_max_bytes : DEFAULT_UPLOAD_MAX_BYTES; + let stored: { ok: true; fileId: string; processing: boolean } | { ok: false; status: number; error: string; message: string }; + + if (type === "video/mp4") { + const dimensions = await readVideoDimensionsFromFile(file.path); + if (!dimensions) { + res.status(400).json({ error: "invalid_file", message: "The MP4 could not be read" }); + return; + } + const video = await storeUploadedFile({ + bucket, + path: file.path, + size: file.size, + mimetype: type, + originalName: file.originalname, + sealed: false, + uploadedBy: serverUserId, + maxBytes, + claimedWidth: dimensions.width, + claimedHeight: dimensions.height, + }); + stored = video.ok ? { ...video, processing: true } : video; + } else { + if (type === "image/svg+xml") { + res.status(400).json({ error: "invalid_file", message: "A banner can't be an SVG." }); + return; + } + stored = await storeAvatarPicture({ + bucket, + bytes: await readFile(file.path), + mime: type, + originalName: file.originalname || null, + uploadedBy: serverUserId, + maxBytes, + what: "Banner", + box: BANNER_BOX, + prefix: "banners", + }); + } + + if (!stored.ok) { + res.status(stored.status).json({ error: stored.error, message: stored.message }); + return; + } + const previous = (await getUserByServerId(serverUserId))?.banner_file_id ?? null; + await setUserBanner(serverUserId, stored.fileId); + if (previous && previous !== stored.fileId) await deleteUnreferencedFiles([previous]); + res.status(201).json({ bannerFileId: stored.fileId, processing: stored.processing, mime: type }); + }) + .finally(() => discardTemp(file)) + .catch(next); +}; + uploadsRouter.post( "/avatar", requireBearerToken, @@ -261,8 +324,8 @@ uploadsRouter.post( .then((ok) => { if (ok) next(); }) .catch(next); }, - uploadAvatarToMemory("file"), - storeAvatarImage("banner"), + uploadToDisk("file"), + storeBannerMedia, ); // Ungated, unlike DELETE /avatar: losing the upload permission must not strand a banner. From 1c19c1d1227aece5b8feaf9b061b0dd84ea1631e Mon Sep 17 00:00:00 2001 From: Sivert Date: Fri, 2 Oct 2026 19:57:51 +0200 Subject: [PATCH 2/3] fix(server): quarantine banners until worker approval Co-Authored-By: OpenAI Codex --- src/routes/bannerUpload.test.ts | 65 +++++++++++++++++++++++++------ src/routes/uploads.ts | 55 +++++++------------------- src/services/storeBannerUpload.ts | 27 +++++++++++++ 3 files changed, 94 insertions(+), 53 deletions(-) create mode 100644 src/services/storeBannerUpload.ts diff --git a/src/routes/bannerUpload.test.ts b/src/routes/bannerUpload.test.ts index a1b7f142..470bcde7 100644 --- a/src/routes/bannerUpload.test.ts +++ b/src/routes/bannerUpload.test.ts @@ -12,13 +12,15 @@ import sharp from "sharp"; import { initSqlite } from "../db/sqlite/connection"; import { getFile } from "../db/sqlite/messages"; import { createRoleDefinition } from "../db/sqlite/roleDefinitions"; -import { createServerConfigIfNotExists, setServerRole } from "../db/sqlite/servers"; +import { createServerConfigIfNotExists, setServerRole, updateServerConfig } from "../db/sqlite/servers"; import { getUserByServerId, upsertUser } from "../db/sqlite/users"; import { unreferencedAmong } from "../jobs/mediaSweep"; import { fileReadVerdict } from "../services/fileAccess"; import { buildMemberList } from "../socket/utils/clients"; import { initStorage } from "../storage"; -import { generateAccessToken } from "../utils/jwt"; +import { generateAccessToken, generateFileToken } from "../utils/jwt"; +import { getImageJob, updateImageJobStatus } from "../db/sqlite/imageJobs"; +import { apiErrorHandler } from "../utils/httpErrors"; import { uploadsRouter } from "./uploads"; /** The member card banner: the avatar pipeline cut to 5:2. */ @@ -73,6 +75,7 @@ before(async () => { const app = express(); app.use("/api/uploads", uploadsRouter); + app.use(apiErrorHandler); server = app.listen(0, "127.0.0.1"); await new Promise((resolve) => server.once("listening", () => resolve())); const { port } = server.address() as AddressInfo; @@ -89,7 +92,7 @@ after(async () => { }); describe("POST /api/uploads/banner", () => { - it("stores the picture cut to the banner box, beside the avatar", async () => { + it("quarantines the picture for the worker, beside the avatar", async () => { const alice = await member("alice"); const res = await upload(alice, await png()); @@ -100,10 +103,11 @@ describe("POST /api/uploads/banner", () => { assert.equal((await getUserByServerId(alice.serverUserId))?.avatar_file_id ?? null, null, "it became the avatar"); const file = await getFile(fileId); - assert.equal(file?.mime, "image/avif"); - assert.equal(file?.width, 960); - assert.equal(file?.height, 384); - assert.ok(file?.s3_key.startsWith("banners/")); + assert.equal(file?.mime, "image/png"); + assert.equal(file?.width, null); + assert.equal(file?.height, null); + assert.ok(file?.s3_key.startsWith("quarantine/banners/")); + assert.equal((await getImageJob(fileId))?.status, "queued"); }); it("is refused to a member who may not upload, and nothing is stored", async () => { @@ -128,15 +132,52 @@ describe("POST /api/uploads/banner", () => { const file = await getFile(res.body.bannerFileId as string); assert.equal(file?.mime, "video/mp4"); - assert.equal(file?.width, 160); - assert.equal(file?.height, 90); + assert.equal(file?.width, null); + assert.equal(file?.height, null); + assert.equal((await getImageJob(res.body.bannerFileId as string))?.status, "queued"); }); - it("refuses bytes merely labelled as MP4", async () => { + it("keeps bytes merely labelled as MP4 unreadable until the worker checks them", async () => { const poser = await member("poser"); const res = await upload(poser, Buffer.from("not a video"), "video/mp4", "fake.mp4"); - assert.equal(res.status, 400); - assert.equal(await bannerOf(poser), null); + assert.equal(res.status, 201); + const fileId = res.body.bannerFileId as string; + const token = generateFileToken({ ...poser, serverHost: host, tokenVersion: 0 }); + const url = `${base}/api/uploads/files/${fileId}?t=${encodeURIComponent(token)}`; + const pending = await fetch(url); + assert.equal(pending.status, 503); + assert.equal(pending.headers.get("cache-control"), "no-store"); + await updateImageJobStatus({ job_id: fileId, status: "error", error_message: "invalid video" }); + assert.equal((await fetch(url)).status, 422); + }); + + it("serves a video only after a successful worker verdict, including range reads", async () => { + const verified = await member("verified"); + const res = await upload(verified, mp4(), "video/mp4", "loop.mp4"); + const fileId = res.body.bannerFileId as string; + const token = generateFileToken({ ...verified, serverHost: host, tokenVersion: 0 }); + const url = `${base}/api/uploads/files/${fileId}?t=${encodeURIComponent(token)}`; + assert.equal((await fetch(url, { method: "HEAD" })).status, 503); + assert.equal((await fetch(`${url}&thumb=1`)).status, 503); + await updateImageJobStatus({ job_id: fileId, status: "done" }); + const ready = await fetch(url, { headers: { Range: "bytes=0-15" } }); + assert.equal(ready.status, 206); + assert.equal((await ready.arrayBuffer()).byteLength, 16); + }); + + it("enforces the configured upload limit on both pictures and videos", async () => { + const limited = await member("limited"); + await updateServerConfig({ uploadMaxBytes: 512 }); + try { + for (const [bytes, type] of [[await png(), "image/png"], [mp4(), "video/mp4"]] as const) { + const res = await upload(limited, bytes, type); + assert.equal(res.status, 413, JSON.stringify(res.body)); + assert.equal(res.body.error, "file_too_large"); + assert.equal(await bannerOf(limited), null); + } + } finally { + await updateServerConfig({ uploadMaxBytes: null }); + } }); it("deletes the old banner when it is replaced", async () => { diff --git a/src/routes/uploads.ts b/src/routes/uploads.ts index dd59b791..f5a37661 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -4,10 +4,9 @@ import type { Request, Response, NextFunction } from "express"; import multer from "multer"; import { v4 as uuidv4 } from "uuid"; import mime from "mime-types"; -import { readFile, unlink } from "fs/promises"; +import { unlink } from "fs/promises"; import { putObject, getObject } from "../storage"; import { insertFile, getFile, updateUserAvatar, setUserAvatar, setUserBanner, getServerConfig, getUserByServerId, DEFAULT_AVATAR_MAX_BYTES, DEFAULT_UPLOAD_MAX_BYTES } from "../db"; -import { BANNER_BOX } from "../constants/media"; import { deleteUnreferencedFiles } from "../jobs/mediaSweep"; import { isSealedUpload } from "./uploadStorage"; import { requireBearerToken } from "../middleware/requireBearerToken"; @@ -20,7 +19,8 @@ import { sendStoredBody } from "../utils/sendStoredBody"; import { ensurePermission } from "../middleware/requirePermission"; import { sanitizeSvg } from "../utils/svgSanitize"; import { storeUploadedFile } from "../services/storeUploadedFile"; -import { readVideoDimensionsFromFile } from "../utils/videoDimensions"; +import { storeBannerUpload } from "../services/storeBannerUpload"; +import { getImageJob } from "../db"; /** SVG is on this list only because it has been through sanitizeSvg(), is drawn through ``, and is sandboxed by the CSP below. That header is required. */ @@ -250,44 +250,7 @@ const storeBannerMedia = (req: Request, res: Response, next: NextFunction): void .then(async () => { const cfg = await getServerConfig().catch(() => null); const maxBytes = typeof cfg?.upload_max_bytes === "number" ? cfg.upload_max_bytes : DEFAULT_UPLOAD_MAX_BYTES; - let stored: { ok: true; fileId: string; processing: boolean } | { ok: false; status: number; error: string; message: string }; - - if (type === "video/mp4") { - const dimensions = await readVideoDimensionsFromFile(file.path); - if (!dimensions) { - res.status(400).json({ error: "invalid_file", message: "The MP4 could not be read" }); - return; - } - const video = await storeUploadedFile({ - bucket, - path: file.path, - size: file.size, - mimetype: type, - originalName: file.originalname, - sealed: false, - uploadedBy: serverUserId, - maxBytes, - claimedWidth: dimensions.width, - claimedHeight: dimensions.height, - }); - stored = video.ok ? { ...video, processing: true } : video; - } else { - if (type === "image/svg+xml") { - res.status(400).json({ error: "invalid_file", message: "A banner can't be an SVG." }); - return; - } - stored = await storeAvatarPicture({ - bucket, - bytes: await readFile(file.path), - mime: type, - originalName: file.originalname || null, - uploadedBy: serverUserId, - maxBytes, - what: "Banner", - box: BANNER_BOX, - prefix: "banners", - }); - } + const stored = await storeBannerUpload({ bucket, file, uploadedBy: serverUserId, maxBytes }); if (!stored.ok) { res.status(stored.status).json({ error: stored.error, message: stored.message }); @@ -481,6 +444,16 @@ uploadsRouter.get( const fileMeta = verdict === "allowed" ? await getFile(fileId) : null; if (!fileMeta) { res.status(404).json({ error: "File not found" }); return; } + // Quarantined banners stay unreadable when the worker is absent or fails. + if (fileMeta.s3_key.startsWith("quarantine/banners/") || fileMeta.s3_key.startsWith("banners/verified/")) { + const job = await getImageJob(fileId); + if (job?.status !== "done") { + res.setHeader("Cache-Control", "no-store"); + res.status(job?.status === "error" ? 422 : 503).json({ error: "banner_not_ready" }); + return; + } + } + const useThumb = req.query.thumb === "1" && fileMeta.thumbnail_key; const s3Key = useThumb ? fileMeta.thumbnail_key! : fileMeta.s3_key; const totalSize = useThumb ? null : (fileMeta.size ?? null); diff --git a/src/services/storeBannerUpload.ts b/src/services/storeBannerUpload.ts new file mode 100644 index 00000000..357d9738 --- /dev/null +++ b/src/services/storeBannerUpload.ts @@ -0,0 +1,27 @@ +import { v4 as uuidv4 } from "uuid"; +import { insertFile, insertImageJob } from "../db"; +import { deleteObject, putObject } from "../storage"; + +const bannerTypes = new Set(["image/png", "image/jpeg", "image/gif", "image/webp", "image/avif", "video/mp4"]); + +export async function storeBannerUpload(input: { + bucket: string; file: Express.Multer.File; uploadedBy: string; maxBytes: number; +}) { + const { bucket, file, uploadedBy, maxBytes } = input; + const type = file.mimetype.toLowerCase(); + if (!bannerTypes.has(type)) return { ok: false as const, status: 400, error: "invalid_file", message: "Only raster images and MP4 videos are allowed" }; + if (maxBytes > 0 && file.size > maxBytes) return { ok: false as const, status: 413, error: "file_too_large", message: "Banner exceeds the server upload limit" }; + const fileId = uuidv4(); + const key = `quarantine/banners/${fileId}`; + await putObject({ bucket, key, sourcePath: file.path, contentType: type }); + try { + await insertFile({ file_id: fileId, s3_key: key, mime: type, size: file.size, + width: null, height: null, thumbnail_key: null, original_name: file.originalname, + uploaded_by_server_user_id: uploadedBy }); + await insertImageJob({ job_id: fileId, file_id: fileId, raw_s3_key: key, raw_content_type: type, raw_bytes: file.size }); + } catch (error) { + await deleteObject({ bucket, key }).catch(() => undefined); + throw error; + } + return { ok: true as const, fileId, processing: true }; +} From 753512cb868e799e759057649f4b15343e306dd7 Mon Sep 17 00:00:00 2001 From: Sivert Date: Fri, 2 Oct 2026 20:20:41 +0200 Subject: [PATCH 3/3] fix(server): share media quarantine across banners and chat uploads Co-Authored-By: OpenAI Codex --- src/routes/bannerUpload.test.ts | 49 +++++++++++++++++++++++++++---- src/routes/uploadStorage.test.ts | 6 ++-- src/routes/uploads.ts | 14 +++++---- src/services/storeBannerUpload.ts | 20 +++---------- src/services/storeUploadedFile.ts | 28 +++++++++++------- 5 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/routes/bannerUpload.test.ts b/src/routes/bannerUpload.test.ts index 470bcde7..0273c21d 100644 --- a/src/routes/bannerUpload.test.ts +++ b/src/routes/bannerUpload.test.ts @@ -10,7 +10,7 @@ import express from "express"; import sharp from "sharp"; import { initSqlite } from "../db/sqlite/connection"; -import { getFile } from "../db/sqlite/messages"; +import { getFile, updateFileRecord } from "../db/sqlite/messages"; import { createRoleDefinition } from "../db/sqlite/roleDefinitions"; import { createServerConfigIfNotExists, setServerRole, updateServerConfig } from "../db/sqlite/servers"; import { getUserByServerId, upsertUser } from "../db/sqlite/users"; @@ -43,10 +43,11 @@ function auth(who: Member) { return { Authorization: `Bearer ${generateAccessToken({ ...who, serverHost: host, tokenVersion: 0 })}` }; } -async function upload(who: Member, body: Buffer, type = "image/png", name = "banner.png") { +async function upload(who: Member, body: Buffer, type = "image/png", name = "banner.png", endpoint = "/banner", sealed = false) { const form = new FormData(); form.append("file", new Blob([new Uint8Array(body)], { type }), name); - const res = await fetch(`${base}/api/uploads/banner`, { method: "POST", headers: auth(who), body: form }); + if (sealed) form.append("sealed", "1"); + const res = await fetch(`${base}/api/uploads${endpoint}`, { method: "POST", headers: auth(who), body: form }); return { status: res.status, body: (await res.json()) as Record }; } @@ -83,6 +84,42 @@ before(async () => { base = `http://${host}`; }); +describe("shared chat upload processing", () => { + it("blocks new unencrypted pictures until the worker approves them", async () => { + const who = await member("chat-picture"); + const bytes = await png(16, 16); + const response = await upload(who, bytes, "image/png", "chat.png", ""); + assert.equal(response.status, 201, JSON.stringify(response.body)); + const fileId = response.body.fileId as string; + assert.equal(response.body.processing, true); + assert.ok((await getFile(fileId))?.s3_key.startsWith("quarantine/uploads/")); + assert.equal((await getImageJob(fileId))?.status, "queued"); + const token = generateFileToken({ ...who, serverHost: host, tokenVersion: 0 }); + const url = `${base}/api/uploads/files/${fileId}?t=${encodeURIComponent(token)}`; + assert.equal((await fetch(url)).status, 503); + await updateImageJobStatus({ job_id: fileId, status: "done" }); + const ready = await fetch(url); + assert.equal(ready.status, 200); + assert.deepEqual(Buffer.from(await ready.arrayBuffer()), bytes); + }); + + it("keeps encrypted attachments opaque and does not pass ciphertext to a decoder", async () => { + const who = await member("sealed-chat"); + const bytes = Buffer.from("opaque encrypted bytes"); + const response = await upload(who, bytes, "image/png", "private.png", "", true); + assert.equal(response.status, 201, JSON.stringify(response.body)); + const fileId = response.body.fileId as string; + assert.equal(response.body.processing, false); + assert.equal(await getImageJob(fileId), null); + assert.equal((await getFile(fileId))?.mime, "application/octet-stream"); + const token = generateFileToken({ ...who, serverHost: host, tokenVersion: 0 }); + const ready = await fetch(`${base}/api/uploads/files/${fileId}?t=${encodeURIComponent(token)}`); + assert.equal(ready.status, 200); + assert.match(ready.headers.get("content-disposition") ?? "", /^attachment;/); + assert.deepEqual(Buffer.from(await ready.arrayBuffer()), bytes); + }); +}); + after(async () => { await new Promise((resolve) => server.close(() => resolve())); delete process.env.DATA_DIR; @@ -132,8 +169,8 @@ describe("POST /api/uploads/banner", () => { const file = await getFile(res.body.bannerFileId as string); assert.equal(file?.mime, "video/mp4"); - assert.equal(file?.width, null); - assert.equal(file?.height, null); + assert.equal(file?.width, 160); + assert.equal(file?.height, 90); assert.equal((await getImageJob(res.body.bannerFileId as string))?.status, "queued"); }); @@ -160,6 +197,8 @@ describe("POST /api/uploads/banner", () => { assert.equal((await fetch(url, { method: "HEAD" })).status, 503); assert.equal((await fetch(`${url}&thumb=1`)).status, 503); await updateImageJobStatus({ job_id: fileId, status: "done" }); + assert.equal((await fetch(url)).status, 422, "a done job without a decoded poster must remain blocked"); + await updateFileRecord(fileId, { thumbnail_key: `thumbnails/${fileId}.jpg` }); const ready = await fetch(url, { headers: { Range: "bytes=0-15" } }); assert.equal(ready.status, 206); assert.equal((await ready.arrayBuffer()).byteLength, 16); diff --git a/src/routes/uploadStorage.test.ts b/src/routes/uploadStorage.test.ts index 6733db41..cf83becc 100644 --- a/src/routes/uploadStorage.test.ts +++ b/src/routes/uploadStorage.test.ts @@ -172,7 +172,7 @@ describe("the route uses it", () => { // sharp, the worker and the SVG sanitiser would each be handed it. for (const [what, gate] of [ ["the SVG sanitiser", "if (storage.treatAsSvg)"], - ["image validation", "if (storage.validateAsImage)"], + ["image validation", "if (storage.validateAsImage && input.purpose !== \"banner\")"], ["the video container parser", "if (storage.measureAsVideo)"], ["the image worker", "if (storage.queueImageJob)"], ]) { @@ -187,7 +187,7 @@ describe("the route uses it", () => { assert.doesNotMatch(service, /ffmpeg|child_process|execFile|spawn\(/); }); - it("answers the uploader the same whether or not a poster ever comes", () => { - assert.match(route, /res\.status\(201\)\.json\(\{ fileId: stored\.fileId, key: stored\.key, thumbnailKey: null \}\)/); + it("reports whether the shared worker must finish before media can be read", () => { + assert.match(route, /res\.status\(201\)\.json\(\{ fileId: stored\.fileId, key: stored\.key, thumbnailKey: null, processing: stored\.processing \}\)/); }); }); diff --git a/src/routes/uploads.ts b/src/routes/uploads.ts index f5a37661..474490fc 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -131,7 +131,7 @@ uploadsRouter.post( res.status(stored.status).json({ error: stored.error, message: stored.message }); return; } - res.status(201).json({ fileId: stored.fileId, key: stored.key, thumbnailKey: null }); + res.status(201).json({ fileId: stored.fileId, key: stored.key, thumbnailKey: null, processing: stored.processing }); }) // Every exit path, including the early returns and anything that threw: // multer's temp file is ours and nothing else removes it. @@ -444,12 +444,16 @@ uploadsRouter.get( const fileMeta = verdict === "allowed" ? await getFile(fileId) : null; if (!fileMeta) { res.status(404).json({ error: "File not found" }); return; } - // Quarantined banners stay unreadable when the worker is absent or fails. - if (fileMeta.s3_key.startsWith("quarantine/banners/") || fileMeta.s3_key.startsWith("banners/verified/")) { - const job = await getImageJob(fileId); + // New media stays unreadable when the worker is absent or fails. + const job = await getImageJob(fileId); + if (job || fileMeta.s3_key.startsWith("quarantine/") || fileMeta.s3_key.startsWith("banners/verified/")) { if (job?.status !== "done") { res.setHeader("Cache-Control", "no-store"); - res.status(job?.status === "error" ? 422 : 503).json({ error: "banner_not_ready" }); + res.status(job?.status === "error" ? 422 : 503).json({ error: "media_not_ready" }); + return; + } + if (fileMeta.mime?.startsWith("video/") && !fileMeta.thumbnail_key) { + res.status(422).json({ error: "media_not_validated" }); return; } } diff --git a/src/services/storeBannerUpload.ts b/src/services/storeBannerUpload.ts index 357d9738..ed3f82ce 100644 --- a/src/services/storeBannerUpload.ts +++ b/src/services/storeBannerUpload.ts @@ -1,6 +1,4 @@ -import { v4 as uuidv4 } from "uuid"; -import { insertFile, insertImageJob } from "../db"; -import { deleteObject, putObject } from "../storage"; +import { storeUploadedFile } from "./storeUploadedFile"; const bannerTypes = new Set(["image/png", "image/jpeg", "image/gif", "image/webp", "image/avif", "video/mp4"]); @@ -11,17 +9,7 @@ export async function storeBannerUpload(input: { const type = file.mimetype.toLowerCase(); if (!bannerTypes.has(type)) return { ok: false as const, status: 400, error: "invalid_file", message: "Only raster images and MP4 videos are allowed" }; if (maxBytes > 0 && file.size > maxBytes) return { ok: false as const, status: 413, error: "file_too_large", message: "Banner exceeds the server upload limit" }; - const fileId = uuidv4(); - const key = `quarantine/banners/${fileId}`; - await putObject({ bucket, key, sourcePath: file.path, contentType: type }); - try { - await insertFile({ file_id: fileId, s3_key: key, mime: type, size: file.size, - width: null, height: null, thumbnail_key: null, original_name: file.originalname, - uploaded_by_server_user_id: uploadedBy }); - await insertImageJob({ job_id: fileId, file_id: fileId, raw_s3_key: key, raw_content_type: type, raw_bytes: file.size }); - } catch (error) { - await deleteObject({ bucket, key }).catch(() => undefined); - throw error; - } - return { ok: true as const, fileId, processing: true }; + const stored = await storeUploadedFile({ bucket, path: file.path, size: file.size, mimetype: type, + originalName: file.originalname, uploadedBy, maxBytes, sealed: false, purpose: "banner" }); + return stored.ok ? { ...stored, processing: true } : stored; } diff --git a/src/services/storeUploadedFile.ts b/src/services/storeUploadedFile.ts index 45d8abc1..c83abc22 100644 --- a/src/services/storeUploadedFile.ts +++ b/src/services/storeUploadedFile.ts @@ -1,10 +1,9 @@ -import consola from "consola"; import { readFile } from "fs/promises"; import { v4 as uuidv4 } from "uuid"; import { insertFile, insertImageJob } from "../db"; import { storageForUpload } from "../routes/uploadStorage"; -import { putObject } from "../storage"; +import { deleteObject, putObject } from "../storage"; import { validateImage } from "../utils/imageValidation"; import { sanitizeSvg } from "../utils/svgSanitize"; import { PARSE_LIMITS, readVideoDimensionsFromFile } from "../utils/videoDimensions"; @@ -14,7 +13,7 @@ import { PARSE_LIMITS, readVideoDimensionsFromFile } from "../utils/videoDimensi export const IMAGE_VALIDATION_MAX_BYTES = 64 * 1024 * 1024; export type StoreUploadedFileResult = - | { ok: true; fileId: string; key: string } + | { ok: true; fileId: string; key: string; processing: boolean } | { ok: false; status: 400 | 413; error: "invalid_file" | "file_too_large"; message: string }; function positiveInt(n: number | null | undefined): number | null { @@ -39,6 +38,7 @@ export async function storeUploadedFile(input: { claimedHeight?: number | null; /** A caller that needs the same id on every run passes its own. */ fileId?: string; + purpose?: "banner"; }): Promise { const fileId = input.fileId ?? uuidv4(); const { bucket, path, size } = input; @@ -63,7 +63,10 @@ export async function storeUploadedFile(input: { }; } - const { key, storedMime } = storage; + const { storedMime } = storage; + const key = input.purpose === "banner" + ? `quarantine/banners/${fileId}` + : storage.queueImageJob ? `quarantine/${storage.key}` : storage.key; let width: number | null = null; let height: number | null = null; @@ -91,10 +94,10 @@ export async function storeUploadedFile(input: { uploaded_by_server_user_id: input.uploadedBy, created_at: new Date(), }); - return { ok: true, fileId, key: svgKey }; + return { ok: true, fileId, key: svgKey, processing: false }; } - if (storage.validateAsImage) { + if (storage.validateAsImage && input.purpose !== "banner") { // The mime off the request is a claim, and taking it meant an SVG // carrying