fix(comments): require canView access before posting a comment - #2070
Merged
richiemcilroy merged 4 commits intoAug 3, 2026
Merged
fix(comments): require canView access before posting a comment#2070richiemcilroy merged 4 commits into
richiemcilroy merged 4 commits into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
added 4 commits
August 3, 2026 21:07
newComment only checked that the requester was authenticated, not that they could actually view the target video. Any logged-in user could POST a comment (or reply/reaction) onto someone else's private video by supplying its videoId directly — an IDOR. Guard the insert with VideosPolicy.canView, matching the pattern already used in get-transcript.ts (and the canView guards added to other video endpoints in #1926/#1927/#1936). Access failures are reported as "Video not found" rather than "Forbidden" to avoid leaking video existence to unauthorized users.
- Drop subjective 'CVE-worthy' framing from the code comment per review. - canView returns true when a video doesn't exist (by design, so not-found doesn't leak existence elsewhere). Fetch the video row under the same policy, like get-transcript.ts does, so a bogus videoId is rejected instead of reaching the insert.
Splitting these means a genuine failure (e.g. a DB error) is logged before surfacing the same user-facing 'Video not found' message, instead of silently looking identical to a real not-found case.
Makes the intent (existence check, not a full row scan) explicit.
Copy link
Copy Markdown
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1982.
Problem
The newComment server action inserted a comment for any authenticated user on any caller-supplied videoId, with no check that the caller can view the video. Any authenticated user could inject comments, replies, and reactions into other users' private videos (IDOR).
Fix
Gate the insert on the existing view policy, following the exact pattern used in get-transcript.ts / get-status.ts: fetch the video id inside an Effect.gen, run it through Policy.withPublicPolicy(videosPolicy.canView(videoId)), and throw "Video not found" when the policy denies or the row does not exist. The id-only select doubles as an existence check, since canView returns true for nonexistent videos by design.
Denial is reported as "Video not found" rather than "Forbidden" so the endpoint does not become an existence oracle for private videos.
Verification
Note: denied comments fail silently in the UI after an optimistic insert, which matches the pre-existing failure UX of this action. A sibling low-severity leak (per-day comment counts via dashboard analytics capId) was found during review and will be filed as a follow-up; it is out of scope here.
Greptile Summary
The PR closes the comment-posting IDOR by requiring the authenticated caller to pass the existing video view policy and by verifying that the referenced video exists before insertion.
Confidence Score: 5/5
The PR appears safe to merge and closes the unauthorized private-video comment path without breaking established access flows.
The new gate uses the repository's established optional-auth and password-aware Effect runtime pattern, denies callers who cannot view the video, and separately rejects nonexistent video IDs before insertion.
Important Files Changed
Reviews (1): Last reviewed commit: "address review: cap existence check with..." | Re-trigger Greptile
Context used: