Skip to content

Commit 05cd473

Browse files
committed
fix(webapp): scope member removal and run replay to the caller org
Member removal looked up and deleted an orgMember by its globally unique id without binding it to the resolved organization, so a manager in one org could remove members of another by submitting a foreign id. Scope the lookup and delete to the org at both the route and the model layer, and reject a foreign id. A run replay can target a different environment, but the user-supplied override was forwarded to the trigger service without scoping, so a run could be created in another tenant environment. Validate the override belongs to the source run project before triggering. When the inviter has no resolvable role, the role-ladder check returned true and offered every role including the highest. Fail closed instead: the invite still proceeds, but assigning an explicit role is refused.
1 parent bbf86c8 commit 05cd473

4 files changed

Lines changed: 58 additions & 18 deletions

File tree

apps/webapp/app/models/member.server.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,24 @@ export async function removeTeamMember({
7373
throw new Error("User does not have access to this organization");
7474
}
7575

76-
return prisma.orgMember.delete({
77-
where: {
78-
id: memberId,
79-
},
76+
// Scope the target to this org. A member id is a globally unique key, so
77+
// deleting by id alone would remove members of other orgs; bind it to the
78+
// resolved org and reject a foreign id.
79+
const member = await prisma.orgMember.findFirst({
80+
where: { id: memberId, organizationId: org.id },
8081
include: {
8182
organization: true,
8283
user: true,
8384
},
8485
});
86+
87+
if (!member) {
88+
throw new Error("Member not found in this organization");
89+
}
90+
91+
await prisma.orgMember.delete({ where: { id: member.id } });
92+
93+
return member;
8594
}
8695

8796
export async function inviteMembers({

apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,11 @@ function isAtOrBelow(
132132
inviterRoleId: string | null,
133133
invitedRoleId: string
134134
): boolean {
135-
// No RBAC role on inviter (e.g. the runtime fallback couldn't derive
136-
// one) → fall back to the legacy OrgMember.role check the calling
137-
// code already enforces. Allow the invite to proceed; the action
138-
// would have already failed earlier if the inviter wasn't allowed
139-
// to invite at all.
140-
if (!inviterRoleId) return true;
135+
// No resolvable role for the inviter → fail closed: we can't confirm a
136+
// target role is at or below an unknown level, so refuse it. The invite
137+
// itself still proceeds (it's gated by manage:members); only assigning an
138+
// explicit role is refused, and the picker offers nothing in this case.
139+
if (!inviterRoleId) return false;
141140
const level = buildRoleLevel(roles);
142141
const inviter = level[inviterRoleId];
143142
const invited = level[invitedRoleId];

apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -253,17 +253,24 @@ export const action = dashboardAction(
253253
return json(submission);
254254
}
255255

256-
// Default intent: remove a member or leave the org. Self-leave (the
257-
// actor removing their own membership) is always allowed. Removing
258-
// another member requires `manage:members` — pre-RBAC the
259-
// `removeTeamMember` model fn only verified the actor was a member
260-
// of the target org, so any org member could remove any other
261-
// member by id; this gate fixes that latent permissions hole.
256+
// Default intent: remove a member or leave the org. Scope the target to
257+
// the actor's organization: an orgMember id is a globally unique key, so an
258+
// unscoped lookup (plus an unscoped delete in the model) would let a
259+
// manager in one org remove members of another by submitting a foreign id.
260+
// Self-leave is always allowed; removing someone else requires
261+
// manage:members.
262+
const orgId = context.organizationId;
263+
if (!orgId) {
264+
return json({ ok: false, error: "Organization not found" } as const, { status: 404 });
265+
}
262266
const targetMember = await $replica.orgMember.findFirst({
263-
where: { id: submission.value.memberId },
267+
where: { id: submission.value.memberId, organizationId: orgId },
264268
select: { userId: true },
265269
});
266-
const isSelfLeave = targetMember?.userId === userId;
270+
if (!targetMember) {
271+
return json({ ok: false, error: "Member not found" } as const, { status: 404 });
272+
}
273+
const isSelfLeave = targetMember.userId === userId;
267274
if (!isSelfLeave && !ability.can("manage", { type: "members" })) {
268275
return json({ ok: false, error: "Unauthorized" } as const, { status: 403 });
269276
}

apps/webapp/app/routes/resources.taskruns.$runParam.replay.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,31 @@ export const action = dashboardAction(
382382
return redirectWithErrorMessage(submission.value.failedRedirect, request, "Run not found");
383383
}
384384

385+
// A replay can target a different environment, but only within the source
386+
// run's own project. The override id is user-supplied and the downstream
387+
// service looks it up without scoping, so confirm it belongs to this
388+
// project before triggering, otherwise a run could be created in another
389+
// tenant's environment.
390+
if (submission.value.environment) {
391+
const overrideEnvironment = await prisma.runtimeEnvironment.findFirst({
392+
where: {
393+
id: submission.value.environment,
394+
project: {
395+
slug: taskRun.project.slug,
396+
organization: { slug: taskRun.project.organization.slug },
397+
},
398+
},
399+
select: { id: true },
400+
});
401+
if (!overrideEnvironment) {
402+
return redirectWithErrorMessage(
403+
submission.value.failedRedirect,
404+
request,
405+
"Environment not found"
406+
);
407+
}
408+
}
409+
385410
const replayRunService = new ReplayTaskRunService();
386411
const newRun = await replayRunService.call(taskRun, {
387412
environmentId: submission.value.environment,

0 commit comments

Comments
 (0)