Skip to content

Commit f02d22a

Browse files
authored
fix: allow access to files reuploaded on a draft (#17209)
#### Problem On an upload collection with `versions.drafts` enabled, reuploading a file and saving as a **draft** (without publishing) makes the new file return `403 Forbidden` from `GET /api/<collection>/file/<filename>?prefix=<prefix>`. Publishing the document clears the error. The file itself is uploaded to storage correctly, only the access check fails. **Regression window:** surfaced in `3.85.2` #### Cause `checkFileAccess` resolves the file's document with `req.payload.db.findOne(...)`, which only reads the **base collection row**. When a file is reuploaded on a draft, the new filename is written to the latest **version** only. The base row still points at the previously published filename. The lookup therefore misses the draft filename and access is wrongly denied. #### Solution - Fallback to the latest draft via `db.queryDrafts` before throwing `Forbidden`. - Added an integration test in `test/uploads` (file access on draft reupload) - Publish a file, reupload a different file saved as a draft, then request the new file with the `prefix` query param. Expects 200 (was 403). - A filename matching no document still returns 403
1 parent c7dc68e commit f02d22a

5 files changed

Lines changed: 258 additions & 97 deletions

File tree

packages/payload/src/uploads/checkFileAccess.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type { PayloadRequest, Where } from '../types/index.js'
33

44
import { executeAccess } from '../auth/executeAccess.js'
55
import { Forbidden } from '../errors/Forbidden.js'
6+
import { hasDraftsEnabled } from '../utilities/getVersionsConfig.js'
7+
import { appendVersionToQueryKey } from '../versions/drafts/appendVersionToQueryKey.js'
68

79
export const checkFileAccess = async ({
810
collection,
@@ -48,16 +50,38 @@ export const checkFileAccess = async ({
4850
})
4951
}
5052

53+
const where: Where = { and: [filenameCondition, ...constraints] }
54+
5155
const doc = await req.payload.db.findOne({
5256
collection: config.slug,
5357
req,
54-
where: { and: [filenameCondition, ...constraints] },
58+
where,
5559
})
5660

57-
if (!doc) {
58-
throw new Forbidden(req.t)
61+
if (doc) {
62+
return doc
63+
}
64+
65+
// Queries the version collection if `hasDraftsEnabled` is true:
66+
//
67+
// The base collection row only reflects the most recently committed
68+
// (published) state. When a file is reuploaded on a draft, the new
69+
// filename is written to the latest *version*, while the base row still
70+
// points at the previously committed filename. The findOne above therefore
71+
// misses the draft-only filename and access is wrongly denied.
72+
if (hasDraftsEnabled(config)) {
73+
const { docs } = await req.payload.db.queryDrafts({
74+
collection: config.slug,
75+
limit: 1,
76+
req,
77+
where: appendVersionToQueryKey(where),
78+
})
79+
80+
if (docs[0]) {
81+
return docs[0]
82+
}
5983
}
6084

61-
return doc
85+
throw new Forbidden(req.t)
6286
}
6387
}

test/uploads/config.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
audioSlug,
2424
constructorOptionsSlug,
2525
customFileNameMediaSlug,
26+
draftReuploadMediaSlug,
2627
enlargeSlug,
2728
focalNoSizesSlug,
2829
hideFileInputOnCreateSlug,
@@ -1082,6 +1083,27 @@ export default buildConfigWithDefaults({
10821083
staticDir: path.resolve(dirname, './prefix-media'),
10831084
},
10841085
},
1086+
{
1087+
// Drafts + a `prefix` field reproduce the reupload-on-draft access bug:
1088+
// the file endpoint's access check queries the base row (published state),
1089+
// which misses a filename that only exists on the latest draft version.
1090+
slug: draftReuploadMediaSlug,
1091+
access: {
1092+
read: () => true,
1093+
},
1094+
fields: [
1095+
{
1096+
name: 'prefix',
1097+
type: 'text',
1098+
},
1099+
],
1100+
upload: {
1101+
staticDir: path.resolve(dirname, `./${draftReuploadMediaSlug}`),
1102+
},
1103+
versions: {
1104+
drafts: true,
1105+
},
1106+
},
10851107
],
10861108
onInit: async (payload) => {
10871109
if (process.env.SEED_IN_CONFIG_ONINIT !== 'false') {

test/uploads/int.spec.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
adminThumbnailSizeSlug,
2727
allowListMediaSlug,
2828
anyImagesSlug,
29+
draftReuploadMediaSlug,
2930
enlargeSlug,
3031
focalNoSizesSlug,
3132
focalOnlySlug,
@@ -64,6 +65,74 @@ describe('Collections - Uploads', () => {
6465
await payload.destroy()
6566
})
6667

68+
describe('file access on draft reupload', () => {
69+
const createdIDs: (number | string)[] = []
70+
const filePrefix = 'test'
71+
const staticDir = path.resolve(dirname, `./${draftReuploadMediaSlug}`)
72+
73+
afterEach(async () => {
74+
for (const id of createdIDs) {
75+
await payload.delete({ collection: draftReuploadMediaSlug as CollectionSlug, id })
76+
}
77+
createdIDs.length = 0
78+
79+
// Deleting the document only removes the base row's file; the draft
80+
// version's reuploaded file is orphaned on disk. Clear the whole static
81+
// dir so filenames don't collide (and get suffixed) across runs.
82+
fs.rmSync(staticDir, { force: true, recursive: true })
83+
})
84+
85+
it('should serve a file reuploaded on a draft over a published document', async () => {
86+
const published = await payload.create({
87+
collection: draftReuploadMediaSlug as CollectionSlug,
88+
data: { prefix: filePrefix },
89+
draft: false,
90+
file: await getFileByPath(path.resolve(dirname, './image.png')),
91+
})
92+
createdIDs.push(published.id)
93+
94+
// Reupload a different file and save as a draft (do not publish). The new
95+
// filename is written to the latest version only; the base row still
96+
// points at the published file.
97+
const draft = await payload.update({
98+
collection: draftReuploadMediaSlug as CollectionSlug,
99+
id: published.id,
100+
data: { prefix: filePrefix },
101+
draft: true,
102+
file: await getFileByPath(path.resolve(dirname, './test-image.png')),
103+
})
104+
105+
expect(draft.filename).not.toBe(published.filename)
106+
107+
// The prefix query param (as cloud-storage adapters generate) forces the
108+
// access check's constrained lookup — the base-row miss this fix repairs
109+
// by falling back to the latest draft version.
110+
const draftFileResponse = await restClient.GET(
111+
`/${draftReuploadMediaSlug}/file/${draft.filename}`,
112+
{ query: { prefix: filePrefix } },
113+
)
114+
115+
expect(draftFileResponse.status).toBe(200)
116+
expect(draftFileResponse.headers.get('content-type')).toContain('image/png')
117+
})
118+
119+
it('should still deny access to a filename that matches no document', async () => {
120+
const doc = await payload.create({
121+
collection: draftReuploadMediaSlug as CollectionSlug,
122+
data: { prefix: filePrefix },
123+
draft: true,
124+
file: await getFileByPath(path.resolve(dirname, './image.png')),
125+
})
126+
createdIDs.push(doc.id)
127+
128+
const response = await restClient.GET(`/${draftReuploadMediaSlug}/file/does-not-exist.png`, {
129+
query: { prefix: filePrefix },
130+
})
131+
132+
expect(response.status).toBe(403)
133+
})
134+
})
135+
67136
describe('REST API', () => {
68137
describe('create', () => {
69138
it('creates from form data given a png', async () => {

0 commit comments

Comments
 (0)