mirror of
https://github.com/djdevin/recflare.git
synced 2026-09-09 15:11:29 -07:00
[api] fix(security): bound API-owned uploads (#56)
Co-authored-by: Nexi (CWN) <communityshieldofficial@gmail.com>
This commit is contained in:
@@ -92,6 +92,12 @@ RECFLARE_DOMAIN=rec.example.com
|
|||||||
# value must be a positive integer; zero or an invalid value restores the default.
|
# value must be a positive integer; zero or an invalid value restores the default.
|
||||||
# RECFLARE_MAX_UPLOAD_BYTES=67108864
|
# RECFLARE_MAX_UPLOAD_BYTES=67108864
|
||||||
|
|
||||||
|
# Largest file the API worker accepts through either saved-image upload or either
|
||||||
|
# custom-avatar-item file field, in bytes. The default is 64 MiB PER FILE. Larger
|
||||||
|
# parsed files receive HTTP 413 before arrayBuffer() or an R2 write. This complements
|
||||||
|
# RECFLARE_MAX_UPLOAD_BYTES, which protects the separate storage worker.
|
||||||
|
# RECFLARE_MAX_API_UPLOAD_BYTES=67108864
|
||||||
|
|
||||||
# Rooms to switch out at matchmake time (`match`), as comma-separated <fromRoomId>=<to>
|
# Rooms to switch out at matchmake time (`match`), as comma-separated <fromRoomId>=<to>
|
||||||
# pairs, where <to> is a room id or room name. This is how a stock RRO room is replaced
|
# pairs, where <to> is a room id or room name. This is how a stock RRO room is replaced
|
||||||
# with your own: 2=MyHub sends everyone who matchmakes into the Rec Center (room 2) to the
|
# with your own: 2=MyHub sends everyone who matchmakes into the Rec Center (room 2) to the
|
||||||
|
|||||||
@@ -8,6 +8,8 @@ API surface. Database-backed queries and on-disk JSON files are stubbed for now
|
|||||||
|
|
||||||
- **Auth-gated routes** validate the Bearer JWT issued by the `auth` worker
|
- **Auth-gated routes** validate the Bearer JWT issued by the `auth` worker
|
||||||
(same dev secret, see `src/jwt.ts`) and 401 when it's missing/invalid.
|
(same dev secret, see `src/jwt.ts`) and 401 when it's missing/invalid.
|
||||||
|
- **API-owned uploads** enforce `RECFLARE_MAX_API_UPLOAD_BYTES` per file (64 MiB
|
||||||
|
by default) before copying a parsed file into an `ArrayBuffer` or writing it to R2.
|
||||||
- **Static data** is served verbatim:
|
- **Static data** is served verbatim:
|
||||||
- `src/default-avatar-items.ts` → `GET /api/avatar/v4/items`
|
- `src/default-avatar-items.ts` → `GET /api/avatar/v4/items`
|
||||||
- `src/default-settings.ts` → `GET /api/settings/v2`
|
- `src/default-settings.ts` → `GET /api/settings/v2`
|
||||||
|
|||||||
@@ -15,6 +15,8 @@ export type Env = SharedHonoEnv & {
|
|||||||
* for local dev and tests.
|
* for local dev and tests.
|
||||||
*/
|
*/
|
||||||
DOMAIN: string
|
DOMAIN: string
|
||||||
|
/** Maximum accepted size of each API-owned image upload, in bytes. */
|
||||||
|
RECFLARE_MAX_API_UPLOAD_BYTES?: string
|
||||||
// Shared rooms database (schema/migrations owned by the `rooms` worker). Used
|
// Shared rooms database (schema/migrations owned by the `rooms` worker). Used
|
||||||
// read-only here to resolve room roles for `/api/rooms/v1/verifyRole`.
|
// read-only here to resolve room roles for `/api/rooms/v1/verifyRole`.
|
||||||
DB: D1Database
|
DB: D1Database
|
||||||
|
|||||||
@@ -104,6 +104,7 @@ import {
|
|||||||
UpdatePriceRequest,
|
UpdatePriceRequest,
|
||||||
} from '../openapi'
|
} from '../openapi'
|
||||||
import { createReport } from '../reports-db'
|
import { createReport } from '../reports-db'
|
||||||
|
import { exceedsApiUploadLimit, maxApiUploadBytes } from '../upload-limit'
|
||||||
|
|
||||||
import type { Context } from 'hono'
|
import type { Context } from 'hono'
|
||||||
import type { App } from '../context'
|
import type { App } from '../context'
|
||||||
@@ -455,6 +456,7 @@ export const avatarRoutes = new Hono<App>({ strict: false })
|
|||||||
200: json(CustomAvatarItemResponse, 'The created item'),
|
200: json(CustomAvatarItemResponse, 'The created item'),
|
||||||
400: json(CustomAvatarItemResponse, 'Missing or malformed metadata / files'),
|
400: json(CustomAvatarItemResponse, 'Missing or malformed metadata / files'),
|
||||||
401: UNAUTHORIZED_RESPONSE,
|
401: UNAUTHORIZED_RESPONSE,
|
||||||
|
413: json(CustomAvatarItemResponse, 'Either file exceeds the configured per-file limit'),
|
||||||
},
|
},
|
||||||
}),
|
}),
|
||||||
async (c) => {
|
async (c) => {
|
||||||
@@ -480,6 +482,31 @@ export const avatarRoutes = new Hono<App>({ strict: false })
|
|||||||
return fail('BaseAvatarItemColor is required')
|
return fail('BaseAvatarItemColor is required')
|
||||||
if (!(body.thumbnailImage instanceof File)) return fail('thumbnailImage is required')
|
if (!(body.thumbnailImage instanceof File)) return fail('thumbnailImage is required')
|
||||||
if (!(body.design instanceof File)) return fail('design is required')
|
if (!(body.design instanceof File)) return fail('design is required')
|
||||||
|
const limit = maxApiUploadBytes(c.env)
|
||||||
|
// Each file gets the full per-file ceiling. Check both before either is copied into
|
||||||
|
// an ArrayBuffer or written, so a rejected request never leaves half an item in R2.
|
||||||
|
if (exceedsApiUploadLimit(body.thumbnailImage, limit)) {
|
||||||
|
return c.json(
|
||||||
|
{
|
||||||
|
Value: null,
|
||||||
|
Success: false,
|
||||||
|
Error: `thumbnailImage exceeds the ${limit}-byte upload limit`,
|
||||||
|
error_id: null,
|
||||||
|
},
|
||||||
|
413
|
||||||
|
)
|
||||||
|
}
|
||||||
|
if (exceedsApiUploadLimit(body.design, limit)) {
|
||||||
|
return c.json(
|
||||||
|
{
|
||||||
|
Value: null,
|
||||||
|
Success: false,
|
||||||
|
Error: `design exceeds the ${limit}-byte upload limit`,
|
||||||
|
error_id: null,
|
||||||
|
},
|
||||||
|
413
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// Both files go to the shared image bucket, foldered by upload date and keyed by
|
// 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
|
// the item's id (chosen here so the keys can carry it). The `img` worker serves
|
||||||
|
|||||||
@@ -46,6 +46,7 @@ import {
|
|||||||
UploadImageRequest,
|
UploadImageRequest,
|
||||||
UploadImageResponse,
|
UploadImageResponse,
|
||||||
} from '../openapi'
|
} from '../openapi'
|
||||||
|
import { exceedsApiUploadLimit, maxApiUploadBytes } from '../upload-limit'
|
||||||
|
|
||||||
import type { Context } from 'hono'
|
import type { Context } from 'hono'
|
||||||
import type { App } from '../context'
|
import type { App } from '../context'
|
||||||
@@ -210,6 +211,7 @@ export const imageRoutes = new Hono<App>({ strict: false })
|
|||||||
200: json(UploadImageResponse, 'The stored bucket key'),
|
200: json(UploadImageResponse, 'The stored bucket key'),
|
||||||
400: json(ErrorResponse, 'No file in the request'),
|
400: json(ErrorResponse, 'No file in the request'),
|
||||||
401: UNAUTHORIZED_RESPONSE,
|
401: UNAUTHORIZED_RESPONSE,
|
||||||
|
413: json(ErrorResponse, 'The image exceeds the configured per-file limit'),
|
||||||
},
|
},
|
||||||
}),
|
}),
|
||||||
async (c) => {
|
async (c) => {
|
||||||
@@ -221,6 +223,12 @@ export const imageRoutes = new Hono<App>({ strict: false })
|
|||||||
const candidate = body.image ?? body.file
|
const candidate = body.image ?? body.file
|
||||||
if (!(candidate instanceof File)) return c.json({ error: 'No file found in request' }, 400)
|
if (!(candidate instanceof File)) return c.json({ error: 'No file found in request' }, 400)
|
||||||
const file = candidate
|
const file = candidate
|
||||||
|
const limit = maxApiUploadBytes(c.env)
|
||||||
|
// parseBody has already materialized the multipart part. Reject it before arrayBuffer()
|
||||||
|
// creates another full-size allocation and before the object can consume R2 storage.
|
||||||
|
if (exceedsApiUploadLimit(file, limit)) {
|
||||||
|
return c.json({ error: `image exceeds the ${limit}-byte upload limit` }, 413)
|
||||||
|
}
|
||||||
|
|
||||||
// `imgMeta` is a JSON blob describing the upload (`SavedImageMetaDTO`),
|
// `imgMeta` is a JSON blob describing the upload (`SavedImageMetaDTO`),
|
||||||
// posted as a multipart field. It carries the metadata we record on the image
|
// posted as a multipart field. It carries the metadata we record on the image
|
||||||
|
|||||||
@@ -4014,6 +4014,53 @@ describe('custom avatar items', () => {
|
|||||||
expect(await res.json()).toMatchObject({ Success: false, Value: null })
|
expect(await res.json()).toMatchObject({ Success: false, Value: null })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('POST rejects either oversized file before writing anything to R2', async () => {
|
||||||
|
const previous = env.RECFLARE_MAX_API_UPLOAD_BYTES
|
||||||
|
env.RECFLARE_MAX_API_UPLOAD_BYTES = '3'
|
||||||
|
try {
|
||||||
|
const objectsBefore = (await env.IMAGES.list({ prefix: 'avatar-item/' })).objects.length
|
||||||
|
const upload = async (thumbnail: Uint8Array, design: Uint8Array) => {
|
||||||
|
const form = new FormData()
|
||||||
|
form.set(
|
||||||
|
'metadata',
|
||||||
|
JSON.stringify({ Name: 'bounded', BaseAvatarItemId: 1, BaseAvatarItemColor: '#fff' })
|
||||||
|
)
|
||||||
|
form.set('thumbnailImage', new File([thumbnail], 'thumb.png', { type: 'image/png' }))
|
||||||
|
form.set('design', new File([design], 'design.png', { type: 'image/png' }))
|
||||||
|
return exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, {
|
||||||
|
method: 'POST',
|
||||||
|
headers: await bearer('205'),
|
||||||
|
body: form,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
const oversizedThumbnail = await upload(new Uint8Array(4), new Uint8Array(3))
|
||||||
|
expect(oversizedThumbnail.status).toBe(413)
|
||||||
|
expect(await oversizedThumbnail.json()).toMatchObject({
|
||||||
|
Success: false,
|
||||||
|
Error: 'thumbnailImage exceeds the 3-byte upload limit',
|
||||||
|
})
|
||||||
|
|
||||||
|
const oversizedDesign = await upload(new Uint8Array(3), new Uint8Array(4))
|
||||||
|
expect(oversizedDesign.status).toBe(413)
|
||||||
|
expect(await oversizedDesign.json()).toMatchObject({
|
||||||
|
Success: false,
|
||||||
|
Error: 'design exceeds the 3-byte upload limit',
|
||||||
|
})
|
||||||
|
|
||||||
|
// Neither rejected request may create metadata or leave one of its two objects behind.
|
||||||
|
const row = await env.DB.prepare(
|
||||||
|
"SELECT COUNT(*) AS n FROM custom_avatar_item WHERE name = 'bounded'"
|
||||||
|
).first<{ n: number }>()
|
||||||
|
expect(row?.n).toBe(0)
|
||||||
|
expect((await env.IMAGES.list({ prefix: 'avatar-item/' })).objects).toHaveLength(
|
||||||
|
objectsBefore
|
||||||
|
)
|
||||||
|
} finally {
|
||||||
|
env.RECFLARE_MAX_API_UPLOAD_BYTES = previous
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
test('POST 401s without a token', async () => {
|
test('POST 401s without a token', async () => {
|
||||||
const res = await exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, {
|
const res = await exports.default.fetch(`${ORIGIN}/api/customAvatarItems/v1`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
@@ -5303,6 +5350,26 @@ describe('images', () => {
|
|||||||
expect(res.status).toBe(400)
|
expect(res.status).toBe(400)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('POST /api/images/v4/uploadsaved rejects an oversized image before storing it', async () => {
|
||||||
|
const previous = env.RECFLARE_MAX_API_UPLOAD_BYTES
|
||||||
|
env.RECFLARE_MAX_API_UPLOAD_BYTES = '3'
|
||||||
|
try {
|
||||||
|
const objectsBefore = (await env.IMAGES.list()).objects.length
|
||||||
|
const fd = new FormData()
|
||||||
|
fd.append('image', new File([new Uint8Array(4)], 'large.png', { type: 'image/png' }))
|
||||||
|
const res = await exports.default.fetch(`${ORIGIN}/api/images/v4/uploadsaved`, {
|
||||||
|
method: 'POST',
|
||||||
|
headers: await bearer('42'),
|
||||||
|
body: fd,
|
||||||
|
})
|
||||||
|
expect(res.status).toBe(413)
|
||||||
|
expect(await res.json()).toEqual({ error: 'image exceeds the 3-byte upload limit' })
|
||||||
|
expect((await env.IMAGES.list()).objects).toHaveLength(objectsBefore)
|
||||||
|
} finally {
|
||||||
|
env.RECFLARE_MAX_API_UPLOAD_BYTES = previous
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
test('GET /api/images/v4/room/:id returns a public room feed, filtered/sorted/paginated', async () => {
|
test('GET /api/images/v4/room/:id returns a public room feed, filtered/sorted/paginated', async () => {
|
||||||
// Seed images in room 54: two public (one with more cheers, of different
|
// Seed images in room 54: two public (one with more cheers, of different
|
||||||
// types), one private (hidden), and one in another room (excluded).
|
// types), one private (hidden), and one in another room (excluded).
|
||||||
|
|||||||
@@ -0,0 +1,21 @@
|
|||||||
|
import { intVar } from '@repo/hono-helpers'
|
||||||
|
|
||||||
|
import type { App } from './context'
|
||||||
|
|
||||||
|
/** Safe fallback when the deployment does not configure an API upload ceiling. */
|
||||||
|
export const DEFAULT_MAX_API_UPLOAD_BYTES = 64 * 1024 * 1024
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve the per-file ceiling shared by API-owned image uploads. A non-positive
|
||||||
|
* setting does not disable the protection: public upload routes must always remain
|
||||||
|
* bounded, so invalid values fall back to the safe default.
|
||||||
|
*/
|
||||||
|
export function maxApiUploadBytes(env: App['Bindings']): number {
|
||||||
|
const configured = intVar(env.RECFLARE_MAX_API_UPLOAD_BYTES, DEFAULT_MAX_API_UPLOAD_BYTES)
|
||||||
|
return configured > 0 ? configured : DEFAULT_MAX_API_UPLOAD_BYTES
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Whether a parsed multipart file is safe to copy into memory and persist to R2. */
|
||||||
|
export function exceedsApiUploadLimit(file: File, limit: number): boolean {
|
||||||
|
return file.size > limit
|
||||||
|
}
|
||||||
@@ -74,6 +74,10 @@
|
|||||||
"vars": {
|
"vars": {
|
||||||
"ENVIRONMENT": "development", // overridden during deployment
|
"ENVIRONMENT": "development", // overridden during deployment
|
||||||
"SENTRY_RELEASE": "unknown", // overridden during deployment
|
"SENTRY_RELEASE": "unknown", // overridden during deployment
|
||||||
"DOMAIN": "rec.example.com" // base domain; overridden during deployment
|
"DOMAIN": "rec.example.com", // base domain; overridden during deployment
|
||||||
|
// Per-file ceiling for saved images and custom-avatar-item files. Multipart parsing
|
||||||
|
// happens first, but this prevents oversized files from being copied into another
|
||||||
|
// ArrayBuffer and persisted to R2. Invalid/non-positive values fall back to 64 MiB.
|
||||||
|
"RECFLARE_MAX_API_UPLOAD_BYTES": "67108864"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user