From 306ed44ffa88e5c1b09d11b0a71572cb30a8bc17 Mon Sep 17 00:00:00 2001 From: FahadKhawaja Date: Thu, 22 Oct 2026 15:05:10 +0500 Subject: [PATCH 1/2] Report local share validation failures as preflight errors --- packages/review/src/sharing/client.ts | 12 +++- packages/review/src/sharing/publish.test.ts | 69 +++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/review/src/sharing/client.ts b/packages/review/src/sharing/client.ts index 3a7145710..20545b4bb 100644 --- a/packages/review/src/sharing/client.ts +++ b/packages/review/src/sharing/client.ts @@ -183,7 +183,17 @@ export class ShareClient { requestId: string = randomUUID(), beforeUpload?: () => Promise, ) { - validateShareBundle(bundle); + try { + validateShareBundle(bundle); + } catch (cause) { + throw new SharePreflightError( + new Error( + "Whiteboard could not prepare this review for sharing. Please report this issue.", + { cause }, + ), + ); + } + const manifestBytes = Buffer.from(JSON.stringify(bundle.manifest)); const created = createdSchema.parse( diff --git a/packages/review/src/sharing/publish.test.ts b/packages/review/src/sharing/publish.test.ts index 4e36579a6..dd1826eed 100644 --- a/packages/review/src/sharing/publish.test.ts +++ b/packages/review/src/sharing/publish.test.ts @@ -201,3 +201,72 @@ it("signs the user out when the share service rejects the stored token", async ( await rm(root, { recursive: true, force: true }); } }); + +it("reports incompatible review data without uploading or clearing the login", async () => { + const root = await mkdtemp(path.join(tmpdir(), "sharing-publish-")); + const fixture = await createShareFixture(root); + vi.stubEnv("DEV_REVIEW_HOME", root); + const fetch = vi.fn(); + const published = vi.fn<() => void>(); + const read = fixture.store.read.bind(fixture.store); + + // A saved review can contain metadata the portable share format cannot read. + vi.spyOn(fixture.store, "read").mockImplementation((...args) => { + const snapshot = read(...args); + snapshot.origin = { ...snapshot.origin }; + Object.assign(snapshot.origin, { unsupportedMetadata: "fixture" }); + + return snapshot; + }); + + try { + const account = { + origin: "https://app.dev.fast", + token: "fixture", + login: "fixture", + savedAt: new Date().toISOString(), + }; + + await writeStoreAuth(account); + const api = new Hono(); + api.onError((error, context) => + context.json( + { error: error.message }, + error instanceof ReviewInputError ? error.status : 500, + ), + ); + mountSharingHost( + api, + fixture.store, + fixture.data, + new SharedReviewStore(path.join(root, "shared")), + { + readRepository: async () => fixture.repository, + verifyRepository: async () => {}, + fetch, + onPublished: published, + }, + ); + + const response = await api.request("/sharing/publish", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ reviewId: fixture.reviewId }), + }); + + expect(response.status).toBe(422); + expect(await response.json()).toEqual({ + error: + "Whiteboard could not prepare this review for sharing. Please report this issue.", + }); + expect(fetch).not.toHaveBeenCalled(); + expect(published).not.toHaveBeenCalled(); + expect(await readStoreAuth()).toEqual(account); + } finally { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + await fixture.data.close(); + await fixture.store.close(); + await rm(root, { recursive: true, force: true }); + } +}); From e30ad085cccf460f5c37f9b8a669caddfcc7c0f0 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Thu, 22 Oct 2026 12:31:17 -0400 Subject: [PATCH 2/2] Test invalid share bundles at the client Agent-Session: e05d0ee1-ebec-4824-922f-319222ec3817 Agent-Session: e8809ee0-b788-4f9d-a2e1-0cc7df5810e0 Agent-Session: d378b298-4d50-49a2-8569-c9143af1de5f Agent-Session: 3b69532b-b299-4476-8f76-f025671a4a44 Agent-Session: 23dfa6c6-9dcd-4b6d-a1f1-92c1a9513386 Agent-Session: 5375f3cc-3ab7-4c80-a744-51c661db9638 Agent-Session: 470db778-c191-40f3-98a1-b4d661e0aff9 --- packages/review/src/sharing/client.test.ts | 21 ++++++- packages/review/src/sharing/publish.test.ts | 69 --------------------- 2 files changed, 20 insertions(+), 70 deletions(-) diff --git a/packages/review/src/sharing/client.test.ts b/packages/review/src/sharing/client.test.ts index 1b1c182e9..8a8db9f67 100644 --- a/packages/review/src/sharing/client.test.ts +++ b/packages/review/src/sharing/client.test.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { mkdtemp, rm } from "node:fs/promises"; -import { afterEach, expect, it } from "vitest"; +import { afterEach, expect, it, vi } from "vitest"; import { createShareFixture } from "../../test/fixtures/share/create.js"; import { @@ -255,6 +255,25 @@ it.each([409, 503])( }, ); +it("rejects an invalid bundle before contacting the share service", async () => { + const root = await mkdtemp("/tmp/share-client-"); + cleanup.push(() => rm(root, { recursive: true, force: true })); + const fixture = await createShareFixture(root); + cleanup.push(async () => { + await fixture.data.close(); + fixture.store.close(); + }); + + const bundle = await exportShare(fixture); + bundle.objects.clear(); + const network = vi.fn(); + + await expect( + new ShareClient("https://app.dev.fast", undefined, network).create(bundle), + ).rejects.toBeInstanceOf(SharePreflightError); + expect(network).not.toHaveBeenCalled(); +}); + it("bounds concurrent downloads and still rejects corrupt objects", async () => { const root = await mkdtemp("/tmp/share-download-"); cleanup.push(() => rm(root, { recursive: true, force: true })); diff --git a/packages/review/src/sharing/publish.test.ts b/packages/review/src/sharing/publish.test.ts index dd1826eed..4e36579a6 100644 --- a/packages/review/src/sharing/publish.test.ts +++ b/packages/review/src/sharing/publish.test.ts @@ -201,72 +201,3 @@ it("signs the user out when the share service rejects the stored token", async ( await rm(root, { recursive: true, force: true }); } }); - -it("reports incompatible review data without uploading or clearing the login", async () => { - const root = await mkdtemp(path.join(tmpdir(), "sharing-publish-")); - const fixture = await createShareFixture(root); - vi.stubEnv("DEV_REVIEW_HOME", root); - const fetch = vi.fn(); - const published = vi.fn<() => void>(); - const read = fixture.store.read.bind(fixture.store); - - // A saved review can contain metadata the portable share format cannot read. - vi.spyOn(fixture.store, "read").mockImplementation((...args) => { - const snapshot = read(...args); - snapshot.origin = { ...snapshot.origin }; - Object.assign(snapshot.origin, { unsupportedMetadata: "fixture" }); - - return snapshot; - }); - - try { - const account = { - origin: "https://app.dev.fast", - token: "fixture", - login: "fixture", - savedAt: new Date().toISOString(), - }; - - await writeStoreAuth(account); - const api = new Hono(); - api.onError((error, context) => - context.json( - { error: error.message }, - error instanceof ReviewInputError ? error.status : 500, - ), - ); - mountSharingHost( - api, - fixture.store, - fixture.data, - new SharedReviewStore(path.join(root, "shared")), - { - readRepository: async () => fixture.repository, - verifyRepository: async () => {}, - fetch, - onPublished: published, - }, - ); - - const response = await api.request("/sharing/publish", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ reviewId: fixture.reviewId }), - }); - - expect(response.status).toBe(422); - expect(await response.json()).toEqual({ - error: - "Whiteboard could not prepare this review for sharing. Please report this issue.", - }); - expect(fetch).not.toHaveBeenCalled(); - expect(published).not.toHaveBeenCalled(); - expect(await readStoreAuth()).toEqual(account); - } finally { - vi.restoreAllMocks(); - vi.unstubAllEnvs(); - await fixture.data.close(); - await fixture.store.close(); - await rm(root, { recursive: true, force: true }); - } -});