Skip to content

Commit 423e2e9

Browse files
mrptatoclaude
mrptato
and
claude
authored
fix(security): enforce project-membership authorization across all tRPC routers (IDOR) (#3129)
Closes #3122. The Drizzle client connects as an RLS-exempt Postgres superuser, so authorization must be enforced in tRPC procedure code. `verifyProjectAccess` existed but was applied to only a handful of procedures; every other project-scoped procedure trusted a client-supplied id (projectId / conversationId / branchId / sandboxId / deploymentId / verificationId / ...), so an authenticated user could read or mutate another user's data. This audits the whole tRPC surface and closes it with one resolve-then-verify pattern, all sharing a merged "Unauthorized or not found" error so the checks can't be used to enumerate resource existence. Helpers (project/helper.ts): - verifyProjectAccess (existing) + verifyConversationAccess, verifyMessagesAccess, verifyBranchAccess, verifyCanvasAccess, verifyFrameAccess, verifyInvitationAccess - verifySandboxAccess — resolves sandbox -> branch/project; a sandbox not yet tied to a project (fresh create/fork/template/import, before a branch row exists) is allowed so blank-project / local-import / fork flows keep working - verifyDeploymentAccess, verifyDomainVerificationAccess - listAccessibleSandboxIds — scopes sandbox.list (whose provider call returns the whole account) to the caller's own sandboxes Routers hardened: project, chat (conversation/message/suggestion), branch, frame, settings, createRequest, sandbox, publish (deployment + unpublish), domain (preview/custom/verification), user (getById self-only, upsert pinned to session), subscription, usage, user-canvas, user-settings. Also: auth checks moved out of catch-and-return-false blocks so denials propagate as errors; verifyMessagesAccess dedupes ids so a bulk op with a repeated id isn't falsely rejected; getPreviewProjects throws TRPCError. Adds unit tests for the authorization helpers (project/helper.test.ts, 19 cases). Web-client typecheck passes. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ab57c09 commit 423e2e9

25 files changed

Lines changed: 673 additions & 26 deletions

‎apps/web/client/src/server/api/routers/chat/conversation.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ import { eq } from 'drizzle-orm';
1111
import { v4 as uuidv4 } from 'uuid';
1212
import { z } from 'zod';
1313
import { createTRPCRouter, protectedProcedure } from '../../trpc';
14+
import { verifyConversationAccess, verifyProjectAccess } from '../project/helper';
1415

1516
export const conversationRouter = createTRPCRouter({
1617
getAll: protectedProcedure
1718
.input(z.object({ projectId: z.string() }))
1819
.query(async ({ ctx, input }) => {
20+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
1921
const dbConversations = await ctx.db.query.conversations.findMany({
2022
where: eq(conversations.projectId, input.projectId),
2123
orderBy: (conversations, { desc }) => [desc(conversations.updatedAt)],
@@ -25,6 +27,7 @@ export const conversationRouter = createTRPCRouter({
2527
get: protectedProcedure
2628
.input(z.object({ conversationId: z.string() }))
2729
.query(async ({ ctx, input }) => {
30+
await verifyConversationAccess(ctx.db, ctx.user.id, input.conversationId);
2831
const conversation = await ctx.db.query.conversations.findFirst({
2932
where: eq(conversations.id, input.conversationId),
3033
});
@@ -36,6 +39,7 @@ export const conversationRouter = createTRPCRouter({
3639
upsert: protectedProcedure
3740
.input(conversationInsertSchema)
3841
.mutation(async ({ ctx, input }) => {
42+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
3943
const [conversation] = await ctx.db.insert(conversations).values(input).returning();
4044
if (!conversation) {
4145
throw new Error('Conversation not created');
@@ -45,6 +49,10 @@ export const conversationRouter = createTRPCRouter({
4549
update: protectedProcedure
4650
.input(conversationUpdateSchema)
4751
.mutation(async ({ ctx, input }) => {
52+
if (!input.id) {
53+
throw new Error('Conversation id is required');
54+
}
55+
await verifyConversationAccess(ctx.db, ctx.user.id, input.id);
4856
const [conversation] = await ctx.db.update({
4957
...conversations,
5058
updatedAt: new Date(),
@@ -60,6 +68,7 @@ export const conversationRouter = createTRPCRouter({
6068
conversationId: z.string()
6169
}))
6270
.mutation(async ({ ctx, input }) => {
71+
await verifyConversationAccess(ctx.db, ctx.user.id, input.conversationId);
6372
await ctx.db.delete(conversations).where(eq(conversations.id, input.conversationId));
6473
}),
6574
generateTitle: protectedProcedure
@@ -68,6 +77,7 @@ export const conversationRouter = createTRPCRouter({
6877
content: z.string(),
6978
}))
7079
.mutation(async ({ ctx, input }) => {
80+
await verifyConversationAccess(ctx.db, ctx.user.id, input.conversationId);
7181
const { model, providerOptions, headers } = initModel({
7282
provider: LLMProvider.OPENROUTER,
7383
model: OPENROUTER_MODELS.CLAUDE_3_5_HAIKU,

‎apps/web/client/src/server/api/routers/chat/message.ts‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ import { MessageCheckpointType } from '@onlook/models';
99
import { asc, eq, inArray } from 'drizzle-orm';
1010
import { z } from 'zod';
1111
import { createTRPCRouter, protectedProcedure } from '../../trpc';
12+
import { verifyConversationAccess, verifyMessagesAccess } from '../project/helper';
1213

1314
export const messageRouter = createTRPCRouter({
1415
getAll: protectedProcedure
1516
.input(z.object({
1617
conversationId: z.string(),
1718
}))
1819
.query(async ({ ctx, input }) => {
20+
await verifyConversationAccess(ctx.db, ctx.user.id, input.conversationId);
1921
const result = await ctx.db.query.messages.findMany({
2022
where: eq(messages.conversationId, input.conversationId),
2123
orderBy: [asc(messages.createdAt)],
@@ -29,12 +31,7 @@ export const messageRouter = createTRPCRouter({
2931
.mutation(async ({ ctx, input }) => {
3032
const conversationId = input.message.conversationId;
3133
if (conversationId) {
32-
const conversation = await ctx.db.query.conversations.findFirst({
33-
where: eq(conversations.id, conversationId),
34-
});
35-
if (!conversation) {
36-
throw new Error(`Conversation not found`);
37-
}
34+
await verifyConversationAccess(ctx.db, ctx.user.id, conversationId);
3835
}
3936
const normalizedMessage = normalizeMessage(input.message);
4037
return await ctx.db
@@ -52,6 +49,12 @@ export const messageRouter = createTRPCRouter({
5249
messages: messageInsertSchema.array(),
5350
}))
5451
.mutation(async ({ ctx, input }) => {
52+
const conversationIds = new Set(
53+
input.messages.map((m) => m.conversationId).filter((id): id is string => !!id),
54+
);
55+
for (const conversationId of conversationIds) {
56+
await verifyConversationAccess(ctx.db, ctx.user.id, conversationId);
57+
}
5558
const normalizedMessages = input.messages.map(normalizeMessage);
5659
await ctx.db.insert(messages).values(normalizedMessages);
5760
}),
@@ -61,6 +64,7 @@ export const messageRouter = createTRPCRouter({
6164
message: messageUpdateSchema
6265
}))
6366
.mutation(async ({ ctx, input }) => {
67+
await verifyMessagesAccess(ctx.db, ctx.user.id, [input.messageId]);
6468
await ctx.db.update(messages).set({
6569
...input.message,
6670
}).where(eq(messages.id, input.messageId));
@@ -76,6 +80,7 @@ export const messageRouter = createTRPCRouter({
7680
})),
7781
}))
7882
.mutation(async ({ ctx, input }) => {
83+
await verifyMessagesAccess(ctx.db, ctx.user.id, [input.messageId]);
7984
await ctx.db.update(messages).set({
8085
checkpoints: input.checkpoints,
8186
}).where(eq(messages.id, input.messageId));
@@ -85,6 +90,7 @@ export const messageRouter = createTRPCRouter({
8590
messageIds: z.array(z.string()),
8691
}))
8792
.mutation(async ({ ctx, input }) => {
93+
await verifyMessagesAccess(ctx.db, ctx.user.id, input.messageIds);
8894
await ctx.db.delete(messages).where(inArray(messages.id, input.messageIds));
8995
}),
9096

@@ -99,11 +105,18 @@ export const messageRouter = createTRPCRouter({
99105
messages: messageInsertSchema.array(),
100106
}))
101107
.mutation(async ({ ctx, input }) => {
108+
await verifyConversationAccess(ctx.db, ctx.user.id, input.conversationId);
102109
await ctx.db.transaction(async (tx) => {
103110
await tx.delete(messages).where(eq(messages.conversationId, input.conversationId));
104111

105112
if (input.messages.length > 0) {
106-
const normalizedMessages = input.messages.map(normalizeMessage);
113+
// Force each inserted message onto the authorized conversation,
114+
// ignoring any conversationId embedded in the client payload --
115+
// otherwise a caller authorized for input.conversationId could
116+
// smuggle messages into a different conversation via the array.
117+
const normalizedMessages = input.messages.map((m) =>
118+
normalizeMessage({ ...m, conversationId: input.conversationId }),
119+
);
107120
await tx.insert(messages).values(normalizedMessages);
108121
}
109122

‎apps/web/client/src/server/api/routers/chat/suggestion.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { convertToModelMessages, generateObject } from 'ai';
77
import { eq } from 'drizzle-orm';
88
import { z } from 'zod';
99
import { createTRPCRouter, protectedProcedure } from '../../trpc';
10+
import { verifyConversationAccess } from '../project/helper';
1011

1112
export const suggestionsRouter = createTRPCRouter({
1213
generate: protectedProcedure
@@ -18,6 +19,7 @@ export const suggestionsRouter = createTRPCRouter({
1819
})),
1920
}))
2021
.mutation(async ({ ctx, input }) => {
22+
await verifyConversationAccess(ctx.db, ctx.user.id, input.conversationId);
2123
const { model, headers } = initModel({
2224
provider: LLMProvider.OPENROUTER,
2325
model: OPENROUTER_MODELS.OPEN_AI_GPT_5_NANO,

‎apps/web/client/src/server/api/routers/domain/custom.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import { customDomainVerification, projectCustomDomains, ProjectCustomDomainStatus, toDomainInfoFromPublished, userProjects } from '@onlook/db';
22
import { VerificationRequestStatus } from '@onlook/models';
33
import { TRPCError } from '@trpc/server';
4-
import { eq } from 'drizzle-orm';
4+
import { and, eq } from 'drizzle-orm';
55
import { z } from 'zod';
66
import { createTRPCRouter, protectedProcedure } from '../../trpc';
7+
import { verifyProjectAccess } from '../project/helper';
78

89
export const customRouter = createTRPCRouter({
910
get: protectedProcedure.input(z.object({
1011
projectId: z.string(),
1112
})).query(async ({ ctx, input }) => {
13+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
1214
const customDomain = await ctx.db.query.projectCustomDomains.findFirst({
1315
where: eq(projectCustomDomains.projectId, input.projectId),
1416
});
@@ -18,14 +20,24 @@ export const customRouter = createTRPCRouter({
1820
domain: z.string(),
1921
projectId: z.string(),
2022
})).mutation(async ({ ctx, input }): Promise<boolean> => {
23+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
2124
try {
2225
await ctx.db.transaction(async (tx) => {
26+
// Scope the cancellation to the caller's project: `input.projectId`
27+
// was previously accepted but ignored, so the domain string alone
28+
// keyed the update — letting anyone cancel any project's domain.
2329
await tx.update(customDomainVerification).set({
2430
status: VerificationRequestStatus.CANCELLED,
25-
}).where(eq(customDomainVerification.fullDomain, input.domain));
31+
}).where(and(
32+
eq(customDomainVerification.fullDomain, input.domain),
33+
eq(customDomainVerification.projectId, input.projectId),
34+
));
2635
await tx.update(projectCustomDomains).set({
2736
status: ProjectCustomDomainStatus.CANCELLED,
28-
}).where(eq(projectCustomDomains.fullDomain, input.domain));
37+
}).where(and(
38+
eq(projectCustomDomains.fullDomain, input.domain),
39+
eq(projectCustomDomains.projectId, input.projectId),
40+
));
2941
});
3042
return true;
3143
} catch (error) {

‎apps/web/client/src/server/api/routers/domain/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { previewDomains, projectCustomDomains, toDomainInfoFromPreview, toDomain
22
import { eq } from 'drizzle-orm';
33
import { z } from 'zod';
44
import { createTRPCRouter, protectedProcedure } from '../../trpc';
5+
import { verifyProjectAccess } from '../project/helper';
56
import { customRouter } from './custom';
67
import { previewRouter } from './preview';
78
import { verificationRouter } from './verify';
@@ -13,6 +14,7 @@ export const domainRouter = createTRPCRouter({
1314
getAll: protectedProcedure.input(z.object({
1415
projectId: z.string(),
1516
})).query(async ({ ctx, input }) => {
17+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
1618
const preview = await ctx.db.query.previewDomains.findFirst({
1719
where: eq(previewDomains.projectId, input.projectId),
1820
});

‎apps/web/client/src/server/api/routers/domain/preview.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import { TRPCError } from '@trpc/server';
55
import { and, eq, ne } from 'drizzle-orm';
66
import { z } from 'zod';
77
import { createTRPCRouter, protectedProcedure } from '../../trpc';
8+
import { verifyProjectAccess } from '../project/helper';
89

910
export const previewRouter = createTRPCRouter({
1011
get: protectedProcedure.input(z.object({
1112
projectId: z.string(),
1213
})).query(async ({ ctx, input }) => {
14+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
1315
const preview = await ctx.db.query.previewDomains.findFirst({
1416
where: eq(previewDomains.projectId, input.projectId),
1517
});
@@ -18,6 +20,7 @@ export const previewRouter = createTRPCRouter({
1820
create: protectedProcedure.input(z.object({
1921
projectId: z.string(),
2022
})).mutation(async ({ ctx, input }) => {
23+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
2124
// Check if the domain is already taken by another project
2225
// This should never happen, but just in case
2326
const domain = `${getValidSubdomain(input.projectId)}.${env.NEXT_PUBLIC_HOSTING_DOMAIN}`;

‎apps/web/client/src/server/api/routers/domain/verify/index.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import { TRPCError } from '@trpc/server';
55
import { and, eq, or } from 'drizzle-orm';
66
import { z } from 'zod';
77
import { createTRPCRouter, protectedProcedure } from '../../../trpc';
8+
import { verifyDomainVerificationAccess, verifyProjectAccess } from '../../project/helper';
89
import { createDomainVerification, ensureUserOwnsDomain, getCustomDomain, getFailureReason, getVerification, verifyFreestyleDomain, verifyFreestyleDomainWithCustomDomain } from './helpers';
910

1011
export const verificationRouter = createTRPCRouter({
1112
getActive: protectedProcedure.input(z.object({
1213
projectId: z.string(),
1314
})).query(async ({ ctx, input }): Promise<CustomDomainVerification | null> => {
15+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
1416
const verification = await ctx.db.query.customDomainVerification.findFirst({
1517
where: and(
1618
eq(customDomainVerification.projectId, input.projectId),
@@ -29,6 +31,7 @@ export const verificationRouter = createTRPCRouter({
2931
domain: z.string(),
3032
projectId: z.string(),
3133
})).mutation(async ({ ctx, input }): Promise<CustomDomainVerification> => {
34+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
3235
const { customDomain, subdomain } = await getCustomDomain(ctx.db, input.domain);
3336
const existingVerification = await getVerification(ctx.db, input.projectId, customDomain.id);
3437
if (existingVerification) {
@@ -40,6 +43,7 @@ export const verificationRouter = createTRPCRouter({
4043
remove: protectedProcedure.input(z.object({
4144
verificationId: z.string(),
4245
})).mutation(async ({ ctx, input }) => {
46+
await verifyDomainVerificationAccess(ctx.db, ctx.user.id, input.verificationId);
4347
await ctx.db.update(customDomainVerification).set({
4448
status: VerificationRequestStatus.CANCELLED,
4549
updatedAt: new Date(),
@@ -51,6 +55,7 @@ export const verificationRouter = createTRPCRouter({
5155
success: boolean;
5256
failureReason: string | null;
5357
}> => {
58+
await verifyDomainVerificationAccess(ctx.db, ctx.user.id, input.verificationId);
5459
const verification = await ctx.db.query.customDomainVerification.findFirst({
5560
where: and(
5661
eq(customDomainVerification.id, input.verificationId),
@@ -118,6 +123,10 @@ export const verificationRouter = createTRPCRouter({
118123
message: 'Unauthorized',
119124
});
120125
}
126+
// Owning the domain is not enough — the caller must also be a member of
127+
// the project the domain is about to be attached to, or they could bind
128+
// a domain they own onto someone else's project.
129+
await verifyProjectAccess(ctx.db, user.id, input.projectId);
121130
const ownsDomain = await ensureUserOwnsDomain(ctx.db, user.id, input.fullDomain);
122131
if (!ownsDomain) {
123132
return {

‎apps/web/client/src/server/api/routers/project/branch.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { and, eq } from 'drizzle-orm';
88
import { v4 as uuidv4 } from 'uuid';
99
import { z } from 'zod';
1010
import { createTRPCRouter, protectedProcedure } from '../../trpc';
11-
import { extractCsbPort } from './helper';
11+
import { extractCsbPort, verifyBranchAccess, verifyProjectAccess } from './helper';
1212

1313
// Helper function to get existing frames in a canvas
1414
async function getExistingFrames(tx: any, canvasId: string): Promise<Frame[]> {
@@ -27,6 +27,7 @@ export const branchRouter = createTRPCRouter({
2727
}),
2828
)
2929
.query(async ({ ctx, input }) => {
30+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
3031
const dbBranches = await ctx.db.query.branches.findMany({
3132
where: input.onlyDefault ?
3233
and(eq(branches.isDefault, true), eq(branches.projectId, input.projectId)) :
@@ -45,6 +46,7 @@ export const branchRouter = createTRPCRouter({
4546
create: protectedProcedure
4647
.input(branchInsertSchema)
4748
.mutation(async ({ ctx, input }) => {
49+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
4850
try {
4951
await ctx.db.insert(branches).values(input);
5052
return true;
@@ -54,6 +56,7 @@ export const branchRouter = createTRPCRouter({
5456
}
5557
}),
5658
update: protectedProcedure.input(branchUpdateSchema).mutation(async ({ ctx, input }) => {
59+
await verifyBranchAccess(ctx.db, ctx.user.id, input.id);
5760
try {
5861
await ctx.db
5962
.update(branches)
@@ -74,6 +77,7 @@ export const branchRouter = createTRPCRouter({
7477
}),
7578
)
7679
.mutation(async ({ ctx, input }) => {
80+
await verifyBranchAccess(ctx.db, ctx.user.id, input.branchId);
7781
try {
7882
await ctx.db.delete(branches).where(eq(branches.id, input.branchId));
7983
return true;
@@ -89,6 +93,7 @@ export const branchRouter = createTRPCRouter({
8993
}),
9094
)
9195
.mutation(async ({ ctx, input }) => {
96+
await verifyBranchAccess(ctx.db, ctx.user.id, input.branchId);
9297
try {
9398
// Get source branch with its frames to extract port
9499
const sourceBranch = await ctx.db.query.branches.findFirst({
@@ -237,6 +242,7 @@ export const branchRouter = createTRPCRouter({
237242
}),
238243
)
239244
.mutation(async ({ ctx, input }) => {
245+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
240246
try {
241247
return await ctx.db.transaction(async (tx) => {
242248
// Get existing branches with frames for unique name generation and port extraction

‎apps/web/client/src/server/api/routers/project/createRequest.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import { ProjectCreateRequestStatus } from '@onlook/models';
55
import { and, eq } from 'drizzle-orm';
66
import { z } from 'zod';
77
import { createTRPCRouter, protectedProcedure } from '../../trpc';
8+
import { verifyProjectAccess } from './helper';
89

910
export const projectCreateRequestRouter = createTRPCRouter({
1011
getPendingRequest: protectedProcedure
1112
.input(z.object({ projectId: z.string() }))
1213
.query(async ({ ctx, input }) => {
14+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
1315
const request = await ctx.db.query.projectCreateRequests.findFirst({
1416
where: and(
1517
eq(projectCreateRequests.projectId, input.projectId),
@@ -24,6 +26,7 @@ export const projectCreateRequestRouter = createTRPCRouter({
2426
status: z.nativeEnum(ProjectCreateRequestStatus),
2527
}))
2628
.mutation(async ({ ctx, input }) => {
29+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
2730
await ctx.db.update(projectCreateRequests).set({
2831
status: input.status,
2932
}).where(

‎apps/web/client/src/server/api/routers/project/fork.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { ProjectRole } from '@onlook/models';
2222
import { eq } from 'drizzle-orm';
2323
import { v4 as uuidv4 } from 'uuid';
2424
import { z } from 'zod';
25+
import { verifyProjectAccess } from './helper';
2526

2627
type ForkedBranch = {
2728
newBranch: Branch;
@@ -163,6 +164,11 @@ export const fork = protectedProcedure
163164
name: z.string().optional(),
164165
}))
165166
.mutation(async ({ ctx, input }) => {
167+
// Forking clones the full private contents of the source project
168+
// (canvas, branches, live sandboxes) into a new project the caller
169+
// owns -- there is no public/template concept on `projects`, so this
170+
// must be gated the same as any other read of a project's contents.
171+
await verifyProjectAccess(ctx.db, ctx.user.id, input.projectId);
166172
// 1. Get the source project with canvas, frames, and branches
167173
const sourceProject = await ctx.db.query.projects.findFirst({
168174
where: eq(projects.id, input.projectId),

0 commit comments

Comments
 (0)