fix(permissions): require explicit organization share for video downloads by wasim-builds · Pull Request #2052 · CapSoftware/Cap

wasim-builds · 2026-07-29T16:00:56Z

Fixes #2037 by requiring an explicit sharedVideos entry for organization-level video download permission checks, aligning it with the view policy and space-level download checks.

Greptile Summary

This PR narrows organization-level video download access to explicitly shared organizations.

Removes implicit download authorization based solely on membership in the video's owning organization.
Skips the organization-membership query when the video has no explicit organization shares.
Adds an owner-access unit test.

Confidence Score: 4/5

The implementation appears safe to merge, with the non-blocking caveat that its central permission regression is not covered by the added test.

The authorization change implements the stated explicit-share requirement, but the only new test exits through the unchanged owner shortcut and would not detect a regression in organization-level checks.

Files Needing Attention: apps/web/tests/unit/video-download-permissions.test.ts

Important Files Changed

Filename	Overview
apps/web/lib/video-download-permissions.ts	Organization download authorization now checks only organizations explicitly associated through sharedVideos, matching the stated permission change.
apps/web/tests/unit/video-download-permissions.test.ts	Adds owner-access coverage but does not execute or protect the changed organization-sharing behavior.

Prompt To Fix All With AI

### Issue 1
apps/web/__tests__/unit/video-download-permissions.test.ts:5
**Permission change remains untested**

The sole test uses identical `userId` and `ownerId` values, so it returns through the unchanged owner shortcut without exercising the new organization-sharing check. Add cases proving that an organization member is denied without a `sharedVideos` entry and allowed with one, otherwise the regression addressed by this PR can return without failing this suite.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(permissions): require explicit organ..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

Knowledge Base — Web App (apps/web)

…oads (fixes CapSoftware#2037)

superagent-security · 2026-07-29T16:01:03Z

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

greptile-apps · 2026-07-29T16:03:08Z

+import { canUserDownloadVideo } from "@/lib/video-download-permissions";
+
+describe("canUserDownloadVideo", () => {
+	it("grants download access to the video owner", async () => {


Permission change remains untested

The sole test uses identical userId and ownerId values, so it returns through the unchanged owner shortcut without exercising the new organization-sharing check. Add cases proving that an organization member is denied without a sharedVideos entry and allowed with one, otherwise the regression addressed by this PR can return without failing this suite.

Knowledge Base Used: Web App (apps/web)

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/web/__tests__/unit/video-download-permissions.test.ts Line: 5 Comment: **Permission change remains untested** The sole test uses identical `userId` and `ownerId` values, so it returns through the unchanged owner shortcut without exercising the new organization-sharing check. Add cases proving that an organization member is denied without a `sharedVideos` entry and allowed with one, otherwise the regression addressed by this PR can return without failing this suite. **Knowledge Base Used:** [Web App (apps/web)](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/web-app.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

tembo · 2026-07-29T16:06:15Z

 		.where(eq(sharedVideos.videoId, videoId));

-	const orgIds = [orgId, ...sharedOrgs.map((org) => org.organizationId)];
+	if (sharedOrgs.length > 0) {


This change makes the orgId arg effectively unused in this function. If the signature is meant to stay stable, consider aliasing it to _orgId (or dropping it entirely) so Biome doesn’t flag an unused variable and the API stays less confusing.

tembo · 2026-07-29T16:06:24Z

+import { canUserDownloadVideo } from "@/lib/video-download-permissions";
+
+describe("canUserDownloadVideo", () => {
+	it("grants download access to the video owner", async () => {


This test currently only covers the owner short-circuit. Since this PR changes org-level download behavior, it’d be useful to add a case that exercises the new shared_videos gating (member of org + explicit share => allow; member of org without share => deny), to prevent regressions.

…load sharing

tembo · 2026-07-30T04:14:35Z

 		}
 		const set = this.listeners.get(event);
-		if (set) set.add(handler as EventHandler<keyof RecorderEventMap>);
+		if (set) set.add(handler as any);


Using as any here drops a lot of type safety and can also make add/remove symmetry less obvious. You can keep the set typed and just normalize the handler once:

Suggested change

if (set) set.add(handler as any);

const set = this.listeners.get(event);

if (!set) return () => {};

const typedHandler = handler as EventHandler<keyof RecorderEventMap>;

set.add(typedHandler);

return () => {

set.delete(typedHandler);

};

tembo · 2026-07-30T04:14:40Z

+	db: () => ({
+		select: () => ({
+			from: (table: unknown) => ({
+				where: () => {


This db() mock ignores the where(...) conditions entirely, so the “explicitly shared org” test is really only asserting the sharedOrgs.length > 0 gating. If you want the test to actually protect the inArray(...orgIds) behavior, consider making the mock return org membership conditionally (or add a mismatch case where sharedVideos contains a different orgId and expect false).

tembo · 2026-07-30T04:30:32Z

 		}
 		const set = this.listeners.get(event);
-		if (set) set.add(handler as EventHandler<keyof RecorderEventMap>);
+		const castHandler = handler as unknown as EventHandler<keyof RecorderEventMap>;


Since the Set is always created right above, you can simplify this a bit and avoid the extra Map.get(...) in the unsubscribe closure:

Suggested change

const castHandler = handler as unknown as EventHandler<keyof RecorderEventMap>;

const set = this.listeners.get(event)!;

const castHandler = handler as unknown as EventHandler<keyof RecorderEventMap>;

set.add(castHandler);

return () => set.delete(castHandler);

tembo · 2026-07-30T16:05:26Z

+} 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";


This relative path resolves to apps/lib/video-download-permissions, which doesn't exist — the source file is at apps/web/lib/. The __tests__/unit/ directory is only two levels below apps/web/, so you'd need ../../lib/... for a relative path. Since @/lib is already aliased in both tsconfig.json and vitest.config.ts (and used by other unit tests like roles-permissions.test.ts), reverting to the alias is probably cleanest:

Suggested change

import { canUserDownloadVideo } from "../../../lib/video-download-permissions";

import { canUserDownloadVideo } from "@/lib/video-download-permissions";

fix(permissions): require explicit organization share for video downl…

1829509

…oads (fixes CapSoftware#2037)

superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 29, 2026

greptile-apps Bot reviewed Jul 29, 2026

View reviewed changes

tembo Bot reviewed Jul 29, 2026

View reviewed changes

wasim-builds added 2 commits July 30, 2026 01:14

fix(permissions): prefix unused orgId and add unit tests for org down…

ad473ee

…load sharing

Fix typescript error in sdk-recorder EventHandlers

06bba3c

tembo Bot reviewed Jul 30, 2026

View reviewed changes

refactor: Hoist type cast as suggested

f8359e6

tembo Bot reviewed Jul 30, 2026

View reviewed changes

wasim-builds added 2 commits July 30, 2026 10:02

test: Fix db mock and add mismatch test

7bb947d

Fix import path in video-download-permissions test

ae291f6

tembo Bot reviewed Jul 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(permissions): require explicit organization share for video downloads - #2052

fix(permissions): require explicit organization share for video downloads#2052
wasim-builds wants to merge 6 commits into
CapSoftware:mainfrom
wasim-builds:fix/download-permission-org-share

wasim-builds commented Jul 29, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

superagent-security Bot commented Jul 29, 2026

Uh oh!

greptile-apps Bot Jul 29, 2026

Uh oh!

tembo Bot Jul 29, 2026

Uh oh!

tembo Bot Jul 29, 2026

Uh oh!

tembo Bot Jul 30, 2026

Uh oh!

tembo Bot Jul 30, 2026

Uh oh!

tembo Bot Jul 30, 2026

Uh oh!

tembo Bot Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-		if (set) set.add(handler as any);
+		const set = this.listeners.get(event);
+		if (!set) return () => {};
+		const typedHandler = handler as EventHandler<keyof RecorderEventMap>;
+		set.add(typedHandler);
+		return () => {
+			set.delete(typedHandler);
+		};

-		const castHandler = handler as unknown as EventHandler<keyof RecorderEventMap>;
+		const set = this.listeners.get(event)!;
+		const castHandler = handler as unknown as EventHandler<keyof RecorderEventMap>;
+		set.add(castHandler);
+		return () => set.delete(castHandler);

	import { canUserDownloadVideo } from "../../../lib/video-download-permissions";
	import { canUserDownloadVideo } from "@/lib/video-download-permissions";

Uh oh!

Conversation

wasim-builds commented Jul 29, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

superagent-security Bot commented Jul 29, 2026

Uh oh!

greptile-apps Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

wasim-builds commented Jul 29, 2026 •

edited by greptile-apps Bot

Loading