fix(webapp): stop API auth failures leaking the controller's raw error string
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 <prod RDS hostname>")
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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) };
|
||||
}
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user