Skip to content

Commit 7ffd452

Browse files
ericallamTrigger.dev RepoOps
authored andcommitted
fix(webapp): tighten API key handling
Mono-RevId: 2f0eb0ac720ccb1283629d887348feeeeedfe3db
1 parent e33ace2 commit 7ffd452

4 files changed

Lines changed: 167 additions & 3 deletions

File tree

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { json } from "@remix-run/server-runtime";
33
import { prisma } from "~/db.server";
44
import { authenticateApiRequest } from "~/services/apiAuth.server";
55
import { logger } from "~/services/logger.server";
6+
import { isApiKeyInGraceWindow } from "~/services/apiKeyGraceWindow.server";
67

78
export async function loader({ request }: LoaderFunctionArgs) {
89
try {
@@ -13,6 +14,12 @@ export async function loader({ request }: LoaderFunctionArgs) {
1314
return json({ error: "Invalid or Missing API key" }, { status: 401 });
1415
}
1516

17+
// A rotated key still authenticates during its grace window; it must not
18+
// be usable to look up the environment it was rotated out of.
19+
if (await isApiKeyInGraceWindow(authenticationResult.apiKey)) {
20+
return json({ error: "Invalid or Missing API key" }, { status: 401 });
21+
}
22+
1623
const environmentWithUser = await prisma.runtimeEnvironment.findUnique({
1724
select: {
1825
orgMember: {
@@ -26,8 +33,29 @@ export async function loader({ request }: LoaderFunctionArgs) {
2633
},
2734
});
2835

36+
// Identity only: never return API key material from this endpoint.
37+
const environment = authenticationResult.environment;
2938
const result = {
30-
...authenticationResult.environment,
39+
id: environment.id,
40+
slug: environment.slug,
41+
type: environment.type,
42+
shortcode: environment.shortcode,
43+
branchName: environment.branchName,
44+
paused: environment.paused,
45+
archivedAt: environment.archivedAt,
46+
createdAt: environment.createdAt,
47+
updatedAt: environment.updatedAt,
48+
project: {
49+
id: environment.project.id,
50+
slug: environment.project.slug,
51+
name: environment.project.name,
52+
externalRef: environment.project.externalRef,
53+
},
54+
organization: {
55+
id: environment.organization.id,
56+
slug: environment.organization.slug,
57+
title: environment.organization.title,
58+
},
3159
userId: environmentWithUser?.orgMember?.userId,
3260
};
3361

‎apps/webapp/app/services/apiAuth.server.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import {
3636
isOrganizationAccessToken,
3737
} from "./organizationAccessToken.server";
3838
import { isPublicJWT, validatePublicJwtKey } from "./realtime/jwtAuth.server";
39+
import { isApiKeyInGraceWindow } from "./apiKeyGraceWindow.server";
40+
import { isAdditionalApiKey } from "@trigger.dev/core/v3/apiKeys";
3941
import { isDefaultDevBranch, sanitizeBranchName } from "@trigger.dev/core/v3/utils/gitBranch";
4042
import {
4143
authenticateAuthorizeBearerWithTelemetry,
@@ -355,10 +357,21 @@ export type ApiKeyScopeAuthorization = {
355357
allowPreviewParent?: boolean;
356358
};
357359

360+
// A rotated root key keeps working during its grace window so callers can
361+
// switch over without downtime. It must not be able to ship code or change the
362+
// environment's variables: deployed runs receive the current root key, so
363+
// either would let the old key recover its replacement. It also can't read
364+
// variables, which may hold secrets updated after the rotation.
365+
function isDeniedDuringGraceWindow(action: string, resource: RbacResource): boolean {
366+
if (resource.type === "envvars") return true;
367+
return resource.type === "deployments" && action !== "read";
368+
}
369+
358370
export async function authenticateApiKeyWithScope(
359371
request: Request,
360372
{ action, resource, allowJWT = false, allowPreviewParent = false }: ApiKeyScopeAuthorization,
361-
authorizeBearer: typeof authenticateAuthorizeBearerWithTelemetry = authenticateAuthorizeBearerWithTelemetry
373+
authorizeBearer: typeof authenticateAuthorizeBearerWithTelemetry = authenticateAuthorizeBearerWithTelemetry,
374+
isInGraceWindow: typeof isApiKeyInGraceWindow = isApiKeyInGraceWindow
362375
): Promise<
363376
| { ok: true; authentication: ApiAuthenticationResultSuccess }
364377
| { ok: false; status: 401 | 403; error: string }
@@ -380,6 +393,19 @@ export async function authenticateApiKeyWithScope(
380393
return { ok: false, status: 401, error: "Run tokens are not accepted for this endpoint" };
381394
}
382395

396+
if (
397+
isDeniedDuringGraceWindow(action, resource) &&
398+
!isAdditionalApiKey(apiKey.trim()) &&
399+
// The bearer resolvers trim the token, so the lookup must match on the same value.
400+
(await isInGraceWindow(apiKey.trim()))
401+
) {
402+
return {
403+
ok: false,
404+
status: 401,
405+
error: "This API key has been rotated. Use the environment's current secret key.",
406+
};
407+
}
408+
383409
return {
384410
ok: true,
385411
authentication: {

‎apps/webapp/test/apiAuthScope.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ describe("authenticateApiKeyWithScope", () => {
6161
const result = await authenticateApiKeyWithScope(
6262
request,
6363
{ action: "read", resource: { type: "envvars" }, allowJWT: true },
64-
authorizeBearer
64+
authorizeBearer,
65+
vi.fn().mockResolvedValue(false)
6566
);
6667

6768
expect(authorizeBearer).toHaveBeenCalledWith(
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// A rotated root key keeps authenticating during its grace window so callers
2+
// can switch over without downtime. These tests lock in that the old key can't
3+
// use that window to recover its replacement: it can't read whoami, ship a
4+
// deployment, or touch the environment's variables, while the current key can.
5+
6+
import { describe, expect, it } from "vitest";
7+
import { getTestServer } from "./helpers/sharedTestServer";
8+
import { seedTestEnvironment } from "./helpers/seedTestEnvironment";
9+
10+
async function seedRotatedEnvironment() {
11+
const server = getTestServer();
12+
const seeded = await seedTestEnvironment(server.prisma);
13+
const currentKey = `tr_dev_rotated_${Math.random().toString(36).slice(2)}`;
14+
15+
// Rotate exactly as regenerate-api-key does: new value on the env, old value
16+
// parked in RevokedApiKey with a future grace expiry.
17+
await server.prisma.$transaction([
18+
server.prisma.revokedApiKey.create({
19+
data: {
20+
apiKey: seeded.apiKey,
21+
runtimeEnvironmentId: seeded.environment.id,
22+
expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000),
23+
},
24+
}),
25+
server.prisma.runtimeEnvironment.update({
26+
where: { id: seeded.environment.id },
27+
data: { apiKey: currentKey },
28+
}),
29+
]);
30+
31+
return { ...seeded, rotatedKey: seeded.apiKey, currentKey };
32+
}
33+
34+
function request(path: string, authorization: string, init: RequestInit = {}) {
35+
return getTestServer().webapp.fetch(path, {
36+
...init,
37+
headers: {
38+
Authorization: authorization,
39+
"Content-Type": "application/json",
40+
...init.headers,
41+
},
42+
});
43+
}
44+
45+
describe("rotated root keys during the grace window", () => {
46+
// Catches: whoami handing a rotated key the environment's replacement root key.
47+
it("whoami rejects the rotated key and never returns key material", async () => {
48+
const { rotatedKey, currentKey } = await seedRotatedEnvironment();
49+
50+
const rotated = await request("/api/v1/whoami", `Bearer ${rotatedKey}`);
51+
expect(rotated.status).toBe(401);
52+
53+
const current = await request("/api/v1/whoami", `Bearer ${currentKey}`);
54+
expect(current.status).toBe(200);
55+
const body = JSON.stringify(await current.json());
56+
expect(body).not.toContain(currentKey);
57+
expect(body).not.toContain("apiKey");
58+
});
59+
60+
// Catches: a rotated key reading env vars, which can hold secrets updated after the rotation.
61+
it("rejects env var reads with the rotated key", async () => {
62+
const { project, rotatedKey, currentKey } = await seedRotatedEnvironment();
63+
const path = `/api/v1/projects/${project.externalRef}/envvars/dev`;
64+
65+
expect((await request(path, `Bearer ${rotatedKey}`)).status).toBe(401);
66+
expect((await request(path, `Bearer ${currentKey}`)).status).toBe(200);
67+
});
68+
69+
// Catches: a rotated key setting a variable such as NODE_OPTIONS that runs code with the new key.
70+
it("rejects env var writes with the rotated key", async () => {
71+
const { project, rotatedKey, currentKey } = await seedRotatedEnvironment();
72+
const path = `/api/v1/projects/${project.externalRef}/envvars/dev`;
73+
const init = { method: "POST", body: JSON.stringify({ name: "E2E_VAR", value: "x" }) };
74+
75+
expect((await request(path, `Bearer ${rotatedKey}`, init)).status).toBe(401);
76+
expect((await request(path, `Bearer ${currentKey}`, init)).status).toBe(200);
77+
});
78+
79+
// Catches: a rotated key driving a deployment whose runs receive the new key.
80+
it("rejects deployment writes with the rotated key", async () => {
81+
const { rotatedKey, currentKey } = await seedRotatedEnvironment();
82+
const path = "/api/v1/deployments/deployment_missing/progress";
83+
const init = { method: "POST", body: JSON.stringify({}) };
84+
85+
expect((await request(path, `Bearer ${rotatedKey}`, init)).status).toBe(401);
86+
// Past auth, the current key reaches the handler, which finds no deployment.
87+
expect((await request(path, `Bearer ${currentKey}`, init)).status).toBe(404);
88+
});
89+
90+
// Catches: extra whitespace after "Bearer" authenticating the rotated key while the grace-window lookup misses it.
91+
it("rejects the rotated key with extra whitespace in the header", async () => {
92+
const { project, rotatedKey } = await seedRotatedEnvironment();
93+
94+
const res = await request(
95+
`/api/v1/projects/${project.externalRef}/envvars/dev`,
96+
`Bearer ${rotatedKey}`
97+
);
98+
expect(res.status).toBe(401);
99+
});
100+
101+
// Catches: the restriction spilling over and breaking the zero-downtime switchover for reads.
102+
it("still lets the rotated key read deployments", async () => {
103+
const { rotatedKey } = await seedRotatedEnvironment();
104+
105+
// Past auth, the handler finds no deployment for the fresh environment.
106+
const res = await request("/api/v1/deployments/latest", `Bearer ${rotatedKey}`);
107+
expect(res.status).toBe(404);
108+
});
109+
});

0 commit comments

Comments
 (0)