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 <noreply@paperclip.ing>
This commit is contained in:
@@ -192,6 +192,7 @@ export const projectRouter = {
|
|||||||
if (!character) {
|
if (!character) {
|
||||||
throw new Error(`Character ${input.id} not found`);
|
throw new Error(`Character ${input.id} not found`);
|
||||||
}
|
}
|
||||||
|
await verifyProjectOwnership(ctx.db!, character.projectId, ctx.userId!);
|
||||||
return character;
|
return character;
|
||||||
}),
|
}),
|
||||||
|
|
||||||
@@ -332,12 +333,11 @@ export const projectRouter = {
|
|||||||
.query(async ({ input, ctx }) => {
|
.query(async ({ input, ctx }) => {
|
||||||
await verifyProjectOwnership(ctx.db!, input.projectId, ctx.userId!);
|
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) {
|
if (input.query) {
|
||||||
const q = `%${input.query.toLowerCase()}%`;
|
const q = `%${input.query.toLowerCase()}%`;
|
||||||
conditions = [
|
conditions.push(
|
||||||
eq(characters.projectId, input.projectId),
|
|
||||||
or(
|
or(
|
||||||
like(sql`LOWER(${characters.name})`, q),
|
like(sql`LOWER(${characters.name})`, q),
|
||||||
like(sql`LOWER(COALESCE(${characters.description}, ''))`, 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.traits}, ''))`, q),
|
||||||
like(sql`LOWER(COALESCE(${characters.motivation}, ''))`, q)
|
like(sql`LOWER(COALESCE(${characters.motivation}, ''))`, q)
|
||||||
)!,
|
)!,
|
||||||
];
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (input.role) {
|
if (input.role) {
|
||||||
conditions = [
|
conditions.push(eq(characters.role, input.role));
|
||||||
eq(characters.projectId, input.projectId),
|
|
||||||
eq(characters.role, input.role),
|
|
||||||
];
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (input.arcType) {
|
if (input.arcType) {
|
||||||
conditions = [
|
conditions.push(eq(characters.arcType, input.arcType));
|
||||||
eq(characters.projectId, input.projectId),
|
|
||||||
eq(characters.arcType, input.arcType),
|
|
||||||
];
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return await ctx.db!.select()
|
return await ctx.db!.select()
|
||||||
@@ -421,6 +415,7 @@ export const projectRouter = {
|
|||||||
if (!rows[0]) {
|
if (!rows[0]) {
|
||||||
throw new Error(`Character ${input.characterId} not found`);
|
throw new Error(`Character ${input.characterId} not found`);
|
||||||
}
|
}
|
||||||
|
await verifyProjectOwnership(ctx.db!, rows[0].projectId, ctx.userId!);
|
||||||
return await ctx.db!.select()
|
return await ctx.db!.select()
|
||||||
.from(characterRelationships)
|
.from(characterRelationships)
|
||||||
.where(
|
.where(
|
||||||
@@ -454,6 +449,7 @@ export const projectRouter = {
|
|||||||
if (!charARows[0] || !charBRows[0]) {
|
if (!charARows[0] || !charBRows[0]) {
|
||||||
throw new Error('Both characters must exist');
|
throw new Error('Both characters must exist');
|
||||||
}
|
}
|
||||||
|
await verifyProjectOwnership(ctx.db!, charARows[0].projectId, ctx.userId!);
|
||||||
|
|
||||||
const existing = await ctx.db!.select()
|
const existing = await ctx.db!.select()
|
||||||
.from(characterRelationships)
|
.from(characterRelationships)
|
||||||
@@ -495,13 +491,21 @@ export const projectRouter = {
|
|||||||
isAntagonistic: z.boolean().optional(),
|
isAntagonistic: z.boolean().optional(),
|
||||||
}))
|
}))
|
||||||
.mutation(async ({ input, ctx }) => {
|
.mutation(async ({ input, ctx }) => {
|
||||||
const rows = await ctx.db!.select()
|
const relRows = await ctx.db!.select()
|
||||||
.from(characterRelationships)
|
.from(characterRelationships)
|
||||||
.where(eq(characterRelationships.id, input.id));
|
.where(eq(characterRelationships.id, input.id));
|
||||||
if (!rows[0]) {
|
if (!relRows[0]) {
|
||||||
throw new Error(`Relationship ${input.id} not found`);
|
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<string, any> = { updatedAt: new Date() };
|
const updateData: Record<string, any> = { updatedAt: new Date() };
|
||||||
if (input.relationshipType !== undefined) updateData.relationshipType = input.relationshipType;
|
if (input.relationshipType !== undefined) updateData.relationshipType = input.relationshipType;
|
||||||
if (input.description !== undefined) updateData.description = input.description ?? null;
|
if (input.description !== undefined) updateData.description = input.description ?? null;
|
||||||
@@ -518,13 +522,21 @@ export const projectRouter = {
|
|||||||
deleteRelationship: protectedProcedure
|
deleteRelationship: protectedProcedure
|
||||||
.input(z.object({ id: z.number().int().positive() }))
|
.input(z.object({ id: z.number().int().positive() }))
|
||||||
.mutation(async ({ input, ctx }) => {
|
.mutation(async ({ input, ctx }) => {
|
||||||
const rows = await ctx.db!.select()
|
const relRows = await ctx.db!.select()
|
||||||
.from(characterRelationships)
|
.from(characterRelationships)
|
||||||
.where(eq(characterRelationships.id, input.id));
|
.where(eq(characterRelationships.id, input.id));
|
||||||
if (!rows[0]) {
|
if (!relRows[0]) {
|
||||||
throw new Error(`Relationship ${input.id} not found`);
|
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)
|
await ctx.db!.delete(characterRelationships)
|
||||||
.where(eq(characterRelationships.id, input.id));
|
.where(eq(characterRelationships.id, input.id));
|
||||||
|
|
||||||
@@ -552,6 +564,7 @@ export const projectRouter = {
|
|||||||
if (!scene) {
|
if (!scene) {
|
||||||
throw new Error(`Scene ${input.id} not found`);
|
throw new Error(`Scene ${input.id} not found`);
|
||||||
}
|
}
|
||||||
|
await verifyProjectOwnership(ctx.db!, scene.projectId, ctx.userId!);
|
||||||
return scene;
|
return scene;
|
||||||
}),
|
}),
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,7 @@
|
|||||||
import { initTRPC, TRPCError } from '@trpc/server';
|
import { initTRPC, TRPCError } from '@trpc/server';
|
||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
import { eq } from 'drizzle-orm';
|
||||||
|
import { projects } from '../../src/db/schema';
|
||||||
import type { TRPCContext } from './types';
|
import type { TRPCContext } from './types';
|
||||||
|
|
||||||
// Initialize tRPC with context
|
// Initialize tRPC with context
|
||||||
@@ -21,18 +23,36 @@ const hasDb = t.middleware(({ ctx, next }) => {
|
|||||||
return next({ ctx: { ...ctx, db: ctx.db } });
|
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
|
// Base router
|
||||||
export const baseRouter = t.router;
|
export const baseRouter = t.router;
|
||||||
|
|
||||||
// Procedure builders
|
// Procedure builders
|
||||||
export const publicProcedure = t.procedure.use(hasDb);
|
export const publicProcedure = t.procedure.use(hasDb);
|
||||||
export const protectedProcedure = t.procedure.use(isAuthenticated).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
|
export const projectProcedure = t.procedure
|
||||||
.use(isAuthenticated)
|
.use(isAuthenticated)
|
||||||
|
|||||||
Reference in New Issue
Block a user