diff --git a/src/routes/bannerUpload.test.ts b/src/routes/bannerUpload.test.ts index c5c98dbc..0273c21d 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"; @@ -10,15 +10,17 @@ 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 } 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. */ @@ -41,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 }; } @@ -55,6 +58,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); @@ -72,6 +76,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; @@ -79,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; @@ -88,7 +129,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()); @@ -99,10 +140,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 () => { @@ -120,6 +162,63 @@ 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); + assert.equal((await getImageJob(res.body.bannerFileId as string))?.status, "queued"); + }); + + 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, 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" }); + 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); + }); + + 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 () => { const carol = await member("carol"); const first = (await upload(carol, await png())).body.bannerFileId as string; 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 187f3f77..474490fc 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -7,7 +7,6 @@ import mime from "mime-types"; 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,6 +19,8 @@ import { sendStoredBody } from "../utils/sendStoredBody"; import { ensurePermission } from "../middleware/requirePermission"; import { sanitizeSvg } from "../utils/svgSanitize"; import { storeUploadedFile } from "../services/storeUploadedFile"; +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. */ @@ -130,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. @@ -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,42 @@ 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; + const stored = await storeBannerUpload({ bucket, file, uploadedBy: serverUserId, maxBytes }); + + 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 +287,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. @@ -418,6 +444,20 @@ uploadsRouter.get( const fileMeta = verdict === "allowed" ? await getFile(fileId) : null; if (!fileMeta) { res.status(404).json({ error: "File not found" }); return; } + // 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: "media_not_ready" }); + return; + } + if (fileMeta.mime?.startsWith("video/") && !fileMeta.thumbnail_key) { + res.status(422).json({ error: "media_not_validated" }); + 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..ed3f82ce --- /dev/null +++ b/src/services/storeBannerUpload.ts @@ -0,0 +1,15 @@ +import { storeUploadedFile } from "./storeUploadedFile"; + +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 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