fix: allow Convex base64 upload digests (#3395)

convex/skillPublishUploads.test.ts

@@ -2,10 +2,22 @@
 
 import { afterEach, describe, expect, it, vi } from "vitest";
 import {
+  attachSkillPublishUploadInternal,
   cleanupSkillPublishUploadInternal,
   consumeSkillPublishUploads,
 } from "./skillPublishUploads";
 
+type AttachHandler = {
+  _handler: (
+    ctx: unknown,
+    args: {
+      userId: "users:1";
+      uploadTicket: "skillPublishUploadTickets:1";
+      storageId: "storage:1";
+    },
+  ) => Promise<unknown>;
+};
+
 type CleanupHandler = {
   _handler: (
     ctx: unknown,
@@ -13,9 +25,15 @@ type CleanupHandler = {
   ) => Promise<unknown>;
 };
 
+const attachHandler = (attachSkillPublishUploadInternal as unknown as AttachHandler)._handler;
 const cleanupHandler = (cleanupSkillPublishUploadInternal as unknown as CleanupHandler)._handler;
+const HELLO_SHA256_HEX = "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824";
+const HELLO_SHA256_BASE64 = "LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ=";
 
-function makeCtx(ticket: Record<string, unknown> | null) {
+function makeCtx(
+  ticket: Record<string, unknown> | null,
+  metadata: Record<string, unknown> | null = null,
+) {
   return {
     db: {
       get: vi.fn(async () => ticket),
@@ -25,12 +43,33 @@ function makeCtx(ticket: Record<string, unknown> | null) {
       query: vi.fn(),
       replace: vi.fn(),
       insert: vi.fn(),
-      system: { get: vi.fn(), query: vi.fn() },
+      system: { get: vi.fn(async () => metadata), query: vi.fn() },
     },
     storage: { delete: vi.fn() },
   };
 }
 
+function makeAttachCtx(storageSha256: string) {
+  return makeCtx(
+    {
+      _id: "skillPublishUploadTickets:1",
+      userId: "users:1",
+      path: "SKILL.md",
+      size: 5,
+      sha256: HELLO_SHA256_HEX,
+      contentType: "text/markdown",
+      createdAt: 1_000,
+      expiresAt: 10_000,
+    },
+    {
+      _creationTime: 1_500,
+      size: 5,
+      sha256: storageSha256,
+      contentType: "text/markdown",
+    },
+  );
+}
+
 describe("skill publish upload tickets", () => {
   afterEach(() => {
     vi.restoreAllMocks();
@@ -95,6 +134,38 @@ describe("skill publish upload tickets", () => {
     expect(ctx.db.patch).not.toHaveBeenCalled();
   });
 
+  it.each([
+    ["base64", HELLO_SHA256_BASE64],
+    ["hex", HELLO_SHA256_HEX],
+  ])("attaches an upload when Convex reports its SHA-256 as %s", async (_format, sha256) => {
+    vi.spyOn(Date, "now").mockReturnValue(2_000);
+    const ctx = makeAttachCtx(sha256);
+
+    await attachHandler(ctx, {
+      userId: "users:1",
+      uploadTicket: "skillPublishUploadTickets:1",
+      storageId: "storage:1",
+    });
+
+    expect(ctx.db.patch).toHaveBeenCalledWith("skillPublishUploadTickets:1", {
+      storageId: "storage:1",
+    });
+  });
+
+  it("rejects an upload whose base64 SHA-256 does not match its ticket", async () => {
+    vi.spyOn(Date, "now").mockReturnValue(2_000);
+    const ctx = makeAttachCtx("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=");
+
+    await expect(
+      attachHandler(ctx, {
+        userId: "users:1",
+        uploadTicket: "skillPublishUploadTickets:1",
+        storageId: "storage:1",
+      }),
+    ).rejects.toThrow("does not match its skill upload ticket");
+    expect(ctx.db.patch).not.toHaveBeenCalled();
+  });
+
   it("deletes an abandoned staged file when its ticket expires", async () => {
     const ctx = makeCtx({
       _id: "skillPublishUploadTickets:1",

convex/skillPublishUploads.ts

@@ -22,6 +22,28 @@ function assertExpectedUpload(args: {
   if (!/^[a-f0-9]{64}$/i.test(args.sha256)) throw new Error("Invalid upload SHA-256");
 }
 
+function matchesStorageSha256(storageSha256: string, expectedHex: string) {
+  const normalized = storageSha256.trim();
+  if (/^[a-f0-9]{64}$/i.test(normalized)) {
+    return normalized.toLowerCase() === expectedHex;
+  }
+  if (!/^[A-Za-z0-9+/]{43}=$/.test(normalized)) return false;
+
+  // Convex storage metadata has returned both documented hex and base64 digests
+  // across runtimes. Compare the decoded bytes so either representation is safe.
+  try {
+    const decoded = atob(normalized);
+    if (decoded.length !== 32) return false;
+    for (let index = 0; index < decoded.length; index += 1) {
+      const expectedByte = Number.parseInt(expectedHex.slice(index * 2, index * 2 + 2), 16);
+      if (decoded.charCodeAt(index) !== expectedByte) return false;
+    }
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 export const createSkillPublishUploadInternal = internalMutation({
   args: {
     userId: v.id("users"),
@@ -102,7 +124,7 @@ export const attachSkillPublishUploadInternal = internalMutation({
       !metadata ||
       metadata._creationTime < ticket.createdAt ||
       metadata.size !== ticket.size ||
-      metadata.sha256.toLowerCase() !== ticket.sha256 ||
+      !matchesStorageSha256(metadata.sha256, ticket.sha256) ||
       normalizeContentType(metadata.contentType) !== ticket.contentType
     ) {
       throw new Error("Uploaded file does not match its skill upload ticket");

specs/security-moderation.md

@@ -273,6 +273,9 @@ See also: [acceptable-usage.md](./acceptable-usage.md) for the marketplace polic
 - The server creates a short-lived, user-bound ticket before accepting a file.
   The upload action enforces the declared path, byte size, content type, and
   SHA-256 before attaching the resulting storage id to that ticket.
+- Storage metadata SHA-256 values may be hex or base64 depending on the Convex
+  runtime. Attachment validation compares the decoded digest bytes while the
+  ticket and public publish contract remain canonical lowercase hex.
 - JSON skill publishes must present the matching ticket for every staged file.
   Ticket ownership and file metadata are revalidated, and ticket consumption is
   committed atomically with the new skill version.