feat(teams): harden the upload path for an uploader who is not an admin
The existing admin upload path is already good for an admin: an 8 MB cap, a mimetype allowlist, a random filename, an extension derived from the mimetype map and never from originalname, and nosniff forced on serve. All of it is kept. What it does not have is anything that assumes a hostile uploader, because until now it has not had one. Magic-byte sniffing, because `file.mimetype` is the client's own Content-Type header — a player can send image/png with arbitrary bytes and land arbitrary content under a .png. Unrecognised bytes are a rejection and never a fallback to what the header claimed. The file is on disk before it can be sniffed, so the rejection path removes it: a rejected upload left on disk is the same disk-exhaustion vector reached another way. A rolling per-account byte quota and a per-IP rate limit, because community uploads with no ceiling is disk exhaustion on the operator's own host. An attribution row per accepted file. Not bookkeeping: the acknowledgement is meaningless if "who uploaded this" cannot be answered afterwards, which is exactly what the operator has just accepted responsibility for. A nightly sweep for soft-deleted files past retention and for never-referenced orphans, in the same in-process shape as the activity prune. It runs whether or not `uploads` is the current mode, and that is the point — an operator who turns uploads off after a problem still has the files, and a sweep that switched itself off with the setting would strand exactly the bytes they were trying to be rid of. It works from the forum's own rows outward and never from the directory listing inward, because UPLOAD_DIR is shared with the admin upload path. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
177
server/src/model/teams/teamForumUploads.model.js
Normal file
177
server/src/model/teams/teamForumUploads.model.js
Normal file
@@ -0,0 +1,177 @@
|
||||
// ── `uploads` mode, and what had to harden first (TEAMS.md §5.5.4) ─────────
|
||||
//
|
||||
// The existing admin upload path (router/v1/admin/imageUpload.js) is already good
|
||||
// for an admin: an 8 MB cap, a mimetype allowlist, a random filename, an extension
|
||||
// derived from the MIMETYPE MAP and never from `originalname`, and
|
||||
// `X-Content-Type-Options: nosniff` forced on serve. All of that is kept and this
|
||||
// file adds the four things that path never needed, because until now it has never
|
||||
// had a hostile uploader.
|
||||
//
|
||||
// 1. MAGIC-BYTE SNIFFING. `file.mimetype` is the client's own Content-Type
|
||||
// header. A player can send `image/png` with arbitrary bytes and land
|
||||
// arbitrary content under a `.png`. Trusted from an admin, not from a player.
|
||||
// 2. QUOTAS. A per-post attachment cap and a per-account daily byte quota.
|
||||
// Community uploads with no ceiling is disk exhaustion on the operator's own
|
||||
// host. (The per-request RATE limit is core's rateLimit middleware, applied
|
||||
// at the route.)
|
||||
// 3. ATTRIBUTION. Every accepted file gets a `team_forum_uploads` row. Not
|
||||
// bookkeeping: the acknowledgement in §5.5.5 is meaningless if "who uploaded
|
||||
// this" cannot be answered afterwards.
|
||||
// 4. LIFECYCLE. Deleting a post soft-deletes its uploads; the sweep removes the
|
||||
// bytes after a retention window, and files with no row at all. The admin
|
||||
// upload path never deletes anything, which is fine at admin volume and is
|
||||
// not fine here.
|
||||
|
||||
const fs = require('fs/promises')
|
||||
const path = require('path')
|
||||
|
||||
const forumDb = require('./teamForum.db')
|
||||
const { UPLOAD_DIR } = require('../../router/v1/admin/imageUpload')
|
||||
|
||||
// Leading bytes → the type they actually are. Deliberately not a library: five
|
||||
// signatures, checked exactly, is less surface than a dependency that accepts
|
||||
// hundreds of formats when the allowlist only wants these.
|
||||
//
|
||||
// WebP and AVIF are container formats, so both need a second check past the first
|
||||
// four bytes — RIFF alone is also .wav, and the `ftyp` box also fronts .mp4.
|
||||
const SIGNATURES = [
|
||||
{ mime: 'image/png', test: (b) => b.subarray(0, 8).equals(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])) },
|
||||
{ mime: 'image/jpeg', test: (b) => b[0] === 0xff && b[1] === 0xd8 && b[2] === 0xff },
|
||||
{ mime: 'image/gif', test: (b) => b.subarray(0, 6).toString('latin1').match(/^GIF8[79]a$/) != null },
|
||||
{
|
||||
mime: 'image/webp',
|
||||
test: (b) => b.subarray(0, 4).toString('latin1') === 'RIFF' && b.subarray(8, 12).toString('latin1') === 'WEBP',
|
||||
},
|
||||
{
|
||||
mime: 'image/avif',
|
||||
test: (b) => b.subarray(4, 8).toString('latin1') === 'ftyp'
|
||||
&& ['avif', 'avis'].includes(b.subarray(8, 12).toString('latin1')),
|
||||
},
|
||||
]
|
||||
|
||||
// Per-post attachment cap and per-account rolling byte quota.
|
||||
const MAX_ATTACHMENTS_PER_POST = 6
|
||||
const DAILY_QUOTA_BYTES = 25 * 1024 * 1024
|
||||
const QUOTA_WINDOW_HOURS = 24
|
||||
|
||||
// Lifecycle windows. A soft-deleted file survives long enough for a mis-click to
|
||||
// be recoverable; an orphan is one uploaded into a composer that was never
|
||||
// submitted, which is a normal thing to do and so gets a generous grace.
|
||||
const RETENTION_DAYS = 30
|
||||
const ORPHAN_GRACE_HOURS = 48
|
||||
|
||||
/**
|
||||
* What do these bytes actually claim to be?
|
||||
*
|
||||
* Returns the sniffed mimetype, or null when nothing matches. Null is a rejection
|
||||
* and never a "trust the header instead" — an unrecognised file is exactly the
|
||||
* case this check exists for.
|
||||
*/
|
||||
function sniff(buffer) {
|
||||
if (!Buffer.isBuffer(buffer) || buffer.length < 12) return null
|
||||
return SIGNATURES.find((s) => s.test(buffer))?.mime || null
|
||||
}
|
||||
|
||||
/**
|
||||
* Accept a file multer has already written to disk.
|
||||
*
|
||||
* The file is on disk before it can be sniffed — multer streams it there — so the
|
||||
* rejection path has to REMOVE it. A rejected upload that stays on disk is exactly
|
||||
* the disk-exhaustion vector the quota exists to close, reached by a different
|
||||
* route.
|
||||
*/
|
||||
async function accept({ team, actor, file }) {
|
||||
const stored = path.join(UPLOAD_DIR, file.filename)
|
||||
const discard = async () => { await fs.rm(stored, { force: true }) }
|
||||
|
||||
let head
|
||||
try {
|
||||
const handle = await fs.open(stored, 'r')
|
||||
try {
|
||||
head = Buffer.alloc(16)
|
||||
await handle.read(head, 0, 16, 0)
|
||||
} finally {
|
||||
await handle.close()
|
||||
}
|
||||
} catch {
|
||||
await discard()
|
||||
return { ok: false, status: 400, error: 'Could not read the uploaded file' }
|
||||
}
|
||||
|
||||
const sniffed = sniff(head)
|
||||
if (!sniffed || sniffed !== file.mimetype) {
|
||||
await discard()
|
||||
return { ok: false, status: 400, error: 'That file is not the image type it claims to be' }
|
||||
}
|
||||
|
||||
const used = await forumDb.bytesUploadedSince(actor.id, QUOTA_WINDOW_HOURS)
|
||||
if (used + file.size > DAILY_QUOTA_BYTES) {
|
||||
await discard()
|
||||
return { ok: false, status: 429, error: 'Daily upload limit reached. Try again tomorrow.' }
|
||||
}
|
||||
|
||||
const id = await forumDb.insertUpload({
|
||||
teamId: team.id,
|
||||
postId: null, // attached when the post that embeds it is written
|
||||
uploaderUserId: actor.id,
|
||||
uploaderUsername: actor.username,
|
||||
filename: file.filename,
|
||||
mimetype: sniffed, // the SNIFFED type, never the client's header
|
||||
byteSize: file.size,
|
||||
})
|
||||
return { ok: true, id, url: `/uploads/${file.filename}`, bytes: file.size }
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove an upload. The uploader may, within the edit window; staff may at any
|
||||
* time. Soft — the bytes go with the sweep, not with the button.
|
||||
*/
|
||||
async function remove({ id, actor, isStaff }) {
|
||||
const row = await forumDb.uploadById(id)
|
||||
if (!row || row.deleted_at) return { ok: false, status: 404, error: 'No such upload' }
|
||||
if (!isStaff && row.uploader_user_id !== actor.id) {
|
||||
return { ok: false, status: 403, error: 'Not your upload' }
|
||||
}
|
||||
await forumDb.softDeleteUpload(id, actor.id)
|
||||
return { ok: true }
|
||||
}
|
||||
|
||||
/**
|
||||
* The nightly sweep: bytes for soft-deleted rows past retention, plus files on
|
||||
* disk with no row at all.
|
||||
*
|
||||
* The orphan half deliberately only considers files whose names match the upload
|
||||
* naming scheme AND appear in no row. UPLOAD_DIR is shared with the admin upload
|
||||
* path, whose files have no row here and must never be swept — so the sweep works
|
||||
* from the FORUM's own rows outward and never from the directory listing inward.
|
||||
*/
|
||||
async function sweep({ retentionDays = RETENTION_DAYS, orphanGraceHours = ORPHAN_GRACE_HOURS } = {}) {
|
||||
const expired = await forumDb.sweepableUploads(retentionDays)
|
||||
const orphans = await forumDb.orphanedUploads(orphanGraceHours)
|
||||
const doomed = [...expired, ...orphans]
|
||||
const cleared = []
|
||||
for (const row of doomed) {
|
||||
try {
|
||||
await fs.rm(path.join(UPLOAD_DIR, row.filename), { force: true })
|
||||
cleared.push(row.id)
|
||||
} catch {
|
||||
// Leave the ROW as well as the file. A file we could not delete is one the
|
||||
// next run should try again, and dropping its row would lose the only
|
||||
// record that the bytes are still there.
|
||||
}
|
||||
}
|
||||
await forumDb.deleteUploadRows(cleared)
|
||||
return { swept: doomed.length, filesRemoved: cleared.length }
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
MAX_ATTACHMENTS_PER_POST,
|
||||
DAILY_QUOTA_BYTES,
|
||||
QUOTA_WINDOW_HOURS,
|
||||
RETENTION_DAYS,
|
||||
ORPHAN_GRACE_HOURS,
|
||||
sniff,
|
||||
accept,
|
||||
remove,
|
||||
sweep,
|
||||
}
|
||||
Reference in New Issue
Block a user