diff --git a/src/core/GitHubUtils.ts b/src/core/GitHubUtils.ts index 41ee6c3..e47f5ea 100644 --- a/src/core/GitHubUtils.ts +++ b/src/core/GitHubUtils.ts @@ -249,6 +249,17 @@ export function octokit(token: string) { }); if (context) { oct.hook.before("request", async options => { + if (context.publicRepository) { + const url = new URL(oct.request.endpoint(options).url); + const prefix = `/repos/${context.publicRepository.split("/").map(encodeURIComponent).join("/")}`.toLowerCase(); + const path = url.pathname.toLowerCase(); + const suffix = path.slice(prefix.length); + const readable = /^(?:\/?|\/branches(?:\/[^/]+)?|\/commits(?:\/[^/]+)?|\/readme|\/pages|\/zipball\/[^/]+|\/git\/(?:trees|blobs)\/[^/]+|\/pulls\/\d+|\/issues\/\d+\/comments)$/.test(suffix); + if (!readable || (options.method !== "GET" && !(options.method === "HEAD" && suffix.startsWith("/zipball/"))) || url.origin !== "https://api.github.com" || + (path !== prefix && !path.startsWith(prefix + "/"))) { + throw new AnonymousError("github_app_access_required", { httpStatus: 403 }); + } + } options.headers.authorization = `token ${await context.renew()}`; }); oct.hook.wrap("request", async (request, options) => { @@ -278,6 +289,8 @@ export { waitForTokenGate }; export async function checkToken(token: string) { const oct = octokit(token); try { + const context = githubTokenContext(token); + if (context?.publicRepository) { await context.renew(); return true; } if (token.startsWith("ghs_")) await oct.request("GET /installation/repositories"); else await oct.users.getAuthenticated(); return true; diff --git a/src/core/github-app.ts b/src/core/github-app.ts index 03aea59..0873a75 100644 --- a/src/core/github-app.ts +++ b/src/core/github-app.ts @@ -1,5 +1,5 @@ import { registerGitHubToken } from "./github-token-context"; -import { createHash, createSign, randomUUID } from "crypto"; +import { createSign, randomUUID } from "crypto"; import { readFileSync } from "fs"; import config from "../config"; import AnonymousError from "./AnonymousError"; @@ -180,7 +180,7 @@ export async function appRepositories(ownerId: string) { const installationTokens = new Map(); const minting = new Map>(); -export function clearAppTokenCache() { installationTokens.clear(); publicTokens.clear(); } +export function clearAppTokenCache() { installationTokens.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,54 +219,27 @@ async function installationToken(binding: RepositoryAccess, ownerId: string): Pr try { return await work; } finally { minting.delete(key); } } -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, force = false): 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 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( - `/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 || repo.visibility !== "public") throw appError("github_app_access_required"); - 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; + const renew = async () => { + const userToken = await appUserToken(ownerId); + const repo = await githubRequest( + `/repos/${sourceName.split("/").map(encodeURIComponent).join("/")}`, userToken); + // A public binding must not acquire private access or follow a replacement + // repository at an old name, even if the user can read it. + if (repo.id !== binding.repositoryId || repo.private !== false || repo.visibility !== "public") throw appError("github_app_access_required"); + return userToken; + }; + await renew(); + // This process-local handle keeps concurrent repositories independent without + // asking GitHub to mint an installation-scoped token for an uninstalled repo. + const handle = `public-read:${randomUUID()}`; + registerGitHubToken(handle, { quotaKey: `app-user:${ownerId}`, renew, publicRepository: sourceName }); + return handle; } + 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. diff --git a/src/core/github-token-context.ts b/src/core/github-token-context.ts index 3a801cb..3ea47ae 100644 --- a/src/core/github-token-context.ts +++ b/src/core/github-token-context.ts @@ -1,12 +1,16 @@ import { createHash } from "crypto"; -interface TokenContext { quotaKey: string; renew: (force?: boolean) => Promise; } +interface TokenContext { quotaKey: string; renew: (force?: boolean) => Promise; publicRepository?: string; } const contexts = new Map(); export function registerGitHubToken(token: string, context: TokenContext) { if (contexts.size >= 2000 && !contexts.has(token)) contexts.delete(contexts.keys().next().value!); contexts.set(token, context); } -export function githubTokenContext(token: string) { return contexts.get(token); } +export function githubTokenContext(token: string) { + const context = contexts.get(token); + if (!context && token.startsWith("public-read:")) throw new Error("Public repository access context expired"); + return context; +} export function githubQuotaKey(token: string) { return contexts.get(token)?.quotaKey || createHash("sha256").update(token).digest("hex").slice(0, 24); } diff --git a/src/core/source/GitHubStream.ts b/src/core/source/GitHubStream.ts index 4c7f7e6..06fe5ff 100644 --- a/src/core/source/GitHubStream.ts +++ b/src/core/source/GitHubStream.ts @@ -1,3 +1,4 @@ +import { githubTokenContext } from "../github-token-context"; import AnonymizedFile from "../AnonymizedFile"; import GitHubBase, { GitHubBaseData, @@ -79,10 +80,11 @@ export default class GitHubStream extends GitHubBase { }); logger.debug("downloading file", { url }); return got.stream(url, { + hooks: { beforeRequest: [async () => { await githubTokenContext(token)?.renew(); }] }, headers: { "X-GitHub-Api-Version": "2022-11-28", accept: "application/vnd.github.raw+json", - authorization: `token ${token}`, + ...(githubTokenContext(token)?.publicRepository ? {} : { authorization: `token ${token}` }), }, }); } catch (error) { @@ -108,7 +110,8 @@ export default class GitHubStream extends GitHubBase { ); logger.debug("downloading via raw URL (LFS)", { url }); return got.stream(url, { - headers: { authorization: `token ${token}` }, + hooks: { beforeRequest: [async () => { await githubTokenContext(token)?.renew(); }] }, + headers: githubTokenContext(token)?.publicRepository ? {} : { authorization: `token ${token}` }, followRedirect: true, }); } @@ -122,6 +125,11 @@ export default class GitHubStream extends GitHubBase { sha: string, filePath: string ): Promise { + // Public raw downloads need no bearer token and do not consume the + // unauthenticated REST API quota. GitHub also resolves LFS pointers here. + if (githubTokenContext(token)?.publicRepository) { + return Promise.resolve(this.downloadFileViaRaw(token, filePath)); + } return new Promise((resolve) => { const blobStream = this.downloadFile(token, sha); let settled = false; diff --git a/test/github-app.test.js b/test/github-app.test.js index 7745705..e4b3a45 100644 --- a/test/github-app.test.js +++ b/test/github-app.test.js @@ -15,7 +15,7 @@ const Users = require("../src/core/model/users/users.model").default; const Installations = require("../src/core/model/github-installation").default; const { getCredentialToken, setCredential } = require("../src/core/credentials"); const { verifyCredentials } = require("../src/core/migrate-credentials"); -const { registerGitHubToken, githubQuotaKey } = require("../src/core/github-token-context"); +const { registerGitHubToken, githubQuotaKey, githubTokenContext } = require("../src/core/github-token-context"); const keys = JSON.stringify({ test: Buffer.alloc(32, 9).toString("base64") }); const { privateKey, publicKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); const pem = privateKey.export({ type: "pkcs8", format: "pem" }); @@ -79,6 +79,32 @@ describe("GitHub App protocol boundaries", () => { } finally { globalThis.fetch = previousFetch; } }); + it("downloads public files through raw URLs without sending a bearer token", async () => { + const got = require("got"); + const originalStream = got.stream; + const { Readable } = require("stream"); + let captured; + let valid = true; + const handle = "public-read:download-test"; + registerGitHubToken(handle, { quotaKey: "app-user:download", publicRepository: "other/public", renew: async () => { + if (!valid) throw app.appError("github_app_access_required"); + return "ghu_private_credential"; + } }); + got.stream = (url, options) => { captured = { url, options }; return Readable.from(["file"]); }; + try { + const GitHubStream = require("../src/core/source/GitHubStream").default; + const source = Object.create(GitHubStream.prototype); + source.data = { organization: "other", repoName: "public", commit: "abc" }; + await source.downloadWithFallback(handle, "blob", "README.md"); + expect(captured.url).to.include("other/public"); + expect(captured.url).to.include("README.md"); + expect(captured.options.headers.authorization).to.equal(undefined); + await captured.options.hooks.beforeRequest[0](); + valid = false; + await rejects(captured.options.hooks.beforeRequest[0](), "github_app_access_required"); + } finally { got.stream = originalStream; } + }); + it("signs a short-lived App JWT with clock skew", () => { const previous = { enabled: config.GITHUB_APP_ENABLED, key: config.GITHUB_APP_PRIVATE_KEY, client: config.GITHUB_APP_CLIENT_ID }; Object.assign(config, { GITHUB_APP_ENABLED: true, GITHUB_APP_PRIVATE_KEY: pem, GITHUB_APP_CLIENT_ID: "Iv.test" }); @@ -147,7 +173,7 @@ describeMongo("GitHub App credential and repository integration", function () { const body = options?.body ? JSON.parse(options.body) : undefined; calls.push({ url: String(url), body, options }); 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() } + ? scopeHandler ? await scopeHandler(body, options) : { status: 403 } : await handler(String(url), options, body); return new globalThis.Response(JSON.stringify(result.body || result), { status: result.status || 200, headers: { "content-type": "application/json" } }); }; @@ -210,7 +236,8 @@ 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_scoped_7_ghu_access1"); + expect(selected.token).to.match(/^public-read:/); + expect(await githubTokenContext(selected.token).renew()).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; @@ -242,7 +269,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_scoped_7_ghu_access1"); + expect(await app.boundAppToken(owner.id, binding, "other/public")).to.match(/^public-read:/); 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"); @@ -255,7 +282,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_scoped_7_ghu_accessnew"); + .to.match(/^public-read:/); }); it("renews a public token inside an existing Octokit traversal without changing owner or quota", async () => { await app.saveAppGrant(owner.id, data("old")); @@ -275,9 +302,9 @@ 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_scoped_7_ghu_accessold", "token ghu_scoped_7_ghu_accessnew"]); + expect(sent).to.deep.equal(["token ghu_accessold", "token ghu_accessnew"]); expect(githubQuotaKey(token)).to.equal(`app-user:${owner.id}`); - expect(githubQuotaKey("ghu_scoped_7_ghu_accessnew")).to.equal(githubQuotaKey(token)); + expect(await githubTokenContext(token).renew()).to.equal("ghu_accessnew"); }); it("revalidates each public traversal independently after another repository binds the same user", async () => { await app.saveAppGrant(owner.id, data()); @@ -302,21 +329,23 @@ describeMongo("GitHub App credential and repository integration", function () { 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(scopes).to.have.length(0); expect(githubQuotaKey(first)).to.equal(githubQuotaKey(second)); }); - it("never returns the unrestricted user token when scoping fails", async () => { + it("does not require scoped-token minting for an uninstalled public repository", 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"); - } + mock(url => url.includes("/user/installations") ? { installations: [] } : + { id: 7, private: false, visibility: "public" }, () => ({ status: 403 })); + const selected = await app.selectRepositoryAccess(owner.id, "other/public", "github-app"); + expect(selected.token).to.match(/^public-read:/); + expect(calls.some(call => call.url.endsWith("/token/scoped"))).to.equal(false); + const client = require("../src/core/GitHubUtils").octokit(selected.token); + const before = calls.length; + await rejects(client.request("POST /repos/other/public/issues", { title: "must not write" }), "github_app_access_required"); + await rejects(client.request("GET /repos/other/private"), "github_app_access_required"); + await rejects(client.request("GET /repos/other/public/actions/secrets"), "github_app_access_required"); + await rejects(client.request("GET https://example.com/repos/other/public"), "github_app_access_required"); + expect(calls.length).to.equal(before); }); it("rejects a replacement at the original name for repository and PR source reads", async () => { await app.saveAppGrant(owner.id, data()); @@ -337,7 +366,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_scoped_7_ghu_access1"); + for (const resource of resources) expect(await resource.getToken()).to.match(/^public-read:/); }); it("does not reinterpret missing or mixed installation bindings as public access", async () => { await app.saveAppGrant(owner.id, data()); diff --git a/test/github-repository-pages.test.js b/test/github-repository-pages.test.js new file mode 100644 index 0000000..f9b6ab0 --- /dev/null +++ b/test/github-repository-pages.test.js @@ -0,0 +1,61 @@ +const { expect } = require("chai"); +require("ts-node/register/transpile-only"); +const db = require("../src/server/database"); +const gh = require("../src/core/GitHubUtils"); +const { getRepositoryFromGitHub } = require("../src/core/source/GitHubRepository"); + +describe("repository metadata with restricted Pages settings", function () { + let originalOctokit; + let originalConnected; + beforeEach(function () { + originalOctokit = gh.octokit; + originalConnected = db.isConnected; + db.isConnected = false; + }); + afterEach(function () { + gh.octokit = originalOctokit; + db.isConnected = originalConnected; + }); + + function mockPages(getPages) { + gh.octokit = () => ({ repos: { + get: async () => ({ data: { + id: 123, full_name: "ncusi/PatchScope", name: "PatchScope", + owner: { login: "ncusi" }, html_url: "https://github.com/ncusi/PatchScope", + default_branch: "main", has_pages: true, size: 42, + } }), + getPages, + } }); + } + const options = { owner: "ncusi", repo: "PatchScope", accessToken: "test", force: true }; + + for (const status of [403, 404]) { + it(`loads repository details when Pages settings return ${status}`, async function () { + mockPages(async () => { throw Object.assign(new Error("Pages unavailable"), { status }); }); + const repo = await getRepositoryFromGitHub({ ...options }); + expect(repo.toJSON()).to.include({ fullName: "ncusi/PatchScope", defaultBranch: "main", hasPage: true }); + expect(repo.toJSON().pageSource.branch).to.equal(undefined); + expect(repo.toJSON().pageSource.path).to.equal(undefined); + }); + } + + it("preserves the Pages source when settings are accessible", async function () { + const source = { branch: "main", path: "/docs" }; + mockPages(async args => { + expect(args).to.deep.equal({ owner: "ncusi", repo: "PatchScope" }); + return { data: { source } }; + }); + const repo = await getRepositoryFromGitHub({ ...options }); + expect(repo.toJSON().pageSource.toObject()).to.deep.equal(source); + }); + + for (const status of [401, 429, 500]) { + it(`propagates Pages failures with status ${status}`, async function () { + const failure = Object.assign(new Error("GitHub failure"), { status }); + mockPages(async () => { throw failure; }); + let caught; + try { await getRepositoryFromGitHub({ ...options }); } catch (error) { caught = error; } + expect(caught).to.equal(failure); + }); + } +});