Skip to content

Commit 79aae3b

Browse files
pullfrog[bot]Trigger.dev RepoOps
authored andcommitted
fix(webapp): tighten ingestion credential checks
Mono-RevId: 8df337b2de2c4312a39b734e29a840b846628f98
1 parent a9e69ab commit 79aae3b

8 files changed

Lines changed: 229 additions & 43 deletions

‎apps/webapp/app/routes/api.v1.tasks.$taskId.batch.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,17 @@ const { action } = createActionApiRoute(
2929
resource: (params) => ({ type: "tasks", id: params.taskId }),
3030
},
3131
},
32-
async ({ params, headers, body, authentication, ability }) => {
32+
async ({ params, headers, body, authentication, restrictedApiKey, ability }) => {
33+
if (
34+
(authentication.type !== "PRIVATE" || restrictedApiKey) &&
35+
body.items.some((item) => item.options?.payloadType === "application/store")
36+
) {
37+
return json(
38+
{ error: "Stored payloads require an unrestricted private API key" },
39+
{ status: 403 }
40+
);
41+
}
42+
3343
const { taskId } = params;
3444
const {
3545
"idempotency-key": idempotencyKey,

‎apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,15 @@ const { action, loader } = createActionApiRoute(
6262
},
6363
corsStrategy: "all",
6464
},
65-
async ({ body, headers, params, authentication, ability }) => {
66-
if (authentication.type !== "PRIVATE" && body.options?.payloadType === "application/store") {
67-
return json({ error: "Stored payloads require a private API key" }, { status: 403 });
65+
async ({ body, headers, params, authentication, restrictedApiKey, ability }) => {
66+
if (
67+
(authentication.type !== "PRIVATE" || restrictedApiKey) &&
68+
body.options?.payloadType === "application/store"
69+
) {
70+
return json(
71+
{ error: "Stored payloads require an unrestricted private API key" },
72+
{ status: 403 }
73+
);
6874
}
6975

7076
const {

‎apps/webapp/app/routes/api.v1.tasks.batch.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,15 @@ const { action, loader } = createActionApiRoute(
4040
},
4141
corsStrategy: "all",
4242
},
43-
async ({ body, headers, params, authentication }) => {
43+
async ({ body, headers, params, authentication, restrictedApiKey }) => {
4444
if (
45-
authentication.type !== "PRIVATE" &&
45+
(authentication.type !== "PRIVATE" || restrictedApiKey) &&
4646
body.items.some((item) => item.options?.payloadType === "application/store")
4747
) {
48-
return json({ error: "Stored payloads require a private API key" }, { status: 403 });
48+
return json(
49+
{ error: "Stored payloads require an unrestricted private API key" },
50+
{ status: 403 }
51+
);
4952
}
5053

5154
if (!body.items.length) {

‎apps/webapp/app/routes/api.v2.tasks.batch.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,15 @@ const { action, loader } = createActionApiRoute(
4848
},
4949
corsStrategy: "all",
5050
},
51-
async ({ body, headers, params, authentication, ability }) => {
51+
async ({ body, headers, params, authentication, restrictedApiKey, ability }) => {
5252
if (
53-
authentication.type !== "PRIVATE" &&
53+
(authentication.type !== "PRIVATE" || restrictedApiKey) &&
5454
body.items.some((item) => item.options?.payloadType === "application/store")
5555
) {
56-
return json({ error: "Stored payloads require a private API key" }, { status: 403 });
56+
return json(
57+
{ error: "Stored payloads require an unrestricted private API key" },
58+
{ status: 403 }
59+
);
5760
}
5861

5962
if (!body.items.length) {

‎apps/webapp/app/routes/api.v3.batches.$batchId.items.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ export async function action({ request, params }: ActionFunctionArgs) {
7272
return json({ error: authResult.error }, { status: authResult.status });
7373
}
7474

75+
const restrictedApiKey = authResult.subject.type === "apiKey" && authResult.subject.restricted;
76+
7577
// Get the request body stream
7678
const body = request.body;
7779
if (!body) {
@@ -98,7 +100,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
98100
streamToAsyncIterable(parsedStream),
99101
authResult.ability,
100102
batchId,
101-
authResult.subject.type !== "publicJWT"
103+
authResult.subject.type !== "publicJWT" && !restrictedApiKey
102104
);
103105

104106
// Process the stream

‎apps/webapp/app/services/routeBuilders/apiBuilder.server.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,7 @@ type ApiKeyActionHandlerFunction<
12071207
? z.infer<TBodySchema>
12081208
: undefined;
12091209
authentication: ApiAuthenticationResultSuccess;
1210+
restrictedApiKey: boolean;
12101211
ability: RbacAbility;
12111212
request: Request;
12121213
resource?: TResource;
@@ -1444,6 +1445,7 @@ export function createActionApiRoute<
14441445
headers: parsedHeaders,
14451446
body: parsedBody,
14461447
authentication: authenticationResult,
1448+
restrictedApiKey: authResult.restrictedApiKey,
14471449
ability,
14481450
request,
14491451
resource,

‎apps/webapp/test/api-auth.e2e.test.ts‎

Lines changed: 152 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
1010
import type { TestServer } from "@internal/testcontainers/webapp";
1111
import { startTestServer } from "@internal/testcontainers/webapp";
1212
import { generateJWT } from "@trigger.dev/core/v3/jwt";
13+
import { createHash } from "node:crypto";
14+
import { generateAdditionalApiKey } from "~/utils/apiKeys";
1315
import { seedTestEnvironment } from "./helpers/seedTestEnvironment";
1416
import { seedTestPAT, seedTestUser } from "./helpers/seedTestPAT";
1517
import { seedTestRun } from "./helpers/seedTestRun";
@@ -146,41 +148,159 @@ describe("Packet download authorization", () => {
146148
expect(inlineOutput.status).toBe(404);
147149
});
148150

149-
it("rejects stored-payload pointers from public JWTs on trigger", async () => {
150-
const { environment } = await seedTestEnvironment(server.prisma);
151-
const jwt = await generateTestJWT(environment, { scopes: ["write:tasks:test-task"] });
152-
153-
const res = await server.webapp.fetch("/api/v1/tasks/test-task/trigger", {
154-
method: "POST",
155-
headers: { Authorization: `Bearer ${jwt}`, "Content-Type": "application/json" },
156-
body: JSON.stringify({
157-
payload: "run_victim/payload.json",
158-
options: { payloadType: "application/store" },
159-
}),
160-
});
161-
162-
expect(res.status).toBe(403);
163-
});
164-
165-
it("rejects stored-payload pointers from public JWTs on batch trigger", async () => {
166-
const { environment } = await seedTestEnvironment(server.prisma);
167-
const jwt = await generateTestJWT(environment, { scopes: ["write:tasks:test-task"] });
151+
describe.each([
152+
"public JWT",
153+
"restricted API key",
154+
"root API key",
155+
"unrestricted API key",
156+
] as const)("%s stored pointers", (credential) => {
157+
it.each([
158+
"trigger",
159+
"v1 batch",
160+
"v2 batch",
161+
"v3 items",
162+
// This legacy route does not accept public JWTs.
163+
...(credential === "public JWT" ? [] : (["task-scoped v1 batch"] as const)),
164+
] as const)("enforces stored-pointer permission on %s", async (route) => {
165+
const { environment, project } = await seedTestEnvironment(server.prisma);
166+
const { batchFriendlyId: seededBatchFriendlyId, run } = await seedTestRun(server.prisma, {
167+
environmentId: environment.id,
168+
projectId: project.id,
169+
withBatch: route !== "v3 items",
170+
});
171+
let batchFriendlyId = seededBatchFriendlyId;
172+
if (route === "v3 items") {
173+
// Phase 1 initializes both the database record and Redis queue metadata.
174+
const response = await server.webapp.fetch("/api/v3/batches", {
175+
method: "POST",
176+
headers: {
177+
Authorization: `Bearer ${environment.apiKey}`,
178+
"Content-Type": "application/json",
179+
},
180+
body: JSON.stringify({ runCount: 1, taskIdentifiers: ["test-task"] }),
181+
});
182+
expect(response.status).toBe(202);
183+
batchFriendlyId = (await response.json()).id;
184+
} else {
185+
await server.prisma.batchTaskRun.update({
186+
where: { id: run.batchId! },
187+
data: { status: "PENDING", runCount: 1 },
188+
});
189+
}
190+
// Keep JSON-route controls independent of worker/deployment setup.
191+
// The payload gate must run even when a matching run is cached.
192+
const idempotencyKey = "stored-payload-authorization";
193+
await server.prisma.taskRun.update({
194+
where: { id: run.id },
195+
data: {
196+
idempotencyKey,
197+
idempotencyKeyExpiresAt: new Date(Date.now() + 3_600_000),
198+
},
199+
});
168200

169-
const res = await server.webapp.fetch("/api/v2/tasks/batch", {
170-
method: "POST",
171-
headers: { Authorization: `Bearer ${jwt}`, "Content-Type": "application/json" },
172-
body: JSON.stringify({
173-
items: [
174-
{
175-
task: "test-task",
176-
payload: "run_victim/payload.json",
177-
options: { payloadType: "application/store" },
201+
let token: string;
202+
if (credential === "public JWT") {
203+
// Include a batch-wide grant to ensure it cannot bypass the payload gate.
204+
token = await generateTestJWT(environment, {
205+
scopes: ["write:tasks:test-task", `write:batch:${batchFriendlyId}`],
206+
});
207+
} else if (credential === "root API key") {
208+
token = environment.apiKey;
209+
} else {
210+
const user = await seedTestUser(server.prisma);
211+
token = generateAdditionalApiKey("DEVELOPMENT").apiKey;
212+
await server.prisma.apiKey.create({
213+
data: {
214+
name: "Task-scoped ingestion key",
215+
keyHash: createHash("sha256").update(token).digest("hex"),
216+
lastFour: token.slice(-4),
217+
runtimeEnvironmentId: environment.id,
218+
createdByUserId: user.id,
219+
presetId: credential === "restricted API key" ? "TRIGGER_ONLY" : null,
220+
scopes:
221+
credential === "restricted API key"
222+
? ["trigger:tasks:test-task", "batchTrigger:tasks:test-task"]
223+
: ["admin"],
178224
},
179-
],
180-
}),
181-
});
225+
});
226+
}
227+
228+
const item = {
229+
task: "test-task",
230+
index: 0,
231+
payload: "run_other/payload.json",
232+
options: { payloadType: "application/store", idempotencyKey },
233+
};
234+
const path =
235+
route === "trigger"
236+
? "/api/v1/tasks/test-task/trigger"
237+
: route === "v3 items"
238+
? `/api/v3/batches/${batchFriendlyId}/items`
239+
: route === "task-scoped v1 batch"
240+
? "/api/v1/tasks/test-task/batch"
241+
: `/api/${route === "v1 batch" ? "v1" : "v2"}/tasks/batch`;
242+
const res = await server.webapp.fetch(path, {
243+
method: "POST",
244+
headers: {
245+
Authorization: `Bearer ${token}`,
246+
"idempotency-key": idempotencyKey,
247+
"Content-Type": route === "v3 items" ? "application/x-ndjson" : "application/json",
248+
},
249+
body:
250+
route === "v3 items"
251+
? `${JSON.stringify(item)}\n`
252+
: JSON.stringify(route === "trigger" ? item : { items: [item] }),
253+
});
182254

183-
expect(res.status).toBe(403);
255+
if (credential === "public JWT" || credential === "restricted API key") {
256+
expect(res.status).toBe(403);
257+
expect(await res.json()).toEqual({
258+
error:
259+
route === "v3 items"
260+
? "Unauthorized"
261+
: "Stored payloads require an unrestricted private API key",
262+
});
263+
expect(
264+
await server.prisma.taskRun.count({ where: { runtimeEnvironmentId: environment.id } })
265+
).toBe(1);
266+
expect(
267+
await server.prisma.batchTaskRun.count({
268+
where: { runtimeEnvironmentId: environment.id },
269+
})
270+
).toBe(1);
271+
expect(
272+
await server.prisma.batchTaskRun.findFirst({ where: { friendlyId: batchFriendlyId } })
273+
).toMatchObject({ status: "PENDING", sealed: false });
274+
if (route === "task-scoped v1 batch" && credential === "restricted API key") {
275+
const inlineResponse = await server.webapp.fetch(path, {
276+
method: "POST",
277+
headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" },
278+
body: JSON.stringify({
279+
items: [{ payload: { message: "inline" }, options: { idempotencyKey } }],
280+
}),
281+
});
282+
expect(inlineResponse.status).toBe(200);
283+
expect(await inlineResponse.json()).toMatchObject({
284+
batchId: expect.stringMatching(/^batch_/),
285+
runs: [run.friendlyId],
286+
});
287+
}
288+
} else {
289+
expect(res.status).toBe(route === "v1 batch" || route === "v2 batch" ? 202 : 200);
290+
if (route === "trigger") {
291+
expect(await res.json()).toMatchObject({ id: run.friendlyId, isCached: true });
292+
} else if (route === "v3 items") {
293+
expect(await res.json()).toMatchObject({ itemsAccepted: 1, sealed: true });
294+
} else if (route === "task-scoped v1 batch") {
295+
expect(await res.json()).toMatchObject({
296+
batchId: expect.stringMatching(/^batch_/),
297+
runs: [run.friendlyId],
298+
});
299+
} else {
300+
expect(await res.json()).toMatchObject({ id: expect.stringMatching(/^batch_/) });
301+
}
302+
}
303+
});
184304
});
185305
});
186306

‎apps/webapp/test/streamBatchItemsAuthorization.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,46 @@ describe("streaming batch item authorization", () => {
6060
).rejects.toThrow();
6161
});
6262

63+
it.each([
64+
{ name: "public JWT", scopes: ["write:batch:batch_123"], allowStoredPayloads: false },
65+
{
66+
name: "restricted API key",
67+
scopes: ["batchTrigger:tasks:task-a"],
68+
allowStoredPayloads: false,
69+
},
70+
{ name: "unrestricted API key", scopes: ["admin"], allowStoredPayloads: true },
71+
])(
72+
"enforces stored-payload permission for $name on every item",
73+
async ({ scopes, allowStoredPayloads }) => {
74+
const ability = withActionAliases(buildJwtAbility(scopes));
75+
const inline = { task: "task-a", payload: {} };
76+
const stored = {
77+
task: "task-a",
78+
payload: "run_other/payload.json",
79+
options: { payloadType: "application/store" },
80+
};
81+
async function* items() {
82+
yield inline;
83+
yield stored;
84+
}
85+
86+
const stream = await authorizedBatchItemStream(
87+
items(),
88+
ability,
89+
"batch_123",
90+
allowStoredPayloads
91+
);
92+
const iterator = stream[Symbol.asyncIterator]();
93+
await expect(iterator.next()).resolves.toMatchObject({ value: inline, done: false });
94+
if (allowStoredPayloads) {
95+
await expect(iterator.next()).resolves.toMatchObject({ value: stored, done: false });
96+
await expect(iterator.next()).resolves.toMatchObject({ done: true });
97+
} else {
98+
await expect(iterator.next()).rejects.toThrow();
99+
}
100+
}
101+
);
102+
63103
it("does not delegate batch-wide writes from selected-task credentials", () => {
64104
const ability = withActionAliases(buildJwtAbility(["batchTrigger:tasks:task-a"]));
65105

0 commit comments

Comments
 (0)