| author |  shivam <91240327+shivamhwp@users.noreply.github.com> 2026-10-15 05:04:13+05:30 |
| committer |  GitHub <noreply@github.com> 2026-10-14 20:34:13-03:00 |
| log | 742c229957b69a0cc5a1379dff6c66b595854057 |
| tree | a72e7f71e5e52610a4752c8311aa30b52b27d5f1 |
| parent | 40a7908df54d5069c93835032c9e955b56b55f1d |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): GitHub App tokens can merge and use other write actions (#19103)
Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>4 files changed, 37 insertions(+), 8 deletions(-)
packages
source-control-github
packages/source-control-github/src/server/gitHubPullRequestJson.test.ts+6-1
| ... | @@ -1017,7 +1017,12 @@ describe("repository access decoding", () => { | ... | @@ -1017,7 +1017,12 @@ describe("repository access decoding", () => { |
| 1017 | // The one place an unknown answer is not granted: a Merge button a reader cannot use wastes | 1017 | // The one place an unknown answer is not granted: a Merge button a reader cannot use wastes |
| 1018 | // the press, where a missing one still leaves the pull request open on its host. | 1018 | // the press, where a missing one still leaves the pull request open on its host. |
| 1019 | expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson())).canWrite).toBe(false); | 1019 | expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson())).canWrite).toBe(false); |
| 1020 | expect(expectSuccess(decodeViewerPermissionsJson(repositoryJson(null))).canWrite).toBe(false); | 1020 | }); |
| 1021 | |||
| 1022 | it("grants write and triage to a GitHub App, which GitHub answers with a null permission", () => { | ||
| 1023 | const access = expectSuccess(decodeViewerPermissionsJson(repositoryJson(null))); | ||
| 1024 | expect(access.canWrite).toBe(true); | ||
| 1025 | expect(access.canTriage).toBe(true); | ||
| 1021 | }); | 1026 | }); |
| 1022 | }); | 1027 | }); |
| 1023 | 1028 |
packages/source-control-github/src/server/gitHubPullRequestJson.ts+9-4
| ... | @@ -708,9 +708,9 @@ const RawRepositoryAccessSchema = Schema.Struct({ | ... | @@ -708,9 +708,9 @@ const RawRepositoryAccessSchema = Schema.Struct({ |
| 708 | squashMergeAllowed: Schema.Boolean, | 708 | squashMergeAllowed: Schema.Boolean, |
| 709 | rebaseMergeAllowed: Schema.Boolean, | 709 | rebaseMergeAllowed: Schema.Boolean, |
| 710 | /** | 710 | /** |
| 711 | * ADMIN, MAINTAIN, WRITE, TRIAGE, READ or NONE. Optional rather than required, unlike the | 711 | * ADMIN, MAINTAIN, WRITE, TRIAGE, READ or NONE, or null for a GitHub App. Optional rather than |
| 712 | * three above: an install that does not report it leaves the viewer's standing unknown, which | 712 | * required, unlike the three above: an install that does not report it still decodes rather |
| 713 | * is answered by granting rather than by failing the whole detail read. | 713 | * than failing the whole detail read, and `toCanWrite` decides what that standing allows. |
| 714 | */ | 714 | */ |
| 715 | viewerPermission: Schema.optional(Schema.NullOr(Schema.String)), | 715 | viewerPermission: Schema.optional(Schema.NullOr(Schema.String)), |
| 716 | }); | 716 | }); |
| ... | @@ -2919,8 +2919,13 @@ export interface GitHubRepositoryAccess { | ... | @@ -2919,8 +2919,13 @@ export interface GitHubRepositoryAccess { |
| 2919 | * need, and offering those to a reader who cannot use them wastes the press and reads as the app | 2919 | * need, and offering those to a reader who cannot use them wastes the press and reads as the app |
| 2920 | * being wrong. Everything softer — commenting, reviewing, resolving — keeps the granting default, | 2920 | * being wrong. Everything softer — commenting, reviewing, resolving — keeps the granting default, |
| 2921 | * because being unable to say something is the worse failure there. | 2921 | * because being unable to say something is the worse failure there. |
| 2922 | * | ||
| 2923 | * A null permission is not that. GitHub answers null only to a GitHub App installation, and no | ||
| 2924 | * API says what an installation may do, so null is granted and GitHub refuses whatever the app | ||
| 2925 | * was not given. A person always gets a role, READ at the least. | ||
| 2922 | */ | 2926 | */ |
| 2923 | function toCanWrite(viewerPermission: string | null | undefined): boolean { | 2927 | export function toCanWrite(viewerPermission: string | null | undefined): boolean { |
| 2928 | if (viewerPermission === null) return true; | ||
| 2924 | switch (viewerPermission?.trim().toUpperCase()) { | 2929 | switch (viewerPermission?.trim().toUpperCase()) { |
| 2925 | case "ADMIN": | 2930 | case "ADMIN": |
| 2926 | case "MAINTAIN": | 2931 | case "MAINTAIN": |
packages/source-control-github/src/server/githubStackActions.test.ts+20
| ... | @@ -325,6 +325,26 @@ it.effect("allows a fork that explicitly permits maintainer updates", () => | ... | @@ -325,6 +325,26 @@ it.effect("allows a fork that explicitly permits maintainer updates", () => |
| 325 | }), | 325 | }), |
| 326 | ); | 326 | ); |
| 327 | 327 | ||
| 328 | it.effect("rebases for a GitHub App, which GitHub gives no permission to read", () => | ||
| 329 | Effect.gen(function* () { | ||
| 330 | gitCalls = []; | ||
| 331 | const api = fake([ | ||
| 332 | stack, | ||
| 333 | { | ||
| 334 | data: { | ||
| 335 | repository: { | ||
| 336 | pr2: { headRepository: { viewerPermission: null }, maintainerCanModify: false }, | ||
| 337 | pr3: { headRepository: { viewerPermission: null }, maintainerCanModify: false }, | ||
| 338 | }, | ||
| 339 | }, | ||
| 340 | }, | ||
| 341 | ...rebaseResponses, | ||
| 342 | ]); | ||
| 343 | yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); | ||
| 344 | expect(gitCalls.filter((args) => args[0] === "push")).toHaveLength(2); | ||
| 345 | }), | ||
| 346 | ); | ||
| 347 | |||
| 328 | it.effect("bounds polling and reports a still-running merge without claiming success", () => | 348 | it.effect("bounds polling and reports a still-running merge without claiming success", () => |
| 329 | Effect.gen(function* () { | 349 | Effect.gen(function* () { |
| 330 | const api = fake([ | 350 | const api = fake([ |
packages/source-control-github/src/server/githubStackActions.ts+2-3
| ... | @@ -10,7 +10,7 @@ import * as Schema from "effect/Schema"; | ... | @@ -10,7 +10,7 @@ import * as Schema from "effect/Schema"; |
| 10 | 10 | ||
| 11 | import * as GitHubApi from "./GitHubApi.ts"; | 11 | import * as GitHubApi from "./GitHubApi.ts"; |
| 12 | import { aliasedGraphQlDocument } from "./githubGraphQl.ts"; | 12 | import { aliasedGraphQlDocument } from "./githubGraphQl.ts"; |
| 13 | import { decodePullRequestStacksJson } from "./gitHubPullRequestJson.ts"; | 13 | import { decodePullRequestStacksJson, toCanWrite } from "./gitHubPullRequestJson.ts"; |
| 14 | import { cascadeRebaseStack } from "./githubStackRebase.ts"; | 14 | import { cascadeRebaseStack } from "./githubStackRebase.ts"; |
| 15 | 15 | ||
| 16 | const stackErrorIdentity = { | 16 | const stackErrorIdentity = { |
| ... | @@ -216,8 +216,7 @@ export const runGitHubStackAction = Effect.fn("runGitHubStackAction")(function* | ... | @@ -216,8 +216,7 @@ export const runGitHubStackAction = Effect.fn("runGitHubStackAction")(function* |
| 216 | const pr = access.data.repository?.[`pr${layer.number}`]; | 216 | const pr = access.data.repository?.[`pr${layer.number}`]; |
| 217 | return ( | 217 | return ( |
| 218 | !pr?.headRepository || | 218 | !pr?.headRepository || |
| 219 | (!pr.maintainerCanModify && | 219 | (!pr.maintainerCanModify && !toCanWrite(pr.headRepository.viewerPermission)) |
| 220 | !["ADMIN", "MAINTAIN", "WRITE"].includes(pr.headRepository.viewerPermission ?? "")) | ||
| 221 | ); | 220 | ); |
| 222 | }) | 221 | }) |
| 223 | ) | 222 | ) |