Skip to content

fix: Pentest remediation #3885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
perkinsjr merged 1 commit into from
Aug 29, 2025
Merged

fix: Pentest remediation #3885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/dashboard/lib/trpc/routers/org/getInvitationList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ export const getInvitationList = t.procedure
.use(requireUser)
.use(requireOrgAdmin)
.input(z.string())
.query(async ({ input: orgId }) => {
.query(async ({ ctx, input: orgId }) => {
try {
if (orgId !== ctx.workspace?.orgId) {
Copy link
Copy Markdown
Collaborator

@chronark chronark Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should've never been passed from the frontend and still shouldn't, why don't we use ctx.workspace.orgId and remove the input altogether?

throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
Comment on lines 9 to +17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Input shape likely breaks requireOrgAdmin; also use FORBIDDEN (403)

requireOrgAdmin typically expects input.orgId. Current z.string() input won’t expose orgId to the middleware. Also, make the auth failure 403.

-  .input(z.string())
-  .query(async ({ ctx, input: orgId }) => {
+  .input(z.object({ orgId: z.string() }))
+  .query(async ({ ctx, input: { orgId } }) => {
     try {
-      if (orgId !== ctx.workspace?.orgId) {
+      if (orgId !== ctx.workspace?.orgId) {
         throw new TRPCError({
-          code: "BAD_REQUEST",
-          message: "Invalid organization ID",
+          code: "FORBIDDEN",
+          message: "Forbidden",
         });
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.input(z.string())
.query(async ({ input: orgId }) => {
.query(async ({ ctx, input: orgId }) => {
try {
if (orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
.input(z.object({ orgId: z.string() }))
.query(async ({ ctx, input: { orgId } }) => {
try {
if (orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Forbidden",
});
}
// ...rest of your query logic
} catch (error) {
// ...existing error handling
}
})
🤖 Prompt for AI Agents 
In apps/dashboard/lib/trpc/routers/org/getInvitationList.ts around lines 9 to
17, the route currently declares .input(z.string()) and compares the raw input
to ctx.workspace?.orgId which breaks middleware like requireOrgAdmin that
expects an object with orgId; change the input schema to an object (e.g.,
z.object({ orgId: z.string() })), use input.orgId in the comparison and anywhere
else, and when the orgId does not match throw a TRPCError with code "FORBIDDEN"
and an appropriate message.

return await authProvider.getInvitationList(orgId);
} catch (error) {
console.error("Error retrieving organization member list:", error);
Expand Down
8 changes: 7 additions & 1 deletion apps/dashboard/lib/trpc/routers/org/inviteMember.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@ export const inviteMember = t.procedure
role: z.enum(["basic_member", "admin"]),
}),
)
.mutation(async ({ input }) => {
.mutation(async ({ ctx, input }) => {
try {
if (input.orgId !== ctx.workspace?.orgId) {
Copy link
Copy Markdown
Collaborator

@chronark chronark Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
return await authProvider.inviteMember({
email: input.email,
role: input.role,
Expand Down
8 changes: 7 additions & 1 deletion apps/dashboard/lib/trpc/routers/org/removeMembership.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ export const removeMembership = t.procedure
orgId: z.string(), // needed for the requireOrgAdmin middleware
}),
)
.mutation(async ({ input }) => {
.mutation(async ({ ctx, input }) => {
try {
if (input.orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
return await authProvider.removeMembership(input.membershipId);
Comment on lines +15 to 23
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure membershipId is scoped to the same org (potential auth bypass)

An admin of org A could supply orgId of A but a membershipId from org B if the provider doesn’t enforce scoping. Pass orgId to the provider or pre-validate membership ownership.

-  .mutation(async ({ ctx, input }) => {
+  .mutation(async ({ ctx, input }) => {
     try {
-      if (input.orgId !== ctx.workspace?.orgId) {
+      if (input.orgId !== ctx.workspace?.orgId) {
         throw new TRPCError({
-          code: "BAD_REQUEST",
-          message: "Invalid organization ID",
+          code: "FORBIDDEN",
+          message: "Forbidden",
         });
       }
-      return await authProvider.removeMembership(input.membershipId);
+      // Option A: enforce scoping in provider call
+      return await authProvider.removeMembership({
+        membershipId: input.membershipId,
+        orgId: ctx.workspace!.orgId,
+      });

If the provider doesn’t accept an object, pre-fetch the membership and verify its org matches ctx.workspace.orgId before removal.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.mutation(async ({ ctx, input }) => {
try {
if (input.orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
return await authProvider.removeMembership(input.membershipId);
.mutation(async ({ ctx, input }) => {
try {
if (input.orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Forbidden",
});
}
return await authProvider.removeMembership({
membershipId: input.membershipId,
orgId: ctx.workspace!.orgId,
});
🤖 Prompt for AI Agents 
In apps/dashboard/lib/trpc/routers/org/removeMembership.ts around lines 15 to
23, the current check only validates input.orgId against ctx.workspace.orgId but
does not ensure input.membershipId actually belongs to that org, allowing an
admin to remove a membership from another org; fix by either passing the orgId
into authProvider.removeMembership (e.g., removeMembership({ membershipId, orgId
})) so the provider enforces scoping, or if the provider API cannot be changed,
pre-fetch the membership by membershipId, verify its orgId equals
ctx.workspace.orgId and throw a TRPCError (BAD_REQUEST or FORBIDDEN) if it does
not, then proceed to call removeMembership.

} catch (error) {
throw new TRPCError({
Expand Down
8 changes: 7 additions & 1 deletion apps/dashboard/lib/trpc/routers/org/revokeInvitation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ export const revokeInvitation = t.procedure
orgId: z.string(), // needed for the requireOrgAdmin middleware
}),
)
.mutation(async ({ input }) => {
.mutation(async ({ ctx, input }) => {
try {
if (input.orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
return await authProvider.revokeOrgInvitation(input.invitationId);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Enforce org scoping at the provider boundary.

Passing only invitationId relies on upstream checks; ensure the provider validates the invitation belongs to the same org to prevent cross-org revocations. Ideally pass orgId too or have the provider verify via membership/ownership.

🏁 Script executed:

#!/bin/bash
# Verify provider method enforces org scoping
rg -nP -C3 --type=ts --type=tsx '\brevokeOrgInvitation\s*\('
rg -nP -C3 --type=ts --type=tsx 'function\s+revokeOrgInvitation\b|revokeOrgInvitation:\s*\('

Length of output: 213

🏁 Script executed:

#!/bin/bash
set -e
# Search for all occurrences of revokeOrgInvitation across the codebase (excluding node_modules)
rg -n --color never --glob '!node_modules/*' 'revokeOrgInvitation' .
# Search for authProvider definition to inspect its methods
rg -n --color never --glob '!node_modules/*' 'authProvider' .

Length of output: 3501

🏁 Script executed:

#!/bin/bash
set -e
# Print base-provider revokeOrgInvitation signature
sed -n '240,260p' apps/dashboard/lib/auth/base-provider.ts
# Print local provider implementation
sed -n '320,360p' apps/dashboard/lib/auth/local.ts
# Print workos provider implementation
sed -n '520,560p' apps/dashboard/lib/auth/workos.ts

Length of output: 3024

Enforce org scoping in revokeOrgInvitation: update the revokeOrgInvitation signature in apps/dashboard/lib/auth/base-provider.ts to accept an orgId, pass ctx.tenant.id from the TRPC router, and add validation in both local.ts and workos.ts implementations to ensure the invitation’s organizationId matches before revoking.

} catch (error) {
throw new TRPCError({
Expand Down
8 changes: 7 additions & 1 deletion apps/dashboard/lib/trpc/routers/org/updateMembership.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@ export const updateMembership = t.procedure
role: z.string(),
}),
)
.mutation(async ({ input }) => {
.mutation(async ({ ctx, input }) => {
try {
if (input.orgId !== ctx.workspace?.orgId) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid organization ID",
});
}
return await authProvider.updateMembership({
membershipId: input.membershipId,
role: input.role,
Expand Down
6 changes: 6 additions & 0 deletions apps/dashboard/lib/trpc/routers/workspace/changeName.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ export const changeWorkspaceName = t.procedure
}),
)
.mutation(async ({ ctx, input }) => {
if (input.workspaceId !== ctx.workspace.id) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid workspace ID",
});
}
Comment on lines +20 to +25
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return FORBIDDEN (403) instead of BAD_REQUEST (400) to avoid ID enumeration

This is an auth failure, not an input error. Use a generic message.

-    if (input.workspaceId !== ctx.workspace.id) {
-      throw new TRPCError({
-        code: "BAD_REQUEST",
-        message: "Invalid workspace ID",
-      });
-    }
+    if (input.workspaceId !== ctx.workspace.id) {
+      throw new TRPCError({
+        code: "FORBIDDEN",
+        message: "Forbidden",
+      });
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (input.workspaceId !== ctx.workspace.id) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Invalid workspace ID",
});
}
if (input.workspaceId !== ctx.workspace.id) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Forbidden",
});
}
🤖 Prompt for AI Agents 
In apps/dashboard/lib/trpc/routers/workspace/changeName.ts around lines 20 to
25, the handler currently throws a TRPCError with code "BAD_REQUEST" and a
specific "Invalid workspace ID" message; change this to throw a TRPCError with
code "FORBIDDEN" (HTTP 403) and a generic message such as "Access denied" or
"Forbidden" to reflect an authorization failure and avoid leaking whether the ID
exists.

await db
.transaction(async (tx) => {
await tx
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const nextConfig = {
experimental: {
esmExternals: "loose",
},
poweredByHeader: false,
webpack: (config) => {
config.cache = Object.freeze({
type: "memory",
Expand Down
Loading