Skip to content

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

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 right now — but as written, exposing them would let any authenticated user edit or delete any comment in the system 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 I'm filing it despite it being unreachable

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

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

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

Suggested fix

Take the caller's identity and filter on it in the query rather than checking after the fetch:

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

Filtering in the where rather than fetching-then-comparing matters for the same reason it did in the session routes: a post-fetch check leaks whether the comment exists, and the record is briefly in memory regardless. Prisma will throw P2025 when nothing matches, which the caller can map 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 — that's a product call rather than a mechanical one, so worth deciding before the route exists rather than after.

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

Happy to take this once the moderation question is settled.

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