Skip to content

fix(web): restrict comment deletion to the author's own replies#1936

Merged
richiemcilroy merged 1 commit into
mainfrom
security/comment-delete-idor
Jun 19, 2026
Merged

fix(web): restrict comment deletion to the author's own replies#1936
richiemcilroy merged 1 commit into
mainfrom
security/comment-delete-idor

Conversation

@richiemcilroy

@richiemcilroy richiemcilroy commented Jun 19, 2026

Copy link
Copy Markdown
Member

Restricts comment deletion so a user can only delete their own comment and their own replies, instead of also deleting replies authored by other users.

Greptile Summary

This PR narrows the comment-delete query to only remove replies authored by the same user, rather than all replies to the deleted parent comment. The ownership pre-check (lines 27–42) that guards access to the parent comment is unchanged.

  • The DELETE query now wraps the existing or(id = X, parentCommentId = X) predicate inside and(authorId = user.id, …), preventing other users' replies from being wiped when a parent comment is deleted.
  • No other logic, auth checks, or response shapes are modified.

Confidence Score: 5/5

Safe to merge — the change is a single, targeted predicate addition that correctly restricts bulk deletion to the requesting user's own content.

The existing ownership pre-check (verify the parent comment belongs to the caller before proceeding) is untouched. The new authorId = user.id constraint in the delete query is the minimal, correct fix: it scopes the bulk delete to the caller's own replies without opening any new surface for data loss or privilege escalation.

No files require special attention.

Important Files Changed

Filename Overview
apps/web/app/api/video/comment/delete/route.ts Adds authorId = user.id guard to the bulk-delete query so only the requesting user's own replies are removed alongside a deleted parent comment; ownership of the parent is already enforced by the pre-check at line 27.

Reviews (2): Last reviewed commit: "fix(web): restrict comment deletion to t..." | Re-trigger Greptile

@polarityinc

polarityinc Bot commented Jun 19, 2026

Copy link
Copy Markdown

Paragon Review Skipped

Hi @richiemcilroy! Your Polarity credit balance is insufficient to complete this review.

Please visit https://app.paragon.run to finish your review.

@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

Comment on lines +44 to 57
// Delete the comment and only the replies authored by the same user.
// Replies written by other users are preserved (never delete others'
// content), even though that can leave them visually orphaned.
await db()
.delete(comments)
.where(
or(
eq(comments.id, commentId.value),
eq(comments.parentCommentId, commentId.value),
and(
eq(comments.authorId, user.id),
or(
eq(comments.id, commentId.value),
eq(comments.parentCommentId, commentId.value),
),
),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Orphaned replies may be invisible or error-prone in the UI

When the parent comment is deleted but other users' replies survive (with parentCommentId pointing to the now-deleted row), the frontend needs to handle this case explicitly. If the UI renders replies by first looking up the parent from local state or a cached result set, orphaned replies could either silently disappear or trigger a rendering error depending on how null/missing parents are handled. The code comment acknowledges this trade-off, but it's worth confirming the front-end comment list component guards against a missing parent before this ships.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/api/video/comment/delete/route.ts
Line: 44-57

Comment:
**Orphaned replies may be invisible or error-prone in the UI**

When the parent comment is deleted but other users' replies survive (with `parentCommentId` pointing to the now-deleted row), the frontend needs to handle this case explicitly. If the UI renders replies by first looking up the parent from local state or a cached result set, orphaned replies could either silently disappear or trigger a rendering error depending on how null/missing parents are handled. The code comment acknowledges this trade-off, but it's worth confirming the front-end comment list component guards against a missing parent before this ships.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

// Delete the comment and all its replies
// Delete the comment and only the replies authored by the same user.
// Replies written by other users are preserved (never delete others'
// content), even though that can leave them visually orphaned.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this is only deleting direct replies (same as before), so the comment may read a bit stronger than the behavior. Also worth calling out that preserving other users’ replies means they’ll reference a deleted parent unless you soft-delete the parent or re-parent them.

Suggested change
// content), even though that can leave them visually orphaned.
// Delete the comment and only direct replies authored by the same user.
// Replies written by other users are preserved (never delete others' content),
// which can leave them visually orphaned unless you soft-delete the parent.

@richiemcilroy

Copy link
Copy Markdown
Member Author

@greptileai please review the PR

@richiemcilroy richiemcilroy merged commit b66fad3 into main Jun 19, 2026
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant