From 18295099719243409ad4e76a3326ea0c021727e7 Mon Sep 17 00:00:00 2001 From: Wasim Date: Wed, 29 Jul 2026 21:30:27 +0530 Subject: [PATCH 1/6] fix(permissions): require explicit organization share for video downloads (fixes #2037) --- .../unit/video-download-permissions.test.ts | 15 +++++++++++ apps/web/lib/video-download-permissions.ts | 26 ++++++++++--------- 2 files changed, 29 insertions(+), 12 deletions(-) create mode 100644 apps/web/__tests__/unit/video-download-permissions.test.ts diff --git a/apps/web/__tests__/unit/video-download-permissions.test.ts b/apps/web/__tests__/unit/video-download-permissions.test.ts new file mode 100644 index 00000000000..500c8074552 --- /dev/null +++ b/apps/web/__tests__/unit/video-download-permissions.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from "vitest"; +import { canUserDownloadVideo } from "@/lib/video-download-permissions"; + +describe("canUserDownloadVideo", () => { + it("grants download access to the video owner", async () => { + const allowed = await canUserDownloadVideo({ + userId: "user-123" as any, + ownerId: "user-123" as any, + videoId: "vid-456" as any, + orgId: "org-789" as any, + }); + + expect(allowed).toBe(true); + }); +}); diff --git a/apps/web/lib/video-download-permissions.ts b/apps/web/lib/video-download-permissions.ts index 0fc626bf647..558aab49064 100644 --- a/apps/web/lib/video-download-permissions.ts +++ b/apps/web/lib/video-download-permissions.ts @@ -26,20 +26,22 @@ export async function canUserDownloadVideo({ .from(sharedVideos) .where(eq(sharedVideos.videoId, videoId)); - const orgIds = [orgId, ...sharedOrgs.map((org) => org.organizationId)]; + if (sharedOrgs.length > 0) { + const orgIds = sharedOrgs.map((org) => org.organizationId); - const [orgMembership] = await db() - .select({ id: organizationMembers.id }) - .from(organizationMembers) - .where( - and( - eq(organizationMembers.userId, userId), - inArray(organizationMembers.organizationId, orgIds), - ), - ) - .limit(1); + const [orgMembership] = await db() + .select({ id: organizationMembers.id }) + .from(organizationMembers) + .where( + and( + eq(organizationMembers.userId, userId), + inArray(organizationMembers.organizationId, orgIds), + ), + ) + .limit(1); - if (orgMembership) return true; + if (orgMembership) return true; + } const sharedSpaces = await db() .select({ spaceId: spaceVideos.spaceId }) From ad473ee039550263d59110be81e6e372fe827878 Mon Sep 17 00:00:00 2001 From: Wasim Date: Thu, 30 Jul 2026 01:14:13 +0530 Subject: [PATCH 2/6] fix(permissions): prefix unused orgId and add unit tests for org download sharing --- .../unit/video-download-permissions.test.ts | 86 +++++++++++++++++-- apps/web/lib/video-download-permissions.ts | 2 +- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/apps/web/__tests__/unit/video-download-permissions.test.ts b/apps/web/__tests__/unit/video-download-permissions.test.ts index 500c8074552..6ce76c0c15e 100644 --- a/apps/web/__tests__/unit/video-download-permissions.test.ts +++ b/apps/web/__tests__/unit/video-download-permissions.test.ts @@ -1,13 +1,89 @@ -import { describe, expect, it } from "vitest"; +import { + organizationMembers, + sharedVideos, + spaceMembers, + spaceVideos, +} from "@cap/database/schema"; +import type { Organisation, User, Video } from "@cap/web-domain"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { canUserDownloadVideo } from "@/lib/video-download-permissions"; +let mockSharedOrgs: Array<{ organizationId: string }> = []; +let mockOrgMembers: Array<{ id: string }> = []; +let mockSharedSpaces: Array<{ spaceId: string }> = []; +let mockSpaceMembers: Array<{ id: string }> = []; + +vi.mock("@cap/database", () => ({ + db: () => ({ + select: () => ({ + from: (table: unknown) => ({ + where: () => { + let result: unknown[] = []; + if (table === sharedVideos) { + result = mockSharedOrgs; + } else if (table === organizationMembers) { + result = mockOrgMembers; + } else if (table === spaceVideos) { + result = mockSharedSpaces; + } else if (table === spaceMembers) { + result = mockSpaceMembers; + } + const promise = Promise.resolve(result); + (promise as Record).limit = () => + Promise.resolve(result); + return promise; + }, + }), + }), + }), +})); + describe("canUserDownloadVideo", () => { + beforeEach(() => { + mockSharedOrgs = []; + mockOrgMembers = []; + mockSharedSpaces = []; + mockSpaceMembers = []; + }); + it("grants download access to the video owner", async () => { const allowed = await canUserDownloadVideo({ - userId: "user-123" as any, - ownerId: "user-123" as any, - videoId: "vid-456" as any, - orgId: "org-789" as any, + userId: "user-123" as User.UserId, + ownerId: "user-123" as User.UserId, + videoId: "vid-456" as Video.VideoId, + orgId: "org-789" as Organisation.OrganisationId, + }); + + expect(allowed).toBe(true); + }); + + it("denies download access to an org member when the video is not explicitly shared with the organization", async () => { + mockSharedOrgs = []; + mockOrgMembers = [{ id: "member-1" }]; + mockSharedSpaces = []; + mockSpaceMembers = []; + + const allowed = await canUserDownloadVideo({ + userId: "user-123" as User.UserId, + ownerId: "user-456" as User.UserId, + videoId: "vid-789" as Video.VideoId, + orgId: "org-789" as Organisation.OrganisationId, + }); + + expect(allowed).toBe(false); + }); + + it("grants download access to an org member when the video is explicitly shared with the organization", async () => { + mockSharedOrgs = [{ organizationId: "org-789" }]; + mockOrgMembers = [{ id: "member-1" }]; + mockSharedSpaces = []; + mockSpaceMembers = []; + + const allowed = await canUserDownloadVideo({ + userId: "user-123" as User.UserId, + ownerId: "user-456" as User.UserId, + videoId: "vid-789" as Video.VideoId, + orgId: "org-789" as Organisation.OrganisationId, }); expect(allowed).toBe(true); diff --git a/apps/web/lib/video-download-permissions.ts b/apps/web/lib/video-download-permissions.ts index 558aab49064..d718bad01d7 100644 --- a/apps/web/lib/video-download-permissions.ts +++ b/apps/web/lib/video-download-permissions.ts @@ -12,7 +12,7 @@ export async function canUserDownloadVideo({ userId, ownerId, videoId, - orgId, + orgId: _orgId, }: { userId: User.UserId; ownerId: User.UserId; From 06bba3c086d145024ac43391bed1a8971cb7d2ff Mon Sep 17 00:00:00 2001 From: Wasim Date: Thu, 30 Jul 2026 09:41:51 +0530 Subject: [PATCH 3/6] Fix typescript error in sdk-recorder EventHandlers --- packages/sdk-recorder/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sdk-recorder/src/index.ts b/packages/sdk-recorder/src/index.ts index 0f7eb10e1fb..ddc7088945c 100644 --- a/packages/sdk-recorder/src/index.ts +++ b/packages/sdk-recorder/src/index.ts @@ -82,9 +82,9 @@ export class CapRecorder { this.listeners.set(event, new Set()); } const set = this.listeners.get(event); - if (set) set.add(handler as EventHandler); + if (set) set.add(handler as any); return () => { - this.listeners.get(event)?.delete(handler); + this.listeners.get(event)?.delete(handler as any); }; } From f8359e65be0aa7193dd09629e454c4c281270cd1 Mon Sep 17 00:00:00 2001 From: Wasim Date: Thu, 30 Jul 2026 09:56:34 +0530 Subject: [PATCH 4/6] refactor: Hoist type cast as suggested --- packages/sdk-recorder/src/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/sdk-recorder/src/index.ts b/packages/sdk-recorder/src/index.ts index ddc7088945c..72757aeba6d 100644 --- a/packages/sdk-recorder/src/index.ts +++ b/packages/sdk-recorder/src/index.ts @@ -82,9 +82,10 @@ export class CapRecorder { this.listeners.set(event, new Set()); } const set = this.listeners.get(event); - if (set) set.add(handler as any); + const castHandler = handler as unknown as EventHandler; + if (set) set.add(castHandler); return () => { - this.listeners.get(event)?.delete(handler as any); + this.listeners.get(event)?.delete(castHandler); }; } From 7bb947d988e2d50e68f664903fbed7ce817d11a4 Mon Sep 17 00:00:00 2001 From: Wasim Date: Thu, 30 Jul 2026 10:02:50 +0530 Subject: [PATCH 5/6] test: Fix db mock and add mismatch test --- .../unit/video-download-permissions.test.ts | 36 +++++++++++++++++-- packages/sdk-recorder/src/index.ts | 7 ++-- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/apps/web/__tests__/unit/video-download-permissions.test.ts b/apps/web/__tests__/unit/video-download-permissions.test.ts index 6ce76c0c15e..e92eccafde3 100644 --- a/apps/web/__tests__/unit/video-download-permissions.test.ts +++ b/apps/web/__tests__/unit/video-download-permissions.test.ts @@ -17,12 +17,28 @@ vi.mock("@cap/database", () => ({ db: () => ({ select: () => ({ from: (table: unknown) => ({ - where: () => { + where: (condition: any) => { let result: unknown[] = []; if (table === sharedVideos) { result = mockSharedOrgs; } else if (table === organizationMembers) { - result = mockOrgMembers; + // Simple condition check for tests: if the where clause doesn't include the target orgs/users, return empty + const getCircularReplacer = () => { + const seen = new WeakSet(); + return (key: string, value: any) => { + if (typeof value === "object" && value !== null) { + if (seen.has(value)) return; + seen.add(value); + } + return value; + }; + }; + const conditionStr = JSON.stringify(condition, getCircularReplacer()) || ""; + if (mockOrgMembers.length > 0 && !conditionStr.includes("org-789")) { + result = []; + } else { + result = mockOrgMembers; + } } else if (table === spaceVideos) { result = mockSharedSpaces; } else if (table === spaceMembers) { @@ -88,4 +104,20 @@ describe("canUserDownloadVideo", () => { expect(allowed).toBe(true); }); + + it("denies download access when the video is shared with a different organization", async () => { + mockSharedOrgs = [{ organizationId: "other-org-id" }]; + mockOrgMembers = [{ id: "member-1" }]; + mockSharedSpaces = []; + mockSpaceMembers = []; + + const allowed = await canUserDownloadVideo({ + userId: "user-123" as User.UserId, + ownerId: "user-456" as User.UserId, + videoId: "vid-789" as Video.VideoId, + orgId: "org-789" as Organisation.OrganisationId, + }); + + expect(allowed).toBe(false); + }); }); diff --git a/packages/sdk-recorder/src/index.ts b/packages/sdk-recorder/src/index.ts index 72757aeba6d..991d95e8d05 100644 --- a/packages/sdk-recorder/src/index.ts +++ b/packages/sdk-recorder/src/index.ts @@ -82,10 +82,11 @@ export class CapRecorder { this.listeners.set(event, new Set()); } const set = this.listeners.get(event); - const castHandler = handler as unknown as EventHandler; - if (set) set.add(castHandler); + if (!set) return () => {}; + const typedHandler = handler as unknown as EventHandler; + set.add(typedHandler); return () => { - this.listeners.get(event)?.delete(castHandler); + set.delete(typedHandler); }; } From ae291f6eb8abd18465931e86efdfb92e759978dc Mon Sep 17 00:00:00 2001 From: Wasim Date: Thu, 30 Jul 2026 21:28:27 +0530 Subject: [PATCH 6/6] Fix import path in video-download-permissions test --- apps/web/__tests__/unit/video-download-permissions.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/__tests__/unit/video-download-permissions.test.ts b/apps/web/__tests__/unit/video-download-permissions.test.ts index e92eccafde3..2a13a7fc1e1 100644 --- a/apps/web/__tests__/unit/video-download-permissions.test.ts +++ b/apps/web/__tests__/unit/video-download-permissions.test.ts @@ -6,7 +6,7 @@ import { } from "@cap/database/schema"; import type { Organisation, User, Video } from "@cap/web-domain"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { canUserDownloadVideo } from "@/lib/video-download-permissions"; +import { canUserDownloadVideo } from "../../../lib/video-download-permissions"; let mockSharedOrgs: Array<{ organizationId: string }> = []; let mockOrgMembers: Array<{ id: string }> = [];