Skip to content

Commit 22f8fb2

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
fix(webapp,sdk): enforce scoped token resource boundaries
Mono-RevId: 941c01f76cc443b5b6c74f3444029abf052163c1
1 parent eaa9050 commit 22f8fb2

10 files changed

Lines changed: 322 additions & 45 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@trigger.dev/sdk": patch
3+
---
4+
5+
Validate resource IDs when creating scoped public tokens. Explicitly empty IDs are now rejected instead of being interpreted as type-wide permissions.

‎apps/webapp/app/routes/api.v1.projects.$projectRef.$env.regenerate-api-key.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { regenerateApiKey, RootApiKeyNotVisibleError } from "~/models/api-key.se
55
import {
66
authenticatedEnvironmentForAuthentication,
77
authenticateRequest,
8-
branchNameFromRequest,
98
} from "~/services/apiAuth.server";
109
import { authorizePatEnvironmentAccess } from "~/services/environmentVariableApiAccess.server";
1110
import { logger } from "~/services/logger.server";
@@ -39,11 +38,15 @@ export async function action({ request, params }: ActionFunctionArgs) {
3938
return json({ error: "Invalid or Missing Access Token" }, { status: 401 });
4039
}
4140

41+
if (request.headers.has("x-trigger-branch")) {
42+
return json({ error: "Branch-scoped key rotation is not supported" }, { status: 400 });
43+
}
44+
4245
const environment = await authenticatedEnvironmentForAuthentication(
4346
authenticationResult,
4447
projectRef,
4548
env,
46-
branchNameFromRequest(request)
49+
undefined
4750
);
4851

4952
// Rotating the key requires env-tier write:apiKeys — same gate the

‎apps/webapp/app/routes/api.v1.runs.$runId.spans.$spanId.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { getTaskEventStoreTableForRun } from "~/v3/taskEventStore.server";
99
import { findRunByIdWithMollifierFallback } from "~/v3/mollifier/readFallback.server";
1010
import { buildSyntheticSpanDetailBody } from "~/v3/mollifier/syntheticApiResponses.server";
1111
import { runStore } from "~/v3/runStore.server";
12+
import { canReadRunWithAliases } from "~/utils/runReadAuthorization";
1213

1314
const ParamsSchema = z.object({
1415
runId: z.string(),
@@ -79,7 +80,7 @@ export const loader = createLoaderApiRoute(
7980
},
8081
},
8182
},
82-
async ({ params, resource: resolved, authentication }) => {
83+
async ({ params, resource: resolved, authentication, ability }) => {
8384
if (resolved.source === "buffer") {
8485
// Buffered runs have exactly one valid spanId — the queued span the
8586
// mollifier gate recorded at trigger time, which becomes the run's
@@ -126,6 +127,8 @@ export const loader = createLoaderApiRoute(
126127
select: {
127128
friendlyId: true,
128129
taskIdentifier: true,
130+
runTags: true,
131+
batchId: true,
129132
status: true,
130133
createdAt: true,
131134
},
@@ -137,6 +140,10 @@ export const loader = createLoaderApiRoute(
137140
$replica
138141
);
139142

143+
const authorizedTriggeredRuns = triggeredRuns.filter((run) =>
144+
canReadRunWithAliases(ability, run)
145+
);
146+
140147
const properties =
141148
span.properties &&
142149
typeof span.properties === "object" &&
@@ -182,8 +189,8 @@ export const loader = createLoaderApiRoute(
182189
}
183190
: undefined,
184191
triggeredRuns:
185-
triggeredRuns.length > 0
186-
? triggeredRuns.map((r) => ({
192+
authorizedTriggeredRuns.length > 0
193+
? authorizedTriggeredRuns.map((r) => ({
187194
runId: r.friendlyId,
188195
taskIdentifier: r.taskIdentifier,
189196
status: r.status,
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { buildJwtAbility } from "@trigger.dev/rbac";
2+
import { describe, expect, it } from "vitest";
3+
import { canReadRunWithAliases } from "./runReadAuthorization";
4+
5+
const run = {
6+
friendlyId: "run_child",
7+
taskIdentifier: "child-task",
8+
runTags: ["user:alice", "paid"],
9+
batchId: "abcdefghijklmnopqrstuvwx",
10+
};
11+
12+
describe("canReadRunWithAliases", () => {
13+
it.each([
14+
"read:runs:run_child",
15+
"read:tasks:child-task",
16+
"read:tags:user:alice",
17+
"read:batch:batch_abcdefghijklmnopqrstuvwx",
18+
"read:runs",
19+
])("allows child metadata through the %s alias", (scope) => {
20+
expect(canReadRunWithAliases(buildJwtAbility([scope]), run)).toBe(true);
21+
});
22+
23+
it.each([
24+
"read:runs:run_parent",
25+
"read:tasks:parent-task",
26+
"read:tags:user:bob",
27+
"read:batch:batch_other",
28+
])("denies child metadata when only %s is authorized", (scope) => {
29+
expect(canReadRunWithAliases(buildJwtAbility([scope]), run)).toBe(false);
30+
});
31+
32+
it("checks all 13 bounded aliases in one ability call", () => {
33+
const runWithMaxTags = {
34+
...run,
35+
runTags: Array.from({ length: 10 }, (_, index) => `tag:${index}`),
36+
};
37+
let checks = 0;
38+
39+
expect(
40+
canReadRunWithAliases(
41+
{
42+
can(action, resources) {
43+
checks++;
44+
expect(action).toBe("read");
45+
expect(resources).toHaveLength(13);
46+
return false;
47+
},
48+
canSuper: () => false,
49+
},
50+
runWithMaxTags
51+
)
52+
).toBe(false);
53+
expect(checks).toBe(1);
54+
});
55+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type { RbacAbility, RbacResource } from "@trigger.dev/rbac";
2+
import { BatchId } from "@trigger.dev/core/v3/isomorphic";
3+
4+
type RunReadAliases = {
5+
friendlyId: string;
6+
taskIdentifier: string;
7+
runTags: string[];
8+
batchId: string | null;
9+
};
10+
11+
export function canReadRunWithAliases(ability: RbacAbility, run: RunReadAliases): boolean {
12+
const resources: RbacResource[] = [
13+
{ type: "runs", id: run.friendlyId },
14+
{ type: "tasks", id: run.taskIdentifier },
15+
...run.runTags.map((tag) => ({ type: "tags", id: tag })),
16+
];
17+
18+
if (run.batchId) {
19+
resources.push({ type: "batch", id: BatchId.toFriendlyId(run.batchId) });
20+
}
21+
22+
return ability.can("read", resources);
23+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { describe, expect, it } from "vitest";
2+
import { getTestServer } from "./helpers/sharedTestServer";
3+
import { seedTestUserProject } from "./helpers/seedTestUserProject";
4+
5+
describe("regenerate API key branch handling", () => {
6+
it.each(["feature", "", "default"])(
7+
"rejects a present branch header without rotating the root key (%s)",
8+
async (branch) => {
9+
const server = getTestServer();
10+
const { environment, pat, project } = await seedTestUserProject(server.prisma);
11+
12+
const response = await server.webapp.fetch(
13+
`/api/v1/projects/${project.externalRef}/dev/regenerate-api-key`,
14+
{
15+
method: "POST",
16+
headers: {
17+
Authorization: `Bearer ${pat.token}`,
18+
"x-trigger-branch": branch,
19+
},
20+
}
21+
);
22+
23+
expect(response.status).toBe(400);
24+
expect(await response.json()).toEqual({
25+
error: "Branch-scoped key rotation is not supported",
26+
});
27+
const persisted = await server.prisma.runtimeEnvironment.findUniqueOrThrow({
28+
where: { id: environment.id },
29+
select: { apiKey: true },
30+
});
31+
expect(persisted.apiKey).toBe(environment.apiKey);
32+
}
33+
);
34+
35+
it("preserves root key rotation and grace behavior without a branch header", async () => {
36+
const server = getTestServer();
37+
const { environment, pat, project } = await seedTestUserProject(server.prisma);
38+
39+
const response = await server.webapp.fetch(
40+
`/api/v1/projects/${project.externalRef}/dev/regenerate-api-key`,
41+
{
42+
method: "POST",
43+
headers: { Authorization: `Bearer ${pat.token}` },
44+
}
45+
);
46+
47+
expect(response.status).toBe(200);
48+
const body = (await response.json()) as { apiKey: string };
49+
expect(body.apiKey).not.toBe(environment.apiKey);
50+
51+
const revoked = await server.prisma.revokedApiKey.findFirst({
52+
where: { apiKey: environment.apiKey, runtimeEnvironmentId: environment.id },
53+
});
54+
expect(revoked?.expiresAt.getTime()).toBeGreaterThan(Date.now());
55+
});
56+
});

‎internal-packages/rbac/src/ability.test.ts‎

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { buildScope } from "@trigger.dev/plugins";
12
import { describe, it, expect } from "vitest";
23
import {
34
permissiveAbility,
@@ -60,17 +61,42 @@ describe("buildJwtAbility", () => {
6061
expect(ability.can("read", { type: "runs" })).toBe(false);
6162
});
6263

64+
it.each(["read:runs:", "read:runs: "])(
65+
"rejects an explicitly empty resource ID in %s",
66+
(scope) => {
67+
const ability = buildJwtAbility([scope]);
68+
expect(ability.can("read", { type: "runs", id: "run_abc" })).toBe(false);
69+
expect(ability.can("read", { type: "runs" })).toBe(false);
70+
}
71+
);
72+
6373
it("preserves colons in the resource id (everything after the 2nd colon)", () => {
64-
// Resource ids can contain colons (e.g. user-provided tags like
65-
// `env:staging`). The naive `[a, b, c] = scope.split(":")` form
66-
// truncated `read:tags:env:staging` → scopeId="env" and silently
67-
// mis-matched. Regression coverage for the multi-colon id path.
6874
const ability = buildJwtAbility(["read:tags:env:staging"]);
6975
expect(ability.can("read", { type: "tags", id: "env:staging" })).toBe(true);
7076
expect(ability.can("read", { type: "tags", id: "env" })).toBe(false);
7177
expect(ability.can("read", { type: "tags", id: "env:prod" })).toBe(false);
7278
});
7379

80+
it("preserves meaningful whitespace around resource ids", () => {
81+
const ability = buildJwtAbility(["read:tags: env:staging "]);
82+
expect(ability.can("read", { type: "tags", id: " env:staging " })).toBe(true);
83+
expect(ability.can("read", { type: "tags", id: "env:staging" })).toBe(false);
84+
});
85+
86+
it("preserves wildcard actions and ignores resource condition fields", () => {
87+
const ability = buildJwtAbility(["*:runs:run_abc"]);
88+
const resource = {
89+
type: "runs",
90+
id: "run_abc",
91+
projectId: "proj_123",
92+
environmentId: "env_123",
93+
};
94+
95+
expect(ability.can("read", resource)).toBe(true);
96+
expect(ability.can("write", resource)).toBe(true);
97+
expect(ability.can("read", { ...resource, id: "run_other" })).toBe(false);
98+
});
99+
74100
it("allows any read with read:all scope", () => {
75101
const ability = buildJwtAbility(["read:all"]);
76102
expect(ability.can("read", { type: "runs" })).toBe(true);
@@ -122,6 +148,32 @@ describe("buildJwtAbility", () => {
122148
const ability = buildJwtAbility(["read:runs"]);
123149
expect(ability.can("write", { type: "runs" })).toBe(false);
124150
});
151+
152+
it("does not revisit the supplied scope set during repeated checks", () => {
153+
const rawScopes = Array.from({ length: 300 }, (_, index) => `read:runs:allowed_${index}`);
154+
let scopeReads = 0;
155+
const scopes = new Proxy(rawScopes, {
156+
get(target, property, receiver) {
157+
if (typeof property === "string" && /^\d+$/.test(property)) {
158+
scopeReads++;
159+
}
160+
return Reflect.get(target, property, receiver);
161+
},
162+
});
163+
const ability = buildJwtAbility(scopes);
164+
const readsAfterConstruction = scopeReads;
165+
const childAliases = Array.from({ length: 13 }, (_, index) => ({
166+
type: "runs",
167+
id: `denied_${index}`,
168+
}));
169+
170+
for (let child = 0; child < 50; child++) {
171+
expect(ability.can("read", childAliases)).toBe(false);
172+
}
173+
174+
expect(readsAfterConstruction).toBeGreaterThan(0);
175+
expect(scopeReads).toBe(readsAfterConstruction);
176+
});
125177
});
126178

127179
describe("scopesWithinAbility", () => {
@@ -189,13 +241,27 @@ describe("scopesWithinAbility", () => {
189241
});
190242

191243
it("rejects malformed scopes for restricted abilities", () => {
192-
expect(scopesWithinAbility(["read"], buildJwtAbility(["read:all"]))).toEqual({
244+
expect(
245+
scopesWithinAbility(["read", "read:runs:", "read:tags: "], buildJwtAbility(["read:all"]))
246+
).toEqual({
193247
ok: false,
194-
deniedScopes: ["read"],
248+
deniedScopes: ["read", "read:runs:", "read:tags: "],
195249
});
196250
});
197251
});
198252

253+
describe("buildScope", () => {
254+
it("distinguishes an absent ID from an explicitly empty ID", () => {
255+
expect(buildScope("read", "runs")).toBe("read:runs");
256+
expect(() => buildScope("read", "runs", "")).toThrow("Scope resource IDs must not be empty");
257+
expect(() => buildScope("read", "runs", " ")).toThrow("Scope resource IDs must not be empty");
258+
});
259+
260+
it("preserves nonempty IDs exactly", () => {
261+
expect(buildScope("read", "tags", " env:staging ")).toBe("read:tags: env:staging ");
262+
});
263+
});
264+
199265
describe("buildJwtAbility — array resources", () => {
200266
it("authorizes when any resource in the array passes a scope check", () => {
201267
const ability = buildJwtAbility(["read:batch:batch_abc"]);

0 commit comments

Comments
 (0)