From 2bd1bf6189f826cfab9a2efd9c003abb2efa419f Mon Sep 17 00:00:00 2001 From: Daniel Sutton Date: Fri, 5 Jun 2026 13:22:29 +0100 Subject: [PATCH] fix(webapp): stop API auth failures leaking the controller's raw error string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auth controllers (the OSS RBAC fallback and the cloud RBAC plugin) return an `error` string on failure that the apiBuilder forwarded verbatim into the response body. A controller can conflate an infrastructure failure with an auth rejection — when the database is unreachable the plugin's key lookup throws a Prisma error ("Can't reach database server at ") and returns it as the auth error. The SDK then surfaced that string in the customer's run view via TriggerApiError, leaking internal infra detail. This evaded the two prior leak fixes because both were scoped to exceptions on 5xx responses; this leak is a returned value on a 4xx through the auth channel. Sanitize at the single auth chokepoint: derive the client message purely from the status (401 -> "Invalid credentials", 403 -> "Forbidden") and log the controller's raw string server-side. Applied to both the bearer bridge and the PAT path. Status codes, body shape, and machine-readable fields are unchanged; only the human-readable message text changes. Co-Authored-By: Claude Opus 4.8 --- .../sanitize-auth-failure-messages.md | 6 +++ .../routeBuilders/apiBuilder.server.ts | 17 +++++++- .../services/routeBuilders/publicAuthError.ts | 39 +++++++++++++++++++ apps/webapp/test/api-auth.e2e.test.ts | 15 +++++++ apps/webapp/test/publicAuthError.test.ts | 39 +++++++++++++++++++ 5 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 .server-changes/sanitize-auth-failure-messages.md create mode 100644 apps/webapp/app/services/routeBuilders/publicAuthError.ts create mode 100644 apps/webapp/test/publicAuthError.test.ts diff --git a/.server-changes/sanitize-auth-failure-messages.md b/.server-changes/sanitize-auth-failure-messages.md new file mode 100644 index 00000000000..a29d08a4d2e --- /dev/null +++ b/.server-changes/sanitize-auth-failure-messages.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Stop API auth failures from leaking the auth controller's raw error string (e.g. internal DB connection errors) to clients; return a fixed status-derived message instead. diff --git a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts index 0c661a7e684..425905c9cf5 100644 --- a/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts +++ b/apps/webapp/app/services/routeBuilders/apiBuilder.server.ts @@ -4,6 +4,7 @@ import { ActionFunctionArgs, json, LoaderFunctionArgs } from "@remix-run/server- import { fromZodError } from "zod-validation-error"; import { apiCors } from "~/utils/apiCors"; import { logger } from "../logger.server"; +import { sanitizeAuthFailure } from "./publicAuthError"; import { rbac } from "../rbac.server"; import type { RbacAbility, RbacResource } from "@trigger.dev/rbac"; import { @@ -64,7 +65,14 @@ async function authenticateRequestForApiBuilder( // Plugin auth distinguishes 401 (who are you?) from 403 (you're not // allowed) — e.g. a suspended account or IP block returns 403. // Forwarding the status preserves that semantic for client retry logic. - return { ok: false, status: result.status, error: result.error }; + // + // Never forward the controller's `error` string: a controller can + // conflate an infra failure with an auth rejection (e.g. an + // unreachable DB throws a Prisma error carrying the prod RDS hostname, + // which the plugin returns as the auth error). Log it server-side and + // return a fixed status-derived message instead. See sanitizeAuthFailure. + logger.warn("API bearer auth failed", { status: result.status, error: result.error }); + return { ok: false, ...sanitizeAuthFailure(result) }; } // Plugins return the full AuthenticatedEnvironment shape directly — no @@ -554,9 +562,14 @@ export function createLoaderPATApiRoute< const ctx = contextFn ? await contextFn(parsedParams, request) : {}; const patAuth = await rbac.authenticatePat(request, ctx); if (!patAuth.ok) { + // Same provenance rule as bearer auth: never forward the + // controller's raw `error` string to the client. Log it + // server-side, return a fixed status-derived message. + logger.warn("API PAT auth failed", { status: patAuth.status, error: patAuth.error }); + const safe = sanitizeAuthFailure(patAuth); return await wrapResponse( request, - json({ error: patAuth.error }, { status: patAuth.status }), + json({ error: safe.error }, { status: safe.status }), corsStrategy !== "none" ); } diff --git a/apps/webapp/app/services/routeBuilders/publicAuthError.ts b/apps/webapp/app/services/routeBuilders/publicAuthError.ts new file mode 100644 index 00000000000..eced7800a07 --- /dev/null +++ b/apps/webapp/app/services/routeBuilders/publicAuthError.ts @@ -0,0 +1,39 @@ +/** + * Client-safe auth-failure messages. + * + * Auth controllers (the OSS RBAC fallback and the cloud RBAC plugin) return + * an `error` string on failure. That string is NOT safe to forward to the + * client: a controller can conflate an infrastructure failure with an auth + * rejection — e.g. when the database is unreachable the plugin's key lookup + * throws a Prisma error ("Can't reach database server at ") which it returns as the auth `error`. The apiBuilder then put + * that string into the response body, and the SDK surfaced it verbatim in + * the customer's run view via `TriggerApiError`, leaking internal infra + * detail. + * + * The client only ever needs to know *that* auth failed and *which kind* + * (401 = who are you / 403 = not allowed) so its retry logic can branch. + * It never needs the controller's prose. So we derive the message purely + * from the status and drop the controller's string (logged server-side at + * the call site). This makes leakage impossible regardless of what any + * current or future controller returns — there is no path by which a raw + * internal string reaches the client. + */ +export type AuthFailureStatus = 401 | 403; + +export function publicAuthError(status: AuthFailureStatus): string { + return status === 403 ? "Forbidden" : "Invalid credentials"; +} + +/** + * Replace a controller auth failure's `error` with the status-derived + * client-safe message, preserving the status. The original `error` is + * intentionally discarded here — callers log it server-side before + * sanitizing so full detail is retained in logs. + */ +export function sanitizeAuthFailure(failure: { status: AuthFailureStatus; error: string }): { + status: AuthFailureStatus; + error: string; +} { + return { status: failure.status, error: publicAuthError(failure.status) }; +} diff --git a/apps/webapp/test/api-auth.e2e.test.ts b/apps/webapp/test/api-auth.e2e.test.ts index 31e365d6d40..0dd3d1e6f73 100644 --- a/apps/webapp/test/api-auth.e2e.test.ts +++ b/apps/webapp/test/api-auth.e2e.test.ts @@ -68,6 +68,21 @@ describe("API bearer auth — baseline behavior", () => { const body = await res.json(); expect(body).toHaveProperty("error"); }); + + it("401 body carries only a fixed status-derived message, not the controller's string", async () => { + // The auth controller's own `error` string must never reach the client — + // a controller can conflate an infra failure with an auth rejection and + // leak internal detail (e.g. a Prisma "Can't reach database server" error + // carrying the prod RDS hostname). The body is derived purely from status. + // `/api/v1/runs` uses the apiBuilder/RBAC bridge (createLoaderApiRoute), + // which is the path that surfaced the prod leak via trigger(). + const res = await server.webapp.fetch("/api/v1/runs", { + headers: { Authorization: "Bearer tr_dev_completely_invalid_key_xyz_not_real" }, + }); + const body = (await res.json()) as { error?: string }; + expect(res.status).toBe(401); + expect(body.error).toBe("Invalid credentials"); + }); }); describe("JWT bearer auth — baseline behavior", () => { diff --git a/apps/webapp/test/publicAuthError.test.ts b/apps/webapp/test/publicAuthError.test.ts new file mode 100644 index 00000000000..5a2e884689f --- /dev/null +++ b/apps/webapp/test/publicAuthError.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { publicAuthError, sanitizeAuthFailure } from "../app/services/routeBuilders/publicAuthError.js"; + +describe("publicAuthError", () => { + it("returns a fixed client-safe message for 401", () => { + expect(publicAuthError(401)).toBe("Invalid credentials"); + }); + + it("returns a fixed client-safe message for 403", () => { + expect(publicAuthError(403)).toBe("Forbidden"); + }); +}); + +describe("sanitizeAuthFailure", () => { + it("preserves the status so client retry logic still works", () => { + expect(sanitizeAuthFailure({ status: 401, error: "Invalid API key" }).status).toBe(401); + expect(sanitizeAuthFailure({ status: 403, error: "Unauthorized" }).status).toBe(403); + }); + + it("never echoes the controller's raw error string", () => { + // The exact production leak: the RBAC plugin conflated an infra failure + // with an auth rejection and returned the raw Prisma message — carrying + // the prod RDS hostname — as its `error` string. The SDK then surfaced + // it verbatim in the customer's run view via TriggerApiError. + const leaked = + "Invalid `prisma.project.findFirst()` invocation:\n\nCan't reach database server at " + + "`trigger-app-prod-database.cluster-cghdbxygvjc4.us-east-1.rds.amazonaws.com:5432`"; + + // Sanity: the fixture is genuinely leaky. + expect(leaked).toContain("rds.amazonaws.com"); + + const sanitized = sanitizeAuthFailure({ status: 401, error: leaked }); + + expect(sanitized.error).toBe("Invalid credentials"); + expect(sanitized.error).not.toContain("prisma"); + expect(sanitized.error).not.toContain("rds.amazonaws.com"); + expect(sanitized.error).not.toContain("database server"); + }); +});