Plugin cloud storage upload file even when record is invalid

[cortopy]

Link to reproduction

No response

Environment Info

Payload: Beta 107

Describe the Bug

When a file is uploaded to external storage using plugin-cloud-storage, a file is uploaded to the cloud storage even when it shouldn't.

As per this line, the plugin-cloud-storage will add a beforeChange hook so that a file is uploaded to external storage. This is problematic because validation has not run yet, and the upload will happen even if there are invalid fields.

When it happens, there may be unintended consequences: - If the user doesn't fix the error, the file will be orphan in storage, with no reference in payload. It will never be deleted - If storage doesn't allow re-writing the same file, new uploads will always fail.

Reproduction Steps

1. Add extra fields to the media file. For example alt that is required
2. Configure any of the adapters that rely on plugin-cloud-storage
3. Create a media file using the admin console, and leave alt blank. The UI will say that the input is invalid and needs fixing. However, the file will be uploaded already.

Adapters and Plugins

plugin-cloud-storage

[github-actions[bot]]

This issue has been marked as stale due to lack of activity.

To keep this issue open, please indicate that it is still relevant in a comment below.

[cortopy]

This is still happening

[alexisoney]

Hello!

I'm facing the same issue using payload-storage-bunny.

I tried upgrading to version 3.45.0, but this line still seems to be invoked without any guard.

@cortopy, have you found any workaround or solution since then? I’d be interested to know.

Thanks!

[damnsamn]

We're also encountering the same issue. This is the sequence of events as I've been able to diagnose it:

1. "Images" upload collection has a required field “alt”
2. User successfully creates an Image doc without validation issues
3. The user then updates the Image doc, with an attempt to clear the “alt” field.
   • beforeChange hook runs, which triggers handleDelete and then handleUpload .
   • Storage provider returns a new file ID (or equivalent), which we update on the document
4. After that has happened, validation runs and fails. “alt” is required! The new file ID is not saved, but the file referenced by the old file ID is now deleted
5. The user adds an “alt” value and saves again
6. staticHandler runs, using the file ID of the saved document, which is outdated and no longer exists. The storage provider returns an error, we return a 404
7. generateFileData throws a FileRetrievalError which prevents the document from saving
8. End result is that this document’s saved file information is now outdated (as the source file on the storage provider was deleted during the beforeChange hook). All instances of this image on the website now 404

A short-term fix for us is to have handleDelete do nothing, so we’re at least not losing files irrevocably - but this also results in every save to the doc uploading a duplicate to the storage provider

[damnsamn]

It would be great if the beforeChange hook were better able to determine when the file upload has been changed. Re-uploading the file every time the document changes feels wrong

[r1tsuu]

Hey guys, this was fixed here - https://github.com/payloadcms/payload/pull/13500 and will be out in the next release!

[github-actions[bot]]

This issue has been automatically locked. Please open a new issue if this issue persists with any additional detail.