Skip to content

Commit 2f261e5

Browse files
d-csd-cs
authored
fix(webapp): catch loader/action throws before Remix serializes them (#3664)
## Summary Companion to #3536, which patched routes that already had a leaking `catch (e) { return json({error: e.message}, 500) }`. That pattern can't reach routes which have no catch in the first place — when those throw, Remix's default error path serializes `error.message` into the response body, and the SDK then wraps the leaked string as `TriggerApiError`. Across 28 raw api.v1 loaders/actions plus one dashboard polling endpoint, each handler now: - Wraps its body in `try { ... } catch (error) { ... }`. - Re-throws `Response` instances so auth helpers' `throw json(...)` / `throw redirect(...)` pass through unchanged. - Logs non-Response errors via `logger.error` so server-side visibility is preserved. - Returns a generic body — `{"error": "Internal Server Error"}` 500 for raw API routes, or `{ changelogs: [] }` 200 for the polling widget (degrade silently across transient blips; the consumer hook already coped with empty payloads). For six routes where #3536 left an inner try/catch covering only a service call (`alertChannels`, `batches.results`, `deployments.finalize`, `deployments.background-workers`, `deployments.promote`, `projects.background-workers`): an outer try/catch is added so auth/parsing failures are also sanitized. Inner typed-error handling (`ServiceValidationError` → 422 with message, etc.) is preserved exactly. For two routes whose existing catch returned 400 + `error.message` (`api.v1.authorization-code`, `api.v1.orgs.\$orgParam.projects` action): the body is sanitized to a generic per-route string. **Status code stays 400** — clients that key on the 4xx/5xx distinction (and the SDK's no-retry-on-4xx behavior) are unaffected. ## Test plan - [x] \`pnpm run typecheck --filter webapp\` - [x] Per-route synthetic-throw probe: inject \`throw new Error("SYNTHETIC ...")\` at the top of each catch'd try, curl the route with a dummy bearer, confirm the response body is the generic shape and that the synthetic message lands server-side via \`logger.error\`. 29 routes verified. - [x] Real-P1001 probe on the envvars loader: \`docker stop database\` mid-flight, confirm response is generic 500 (not the leaked Prisma message). - [x] Sampled legitimate 4xx/2xx paths across each pattern variant (naked-wrap, partial-expanded, 400-preserved) to confirm the wraps don't interfere with normal control flow.
1 parent 5dacab0 commit 2f261e5

29 files changed

Lines changed: 1324 additions & 1121 deletions

‎.server-changes/sanitize-loader-action-leaks.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Expand API error response sanitization to additional loaders and actions so internal exception messages (Prisma errors, etc.) no longer leak to callers via 5xx response bodies.

‎apps/webapp/app/routes/api.v1.artifacts.ts‎

Lines changed: 71 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -13,79 +13,85 @@ export async function action({ request }: ActionFunctionArgs) {
1313
return json({ error: "Method Not Allowed" }, { status: 405 });
1414
}
1515

16-
const authenticationResult = await authenticateRequest(request, {
17-
apiKey: true,
18-
organizationAccessToken: false,
19-
personalAccessToken: false,
20-
});
16+
try {
17+
const authenticationResult = await authenticateRequest(request, {
18+
apiKey: true,
19+
organizationAccessToken: false,
20+
personalAccessToken: false,
21+
});
2122

22-
if (!authenticationResult || !authenticationResult.result.ok) {
23-
logger.info("Invalid or missing api key", { url: request.url });
24-
return json({ error: "Invalid or Missing API key" }, { status: 401 });
25-
}
23+
if (!authenticationResult || !authenticationResult.result.ok) {
24+
logger.info("Invalid or missing api key", { url: request.url });
25+
return json({ error: "Invalid or Missing API key" }, { status: 401 });
26+
}
2627

27-
const [, rawBody] = await tryCatch(request.json());
28-
const body = CreateArtifactRequestBody.safeParse(rawBody ?? {});
28+
const [, rawBody] = await tryCatch(request.json());
29+
const body = CreateArtifactRequestBody.safeParse(rawBody ?? {});
2930

30-
if (!body.success) {
31-
return json({ error: "Invalid request body", issues: body.error.issues }, { status: 400 });
32-
}
31+
if (!body.success) {
32+
return json({ error: "Invalid request body", issues: body.error.issues }, { status: 400 });
33+
}
3334

34-
const { environment: authenticatedEnv } = authenticationResult.result;
35+
const { environment: authenticatedEnv } = authenticationResult.result;
3536

36-
const service = new ArtifactsService();
37-
return await service
38-
.createArtifact(body.data.type, authenticatedEnv, body.data.contentLength)
39-
.match(
40-
(result) => {
41-
return json(
42-
{
43-
artifactKey: result.artifactKey,
44-
uploadUrl: result.uploadUrl,
45-
uploadFields: result.uploadFields,
46-
expiresAt: result.expiresAt.toISOString(),
47-
} satisfies CreateArtifactResponseBody,
48-
{ status: 201 }
49-
);
50-
},
51-
(error) => {
52-
switch (error.type) {
53-
case "artifact_size_exceeds_limit": {
54-
logger.warn("Artifact size exceeds limit", { error });
55-
const sizeMB = parseFloat((error.contentLength / (1024 * 1024)).toFixed(1));
56-
const limitMB = parseFloat((error.sizeLimit / (1024 * 1024)).toFixed(1));
37+
const service = new ArtifactsService();
38+
return await service
39+
.createArtifact(body.data.type, authenticatedEnv, body.data.contentLength)
40+
.match(
41+
(result) => {
42+
return json(
43+
{
44+
artifactKey: result.artifactKey,
45+
uploadUrl: result.uploadUrl,
46+
uploadFields: result.uploadFields,
47+
expiresAt: result.expiresAt.toISOString(),
48+
} satisfies CreateArtifactResponseBody,
49+
{ status: 201 }
50+
);
51+
},
52+
(error) => {
53+
switch (error.type) {
54+
case "artifact_size_exceeds_limit": {
55+
logger.warn("Artifact size exceeds limit", { error });
56+
const sizeMB = parseFloat((error.contentLength / (1024 * 1024)).toFixed(1));
57+
const limitMB = parseFloat((error.sizeLimit / (1024 * 1024)).toFixed(1));
5758

58-
let errorMessage;
59+
let errorMessage;
5960

60-
switch (body.data.type) {
61-
case "deployment_context":
62-
errorMessage = `Artifact size (${sizeMB} MB) exceeds the allowed limit of ${limitMB} MB. Make sure you are in the correct directory of your Trigger.dev project. Reach out to us if you are seeing this error consistently.`;
63-
break;
64-
default:
65-
body.data.type satisfies never;
66-
errorMessage = `Artifact size (${sizeMB} MB) exceeds the allowed limit of ${limitMB} MB`;
61+
switch (body.data.type) {
62+
case "deployment_context":
63+
errorMessage = `Artifact size (${sizeMB} MB) exceeds the allowed limit of ${limitMB} MB. Make sure you are in the correct directory of your Trigger.dev project. Reach out to us if you are seeing this error consistently.`;
64+
break;
65+
default:
66+
body.data.type satisfies never;
67+
errorMessage = `Artifact size (${sizeMB} MB) exceeds the allowed limit of ${limitMB} MB`;
68+
}
69+
return json(
70+
{
71+
error: errorMessage,
72+
},
73+
{ status: 400 }
74+
);
75+
}
76+
case "failed_to_create_presigned_post": {
77+
logger.error("Failed to create presigned POST", { error });
78+
return json({ error: "Failed to generate artifact upload URL" }, { status: 500 });
79+
}
80+
case "artifacts_bucket_not_configured": {
81+
logger.error("Artifacts bucket not configured", { error });
82+
return json({ error: "Internal server error" }, { status: 500 });
83+
}
84+
default: {
85+
error satisfies never;
86+
logger.error("Failed creating artifact", { error });
87+
return json({ error: "Internal server error" }, { status: 500 });
6788
}
68-
return json(
69-
{
70-
error: errorMessage,
71-
},
72-
{ status: 400 }
73-
);
74-
}
75-
case "failed_to_create_presigned_post": {
76-
logger.error("Failed to create presigned POST", { error });
77-
return json({ error: "Failed to generate artifact upload URL" }, { status: 500 });
78-
}
79-
case "artifacts_bucket_not_configured": {
80-
logger.error("Artifacts bucket not configured", { error });
81-
return json({ error: "Internal server error" }, { status: 500 });
82-
}
83-
default: {
84-
error satisfies never;
85-
logger.error("Failed creating artifact", { error });
86-
return json({ error: "Internal server error" }, { status: 500 });
8789
}
8890
}
89-
}
90-
);
91+
);
92+
} catch (error) {
93+
if (error instanceof Response) throw error;
94+
logger.error("Failed to create artifact", { error });
95+
return json({ error: "Internal Server Error" }, { status: 500 });
96+
}
9197
}

‎apps/webapp/app/routes/api.v1.auth.jwt.claims.ts‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,26 @@
11
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
22
import { json } from "@remix-run/server-runtime";
33
import { authenticateApiRequest } from "~/services/apiAuth.server";
4+
import { logger } from "~/services/logger.server";
45

56
export async function action({ request }: LoaderFunctionArgs) {
6-
// Next authenticate the request
7-
const authenticationResult = await authenticateApiRequest(request);
7+
try {
8+
// Next authenticate the request
9+
const authenticationResult = await authenticateApiRequest(request);
810

9-
if (!authenticationResult) {
10-
return json({ error: "Invalid or Missing API key" }, { status: 401 });
11-
}
11+
if (!authenticationResult) {
12+
return json({ error: "Invalid or Missing API key" }, { status: 401 });
13+
}
1214

13-
const claims = {
14-
sub: authenticationResult.environment.id,
15-
pub: true,
16-
};
15+
const claims = {
16+
sub: authenticationResult.environment.id,
17+
pub: true,
18+
};
1719

18-
return json(claims);
20+
return json(claims);
21+
} catch (error) {
22+
if (error instanceof Response) throw error;
23+
logger.error("Failed to read auth jwt claims", { error });
24+
return json({ error: "Internal Server Error" }, { status: 500 });
25+
}
1926
}

‎apps/webapp/app/routes/api.v1.auth.jwt.ts‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
22
import { json } from "@remix-run/server-runtime";
33
import { authenticateApiRequest } from "~/services/apiAuth.server";
4+
import { logger } from "~/services/logger.server";
45
import { z } from "zod";
56
import { generateJWT as internal_generateJWT } from "@trigger.dev/core/v3";
67

@@ -14,36 +15,42 @@ const RequestBodySchema = z.object({
1415
});
1516

1617
export async function action({ request }: LoaderFunctionArgs) {
17-
// Next authenticate the request
18-
const authenticationResult = await authenticateApiRequest(request);
19-
20-
if (!authenticationResult) {
21-
return json({ error: "Invalid or Missing API key" }, { status: 401 });
18+
try {
19+
// Next authenticate the request
20+
const authenticationResult = await authenticateApiRequest(request);
21+
22+
if (!authenticationResult) {
23+
return json({ error: "Invalid or Missing API key" }, { status: 401 });
24+
}
25+
26+
const parsedBody = RequestBodySchema.safeParse(await request.json());
27+
28+
if (!parsedBody.success) {
29+
return json(
30+
{ error: "Invalid request body", issues: parsedBody.error.issues },
31+
{ status: 400 }
32+
);
33+
}
34+
35+
const claims = {
36+
sub: authenticationResult.environment.id,
37+
pub: true,
38+
...parsedBody.data.claims,
39+
};
40+
41+
// Sign with the environment's current canonical key, not the raw header key,
42+
// so JWTs minted with a revoked (grace-window) key still validate — validation
43+
// in jwtAuth.server.ts uses environment.apiKey.
44+
const jwt = await internal_generateJWT({
45+
secretKey: authenticationResult.environment.apiKey,
46+
payload: claims,
47+
expirationTime: parsedBody.data.expirationTime ?? "1h",
48+
});
49+
50+
return json({ token: jwt });
51+
} catch (error) {
52+
if (error instanceof Response) throw error;
53+
logger.error("Failed to mint auth jwt", { error });
54+
return json({ error: "Internal Server Error" }, { status: 500 });
2255
}
23-
24-
const parsedBody = RequestBodySchema.safeParse(await request.json());
25-
26-
if (!parsedBody.success) {
27-
return json(
28-
{ error: "Invalid request body", issues: parsedBody.error.issues },
29-
{ status: 400 }
30-
);
31-
}
32-
33-
const claims = {
34-
sub: authenticationResult.environment.id,
35-
pub: true,
36-
...parsedBody.data.claims,
37-
};
38-
39-
// Sign with the environment's current canonical key, not the raw header key,
40-
// so JWTs minted with a revoked (grace-window) key still validate — validation
41-
// in jwtAuth.server.ts uses environment.apiKey.
42-
const jwt = await internal_generateJWT({
43-
secretKey: authenticationResult.environment.apiKey,
44-
payload: claims,
45-
expirationTime: parsedBody.data.expirationTime ?? "1h",
46-
});
47-
48-
return json({ token: jwt });
4956
}

‎apps/webapp/app/routes/api.v1.authorization-code.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export async function action({ request }: ActionFunctionArgs) {
3232
error: error.message,
3333
});
3434

35-
return json({ error: error.message }, { status: 400 });
35+
return json({ error: "Failed to create authorization code" }, { status: 400 });
3636
}
3737

3838
return json({ error: "Something went wrong" }, { status: 500 });

‎apps/webapp/app/routes/api.v1.batches.$batchParam.results.ts‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,38 @@ const ParamsSchema = z.object({
1212
});
1313

1414
export async function loader({ request, params }: LoaderFunctionArgs) {
15-
// Authenticate the request
16-
const authenticationResult = await authenticateApiRequest(request);
15+
try {
16+
// Authenticate the request
17+
const authenticationResult = await authenticateApiRequest(request);
1718

18-
if (!authenticationResult) {
19-
return json({ error: "Invalid or Missing API Key" }, { status: 401 });
20-
}
19+
if (!authenticationResult) {
20+
return json({ error: "Invalid or Missing API Key" }, { status: 401 });
21+
}
2122

22-
const parsed = ParamsSchema.safeParse(params);
23+
const parsed = ParamsSchema.safeParse(params);
2324

24-
if (!parsed.success) {
25-
return json({ error: "Invalid or missing run ID" }, { status: 400 });
26-
}
25+
if (!parsed.success) {
26+
return json({ error: "Invalid or missing run ID" }, { status: 400 });
27+
}
2728

28-
const { batchParam } = parsed.data;
29+
const { batchParam } = parsed.data;
2930

30-
try {
31-
const presenter = new ApiBatchResultsPresenter();
32-
const result = await presenter.call(batchParam, authenticationResult.environment);
31+
try {
32+
const presenter = new ApiBatchResultsPresenter();
33+
const result = await presenter.call(batchParam, authenticationResult.environment);
3334

34-
if (!result) {
35-
return json({ error: "Batch not found" }, { status: 404 });
36-
}
35+
if (!result) {
36+
return json({ error: "Batch not found" }, { status: 404 });
37+
}
3738

38-
return json(result);
39+
return json(result);
40+
} catch (error) {
41+
logger.error("Failed to load batch results", { error });
42+
return json({ error: "Something went wrong, please try again." }, { status: 500 });
43+
}
3944
} catch (error) {
40-
logger.error("Failed to load batch results", { error });
41-
return json({ error: "Something went wrong, please try again." }, { status: 500 });
45+
if (error instanceof Response) throw error;
46+
logger.error("Failed to load batch results (outer)", { error });
47+
return json({ error: "Internal Server Error" }, { status: 500 });
4248
}
4349
}

0 commit comments

Comments
 (0)