From 7abef376b6faa031e3c3464198284960118fa201 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Wed, 21 Oct 2026 22:50:07 -0400 Subject: [PATCH] [storage] upload hardening --- apps/api/src/routes/avatar.ts | 34 ++++-- apps/api/src/routes/images.ts | 26 +++-- apps/api/src/test/integration/api.test.ts | 104 +++++++++++++++--- apps/cdn/src/cdn.app.ts | 10 ++ apps/cdn/src/test/integration/api.test.ts | 32 ++++++ apps/img/src/img.app.ts | 32 ++---- apps/storage/src/storage.app.ts | 25 ++++- apps/storage/src/test/integration/api.test.ts | 55 ++++++++- packages/hono-helpers/src/helpers/image.ts | 63 +++++++++++ packages/hono-helpers/src/index.ts | 2 + .../src/middleware/withNoExecute.ts | 37 +++++++ 11 files changed, 361 insertions(+), 59 deletions(-) create mode 100644 packages/hono-helpers/src/helpers/image.ts create mode 100644 packages/hono-helpers/src/middleware/withNoExecute.ts diff --git a/apps/api/src/routes/avatar.ts b/apps/api/src/routes/avatar.ts index d4d6b46..694b043 100644 --- a/apps/api/src/routes/avatar.ts +++ b/apps/api/src/routes/avatar.ts @@ -13,6 +13,7 @@ import { MAX_BULK_OUTFIT_ACCOUNTS, setOutfit, } from '@repo/domain' +import { SNIFFED_IMAGE_FORMATS, sniffImage } from '@repo/hono-helpers' import { createCustomAvatarItem, @@ -473,13 +474,17 @@ export const avatarRoutes = new Hono({ strict: false }) summary: 'Create a custom avatar item', description: 'Multipart: a `metadata` JSON text field plus two file parts, `thumbnailImage` ' + - '(PNG) and `design` (the design blob). Inserts a `custom_avatar_item` row owned ' + + '(PNG) and `design` (the design PNG). Inserts a `custom_avatar_item` row owned ' + 'by the caller and answers with it in the PascalCase `{ Value, Success, Error, ' + 'error_id }` envelope. The item is filed under `OutfitType` 105, the custom shirt, ' + 'which is what the store’s user-generated-content tab searches for.\n\n' + 'The two files go to the shared image bucket (`recflare-img`) under ' + '`avatar-item//-thumb.png` and `avatar-item//-design.png`; those ' + - 'keys are the `ThumbnailImageFilename` / `DesignFilename` on the row.\n\n' + + 'keys are the `ThumbnailImageFilename` / `DesignFilename` on the row. Each file is ' + + 'identified by its bytes — the client posts them under `.bin` names — and must be a ' + + 'PNG, JPEG, GIF, WebP or BMP; the key’s extension and the stored content-type come ' + + 'from that signature, and anything else is refused with a 400 before either file is ' + + 'written.\n\n' + '`Price` is capped at 1000 tokens (`MAX_PLAYER_ITEM_PRICE`); one above it, or below ' + '0, is refused with a 400 rather than clamped.', security: AUTHED, @@ -488,7 +493,7 @@ export const avatarRoutes = new Hono({ strict: false }) 200: json(CustomAvatarItemResponse, 'The created item'), 400: json( CustomAvatarItemResponse, - 'Missing or malformed metadata / files, or a `Price` outside 0–1000' + 'Missing or malformed metadata / files, a file that is not an image, or a `Price` outside 0–1000' ), 401: UNAUTHORIZED_RESPONSE, 413: json(CustomAvatarItemResponse, 'Either file exceeds the configured per-file limit'), @@ -550,19 +555,30 @@ export const avatarRoutes = new Hono({ strict: false }) ) } + // The parts' filenames and content-types are the uploader's claims (the client + // posts both as `.bin`); the `img` worker serves each back under whatever type is + // stored, so the extension and the stored type come from the bytes. Both are + // checked before either is written, so a refusal leaves nothing in R2. + const thumbnailBytes = new Uint8Array(await body.thumbnailImage.arrayBuffer()) + const thumbnail = sniffImage(thumbnailBytes) + if (!thumbnail) return fail(`thumbnailImage is not a ${SNIFFED_IMAGE_FORMATS} image`) + const designBytes = new Uint8Array(await body.design.arrayBuffer()) + const design = sniffImage(designBytes) + if (!design) return fail(`design is not a ${SNIFFED_IMAGE_FORMATS} image`) + // Both files go to the shared image bucket, foldered by upload date and keyed by // the item's id (chosen here so the keys can carry it). The `img` worker serves // them back by key. const customAvatarItemId = crypto.randomUUID() const prefix = `avatar-item/${new Date().toISOString().slice(0, 10)}/${customAvatarItemId}` - const thumbnailImageFilename = `${prefix}-thumb.png` - const designFilename = `${prefix}-design.png` + const thumbnailImageFilename = `${prefix}-thumb${thumbnail.extension}` + const designFilename = `${prefix}-design${design.extension}` await Promise.all([ - c.env.IMAGES.put(thumbnailImageFilename, await body.thumbnailImage.arrayBuffer(), { - httpMetadata: { contentType: body.thumbnailImage.type || 'image/png' }, + c.env.IMAGES.put(thumbnailImageFilename, thumbnailBytes, { + httpMetadata: { contentType: thumbnail.contentType }, }), - c.env.IMAGES.put(designFilename, await body.design.arrayBuffer(), { - httpMetadata: { contentType: body.design.type || 'image/png' }, + c.env.IMAGES.put(designFilename, designBytes, { + httpMetadata: { contentType: design.contentType }, }), ]) diff --git a/apps/api/src/routes/images.ts b/apps/api/src/routes/images.ts index ffe2121..38e08fa 100644 --- a/apps/api/src/routes/images.ts +++ b/apps/api/src/routes/images.ts @@ -22,6 +22,7 @@ import { SLIDESHOW_MAX_LIMIT, toImagesPlayer, } from '@repo/domain' +import { SNIFFED_IMAGE_FORMATS, sniffImage } from '@repo/hono-helpers' import { authedId, unauthorized } from '../http' import { @@ -217,12 +218,17 @@ export const imageRoutes = new Hono({ strict: false }) 'The `imgMeta` multipart field is a JSON `SavedImageMetaDTO` describing the upload; ' + 'malformed JSON is tolerated and the image is still stored, just untyped. A ' + '`savedImageType` of 4 (profile thumbnail) additionally becomes the account’s ' + - 'avatar, persisted on the account row.', + 'avatar, persisted on the account row.\n\n' + + 'The file is identified by its bytes, not by the part’s filename or content-type: ' + + 'it must be a PNG, JPEG, GIF, WebP or BMP, and the stored key’s extension and ' + + 'content-type come from that signature. Anything else is refused with a 400 — the ' + + '`img` worker serves the object back under its stored type, so an HTML body under ' + + 'an image name would otherwise be a page on the image origin.', security: AUTHED, requestBody: form(UploadImageRequest, 'The image file plus its metadata'), responses: { 200: json(UploadImageResponse, 'The stored bucket key'), - 400: json(ErrorResponse, 'No file in the request'), + 400: json(ErrorResponse, 'No file in the request, or the file is not an image'), 401: UNAUTHORIZED_RESPONSE, 413: json(ErrorResponse, 'The image exceeds the configured per-file limit'), }, @@ -262,10 +268,12 @@ export const imageRoutes = new Hono({ strict: false }) const roomId = num(meta.roomId) const playerEventId = num(meta.playerEventId) - const valid = ['.png', '.jpg', '.jpeg', '.gif', '.webp', '.bmp'] - const dot = file.name.lastIndexOf('.') - const ext = dot >= 0 ? file.name.slice(dot).toLowerCase() : '' - const extension = valid.includes(ext) ? ext : '.jpg' + // The part's filename and content-type are the uploader's claims; the `img` worker + // serves the object back under whatever type is stored, so both the extension and + // the stored type come from the bytes. A body that is not an image is refused. + const bytes = new Uint8Array(await file.arrayBuffer()) + const image = sniffImage(bytes) + if (!image) return c.json({ error: `image is not a ${SNIFFED_IMAGE_FORMATS} image` }, 400) // Store the upload in the shared image bucket under a random key, foldered by // the image type and then the upload date (e.g. `sharecamera/2026-06-15/`) so @@ -273,10 +281,8 @@ export const imageRoutes = new Hono({ strict: false }) // key (slashes and all), which is the returned ImageName. const typePrefix = (typeFolder[savedImageType] ?? typeFolder[SavedImageType.None]) + '/' const datePrefix = new Date().toISOString().slice(0, 10) + '/' - const name = typePrefix + datePrefix + crypto.randomUUID() + extension - await c.env.IMAGES.put(name, await file.arrayBuffer(), { - httpMetadata: { contentType: file.type || 'image/jpeg' }, - }) + const name = typePrefix + datePrefix + crypto.randomUUID() + image.extension + await c.env.IMAGES.put(name, bytes, { httpMetadata: { contentType: image.contentType } }) // A profile thumbnail becomes the account's avatar — persist it on the // account row (a JSON blob in the shared accounts table) so it sticks. diff --git a/apps/api/src/test/integration/api.test.ts b/apps/api/src/test/integration/api.test.ts index 5812c81..31bf505 100644 --- a/apps/api/src/test/integration/api.test.ts +++ b/apps/api/src/test/integration/api.test.ts @@ -95,6 +95,9 @@ declare module 'cloudflare:test' { const ORIGIN = 'https://example.com' +/** The PNG signature — the least an upload route accepts as an image. */ +const PNG_BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) + // `/api/rooms/v1/verifyRole` reads room roles from the shared recflare D1. Set // up the schema (matching the rooms worker's migration) + a couple of rooms. const TEST_ROOMS = [ @@ -1788,8 +1791,8 @@ describe('public endpoints', () => { ...metadata, }) ) - form.set('thumbnailImage', new File([new Uint8Array([1])], 'f.bin', { type: 'image/png' })) - form.set('design', new File([new Uint8Array([2])], 'f.bin', { type: 'image/png' })) + form.set('thumbnailImage', new File([PNG_BYTES], 'f.bin', { type: 'image/png' })) + form.set('design', new File([PNG_BYTES], 'f.bin', { type: 'image/png' })) const res = await exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, { method: 'POST', headers: await bearer(creator), @@ -4966,11 +4969,8 @@ describe('custom avatar items', () => { Accessibility: 0, }) ) - form.set( - 'thumbnailImage', - new File([new Uint8Array([1, 2, 3])], 'file.bin', { type: 'image/png' }) - ) - form.set('design', new File([new Uint8Array([4, 5, 6])], 'file.bin', { type: 'image/png' })) + form.set('thumbnailImage', new File([PNG_BYTES], 'file.bin', { type: 'image/png' })) + form.set('design', new File([PNG_BYTES], 'file.bin', { type: 'image/png' })) const res = await exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, { method: 'POST', headers: await bearer('205'), @@ -5113,8 +5113,8 @@ describe('custom avatar items', () => { 'metadata', JSON.stringify({ Name: 'Pricey', BaseAvatarItemId: 1, BaseAvatarItemColor: '#fff', Price }) ) - form.set('thumbnailImage', new File([new Uint8Array([1])], 't.png', { type: 'image/png' })) - form.set('design', new File([new Uint8Array([2])], 'd.png', { type: 'image/png' })) + form.set('thumbnailImage', new File([PNG_BYTES], 't.png', { type: 'image/png' })) + form.set('design', new File([PNG_BYTES], 'd.png', { type: 'image/png' })) return exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, { method: 'POST', headers: await bearer('205'), @@ -5253,8 +5253,8 @@ describe('custom avatar items', () => { 'metadata', JSON.stringify({ Name: 'Gone', BaseAvatarItemId: 1, BaseAvatarItemColor: '#fff' }) ) - form.set('thumbnailImage', new File([new Uint8Array([1])], 'file.bin', { type: 'image/png' })) - form.set('design', new File([new Uint8Array([2])], 'file.bin', { type: 'image/png' })) + form.set('thumbnailImage', new File([PNG_BYTES], 'file.bin', { type: 'image/png' })) + form.set('design', new File([PNG_BYTES], 'file.bin', { type: 'image/png' })) const created = (await ( await exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, { method: 'POST', @@ -5359,6 +5359,48 @@ describe('custom avatar items', () => { } }) + // Both files are identified by their bytes — the client posts them as `.bin` under + // `image/png` — and the `img` worker serves each under its stored type, so a non-image + // is refused before either file or the row is written. + test('POST refuses a file whose bytes are not an image, writing nothing', async () => { + const objectsBefore = (await env.IMAGES.list({ prefix: 'avatar-item/' })).objects.length + const upload = async (thumbnail: string | Uint8Array, design: string | Uint8Array) => { + const form = new FormData() + form.set( + 'metadata', + JSON.stringify({ Name: 'not-image', BaseAvatarItemId: 1, BaseAvatarItemColor: '#fff' }) + ) + form.set('thumbnailImage', new File([thumbnail], 'f.bin', { type: 'image/png' })) + form.set('design', new File([design], 'f.bin', { type: 'image/png' })) + return exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, { + method: 'POST', + headers: await bearer('205'), + body: form, + }) + } + const html = '' + + const badThumbnail = await upload(html, PNG_BYTES) + expect(badThumbnail.status).toBe(400) + expect(await badThumbnail.json()).toMatchObject({ + Success: false, + Error: 'thumbnailImage is not a PNG, JPEG, GIF, WebP or BMP image', + }) + + const badDesign = await upload(PNG_BYTES, html) + expect(badDesign.status).toBe(400) + expect(await badDesign.json()).toMatchObject({ + Success: false, + Error: 'design is not a PNG, JPEG, GIF, WebP or BMP image', + }) + + const row = await env.DB.prepare( + "SELECT COUNT(*) AS n FROM custom_avatar_item WHERE json_extract(data, '$.Name') = 'not-image'" + ).first<{ n: number }>() + expect(row?.n).toBe(0) + expect((await env.IMAGES.list({ prefix: 'avatar-item/' })).objects).toHaveLength(objectsBefore) + }) + test('POST 401s without a token', async () => { const res = await exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, { method: 'POST', @@ -6884,7 +6926,7 @@ describe('rooms', () => { describe('images', () => { test('POST /api/images/v4/uploadsaved stores the file in R2 and returns its name', async () => { - const bytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 1, 2, 3, 4]) + const bytes = new Uint8Array([...PNG_BYTES, 1, 2, 3, 4]) const fd = new FormData() fd.append('imgMeta', JSON.stringify({ savedImageType: 1 })) // ShareCamera fd.append('image', new File([bytes], 'avatar.png', { type: 'image/png' })) @@ -6917,6 +6959,42 @@ describe('images', () => { expect(meta.CheerCount).toBe(0) }) + // The part's filename and content-type are the uploader's claims, and the `img` + // worker serves the object under whatever type is stored — so an HTML body under an + // image name would be a page on the image origin. The bytes decide, and only images + // get in. + test('POST /api/images/v4/uploadsaved refuses a file whose bytes are not an image', async () => { + const objectsBefore = (await env.IMAGES.list()).objects.length + const fd = new FormData() + fd.append('imgMeta', JSON.stringify({ savedImageType: 1 })) + fd.append('image', new File([''], 'photo.jpg', { type: 'image/jpeg' })) + const res = await exports.default.fetch(`${ORIGIN}/api/images/v4/uploadsaved`, { + method: 'POST', + headers: await bearer(), + body: fd, + }) + expect(res.status).toBe(400) + expect(await res.json()).toEqual({ error: 'image is not a PNG, JPEG, GIF, WebP or BMP image' }) + expect((await env.IMAGES.list()).objects).toHaveLength(objectsBefore) + }) + + test('POST /api/images/v4/uploadsaved stores the type and extension the bytes say, not the claim', async () => { + const fd = new FormData() + fd.append('imgMeta', JSON.stringify({ savedImageType: 1 })) + // A real PNG posted as a JPEG under an HTML name. + fd.append('image', new File([PNG_BYTES], 'photo.html', { type: 'text/html' })) + const res = await exports.default.fetch(`${ORIGIN}/api/images/v4/uploadsaved`, { + method: 'POST', + headers: await bearer(), + body: fd, + }) + expect(res.status).toBe(200) + const { ImageName } = (await res.json()) as { ImageName: string } + expect(ImageName).toMatch(/\.png$/) + const stored = await env.IMAGES.get(ImageName) + expect(stored?.httpMetadata?.contentType).toBe('image/png') + }) + test('GET /api/images/v1/slideshow is public and joins username + room name', async () => { // Seed a public image (Accessibility 1) taken in RecCenter (room 2) by account 42. await env.DB.prepare('INSERT INTO image (data) VALUES (?1)') @@ -7378,7 +7456,7 @@ describe('images', () => { accessibility: 2, }) ) - fd.append('image', new File([new Uint8Array([1, 2, 3])], 'pic.png', { type: 'image/png' })) + fd.append('image', new File([PNG_BYTES], 'pic.png', { type: 'image/png' })) const res = await exports.default.fetch(`${ORIGIN}/api/images/v4/uploadsaved`, { method: 'POST', headers: await bearer('42'), diff --git a/apps/cdn/src/cdn.app.ts b/apps/cdn/src/cdn.app.ts index 33d3c10..498c292 100644 --- a/apps/cdn/src/cdn.app.ts +++ b/apps/cdn/src/cdn.app.ts @@ -3,8 +3,10 @@ import { describeRoute, openAPIRouteHandler, resolver } from 'hono-openapi' import { useWorkersLogger } from 'workers-tagged-logger' import { + applyNoExecuteHeaders, withCleanSpec, withDefaultCors, + withNoExecute, withNotFound, withOnError, writeContentRange, @@ -167,6 +169,13 @@ const app = new Hono() // unchanged by who may read a response they already had to name exactly. .use('*', withDefaultCors()) + // The blob routes force `application/octet-stream` over whatever type an upload was + // stored with, which is what keeps a stored HTML body from rendering; `nosniff` makes + // that type binding on the browser, and the sandboxing CSP covers the config files, + // which go out under their own types. The hand-rolled HEAD below is outside this chain + // and stamps the same headers itself. + .use('*', withNoExecute()) + .onError(withOnError()) .notFound(withNotFound()) @@ -458,6 +467,7 @@ async function studioUnityAssetHead(env: StudioAssetEnv, name: string): Promise< headers.set('accept-ranges', 'bytes') headers.set('cache-control', CACHE_CONTROL) headers.set('access-control-allow-origin', '*') + applyNoExecuteHeaders(headers) const response = new Response(null, { status: 200, headers, encodeBody: 'manual' }) response.headers.set('content-length', String(head.size)) return response diff --git a/apps/cdn/src/test/integration/api.test.ts b/apps/cdn/src/test/integration/api.test.ts index 00033eb..e3079cf 100644 --- a/apps/cdn/src/test/integration/api.test.ts +++ b/apps/cdn/src/test/integration/api.test.ts @@ -19,6 +19,38 @@ describe('cdn endpoints', () => { expect(await res.json()).toEqual({ service: 'cdn', status: 'ok' }) }) + // A stored blob carries whatever type it was uploaded with; the routes force + // octet-stream, and these headers make that binding and sandbox anything that is served + // under its own type (the configs). The hand-rolled HEAD sits outside Hono and must + // carry them too. + test('every response carries headers that stop a stored document executing', async () => { + await env.CDN_ASSETS.put('data/a-stored-page', '', { + httpMetadata: { contentType: 'text/html' }, + }) + for (const path of [ + '/', + '/config/LoadingScreenTipData', + '/config/RRPlusConfig_v3', + '/data/a-stored-page', + ]) { + const res = await exports.default.fetch(`${ORIGIN}${path}`) + expect(res.status, path).toBe(200) + expect(res.headers.get('x-content-type-options'), path).toBe('nosniff') + expect(res.headers.get('content-security-policy'), path).toBe("default-src 'none'; sandbox") + } + const blob = await exports.default.fetch(`${ORIGIN}/data/a-stored-page`) + expect(blob.headers.get('content-type')).toBe('application/octet-stream') + + await env.CDN_ASSETS.put('unityasset/plain.assetbundle', new Uint8Array([1, 2, 3])) + const head = await exports.default.fetch(`${ORIGIN}/unityasset/plain.assetbundle`, { + method: 'HEAD', + }) + expect(head.status).toBe(200) + expect(head.headers.get('content-length')).toBe('3') + expect(head.headers.get('x-content-type-options')).toBe('nosniff') + expect(head.headers.get('content-security-policy')).toBe("default-src 'none'; sandbox") + }) + test('GET /config/LoadingScreenTipData returns the tip array', async () => { const res = await exports.default.fetch(`${ORIGIN}/config/LoadingScreenTipData`) expect(res.status).toBe(200) diff --git a/apps/img/src/img.app.ts b/apps/img/src/img.app.ts index bc85312..8f6c281 100644 --- a/apps/img/src/img.app.ts +++ b/apps/img/src/img.app.ts @@ -3,7 +3,13 @@ import { Hono } from 'hono' import { describeRoute, openAPIRouteHandler } from 'hono-openapi' import { useWorkersLogger } from 'workers-tagged-logger' -import { withCleanSpec, withNotFound, withOnError, writeContentRange } from '@repo/hono-helpers' +import { + withCleanSpec, + withNoExecute, + withNotFound, + withOnError, + writeContentRange, +} from '@repo/hono-helpers' import { imageBytes, json, ServiceStatus } from './openapi' @@ -303,20 +309,6 @@ async function serveStaticAsset( return new Response(asset.body, { headers }) } -/** - * Headers that keep a stored object from running as a document. An upload reaches R2 - * with whatever content-type the uploader claimed (`api` writes `file.type` through), - * so a `text/html` body with a `.jpg` name is served back as HTML on this origin. - * `nosniff` stops the browser upgrading an image type to HTML on its own, and the - * `sandbox` CSP denies scripts, plugins and same-origin access even when the stored - * type really is HTML or SVG — the page renders inert. Applied to every response, the - * status JSON and 304s included; there is no response here that should ever execute. - */ -const NO_EXECUTE_HEADERS = { - 'x-content-type-options': 'nosniff', - 'content-security-policy': "default-src 'none'; sandbox", -} as const - const app = new Hono() .use( '*', @@ -327,12 +319,10 @@ const app = new Hono() release: c.env.SENTRY_RELEASE, })(c, next) ) - .use('*', async (c, next) => { - await next() - for (const [name, value] of Object.entries(NO_EXECUTE_HEADERS)) { - c.res.headers.set(name, value) - } - }) + // An upload reaches R2 with whatever content-type the uploader claimed — `api` and + // `storage` now sniff image uploads, but the bucket holds older objects — so every + // response says it must not run as a document. See `withNoExecute`. + .use('*', withNoExecute()) .onError(withOnError()) .notFound(withNotFound()) diff --git a/apps/storage/src/storage.app.ts b/apps/storage/src/storage.app.ts index 23ed89c..b5695ac 100644 --- a/apps/storage/src/storage.app.ts +++ b/apps/storage/src/storage.app.ts @@ -4,6 +4,8 @@ import { useWorkersLogger } from 'workers-tagged-logger' import { intVar, + SNIFFED_IMAGE_FORMATS, + sniffImage, withCleanSpec, withDefaultCors, withNotFound, @@ -309,7 +311,10 @@ const app = new Hono() '`/` part the client references it by (the same name the', '`cdn` worker serves back). The subfolder comes from `FileType`; RoomSave (1) lands', 'under `room/` so the cdn worker’s `GET /room/:dataBlob` finds it, and an Invention', - '(5) keeps a `.inv` extension on both the key and the returned name. Auth-gated —', + '(5) keeps a `.inv` extension on both the key and the returned name. An Image (3) is', + 'identified by its bytes — it must be a PNG, JPEG, GIF, WebP or BMP, and is stored under', + 'that content-type rather than the part’s claimed one — because the `img` worker serves', + 'it back under the stored type; anything else is refused with a 400. Auth-gated —', 'any valid account token is allowed, no role check; staff (developer/moderator) skip the size limit. A post with no binary part but', 'an explicit `imageName` / `filename` / `name` just echoes that name back. Mirrors', 'the reference server’s `Upload`.', @@ -318,7 +323,10 @@ const app = new Hono() requestBody: UPLOAD_REQUEST_BODY, responses: { 200: json(UploadResponse, 'The stored (or echoed) file name'), - 400: json(ErrorResponse, 'Unknown/missing FileType, or neither a file nor a name'), + 400: json( + ErrorResponse, + 'Unknown/missing FileType, an Image (3) that is not an image, or neither a file nor a name' + ), 401: UNAUTHORIZED_RESPONSE, 413: json( ErrorResponse, @@ -360,8 +368,19 @@ const app = new Hono() const datePrefix = new Date().toISOString().slice(0, 10) const filename = `${datePrefix}/${crypto.randomUUID()}${extensionForFileType(fileType)}` const bytes = await file.arrayBuffer() + // An image is served back by the `img` worker under the stored content-type, so + // the type comes from the bytes, not the part's claim — and a body that is not + // an image is refused rather than stored as a document on the image origin. + let contentType = file.type || 'application/octet-stream' + if (subfolder === 'image') { + const image = sniffImage(new Uint8Array(bytes)) + if (!image) { + return c.json({ error: `file is not a ${SNIFFED_IMAGE_FORMATS} image` }, 400) + } + contentType = image.contentType + } await c.env.CDN_ASSETS.put(`${subfolder}/${filename}`, bytes, { - httpMetadata: { contentType: file.type || 'application/octet-stream' }, + httpMetadata: { contentType }, // Record the SHA-256 on the object. R2 stores an md5 on its own, but the // hashes the client is served (an invention's `BlobHash`) are SHA-256, and // only a checksum given at put time is readable later — this lets the `api` diff --git a/apps/storage/src/test/integration/api.test.ts b/apps/storage/src/test/integration/api.test.ts index d36dab5..4e91877 100644 --- a/apps/storage/src/test/integration/api.test.ts +++ b/apps/storage/src/test/integration/api.test.ts @@ -9,6 +9,9 @@ declare module 'cloudflare:test' { const ORIGIN = 'https://example.com' +/** The PNG signature — what an Image (FileType 3) upload must at least begin with. */ +const PNG_BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]) + beforeAll(async () => { // Seed the shared JWT signing key into the local Secrets Store so .get() resolves. await adminSecretsStore(env.JWT_SECRET).create('test-signing-key') @@ -96,7 +99,8 @@ it('POST /upload folders each FileType under its own subfolder', async () => { const res = await SELF.fetch(`${ORIGIN}/upload`, { method: 'POST', headers, - body: uploadForm(fileType, new Uint8Array([1, 2, 3])), + // An image has to look like one; the other types take any bytes. + body: uploadForm(fileType, subfolder === 'image' ? PNG_BYTES : new Uint8Array([1, 2, 3])), }) expect(res.status).toBe(200) const { filename } = (await res.json()) as { filename: string } @@ -161,17 +165,62 @@ it('POST /upload lets staff upload past the size limit', async () => { const res = await SELF.fetch(`${ORIGIN}/upload`, { method: 'POST', headers: await bearer('42', ['gameClient', role]), - body: uploadForm('3', new Uint8Array([1, 2, 3, 4])), + body: uploadForm('3', PNG_BYTES), }) expect(res.status).toBe(200) const { filename } = (await res.json()) as { filename: string } - expect((await env.CDN_ASSETS.get(`image/${filename}`))?.size).toBe(4) + expect((await env.CDN_ASSETS.get(`image/${filename}`))?.size).toBe(PNG_BYTES.length) } } finally { env.MAX_UPLOAD_BYTES = original } }) +// An Image (3) is served back by the `img` worker under the stored content-type, so the +// type comes from the bytes rather than the part's claim, and a non-image is refused — +// otherwise an HTML body would be a page on the image origin. +it('POST /upload refuses an Image (FileType 3) whose bytes are not an image', async () => { + const form = new FormData() + form.set('FileType', '3') + form.set('File', new File([''], 'photo.jpg', { type: 'image/jpeg' })) + const res = await SELF.fetch(`${ORIGIN}/upload`, { + method: 'POST', + headers: await bearer(), + body: form, + }) + expect(res.status).toBe(400) + expect(await res.json()).toEqual({ error: 'file is not a PNG, JPEG, GIF, WebP or BMP image' }) +}) + +it('POST /upload stores an Image (FileType 3) under the type its bytes say, not the claim', async () => { + const form = new FormData() + form.set('FileType', '3') + form.set('File', new File([PNG_BYTES], 'photo.html', { type: 'text/html' })) + const res = await SELF.fetch(`${ORIGIN}/upload`, { + method: 'POST', + headers: await bearer(), + body: form, + }) + expect(res.status).toBe(200) + const { filename } = (await res.json()) as { filename: string } + const stored = await env.CDN_ASSETS.get(`image/${filename}`) + expect(stored?.httpMetadata?.contentType).toBe('image/png') + + // Other types keep the claimed content-type: they are not served as images. + const blob = new FormData() + blob.set('FileType', '2') + blob.set( + 'File', + new File([new Uint8Array([1, 2, 3])], 'file.bin', { type: 'application/octet-stream' }) + ) + const other = await SELF.fetch(`${ORIGIN}/upload`, { + method: 'POST', + headers: await bearer(), + body: blob, + }) + expect(other.status).toBe(200) +}) + it('POST /upload echoes an explicit name when no binary is posted', async () => { const form = new FormData() form.set('FileType', '3') diff --git a/packages/hono-helpers/src/helpers/image.ts b/packages/hono-helpers/src/helpers/image.ts new file mode 100644 index 0000000..bfcb9d1 --- /dev/null +++ b/packages/hono-helpers/src/helpers/image.ts @@ -0,0 +1,63 @@ +/** + * Recognising an uploaded image by its bytes. + * + * Every image upload (`api`'s saved images and custom avatar item files, `storage`'s + * FileType 3) is served back by the `img` worker, which plays the stored content-type + * straight to the browser. Nothing about the upload is trustworthy: the multipart part's + * `type` and filename are whatever the client chose to send, so a `text/html` body under a + * `.jpg` name is an HTML document on the image origin unless something looks at the bytes. + * This does — the stored type and extension come from the signature, never the claim. + */ + +/** What an image's leading bytes say it is. */ +export interface SniffedImage { + /** The media type to store and serve it under. */ + contentType: string + /** The extension to key it with, dot included. */ + extension: string +} + +interface Signature extends SniffedImage { + /** Byte sequences that must appear at the given offsets. */ + parts: Array<{ offset: number; bytes: number[] }> +} + +const ascii = (s: string): number[] => Array.from(s, (ch) => ch.charCodeAt(0)) + +/** The formats an upload may be — the set the `img` worker decodes and resizes. */ +const SIGNATURES: Signature[] = [ + { + contentType: 'image/png', + extension: '.png', + parts: [{ offset: 0, bytes: [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a] }], + }, + { + contentType: 'image/jpeg', + extension: '.jpg', + parts: [{ offset: 0, bytes: [0xff, 0xd8, 0xff] }], + }, + { contentType: 'image/gif', extension: '.gif', parts: [{ offset: 0, bytes: ascii('GIF8') }] }, + { + contentType: 'image/webp', + extension: '.webp', + parts: [ + { offset: 0, bytes: ascii('RIFF') }, + { offset: 8, bytes: ascii('WEBP') }, + ], + }, + { contentType: 'image/bmp', extension: '.bmp', parts: [{ offset: 0, bytes: ascii('BM') }] }, +] + +/** The formats `sniffImage` accepts, for an error message. */ +export const SNIFFED_IMAGE_FORMATS = 'PNG, JPEG, GIF, WebP or BMP' + +/** + * Identify an image by its signature, or `undefined` when the bytes are not one of the + * supported formats. Operates on the leading bytes only; it does not validate the rest. + */ +export function sniffImage(bytes: Uint8Array): SniffedImage | undefined { + const match = SIGNATURES.find(({ parts }) => + parts.every(({ offset, bytes: expected }) => expected.every((b, i) => bytes[offset + i] === b)) + ) + return match ? { contentType: match.contentType, extension: match.extension } : undefined +} diff --git a/packages/hono-helpers/src/index.ts b/packages/hono-helpers/src/index.ts index 592505e..7f5df91 100644 --- a/packages/hono-helpers/src/index.ts +++ b/packages/hono-helpers/src/index.ts @@ -3,10 +3,12 @@ export * from './helpers/env' export { logger } from './helpers/logger' export { getRequestLogData, type LogDataRequest } from './helpers/request' export * from './helpers/errors' +export * from './helpers/image' export * from './helpers/openapi' export * from './helpers/r2' export * from './helpers/url' export * from './middleware/withCache' export * from './middleware/withDefaultCors' +export * from './middleware/withNoExecute' export * from './middleware/withNotFound' export * from './middleware/withOnError' diff --git a/packages/hono-helpers/src/middleware/withNoExecute.ts b/packages/hono-helpers/src/middleware/withNoExecute.ts new file mode 100644 index 0000000..1c3f901 --- /dev/null +++ b/packages/hono-helpers/src/middleware/withNoExecute.ts @@ -0,0 +1,37 @@ +import type { MiddlewareHandler } from 'hono' + +/** + * Headers that keep a served object from running as a document. + * + * `img` and `cdn` stream user uploads back from R2, where an object carries whatever + * content-type the uploader claimed — so a `text/html` body under a `.jpg` name is an HTML + * page on that origin unless the response says otherwise. `nosniff` stops the browser + * upgrading an image or octet-stream type to HTML on its own, and the `sandbox` CSP denies + * scripts, plugins and same-origin access even when the stored type really is HTML or SVG: + * the page renders inert. Nothing either worker serves should ever execute, so these go on + * every response, the status JSON and 304s included. + */ +export const NO_EXECUTE_HEADERS = { + 'x-content-type-options': 'nosniff', + 'content-security-policy': "default-src 'none'; sandbox", +} as const + +/** Stamp the no-execute headers onto a response built outside Hono (a hand-rolled HEAD). */ +export function applyNoExecuteHeaders(headers: Headers): void { + for (const [name, value] of Object.entries(NO_EXECUTE_HEADERS)) headers.set(name, value) +} + +/** + * Apply `NO_EXECUTE_HEADERS` to every response of the app. Set after `next()`, like + * `withDefaultCors`, so the route's Response is mutated rather than rebuilt and a HEAD's + * `encodeBody: 'manual'` Content-Length survives. + */ +export function withNoExecute(): MiddlewareHandler { + return async (c, next) => { + try { + await next() + } finally { + applyNoExecuteHeaders(c.res.headers) + } + } +}