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