fix(comments): require canView access before posting a comment

[eponomariova-dotcom]

Problem

Fixes #1982.

newComment (in apps/web/actions/videos/new-comment.ts) only checked that the requester was authenticated (getCurrentUser()), not that they actually had access to the target video. Since the action inserts directly into the comments table keyed only by a client-supplied videoId, any authenticated user could post a comment (or reply/reaction) onto someone else's private video just by knowing/guessing its id — an IDOR.

Fix

Adds a VideosPolicy.canView guard before the insert, following the exact pattern already used in get-transcript.ts (provideOptionalAuth + Policy.withPublicPolicy(videosPolicy.canView(videoId))), and the same approach used to close the equivalent gaps in #1926, #1927, and #1936 for other video endpoints. On denial the action throws "Video not found" (not "Forbidden"), so the error doesn't leak whether a private video exists to someone who can't view it.

Testing

• pnpm typecheck — clean (scoped to packages/web-backend + apps/web project references; ran with --max-old-space-size=8192 since the default heap OOMs the full monorepo tsc -b on this machine).
• pnpm exec biome lint on the changed file — clean.
• vitest run __tests__/unit/videos-policy.test.ts — all 40 existing cases pass, including the exact scenario this fix relies on ("denies logged-in user without membership" on a private video).

Limitation: I don't have Docker available in this environment, so I could not run the full dev stack (MySQL/MinIO) to exercise newComment end-to-end against a real private video + second account. The change itself is a minimal, mechanical application of the same already-tested canView policy used elsewhere in the codebase, but flagging this so a maintainer can do a quick manual pass if desired.

Greptile Summary

This PR adds an authorization guard to newComment so that posting a comment (or reply/reaction) now requires canView access on the target video, not just authentication. Without this check, any logged-in user could post to a private video by knowing its ID.

• Inserts a VideosPolicy.canView check using the same provideOptionalAuth + Policy.withPublicPolicy + EffectRuntime.runPromiseExit pattern already present in get-transcript.ts and several other actions.
• On authorization failure, throws \"Video not found\" rather than \"Forbidden\" to avoid leaking the existence of private videos to callers who lack access.

Confidence Score: 5/5

Safe to merge. The change is a focused, minimal addition of a well-established access-control pattern that closes a real vulnerability without touching any other logic.

The authorization gate is a direct mechanical copy of the pattern used in get-transcript.ts and several other actions — same provideOptionalAuth, same Policy.withPublicPolicy, same runPromiseExit/Exit.isFailure check. The error message deliberately obscures private-video existence. No existing logic was modified; the insert, notification, and return path are unchanged.

No files require special attention.

Important Files Changed

Filename	Overview
apps/web/actions/videos/new-comment.ts	Adds a VideosPolicy.canView authorization gate before inserting a comment, closing an IDOR that let any authenticated user post to private videos. Pattern is mechanically identical to get-transcript.ts.

_{Reviews (1): Last reviewed commit: "fix(comments): require canView access be..." | Re-trigger Greptile}

[tembo]

Nit: might want to avoid wording like “CVE-worthy” in a code comment (it’s subjective / can get stale). Something like this stays factual:

Suggested change

	// Authentication alone isn't authorization: without this, any logged-in
	// user could comment on someone else's private video (CVE-worthy IDOR).
	// Authentication alone isn't authorization: without this, any logged-in user could
	// comment on someone else's private video by guessing its id.

[eponomariova-dotcom]

Done — updated to neutral wording.

[tembo]

Small edge case: VideosPolicy.canView currently returns true when the video doesn’t exist (so callers can return “not found” without leaking existence). Since this guard runs against Effect.void, a bogus videoId can still reach the insert and potentially throw a FK error (or create an orphan if there’s no FK). If you want consistent behavior, consider fetching the video (or just id) under the same policy like get-transcript.ts and treat an empty result as Video not found.

[eponomariova-dotcom]

Good catch — fetching the video row under the same policy now, so a nonexistent videoId is rejected instead of reaching the insert (matches get-transcript.ts).

[tembo]

Minor: Exit.isFailure(accessExit) will also be true for operational errors (DB connection, etc), which would get surfaced as Video not found and make debugging harder. Consider splitting the checks and logging the failure before returning the user-facing not-found.

Suggested change

	if (Exit.isFailure(accessExit) || accessExit.value.length === 0) {
	if (Exit.isFailure(accessExit)) {
	console.error("Video access check failed:", accessExit);
	throw new Error("Video not found");
	}
	if (accessExit.value.length === 0) {
	throw new Error("Video not found");
	}

[eponomariova-dotcom]

Applied, thanks.

[tembo]

Minor: since this query is only checking existence, adding .limit(1) makes the intent explicit (and avoids accidentally scanning more than needed if the shape ever changes).

Suggested change

	db().select({ id: videos.id }).from(videos).where(eq(videos.id, videoId)),
	db().select({ id: videos.id }).from(videos).where(eq(videos.id, videoId)).limit(1),

[eponomariova-dotcom]

Applied.