From 90dcb47c250e83085935ffe6713224662bff77c1 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 29 Apr 2026 03:13:01 +0200 Subject: [PATCH] fix(security): assertWorkspaceAccess on mentor workspace messaging workspaceSendMessage, workspaceGetMessages, workspaceMarkRead, and workspaceAddFileComment previously trusted the caller-supplied ID and only checked workspaceEnabled. Any user with the MENTOR role could read/post in any workspace, impersonating the assigned mentor and inserting comments under any team's deliverables. All four now run assertWorkspaceAccess (assigned mentor or team member of the project), mirroring the file-handling procedures in the same router. workspaceMarkRead resolves the message -> workspaceId first, and additionally short-circuits when the caller is the sender so unread state stays honest. workspaceAddFileComment resolves the file -> mentorAssignmentId before the access check. Procedures downgraded from mentorProcedure to protectedProcedure since assertWorkspaceAccess is the real gate. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/server/routers/mentor.ts | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/server/routers/mentor.ts b/src/server/routers/mentor.ts index 0fe8ff4..f818aad 100644 --- a/src/server/routers/mentor.ts +++ b/src/server/routers/mentor.ts @@ -2000,7 +2000,7 @@ export const mentorRouter = router({ /** * Send a message in a mentor workspace */ - workspaceSendMessage: mentorProcedure + workspaceSendMessage: protectedProcedure .input( z.object({ mentorAssignmentId: z.string(), @@ -2009,6 +2009,7 @@ export const mentorRouter = router({ }) ) .mutation(async ({ ctx, input }) => { + await assertWorkspaceAccess(ctx.prisma, ctx.user.id, input.mentorAssignmentId) return workspaceSendMessage( { workspaceId: input.mentorAssignmentId, @@ -2023,18 +2024,34 @@ export const mentorRouter = router({ /** * Get workspace messages */ - workspaceGetMessages: mentorProcedure + workspaceGetMessages: protectedProcedure .input(z.object({ mentorAssignmentId: z.string() })) .query(async ({ ctx, input }) => { + await assertWorkspaceAccess(ctx.prisma, ctx.user.id, input.mentorAssignmentId) return workspaceGetMessages(input.mentorAssignmentId, ctx.prisma) }), /** - * Mark a workspace message as read + * Mark a workspace message as read. + * Resolves the message to its workspace and verifies caller belongs to it. + * Caller must also not be the sender (you only mark someone else's message + * as read, not your own — keeps unread state honest). */ - workspaceMarkRead: mentorProcedure + workspaceMarkRead: protectedProcedure .input(z.object({ messageId: z.string() })) .mutation(async ({ ctx, input }) => { + const message = await ctx.prisma.mentorMessage.findUnique({ + where: { id: input.messageId }, + select: { workspaceId: true, senderId: true }, + }) + if (!message || !message.workspaceId) { + throw new TRPCError({ code: 'NOT_FOUND', message: 'Message not found' }) + } + await assertWorkspaceAccess(ctx.prisma, ctx.user.id, message.workspaceId) + if (message.senderId === ctx.user.id) { + // Senders can't mark their own messages as read by others. + return { success: true } + } await workspaceMarkRead(input.messageId, ctx.prisma) return { success: true } }), @@ -2176,7 +2193,7 @@ export const mentorRouter = router({ /** * Add a comment to a workspace file */ - workspaceAddFileComment: mentorProcedure + workspaceAddFileComment: protectedProcedure .input( z.object({ mentorFileId: z.string(), @@ -2185,6 +2202,14 @@ export const mentorRouter = router({ }) ) .mutation(async ({ ctx, input }) => { + const file = await ctx.prisma.mentorFile.findUnique({ + where: { id: input.mentorFileId }, + select: { mentorAssignmentId: true }, + }) + if (!file) { + throw new TRPCError({ code: 'NOT_FOUND', message: 'File not found' }) + } + await assertWorkspaceAccess(ctx.prisma, ctx.user.id, file.mentorAssignmentId) return workspaceAddFileComment( { mentorFileId: input.mentorFileId,