fix(web): enforce video access policy before creating comments (#1982)

[DPS0340]

Fixes #1982 (GHSA-5r3r-v2f5-wqwh).

Problem

The newComment server action authenticates the caller, then inserts the comment without ever checking whether that user is allowed to access the target video:

const user = await getCurrentUser();
if (!user) throw new Error("User not authenticated");
// ...no video access check...
await db().insert(comments).values(newComment);

videoId is a client-supplied argument to a server action, so being signed in is the only barrier. Any authenticated user can post comments — or emoji reactions, or threaded replies — into any video by ID, including another user's private recording. The comment then shows up in the owner's activity feed and triggers a notification to them via createNotification.

Worth noting the same file already treats deletion correctly: deleteComment verifies authorId before removing anything. Creation was the gap.

Fix

Gate the insert behind the existing VideosPolicy.canView policy before any write happens.

I deliberately did not invent a new access-check shape — this mirrors the pattern already used in this exact directory by getTranscript (apps/web/actions/videos/get-transcript.ts), and it is the same policy the /api/video/ai fix in #1926 used to close the sibling advisory #1981:

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

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

Because it reuses canView, it inherits the full existing access model for free — owner, org membership, space membership, public videos, password-protected videos, and email-domain restrictions — rather than approximating it with an ownership check that would have broken legitimate commenting on shared videos.

The error message is deliberately "Video not found" rather than "Access denied", so this does not become an oracle for probing which video IDs exist.

Verification

• biome check apps/web/actions/videos/new-comment.ts — clean.
• tsc --noEmit on apps/web: the remaining diagnostics on this file (TS6305, plus TS2488/TS18046 on the Effect generator) are not introduced by this change — the untouched get-transcript.ts emits the identical set, since it uses the same pattern and the workspace tsconfig project references aren't built in a plain tsc run. Verified by diffing the two files' diagnostics.
• @cap/web-domain, @cap/web-backend and @cap/database build clean via turbo.

Diff: 1 file, +22/-2.

[tembo]

Minor: splitting the Exit.isFailure check from the .value access lets the type guard narrow cleanly (and matches the pattern used in get-transcript.ts).

Suggested change

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

[DPS0340]

@tembo please review

[DPS0340]

Adopted — pushed. Split the branches so the type guard narrows cleanly, matching get-transcript.ts. Also added .limit(1) to the lookup while I was in there, since videos.id is unique.

[DPS0340]

@tembo please re-review

[tembo]

Minor perf/consistency nit: provideOptionalAuth will call getServerSession() + query users again. Since this action already requires auth and you already have user, you can provide the current-user layer directly and skip the extra DB hit.

Suggested change

	import type { ImageUpload } from "@cap/web-domain";
	import { makeCurrentUserLayer, VideosPolicy } from "@cap/web-backend";

[tembo]

Following on the above import, this avoids re-fetching the current user inside the Effect env.

Suggested change

	}).pipe(Effect.provide(makeCurrentUserLayer(user)), EffectRuntime.runPromiseExit);

[DPS0340]

Adopted — pushed.

Verified the redundancy before changing it: provideOptionalAuth is literally

const user = yield* getCurrentUser;   // getServerSession() + select from users
return yield* user.pipe(Option.match({
  onNone: () => app,
  onSome: (user) => app.pipe(Effect.provide(makeCurrentUserLayer(user))),
}));

so on a path that has already established user it re-runs both the session lookup and the users query to arrive at the same layer. Since this action throws on !user before the policy check, the onNone branch is unreachable here — provideOptionalAuth was pure overhead.

Now Effect.provide(makeCurrentUserLayer(user)), which matches what dashboard/folder/[id]/page.tsx and dashboard/spaces/[spaceId]/page.tsx already do.

Small bonus: this also removed a TS18046 'accessExit.value' is of type 'unknown' diagnostic — providing the layer concretely gives the compiler a narrower env type than the provideOptionalAuth wrapper did.

[DPS0340]

@tembo please re-review

[DPS0340]

@tembo please re-review