From dc70a28919784f8046040e90b759893b1f1ceb8b Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 20:22:56 +0000 Subject: [PATCH 1/9] Authenticate API GitHub reads with the Integrations App --- .gitignore | 1 + package-lock.json | 7 + services/api/.dev.vars.example | 6 + services/api/AGENTS.md | 2 +- services/api/README.md | 44 ++++-- services/api/package.json | 13 +- services/api/src/lib/github-app.ts | 100 +++++++++++++ .../extensions/v2/db/developer-claims.ts | 7 +- .../extensions/v2/db/developer-profiles.ts | 18 ++- .../src/services/extensions/v2/email/types.ts | 2 +- .../services/extensions/v2/github/identity.ts | 7 +- .../extensions/v2/github/verification.ts | 5 +- .../v2/routes/developer-profiles.ts | 5 +- .../extensions/v2/routes/ownership.ts | 3 +- .../api/src/services/previews/v1/README.md | 4 +- .../services/previews/v1/github/artifacts.ts | 15 +- .../services/previews/v1/github/request.ts | 11 +- services/api/src/services/stats/v1/index.ts | 10 +- .../api/src/services/versions/v1/README.md | 2 +- .../api/src/services/versions/v1/index.ts | 11 +- services/api/test/integration/app.test.ts | 4 + .../api/test/integration/github-app.test.ts | 83 +++++++++++ .../test/integration/versions/index.test.ts | 4 + services/api/test/lib/github-app.test.ts | 135 ++++++++++++++++++ .../services/extensions/v2/account.test.ts | 4 + .../services/extensions/v2/badges.test.ts | 4 + .../extensions/v2/claim-budget.test.ts | 4 + .../services/extensions/v2/contract.test.ts | 4 + .../extensions/v2/developer-approval.test.ts | 4 + .../extensions/v2/developer-profiles.test.ts | 4 + .../extensions/v2/directory-identity.test.ts | 4 + .../extensions/v2/extension-writes.test.ts | 4 + .../test/services/extensions/v2/index.test.ts | 4 + .../services/extensions/v2/manifest.test.ts | 4 + .../extensions/v2/moderation-correct.test.ts | 4 + .../extensions/v2/moderation-notify.test.ts | 4 + .../v2/moderation-revalidate.test.ts | 4 + .../v2/moderation-revocation.test.ts | 4 + .../services/extensions/v2/moderation.test.ts | 4 + .../services/extensions/v2/ownership.test.ts | 4 + .../services/extensions/v2/packages.test.ts | 4 + .../extensions/v2/pending-edits.test.ts | 4 + .../extensions/v2/public-extensions.test.ts | 4 + .../services/extensions/v2/publisher.test.ts | 4 + .../extensions/v2/resource-bounds.test.ts | 4 + .../test/services/previews/v1/budget.test.ts | 14 +- .../test/services/previews/v1/commit.test.ts | 4 + .../test/services/previews/v1/main.test.ts | 4 + .../api/test/services/previews/v1/pr.test.ts | 4 + .../api/test/services/stats/v1/index.test.ts | 4 + .../test/services/versions/v1/errors.test.ts | 4 + .../test/services/versions/v1/index.test.ts | 4 + .../services/versions/v1/middleware.test.ts | 4 + services/api/vitest.config.ts | 1 - services/api/vitest.node.config.ts | 3 +- services/api/worker-configuration.d.ts | 10 +- 56 files changed, 571 insertions(+), 68 deletions(-) create mode 100644 services/api/.dev.vars.example create mode 100644 services/api/src/lib/github-app.ts create mode 100644 services/api/test/integration/github-app.test.ts create mode 100644 services/api/test/lib/github-app.test.ts diff --git a/.gitignore b/.gitignore index 06026e2b..d8a11f28 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ node_modules/ .env.production .dev.vars .dev.vars.* +!services/api/.dev.vars.example # logs logs/ diff --git a/package-lock.json b/package-lock.json index 8f563ec7..86b27ef0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5830,6 +5830,12 @@ "pathe": "^2.0.3" } }, + "node_modules/universal-github-app-jwt": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/universal-github-app-jwt/-/universal-github-app-jwt-2.2.2.tgz", + "integrity": "sha512-dcmbeSrOdTnsjGjUfAlqNDJrhxXizjAz94ija9Qw8YkZ1uu0d+GoZzyH+Jb9tIIqvGsadUfwg+22k5aDqqwzbw==", + "license": "MIT" + }, "node_modules/universal-user-agent": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/universal-user-agent/-/universal-user-agent-7.0.3.tgz", @@ -6661,6 +6667,7 @@ "drizzle-orm": "0.45.4", "hono": "4.13.17", "semver": "7.8.6", + "universal-github-app-jwt": "2.2.2", "uuid": "14.0.3", "zod": "4.6.7" }, diff --git a/services/api/.dev.vars.example b/services/api/.dev.vars.example new file mode 100644 index 00000000..e64e422e --- /dev/null +++ b/services/api/.dev.vars.example @@ -0,0 +1,6 @@ +GITHUB_APP_CLIENT_ID="your-client-id" +GITHUB_APP_INSTALLATION_ID="your-installation-id" +GITHUB_APP_PRIVATE_KEY="-----BEGIN PRIVATE KEY-----\n...\n-----END PRIVATE KEY-----" +ASSERTION_SIGNING_SECRET="local-shared-secret" +EXTENSIONS_REVALIDATE_SECRET="local-revalidate-secret" +EXTENSIONS_V2_MXROUTE_PASSWORD="local-mailbox-password" diff --git a/services/api/AGENTS.md b/services/api/AGENTS.md index 0f118e90..4d44bd0f 100644 --- a/services/api/AGENTS.md +++ b/services/api/AGENTS.md @@ -65,7 +65,7 @@ Each service documents its own contract and operational detail in its own `READM ## Configuration & Secrets -- Local secrets go in `.dev.vars` (for example `GITHUB_TOKEN="..."`). +- Local secrets go in `.dev.vars` (for example `GITHUB_APP_PRIVATE_KEY="..."`). - Bindings, cron, and vars are defined in `wrangler.jsonc` (D1, KV, R2, Durable Objects, rate limiters, service bindings); keep names aligned with `CloudflareBindings`. - Use Wrangler secrets for production tokens instead of committing them. - `ASSERTION_SIGNING_SECRET`: HMAC key the extensions v2 API uses to verify short-lived bearer assertions minted by the extensions site (`src/lib/auth/bearer-assertion.ts`). Not sent over the wire — only signs/verifies server-side in each Worker. Add `ASSERTION_SIGNING_SECRET="..."` to `.dev.vars` for local dev; set via `wrangler secret put ASSERTION_SIGNING_SECRET` in production, matching the value configured in the extensions site's Worker. diff --git a/services/api/README.md b/services/api/README.md index 133df719..5efe0f62 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -97,7 +97,9 @@ For local development, put operational values and credentials in the ignored `.dev.vars` file. Document variable names and placeholders, not production values. Package storage configuration is listed in [PACKAGING.md](src/services/extensions/v2/PACKAGING.md). -- `GITHUB_TOKEN`: A GitHub Personal Access Token (classic) with public repo read access. +- `GITHUB_APP_CLIENT_ID`: Client ID of the FOSSBilling Integrations GitHub App. +- `GITHUB_APP_INSTALLATION_ID`: Its installation ID in the FOSSBilling organization. +- `GITHUB_APP_PRIVATE_KEY`: App signing key (PEM), stored as a secret. Use PKCS#8 for portability across runtimes; real newlines or escaped `\n` are accepted. - `ASSERTION_SIGNING_SECRET`: Shared HMAC secret used to verify the short-lived bearer assertions minted by the Extensions site. Configure the same value in both Workers; it is never sent to clients. - `ASSERTION_SIGNING_SECRET_PREVIOUS`: Optional previous HMAC secret accepted during a signing-key rotation. - `EXTENSIONS_REVALIDATE_SECRET`: Bearer token the API sends to the Extensions site's `POST /api/revalidate` (over the `EXTENSIONS_FRONTEND` service binding) to purge the site's CDN-cached extension pages after content mutations. Configure the same value in both Workers; it is never sent to clients. @@ -111,21 +113,43 @@ Package storage configuration is listed in [PACKAGING.md](src/services/extension - `M2M_ISSUER`, `M2M_JWKS_URL`, `M2M_AUDIENCE`: Issuer, JWKS endpoint, and expected audience for machine-to-machine access tokens minted by the central auth service (`FOSSBilling/auth`). Defaults live in `wrangler.jsonc`; preview environments need them pointed at the issuing preview. See [extensions v2 README](src/services/extensions/v2/README.md#machine-publishing). The extension-specific settings apply to extensions v2; Versions, Previews and -Stats also use `GITHUB_TOKEN`. For the assertion format and the rotation procedure, see [its README](src/services/extensions/v2/README.md#authentication). +Stats also use the GitHub App credentials. For the assertion format and the rotation procedure, see [its README](src/services/extensions/v2/README.md#authentication). + +### GitHub App authentication + +Install **FOSSBilling Integrations** on `FOSSBilling/FOSSBilling` with repository +**Contents**, **Actions** and **Pull requests** permissions set to **Read-only** +(Metadata read is automatic). Contents covers releases and compatibility files; +Actions covers preview artifacts and runs; Pull requests resolves preview heads. +Developer checks read public GitHub profiles; identity and organization ownership +still come from the caller's linked identity in the auth service. + +Use a dedicated API private key so it can be rotated independently of other +Integrations consumers; separate keys still have the same App authority. + +Set the three `GITHUB_APP_*` values as runtime variables/secrets in each deployment +that needs GitHub access, and in `services/api/.dev.vars` locally. Do not use build +variables or a static installation token. If the signing key is PKCS#1, convert it +once with `openssl pkcs8 -topk8 -nocrypt -in app.pem -out app.pkcs8.pem` before +storing it. Approve the installation's permission changes before deployment. + +The API mints installation tokens restricted to `FOSSBilling/FOSSBilling` and those +three read permissions, and caches resolved tokens in memory until one minute +before expiry. Key/configuration changes invalidate that cache. Authentication is +lazy: cached responses and profile operations without a GitHub lookup do not mint +tokens. Authentication failures use the existing unavailable/stale-cache paths; +they never become a missing GitHub user or anonymous lookup. There are no runtime +calls to a secrets manager. Remove the old runtime `GITHUB_TOKEN` after setting the +App credentials; GitHub Actions' built-in `GITHUB_TOKEN` is unrelated. ## Development ### Local Setup -1. Create a `.dev.vars` file for local runtime variables and secrets: +1. Copy `services/api/.dev.vars.example` to `services/api/.dev.vars` and set your runtime variables and secrets: - ```env - GITHUB_TOKEN="your-token" - ASSERTION_SIGNING_SECRET="local-shared-secret" - EXTENSIONS_REVALIDATE_SECRET="local-revalidate-secret" - # Optional while rotating the shared assertion secret. - # ASSERTION_SIGNING_SECRET_PREVIOUS="previous-local-shared-secret" - ``` + The example contains placeholders only. `cf-typegen` uses it to generate stable + binding types without reading local credentials. 2. Apply migrations to the local D1 databases: diff --git a/services/api/package.json b/services/api/package.json index a50a79e0..c44ed681 100644 --- a/services/api/package.json +++ b/services/api/package.json @@ -5,7 +5,7 @@ "type": "module", "license": "AGPL-3.0-only", "scripts": { - "cf-typegen": "wrangler types --env-interface CloudflareBindings", + "cf-typegen": "wrangler types --env-interface CloudflareBindings --env-file .dev.vars.example", "cf-deploy": "npm run db:migrate:all:remote && wrangler deploy --minify", "dev": "wrangler dev", "db:generate:extensions": "drizzle-kit generate --config drizzle.extensions.config.ts", @@ -25,6 +25,8 @@ "cf-preview": "wrangler preview --config dist/wrangler.json" }, "dependencies": { + "@fossbilling/extension-packaging": "*", + "@fossbilling/logging": "*", "@hono/zod-openapi": "1.6.3", "@octokit/graphql": "9.0.5", "@octokit/request": "10.0.16", @@ -34,16 +36,15 @@ "drizzle-orm": "0.45.4", "hono": "4.13.17", "semver": "7.8.6", + "universal-github-app-jwt": "2.2.2", "uuid": "14.0.3", - "zod": "4.6.7", - "@fossbilling/extension-packaging": "*", - "@fossbilling/logging": "*" + "zod": "4.6.7" }, "devDependencies": { "@cloudflare/vitest-plugin": "1.6.1", "@types/semver": "7.8.0", + "@zip.js/zip.js": "2.24.0", "drizzle-kit": "0.31.11", - "wrangler": "4.158.0", - "@zip.js/zip.js": "2.24.0" + "wrangler": "4.158.0" } } diff --git a/services/api/src/lib/github-app.ts b/services/api/src/lib/github-app.ts new file mode 100644 index 00000000..bc8f2907 --- /dev/null +++ b/services/api/src/lib/github-app.ts @@ -0,0 +1,100 @@ +import githubAppJwt from "universal-github-app-jwt"; +import type { PlatformContext } from "./context"; +import { GitHubError } from "./github-errors"; + +export type GitHubTokenProvider = () => Promise; + +// Cache resolved credentials only: an in-flight fetch belongs to its request +// context in Workers. Keep just the active configuration so key rotation also +// invalidates the cached token. +let cached: + | { + clientId: string; + installationId: string; + privateKey: string; + token: string; + expiresAt: number; + } + | undefined; + +export function githubTokenProvider( + platform: Pick +): GitHubTokenProvider { + let pending: Promise | undefined; + return () => (pending ??= installationToken(platform)); +} + +async function installationToken( + platform: Pick +): Promise { + const clientId = platform.getEnv("GITHUB_APP_CLIENT_ID"); + const installationId = platform.getEnv("GITHUB_APP_INSTALLATION_ID"); + const privateKey = platform.getEnv("GITHUB_APP_PRIVATE_KEY"); + if ( + !clientId || + !installationId || + !/^\d+$/.test(installationId) || + !privateKey + ) { + throw new GitHubError("GitHub App credentials are not configured", 503); + } + if ( + cached?.clientId === clientId && + cached.installationId === installationId && + cached.privateKey === privateKey && + cached.expiresAt > Date.now() + 60_000 + ) { + return cached.token; + } + + try { + const { token: jwt } = await githubAppJwt({ id: clientId, privateKey }); + const response = await fetch( + `https://api.github.com/app/installations/${installationId}/access_tokens`, + { + method: "POST", + signal: AbortSignal.timeout(10_000), + headers: { + authorization: `Bearer ${jwt}`, + accept: "application/vnd.github+json", + "content-type": "application/json", + "user-agent": "FOSSBilling-API" + }, + body: JSON.stringify({ + repositories: ["FOSSBilling"], + permissions: { + contents: "read", + actions: "read", + pull_requests: "read" + } + }) + } + ); + if (!response.ok) throw new Error("Token request failed"); + const data = (await response.json()) as { + token?: unknown; + expires_at?: unknown; + }; + const expiresAt = + typeof data.expires_at === "string" ? Date.parse(data.expires_at) : NaN; + if ( + typeof data.token !== "string" || + !data.token || + !(expiresAt > Date.now() + 60_000) + ) { + throw new Error("Invalid token response"); + } + cached = { + clientId, + installationId, + privateKey, + token: data.token, + expiresAt + }; + return data.token; + } catch { + // Never expose the signing key, JWT or GitHub's request/response objects in + // the callers' logs. A token-endpoint 404 must not become an entity 404. + throw new GitHubError("GitHub App authentication is unavailable", 503); + } +} diff --git a/services/api/src/services/extensions/v2/db/developer-claims.ts b/services/api/src/services/extensions/v2/db/developer-claims.ts index d9926c4b..2e7517f6 100644 --- a/services/api/src/services/extensions/v2/db/developer-claims.ts +++ b/services/api/src/services/extensions/v2/db/developer-claims.ts @@ -1,3 +1,4 @@ +import type { GitHubTokenProvider } from "../../../../lib/github-app"; import { and, asc, desc, eq, gt, lt, or, sql, SQL } from "drizzle-orm"; import { DatabaseResult } from "../../../../lib/interfaces"; import { ExtensionsDb } from "../../../../lib/db"; @@ -119,8 +120,8 @@ export class DeveloperClaimsDatabase { async claim( identifier: string, claimantId: string, - note?: string, - githubToken?: string + note: string | undefined, + getGithubToken: GitHubTokenProvider ): Promise> { try { const [developer] = await this.db @@ -182,7 +183,7 @@ export class DeveloperClaimsDatabase { developer.identifier, developer.type as Developer["type"], claimantId, - githubToken + getGithubToken ); if ("error" in check) { diff --git a/services/api/src/services/extensions/v2/db/developer-profiles.ts b/services/api/src/services/extensions/v2/db/developer-profiles.ts index 814e7659..bd5510c9 100644 --- a/services/api/src/services/extensions/v2/db/developer-profiles.ts +++ b/services/api/src/services/extensions/v2/db/developer-profiles.ts @@ -1,3 +1,4 @@ +import type { GitHubTokenProvider } from "../../../../lib/github-app"; import { v7 as uuidv7 } from "uuid"; import { and, asc, desc, eq, gt, isNull, lt, or, sql, SQL } from "drizzle-orm"; import { DatabaseResult } from "../../../../lib/interfaces"; @@ -158,7 +159,7 @@ export class DeveloperProfilesDatabase { } } - // githubToken — see the comment on verifyGithubOwnership(). Only consulted + // getGithubToken — see the comment on verifyGithubOwnership(). Only consulted // when creating a brand-new profile (the identifier is immutable once // owned, so an update can't need re-verifying); guards against squatting // on an id that matches a real GitHub org/user the caller doesn't control, @@ -167,7 +168,7 @@ export class DeveloperProfilesDatabase { async upsertOwn( userId: string, developer: Developer, - githubToken?: string, + getGithubToken: GitHubTokenProvider, allowCreationAttempt: () => Promise = async () => true ): Promise & { changed?: boolean }> { try { @@ -230,7 +231,7 @@ export class DeveloperProfilesDatabase { developer.identifier, developer.type, userId, - githubToken, + getGithubToken, developer.URL ); @@ -1010,15 +1011,15 @@ export class DeveloperProfilesDatabase { // own already-synced github_login/github_orgs. Called opportunistically // on every login for a developer-owning user, and by the owner's own // "Re-verify" action — both share this one method. - // checkUrl/githubToken — only set by the owner's own manual "Re-verify" + // checkUrl/getGithubToken — only set by the owner's own manual "Re-verify" // button, never by the opportunistic per-login call in extensions' // auth/callback.ts. Re-checking Publisher URL against GitHub's on-file // website needs a fresh GitHub API call (unlike the identity match below), // so it stays opt-in to keep the automatic login path GitHub-API-free. async reverifyOwn( userId: string, - checkUrl?: boolean, - githubToken?: string + checkUrl: boolean, + getGithubToken: GitHubTokenProvider ): Promise> { try { const [row] = await this.db @@ -1143,10 +1144,7 @@ export class DeveloperProfilesDatabase { if (!matches) { writeUrlVerified = true; } else if (checkUrl) { - const entity = await checkGithubEntity( - row.identifier, - githubToken ?? "" - ); + const entity = await checkGithubEntity(row.identifier, getGithubToken); // An unavailable lookup is explicitly inconclusive, not a disproof, // so it leaves the stored URL verification signal untouched rather // than clearing a real prior verification over a transient failure. diff --git a/services/api/src/services/extensions/v2/email/types.ts b/services/api/src/services/extensions/v2/email/types.ts index 0e16130a..17269a8a 100644 --- a/services/api/src/services/extensions/v2/email/types.ts +++ b/services/api/src/services/extensions/v2/email/types.ts @@ -21,7 +21,7 @@ export const defaultFetch: FetchFn = (...args) => globalThis.fetch(...args); // Minimal env surface the email subsystem needs. Shaped to match // PlatformContext, so routes pass getPlatform(c) directly — the same way -// GITHUB_TOKEN and the assertion secrets are read everywhere else. +// GitHub App credentials and the assertion secrets are read everywhere else. export interface EnvReader { getEnv(key: string): string | undefined; } diff --git a/services/api/src/services/extensions/v2/github/identity.ts b/services/api/src/services/extensions/v2/github/identity.ts index a505d00c..3c391b67 100644 --- a/services/api/src/services/extensions/v2/github/identity.ts +++ b/services/api/src/services/extensions/v2/github/identity.ts @@ -1,3 +1,4 @@ +import type { GitHubTokenProvider } from "../../../../lib/github-app"; import { DatabaseError } from "../../../../lib/interfaces"; import { ExtensionsDb } from "../../../../lib/db"; import { @@ -39,7 +40,7 @@ export function githubUnavailableError( }; } -// `githubToken` authenticates the GitHub entity-existence lookup only (a +// `getGithubToken` authenticates the GitHub entity-existence lookup only (a // service-level credential, raises the public rate limit) — it is never // the claimant's own token, which never leaves the auth service. Shared by // claim() and upsertOwn(): both need the same question answered — does a @@ -60,7 +61,7 @@ export async function verifyGithubOwnership( developerId: string, developerType: Developer["type"], callerId: string, - githubToken?: string, + getGithubToken: GitHubTokenProvider, publisherUrl?: string ): Promise { // The D1 identity read and the GitHub entity lookup are independent - @@ -69,7 +70,7 @@ export async function verifyGithubOwnership( // leave it to finish (it never rejects: DatabaseResult methods classify // their own errors). const identityLookup = new UsersDatabase(db).getGithubIdentity(callerId); - const githubEntity = await checkGithubEntity(developerId, githubToken ?? ""); + const githubEntity = await checkGithubEntity(developerId, getGithubToken); if (githubEntity.status === "unavailable") { return { error: githubUnavailableError(githubEntity.reason) }; diff --git a/services/api/src/services/extensions/v2/github/verification.ts b/services/api/src/services/extensions/v2/github/verification.ts index 5f49fd29..6a7bc800 100644 --- a/services/api/src/services/extensions/v2/github/verification.ts +++ b/services/api/src/services/extensions/v2/github/verification.ts @@ -1,3 +1,4 @@ +import type { GitHubTokenProvider } from "../../../../lib/github-app"; import { request as ghRequest } from "@octokit/request"; import { AuthError, @@ -71,12 +72,12 @@ function redactedFailureMessage(message: string): string { // and invalid data so callers never mistake an outage for a missing account. export async function checkGithubEntity( id: string, - githubToken: string + getGithubToken: GitHubTokenProvider ): Promise { try { const result = await ghRequest("GET /users/{username}", { username: id, - headers: githubToken ? { authorization: `Bearer ${githubToken}` } : {} + headers: { authorization: `Bearer ${await getGithubToken()}` } }); if ( typeof result.data !== "object" || diff --git a/services/api/src/services/extensions/v2/routes/developer-profiles.ts b/services/api/src/services/extensions/v2/routes/developer-profiles.ts index 216d3386..93d9b421 100644 --- a/services/api/src/services/extensions/v2/routes/developer-profiles.ts +++ b/services/api/src/services/extensions/v2/routes/developer-profiles.ts @@ -1,3 +1,4 @@ +import { githubTokenProvider } from "../../../../lib/github-app"; import { getOptionalAuth, optionalAuth, @@ -194,7 +195,7 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { const { data, error, changed } = await db.upsertOwn( auth.userId, body, - platform.getEnv("GITHUB_TOKEN"), + githubTokenProvider(platform), // Keep this as a callback so upsertOwn can run its cheap // existing-profile/id checks first; updates and known-taken ids must not // spend creation allowance. @@ -318,7 +319,7 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { const { data, error } = await db.reverifyOwn( auth.userId, check_url, - platform.getEnv("GITHUB_TOKEN") + githubTokenProvider(platform) ); if (error || !data) { const status = diff --git a/services/api/src/services/extensions/v2/routes/ownership.ts b/services/api/src/services/extensions/v2/routes/ownership.ts index e3f8b8c4..4408cdcc 100644 --- a/services/api/src/services/extensions/v2/routes/ownership.ts +++ b/services/api/src/services/extensions/v2/routes/ownership.ts @@ -1,3 +1,4 @@ +import { githubTokenProvider } from "../../../../lib/github-app"; import { requireActiveAuth, requireModerator } from "../middleware"; import { getExtensionsDb } from "../../../../lib/db"; import { getPlatform } from "../../../../lib/middleware"; @@ -93,7 +94,7 @@ export function registerOwnershipRoutes(app: ExtensionsV2App): void { id, auth.userId, note, - platform.getEnv("GITHUB_TOKEN") + githubTokenProvider(platform) ); if (error || !data) { return c.json( diff --git a/services/api/src/services/previews/v1/README.md b/services/api/src/services/previews/v1/README.md index 8b37e471..d705893c 100644 --- a/services/api/src/services/previews/v1/README.md +++ b/services/api/src/services/previews/v1/README.md @@ -95,8 +95,8 @@ Endpoints are not listed here. The service publishes its own contract: resolve GitHub's signed redirect URL live, never cached - it expires in about a minute, and Cloudflare KV's 60s minimum TTL leaves no safe margin to cache it without risking handing out an already-expired URL. -- `GITHUB_TOKEN` is required for GitHub API access (shared with - `versions/v1`). +- GitHub App credentials are required for GitHub reads (shared with + `versions/v1`); see [API configuration](../../../../README.md#github-app-authentication). - `DOWNLOAD_BUCKET` (R2 binding) backs `/main` - see `wrangler.jsonc` for the bucket this points at and why. diff --git a/services/api/src/services/previews/v1/github/artifacts.ts b/services/api/src/services/previews/v1/github/artifacts.ts index 24818d5d..ee62069a 100644 --- a/services/api/src/services/previews/v1/github/artifacts.ts +++ b/services/api/src/services/previews/v1/github/artifacts.ts @@ -89,8 +89,7 @@ async function listArtifacts( repo: REPO_NAME, ...(name ? { name } : {}), per_page: 100, - page, - headers: { Authorization: `Bearer ${github.token}` } + page } ); return result.data.artifacts as RawArtifact[]; @@ -156,8 +155,7 @@ async function listRunArtifacts( owner: REPO_OWNER, repo: REPO_NAME, run_id: runId, - per_page: 100, - headers: { Authorization: `Bearer ${github.token}` } + per_page: 100 } ); return result.data.artifacts as RawArtifact[]; @@ -187,8 +185,7 @@ async function findArtifactByRunHeadSha( repo: REPO_NAME, head_sha: shaLower, per_page: 100, - page, - headers: { Authorization: `Bearer ${github.token}` } + page } ); const runs = (result.data.workflow_runs ?? []) as Array<{ @@ -322,8 +319,7 @@ export async function resolvePullRequestHeadSha( { owner: REPO_OWNER, repo: REPO_NAME, - pull_number: prNumber, - headers: { Authorization: `Bearer ${github.token}` } + pull_number: prNumber } ); return { status: "found", data: result.data.head.sha }; @@ -351,8 +347,7 @@ export async function getArtifactDownloadUrl( repo: REPO_NAME, artifact_id: artifactId, archive_format: "zip", - request: { redirect: "manual" }, - headers: { Authorization: `Bearer ${github.token}` } + request: { redirect: "manual" } } ); diff --git a/services/api/src/services/previews/v1/github/request.ts b/services/api/src/services/previews/v1/github/request.ts index 0a2f4bd4..6258e29a 100644 --- a/services/api/src/services/previews/v1/github/request.ts +++ b/services/api/src/services/previews/v1/github/request.ts @@ -1,10 +1,12 @@ +import { githubTokenProvider } from "../../../../lib/github-app"; import { Context } from "hono"; import { request } from "@octokit/request"; import { GitHubError } from "../../../../lib/github-errors"; +import { getPlatform } from "../../../../lib/middleware"; import { sha256Hex } from "../../../../lib/hash"; export interface PreviewGitHub { - token: string; + getToken: () => Promise; reserve: () => Promise; } @@ -64,7 +66,7 @@ export function previewGitHub( const identity = clientBudgetIdentity(c.req.header("CF-Connecting-IP")); let remaining = MAX_REQUEST_SUBREQUESTS; return { - token: c.env.GITHUB_TOKEN, + getToken: githubTokenProvider(getPlatform(c)), reserve: async () => { if (remaining === 0) return false; // Consume before awaiting so concurrent calls cannot exceed the ceiling. @@ -100,5 +102,8 @@ export async function previewRequest( "preview_budget_exhausted" ); } - return request(route, parameters); + return request(route, { + ...parameters, + headers: { authorization: `Bearer ${await github.getToken()}` } + }); } diff --git a/services/api/src/services/stats/v1/index.ts b/services/api/src/services/stats/v1/index.ts index 2becccfb..99cea171 100644 --- a/services/api/src/services/stats/v1/index.ts +++ b/services/api/src/services/stats/v1/index.ts @@ -1,3 +1,7 @@ +import { + githubTokenProvider, + type GitHubTokenProvider +} from "../../../lib/github-app"; import { Hono } from "hono"; import { cors } from "hono/cors"; import { etag } from "hono/etag"; @@ -120,7 +124,7 @@ function aggregateStats(releases: Releases): StatsData { async function getStats( cache: ICache, - githubToken: string, + getGithubToken: GitHubTokenProvider, downloadBucket: R2Bucket, waitUntil?: (promise: Promise) => void ): Promise<{ @@ -146,7 +150,7 @@ async function getStats( // overwrite that cache with GitHub-only URLs for up to a day. releases = await getReleases( cache, - githubToken, + getGithubToken, downloadBucket, false, waitUntil @@ -223,7 +227,7 @@ registerCachedRoute("/data", async (c) => { const platform = getPlatform(c); const result = await getStats( platform.getCache("CACHE_KV"), - platform.getEnv("GITHUB_TOKEN") || "", + githubTokenProvider(platform), c.env.DOWNLOAD_BUCKET, platform.waitUntil ); diff --git a/services/api/src/services/versions/v1/README.md b/services/api/src/services/versions/v1/README.md index ee5b0714..377a5a00 100644 --- a/services/api/src/services/versions/v1/README.md +++ b/services/api/src/services/versions/v1/README.md @@ -141,7 +141,7 @@ When GitHub is unavailable and no cached data exists: - All responses include a `stale` field that is `true` when cached data is served after a failed fetch. - `details` includes the GitHub HTTP status and error code when available. -- `GITHUB_TOKEN` is required for GitHub API access. +- GitHub App credentials are required for uncached GitHub reads; see [API configuration](../../../../README.md#github-app-authentication). - `DOWNLOAD_BUCKET` (R2 binding, shared with `previews/v1`) backs `download_url`/`digest` below. - Releases before 0.5.0 read `src/composer.json`; newer releases use `composer.json`. - `download_url` is `download.fossbilling.org` when the release has been mirrored to R2 and the requesting client's own version trusts that host (see `Update::$allowedDownloadPrefixes` in FOSSBilling/FOSSBilling) - github.com has no AAAA record, so IPv6-only hosts can't reach a GitHub asset URL. It falls back to the GitHub asset URL otherwise. Mirroring began at 0.8.0, so releases before that are never looked up in R2 at all. diff --git a/services/api/src/services/versions/v1/index.ts b/services/api/src/services/versions/v1/index.ts index e0d16abe..95d3cc49 100644 --- a/services/api/src/services/versions/v1/index.ts +++ b/services/api/src/services/versions/v1/index.ts @@ -1,3 +1,7 @@ +import { + githubTokenProvider, + type GitHubTokenProvider +} from "../../../lib/github-app"; import { bearerAuth } from "hono/bearer-auth"; import { Hono, type Context, type Handler, type MiddlewareHandler } from "hono"; import { cors } from "hono/cors"; @@ -173,7 +177,7 @@ async function loadReleases( const platform = getPlatform(c); return getReleases( platform.getCache("CACHE_KV"), - platform.getEnv("GITHUB_TOKEN") || "", + githubTokenProvider(platform), c.env.DOWNLOAD_BUCKET, updateCache, platform.waitUntil @@ -302,7 +306,7 @@ versionsV1.get( const platform = getPlatform(c); const result = await getReleases( platform.getCache("CACHE_KV"), - platform.getEnv("GITHUB_TOKEN") || "", + githubTokenProvider(platform), c.env.DOWNLOAD_BUCKET, true // No waitUntil: /update's contract is "refresh the cache", so the @@ -484,7 +488,7 @@ export function resetUpdateTokenCache() { export async function getReleases( cache: ICache, - githubToken: string, + getGithubToken: GitHubTokenProvider, downloadBucket: R2Bucket, updateCache: boolean = false, waitUntil?: (promise: Promise) => void @@ -515,6 +519,7 @@ export async function getReleases( // per isolate instead of stampeding GitHub with N copies of it. return singleFlight("versions:releases", async () => { try { + const githubToken = await getGithubToken(); const result = await ghRequest("GET /repos/{owner}/{repo}/releases", { owner: REPO_OWNER, repo: REPO_NAME, diff --git a/services/api/test/integration/app.test.ts b/services/api/test/integration/app.test.ts index 8a042670..ec4b7cbe 100644 --- a/services/api/test/integration/app.test.ts +++ b/services/api/test/integration/app.test.ts @@ -20,6 +20,10 @@ import { VersionInfo } from "../utils/test-types"; +vi.mock("../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/integration/github-app.test.ts b/services/api/test/integration/github-app.test.ts new file mode 100644 index 00000000..e29c2524 --- /dev/null +++ b/services/api/test/integration/github-app.test.ts @@ -0,0 +1,83 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { + createExecutionContext, + waitOnExecutionContext +} from "cloudflare:test"; +import { env } from "cloudflare:workers"; +import { request } from "@octokit/request"; +import app from "../../src/app/index"; +import { githubTokenProvider } from "../../src/lib/github-app"; +import { checkGithubEntity } from "../../src/services/extensions/v2/github/verification"; +import { isolateEdgeCache } from "../utils/isolate-edge-cache"; +import { suppressConsole } from "../utils/mock-helpers"; + +vi.mock("@octokit/request", async () => + (await import("../mocks/octokit")).octokitRequestMock() +); + +const bindings = { + ...env, + GITHUB_APP_CLIENT_ID: "", + GITHUB_APP_INSTALLATION_ID: "", + GITHUB_APP_PRIVATE_KEY: "" +}; +const resetEdgeCache = isolateEdgeCache(); +let restoreConsole: () => void; + +beforeEach(async () => { + restoreConsole = suppressConsole(); + vi.clearAllMocks(); + resetEdgeCache(); + await env.CACHE_KV.delete("gh-fossbilling-releases"); +}); +afterEach(() => restoreConsole()); + +async function releases() { + const ctx = createExecutionContext(); + const response = await app.request("/versions/v1", {}, bindings, ctx); + await waitOnExecutionContext(ctx); + return response; +} + +it("serves the release cache even when App credentials are unavailable", async () => { + await env.CACHE_KV.put( + "gh-fossbilling-releases", + JSON.stringify({ + "1.0.0": { + version: "1.0.0", + released_on: "2026-01-01T00:00:00Z", + minimum_php_version: "8.3", + download_url: "https://example.com/release.zip", + size_bytes: 100, + is_prerelease: false, + github_release_id: 1, + changelog: "Release notes", + mirror_download_url: null, + mirror_digest: null + } + }) + ); + const response = await releases(); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + result: { "1.0.0": { version: "1.0.0" } } + }); + expect(request).not.toHaveBeenCalled(); +}); + +it("returns unavailable on an uncached release lookup without App credentials", async () => { + const response = await releases(); + expect(response.status).toBe(503); + expect(request).not.toHaveBeenCalled(); +}); + +it("does not turn an authentication outage into a missing developer", async () => { + const result = await checkGithubEntity( + "fossbilling", + githubTokenProvider({ + getEnv: () => undefined + }) + ); + expect(result.status).toBe("unavailable"); + expect(request).not.toHaveBeenCalled(); +}); diff --git a/services/api/test/integration/versions/index.test.ts b/services/api/test/integration/versions/index.test.ts index 4c79e760..ce0233a8 100644 --- a/services/api/test/integration/versions/index.test.ts +++ b/services/api/test/integration/versions/index.test.ts @@ -20,6 +20,10 @@ import { VersionInfo } from "../../utils/test-types"; +vi.mock("../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/lib/github-app.test.ts b/services/api/test/lib/github-app.test.ts new file mode 100644 index 00000000..63c7e7f6 --- /dev/null +++ b/services/api/test/lib/github-app.test.ts @@ -0,0 +1,135 @@ +import { + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi +} from "vitest"; +import { githubTokenProvider } from "../../src/lib/github-app"; + +let privateKey: string; +let publicKey: CryptoKey; +let sequence = 0; +let settings: Record; +const fetchMock = vi.fn(); +const platform = { getEnv: (name: string) => settings[name] }; + +beforeAll(async () => { + const pair = (await crypto.subtle.generateKey( + { + name: "RSASSA-PKCS1-v1_5", + modulusLength: 2048, + publicExponent: new Uint8Array([1, 0, 1]), + hash: "SHA-256" + }, + true, + ["sign", "verify"] + )) as CryptoKeyPair; + publicKey = pair.publicKey; + const bytes = new Uint8Array( + (await crypto.subtle.exportKey("pkcs8", pair.privateKey)) as ArrayBuffer + ); + privateKey = `-----BEGIN PRIVATE KEY-----\n${btoa(String.fromCharCode(...bytes))}\n-----END PRIVATE KEY-----`; +}); + +beforeEach(() => { + settings = { + GITHUB_APP_CLIENT_ID: `test-client-${++sequence}`, + GITHUB_APP_INSTALLATION_ID: "123", + GITHUB_APP_PRIVATE_KEY: privateKey + }; + fetchMock.mockReset().mockImplementation(async () => + Response.json({ + token: "installation-token", + expires_at: new Date(Date.now() + 3600_000).toISOString() + }) + ); + vi.stubGlobal("fetch", fetchMock); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe("GitHub App authentication", () => { + it("lazily signs an App JWT and requests only the repository's read permissions", async () => { + const getToken = githubTokenProvider(platform); + expect(fetchMock).not.toHaveBeenCalled(); + expect(await getToken()).toBe("installation-token"); + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe( + "https://api.github.com/app/installations/123/access_tokens" + ); + expect(init?.method).toBe("POST"); + expect(JSON.parse(init?.body as string)).toEqual({ + repositories: ["FOSSBilling"], + permissions: { contents: "read", actions: "read", pull_requests: "read" } + }); + const jwt = new Headers(init?.headers).get("authorization")!.slice(7); + const [header, payload, signature] = jwt.split("."); + const decode = (value: string) => + Uint8Array.from( + atob(value.replace(/-/g, "+").replace(/_/g, "/")), + (char) => char.charCodeAt(0) + ); + expect(JSON.parse(new TextDecoder().decode(decode(payload)))).toMatchObject( + { + iss: settings.GITHUB_APP_CLIENT_ID + } + ); + expect( + await crypto.subtle.verify( + "RSASSA-PKCS1-v1_5", + publicKey, + decode(signature), + new TextEncoder().encode(`${header}.${payload}`) + ) + ).toBe(true); + }); + + it("coalesces callers within a request and reuses resolved tokens across requests", async () => { + const getToken = githubTokenProvider(platform); + expect(await Promise.all([getToken(), getToken()])).toEqual([ + "installation-token", + "installation-token" + ]); + expect(await githubTokenProvider(platform)()).toBe("installation-token"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("refreshes before expiry and invalidates the cache on key rotation", async () => { + await githubTokenProvider(platform)(); + const now = Date.now(); + vi.spyOn(Date, "now").mockReturnValue(now + 3541_000); + await githubTokenProvider(platform)(); + expect(fetchMock).toHaveBeenCalledTimes(2); + // Escaped-newline PEMs are accepted and a changed key value invalidates the cache. + settings.GITHUB_APP_PRIVATE_KEY = privateKey.replace(/\n/g, "\\n"); + await githubTokenProvider(platform)(); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it("fails closed when credentials are missing", async () => { + delete settings.GITHUB_APP_PRIVATE_KEY; + await expect(githubTokenProvider(platform)()).rejects.toMatchObject({ + httpStatus: 503, + message: "GitHub App credentials are not configured" + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("sanitizes token failures, preserves unavailable semantics and retries on a new request", async () => { + fetchMock.mockResolvedValueOnce( + new Response("sensitive upstream response", { status: 404 }) + ); + await expect(githubTokenProvider(platform)()).rejects.toMatchObject({ + httpStatus: 503, + message: "GitHub App authentication is unavailable" + }); + expect(await githubTokenProvider(platform)()).toBe("installation-token"); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/services/api/test/services/extensions/v2/account.test.ts b/services/api/test/services/extensions/v2/account.test.ts index ebb494f0..9d6bc3ae 100644 --- a/services/api/test/services/extensions/v2/account.test.ts +++ b/services/api/test/services/extensions/v2/account.test.ts @@ -43,6 +43,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/badges.test.ts b/services/api/test/services/extensions/v2/badges.test.ts index 4995ca4c..76d639c9 100644 --- a/services/api/test/services/extensions/v2/badges.test.ts +++ b/services/api/test/services/extensions/v2/badges.test.ts @@ -4,6 +4,10 @@ import { db, get, setupExtensionsV2Tests } from "./harness"; import { insertDeveloper, insertExtension } from "./db-fixtures"; import { wrapD1WithHook } from "./db-interceptor"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/claim-budget.test.ts b/services/api/test/services/extensions/v2/claim-budget.test.ts index bbfc445b..44056925 100644 --- a/services/api/test/services/extensions/v2/claim-budget.test.ts +++ b/services/api/test/services/extensions/v2/claim-budget.test.ts @@ -13,6 +13,10 @@ import { post } from "./harness"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/contract.test.ts b/services/api/test/services/extensions/v2/contract.test.ts index ecf415e8..28ae6ef5 100644 --- a/services/api/test/services/extensions/v2/contract.test.ts +++ b/services/api/test/services/extensions/v2/contract.test.ts @@ -14,6 +14,10 @@ import { import { getDeveloper, insertDeveloper, insertUser } from "./db-fixtures"; // Hoisted so no v2 suite can make a real GitHub call. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/developer-approval.test.ts b/services/api/test/services/extensions/v2/developer-approval.test.ts index 988e3f9c..0b151a0c 100644 --- a/services/api/test/services/extensions/v2/developer-approval.test.ts +++ b/services/api/test/services/extensions/v2/developer-approval.test.ts @@ -19,6 +19,10 @@ import { insertUser } from "./db-fixtures"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/developer-profiles.test.ts b/services/api/test/services/extensions/v2/developer-profiles.test.ts index 7bcb28e9..2a6eabfc 100644 --- a/services/api/test/services/extensions/v2/developer-profiles.test.ts +++ b/services/api/test/services/extensions/v2/developer-profiles.test.ts @@ -37,6 +37,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/directory-identity.test.ts b/services/api/test/services/extensions/v2/directory-identity.test.ts index 868ffe74..5d873778 100644 --- a/services/api/test/services/extensions/v2/directory-identity.test.ts +++ b/services/api/test/services/extensions/v2/directory-identity.test.ts @@ -11,6 +11,10 @@ import { setupExtensionsV2Tests } from "./harness"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/extension-writes.test.ts b/services/api/test/services/extensions/v2/extension-writes.test.ts index 85893aac..359648db 100644 --- a/services/api/test/services/extensions/v2/extension-writes.test.ts +++ b/services/api/test/services/extensions/v2/extension-writes.test.ts @@ -29,6 +29,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/index.test.ts b/services/api/test/services/extensions/v2/index.test.ts index 0ed985e7..9a261c18 100644 --- a/services/api/test/services/extensions/v2/index.test.ts +++ b/services/api/test/services/extensions/v2/index.test.ts @@ -3,6 +3,10 @@ import { setupExtensionsV2Tests, get } from "./harness"; // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/manifest.test.ts b/services/api/test/services/extensions/v2/manifest.test.ts index 9eb547cb..41aed446 100644 --- a/services/api/test/services/extensions/v2/manifest.test.ts +++ b/services/api/test/services/extensions/v2/manifest.test.ts @@ -13,6 +13,10 @@ import { import { StoredExtensionContentSchema } from "../../../../src/services/extensions/v2/schemas/extensions"; import { insertExtension, insertUser } from "./db-fixtures"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/moderation-correct.test.ts b/services/api/test/services/extensions/v2/moderation-correct.test.ts index cee38394..336bb52b 100644 --- a/services/api/test/services/extensions/v2/moderation-correct.test.ts +++ b/services/api/test/services/extensions/v2/moderation-correct.test.ts @@ -25,6 +25,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/moderation-notify.test.ts b/services/api/test/services/extensions/v2/moderation-notify.test.ts index e3630072..fc5f74de 100644 --- a/services/api/test/services/extensions/v2/moderation-notify.test.ts +++ b/services/api/test/services/extensions/v2/moderation-notify.test.ts @@ -19,6 +19,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/moderation-revalidate.test.ts b/services/api/test/services/extensions/v2/moderation-revalidate.test.ts index 05ae419a..177b41be 100644 --- a/services/api/test/services/extensions/v2/moderation-revalidate.test.ts +++ b/services/api/test/services/extensions/v2/moderation-revalidate.test.ts @@ -24,6 +24,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/moderation-revocation.test.ts b/services/api/test/services/extensions/v2/moderation-revocation.test.ts index 4d11500b..2ca89d9e 100644 --- a/services/api/test/services/extensions/v2/moderation-revocation.test.ts +++ b/services/api/test/services/extensions/v2/moderation-revocation.test.ts @@ -20,6 +20,10 @@ import { getDeveloperClaim } from "./db-fixtures"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/moderation.test.ts b/services/api/test/services/extensions/v2/moderation.test.ts index 9e02003b..d81f70d9 100644 --- a/services/api/test/services/extensions/v2/moderation.test.ts +++ b/services/api/test/services/extensions/v2/moderation.test.ts @@ -32,6 +32,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/ownership.test.ts b/services/api/test/services/extensions/v2/ownership.test.ts index 85bcb915..1839efc3 100644 --- a/services/api/test/services/extensions/v2/ownership.test.ts +++ b/services/api/test/services/extensions/v2/ownership.test.ts @@ -33,6 +33,10 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/packages.test.ts b/services/api/test/services/extensions/v2/packages.test.ts index 875f4c6e..6de44c09 100644 --- a/services/api/test/services/extensions/v2/packages.test.ts +++ b/services/api/test/services/extensions/v2/packages.test.ts @@ -47,6 +47,10 @@ import { } from "./harness"; import { insertUser, getExtension, getDeveloper, UUID_V7 } from "./db-fixtures"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/pending-edits.test.ts b/services/api/test/services/extensions/v2/pending-edits.test.ts index 59e89aa7..81967a34 100644 --- a/services/api/test/services/extensions/v2/pending-edits.test.ts +++ b/services/api/test/services/extensions/v2/pending-edits.test.ts @@ -20,6 +20,10 @@ import { insertRevision, insertUnpublishedExtension } from "./db-fixtures"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/public-extensions.test.ts b/services/api/test/services/extensions/v2/public-extensions.test.ts index 9b9173d0..5459ee8a 100644 --- a/services/api/test/services/extensions/v2/public-extensions.test.ts +++ b/services/api/test/services/extensions/v2/public-extensions.test.ts @@ -5,6 +5,10 @@ import { insertDeveloper, insertExtension } from "./db-fixtures"; // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/publisher.test.ts b/services/api/test/services/extensions/v2/publisher.test.ts index 297be069..a48c1c32 100644 --- a/services/api/test/services/extensions/v2/publisher.test.ts +++ b/services/api/test/services/extensions/v2/publisher.test.ts @@ -26,6 +26,10 @@ import { } from "../../../lib/auth/m2m-helper"; // Hoisted so no v2 suite can make a real GitHub call (see moderation.test.ts). +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/extensions/v2/resource-bounds.test.ts b/services/api/test/services/extensions/v2/resource-bounds.test.ts index 0a27a633..c7dbb2af 100644 --- a/services/api/test/services/extensions/v2/resource-bounds.test.ts +++ b/services/api/test/services/extensions/v2/resource-bounds.test.ts @@ -46,6 +46,10 @@ import { countExtensions, countRevisions } from "./db-fixtures"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/previews/v1/budget.test.ts b/services/api/test/services/previews/v1/budget.test.ts index 02654372..1f316479 100644 --- a/services/api/test/services/previews/v1/budget.test.ts +++ b/services/api/test/services/previews/v1/budget.test.ts @@ -1,3 +1,5 @@ +import { createCloudflareBindings } from "../../../../src/lib/adapters/cloudflare"; +import { platformMiddleware } from "../../../../src/lib/middleware"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { env } from "cloudflare:workers"; import { @@ -12,6 +14,10 @@ import { previewRequest } from "../../../../src/services/previews/v1/github/request"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); @@ -200,6 +206,10 @@ describe("preview GitHub budget", () => { it("caps concurrent subrequests before awaiting reservations", async () => { const probe = new Hono<{ Bindings: CloudflareBindings }>(); + probe.use( + "*", + platformMiddleware(createCloudflareBindings(env), () => {}) + ); probe.get("/", async (c) => { const github = previewGitHub(c); const allowed = await Promise.all( @@ -242,10 +252,11 @@ describe("preview GitHub budget", () => { }); it("fails closed if reservation fails", async () => { + const getToken = vi.fn(async () => "test"); await expect( previewRequest( { - token: "test", + getToken, reserve: async () => { throw new Error("budget unavailable"); } @@ -254,6 +265,7 @@ describe("preview GitHub budget", () => { {} ) ).rejects.toThrow("Preview GitHub budget unavailable"); + expect(getToken).not.toHaveBeenCalled(); expect(request).not.toHaveBeenCalled(); }); }); diff --git a/services/api/test/services/previews/v1/commit.test.ts b/services/api/test/services/previews/v1/commit.test.ts index cb00c077..3e023ba5 100644 --- a/services/api/test/services/previews/v1/commit.test.ts +++ b/services/api/test/services/previews/v1/commit.test.ts @@ -9,6 +9,10 @@ import app from "../../../../src/app/index"; import { MockGitHubRequest } from "../../../utils/test-types"; import { suppressConsole } from "../../../utils/mock-helpers"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/previews/v1/main.test.ts b/services/api/test/services/previews/v1/main.test.ts index 0c3e605a..6b15f41e 100644 --- a/services/api/test/services/previews/v1/main.test.ts +++ b/services/api/test/services/previews/v1/main.test.ts @@ -9,6 +9,10 @@ import app from "../../../../src/app/index"; import { MockGitHubRequest } from "../../../utils/test-types"; import { suppressConsole } from "../../../utils/mock-helpers"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/previews/v1/pr.test.ts b/services/api/test/services/previews/v1/pr.test.ts index dc4baefa..571230e9 100644 --- a/services/api/test/services/previews/v1/pr.test.ts +++ b/services/api/test/services/previews/v1/pr.test.ts @@ -9,6 +9,10 @@ import app from "../../../../src/app/index"; import { MockGitHubRequest } from "../../../utils/test-types"; import { suppressConsole } from "../../../utils/mock-helpers"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/stats/v1/index.test.ts b/services/api/test/services/stats/v1/index.test.ts index 2bf89d6a..66ec367b 100644 --- a/services/api/test/services/stats/v1/index.test.ts +++ b/services/api/test/services/stats/v1/index.test.ts @@ -25,6 +25,10 @@ import { request as ghRequest } from "@octokit/request"; import { graphql } from "@octokit/graphql"; import { ApiResponse } from "../../../utils/test-types"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/versions/v1/errors.test.ts b/services/api/test/services/versions/v1/errors.test.ts index c58db157..0c6d4027 100644 --- a/services/api/test/services/versions/v1/errors.test.ts +++ b/services/api/test/services/versions/v1/errors.test.ts @@ -21,6 +21,10 @@ import { VersionInfo } from "../../../utils/test-types"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/versions/v1/index.test.ts b/services/api/test/services/versions/v1/index.test.ts index f166eef3..a132762a 100644 --- a/services/api/test/services/versions/v1/index.test.ts +++ b/services/api/test/services/versions/v1/index.test.ts @@ -29,6 +29,10 @@ import { // Edge entries are cleared between tests to exercise live handlers. const PUBLIC_HEADERS = { authorization: "test-bypass-cache" } as const; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/test/services/versions/v1/middleware.test.ts b/services/api/test/services/versions/v1/middleware.test.ts index 633b5170..3cd7b741 100644 --- a/services/api/test/services/versions/v1/middleware.test.ts +++ b/services/api/test/services/versions/v1/middleware.test.ts @@ -19,6 +19,10 @@ import { MockGitHubRequest } from "../../../utils/test-types"; +vi.mock("../../../../src/lib/github-app", () => ({ + githubTokenProvider: () => async () => "test-github-token" +})); + vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() ); diff --git a/services/api/vitest.config.ts b/services/api/vitest.config.ts index b2beaf96..16108dc6 100644 --- a/services/api/vitest.config.ts +++ b/services/api/vitest.config.ts @@ -20,7 +20,6 @@ export default defineConfig({ miniflare: { // Add test environment variables bindings: { - GITHUB_TOKEN: "test-github-token", UPDATE_TOKEN: "test-update-token", ASSERTION_SIGNING_SECRET: "test-assertion-signing-secret", M2M_ISSUER: "https://auth.example.test", diff --git a/services/api/vitest.node.config.ts b/services/api/vitest.node.config.ts index 22bdcf76..bcafa05e 100644 --- a/services/api/vitest.node.config.ts +++ b/services/api/vitest.node.config.ts @@ -4,6 +4,7 @@ export default defineConfig({ test: { environment: "node", include: [ + "test/lib/github-app.test.ts", "test/lib/adapters/node/**/*.test.ts", "test/services/extensions/v2/migrations.test.ts", "test/**/*.node.test.ts" @@ -14,7 +15,7 @@ export default defineConfig({ coverage: { provider: "istanbul", reporter: ["text", "json", "html"], - include: ["src/lib/adapters/node/**"] + include: ["test/lib/github-app.test.ts", "src/lib/adapters/node/**"] } } }); diff --git a/services/api/worker-configuration.d.ts b/services/api/worker-configuration.d.ts index ea247648..6ecc9121 100644 --- a/services/api/worker-configuration.d.ts +++ b/services/api/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: d1888a571fcbd45ad7ca22833c048474) +// Generated by Wrangler by running `wrangler types --env-file=.dev.vars.example --env-interface=CloudflareBindings` (hash: 8625c7a110081a79725d5824556790df) // Runtime types generated with workerd@1.20261104.1 2026-10-01 interface __BaseEnv_CloudflareBindings { AUTH_KV: KVNamespace; @@ -19,10 +19,12 @@ interface __BaseEnv_CloudflareBindings { M2M_ISSUER: "https://auth.fossbilling.net"; M2M_JWKS_URL: "https://auth.fossbilling.net/.well-known/jwks.json"; M2M_AUDIENCE: "https://api.fossbilling.net"; - GITHUB_TOKEN: string; + GITHUB_APP_CLIENT_ID: string; + GITHUB_APP_INSTALLATION_ID: string; + GITHUB_APP_PRIVATE_KEY: string; ASSERTION_SIGNING_SECRET: string; - EXTENSIONS_V2_MXROUTE_PASSWORD: string; EXTENSIONS_REVALIDATE_SECRET: string; + EXTENSIONS_V2_MXROUTE_PASSWORD: string; PREVIEW_GITHUB_BUDGET: DurableObjectNamespace; EXTENSIONS_FRONTEND: Fetcher /* extensions */; } @@ -38,7 +40,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } declare module "*.txt" { const value: string; From 1fb33868e5f8930d0685d87d1e98626efe5b3ecf Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 20:46:25 +0000 Subject: [PATCH 2/9] Clarify GitHub App preview and production key setup --- services/api/README.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/services/api/README.md b/services/api/README.md index 5efe0f62..bc62bcef 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -124,8 +124,10 @@ Actions covers preview artifacts and runs; Pull requests resolves preview heads. Developer checks read public GitHub profiles; identity and organization ownership still come from the caller's linked identity in the auth service. -Use a dedicated API private key so it can be rotated independently of other -Integrations consumers; separate keys still have the same App authority. +Use separate API preview and production private keys so they can be rotated +independently. Use the preview key locally and in Previews Base, and the production +key in the API Worker's production settings. Both use the same App client and +installation IDs; separate keys still have the same App authority. Set the three `GITHUB_APP_*` values as runtime variables/secrets in each deployment that needs GitHub access, and in `services/api/.dev.vars` locally. Do not use build @@ -233,8 +235,10 @@ Migration jobs serialize access to the shared staging databases. staging resources; they do not automatically receive separate D1/KV/R2 data. 3. Configure Preview runtime secrets in the API Worker's **Previews Base** settings. - Use a separate assertion-signing secret, revalidation secret and read-only GitHub - token. Match the revalidation secret with the staging frontend. Set M2M issuer, + Use a separate assertion-signing secret, revalidation secret and GitHub App + preview key, alongside `GITHUB_APP_CLIENT_ID` and `GITHUB_APP_INSTALLATION_ID` + (see [GitHub App authentication](#github-app-authentication)). Match the + revalidation secret with the staging frontend. Set M2M issuer, JWKS and audience variables there when using a separate test issuer. Operational production variables and secrets remain dashboard-managed with `keep_vars: true`. 4. Configure required checks for the new validation/build job names, then disable the From 27ce96d636ec5a05667ce2ab24127e326b334649 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 21:39:30 +0000 Subject: [PATCH 3/9] Update local secrets example and ignore rules Ignore all environment and dev vars files while allowing example dev vars files. Update the private key format and clarify the shared assertion secret. --- .gitignore | 8 +++----- services/api/.dev.vars.example | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index d8a11f28..474e0a0a 100644 --- a/.gitignore +++ b/.gitignore @@ -10,11 +10,9 @@ node_modules/ .wrangler # env -.env -.env.production -.dev.vars -.dev.vars.* -!services/api/.dev.vars.example +.env* +.dev.vars* +!.dev.vars.example # logs logs/ diff --git a/services/api/.dev.vars.example b/services/api/.dev.vars.example index e64e422e..f9018eb6 100644 --- a/services/api/.dev.vars.example +++ b/services/api/.dev.vars.example @@ -1,6 +1,6 @@ GITHUB_APP_CLIENT_ID="your-client-id" GITHUB_APP_INSTALLATION_ID="your-installation-id" -GITHUB_APP_PRIVATE_KEY="-----BEGIN PRIVATE KEY-----\n...\n-----END PRIVATE KEY-----" -ASSERTION_SIGNING_SECRET="local-shared-secret" +GITHUB_APP_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY-----\n...\n-----END RSA PRIVATE KEY-----" +ASSERTION_SIGNING_SECRET="local-shared-secret" # Must match `FOSSBilling/extensions` EXTENSIONS_REVALIDATE_SECRET="local-revalidate-secret" EXTENSIONS_V2_MXROUTE_PASSWORD="local-mailbox-password" From 1b3c83c606a7fafd3ce3513534b7d6d1aebf7a1c Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 22:52:47 +0000 Subject: [PATCH 4/9] Share an Octokit client for authenticated GitHub reads --- package-lock.json | 137 +++++++++++++- services/api/README.md | 19 +- services/api/package.json | 11 +- services/api/src/lib/github-app.ts | 111 +++++------ .../extensions/v2/db/developer-claims.ts | 6 +- .../extensions/v2/db/developer-profiles.ts | 14 +- .../services/extensions/v2/github/identity.ts | 11 +- .../extensions/v2/github/verification.ts | 11 +- .../v2/routes/developer-profiles.ts | 6 +- .../extensions/v2/routes/ownership.ts | 4 +- .../services/previews/v1/github/request.ts | 16 +- services/api/src/services/stats/v1/index.ts | 10 +- .../api/src/services/versions/v1/index.ts | 44 ++--- services/api/test/integration/app.test.ts | 6 +- .../api/test/integration/github-app.test.ts | 4 +- .../test/integration/versions/index.test.ts | 6 +- services/api/test/lib/github-app.test.ts | 172 +++++++++--------- services/api/test/mocks/octokit.ts | 12 ++ .../services/extensions/v2/account.test.ts | 6 +- .../services/extensions/v2/badges.test.ts | 6 +- .../extensions/v2/claim-budget.test.ts | 6 +- .../services/extensions/v2/contract.test.ts | 6 +- .../extensions/v2/developer-approval.test.ts | 6 +- .../extensions/v2/developer-profiles.test.ts | 6 +- .../extensions/v2/directory-identity.test.ts | 6 +- .../extensions/v2/extension-writes.test.ts | 6 +- .../test/services/extensions/v2/index.test.ts | 6 +- .../services/extensions/v2/manifest.test.ts | 6 +- .../extensions/v2/moderation-correct.test.ts | 6 +- .../extensions/v2/moderation-notify.test.ts | 6 +- .../v2/moderation-revalidate.test.ts | 6 +- .../v2/moderation-revocation.test.ts | 6 +- .../services/extensions/v2/moderation.test.ts | 6 +- .../services/extensions/v2/ownership.test.ts | 6 +- .../services/extensions/v2/packages.test.ts | 6 +- .../extensions/v2/pending-edits.test.ts | 6 +- .../extensions/v2/public-extensions.test.ts | 6 +- .../services/extensions/v2/publisher.test.ts | 6 +- .../extensions/v2/resource-bounds.test.ts | 6 +- .../test/services/previews/v1/budget.test.ts | 14 +- .../test/services/previews/v1/commit.test.ts | 6 +- .../test/services/previews/v1/main.test.ts | 6 +- .../api/test/services/previews/v1/pr.test.ts | 6 +- .../api/test/services/stats/v1/index.test.ts | 6 +- .../test/services/versions/v1/errors.test.ts | 6 +- .../test/services/versions/v1/index.test.ts | 6 +- .../services/versions/v1/middleware.test.ts | 6 +- services/api/vitest.config.ts | 2 + services/api/vitest.node.config.ts | 2 +- 49 files changed, 458 insertions(+), 328 deletions(-) diff --git a/package-lock.json b/package-lock.json index 86b27ef0..9748d1ab 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2244,6 +2244,99 @@ "url": "https://paulmillr.com/funding/" } }, + "node_modules/@octokit/auth-app": { + "version": "8.3.1", + "resolved": "https://registry.npmjs.org/@octokit/auth-app/-/auth-app-8.3.1.tgz", + "integrity": "sha512-XHWxZsF4mvGTQCv7AOxgby9t9OjVO19FBuKq+QmyBxFrf+B2hR/P5j/MTF8g95bITf8cRtR17KIXThpFNqiKbg==", + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-app": "^9.0.5", + "@octokit/auth-oauth-user": "^6.0.4", + "@octokit/request": "^10.0.16", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0", + "toad-cache": "^3.7.0", + "universal-github-app-jwt": "^2.2.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/auth-oauth-app": { + "version": "9.0.5", + "resolved": "https://registry.npmjs.org/@octokit/auth-oauth-app/-/auth-oauth-app-9.0.5.tgz", + "integrity": "sha512-Hn2yAlIDjFLGjAz7t+CRu7e9VYoFW1W3LLyOLyahFoJGlCf4PtDkHKtRET8w/2CJpsrC0FxIF5g5Xtx9E4O78w==", + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-device": "^8.0.5", + "@octokit/auth-oauth-user": "^6.0.4", + "@octokit/request": "^10.0.16", + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/auth-oauth-device": { + "version": "8.0.5", + "resolved": "https://registry.npmjs.org/@octokit/auth-oauth-device/-/auth-oauth-device-8.0.5.tgz", + "integrity": "sha512-kdJtjRhykMLJrMdcoC56XUBDfoUAuWEFKpkDxwYim+bNwYw1MreLolk+PLnwEYT8djm589YepeMuJ8+ECAIzVQ==", + "license": "MIT", + "dependencies": { + "@octokit/oauth-methods": "^6.0.5", + "@octokit/request": "^10.0.16", + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/auth-oauth-user": { + "version": "6.0.4", + "resolved": "https://registry.npmjs.org/@octokit/auth-oauth-user/-/auth-oauth-user-6.0.4.tgz", + "integrity": "sha512-eya2pMJ9pCBq1jrm1Rf6J3NI+NemEpzDMLo9J/GpRoK0KgdxIVXWX4LESIfwJ+GYeoq6QFIlUJNi1lVp5FBgmw==", + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-device": "^8.0.5", + "@octokit/oauth-methods": "^6.0.5", + "@octokit/request": "^10.0.16", + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/auth-token": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-6.0.0.tgz", + "integrity": "sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==", + "license": "MIT", + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/core": { + "version": "7.0.8", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-7.0.8.tgz", + "integrity": "sha512-L7y8eYc+AwxGr2PWI4WFt1VG4TiJ66c26BD16mXpYIlXxG0SMigM1+m4aTSlYyBr5BlQsGAlz8uDCoZN4SEMcg==", + "license": "MIT", + "dependencies": { + "@octokit/auth-token": "^6.0.0", + "@octokit/graphql": "^9.0.5", + "@octokit/request": "^10.0.16", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0", + "before-after-hook": "^4.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, "node_modules/@octokit/endpoint": { "version": "11.0.5", "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-11.0.5.tgz", @@ -2271,6 +2364,30 @@ "node": ">= 20" } }, + "node_modules/@octokit/oauth-authorization-url": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/@octokit/oauth-authorization-url/-/oauth-authorization-url-8.0.0.tgz", + "integrity": "sha512-7QoLPRh/ssEA/HuHBHdVdSgF8xNLz/Bc5m9fZkArJE5bb6NmVkDm3anKxXPmN1zh6b5WKZPRr3697xKT/yM3qQ==", + "license": "MIT", + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/oauth-methods": { + "version": "6.0.5", + "resolved": "https://registry.npmjs.org/@octokit/oauth-methods/-/oauth-methods-6.0.5.tgz", + "integrity": "sha512-/mAz7taDZD7DcV4zcG6TDw3Kr6TOldmYBRpvA62C3WIxRMmcgX8XxsTvVQomqKE7VEk/BgYhUao1aR2tYGyIKg==", + "license": "MIT", + "dependencies": { + "@octokit/oauth-authorization-url": "^8.0.0", + "@octokit/request": "^10.0.16", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0" + }, + "engines": { + "node": ">= 20" + } + }, "node_modules/@octokit/openapi-types": { "version": "29.0.1", "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-29.0.1.tgz", @@ -3270,6 +3387,12 @@ "node": "18 || 20 || >=22" } }, + "node_modules/before-after-hook": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-4.0.0.tgz", + "integrity": "sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==", + "license": "Apache-2.0" + }, "node_modules/better-auth": { "version": "1.7.18", "resolved": "https://registry.npmjs.org/better-auth/-/better-auth-1.7.18.tgz", @@ -5698,6 +5821,15 @@ "node": ">=14.0.0" } }, + "node_modules/toad-cache": { + "version": "3.7.4", + "resolved": "https://registry.npmjs.org/toad-cache/-/toad-cache-3.7.4.tgz", + "integrity": "sha512-m1TdR/rvT7kgGJZhspNtXdsdYk0fddFpJJFlG5s+UkPFo6lkLoZ3YLOaovPYjq1R75NP5JfeTlSHaOsE09peCg==", + "license": "MIT", + "engines": { + "node": ">=20" + } + }, "node_modules/ts-api-utils": { "version": "2.5.0", "resolved": "https://registry.npmjs.org/ts-api-utils/-/ts-api-utils-2.5.0.tgz", @@ -6659,7 +6791,8 @@ "@fossbilling/extension-packaging": "*", "@fossbilling/logging": "*", "@hono/zod-openapi": "1.6.3", - "@octokit/graphql": "9.0.5", + "@octokit/auth-app": "8.3.1", + "@octokit/core": "7.0.8", "@octokit/request": "10.0.16", "@scalar/hono-api-reference": "0.12.18", "badge-maker": "6.0.0", @@ -6667,12 +6800,12 @@ "drizzle-orm": "0.45.4", "hono": "4.13.17", "semver": "7.8.6", - "universal-github-app-jwt": "2.2.2", "uuid": "14.0.3", "zod": "4.6.7" }, "devDependencies": { "@cloudflare/vitest-plugin": "1.6.1", + "@octokit/graphql": "9.0.5", "@types/semver": "7.8.0", "@zip.js/zip.js": "2.24.0", "drizzle-kit": "0.31.11", diff --git a/services/api/README.md b/services/api/README.md index bc62bcef..4b716f29 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -99,7 +99,7 @@ Package storage configuration is listed in [PACKAGING.md](src/services/extension - `GITHUB_APP_CLIENT_ID`: Client ID of the FOSSBilling Integrations GitHub App. - `GITHUB_APP_INSTALLATION_ID`: Its installation ID in the FOSSBilling organization. -- `GITHUB_APP_PRIVATE_KEY`: App signing key (PEM), stored as a secret. Use PKCS#8 for portability across runtimes; real newlines or escaped `\n` are accepted. +- `GITHUB_APP_PRIVATE_KEY`: App signing key (PEM), stored as a secret. GitHub's downloaded PKCS#1 PEM and PKCS#8 are accepted, with real newlines or escaped `\n`. - `ASSERTION_SIGNING_SECRET`: Shared HMAC secret used to verify the short-lived bearer assertions minted by the Extensions site. Configure the same value in both Workers; it is never sent to clients. - `ASSERTION_SIGNING_SECRET_PREVIOUS`: Optional previous HMAC secret accepted during a signing-key rotation. - `EXTENSIONS_REVALIDATE_SECRET`: Bearer token the API sends to the Extensions site's `POST /api/revalidate` (over the `EXTENSIONS_FRONTEND` service binding) to purge the site's CDN-cached extension pages after content mutations. Configure the same value in both Workers; it is never sent to clients. @@ -131,13 +131,16 @@ installation IDs; separate keys still have the same App authority. Set the three `GITHUB_APP_*` values as runtime variables/secrets in each deployment that needs GitHub access, and in `services/api/.dev.vars` locally. Do not use build -variables or a static installation token. If the signing key is PKCS#1, convert it -once with `openssl pkcs8 -topk8 -nocrypt -in app.pem -out app.pkcs8.pem` before -storing it. Approve the installation's permission changes before deployment. - -The API mints installation tokens restricted to `FOSSBilling/FOSSBilling` and those -three read permissions, and caches resolved tokens in memory until one minute -before expiry. Key/configuration changes invalidate that cache. Authentication is +variables or a static installation token. Use the full downloaded PEM; no manual +key conversion is needed. Approve the installation's permission changes before +deployment. + +A shared Octokit REST/GraphQL client uses App installation tokens restricted to +`FOSSBilling/FOSSBilling` and those three read permissions. Octokit manages token +caching and refresh; credential changes replace its auth instance and cache. +Wrangler builds and the Workers test runner select Node-compatible dependency +exports (`workerd,worker,node`) so Octokit accepts GitHub's downloaded PKCS#1 PEM +directly, just as it does in Node. PKCS#8 keys also work. Authentication is lazy: cached responses and profile operations without a GitHub lookup do not mint tokens. Authentication failures use the existing unavailable/stale-cache paths; they never become a missing GitHub user or anonymous lookup. There are no runtime diff --git a/services/api/package.json b/services/api/package.json index c44ed681..c7987f06 100644 --- a/services/api/package.json +++ b/services/api/package.json @@ -6,8 +6,8 @@ "license": "AGPL-3.0-only", "scripts": { "cf-typegen": "wrangler types --env-interface CloudflareBindings --env-file .dev.vars.example", - "cf-deploy": "npm run db:migrate:all:remote && wrangler deploy --minify", - "dev": "wrangler dev", + "cf-deploy": "npm run db:migrate:all:remote && WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler deploy --minify", + "dev": "WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler dev", "db:generate:extensions": "drizzle-kit generate --config drizzle.extensions.config.ts", "db:generate:central-alerts": "drizzle-kit generate --config drizzle.central-alerts.config.ts", "db:migrate:extensions-v2:local": "wrangler d1 migrations apply DB_EXTENSIONS --local", @@ -21,14 +21,15 @@ "test:coverage": "vitest run --coverage", "typecheck": "tsc --noEmit", "test:workers": "vitest run", - "build": "wrangler deploy --dry-run --outdir dist --minify && node scripts/prepare-deployment.ts", + "build": "WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler deploy --dry-run --outdir dist --minify && node scripts/prepare-deployment.ts", "cf-preview": "wrangler preview --config dist/wrangler.json" }, "dependencies": { "@fossbilling/extension-packaging": "*", "@fossbilling/logging": "*", "@hono/zod-openapi": "1.6.3", - "@octokit/graphql": "9.0.5", + "@octokit/auth-app": "8.3.1", + "@octokit/core": "7.0.8", "@octokit/request": "10.0.16", "@scalar/hono-api-reference": "0.12.18", "badge-maker": "6.0.0", @@ -36,12 +37,12 @@ "drizzle-orm": "0.45.4", "hono": "4.13.17", "semver": "7.8.6", - "universal-github-app-jwt": "2.2.2", "uuid": "14.0.3", "zod": "4.6.7" }, "devDependencies": { "@cloudflare/vitest-plugin": "1.6.1", + "@octokit/graphql": "9.0.5", "@types/semver": "7.8.0", "@zip.js/zip.js": "2.24.0", "drizzle-kit": "0.31.11", diff --git a/services/api/src/lib/github-app.ts b/services/api/src/lib/github-app.ts index bc8f2907..84b93993 100644 --- a/services/api/src/lib/github-app.ts +++ b/services/api/src/lib/github-app.ts @@ -1,32 +1,38 @@ -import githubAppJwt from "universal-github-app-jwt"; +import { createAppAuth } from "@octokit/auth-app"; +import { Octokit } from "@octokit/core"; +import { request } from "@octokit/request"; import type { PlatformContext } from "./context"; import { GitHubError } from "./github-errors"; -export type GitHubTokenProvider = () => Promise; +export type GitHubClient = Pick; +export type GitHubClientProvider = () => Promise; -// Cache resolved credentials only: an in-flight fetch belongs to its request -// context in Workers. Keep just the active configuration so key rotation also -// invalidates the cached token. +const transport = { + fetch: (...args: Parameters) => globalThis.fetch(...args), + timeout: 10_000 +}; + +// Reuse Octokit's installation-token cache for the active credentials. Replacing +// the auth instance on a configuration change also drops its cached tokens. let cached: | { clientId: string; installationId: string; privateKey: string; - token: string; - expiresAt: number; + authenticate: ReturnType; } | undefined; -export function githubTokenProvider( +export function githubClientProvider( platform: Pick -): GitHubTokenProvider { - let pending: Promise | undefined; - return () => (pending ??= installationToken(platform)); +): GitHubClientProvider { + let pending: Promise | undefined; + return () => (pending ??= createClient(platform)); } -async function installationToken( +async function createClient( platform: Pick -): Promise { +): Promise { const clientId = platform.getEnv("GITHUB_APP_CLIENT_ID"); const installationId = platform.getEnv("GITHUB_APP_INSTALLATION_ID"); const privateKey = platform.getEnv("GITHUB_APP_PRIVATE_KEY"); @@ -38,63 +44,40 @@ async function installationToken( ) { throw new GitHubError("GitHub App credentials are not configured", 503); } - if ( - cached?.clientId === clientId && - cached.installationId === installationId && - cached.privateKey === privateKey && - cached.expiresAt > Date.now() + 60_000 - ) { - return cached.token; - } try { - const { token: jwt } = await githubAppJwt({ id: clientId, privateKey }); - const response = await fetch( - `https://api.github.com/app/installations/${installationId}/access_tokens`, - { - method: "POST", - signal: AbortSignal.timeout(10_000), - headers: { - authorization: `Bearer ${jwt}`, - accept: "application/vnd.github+json", - "content-type": "application/json", - "user-agent": "FOSSBilling-API" - }, - body: JSON.stringify({ - repositories: ["FOSSBilling"], - permissions: { - contents: "read", - actions: "read", - pull_requests: "read" - } - }) - } - ); - if (!response.ok) throw new Error("Token request failed"); - const data = (await response.json()) as { - token?: unknown; - expires_at?: unknown; - }; - const expiresAt = - typeof data.expires_at === "string" ? Date.parse(data.expires_at) : NaN; if ( - typeof data.token !== "string" || - !data.token || - !(expiresAt > Date.now() + 60_000) + cached?.clientId !== clientId || + cached.installationId !== installationId || + cached.privateKey !== privateKey ) { - throw new Error("Invalid token response"); + cached = { + clientId, + installationId, + privateKey, + authenticate: createAppAuth({ + appId: clientId, + privateKey, + installationId, + request: request.defaults({ request: transport }) + }) + }; } - cached = { - clientId, - installationId, - privateKey, - token: data.token, - expiresAt - }; - return data.token; + // Request explicit scope before constructing the client: the automatic App + // auth hook otherwise issues a token with the installation's full access. + const { token } = await cached.authenticate({ + type: "installation", + repositoryNames: ["FOSSBilling"], + permissions: { contents: "read", actions: "read", pull_requests: "read" } + }); + return new Octokit({ + auth: token, + userAgent: "FOSSBilling-API", + request: transport + }); } catch { - // Never expose the signing key, JWT or GitHub's request/response objects in - // the callers' logs. A token-endpoint 404 must not become an entity 404. + // Never expose signing keys, tokens or SDK request/response objects in logs. + // A token-endpoint 404 must not become a missing GitHub user or artifact. throw new GitHubError("GitHub App authentication is unavailable", 503); } } diff --git a/services/api/src/services/extensions/v2/db/developer-claims.ts b/services/api/src/services/extensions/v2/db/developer-claims.ts index 2e7517f6..57373765 100644 --- a/services/api/src/services/extensions/v2/db/developer-claims.ts +++ b/services/api/src/services/extensions/v2/db/developer-claims.ts @@ -1,4 +1,4 @@ -import type { GitHubTokenProvider } from "../../../../lib/github-app"; +import type { GitHubClientProvider } from "../../../../lib/github-app"; import { and, asc, desc, eq, gt, lt, or, sql, SQL } from "drizzle-orm"; import { DatabaseResult } from "../../../../lib/interfaces"; import { ExtensionsDb } from "../../../../lib/db"; @@ -121,7 +121,7 @@ export class DeveloperClaimsDatabase { identifier: string, claimantId: string, note: string | undefined, - getGithubToken: GitHubTokenProvider + getGithubClient: GitHubClientProvider ): Promise> { try { const [developer] = await this.db @@ -183,7 +183,7 @@ export class DeveloperClaimsDatabase { developer.identifier, developer.type as Developer["type"], claimantId, - getGithubToken + getGithubClient ); if ("error" in check) { diff --git a/services/api/src/services/extensions/v2/db/developer-profiles.ts b/services/api/src/services/extensions/v2/db/developer-profiles.ts index bd5510c9..b31e9f23 100644 --- a/services/api/src/services/extensions/v2/db/developer-profiles.ts +++ b/services/api/src/services/extensions/v2/db/developer-profiles.ts @@ -1,4 +1,4 @@ -import type { GitHubTokenProvider } from "../../../../lib/github-app"; +import type { GitHubClientProvider } from "../../../../lib/github-app"; import { v7 as uuidv7 } from "uuid"; import { and, asc, desc, eq, gt, isNull, lt, or, sql, SQL } from "drizzle-orm"; import { DatabaseResult } from "../../../../lib/interfaces"; @@ -159,7 +159,7 @@ export class DeveloperProfilesDatabase { } } - // getGithubToken — see the comment on verifyGithubOwnership(). Only consulted + // getGithubClient — see the comment on verifyGithubOwnership(). Only consulted // when creating a brand-new profile (the identifier is immutable once // owned, so an update can't need re-verifying); guards against squatting // on an id that matches a real GitHub org/user the caller doesn't control, @@ -168,7 +168,7 @@ export class DeveloperProfilesDatabase { async upsertOwn( userId: string, developer: Developer, - getGithubToken: GitHubTokenProvider, + getGithubClient: GitHubClientProvider, allowCreationAttempt: () => Promise = async () => true ): Promise & { changed?: boolean }> { try { @@ -231,7 +231,7 @@ export class DeveloperProfilesDatabase { developer.identifier, developer.type, userId, - getGithubToken, + getGithubClient, developer.URL ); @@ -1011,7 +1011,7 @@ export class DeveloperProfilesDatabase { // own already-synced github_login/github_orgs. Called opportunistically // on every login for a developer-owning user, and by the owner's own // "Re-verify" action — both share this one method. - // checkUrl/getGithubToken — only set by the owner's own manual "Re-verify" + // checkUrl/getGithubClient — only set by the owner's own manual "Re-verify" // button, never by the opportunistic per-login call in extensions' // auth/callback.ts. Re-checking Publisher URL against GitHub's on-file // website needs a fresh GitHub API call (unlike the identity match below), @@ -1019,7 +1019,7 @@ export class DeveloperProfilesDatabase { async reverifyOwn( userId: string, checkUrl: boolean, - getGithubToken: GitHubTokenProvider + getGithubClient: GitHubClientProvider ): Promise> { try { const [row] = await this.db @@ -1144,7 +1144,7 @@ export class DeveloperProfilesDatabase { if (!matches) { writeUrlVerified = true; } else if (checkUrl) { - const entity = await checkGithubEntity(row.identifier, getGithubToken); + const entity = await checkGithubEntity(row.identifier, getGithubClient); // An unavailable lookup is explicitly inconclusive, not a disproof, // so it leaves the stored URL verification signal untouched rather // than clearing a real prior verification over a transient failure. diff --git a/services/api/src/services/extensions/v2/github/identity.ts b/services/api/src/services/extensions/v2/github/identity.ts index 3c391b67..ebc7f279 100644 --- a/services/api/src/services/extensions/v2/github/identity.ts +++ b/services/api/src/services/extensions/v2/github/identity.ts @@ -1,4 +1,4 @@ -import type { GitHubTokenProvider } from "../../../../lib/github-app"; +import type { GitHubClientProvider } from "../../../../lib/github-app"; import { DatabaseError } from "../../../../lib/interfaces"; import { ExtensionsDb } from "../../../../lib/db"; import { @@ -40,9 +40,8 @@ export function githubUnavailableError( }; } -// `getGithubToken` authenticates the GitHub entity-existence lookup only (a -// service-level credential, raises the public rate limit) — it is never -// the claimant's own token, which never leaves the auth service. Shared by +// `getGithubClient` supplies service-authenticated GitHub access for the entity +// lookup. The claimant's own token never leaves the auth service. Shared by // claim() and upsertOwn(): both need the same question answered — does a // real GitHub org/user exist for this id, and if so, does the caller's own // linked GitHub identity match it? A positive mismatch is the only automated @@ -61,7 +60,7 @@ export async function verifyGithubOwnership( developerId: string, developerType: Developer["type"], callerId: string, - getGithubToken: GitHubTokenProvider, + getGithubClient: GitHubClientProvider, publisherUrl?: string ): Promise { // The D1 identity read and the GitHub entity lookup are independent - @@ -70,7 +69,7 @@ export async function verifyGithubOwnership( // leave it to finish (it never rejects: DatabaseResult methods classify // their own errors). const identityLookup = new UsersDatabase(db).getGithubIdentity(callerId); - const githubEntity = await checkGithubEntity(developerId, getGithubToken); + const githubEntity = await checkGithubEntity(developerId, getGithubClient); if (githubEntity.status === "unavailable") { return { error: githubUnavailableError(githubEntity.reason) }; diff --git a/services/api/src/services/extensions/v2/github/verification.ts b/services/api/src/services/extensions/v2/github/verification.ts index 6a7bc800..9cb30dd1 100644 --- a/services/api/src/services/extensions/v2/github/verification.ts +++ b/services/api/src/services/extensions/v2/github/verification.ts @@ -1,5 +1,4 @@ -import type { GitHubTokenProvider } from "../../../../lib/github-app"; -import { request as ghRequest } from "@octokit/request"; +import type { GitHubClientProvider } from "../../../../lib/github-app"; import { AuthError, classifyGitHubError, @@ -72,12 +71,12 @@ function redactedFailureMessage(message: string): string { // and invalid data so callers never mistake an outage for a missing account. export async function checkGithubEntity( id: string, - getGithubToken: GitHubTokenProvider + getGithubClient: GitHubClientProvider ): Promise { try { - const result = await ghRequest("GET /users/{username}", { - username: id, - headers: { authorization: `Bearer ${await getGithubToken()}` } + const github = await getGithubClient(); + const result = await github.request("GET /users/{username}", { + username: id }); if ( typeof result.data !== "object" || diff --git a/services/api/src/services/extensions/v2/routes/developer-profiles.ts b/services/api/src/services/extensions/v2/routes/developer-profiles.ts index 93d9b421..ef698eb1 100644 --- a/services/api/src/services/extensions/v2/routes/developer-profiles.ts +++ b/services/api/src/services/extensions/v2/routes/developer-profiles.ts @@ -1,4 +1,4 @@ -import { githubTokenProvider } from "../../../../lib/github-app"; +import { githubClientProvider } from "../../../../lib/github-app"; import { getOptionalAuth, optionalAuth, @@ -195,7 +195,7 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { const { data, error, changed } = await db.upsertOwn( auth.userId, body, - githubTokenProvider(platform), + githubClientProvider(platform), // Keep this as a callback so upsertOwn can run its cheap // existing-profile/id checks first; updates and known-taken ids must not // spend creation allowance. @@ -319,7 +319,7 @@ export function registerDeveloperProfileRoutes(app: ExtensionsV2App): void { const { data, error } = await db.reverifyOwn( auth.userId, check_url, - githubTokenProvider(platform) + githubClientProvider(platform) ); if (error || !data) { const status = diff --git a/services/api/src/services/extensions/v2/routes/ownership.ts b/services/api/src/services/extensions/v2/routes/ownership.ts index 4408cdcc..f05996ca 100644 --- a/services/api/src/services/extensions/v2/routes/ownership.ts +++ b/services/api/src/services/extensions/v2/routes/ownership.ts @@ -1,4 +1,4 @@ -import { githubTokenProvider } from "../../../../lib/github-app"; +import { githubClientProvider } from "../../../../lib/github-app"; import { requireActiveAuth, requireModerator } from "../middleware"; import { getExtensionsDb } from "../../../../lib/db"; import { getPlatform } from "../../../../lib/middleware"; @@ -94,7 +94,7 @@ export function registerOwnershipRoutes(app: ExtensionsV2App): void { id, auth.userId, note, - githubTokenProvider(platform) + githubClientProvider(platform) ); if (error || !data) { return c.json( diff --git a/services/api/src/services/previews/v1/github/request.ts b/services/api/src/services/previews/v1/github/request.ts index 6258e29a..682588c7 100644 --- a/services/api/src/services/previews/v1/github/request.ts +++ b/services/api/src/services/previews/v1/github/request.ts @@ -1,12 +1,14 @@ -import { githubTokenProvider } from "../../../../lib/github-app"; +import { + githubClientProvider, + type GitHubClientProvider +} from "../../../../lib/github-app"; import { Context } from "hono"; -import { request } from "@octokit/request"; import { GitHubError } from "../../../../lib/github-errors"; import { getPlatform } from "../../../../lib/middleware"; import { sha256Hex } from "../../../../lib/hash"; export interface PreviewGitHub { - getToken: () => Promise; + getClient: GitHubClientProvider; reserve: () => Promise; } @@ -66,7 +68,7 @@ export function previewGitHub( const identity = clientBudgetIdentity(c.req.header("CF-Connecting-IP")); let remaining = MAX_REQUEST_SUBREQUESTS; return { - getToken: githubTokenProvider(getPlatform(c)), + getClient: githubClientProvider(getPlatform(c)), reserve: async () => { if (remaining === 0) return false; // Consume before awaiting so concurrent calls cannot exceed the ceiling. @@ -102,8 +104,6 @@ export async function previewRequest( "preview_budget_exhausted" ); } - return request(route, { - ...parameters, - headers: { authorization: `Bearer ${await github.getToken()}` } - }); + const client = await github.getClient(); + return client.request(route, parameters); } diff --git a/services/api/src/services/stats/v1/index.ts b/services/api/src/services/stats/v1/index.ts index 99cea171..94958dab 100644 --- a/services/api/src/services/stats/v1/index.ts +++ b/services/api/src/services/stats/v1/index.ts @@ -1,6 +1,6 @@ import { - githubTokenProvider, - type GitHubTokenProvider + githubClientProvider, + type GitHubClientProvider } from "../../../lib/github-app"; import { Hono } from "hono"; import { cors } from "hono/cors"; @@ -124,7 +124,7 @@ function aggregateStats(releases: Releases): StatsData { async function getStats( cache: ICache, - getGithubToken: GitHubTokenProvider, + getGithubClient: GitHubClientProvider, downloadBucket: R2Bucket, waitUntil?: (promise: Promise) => void ): Promise<{ @@ -150,7 +150,7 @@ async function getStats( // overwrite that cache with GitHub-only URLs for up to a day. releases = await getReleases( cache, - getGithubToken, + getGithubClient, downloadBucket, false, waitUntil @@ -227,7 +227,7 @@ registerCachedRoute("/data", async (c) => { const platform = getPlatform(c); const result = await getStats( platform.getCache("CACHE_KV"), - githubTokenProvider(platform), + githubClientProvider(platform), c.env.DOWNLOAD_BUCKET, platform.waitUntil ); diff --git a/services/api/src/services/versions/v1/index.ts b/services/api/src/services/versions/v1/index.ts index 95d3cc49..b752ab5d 100644 --- a/services/api/src/services/versions/v1/index.ts +++ b/services/api/src/services/versions/v1/index.ts @@ -1,14 +1,13 @@ import { - githubTokenProvider, - type GitHubTokenProvider + githubClientProvider, + type GitHubClient, + type GitHubClientProvider } from "../../../lib/github-app"; import { bearerAuth } from "hono/bearer-auth"; import { Hono, type Context, type Handler, type MiddlewareHandler } from "hono"; import { cors } from "hono/cors"; import { etag } from "hono/etag"; import { prettyJSON } from "hono/pretty-json"; -import { graphql } from "@octokit/graphql"; -import { request as ghRequest } from "@octokit/request"; import { trimTrailingSlash } from "hono/trailing-slash"; import { compare as semverCompare, @@ -177,7 +176,7 @@ async function loadReleases( const platform = getPlatform(c); return getReleases( platform.getCache("CACHE_KV"), - githubTokenProvider(platform), + githubClientProvider(platform), c.env.DOWNLOAD_BUCKET, updateCache, platform.waitUntil @@ -306,7 +305,7 @@ versionsV1.get( const platform = getPlatform(c); const result = await getReleases( platform.getCache("CACHE_KV"), - githubTokenProvider(platform), + githubClientProvider(platform), c.env.DOWNLOAD_BUCKET, true // No waitUntil: /update's contract is "refresh the cache", so the @@ -488,7 +487,7 @@ export function resetUpdateTokenCache() { export async function getReleases( cache: ICache, - getGithubToken: GitHubTokenProvider, + getGithubClient: GitHubClientProvider, downloadBucket: R2Bucket, updateCache: boolean = false, waitUntil?: (promise: Promise) => void @@ -519,15 +518,15 @@ export async function getReleases( // per isolate instead of stampeding GitHub with N copies of it. return singleFlight("versions:releases", async () => { try { - const githubToken = await getGithubToken(); - const result = await ghRequest("GET /repos/{owner}/{repo}/releases", { - owner: REPO_OWNER, - repo: REPO_NAME, - headers: { - Authorization: `Bearer ${githubToken}` - }, - per_page: 100 - }); + const github = await getGithubClient(); + const result = await github.request( + "GET /repos/{owner}/{repo}/releases", + { + owner: REPO_OWNER, + repo: REPO_NAME, + per_page: 100 + } + ); if (!Array.isArray(result.data)) { logWarn("versions", "Unexpected GitHub releases response format", { @@ -609,10 +608,7 @@ export async function getReleases( let batchPhpVersions = new Map(); if (releasesToFetch.length > 0) { try { - batchPhpVersions = await getBatchPhpVersions( - githubToken, - releasesToFetch - ); + batchPhpVersions = await getBatchPhpVersions(github, releasesToFetch); logInfo("versions", "Batch fetched PHP versions via GraphQL", { requested: releasesToFetch.length, resolved: batchPhpVersions.size @@ -863,7 +859,7 @@ interface PhpVersionBatchItem { } async function getBatchPhpVersions( - githubToken: string, + github: GitHubClient, releases: PhpVersionBatchItem[] ): Promise> { if (releases.length === 0) return new Map(); @@ -877,11 +873,9 @@ async function getBatchPhpVersions( const query = `{ repository(owner: "${REPO_OWNER}", name: "${REPO_NAME}") {\n${fields}\n} }`; - const data = await graphql<{ + const data = await github.graphql<{ repository: Record; - }>(query, { - headers: { authorization: `Bearer ${githubToken}` } - }); + }>(query); const result = new Map(); diff --git a/services/api/test/integration/app.test.ts b/services/api/test/integration/app.test.ts index ec4b7cbe..c6cafa2e 100644 --- a/services/api/test/integration/app.test.ts +++ b/services/api/test/integration/app.test.ts @@ -20,9 +20,9 @@ import { VersionInfo } from "../utils/test-types"; -vi.mock("../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../src/lib/github-app", async () => + (await import("../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/integration/github-app.test.ts b/services/api/test/integration/github-app.test.ts index e29c2524..03e57c6b 100644 --- a/services/api/test/integration/github-app.test.ts +++ b/services/api/test/integration/github-app.test.ts @@ -6,7 +6,7 @@ import { import { env } from "cloudflare:workers"; import { request } from "@octokit/request"; import app from "../../src/app/index"; -import { githubTokenProvider } from "../../src/lib/github-app"; +import { githubClientProvider } from "../../src/lib/github-app"; import { checkGithubEntity } from "../../src/services/extensions/v2/github/verification"; import { isolateEdgeCache } from "../utils/isolate-edge-cache"; import { suppressConsole } from "../utils/mock-helpers"; @@ -74,7 +74,7 @@ it("returns unavailable on an uncached release lookup without App credentials", it("does not turn an authentication outage into a missing developer", async () => { const result = await checkGithubEntity( "fossbilling", - githubTokenProvider({ + githubClientProvider({ getEnv: () => undefined }) ); diff --git a/services/api/test/integration/versions/index.test.ts b/services/api/test/integration/versions/index.test.ts index ce0233a8..09711416 100644 --- a/services/api/test/integration/versions/index.test.ts +++ b/services/api/test/integration/versions/index.test.ts @@ -20,9 +20,9 @@ import { VersionInfo } from "../../utils/test-types"; -vi.mock("../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../src/lib/github-app", async () => + (await import("../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/lib/github-app.test.ts b/services/api/test/lib/github-app.test.ts index 63c7e7f6..47218f7f 100644 --- a/services/api/test/lib/github-app.test.ts +++ b/services/api/test/lib/github-app.test.ts @@ -7,45 +7,54 @@ import { it, vi } from "vitest"; -import { githubTokenProvider } from "../../src/lib/github-app"; +import { createPrivateKey, generateKeyPairSync } from "node:crypto"; +import { githubClientProvider } from "../../src/lib/github-app"; -let privateKey: string; -let publicKey: CryptoKey; +let pkcs1: string; +let pkcs8: string; let sequence = 0; let settings: Record; const fetchMock = vi.fn(); const platform = { getEnv: (name: string) => settings[name] }; - -beforeAll(async () => { - const pair = (await crypto.subtle.generateKey( - { - name: "RSASSA-PKCS1-v1_5", - modulusLength: 2048, - publicExponent: new Uint8Array([1, 0, 1]), - hash: "SHA-256" - }, - true, - ["sign", "verify"] - )) as CryptoKeyPair; - publicKey = pair.publicKey; - const bytes = new Uint8Array( - (await crypto.subtle.exportKey("pkcs8", pair.privateKey)) as ArrayBuffer +const tokenRequests = () => + fetchMock.mock.calls.filter(([url]) => + String(url).endsWith("/access_tokens") ); - privateKey = `-----BEGIN PRIVATE KEY-----\n${btoa(String.fromCharCode(...bytes))}\n-----END PRIVATE KEY-----`; + +beforeAll(() => { + pkcs1 = generateKeyPairSync("rsa", { + modulusLength: 2048, + privateKeyEncoding: { type: "pkcs1", format: "pem" }, + publicKeyEncoding: { type: "spki", format: "pem" } + }).privateKey; + pkcs8 = createPrivateKey(pkcs1) + .export({ type: "pkcs8", format: "pem" }) + .toString(); }); beforeEach(() => { settings = { GITHUB_APP_CLIENT_ID: `test-client-${++sequence}`, GITHUB_APP_INSTALLATION_ID: "123", - GITHUB_APP_PRIVATE_KEY: privateKey + GITHUB_APP_PRIVATE_KEY: pkcs1 }; - fetchMock.mockReset().mockImplementation(async () => - Response.json({ - token: "installation-token", - expires_at: new Date(Date.now() + 3600_000).toISOString() - }) - ); + fetchMock.mockReset().mockImplementation(async (url, init) => { + if (String(url).endsWith("/access_tokens")) { + const body = JSON.parse(init?.body as string); + return Response.json({ + token: "installation-token", + expires_at: new Date(Date.now() + 3600_000).toISOString(), + permissions: body.permissions, + repository_selection: "selected" + }); + } + expect(new Headers(init?.headers).get("authorization")).toBe( + "token installation-token" + ); + return String(url).endsWith("/graphql") + ? Response.json({ data: { repository: { name: "FOSSBilling" } } }) + : Response.json({ login: "FOSSBilling" }); + }); vi.stubGlobal("fetch", fetchMock); }); @@ -54,82 +63,75 @@ afterEach(() => { vi.restoreAllMocks(); }); -describe("GitHub App authentication", () => { - it("lazily signs an App JWT and requests only the repository's read permissions", async () => { - const getToken = githubTokenProvider(platform); - expect(fetchMock).not.toHaveBeenCalled(); - expect(await getToken()).toBe("installation-token"); - const [url, init] = fetchMock.mock.calls[0]; - expect(url).toBe( - "https://api.github.com/app/installations/123/access_tokens" - ); - expect(init?.method).toBe("POST"); - expect(JSON.parse(init?.body as string)).toEqual({ - repositories: ["FOSSBilling"], - permissions: { contents: "read", actions: "read", pull_requests: "read" } - }); - const jwt = new Headers(init?.headers).get("authorization")!.slice(7); - const [header, payload, signature] = jwt.split("."); - const decode = (value: string) => - Uint8Array.from( - atob(value.replace(/-/g, "+").replace(/_/g, "/")), - (char) => char.charCodeAt(0) +describe("GitHub client", () => { + it.each(["PKCS#1", "PKCS#8"])( + "accepts %s PEMs and authenticates REST and GraphQL with restricted tokens", + async (format) => { + settings.GITHUB_APP_PRIVATE_KEY = + format === "PKCS#1" ? pkcs1 : pkcs8.replace(/\n/g, "\\n"); + const getClient = githubClientProvider(platform); + expect(fetchMock).not.toHaveBeenCalled(); + const github = await getClient(); + expect( + ( + await github.request("GET /users/{username}", { + username: "FOSSBilling" + }) + ).data.login + ).toBe("FOSSBilling"); + expect( + await github.graphql( + '{ repository(owner: "FOSSBilling", name: "FOSSBilling") { name } }' + ) + ).toEqual({ repository: { name: "FOSSBilling" } }); + expect(tokenRequests()).toHaveLength(1); + const [url, init] = tokenRequests()[0]; + expect(url).toBe( + "https://api.github.com/app/installations/123/access_tokens" ); - expect(JSON.parse(new TextDecoder().decode(decode(payload)))).toMatchObject( - { - iss: settings.GITHUB_APP_CLIENT_ID - } - ); - expect( - await crypto.subtle.verify( - "RSASSA-PKCS1-v1_5", - publicKey, - decode(signature), - new TextEncoder().encode(`${header}.${payload}`) - ) - ).toBe(true); - }); - - it("coalesces callers within a request and reuses resolved tokens across requests", async () => { - const getToken = githubTokenProvider(platform); - expect(await Promise.all([getToken(), getToken()])).toEqual([ - "installation-token", - "installation-token" - ]); - expect(await githubTokenProvider(platform)()).toBe("installation-token"); - expect(fetchMock).toHaveBeenCalledTimes(1); - }); + expect(init?.method).toBe("POST"); + expect(JSON.parse(init?.body as string)).toEqual({ + repositories: ["FOSSBilling"], + permissions: { + contents: "read", + actions: "read", + pull_requests: "read" + } + }); + } + ); - it("refreshes before expiry and invalidates the cache on key rotation", async () => { - await githubTokenProvider(platform)(); - const now = Date.now(); - vi.spyOn(Date, "now").mockReturnValue(now + 3541_000); - await githubTokenProvider(platform)(); - expect(fetchMock).toHaveBeenCalledTimes(2); - // Escaped-newline PEMs are accepted and a changed key value invalidates the cache. - settings.GITHUB_APP_PRIVATE_KEY = privateKey.replace(/\n/g, "\\n"); - await githubTokenProvider(platform)(); - expect(fetchMock).toHaveBeenCalledTimes(3); + it("reuses the request's client and Octokit's token cache until credentials change", async () => { + const getClient = githubClientProvider(platform); + const [first, second] = await Promise.all([getClient(), getClient()]); + expect(second).toBe(first); + await githubClientProvider(platform)(); + expect(tokenRequests()).toHaveLength(1); + settings.GITHUB_APP_PRIVATE_KEY = pkcs8; + await githubClientProvider(platform)(); + expect(tokenRequests()).toHaveLength(2); }); it("fails closed when credentials are missing", async () => { delete settings.GITHUB_APP_PRIVATE_KEY; - await expect(githubTokenProvider(platform)()).rejects.toMatchObject({ + await expect(githubClientProvider(platform)()).rejects.toMatchObject({ httpStatus: 503, message: "GitHub App credentials are not configured" }); expect(fetchMock).not.toHaveBeenCalled(); }); - it("sanitizes token failures, preserves unavailable semantics and retries on a new request", async () => { + it("sanitizes token failures and permits a retry on the next request", async () => { fetchMock.mockResolvedValueOnce( new Response("sensitive upstream response", { status: 404 }) ); - await expect(githubTokenProvider(platform)()).rejects.toMatchObject({ + await expect(githubClientProvider(platform)()).rejects.toMatchObject({ httpStatus: 503, message: "GitHub App authentication is unavailable" }); - expect(await githubTokenProvider(platform)()).toBe("installation-token"); - expect(fetchMock).toHaveBeenCalledTimes(2); + await expect(githubClientProvider(platform)()).resolves.toHaveProperty( + "request" + ); + expect(tokenRequests()).toHaveLength(2); }); }); diff --git a/services/api/test/mocks/octokit.ts b/services/api/test/mocks/octokit.ts index 0c711b03..4b47328e 100644 --- a/services/api/test/mocks/octokit.ts +++ b/services/api/test/mocks/octokit.ts @@ -13,3 +13,15 @@ export function octokitRequestMock() { }); return { request }; } + +// Feature suites supply REST/GraphQL responses; App authentication has its own +// tests against real Octokit clients and a mocked HTTP transport. +export function githubClientMock() { + return { + githubClientProvider: () => async () => ({ + request: (await import("@octokit/request")).request, + graphql: async (query: string, options?: Record) => + (await import("@octokit/graphql")).graphql(query, options) + }) + }; +} diff --git a/services/api/test/services/extensions/v2/account.test.ts b/services/api/test/services/extensions/v2/account.test.ts index 9d6bc3ae..542c4e69 100644 --- a/services/api/test/services/extensions/v2/account.test.ts +++ b/services/api/test/services/extensions/v2/account.test.ts @@ -43,9 +43,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/badges.test.ts b/services/api/test/services/extensions/v2/badges.test.ts index 76d639c9..c51f01a8 100644 --- a/services/api/test/services/extensions/v2/badges.test.ts +++ b/services/api/test/services/extensions/v2/badges.test.ts @@ -4,9 +4,9 @@ import { db, get, setupExtensionsV2Tests } from "./harness"; import { insertDeveloper, insertExtension } from "./db-fixtures"; import { wrapD1WithHook } from "./db-interceptor"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/claim-budget.test.ts b/services/api/test/services/extensions/v2/claim-budget.test.ts index 44056925..2b14280d 100644 --- a/services/api/test/services/extensions/v2/claim-budget.test.ts +++ b/services/api/test/services/extensions/v2/claim-budget.test.ts @@ -13,9 +13,9 @@ import { post } from "./harness"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/contract.test.ts b/services/api/test/services/extensions/v2/contract.test.ts index 28ae6ef5..fec193e1 100644 --- a/services/api/test/services/extensions/v2/contract.test.ts +++ b/services/api/test/services/extensions/v2/contract.test.ts @@ -14,9 +14,9 @@ import { import { getDeveloper, insertDeveloper, insertUser } from "./db-fixtures"; // Hoisted so no v2 suite can make a real GitHub call. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/developer-approval.test.ts b/services/api/test/services/extensions/v2/developer-approval.test.ts index 0b151a0c..c8a5050a 100644 --- a/services/api/test/services/extensions/v2/developer-approval.test.ts +++ b/services/api/test/services/extensions/v2/developer-approval.test.ts @@ -19,9 +19,9 @@ import { insertUser } from "./db-fixtures"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/developer-profiles.test.ts b/services/api/test/services/extensions/v2/developer-profiles.test.ts index 2a6eabfc..f1f1a04f 100644 --- a/services/api/test/services/extensions/v2/developer-profiles.test.ts +++ b/services/api/test/services/extensions/v2/developer-profiles.test.ts @@ -37,9 +37,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/directory-identity.test.ts b/services/api/test/services/extensions/v2/directory-identity.test.ts index 5d873778..24763f4c 100644 --- a/services/api/test/services/extensions/v2/directory-identity.test.ts +++ b/services/api/test/services/extensions/v2/directory-identity.test.ts @@ -11,9 +11,9 @@ import { setupExtensionsV2Tests } from "./harness"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/extension-writes.test.ts b/services/api/test/services/extensions/v2/extension-writes.test.ts index 359648db..1bb5bb06 100644 --- a/services/api/test/services/extensions/v2/extension-writes.test.ts +++ b/services/api/test/services/extensions/v2/extension-writes.test.ts @@ -29,9 +29,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/index.test.ts b/services/api/test/services/extensions/v2/index.test.ts index 9a261c18..1334259b 100644 --- a/services/api/test/services/extensions/v2/index.test.ts +++ b/services/api/test/services/extensions/v2/index.test.ts @@ -3,9 +3,9 @@ import { setupExtensionsV2Tests, get } from "./harness"; // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/manifest.test.ts b/services/api/test/services/extensions/v2/manifest.test.ts index 41aed446..51883b87 100644 --- a/services/api/test/services/extensions/v2/manifest.test.ts +++ b/services/api/test/services/extensions/v2/manifest.test.ts @@ -13,9 +13,9 @@ import { import { StoredExtensionContentSchema } from "../../../../src/services/extensions/v2/schemas/extensions"; import { insertExtension, insertUser } from "./db-fixtures"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/moderation-correct.test.ts b/services/api/test/services/extensions/v2/moderation-correct.test.ts index 336bb52b..a4bd6f3d 100644 --- a/services/api/test/services/extensions/v2/moderation-correct.test.ts +++ b/services/api/test/services/extensions/v2/moderation-correct.test.ts @@ -25,9 +25,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/moderation-notify.test.ts b/services/api/test/services/extensions/v2/moderation-notify.test.ts index fc5f74de..33022c8c 100644 --- a/services/api/test/services/extensions/v2/moderation-notify.test.ts +++ b/services/api/test/services/extensions/v2/moderation-notify.test.ts @@ -19,9 +19,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/moderation-revalidate.test.ts b/services/api/test/services/extensions/v2/moderation-revalidate.test.ts index 177b41be..c867386e 100644 --- a/services/api/test/services/extensions/v2/moderation-revalidate.test.ts +++ b/services/api/test/services/extensions/v2/moderation-revalidate.test.ts @@ -24,9 +24,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/moderation-revocation.test.ts b/services/api/test/services/extensions/v2/moderation-revocation.test.ts index 2ca89d9e..65895d0f 100644 --- a/services/api/test/services/extensions/v2/moderation-revocation.test.ts +++ b/services/api/test/services/extensions/v2/moderation-revocation.test.ts @@ -20,9 +20,9 @@ import { getDeveloperClaim } from "./db-fixtures"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/moderation.test.ts b/services/api/test/services/extensions/v2/moderation.test.ts index d81f70d9..c2cb7fd2 100644 --- a/services/api/test/services/extensions/v2/moderation.test.ts +++ b/services/api/test/services/extensions/v2/moderation.test.ts @@ -32,9 +32,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/ownership.test.ts b/services/api/test/services/extensions/v2/ownership.test.ts index 1839efc3..8e08ea20 100644 --- a/services/api/test/services/extensions/v2/ownership.test.ts +++ b/services/api/test/services/extensions/v2/ownership.test.ts @@ -33,9 +33,9 @@ import { // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/packages.test.ts b/services/api/test/services/extensions/v2/packages.test.ts index 6de44c09..c047d1bf 100644 --- a/services/api/test/services/extensions/v2/packages.test.ts +++ b/services/api/test/services/extensions/v2/packages.test.ts @@ -47,9 +47,9 @@ import { } from "./harness"; import { insertUser, getExtension, getDeveloper, UUID_V7 } from "./db-fixtures"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/pending-edits.test.ts b/services/api/test/services/extensions/v2/pending-edits.test.ts index 81967a34..9169697d 100644 --- a/services/api/test/services/extensions/v2/pending-edits.test.ts +++ b/services/api/test/services/extensions/v2/pending-edits.test.ts @@ -20,9 +20,9 @@ import { insertRevision, insertUnpublishedExtension } from "./db-fixtures"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/public-extensions.test.ts b/services/api/test/services/extensions/v2/public-extensions.test.ts index 5459ee8a..b25fc02b 100644 --- a/services/api/test/services/extensions/v2/public-extensions.test.ts +++ b/services/api/test/services/extensions/v2/public-extensions.test.ts @@ -5,9 +5,9 @@ import { insertDeveloper, insertExtension } from "./db-fixtures"; // Hoisted so no v2 suite can make a real GitHub call. harness.ts applies the // default "not found" behaviour in beforeEach and documents why. -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/publisher.test.ts b/services/api/test/services/extensions/v2/publisher.test.ts index a48c1c32..2770c64a 100644 --- a/services/api/test/services/extensions/v2/publisher.test.ts +++ b/services/api/test/services/extensions/v2/publisher.test.ts @@ -26,9 +26,9 @@ import { } from "../../../lib/auth/m2m-helper"; // Hoisted so no v2 suite can make a real GitHub call (see moderation.test.ts). -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/extensions/v2/resource-bounds.test.ts b/services/api/test/services/extensions/v2/resource-bounds.test.ts index c7dbb2af..af61fc2f 100644 --- a/services/api/test/services/extensions/v2/resource-bounds.test.ts +++ b/services/api/test/services/extensions/v2/resource-bounds.test.ts @@ -46,9 +46,9 @@ import { countExtensions, countRevisions } from "./db-fixtures"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/previews/v1/budget.test.ts b/services/api/test/services/previews/v1/budget.test.ts index 1f316479..e9c08f6d 100644 --- a/services/api/test/services/previews/v1/budget.test.ts +++ b/services/api/test/services/previews/v1/budget.test.ts @@ -14,9 +14,9 @@ import { previewRequest } from "../../../../src/services/previews/v1/github/request"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() @@ -252,11 +252,13 @@ describe("preview GitHub budget", () => { }); it("fails closed if reservation fails", async () => { - const getToken = vi.fn(async () => "test"); + const getClient = vi.fn(async () => { + throw new Error("Authentication must not run without a budget"); + }); await expect( previewRequest( { - getToken, + getClient, reserve: async () => { throw new Error("budget unavailable"); } @@ -265,7 +267,7 @@ describe("preview GitHub budget", () => { {} ) ).rejects.toThrow("Preview GitHub budget unavailable"); - expect(getToken).not.toHaveBeenCalled(); + expect(getClient).not.toHaveBeenCalled(); expect(request).not.toHaveBeenCalled(); }); }); diff --git a/services/api/test/services/previews/v1/commit.test.ts b/services/api/test/services/previews/v1/commit.test.ts index 3e023ba5..de731bd0 100644 --- a/services/api/test/services/previews/v1/commit.test.ts +++ b/services/api/test/services/previews/v1/commit.test.ts @@ -9,9 +9,9 @@ import app from "../../../../src/app/index"; import { MockGitHubRequest } from "../../../utils/test-types"; import { suppressConsole } from "../../../utils/mock-helpers"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/previews/v1/main.test.ts b/services/api/test/services/previews/v1/main.test.ts index 6b15f41e..7b889fcc 100644 --- a/services/api/test/services/previews/v1/main.test.ts +++ b/services/api/test/services/previews/v1/main.test.ts @@ -9,9 +9,9 @@ import app from "../../../../src/app/index"; import { MockGitHubRequest } from "../../../utils/test-types"; import { suppressConsole } from "../../../utils/mock-helpers"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/previews/v1/pr.test.ts b/services/api/test/services/previews/v1/pr.test.ts index 571230e9..cd4ed127 100644 --- a/services/api/test/services/previews/v1/pr.test.ts +++ b/services/api/test/services/previews/v1/pr.test.ts @@ -9,9 +9,9 @@ import app from "../../../../src/app/index"; import { MockGitHubRequest } from "../../../utils/test-types"; import { suppressConsole } from "../../../utils/mock-helpers"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/stats/v1/index.test.ts b/services/api/test/services/stats/v1/index.test.ts index 66ec367b..bfae4089 100644 --- a/services/api/test/services/stats/v1/index.test.ts +++ b/services/api/test/services/stats/v1/index.test.ts @@ -25,9 +25,9 @@ import { request as ghRequest } from "@octokit/request"; import { graphql } from "@octokit/graphql"; import { ApiResponse } from "../../../utils/test-types"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/versions/v1/errors.test.ts b/services/api/test/services/versions/v1/errors.test.ts index 0c6d4027..77a1606d 100644 --- a/services/api/test/services/versions/v1/errors.test.ts +++ b/services/api/test/services/versions/v1/errors.test.ts @@ -21,9 +21,9 @@ import { VersionInfo } from "../../../utils/test-types"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/versions/v1/index.test.ts b/services/api/test/services/versions/v1/index.test.ts index a132762a..65d46704 100644 --- a/services/api/test/services/versions/v1/index.test.ts +++ b/services/api/test/services/versions/v1/index.test.ts @@ -29,9 +29,9 @@ import { // Edge entries are cleared between tests to exercise live handlers. const PUBLIC_HEADERS = { authorization: "test-bypass-cache" } as const; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/test/services/versions/v1/middleware.test.ts b/services/api/test/services/versions/v1/middleware.test.ts index 3cd7b741..f4814828 100644 --- a/services/api/test/services/versions/v1/middleware.test.ts +++ b/services/api/test/services/versions/v1/middleware.test.ts @@ -19,9 +19,9 @@ import { MockGitHubRequest } from "../../../utils/test-types"; -vi.mock("../../../../src/lib/github-app", () => ({ - githubTokenProvider: () => async () => "test-github-token" -})); +vi.mock("../../../../src/lib/github-app", async () => + (await import("../../../mocks/octokit")).githubClientMock() +); vi.mock("@octokit/request", async () => (await import("../../../mocks/octokit")).octokitRequestMock() diff --git a/services/api/vitest.config.ts b/services/api/vitest.config.ts index 16108dc6..508462c3 100644 --- a/services/api/vitest.config.ts +++ b/services/api/vitest.config.ts @@ -14,6 +14,8 @@ const centralAlertsMigrations = await readD1Migrations( ); export default defineConfig({ + // Match Wrangler: use Node-capable dependencies in the Workers runtime. + resolve: { conditions: ["workerd", "worker", "node"] }, plugins: [ cloudflareTest({ wrangler: { configPath: "./wrangler.jsonc" }, diff --git a/services/api/vitest.node.config.ts b/services/api/vitest.node.config.ts index bcafa05e..b005160a 100644 --- a/services/api/vitest.node.config.ts +++ b/services/api/vitest.node.config.ts @@ -15,7 +15,7 @@ export default defineConfig({ coverage: { provider: "istanbul", reporter: ["text", "json", "html"], - include: ["test/lib/github-app.test.ts", "src/lib/adapters/node/**"] + include: ["src/lib/github-app.ts", "src/lib/adapters/node/**"] } } }); From 5f210506f56ae33307cd850ec829c717be351a7b Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 22:55:11 +0000 Subject: [PATCH 5/9] Preserve GitHub request timeouts in the shared transport --- services/api/src/lib/github-app.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/api/src/lib/github-app.ts b/services/api/src/lib/github-app.ts index 84b93993..c65996a2 100644 --- a/services/api/src/lib/github-app.ts +++ b/services/api/src/lib/github-app.ts @@ -8,8 +8,11 @@ export type GitHubClient = Pick; export type GitHubClientProvider = () => Promise; const transport = { - fetch: (...args: Parameters) => globalThis.fetch(...args), - timeout: 10_000 + fetch: ((input, init) => + globalThis.fetch(input, { + ...init, + signal: init?.signal ?? AbortSignal.timeout(10_000) + })) satisfies typeof fetch }; // Reuse Octokit's installation-token cache for the active credentials. Replacing From bc87448e84c3f02d92b6d577de5d30f563110a80 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 23:13:40 +0000 Subject: [PATCH 6/9] Tighten GitHub auth tests and rollout guidance --- services/api/README.md | 17 ++++++----------- services/api/test/lib/github-app.test.ts | 23 +++++++++-------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/services/api/README.md b/services/api/README.md index 4b716f29..17b1594c 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -135,17 +135,12 @@ variables or a static installation token. Use the full downloaded PEM; no manual key conversion is needed. Approve the installation's permission changes before deployment. -A shared Octokit REST/GraphQL client uses App installation tokens restricted to -`FOSSBilling/FOSSBilling` and those three read permissions. Octokit manages token -caching and refresh; credential changes replace its auth instance and cache. -Wrangler builds and the Workers test runner select Node-compatible dependency -exports (`workerd,worker,node`) so Octokit accepts GitHub's downloaded PKCS#1 PEM -directly, just as it does in Node. PKCS#8 keys also work. Authentication is -lazy: cached responses and profile operations without a GitHub lookup do not mint -tokens. Authentication failures use the existing unavailable/stale-cache paths; -they never become a missing GitHub user or anonymous lookup. There are no runtime -calls to a secrets manager. Remove the old runtime `GITHUB_TOKEN` after setting the -App credentials; GitHub Actions' built-in `GITHUB_TOKEN` is unrelated. +The GitHub client requests installation tokens restricted to `FOSSBilling/FOSSBilling` +and the three read permissions above. Octokit handles token caching and refresh; +authentication runs only for live GitHub lookups. Wrangler builds and Workers tests +select `workerd,worker,node` dependency exports to use Octokit's native Node key +handling. After deploying and verifying App authentication, remove the old runtime +`GITHUB_TOKEN`. GitHub Actions' built-in token is separate and should remain. ## Development diff --git a/services/api/test/lib/github-app.test.ts b/services/api/test/lib/github-app.test.ts index 47218f7f..9a4e9276 100644 --- a/services/api/test/lib/github-app.test.ts +++ b/services/api/test/lib/github-app.test.ts @@ -21,12 +21,16 @@ const tokenRequests = () => String(url).endsWith("/access_tokens") ); -beforeAll(() => { - pkcs1 = generateKeyPairSync("rsa", { +function generateTestKey() { + return generateKeyPairSync("rsa", { modulusLength: 2048, privateKeyEncoding: { type: "pkcs1", format: "pem" }, publicKeyEncoding: { type: "spki", format: "pem" } }).privateKey; +} + +beforeAll(() => { + pkcs1 = generateTestKey(); pkcs8 = createPrivateKey(pkcs1) .export({ type: "pkcs8", format: "pem" }) .toString(); @@ -48,8 +52,8 @@ beforeEach(() => { repository_selection: "selected" }); } - expect(new Headers(init?.headers).get("authorization")).toBe( - "token installation-token" + expect(new Headers(init?.headers).get("authorization")).toMatch( + /^(?:token|bearer) installation-token$/i ); return String(url).endsWith("/graphql") ? Response.json({ data: { repository: { name: "FOSSBilling" } } }) @@ -107,20 +111,11 @@ describe("GitHub client", () => { expect(second).toBe(first); await githubClientProvider(platform)(); expect(tokenRequests()).toHaveLength(1); - settings.GITHUB_APP_PRIVATE_KEY = pkcs8; + settings.GITHUB_APP_PRIVATE_KEY = generateTestKey(); await githubClientProvider(platform)(); expect(tokenRequests()).toHaveLength(2); }); - it("fails closed when credentials are missing", async () => { - delete settings.GITHUB_APP_PRIVATE_KEY; - await expect(githubClientProvider(platform)()).rejects.toMatchObject({ - httpStatus: 503, - message: "GitHub App credentials are not configured" - }); - expect(fetchMock).not.toHaveBeenCalled(); - }); - it("sanitizes token failures and permits a retry on the next request", async () => { fetchMock.mockResolvedValueOnce( new Response("sensitive upstream response", { status: 404 }) From f68a68cb7077ee978200f74d6b281075ef34b8c8 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 23:16:50 +0000 Subject: [PATCH 7/9] Address GitHub App test and setup review findings --- package-lock.json | 26 +++++++++++++++++++ services/api/README.md | 2 +- services/api/package.json | 7 ++--- .../api/test/integration/github-app.test.ts | 13 ---------- services/api/test/lib/github-app.test.ts | 13 ++++++++++ 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9748d1ab..9234e9b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -444,6 +444,13 @@ "tslib": "^2.4.0" } }, + "node_modules/@epic-web/invariant": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@epic-web/invariant/-/invariant-1.0.0.tgz", + "integrity": "sha512-lrTPqgvfFQtR/eY/qkIzp98OGdNJu0m5ji3q/nJI8v3SXkRKEnWiOxMmbvcSoAIzv/cGiuvRy57k4suKQSAdwA==", + "dev": true, + "license": "MIT" + }, "node_modules/@esbuild-kit/core-utils": { "version": "3.3.2", "resolved": "https://registry.npmjs.org/@esbuild-kit/core-utils/-/core-utils-3.3.2.tgz", @@ -3625,6 +3632,24 @@ "url": "https://opencollective.com/express" } }, + "node_modules/cross-env": { + "version": "10.1.0", + "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-10.1.0.tgz", + "integrity": "sha512-GsYosgnACZTADcmEyJctkJIoqAhHjttw7RsFrVoJNXbsWWqaq6Ym+7kZjq6mS45O0jij6vtiReppKQEtqWy6Dw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@epic-web/invariant": "^1.0.0", + "cross-spawn": "^7.0.6" + }, + "bin": { + "cross-env": "dist/bin/cross-env.js", + "cross-env-shell": "dist/bin/cross-env-shell.js" + }, + "engines": { + "node": ">=20" + } + }, "node_modules/cross-spawn": { "version": "7.0.6", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", @@ -6808,6 +6833,7 @@ "@octokit/graphql": "9.0.5", "@types/semver": "7.8.0", "@zip.js/zip.js": "2.24.0", + "cross-env": "10.1.0", "drizzle-kit": "0.31.11", "wrangler": "4.158.0" } diff --git a/services/api/README.md b/services/api/README.md index 17b1594c..7308f578 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -146,7 +146,7 @@ handling. After deploying and verifying App authentication, remove the old runti ### Local Setup -1. Copy `services/api/.dev.vars.example` to `services/api/.dev.vars` and set your runtime variables and secrets: +1. Copy `.dev.vars.example` to `.dev.vars` and set your runtime variables and secrets: The example contains placeholders only. `cf-typegen` uses it to generate stable binding types without reading local credentials. diff --git a/services/api/package.json b/services/api/package.json index c7987f06..7b495664 100644 --- a/services/api/package.json +++ b/services/api/package.json @@ -6,8 +6,8 @@ "license": "AGPL-3.0-only", "scripts": { "cf-typegen": "wrangler types --env-interface CloudflareBindings --env-file .dev.vars.example", - "cf-deploy": "npm run db:migrate:all:remote && WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler deploy --minify", - "dev": "WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler dev", + "cf-deploy": "npm run db:migrate:all:remote && cross-env WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler deploy --minify", + "dev": "cross-env WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler dev", "db:generate:extensions": "drizzle-kit generate --config drizzle.extensions.config.ts", "db:generate:central-alerts": "drizzle-kit generate --config drizzle.central-alerts.config.ts", "db:migrate:extensions-v2:local": "wrangler d1 migrations apply DB_EXTENSIONS --local", @@ -21,7 +21,7 @@ "test:coverage": "vitest run --coverage", "typecheck": "tsc --noEmit", "test:workers": "vitest run", - "build": "WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler deploy --dry-run --outdir dist --minify && node scripts/prepare-deployment.ts", + "build": "cross-env WRANGLER_BUILD_CONDITIONS=workerd,worker,node wrangler deploy --dry-run --outdir dist --minify && node scripts/prepare-deployment.ts", "cf-preview": "wrangler preview --config dist/wrangler.json" }, "dependencies": { @@ -45,6 +45,7 @@ "@octokit/graphql": "9.0.5", "@types/semver": "7.8.0", "@zip.js/zip.js": "2.24.0", + "cross-env": "10.1.0", "drizzle-kit": "0.31.11", "wrangler": "4.158.0" } diff --git a/services/api/test/integration/github-app.test.ts b/services/api/test/integration/github-app.test.ts index 03e57c6b..79900648 100644 --- a/services/api/test/integration/github-app.test.ts +++ b/services/api/test/integration/github-app.test.ts @@ -6,8 +6,6 @@ import { import { env } from "cloudflare:workers"; import { request } from "@octokit/request"; import app from "../../src/app/index"; -import { githubClientProvider } from "../../src/lib/github-app"; -import { checkGithubEntity } from "../../src/services/extensions/v2/github/verification"; import { isolateEdgeCache } from "../utils/isolate-edge-cache"; import { suppressConsole } from "../utils/mock-helpers"; @@ -70,14 +68,3 @@ it("returns unavailable on an uncached release lookup without App credentials", expect(response.status).toBe(503); expect(request).not.toHaveBeenCalled(); }); - -it("does not turn an authentication outage into a missing developer", async () => { - const result = await checkGithubEntity( - "fossbilling", - githubClientProvider({ - getEnv: () => undefined - }) - ); - expect(result.status).toBe("unavailable"); - expect(request).not.toHaveBeenCalled(); -}); diff --git a/services/api/test/lib/github-app.test.ts b/services/api/test/lib/github-app.test.ts index 9a4e9276..c99913b5 100644 --- a/services/api/test/lib/github-app.test.ts +++ b/services/api/test/lib/github-app.test.ts @@ -9,6 +9,7 @@ import { } from "vitest"; import { createPrivateKey, generateKeyPairSync } from "node:crypto"; import { githubClientProvider } from "../../src/lib/github-app"; +import { checkGithubEntity } from "../../src/services/extensions/v2/github/verification"; let pkcs1: string; let pkcs8: string; @@ -116,6 +117,18 @@ describe("GitHub client", () => { expect(tokenRequests()).toHaveLength(2); }); + it("keeps a token-endpoint 404 unavailable during developer verification", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + fetchMock.mockResolvedValueOnce(new Response(null, { status: 404 })); + const result = await checkGithubEntity( + "fossbilling", + githubClientProvider(platform) + ); + expect(result.status).toBe("unavailable"); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(tokenRequests()).toHaveLength(1); + }); + it("sanitizes token failures and permits a retry on the next request", async () => { fetchMock.mockResolvedValueOnce( new Response("sensitive upstream response", { status: 404 }) From 9a35ff18da9a2da47d2c397745f7409d129b46a7 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 12 Nov 2026 23:51:44 +0000 Subject: [PATCH 8/9] Trim GitHub App setup and key format tests --- services/api/README.md | 34 +++-------- services/api/test/lib/github-app.test.ts | 73 +++++++++++------------- 2 files changed, 41 insertions(+), 66 deletions(-) diff --git a/services/api/README.md b/services/api/README.md index 7308f578..9937b9f5 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -99,7 +99,7 @@ Package storage configuration is listed in [PACKAGING.md](src/services/extension - `GITHUB_APP_CLIENT_ID`: Client ID of the FOSSBilling Integrations GitHub App. - `GITHUB_APP_INSTALLATION_ID`: Its installation ID in the FOSSBilling organization. -- `GITHUB_APP_PRIVATE_KEY`: App signing key (PEM), stored as a secret. GitHub's downloaded PKCS#1 PEM and PKCS#8 are accepted, with real newlines or escaped `\n`. +- `GITHUB_APP_PRIVATE_KEY`: GitHub's downloaded App private key (PEM), stored as a secret. - `ASSERTION_SIGNING_SECRET`: Shared HMAC secret used to verify the short-lived bearer assertions minted by the Extensions site. Configure the same value in both Workers; it is never sent to clients. - `ASSERTION_SIGNING_SECRET_PREVIOUS`: Optional previous HMAC secret accepted during a signing-key rotation. - `EXTENSIONS_REVALIDATE_SECRET`: Bearer token the API sends to the Extensions site's `POST /api/revalidate` (over the `EXTENSIONS_FRONTEND` service binding) to purge the site's CDN-cached extension pages after content mutations. Configure the same value in both Workers; it is never sent to clients. @@ -117,30 +117,14 @@ Stats also use the GitHub App credentials. For the assertion format and the rota ### GitHub App authentication -Install **FOSSBilling Integrations** on `FOSSBilling/FOSSBilling` with repository -**Contents**, **Actions** and **Pull requests** permissions set to **Read-only** -(Metadata read is automatic). Contents covers releases and compatibility files; -Actions covers preview artifacts and runs; Pull requests resolves preview heads. -Developer checks read public GitHub profiles; identity and organization ownership -still come from the caller's linked identity in the auth service. - -Use separate API preview and production private keys so they can be rotated -independently. Use the preview key locally and in Previews Base, and the production -key in the API Worker's production settings. Both use the same App client and -installation IDs; separate keys still have the same App authority. - -Set the three `GITHUB_APP_*` values as runtime variables/secrets in each deployment -that needs GitHub access, and in `services/api/.dev.vars` locally. Do not use build -variables or a static installation token. Use the full downloaded PEM; no manual -key conversion is needed. Approve the installation's permission changes before -deployment. - -The GitHub client requests installation tokens restricted to `FOSSBilling/FOSSBilling` -and the three read permissions above. Octokit handles token caching and refresh; -authentication runs only for live GitHub lookups. Wrangler builds and Workers tests -select `workerd,worker,node` dependency exports to use Octokit's native Node key -handling. After deploying and verifying App authentication, remove the old runtime -`GITHUB_TOKEN`. GitHub Actions' built-in token is separate and should remain. +Install **FOSSBilling Integrations** on `FOSSBilling/FOSSBilling` with **Contents**, +**Actions** and **Pull requests** permissions set to **Read-only**. Set the three +`GITHUB_APP_*` runtime values above, using GitHub's downloaded PEM without conversion. +Use the preview key locally and in Previews Base, and a separate production key. + +Octokit manages short-lived installation tokens, restricted to that repository and +those read permissions. Remove the old runtime `GITHUB_TOKEN` after deployment is +verified; GitHub Actions' built-in token remains unchanged. ## Development diff --git a/services/api/test/lib/github-app.test.ts b/services/api/test/lib/github-app.test.ts index c99913b5..43fb023a 100644 --- a/services/api/test/lib/github-app.test.ts +++ b/services/api/test/lib/github-app.test.ts @@ -7,12 +7,11 @@ import { it, vi } from "vitest"; -import { createPrivateKey, generateKeyPairSync } from "node:crypto"; +import { generateKeyPairSync } from "node:crypto"; import { githubClientProvider } from "../../src/lib/github-app"; import { checkGithubEntity } from "../../src/services/extensions/v2/github/verification"; let pkcs1: string; -let pkcs8: string; let sequence = 0; let settings: Record; const fetchMock = vi.fn(); @@ -32,9 +31,6 @@ function generateTestKey() { beforeAll(() => { pkcs1 = generateTestKey(); - pkcs8 = createPrivateKey(pkcs1) - .export({ type: "pkcs8", format: "pem" }) - .toString(); }); beforeEach(() => { @@ -69,42 +65,37 @@ afterEach(() => { }); describe("GitHub client", () => { - it.each(["PKCS#1", "PKCS#8"])( - "accepts %s PEMs and authenticates REST and GraphQL with restricted tokens", - async (format) => { - settings.GITHUB_APP_PRIVATE_KEY = - format === "PKCS#1" ? pkcs1 : pkcs8.replace(/\n/g, "\\n"); - const getClient = githubClientProvider(platform); - expect(fetchMock).not.toHaveBeenCalled(); - const github = await getClient(); - expect( - ( - await github.request("GET /users/{username}", { - username: "FOSSBilling" - }) - ).data.login - ).toBe("FOSSBilling"); - expect( - await github.graphql( - '{ repository(owner: "FOSSBilling", name: "FOSSBilling") { name } }' - ) - ).toEqual({ repository: { name: "FOSSBilling" } }); - expect(tokenRequests()).toHaveLength(1); - const [url, init] = tokenRequests()[0]; - expect(url).toBe( - "https://api.github.com/app/installations/123/access_tokens" - ); - expect(init?.method).toBe("POST"); - expect(JSON.parse(init?.body as string)).toEqual({ - repositories: ["FOSSBilling"], - permissions: { - contents: "read", - actions: "read", - pull_requests: "read" - } - }); - } - ); + it("accepts GitHub's downloaded PEM and authenticates REST and GraphQL with restricted tokens", async () => { + const getClient = githubClientProvider(platform); + expect(fetchMock).not.toHaveBeenCalled(); + const github = await getClient(); + expect( + ( + await github.request("GET /users/{username}", { + username: "FOSSBilling" + }) + ).data.login + ).toBe("FOSSBilling"); + expect( + await github.graphql( + '{ repository(owner: "FOSSBilling", name: "FOSSBilling") { name } }' + ) + ).toEqual({ repository: { name: "FOSSBilling" } }); + expect(tokenRequests()).toHaveLength(1); + const [url, init] = tokenRequests()[0]; + expect(url).toBe( + "https://api.github.com/app/installations/123/access_tokens" + ); + expect(init?.method).toBe("POST"); + expect(JSON.parse(init?.body as string)).toEqual({ + repositories: ["FOSSBilling"], + permissions: { + contents: "read", + actions: "read", + pull_requests: "read" + } + }); + }); it("reuses the request's client and Octokit's token cache until credentials change", async () => { const getClient = githubClientProvider(platform); From 011dadd41090790dca9713dc30b6fc4625c6ee8e Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Fri, 13 Nov 2026 00:14:31 +0000 Subject: [PATCH 9/9] Document separate API development signing key --- services/api/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/api/README.md b/services/api/README.md index 9937b9f5..a7f13fe6 100644 --- a/services/api/README.md +++ b/services/api/README.md @@ -120,7 +120,9 @@ Stats also use the GitHub App credentials. For the assertion format and the rota Install **FOSSBilling Integrations** on `FOSSBilling/FOSSBilling` with **Contents**, **Actions** and **Pull requests** permissions set to **Read-only**. Set the three `GITHUB_APP_*` runtime values above, using GitHub's downloaded PEM without conversion. -Use the preview key locally and in Previews Base, and a separate production key. +Use separate keys: `api-dev.private-key` for local development, +`api-preview.private-key` for Previews Base, and `api-production.private-key` for +production. All use the same App client and installation IDs. Octokit manages short-lived installation tokens, restricted to that repository and those read permissions. Remove the old runtime `GITHUB_TOKEN` after deployment is