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());