Skip to content

Commit 5126bae

Browse files
committed
fix(webapp): gate a run's commit metadata on reading deployments
The route served a deployment's git blob — commit message, author, branch, PR title — with no ability check, while the deployments list serves the same blob behind read on deployments. Apply that check here too.
1 parent 983055a commit 5126bae

4 files changed

Lines changed: 176 additions & 1 deletion

File tree

‎apps/webapp/app/routes/api.v1.projects.$projectRef.$env.runs.$runId.commit.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
type AuthenticatedEnvironment,
77
} from "~/services/apiAuth.server";
88
import { resolveRunCommit } from "~/services/dashboardAgent.server";
9+
import { authorizePatEnvironmentAccess } from "~/services/environmentVariableApiAccess.server";
910
import { logger } from "~/services/logger.server";
1011
import { authenticateUatOrApiRequest } from "~/services/uatRoutePreamble.server";
1112

@@ -51,6 +52,18 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
5152
triggerBranch
5253
);
5354

55+
// The answer is a deployment's git metadata, so it's gated like the deployments list.
56+
const denied = await authorizePatEnvironmentAccess({
57+
request,
58+
authType: authentication.authenticationResult.type,
59+
organizationId: runtimeEnv.organizationId,
60+
projectId: runtimeEnv.project.id,
61+
envType: runtimeEnv.type,
62+
resource: "deployments",
63+
action: "read",
64+
});
65+
if (denied) return denied;
66+
5467
const commit = await resolveRunCommit(runtimeEnv.id, runId);
5568
if (!commit) {
5669
return json(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
} from "~/services/apiAuth.server";
1010
import { rbac } from "~/services/rbac.server";
1111

12-
type EnvironmentScopedResource = "envvars" | "apiKeys";
12+
type EnvironmentScopedResource = "envvars" | "apiKeys" | "deployments";
1313

1414
type EnvironmentScopedAuthentication =
1515
| { ok: true; authentication: AuthenticationResult }
@@ -78,6 +78,7 @@ export function authenticateEnvVarApiRequest(
7878
const RESOURCE_LABELS: Record<EnvironmentScopedResource, string> = {
7979
envvars: "environment variables",
8080
apiKeys: "API keys",
81+
deployments: "deployments",
8182
};
8283

8384
/**

‎apps/webapp/test/dashboardAgentRoutes.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ vi.mock("~/services/rbac.server", () => ({
5959
ability: { can: () => true, canSuper: () => true },
6060
jwt: undefined,
6161
}),
62+
authenticateUserActor: async () => ({
63+
ok: true,
64+
userId: "usr_1",
65+
ability: { can: () => true, canSuper: () => true },
66+
}),
67+
authenticatePat: async () => ({
68+
ok: true,
69+
userId: "usr_1",
70+
ability: { can: () => true, canSuper: () => true },
71+
}),
6272
},
6373
}));
6474

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
/**
4+
* The run-commit route answers with a deployment's git metadata — commit message, author, branch,
5+
* PR title. The deployments list serves the same blob behind `read` on `deployments`, so a
6+
* credential without that scope must not get it from here either.
7+
*/
8+
9+
const { SESSION_SECRET } = vi.hoisted(() => ({
10+
SESSION_SECRET: "test-session-secret-for-run-commit-authorization",
11+
}));
12+
13+
const mocks = vi.hoisted(() => ({
14+
can: vi.fn<(...args: any[]) => boolean>(),
15+
resolveRunCommit: vi.fn<(...args: any[]) => Promise<any>>(),
16+
}));
17+
18+
vi.mock("@internal/tracing", () => ({
19+
getMeter: () => ({
20+
createCounter: () => ({ add: vi.fn() }),
21+
createHistogram: () => ({ record: vi.fn() }),
22+
createObservableGauge: () => ({ addCallback: vi.fn() }),
23+
}),
24+
}));
25+
vi.mock("~/env.server", () => ({
26+
env: { SESSION_SECRET, APP_ORIGIN: "https://example.com" },
27+
}));
28+
vi.mock("~/services/logger.server", () => ({
29+
logger: { debug: vi.fn(), error: vi.fn(), warn: vi.fn(), info: vi.fn() },
30+
}));
31+
vi.mock("~/services/rbac.server", () => ({
32+
rbac: {
33+
authenticateBearer: vi.fn(),
34+
authenticateUserActor: async () => ({ ok: true, ability: { can: mocks.can } }),
35+
authenticatePat: async () => ({ ok: true, ability: { can: mocks.can } }),
36+
},
37+
}));
38+
vi.mock("~/services/personalAccessToken.server", () => ({
39+
authenticateApiRequestWithPersonalAccessToken: vi.fn(),
40+
isPersonalAccessToken: () => false,
41+
}));
42+
vi.mock("~/services/organizationAccessToken.server", () => ({
43+
authenticateApiRequestWithOrganizationAccessToken: vi.fn(),
44+
isOrganizationAccessToken: () => false,
45+
}));
46+
vi.mock("~/services/realtime/jwtAuth.server", () => ({
47+
isPublicJWT: () => false,
48+
validatePublicJwtKey: vi.fn(),
49+
}));
50+
vi.mock("~/models/project.server", () => ({
51+
findProjectByRef: async (externalRef: string, userId: string) =>
52+
externalRef === PROJECT.externalRef && userId === USER_ID ? PROJECT : null,
53+
}));
54+
vi.mock("~/models/runtimeEnvironment.server", () => ({
55+
authIncludeBase: {},
56+
authIncludeWithParent: {},
57+
findEnvironmentByApiKey: vi.fn(),
58+
findEnvironmentByApiKeyWithResolution: vi.fn(),
59+
findEnvironmentByPublicApiKey: vi.fn(),
60+
toAuthenticated: (environment: any) => environment,
61+
}));
62+
vi.mock("~/db.server", () => ({
63+
prisma: {},
64+
$replica: {
65+
user: {
66+
findUnique: async ({ where }: any) => (where.id === USER_ID ? { id: where.id } : null),
67+
},
68+
runtimeEnvironment: {
69+
findFirst: async ({ where }: any) => (where.slug === ENVIRONMENT.slug ? ENVIRONMENT : null),
70+
},
71+
workerDeployment: {
72+
findFirst: async () => ({
73+
git: { commitMessage: "fix billing for Acme Corp", commitAuthorName: "Alice" },
74+
shortCode: "abcd",
75+
deployedAt: new Date(),
76+
}),
77+
},
78+
},
79+
}));
80+
vi.mock("~/services/dashboardAgent.server", () => ({
81+
resolveRunCommit: mocks.resolveRunCommit,
82+
}));
83+
84+
import { signUserActorToken } from "@trigger.dev/rbac";
85+
import { loader as commitLoader } from "~/routes/api.v1.projects.$projectRef.$env.runs.$runId.commit";
86+
87+
const ORGANIZATION = { id: "org_1234", slug: "test-org" };
88+
const PROJECT = { id: "proj_1234", externalRef: "proj_ref_1234", slug: "test-project" };
89+
const USER_ID = "usr_member";
90+
const ENVIRONMENT = {
91+
id: "env_prod",
92+
slug: "prod",
93+
type: "PRODUCTION" as const,
94+
apiKey: "tr_prod_abcdefghijklmnop",
95+
organizationId: ORGANIZATION.id,
96+
organization: ORGANIZATION,
97+
projectId: PROJECT.id,
98+
project: PROJECT,
99+
};
100+
101+
async function getCommit(): Promise<{ status: number; body: any }> {
102+
const token = await signUserActorToken(SESSION_SECRET, {
103+
userId: USER_ID,
104+
client: "dashboard-agent",
105+
environmentId: ENVIRONMENT.id,
106+
cap: ["read:runs"],
107+
});
108+
109+
const response = await commitLoader({
110+
request: new Request(
111+
`https://example.com/api/v1/projects/${PROJECT.externalRef}/prod/runs/run_1/commit`,
112+
{ headers: { Authorization: `Bearer ${token}` } }
113+
),
114+
params: { projectRef: PROJECT.externalRef, env: "prod", runId: "run_1" },
115+
context: {} as any,
116+
} as any);
117+
return { status: response.status, body: await response.json() };
118+
}
119+
120+
describe("reading a run's commit metadata", () => {
121+
beforeEach(() => {
122+
mocks.can.mockReset();
123+
mocks.resolveRunCommit.mockReset();
124+
// The commit resolves fine, so a refusal can only come from the gate.
125+
mocks.resolveRunCommit.mockResolvedValue({
126+
version: "20240101.1",
127+
sha: "abc123",
128+
dirty: false,
129+
});
130+
});
131+
132+
it("refuses a caller who may not read deployments", async () => {
133+
mocks.can.mockImplementation((_action, resource) => resource?.type !== "deployments");
134+
135+
const result = await getCommit();
136+
137+
expect(result.status).toBe(403);
138+
expect(JSON.stringify(result.body)).not.toContain("Acme Corp");
139+
expect(mocks.resolveRunCommit).not.toHaveBeenCalled();
140+
});
141+
142+
it("answers a caller who may", async () => {
143+
mocks.can.mockReturnValue(true);
144+
145+
const result = await getCommit();
146+
147+
expect(result.status).toBe(200);
148+
expect(result.body.sha).toBe("abc123");
149+
expect(result.body.git.commitMessage).toBe("fix billing for Acme Corp");
150+
});
151+
});

0 commit comments

Comments
 (0)