Skip to content

[Security] Comment update and delete have no ownership filter — unreachable today, an IDOR if wired up #207

Description

@SakethSumanBathini

Summary

updateComment and deleteComment in backend/src/repositories/workspace.repository.ts key on commentId alone, with no filter on author or session. Neither has a route or a caller, so nothing is exploitable now — but exposing them as written would let any authenticated user edit or delete any comment by ID.

The code

workspace.repository.ts:163

return prisma.comment.update({
  where: { id: commentId },
  ...
});

workspace.repository.ts:170

return prisma.comment.delete({
  where: { id: commentId },
});

Why file it if it's unreachable

grep -rn "updateComment\|deleteComment" backend/src returns only the definitions — no controller, no route in workspace.routes.ts. So this is genuinely not live, and I'd rather say that than overstate it.

It's worth closing because of how it will be found. Someone adding comment editing will look for a repository method, find one that looks complete, wire it to a route, and ship an IDOR — with nothing in the function suggesting a check was ever intended. The missing filter is invisible by its absence.

The rest of the file gets this right: deleteMany({ where: { sessionId } }) at line 113 scopes correctly, and the session accessors resolve through ownership. These two are the outliers.

Suggested fix

Filter in the query rather than fetching and comparing:

return prisma.comment.update({
  where: { id: commentId, authorLogin: login },
  ...
});

Filtering in the where matters for the same reason it does elsewhere: a post-fetch check leaks whether the comment exists, and the record is briefly in memory either way. Prisma throws P2025 when nothing matches, which maps cleanly to a 404 covering both "no such comment" and "not yours".

Whether the right filter is authorLogin, sessionId, or both depends on whether a workspace owner should be able to moderate others' comments — a product call worth making before the route exists rather than after.

Alternatively, if comment editing isn't planned, deleting both methods is safer. Dead code that looks ready to use is worse than no code.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions