Skip to content

Commit bb749a5

Browse files
eddieranclaudePatrikKozakpaulpopus
authored
fix: enforce mimeTypes restriction when useTempFiles is enabled (#16255)
This replaces #16236 which was auto-closed when the fork was deleted. # Overview Fixes MIME type validation being skipped on upload collections when `upload.useTempFiles: true` is set globally. ## Key Changes - Use `fileTypeFromFile` for temp files instead of loading the full buffer — avoids reading large files (e.g. 2GB video) into memory just for MIME detection - Removed the `!useTempFiles` gate that was causing the entire fallback validation block to be skipped - Added `tempFilePath` to the `File` type directly instead of intersecting it at the call site - Full file content is loaded lazily via `getFileBuffer()` and only when needed for SVG/PDF content validation ## Design Decisions The original code used the `useTempFiles` config flag to decide whether to run fallback validation, but checking `tempFilePath` directly is more accurate, files uploaded via the local API always have `file.data` populated even when `useTempFiles` is enabled. Simply removing the gate (as originally suggested) would've fixed the extension check but left `validateSvg` broken, since it would still run on an empty buffer and always return safe. Fixes #16233. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Patrik Kozak <35232443+PatrikKozak@users.noreply.github.com> Co-authored-by: Paul Popus <paul@payloadcms.com>
1 parent 4912005 commit bb749a5

4 files changed

Lines changed: 194 additions & 13 deletions

File tree

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ test/azurestoragedata/
348348
/media-without-delete-access
349349
/media-documents
350350
/media-with-always-insert-fields
351-
351+
/tmp
352352

353353

354354
licenses.csv

packages/payload/src/uploads/checkFileRestrictions.ts

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { fileTypeFromBuffer } from 'file-type'
1+
import { fileTypeFromBuffer, fileTypeFromFile } from 'file-type'
2+
import fs from 'fs/promises'
23

34
import type { checkFileRestrictionsParams, FileAllowList } from './types.js'
45

@@ -53,7 +54,6 @@ export const checkFileRestrictions = async ({
5354
}: checkFileRestrictionsParams): Promise<void> => {
5455
const errors: string[] = []
5556
const { upload: uploadConfig } = collection
56-
const useTempFiles = req?.payload?.config?.upload?.useTempFiles ?? false
5757
const configMimeTypes =
5858
uploadConfig &&
5959
typeof uploadConfig === 'object' &&
@@ -87,9 +87,44 @@ export const checkFileRestrictions = async ({
8787
return
8888
}
8989

90+
// For temp files, use fileTypeFromFile so large files (e.g. video) are never loaded into memory
91+
// just for detection. For content validation (SVG safety, PDF integrity), the full buffer is
92+
// loaded lazily and only when the file type actually requires it.
93+
const { tempFilePath } = file
94+
const isTempFile = !!tempFilePath && (!file.data || file.data.length === 0)
95+
96+
// Lazily reads the full file — only reached for small text-based types (SVG, PDF).
97+
let _fileBuffer: Buffer | undefined
98+
const getFileBuffer = async (): Promise<Buffer> => {
99+
if (_fileBuffer) {
100+
return _fileBuffer
101+
}
102+
if (!isTempFile || !tempFilePath) {
103+
return (_fileBuffer = file.data)
104+
}
105+
try {
106+
_fileBuffer = await fs.readFile(tempFilePath)
107+
return _fileBuffer
108+
} catch {
109+
throw new ValidationError({
110+
errors: [{ message: 'Could not read uploaded file for validation.', path: 'file' }],
111+
})
112+
}
113+
}
114+
90115
// Secondary mimetype check to assess file type from buffer
91116
if (configMimeTypes.length > 0) {
92-
let detected = await fileTypeFromBuffer(file.data)
117+
let detected
118+
try {
119+
detected =
120+
isTempFile && tempFilePath
121+
? await fileTypeFromFile(tempFilePath)
122+
: await fileTypeFromBuffer(file.data)
123+
} catch {
124+
throw new ValidationError({
125+
errors: [{ message: 'Could not read uploaded file for type detection.', path: 'file' }],
126+
})
127+
}
93128
const typeFromExtension = file.name.split('.').pop() || ''
94129

95130
// Handle SVG files that are detected as XML due to <?xml declarations
@@ -99,13 +134,13 @@ export const checkFileRestrictions = async ({
99134
(type) => type.includes('image/') && (type.includes('svg') || type === 'image/*'),
100135
)
101136
) {
102-
const isSvg = detectSvgFromXml(file.data)
137+
const isSvg = detectSvgFromXml(await getFileBuffer())
103138
if (isSvg) {
104-
detected = { ext: 'svg' as any, mime: 'image/svg+xml' as any }
139+
detected = { ext: 'svg', mime: 'image/svg+xml' }
105140
}
106141
}
107142

108-
if (!detected && !useTempFiles) {
143+
if (!detected) {
109144
const mimeTypeFromExtension = getFileTypeFallback(file.name).mime
110145
const extIsValid = validateMimeType(mimeTypeFromExtension, configMimeTypes)
111146

@@ -116,15 +151,15 @@ export const checkFileRestrictions = async ({
116151
} else {
117152
// SVG security check (text-based files not detectable by buffer)
118153
if (typeFromExtension.toLowerCase() === 'svg') {
119-
const isSafeSvg = validateSvg(file.data)
154+
const isSafeSvg = validateSvg(await getFileBuffer())
120155
if (!isSafeSvg) {
121156
errors.push('SVG file contains potentially harmful content.')
122157
}
123158
}
124159

125160
// PDF validation
126161
if (mimeTypeFromExtension === 'application/pdf') {
127-
const isValidPDF = validatePDF(file.data)
162+
const isValidPDF = validatePDF(await getFileBuffer())
128163
if (!isValidPDF) {
129164
errors.push('Invalid or corrupted PDF file.')
130165
}
@@ -141,7 +176,7 @@ export const checkFileRestrictions = async ({
141176
const passesMimeTypeCheck = detected?.mime && validateMimeType(detected.mime, configMimeTypes)
142177

143178
if (passesMimeTypeCheck && detected?.mime === 'application/pdf') {
144-
const isValidPDF = validatePDF(file?.data)
179+
const isValidPDF = validatePDF(await getFileBuffer())
145180
if (!isValidPDF) {
146181
errors.push('Invalid PDF file.')
147182
}

packages/payload/src/uploads/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,10 @@ export type File = {
341341
* The size of the file in bytes.
342342
*/
343343
size: number
344+
/**
345+
* Path to the temp file on disk when useTempFiles is enabled. In this case file.data will be an empty buffer.
346+
*/
347+
tempFilePath?: string
344348
}
345349

346350
export type FileToSave = {

test/uploads/int.spec.ts

Lines changed: 145 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { CollectionSlug, Payload, PayloadRequest } from 'payload'
44
import { randomUUID } from 'crypto'
55
import fs from 'fs'
66
import { createServer } from 'http'
7+
import os from 'os'
78
import path from 'path'
89
import { _internal_safeFetchGlobal, createPayloadRequest, getFileByPath } from 'payload'
910
import { fileURLToPath } from 'url'
@@ -13,6 +14,8 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vitest } from 'vi
1314
import type { NextRESTClient } from '../__helpers/shared/NextRESTClient.js'
1415
import type { Enlarge, Media } from './payload-types.js'
1516

17+
// eslint-disable-next-line payload/no-relative-monorepo-imports
18+
import { checkFileRestrictions } from '../../packages/payload/src/uploads/checkFileRestrictions.js'
1619
// eslint-disable-next-line payload/no-relative-monorepo-imports
1720
import { getExternalFile } from '../../packages/payload/src/uploads/getExternalFile.js'
1821
// eslint-disable-next-line payload/no-relative-monorepo-imports
@@ -502,13 +505,13 @@ describe('Collections - Uploads', () => {
502505
it('should serve files with hash characters in filename', async () => {
503506
const filePath = path.resolve(dirname, './image.png')
504507
const file = await getFileByPath(filePath)
505-
file!.name = "file #hash.png"
508+
file!.name = 'file #hash.png'
506509

507-
const mediaDoc = (await payload.create({
510+
const mediaDoc = await payload.create({
508511
collection: mediaSlug,
509512
data: {},
510513
file,
511-
}))
514+
})
512515

513516
expect(mediaDoc.url).toContain('%23')
514517
expect(mediaDoc.url).not.toContain('#')
@@ -1113,6 +1116,145 @@ describe('Collections - Uploads', () => {
11131116
}),
11141117
).resolves.not.toThrow()
11151118
})
1119+
1120+
describe('useTempFiles MIME type bypass', () => {
1121+
const createdTmpFiles: string[] = []
1122+
1123+
const mockReq = {
1124+
payload: {
1125+
config: { upload: { useTempFiles: true } },
1126+
logger: { warn: () => {}, error: () => {} },
1127+
},
1128+
} as unknown as PayloadRequest
1129+
1130+
afterEach(async () => {
1131+
for (const tmpFile of createdTmpFiles) {
1132+
try {
1133+
await fs.promises.unlink(tmpFile)
1134+
} catch {
1135+
// ignore cleanup errors
1136+
}
1137+
}
1138+
createdTmpFiles.length = 0
1139+
})
1140+
1141+
it('should not bypass mimeTypes restriction when useTempFiles is enabled and file is HTML', async () => {
1142+
const htmlContent = Buffer.from('<html><script>alert("xss")</script></html>')
1143+
const tmpFile = path.join(os.tmpdir(), `payload-test-${randomUUID()}.html`)
1144+
createdTmpFiles.push(tmpFile)
1145+
await fs.promises.writeFile(tmpFile, htmlContent)
1146+
1147+
await expect(
1148+
checkFileRestrictions({
1149+
collection: {
1150+
slug: 'test',
1151+
upload: { mimeTypes: ['image/*'], staticDir: '/tmp' },
1152+
} as any,
1153+
file: {
1154+
data: Buffer.alloc(0),
1155+
mimetype: 'text/html',
1156+
name: 'malicious.html',
1157+
size: htmlContent.length,
1158+
tempFilePath: tmpFile,
1159+
},
1160+
req: mockReq,
1161+
}),
1162+
).rejects.toMatchObject({ name: 'ValidationError' })
1163+
})
1164+
1165+
it('should not bypass SVG content validation when useTempFiles is enabled', async () => {
1166+
const svgContent = Buffer.from(
1167+
'<svg xmlns="http://www.w3.org/2000/svg"><script>alert("xss")</script></svg>',
1168+
)
1169+
const tmpFile = path.join(os.tmpdir(), `payload-test-${randomUUID()}.svg`)
1170+
createdTmpFiles.push(tmpFile)
1171+
await fs.promises.writeFile(tmpFile, svgContent)
1172+
1173+
await expect(
1174+
checkFileRestrictions({
1175+
collection: {
1176+
slug: 'test',
1177+
upload: { mimeTypes: ['image/svg+xml', 'image/*'], staticDir: '/tmp' },
1178+
} as any,
1179+
file: {
1180+
data: Buffer.alloc(0),
1181+
mimetype: 'image/svg+xml',
1182+
name: 'malicious.svg',
1183+
size: svgContent.length,
1184+
tempFilePath: tmpFile,
1185+
},
1186+
req: mockReq,
1187+
}),
1188+
).rejects.toMatchObject({ name: 'ValidationError' })
1189+
})
1190+
1191+
it('should allow a valid image file when useTempFiles is enabled', async () => {
1192+
const pngData = await fs.promises.readFile(path.resolve(dirname, './image.png'))
1193+
const tmpFile = path.join(os.tmpdir(), `payload-test-${randomUUID()}.png`)
1194+
createdTmpFiles.push(tmpFile)
1195+
await fs.promises.writeFile(tmpFile, pngData)
1196+
1197+
await expect(
1198+
checkFileRestrictions({
1199+
collection: {
1200+
slug: 'test',
1201+
upload: { mimeTypes: ['image/*'], staticDir: '/tmp' },
1202+
} as any,
1203+
file: {
1204+
data: Buffer.alloc(0),
1205+
mimetype: 'image/png',
1206+
name: 'valid.png',
1207+
size: pngData.length,
1208+
tempFilePath: tmpFile,
1209+
},
1210+
req: mockReq,
1211+
}),
1212+
).resolves.not.toThrow()
1213+
})
1214+
1215+
it('should throw ValidationError when tempFilePath is missing and file.data is empty', async () => {
1216+
// No tempFilePath — falls through to extension-based check, which should still reject
1217+
await expect(
1218+
checkFileRestrictions({
1219+
collection: {
1220+
slug: 'test',
1221+
upload: { mimeTypes: ['image/*'], staticDir: '/tmp' },
1222+
} as any,
1223+
file: {
1224+
data: Buffer.alloc(0),
1225+
mimetype: 'text/html',
1226+
name: 'malicious.html',
1227+
size: 0,
1228+
},
1229+
req: mockReq,
1230+
}),
1231+
).rejects.toMatchObject({ name: 'ValidationError' })
1232+
})
1233+
1234+
it('should reject an invalid PDF when useTempFiles is enabled', async () => {
1235+
const invalidPdfContent = Buffer.from('not a pdf')
1236+
const tmpFile = path.join(os.tmpdir(), `payload-test-${randomUUID()}.pdf`)
1237+
createdTmpFiles.push(tmpFile)
1238+
await fs.promises.writeFile(tmpFile, invalidPdfContent)
1239+
1240+
await expect(
1241+
checkFileRestrictions({
1242+
collection: {
1243+
slug: 'test',
1244+
upload: { mimeTypes: ['application/pdf'], staticDir: '/tmp' },
1245+
} as any,
1246+
file: {
1247+
data: Buffer.alloc(0),
1248+
mimetype: 'application/pdf',
1249+
name: 'invalid.pdf',
1250+
size: invalidPdfContent.length,
1251+
tempFilePath: tmpFile,
1252+
},
1253+
req: mockReq,
1254+
}),
1255+
).rejects.toMatchObject({ name: 'ValidationError' })
1256+
})
1257+
})
11161258
})
11171259
})
11181260

0 commit comments

Comments
 (0)