Download permission grants the owner's whole org, while view permission requires an explicit share

[DPS0340]

Summary

canUserDownloadVideo() in apps/web/lib/video-download-permissions.ts treats the video's own organization as if it were a share target, so every member of the owner's org can download any video that org owns, whether or not it was ever shared with them.

const sharedOrgs = await db()
  .select({ organizationId: sharedVideos.organizationId })
  .from(sharedVideos)
  .where(eq(sharedVideos.videoId, videoId));

const orgIds = [orgId, ...sharedOrgs.map((org) => org.organizationId)];
//              ^^^^^ the owner's org, unconditionally

orgId comes straight from video.orgId at the call site in apps/web/actions/videos/download.ts:

const allowed = await canUserDownloadVideo({
  userId: user.id,
  ownerId: video.ownerId,
  videoId,
  orgId: video.orgId,
});

video.orgId is the organization the video belongs to, not an organization it was shared with. Mixing it into the same list as the sharedVideos rows means membership of the owner's org is sufficient, and the sharedVideos lookup below it becomes redundant for that org.

Why this looks unintended rather than by design

The view policy right next door requires an explicit share. buildCanView in packages/web-backend/src/Videos/VideosPolicy.ts uses orgsRepo.membershipForVideo, which inner-joins through sharedVideos:

db.select({ membershipId: Db.organizationMembers.id })
  .from(Db.organizationMembers)
  .leftJoin(Db.sharedVideos, Dz.eq(Db.organizationMembers.organizationId, Db.sharedVideos.organizationId))
  .where(Dz.and(
    Dz.eq(Db.organizationMembers.userId, userId),
    Dz.eq(Db.sharedVideos.videoId, videoId),      // <- the share row must exist
  ))

and logs "Explicit org/space membership found. Access granted." when it matches. video.orgId is used in that policy only to look up the allowed email domain, never as a grant.

So the two paths disagree: a colleague in the owner's org who cannot view an unshared private video can still download it, because the download check never consults sharedVideos for that org — it short-circuits on orgIds containing video.orgId.

The space branch in the same function is written the stricter way, which is what makes the org branch look like an oversight: it derives space ids purely from spaceVideos rows for this video, and returns false when there are none.

Expected

Download permission is granted by the same rule as view permission: ownership, an explicit sharedVideos row for an org the user belongs to, or an explicit spaceVideos row for a space the user belongs to.

Reproduction (by inspection of the query, not run against a live DB)

1. User A and user B are both members of org O.
2. A records a private video; it is stored with orgId = O and is never shared to any org or space.
3. B calls getVideoDownloadInfo(videoId).
4. canUserDownloadVideo builds orgIds = [O], finds B's organizationMembers row for O, and returns true.
5. The same B loading the video page is refused by buildCanView, which finds no sharedVideos row.

I want to be straight about the limits of my evidence here: I traced this by reading the two queries and the call site, and I do not have a running Cap instance with a seeded database to demonstrate the divergence end to end. If video.orgId is intended to be a grant and the view policy is the stricter one on purpose, then this is a documentation gap rather than a bug and I would rather be told so than have it merged. video-download-permissions.ts currently has no test file, so nothing pins either interpretation.

I have a fix ready and will open a PR referencing this issue.

[linear]

CAP-764

[tembo]

Your analysis looks correct. Looking at OrganisationsRepo.membershipForVideo in packages/web-backend/src/Organisations/OrganisationsRepo.ts:15-33, the view policy explicitly joins through sharedVideos:

.leftJoin(
    Db.sharedVideos,
    Dz.eq(Db.organizationMembers.organizationId, Db.sharedVideos.organizationId),
)
.where(Dz.and(
    Dz.eq(Db.organizationMembers.userId, userId),
    Dz.eq(Db.sharedVideos.videoId, videoId),
))

The download permission check in canUserDownloadVideo unconditionally includes video.orgId in the membership lookup, bypassing the share requirement entirely for the owning org.

There's test coverage for the view policy in apps/web/__tests__/unit/videos-policy.test.ts that validates the "explicit org/space membership" behavior requires membershipForVideo to return a match (which depends on a sharedVideos row). As you noted, video-download-permissions.ts has no test file.

Related: #1982 describes a similar pattern where the comment endpoint also lacks proper access checks against video visibility. These two issues together suggest a broader audit of action-level permission checks that were written separately from the core VideosPolicy might be worthwhile.