Fix CVE-2025-63783: Add authorization checks to project mutation APIs#3062
Fix CVE-2025-63783: Add authorization checks to project mutation APIs#3062
Conversation
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 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded a new exported function, Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| }, | ||
| }); | ||
|
|
||
| if (!project) { |
There was a problem hiding this comment.
Consider using a uniform error response for both missing projects and unauthorized access to avoid disclosing whether a project exists. For example, both conditions could throw a generic unauthorized error.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/server/api/routers/project/project.ts (1)
58-174: Critical: Missing authorization check in captureScreenshot mutation.The
captureScreenshotmutation accepts aprojectIdand updates the project's preview image (lines 159-167) without verifying the user has access to that project. This is the same BOLA vulnerability pattern that CVE-2025-63783 addresses in other mutations.Apply this diff to add the authorization check:
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) {
🧹 Nitpick comments (2)
apps/web/client/src/server/api/routers/project/project.ts (2)
385-404: Optimize: Redundant database query after authorization check.The
addTagmutation queries the project at lines 386-388 afterverifyProjectAccessalready fetched and validated it at line 385. The subsequent null check at lines 390-392 is also redundant sinceverifyProjectAccessthrows if the project doesn't exist.Consider refactoring
verifyProjectAccessto return the project or modifying this mutation to reuse the already-fetched data:addTag: protectedProcedure.input(z.object({ 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), - }); - - if (!project) { - throw new Error('Project not found'); - } - - const currentTags = project.tags ?? []; + + // Fetch only the tags field since access is already verified + const [project] = await ctx.db.select({ tags: projects.tags }) + .from(projects) + .where(eq(projects.id, input.projectId)); + + const currentTags = project?.tags ?? []; const newTags = currentTags.includes(input.tag) ? currentTags : [...currentTags, input.tag];
410-428: Optimize: Redundant database query after authorization check.The
removeTagmutation queries the project at lines 411-413 afterverifyProjectAccessalready fetched and validated it at line 410. The subsequent null check at lines 415-417 is also redundant.Apply the same optimization as suggested for
addTag:removeTag: protectedProcedure.input(z.object({ 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), - }); - - if (!project) { - throw new Error('Project not found'); - } - - const currentTags = project.tags ?? []; + + // Fetch only the tags field since access is already verified + const [project] = await ctx.db.select({ tags: projects.tags }) + .from(projects) + .where(eq(projects.id, input.projectId)); + + const currentTags = project?.tags ?? []; const newTags = currentTags.filter(tag => tag !== input.tag);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/server/api/routers/project/helper.ts(2 hunks)apps/web/client/src/server/api/routers/project/project.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
🧠 Learnings (6)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Place tRPC routers under apps/web/client/src/server/api/routers/**
Applied to files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Place tRPC routers under src/server/api/routers/**
Applied to files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from apps/web/client/src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/server/api/routers/project/helper.ts
🧬 Code graph analysis (2)
apps/web/client/src/server/api/routers/project/project.ts (1)
apps/web/client/src/server/api/routers/project/helper.ts (1)
verifyProjectAccess(26-47)
apps/web/client/src/server/api/routers/project/helper.ts (3)
packages/db/src/client.ts (1)
db(16-16)packages/db/src/schema/project/project.ts (1)
projects(13-32)packages/db/src/schema/user/user-project.ts (1)
userProjects(10-23)
🔇 Additional comments (4)
apps/web/client/src/server/api/routers/project/project.ts (3)
40-40: LGTM: Authorization helper imported correctly.The import of
verifyProjectAccessenables consistent authorization checks across project mutations.
351-351: LGTM: Authorization check properly guards delete mutation.The authorization check correctly verifies user access before allowing project deletion.
369-369: LGTM: Authorization check properly guards update mutation.The authorization check correctly verifies user access before allowing project updates.
apps/web/client/src/server/api/routers/project/helper.ts (1)
1-2: LGTM: Imports are correct for the new authorization function.The imports provide the necessary Drizzle ORM utilities and schema types for querying user project access.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @drfarrell. * #3062 (comment) The following files were modified: * `apps/web/client/src/server/api/routers/project/helper.ts`
…losure 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/client/src/server/api/routers/project/project.ts (2)
349-357: Avoid TOCTOU indelete: perform access check inside the same transaction and prefer deleting junction rows first.
You currently verify withctx.dband then mutate withtx. A membership change between those steps can bypass the intended “authorize + mutate atomically” guarantee. Also, deletinguserProjectsfirst is generally safer if FKs don’t cascade..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)); - }); + await ctx.db.transaction(async (tx) => { + 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)); + }); });
58-65: MoveverifyProjectAccessinside the try/catch to ensure consistent error responses.The
captureScreenshotprocedure returns{ success: true/false, error?: string }, butverifyProjectAccessthrows outside thetry/catchblock. Unauthorized calls will throw a tRPC error instead of the expected{ success: false, error: "..." }response, breaking the boolean response contract. Move the access check inside thetryblock to handle all error cases consistently.Also applies to: 171-174 (the catch block)
🧹 Nitpick comments (3)
apps/web/client/src/server/api/routers/project/project.ts (3)
40-40: Import looks fine; watch for circular deps and keep auth helpers throwing typed tRPC errors.
AddingverifyProjectAccessis the right direction, but since it currently throws a plainError, it will typically surface as an INTERNAL_SERVER_ERROR unless you map it elsewhere. Consider switching the helper to throwTRPCError(e.g.,FORBIDDENorNOT_FOUNDwith a generic message) so clients get a consistent, intentional status.+import { TRPCError } from '@trpc/server'; // ... export async function verifyProjectAccess(/* ... */) { // ... if (!project || project.userProjects.length === 0) { - throw new Error('Unauthorized or not found'); + throw new TRPCError({ code: 'FORBIDDEN', message: 'Unauthorized or not found' }); } }
369-381:updateauthorization is present; consider making it single-roundtrip/atomic (authorize-in-WHERE) to prevent TOCTOU.
Current pattern is “verify then update”. The stronger pattern is “update where projectId = … AND user is a member”, then treat 0 updated rows as unauthorized/not found.
382-406:addTag/removeTagfixed for BOLA; remaining risk is lost updates due to read-modify-write.
Two concurrent tag updates can clobber each other because you read tags, compute, then write. If this endpoint is user-facing and can be hit concurrently, consider an atomic update strategy (or optimistic concurrency usingupdatedAtin the WHERE and retry on 0 rows).Also applies to: 407-429
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/server/api/routers/project/helper.ts(2 hunks)apps/web/client/src/server/api/routers/project/project.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/server/api/routers/project/helper.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/project/project.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/server/api/routers/project/project.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/server/api/routers/project/project.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/server/api/routers/project/project.ts
🧠 Learnings (6)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Place tRPC routers under apps/web/client/src/server/api/routers/**
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Place tRPC routers under src/server/api/routers/**
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from apps/web/client/src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/routers/project/project.ts
🧬 Code graph analysis (1)
apps/web/client/src/server/api/routers/project/project.ts (1)
apps/web/client/src/server/api/routers/project/helper.ts (1)
verifyProjectAccess(30-47)
…captureScreenshot
- 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 <noreply@anthropic.com>
1 participant
Summary
This PR fixes CVE-2025-63783, a critical Broken Object Level Authorization (BOLA) vulnerability in the tRPC project mutation APIs.
Vulnerability Details
CVE-2025-63783 allowed authenticated users to:
The vulnerability existed because the
delete,update,addTag, andremoveTagmutations did not verify that the authenticated user had access to the project before performing operations.Changes Made
Added
verifyProjectAccess()helper function (apps/web/client/src/server/api/routers/project/helper.ts:26)userProjectsjunction tableAdded authorization checks to all vulnerable mutations:
deletemutation (project.ts:351)updatemutation (project.ts:369)addTagmutation (project.ts:385)removeTagmutation (project.ts:410)Security Impact
userProjectstable)Testing
userProjectsjunction tableReferences
🤖 Generated with Claude Code
Important
Fixes CVE-2025-63783 by adding authorization checks to project mutation APIs, ensuring only authorized users can modify or delete projects.
verifyProjectAccess()inhelper.tsto check user access viauserProjectstable.delete,update,addTag, andremoveTagmutations inproject.ts.This description was created by
for 6138b0c. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.