diff --git a/.server-changes/reject-benchmarking-webhook-addresses.md b/.server-changes/reject-benchmarking-webhook-addresses.md new file mode 100644 index 00000000000..32d7792c746 --- /dev/null +++ b/.server-changes/reject-benchmarking-webhook-addresses.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Reject alert webhook destinations in reserved benchmarking IP ranges. diff --git a/apps/webapp/app/models/organization.server.ts b/apps/webapp/app/models/organization.server.ts index 7022b60978b..9dc02e4abf5 100644 --- a/apps/webapp/app/models/organization.server.ts +++ b/apps/webapp/app/models/organization.server.ts @@ -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"; @@ -41,8 +42,12 @@ 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 { - const fromReplica = await $replica.organization.findFirst({ +export async function resolveOrgIdFromSlug( + slug: string, + replicaClient: PrismaReplicaClient = $replica, + prismaClient: PrismaClientOrTransaction = prisma +): Promise { + const fromReplica = await replicaClient.organization.findFirst({ where: { slug }, select: { id: true }, }); @@ -50,13 +55,36 @@ export async function resolveOrgIdFromSlug(slug: string): Promise 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 { + 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; +} + export async function createOrganization( { title, diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx index 84bf36e37c2..242a3425f14 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx @@ -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"; @@ -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 }, }); } @@ -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 @@ -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" } }, diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx index 868985b2057..7e37e9817c3 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx @@ -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"; @@ -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" } }, @@ -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 diff --git a/apps/webapp/app/v3/services/alerts/safeWebhookUrl.server.ts b/apps/webapp/app/v3/services/alerts/safeWebhookUrl.server.ts index 37a4c1e15dc..b9d4cac765c 100644 --- a/apps/webapp/app/v3/services/alerts/safeWebhookUrl.server.ts +++ b/apps/webapp/app/v3/services/alerts/safeWebhookUrl.server.ts @@ -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; // 224/4 multicast if (a >= 224 && a <= 239) return true; // 240/4 reserved diff --git a/apps/webapp/test/auth-dashboard.e2e.full.test.ts b/apps/webapp/test/auth-dashboard.e2e.full.test.ts index 3fb47057d35..556317b2e0a 100644 --- a/apps/webapp/test/auth-dashboard.e2e.full.test.ts +++ b/apps/webapp/test/auth-dashboard.e2e.full.test.ts @@ -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"; @@ -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); + }); + }); }); diff --git a/apps/webapp/test/rbacFallbackSessionFloor.test.ts b/apps/webapp/test/rbacFallbackSessionFloor.test.ts new file mode 100644 index 00000000000..edf7a2b671a --- /dev/null +++ b/apps/webapp/test/rbacFallbackSessionFloor.test.ts @@ -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; + + 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); + }); +}); diff --git a/apps/webapp/test/resolveOrgIdFromSlugForUser.test.ts b/apps/webapp/test/resolveOrgIdFromSlugForUser.test.ts new file mode 100644 index 00000000000..422f5aadc87 --- /dev/null +++ b/apps/webapp/test/resolveOrgIdFromSlugForUser.test.ts @@ -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(); + }); +}); diff --git a/apps/webapp/test/safeWebhookUrl.test.ts b/apps/webapp/test/safeWebhookUrl.test.ts index a66534fdee2..e6072b4a17b 100644 --- a/apps/webapp/test/safeWebhookUrl.test.ts +++ b/apps/webapp/test/safeWebhookUrl.test.ts @@ -48,8 +48,15 @@ describe("assertSafeWebhookUrl", () => { ).rejects.toBeInstanceOf(UnsafeWebhookUrlError); }); - it("rejects CGNAT, multicast and reserved ranges", async () => { - for (const host of ["100.64.0.1", "224.0.0.1", "239.1.1.1", "240.0.0.1"]) { + it("rejects CGNAT, benchmarking, multicast and reserved ranges", async () => { + for (const host of [ + "100.64.0.1", + "198.18.0.0", + "198.19.255.255", + "224.0.0.1", + "239.1.1.1", + "240.0.0.1", + ]) { await expect(assertSafeWebhookUrl(`http://${host}/hook`)).rejects.toBeInstanceOf( UnsafeWebhookUrlError ); @@ -92,6 +99,8 @@ describe("assertSafeWebhookUrl", () => { describe("assertAddressAllowed", () => { it("allows public IPv4 / IPv6 addresses", () => { expect(() => assertAddressAllowed("93.184.216.34", 4)).not.toThrow(); + expect(() => assertAddressAllowed("198.17.255.255", 4)).not.toThrow(); + expect(() => assertAddressAllowed("198.20.0.0", 4)).not.toThrow(); expect(() => assertAddressAllowed("2606:2800:220:1:248:1893:25c8:1946", 6)).not.toThrow(); }); @@ -104,6 +113,8 @@ describe("assertAddressAllowed", () => { "192.168.1.1", "169.254.169.254", "100.64.0.1", + "198.18.0.0", + "198.19.255.255", ]) { expect(() => assertAddressAllowed(addr, 4)).toThrow(UnsafeWebhookUrlError); } diff --git a/internal-packages/rbac/src/fallback.ts b/internal-packages/rbac/src/fallback.ts index 9054c0a43e1..6dc04d49455 100644 --- a/internal-packages/rbac/src/fallback.ts +++ b/internal-packages/rbac/src/fallback.ts @@ -98,6 +98,21 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController { const user = await this.replica.user.findFirst({ where: { id: context.userId } }); if (!user) return { ok: false, reason: "unauthenticated" }; + // A non-member in a scoped context is denied, not handed a permissive + // ability. buildFallbackAbility is permissive for a non-admin + // (can: () => true), so ability.can is not a tenant floor; returning it here + // let a non-member act on any org whose slug they knew. An unscoped context + // stays permissive (identity-only checks predate any scope), and a platform + // admin keeps their ability. + if (!user.admin) { + const denied = await this.deniedByMembership( + context.organizationId, + context.projectId, + user.id + ); + if (denied) return { ok: false, reason: "unauthorized" }; + } + const subject: RbacSubject = { type: "user", userId: user.id, @@ -113,6 +128,46 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController { }; } + /** + * Whether a non-admin user is outside the tenant a scoped context names. A project-only scope + * resolves through the project's organization, so the floor holds whichever scope a route + * resolves; an unscoped context is not a tenant claim and is never denied here. + * + * Both lookups read the replica first and fall back to the primary before denying: org creation, + * invite acceptance and SSO provisioning all write to the primary, so a member who just joined + * must not be bounced while the row replicates. + */ + private async deniedByMembership( + organizationId: string | undefined, + projectId: string | undefined, + userId: string + ): Promise { + let orgId = organizationId; + + if (!orgId && projectId) { + const project = + (await this.replica.project.findFirst({ + where: { id: projectId }, + select: { organizationId: true }, + })) ?? + (await this.prisma.project.findFirst({ + where: { id: projectId }, + select: { organizationId: true }, + })); + // An unresolvable project names no tenant, so there is nothing to deny against. + if (!project) return false; + orgId = project.organizationId; + } + + if (!orgId) return false; + + const where = { organizationId: orgId, userId }; + const member = + (await this.replica.orgMember.findFirst({ where, select: { id: true } })) ?? + (await this.prisma.orgMember.findFirst({ where, select: { id: true } })); + return !member; + } + async authenticateAuthorizeBearer( request: Request, check: { action: string; resource: RbacResource | RbacResource[] },