fix(web): validate attachment filename types - #380
Conversation
Signed-off-by: seekskyworld <djh1813553759@gmail.com>
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Exact-head review of 5bf88a1. The MIME/extension admission boundary is useful, but malformed MIME metadata currently throws instead of returning the validator's bounded rejection result. The existing integration concerns with #365/#388 also remain; this review adds one independently reproduced correctness finding. No deployed HTTP or installed runtime impact is claimed.
| for (const attachment of attachments) { | ||
| if (!/^(?!\.\.?(?:$|\.))[\w .()\[\]-]{1,120}$/u.test(attachment.name) || attachment.name.includes("..")) return { ok: false as const, error: "invalid attachment name" }; | ||
| if (!MIME_EXTENSIONS[attachment.mime]) return { ok: false as const, error: "unsupported attachment type" }; | ||
| if (!MIME_EXTENSIONS[attachment.mime].some((extension) => attachment.name.toLocaleLowerCase().endsWith(extension))) return { ok: false as const, error: "attachment extension does not match type" }; |
There was a problem hiding this comment.
[P2] Reject inherited object keys before invoking the extension list
MIME_EXTENSIONS is an ordinary object, so a MIME value of constructor or __proto__ passes the truthiness check on the preceding line. This line then throws TypeError: MIME_EXTENSIONS[attachment.mime].some is not a function, rather than returning { ok: false, error: "unsupported attachment type" }. I reproduced both with the exact-head module and validateWebAttachments([{ name: "x.txt", mime, size: 1 }]); the normal text/plain case succeeds.
Use an own-property whitelist lookup (or a Map/null-prototype record) and add regression cases for inherited keys so arbitrary unsupported MIME strings are rejected without throwing. This is the pure admission contract, not evidence of an active upload endpoint.
2 participants
Problem
Adds the MIME/filename consistency slice of #344. Attachment admission could accept a filename whose extension contradicted its declared content type.
Value
Future upload paths get a clearer fail-closed boundary against misleading or malformed attachment metadata.
Approach
Require supported MIME types to use an allowed filename extension while retaining count, traversal, per-file, and aggregate byte limits.
Validation
npx tsc --noEmitgit diff --checkImpact