fix: validate file contents in dynamic mode#1924
Conversation
`t.File`/`t.Files` with a declared type validates the actual file bytes (magic-byte check via `fileType`) in AOT mode, but the dynamic handler (`aot: false`) only ran the synchronous schema check, accepting a file whose contents do not match the declared type. Mirror the AOT path in `createDynamicHandler`: after body validation, run `fileType(...)` for File/Files fields that declare an extension, so both modes enforce the same guarantee. Closes elysiajs#1922
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough
ChangesDynamic mode file-type validation
Sequence Diagram(s)sequenceDiagram
participant Client
participant createDynamicHandler
participant unwrapImportSchema
participant fileType
Client->>createDynamicHandler: multipart POST (File field)
createDynamicHandler->>createDynamicHandler: parse & decode body
createDynamicHandler->>unwrapImportSchema: resolve body validator schema
unwrapImportSchema-->>createDynamicHandler: schema with File/Files properties
loop each property with extension
createDynamicHandler->>fileType: fileType(decoded file value)
fileType-->>createDynamicHandler: { ext } or null
alt extension mismatch
createDynamicHandler-->>Client: 422 Unprocessable Entity
end
end
createDynamicHandler-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/dynamic-handle.tsOops! Something went wrong! :( ESLint: 9.39.4 Error: ESLint configuration in --config » plugin:sonarjs/recommended is invalid:
Referenced from: /.eslintrc.json ... [truncated 291 characters] ... /node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2952:16) test/validator/body.test.tsOops! Something went wrong! :( ESLint: 9.39.4 Error: ESLint configuration in --config » plugin:sonarjs/recommended is invalid:
Referenced from: /.eslintrc.json ... [truncated 291 characters] ... /node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2952:16) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
You're just trying to take credit for someone else's discovery. Do better, man. |
|
@bmtec good question — yes, this PR is the implementation of #1922, which is why it opens with "Closes #1922" and reuses @rafaelcoelhox's repro. The discovery and the diagnosis are theirs; I have not claimed otherwise. The issue was open with no PR attached, so I took a shot at the actual fix — mirroring the AOT |
3 participants
Closes #1922.
t.File({ type })/t.Files({ type })validates the actual file bytes (magic-byte check viafileType→file-type) in AOT mode, but in dynamic mode (aot: false) the same schema only ran the synchronous TypeBox check, which trusts the client-supplied multipart MIME type. A file whose bytes do not match the declared type was accepted in dynamic mode and rejected in AOT mode — the two pipelines disagreed on the same schema.Repro from the issue (SVG bytes uploaded as
image/png): AOT →422, dynamic →200.Fix
compose.ts(AOT) injectsfileType(c.body.k, extension, 'body.k')for every File/Files field that declares an extension.createDynamicHandlerdid not. After body validation, iterate the body schema's properties (via the existinggetSchemaProperties, which also flattens unions) and run the samefileType(...)for File/Files fields with an extension — so both modes enforce the identical guarantee using the identical code path and error (InvalidFileType→422).Tests
Added
validate actual file in dynamic modetotest/validator/body.test.ts, mirroring the existing AOTvalidate actual filetest but withnew Elysia({ aot: false }): a realmillenium.jpgpasses (200), a spoofedfake.jpgis rejected (422). Red before the change (dynamic returned200), green after.test/validator/body.test.ts(57),test/type-system/files.test.ts+formdata.test.ts(31), andtest/core/dynamic.test.ts(32) all pass;tsc --project tsconfig.test.jsonis clean.Summary by CodeRabbit
Bug Fixes