FRE-588: Fix IDOR vulnerabilities and security findings

H1: Add verifyScriptAccess/verifyRevisionAccess to all 14 revisions endpoints
H2: Add verifyProjectAccess to listScripts and searchScripts
M2: Add cascade delete for projectMembers on project deletion
M4: Replace plain Error throws with TRPCError for consistent error handling
M5: Use crypto.randomUUID for team ID generation (was Date.now + Math.random)
L1: Add 100KB content size limit on revision content
L2: Add unique constraint to script slug column
L3: Update hasProjectAccess middleware to check project membership
This commit is contained in:
Senior Engineer
2026-04-29 06:57:20 -04:00
committed by Michael Freno
parent eab380b76b
commit c142611470
7 changed files with 154 additions and 114 deletions

View File

@@ -221,6 +221,10 @@ export const projectRouter = {
await ctx.db!.delete(scenes) await ctx.db!.delete(scenes)
.where(eq(scenes.projectId, input.id)); .where(eq(scenes.projectId, input.id));
// M2 fix: remove project members
await ctx.db!.delete(projectMembers)
.where(eq(projectMembers.projectId, input.id));
// Get character IDs for this project // Get character IDs for this project
const projectCharacters = await ctx.db!.select({ id: characters.id }) const projectCharacters = await ctx.db!.select({ id: characters.id })
.from(characters) .from(characters)
@@ -544,7 +548,7 @@ export const projectRouter = {
) )
); );
if (existing.length > 0) { if (existing.length > 0) {
throw new Error('Relationship already exists between these characters'); throw new TRPCError({ code: 'CONFLICT', message: 'Relationship already exists between these characters' });
} }
const result = await ctx.db!.insert(characterRelationships) const result = await ctx.db!.insert(characterRelationships)

View File

@@ -59,11 +59,18 @@ describe('revisionsRouter', () => {
}); });
describe('listRevisions', () => { describe('listRevisions', () => {
it('should return empty array for unknown script', async () => { it('should return empty array for script with no revisions', async () => {
const result = await caller.revisions.listRevisions({ scriptId: 999 }); const result = await caller.revisions.listRevisions({ scriptId: 1 });
expect(result).toEqual([]); expect(result).toEqual([]);
}); });
it('should throw NOT_FOUND for unknown script', async () => {
const { TRPCError } = await import('./router');
await expect(
caller.revisions.listRevisions({ scriptId: 999 })
).rejects.toThrow(TRPCError);
});
it('should filter by branch', async () => { it('should filter by branch', async () => {
await caller.revisions.createRevision({ await caller.revisions.createRevision({
scriptId: 1, scriptId: 1,

View File

@@ -5,8 +5,71 @@ import type { DrizzleDB } from '../../src/db/config/migrations';
import { import {
revisions, revisions,
revisionChanges, revisionChanges,
scripts,
projects,
projectMembers,
} from '../../src/db/schema'; } from '../../src/db/schema';
// H1 fix: verifies user has access to the project owning the script a revision belongs to
async function verifyScriptAccess(
db: DrizzleDB,
scriptId: number,
userId: number
) {
const scriptRows = await db
.select({ id: scripts.id, projectId: scripts.projectId })
.from(scripts)
.where(eq(scripts.id, scriptId));
const script = scriptRows[0];
if (!script) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Script ${scriptId} not found` });
}
const projectRows = await db
.select({ id: projects.id, ownerId: projects.ownerId })
.from(projects)
.where(eq(projects.id, script.projectId));
const project = projectRows[0];
if (!project) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Project ${script.projectId} not found` });
}
if (project.ownerId === userId) return { script, project };
const memberRows = await db
.select()
.from(projectMembers)
.where(and(eq(projectMembers.projectId, script.projectId), eq(projectMembers.userId, userId)));
if (memberRows.length === 0) {
throw new TRPCError({ code: 'FORBIDDEN', message: `You do not have access to script ${scriptId}` });
}
return { script, project };
}
// Resolves revision → script → project and verifies user access
async function verifyRevisionAccess(
db: DrizzleDB,
revisionId: number,
userId: number
) {
const revisionRows = await db
.select()
.from(revisions)
.where(eq(revisions.id, revisionId));
const revision = revisionRows[0];
if (!revision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${revisionId} not found` });
}
const { script, project } = await verifyScriptAccess(db, revision.scriptId, userId);
return { revision, script, project };
}
function computeDiffForContent( function computeDiffForContent(
db: DrizzleDB, db: DrizzleDB,
oldContent: string, oldContent: string,
@@ -94,8 +157,9 @@ export const revisionsRouter = {
branchName: z.string().optional(), branchName: z.string().optional(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
const conditions = [eq(revisions.scriptId, input.scriptId)]; const conditions = [eq(revisions.scriptId, input.scriptId)];
if (input.branchName) { if (input.branchName) {
conditions.push(eq(revisions.branchName, input.branchName)); conditions.push(eq(revisions.branchName, input.branchName));
} }
@@ -105,7 +169,7 @@ export const revisionsRouter = {
.from(revisions) .from(revisions)
.where(and(...conditions)) .where(and(...conditions))
.orderBy(desc(revisions.versionNumber)); .orderBy(desc(revisions.versionNumber));
return results; return results;
}), }),
@@ -114,15 +178,7 @@ export const revisionsRouter = {
id: z.number().int().positive(), id: z.number().int().positive(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
const rows = await ctx.db! const { revision } = await verifyRevisionAccess(ctx.db!, input.id, ctx.userId!);
.select()
.from(revisions)
.where(eq(revisions.id, input.id));
const revision = rows[0];
if (!revision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.id} not found` });
}
return revision; return revision;
}), }),
@@ -131,7 +187,7 @@ export const revisionsRouter = {
scriptId: z.number().int().positive(), scriptId: z.number().int().positive(),
title: z.string().min(1).max(255), title: z.string().min(1).max(255),
summary: z.string().max(2000).optional(), summary: z.string().max(2000).optional(),
content: z.string(), content: z.string().max(100000),
branchName: z.string().default('main'), branchName: z.string().default('main'),
parentRevisionId: z.number().int().positive().optional(), parentRevisionId: z.number().int().positive().optional(),
})) }))
@@ -139,6 +195,7 @@ export const revisionsRouter = {
if (!ctx.userId) { if (!ctx.userId) {
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
} }
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
const nextVersion = await getLatestVersionForScript( const nextVersion = await getLatestVersionForScript(
ctx.db, ctx.db,
@@ -193,14 +250,7 @@ export const revisionsRouter = {
status: z.enum(['draft', 'pending_review', 'accepted', 'rejected']).optional(), status: z.enum(['draft', 'pending_review', 'accepted', 'rejected']).optional(),
})) }))
.mutation(async ({ input, ctx }) => { .mutation(async ({ input, ctx }) => {
const result = await ctx.db const { revision } = await verifyRevisionAccess(ctx.db!, input.id, ctx.userId!);
.select()
.from(revisions)
.where(eq(revisions.id, input.id));
const revision = result[0];
if (!revision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.id} not found` });
}
const updated = await ctx.db const updated = await ctx.db
.update(revisions) .update(revisions)
@@ -222,15 +272,13 @@ export const revisionsRouter = {
id: z.number().int().positive(), id: z.number().int().positive(),
})) }))
.mutation(async ({ input, ctx }) => { .mutation(async ({ input, ctx }) => {
await verifyRevisionAccess(ctx.db!, input.id, ctx.userId!);
const result = await ctx.db const result = await ctx.db
.delete(revisions) .delete(revisions)
.where(eq(revisions.id, input.id)) .where(eq(revisions.id, input.id))
.returning(); .returning();
if (result.length === 0) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.id} not found` });
}
await ctx.db await ctx.db
.delete(revisionChanges) .delete(revisionChanges)
.where(eq(revisionChanges.revisionId, input.id)); .where(eq(revisionChanges.revisionId, input.id));
@@ -243,14 +291,7 @@ export const revisionsRouter = {
revisionId: z.number().int().positive(), revisionId: z.number().int().positive(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
const revisionResult = await ctx.db await verifyRevisionAccess(ctx.db!, input.revisionId, ctx.userId!);
.select()
.from(revisions)
.where(eq(revisions.id, input.revisionId));
const revision = revisionResult[0];
if (!revision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.revisionId} not found` });
}
const changes = await ctx.db const changes = await ctx.db
.select() .select()
@@ -267,24 +308,8 @@ export const revisionsRouter = {
targetRevisionId: z.number().int().positive(), targetRevisionId: z.number().int().positive(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
const baseResult = await ctx.db const { revision: baseRevision } = await verifyRevisionAccess(ctx.db!, input.baseRevisionId, ctx.userId!);
.select() const { revision: targetRevision } = await verifyRevisionAccess(ctx.db!, input.targetRevisionId, ctx.userId!);
.from(revisions)
.where(eq(revisions.id, input.baseRevisionId));
const baseRevision = baseResult[0];
const targetResult = await ctx.db
.select()
.from(revisions)
.where(eq(revisions.id, input.targetRevisionId));
const targetRevision = targetResult[0];
if (!baseRevision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Base revision ${input.baseRevisionId} not found` });
}
if (!targetRevision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Target revision ${input.targetRevisionId} not found` });
}
const oldLines = baseRevision.content.split('\n'); const oldLines = baseRevision.content.split('\n');
const newLines = targetRevision.content.split('\n'); const newLines = targetRevision.content.split('\n');
@@ -328,15 +353,7 @@ export const revisionsRouter = {
if (!ctx.userId) { if (!ctx.userId) {
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
} }
await verifyRevisionAccess(ctx.db!, input.revisionId, ctx.userId!);
const result = await ctx.db
.select()
.from(revisions)
.where(eq(revisions.id, input.revisionId));
const revision = result[0];
if (!revision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.revisionId} not found` });
}
const updated = await ctx.db const updated = await ctx.db
.update(revisions) .update(revisions)
@@ -361,15 +378,7 @@ export const revisionsRouter = {
if (!ctx.userId) { if (!ctx.userId) {
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
} }
const { revision } = await verifyRevisionAccess(ctx.db!, input.revisionId, ctx.userId!);
const result = await ctx.db
.select()
.from(revisions)
.where(eq(revisions.id, input.revisionId));
const revision = result[0];
if (!revision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.revisionId} not found` });
}
const newSummary = input.reason const newSummary = input.reason
? (revision.summary || '') + '\n[Rejected: ' + input.reason + ']' ? (revision.summary || '') + '\n[Rejected: ' + input.reason + ']'
@@ -399,15 +408,8 @@ export const revisionsRouter = {
if (!ctx.userId) { if (!ctx.userId) {
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
} }
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
const targetResult = await ctx.db const { revision: targetRevision } = await verifyRevisionAccess(ctx.db!, input.revisionId, ctx.userId!);
.select()
.from(revisions)
.where(eq(revisions.id, input.revisionId));
const targetRevision = targetResult[0];
if (!targetRevision) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Revision ${input.revisionId} not found` });
}
if (targetRevision.scriptId !== input.scriptId) { if (targetRevision.scriptId !== input.scriptId) {
throw new TRPCError({ code: 'BAD_REQUEST', message: 'Revision does not belong to the specified script' }); throw new TRPCError({ code: 'BAD_REQUEST', message: 'Revision does not belong to the specified script' });
@@ -449,6 +451,7 @@ export const revisionsRouter = {
scriptId: z.number().int().positive(), scriptId: z.number().int().positive(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
const scriptRevisions = await ctx.db const scriptRevisions = await ctx.db
.select() .select()
.from(revisions) .from(revisions)
@@ -484,6 +487,7 @@ export const revisionsRouter = {
scriptId: z.number().int().positive(), scriptId: z.number().int().positive(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
const scriptRevisions = await ctx.db const scriptRevisions = await ctx.db
.select() .select()
.from(revisions) .from(revisions)
@@ -522,6 +526,7 @@ export const revisionsRouter = {
if (!ctx.userId) { if (!ctx.userId) {
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
} }
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
const existingResult = await ctx.db const existingResult = await ctx.db
.select({ id: revisions.id }) .select({ id: revisions.id })
@@ -601,6 +606,7 @@ export const revisionsRouter = {
if (!ctx.userId) { if (!ctx.userId) {
throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' }); throw new TRPCError({ code: 'UNAUTHORIZED', message: 'User not authenticated' });
} }
await verifyScriptAccess(ctx.db!, input.scriptId, ctx.userId!);
if (input.sourceBranch === input.targetBranch) { if (input.sourceBranch === input.targetBranch) {
throw new TRPCError({ code: 'BAD_REQUEST', message: 'Cannot merge a branch into itself' }); throw new TRPCError({ code: 'BAD_REQUEST', message: 'Cannot merge a branch into itself' });

View File

@@ -1,7 +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 { eq, and } from 'drizzle-orm';
import { projects } from '../../src/db/schema'; import { projects, projectMembers } from '../../src/db/schema';
import type { TRPCContext } from './types'; import type { TRPCContext } from './types';
// Initialize tRPC with context // Initialize tRPC with context
@@ -61,7 +61,14 @@ const hasProjectAccess = t.middleware(async ({ ctx, next }) => {
if (!project) { if (!project) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Project ${ctx.projectId} not found` }); throw new TRPCError({ code: 'NOT_FOUND', message: `Project ${ctx.projectId} not found` });
} }
if (project.ownerId !== dbUser.id) { if (project.ownerId === dbUser.id) {
return next({ ctx: { ...ctx, projectId: ctx.projectId, userId: dbUser.id } });
}
// L3 fix: also check project membership
const memberRows = await ctx.db.select()
.from(projectMembers)
.where(and(eq(projectMembers.projectId, ctx.projectId), eq(projectMembers.userId, dbUser.id)));
if (memberRows.length === 0) {
throw new TRPCError({ code: 'FORBIDDEN', message: `You do not have access to project ${ctx.projectId}` }); throw new TRPCError({ code: 'FORBIDDEN', message: `You do not have access to project ${ctx.projectId}` });
} }
return next({ ctx: { ...ctx, projectId: ctx.projectId, userId: dbUser.id } }); return next({ ctx: { ...ctx, projectId: ctx.projectId, userId: dbUser.id } });

View File

@@ -1,4 +1,4 @@
import { protectedProcedure } from './router'; import { protectedProcedure, TRPCError } from './router';
import { z } from 'zod'; import { z } from 'zod';
import { eq, and, like, sql, inArray } from 'drizzle-orm'; import { eq, and, like, sql, inArray } from 'drizzle-orm';
import type { DrizzleDB } from '../../src/db/config/migrations'; import type { DrizzleDB } from '../../src/db/config/migrations';
@@ -7,12 +7,43 @@ import {
revisions, revisions,
revisionChanges, revisionChanges,
projects, projects,
projectMembers,
} from '../../src/db/schema'; } from '../../src/db/schema';
function slugify(title: string): string { function slugify(title: string): string {
return title.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/(^-|-$)/g, ''); return title.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/(^-|-$)/g, '');
} }
// H2 fix: verifies user has access to a project (owner or member)
async function verifyProjectAccess(
db: DrizzleDB,
projectId: number,
userId: number
) {
const projectRows = await db
.select({ id: projects.id, ownerId: projects.ownerId })
.from(projects)
.where(eq(projects.id, projectId));
const project = projectRows[0];
if (!project) {
throw new TRPCError({ code: 'NOT_FOUND', message: `Project ${projectId} not found` });
}
if (project.ownerId === userId) return project;
const memberRows = await db
.select()
.from(projectMembers)
.where(and(eq(projectMembers.projectId, projectId), eq(projectMembers.userId, userId)));
if (memberRows.length === 0) {
throw new TRPCError({ code: 'FORBIDDEN', message: `You do not have access to project ${projectId}` });
}
return project;
}
async function verifyScriptOwnership( async function verifyScriptOwnership(
db: DrizzleDB, db: DrizzleDB,
scriptId: number, scriptId: number,
@@ -24,18 +55,11 @@ async function verifyScriptOwnership(
const script = scriptRows[0]; const script = scriptRows[0];
if (!script) { if (!script) {
throw new Error(`Script ${scriptId} not found`); throw new TRPCError({ code: 'NOT_FOUND', message: `Script ${scriptId} not found` });
} }
const projectRows = await db.select({ ownerId: projects.ownerId }) await verifyProjectAccess(db, script.projectId, userId);
.from(projects)
.where(eq(projects.id, script.projectId));
const project = projectRows[0];
if (!project || project.ownerId !== userId) {
throw new Error(`You do not have access to script ${scriptId}`);
}
return script; return script;
} }
@@ -43,10 +67,8 @@ export const scriptsRouter = {
listScripts: protectedProcedure listScripts: protectedProcedure
.input(z.object({ projectId: z.number().int().positive() })) .input(z.object({ projectId: z.number().int().positive() }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
await ctx.db!.select() await verifyProjectAccess(ctx.db!, input.projectId, ctx.userId!);
.from(projects)
.where(eq(projects.id, input.projectId));
return await ctx.db!.select() return await ctx.db!.select()
.from(scripts) .from(scripts)
.where(eq(scripts.projectId, input.projectId)) .where(eq(scripts.projectId, input.projectId))
@@ -56,14 +78,14 @@ export const scriptsRouter = {
getScript: protectedProcedure getScript: protectedProcedure
.input(z.object({ id: z.number().int().positive() })) .input(z.object({ id: z.number().int().positive() }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
await verifyScriptOwnership(ctx.db!, input.id, ctx.userId!);
const rows = await ctx.db!.select() const rows = await ctx.db!.select()
.from(scripts) .from(scripts)
.where(eq(scripts.id, input.id)); .where(eq(scripts.id, input.id));
const script = rows[0]; const script = rows[0];
if (!script) { if (!script) {
throw new Error(`Script ${input.id} not found`); throw new TRPCError({ code: 'NOT_FOUND', message: `Script ${input.id} not found` });
} }
await verifyScriptOwnership(ctx.db!, input.id, ctx.userId!);
return script; return script;
}), }),
@@ -76,12 +98,7 @@ export const scriptsRouter = {
status: z.enum(['draft', 'revision', 'final', 'published']).optional(), status: z.enum(['draft', 'revision', 'final', 'published']).optional(),
})) }))
.mutation(async ({ input, ctx }) => { .mutation(async ({ input, ctx }) => {
const projectRows = await ctx.db!.select() await verifyProjectAccess(ctx.db!, input.projectId, ctx.userId!);
.from(projects)
.where(eq(projects.id, input.projectId));
if (!projectRows[0]) {
throw new Error(`Project ${input.projectId} not found`);
}
const result = await ctx.db!.insert(scripts) const result = await ctx.db!.insert(scripts)
.values({ .values({
@@ -156,9 +173,7 @@ export const scriptsRouter = {
query: z.string().optional(), query: z.string().optional(),
})) }))
.query(async ({ input, ctx }) => { .query(async ({ input, ctx }) => {
await ctx.db!.select() await verifyProjectAccess(ctx.db!, input.projectId, ctx.userId!);
.from(projects)
.where(eq(projects.id, input.projectId));
const conditions: any[] = [eq(scripts.projectId, input.projectId)]; const conditions: any[] = [eq(scripts.projectId, input.projectId)];

View File

@@ -54,8 +54,9 @@ async function verifyTeamRole(
} }
} }
function generateTeamId(): string { async function generateTeamId(): Promise<string> {
return `team_${Date.now()}_${Math.random().toString(36).slice(2, 9)}`; const { randomUUID } = await import('crypto');
return `team_${randomUUID()}`;
} }
export const teamRouter = { export const teamRouter = {
@@ -101,7 +102,7 @@ export const teamRouter = {
name: z.string().min(1).max(255), name: z.string().min(1).max(255),
})) }))
.mutation(async ({ input, ctx }) => { .mutation(async ({ input, ctx }) => {
const teamId = generateTeamId(); const teamId = await generateTeamId();
const result = await ctx.db!.insert(teams) const result = await ctx.db!.insert(teams)
.values({ .values({
id: teamId, id: teamId,

View File

@@ -7,7 +7,7 @@ export const scripts = sqliteTable("scripts", {
.notNull() .notNull()
.references(() => projects.id), .references(() => projects.id),
title: text("title").notNull(), title: text("title").notNull(),
slug: text("slug").notNull(), slug: text("slug").notNull().unique(),
genre: text("genre"), genre: text("genre"),
logline: text("logline"), logline: text("logline"),
status: text("status", { enum: ["draft", "revision", "final", "published"] }).notNull().default("draft"), status: text("status", { enum: ["draft", "revision", "final", "published"] }).notNull().default("draft"),