Skip to content

Sign only the parameters Cloudinary verifies, unbreaking uploads - #187

Merged
renrenmimi merged 1 commit into
mainfrom
fix/cloudinary-signature-max-file-size
Sep 6, 2026
Merged

Sign only the parameters Cloudinary verifies, unbreaking uploads#187
renrenmimi merged 1 commit into
mainfrom
fix/cloudinary-signature-max-file-size

Conversation

@renrenmimi

Copy link
Copy Markdown
Owner

Photo upload has been failing with Invalid Signature. The server signed four parameters; Cloudinary verifies three.

Cloudinary signs only the upload parameters it recognises and silently drops the rest. max_file_size is not one of them, so including it produced a signature that could never match. Its own error said so — the String to sign it echoed back listed folder, timestamp, upload_preset, and nothing else.

Not a regression from #185. That PR never touched media.ts or the client upload path. This dates from whenever max_file_size entered the signature and has been broken ever since; nobody noticed because nobody had uploaded a photo through the UI in between.

The comment is what caused this

// Enforced on the signature itself so a leaked signature can't be reused to
// upload a bigger file than we allow.

A reasonable-sounding guarantee that Cloudinary does not offer. Someone read it and wired the limit into the signature. It's replaced with what's true: the limits are advisory, they feed the client-side check, and the only enforceable ceiling is whatever the upload presets have configured in the Cloudinary console.

The client stops sending max_file_size too — Cloudinary ignores it, so it bought nothing but the appearance that the limit went somewhere meaningful. The size check that gives a useful error before a doomed upload stays, and the callable still returns maxFileSize to feed it.

The test that should have caught this, and why it didn't

"signs exactly the parameters it returns" already carried this comment:

If the handler ever signs a different set of params than it hands back, Cloudinary rejects every upload.

It was right. It also passed — because it recomputed the same wrong set the handler used. max_file_size was on both sides of that comparison and on neither side of Cloudinary's. A test that mirrors the implementation cannot catch the implementation being wrong about a third party.

Its list is now written as the contract with Cloudinary rather than a copy of what the handler does, with a note not to add a parameter without confirming Cloudinary signs it. The sibling test that claimed the limit was "bound into the signature" is retitled and its comment corrected.

Two things I could not verify

1. Whether the upload presets carry a max file size. I cannot read the Cloudinary console from the repo. If petnote_image_signed / petnote_video_signed have no limit configured, then after this change there is no server-side size ceiling at all — only the advisory client check, which a crafted request skips. Worth checking before launch; if it's unset, that's a separate PR to set it.

2. That uploads actually work again. Local lint and build pass for both trees, and CI runs the emulator suite here — but none of that touches Cloudinary. This is precisely the class of bug CI cannot catch, which is the whole point of this PR. A real upload has to happen after deploy before this is called done.

🤖 Generated with Claude Code

Photo upload has been failing with Invalid Signature. The server signed four
parameters; Cloudinary verifies three.

Cloudinary signs only the upload parameters it recognises and silently drops
the rest. max_file_size is not one it recognises, so including it in the signed
set produced a signature that could never match. Its own error said so — the
String to sign it echoed back listed folder, timestamp and upload_preset, and
nothing else.

This is not a regression from #185, which never touched media.ts or the client
upload path. It dates from whenever max_file_size entered the signature and has
been broken ever since; nobody noticed because nobody had uploaded a photo
through the UI in between.

The comment above the limits is what caused this. It claimed they were
"enforced on the signature itself so a leaked signature can't be reused to
upload a bigger file than we allow" — a reasonable-sounding guarantee that
Cloudinary does not offer. Replaced with what is actually true: the limits are
advisory, they exist to feed the client-side check, and the only enforceable
ceiling is whatever the petnote_image_signed / petnote_video_signed upload
presets have configured in the Cloudinary console.

The client stops sending max_file_size too. Cloudinary ignores it, so it bought
nothing except the appearance that the limit was being transmitted somewhere
meaningful. The size check that produces a useful error before a doomed upload
stays, and the callable still returns maxFileSize to feed it.

Two tests encoded the same false premise and are corrected. One of them —
"signs exactly the parameters it returns" — already carried the comment "if the
handler ever signs a different set of params than it hands back, Cloudinary
rejects every upload". It was right, and it still passed, because it recomputed
the same wrong set the handler used. max_file_size was on both sides of that
comparison and on neither side of Cloudinary's. Its list is now written as the
contract with Cloudinary rather than a mirror of the implementation.

NOT VERIFIED HERE: whether the upload presets actually carry a max file size in
the Cloudinary console. I cannot read that from the repo. If no limit is set
there, then after this change there is no server-side size ceiling at all —
only the advisory client check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 6, 2026 06:22
@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
pet-note Ready Ready Preview Sep 6, 2026 6:22am UTC

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@renrenmimi
renrenmimi merged commit 3ffe5dc into main Sep 6, 2026
6 checks passed
@renrenmimi
renrenmimi deleted the fix/cloudinary-signature-max-file-size branch September 6, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants