diff --git a/src/routes/server.ts b/src/routes/server.ts index 1259a6c1..92d25ad2 100644 --- a/src/routes/server.ts +++ b/src/routes/server.ts @@ -1,4 +1,6 @@ import consola from "consola"; +import { reencodeThroughWorker } from "../services/workerReencode"; +import { workerClearsQuarantine } from "../services/workerCapabilities"; import express from "express"; import type { Request, Response, NextFunction } from "express"; import multer from "multer"; @@ -208,9 +210,18 @@ serverRouter.post( iconMime === "image/webp" || iconMime === "image/avif"; - const validation = await validateImage(file.buffer, { - animated: isAnimated, - }); + // Written out in the worker's jail where there is one, at the same 256px square. + const fromWorker = workerClearsQuarantine() + ? await reencodeThroughWorker(file.buffer, iconMime, "avatars").catch(() => null) + : undefined; + if (fromWorker === null) { + res.status(400).json({ error: "invalid_file", message: "Could not process image. Please upload a valid PNG/JPEG/WebP/GIF/AVIF under the size limit." }); + return; + } + + const validation = fromWorker + ? { valid: true as const, pages: 1 } + : await validateImage(file.buffer, { animated: isAnimated }); if (!validation.valid) { res .status(400) @@ -238,7 +249,9 @@ serverRouter.post( const outExt = isAnimated ? "webp" : "avif"; let out: Buffer; - try { + if (fromWorker) { + out = fromWorker.body; + } else try { const pipeline = sharp(file.buffer, { animated: isAnimated, failOn: "error", @@ -259,9 +272,9 @@ serverRouter.post( return; } - const key = `server-icons/${safeHost}/${uuidv4()}.${outExt}`; + const key = `server-icons/${safeHost}/${uuidv4()}.${fromWorker ? fromWorker.ext : outExt}`; try { - await putObject({ bucket, key, body: out, contentType: outMime }); + await putObject({ bucket, key, body: out, contentType: fromWorker ? fromWorker.mime : outMime }); } catch (e) { const raw = e instanceof Error ? e.message : ""; consola.error("icon upload s3 error", { bucket, key, message: raw }); diff --git a/src/routes/uploadQuarantine.test.ts b/src/routes/uploadQuarantine.test.ts index 77d6c243..693f0664 100644 --- a/src/routes/uploadQuarantine.test.ts +++ b/src/routes/uploadQuarantine.test.ts @@ -250,4 +250,37 @@ describe("uploads with a worker that clears quarantine", () => { const after = (await enrichAttachments([message]))[0].enriched_attachments?.[0]; assert.deepEqual([after?.processing, after?.refused], [false, true]); }); + + it("takes a group picture through quarantine too", async () => { + await workerSays(["quarantine-v1"]); + const jo = await member("jo"); + const form = new FormData(); + form.append("file", new Blob([new Uint8Array(await png())], { type: "image/png" }), "group.png"); + const raw = await fetch(`${base}/api/uploads/group-icon`, { method: "POST", headers: auth(jo), body: form }); + const res = { status: raw.status, body: (await raw.json()) as Record }; + assert.equal(res.status, 201, JSON.stringify(res.body)); + const id = (res.body.fileId ?? res.body.avatarFileId) as string; + assert.ok((await getFile(id))?.s3_key.startsWith("quarantine/avatars/")); + }); + + it("hands an emoji to the worker and takes back only its copy", async () => { + await workerSays(["quarantine-v1"]); + // A stand-in worker: writes its own bytes for whatever lands in quarantine/emojis/. + const fake = setInterval(() => { + const row = getSqliteDb().prepare("SELECT file_id FROM files WHERE s3_key LIKE 'quarantine/emojis/%' LIMIT 1").get() as { file_id: string } | undefined; + if (!row) return; + void putObject({ bucket: "gryt-test", key: `emojis/${row.file_id}.avif`, body: Buffer.from("worker copy"), contentType: "image/avif" }).then(() => + workerWritesOut(row.file_id, `emojis/${row.file_id}.avif`, "image/avif")); + }, 50); + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { processEmojiToOptimizedImage } = require("../utils/emojiProcessing") as typeof import("../utils/emojiProcessing"); + const out = await processEmojiToOptimizedImage(await png(), "image/png"); + assert.deepEqual([out.processed.toString(), out.ext, out.contentType], ["worker copy", "avif", "image/avif"]); + const left = getSqliteDb().prepare("SELECT COUNT(*) AS n FROM files WHERE s3_key LIKE '%emojis/%'").get() as { n: number }; + assert.equal(left.n, 0, "the temporary file is gone; the caller stores its own"); + } finally { + clearInterval(fake); + } + }); }); diff --git a/src/routes/uploads.ts b/src/routes/uploads.ts index 4ebfb489..f4c56cc7 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -269,12 +269,12 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => return; } - // With a worker that clears quarantine, the server never decodes the file itself. + // With a worker that clears quarantine the server never decodes it, whatever the picture is for. // A video only ever gets here with a worker that transcodes it. - const stored = (purpose === "avatar" || purpose === "banner") && (isVideo || workerClearsQuarantine()) + const stored = isVideo || workerClearsQuarantine() ? { ok: true as const, processing: true, ...(await quarantineUpload({ bucket, - use: purpose === "avatar" ? "avatars" : "banners", + use: purpose === "banner" ? "banners" : "avatars", bytes: file.buffer, mime: (file.mimetype || "application/octet-stream").toLowerCase(), originalName: file.originalname || null, diff --git a/src/services/webhookMedia.ts b/src/services/webhookMedia.ts index f290deee..3566b99c 100644 --- a/src/services/webhookMedia.ts +++ b/src/services/webhookMedia.ts @@ -19,6 +19,9 @@ import { putObject } from "../storage"; import { validateImage } from "../utils/imageValidation"; import { fetchFollowingSafely } from "../utils/safePreviewFetch"; import { storeAvatarPicture } from "./avatarImage"; +import { QUARANTINE_PREFIX, quarantineUpload } from "./quarantineUpload"; +import { workerClearsQuarantine } from "./workerCapabilities"; +import { settleChatAttachment } from "./chatAttachmentSettle"; /** The whole request's picture fetching, so a slow host can't hold a webhook call open. */ export const MEDIA_DEADLINE_MS = 12_000; @@ -43,7 +46,7 @@ export type FetchOutcome = export interface MediaDeps { fetchBytes(url: string, signal: AbortSignal, maxBytes: number): Promise; - storeImage(webhookId: string, bytes: Buffer, format: ImageFormat, width: number, height: number): Promise; + storeImage(webhookId: string, bytes: Buffer, format: ImageFormat, width: number | null, height: number | null): Promise; storeAvatar(webhookId: string, bytes: Buffer, format: ImageFormat): Promise; } @@ -115,7 +118,7 @@ async function fetchBytesSafely(url: string, signal: AbortSignal, maxBytes: numb /** One stored file per distinct picture a webhook sends. Only reused while a message still points at it, since an unreferenced file may be swept at any moment. */ -async function storeWebhookImage(webhookId: string, bytes: Buffer, format: ImageFormat, width: number, height: number): Promise { +async function storeWebhookImage(webhookId: string, bytes: Buffer, format: ImageFormat, width: number | null, height: number | null): Promise { const sha256 = createHash("sha256").update(bytes).digest("hex"); const known = await getWebhookMediaFileId(webhookId, sha256); if (known && (await getFile(known)) && (await isFileReferencedByMessage(known))) return known; @@ -123,10 +126,12 @@ async function storeWebhookImage(webhookId: string, bytes: Buffer, format: Image const bucket = process.env.S3_BUCKET as string; const fileId = uuidv4(); const storage = storageForUpload({ sealed: false, fileId, mimetype: MIME[format], originalName: `webhook.${format}` }); - await putObject({ bucket, key: storage.key, body: bytes, contentType: storage.storedMime }); + // Fetched from anywhere, so written out again in the worker's jail where there is one. + const key = workerClearsQuarantine() ? `${QUARANTINE_PREFIX}${storage.key}` : storage.key; + await putObject({ bucket, key, body: bytes, contentType: storage.storedMime }); await insertFile({ file_id: fileId, - s3_key: storage.key, + s3_key: key, mime: storage.storedMime, size: bytes.length, width, @@ -138,10 +143,11 @@ async function storeWebhookImage(webhookId: string, bytes: Buffer, format: Image created_at: new Date(), }); if (storage.queueImageJob) { - await insertImageJob({ file_id: fileId, raw_s3_key: storage.key, raw_content_type: storage.storedMime, raw_bytes: bytes.length }) + await insertImageJob({ file_id: fileId, raw_s3_key: key, raw_content_type: storage.storedMime, raw_bytes: bytes.length }) .catch((e: unknown) => consola.warn("Failed to queue webhook image job", e)); } await setWebhookMediaFileId(webhookId, sha256, fileId); + if (key !== storage.key) void settleChatAttachment(fileId); return fileId; } @@ -152,7 +158,16 @@ async function storeWebhookAvatar(webhookId: string, bytes: Buffer, format: Imag const known = await getWebhookMediaFileId(webhookId, mediaKey); if (known && (await getFile(known)) && (await isFileReferencedByMessage(known))) return known; - const stored = await storeAvatarPicture({ + const stored = workerClearsQuarantine() + ? { ok: true as const, ...(await quarantineUpload({ + bucket: process.env.S3_BUCKET as string, + use: "avatars", + bytes, + mime: MIME[format], + originalName: `webhook.${format}`, + uploadedBy: `webhook:${webhookId}`, + })) } + : await storeAvatarPicture({ bucket: process.env.S3_BUCKET as string, bytes, mime: MIME[format], @@ -240,7 +255,10 @@ export async function resolveWebhookMedia( if (!format) { entry.code = "unsupported_type"; return; } if (fetched.bytes.length > budget) { entry.code = "media_budget"; return; } budget -= fetched.bytes.length; - const checked = await validateImage(fetched.bytes, { animated: true }); + // The worker decodes it where there is one; a picture it refuses is never served. + const checked = workerClearsQuarantine() + ? { valid: true as const, width: null, height: null } + : await validateImage(fetched.bytes, { animated: true }); if (!checked.valid) { entry.code = "invalid_image"; return; } const store = async (save: () => Promise): Promise => { try { diff --git a/src/services/workerReencode.ts b/src/services/workerReencode.ts new file mode 100644 index 00000000..2e09a51d --- /dev/null +++ b/src/services/workerReencode.ts @@ -0,0 +1,51 @@ +import { v4 as uuidv4 } from "uuid"; + +import { getFile, insertFile, insertImageJob } from "../db"; +import { deleteFilesNow } from "../jobs/mediaSweep"; +import { getObjectAsBuffer, putObject } from "../storage"; +import { QUARANTINE_PREFIX, settleQuarantine } from "./quarantineUpload"; + +/* A picture the server needs back, such as an emoji or its icon, decoded in the worker's + jail instead of here. The worker's copy is returned and the temporary file is dropped. */ +const WAIT_MS = 2 * 60_000; + +export interface WorkerCopy { + body: Buffer; + mime: string; + ext: string; +} + +export async function reencodeThroughWorker( + bytes: Buffer, + mime: string, + use: "emojis" | "avatars", +): Promise { + const bucket = process.env.S3_BUCKET as string; + const fileId = uuidv4(); + const key = `${QUARANTINE_PREFIX}${use}/${fileId}`; + await putObject({ bucket, key, body: bytes, contentType: mime }); + await insertFile({ + file_id: fileId, + s3_key: key, + mime, + size: bytes.length, + width: null, + height: null, + thumbnail_key: null, + original_name: null, + uploaded_by_server_user_id: null, + created_at: new Date(), + }); + await insertImageJob({ job_id: uuidv4(), file_id: fileId, raw_s3_key: key, raw_content_type: mime, raw_bytes: bytes.length }); + try { + const verdict = await settleQuarantine(fileId, undefined, WAIT_MS); + if (verdict !== "ready") throw new Error(verdict === "refused" ? "The image could not be processed." : "The image worker took too long."); + const done = await getFile(fileId); + if (!done) throw new Error("The image could not be processed."); + const out = await getObjectAsBuffer({ bucket, key: done.s3_key }); + return { body: out, mime: done.mime ?? mime, ext: done.s3_key.split(".").pop() || "bin" }; + } finally { + // The caller stores its own copy, under its own key. + await deleteFilesNow([fileId]).catch(() => undefined); + } +} diff --git a/src/utils/emojiProcessing.ts b/src/utils/emojiProcessing.ts index 22d206dd..3b2143ba 100644 --- a/src/utils/emojiProcessing.ts +++ b/src/utils/emojiProcessing.ts @@ -1,5 +1,7 @@ import sharp from "sharp"; +import { reencodeThroughWorker } from "../services/workerReencode"; +import { workerClearsQuarantine } from "../services/workerCapabilities"; import { MAX_INPUT_PIXELS, validateImage } from "./imageValidation"; const ANIMATED_MIME_SET = new Set(["image/gif", "image/webp", "image/avif"]); @@ -8,6 +10,12 @@ export async function processEmojiToOptimizedImage( buffer: Buffer, mime: string, ): Promise<{ processed: Buffer; ext: string; contentType: string }> { + // Decoded in the worker's jail where there is one; every emoji path comes through here (GRYT-1664). + if (workerClearsQuarantine()) { + const copy = await reencodeThroughWorker(buffer, mime, "emojis"); + return { processed: copy.body, ext: copy.ext, contentType: copy.mime }; + } + const animated = ANIMATED_MIME_SET.has(mime); const startedAt = Date.now(); console.log("[EmojiProcess] start", { mime, animated, bytes: buffer.length }); diff --git a/src/utils/imageHeaderSize.test.ts b/src/utils/imageHeaderSize.test.ts new file mode 100644 index 00000000..599096ba --- /dev/null +++ b/src/utils/imageHeaderSize.test.ts @@ -0,0 +1,28 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import sharp from "sharp"; + +import { imageHeaderSize } from "./imageHeaderSize"; + +/* GRYT-1664: link previews read a stranger's picture, so the server takes its size from the + header and never decodes it. The pictures here are made by sharp only to have real headers. */ +describe("imageHeaderSize", () => { + const make = (format: "png" | "jpeg" | "gif" | "webp", lossless = false) => + sharp({ create: { width: 333, height: 177, channels: 3, background: "#4a8" } })[format](format === "webp" ? { lossless } : {}).toBuffer(); + + for (const [name, f, lossless] of [["PNG", "png"], ["JPEG", "jpeg"], ["GIF", "gif"], ["lossy WebP", "webp", false], ["lossless WebP", "webp", true]] as const) { + it(`reads a ${name}'s size`, async () => { + assert.deepEqual(imageHeaderSize(await make(f, lossless)), { width: 333, height: 177 }); + }); + } + + it("answers null for anything else, or a header that lies", () => { + assert.equal(imageHeaderSize(Buffer.from("")), null); + assert.equal(imageHeaderSize(Buffer.alloc(0)), null); + const zero = Buffer.alloc(24); + zero.writeUInt32BE(0x89504e47, 0); + zero.write("IHDR", 12, "latin1"); + assert.equal(imageHeaderSize(zero), null, "a zero-sized PNG is not believed"); + }); +}); diff --git a/src/utils/imageHeaderSize.ts b/src/utils/imageHeaderSize.ts new file mode 100644 index 00000000..2313e439 --- /dev/null +++ b/src/utils/imageHeaderSize.ts @@ -0,0 +1,35 @@ +/* A remote picture's size from its header, for laying out a link preview. Never a decode: the + bytes are a stranger's, and this runs in the server. Unknown formats answer null. */ + +export function imageHeaderSize(b: Buffer): { width: number; height: number } | null { + const ok = (w: number, h: number) => (w > 0 && h > 0 && w <= 65535 && h <= 65535 ? { width: w, height: h } : null); + if (b.length >= 24 && b.readUInt32BE(0) === 0x89504e47 && b.toString("latin1", 12, 16) === "IHDR") { + return ok(b.readUInt32BE(16), b.readUInt32BE(20)); + } + if (b.length >= 10 && b.toString("latin1", 0, 4) === "GIF8") return ok(b.readUInt16LE(6), b.readUInt16LE(8)); + if (b.length >= 30 && b.toString("latin1", 0, 4) === "RIFF" && b.toString("latin1", 8, 12) === "WEBP") { + const chunk = b.toString("latin1", 12, 16); + if (chunk === "VP8X") return ok(1 + b.readUIntLE(24, 3), 1 + b.readUIntLE(27, 3)); + if (chunk === "VP8 " && b[23] === 0x9d && b[24] === 0x01 && b[25] === 0x2a) return ok(b.readUInt16LE(26) & 0x3fff, b.readUInt16LE(28) & 0x3fff); + if (chunk === "VP8L" && b[20] === 0x2f) { + const bits = b.readUInt32LE(21); + return ok((bits & 0x3fff) + 1, ((bits >> 14) & 0x3fff) + 1); + } + return null; + } + if (b.length >= 4 && b[0] === 0xff && b[1] === 0xd8) { + // Walk the markers to the first start-of-frame, which holds the size. + let at = 2; + while (at + 9 < b.length) { + if (b[at] !== 0xff) return null; + const marker = b[at + 1]; + if (marker === 0xd8 || (marker >= 0xd0 && marker <= 0xd7) || marker === 0x01) { at += 2; continue; } + const length = b.readUInt16BE(at + 2); + if (length < 2) return null; + const sof = marker >= 0xc0 && marker <= 0xcf && marker !== 0xc4 && marker !== 0xc8 && marker !== 0xcc; + if (sof) return ok(b.readUInt16BE(at + 7), b.readUInt16BE(at + 5)); + at += 2 + length; + } + } + return null; +} diff --git a/src/utils/remoteImageMetadata.ts b/src/utils/remoteImageMetadata.ts index ebe96aeb..7c100bff 100644 --- a/src/utils/remoteImageMetadata.ts +++ b/src/utils/remoteImageMetadata.ts @@ -1,7 +1,6 @@ import consola from "consola"; -import sharp from "sharp"; -import { MAX_INPUT_PIXELS } from "./imageValidation"; +import { imageHeaderSize } from "./imageHeaderSize"; import { fetchFollowingSafely, type FetchGuard } from "./safePreviewFetch"; export type RemoteImageMetadata = { @@ -68,7 +67,7 @@ export async function fetchRemoteImageMetadata(url: string, guard?: FetchGuard): const buf = await readUpToBytes(res, 450_000); if (buf === null) return empty; - const meta = await sharp(buf, { animated: true, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }).metadata().catch(() => null); + const meta = imageHeaderSize(buf); const data: RemoteImageMetadata = { url, mime: contentType || null,