fix(web): verify space membership before listing space and folder videos

[DPS0340]

Problem

getSpaceVideoIds and getFolderVideoIds are both "use server" actions that authenticate the caller and then trust the spaceId argument:

const user = await getCurrentUser();
if (!user || !user.id) throw new Error("Unauthorized");

const isAllSpacesEntry = user.activeOrganizationId === spaceId;

const videoIds = isAllSpacesEntry
  ? /* scoped to the user's own org */
  : await db().select({ videoId: spaceVideos.videoId })
      .from(spaceVideos)
      .where(and(eq(spaceVideos.spaceId, spaceId), isNull(spaceVideos.folderId)));

In the else branch there is no membership check at all. Any authenticated user can pass an arbitrary spaceId and get back the list of video IDs in a space belonging to an organization they have nothing to do with. getFolderVideoIds has the identical shape for folder contents.

Video IDs are the sensitive part here — they're the key into /s/{videoId} and into the other video endpoints, so leaking the set of IDs in a private space is a meaningful disclosure even before any per-video check runs.

Fix

Reuse the existing getSpaceAccess helper from @/actions/organization/space-authorization, which already resolves both organization-level and space-level roles:

if (!isAllSpacesEntry) {
  const access = await getSpaceAccess(user.id, spaceId);
  if (!access || (!access.organizationRole && !access.spaceRole)) {
    throw new Error("Space not found");
  }
}

Two deliberate choices:

• getSpaceAccess, not requireSpaceManager. These are read paths. requireSpaceManager would have rejected ordinary members, breaking legitimate use. Requiring either an org role or a space role matches who can actually see a space today.
• The isAllSpacesEntry branch is left untouched. It keys off user.activeOrganizationId, which is read from the user's own DB record rather than supplied by the caller, so that branch is already scoped correctly. Adding a check there would have been noise.

Error text is "Space not found" / "Folder not found" rather than "forbidden", so this doesn't become a probe for which space IDs exist.

Verification

• biome check on both files — clean.
• tsc --noEmit on apps/web: the TS7006 diagnostics on these two files are pre-existing. Confirmed by stashing the change and re-running — they appear identically, just at the earlier line numbers (38→49, 46→57). No new diagnostics.
• @cap/web-domain, @cap/web-backend, @cap/database build clean.

Diff: 2 files, +20/-0.

Related, same class of missing access check: #2029 (newComment, #1982) and #2030 (getVideoAnalytics).

[tembo]

Good hardening. One concern: this new access check keys off the caller-supplied spaceId, but the actual query below only filters by folderId (no spaceId / org constraint). If someone can obtain/guess a folderId from another space/org, they could pass a space they do have access to and still enumerate that folder’s video IDs. Might be worth also scoping the query by spaceVideos.spaceId in the space branch, and by sharedVideos.organizationId (ideally user.activeOrganizationId, not spaceId) in the all-spaces branch.

[DPS0340]

@tembo please review

[DPS0340]

Good catch — that was a real hole in my fix, not a nitpick. Pushed 5c58b58.

You're right that the membership check alone was insufficient: folderId is also caller-supplied, and the queries only filtered on it. So a caller could pass a space they legitimately belong to together with a folder from another space and still read its contents — the check would pass and the query would happily return the wrong folder's rows.

Both queries are now constrained to the space/org that was just authorized:

// space branch
.where(and(eq(spaceVideos.folderId, folderId), eq(spaceVideos.spaceId, spaceId)))

// all-spaces branch
.where(and(eq(sharedVideos.folderId, folderId), eq(sharedVideos.organizationId, spaceId)))

On your activeOrganizationId vs spaceId point for the all-spaces branch: they're provably equal there, since that branch is only taken when user.activeOrganizationId === spaceId. I used spaceId to keep both branches reading the same way, but I'm happy to switch it to user.activeOrganizationId if you'd rather the authorization source be explicit at the point of use — that's a fair readability argument.

I also checked get-space-videos.ts for the same problem: it was already scoping by spaceVideos.spaceId / sharedVideos.organizationId, so only the folder path was affected.

Updated the comment to explain both layers, since the reason the query scoping is load-bearing isn't obvious from the check alone.

[DPS0340]

@tembo please re-review

[tembo]

Minor: consider validating spaceId up front (like getSpaceVideoIds), so a missing/empty value doesn't fall through to getSpaceAccess and return a misleading "Folder not found".

Suggested change

	const isAllSpacesEntry = user.activeOrganizationId === spaceId;
	if (!spaceId) {
	throw new Error("Space ID is required");
	}

[DPS0340]

Adopted — pushed. You're right that it produced a misleading error: an empty spaceId would reach getSpaceAccess, find nothing, and surface "Folder not found" when the actual problem was a missing argument. Now validated up front, matching getSpaceVideoIds.

[DPS0340]

@tembo please re-review

[tembo]

Small readability/type-safety thing: in this branch we already know spaceId === user.activeOrganizationId, so using user.activeOrganizationId here makes it explicit we're scoping by a trusted org id (and avoids leaning on the caller-supplied arg in the query).

Suggested change

	eq(sharedVideos.organizationId, spaceId),
	eq(sharedVideos.organizationId, user.activeOrganizationId),

[DPS0340]

Adopted — pushed. This is the same point I raised in my earlier reply and you're right that explicit is better: in that branch spaceId === user.activeOrganizationId holds by construction, so reading the trusted value directly means the query no longer appears to depend on caller input even though the two are equal there.

That leaves a clean split in the file: the space branch scopes by the spaceId we just authorized via getSpaceAccess, and the all-spaces branch scopes by the org id off the user record. Neither query now takes an unvalidated argument.

tsc is happy with it — activeOrganizationId narrows fine inside the branch since the isAllSpacesEntry comparison already established it.

[DPS0340]

@tembo please re-review