diff --git a/packages/payload/src/uploads/checkFileAccess.spec.ts b/packages/payload/src/uploads/checkFileAccess.spec.ts new file mode 100644 index 00000000000..c7cad1fe00b --- /dev/null +++ b/packages/payload/src/uploads/checkFileAccess.spec.ts @@ -0,0 +1,99 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { Collection } from '../collections/config/types.js' +import type { PayloadRequest } from '../types/index.js' + +vi.mock('../auth/executeAccess.js', () => ({ + executeAccess: vi.fn(), +})) + +import { executeAccess } from '../auth/executeAccess.js' + +import { checkFileAccess } from './checkFileAccess.js' + +const makeFindOne = (result: unknown = { id: '1', filename: 'logo.png' }) => + vi.fn().mockResolvedValue(result) + +const makeCollection = (): Collection => + ({ + config: { + slug: 'test-media', + access: { read: vi.fn() }, + upload: {}, + }, + }) as unknown as Collection + +const makeReq = (findOne: ReturnType): PayloadRequest => + ({ + t: vi.fn(), + payload: { + db: { findOne }, + }, + }) as unknown as PayloadRequest + +describe('checkFileAccess', () => { + beforeEach(() => { + vi.mocked(executeAccess).mockResolvedValue({}) + }) + + describe('prefix filtering', () => { + it('should add prefix clause to where query when prefix is provided', async () => { + const findOne = makeFindOne() + const req = makeReq(findOne) + const collection = makeCollection() + + await checkFileAccess({ collection, filename: 'logo.png', prefix: 'abc123', req }) + + const whereArg = findOne.mock.calls[0]?.[0]?.where + expect(whereArg?.and).toEqual(expect.arrayContaining([{ prefix: { equals: 'abc123' } }])) + }) + + it('should not add prefix clause to where query when prefix is omitted', async () => { + const findOne = makeFindOne() + const req = makeReq(findOne) + const collection = makeCollection() + + await checkFileAccess({ collection, filename: 'logo.png', req }) + + const whereArg = findOne.mock.calls[0]?.[0]?.where + const hasPrefixCondition = whereArg?.and?.some( + (clause: Record) => 'prefix' in clause, + ) + expect(hasPrefixCondition).toBeFalsy() + }) + + it('should still include filename in where query when prefix is provided', async () => { + const findOne = makeFindOne() + const req = makeReq(findOne) + const collection = makeCollection() + + await checkFileAccess({ collection, filename: 'logo.png', prefix: 'abc123', req }) + + const whereArg = findOne.mock.calls[0]?.[0]?.where + const filenameCondition = whereArg?.and?.[0] + expect(filenameCondition?.or).toEqual( + expect.arrayContaining([{ filename: { equals: 'logo.png' } }]), + ) + }) + + it('should throw when no doc matches the given prefix', async () => { + const findOne = makeFindOne(null) + const req = makeReq(findOne) + const collection = makeCollection() + + await expect( + checkFileAccess({ collection, filename: 'logo.png', prefix: 'nonexistent', req }), + ).rejects.toThrow() + }) + + it('should throw when filename contains path traversal sequence', async () => { + const findOne = makeFindOne() + const req = makeReq(findOne) + const collection = makeCollection() + + await expect( + checkFileAccess({ collection, filename: '../etc/passwd', req }), + ).rejects.toThrow() + }) + }) +}) diff --git a/packages/payload/src/uploads/checkFileAccess.ts b/packages/payload/src/uploads/checkFileAccess.ts index 3516ba8c8b9..61b7bfe7281 100644 --- a/packages/payload/src/uploads/checkFileAccess.ts +++ b/packages/payload/src/uploads/checkFileAccess.ts @@ -7,10 +7,12 @@ import { Forbidden } from '../errors/Forbidden.js' export const checkFileAccess = async ({ collection, filename, + prefix, req, }: { collection: Collection filename: string + prefix?: string req: PayloadRequest }): Promise => { if (filename.includes('../') || filename.includes('..\\')) { @@ -49,6 +51,12 @@ export const checkFileAccess = async ({ }) } + if (typeof prefix === 'string') { + queryToBuild.and!.push({ + prefix: { equals: prefix }, + }) + } + const doc = await req.payload.db.findOne({ collection: config.slug, req, diff --git a/packages/payload/src/uploads/endpoints/getFile.ts b/packages/payload/src/uploads/endpoints/getFile.ts index 2d8eef829e8..fdcf9bf1410 100644 --- a/packages/payload/src/uploads/endpoints/getFile.ts +++ b/packages/payload/src/uploads/endpoints/getFile.ts @@ -19,6 +19,7 @@ export const getFileHandler: PayloadHandler = async (req) => { const collection = getRequestCollection(req) const filename = req.routeParams?.filename as string + const prefix = req.searchParams?.get('prefix') ?? undefined if (!collection.config.upload) { throw new APIError( @@ -30,6 +31,7 @@ export const getFileHandler: PayloadHandler = async (req) => { const accessResult = (await checkFileAccess({ collection, filename, + prefix, req, }))! @@ -48,6 +50,7 @@ export const getFileHandler: PayloadHandler = async (req) => { params: { collection: collection.config.slug, filename, + prefix, }, }) if (customResponse && customResponse instanceof Response) { diff --git a/packages/payload/src/uploads/types.ts b/packages/payload/src/uploads/types.ts index fb8bea0ea06..79349c01200 100644 --- a/packages/payload/src/uploads/types.ts +++ b/packages/payload/src/uploads/types.ts @@ -245,7 +245,12 @@ export type UploadConfig = { args: { doc: TypeWithID headers?: Headers - params: { clientUploadContext?: unknown; collection: string; filename: string } + params: { + clientUploadContext?: unknown + collection: string + filename: string + prefix?: string + } }, ) => Promise | Promise | Response | void)[] /** diff --git a/packages/plugin-cloud-storage/src/hooks/afterRead.ts b/packages/plugin-cloud-storage/src/hooks/afterRead.ts index 823c5951706..6866efaebfc 100644 --- a/packages/plugin-cloud-storage/src/hooks/afterRead.ts +++ b/packages/plugin-cloud-storage/src/hooks/afterRead.ts @@ -32,6 +32,9 @@ export const getAfterReadHook = filename, prefix, }) + } else if (url && prefix) { + const separator = url.includes('?') ? '&' : '?' + url = `${url}${separator}prefix=${encodeURIComponent(prefix)}` } } diff --git a/packages/plugin-cloud-storage/src/types.ts b/packages/plugin-cloud-storage/src/types.ts index 9c36c3f4f4a..a8f1bd75a7b 100644 --- a/packages/plugin-cloud-storage/src/types.ts +++ b/packages/plugin-cloud-storage/src/types.ts @@ -63,7 +63,7 @@ export type StaticHandler = ( args: { doc?: TypeWithID headers?: Headers - params: { clientUploadContext?: unknown; collection: string; filename: string } + params: { clientUploadContext?: unknown; collection: string; filename: string; prefix?: string } }, ) => Promise | Response diff --git a/packages/plugin-cloud-storage/src/utilities/getFilePrefix.spec.ts b/packages/plugin-cloud-storage/src/utilities/getFilePrefix.spec.ts new file mode 100644 index 00000000000..00f880b74a6 --- /dev/null +++ b/packages/plugin-cloud-storage/src/utilities/getFilePrefix.spec.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, vi } from 'vitest' + +import type { CollectionConfig, PayloadRequest } from 'payload' + +import { getFilePrefix } from './getFilePrefix.js' + +const makeReq = (docs: unknown[] = []) => + ({ + payload: { + find: vi.fn().mockResolvedValue({ docs }), + }, + }) as unknown as PayloadRequest + +const makeCollection = (): CollectionConfig => + ({ slug: 'media', upload: {} }) as unknown as CollectionConfig + +describe('getFilePrefix', () => { + describe('explicitPrefix shortcut', () => { + it('should return explicitPrefix immediately without querying the database', async () => { + const req = makeReq() + + const result = await getFilePrefix({ + collection: makeCollection(), + explicitPrefix: 'uuid-prefix', + filename: 'logo.png', + req, + }) + + expect(result).toBe('uuid-prefix') + expect(req.payload.find).not.toHaveBeenCalled() + }) + + it('should return an empty string when explicitPrefix is an empty string', async () => { + const req = makeReq() + + const result = await getFilePrefix({ + collection: makeCollection(), + explicitPrefix: '', + filename: 'logo.png', + req, + }) + + expect(result).toBe('') + expect(req.payload.find).not.toHaveBeenCalled() + }) + }) + + describe('database fallback', () => { + it('should query the database when explicitPrefix is not provided', async () => { + const req = makeReq([{ prefix: 'db-prefix' }]) + + const result = await getFilePrefix({ + collection: makeCollection(), + filename: 'logo.png', + req, + }) + + expect(result).toBe('db-prefix') + expect(req.payload.find).toHaveBeenCalledOnce() + }) + + it('should return empty string when no document is found', async () => { + const req = makeReq([]) + + const result = await getFilePrefix({ + collection: makeCollection(), + filename: 'logo.png', + req, + }) + + expect(result).toBe('') + }) + + it('should prioritize clientUploadContext prefix over database query', async () => { + const req = makeReq([{ prefix: 'db-prefix' }]) + + const result = await getFilePrefix({ + clientUploadContext: { prefix: 'context-prefix' }, + collection: makeCollection(), + filename: 'logo.png', + req, + }) + + expect(result).toBe('context-prefix') + expect(req.payload.find).not.toHaveBeenCalled() + }) + }) +}) diff --git a/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts b/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts index e823a6d8614..4c1d7085ef0 100644 --- a/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts +++ b/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts @@ -3,14 +3,20 @@ import type { CollectionConfig, PayloadRequest, UploadConfig } from 'payload' export async function getFilePrefix({ clientUploadContext, collection, + explicitPrefix, filename, req, }: { clientUploadContext?: unknown collection: CollectionConfig + explicitPrefix?: string filename: string req: PayloadRequest }): Promise { + if (typeof explicitPrefix === 'string') { + return explicitPrefix + } + // Prioritize from clientUploadContext if there is: if ( clientUploadContext && diff --git a/packages/storage-s3/src/staticHandler.ts b/packages/storage-s3/src/staticHandler.ts index 7597a1aa46f..1ca57d7d251 100644 --- a/packages/storage-s3/src/staticHandler.ts +++ b/packages/storage-s3/src/staticHandler.ts @@ -62,7 +62,10 @@ export const getHandler = ({ getStorageClient, signedDownloads, }: Args): StaticHandler => { - return async (req, { headers: incomingHeaders, params: { clientUploadContext, filename } }) => { + return async ( + req, + { headers: incomingHeaders, params: { clientUploadContext, filename, prefix: explicitPrefix } }, + ) => { let object: AWS.GetObjectOutput | undefined = undefined let streamed = false @@ -74,7 +77,13 @@ export const getHandler = ({ } try { - const prefix = await getFilePrefix({ clientUploadContext, collection, filename, req }) + const prefix = await getFilePrefix({ + clientUploadContext, + collection, + explicitPrefix, + filename, + req, + }) const key = path.posix.join(prefix, filename) diff --git a/packages/ui/src/elements/EditUpload/index.tsx b/packages/ui/src/elements/EditUpload/index.tsx index 73197e15e3b..b2fe06ecda5 100644 --- a/packages/ui/src/elements/EditUpload/index.tsx +++ b/packages/ui/src/elements/EditUpload/index.tsx @@ -169,7 +169,8 @@ export const EditUpload: React.FC = ({ setFocalPosition({ x: xCenter, y: yCenter }) } - const fileSrcToUse = imageCacheTag ? `${fileSrc}?${encodeURIComponent(imageCacheTag)}` : fileSrc + const queryChar = fileSrc?.includes('?') ? '&' : '?' + const fileSrcToUse = imageCacheTag ? `${fileSrc}${queryChar}${encodeURIComponent(imageCacheTag)}` : fileSrc return (
diff --git a/packages/ui/src/elements/PreviewSizes/index.tsx b/packages/ui/src/elements/PreviewSizes/index.tsx index f75889f00ef..226ef49d9fb 100644 --- a/packages/ui/src/elements/PreviewSizes/index.tsx +++ b/packages/ui/src/elements/PreviewSizes/index.tsx @@ -99,7 +99,8 @@ export const PreviewSizes: React.FC = ({ doc, imageCacheTag, return null } if (doc.url) { - return `${doc.url}${imageCacheTag ? `?${encodeURIComponent(imageCacheTag)}` : ''}` + const queryChar = doc.url.includes('?') ? '&' : '?' + return `${doc.url}${imageCacheTag ? `${queryChar}${encodeURIComponent(imageCacheTag)}` : ''}` } } useEffect(() => { diff --git a/test/uploads/config.ts b/test/uploads/config.ts index c81740352bc..c3a1c75d5a8 100644 --- a/test/uploads/config.ts +++ b/test/uploads/config.ts @@ -36,6 +36,7 @@ import { noRestrictFileMimeTypesSlug, noRestrictFileTypesSlug, pdfOnlySlug, + prefixMediaSlug, reduceSlug, relationPreviewSlug, relationSlug, @@ -1061,6 +1062,18 @@ export default buildConfigWithDefaults({ ], }, }, + { + slug: prefixMediaSlug, + fields: [ + { + name: 'prefix', + type: 'text', + }, + ], + upload: { + staticDir: path.resolve(dirname, './prefix-media'), + }, + }, ], onInit: async (payload) => { if (process.env.SEED_IN_CONFIG_ONINIT !== 'false') { diff --git a/test/uploads/int.spec.ts b/test/uploads/int.spec.ts index 7d2fe206b13..e998c03a25b 100644 --- a/test/uploads/int.spec.ts +++ b/test/uploads/int.spec.ts @@ -8,7 +8,7 @@ import path from 'path' import { _internal_safeFetchGlobal, createPayloadRequest, getFileByPath } from 'payload' import { fileURLToPath } from 'url' import { promisify } from 'util' -import { afterAll, beforeAll, describe, expect, it, vitest } from 'vitest' +import { afterAll, afterEach, beforeAll, describe, expect, it, vitest } from 'vitest' import type { NextRESTClient } from '../__helpers/shared/NextRESTClient.js' import type { Enlarge, Media } from './payload-types.js' @@ -30,6 +30,7 @@ import { noRestrictFileMimeTypesSlug, noRestrictFileTypesSlug, pdfOnlySlug, + prefixMediaSlug, reduceSlug, relationSlug, restrictedMimeTypesSlug, @@ -1901,6 +1902,95 @@ describe('Collections - Uploads', () => { handler.cleanup() }) }) + + describe('prefix query parameter', () => { + const docIDs: (number | string)[] = [] + + afterEach(async () => { + for (const id of docIDs) { + try { + await payload.delete({ collection: prefixMediaSlug, id }) + } catch { + // noop — file may already have been deleted + } + } + docIDs.length = 0 + }) + + it('should return 200 when the prefix query param matches the stored document prefix', async () => { + const filePath = path.resolve(dirname, './image.png') + const file = await getFileByPath(filePath) + + const doc = await payload.create({ + collection: prefixMediaSlug, + data: { prefix: 'abc123' }, + file, + }) + + docIDs.push(doc.id) + + const response = await restClient.GET( + `/${prefixMediaSlug}/file/${doc.filename}?prefix=abc123`, + ) + + expect(response.status).toBe(200) + }) + + it('should return 403 when the prefix query param does not match the stored document prefix', async () => { + const filePath = path.resolve(dirname, './image.png') + const file = await getFileByPath(filePath) + + const doc = await payload.create({ + collection: prefixMediaSlug, + data: { prefix: 'abc123' }, + file, + }) + + docIDs.push(doc.id) + + const response = await restClient.GET( + `/${prefixMediaSlug}/file/${doc.filename}?prefix=wrongprefix`, + ) + + expect(response.status).toBe(403) + }) + + it('should return 200 without prefix param for documents that have no prefix (backward compatibility)', async () => { + const filePath = path.resolve(dirname, './image.png') + const file = await getFileByPath(filePath) + + const doc = await payload.create({ + collection: prefixMediaSlug, + data: {}, + file, + }) + + docIDs.push(doc.id) + + const response = await restClient.GET(`/${prefixMediaSlug}/file/${doc.filename}`) + + expect(response.status).toBe(200) + }) + + it('should return 403 when prefix param is provided but no document has a matching prefix', async () => { + const filePath = path.resolve(dirname, './image.png') + const file = await getFileByPath(filePath) + + const doc = await payload.create({ + collection: prefixMediaSlug, + data: {}, + file, + }) + + docIDs.push(doc.id) + + const response = await restClient.GET( + `/${prefixMediaSlug}/file/${doc.filename}?prefix=nonexistent`, + ) + + expect(response.status).toBe(403) + }) + }) }) async function fileExists(fileName: string): Promise { diff --git a/test/uploads/shared.ts b/test/uploads/shared.ts index d5d666629d5..1fce3e5fffb 100644 --- a/test/uploads/shared.ts +++ b/test/uploads/shared.ts @@ -47,3 +47,4 @@ export const mediaWithImageSizeAdminPropsSlug = 'media-with-image-size-admin-pro export const uploads2Slug = 'uploads-2' export const noFilesRequiredSlug = 'no-files-required' export const relationToNoFilesRequiredSlug = 'relation-to-no-files-required' +export const prefixMediaSlug = 'prefix-media'