Skip to content

Commit c7faef8

Browse files
committed
fix(github): allow installation tokens to create fork pull requests
1 parent 7ce80cb commit c7faef8

2 files changed

Lines changed: 101 additions & 16 deletions

File tree

‎packages/source-control-github/src/server/GitHubSourceControlProvider.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,72 @@ describe("GitHubSourceControlProvider.getChangeRequest", () => {
708708
});
709709

710710
describe("GitHubSourceControlProvider writes", () => {
711+
it.effect("creates an App fork PR without granting fork edits and pins the acting token", () => {
712+
let current = "before";
713+
const posts: Array<GitHubRest.GitHubRestInput> = [];
714+
const pinnedTokens: Array<string | null> = [];
715+
const { layer, graphqlCalls } = harness({
716+
remotes: remotesOutput(["origin", "[email protected]:acme/web.git"]),
717+
api: {
718+
credential: (host) =>
719+
Effect.succeed({
720+
token: Redacted.make(current),
721+
fingerprint: `${host}:${current}`,
722+
}),
723+
rest: (input) =>
724+
Effect.gen(function* () {
725+
if (input.method === "POST") {
726+
const pinned = yield* GitHubRequestPolicy.PinnedGitHubCredential;
727+
pinnedTokens.push(pinned === null ? null : Redacted.value(pinned.token));
728+
posts.push(input);
729+
return restResponse({ number: 1 }, 201);
730+
}
731+
current = "after";
732+
return yield* Effect.fail(
733+
new GitHubRequestPolicy.GitHubApiResponseError({
734+
host: input.host,
735+
operation: "getViewer",
736+
status: 403,
737+
}),
738+
);
739+
}),
740+
},
741+
graphql: () =>
742+
new Response(
743+
encodeJson({
744+
data: { viewer: { login: "example[bot]", databaseId: 123 } },
745+
}),
746+
{ headers: { "content-type": "application/json" } },
747+
),
748+
});
749+
return Effect.gen(function* () {
750+
const fs = yield* FileSystem.FileSystem;
751+
const bodyFile = yield* fs.makeTempFileScoped({ suffix: ".md" });
752+
yield* fs.writeFileString(bodyFile, "Body");
753+
const provider = yield* GitHubSourceControlProvider.make;
754+
yield* provider.createChangeRequest({
755+
cwd: "/repo",
756+
baseRefName: "main",
757+
headSelector: "other:feature",
758+
title: "Title",
759+
bodyFile,
760+
});
761+
assert.strictEqual(posts.length, 1);
762+
assert.deepStrictEqual(posts[0]?.body, {
763+
base: "main",
764+
head: "other:feature",
765+
title: "Title",
766+
body: "Body",
767+
maintainer_can_modify: false,
768+
});
769+
assert.deepStrictEqual(pinnedTokens, ["before"]);
770+
assert.deepStrictEqual(
771+
graphqlCalls.map((call) => call.authorization),
772+
["Bearer before"],
773+
);
774+
}).pipe(Effect.provide(Layer.merge(layer, NodeServices.layer)), Effect.scoped);
775+
});
776+
711777
it.effect("creates a cross-repository pull request with an owner:branch head", () => {
712778
const requests: Array<GitHubRest.GitHubRestInput> = [];
713779
const { layer } = harness({
@@ -718,6 +784,7 @@ describe("GitHubSourceControlProvider writes", () => {
718784
api: {
719785
rest: (input) =>
720786
Effect.sync(() => {
787+
if (input.path === "user") return restResponse({ login: "me" });
721788
requests.push(input);
722789
return restResponse({ number: 1 }, 201);
723790
}),
@@ -1742,6 +1809,7 @@ it.effect.each([
17421809
api: {
17431810
rest: (input) =>
17441811
Effect.sync(() => {
1812+
if (input.path === "user") return restResponse({ login: "me" });
17451813
targets.push(`${input.host}/${input.path}`);
17461814
return restResponse({
17471815
full_name: expected,

‎packages/source-control-github/src/server/GitHubSourceControlProvider.ts‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,22 +1055,39 @@ export const make = Effect.gen(function* () {
10551055
failure("The pull request description could not be read.", cause),
10561056
),
10571057
);
1058-
yield* rest({
1059-
host: locator.host,
1060-
operation: "createPullRequest",
1061-
method: "POST",
1062-
allowReserve: true,
1063-
path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls`,
1064-
// `owner:branch` is how the REST API takes a fork's head, the same as `gh --head`.
1065-
// gh allows maintainer edits unless told otherwise; the API's default is not documented.
1066-
body: {
1067-
base: input.baseRefName,
1068-
head: input.headSelector,
1069-
title: input.title,
1070-
body,
1071-
maintainer_can_modify: true,
1072-
},
1073-
});
1058+
const credential = yield* api
1059+
.credential(locator.host)
1060+
.pipe(Effect.mapError(fromGitHubApiError));
1061+
yield* Effect.gen(function* () {
1062+
const headOwner = input.headSelector.includes(":")
1063+
? input.headSelector.split(":")[0]
1064+
: undefined;
1065+
const isFork =
1066+
headOwner !== undefined && headOwner.toLowerCase() !== locator.owner.toLowerCase();
1067+
const maintainerCanModify =
1068+
!isFork || !(yield* readViewerLogin(locator.host)).endsWith("[bot]");
1069+
yield* rest({
1070+
host: locator.host,
1071+
operation: "createPullRequest",
1072+
method: "POST",
1073+
allowReserve: true,
1074+
path: `repos/${encodeURIComponent(locator.owner)}/${encodeURIComponent(locator.name)}/pulls`,
1075+
// Installation tokens cannot grant maintainers access to another account's fork.
1076+
body: {
1077+
base: input.baseRefName,
1078+
head: input.headSelector,
1079+
title: input.title,
1080+
body,
1081+
maintainer_can_modify: maintainerCanModify,
1082+
},
1083+
});
1084+
}).pipe(
1085+
Effect.provideService(GitHubRequestPolicy.PinnedGitHubCredential, {
1086+
host: locator.host,
1087+
token: credential.token,
1088+
credentialFingerprint: credential.fingerprint,
1089+
}),
1090+
);
10741091
}).pipe(
10751092
Effect.mapError(
10761093
providerError("createChangeRequest", input.cwd, { reference: input.headSelector }),

0 commit comments

Comments
 (0)