Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/sanitize-auth-failure-messages.md
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 15 additions & 2 deletions apps/webapp/app/services/routeBuilders/apiBuilder.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
);
}
Expand Down
39 changes: 39 additions & 0 deletions apps/webapp/app/services/routeBuilders/publicAuthError.ts
Original file line number Diff line number Diff line change
@@ -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) };
}
15 changes: 15 additions & 0 deletions apps/webapp/test/api-auth.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
39 changes: 39 additions & 0 deletions apps/webapp/test/publicAuthError.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
Loading