From 6fb93b35270b225b64e816f6092a9ca4cce7320b Mon Sep 17 00:00:00 2001 From: Daniel R Farrell Date: Sat, 13 Dec 2025 22:57:04 -0800 Subject: [PATCH 1/3] Fix CVE-2025-63783: Add authorization checks to project mutation APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses a critical Broken Object Level Authorization (BOLA) vulnerability where authenticated users could modify, delete, or manipulate tags on projects they don't own by sending requests with arbitrary project IDs. Changes: - Add verifyProjectAccess() helper function to verify user project membership - Add authorization checks to delete, update, addTag, and removeTag mutations - Ensure all project mutations verify ownership before performing operations The fix validates that the authenticated user has access to the project via the userProjects junction table before allowing any mutation operations. Security Impact: Prevents unauthorized users from modifying or deleting projects that don't belong to them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/server/api/routers/project/helper.ts | 30 ++++++++++++++++++- .../src/server/api/routers/project/project.ts | 6 +++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/apps/web/client/src/server/api/routers/project/helper.ts b/apps/web/client/src/server/api/routers/project/helper.ts index e2f6037378..2216ffaf47 100644 --- a/apps/web/client/src/server/api/routers/project/helper.ts +++ b/apps/web/client/src/server/api/routers/project/helper.ts @@ -1,4 +1,5 @@ -import { type Frame } from "@onlook/db"; +import { eq } from "drizzle-orm"; +import { type Frame, projects, userProjects, type DrizzleDb } from "@onlook/db"; export function extractCsbPort(frames: Frame[]): number | null { if (!frames || frames.length === 0) return null; @@ -17,3 +18,30 @@ export function extractCsbPort(frames: Frame[]): number | null { } return null; } + +/** + * Verifies that a user has access to a project by checking the userProjects table. + * @throws Error if the user does not have access to the project + */ +export async function verifyProjectAccess( + db: DrizzleDb, + userId: string, + projectId: string, +): Promise { + const project = await db.query.projects.findFirst({ + where: eq(projects.id, projectId), + with: { + userProjects: { + where: eq(userProjects.userId, userId), + }, + }, + }); + + if (!project) { + throw new Error('Project not found'); + } + + if (project.userProjects.length === 0) { + throw new Error('Unauthorized: You do not have access to this project'); + } +} diff --git a/apps/web/client/src/server/api/routers/project/project.ts b/apps/web/client/src/server/api/routers/project/project.ts index a110db5bbf..2747860a92 100644 --- a/apps/web/client/src/server/api/routers/project/project.ts +++ b/apps/web/client/src/server/api/routers/project/project.ts @@ -37,7 +37,7 @@ import { and, eq, ne } from 'drizzle-orm'; import { z } from 'zod'; import { projectCreateRequestRouter } from './createRequest'; import { fork } from './fork'; -import { extractCsbPort } from './helper'; +import { extractCsbPort, verifyProjectAccess } from './helper'; export const projectRouter = createTRPCRouter({ hasAccess: protectedProcedure @@ -348,6 +348,7 @@ export const projectRouter = createTRPCRouter({ delete: protectedProcedure .input(z.object({ id: z.string() })) .mutation(async ({ ctx, input }) => { + await verifyProjectAccess(ctx.db, ctx.user.id, input.id); await ctx.db.transaction(async (tx) => { await tx.delete(projects).where(eq(projects.id, input.id)); await tx.delete(userProjects).where(eq(userProjects.projectId, input.id)); @@ -365,6 +366,7 @@ export const projectRouter = createTRPCRouter({ return projects.map((project) => fromDbProject(project.project)); }), update: protectedProcedure.input(projectUpdateSchema).mutation(async ({ ctx, input }) => { + await verifyProjectAccess(ctx.db, ctx.user.id, input.id); const [updatedProject] = await ctx.db.update(projects).set({ ...input, updatedAt: new Date(), @@ -380,6 +382,7 @@ export const projectRouter = createTRPCRouter({ projectId: z.string(), tag: z.string(), })).mutation(async ({ ctx, input }) => { + await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId); const project = await ctx.db.query.projects.findFirst({ where: eq(projects.id, input.projectId), }); @@ -404,6 +407,7 @@ export const projectRouter = createTRPCRouter({ projectId: z.string(), tag: z.string(), })).mutation(async ({ ctx, input }) => { + await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId); const project = await ctx.db.query.projects.findFirst({ where: eq(projects.id, input.projectId), }); From ccf91a639669abf37cd96041d9f5203061b2d8ae Mon Sep 17 00:00:00 2001 From: Daniel R Farrell Date: Sat, 13 Dec 2025 23:07:10 -0800 Subject: [PATCH 2/3] Add authorization check to captureScreenshot and fix information disclosure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additional security improvements to CVE-2025-63783 fix: 1. Add authorization check to captureScreenshot mutation - The captureScreenshot endpoint was missing the authorization check - This prevented unauthorized users from updating project preview images 2. Fix information disclosure in verifyProjectAccess helper - Previously returned different error messages for "Project not found" vs "Unauthorized access", which leaked project existence information - Now returns single generic error "Unauthorized or not found" for both cases to prevent enumeration attacks These changes ensure complete protection against BOLA attacks on project mutations and prevent attackers from discovering valid project IDs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/server/api/routers/project/helper.ts | 14 +++++++------- .../src/server/api/routers/project/project.ts | 1 + 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/apps/web/client/src/server/api/routers/project/helper.ts b/apps/web/client/src/server/api/routers/project/helper.ts index 2216ffaf47..08a6927f53 100644 --- a/apps/web/client/src/server/api/routers/project/helper.ts +++ b/apps/web/client/src/server/api/routers/project/helper.ts @@ -21,7 +21,11 @@ export function extractCsbPort(frames: Frame[]): number | null { /** * Verifies that a user has access to a project by checking the userProjects table. - * @throws Error if the user does not have access to the project + * @throws Error if the user does not have access to the project or if it doesn't exist + * + * Note: This function intentionally returns the same error message whether the project + * doesn't exist or the user lacks access to prevent information disclosure about + * project existence. */ export async function verifyProjectAccess( db: DrizzleDb, @@ -37,11 +41,7 @@ export async function verifyProjectAccess( }, }); - if (!project) { - throw new Error('Project not found'); - } - - if (project.userProjects.length === 0) { - throw new Error('Unauthorized: You do not have access to this project'); + if (!project || project.userProjects.length === 0) { + throw new Error('Unauthorized or not found'); } } diff --git a/apps/web/client/src/server/api/routers/project/project.ts b/apps/web/client/src/server/api/routers/project/project.ts index 2747860a92..074b64c1bf 100644 --- a/apps/web/client/src/server/api/routers/project/project.ts +++ b/apps/web/client/src/server/api/routers/project/project.ts @@ -58,6 +58,7 @@ export const projectRouter = createTRPCRouter({ captureScreenshot: protectedProcedure .input(z.object({ projectId: z.string() })) .mutation(async ({ ctx, input }) => { + await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId); try { if (!env.FIRECRAWL_API_KEY) { throw new Error('FIRECRAWL_API_KEY is not configured'); From 6138b0cb2199d042236b3cc11964a35a5744ec7c Mon Sep 17 00:00:00 2001 From: Daniel R Farrell Date: Sat, 13 Dec 2025 23:20:54 -0800 Subject: [PATCH 3/3] Fix TOCTOU race condition in delete and consistent error handling in captureScreenshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move verifyProjectAccess inside transaction for delete mutation to ensure atomic authorization + mutation (prevents race between access check and delete) - Reorder delete operations: userProjects first, then projects (safer FK handling) - Move verifyProjectAccess inside try/catch for captureScreenshot to maintain consistent { success: false, error: string } response contract - Update verifyProjectAccess to accept both db and transaction types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- apps/web/client/src/server/api/routers/project/helper.ts | 7 ++++++- apps/web/client/src/server/api/routers/project/project.ts | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/web/client/src/server/api/routers/project/helper.ts b/apps/web/client/src/server/api/routers/project/helper.ts index 08a6927f53..b522900dd5 100644 --- a/apps/web/client/src/server/api/routers/project/helper.ts +++ b/apps/web/client/src/server/api/routers/project/helper.ts @@ -1,6 +1,9 @@ import { eq } from "drizzle-orm"; import { type Frame, projects, userProjects, type DrizzleDb } from "@onlook/db"; +/** Type representing a db instance or transaction that has query capabilities */ +type DbOrTx = Pick; + export function extractCsbPort(frames: Frame[]): number | null { if (!frames || frames.length === 0) return null; @@ -26,9 +29,11 @@ export function extractCsbPort(frames: Frame[]): number | null { * Note: This function intentionally returns the same error message whether the project * doesn't exist or the user lacks access to prevent information disclosure about * project existence. + * + * Accepts either a db instance or a transaction to support atomic authorization checks. */ export async function verifyProjectAccess( - db: DrizzleDb, + db: DbOrTx, userId: string, projectId: string, ): Promise { diff --git a/apps/web/client/src/server/api/routers/project/project.ts b/apps/web/client/src/server/api/routers/project/project.ts index 074b64c1bf..bfece724ee 100644 --- a/apps/web/client/src/server/api/routers/project/project.ts +++ b/apps/web/client/src/server/api/routers/project/project.ts @@ -58,8 +58,8 @@ export const projectRouter = createTRPCRouter({ captureScreenshot: protectedProcedure .input(z.object({ projectId: z.string() })) .mutation(async ({ ctx, input }) => { - await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId); try { + await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId); if (!env.FIRECRAWL_API_KEY) { throw new Error('FIRECRAWL_API_KEY is not configured'); } @@ -349,10 +349,10 @@ export const projectRouter = createTRPCRouter({ delete: protectedProcedure .input(z.object({ id: z.string() })) .mutation(async ({ ctx, input }) => { - await verifyProjectAccess(ctx.db, ctx.user.id, input.id); await ctx.db.transaction(async (tx) => { - await tx.delete(projects).where(eq(projects.id, input.id)); + await verifyProjectAccess(tx, ctx.user.id, input.id); await tx.delete(userProjects).where(eq(userProjects.projectId, input.id)); + await tx.delete(projects).where(eq(projects.id, input.id)); }); }), getPreviewProjects: protectedProcedure