Skip to content

Commit 45bd2f1

Browse files
authored
fix: sanitize filenames in storage adapters (#15746)
### What Adds filename + prefix sanitization to all storage adapters (S3, GCS, Azure, R2). ### Why Storage adapter uploads pass filenames directly to `path.posix.join(prefix, filename)` without sanitization. Regular uploads don't have this issue because they go through `generateFileData`, which already sanitizes filenames via the `sanitize-filename` package. This brings storage adapter filename handling in line with the rest of the upload pipeline. ### How Exported `sanitizeFilename` from `payload` to reuse in the storage adapters. #### Testing Added tests to `test/storage-s3/clientUploads.int.spec.ts`
1 parent c74d91d commit 45bd2f1

8 files changed

Lines changed: 114 additions & 6 deletions

File tree

packages/payload/src/exports/shared.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ export {
134134

135135
export { reduceFieldsToValues } from '../utilities/reduceFieldsToValues.js'
136136

137+
export { sanitizeFilename } from '../utilities/sanitizeFilename.js'
138+
137139
export { sanitizeUserDataForEmail } from '../utilities/sanitizeUserDataForEmail.js'
138140

139141
export { setsAreEqual } from '../utilities/setsAreEqual.js'
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { APIError } from '../errors/APIError.js'
2+
3+
/**
4+
* Strips directory components and control characters from a filename,
5+
* leaving only the base filename.
6+
*/
7+
export function sanitizeFilename(filename: string): string {
8+
let sanitized = filename.replace(/\\/g, '/')
9+
10+
const lastSlash = sanitized.lastIndexOf('/')
11+
if (lastSlash !== -1) {
12+
sanitized = sanitized.slice(lastSlash + 1)
13+
}
14+
15+
if (sanitized === '.' || sanitized === '..') {
16+
sanitized = ''
17+
}
18+
19+
// eslint-disable-next-line no-control-regex
20+
sanitized = sanitized.replace(/[\x00-\x1f\x80-\x9f]/g, '')
21+
22+
if (!sanitized) {
23+
throw new APIError('Invalid filename', 400)
24+
}
25+
26+
return sanitized
27+
}

packages/storage-azure/src/generateSignedURL.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { PayloadHandler } from 'payload'
55
import { BlobSASPermissions, generateBlobSASQueryParameters } from '@azure/storage-blob'
66
import path from 'path'
77
import { APIError, Forbidden } from 'payload'
8+
import { sanitizeFilename } from 'payload/shared'
89

910
import type { AzureStorageOptions } from './index.js'
1011

@@ -45,7 +46,8 @@ export const getGenerateSignedURLHandler = ({
4546
throw new Forbidden()
4647
}
4748

48-
const fileKey = path.posix.join(prefix, filename)
49+
const sanitizedFilename = sanitizeFilename(filename)
50+
const fileKey = path.posix.join(prefix, sanitizedFilename)
4951

5052
const blobClient = getStorageClient().getBlobClient(fileKey)
5153

packages/storage-gcs/src/generateSignedURL.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { PayloadHandler } from 'payload'
44

55
import path from 'path'
66
import { APIError, Forbidden } from 'payload'
7+
import { sanitizeFilename } from 'payload/shared'
78

89
import type { GcsStorageOptions } from './index.js'
910

@@ -45,7 +46,8 @@ export const getGenerateSignedURLHandler = ({
4546
throw new Forbidden()
4647
}
4748

48-
const fileKey = path.posix.join(prefix, filename)
49+
const sanitizedFilename = sanitizeFilename(filename)
50+
const fileKey = path.posix.join(prefix, sanitizedFilename)
4951

5052
const [url] = await getStorageClient()
5153
.bucket(bucket)

packages/storage-r2/src/handleMultiPartUpload.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { PayloadHandler } from 'payload'
33

44
import path from 'path'
55
import { APIError, Forbidden } from 'payload'
6+
import { sanitizeFilename } from 'payload/shared'
67

78
import type { R2StorageOptions } from './index.js'
89
import type { R2Bucket, R2StorageMultipartUploadHandlerParams } from './types.js'
@@ -51,7 +52,8 @@ export const getHandleMultiPartUpload =
5152
}
5253

5354
const prefix = (typeof collectionConfig === 'object' && collectionConfig.prefix) || ''
54-
const fileKey = path.posix.join(prefix, params.fileName)
55+
const sanitizedFilename = sanitizeFilename(params.fileName)
56+
const fileKey = path.posix.join(prefix, sanitizedFilename)
5557

5658
const multipartId = params.multipartId
5759
const multipartKey = params.multipartKey

packages/storage-s3/src/generateSignedURL.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as AWS from '@aws-sdk/client-s3'
55
import { getSignedUrl } from '@aws-sdk/s3-request-presigner'
66
import path from 'path'
77
import { APIError, Forbidden, ValidationError } from 'payload'
8+
import { sanitizeFilename } from 'payload/shared'
89

910
import type { S3StorageOptions } from './index.js'
1011

@@ -58,7 +59,8 @@ export const getGenerateSignedURLHandler = ({
5859
throw new Forbidden()
5960
}
6061

61-
const fileKey = path.posix.join(prefix, filename)
62+
const sanitizedFilename = sanitizeFilename(filename)
63+
const fileKey = path.posix.join(prefix, sanitizedFilename)
6264

6365
const signableHeaders = new Set<string>()
6466

test/storage-s3/clientUploads.config.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ import path from 'path'
66
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
77
import { devUser } from '../credentials.js'
88
import { Media } from './collections/Media.js'
9+
import { MediaWithPrefix } from './collections/MediaWithPrefix.js'
910
import { Users } from './collections/Users.js'
10-
import { mediaSlug } from './shared.js'
11+
import { mediaSlug, mediaWithPrefixSlug } from './shared.js'
1112
import { MB } from './test-utils.js'
1213
const filename = fileURLToPath(import.meta.url)
1314
const dirname = path.dirname(filename)
@@ -23,7 +24,7 @@ export default buildConfigWithDefaults({
2324
baseDir: path.resolve(dirname),
2425
},
2526
},
26-
collections: [Media, Users],
27+
collections: [Media, MediaWithPrefix, Users],
2728
onInit: async (payload) => {
2829
await payload.create({
2930
collection: 'users',
@@ -37,6 +38,9 @@ export default buildConfigWithDefaults({
3738
s3Storage({
3839
collections: {
3940
[mediaSlug]: true,
41+
[mediaWithPrefixSlug]: {
42+
prefix: 'test-prefix',
43+
},
4044
},
4145
bucket: process.env.S3_BUCKET!,
4246
clientUploads: {

test/storage-s3/clientUploads.int.spec.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,73 @@ describe('@payloadcms/storage-s3 clientUploads', () => {
204204
expect(uploadResponse.status).toBe(403) // S3 should reject the upload
205205
})
206206

207+
describe('filename handling', () => {
208+
it('should sanitize special characters in filename', async () => {
209+
const file = readFileSync(path.resolve(dirname, '../uploads/image.png'))
210+
211+
const { url } = await restClient
212+
.POST(signedURLEndpoint, {
213+
body: signedURLBody('media-with-prefix', '../photo.png', file.length, 'image/png'),
214+
})
215+
.then((res) => res.json<{ url: string }>())
216+
217+
expect(url).toBeDefined()
218+
expect(url).toContain('test-prefix')
219+
expect(url).toContain('photo.png')
220+
expect(url).not.toContain('..')
221+
})
222+
223+
it('should sanitize deeply nested special characters in filename', async () => {
224+
const file = readFileSync(path.resolve(dirname, '../uploads/image.png'))
225+
226+
const { url } = await restClient
227+
.POST(signedURLEndpoint, {
228+
body: signedURLBody(
229+
'media-with-prefix',
230+
'../../other-prefix/document.js',
231+
file.length,
232+
'image/png',
233+
),
234+
})
235+
.then((res) => res.json<{ url: string }>())
236+
237+
expect(url).toBeDefined()
238+
expect(url).toContain('test-prefix')
239+
expect(url).toContain('document.js')
240+
expect(url).not.toContain('..')
241+
expect(url).not.toContain('other-prefix')
242+
})
243+
244+
it('should sanitize backslash characters in filename', async () => {
245+
const file = readFileSync(path.resolve(dirname, '../uploads/image.png'))
246+
247+
const { url } = await restClient
248+
.POST(signedURLEndpoint, {
249+
body: signedURLBody('media-with-prefix', '..\\..\\photo.png', file.length, 'image/png'),
250+
})
251+
.then((res) => res.json<{ url: string }>())
252+
253+
expect(url).toBeDefined()
254+
expect(url).toContain('test-prefix')
255+
expect(url).toContain('photo.png')
256+
expect(url).not.toContain('..')
257+
})
258+
259+
it('should allow normal filenames with prefix', async () => {
260+
const file = readFileSync(path.resolve(dirname, '../uploads/image.png'))
261+
262+
const { url } = await restClient
263+
.POST(signedURLEndpoint, {
264+
body: signedURLBody('media-with-prefix', 'safe-image.png', file.length, 'image/png'),
265+
})
266+
.then((res) => res.json<{ url: string }>())
267+
268+
expect(url).toBeDefined()
269+
expect(url).toContain('test-prefix')
270+
expect(url).toContain('safe-image.png')
271+
})
272+
})
273+
207274
afterAll(async () => {
208275
await payload.destroy()
209276
})

0 commit comments

Comments
 (0)