From 0ba20e5b312408f829601285f31e1a4dad729d4f Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Fri, 24 Apr 2026 10:13:49 -0400 Subject: [PATCH] FRE-592: Fix 4 code review blockers (security + correctness) - Add project ownership verification to relationship mutations (createRelationship, updateRelationship, deleteRelationship, getRelationshipsForCharacter) - Add project ownership verification to getCharacter and getScene - Add ownership check to projectProcedure middleware (hasProjectAccess) - Fix searchCharacters filter combination bug (accumulate conditions instead of overwriting) Co-Authored-By: Paperclip --- server/trpc/project-router.ts | 45 ++++++++++++++++++++++------------- server/trpc/router.ts | 32 ++++++++++++++++++++----- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/server/trpc/project-router.ts b/server/trpc/project-router.ts index bf98de51f..cc92aaf4d 100644 --- a/server/trpc/project-router.ts +++ b/server/trpc/project-router.ts @@ -192,6 +192,7 @@ export const projectRouter = { if (!character) { throw new Error(`Character ${input.id} not found`); } + await verifyProjectOwnership(ctx.db!, character.projectId, ctx.userId!); return character; }), @@ -332,12 +333,11 @@ export const projectRouter = { .query(async ({ input, ctx }) => { await verifyProjectOwnership(ctx.db!, input.projectId, ctx.userId!); - let conditions: import('drizzle-orm').SQL[] = [eq(characters.projectId, input.projectId)]; + const conditions: import('drizzle-orm').SQL[] = [eq(characters.projectId, input.projectId)]; if (input.query) { const q = `%${input.query.toLowerCase()}%`; - conditions = [ - eq(characters.projectId, input.projectId), + conditions.push( or( like(sql`LOWER(${characters.name})`, q), like(sql`LOWER(COALESCE(${characters.description}, ''))`, q), @@ -345,21 +345,15 @@ export const projectRouter = { like(sql`LOWER(COALESCE(${characters.traits}, ''))`, q), like(sql`LOWER(COALESCE(${characters.motivation}, ''))`, q) )!, - ]; + ); } if (input.role) { - conditions = [ - eq(characters.projectId, input.projectId), - eq(characters.role, input.role), - ]; + conditions.push(eq(characters.role, input.role)); } if (input.arcType) { - conditions = [ - eq(characters.projectId, input.projectId), - eq(characters.arcType, input.arcType), - ]; + conditions.push(eq(characters.arcType, input.arcType)); } return await ctx.db!.select() @@ -421,6 +415,7 @@ export const projectRouter = { if (!rows[0]) { throw new Error(`Character ${input.characterId} not found`); } + await verifyProjectOwnership(ctx.db!, rows[0].projectId, ctx.userId!); return await ctx.db!.select() .from(characterRelationships) .where( @@ -454,6 +449,7 @@ export const projectRouter = { if (!charARows[0] || !charBRows[0]) { throw new Error('Both characters must exist'); } + await verifyProjectOwnership(ctx.db!, charARows[0].projectId, ctx.userId!); const existing = await ctx.db!.select() .from(characterRelationships) @@ -495,13 +491,21 @@ export const projectRouter = { isAntagonistic: z.boolean().optional(), })) .mutation(async ({ input, ctx }) => { - const rows = await ctx.db!.select() + const relRows = await ctx.db!.select() .from(characterRelationships) .where(eq(characterRelationships.id, input.id)); - if (!rows[0]) { + if (!relRows[0]) { throw new Error(`Relationship ${input.id} not found`); } + const charARows = await ctx.db!.select() + .from(characters) + .where(eq(characters.id, relRows[0].characterIdA)); + if (!charARows[0]) { + throw new Error('Character not found'); + } + await verifyProjectOwnership(ctx.db!, charARows[0].projectId, ctx.userId!); + const updateData: Record = { updatedAt: new Date() }; if (input.relationshipType !== undefined) updateData.relationshipType = input.relationshipType; if (input.description !== undefined) updateData.description = input.description ?? null; @@ -518,13 +522,21 @@ export const projectRouter = { deleteRelationship: protectedProcedure .input(z.object({ id: z.number().int().positive() })) .mutation(async ({ input, ctx }) => { - const rows = await ctx.db!.select() + const relRows = await ctx.db!.select() .from(characterRelationships) .where(eq(characterRelationships.id, input.id)); - if (!rows[0]) { + if (!relRows[0]) { throw new Error(`Relationship ${input.id} not found`); } + const charARows = await ctx.db!.select() + .from(characters) + .where(eq(characters.id, relRows[0].characterIdA)); + if (!charARows[0]) { + throw new Error('Character not found'); + } + await verifyProjectOwnership(ctx.db!, charARows[0].projectId, ctx.userId!); + await ctx.db!.delete(characterRelationships) .where(eq(characterRelationships.id, input.id)); @@ -552,6 +564,7 @@ export const projectRouter = { if (!scene) { throw new Error(`Scene ${input.id} not found`); } + await verifyProjectOwnership(ctx.db!, scene.projectId, ctx.userId!); return scene; }), diff --git a/server/trpc/router.ts b/server/trpc/router.ts index 9376c1685..532184691 100644 --- a/server/trpc/router.ts +++ b/server/trpc/router.ts @@ -1,5 +1,7 @@ import { initTRPC, TRPCError } from '@trpc/server'; import { z } from 'zod'; +import { eq } from 'drizzle-orm'; +import { projects } from '../../src/db/schema'; import type { TRPCContext } from './types'; // Initialize tRPC with context @@ -21,18 +23,36 @@ const hasDb = t.middleware(({ ctx, next }) => { return next({ ctx: { ...ctx, db: ctx.db } }); }); +// Middleware for project ownership verification +const hasProjectAccess = t.middleware(async ({ ctx, next }) => { + if (!ctx.projectId) { + throw new TRPCError({ code: 'FORBIDDEN', message: 'Project access required' }); + } + if (!ctx.userId) { + throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); + } + if (!ctx.db) { + throw new TRPCError({ code: 'INTERNAL_SERVER_ERROR', message: 'Database not available' }); + } + const rows = await ctx.db.select({ id: projects.id, ownerId: projects.ownerId }) + .from(projects) + .where(eq(projects.id, ctx.projectId)); + const project = rows[0]; + if (!project) { + throw new TRPCError({ code: 'NOT_FOUND', message: `Project ${ctx.projectId} not found` }); + } + if (project.ownerId !== ctx.userId) { + throw new TRPCError({ code: 'FORBIDDEN', message: `You do not have access to project ${ctx.projectId}` }); + } + return next({ ctx: { ...ctx, projectId: ctx.projectId } }); +}); + // Base router export const baseRouter = t.router; // Procedure builders export const publicProcedure = t.procedure.use(hasDb); export const protectedProcedure = t.procedure.use(isAuthenticated).use(hasDb); -const hasProjectAccess = t.middleware(({ ctx, next }) => { - if (!ctx.projectId) { - throw new TRPCError({ code: 'FORBIDDEN', message: 'Project access required' }); - } - return next({ ctx: { ...ctx, projectId: ctx.projectId } }); -}); export const projectProcedure = t.procedure .use(isAuthenticated)