Skip to content

Commit 05ddec1

Browse files
feat: disambiguate media files by prefix via query parameter (#15844)
Add optional `?prefix=` query string support to the existing `/api/:collection/file/:filename` endpoint to disambiguate files that share the same filename but have different storage prefixes. When a storage prefix (e.g., UUID or folder path) is used to make S3 keys unique, multiple documents can share the same `filename`. The current endpoint resolves by filename alone, which can match the wrong document in both access control and file retrieval. - `checkFileAccess` accepts optional `prefix` and adds it to the `where` clause alongside existing access filters - `getFile` handler reads `prefix` from `req.searchParams` and threads it to `checkFileAccess` and handler `params` - `getFilePrefix` accepts `explicitPrefix` to skip the DB query when the prefix is already known from the URL - S3 `staticHandler` forwards `prefix` from params to `getFilePrefix` - `afterRead` hook appends `?prefix=` to Payload-proxied URLs - Added unit tests for `checkFileAccess` and `getFilePrefix`, and integration tests for the endpoint' --------- Co-authored-by: Jarrod Flesch <jarrodmflesch@gmail.com>
1 parent 3a1f31a commit 05ddec1

24 files changed

Lines changed: 534 additions & 41 deletions

File tree

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
3+
import type { Collection } from '../collections/config/types.js'
4+
import type { PayloadRequest } from '../types/index.js'
5+
6+
vi.mock('../auth/executeAccess.js', () => ({
7+
executeAccess: vi.fn(),
8+
}))
9+
10+
import { executeAccess } from '../auth/executeAccess.js'
11+
12+
import { checkFileAccess } from './checkFileAccess.js'
13+
14+
const makeFindOne = (result: unknown = { id: '1', filename: 'logo.png' }) =>
15+
vi.fn().mockResolvedValue(result)
16+
17+
const makeCollection = (): Collection =>
18+
({
19+
config: {
20+
slug: 'test-media',
21+
access: { read: vi.fn() },
22+
upload: {},
23+
},
24+
}) as unknown as Collection
25+
26+
const makeReq = (findOne: ReturnType<typeof vi.fn>): PayloadRequest =>
27+
({
28+
t: vi.fn(),
29+
payload: {
30+
db: { findOne },
31+
},
32+
}) as unknown as PayloadRequest
33+
34+
describe('checkFileAccess', () => {
35+
beforeEach(() => {
36+
vi.mocked(executeAccess).mockResolvedValue({})
37+
})
38+
39+
describe('prefix filtering', () => {
40+
it('should add prefix clause to where query when prefix is provided', async () => {
41+
const findOne = makeFindOne()
42+
const req = makeReq(findOne)
43+
const collection = makeCollection()
44+
45+
await checkFileAccess({ collection, filename: 'logo.png', prefix: 'abc123', req })
46+
47+
const whereArg = findOne.mock.calls[0]?.[0]?.where
48+
expect(whereArg?.and).toEqual(expect.arrayContaining([{ prefix: { equals: 'abc123' } }]))
49+
})
50+
51+
it('should not add prefix clause to where query when prefix is omitted', async () => {
52+
const findOne = makeFindOne()
53+
const req = makeReq(findOne)
54+
const collection = makeCollection()
55+
56+
await checkFileAccess({ collection, filename: 'logo.png', req })
57+
58+
const whereArg = findOne.mock.calls[0]?.[0]?.where
59+
const hasPrefixCondition = whereArg?.and?.some(
60+
(clause: Record<string, unknown>) => 'prefix' in clause,
61+
)
62+
expect(hasPrefixCondition).toBeFalsy()
63+
})
64+
65+
it('should still include filename in where query when prefix is provided', async () => {
66+
const findOne = makeFindOne()
67+
const req = makeReq(findOne)
68+
const collection = makeCollection()
69+
70+
await checkFileAccess({ collection, filename: 'logo.png', prefix: 'abc123', req })
71+
72+
const whereArg = findOne.mock.calls[0]?.[0]?.where
73+
const filenameCondition = whereArg?.and?.[0]
74+
expect(filenameCondition?.or).toEqual(
75+
expect.arrayContaining([{ filename: { equals: 'logo.png' } }]),
76+
)
77+
})
78+
79+
it('should throw when no doc matches the given prefix', async () => {
80+
const findOne = makeFindOne(null)
81+
const req = makeReq(findOne)
82+
const collection = makeCollection()
83+
84+
await expect(
85+
checkFileAccess({ collection, filename: 'logo.png', prefix: 'nonexistent', req }),
86+
).rejects.toThrow()
87+
})
88+
89+
it('should throw when filename contains path traversal sequence', async () => {
90+
const findOne = makeFindOne()
91+
const req = makeReq(findOne)
92+
const collection = makeCollection()
93+
94+
await expect(
95+
checkFileAccess({ collection, filename: '../etc/passwd', req }),
96+
).rejects.toThrow()
97+
})
98+
})
99+
})

packages/payload/src/uploads/checkFileAccess.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import { Forbidden } from '../errors/Forbidden.js'
77
export const checkFileAccess = async ({
88
collection,
99
filename,
10+
prefix,
1011
req,
1112
}: {
1213
collection: Collection
1314
filename: string
15+
prefix?: string
1416
req: PayloadRequest
1517
}): Promise<TypeWithID | undefined> => {
1618
if (filename.includes('../') || filename.includes('..\\')) {
@@ -23,36 +25,33 @@ export const checkFileAccess = async ({
2325
config.access.read,
2426
)
2527

28+
const constraints: Where[] = []
29+
2630
if (typeof accessResult === 'object') {
27-
const queryToBuild: Where = {
28-
and: [
29-
{
30-
or: [
31-
{
32-
filename: {
33-
equals: filename,
34-
},
35-
},
36-
],
37-
},
38-
accessResult,
39-
],
31+
constraints.push(accessResult)
32+
}
33+
34+
if (typeof prefix === 'string') {
35+
constraints.push({ prefix: { equals: prefix } })
36+
}
37+
38+
if (constraints.length > 0) {
39+
const filenameCondition: Where = {
40+
or: [{ filename: { equals: filename } }],
4041
}
4142

4243
if (config.upload.imageSizes) {
4344
config.upload.imageSizes.forEach(({ name }) => {
44-
queryToBuild.and?.[0]?.or?.push({
45-
[`sizes.${name}.filename`]: {
46-
equals: filename,
47-
},
45+
filenameCondition.or!.push({
46+
[`sizes.${name}.filename`]: { equals: filename },
4847
})
4948
})
5049
}
5150

5251
const doc = await req.payload.db.findOne({
5352
collection: config.slug,
5453
req,
55-
where: queryToBuild,
54+
where: { and: [filenameCondition, ...constraints] },
5655
})
5756

5857
if (!doc) {

packages/payload/src/uploads/endpoints/getFile.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export const getFileHandler: PayloadHandler = async (req) => {
1919
const collection = getRequestCollection(req)
2020

2121
const filename = req.routeParams?.filename as string
22+
const prefix = req.searchParams?.get('prefix') ?? undefined
2223

2324
if (!collection.config.upload) {
2425
throw new APIError(
@@ -30,6 +31,7 @@ export const getFileHandler: PayloadHandler = async (req) => {
3031
const accessResult = (await checkFileAccess({
3132
collection,
3233
filename,
34+
prefix,
3335
req,
3436
}))!
3537

@@ -48,6 +50,7 @@ export const getFileHandler: PayloadHandler = async (req) => {
4850
params: {
4951
collection: collection.config.slug,
5052
filename,
53+
prefix,
5154
},
5255
})
5356
if (customResponse && customResponse instanceof Response) {

packages/payload/src/uploads/types.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,12 @@ export type UploadConfig = {
245245
args: {
246246
doc: TypeWithID
247247
headers?: Headers
248-
params: { clientUploadContext?: unknown; collection: string; filename: string }
248+
params: {
249+
clientUploadContext?: unknown
250+
collection: string
251+
filename: string
252+
prefix?: string
253+
}
249254
},
250255
) => Promise<Response> | Promise<void> | Response | void)[]
251256
/**

packages/plugin-cloud-storage/src/hooks/afterRead.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ export const getAfterReadHook =
3232
filename,
3333
prefix,
3434
})
35+
} else if (url && prefix) {
36+
const separator = url.includes('?') ? '&' : '?'
37+
url = `${url}${separator}prefix=${encodeURIComponent(prefix)}`
3538
}
3639
}
3740

packages/plugin-cloud-storage/src/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export type StaticHandler = (
6363
args: {
6464
doc?: TypeWithID
6565
headers?: Headers
66-
params: { clientUploadContext?: unknown; collection: string; filename: string }
66+
params: { clientUploadContext?: unknown; collection: string; filename: string; prefix?: string }
6767
},
6868
) => Promise<Response> | Response
6969

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { describe, expect, it, vi } from 'vitest'
2+
3+
import type { CollectionConfig, PayloadRequest } from 'payload'
4+
5+
import { getFilePrefix } from './getFilePrefix.js'
6+
7+
const makeReq = (docs: unknown[] = []) =>
8+
({
9+
payload: {
10+
find: vi.fn().mockResolvedValue({ docs }),
11+
},
12+
}) as unknown as PayloadRequest
13+
14+
const makeCollection = (): CollectionConfig =>
15+
({ slug: 'media', upload: {} }) as unknown as CollectionConfig
16+
17+
describe('getFilePrefix', () => {
18+
describe('prefixQueryParam shortcut', () => {
19+
it('should return prefixQueryParam immediately without querying the database', async () => {
20+
const req = makeReq()
21+
22+
const result = await getFilePrefix({
23+
collection: makeCollection(),
24+
prefixQueryParam: 'uuid-prefix',
25+
filename: 'logo.png',
26+
req,
27+
})
28+
29+
expect(result).toBe('uuid-prefix')
30+
expect(req.payload.find).not.toHaveBeenCalled()
31+
})
32+
33+
it('should return an empty string when prefixQueryParam is an empty string', async () => {
34+
const req = makeReq()
35+
36+
const result = await getFilePrefix({
37+
collection: makeCollection(),
38+
prefixQueryParam: '',
39+
filename: 'logo.png',
40+
req,
41+
})
42+
43+
expect(result).toBe('')
44+
expect(req.payload.find).not.toHaveBeenCalled()
45+
})
46+
47+
it('should reject multi-encoded values', async () => {
48+
const req = makeReq()
49+
50+
const result = await getFilePrefix({
51+
collection: makeCollection(),
52+
prefixQueryParam: '%252e%252e%252fsecret%252f..%252fok',
53+
filename: 'logo.png',
54+
req,
55+
})
56+
57+
expect(result).toBe('')
58+
expect(req.payload.find).not.toHaveBeenCalled()
59+
})
60+
61+
it('should reject malformed percent-encoding', async () => {
62+
const req = makeReq()
63+
64+
const result = await getFilePrefix({
65+
collection: makeCollection(),
66+
prefixQueryParam: '%E0%A4%A',
67+
filename: 'logo.png',
68+
req,
69+
})
70+
71+
expect(result).toBe('')
72+
expect(req.payload.find).not.toHaveBeenCalled()
73+
})
74+
})
75+
76+
describe('database fallback', () => {
77+
it('should query the database when prefixQueryParam is not provided', async () => {
78+
const req = makeReq([{ prefix: 'db-prefix' }])
79+
80+
const result = await getFilePrefix({
81+
collection: makeCollection(),
82+
filename: 'logo.png',
83+
req,
84+
})
85+
86+
expect(result).toBe('db-prefix')
87+
expect(req.payload.find).toHaveBeenCalledOnce()
88+
})
89+
90+
it('should return empty string when no document is found', async () => {
91+
const req = makeReq([])
92+
93+
const result = await getFilePrefix({
94+
collection: makeCollection(),
95+
filename: 'logo.png',
96+
req,
97+
})
98+
99+
expect(result).toBe('')
100+
})
101+
102+
it('should prioritize clientUploadContext prefix over database query', async () => {
103+
const req = makeReq([{ prefix: 'db-prefix' }])
104+
105+
const result = await getFilePrefix({
106+
clientUploadContext: { prefix: 'context-prefix' },
107+
collection: makeCollection(),
108+
filename: 'logo.png',
109+
req,
110+
})
111+
112+
expect(result).toBe('context-prefix')
113+
expect(req.payload.find).not.toHaveBeenCalled()
114+
})
115+
})
116+
})

packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,21 @@ import type { CollectionConfig, PayloadRequest, UploadConfig } from 'payload'
44
* Normalizes a storage prefix to ensure only valid path segments are included.
55
*/
66
function sanitizePrefix(prefix: string): string {
7+
let decodedPrefix: string
8+
9+
try {
10+
decodedPrefix = decodeURIComponent(prefix)
11+
} catch {
12+
return ''
13+
}
14+
15+
// Reject multi-encoded values (e.g. `%252f`) by allowing only a single decode pass.
16+
if (/%[0-9a-f]{2}/i.test(decodedPrefix)) {
17+
return ''
18+
}
19+
720
return (
8-
prefix
21+
decodedPrefix
922
.replace(/\\/g, '/')
1023
.split('/')
1124
.filter((segment) => segment !== '..' && segment !== '.')
@@ -16,17 +29,36 @@ function sanitizePrefix(prefix: string): string {
1629
)
1730
}
1831

32+
/**
33+
* Resolves the file prefix from the highest-priority available source and
34+
* always returns a sanitized value.
35+
*
36+
* Resolution order:
37+
* 1. `prefixQueryParam`
38+
* 2. `clientUploadContext.prefix`
39+
* 3. Stored document `prefix` from the database
40+
*
41+
* Query / client input is decoded once; malformed and multi-encoded values are
42+
* rejected. Sanitization then normalizes slashes, removes `.` / `..` path
43+
* traversal segments, strips leading slashes, and removes control characters.
44+
*/
1945
export async function getFilePrefix({
2046
clientUploadContext,
2147
collection,
2248
filename,
49+
prefixQueryParam,
2350
req,
2451
}: {
2552
clientUploadContext?: unknown
2653
collection: CollectionConfig
2754
filename: string
55+
prefixQueryParam?: string
2856
req: PayloadRequest
2957
}): Promise<string> {
58+
if (typeof prefixQueryParam === 'string') {
59+
return sanitizePrefix(prefixQueryParam)
60+
}
61+
3062
// Prioritize from clientUploadContext if there is:
3163
if (
3264
clientUploadContext &&

0 commit comments

Comments
 (0)