Sign only the parameters Cloudinary verifies, unbreaking uploads

functions/src/__tests__/flows.test.ts

@@ -296,17 +296,26 @@ describe("cloudinary upload signature", () => {
     expect(res.folder).toBe(`petnote/users/${ALICE}`);
   });
 
-  it("signs exactly the parameters it returns", async () => {
+  it("signs exactly the three parameters Cloudinary verifies", async () => {
     // Recomputed independently here. If the handler ever signs a different set
-    // of params than it hands back, Cloudinary rejects every upload.
+    // of params than Cloudinary verifies, Cloudinary rejects every upload.
+    //
+    // That comment was already here and it was right — but the test recomputed
+    // the same wrong set the handler used, so it passed while every real
+    // upload failed. max_file_size was in both sides of the comparison and in
+    // neither side of Cloudinary's. A test that mirrors the implementation
+    // cannot catch the implementation being wrong about a third party.
+    //
+    // The list below is now the contract with Cloudinary, not a copy of what
+    // the handler happens to do: folder, timestamp, upload_preset. Do not add
+    // a parameter here without confirming Cloudinary signs it.
     const res = await callAs<{
       signature: string; timestamp: number; uploadPreset: string;
       folder: string; maxFileSize: number; cloudName: string; apiKey: string;
     }>(getCloudinaryUploadSignature, ALICE, { resourceType: "image" });
 
     const toSign = [
       `folder=${res.folder}`,
-      `max_file_size=${res.maxFileSize}`,
       `timestamp=${res.timestamp}`,
       `upload_preset=${res.uploadPreset}`,
     ].join("&");
@@ -319,9 +328,11 @@ describe("cloudinary upload signature", () => {
     expect(res.apiKey).toBe(process.env.CLOUDINARY_API_KEY);
   });
 
-  it("binds the size limit into the signature, per resource type", async () => {
-    // The limit is signed so a leaked signature cannot be replayed to upload
-    // something larger than allowed.
+  it("returns the advisory size limit and preset, per resource type", async () => {
+    // These are returned so the client can reject an oversized file before
+    // starting a doomed upload. They are NOT enforced by the signature — that
+    // was the belief that broke uploads. The enforceable ceiling, if there is
+    // one, is configured on the upload preset in the Cloudinary console.
     const image = await callAs<{ maxFileSize: number; uploadPreset: string }>(
       getCloudinaryUploadSignature, ALICE, { resourceType: "image" }
     );

functions/src/media.ts

@@ -21,9 +21,23 @@ function signCloudinaryParams(params: Record<string, string>, apiSecret: string)
     .digest("hex");
 }
 
-// Enforced on the signature itself so a leaked signature can't be reused to
-// upload a bigger file than we allow. Matches the limits surfaced to the
-// client so size checks stay in sync.
+// Advisory only. These feed the size check in src/services/cloudinary.ts so a
+// user gets a clear error before a doomed upload starts, and they are returned
+// to the client for exactly that.
+//
+// They are NOT a server-side ceiling. The comment that used to sit here said
+// they were "enforced on the signature itself", and that claim is what led to
+// max_file_size being added to the signed parameter set — which broke every
+// upload. Cloudinary signs only the upload parameters it recognises and
+// silently drops the rest, so signing a fourth parameter it never verifies
+// produced a signature that could not match. Its own error proved it: the
+// String to sign Cloudinary echoed back listed folder, timestamp and
+// upload_preset, and nothing else.
+//
+// The real server-side ceiling is whatever the petnote_image_signed and
+// petnote_video_signed upload presets have configured in the Cloudinary
+// console. If a hard limit matters, it has to be set there — it cannot be
+// enforced from this file.
 const CLOUDINARY_MAX_IMAGE_BYTES = 10 * 1024 * 1024; // 10 MB
 const CLOUDINARY_MAX_VIDEO_BYTES = 80 * 1024 * 1024; // 80 MB
 
@@ -76,9 +90,10 @@ export const getCloudinaryUploadSignature = onCall(
     const timestamp = Math.floor(Date.now() / 1000);
     const folder = userFolder(callerUid);
     const signature = signCloudinaryParams(
+      // Only parameters Cloudinary actually verifies. Adding one it ignores
+      // makes the signature unmatchable — see the note above.
       {
         folder,
-        max_file_size: String(maxFileSize),
         timestamp: String(timestamp),
         upload_preset: uploadPreset,
       },

src/services/cloudinary.ts

@@ -69,9 +69,10 @@ async function uploadToCloudinary(
   formData.append("signature", data.signature);
   formData.append("upload_preset", data.uploadPreset);
   formData.append("folder", data.folder);
-  if (data.maxFileSize) {
-    formData.append("max_file_size", String(data.maxFileSize));
-  }
+  // No max_file_size here. Cloudinary ignores it on upload, so sending it
+  // achieved nothing except tempting the server to sign it — which is what
+  // broke uploads. The size check above is where the limit is applied, and it
+  // is advisory; the enforceable ceiling lives in the upload preset.
 
   // 90s covers a max-size video on a sluggish uplink while still killing
   // pathological hangs that used to leave the upload spinner stuck