diff --git a/openapi/webhooks.json b/openapi/webhooks.json index 7b5ffe5a..51ac5d20 100644 --- a/openapi/webhooks.json +++ b/openapi/webhooks.json @@ -42,7 +42,7 @@ "type": "string" }, "avatar_url": { - "description": "Posts with this picture instead of the webhook's. Fetched once and stored, same as card images.", + "description": "Posts with this picture instead of the webhook's. Downloaded once and resized to 256 px, like any other avatar.", "type": "string", "maxLength": 2048, "format": "uri" diff --git a/src/routes/uploadAuthOrder.test.ts b/src/routes/uploadAuthOrder.test.ts index 5cdd1b12..d1a9949e 100644 --- a/src/routes/uploadAuthOrder.test.ts +++ b/src/routes/uploadAuthOrder.test.ts @@ -49,7 +49,13 @@ describe("multer never runs before authentication", () => { assert.deepEqual( buffering, - ["POST /api/server/icon", "POST /api/uploads/", "POST /api/uploads/avatar", "POST /api/uploads/group-icon"], + [ + "POST /api/server/icon", + "POST /api/uploads/", + "POST /api/uploads/avatar", + "POST /api/uploads/group-icon", + "POST /api/uploads/webhook-avatar", + ], "the set of body-buffering routes changed; if that is deliberate, update this list", ); }); diff --git a/src/routes/uploads.ts b/src/routes/uploads.ts index f2451cc6..cabf79e7 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -4,21 +4,20 @@ import type { Request, Response, NextFunction } from "express"; import multer from "multer"; import { v4 as uuidv4 } from "uuid"; import mime from "mime-types"; -import sharp from "sharp"; import { execFile } from "child_process"; import { unlink, readFile } from "fs/promises"; import { tmpdir } from "os"; import { join } from "path"; -import { deleteObject, putObject, getObject } from "../storage"; -import { insertFile, insertImageJob, getFile, updateFileRecord, updateUserAvatar, setUserAvatar, getServerConfig, getUserByServerId, DEFAULT_AVATAR_MAX_BYTES, DEFAULT_UPLOAD_MAX_BYTES } from "../db"; +import { putObject, getObject } from "../storage"; +import { insertFile, insertImageJob, getFile, updateUserAvatar, setUserAvatar, getServerConfig, getUserByServerId, DEFAULT_AVATAR_MAX_BYTES, DEFAULT_UPLOAD_MAX_BYTES } from "../db"; import { isSealedUpload, storageForUpload } from "./uploadStorage"; import { requireBearerToken } from "../middleware/requireBearerToken"; import { verifyFileToken, type FileTokenPayload } from "../utils/jwt"; +import { storeAvatarPicture } from "../services/avatarImage"; import { fileReadVerdict } from "../services/fileAccess"; import { RangeNotSatisfiableError } from "../utils/byteRange"; import { ensurePermission } from "../middleware/requirePermission"; -import { AVATAR_MAX_PX, AVATAR_THUMB_PX } from "../constants/media"; -import { findDominantColor, validateImage, MAX_INPUT_PIXELS } from "../utils/imageValidation"; +import { validateImage } from "../utils/imageValidation"; import { sanitizeSvg } from "../utils/svgSanitize"; /** Takes multer's path, so a video never has to fit in memory. */ @@ -275,15 +274,15 @@ uploadsRouter.post( }, ); -/** One pipeline for both 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") => +/** One pipeline for all three 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") => (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 = purpose === "avatar" ? "Avatar" : "Group picture"; + const what = purpose === "avatar" ? "Avatar" : purpose === "group" ? "Group picture" : "Webhook avatar"; 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; } @@ -292,11 +291,6 @@ const storeAvatarImage = (purpose: "avatar" | "group") => const serverUserId = req.tokenPayload?.serverUserId; if (!serverUserId) { res.status(401).json({ error: "auth_required" }); return; } - const fileId = uuidv4(); - const inputMime = (file.mimetype || "").toLowerCase(); - const isAnimated = inputMime === "image/gif" || inputMime === "image/webp"; - const animExt = inputMime === "image/gif" ? "gif" : "webp"; - Promise.resolve() .then(async () => { const cfg = await getServerConfig().catch(() => null); @@ -312,15 +306,6 @@ const storeAvatarImage = (purpose: "avatar" | "group") => return; } - let key: string; - let storedBody: Buffer; - let storedMime: string; - let storedSize: number; - let width: number | null = null; - let height: number | null = null; - let thumbKey: string | null = null; - let processing = false; - // SVG never reaches sharp. Stored as the sanitised vector; see // svgSanitize.ts for why that is enough. if ((file.mimetype || "").toLowerCase() === "image/svg+xml") { @@ -330,8 +315,9 @@ const storeAvatarImage = (purpose: "avatar" | "group") => return; } + const fileId = uuidv4(); const body = Buffer.from(svg.svg, "utf8"); - key = `avatars/${fileId}.svg`; + const key = `avatars/${fileId}.svg`; await putObject({ bucket, key, body, contentType: "image/svg+xml" }); // No thumbnail: a vector is already the small one, and anything @@ -353,173 +339,25 @@ const storeAvatarImage = (purpose: "avatar" | "group") => return; } - const validation = await validateImage(file.buffer, { animated: isAnimated }); - if (!validation.valid) { - res.status(400).json({ error: "invalid_file", message: validation.reason }); - return; - } - width = validation.width; - height = validation.height; - - // Dimensions, not bytes: what is left is the modestly-sized animated - // avatar with large dimensions. - const withinBounds = - file.size <= maxBytes && - (width ?? 0) <= AVATAR_MAX_PX && - (height ?? 0) <= AVATAR_MAX_PX; - - if (isAnimated && withinBounds) { - key = `avatars/${fileId}.${animExt}`; - storedBody = file.buffer; - storedMime = inputMime; - storedSize = file.size; - - const thumb = await sharp(file.buffer, { pages: 1, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) - .resize({ width: AVATAR_THUMB_PX, height: AVATAR_THUMB_PX, fit: "cover" }) - .avif({ quality: 50 }) - .toBuffer() - .catch(() => null); - - if (thumb) { - thumbKey = `avatars/thumb_${fileId}.avif`; - await putObject({ bucket, key: thumbKey, body: thumb, contentType: "image/avif" }).catch((e) => { - console.error("avatar_thumb_s3_error", { bucket, key: thumbKey, message: (e instanceof Error ? e.message : "S3 upload failed.") }); - thumbKey = null; - }); - } - } else if (isAnimated) { - key = `avatars/${fileId}.avif`; - processing = true; - try { - storedBody = await sharp(file.buffer, { pages: 1, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) - .resize({ width: AVATAR_MAX_PX, height: AVATAR_MAX_PX, fit: "cover" }) - .avif() - .toBuffer(); - } catch { - res.status(400).json({ error: "invalid_file", message: "Could not process image." }); - return; - } - storedMime = "image/avif"; - storedSize = storedBody.length; - // What was stored, not what was uploaded: `cover` crops to exactly - // this box, so the original describes a file that is gone. - width = AVATAR_MAX_PX; - height = AVATAR_MAX_PX; - } else { - key = `avatars/${fileId}.avif`; - try { - storedBody = await sharp(file.buffer, { failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) - .resize({ width: AVATAR_MAX_PX, height: AVATAR_MAX_PX, fit: "cover" }) - .avif() - .toBuffer(); - } catch { - res.status(400).json({ error: "invalid_file", message: "Could not process image. Please upload a valid image under the size limit." }); - return; - } - storedMime = "image/avif"; - storedSize = storedBody.length; - width = AVATAR_MAX_PX; - height = AVATAR_MAX_PX; - - const thumb = await sharp(file.buffer, { failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) - .resize({ width: AVATAR_THUMB_PX, height: AVATAR_THUMB_PX, fit: "cover" }) - .avif({ quality: 50 }) - .toBuffer() - .catch(() => null); - - if (thumb) { - thumbKey = `avatars/thumb_${fileId}.avif`; - await putObject({ bucket, key: thumbKey, body: thumb, contentType: "image/avif" }).catch((e) => { - console.error("avatar_thumb_s3_error", { bucket, key: thumbKey, message: (e instanceof Error ? e.message : "S3 upload failed.") }); - thumbKey = null; - }); - } - } - - try { - await putObject({ bucket, key, body: storedBody, contentType: storedMime }); - } catch (e) { - const raw = (e instanceof Error && e.message.trim().length > 0) ? e.message : ""; - console.error("avatar_upload_s3_error", { bucket, key, message: raw }); - const friendly = - /InvalidBucketName|NoSuchBucket|bucket/i.test(raw) - ? "File storage is misconfigured on this server. Please contact the server administrator." - : /AccessDenied|Forbidden/i.test(raw) - ? "File storage access denied. Please contact the server administrator." - : raw.length > 0 - ? `${what} upload failed: ${raw}` - : `${what} upload failed due to a storage error.`; - res.status(502).json({ error: "s3_error", message: friendly }); + const stored = await storeAvatarPicture({ + bucket, + bytes: file.buffer, + mime: file.mimetype || "", + originalName: file.originalname || null, + uploadedBy: serverUserId, + maxBytes, + what, + }); + if (!stored.ok) { + res.status(stored.status).json({ error: stored.error, message: stored.message }); return; } - // From the original upload, not `storedBody`, which for an oversized - // animated avatar is a placeholder replaced further down. - const dominantColor = await findDominantColor(file.buffer, { animated: isAnimated }); - - await insertFile({ - file_id: fileId, - s3_key: key, - mime: storedMime, - size: storedSize, - width, - height, - thumbnail_key: thumbKey, - thumbnail_px: thumbKey ? AVATAR_THUMB_PX : null, - original_name: file.originalname || null, - dominant_color: dominantColor, - uploaded_by_server_user_id: serverUserId, - created_at: new Date(), - }); - if (purpose === "avatar") { - await updateUserAvatar(serverUserId, fileId); - res.status(201).json({ avatarFileId: fileId, processing }); + await updateUserAvatar(serverUserId, stored.fileId); + res.status(201).json({ avatarFileId: stored.fileId, processing: stored.processing }); } else { - res.status(201).json({ fileId, processing }); - } - - // Background: resize oversized animated file and replace the placeholder - if (processing) { - const animBuf = file.buffer; - setImmediate(() => { - (async () => { - try { - const outputFormat = inputMime === "image/gif" ? "gif" : "webp"; - const outputMime = `image/${outputFormat}`; - // Every frame is decoded here, unlike the single-page check - // that let this buffer through, so the ceiling comes with it. - const pipeline = sharp(animBuf, { animated: true, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) - .resize({ width: AVATAR_MAX_PX, height: AVATAR_MAX_PX, fit: "cover" }); - const resized = outputFormat === "gif" - ? await pipeline.gif().toBuffer() - : await pipeline.webp().toBuffer(); - - const animKey = `avatars/${fileId}.${outputFormat}`; - await putObject({ bucket, key: animKey, body: resized, contentType: outputMime }); - - const thumbBuf = await sharp(resized, { pages: 1, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) - .resize({ width: AVATAR_THUMB_PX, height: AVATAR_THUMB_PX, fit: "cover" }) - .avif({ quality: 50 }) - .toBuffer() - .catch(() => null); - const newThumbKey = thumbBuf ? `avatars/thumb_${fileId}.avif` : null; - if (thumbBuf && newThumbKey) { - await putObject({ bucket, key: newThumbKey, body: thumbBuf, contentType: "image/avif" }).catch(() => {}); - } - - await updateFileRecord(fileId, { s3_key: animKey, mime: outputMime, size: resized.length, thumbnail_key: newThumbKey, thumbnail_px: newThumbKey ? AVATAR_THUMB_PX : null }); - - if (animKey !== key) { - await deleteObject({ bucket, key }).catch(() => {}); - } - - consola.info(`Background avatar processing done for ${fileId} (${(resized.length / 1024).toFixed(0)}KB ${outputFormat})`); - } catch (err) { - consola.error(`Background avatar processing failed for ${fileId}`, err); - } - })(); - }); + res.status(201).json({ fileId: stored.fileId, processing: stored.processing }); } }) .catch(next); @@ -552,6 +390,19 @@ uploadsRouter.post( storeAvatarImage("group"), ); +uploadsRouter.post( + "/webhook-avatar", + requireBearerToken, + // The permission every webhook route asks for, since only they use this file. + (req: Request, res: Response, next: NextFunction): void => { + ensurePermission(req, res, "manage_webhooks") + .then((ok) => { if (ok) next(); }) + .catch(next); + }, + uploadAvatarToMemory("file"), + storeAvatarImage("webhook"), +); + uploadsRouter.delete( "/avatar", requireBearerToken, diff --git a/src/routes/webhookAvatar.test.ts b/src/routes/webhookAvatar.test.ts index 3e467236..ed80bfc7 100644 --- a/src/routes/webhookAvatar.test.ts +++ b/src/routes/webhookAvatar.test.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import { mkdtempSync, rmSync } from "node:fs"; import type { AddressInfo } from "node:net"; import type { Server } from "node:http"; +import type { Readable } from "node:stream"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { after, before, describe, it } from "node:test"; @@ -9,27 +10,40 @@ import { after, before, describe, it } from "node:test"; import express from "express"; import sharp from "sharp"; +import { AVATAR_MAX_PX, AVATAR_THUMB_PX } from "../constants/media"; import { initSqlite } from "../db/sqlite/connection"; -import { getFile } from "../db/sqlite/messages"; +import { getAllFileRecords, getFile, getFileOwnership, listMessages } from "../db/sqlite/messages"; import { createServerConfigIfNotExists, setServerRole } from "../db/sqlite/servers"; import { getUserByServerId, upsertUser } from "../db/sqlite/users"; import { unreferencedAmong } from "../jobs/mediaSweep"; -import { initStorage } from "../storage"; +import { fileReadVerdict } from "../services/fileAccess"; +import { realMediaDeps } from "../services/webhookMedia"; +import { getObject, initStorage } from "../storage"; import { generateAccessToken } from "../utils/jwt"; import { uploadsRouter } from "./uploads"; -import { webhooksRouter } from "./webhooks"; +import { setWebhookMediaDepsForTests, webhooksRouter } from "./webhooks"; -/** The sweep kept files that messages, user avatars and group icons point at, - but not webhook avatars, so those went 30 minutes after upload (GRYT-1183). */ +/** The sweep deleted webhook avatars 30 minutes after upload (GRYT-1183), and + they were stored at whatever size they were sent (GRYT-1185). */ + +interface Member { serverUserId: string; grytUserId: string; nickname: string } let dir: string; let server: Server; let base = ""; let host = ""; -let token = ""; -let serverUserId = ""; +let owner: Member; +let big: Buffer; + +async function member(nickname: string, role: "owner" | "member"): Promise { + const grytUserId = `acct-${nickname}`; + const user = await upsertUser(grytUserId, nickname); + await setServerRole(user.server_user_id, role); + return { serverUserId: user.server_user_id, grytUserId, nickname }; +} -async function call(path: string, init: RequestInit) { +async function call(path: string, init: RequestInit, who: Member = owner) { + const token = generateAccessToken({ ...who, serverHost: host, tokenVersion: 0 }); const res = await fetch(`${base}${path}`, { ...init, headers: { Authorization: `Bearer ${token}`, ...(init.headers ?? {}) }, @@ -37,28 +51,32 @@ async function call(path: string, init: RequestInit) { return { status: res.status, body: (await res.json()) as Record }; } +function uploadAvatar(picture: Buffer, who: Member = owner, type = "image/png", name = "hook.png") { + const form = new FormData(); + form.append("file", new Blob([new Uint8Array(picture)], { type }), name); + return call("/api/uploads/webhook-avatar", { method: "POST", body: form }, who); +} + async function uploadPicture(): Promise { const png = await sharp({ create: { width: 64, height: 64, channels: 3, background: { r: 230, g: 90, b: 30 } } }) .png() .toBuffer(); - const form = new FormData(); - form.append("file", new Blob([new Uint8Array(png)], { type: "image/png" }), "hook.png"); - const res = await call("/api/uploads", { method: "POST", body: form }); + const res = await uploadAvatar(png); assert.equal(res.status, 201, JSON.stringify(res.body)); - // The settings tab read `file_id`, which this route has never sent. + // The settings tab once read `file_id`, which no upload route sends. assert.equal(res.body.file_id, undefined); assert.ok(res.body.fileId, "no fileId in the reply"); return res.body.fileId as string; } -async function createHook(): Promise { +async function createHook(channel = "general"): Promise<{ id: string; token: string }> { const res = await call("/api/webhooks", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ channel_id: "general", display_name: "CI" }), + body: JSON.stringify({ channel_id: channel, display_name: "CI" }), }); assert.equal(res.status, 201, JSON.stringify(res.body)); - return res.body.webhook_id as string; + return { id: res.body.webhook_id as string, token: res.body.token as string }; } const setAvatar = (webhookId: string, fileId: string | null) => @@ -68,6 +86,15 @@ const setAvatar = (webhookId: string, fileId: string | null) => body: JSON.stringify({ avatar_file_id: fileId }), }); +/** The dimensions of what storage actually holds, rather than what the row says. */ +async function storedSize(key: string): Promise<[number | undefined, number | undefined]> { + const obj = await getObject({ bucket: "gryt-test", key }); + const chunks: Buffer[] = []; + for await (const chunk of obj.Body as Readable) chunks.push(chunk as Buffer); + const meta = await sharp(Buffer.concat(chunks)).metadata(); + return [meta.width, meta.height]; +} + before(async () => { dir = mkdtempSync(join(tmpdir(), "gryt-webhookavatar-")); process.env.DATA_DIR = dir; @@ -76,11 +103,11 @@ before(async () => { await initSqlite(); initStorage(); await createServerConfigIfNotExists(); + big = await sharp({ create: { width: 1600, height: 1200, channels: 3, background: { r: 40, g: 90, b: 200 } } }) + .png() + .toBuffer(); - const grytUserId = "acct-owner"; - const user = await upsertUser(grytUserId, "owner"); - await setServerRole(user.server_user_id, "owner"); - serverUserId = user.server_user_id; + owner = await member("owner", "owner"); const app = express(); app.use("/api/uploads", uploadsRouter); @@ -90,10 +117,15 @@ before(async () => { const { port } = server.address() as AddressInfo; host = `127.0.0.1:${port}`; base = `http://${host}`; - token = generateAccessToken({ serverUserId, grytUserId, nickname: "owner", serverHost: host, tokenVersion: 0 }); + setWebhookMediaDepsForTests({ + fetchBytes: async () => ({ ok: true, bytes: big }), + storeImage: realMediaDeps.storeImage, + storeAvatar: realMediaDeps.storeAvatar, + }); }); after(async () => { + setWebhookMediaDepsForTests(null); await new Promise((resolve) => server.close(() => resolve())); delete process.env.DATA_DIR; delete process.env.S3_BUCKET; @@ -104,12 +136,12 @@ after(async () => { describe("a webhook's avatar", () => { it("is kept by the media sweep once a webhook wears it", async () => { const fileId = await uploadPicture(); - const webhookId = await createHook(); + const webhook = await createHook(); // Nothing points at it yet, so a picture chosen and then abandoned is swept. assert.deepEqual(await unreferencedAmong([fileId]), [fileId]); - const res = await setAvatar(webhookId, fileId); + const res = await setAvatar(webhook.id, fileId); assert.equal(res.status, 200, JSON.stringify(res.body)); assert.equal(res.body.avatar_file_id, fileId); assert.deepEqual(await unreferencedAmong([fileId]), []); @@ -118,16 +150,116 @@ describe("a webhook's avatar", () => { it("goes back to the sweep once the webhook drops it", async () => { const fileId = await uploadPicture(); - const webhookId = await createHook(); - await setAvatar(webhookId, fileId); + const webhook = await createHook(); + await setAvatar(webhook.id, fileId); - await setAvatar(webhookId, null); + await setAvatar(webhook.id, null); assert.deepEqual(await unreferencedAmong([fileId]), [fileId]); }); it("leaves the uploader's own avatar alone", async () => { const fileId = await uploadPicture(); - await setAvatar(await createHook(), fileId); - assert.equal((await getUserByServerId(serverUserId))?.avatar_file_id ?? null, null); + await setAvatar((await createHook()).id, fileId); + assert.equal((await getUserByServerId(owner.serverUserId))?.avatar_file_id ?? null, null); + }); +}); + +describe("POST /api/uploads/webhook-avatar", () => { + it("stores a big picture at the avatar size, with a thumbnail", async () => { + const res = await uploadAvatar(big); + assert.equal(res.status, 201, JSON.stringify(res.body)); + assert.equal(res.body.avatarFileId, undefined); + + const row = await getFile(res.body.fileId as string); + assert.ok(row, "no file row"); + assert.equal(row.mime, "image/avif", "the picture skipped the avatar processing"); + assert.deepEqual([row.width, row.height], [AVATAR_MAX_PX, AVATAR_MAX_PX]); + // The image worker only rebuilds avatar thumbnails it finds under this prefix. + assert.match(row.s3_key, /^avatars\//); + assert.deepEqual(await storedSize(row.s3_key), [AVATAR_MAX_PX, AVATAR_MAX_PX]); + assert.ok(row.thumbnail_key, "no thumbnail"); + assert.equal(row.thumbnail_px, AVATAR_THUMB_PX); + assert.deepEqual(await storedSize(row.thumbnail_key), [AVATAR_THUMB_PX, AVATAR_THUMB_PX]); + + assert.equal((await getUserByServerId(owner.serverUserId))?.avatar_file_id ?? null, null); + }); + + it("leaves the uploader's own avatar alone on the SVG path too", async () => { + const svg = Buffer.from(''); + const res = await uploadAvatar(svg, owner, "image/svg+xml", "hook.svg"); + assert.equal(res.status, 200, JSON.stringify(res.body)); + assert.ok(res.body.fileId); + assert.equal((await getUserByServerId(owner.serverUserId))?.avatar_file_id ?? null, null); + }); + + it("refuses somebody without manage_webhooks, and stores nothing", async () => { + const carol = await member("carol", "member"); + const before = (await getAllFileRecords()).length; + + const res = await uploadAvatar(big, carol); + assert.equal(res.status, 403, JSON.stringify(res.body)); + assert.equal(res.body.permission, "manage_webhooks"); + assert.equal((await getAllFileRecords()).length, before); + }); + + it("is only the uploader's to read until a webhook wears it", async () => { + const dave = await member("dave", "member"); + const fileId = await uploadPicture(); + + // What PATCH checks before it will set the avatar. + assert.equal(await fileReadVerdict(fileId, owner.serverUserId, owner.grytUserId), "allowed"); + assert.equal(await fileReadVerdict(fileId, dave.serverUserId, dave.grytUserId), "denied"); + + await setAvatar((await createHook()).id, fileId); + assert.equal(await fileReadVerdict(fileId, dave.serverUserId, dave.grytUserId), "allowed"); + }); +}); + +describe("a webhook post's avatar_url", () => { + const post = (webhook: { id: string; token: string }) => + fetch(`${base}/api/webhooks/${webhook.id}/${webhook.token}`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + avatar_url: "https://ci.example/logo.png", + cards: [{ title: "Deployed", image_url: "https://ci.example/logo.png" }], + }), + }); + + it("is stored at the avatar size, while the same picture in a card stays as sent", async () => { + const webhook = await createHook("deploys"); + const res = await post(webhook); + assert.equal(res.status, 200, await res.text()); + + const [message] = await listMessages("deploys"); + const avatar = await getFile(message.sender_avatar_file_id!); + assert.ok(avatar, "no avatar file"); + assert.equal(avatar.mime, "image/avif", "the avatar skipped the avatar processing"); + assert.match(avatar.s3_key, /^avatars\//); + assert.deepEqual(await storedSize(avatar.s3_key), [AVATAR_MAX_PX, AVATAR_MAX_PX]); + assert.ok(avatar.thumbnail_key, "no thumbnail"); + + const image = await getFile(message.cards![0].image_file_id!); + assert.equal(image?.mime, "image/png"); + assert.deepEqual([image?.width, image?.height], [1600, 1200]); + + // The rules GRYT-1186 gave every picture a webhook sends still hold for this one. + const ownership = await getFileOwnership(avatar.file_id); + assert.equal(ownership?.uploadedBy, `webhook:${webhook.id}`); + assert.deepEqual(ownership?.attachedTo, ["deploys"]); + assert.deepEqual(await unreferencedAmong([avatar.file_id]), []); + }); + + it("is stored once for a webhook that sends the same one every time", async () => { + const webhook = await createHook("repeats"); + await post(webhook); + await post(webhook); + + const [first, second] = await listMessages("repeats"); + assert.ok(first.sender_avatar_file_id); + assert.equal(first.sender_avatar_file_id, second.sender_avatar_file_id); + // The same bytes as a card picture are a different file, and stay one on the second post. + const image = await getFile(second.cards![0].image_file_id!); + assert.equal(image?.mime, "image/png", "the second post's card picture is the resized avatar"); }); }); diff --git a/src/routes/webhookCards.test.ts b/src/routes/webhookCards.test.ts index df31feb7..45e191cd 100644 --- a/src/routes/webhookCards.test.ts +++ b/src/routes/webhookCards.test.ts @@ -70,7 +70,7 @@ before(async () => { const host = `127.0.0.1:${(server.address() as AddressInfo).port}`; base = `http://${host}`; token = generateAccessToken({ serverUserId: user.server_user_id, grytUserId: "acct-owner", nickname: "owner", serverHost: host, tokenVersion: 0 }); - setWebhookMediaDepsForTests({ fetchBytes: respond, storeImage: realMediaDeps.storeImage }); + setWebhookMediaDepsForTests({ fetchBytes: respond, storeImage: realMediaDeps.storeImage, storeAvatar: realMediaDeps.storeAvatar }); }); beforeEach(() => { fetches = []; }); diff --git a/src/routes/webhookSchemas.ts b/src/routes/webhookSchemas.ts index 9bc62eb0..1b0d1338 100644 --- a/src/routes/webhookSchemas.ts +++ b/src/routes/webhookSchemas.ts @@ -96,7 +96,7 @@ export const webhookMessageSchema = z .optional() .describe(`Posts under this name instead of the webhook's. Cut to ${L.displayName} characters.`), avatar_url: imageUrl.optional().describe( - "Posts with this picture instead of the webhook's. Fetched once and stored, same as card images.", + "Posts with this picture instead of the webhook's. Downloaded once and resized to 256 px, like any other avatar.", ), cards: z.array(webhookCardSchema).max(L.cards).optional(), }) diff --git a/src/services/avatarImage.ts b/src/services/avatarImage.ts new file mode 100644 index 00000000..9059e77d --- /dev/null +++ b/src/services/avatarImage.ts @@ -0,0 +1,202 @@ +import consola from "consola"; +import sharp from "sharp"; +import { v4 as uuidv4 } from "uuid"; + +import { AVATAR_MAX_PX, AVATAR_THUMB_PX } from "../constants/media"; +import { insertFile, updateFileRecord } from "../db"; +import { deleteObject, putObject } from "../storage"; +import { findDominantColor, validateImage, MAX_INPUT_PIXELS } from "../utils/imageValidation"; + +export type StoredAvatarPicture = + | { ok: true; fileId: string; processing: boolean } + | { ok: false; status: 400 | 502; error: "invalid_file" | "s3_error"; message: string }; + +/** Every raster avatar is stored through here, cut down to AVATAR_MAX_PX with a + thumbnail. SVG must not reach it: sharp renders SVG through librsvg. */ +export async function storeAvatarPicture(input: { + bucket: string; + bytes: Buffer; + /** As claimed. `validateImage` checks what the bytes really are. */ + mime: string; + originalName: string | null; + uploadedBy: string; + /** An animation this size or under, within the pixel bound, is kept as sent. */ + maxBytes: number; + /** Names the picture in a storage error, e.g. "Avatar". */ + what: string; +}): Promise { + const { bucket, bytes, originalName, uploadedBy, maxBytes, what } = input; + const fileId = uuidv4(); + const inputMime = input.mime.toLowerCase(); + const isAnimated = inputMime === "image/gif" || inputMime === "image/webp"; + const animExt = inputMime === "image/gif" ? "gif" : "webp"; + + let key: string; + let storedBody: Buffer; + let storedMime: string; + let storedSize: number; + let width: number | null = null; + let height: number | null = null; + let thumbKey: string | null = null; + let processing = false; + + const validation = await validateImage(bytes, { animated: isAnimated }); + if (!validation.valid) { + return { ok: false, status: 400, error: "invalid_file", message: validation.reason }; + } + width = validation.width; + height = validation.height; + + // Dimensions, not bytes: what is left is the modestly-sized animated + // avatar with large dimensions. + const withinBounds = + bytes.length <= maxBytes && + (width ?? 0) <= AVATAR_MAX_PX && + (height ?? 0) <= AVATAR_MAX_PX; + + if (isAnimated && withinBounds) { + key = `avatars/${fileId}.${animExt}`; + storedBody = bytes; + storedMime = inputMime; + storedSize = bytes.length; + + const thumb = await sharp(bytes, { pages: 1, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) + .resize({ width: AVATAR_THUMB_PX, height: AVATAR_THUMB_PX, fit: "cover" }) + .avif({ quality: 50 }) + .toBuffer() + .catch(() => null); + + if (thumb) { + thumbKey = `avatars/thumb_${fileId}.avif`; + await putObject({ bucket, key: thumbKey, body: thumb, contentType: "image/avif" }).catch((e) => { + console.error("avatar_thumb_s3_error", { bucket, key: thumbKey, message: (e instanceof Error ? e.message : "S3 upload failed.") }); + thumbKey = null; + }); + } + } else if (isAnimated) { + key = `avatars/${fileId}.avif`; + processing = true; + try { + storedBody = await sharp(bytes, { pages: 1, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) + .resize({ width: AVATAR_MAX_PX, height: AVATAR_MAX_PX, fit: "cover" }) + .avif() + .toBuffer(); + } catch { + return { ok: false, status: 400, error: "invalid_file", message: "Could not process image." }; + } + storedMime = "image/avif"; + storedSize = storedBody.length; + // What was stored, not what was uploaded: `cover` crops to exactly + // this box, so the original describes a file that is gone. + width = AVATAR_MAX_PX; + height = AVATAR_MAX_PX; + } else { + key = `avatars/${fileId}.avif`; + try { + storedBody = await sharp(bytes, { failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) + .resize({ width: AVATAR_MAX_PX, height: AVATAR_MAX_PX, fit: "cover" }) + .avif() + .toBuffer(); + } catch { + return { ok: false, status: 400, error: "invalid_file", message: "Could not process image. Please upload a valid image under the size limit." }; + } + storedMime = "image/avif"; + storedSize = storedBody.length; + width = AVATAR_MAX_PX; + height = AVATAR_MAX_PX; + + const thumb = await sharp(bytes, { failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) + .resize({ width: AVATAR_THUMB_PX, height: AVATAR_THUMB_PX, fit: "cover" }) + .avif({ quality: 50 }) + .toBuffer() + .catch(() => null); + + if (thumb) { + thumbKey = `avatars/thumb_${fileId}.avif`; + await putObject({ bucket, key: thumbKey, body: thumb, contentType: "image/avif" }).catch((e) => { + console.error("avatar_thumb_s3_error", { bucket, key: thumbKey, message: (e instanceof Error ? e.message : "S3 upload failed.") }); + thumbKey = null; + }); + } + } + + try { + await putObject({ bucket, key, body: storedBody, contentType: storedMime }); + } catch (e) { + const raw = (e instanceof Error && e.message.trim().length > 0) ? e.message : ""; + console.error("avatar_upload_s3_error", { bucket, key, message: raw }); + const friendly = + /InvalidBucketName|NoSuchBucket|bucket/i.test(raw) + ? "File storage is misconfigured on this server. Please contact the server administrator." + : /AccessDenied|Forbidden/i.test(raw) + ? "File storage access denied. Please contact the server administrator." + : raw.length > 0 + ? `${what} upload failed: ${raw}` + : `${what} upload failed due to a storage error.`; + return { ok: false, status: 502, error: "s3_error", message: friendly }; + } + + // From the original upload, not `storedBody`, which for an oversized + // animated avatar is a placeholder replaced further down. + const dominantColor = await findDominantColor(bytes, { animated: isAnimated }); + + await insertFile({ + file_id: fileId, + s3_key: key, + mime: storedMime, + size: storedSize, + width, + height, + thumbnail_key: thumbKey, + thumbnail_px: thumbKey ? AVATAR_THUMB_PX : null, + original_name: originalName, + dominant_color: dominantColor, + uploaded_by_server_user_id: uploadedBy, + created_at: new Date(), + }); + + // Background: resize oversized animated file and replace the placeholder + if (processing) { + const animBuf = bytes; + setImmediate(() => { + (async () => { + try { + const outputFormat = inputMime === "image/gif" ? "gif" : "webp"; + const outputMime = `image/${outputFormat}`; + // Every frame is decoded here, unlike the single-page check + // that let this buffer through, so the ceiling comes with it. + const pipeline = sharp(animBuf, { animated: true, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) + .resize({ width: AVATAR_MAX_PX, height: AVATAR_MAX_PX, fit: "cover" }); + const resized = outputFormat === "gif" + ? await pipeline.gif().toBuffer() + : await pipeline.webp().toBuffer(); + + const animKey = `avatars/${fileId}.${outputFormat}`; + await putObject({ bucket, key: animKey, body: resized, contentType: outputMime }); + + const thumbBuf = await sharp(resized, { pages: 1, failOn: "error", limitInputPixels: MAX_INPUT_PIXELS }) + .resize({ width: AVATAR_THUMB_PX, height: AVATAR_THUMB_PX, fit: "cover" }) + .avif({ quality: 50 }) + .toBuffer() + .catch(() => null); + const newThumbKey = thumbBuf ? `avatars/thumb_${fileId}.avif` : null; + if (thumbBuf && newThumbKey) { + await putObject({ bucket, key: newThumbKey, body: thumbBuf, contentType: "image/avif" }).catch(() => {}); + } + + await updateFileRecord(fileId, { s3_key: animKey, mime: outputMime, size: resized.length, thumbnail_key: newThumbKey, thumbnail_px: newThumbKey ? AVATAR_THUMB_PX : null }); + + if (animKey !== key) { + await deleteObject({ bucket, key }).catch(() => {}); + } + + consola.info(`Background avatar processing done for ${fileId} (${(resized.length / 1024).toFixed(0)}KB ${outputFormat})`); + } catch (err) { + consola.error(`Background avatar processing failed for ${fileId}`, err); + } + })(); + }); + } + + return { ok: true, fileId, processing }; +} diff --git a/src/services/webhookMedia.test.ts b/src/services/webhookMedia.test.ts index 28fa5b73..08e15c1b 100644 --- a/src/services/webhookMedia.test.ts +++ b/src/services/webhookMedia.test.ts @@ -10,12 +10,13 @@ const png = () => sharp({ create: { width: 8, height: 8, channels: 3, background function fakeDeps(respond: (url: string, max: number, signal: AbortSignal) => Promise) { const fetched: string[] = []; - let stored = 0; + const stored: ("picture" | "avatar")[] = []; const deps: MediaDeps = { fetchBytes: async (url, signal, max) => { fetched.push(url); return respond(url, max, signal); }, - storeImage: async () => `file-${++stored}`, + storeImage: async () => { stored.push("picture"); return `file-${stored.length}`; }, + storeAvatar: async () => { stored.push("avatar"); return `avatar-${stored.length}`; }, }; - return { deps, fetched }; + return { deps, fetched, stored }; } describe("sniffImageFormat", () => { @@ -91,6 +92,36 @@ describe("resolveWebhookMedia", () => { assert.equal(out.cards[1].title, "b"); }); + it("stores the avatar as an avatar, and an icon from the same URL as sent, from one download", async () => { + const bytes = await png(); + const { deps, fetched, stored } = fakeDeps(async () => ({ ok: true, bytes })); + const body = webhookMessageSchema.parse({ + avatar_url: "https://a.example/logo.png", + cards: [{ title: "t", author: { name: "a", icon_url: "https://a.example/logo.png" } }], + }); + const out = await resolveWebhookMedia("hook", body, deps); + assert.deepEqual(out.warnings, []); + assert.deepEqual(fetched, ["https://a.example/logo.png"]); + assert.deepEqual([...stored].sort(), ["avatar", "picture"]); + assert.match(out.avatarFileId ?? "", /^avatar-/, "the avatar went through the card picture store"); + assert.match(out.cards[0].author?.icon_file_id ?? "", /^file-/, "the icon went through the avatar store"); + assert.equal(out.mediaFileIds.length, 2); + }); + + it("leaves out an avatar that fails to store, and keeps the icon it shares a URL with", async () => { + const bytes = await png(); + const { deps } = fakeDeps(async () => ({ ok: true, bytes })); + deps.storeAvatar = async () => { throw new Error("storage is down"); }; + const body = webhookMessageSchema.parse({ + avatar_url: "https://a.example/logo.png", + cards: [{ title: "t", author: { name: "a", icon_url: "https://a.example/logo.png" } }], + }); + const out = await resolveWebhookMedia("hook", body, deps); + assert.deepEqual(out.warnings.map((w) => [w.path, w.code]), [["avatar_url", "store_failed"]]); + assert.equal(out.avatarFileId, undefined); + assert.ok(out.cards[0].author?.icon_file_id); + }); + it("gives an icon the icon limit even when the same URL is also an image", async () => { const bytes = await png(); const limits: number[] = []; diff --git a/src/services/webhookMedia.ts b/src/services/webhookMedia.ts index 12a1acfa..f290deee 100644 --- a/src/services/webhookMedia.ts +++ b/src/services/webhookMedia.ts @@ -18,6 +18,7 @@ import { storageForUpload } from "../routes/uploadStorage"; import { putObject } from "../storage"; import { validateImage } from "../utils/imageValidation"; import { fetchFollowingSafely } from "../utils/safePreviewFetch"; +import { storeAvatarPicture } from "./avatarImage"; /** 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,6 +44,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; + storeAvatar(webhookId: string, bytes: Buffer, format: ImageFormat): Promise; } /** Which picture it is, where it sits in the payload, and how big it may be. */ @@ -50,6 +52,19 @@ interface MediaSlot { path: string; url: string; maxBytes: number; + /** Resized like any avatar, where card pictures are stored as sent. */ + avatar?: boolean; +} + +/** One download, shared by every slot with the same URL and limit. */ +interface MediaEntry { + url: string; + maxBytes: number; + asPicture?: boolean; + asAvatar?: boolean; + fileId?: string; + avatarFileId?: string; + code?: string; } /** From the bytes, never the Content-Type header. SVG and everything else is refused. */ @@ -130,7 +145,28 @@ async function storeWebhookImage(webhookId: string, bytes: Buffer, format: Image return fileId; } -export const realMediaDeps: MediaDeps = { fetchBytes: fetchBytesSafely, storeImage: storeWebhookImage }; +/** Through the same pipeline as a member's avatar. Its own key in `webhook_media`, since the + same bytes sent as a card picture are stored as sent. */ +async function storeWebhookAvatar(webhookId: string, bytes: Buffer, format: ImageFormat): Promise { + const mediaKey = `avatar:${createHash("sha256").update(bytes).digest("hex")}`; + const known = await getWebhookMediaFileId(webhookId, mediaKey); + if (known && (await getFile(known)) && (await isFileReferencedByMessage(known))) return known; + + const stored = await storeAvatarPicture({ + bucket: process.env.S3_BUCKET as string, + bytes, + mime: MIME[format], + originalName: `webhook.${format}`, + uploadedBy: `webhook:${webhookId}`, + maxBytes: ICON_MAX_BYTES, + what: "Webhook avatar", + }); + if (!stored.ok) throw new Error(stored.message); + await setWebhookMediaFileId(webhookId, mediaKey, stored.fileId); + return stored.fileId; +} + +export const realMediaDeps: MediaDeps = { fetchBytes: fetchBytesSafely, storeImage: storeWebhookImage, storeAvatar: storeWebhookAvatar }; const WARNING_TEXT: Record = { blocked: "That address isn't allowed, so the picture was left out.", @@ -145,7 +181,7 @@ const WARNING_TEXT: Record = { function slotsFor(body: WebhookMessageInput): MediaSlot[] { const slots: MediaSlot[] = []; - if (body.avatar_url) slots.push({ path: "avatar_url", url: body.avatar_url, maxBytes: ICON_MAX_BYTES }); + if (body.avatar_url) slots.push({ path: "avatar_url", url: body.avatar_url, maxBytes: ICON_MAX_BYTES, avatar: true }); (body.cards ?? []).forEach((card, i) => { if (card.author?.icon_url) slots.push({ path: `cards[${i}].author.icon_url`, url: card.author.icon_url, maxBytes: ICON_MAX_BYTES }); if (card.thumbnail_url) slots.push({ path: `cards[${i}].thumbnail_url`, url: card.thumbnail_url, maxBytes: IMAGE_MAX_BYTES }); @@ -183,8 +219,13 @@ export async function resolveWebhookMedia( const warnings: PayloadWarning[] = []; // Keyed with the limit too, so a URL used as both an icon and an image is held to each limit. const keyOf = (slot: MediaSlot) => `${slot.maxBytes}\u0000${slot.url}`; - const byUrl = new Map(); - for (const slot of slots) byUrl.set(keyOf(slot), { url: slot.url, maxBytes: slot.maxBytes }); + const byUrl = new Map(); + for (const slot of slots) { + const entry = byUrl.get(keyOf(slot)) ?? { url: slot.url, maxBytes: slot.maxBytes }; + if (slot.avatar) entry.asAvatar = true; + else entry.asPicture = true; + byUrl.set(keyOf(slot), entry); + } const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), deadlineMs); @@ -201,12 +242,18 @@ export async function resolveWebhookMedia( budget -= fetched.bytes.length; const checked = await validateImage(fetched.bytes, { animated: true }); if (!checked.valid) { entry.code = "invalid_image"; return; } - try { - entry.fileId = await deps.storeImage(webhookId, fetched.bytes, format, checked.width, checked.height); - } catch (err) { - consola.warn("webhook media store failed", err); - entry.code = "store_failed"; - } + const store = async (save: () => Promise): Promise => { + try { + return await save(); + } catch (err) { + consola.warn("webhook media store failed", err); + entry.code = "store_failed"; + return undefined; + } + }; + // Downloaded once, but the avatar and a card picture from the same URL are two files. + if (entry.asPicture) entry.fileId = await store(() => deps.storeImage(webhookId, fetched.bytes, format, checked.width, checked.height)); + if (entry.asAvatar) entry.avatarFileId = await store(() => deps.storeAvatar(webhookId, fetched.bytes, format)); }); } finally { clearTimeout(timer); @@ -216,12 +263,13 @@ export async function resolveWebhookMedia( if (!url) return undefined; const slot = slots.find((s) => s.path === path)!; const entry = byUrl.get(keyOf(slot))!; - if (!entry.fileId) { + const fileId = slot.avatar ? entry.avatarFileId : entry.fileId; + if (!fileId) { const code = entry.code ?? "fetch_failed"; warnings.push({ path, code, message: WARNING_TEXT[code] ?? WARNING_TEXT.fetch_failed }); return undefined; } - return entry.fileId; + return fileId; }; const avatarFileId = fileFor("avatar_url", body.avatar_url);