fix(web): gate getVideoAnalytics server action on video access policy

[DPS0340]

Problem

/api/analytics/route.ts correctly gates on VideosPolicy.canView before returning view counts — the comment there even says it exists so that "view counts of private / password-protected videos are not disclosed to unauthorized callers."

But getVideoAnalytics is a "use server" action, so it is its own entry point. Any client can invoke it directly with an arbitrary videoId and never touch that route. The action itself performed no authentication at all — it contains zero calls to getCurrentUser, provideOptionalAuth, or any policy:

export async function getVideoAnalytics(videoId: string, options?) {
  if (!videoId) throw new Error("Video ID is required");

  const [{ orgId } = { orgId: null }] = await db()
    .select({ orgId: videos.orgId })
    .from(videos)
    .where(eq(videos.id, Video.VideoId.make(videoId)))
    .limit(1);
  // ...straight to Tinybird
}

So the protection on the route is bypassable, and view counts for private and password-protected videos are readable by anyone who knows (or guesses) a video ID. It's the same class of gap as #1982 and the /api/video/ai advisory fixed in #1926: the check lives at one caller instead of at the boundary that actually needs it.

The action is already called directly from client code today — Analytics.tsx (a "use client" component) calls it in a useEffect — which is what makes it reachable rather than theoretical.

Fix

Move the gate into the action, using the same canView policy the route uses.

The action already had to query videos to fetch orgId, so I folded the policy onto that existing query rather than adding a second round-trip:

const exit = await Effect.gen(function* () {
  const videosPolicy = yield* VideosPolicy;
  return yield* Effect.promise(() =>
    db().select({ orgId: videos.orgId }).from(videos).where(eq(videos.id, id)).limit(1),
  ).pipe(Policy.withPublicPolicy(videosPolicy.canView(id)));
}).pipe(provideOptionalAuth, EffectRuntime.runPromiseExit);

if (Exit.isFailure(exit)) throw new Error("Video not found");
const orgId = exit.value[0]?.orgId ?? null;

Reusing canView means public videos, owners, org members, space members, and password-protected videos with a valid cookie all keep working exactly as before — the legitimate paths in page.tsx and Analytics.tsx are unaffected. Only unauthorized cross-user reads change behaviour.

The redundant check in /api/analytics is left in place: it returns a clean 404 there, and defence in depth at the route costs nothing.

Verification

• biome check apps/web/actions/videos/get-analytics.ts — clean.
• tsc --noEmit on apps/web: the TS2347 on this file is pre-existing — I confirmed by stashing the change and re-running, where it appears identically (at the old line number). The TS6305/TS2488/TS18046 diagnostics are project-reference artifacts that the untouched get-transcript.ts emits too.
• @cap/web-domain, @cap/web-backend, @cap/database build clean.

Diff: 1 file, +25/-8.

Related: #2029 fixes the same class of issue in newComment (#1982).

[tembo]

If the query returns an empty array (video missing), this still proceeds and queries Tinybird without a tenant filter. Might be safer to treat that as not found too.

Suggested change

	if (Exit.isFailure(exit)) throw new Error("Video not found");
	if (Exit.isFailure(exit) || exit.value.length === 0) throw new Error("Video not found");

[DPS0340]

@tembo please review

[DPS0340]

Adopted — pushed.

You're right, and it's slightly worse than just "safer to treat as not found": with an empty result orgId fell through as null, and the Tinybird conditions are built as

orgId ? `tenant_id = '${escapeLiteral(orgId)}'` : undefined

so a null orgId drops the tenant filter entirely and the query matches pathname = '/s/{videoId}' across every tenant. For a nonexistent video that's just a wasted query, but it's the wrong shape to leave in place.

Now returns not-found before reaching Tinybird, with a comment explaining why the early return is load-bearing rather than a tidiness thing.

[DPS0340]

@tembo please re-review

[tembo]

The tenant-filter rationale in this comment feels a bit off since pathname already scopes by videoId. Might be clearer to just frame this as avoiding querying analytics for non-existent/unauthorized videos.

Suggested change

	// An empty result means the video does not exist. Falling through would
	// If the video doesn't exist or isn't viewable, don't query Tinybird.
	// Otherwise we'd return analytics for an arbitrary `/s/${videoId}` pathname.

[DPS0340]

You're right, and I've taken your wording — pushed.

Checking my own claim: pathname is built as /s/${videoId}, so it already pins the query to one video regardless of the tenant filter. My "widens the match beyond a single org" line overstated the impact. A null orgId does drop tenant_id from the conditions, but with pathname still present the practical effect is a pointless query for a video that doesn't exist, not cross-tenant data.

The early return is still worth having for exactly the reason you framed it, so the behaviour is unchanged — only the comment was wrong.

[DPS0340]

@tembo please re-review

[tembo]

Minor edge case: Video.VideoId.make(videoId) can throw on malformed IDs; might be worth mapping that to the same not-found error so clients don’t get a 500 for invalid input.

Suggested change

	const id = Video.VideoId.make(videoId);
	const id = (() => {
	try {
	return Video.VideoId.make(videoId);
	} catch {
	throw new Error("Video not found");
	}
	})();

[DPS0340]

I checked this one and I don't think the edge case exists — happy to be corrected if I've misread it.

VideoId is an unconstrained branded string:

// packages/web-domain/src/Video.ts:13
export const VideoId = Schema.String.pipe(Schema.brand("VideoId"));

There are no refinements attached (no length, no pattern), so .make() has nothing to reject. I ran it against the inputs I'd expect to be problematic:

OK    ""
OK    "abc"
OK    "!!!"
OK    "aaaa…" (500 chars)
OK    "../../etc/passwd"

All pass, none throw. Schema.brand on a bare Schema.String only validates that the input is a string — and videoId is already typed string at that point, plus the function guards if (!videoId) above.

So a try/catch there would be dead code today. It would become necessary the moment VideoId gains a refinement (a length or nanoid pattern check would be reasonable), but at that point I'd want it handled consistently rather than in this one action — /api/analytics/route.ts:36 calls Video.VideoId.make(videoId) the same unguarded way, as does the rest of the codebase.

Leaving it as-is for now. If you'd like the branded type tightened I'm happy to open that separately, since it'd touch every call site and shouldn't ride along in an access-control fix.

[DPS0340]

@tembo please re-review