Skip to content

Commit 45740b6

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
fix(webapp): verify GitHub App installation ownership
GitHub App installation callbacks now verify that the authenticated installing user can access the installation before linking it to an account. Mono-RevId: c59ade6435294a39335d1b85cc881a0afd1d1452
1 parent bd84ee1 commit 45740b6

10 files changed

Lines changed: 317 additions & 39 deletions

File tree

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+
GitHub App installations are now linked only after the installing GitHub user authorizes the App and is verified to have access to the installation.

‎apps/webapp/app/env.server.ts‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,32 @@ const GithubAppEnvSchema = z.preprocess(
5151
}
5252
return obj;
5353
},
54-
z.discriminatedUnion("GITHUB_APP_ENABLED", [
55-
z.object({
56-
GITHUB_APP_ENABLED: z.literal("1"),
57-
GITHUB_APP_ID: z.string(),
58-
GITHUB_APP_PRIVATE_KEY: z.string(),
59-
GITHUB_APP_WEBHOOK_SECRET: z.string(),
60-
GITHUB_APP_SLUG: z.string(),
61-
}),
62-
z.object({
63-
GITHUB_APP_ENABLED: z.literal("0"),
64-
}),
65-
])
54+
z
55+
.discriminatedUnion("GITHUB_APP_ENABLED", [
56+
z.object({
57+
GITHUB_APP_ENABLED: z.literal("1"),
58+
GITHUB_APP_ID: z.string(),
59+
GITHUB_APP_PRIVATE_KEY: z.string(),
60+
GITHUB_APP_WEBHOOK_SECRET: z.string(),
61+
GITHUB_APP_SLUG: z.string(),
62+
GITHUB_APP_CLIENT_ID: z.string().min(1).optional(),
63+
GITHUB_APP_CLIENT_SECRET: z.string().min(1).optional(),
64+
}),
65+
z.object({
66+
GITHUB_APP_ENABLED: z.literal("0"),
67+
}),
68+
])
69+
.superRefine((value, ctx) => {
70+
if (
71+
value.GITHUB_APP_ENABLED === "1" &&
72+
Boolean(value.GITHUB_APP_CLIENT_ID) !== Boolean(value.GITHUB_APP_CLIENT_SECRET)
73+
) {
74+
ctx.addIssue({
75+
code: z.ZodIssueCode.custom,
76+
message: "GITHUB_APP_CLIENT_ID and GITHUB_APP_CLIENT_SECRET must be configured together",
77+
});
78+
}
79+
})
6680
);
6781

6882
// eventually we can make all S2 env vars required once the S2 OSS version is out

‎apps/webapp/app/routes/_app.github.callback/route.tsx‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,33 @@ import { sanitizeRedirectPath } from "~/utils";
1515
const QuerySchema = z.discriminatedUnion("setup_action", [
1616
z.object({
1717
setup_action: z.literal("install"),
18-
installation_id: z.coerce.number(),
19-
state: z.string(),
18+
installation_id: z.coerce.number().int().positive().safe(),
19+
state: z.string().min(1),
20+
code: z.string().min(1),
2021
}),
2122
z.object({
2223
setup_action: z.literal("update"),
23-
installation_id: z.coerce.number(),
24-
state: z.string(),
24+
installation_id: z.coerce.number().int().positive().safe(),
25+
state: z.string().min(1),
2526
}),
2627
z.object({
2728
setup_action: z.literal("request"),
28-
state: z.string(),
29+
state: z.string().min(1),
2930
}),
3031
]);
3132

33+
function loggableError(error: unknown) {
34+
if (!(error instanceof Error)) {
35+
return { name: "Unknown error" };
36+
}
37+
38+
return {
39+
name: error.name,
40+
message: error.message,
41+
status: "status" in error ? error.status : undefined,
42+
};
43+
}
44+
3245
export async function loader({ request }: LoaderFunctionArgs) {
3346
const url = new URL(request.url);
3447
const queryParams = Object.fromEntries(url.searchParams);
@@ -38,7 +51,8 @@ export async function loader({ request }: LoaderFunctionArgs) {
3851

3952
if (!result.success) {
4053
logger.warn("GitHub App callback with invalid params", {
41-
queryParams,
54+
setupAction: url.searchParams.get("setup_action"),
55+
parameterNames: [...url.searchParams.keys()],
4256
});
4357
return redirectWithErrorMessage("/", request, "Failed to install GitHub app");
4458
}
@@ -49,7 +63,9 @@ export async function loader({ request }: LoaderFunctionArgs) {
4963

5064
if (!sessionResult.valid) {
5165
logger.error("GitHub App callback with invalid session", {
52-
callbackData,
66+
setupAction: callbackData.setup_action,
67+
installationId:
68+
callbackData.setup_action === "request" ? undefined : callbackData.installation_id,
5369
error: sessionResult.error,
5470
});
5571

@@ -90,12 +106,19 @@ export async function loader({ request }: LoaderFunctionArgs) {
90106
switch (callbackData.setup_action) {
91107
case "install": {
92108
const [error] = await tryCatch(
93-
linkGitHubAppInstallation(callbackData.installation_id, organizationId)
109+
linkGitHubAppInstallation({
110+
installationId: callbackData.installation_id,
111+
organizationId,
112+
installedByUserId: user.id,
113+
oauthCode: callbackData.code,
114+
})
94115
);
95116

96117
if (error) {
97118
logger.error("Failed to link GitHub App installation", {
98-
error,
119+
installationId: callbackData.installation_id,
120+
organizationId,
121+
error: loggableError(error),
99122
});
100123
return consumingSession(
101124
await redirectWithErrorMessage(redirectTo, request, "Failed to install GitHub app")
@@ -114,7 +137,9 @@ export async function loader({ request }: LoaderFunctionArgs) {
114137

115138
if (error) {
116139
logger.error("Failed to update GitHub App installation", {
117-
error,
140+
installationId: callbackData.installation_id,
141+
organizationId,
142+
error: loggableError(error),
118143
});
119144
return consumingSession(
120145
await redirectWithErrorMessage(redirectTo, request, "Failed to update GitHub App")
@@ -129,9 +154,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
129154
case "request": {
130155
// This happens when a non-admin user requests installation
131156
// The installation_id won't be available until an admin approves
132-
logger.info("GitHub App installation requested, awaiting approval", {
133-
callbackData,
134-
});
157+
logger.info("GitHub App installation requested, awaiting approval");
135158

136159
return redirectWithSuccessMessage(redirectTo, request, "GitHub App installation requested");
137160
}

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

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
1-
import { App, type Octokit } from "octokit";
1+
import { App, Octokit } from "octokit";
22
import { env } from "../env.server";
33
import { prisma } from "~/db.server";
44
import { logger } from "./logger.server";
55
import { errAsync, fromPromise, okAsync, type ResultAsync } from "neverthrow";
6+
import { tryCatch } from "@trigger.dev/core/utils";
7+
import {
8+
getAuthenticatedGitHubLogin,
9+
verifyGitHubAppInstallationAccess,
10+
} from "./gitHubInstallationOwnership.server";
11+
12+
function isGitHubAppUserOAuthConfigured(): boolean {
13+
return (
14+
env.GITHUB_APP_ENABLED === "1" &&
15+
Boolean(env.GITHUB_APP_CLIENT_ID && env.GITHUB_APP_CLIENT_SECRET)
16+
);
17+
}
618

719
export const githubApp =
820
env.GITHUB_APP_ENABLED === "1"
@@ -12,33 +24,82 @@ export const githubApp =
1224
webhooks: {
1325
secret: env.GITHUB_APP_WEBHOOK_SECRET,
1426
},
27+
...(env.GITHUB_APP_CLIENT_ID && env.GITHUB_APP_CLIENT_SECRET
28+
? {
29+
oauth: {
30+
clientId: env.GITHUB_APP_CLIENT_ID,
31+
clientSecret: env.GITHUB_APP_CLIENT_SECRET,
32+
},
33+
}
34+
: {}),
1535
})
1636
: null;
1737

38+
async function withGitHubUserToken<T>(
39+
oauthCode: string,
40+
fn: (userOctokit: Octokit) => Promise<T>
41+
): Promise<T> {
42+
if (!githubApp) {
43+
throw new Error("GitHub App is not enabled");
44+
}
45+
46+
const { authentication } = await githubApp.oauth.createToken({ code: oauthCode });
47+
48+
const [error, result] = await tryCatch(fn(new Octokit({ auth: authentication.token })));
49+
50+
const [revokeError] = await tryCatch(
51+
githubApp.oauth.deleteToken({ token: authentication.token })
52+
);
53+
if (revokeError) {
54+
logger.warn("Failed to revoke GitHub user token", {
55+
error: revokeError instanceof Error ? revokeError.message : "Unknown error",
56+
});
57+
}
58+
59+
if (error) {
60+
throw error;
61+
}
62+
63+
return result;
64+
}
65+
1866
/**
1967
* Links a GitHub App installation to a Trigger organization
2068
*/
21-
export async function linkGitHubAppInstallation(
22-
installationId: number,
23-
organizationId: string
24-
): Promise<void> {
69+
export async function linkGitHubAppInstallation(params: {
70+
installationId: number;
71+
organizationId: string;
72+
installedByUserId: string;
73+
oauthCode: string;
74+
}): Promise<void> {
2575
if (!githubApp) {
2676
throw new Error("GitHub App is not enabled");
2777
}
2878

29-
const octokit = await githubApp.getInstallationOctokit(installationId);
79+
if (!isGitHubAppUserOAuthConfigured()) {
80+
throw new Error("GitHub App user authorization is not configured");
81+
}
82+
83+
const installedBy = await withGitHubUserToken(params.oauthCode, async (userOctokit) => {
84+
await verifyGitHubAppInstallationAccess(userOctokit, params.installationId);
85+
return getAuthenticatedGitHubLogin(userOctokit);
86+
});
87+
88+
const octokit = await githubApp.getInstallationOctokit(params.installationId);
3089
const { data: installation } = await octokit.rest.apps.getInstallation({
31-
installation_id: installationId,
90+
installation_id: params.installationId,
3291
});
3392

34-
const repositories = await fetchInstallationRepositories(octokit, installationId);
93+
const repositories = await fetchInstallationRepositories(octokit, params.installationId);
3594

3695
const repositorySelection = installation.repository_selection === "all" ? "ALL" : "SELECTED";
3796

3897
await prisma.githubAppInstallation.create({
3998
data: {
40-
appInstallationId: installationId,
41-
organizationId,
99+
appInstallationId: params.installationId,
100+
organizationId: params.organizationId,
101+
installedBy,
102+
installedByUserId: params.installedByUserId,
42103
targetId: installation.target_id,
43104
targetType: installation.target_type,
44105
accountHandle: installation.account
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import type { Octokit } from "octokit";
2+
3+
const installationsPerPage = 100;
4+
5+
type GitHubUserClient = Pick<Octokit, "request">;
6+
7+
export async function verifyGitHubAppInstallationAccess(
8+
userOctokit: GitHubUserClient,
9+
installationId: number
10+
): Promise<void> {
11+
let page = 1;
12+
13+
while (true) {
14+
const { data } = await userOctokit.request("GET /user/installations", {
15+
page,
16+
per_page: installationsPerPage,
17+
});
18+
19+
if (data.installations.some((installation) => installation.id === installationId)) {
20+
return;
21+
}
22+
23+
if (
24+
data.installations.length < installationsPerPage ||
25+
page * installationsPerPage >= data.total_count
26+
) {
27+
break;
28+
}
29+
30+
page++;
31+
}
32+
33+
throw new Error("GitHub App installation is not accessible to the authenticated GitHub user");
34+
}
35+
36+
export async function getAuthenticatedGitHubLogin(userOctokit: GitHubUserClient): Promise<string> {
37+
const { data } = await userOctokit.request("GET /user");
38+
39+
if (!data.login) {
40+
throw new Error("GitHub did not return a login for the authenticated user");
41+
}
42+
43+
return data.login;
44+
}

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,7 @@ export async function validateGitHubAppInstallSession(
8282
}
8383

8484
if (sessionState !== state) {
85-
logger.warn("GitHub App installation state mismatch", {
86-
expectedState: sessionState,
87-
receivedState: state,
88-
});
85+
logger.warn("GitHub App installation state mismatch");
8986
return {
9087
valid: false,
9188
error: "state_mismatch",

‎apps/webapp/test/env.server.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,38 @@ describe("webapp environment secrets", () => {
8181

8282
await expect(import("../app/env.server")).rejects.toThrow("SESSION_SECRET");
8383
});
84+
85+
it("requires GitHub App OAuth credentials to be configured together", async () => {
86+
process.env = {
87+
...requiredEnv,
88+
GITHUB_APP_ENABLED: "1",
89+
GITHUB_APP_ID: "123",
90+
GITHUB_APP_PRIVATE_KEY: "private-key",
91+
GITHUB_APP_WEBHOOK_SECRET: "webhook-secret",
92+
GITHUB_APP_SLUG: "test-app",
93+
GITHUB_APP_CLIENT_ID: "client-id",
94+
};
95+
96+
await expect(import("../app/env.server")).rejects.toThrow(
97+
"GITHUB_APP_CLIENT_ID and GITHUB_APP_CLIENT_SECRET must be configured together"
98+
);
99+
});
100+
101+
it("accepts a complete GitHub App OAuth configuration", async () => {
102+
process.env = {
103+
...requiredEnv,
104+
GITHUB_APP_ENABLED: "1",
105+
GITHUB_APP_ID: "123",
106+
GITHUB_APP_PRIVATE_KEY: "private-key",
107+
GITHUB_APP_WEBHOOK_SECRET: "webhook-secret",
108+
GITHUB_APP_SLUG: "test-app",
109+
GITHUB_APP_CLIENT_ID: "client-id",
110+
GITHUB_APP_CLIENT_SECRET: "client-secret",
111+
};
112+
113+
const { env } = await import("../app/env.server");
114+
115+
expect(env.GITHUB_APP_CLIENT_ID).toBe("client-id");
116+
expect(env.GITHUB_APP_CLIENT_SECRET).toBe("client-secret");
117+
});
84118
});

0 commit comments

Comments
 (0)