Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .server-changes/reject-benchmarking-webhook-addresses.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

Reject alert webhook destinations in reserved benchmarking IP ranges.
34 changes: 31 additions & 3 deletions apps/webapp/app/models/organization.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Prisma as PrismaNamespace,
prisma,
type PrismaClientOrTransaction,
type PrismaReplicaClient,
} from "~/db.server";
import { env } from "~/env.server";
import { featuresForUrl } from "~/features.server";
Expand All @@ -41,22 +42,49 @@ const nanoid = customAlphabet("1234567890abcdef", 4);
* miss, so replica lag never leaves a real org unresolved, which the dashboard
* route builder treats as an unauthorized request.
*/
export async function resolveOrgIdFromSlug(slug: string): Promise<string | null> {
const fromReplica = await $replica.organization.findFirst({
export async function resolveOrgIdFromSlug(
slug: string,
replicaClient: PrismaReplicaClient = $replica,
prismaClient: PrismaClientOrTransaction = prisma
): Promise<string | null> {
const fromReplica = await replicaClient.organization.findFirst({
where: { slug },
select: { id: true },
});
if (fromReplica) {
return fromReplica.id;
}

const fromPrimary = await prisma.organization.findFirst({
const fromPrimary = await prismaClient.organization.findFirst({
where: { slug },
select: { id: true },
});
return fromPrimary?.id ?? null;
}

/**
* Like `resolveOrgIdFromSlug`, but only resolves an org the user is a member of. `ability.can` is not
* a tenant floor (the OSS fallback and the cloud plugin both return a permissive ability for a
* non-member), so a route that scopes only by slug lets a non-member reach the handler; the
* membership filter here is the tenant floor. Returns null for a non-member, which the dashboard
* route builder treats as no scope and fails closed.
*/
export async function resolveOrgIdFromSlugForUser(
slug: string,
userId: string,
replicaClient: PrismaReplicaClient = $replica,
prismaClient: PrismaClientOrTransaction = prisma
): Promise<string | null> {
const where = { slug, members: { some: { userId } } };
const fromReplica = await replicaClient.organization.findFirst({ where, select: { id: true } });
if (fromReplica) {
return fromReplica.id;
}

const fromPrimary = await prismaClient.organization.findFirst({ where, select: { id: true } });
return fromPrimary?.id ?? null;
}
Comment thread
carderne marked this conversation as resolved.

export async function createOrganization(
{
title,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
import { Select, SelectItem } from "~/components/primitives/Select";
import { Switch } from "~/components/primitives/Switch";
import { prisma } from "~/db.server";
import { getUserId } from "~/services/session.server";
import { useOrganization } from "~/hooks/useOrganizations";
import { rbac } from "~/services/rbac.server";
import { ssoController } from "~/services/sso.server";
Expand All @@ -53,11 +54,14 @@ export const meta = pageMeta("SSO & Directory Sync");

const Params = z.object({ organizationSlug: z.string() });

async function resolveOrg(slug: string) {
async function resolveOrg(slug: string, userId: string) {
// Scoped to membership: ability.can is not a tenant floor (the cloud RBAC
// plugin returns a permissive ability for a non-member), so without the
// members filter a non-member reaches the handler for any org slug.
// Primary (not replica): this scopes the RBAC/entitlement checks, so lag
// could run them against a stale/missing org.
return prisma.organization.findFirst({
where: { slug },
where: { slug, members: { some: { userId } } },
select: { id: true, title: true },
});
}
Expand Down Expand Up @@ -117,8 +121,10 @@ const EMPTY_SSO_STATUS = {
export const loader = dashboardLoader(
{
params: Params,
context: async (params) => {
const org = await resolveOrg(params.organizationSlug);
context: async (params, request) => {
const userId = await getUserId(request);
if (!userId) return {};
const org = await resolveOrg(params.organizationSlug, userId);
return org ? { organizationId: org.id, orgTitle: org.title } : {};
},
// Plan-gated before role-gated: non-Enterprise orgs render the upsell for
Expand Down Expand Up @@ -211,8 +217,10 @@ const ActionSchema = z.discriminatedUnion("action", [
export const action = dashboardAction(
{
params: Params,
context: async (params) => {
const org = await resolveOrg(params.organizationSlug);
context: async (params, request) => {
const userId = await getUserId(request);
if (!userId) return {};
const org = await resolveOrg(params.organizationSlug, userId);
return org ? { organizationId: org.id } : {};
},
authorization: { action: "manage", resource: { type: "sso" } },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ import { useOrganization } from "~/hooks/useOrganizations";
import { useUser } from "~/hooks/useUser";
import { removeTeamMember } from "~/models/removeTeamMember.server";
import { redirectWithSuccessMessage } from "~/models/message.server";
import { resolveOrgIdFromSlug } from "~/models/organization.server";
import { resolveOrgIdFromSlugForUser } from "~/models/organization.server";
import { getUserId } from "~/services/session.server";
import { TeamPresenter } from "~/presenters/TeamPresenter.server";
import { getCurrentPlan, getSelfServePurchaseBlockReason } from "~/services/platform.v3.server";
import { rbac } from "~/services/rbac.server";
Expand Down Expand Up @@ -66,8 +67,10 @@ const Params = z.object({
export const loader = dashboardLoader(
{
params: Params,
context: async (params) => {
const orgId = await resolveOrgIdFromSlug(params.organizationSlug);
context: async (params, request) => {
const userId = await getUserId(request);
if (!userId) return {};
const orgId = await resolveOrgIdFromSlugForUser(params.organizationSlug, userId);
return orgId ? { organizationId: orgId } : {};
},
authorization: { action: "read", resource: { type: "members" } },
Expand Down Expand Up @@ -127,8 +130,10 @@ const SetRoleSchema = z.object({
export const action = dashboardAction(
{
params: Params,
context: async (params) => {
const orgId = await resolveOrgIdFromSlug(params.organizationSlug);
context: async (params, request) => {
const userId = await getUserId(request);
if (!userId) return {};
const orgId = await resolveOrgIdFromSlugForUser(params.organizationSlug, userId);
return orgId ? { organizationId: orgId } : {};
},
// No top-level authorization — different intents have different
Expand Down
2 changes: 2 additions & 0 deletions apps/webapp/app/v3/services/alerts/safeWebhookUrl.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ function isUnsafeIPv4(host: string): boolean {
if (a === 169 && b === 254) return true;
// 100.64/10 carrier-grade NAT
if (a === 100 && b >= 64 && b <= 127) return true;
// 198.18/15 benchmarking
if (a === 198 && b >= 18 && b <= 19) return true;
Comment thread
carderne marked this conversation as resolved.
// 224/4 multicast
if (a >= 224 && a <= 239) return true;
// 240/4 reserved
Expand Down
64 changes: 64 additions & 0 deletions apps/webapp/test/auth-dashboard.e2e.full.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Each test seeds a User + session cookie via seedTestUser / seedTestSession
// (helpers/seedTestSession.ts) and hits the shared webapp container.

import { randomBytes } from "node:crypto";
import type { PrismaClient } from "@trigger.dev/database";
import { describe, expect, it } from "vitest";
import { getTestServer } from "./helpers/sharedTestServer";
import { seedTestSession, seedTestUser } from "./helpers/seedTestSession";
Expand Down Expand Up @@ -115,4 +117,66 @@ describe("Dashboard", () => {
expect(new URL(location, "http://localhost").pathname).toBe("/");
});
});

// Cross-tenant tenant floor on org settings routes. settings/roles is the case
// the route-level membership scoping (SSO/Team) did NOT cover, so it exercises
// the RBAC fallback's org-membership floor specifically: the fallback ability
// is permissive (can: () => true), so that floor is the only thing stopping a
// non-member from reading the org's role and permission catalogue.
//
// The request hits the route's own loader directly via Remix's `?_data`, which
// is the exact exploit shape: a plain document GET 404s at the org layout
// (membership) and never reaches this leaf, so it wouldn't test the leaf floor.
// Both users have confirmedBasicDetails set so the `_app` onboarding redirect
// can't stand in for the deny.
describe("Org settings — cross-tenant tenant floor (settings/roles)", () => {
const ROLES_ROUTE_ID = "routes/_app.orgs.$organizationSlug.settings.roles";
const rolesData = (slug: string) =>
`/orgs/${slug}/settings/roles?_data=${encodeURIComponent(ROLES_ROUTE_ID)}`;

async function seedConfirmedUser(prisma: PrismaClient) {
const user = await seedTestUser(prisma);
await prisma.user.update({ where: { id: user.id }, data: { confirmedBasicDetails: true } });
return user;
}

async function seedOrgWithOwner() {
const server = getTestServer();
const owner = await seedConfirmedUser(server.prisma);
const org = await server.prisma.organization.create({
data: {
title: "E2E tenant-floor org",
slug: `e2e-tenant-${randomBytes(6).toString("hex")}`,
members: { create: { userId: owner.id, role: "ADMIN" } },
},
});
return { server, owner, org };
}

it("denies a non-member: no roles catalogue leaked", async () => {
const { server, org } = await seedOrgWithOwner();
const outsider = await seedConfirmedUser(server.prisma);
const cookie = await seedTestSession({ userId: outsider.id });
const res = await server.webapp.fetch(rolesData(org.slug), {
redirect: "manual",
headers: { Cookie: cookie },
});
const body = await res.text();
// With the tenant floor a non-member is denied (a redirect), so they never
// get the loader's 200 payload. Before the fix the permissive ability let
// the loader return the org's role/permission catalogue.
expect(res.status).not.toBe(200);
expect(body).not.toContain("manage:members");
});

it("allows a member: the loader returns the catalogue", async () => {
const { server, owner, org } = await seedOrgWithOwner();
const cookie = await seedTestSession({ userId: owner.id });
const res = await server.webapp.fetch(rolesData(org.slug), {
redirect: "manual",
headers: { Cookie: cookie },
});
expect(res.status).toBe(200);
});
});
});
110 changes: 110 additions & 0 deletions apps/webapp/test/rbacFallbackSessionFloor.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { postgresTest } from "@internal/testcontainers";
import plugin from "@trigger.dev/rbac";
import { type PrismaClient } from "@trigger.dev/database";
import { describe, expect, vi } from "vitest";
import {
createTestOrgProjectWithMember,
createTestUser,
} from "./fixtures/environmentVariablesFixtures";

vi.setConfig({ testTimeout: 60_000 });

// The RBAC fallback ability is permissive (`can: () => true` for a non-admin), so
// `ability.can` is not a tenant floor. `authenticateSession` is the gate every
// org-scoped dashboard route relies on; a non-member in an org context must be
// denied here, or a permissive ability lets them act on any org whose slug they
// know. The route-level e2e (auth-dashboard.e2e.full) covers the HTTP path; this
// pins the fallback gate directly since that path can't run without a container.
function fallback(prisma: PrismaClient) {
// forceFallback skips the closed-source plugin and uses the in-repo fallback.
return plugin.create({ primary: prisma, replica: prisma }, { forceFallback: true });
}

const request = new Request("https://app.trigger.dev/orgs/x/settings/roles");

describe("RBAC fallback authenticateSession — org membership floor", () => {
postgresTest("denies a non-member in an org context", async ({ prisma }) => {
const { organization } = await createTestOrgProjectWithMember(prisma);
const outsider = await createTestUser(prisma);

const result = await fallback(prisma).authenticateSession(request, {
userId: outsider.id,
organizationId: organization.id,
});

expect(result).toMatchObject({ ok: false, reason: "unauthorized" });
});

postgresTest("allows a member in an org context", async ({ prisma }) => {
const { user, organization } = await createTestOrgProjectWithMember(prisma);

const result = await fallback(prisma).authenticateSession(request, {
userId: user.id,
organizationId: organization.id,
});

expect(result.ok).toBe(true);
});

postgresTest(
"stays permissive with no org context, even for a non-member",
async ({ prisma }) => {
// Identity-only checks (no organizationId) predate any scope, so the floor
// does not apply and the permissive baseline is preserved.
const outsider = await createTestUser(prisma);

const result = await fallback(prisma).authenticateSession(request, { userId: outsider.id });

expect(result.ok).toBe(true);
}
);

// A project-only scope is still a tenant claim, so the floor resolves the
// project's organization rather than letting the context through unchecked.
postgresTest("denies a non-member scoped only to a project", async ({ prisma }) => {
const { project } = await createTestOrgProjectWithMember(prisma);
const outsider = await createTestUser(prisma);

const result = await fallback(prisma).authenticateSession(request, {
userId: outsider.id,
projectId: project.id,
});

expect(result).toMatchObject({ ok: false, reason: "unauthorized" });
});

postgresTest("allows a member scoped only to a project", async ({ prisma }) => {
const { user, project } = await createTestOrgProjectWithMember(prisma);

const result = await fallback(prisma).authenticateSession(request, {
userId: user.id,
projectId: project.id,
});

expect(result.ok).toBe(true);
});

// The membership probe reads the replica first and the primary on a miss, so a
// member whose row has not replicated yet is not bounced. Modelled by giving
// the controller a replica that cannot see the row and a primary that can.
postgresTest("allows a member the replica has not caught up on", async ({ prisma }) => {
const { user, organization } = await createTestOrgProjectWithMember(prisma);
const blindReplica = {
...prisma,
orgMember: { findFirst: async () => null },
user: prisma.user,
project: prisma.project,
} as unknown as PrismaClient;
Comment thread
carderne marked this conversation as resolved.

const controller = plugin.create(
{ primary: prisma, replica: blindReplica },
{ forceFallback: true }
);
const result = await controller.authenticateSession(request, {
userId: user.id,
organizationId: organization.id,
});

expect(result.ok).toBe(true);
});
});
37 changes: 37 additions & 0 deletions apps/webapp/test/resolveOrgIdFromSlugForUser.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { postgresTest } from "@internal/testcontainers";
import { describe, expect, vi } from "vitest";

vi.setConfig({ testTimeout: 60_000 });
import { resolveOrgIdFromSlug, resolveOrgIdFromSlugForUser } from "~/models/organization.server";
import {
createTestOrgProjectWithMember,
createTestUser,
} from "./fixtures/environmentVariablesFixtures";

// The org settings routes resolve their org through this helper, so a non-member resolving to null
// is what makes the dashboard route builder fail closed. ability.can is not a tenant floor (the RBAC
// plugin and the OSS fallback both return a permissive ability for a non-member), so without the
// membership filter a non-member reached those routes for any org whose slug they knew: a live
// cross-tenant read of SSO/directory-sync config and an open set-role gate, confirmed on test-cloud.
describe("resolveOrgIdFromSlugForUser", () => {
postgresTest("resolves an org the user is a member of", async ({ prisma }) => {
const { user, organization } = await createTestOrgProjectWithMember(prisma);

const resolved = await resolveOrgIdFromSlugForUser(organization.slug, user.id, prisma, prisma);

expect(resolved).toBe(organization.id);
});

postgresTest("returns null for a non-member, the tenant floor", async ({ prisma }) => {
const { organization: target } = await createTestOrgProjectWithMember(prisma);
const outsider = await createTestUser(prisma);

const resolved = await resolveOrgIdFromSlugForUser(target.slug, outsider.id, prisma, prisma);

// The unscoped resolver still hands the same non-member the org id: this is the exact gap the
// membership filter closes, and why scoping by slug alone was the hole.
const unscoped = await resolveOrgIdFromSlug(target.slug, prisma, prisma);
expect(unscoped).toBe(target.id);
expect(resolved).toBeNull();
});
});
Loading
Loading