diff --git a/convex/skillPublishUploads.test.ts b/convex/skillPublishUploads.test.ts index 403e7367..b7dce157 100644 --- a/convex/skillPublishUploads.test.ts +++ b/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; +}; + type CleanupHandler = { _handler: ( ctx: unknown, @@ -13,9 +25,15 @@ type CleanupHandler = { ) => Promise; }; +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 | null) { +function makeCtx( + ticket: Record | null, + metadata: Record | null = null, +) { return { db: { get: vi.fn(async () => ticket), @@ -25,12 +43,33 @@ function makeCtx(ticket: Record | 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", diff --git a/convex/skillPublishUploads.ts b/convex/skillPublishUploads.ts index 14b14a5a..e06aaeb7 100644 --- a/convex/skillPublishUploads.ts +++ b/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"); diff --git a/specs/security-moderation.md b/specs/security-moderation.md index 24e61cdf..eb8ef5a1 100644 --- a/specs/security-moderation.md +++ b/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.