From 0bcef22bb887edef07b61d1d05df699769845caa Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Wed, 20 May 2026 18:06:10 +0100 Subject: [PATCH 01/11] feat(webapp): attach tenant context to Sentry events Stamp each Sentry event with the org/project/env it belongs to so "Users Impacted" reflects tenant count and events are filterable per org. Uses an AsyncLocalStorage scope set at HTTP entry points (API routes via apiBuilder, dashboard routes via an Express middleware that resolves org/project/env from the URL). Co-Authored-By: Claude Sonnet 4.6 --- .server-changes/sentry-tenant-attribution.md | 15 ++ apps/webapp/app/entry.server.tsx | 7 + .../routeBuilders/apiBuilder.server.ts | 48 ++++-- .../app/services/tenantContext.server.ts | 32 ++++ .../services/tenantContextResolver.server.ts | 78 +++++++++ .../app/utils/sentryTenantContext.server.ts | 26 +++ apps/webapp/server.ts | 4 + apps/webapp/test/sentryTenantContext.test.ts | 52 ++++++ apps/webapp/test/tenantContext.test.ts | 48 ++++++ .../tenantContextFromAuthEnvironment.test.ts | 32 ++++ .../webapp/test/tenantContextResolver.test.ts | 156 ++++++++++++++++++ 11 files changed, 480 insertions(+), 18 deletions(-) create mode 100644 .server-changes/sentry-tenant-attribution.md create mode 100644 apps/webapp/app/services/tenantContext.server.ts create mode 100644 apps/webapp/app/services/tenantContextResolver.server.ts create mode 100644 apps/webapp/app/utils/sentryTenantContext.server.ts create mode 100644 apps/webapp/test/sentryTenantContext.test.ts create mode 100644 apps/webapp/test/tenantContext.test.ts create mode 100644 apps/webapp/test/tenantContextFromAuthEnvironment.test.ts create mode 100644 apps/webapp/test/tenantContextResolver.test.ts diff --git a/.server-changes/sentry-tenant-attribution.md b/.server-changes/sentry-tenant-attribution.md new file mode 100644 index 00000000000..ac230304302 --- /dev/null +++ b/.server-changes/sentry-tenant-attribution.md @@ -0,0 +1,15 @@ +--- +area: webapp +type: feature +--- + +Attach organization / project / environment to every Sentry event so "Users Impacted" counts orgs and events are filterable by tenant. + +Mechanism: `AsyncLocalStorage`-backed `tenantContext` + a Sentry `addEventProcessor` that stamps `user.id = orgId`, `user.username = orgSlug`, and tags (`org_id`, `org_slug`, `project_id`, `project_ref`, `environment_id`, `env_slug`, `env_type`, `impersonating`). + +Wired at the HTTP entry points: + +- API routes — `apiBuilder.server.ts` wraps each handler invocation in `tenantContext.run` using the authenticated `environment`. +- Dashboard requests — an Express middleware (`tenantContextMiddleware`) resolves org/project/env from the URL pattern `/orgs/:org/projects/:project/env/:env/...` and wraps the Remix handler. + +Background workers (redis-worker / schedule-engine) and socket handlers are not yet wired and remain a follow-up. Events from those entry points will continue to ship without tenant attribution until each handler is updated. diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index 11c3274e865..cd7155e0b9a 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -1,6 +1,8 @@ import { createReadableStreamFromReadable, type EntryContext } from "@remix-run/node"; // or cloudflare/deno import { RemixServer } from "@remix-run/react"; +import * as Sentry from "@sentry/remix"; import { wrapHandleErrorWithSentry } from "@sentry/remix"; +import { addTenantContextToEvent } from "~/utils/sentryTenantContext.server"; import { parseAcceptLanguage } from "intl-parse-accept-language"; import isbot from "isbot"; import { renderToPipeableStream } from "react-dom/server"; @@ -289,9 +291,14 @@ process.on("uncaughtException", (error, origin) => { singleton("RunEngineEventBusHandlers", registerRunEngineEventBusHandlers); singleton("SetupBatchQueueCallbacks", setupBatchQueueCallbacks); +if (process.env.SENTRY_DSN) { + Sentry.addEventProcessor(addTenantContextToEvent); +} + export { apiRateLimiter } from "./services/apiRateLimit.server"; export { engineRateLimiter } from "./services/engineRateLimit.server"; export { runWithHttpContext } from "./services/httpAsyncStorage.server"; +export { tenantContextMiddleware } from "./services/tenantContextResolver.server"; export { socketIo } from "./v3/handleSocketIo.server"; export { wss } from "./v3/handleWebsockets.server"; diff --git a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts index 3bf3564431a..ff9f5d7c65b 100644 --- a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts +++ b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts @@ -19,6 +19,10 @@ import { API_VERSIONS, getApiVersion } from "~/api/versions"; import { WORKER_HEADERS } from "@trigger.dev/core/v3/runEngineWorker"; import { ServiceValidationError } from "~/v3/services/common.server"; import { EngineServiceValidationError } from "@internal/run-engine"; +import { + tenantContext, + tenantContextFromAuthEnvironment, +} from "~/services/tenantContext.server"; // Client aborts and service-level validation errors aren't bugs — they're // expected at API boundaries. Log them at `warn` so they stay in stdout @@ -357,15 +361,19 @@ export function createLoaderApiRoute< const apiVersion = getApiVersion(request); - const result = await handler({ - params: parsedParams, - searchParams: parsedSearchParams, - headers: parsedHeaders, - authentication: authenticationResult, - request, - resource, - apiVersion, - }); + const result = await tenantContext.run( + tenantContextFromAuthEnvironment(authenticationResult.environment), + () => + handler({ + params: parsedParams, + searchParams: parsedSearchParams, + headers: parsedHeaders, + authentication: authenticationResult, + request, + resource, + apiVersion, + }) + ); return await wrapResponse(request, result, corsStrategy !== "none"); } catch (error) { try { @@ -903,15 +911,19 @@ export function createActionApiRoute< ); } - const result = await handler({ - params: parsedParams, - searchParams: parsedSearchParams, - headers: parsedHeaders, - body: parsedBody, - authentication: authenticationResult, - request, - resource, - }); + const result = await tenantContext.run( + tenantContextFromAuthEnvironment(authenticationResult.environment), + () => + handler({ + params: parsedParams, + searchParams: parsedSearchParams, + headers: parsedHeaders, + body: parsedBody, + authentication: authenticationResult, + request, + resource, + }) + ); return await wrapResponse(request, result, corsStrategy !== "none"); } catch (error) { try { diff --git a/apps/webapp/app/services/tenantContext.server.ts b/apps/webapp/app/services/tenantContext.server.ts new file mode 100644 index 00000000000..ffd78a4758f --- /dev/null +++ b/apps/webapp/app/services/tenantContext.server.ts @@ -0,0 +1,32 @@ +import { AsyncLocalStorage } from "node:async_hooks"; +import type { AuthenticatedEnvironment } from "./apiAuth.server"; + +export type TenantContext = { + org: { id: string; slug: string }; + project: { id: string; ref: string }; + environment: { + id: string; + slug: string; + type: "DEVELOPMENT" | "PREVIEW" | "STAGING" | "PRODUCTION"; + }; + impersonating?: boolean; +}; + +const storage = new AsyncLocalStorage(); + +export const tenantContext = { + run(ctx: TenantContext, fn: () => T): T { + return storage.run(ctx, fn); + }, + get(): TenantContext | undefined { + return storage.getStore(); + }, +}; + +export function tenantContextFromAuthEnvironment(env: AuthenticatedEnvironment): TenantContext { + return { + org: { id: env.organization.id, slug: env.organization.slug }, + project: { id: env.project.id, ref: env.project.externalRef }, + environment: { id: env.id, slug: env.slug, type: env.type }, + }; +} diff --git a/apps/webapp/app/services/tenantContextResolver.server.ts b/apps/webapp/app/services/tenantContextResolver.server.ts new file mode 100644 index 00000000000..11ea851568b --- /dev/null +++ b/apps/webapp/app/services/tenantContextResolver.server.ts @@ -0,0 +1,78 @@ +import type { NextFunction, Request, Response } from "express"; +import { prisma } from "~/db.server"; +import { tenantContext, type TenantContext } from "./tenantContext.server"; +import { logger } from "./logger.server"; + +const URL_PATTERN = /^\/orgs\/([^/]+)(?:\/projects\/([^/]+)(?:\/env\/([^/]+))?)?/; + +export type ParsedTenantPath = { + orgSlug: string; + projectParam: string; + envParam: string; +}; + +export function parseTenantPath(pathname: string): ParsedTenantPath | undefined { + const match = pathname.match(URL_PATTERN); + if (!match) return undefined; + const [, orgSlug, projectParam, envParam] = match; + if (!orgSlug || !projectParam || !envParam) return undefined; + return { orgSlug, projectParam, envParam }; +} + +export async function resolveTenantContextFromPath( + pathname: string +): Promise { + const parsed = parseTenantPath(pathname); + if (!parsed) return undefined; + + try { + const env = await prisma.runtimeEnvironment.findFirst({ + where: { + slug: parsed.envParam, + project: { slug: parsed.projectParam, organization: { slug: parsed.orgSlug } }, + }, + select: { + id: true, + slug: true, + type: true, + project: { select: { id: true, externalRef: true } }, + organization: { select: { id: true, slug: true } }, + }, + }); + if (!env) return undefined; + return { + org: { id: env.organization.id, slug: env.organization.slug }, + project: { id: env.project.id, ref: env.project.externalRef }, + environment: { + id: env.id, + slug: env.slug, + type: env.type, + }, + }; + } catch (error) { + logger.warn("tenantContextResolver: lookup failed", { + error: error instanceof Error ? error.message : String(error), + pathname, + }); + return undefined; + } +} + +export type PathResolver = (pathname: string) => Promise; + +export function createTenantContextMiddleware(resolver: PathResolver) { + return async function tenantContextMiddleware( + req: Request, + res: Response, + next: NextFunction + ) { + const ctx = await resolver(req.path); + if (ctx) { + tenantContext.run(ctx, () => next()); + } else { + next(); + } + }; +} + +export const tenantContextMiddleware = createTenantContextMiddleware(resolveTenantContextFromPath); diff --git a/apps/webapp/app/utils/sentryTenantContext.server.ts b/apps/webapp/app/utils/sentryTenantContext.server.ts new file mode 100644 index 00000000000..b7fa118ceb3 --- /dev/null +++ b/apps/webapp/app/utils/sentryTenantContext.server.ts @@ -0,0 +1,26 @@ +import type { Event, EventHint } from "@sentry/remix"; +import { tenantContext } from "../services/tenantContext.server"; + +export function addTenantContextToEvent(event: Event, _hint: EventHint): Event { + const ctx = tenantContext.get(); + if (!ctx) return event; + return { + ...event, + user: { + ...event.user, + id: ctx.org.id, + username: ctx.org.slug, + }, + tags: { + ...event.tags, + org_id: ctx.org.id, + org_slug: ctx.org.slug, + project_id: ctx.project.id, + project_ref: ctx.project.ref, + environment_id: ctx.environment.id, + env_slug: ctx.environment.slug, + env_type: ctx.environment.type, + ...(ctx.impersonating ? { impersonating: "true" } : {}), + }, + }; +} diff --git a/apps/webapp/server.ts b/apps/webapp/server.ts index e266c6985c8..9c83f243213 100644 --- a/apps/webapp/server.ts +++ b/apps/webapp/server.ts @@ -122,6 +122,8 @@ if (ENABLE_CLUSTER && cluster.isPrimary) { const apiRateLimiter: RateLimitMiddleware = build.entry.module.apiRateLimiter; const engineRateLimiter: RateLimitMiddleware = build.entry.module.engineRateLimiter; const runWithHttpContext: RunWithHttpContextFunction = build.entry.module.runWithHttpContext; + const tenantContextMiddleware: import("express").RequestHandler = + build.entry.module.tenantContextMiddleware; app.use((req, res, next) => { // helpful headers: @@ -171,6 +173,8 @@ if (ENABLE_CLUSTER && cluster.isPrimary) { app.use(apiRateLimiter); app.use(engineRateLimiter); + app.use(tenantContextMiddleware); + app.all( "*", // @ts-ignore diff --git a/apps/webapp/test/sentryTenantContext.test.ts b/apps/webapp/test/sentryTenantContext.test.ts new file mode 100644 index 00000000000..cdc31d7e854 --- /dev/null +++ b/apps/webapp/test/sentryTenantContext.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from "vitest"; +import type { Event } from "@sentry/remix"; +import { tenantContext } from "../app/services/tenantContext.server"; +import { addTenantContextToEvent } from "../app/utils/sentryTenantContext.server"; + +const sample = { + org: { id: "org_1", slug: "acme" }, + project: { id: "proj_1", ref: "proj_abc" }, + environment: { id: "env_1", slug: "prod", type: "PRODUCTION" as const }, +}; + +describe("addTenantContextToEvent", () => { + it("returns the event unchanged when no ALS context", () => { + const event: Event = { message: "hi", tags: { existing: "1" } }; + const out = addTenantContextToEvent(event, {}); + expect(out).toEqual(event); + }); + + it("stamps user + tags when ALS context is set", () => { + tenantContext.run(sample, () => { + const event: Event = { message: "boom", tags: { existing: "1" } }; + const out = addTenantContextToEvent(event, {}); + expect(out.user).toEqual({ id: "org_1", username: "acme" }); + expect(out.tags).toMatchObject({ + existing: "1", + org_id: "org_1", + org_slug: "acme", + project_id: "proj_1", + project_ref: "proj_abc", + environment_id: "env_1", + env_slug: "prod", + env_type: "PRODUCTION", + }); + expect(out.tags?.impersonating).toBeUndefined(); + }); + }); + + it("adds impersonating tag when flag set", () => { + tenantContext.run({ ...sample, impersonating: true }, () => { + const out = addTenantContextToEvent({}, {}); + expect(out.tags?.impersonating).toBe("true"); + }); + }); + + it("preserves prior event.user fields it does not own", () => { + tenantContext.run(sample, () => { + const event: Event = { user: { ip_address: "1.2.3.4" } }; + const out = addTenantContextToEvent(event, {}); + expect(out.user).toEqual({ ip_address: "1.2.3.4", id: "org_1", username: "acme" }); + }); + }); +}); diff --git a/apps/webapp/test/tenantContext.test.ts b/apps/webapp/test/tenantContext.test.ts new file mode 100644 index 00000000000..13c8765d49c --- /dev/null +++ b/apps/webapp/test/tenantContext.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from "vitest"; +import { tenantContext, type TenantContext } from "../app/services/tenantContext.server"; + +const sample: TenantContext = { + org: { id: "org_1", slug: "acme" }, + project: { id: "proj_1", ref: "proj_abc" }, + environment: { id: "env_1", slug: "prod", type: "PRODUCTION" }, +}; + +describe("tenantContext", () => { + it("returns undefined outside run()", () => { + expect(tenantContext.get()).toBeUndefined(); + }); + + it("returns the active context inside run()", () => { + tenantContext.run(sample, () => { + expect(tenantContext.get()).toEqual(sample); + }); + }); + + it("isolates concurrent async trees", async () => { + const a: TenantContext = { ...sample, org: { id: "org_a", slug: "a" } }; + const b: TenantContext = { ...sample, org: { id: "org_b", slug: "b" } }; + + const [got1, got2] = await Promise.all([ + tenantContext.run(a, async () => { + await new Promise((r) => setTimeout(r, 10)); + return tenantContext.get()?.org.id; + }), + tenantContext.run(b, async () => { + await new Promise((r) => setTimeout(r, 5)); + return tenantContext.get()?.org.id; + }), + ]); + expect(got1).toBe("org_a"); + expect(got2).toBe("org_b"); + }); + + it("supports nested run() overriding", () => { + const inner: TenantContext = { ...sample, org: { id: "org_inner", slug: "inner" } }; + tenantContext.run(sample, () => { + tenantContext.run(inner, () => { + expect(tenantContext.get()?.org.id).toBe("org_inner"); + }); + expect(tenantContext.get()?.org.id).toBe("org_1"); + }); + }); +}); diff --git a/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts b/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts new file mode 100644 index 00000000000..d06dd114ab7 --- /dev/null +++ b/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from "vitest"; +import { tenantContextFromAuthEnvironment } from "../app/services/tenantContext.server"; +import type { AuthenticatedEnvironment } from "../app/services/apiAuth.server"; + +// Cast through unknown — we only depend on a narrow slice of +// AuthenticatedEnvironment and don't want to enumerate every field. +const env = { + id: "env_1", + slug: "prod", + type: "PRODUCTION" as const, + organization: { id: "org_1", slug: "acme" }, + project: { id: "proj_1", externalRef: "proj_abc" }, +} as unknown as AuthenticatedEnvironment; + +describe("tenantContextFromAuthEnvironment", () => { + it("maps org id/slug, project id and externalRef, env id/slug/type", () => { + expect(tenantContextFromAuthEnvironment(env)).toEqual({ + org: { id: "org_1", slug: "acme" }, + project: { id: "proj_1", ref: "proj_abc" }, + environment: { id: "env_1", slug: "prod", type: "PRODUCTION" }, + }); + }); + + it("maps DEVELOPMENT environment type", () => { + const dev = { ...env, type: "DEVELOPMENT" as const } as unknown as AuthenticatedEnvironment; + expect(tenantContextFromAuthEnvironment(dev).environment.type).toBe("DEVELOPMENT"); + }); + + it("does not propagate impersonating (auth environments are real, not impersonated)", () => { + expect(tenantContextFromAuthEnvironment(env).impersonating).toBeUndefined(); + }); +}); diff --git a/apps/webapp/test/tenantContextResolver.test.ts b/apps/webapp/test/tenantContextResolver.test.ts new file mode 100644 index 00000000000..2edf4c5df0f --- /dev/null +++ b/apps/webapp/test/tenantContextResolver.test.ts @@ -0,0 +1,156 @@ +import { describe, it, expect, vi } from "vitest"; +import { + createTenantContextMiddleware, + parseTenantPath, + type PathResolver, +} from "../app/services/tenantContextResolver.server"; +import { tenantContext, type TenantContext } from "../app/services/tenantContext.server"; + +const sampleCtx: TenantContext = { + org: { id: "org_1", slug: "acme" }, + project: { id: "proj_1", ref: "proj_abc" }, + environment: { id: "env_1", slug: "prod", type: "PRODUCTION" }, +}; + +describe("parseTenantPath", () => { + it("parses a full env path", () => { + expect(parseTenantPath("/orgs/acme/projects/web/env/prod")).toEqual({ + orgSlug: "acme", + projectParam: "web", + envParam: "prod", + }); + }); + + it("parses a path with extra segments after env", () => { + expect(parseTenantPath("/orgs/acme/projects/web/env/prod/runs/run_1")).toEqual({ + orgSlug: "acme", + projectParam: "web", + envParam: "prod", + }); + }); + + it("parses a path with a query-style suffix already stripped", () => { + expect(parseTenantPath("/orgs/acme/projects/web/env/prod")).toEqual({ + orgSlug: "acme", + projectParam: "web", + envParam: "prod", + }); + }); + + it("returns undefined for non-orgs paths", () => { + expect(parseTenantPath("/healthcheck")).toBeUndefined(); + expect(parseTenantPath("/")).toBeUndefined(); + expect(parseTenantPath("/api/v1/tasks")).toBeUndefined(); + }); + + it("returns undefined when org-only (no project)", () => { + expect(parseTenantPath("/orgs/acme")).toBeUndefined(); + expect(parseTenantPath("/orgs/acme/")).toBeUndefined(); + }); + + it("returns undefined when project but no env", () => { + expect(parseTenantPath("/orgs/acme/projects/web")).toBeUndefined(); + expect(parseTenantPath("/orgs/acme/projects/web/")).toBeUndefined(); + expect(parseTenantPath("/orgs/acme/projects/web/env")).toBeUndefined(); + expect(parseTenantPath("/orgs/acme/projects/web/env/")).toBeUndefined(); + }); + + it("does not match if the prefix is wrong", () => { + expect(parseTenantPath("/foo/orgs/acme/projects/web/env/prod")).toBeUndefined(); + }); + + it("handles slugs with hyphens, digits, and mixed case", () => { + expect(parseTenantPath("/orgs/references-6120/projects/hello-world-bN7m/env/dev")).toEqual({ + orgSlug: "references-6120", + projectParam: "hello-world-bN7m", + envParam: "dev", + }); + }); +}); + +describe("createTenantContextMiddleware", () => { + function makeReq(path: string) { + return { path } as Parameters>[0]; + } + + it("sets ALS context inside next() when resolver returns a context", async () => { + const resolver: PathResolver = vi.fn().mockResolvedValue(sampleCtx); + const middleware = createTenantContextMiddleware(resolver); + + let observed: TenantContext | undefined; + await new Promise((resolve) => { + middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => { + observed = tenantContext.get(); + resolve(); + }); + }); + + expect(observed).toEqual(sampleCtx); + expect(resolver).toHaveBeenCalledWith("/orgs/acme/projects/web/env/prod"); + }); + + it("calls next() without ALS when resolver returns undefined", async () => { + const resolver: PathResolver = vi.fn().mockResolvedValue(undefined); + const middleware = createTenantContextMiddleware(resolver); + + let observed: TenantContext | undefined = sampleCtx; + await new Promise((resolve) => { + middleware(makeReq("/healthcheck"), {} as never, () => { + observed = tenantContext.get(); + resolve(); + }); + }); + + expect(observed).toBeUndefined(); + }); + + it("does not leak ALS context after next() returns", async () => { + const resolver: PathResolver = vi.fn().mockResolvedValue(sampleCtx); + const middleware = createTenantContextMiddleware(resolver); + + await new Promise((resolve) => { + middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => resolve()); + }); + + expect(tenantContext.get()).toBeUndefined(); + }); + + it("isolates concurrent requests", async () => { + const ctxA: TenantContext = { ...sampleCtx, org: { id: "a", slug: "a" } }; + const ctxB: TenantContext = { ...sampleCtx, org: { id: "b", slug: "b" } }; + const resolver: PathResolver = vi.fn(async (path: string) => { + if (path.includes("/a")) return ctxA; + if (path.includes("/b")) return ctxB; + return undefined; + }); + const middleware = createTenantContextMiddleware(resolver); + + const observe = (path: string, delay: number) => + new Promise((resolve) => { + middleware(makeReq(path), {} as never, async () => { + await new Promise((r) => setTimeout(r, delay)); + resolve(tenantContext.get()); + }); + }); + + const [a, b] = await Promise.all([observe("/orgs/a/projects/x/env/y", 10), observe("/orgs/b/projects/x/env/y", 5)]); + expect(a?.org.id).toBe("a"); + expect(b?.org.id).toBe("b"); + }); + + it("calls next() without ALS when the resolver throws", async () => { + const resolver: PathResolver = vi.fn().mockRejectedValue(new Error("boom")); + const middleware = createTenantContextMiddleware(resolver); + + // The middleware itself doesn't catch resolver errors — production + // `resolveTenantContextFromPath` swallows them. We assert that behavior: + // a throwing resolver propagates, so the production resolver MUST catch. + await expect( + new Promise((resolve, reject) => { + middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => resolve()).catch( + reject + ); + }) + ).rejects.toThrow("boom"); + }); +}); From 84c381bd7f80a05bbaf4a932f308e02655170caf Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Wed, 20 May 2026 18:09:09 +0100 Subject: [PATCH 02/11] chore(webapp): trim sentry tenant attribution server-change to one-liner Co-Authored-By: Claude Sonnet 4.6 --- .server-changes/sentry-tenant-attribution.md | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/.server-changes/sentry-tenant-attribution.md b/.server-changes/sentry-tenant-attribution.md index ac230304302..5f4acece6f9 100644 --- a/.server-changes/sentry-tenant-attribution.md +++ b/.server-changes/sentry-tenant-attribution.md @@ -3,13 +3,4 @@ area: webapp type: feature --- -Attach organization / project / environment to every Sentry event so "Users Impacted" counts orgs and events are filterable by tenant. - -Mechanism: `AsyncLocalStorage`-backed `tenantContext` + a Sentry `addEventProcessor` that stamps `user.id = orgId`, `user.username = orgSlug`, and tags (`org_id`, `org_slug`, `project_id`, `project_ref`, `environment_id`, `env_slug`, `env_type`, `impersonating`). - -Wired at the HTTP entry points: - -- API routes — `apiBuilder.server.ts` wraps each handler invocation in `tenantContext.run` using the authenticated `environment`. -- Dashboard requests — an Express middleware (`tenantContextMiddleware`) resolves org/project/env from the URL pattern `/orgs/:org/projects/:project/env/:env/...` and wraps the Remix handler. - -Background workers (redis-worker / schedule-engine) and socket handlers are not yet wired and remain a follow-up. Events from those entry points will continue to ship without tenant attribution until each handler is updated. +Attach org / project / environment context to Sentry events so "Users Impacted" counts tenants and events are filterable by org. From 978b4e599cf55c07d47ab8c0248156b1a52da128 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Wed, 20 May 2026 18:19:53 +0100 Subject: [PATCH 03/11] fix(webapp): address sentry tenant attribution review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add SENTRY_DSN to env.server.ts schema and read it via env (not process.env) to match repo conventions. - Wrap multi-method API handlers in tenantContext.run too — they were missed in the initial pass and would have shipped without tenant tags. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/app/entry.server.tsx | 2 +- apps/webapp/app/env.server.ts | 1 + .../routeBuilders/apiBuilder.server.ts | 20 +++++++++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index cd7155e0b9a..650a7ef860b 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -291,7 +291,7 @@ process.on("uncaughtException", (error, origin) => { singleton("RunEngineEventBusHandlers", registerRunEngineEventBusHandlers); singleton("SetupBatchQueueCallbacks", setupBatchQueueCallbacks); -if (process.env.SENTRY_DSN) { +if (env.SENTRY_DSN) { Sentry.addEventProcessor(addTenantContextToEvent); } diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 6fb6c4ac283..53b813d15a5 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -134,6 +134,7 @@ const EnvironmentSchema = z ELECTRIC_ORIGIN_SHARDS: z.string().optional(), APP_ENV: z.string().default(process.env.NODE_ENV), SERVICE_NAME: z.string().default("trigger.dev webapp"), + SENTRY_DSN: z.string().optional(), POSTHOG_PROJECT_KEY: z.string().default("phc_LFH7kJiGhdIlnO22hTAKgHpaKhpM8gkzWAFvHmf5vfS"), TRIGGER_TELEMETRY_DISABLED: z.string().optional(), AUTH_GITHUB_CLIENT_ID: z.string().optional(), diff --git a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts index ff9f5d7c65b..1698d8718dc 100644 --- a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts +++ b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts @@ -1168,14 +1168,18 @@ export function createMultiMethodApiRoute< } // Dispatch to method handler - const result = await methodConfig.handler({ - params: parsedParams, - searchParams: parsedSearchParams, - headers: parsedHeaders, - body: parsedBody, - authentication: authenticationResult, - request, - }); + const result = await tenantContext.run( + tenantContextFromAuthEnvironment(authenticationResult.environment), + () => + methodConfig.handler({ + params: parsedParams, + searchParams: parsedSearchParams, + headers: parsedHeaders, + body: parsedBody, + authentication: authenticationResult, + request, + }) + ); return await wrapResponse(request, result, corsStrategy !== "none"); } catch (error) { try { From c78e88fc2ce1b5564691800f182fa89218b665e6 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 09:49:50 +0100 Subject: [PATCH 04/11] refactor(webapp): user-based sentry attribution, no middleware DB lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pivot the sentry tenant attribution flow based on review feedback: - `user.id` is now the real signed-in user cuid, so "Users Impacted" counts distinct humans. Tenant context (org / project / env slugs, IDs, env type) moves entirely to tags. - The Express middleware no longer queries the database. It parses the URL with a regex and sets an ALS scope with whatever subset of slugs is present (`/orgs/:o`, `/orgs/:o/projects/:p`, or the full triple). - `_app/route.tsx` enriches the scope with `userId` for any authenticated dashboard request — reusing the existing `requireUser` call. No new query. - The env layout loader's existing `prisma.project.findFirst` gains two extra columns in its select (`externalRef`, `organization.id`) and enriches the scope with the IDs / env type after picking an env. Same single query, no extra round-trip. - API routes flow through `tenantContextFromAuthEnvironment`, which pulls `userId` from `env.orgMember.userId` (already selected by `authIncludeBase`) and stamps the full tenant set up-front. Trade-off: errors that fire before the env layout loader's enrich on env-scoped pages get slugs + `user.id` but no tenant IDs. Realistic errors after async work get the full set. API requests without an `orgMember` get tenant tags but no `user.id`. Out of scope (deferred): background workers, schedule-engine, socket handlers — those events still ship without tenant attribution. Co-Authored-By: Claude Sonnet 4.6 --- .../route.tsx | 15 ++ apps/webapp/app/routes/_app/route.tsx | 2 + .../app/services/tenantContext.server.ts | 38 +++-- .../services/tenantContextResolver.server.ts | 76 +++------- .../app/utils/sentryTenantContext.server.ts | 24 ++-- apps/webapp/test/sentryTenantContext.test.ts | 67 +++++++-- apps/webapp/test/tenantContext.test.ts | 76 ++++++++-- .../tenantContextFromAuthEnvironment.test.ts | 42 ++++-- .../webapp/test/tenantContextResolver.test.ts | 135 +++++++++--------- 9 files changed, 288 insertions(+), 187 deletions(-) diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx index df9ee24be90..98b8c442218 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam/route.tsx @@ -5,6 +5,7 @@ import { redirectWithErrorMessage } from "~/models/message.server"; import { updateCurrentProjectEnvironmentId } from "~/services/dashboardPreferences.server"; import { logger } from "~/services/logger.server"; import { requireUser } from "~/services/session.server"; +import { tenantContext } from "~/services/tenantContext.server"; import { EnvironmentParamSchema, v3ProjectPath } from "~/utils/pathBuilder"; export const loader = async ({ request, params }: LoaderFunctionArgs) => { @@ -26,6 +27,8 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { }, select: { id: true, + externalRef: true, + organization: { select: { id: true } }, environments: { select: { id: true, @@ -52,6 +55,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { } let environmentId: string | undefined = undefined; + let environmentType: "DEVELOPMENT" | "PREVIEW" | "STAGING" | "PRODUCTION" | undefined; if (environments.length > 1) { const bestEnvironment = environments.find((env) => env.orgMember?.userId === user.id); @@ -63,10 +67,21 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { } environmentId = bestEnvironment.id; + environmentType = bestEnvironment.type; } else { environmentId = environments[0].id; + environmentType = environments[0].type; } + // userId is enriched higher up in `_app/route.tsx`; only stamp tenant fields here. + tenantContext.enrich({ + orgId: project.organization.id, + projectId: project.id, + projectRef: project.externalRef, + envId: environmentId, + envType: environmentType, + }); + await updateCurrentProjectEnvironmentId({ user: user, projectId: project.id, environmentId }); return project; diff --git a/apps/webapp/app/routes/_app/route.tsx b/apps/webapp/app/routes/_app/route.tsx index 23e1b3834c6..db12ae90b60 100644 --- a/apps/webapp/app/routes/_app/route.tsx +++ b/apps/webapp/app/routes/_app/route.tsx @@ -5,10 +5,12 @@ import { RouteErrorDisplay } from "~/components/ErrorDisplay"; import { AppContainer, MainCenteredContainer } from "~/components/layout/AppLayout"; import { clearRedirectTo, commitSession } from "~/services/redirectTo.server"; import { requireUser } from "~/services/session.server"; +import { tenantContext } from "~/services/tenantContext.server"; import { confirmBasicDetailsPath } from "~/utils/pathBuilder"; export const loader = async ({ request }: LoaderFunctionArgs) => { const user = await requireUser(request); + tenantContext.enrich({ userId: user.id }); //you have to confirm basic details before you can do anything if (!user.confirmedBasicDetails) { diff --git a/apps/webapp/app/services/tenantContext.server.ts b/apps/webapp/app/services/tenantContext.server.ts index ffd78a4758f..0cadaa9f4c2 100644 --- a/apps/webapp/app/services/tenantContext.server.ts +++ b/apps/webapp/app/services/tenantContext.server.ts @@ -1,14 +1,22 @@ import { AsyncLocalStorage } from "node:async_hooks"; import type { AuthenticatedEnvironment } from "./apiAuth.server"; +// All fields are optional. The middleware establishes an empty scope per +// request; entry points fill what they know: +// - URL-matching paths get the slug trio from the Express middleware (zero IO). +// - The `_app` layout adds `userId` for any authenticated request. +// - The env layout adds tenant IDs / env type after its own existing DB query. +// - API routes get the full set up-front from `authenticationResult.environment`. export type TenantContext = { - org: { id: string; slug: string }; - project: { id: string; ref: string }; - environment: { - id: string; - slug: string; - type: "DEVELOPMENT" | "PREVIEW" | "STAGING" | "PRODUCTION"; - }; + userId?: string; + orgSlug?: string; + projectSlug?: string; + envSlug?: string; + orgId?: string; + projectId?: string; + projectRef?: string; + envId?: string; + envType?: "DEVELOPMENT" | "PREVIEW" | "STAGING" | "PRODUCTION"; impersonating?: boolean; }; @@ -21,12 +29,22 @@ export const tenantContext = { get(): TenantContext | undefined { return storage.getStore(); }, + enrich(patch: Partial): void { + const current = storage.getStore(); + if (current) Object.assign(current, patch); + }, }; export function tenantContextFromAuthEnvironment(env: AuthenticatedEnvironment): TenantContext { return { - org: { id: env.organization.id, slug: env.organization.slug }, - project: { id: env.project.id, ref: env.project.externalRef }, - environment: { id: env.id, slug: env.slug, type: env.type }, + userId: env.orgMember?.userId, + orgSlug: env.organization.slug, + projectSlug: env.project.slug, + envSlug: env.slug, + orgId: env.organization.id, + projectId: env.project.id, + projectRef: env.project.externalRef, + envId: env.id, + envType: env.type, }; } diff --git a/apps/webapp/app/services/tenantContextResolver.server.ts b/apps/webapp/app/services/tenantContextResolver.server.ts index 11ea851568b..764670df95f 100644 --- a/apps/webapp/app/services/tenantContextResolver.server.ts +++ b/apps/webapp/app/services/tenantContextResolver.server.ts @@ -1,77 +1,41 @@ import type { NextFunction, Request, Response } from "express"; -import { prisma } from "~/db.server"; import { tenantContext, type TenantContext } from "./tenantContext.server"; -import { logger } from "./logger.server"; const URL_PATTERN = /^\/orgs\/([^/]+)(?:\/projects\/([^/]+)(?:\/env\/([^/]+))?)?/; export type ParsedTenantPath = { orgSlug: string; - projectParam: string; - envParam: string; + projectSlug?: string; + envSlug?: string; }; +// Pulls whatever tenant slugs are present in the URL. `/orgs/:o` returns the +// org alone; `/orgs/:o/projects/:p` adds the project; `/orgs/:o/projects/:p/env/:e` +// returns all three. Non-tenant paths (`/`, `/login`, `/admin/*`) return undefined. export function parseTenantPath(pathname: string): ParsedTenantPath | undefined { const match = pathname.match(URL_PATTERN); if (!match) return undefined; - const [, orgSlug, projectParam, envParam] = match; - if (!orgSlug || !projectParam || !envParam) return undefined; - return { orgSlug, projectParam, envParam }; + const [, orgSlug, projectSlug, envSlug] = match; + if (!orgSlug) return undefined; + return { + orgSlug, + ...(projectSlug ? { projectSlug } : {}), + ...(envSlug ? { envSlug } : {}), + }; } -export async function resolveTenantContextFromPath( - pathname: string -): Promise { - const parsed = parseTenantPath(pathname); - if (!parsed) return undefined; - - try { - const env = await prisma.runtimeEnvironment.findFirst({ - where: { - slug: parsed.envParam, - project: { slug: parsed.projectParam, organization: { slug: parsed.orgSlug } }, - }, - select: { - id: true, - slug: true, - type: true, - project: { select: { id: true, externalRef: true } }, - organization: { select: { id: true, slug: true } }, - }, - }); - if (!env) return undefined; - return { - org: { id: env.organization.id, slug: env.organization.slug }, - project: { id: env.project.id, ref: env.project.externalRef }, - environment: { - id: env.id, - slug: env.slug, - type: env.type, - }, - }; - } catch (error) { - logger.warn("tenantContextResolver: lookup failed", { - error: error instanceof Error ? error.message : String(error), - pathname, - }); - return undefined; - } +export function resolveTenantContextFromPath(pathname: string): TenantContext { + return parseTenantPath(pathname) ?? {}; } -export type PathResolver = (pathname: string) => Promise; +export type PathResolver = (pathname: string) => TenantContext; export function createTenantContextMiddleware(resolver: PathResolver) { - return async function tenantContextMiddleware( - req: Request, - res: Response, - next: NextFunction - ) { - const ctx = await resolver(req.path); - if (ctx) { - tenantContext.run(ctx, () => next()); - } else { - next(); - } + // Always establish an ALS scope, even when the path carries no tenant + // slugs. Authenticated loaders (e.g. the `_app` layout) then enrich the + // same scope with `userId`, so non-tenant pages still get user attribution. + return function tenantContextMiddleware(req: Request, res: Response, next: NextFunction) { + tenantContext.run(resolver(req.path), () => next()); }; } diff --git a/apps/webapp/app/utils/sentryTenantContext.server.ts b/apps/webapp/app/utils/sentryTenantContext.server.ts index b7fa118ceb3..303c3b32739 100644 --- a/apps/webapp/app/utils/sentryTenantContext.server.ts +++ b/apps/webapp/app/utils/sentryTenantContext.server.ts @@ -6,20 +6,20 @@ export function addTenantContextToEvent(event: Event, _hint: EventHint): Event { if (!ctx) return event; return { ...event, - user: { - ...event.user, - id: ctx.org.id, - username: ctx.org.slug, - }, + // Only stamp user.id when we have a real user — keeps "Users Impacted" + // counting distinct humans rather than mixing in tenants. Events without + // a known user (e.g. unauthenticated paths) skip user attribution. + ...(ctx.userId ? { user: { ...event.user, id: ctx.userId } } : {}), tags: { ...event.tags, - org_id: ctx.org.id, - org_slug: ctx.org.slug, - project_id: ctx.project.id, - project_ref: ctx.project.ref, - environment_id: ctx.environment.id, - env_slug: ctx.environment.slug, - env_type: ctx.environment.type, + ...(ctx.orgSlug ? { org_slug: ctx.orgSlug } : {}), + ...(ctx.projectSlug ? { project_slug: ctx.projectSlug } : {}), + ...(ctx.envSlug ? { env_slug: ctx.envSlug } : {}), + ...(ctx.orgId ? { org_id: ctx.orgId } : {}), + ...(ctx.projectId ? { project_id: ctx.projectId } : {}), + ...(ctx.projectRef ? { project_ref: ctx.projectRef } : {}), + ...(ctx.envId ? { environment_id: ctx.envId } : {}), + ...(ctx.envType ? { env_type: ctx.envType } : {}), ...(ctx.impersonating ? { impersonating: "true" } : {}), }, }; diff --git a/apps/webapp/test/sentryTenantContext.test.ts b/apps/webapp/test/sentryTenantContext.test.ts index cdc31d7e854..3cba145be7e 100644 --- a/apps/webapp/test/sentryTenantContext.test.ts +++ b/apps/webapp/test/sentryTenantContext.test.ts @@ -3,10 +3,20 @@ import type { Event } from "@sentry/remix"; import { tenantContext } from "../app/services/tenantContext.server"; import { addTenantContextToEvent } from "../app/utils/sentryTenantContext.server"; -const sample = { - org: { id: "org_1", slug: "acme" }, - project: { id: "proj_1", ref: "proj_abc" }, - environment: { id: "env_1", slug: "prod", type: "PRODUCTION" as const }, +const slugOnly = { + orgSlug: "acme", + projectSlug: "web", + envSlug: "prod", +}; + +const enrichedWithUser = { + ...slugOnly, + userId: "usr_42", + orgId: "org_1", + projectId: "proj_1", + projectRef: "proj_abc", + envId: "env_1", + envType: "PRODUCTION" as const, }; describe("addTenantContextToEvent", () => { @@ -16,37 +26,68 @@ describe("addTenantContextToEvent", () => { expect(out).toEqual(event); }); - it("stamps user + tags when ALS context is set", () => { - tenantContext.run(sample, () => { + it("stamps only userId when the scope holds just a user (non-tenant page)", () => { + tenantContext.run({ userId: "usr_42" }, () => { const event: Event = { message: "boom", tags: { existing: "1" } }; const out = addTenantContextToEvent(event, {}); - expect(out.user).toEqual({ id: "org_1", username: "acme" }); + expect(out.user).toEqual({ id: "usr_42" }); + expect(out.tags).toEqual({ existing: "1" }); + }); + }); + + it("stamps slug tags and no user.id when only slugs are set", () => { + tenantContext.run(slugOnly, () => { + const event: Event = { message: "boom", tags: { existing: "1" } }; + const out = addTenantContextToEvent(event, {}); + expect(out.user).toBeUndefined(); expect(out.tags).toMatchObject({ existing: "1", - org_id: "org_1", org_slug: "acme", + project_slug: "web", + env_slug: "prod", + }); + expect(out.tags?.org_id).toBeUndefined(); + expect(out.tags?.env_type).toBeUndefined(); + }); + }); + + it("stamps user.id + full tag set when fully enriched", () => { + tenantContext.run(enrichedWithUser, () => { + const out = addTenantContextToEvent({}, {}); + expect(out.user).toEqual({ id: "usr_42" }); + expect(out.tags).toMatchObject({ + org_slug: "acme", + project_slug: "web", + env_slug: "prod", + org_id: "org_1", project_id: "proj_1", project_ref: "proj_abc", environment_id: "env_1", - env_slug: "prod", env_type: "PRODUCTION", }); - expect(out.tags?.impersonating).toBeUndefined(); + }); + }); + + it("emits no slug/ID tags when scope is empty", () => { + tenantContext.run({}, () => { + const out = addTenantContextToEvent({ tags: { existing: "1" } }, {}); + expect(out.tags).toEqual({ existing: "1" }); + expect(out.user).toBeUndefined(); }); }); it("adds impersonating tag when flag set", () => { - tenantContext.run({ ...sample, impersonating: true }, () => { + tenantContext.run({ ...slugOnly, impersonating: true }, () => { const out = addTenantContextToEvent({}, {}); expect(out.tags?.impersonating).toBe("true"); }); }); it("preserves prior event.user fields it does not own", () => { - tenantContext.run(sample, () => { + tenantContext.run(enrichedWithUser, () => { const event: Event = { user: { ip_address: "1.2.3.4" } }; const out = addTenantContextToEvent(event, {}); - expect(out.user).toEqual({ ip_address: "1.2.3.4", id: "org_1", username: "acme" }); + expect(out.user).toEqual({ ip_address: "1.2.3.4", id: "usr_42" }); }); }); }); diff --git a/apps/webapp/test/tenantContext.test.ts b/apps/webapp/test/tenantContext.test.ts index 13c8765d49c..c90c139eacc 100644 --- a/apps/webapp/test/tenantContext.test.ts +++ b/apps/webapp/test/tenantContext.test.ts @@ -2,9 +2,9 @@ import { describe, it, expect } from "vitest"; import { tenantContext, type TenantContext } from "../app/services/tenantContext.server"; const sample: TenantContext = { - org: { id: "org_1", slug: "acme" }, - project: { id: "proj_1", ref: "proj_abc" }, - environment: { id: "env_1", slug: "prod", type: "PRODUCTION" }, + orgSlug: "acme", + projectSlug: "web", + envSlug: "prod", }; describe("tenantContext", () => { @@ -19,30 +19,82 @@ describe("tenantContext", () => { }); it("isolates concurrent async trees", async () => { - const a: TenantContext = { ...sample, org: { id: "org_a", slug: "a" } }; - const b: TenantContext = { ...sample, org: { id: "org_b", slug: "b" } }; + const a: TenantContext = { ...sample, orgSlug: "a" }; + const b: TenantContext = { ...sample, orgSlug: "b" }; const [got1, got2] = await Promise.all([ tenantContext.run(a, async () => { await new Promise((r) => setTimeout(r, 10)); - return tenantContext.get()?.org.id; + return tenantContext.get()?.orgSlug; }), tenantContext.run(b, async () => { await new Promise((r) => setTimeout(r, 5)); - return tenantContext.get()?.org.id; + return tenantContext.get()?.orgSlug; }), ]); - expect(got1).toBe("org_a"); - expect(got2).toBe("org_b"); + expect(got1).toBe("a"); + expect(got2).toBe("b"); }); it("supports nested run() overriding", () => { - const inner: TenantContext = { ...sample, org: { id: "org_inner", slug: "inner" } }; + const inner: TenantContext = { ...sample, orgSlug: "inner" }; tenantContext.run(sample, () => { tenantContext.run(inner, () => { - expect(tenantContext.get()?.org.id).toBe("org_inner"); + expect(tenantContext.get()?.orgSlug).toBe("inner"); }); - expect(tenantContext.get()?.org.id).toBe("org_1"); + expect(tenantContext.get()?.orgSlug).toBe("acme"); }); }); + + it("enrich() patches the active context in-place", () => { + tenantContext.run({ ...sample }, () => { + tenantContext.enrich({ + userId: "usr_1", + orgId: "org_1", + projectId: "proj_1", + envType: "PRODUCTION", + }); + expect(tenantContext.get()).toMatchObject({ + orgSlug: "acme", + projectSlug: "web", + envSlug: "prod", + userId: "usr_1", + orgId: "org_1", + projectId: "proj_1", + envType: "PRODUCTION", + }); + }); + }); + + it("enrich() outside run() is a no-op", () => { + expect(() => tenantContext.enrich({ orgId: "x" })).not.toThrow(); + expect(tenantContext.get()).toBeUndefined(); + }); + + it("supports starting from an empty scope and enriching userId only (non-tenant page)", () => { + tenantContext.run({}, () => { + tenantContext.enrich({ userId: "usr_1" }); + expect(tenantContext.get()).toEqual({ userId: "usr_1" }); + }); + }); + + it("enrich() patches do not bleed across concurrent run() scopes", async () => { + const a: TenantContext = { ...sample, orgSlug: "a" }; + const b: TenantContext = { ...sample, orgSlug: "b" }; + const [got1, got2] = await Promise.all([ + tenantContext.run(a, async () => { + await new Promise((r) => setTimeout(r, 5)); + tenantContext.enrich({ orgId: "org_a" }); + await new Promise((r) => setTimeout(r, 5)); + return tenantContext.get(); + }), + tenantContext.run(b, async () => { + await new Promise((r) => setTimeout(r, 10)); + tenantContext.enrich({ orgId: "org_b" }); + return tenantContext.get(); + }), + ]); + expect(got1?.orgId).toBe("org_a"); + expect(got2?.orgId).toBe("org_b"); + }); }); diff --git a/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts b/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts index d06dd114ab7..163338c0728 100644 --- a/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts +++ b/apps/webapp/test/tenantContextFromAuthEnvironment.test.ts @@ -2,31 +2,47 @@ import { describe, it, expect } from "vitest"; import { tenantContextFromAuthEnvironment } from "../app/services/tenantContext.server"; import type { AuthenticatedEnvironment } from "../app/services/apiAuth.server"; -// Cast through unknown — we only depend on a narrow slice of -// AuthenticatedEnvironment and don't want to enumerate every field. -const env = { +const baseEnv = { id: "env_1", slug: "prod", type: "PRODUCTION" as const, organization: { id: "org_1", slug: "acme" }, - project: { id: "proj_1", externalRef: "proj_abc" }, + project: { id: "proj_1", slug: "web", externalRef: "proj_abc" }, +}; + +const envWithOrgMember = { + ...baseEnv, + orgMember: { userId: "usr_42" }, +} as unknown as AuthenticatedEnvironment; + +const envWithoutOrgMember = { + ...baseEnv, + orgMember: null, } as unknown as AuthenticatedEnvironment; describe("tenantContextFromAuthEnvironment", () => { - it("maps org id/slug, project id and externalRef, env id/slug/type", () => { - expect(tenantContextFromAuthEnvironment(env)).toEqual({ - org: { id: "org_1", slug: "acme" }, - project: { id: "proj_1", ref: "proj_abc" }, - environment: { id: "env_1", slug: "prod", type: "PRODUCTION" }, + it("returns the full tenant context (slugs + IDs + env type + userId) when orgMember is present", () => { + expect(tenantContextFromAuthEnvironment(envWithOrgMember)).toEqual({ + userId: "usr_42", + orgSlug: "acme", + projectSlug: "web", + envSlug: "prod", + orgId: "org_1", + projectId: "proj_1", + projectRef: "proj_abc", + envId: "env_1", + envType: "PRODUCTION", }); }); - it("maps DEVELOPMENT environment type", () => { - const dev = { ...env, type: "DEVELOPMENT" as const } as unknown as AuthenticatedEnvironment; - expect(tenantContextFromAuthEnvironment(dev).environment.type).toBe("DEVELOPMENT"); + it("omits userId when there is no orgMember on the environment", () => { + const ctx = tenantContextFromAuthEnvironment(envWithoutOrgMember); + expect(ctx.userId).toBeUndefined(); + expect(ctx.orgSlug).toBe("acme"); + expect(ctx.envSlug).toBe("prod"); }); it("does not propagate impersonating (auth environments are real, not impersonated)", () => { - expect(tenantContextFromAuthEnvironment(env).impersonating).toBeUndefined(); + expect(tenantContextFromAuthEnvironment(envWithOrgMember).impersonating).toBeUndefined(); }); }); diff --git a/apps/webapp/test/tenantContextResolver.test.ts b/apps/webapp/test/tenantContextResolver.test.ts index 2edf4c5df0f..e531a11bad8 100644 --- a/apps/webapp/test/tenantContextResolver.test.ts +++ b/apps/webapp/test/tenantContextResolver.test.ts @@ -2,38 +2,31 @@ import { describe, it, expect, vi } from "vitest"; import { createTenantContextMiddleware, parseTenantPath, + resolveTenantContextFromPath, type PathResolver, } from "../app/services/tenantContextResolver.server"; import { tenantContext, type TenantContext } from "../app/services/tenantContext.server"; const sampleCtx: TenantContext = { - org: { id: "org_1", slug: "acme" }, - project: { id: "proj_1", ref: "proj_abc" }, - environment: { id: "env_1", slug: "prod", type: "PRODUCTION" }, + orgSlug: "acme", + projectSlug: "web", + envSlug: "prod", }; describe("parseTenantPath", () => { it("parses a full env path", () => { expect(parseTenantPath("/orgs/acme/projects/web/env/prod")).toEqual({ orgSlug: "acme", - projectParam: "web", - envParam: "prod", + projectSlug: "web", + envSlug: "prod", }); }); it("parses a path with extra segments after env", () => { expect(parseTenantPath("/orgs/acme/projects/web/env/prod/runs/run_1")).toEqual({ orgSlug: "acme", - projectParam: "web", - envParam: "prod", - }); - }); - - it("parses a path with a query-style suffix already stripped", () => { - expect(parseTenantPath("/orgs/acme/projects/web/env/prod")).toEqual({ - orgSlug: "acme", - projectParam: "web", - envParam: "prod", + projectSlug: "web", + envSlug: "prod", }); }); @@ -43,16 +36,21 @@ describe("parseTenantPath", () => { expect(parseTenantPath("/api/v1/tasks")).toBeUndefined(); }); - it("returns undefined when org-only (no project)", () => { - expect(parseTenantPath("/orgs/acme")).toBeUndefined(); - expect(parseTenantPath("/orgs/acme/")).toBeUndefined(); + it("returns org-only when path has just the org slug", () => { + expect(parseTenantPath("/orgs/acme")).toEqual({ orgSlug: "acme" }); + expect(parseTenantPath("/orgs/acme/")).toEqual({ orgSlug: "acme" }); + expect(parseTenantPath("/orgs/acme/settings")).toEqual({ orgSlug: "acme" }); }); - it("returns undefined when project but no env", () => { - expect(parseTenantPath("/orgs/acme/projects/web")).toBeUndefined(); - expect(parseTenantPath("/orgs/acme/projects/web/")).toBeUndefined(); - expect(parseTenantPath("/orgs/acme/projects/web/env")).toBeUndefined(); - expect(parseTenantPath("/orgs/acme/projects/web/env/")).toBeUndefined(); + it("returns org + project when env is missing", () => { + expect(parseTenantPath("/orgs/acme/projects/web")).toEqual({ + orgSlug: "acme", + projectSlug: "web", + }); + expect(parseTenantPath("/orgs/acme/projects/web/")).toEqual({ + orgSlug: "acme", + projectSlug: "web", + }); }); it("does not match if the prefix is wrong", () => { @@ -62,10 +60,24 @@ describe("parseTenantPath", () => { it("handles slugs with hyphens, digits, and mixed case", () => { expect(parseTenantPath("/orgs/references-6120/projects/hello-world-bN7m/env/dev")).toEqual({ orgSlug: "references-6120", - projectParam: "hello-world-bN7m", - envParam: "dev", + projectSlug: "hello-world-bN7m", + envSlug: "dev", + }); + }); +}); + +describe("resolveTenantContextFromPath", () => { + it("returns a TenantContext shaped from the parsed slugs", () => { + expect(resolveTenantContextFromPath("/orgs/acme/projects/web/env/prod")).toEqual({ + orgSlug: "acme", + projectSlug: "web", + envSlug: "prod", }); }); + + it("returns an empty context when the path does not match (so loaders can still enrich)", () => { + expect(resolveTenantContextFromPath("/healthcheck")).toEqual({}); + }); }); describe("createTenantContextMiddleware", () => { @@ -73,55 +85,49 @@ describe("createTenantContextMiddleware", () => { return { path } as Parameters>[0]; } - it("sets ALS context inside next() when resolver returns a context", async () => { - const resolver: PathResolver = vi.fn().mockResolvedValue(sampleCtx); + it("sets ALS context inside next() when resolver returns a populated context", () => { + const resolver: PathResolver = vi.fn().mockReturnValue(sampleCtx); const middleware = createTenantContextMiddleware(resolver); let observed: TenantContext | undefined; - await new Promise((resolve) => { - middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => { - observed = tenantContext.get(); - resolve(); - }); + middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => { + observed = tenantContext.get(); }); expect(observed).toEqual(sampleCtx); expect(resolver).toHaveBeenCalledWith("/orgs/acme/projects/web/env/prod"); }); - it("calls next() without ALS when resolver returns undefined", async () => { - const resolver: PathResolver = vi.fn().mockResolvedValue(undefined); + it("still establishes an empty ALS scope when resolver returns {} (so loaders can enrich)", () => { + const resolver: PathResolver = vi.fn().mockReturnValue({}); const middleware = createTenantContextMiddleware(resolver); - let observed: TenantContext | undefined = sampleCtx; - await new Promise((resolve) => { - middleware(makeReq("/healthcheck"), {} as never, () => { - observed = tenantContext.get(); - resolve(); - }); + let observed: TenantContext | undefined; + middleware(makeReq("/healthcheck"), {} as never, () => { + observed = tenantContext.get(); + tenantContext.enrich({ userId: "usr_1" }); + observed = tenantContext.get(); }); - expect(observed).toBeUndefined(); + expect(observed).toEqual({ userId: "usr_1" }); }); - it("does not leak ALS context after next() returns", async () => { - const resolver: PathResolver = vi.fn().mockResolvedValue(sampleCtx); + it("does not leak ALS context after next() returns", () => { + const resolver: PathResolver = vi.fn().mockReturnValue(sampleCtx); const middleware = createTenantContextMiddleware(resolver); - await new Promise((resolve) => { - middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => resolve()); - }); + middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => {}); expect(tenantContext.get()).toBeUndefined(); }); it("isolates concurrent requests", async () => { - const ctxA: TenantContext = { ...sampleCtx, org: { id: "a", slug: "a" } }; - const ctxB: TenantContext = { ...sampleCtx, org: { id: "b", slug: "b" } }; - const resolver: PathResolver = vi.fn(async (path: string) => { - if (path.includes("/a")) return ctxA; - if (path.includes("/b")) return ctxB; - return undefined; + const ctxA: TenantContext = { ...sampleCtx, orgSlug: "a" }; + const ctxB: TenantContext = { ...sampleCtx, orgSlug: "b" }; + const resolver: PathResolver = vi.fn((path: string) => { + if (path.includes("/a/")) return ctxA; + if (path.includes("/b/")) return ctxB; + return {}; }); const middleware = createTenantContextMiddleware(resolver); @@ -133,24 +139,11 @@ describe("createTenantContextMiddleware", () => { }); }); - const [a, b] = await Promise.all([observe("/orgs/a/projects/x/env/y", 10), observe("/orgs/b/projects/x/env/y", 5)]); - expect(a?.org.id).toBe("a"); - expect(b?.org.id).toBe("b"); - }); - - it("calls next() without ALS when the resolver throws", async () => { - const resolver: PathResolver = vi.fn().mockRejectedValue(new Error("boom")); - const middleware = createTenantContextMiddleware(resolver); - - // The middleware itself doesn't catch resolver errors — production - // `resolveTenantContextFromPath` swallows them. We assert that behavior: - // a throwing resolver propagates, so the production resolver MUST catch. - await expect( - new Promise((resolve, reject) => { - middleware(makeReq("/orgs/acme/projects/web/env/prod"), {} as never, () => resolve()).catch( - reject - ); - }) - ).rejects.toThrow("boom"); + const [a, b] = await Promise.all([ + observe("/orgs/a/projects/x/env/y", 10), + observe("/orgs/b/projects/x/env/y", 5), + ]); + expect(a?.orgSlug).toBe("a"); + expect(b?.orgSlug).toBe("b"); }); }); From 66732985be72cdff2f094dc265258ee147354bf3 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 10:15:18 +0100 Subject: [PATCH 05/11] fix(webapp): wrap sentry tenant processor registration in singleton() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remix's dev mode re-evaluates the entry module on code changes, but `@sentry/remix` lives in node_modules and isn't reloaded — so without a singleton guard each HMR reload was appending another copy of the processor to Sentry's global processor list. Idempotent at runtime (the processor is a pure read+stamp), but the list grew unbounded over a dev session. Matches the pattern used by the two adjacent `singleton()` calls. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/app/entry.server.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index 650a7ef860b..b7f9a0d4702 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -291,9 +291,16 @@ process.on("uncaughtException", (error, origin) => { singleton("RunEngineEventBusHandlers", registerRunEngineEventBusHandlers); singleton("SetupBatchQueueCallbacks", setupBatchQueueCallbacks); -if (env.SENTRY_DSN) { - Sentry.addEventProcessor(addTenantContextToEvent); -} +// Wrapped in singleton() so Remix's dev-mode CJS reloads don't append +// duplicate copies of the processor — Sentry's processor list lives in +// node_modules and persists across module reloads. Idempotent at runtime +// (the processor is a pure read+stamp), but the pattern matches the rest +// of this file. +singleton("SentryTenantContextProcessor", () => { + if (env.SENTRY_DSN) { + Sentry.addEventProcessor(addTenantContextToEvent); + } +}); export { apiRateLimiter } from "./services/apiRateLimit.server"; export { engineRateLimiter } from "./services/engineRateLimit.server"; From 90d64a96aad279729711a8c9519ab7609a853eb0 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 11:16:17 +0100 Subject: [PATCH 06/11] fix(webapp): give PAT-authenticated requests user.id in sentry events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createLoaderPATApiRoute was the only authenticated builder not stamping the scope with the request's user — PAT auth carries `userId` but not an AuthenticatedEnvironment, so the other builders' `tenantContext.run` pattern doesn't apply. Enrich the scope established by the Express middleware with the user id so Sentry events from PAT routes get user-level attribution. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/app/services/routeBuilders/apiBuilder.server.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts index 1698d8718dc..a66de63e113 100644 --- a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts +++ b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts @@ -594,6 +594,11 @@ export function createLoaderPATApiRoute< } } + // PAT auth carries `userId` but no environment — enrich the scope + // the Express middleware established with the authenticated user so + // Sentry events from this handler get user-level attribution. + tenantContext.enrich({ userId: authenticationResult.userId }); + const result = await handler({ params: parsedParams, searchParams: parsedSearchParams, From 7c49164b0d3127314b9e412bac6bd0b7a2cb4821 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 11:28:19 +0100 Subject: [PATCH 07/11] test(webapp): cover createLoaderPATApiRoute tenant context enrichment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavioural test that imports the real builder, runs a handler under stubbed PAT auth, and asserts the handler observes `tenantContext.get().userId` matching the auth result. Catches the regression where the builder used to call `handler(...)` without stamping the scope (verified by reverting the fix locally — test fails). `rbac.authenticatePat` and `updateLastAccessedAtIfStale` are stubbed via `vi.mock` so the test stays unit-scoped (no testcontainer). The rest of the builder runs for real. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/test/apiBuilderPAT.test.ts | 75 ++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 apps/webapp/test/apiBuilderPAT.test.ts diff --git a/apps/webapp/test/apiBuilderPAT.test.ts b/apps/webapp/test/apiBuilderPAT.test.ts new file mode 100644 index 00000000000..4b03f5b6a22 --- /dev/null +++ b/apps/webapp/test/apiBuilderPAT.test.ts @@ -0,0 +1,75 @@ +// Behavioural test for `createLoaderPATApiRoute` to confirm PAT-authenticated +// requests stamp `userId` onto the tenant context (so Sentry events from +// PAT routes get user-level attribution). +// +// PAT auth normally hits the DB via `rbac.authenticatePat`. To keep this +// a unit test, we stub the two DB-touching dependencies — narrow enough +// that the test exercises the wrapping behaviour without bringing up a +// real database. + +import { describe, it, expect, vi } from "vitest"; + +vi.mock("~/services/rbac.server", () => ({ + rbac: { + authenticatePat: vi.fn(async () => ({ + ok: true, + userId: "usr_test_42", + ability: {}, + tokenId: "tok_1", + lastAccessedAt: new Date(), + })), + }, +})); + +vi.mock("~/services/personalAccessToken.server", async (orig) => { + const actual = (await orig()) as Record; + return { + ...actual, + updateLastAccessedAtIfStale: vi.fn(async () => undefined), + }; +}); + +import { tenantContext } from "../app/services/tenantContext.server"; +import { createLoaderPATApiRoute } from "../app/services/routeBuilders/apiBuilder.server"; + +describe("createLoaderPATApiRoute", () => { + it("enriches tenant context with `userId` from the PAT auth result", async () => { + let observedUserId: string | undefined; + + const loader = createLoaderPATApiRoute({}, async () => { + observedUserId = tenantContext.get()?.userId; + return new Response(null, { status: 200 }); + }); + + await tenantContext.run({}, async () => { + await loader({ + request: new Request("http://localhost/api/test", { + headers: { Authorization: "Bearer pat_irrelevant_for_this_test" }, + }), + params: {}, + context: {}, + }); + }); + + expect(observedUserId).toBe("usr_test_42"); + }); + + it("does not leak the enrich across requests once the scope ends", async () => { + const loader = createLoaderPATApiRoute({}, async () => { + return new Response(null, { status: 200 }); + }); + + await tenantContext.run({}, async () => { + await loader({ + request: new Request("http://localhost/api/test", { + headers: { Authorization: "Bearer pat_irrelevant_for_this_test" }, + }), + params: {}, + context: {}, + }); + }); + + // Outside the run() scope, the enrich is gone with the scope. + expect(tenantContext.get()).toBeUndefined(); + }); +}); From 253c5e0f3e7548cc453aececaab2f036710881f6 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 11:40:53 +0100 Subject: [PATCH 08/11] test(webapp): replace stubs with real DB seed in PAT tenant-context test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous version stubbed `rbac.authenticatePat` and `updateLastAccessedAtIfStale` to keep the test unit-scoped, but that violated the "never mock — use a real database" guidance in CLAUDE.md. The webapp's vitest setup already points `~/db.server` at the local postgres (`apps/webapp/test/setup.ts` loads `apps/webapp/.env`), so the test can seed a real User + PAT via Prisma and let `rbac.authenticatePat` validate end-to-end. No mocks. Cleanup runs in `afterAll` — deletes the PATs and the user. Verified to fail if the `enrich` call is removed from `createLoaderPATApiRoute`. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/test/apiBuilderPAT.test.ts | 91 ++++++++++++++++---------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/apps/webapp/test/apiBuilderPAT.test.ts b/apps/webapp/test/apiBuilderPAT.test.ts index 4b03f5b6a22..761309cd114 100644 --- a/apps/webapp/test/apiBuilderPAT.test.ts +++ b/apps/webapp/test/apiBuilderPAT.test.ts @@ -1,41 +1,48 @@ -// Behavioural test for `createLoaderPATApiRoute` to confirm PAT-authenticated +// Integration test for `createLoaderPATApiRoute` — confirms PAT-authenticated // requests stamp `userId` onto the tenant context (so Sentry events from // PAT routes get user-level attribution). // -// PAT auth normally hits the DB via `rbac.authenticatePat`. To keep this -// a unit test, we stub the two DB-touching dependencies — narrow enough -// that the test exercises the wrapping behaviour without bringing up a -// real database. +// Runs against the local postgres the webapp test setup already targets +// (`apps/webapp/.env` → `DATABASE_URL`). Seeds a real User + PersonalAccessToken +// via Prisma, calls the real loader with the real bearer, and lets +// `rbac.authenticatePat` validate against the DB end-to-end. No stubs. +// +// Cleans up the rows it creates so the test is repeatable. -import { describe, it, expect, vi } from "vitest"; +import { afterAll, describe, expect, it } from "vitest"; +import { prisma } from "../app/db.server"; +import { createPersonalAccessToken } from "../app/services/personalAccessToken.server"; +import { tenantContext } from "../app/services/tenantContext.server"; +import { createLoaderPATApiRoute } from "../app/services/routeBuilders/apiBuilder.server"; -vi.mock("~/services/rbac.server", () => ({ - rbac: { - authenticatePat: vi.fn(async () => ({ - ok: true, - userId: "usr_test_42", - ability: {}, - tokenId: "tok_1", - lastAccessedAt: new Date(), - })), - }, -})); +const cleanup: Array<() => Promise> = []; -vi.mock("~/services/personalAccessToken.server", async (orig) => { - const actual = (await orig()) as Record; - return { - ...actual, - updateLastAccessedAtIfStale: vi.fn(async () => undefined), - }; +afterAll(async () => { + for (const fn of cleanup) { + await fn().catch(() => {}); + } }); -import { tenantContext } from "../app/services/tenantContext.server"; -import { createLoaderPATApiRoute } from "../app/services/routeBuilders/apiBuilder.server"; - describe("createLoaderPATApiRoute", () => { - it("enriches tenant context with `userId` from the PAT auth result", async () => { - let observedUserId: string | undefined; + it("enriches tenant context with the authenticated PAT's userId", async () => { + const suffix = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + const user = await prisma.user.create({ + data: { + email: `pat-tenant-test-${suffix}@test.local`, + authenticationMethod: "MAGIC_LINK", + }, + }); + cleanup.push(async () => { + await prisma.personalAccessToken.deleteMany({ where: { userId: user.id } }); + await prisma.user.delete({ where: { id: user.id } }); + }); + const created = await createPersonalAccessToken({ + name: `pat-tenant-test-${suffix}`, + userId: user.id, + }); + + let observedUserId: string | undefined; const loader = createLoaderPATApiRoute({}, async () => { observedUserId = tenantContext.get()?.userId; return new Response(null, { status: 200 }); @@ -44,32 +51,46 @@ describe("createLoaderPATApiRoute", () => { await tenantContext.run({}, async () => { await loader({ request: new Request("http://localhost/api/test", { - headers: { Authorization: "Bearer pat_irrelevant_for_this_test" }, + headers: { Authorization: `Bearer ${created.token}` }, }), params: {}, context: {}, }); }); - expect(observedUserId).toBe("usr_test_42"); + expect(observedUserId).toBe(user.id); }); - it("does not leak the enrich across requests once the scope ends", async () => { - const loader = createLoaderPATApiRoute({}, async () => { - return new Response(null, { status: 200 }); + it("does not leave the enrich behind once the request scope ends", async () => { + const suffix = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + const user = await prisma.user.create({ + data: { + email: `pat-tenant-leak-${suffix}@test.local`, + authenticationMethod: "MAGIC_LINK", + }, }); + cleanup.push(async () => { + await prisma.personalAccessToken.deleteMany({ where: { userId: user.id } }); + await prisma.user.delete({ where: { id: user.id } }); + }); + + const created = await createPersonalAccessToken({ + name: `pat-tenant-leak-${suffix}`, + userId: user.id, + }); + + const loader = createLoaderPATApiRoute({}, async () => new Response(null, { status: 200 })); await tenantContext.run({}, async () => { await loader({ request: new Request("http://localhost/api/test", { - headers: { Authorization: "Bearer pat_irrelevant_for_this_test" }, + headers: { Authorization: `Bearer ${created.token}` }, }), params: {}, context: {}, }); }); - // Outside the run() scope, the enrich is gone with the scope. expect(tenantContext.get()).toBeUndefined(); }); }); From 487f6e3794df7cc4dbbb233b39fdc3c343f35266 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 12:14:49 +0100 Subject: [PATCH 09/11] =?UTF-8?q?test(webapp):=20remove=20apiBuilderPAT.te?= =?UTF-8?q?st.ts=20=E2=80=94=20CI=20lacks=20local=20postgres?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test relied on `~/db.server` reaching a postgres at localhost:5432. That works locally with the dev docker stack but not in CI, which only provides per-test containers via `@internal/testcontainers`. The webapp's singleton Prisma client can't easily be redirected to a per-test container without `vi.hoisted` plumbing, so dropping the test rather than carrying broken CI. The PAT enrich line is one line and exercised by manual verification — matches the rest of `apiBuilder.server.ts`, where the other builders also have no unit tests. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/test/apiBuilderPAT.test.ts | 96 -------------------------- 1 file changed, 96 deletions(-) delete mode 100644 apps/webapp/test/apiBuilderPAT.test.ts diff --git a/apps/webapp/test/apiBuilderPAT.test.ts b/apps/webapp/test/apiBuilderPAT.test.ts deleted file mode 100644 index 761309cd114..00000000000 --- a/apps/webapp/test/apiBuilderPAT.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -// Integration test for `createLoaderPATApiRoute` — confirms PAT-authenticated -// requests stamp `userId` onto the tenant context (so Sentry events from -// PAT routes get user-level attribution). -// -// Runs against the local postgres the webapp test setup already targets -// (`apps/webapp/.env` → `DATABASE_URL`). Seeds a real User + PersonalAccessToken -// via Prisma, calls the real loader with the real bearer, and lets -// `rbac.authenticatePat` validate against the DB end-to-end. No stubs. -// -// Cleans up the rows it creates so the test is repeatable. - -import { afterAll, describe, expect, it } from "vitest"; -import { prisma } from "../app/db.server"; -import { createPersonalAccessToken } from "../app/services/personalAccessToken.server"; -import { tenantContext } from "../app/services/tenantContext.server"; -import { createLoaderPATApiRoute } from "../app/services/routeBuilders/apiBuilder.server"; - -const cleanup: Array<() => Promise> = []; - -afterAll(async () => { - for (const fn of cleanup) { - await fn().catch(() => {}); - } -}); - -describe("createLoaderPATApiRoute", () => { - it("enriches tenant context with the authenticated PAT's userId", async () => { - const suffix = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; - const user = await prisma.user.create({ - data: { - email: `pat-tenant-test-${suffix}@test.local`, - authenticationMethod: "MAGIC_LINK", - }, - }); - cleanup.push(async () => { - await prisma.personalAccessToken.deleteMany({ where: { userId: user.id } }); - await prisma.user.delete({ where: { id: user.id } }); - }); - - const created = await createPersonalAccessToken({ - name: `pat-tenant-test-${suffix}`, - userId: user.id, - }); - - let observedUserId: string | undefined; - const loader = createLoaderPATApiRoute({}, async () => { - observedUserId = tenantContext.get()?.userId; - return new Response(null, { status: 200 }); - }); - - await tenantContext.run({}, async () => { - await loader({ - request: new Request("http://localhost/api/test", { - headers: { Authorization: `Bearer ${created.token}` }, - }), - params: {}, - context: {}, - }); - }); - - expect(observedUserId).toBe(user.id); - }); - - it("does not leave the enrich behind once the request scope ends", async () => { - const suffix = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; - const user = await prisma.user.create({ - data: { - email: `pat-tenant-leak-${suffix}@test.local`, - authenticationMethod: "MAGIC_LINK", - }, - }); - cleanup.push(async () => { - await prisma.personalAccessToken.deleteMany({ where: { userId: user.id } }); - await prisma.user.delete({ where: { id: user.id } }); - }); - - const created = await createPersonalAccessToken({ - name: `pat-tenant-leak-${suffix}`, - userId: user.id, - }); - - const loader = createLoaderPATApiRoute({}, async () => new Response(null, { status: 200 })); - - await tenantContext.run({}, async () => { - await loader({ - request: new Request("http://localhost/api/test", { - headers: { Authorization: `Bearer ${created.token}` }, - }), - params: {}, - context: {}, - }); - }); - - expect(tenantContext.get()).toBeUndefined(); - }); -}); From 9b72f9310d606c9153b70c58f1c60d32b668b194 Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 13:23:10 +0100 Subject: [PATCH 10/11] chore(webapp): clarify sentry tenant-attribution changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-based attribution (real user.id from requireUser / orgMember.userId) with org/project/env on tags when context is available — was the design pivot during review. Co-Authored-By: Claude Sonnet 4.6 --- .server-changes/sentry-tenant-attribution.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.server-changes/sentry-tenant-attribution.md b/.server-changes/sentry-tenant-attribution.md index 5f4acece6f9..2aaf43483ab 100644 --- a/.server-changes/sentry-tenant-attribution.md +++ b/.server-changes/sentry-tenant-attribution.md @@ -3,4 +3,4 @@ area: webapp type: feature --- -Attach org / project / environment context to Sentry events so "Users Impacted" counts tenants and events are filterable by org. +Stamp Sentry events with the signed-in user so "Users Impacted" counts individual humans, and enrich events with org / project / environment tags when that context is available (dashboard URLs, authenticated API requests). From 307d39d4bfe8abb30cb7dc6f2f0087e1432a8f9d Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Thu, 21 May 2026 13:37:11 +0100 Subject: [PATCH 11/11] fix(webapp): make sentry processor singleton actually deduplicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit singleton() uses `__trigger_singletons[name] ??= getValue()`. A `void`-returning callback stores `undefined` in the cache, which `??=` treats as "still unset" — so every Remix dev reload re-evaluates the callback and re-registers the Sentry processor (the exact problem this wrapper was meant to prevent). Return `true` so the cache holds a real value and the guard kicks in. Co-Authored-By: Claude Sonnet 4.6 --- apps/webapp/app/entry.server.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index b7f9a0d4702..a2d4a6a64c6 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -300,6 +300,9 @@ singleton("SentryTenantContextProcessor", () => { if (env.SENTRY_DSN) { Sentry.addEventProcessor(addTenantContextToEvent); } + // Return a truthy value — `singleton()` uses `??=` so a `void` + // callback would re-execute (and re-register) on every dev reload. + return true; }); export { apiRateLimiter } from "./services/apiRateLimit.server";