From 8efb9bd506e82bd6c9ef307b3e77c14753490c34 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 13 Sep 2026 08:42:55 +0000 Subject: [PATCH 1/3] fix: allow GitHub App access to public repositories without installation --- docs/github-app-setup.md | 11 +++- src/core/github-app.ts | 24 +++++++- src/core/model/repository-access.schema.ts | 1 + src/core/repository-access.types.ts | 2 + test/github-app.test.js | 66 ++++++++++++++++++++++ 5 files changed, 100 insertions(+), 4 deletions(-) diff --git a/docs/github-app-setup.md b/docs/github-app-setup.md index 4512d78..9d25b6f 100644 --- a/docs/github-app-setup.md +++ b/docs/github-app-setup.md @@ -95,8 +95,15 @@ form. Existing installations have direct account-specific configuration links. GitHub may require organization administrator approval. Use **Refresh access after approval** on the Connections page when approval is delayed. -App-connected accounts default to the App for new repository/PR access. The -explicit **Use existing OAuth access** choice handles repositories not yet +App-connected accounts default to the App for new repository/PR access. +Public repositories outside the selected installations use the App user grant, +so users can paste a public URL without installing the App on its owner account. +The connection records the repository ID and checks that it is still public on +each source access. If it becomes private, reconnect through an installation +with access. Existing installation bindings retain their installation checks. +This uses GitHub's documented [public resource access for App user tokens](https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/choosing-permissions-for-a-github-app). + +The explicit **Use existing OAuth access** choice handles repositories not yet available through the App. An App error never silently selects OAuth. Gists continue using OAuth. An App-only user entering a gist URL is prompted to connect OAuth, with the current repository permission scope explained. The form draft diff --git a/src/core/github-app.ts b/src/core/github-app.ts index c1ef41d..24b3e1a 100644 --- a/src/core/github-app.ts +++ b/src/core/github-app.ts @@ -220,7 +220,19 @@ async function installationToken(binding: RepositoryAccess, ownerId: string): Pr } export async function boundAppToken(ownerId: string, binding: RepositoryAccess): Promise { - if (!Number.isSafeInteger(binding.repositoryId) || !Number.isSafeInteger(binding.installationId)) throw appError(); + if (!Number.isSafeInteger(binding.repositoryId)) throw appError(); + if (binding.publicRead === true) { + if (binding.installationId !== undefined) throw appError(); + const userToken = await appUserToken(ownerId); + const repo = await githubRequest(`/repositories/${binding.repositoryId}`, userToken); + // A public binding must never gain private access, even if the user later + // installs the App on this repository. Reconnect explicitly to do that. + if (repo.id !== binding.repositoryId || repo.private !== false) throw appError("github_app_access_required"); + // Resolve the current grant again on each source read. Do not register a + // repository-specific renewal callback against a user token shared by repos. + return userToken; + } + if (!Number.isSafeInteger(binding.installationId)) throw appError(); const userToken = await appUserToken(ownerId); // User token checks the intersection of user and App rights on every access. // No indefinite local authorization cache can preserve a departed user's access. @@ -239,7 +251,15 @@ export async function selectRepositoryAccess(ownerId: string, fullName: string, const hasApp = config.GITHUB_APP_ENABLED && await CredentialModel.exists({ ownerId, provider: APP_PROVIDER }); if (choice === "github-app" || (choice === undefined && hasApp)) { const repo = (await appRepositories(ownerId)).find(r => r.full_name.toLowerCase() === fullName.toLowerCase()); - if (!repo) throw appError("github_app_access_required"); + if (!repo) { + const userToken = await appUserToken(ownerId); + const publicRepo = await githubRequest( + `/repos/${fullName.split("/").map(encodeURIComponent).join("/")}`, userToken); + if (publicRepo.private !== false || !Number.isSafeInteger(publicRepo.id)) throw appError("github_app_access_required"); + const binding: RepositoryAccess = { kind: "github-app", publicRead: true, + repositoryId: publicRepo.id, revision: randomUUID() }; + return { binding, token: await boundAppToken(ownerId, binding) }; + } const binding: RepositoryAccess = { kind: "github-app", repositoryId: repo.id, installationId: repo.installationId, revision: randomUUID() }; return { binding, token: await boundAppToken(ownerId, binding) }; } diff --git a/src/core/model/repository-access.schema.ts b/src/core/model/repository-access.schema.ts index 6e18f53..8c7f805 100644 --- a/src/core/model/repository-access.schema.ts +++ b/src/core/model/repository-access.schema.ts @@ -3,6 +3,7 @@ import { Schema } from "mongoose"; export const repositoryAccessSchema = new Schema({ kind: { type: String, enum: ["oauth", "github-app"], required: true }, repositoryId: Number, + publicRead: Boolean, installationId: Number, revision: { type: String, required: true }, }, { _id: false }); diff --git a/src/core/repository-access.types.ts b/src/core/repository-access.types.ts index 7bbeb1a..46ac0a5 100644 --- a/src/core/repository-access.types.ts +++ b/src/core/repository-access.types.ts @@ -2,6 +2,8 @@ export interface RepositoryAccess { kind: "oauth" | "github-app"; repositoryId?: number; + /** App user access to a verified public repository, without an installation. */ + publicRead?: boolean; installationId?: number; revision: string; } diff --git a/test/github-app.test.js b/test/github-app.test.js index 6fb7fdb..447a523 100644 --- a/test/github-app.test.js +++ b/test/github-app.test.js @@ -197,6 +197,72 @@ describeMongo("GitHub App credential and repository integration", function () { expect(selected.token).to.equal("legacy-secret"); expect(selected.binding.kind).to.equal("oauth"); }); + it("opens public repository metadata and branches without an installation or OAuth", async () => { + await app.saveAppGrant(owner.id, data()); + mock(url => { + if (url.includes("/user/installations")) return { installations: [] }; + if (url.endsWith("/branches?per_page=100")) return [{ name: "main", commit: { sha: "abc123" } }]; + if (url.endsWith("/branches")) return [{ name: "main", commit: { sha: "abc123" } }]; + if (url.endsWith("/readme")) return { status: 404 }; + if (url.endsWith("/pages")) return { status: 404 }; + return { id: 7, private: false, full_name: "other/public", name: "public", owner: { login: "other" }, default_branch: "main" }; + }); + const selected = await app.selectRepositoryAccess(owner.id, "other/public", "github-app"); + expect(selected.token).to.equal("ghu_access1"); + expect(selected.binding).to.include({ kind: "github-app", publicRead: true, repositoryId: 7 }); + expect(selected.binding.installationId).to.equal(undefined); + const Repos = require("../src/core/model/anonymizedRepositories/anonymizedRepositories.model").default; + const model = await Repos.create({ repoId: "public-access", owner: owner.id, githubAccess: selected.binding }); + expect((await Repos.findById(model._id)).githubAccess.publicRead).to.equal(true); + const { getRepositoryFromGitHub } = require("../src/core/source/GitHubRepository"); + const repository = await getRepositoryFromGitHub({ owner: "other", repo: "public", accessToken: selected.token }); + expect(repository.fullName).to.equal("other/public"); + expect((await repository.branches({ accessToken: selected.token }))[0].name).to.equal("main"); + expect(calls.some(call => call.url.includes("/access_tokens"))).to.equal(false); + expect(calls.every(call => !String(call.options.headers.authorization || call.options.headers.Authorization).includes("legacy"))).to.equal(true); + }); + it("requires explicit public visibility when selecting an uninstalled repository", async () => { + await app.saveAppGrant(owner.id, data()); + for (const metadata of [{ id: 7, private: true }, { id: 7 }, { private: false }]) { + mock(url => url.includes("/user/installations") ? { installations: [] } : metadata); + await rejects(app.selectRepositoryAccess(owner.id, "other/private", "github-app"), "github_app_access_required"); + } + }); + it("rejects missing repositories and propagates upstream failures instead of using OAuth", async () => { + await setCredential(owner.id, "legacy-secret"); + await app.saveAppGrant(owner.id, data()); + for (const [status, message] of [[404, "github_app_access_required"], [500, "github_unavailable"], [429, "github_rate_limit_exceeded"]]) { + mock(url => url.includes("/user/installations") ? { installations: [] } : { status }); + await rejects(app.selectRepositoryAccess(owner.id, "other/missing", "github-app"), message); + } + }); + it("stops public source reads after visibility changes, deletion, or grant revocation", async () => { + await app.saveAppGrant(owner.id, data()); + const binding = { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }; + mock(() => ({ id: 7, private: false })); + expect(await app.boundAppToken(owner.id, binding)).to.equal("ghu_access1"); + for (const metadata of [{ id: 7, private: true }, { status: 404 }, { id: 8, private: false }]) { + mock(() => metadata); + await rejects(app.boundAppToken(owner.id, binding), "github_app_access_required"); + } + await Credentials.updateOne({ ownerId: owner.id }, { $set: { revoked: true } }); + mock(() => { throw new Error("revoked grant must not reach GitHub"); }); + await rejects(app.boundAppToken(owner.id, binding), "github_app_reconnect_required"); + }); + it("refreshes an expired App grant before reading a public repository", async () => { + await app.saveAppGrant(owner.id, data("old", -1)); + mock(url => url.includes("/login/oauth/access_token") ? data("new") : { id: 7, private: false }); + expect(await app.boundAppToken(owner.id, { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" })) + .to.equal("ghu_accessnew"); + }); + it("does not reinterpret missing or mixed installation bindings as public access", async () => { + await app.saveAppGrant(owner.id, data()); + mock(() => { throw new Error("invalid binding must not reach GitHub"); }); + for (const binding of [ + { kind: "github-app", repositoryId: 7, revision: "one" }, + { kind: "github-app", publicRead: true, repositoryId: 7, installationId: 4, revision: "one" }, + ]) await rejects(app.boundAppToken(owner.id, binding), "github_app_reconnect_required"); + }); it("checks the user's access before minting a repository-restricted token", async () => { await app.saveAppGrant(owner.id, data()); const binding = { kind: "github-app", installationId: 4, repositoryId: 7, revision: "one" }; From 5e5cc6e0c2bce112423468526cde27efd5b5a28a Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 13 Sep 2026 15:05:43 +0000 Subject: [PATCH 2/3] fix: renew public grants and verify source identity and visibility --- docs/github-app-setup.md | 8 ++-- src/core/GitHubUtils.ts | 2 +- src/core/PullRequest.ts | 2 +- src/core/github-app.ts | 29 +++++++----- src/server/routes/repository-private.ts | 2 +- test/github-app.test.js | 63 +++++++++++++++++++++---- 6 files changed, 79 insertions(+), 27 deletions(-) diff --git a/docs/github-app-setup.md b/docs/github-app-setup.md index 9d25b6f..f0ae439 100644 --- a/docs/github-app-setup.md +++ b/docs/github-app-setup.md @@ -98,9 +98,11 @@ after approval** on the Connections page when approval is delayed. App-connected accounts default to the App for new repository/PR access. Public repositories outside the selected installations use the App user grant, so users can paste a public URL without installing the App on its owner account. -The connection records the repository ID and checks that it is still public on -each source access. If it becomes private, reconnect through an installation -with access. Existing installation bindings retain their installation checks. +The connection records the repository ID and verifies that the stored source +name still resolves to that ID with public visibility on each source access. +Private and Enterprise-internal repositories require an installation with access. +Long API traversals renew the App user token under the same user quota. +Existing installation bindings retain their installation checks. This uses GitHub's documented [public resource access for App user tokens](https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/choosing-permissions-for-a-github-app). The explicit **Use existing OAuth access** choice handles repositories not yet diff --git a/src/core/GitHubUtils.ts b/src/core/GitHubUtils.ts index f133860..41ee6c3 100644 --- a/src/core/GitHubUtils.ts +++ b/src/core/GitHubUtils.ts @@ -304,7 +304,7 @@ export async function getToken(repository: Repository) { } } if (repository.model.githubAccess?.kind === "github-app") { - return boundAppToken(repository.owner.id, repository.model.githubAccess); + return boundAppToken(repository.owner.id, repository.model.githubAccess, repository.model.source.repositoryName); } const credential = await getCredential(repository.owner.id); const ownerAccessToken = credential?.token; diff --git a/src/core/PullRequest.ts b/src/core/PullRequest.ts index ce2c474..05e23e1 100644 --- a/src/core/PullRequest.ts +++ b/src/core/PullRequest.ts @@ -26,7 +26,7 @@ export default class PullRequest { } async getToken() { - if (this._model.githubAccess?.kind === "github-app") return boundAppToken(this.owner.id, this._model.githubAccess); + if (this._model.githubAccess?.kind === "github-app") return boundAppToken(this.owner.id, this._model.githubAccess, this._model.source.repositoryFullName); return (await getCredentialToken(this.owner.id, "github", { collection: "anonymizedpullrequests", id: this._model._id })) || config.GITHUB_TOKEN; } diff --git a/src/core/github-app.ts b/src/core/github-app.ts index 24b3e1a..1cff71c 100644 --- a/src/core/github-app.ts +++ b/src/core/github-app.ts @@ -144,7 +144,7 @@ export async function reconcileInstallation(installationId: number, revision: st } export interface GitHubRepositoryInfo { - id: number; full_name: string; name: string; private: boolean; html_url: string; size: number; + id: number; full_name: string; name: string; private: boolean; visibility?: string; html_url: string; size: number; default_branch: string; owner: { id: number; login: string }; } export interface AppInstallation { @@ -219,17 +219,24 @@ async function installationToken(binding: RepositoryAccess, ownerId: string): Pr try { return await work; } finally { minting.delete(key); } } -export async function boundAppToken(ownerId: string, binding: RepositoryAccess): Promise { +async function publicAppUserToken(ownerId: string): Promise { + const token = await appUserToken(ownerId); + registerGitHubToken(token, { quotaKey: `app-user:${ownerId}`, renew: () => publicAppUserToken(ownerId) }); + return token; +} + +export async function boundAppToken(ownerId: string, binding: RepositoryAccess, sourceName?: string): Promise { if (!Number.isSafeInteger(binding.repositoryId)) throw appError(); if (binding.publicRead === true) { - if (binding.installationId !== undefined) throw appError(); - const userToken = await appUserToken(ownerId); - const repo = await githubRequest(`/repositories/${binding.repositoryId}`, userToken); + if (binding.installationId !== undefined || !sourceName || !/^[^/\s]+\/[^/\s]+$/.test(sourceName)) throw appError(); + const userToken = await publicAppUserToken(ownerId); + // Source reads use owner/name, so validate that exact name against the + // bound ID. A replacement at a renamed repository's old URL must fail. + const repo = await githubRequest( + `/repos/${sourceName.split("/").map(encodeURIComponent).join("/")}`, userToken); // A public binding must never gain private access, even if the user later // installs the App on this repository. Reconnect explicitly to do that. - if (repo.id !== binding.repositoryId || repo.private !== false) throw appError("github_app_access_required"); - // Resolve the current grant again on each source read. Do not register a - // repository-specific renewal callback against a user token shared by repos. + if (repo.id !== binding.repositoryId || repo.private !== false || repo.visibility !== "public") throw appError("github_app_access_required"); return userToken; } if (!Number.isSafeInteger(binding.installationId)) throw appError(); @@ -255,13 +262,13 @@ export async function selectRepositoryAccess(ownerId: string, fullName: string, const userToken = await appUserToken(ownerId); const publicRepo = await githubRequest( `/repos/${fullName.split("/").map(encodeURIComponent).join("/")}`, userToken); - if (publicRepo.private !== false || !Number.isSafeInteger(publicRepo.id)) throw appError("github_app_access_required"); + if (publicRepo.private !== false || publicRepo.visibility !== "public" || !Number.isSafeInteger(publicRepo.id)) throw appError("github_app_access_required"); const binding: RepositoryAccess = { kind: "github-app", publicRead: true, repositoryId: publicRepo.id, revision: randomUUID() }; - return { binding, token: await boundAppToken(ownerId, binding) }; + return { binding, token: await boundAppToken(ownerId, binding, fullName) }; } const binding: RepositoryAccess = { kind: "github-app", repositoryId: repo.id, installationId: repo.installationId, revision: randomUUID() }; - return { binding, token: await boundAppToken(ownerId, binding) }; + return { binding, token: await boundAppToken(ownerId, binding, fullName) }; } const token = await getCredentialToken(ownerId); if (!token) throw appError("github_oauth_required"); diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index 12a06bd..a325af0 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -495,7 +495,7 @@ router.post( } if (repoUpdate.fullName !== repo.model.source.repositoryName && user.id !== repo.owner.id) throw appError("not_owner", 403); const sourceAccess = repo.model.githubAccess?.kind === "github-app" && repoUpdate.fullName === repo.model.source.repositoryName - ? { token: await boundAppToken(repo.owner.id, repo.model.githubAccess), binding: repo.model.githubAccess } + ? { token: await boundAppToken(repo.owner.id, repo.model.githubAccess, repo.model.source.repositoryName), binding: repo.model.githubAccess } : await selectRepositoryAccess(repo.owner.id, `${parsedRepository.owner}/${parsedRepository.name}`, repo.model.githubAccess?.kind || "oauth"); const repository = await getRepositoryFromGitHub({ accessToken: sourceAccess.token, diff --git a/test/github-app.test.js b/test/github-app.test.js index 447a523..76ca0de 100644 --- a/test/github-app.test.js +++ b/test/github-app.test.js @@ -205,7 +205,7 @@ describeMongo("GitHub App credential and repository integration", function () { if (url.endsWith("/branches")) return [{ name: "main", commit: { sha: "abc123" } }]; if (url.endsWith("/readme")) return { status: 404 }; if (url.endsWith("/pages")) return { status: 404 }; - return { id: 7, private: false, full_name: "other/public", name: "public", owner: { login: "other" }, default_branch: "main" }; + return { id: 7, private: false, visibility: "public", full_name: "other/public", name: "public", owner: { login: "other" }, default_branch: "main" }; }); const selected = await app.selectRepositoryAccess(owner.id, "other/public", "github-app"); expect(selected.token).to.equal("ghu_access1"); @@ -223,7 +223,7 @@ describeMongo("GitHub App credential and repository integration", function () { }); it("requires explicit public visibility when selecting an uninstalled repository", async () => { await app.saveAppGrant(owner.id, data()); - for (const metadata of [{ id: 7, private: true }, { id: 7 }, { private: false }]) { + for (const metadata of [{ id: 7, private: true }, { id: 7 }, { id: 7, private: false }, { id: 7, private: false, visibility: "internal" }, { private: false, visibility: "public" }]) { mock(url => url.includes("/user/installations") ? { installations: [] } : metadata); await rejects(app.selectRepositoryAccess(owner.id, "other/private", "github-app"), "github_app_access_required"); } @@ -239,29 +239,72 @@ describeMongo("GitHub App credential and repository integration", function () { it("stops public source reads after visibility changes, deletion, or grant revocation", async () => { await app.saveAppGrant(owner.id, data()); const binding = { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }; - mock(() => ({ id: 7, private: false })); - expect(await app.boundAppToken(owner.id, binding)).to.equal("ghu_access1"); - for (const metadata of [{ id: 7, private: true }, { status: 404 }, { id: 8, private: false }]) { + mock(() => ({ id: 7, private: false, visibility: "public" })); + expect(await app.boundAppToken(owner.id, binding, "other/public")).to.equal("ghu_access1"); + for (const metadata of [{ id: 7, private: true }, { id: 7, private: false, visibility: "internal" }, { id: 7, private: false }, { status: 404 }, { id: 8, private: false, visibility: "public" }]) { mock(() => metadata); - await rejects(app.boundAppToken(owner.id, binding), "github_app_access_required"); + await rejects(app.boundAppToken(owner.id, binding, "other/public"), "github_app_access_required"); } await Credentials.updateOne({ ownerId: owner.id }, { $set: { revoked: true } }); mock(() => { throw new Error("revoked grant must not reach GitHub"); }); - await rejects(app.boundAppToken(owner.id, binding), "github_app_reconnect_required"); + await rejects(app.boundAppToken(owner.id, binding, "other/public"), "github_app_reconnect_required"); }); it("refreshes an expired App grant before reading a public repository", async () => { await app.saveAppGrant(owner.id, data("old", -1)); - mock(url => url.includes("/login/oauth/access_token") ? data("new") : { id: 7, private: false }); - expect(await app.boundAppToken(owner.id, { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" })) + mock(url => url.includes("/login/oauth/access_token") ? data("new") : { id: 7, private: false, visibility: "public" }); + expect(await app.boundAppToken(owner.id, { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }, "other/public")) .to.equal("ghu_accessnew"); }); + it("renews a public token inside an existing Octokit traversal without changing owner or quota", async () => { + await app.saveAppGrant(owner.id, data("old")); + const sent = []; + mock((url, options) => { + if (url.includes("/login/oauth/access_token")) return data("new"); + if (url.includes("/git/trees/")) { + sent.push(new globalThis.Headers(options.headers).get("authorization")); + return { tree: [], truncated: false }; + } + return { id: url.endsWith("/second") ? 8 : 7, private: false, visibility: "public" }; + }); + const binding = { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }; + const token = await app.boundAppToken(owner.id, binding, "other/public"); + const oct = require("../src/core/GitHubUtils").octokit(token); + await app.boundAppToken(owner.id, { ...binding, repositoryId: 8 }, "other/second"); + await oct.git.getTree({ owner: "other", repo: "public", tree_sha: "first" }); + await Credentials.updateOne({ ownerId: owner.id, provider: app.APP_PROVIDER }, { $set: { expiresAt: new Date(0) } }); + await oct.git.getTree({ owner: "other", repo: "public", tree_sha: "second" }); + expect(sent).to.deep.equal(["token ghu_accessold", "token ghu_accessnew"]); + expect(githubQuotaKey(token)).to.equal(`app-user:${owner.id}`); + expect(githubQuotaKey("ghu_accessnew")).to.equal(githubQuotaKey(token)); + }); + it("rejects a replacement at the original name for repository and PR source reads", async () => { + await app.saveAppGrant(owner.id, data()); + const binding = { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }; + const Repos = require("../src/core/model/anonymizedRepositories/anonymizedRepositories.model").default; + const PRs = require("../src/core/model/anonymizedPullRequests/anonymizedPullRequests.model").default; + const Repository = require("../src/core/Repository").default; + const PullRequest = require("../src/core/PullRequest").default; + const resources = [ + new Repository(new Repos({ owner: owner.id, githubAccess: binding, source: { repositoryName: "other/original" } })), + new PullRequest(new PRs({ owner: owner.id, githubAccess: binding, source: { repositoryFullName: "other/original" } })), + ]; + mock(url => { + if (url.endsWith("/repositories/7")) return { id: 7, private: false, visibility: "public", full_name: "other/renamed" }; + if (url.endsWith("/repos/other/original")) return { id: 8, private: false, visibility: "public" }; + throw new Error("unexpected source lookup"); + }); + for (const resource of resources) await rejects(resource.getToken(), "github_app_access_required"); + expect(calls.every(call => call.url.endsWith("/repos/other/original"))).to.equal(true); + mock(() => ({ id: 7, private: false, visibility: "public" })); + for (const resource of resources) expect(await resource.getToken()).to.equal("ghu_access1"); + }); it("does not reinterpret missing or mixed installation bindings as public access", async () => { await app.saveAppGrant(owner.id, data()); mock(() => { throw new Error("invalid binding must not reach GitHub"); }); for (const binding of [ { kind: "github-app", repositoryId: 7, revision: "one" }, { kind: "github-app", publicRead: true, repositoryId: 7, installationId: 4, revision: "one" }, - ]) await rejects(app.boundAppToken(owner.id, binding), "github_app_reconnect_required"); + ]) await rejects(app.boundAppToken(owner.id, binding, "other/public"), "github_app_reconnect_required"); }); it("checks the user's access before minting a repository-restricted token", async () => { await app.saveAppGrant(owner.id, data()); From dc026e2ee8414b2271e5419cd0874263f41315cb Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 13 Sep 2026 15:16:45 +0000 Subject: [PATCH 3/3] fix: revalidate scoped public tokens throughout repository reads --- docs/github-app-setup.md | 4 ++- src/core/github-app.ts | 50 +++++++++++++++++++++++++++-------- test/github-app.test.js | 57 ++++++++++++++++++++++++++++++++++------ 3 files changed, 91 insertions(+), 20 deletions(-) diff --git a/docs/github-app-setup.md b/docs/github-app-setup.md index f0ae439..46f61c5 100644 --- a/docs/github-app-setup.md +++ b/docs/github-app-setup.md @@ -101,7 +101,9 @@ so users can paste a public URL without installing the App on its owner account. The connection records the repository ID and verifies that the stored source name still resolves to that ID with public visibility on each source access. Private and Enterprise-internal repositories require an installation with access. -Long API traversals renew the App user token under the same user quota. +Public reads use a [repository-scoped App user token](https://docs.github.com/en/rest/apps/apps#create-a-scoped-access-token) with read-only permissions. +Each API request renews through its own repository binding, rechecking identity +and public visibility while keeping the same user quota. Existing installation bindings retain their installation checks. This uses GitHub's documented [public resource access for App user tokens](https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/choosing-permissions-for-a-github-app). diff --git a/src/core/github-app.ts b/src/core/github-app.ts index 1cff71c..03aea59 100644 --- a/src/core/github-app.ts +++ b/src/core/github-app.ts @@ -1,5 +1,5 @@ import { registerGitHubToken } from "./github-token-context"; -import { createSign, randomUUID } from "crypto"; +import { createHash, createSign, randomUUID } from "crypto"; import { readFileSync } from "fs"; import config from "../config"; import AnonymousError from "./AnonymousError"; @@ -15,12 +15,12 @@ export function appError(code = "github_app_reconnect_required", status = 403) { } // Never expose upstream bodies, bearer credentials or signed URLs in errors. -export async function githubRequest(path: string, token: string, method = "GET", body?: unknown): Promise { +export async function githubRequest(path: string, token: string, method = "GET", body?: unknown, scheme: "Bearer" | "Basic" = "Bearer"): Promise { if (!path.startsWith("/") || path.startsWith("//")) throw appError("invalid_github_path", 400); let response: Response; try { response = await fetch(`https://api.github.com${path}`, { - method, headers: { Accept: "application/vnd.github+json", Authorization: `Bearer ${token}`, + method, headers: { Accept: "application/vnd.github+json", Authorization: `${scheme} ${token}`, "X-GitHub-Api-Version": "2022-11-28", "Content-Type": "application/json" }, body: body === undefined ? undefined : JSON.stringify(body), signal: AbortSignal.timeout(20000), }); @@ -180,7 +180,7 @@ export async function appRepositories(ownerId: string) { const installationTokens = new Map(); const minting = new Map>(); -export function clearAppTokenCache() { installationTokens.clear(); } +export function clearAppTokenCache() { installationTokens.clear(); publicTokens.clear(); } async function installationToken(binding: RepositoryAccess, ownerId: string): Promise { const id = binding.installationId; let local = await InstallationModel.findOne({ appId: config.GITHUB_APP_ID, installationId: id }).lean(); @@ -219,17 +219,40 @@ async function installationToken(binding: RepositoryAccess, ownerId: string): Pr try { return await work; } finally { minting.delete(key); } } -async function publicAppUserToken(ownerId: string): Promise { - const token = await appUserToken(ownerId); - registerGitHubToken(token, { quotaKey: `app-user:${ownerId}`, renew: () => publicAppUserToken(ownerId) }); - return token; +const publicTokens = new Map(); +const publicMinting = new Map>(); + +async function scopedPublicToken(ownerId: string, repositoryId: number, sourceName: string, userToken: string, force: boolean) { + const key = `${ownerId}:${repositoryId}:${createHash("sha256").update(userToken).digest("hex")}`; + if (force) publicTokens.delete(key); + const cached = publicTokens.get(key); + if (cached && cached.expires > Date.now() + 60000) return cached.token; + if (publicMinting.has(key)) return publicMinting.get(key)!; + const mint = (async () => { + const basic = Buffer.from(`${config.GITHUB_APP_CLIENT_ID}:${config.GITHUB_APP_CLIENT_SECRET}`).toString("base64"); + const issued = await githubRequest<{ token: string; expires_at?: string | null }>( + `/applications/${encodeURIComponent(config.GITHUB_APP_CLIENT_ID)}/token/scoped`, basic, "POST", { + access_token: userToken, target: sourceName.split("/")[0], repository_ids: [repositoryId], + permissions: { metadata: "read", contents: "read", pull_requests: "read", pages: "read" }, + }, "Basic"); + if (typeof issued.token !== "string" || !issued.token || issued.token === userToken) throw appError("github_app_access_required"); + // An omitted expiration does not justify caching beyond the current read. + const expires = issued.expires_at ? Date.parse(issued.expires_at) : 0; + if (Number.isFinite(expires) && expires > Date.now() + 60000) { + if (publicTokens.size >= 1000) publicTokens.clear(); + publicTokens.set(key, { token: issued.token, expires }); + } + return issued.token; + })(); + publicMinting.set(key, mint); + try { return await mint; } finally { publicMinting.delete(key); } } -export async function boundAppToken(ownerId: string, binding: RepositoryAccess, sourceName?: string): Promise { +export async function boundAppToken(ownerId: string, binding: RepositoryAccess, sourceName?: string, force = false): Promise { if (!Number.isSafeInteger(binding.repositoryId)) throw appError(); if (binding.publicRead === true) { if (binding.installationId !== undefined || !sourceName || !/^[^/\s]+\/[^/\s]+$/.test(sourceName)) throw appError(); - const userToken = await publicAppUserToken(ownerId); + const userToken = await appUserToken(ownerId); // Source reads use owner/name, so validate that exact name against the // bound ID. A replacement at a renamed repository's old URL must fail. const repo = await githubRequest( @@ -237,7 +260,12 @@ export async function boundAppToken(ownerId: string, binding: RepositoryAccess, // A public binding must never gain private access, even if the user later // installs the App on this repository. Reconnect explicitly to do that. if (repo.id !== binding.repositoryId || repo.private !== false || repo.visibility !== "public") throw appError("github_app_access_required"); - return userToken; + const token = await scopedPublicToken(ownerId, binding.repositoryId!, sourceName, userToken, force); + // Each repository gets a distinct bearer token, so concurrent public + // traversals can revalidate their own binding on every request. + registerGitHubToken(token, { quotaKey: `app-user:${ownerId}`, + renew: force => boundAppToken(ownerId, binding, sourceName, force) }); + return token; } if (!Number.isSafeInteger(binding.installationId)) throw appError(); const userToken = await appUserToken(ownerId); diff --git a/test/github-app.test.js b/test/github-app.test.js index 76ca0de..7745705 100644 --- a/test/github-app.test.js +++ b/test/github-app.test.js @@ -141,12 +141,14 @@ describeMongo("GitHub App credential and repository integration", function () { owner = await Users.create({ username: "owner", externalIDs: { github: "10" } }); }); afterEach(() => { globalThis.fetch = previousFetch; }); - function mock(handler) { + function mock(handler, scopeHandler) { globalThis.fetch = async (url, options) => { if (String(url).startsWith(base)) return previousFetch(url, options); const body = options?.body ? JSON.parse(options.body) : undefined; calls.push({ url: String(url), body, options }); - const result = await handler(String(url), options, body); + const result = String(url).endsWith("/token/scoped") + ? scopeHandler ? await scopeHandler(body, options) : { token: `ghu_scoped_${body.repository_ids[0]}_${body.access_token}`, expires_at: new Date(Date.now() + 3600000).toISOString() } + : await handler(String(url), options, body); return new globalThis.Response(JSON.stringify(result.body || result), { status: result.status || 200, headers: { "content-type": "application/json" } }); }; } @@ -208,7 +210,7 @@ describeMongo("GitHub App credential and repository integration", function () { return { id: 7, private: false, visibility: "public", full_name: "other/public", name: "public", owner: { login: "other" }, default_branch: "main" }; }); const selected = await app.selectRepositoryAccess(owner.id, "other/public", "github-app"); - expect(selected.token).to.equal("ghu_access1"); + expect(selected.token).to.equal("ghu_scoped_7_ghu_access1"); expect(selected.binding).to.include({ kind: "github-app", publicRead: true, repositoryId: 7 }); expect(selected.binding.installationId).to.equal(undefined); const Repos = require("../src/core/model/anonymizedRepositories/anonymizedRepositories.model").default; @@ -240,7 +242,7 @@ describeMongo("GitHub App credential and repository integration", function () { await app.saveAppGrant(owner.id, data()); const binding = { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }; mock(() => ({ id: 7, private: false, visibility: "public" })); - expect(await app.boundAppToken(owner.id, binding, "other/public")).to.equal("ghu_access1"); + expect(await app.boundAppToken(owner.id, binding, "other/public")).to.equal("ghu_scoped_7_ghu_access1"); for (const metadata of [{ id: 7, private: true }, { id: 7, private: false, visibility: "internal" }, { id: 7, private: false }, { status: 404 }, { id: 8, private: false, visibility: "public" }]) { mock(() => metadata); await rejects(app.boundAppToken(owner.id, binding, "other/public"), "github_app_access_required"); @@ -253,7 +255,7 @@ describeMongo("GitHub App credential and repository integration", function () { await app.saveAppGrant(owner.id, data("old", -1)); mock(url => url.includes("/login/oauth/access_token") ? data("new") : { id: 7, private: false, visibility: "public" }); expect(await app.boundAppToken(owner.id, { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }, "other/public")) - .to.equal("ghu_accessnew"); + .to.equal("ghu_scoped_7_ghu_accessnew"); }); it("renews a public token inside an existing Octokit traversal without changing owner or quota", async () => { await app.saveAppGrant(owner.id, data("old")); @@ -273,9 +275,48 @@ describeMongo("GitHub App credential and repository integration", function () { await oct.git.getTree({ owner: "other", repo: "public", tree_sha: "first" }); await Credentials.updateOne({ ownerId: owner.id, provider: app.APP_PROVIDER }, { $set: { expiresAt: new Date(0) } }); await oct.git.getTree({ owner: "other", repo: "public", tree_sha: "second" }); - expect(sent).to.deep.equal(["token ghu_accessold", "token ghu_accessnew"]); + expect(sent).to.deep.equal(["token ghu_scoped_7_ghu_accessold", "token ghu_scoped_7_ghu_accessnew"]); expect(githubQuotaKey(token)).to.equal(`app-user:${owner.id}`); - expect(githubQuotaKey("ghu_accessnew")).to.equal(githubQuotaKey(token)); + expect(githubQuotaKey("ghu_scoped_7_ghu_accessnew")).to.equal(githubQuotaKey(token)); + }); + it("revalidates each public traversal independently after another repository binds the same user", async () => { + await app.saveAppGrant(owner.id, data()); + let firstVisibility = "public"; + const sent = []; + mock(url => { + if (url.includes("/git/trees/")) { sent.push(url); return { tree: [] }; } + return { id: url.endsWith("/first") ? 7 : 8, private: false, + visibility: url.endsWith("/first") ? firstVisibility : "public" }; + }); + const binding = { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }; + const first = await app.boundAppToken(owner.id, binding, "other/first"); + const second = await app.boundAppToken(owner.id, { ...binding, repositoryId: 8 }, "other/second"); + expect(first).not.to.equal(second); + const { octokit } = require("../src/core/GitHubUtils"); + const firstClient = octokit(first), secondClient = octokit(second); + await firstClient.git.getTree({ owner: "other", repo: "first", tree_sha: "one" }); + firstVisibility = "internal"; + await rejects(firstClient.git.getTree({ owner: "other", repo: "first", tree_sha: "two" }), "github_app_access_required"); + firstVisibility = "private"; + await rejects(firstClient.git.getTree({ owner: "other", repo: "first", tree_sha: "three" }), "github_app_access_required"); + await secondClient.git.getTree({ owner: "other", repo: "second", tree_sha: "four" }); + expect(sent).to.have.length(2); + const scopes = calls.filter(call => call.url.endsWith("/token/scoped")); + expect(scopes.map(call => call.body.repository_ids)).to.deep.equal([[7], [8]]); + for (const request of scopes) { + expect(request.options.headers.Authorization).to.match(/^Basic /); + expect(request.body.target).to.equal("other"); + expect(Object.values(request.body.permissions).every(value => value === "read")).to.equal(true); + } + expect(githubQuotaKey(first)).to.equal(githubQuotaKey(second)); + }); + it("never returns the unrestricted user token when scoping fails", async () => { + await app.saveAppGrant(owner.id, data()); + for (const response of [{ status: 403 }, { token: "ghu_access1" }, {}]) { + app.clearAppTokenCache(); + mock(() => ({ id: 7, private: false, visibility: "public" }), () => response); + await rejects(app.boundAppToken(owner.id, { kind: "github-app", publicRead: true, repositoryId: 7, revision: "public" }, "other/public"), "github_app_access_required"); + } }); it("rejects a replacement at the original name for repository and PR source reads", async () => { await app.saveAppGrant(owner.id, data()); @@ -296,7 +337,7 @@ describeMongo("GitHub App credential and repository integration", function () { for (const resource of resources) await rejects(resource.getToken(), "github_app_access_required"); expect(calls.every(call => call.url.endsWith("/repos/other/original"))).to.equal(true); mock(() => ({ id: 7, private: false, visibility: "public" })); - for (const resource of resources) expect(await resource.getToken()).to.equal("ghu_access1"); + for (const resource of resources) expect(await resource.getToken()).to.equal("ghu_scoped_7_ghu_access1"); }); it("does not reinterpret missing or mixed installation bindings as public access", async () => { await app.saveAppGrant(owner.id, data());