fix(webapp): stop API auth failures leaking the controller's raw error string

.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.

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"
         );
       }

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 <prod RDS
+ * hostname>") 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) };
+}

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", () => {

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");
+  });
+});