diff --git a/.dev.vars.example b/.dev.vars.example index d5a4b14a..953dad2b 100644 --- a/.dev.vars.example +++ b/.dev.vars.example @@ -10,13 +10,17 @@ NUXT_OG_IMAGE_SECRET= # Signs same-origin image proxy addresses. Keep it separate from NUXT_TOKEN_KEY. NUXT_IMAGE_PROXY_KEY= NUXT_AI_READY_RUNTIME_SYNC_SECRET= +# Every Worker GitHub read uses the read App (SKILLD_READ_APP_*): metadata read +# only, installed on skilld-dev, with a quota nothing else spends. The key is a +# PKCS #8 PEM. Without the App secrets, as in local development, reads use +# GITHUB_TOKEN. +# +# With the App set, GITHUB_TOKEN is a fallback only: it repeats a read GitHub +# denied the App, such as an organization that restricts Apps. Builds try +# ARTIFACT_GITHUB_TOKEN first. Each fallback read is logged as an app-denied event. +# Both may stay empty. GITHUB_TOKEN= -# Public Artifact builds read GitHub with this token when it is set. Use a token -# from another GitHub account, so runs keep a quota the registry sync cannot spend. ARTIFACT_GITHUB_TOKEN= -# Public Artifact builds read GitHub as the read App: metadata read only, -# installed on skilld-dev. Its installation has a quota nothing else spends. -# The key is a PKCS #8 PEM. Without these, builds use the tokens above. SKILLD_READ_APP_ID= SKILLD_READ_APP_INSTALLATION_ID= SKILLD_READ_APP_PRIVATE_KEY_PKCS8= diff --git a/docs/ops/artifact-delivery-rollout.md b/docs/ops/artifact-delivery-rollout.md index 16e2ad09..eddf17dc 100644 --- a/docs/ops/artifact-delivery-rollout.md +++ b/docs/ops/artifact-delivery-rollout.md @@ -111,17 +111,17 @@ Preserve these existing values: - `NUXT_OAUTH_GITHUB_CLIENT_ID` - `NUXT_OAUTH_GITHUB_CLIENT_SECRET` - `NUXT_TOKEN_KEY` -- `GITHUB_TOKEN` -The existing `GITHUB_TOKEN` reads public [GitHub](https://github.com) sources for the registry sync and `/gh` pages. +Every Worker GitHub read uses the read App `skilld-dev-registry-reads`: the registry sync, repository maintenance, mention checks, `/gh` pages and public Artifact builds. Set `SKILLD_READ_APP_ID`, `SKILLD_READ_APP_INSTALLATION_ID` and `SKILLD_READ_APP_PRIVATE_KEY_PKCS8` (a PKCS #8 PEM). The App has metadata read permission only, and its installation has its own quota. -Public Artifact builds read GitHub with a credential of their own, in this order: +Personal tokens are fallbacks only. They repeat a read GitHub denied the App, such as an organization that restricts Apps: -1. The read App `skilld-dev-registry-reads`: `SKILLD_READ_APP_ID`, `SKILLD_READ_APP_INSTALLATION_ID` and `SKILLD_READ_APP_PRIVATE_KEY_PKCS8` (a PKCS #8 PEM). It has metadata read permission only, and its installation has its own quota. -2. `ARTIFACT_GITHUB_TOKEN`. -3. `GITHUB_TOKEN`. +1. Builds try `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`, then the token of the signed-in account that asked for the run. +2. Every other caller tries `GITHUB_TOKEN`. -If the App secrets are set but unusable, builds read with the next credential and emit an `artifact-github-credential` event with the reason. +Each denial emits an `app-denied` event: `github-credential` for registry reads and `artifact-github-credential` for builds. The `github.credential` field names the fallback, or `none`. Without a fallback, the read fails for that Repository only: a build answers `SOURCE_ACCESS_DENIED`, and the sync records a failure without pausing. + +If the App secrets are set but unusable, reads use the fallback and emit an event with the reason. Keep the read App apart from the `GITHUB_APP_*` secrets below. Those belong to private delivery. Changing `NUXT_TOKEN_KEY` breaks stored GitHub credentials. diff --git a/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts b/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts index 631a6983..fdbc3bff 100644 --- a/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts +++ b/layers/artifact-delivery/server/api/v1/resolutions/index.post.ts @@ -49,6 +49,7 @@ export default withArtifactProblems(defineApiHandler({ repositoryId: privateAccess.repositoryId, } : { visibility: 'public' }, + requesterAccountId: user?.id ?? null, }) if (result._tag === 'idempotency-conflict') { throw createError({ diff --git a/layers/artifact-delivery/server/utils/build.ts b/layers/artifact-delivery/server/utils/build.ts index 7cfb71d5..2d3cce45 100644 --- a/layers/artifact-delivery/server/utils/build.ts +++ b/layers/artifact-delivery/server/utils/build.ts @@ -29,6 +29,7 @@ import { import { checkArtifactSource, checksBlockArtifact, checksPermitSigning } from './checks' import { canonicalJson, digestHex } from './encoding' import { isRetryableProblem, storedFilesPassLoadRules } from './github-source' +import { withRequesterFallback } from './requester-github' import { ARTIFACT_POLICY_VERSION, BYTE_COMPATIBLE_POLICY_VERSIONS, @@ -46,6 +47,12 @@ export interface ArtifactBuildDependencies { db: D1Database github: PublicGithubSourceClient privateGithub?: (row: ResolutionRow) => Promise + /** + * The GitHub client of the signed-in account that asked for a public + * Resolution, or null. A build uses it only for a read the shared + * credential's quota refused, and only for that account's own Resolution. + */ + requesterGithub?: (row: ResolutionRow) => Promise bucket: R2Bucket privateArtifacts?: { put: (input: { @@ -824,8 +831,12 @@ async function githubForResolution( dependencies: ArtifactBuildDependencies, row: ResolutionRow, ): Promise { - if (row.visibility === 'public') - return dependencies.github + if (row.visibility === 'public') { + const requesterGithub = dependencies.requesterGithub + return requesterGithub + ? withRequesterFallback(dependencies.github, () => requesterGithub(row)) + : dependencies.github + } if (!dependencies.privateGithub) { return { _tag: 'rejected', diff --git a/layers/artifact-delivery/server/utils/github-app.ts b/layers/artifact-delivery/server/utils/github-app.ts index f6a7d541..b8b7d695 100644 --- a/layers/artifact-delivery/server/utils/github-app.ts +++ b/layers/artifact-delivery/server/utils/github-app.ts @@ -1,5 +1,6 @@ import { z } from 'zod' import { decryptToken, encryptToken } from '#layers/identity/server/utils/crypto' +import { requestInstallationToken, signGithubAppJwt } from '#shared/server/github-app-credential' import { base64ToBytes, bytesToBase64Url } from './encoding' import { fetchNoRedirect } from './fetch-no-redirect' @@ -48,11 +49,6 @@ const githubUserSchema = z.object({ login: z.string().min(1).max(100), }).passthrough() -const installationTokenSchema = z.object({ - token: z.string().min(1).max(2048), - expires_at: z.string().datetime(), -}).passthrough() - const refreshedUserTokenSchema = z.object({ access_token: z.string().min(1).max(2048), expires_in: z.number().int().positive(), @@ -541,50 +537,6 @@ export async function verifyGithubWebhookSignature( return different === 0 } -export type InstallationTokenResult - = | { _tag: 'created', token: string, expiresAt: string } - /** GitHub answered 401, 403 or 404: the App, its key or the installation is gone. */ - | { _tag: 'refused', status: number } - -/** - * Exchange an App JWT for an installation access token. - * - * A refusal is a value. Any other failure, such as a 5xx or an unreadable - * answer, throws: it says nothing about the App. - */ -export async function requestInstallationToken(input: { - fetch: typeof globalThis.fetch - jwt: string - installationId: number - body: { repository_ids?: number[], permissions: Record } -}): Promise { - const fetched = await fetchNoRedirect(input.fetch, `${GITHUB_API}/app/installations/${input.installationId}/access_tokens`, { - method: 'POST', - headers: { - 'Accept': 'application/vnd.github+json', - 'Authorization': `Bearer ${input.jwt}`, - 'Content-Type': 'application/json', - 'User-Agent': 'skilld.dev', - 'X-GitHub-Api-Version': GITHUB_API_VERSION, - }, - body: JSON.stringify(input.body), - signal: AbortSignal.timeout(GITHUB_REQUEST_TIMEOUT_MS), - }) - if (fetched._tag === 'unexpected-redirect') - throw new Error(`GitHub App request redirected ${fetched.status}`) - const response = fetched.response - if (response.status === 401 || response.status === 403 || response.status === 404) { - await response.body?.cancel() - return { _tag: 'refused', status: response.status } - } - if (!response.ok) - throw new Error(`GitHub App request returned ${response.status}`) - const value = installationTokenSchema.safeParse(await readBoundedJson(response, MAX_GITHUB_TOKEN_RESPONSE_BYTES)) - if (!value.success) - throw new Error('GitHub App returned an invalid response') - return { _tag: 'created', token: value.data.token, expiresAt: value.data.expires_at } -} - async function createGithubAppJwt(config: GithubAppConfig): Promise { const privateKey = decodeCanonicalBase64Url(config.privateKeyPkcs8) if (!privateKey) @@ -592,42 +544,6 @@ async function createGithubAppJwt(config: GithubAppConfig): Promise { return await signGithubAppJwt({ issuer: config.clientId, privateKeyPkcs8: privateKey, now: config.now() }) } -/** - * An RS256 GitHub App JWT. `issuer` is the App's client ID or its App ID; - * GitHub accepts either. - * - * It is valid for ten minutes from a minute in the past, which absorbs clock - * drift between the Worker and GitHub. Ten minutes is GitHub's maximum. - */ -export async function signGithubAppJwt(input: { - issuer: string - privateKeyPkcs8: Uint8Array - /** Unix seconds. */ - now: number -}): Promise { - const issuedAt = input.now - 60 - const header = bytesToBase64Url(new TextEncoder().encode(JSON.stringify({ alg: 'RS256', typ: 'JWT' }))) - const payload = bytesToBase64Url(new TextEncoder().encode(JSON.stringify({ - iat: issuedAt, - exp: issuedAt + 600, - iss: input.issuer, - }))) - const unsigned = `${header}.${payload}` - const imported = await crypto.subtle.importKey( - 'pkcs8', - toArrayBuffer(input.privateKeyPkcs8), - { name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256' }, - false, - ['sign'], - ) - const signature = await crypto.subtle.sign( - 'RSASSA-PKCS1-v1_5', - imported, - new TextEncoder().encode(unsigned), - ) - return `${unsigned}.${bytesToBase64Url(new Uint8Array(signature))}` -} - async function readBoundedJson(response: Response, maximumBytes: number): Promise { const declared = Number(response.headers.get('content-length')) if (Number.isFinite(declared) && declared > maximumBytes) diff --git a/layers/artifact-delivery/server/utils/github-read-credential.ts b/layers/artifact-delivery/server/utils/github-read-credential.ts index 6398b471..73bcc3cb 100644 --- a/layers/artifact-delivery/server/utils/github-read-credential.ts +++ b/layers/artifact-delivery/server/utils/github-read-credential.ts @@ -1,214 +1,52 @@ -import type { PublicGithubSourceClient } from './github-source' -import { requestInstallationToken, signGithubAppJwt } from './github-app' +import type { GithubCredential, GithubCredentialConfig, GithubCredentialEnv, GithubCredentialReport } from '#shared/server/github-app-credential' +import type { LoadSourceResult, PublicGithubSourceClient, ResolveSourceResult } from './github-source' +import { parseGithubCredentialConfig } from '#shared/server/github-app-credential' /** - * Renew an installation token this long before GitHub says it expires, so a - * build that starts with it never sees it lapse part way through. - */ -export const INSTALLATION_TOKEN_RENEW_SECONDS = 5 * 60 - -/** The read App: metadata read only, installed on skilld-dev. */ -export interface ReadAppConfig { - appId: number - installationId: number - privateKeyPkcs8: Uint8Array -} - -/** The token a build reads with when the App gives none, by its secret name. */ -export interface FallbackToken { - name: 'ARTIFACT_GITHUB_TOKEN' | 'GITHUB_TOKEN' - token: string -} - -/** - * Where public Artifact builds get their GitHub credential, parsed once from - * the Worker env. - * - * GitHub counts a personal token's quota per account, so every token of one - * account shares one bucket. An App installation has a bucket of its own. - */ -export type ArtifactGithubCredentialConfig - = | { _tag: 'app', app: ReadAppConfig, fallback: FallbackToken | null } - /** App secrets are set but unusable. Builds read with the fallback and report why. */ - | { _tag: 'app-misconfigured', reason: string, fallback: FallbackToken | null } - | { _tag: 'token', token: FallbackToken } - | { _tag: 'anonymous' } - -export interface ArtifactGithubCredentialEnv { - SKILLD_READ_APP_ID?: string - SKILLD_READ_APP_INSTALLATION_ID?: string - SKILLD_READ_APP_PRIVATE_KEY_PKCS8?: string - ARTIFACT_GITHUB_TOKEN?: string - GITHUB_TOKEN?: string -} - -/** Why a build read with a fallback token. It names secrets, never a value. */ -export interface ArtifactGithubCredentialReport { - outcome: 'app-misconfigured' | 'app-token-unavailable' - reason: string - fallback: FallbackToken['name'] | 'anonymous' -} - -interface CachedInstallationToken { - token: string - /** Unix seconds. */ - expiresAt: number -} - -/** - * Installation tokens kept between builds in one isolate. A token lives an - * hour; minting one per build would add a request and a signature to each. - */ -export interface InstallationTokenCache { - tokens: Map - pending: Map> -} - -export function createInstallationTokenCache(): InstallationTokenCache { - return { tokens: new Map(), pending: new Map() } -} - -export interface ArtifactGithubCredentialRuntime { - fetch: typeof globalThis.fetch - /** Unix seconds. */ - now: () => number - tokenCache: InstallationTokenCache - report: (event: ArtifactGithubCredentialReport) => void -} - -/** The credential for one GitHub read. Undefined reads anonymously. */ -export type ArtifactGithubCredential = () => Promise - -export function parseArtifactGithubCredentialConfig(env: ArtifactGithubCredentialEnv): ArtifactGithubCredentialConfig { - const fallback = fallbackToken(env) - const appSecrets = [env.SKILLD_READ_APP_ID, env.SKILLD_READ_APP_INSTALLATION_ID, env.SKILLD_READ_APP_PRIVATE_KEY_PKCS8] - if (appSecrets.every(value => !value?.trim())) - return fallback ? { _tag: 'token', token: fallback } : { _tag: 'anonymous' } - const misconfigured = (reason: string): ArtifactGithubCredentialConfig => ({ _tag: 'app-misconfigured', reason, fallback }) - const appId = positiveInteger(env.SKILLD_READ_APP_ID) - if (appId === null) - return misconfigured('SKILLD_READ_APP_ID is not a GitHub App ID') - const installationId = positiveInteger(env.SKILLD_READ_APP_INSTALLATION_ID) - if (installationId === null) - return misconfigured('SKILLD_READ_APP_INSTALLATION_ID is not an installation ID') - const privateKeyPkcs8 = pkcs8PemBytes(env.SKILLD_READ_APP_PRIVATE_KEY_PKCS8 ?? '') - if (!privateKeyPkcs8) - return misconfigured('SKILLD_READ_APP_PRIVATE_KEY_PKCS8 is not a PKCS #8 PEM private key') - return { _tag: 'app', app: { appId, installationId, privateKeyPkcs8 }, fallback } -} - -/** - * The GitHub credential public Artifact builds read with: the read App's + * Where public Artifact builds get their GitHub credential: the read App's * installation token, then `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`. - * - * The registry sync and `/gh` page views keep `GITHUB_TOKEN`. When the App - * cannot give a token, the build still runs on the fallback and the reason is - * reported, so a broken key shows up as an event instead of as a spent quota. */ -export function createArtifactGithubCredential( - config: ArtifactGithubCredentialConfig, - runtime: ArtifactGithubCredentialRuntime, -): ArtifactGithubCredential { - if (config._tag === 'anonymous') - return async () => undefined - if (config._tag === 'token') - return async () => config.token.token - const fallbackName = config.fallback?.name ?? 'anonymous' - if (config._tag === 'app-misconfigured') { - return async () => { - runtime.report({ outcome: 'app-misconfigured', reason: config.reason, fallback: fallbackName }) - return config.fallback?.token - } - } - const app = config.app - const key = `${app.appId}:${app.installationId}` - return async () => { - const cached = runtime.tokenCache.tokens.get(key) - if (cached && cached.expiresAt - INSTALLATION_TOKEN_RENEW_SECONDS > runtime.now()) - return cached.token - let pending = runtime.tokenCache.pending.get(key) - if (!pending) { - pending = mintInstallationToken(app, runtime).finally(() => runtime.tokenCache.pending.delete(key)) - runtime.tokenCache.pending.set(key, pending) - } - const minted = await pending - if ('token' in minted) { - runtime.tokenCache.tokens.set(key, minted) - return minted.token - } - runtime.report({ outcome: 'app-token-unavailable', reason: minted.reason, fallback: fallbackName }) - return config.fallback?.token - } +export function parseArtifactGithubCredentialConfig(env: GithubCredentialEnv): GithubCredentialConfig { + return parseGithubCredentialConfig(env, ['ARTIFACT_GITHUB_TOKEN', 'GITHUB_TOKEN']) } /** * A GitHub source client whose every read asks for the current credential, so * a long-lived client never reads with an expired installation token. + * + * Some organizations deny the read App on a public Repository that a personal + * token reads: `neondatabase/agent-skills` answered SOURCE_ACCESS_DENIED to + * every run on 2026-10-06. A read the App was denied repeats once with the + * fallback token. Every denial is reported, with or without a fallback. */ export function withGithubCredential( - credential: ArtifactGithubCredential, + credential: GithubCredential, create: (token: string | undefined) => PublicGithubSourceClient, + report: (event: GithubCredentialReport) => void, ): PublicGithubSourceClient { + const read = async ( + repository: { owner: string, repository: string }, + run: (client: PublicGithubSourceClient) => Promise, + ): Promise => { + const current = await credential.current() + const first = await run(create(current.token)) + if (!current.isApp || first._tag !== 'rejected' || first.code !== 'SOURCE_ACCESS_DENIED') + return first + const fallback = credential.fallback + report({ + outcome: 'app-denied', + reason: `GitHub denied the read App for ${repository.owner}/${repository.repository}`, + fallback: fallback?.name ?? 'none', + }) + if (fallback) + return await run(create(fallback.token)) + return { + ...first, + summary: 'GitHub denies the skilld.dev read App access to this Repository, and no fallback token is set.', + } + } return { - resolve: async request => await create(await credential()).resolve(request), - load: async source => await create(await credential()).load(source), + resolve: async request => await read(request, client => client.resolve(request)), + load: async source => await read(source, client => client.load(source)), } } - -async function mintInstallationToken( - app: ReadAppConfig, - runtime: Pick, -): Promise { - const minted = await signGithubAppJwt({ issuer: String(app.appId), privateKeyPkcs8: app.privateKeyPkcs8, now: runtime.now() }) - .then(jwt => requestInstallationToken({ - fetch: runtime.fetch, - jwt, - installationId: app.installationId, - body: { permissions: { metadata: 'read' } }, - })) - // A network fault, a 5xx or a key WebCrypto rejects: the build reads with - // the fallback token, and the reason reaches the report. - .catch((error: unknown) => ({ _tag: 'failed' as const, reason: error instanceof Error ? error.message : String(error) })) - if (minted._tag === 'failed') - return { _tag: 'unavailable', reason: minted.reason.slice(0, 200) } - if (minted._tag === 'refused') - return { _tag: 'unavailable', reason: `GitHub refused the installation token (${minted.status})` } - const expiresAt = Math.floor(Date.parse(minted.expiresAt) / 1000) - return { token: minted.token, expiresAt } -} - -function fallbackToken(env: ArtifactGithubCredentialEnv): FallbackToken | null { - if (env.ARTIFACT_GITHUB_TOKEN?.trim()) - return { name: 'ARTIFACT_GITHUB_TOKEN', token: env.ARTIFACT_GITHUB_TOKEN.trim() } - if (env.GITHUB_TOKEN?.trim()) - return { name: 'GITHUB_TOKEN', token: env.GITHUB_TOKEN.trim() } - return null -} - -function positiveInteger(value: string | undefined): number | null { - const trimmed = value?.trim() ?? '' - if (!/^[1-9]\d*$/.test(trimmed)) - return null - const parsed = Number(trimmed) - return Number.isSafeInteger(parsed) ? parsed : null -} - -/** - * The DER bytes of a PKCS #8 PEM private key, or null. - * - * A secret pasted through a dashboard or a JSON file can hold its line breaks - * as the two characters `\n`, so both forms parse. A PKCS #1 key - * (`BEGIN RSA PRIVATE KEY`, as GitHub downloads it) does not: WebCrypto - * imports PKCS #8 only. Convert it with `openssl pkcs8 -topk8 -nocrypt`. - */ -function pkcs8PemBytes(value: string): Uint8Array | null { - const text = value.replaceAll('\\n', '\n').trim() - const match = /^-----BEGIN PRIVATE KEY-----([A-Z0-9+/=\s]+)-----END PRIVATE KEY-----$/i.exec(text) - if (!match) - return null - const base64 = match[1]!.replace(/\s+/g, '') - if (!/^[A-Z0-9+/]+={0,2}$/i.test(base64) || base64.length % 4 !== 0) - return null - const binary = atob(base64) - return Uint8Array.from(binary, character => character.charCodeAt(0)) -} diff --git a/layers/artifact-delivery/server/utils/queue.ts b/layers/artifact-delivery/server/utils/queue.ts index aad0a5a4..55fd516e 100644 --- a/layers/artifact-delivery/server/utils/queue.ts +++ b/layers/artifact-delivery/server/utils/queue.ts @@ -1,14 +1,11 @@ import type { QueueBatch } from '#cf-jobs/server' +import type { GithubCredentialEnv, GithubCredentialReport, GithubCredentialRuntime } from '#shared/server/github-app-credential' import type { ArtifactBuildDependencies } from './build' -import type { - ArtifactGithubCredentialEnv, - ArtifactGithubCredentialReport, - ArtifactGithubCredentialRuntime, -} from './github-read-credential' import type { GithubObjectCache, GithubReadFailure, PublicGithubSourceClient } from './github-source' import { createWideEvent } from '@harlan-zw/nuxt-wide-events/standalone' import { z } from 'zod' import { emitOperationalEvent } from '#server/utils/operational-event' +import { createGithubCredential, isolateInstallationTokenCache } from '#shared/server/github-app-credential' import { createArtifactSigner } from './attestation' import { failResolution, processArtifactBuild } from './build' import { @@ -16,16 +13,12 @@ import { githubAppUserTokenDependenciesFromEnv, loadAccountGithubAppUserToken, } from './github-app' -import { - createArtifactGithubCredential, - createInstallationTokenCache, - parseArtifactGithubCredentialConfig, - withGithubCredential, -} from './github-read-credential' +import { parseArtifactGithubCredentialConfig, withGithubCredential } from './github-read-credential' import { createGithubSourceClient, createPublicGithubSourceClient } from './github-source' import { createD1PrivateArtifactKeyProvider, privateArtifactWrappingKeysFromEnv } from './private-crypto' import { privateArtifactAccessEnabled } from './private-feature' import { putPrivateArtifact } from './private-storage' +import { createRequesterGithubSource, forgetResolutionRequester } from './requester-github' import { getResolution } from './state' import { parseTrustedRoot } from './trusted-root' @@ -93,6 +86,10 @@ export async function consumeArtifactBuildBatch( continue } if (outcome._tag === 'ok') { + // A settled build never reads GitHub again, so it no longer needs to + // know who asked. A superseded one leaves that to the build that won. + if (outcome.value._tag !== 'superseded') + await forgetResolutionRequester(dependencies.db, parsed.data.resolutionId) message.ack() continue } @@ -112,6 +109,7 @@ export async function consumeArtifactBuildBatch( const row = await getResolution(dependencies.db, parsed.data.resolutionId) if (row) await failResolution(dependencies, row, 'SERVICE_UNAVAILABLE', true) + await forgetResolutionRequester(dependencies.db, parsed.data.resolutionId) message.ack() } } @@ -159,12 +157,6 @@ export function createKvGithubObjectCache(kv: KVNamespace): GithubObjectCache { } } -/** - * One installation token cache per isolate. A token lives an hour, so it - * outlives the queue batch that minted it. - */ -const installationTokens = createInstallationTokenCache() - /** * The GitHub source public Artifact builds read with. * @@ -175,27 +167,30 @@ const installationTokens = createInstallationTokenCache() * else spends, then `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`. */ export function createArtifactGithubSource( - env: ArtifactGithubCredentialEnv, - runtime: ArtifactGithubCredentialRuntime = defaultGithubSourceRuntime(), + env: GithubCredentialEnv, + runtime: GithubCredentialRuntime = defaultGithubSourceRuntime(), cache?: GithubObjectCache, ): PublicGithubSourceClient { - const credential = createArtifactGithubCredential(parseArtifactGithubCredentialConfig(env), runtime) + const credential = createGithubCredential(parseArtifactGithubCredentialConfig(env), runtime) return withGithubCredential( credential, token => createPublicGithubSourceClient({ fetch: runtime.fetch, token, onReadFailure: reportGithubReadFailure, cache }), + runtime.report, ) } -function defaultGithubSourceRuntime(): ArtifactGithubCredentialRuntime { +function defaultGithubSourceRuntime(): GithubCredentialRuntime { return { fetch: globalThis.fetch.bind(globalThis), now: () => Math.floor(Date.now() / 1000), - tokenCache: installationTokens, + // A token lives an hour, so it outlives the queue batch that minted it, + // and the registry reads in this isolate share it. + tokenCache: isolateInstallationTokenCache(), report: reportGithubCredential, } } -function reportGithubCredential(event: ArtifactGithubCredentialReport): void { +function reportGithubCredential(event: GithubCredentialReport): void { emitOperationalEvent(createWideEvent({ 'operation': 'artifact-github-credential', 'outcome': event.outcome, @@ -209,9 +204,23 @@ export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBu const privateDependencies = privateArtifactAccessEnabled(env) ? createPrivateBuildDependencies(env, runtimeFetch) : {} + const githubObjects = createKvGithubObjectCache(env.KV_CACHE) return { db: env.DB, - github: createArtifactGithubSource(env, defaultGithubSourceRuntime(), createKvGithubObjectCache(env.KV_CACHE)), + github: createArtifactGithubSource(env, defaultGithubSourceRuntime(), githubObjects), + requesterGithub: createRequesterGithubSource({ + db: env.DB, + tokenKey: () => env.NUXT_TOKEN_KEY, + fetch: runtimeFetch, + now: () => Math.floor(Date.now() / 1000), + cache: githubObjects, + onReadFailure: reportGithubReadFailure, + onUnusableToken: reason => emitOperationalEvent(createWideEvent({ + operation: 'artifact-github-credential', + outcome: 'requester-token-unusable', + reason, + })), + }), bucket: env.PUBLIC_ARTIFACTS, signer: createArtifactSigner(env.ARTIFACT_SIGNER), trustedRoot: parseTrustedRoot(env.ARTIFACT_TRUSTED_ROOT_JSON, Math.floor(Date.now() / 1000)), diff --git a/layers/artifact-delivery/server/utils/request-resolution.ts b/layers/artifact-delivery/server/utils/request-resolution.ts index d9556bb6..7a775eb2 100644 --- a/layers/artifact-delivery/server/utils/request-resolution.ts +++ b/layers/artifact-delivery/server/utils/request-resolution.ts @@ -3,6 +3,7 @@ import type { SourceRequest } from '../schemas/contracts' import type { FetchAdmittedSkillIdentity } from './admitted-identity' import type { CreateResolutionResult } from './state' import { admittedSourceRequest } from './admitted-identity' +import { recordResolutionRequester } from './requester-github' import { createResolution, resolutionRequestIdentity } from './state' export interface ResolutionRequestDependencies { @@ -48,7 +49,16 @@ export function fetchAdmittedSkillIdentity(context?: H3EventContext): FetchAdmit */ export async function requestResolution( dependencies: ResolutionRequestDependencies, - input: { source: SourceRequest, idempotencyKey: string, access: ResolutionAccess }, + input: { + source: SourceRequest + idempotencyKey: string + access: ResolutionAccess + /** + * The signed-in account that sent a public request, or null. Its build may + * read GitHub with that account's own token once the shared quota is spent. + */ + requesterAccountId?: number | null + }, ): Promise { const accountId = input.access.visibility === 'private' ? input.access.accountId : undefined const identity = await resolutionRequestIdentity(input.source, input.idempotencyKey, accountId) @@ -56,6 +66,13 @@ export async function requestResolution( ? input.source : await admittedSourceRequest(input.source, dependencies.lookupAdmitted) const result = await createResolution(dependencies.db, source, identity, dependencies.now(), input.access) + if (result._tag === 'created' && input.access.visibility === 'public' && input.requesterAccountId) { + await recordResolutionRequester(dependencies.db, { + resolutionId: result.row.id, + accountId: input.requesterAccountId, + now: dependencies.now(), + }) + } if (result._tag !== 'idempotency-conflict' && result.row.state === 'requested') await dependencies.enqueue(result.row.id) return result diff --git a/layers/artifact-delivery/server/utils/requester-github.ts b/layers/artifact-delivery/server/utils/requester-github.ts new file mode 100644 index 00000000..745bfdd0 --- /dev/null +++ b/layers/artifact-delivery/server/utils/requester-github.ts @@ -0,0 +1,127 @@ +import type { GithubObjectCache, GithubReadFailure, LoadSourceResult, PublicGithubSourceClient, ResolveSourceResult } from './github-source' +import type { ResolutionRow } from './state' +import { decryptToken } from '#layers/identity/server/utils/crypto' +import { createPublicGithubSourceClient } from './github-source' + +/** + * Record the signed-in account that asked for a public Resolution. + * + * Only that Resolution's build reads it, and only after the shared GitHub + * credentials are rate limited. The row goes when the build settles. + */ +export async function recordResolutionRequester( + db: D1Database, + input: { resolutionId: string, accountId: number, now: number }, +): Promise { + await db.prepare( + `INSERT OR IGNORE INTO artifact_resolution_requesters (resolution_id, account_id, created_at) + VALUES (?1, ?2, ?3)`, + ).bind(input.resolutionId, input.accountId, input.now).run() +} + +/** Forget who asked for a Resolution. Its build has settled. */ +export async function forgetResolutionRequester(db: D1Database, resolutionId: string): Promise { + await db.prepare(`DELETE FROM artifact_resolution_requesters WHERE resolution_id = ?1`).bind(resolutionId).run() +} + +export interface RequesterGithubDependencies { + db: D1Database + /** + * `NUXT_TOKEN_KEY`, which encrypts the stored GitHub OAuth tokens. It is read + * only when a requester's token is needed. + */ + tokenKey: () => string + fetch: typeof globalThis.fetch + /** Unix seconds. */ + now: () => number + /** Commits and trees by SHA, shared with the build's own client. */ + cache?: GithubObjectCache + onReadFailure?: (failure: GithubReadFailure) => void + /** Called when a stored token cannot be used. It never receives the token. */ + onUnusableToken?: (reason: string) => void +} + +/** + * The GitHub client of the account that asked for one public Resolution, or + * null when nobody signed in asked for it, or the account holds no usable + * GitHub token. + * + * The token is the account's GitHub sign-in token. Its scopes, `read:user` and + * `user:email`, read public Repositories at that account's own quota, and no + * private one. The public client also refuses any Repository GitHub reports + * as private, so a build read with it can only produce public content. + */ +export function createRequesterGithubSource( + dependencies: RequesterGithubDependencies, +): (row: ResolutionRow) => Promise { + return async (row) => { + if (row.visibility !== 'public') + return null + const stored = await dependencies.db.prepare( + `SELECT u.github_token_encrypted, u.github_token_expires_at + FROM artifact_resolution_requesters r + JOIN users u ON u.id = r.account_id + WHERE r.resolution_id = ?1 + LIMIT 1`, + ).bind(row.id).first<{ github_token_encrypted: string | null, github_token_expires_at: number | null }>() + if (!stored?.github_token_encrypted) + return null + if (stored.github_token_expires_at !== null && stored.github_token_expires_at <= dependencies.now() + 60) + return null + const token = await decryptToken(stored.github_token_encrypted, dependencies.tokenKey()).then( + value => ({ _tag: 'token' as const, value }), + (error: unknown) => ({ _tag: 'unreadable' as const, reason: error instanceof Error ? error.message : String(error) }), + ) + if (token._tag === 'unreadable') { + dependencies.onUnusableToken?.(`The stored GitHub token did not decrypt: ${token.reason}`.slice(0, 200)) + return null + } + return createPublicGithubSourceClient({ + fetch: dependencies.fetch, + token: token.value, + cache: dependencies.cache, + onReadFailure: dependencies.onReadFailure, + }) + } +} + +/** + * A client that reads with the shared credential, and repeats with the + * requester's own client a read the shared credentials could not make: a spent + * quota, or a Repository whose organization denies the read App when no + * fallback token is set. + * + * Every other answer, success or rejection, stays the shared one, so a + * requester's token is spent only when nothing else could read. + */ +export function withRequesterFallback( + shared: PublicGithubSourceClient, + requester: () => Promise, +): PublicGithubSourceClient { + let own: Promise | null = null + const ownClient = () => { + own ??= requester() + return own + } + return { + async resolve(request) { + const first = await shared.resolve(request) + if (!sharedCouldNotRead(first)) + return first + const client = await ownClient() + return client ? await client.resolve(request) : first + }, + async load(source) { + const first = await shared.load(source) + if (!sharedCouldNotRead(first)) + return first + const client = await ownClient() + return client ? await client.load(source) : first + }, + } +} + +/** A spent quota, or a denial no shared credential could get past. */ +function sharedCouldNotRead(result: ResolveSourceResult | LoadSourceResult): boolean { + return result._tag === 'rejected' && (result.code === 'RATE_LIMITED' || result.code === 'SOURCE_ACCESS_DENIED') +} diff --git a/layers/identity/server/utils/account-deletion.ts b/layers/identity/server/utils/account-deletion.ts index 06e82f0c..488a2f59 100644 --- a/layers/identity/server/utils/account-deletion.ts +++ b/layers/identity/server/utils/account-deletion.ts @@ -27,6 +27,7 @@ const ACCOUNT_ROW_DELETES = [ `DELETE FROM digest_runs WHERE user_id = ?1`, `DELETE FROM weekly_runs WHERE user_id = ?1`, `DELETE FROM email_preference_events WHERE user_id = ?1`, + `DELETE FROM artifact_resolution_requesters WHERE account_id = ?1`, `DELETE FROM artifact_download_grants WHERE account_id = ?1`, `DELETE FROM private_artifact_attestations WHERE account_id = ?1`, `DELETE FROM private_artifacts WHERE account_id = ?1`, diff --git a/layers/identity/server/utils/daily-health-check.ts b/layers/identity/server/utils/daily-health-check.ts index 45434f85..c393969c 100644 --- a/layers/identity/server/utils/daily-health-check.ts +++ b/layers/identity/server/utils/daily-health-check.ts @@ -605,9 +605,9 @@ export function evaluateDailyHealthStatus( // while rotating is still routine rather than an incident. const token = summary.credentials.githubToken if (token._tag === 'expired') - red.push(`GITHUB_TOKEN expired ${plural(Math.abs(token.daysRemaining), 'day')} ago. Rotate it: sync is down until you do.`) + red.push(`GITHUB_TOKEN expired ${plural(Math.abs(token.daysRemaining), 'day')} ago. Repositories that deny the read App cannot sync or build until you rotate or delete it.`) else if (token._tag === 'expiring') - amber.push(`GITHUB_TOKEN expires in ${plural(token.daysRemaining, 'day')}. Rotate it before sync starts failing.`) + amber.push(`GITHUB_TOKEN expires in ${plural(token.daysRemaining, 'day')}. Rotate or delete it: it reads only Repositories that deny the read App.`) // Report every finding, ordered by severity, rather than only the winning // tier. Returning `red` alone meant one loud red hid every amber underneath diff --git a/layers/identity/server/utils/data-retention.ts b/layers/identity/server/utils/data-retention.ts index 6e5241eb..cc795f30 100644 --- a/layers/identity/server/utils/data-retention.ts +++ b/layers/identity/server/utils/data-retention.ts @@ -14,6 +14,11 @@ export const RETENTION_SECONDS = { cliSignIn: DAY_SECONDS, /** Delivery history and preference changes support three months of reporting. */ emailHistory: 90 * DAY_SECONDS, + /** + * The account that asked for a public Artifact build. The build deletes it + * when it settles; this removes one a dead build left behind. + */ + runRequester: DAY_SECONDS, } as const export interface RetentionPurgeResult { @@ -23,6 +28,7 @@ export interface RetentionPurgeResult { digestRuns: number weeklyRuns: number emailPreferenceEvents: number + runRequesters: number } /** @@ -40,6 +46,7 @@ export async function purgeRetainedPersonalData( const endedTokenCutoff = now - RETENTION_SECONDS.endedCliToken const signInCutoff = now - RETENTION_SECONDS.cliSignIn const historyCutoff = now - RETENTION_SECONDS.emailHistory + const requesterCutoff = now - RETENTION_SECONDS.runRequester const results = await db.batch([ db.prepare( @@ -62,6 +69,7 @@ export async function purgeRetainedPersonalData( ).bind(historyCutoff), db.prepare(`DELETE FROM weekly_runs WHERE window_end <= ?1`).bind(historyCutoff), db.prepare(`DELETE FROM email_preference_events WHERE occurred_at <= ?1`).bind(historyCutoff), + db.prepare(`DELETE FROM artifact_resolution_requesters WHERE created_at <= ?1`).bind(requesterCutoff), ]) const changes = results.map(result => result.meta.changes) @@ -72,5 +80,6 @@ export async function purgeRetainedPersonalData( digestRuns: changes[3] ?? 0, weeklyRuns: changes[4] ?? 0, emailPreferenceEvents: changes[5] ?? 0, + runRequesters: changes[6] ?? 0, } } diff --git a/layers/registry/server/api/orgs/[owner].get.ts b/layers/registry/server/api/orgs/[owner].get.ts index 751aaf13..c9b26af2 100644 --- a/layers/registry/server/api/orgs/[owner].get.ts +++ b/layers/registry/server/api/orgs/[owner].get.ts @@ -1,11 +1,12 @@ import type { H3Event } from 'h3' import type { TagPayload } from '../../jobs/generate-tags' +import type { GithubBindings } from '../../utils/github-client' import type { RegistrySkill } from '../../utils/skills-registry' import { getDB } from '#server/utils/db' import { cached, readCache, writeCache } from '#shared/server/cache' import { officialRepos } from '../../data/official-repos' import { TAG_BY_SLUG } from '../../jobs/taxonomy' -import { GITHUB_PAGE_READ_TIMEOUT_MS } from '../../utils/github-client' +import { getGithubJson, GITHUB_PAGE_READ_TIMEOUT_MS, resolveGithubBindings } from '../../utils/github-client' import { resolveRepoSourceIdentitiesForOwner } from '../../utils/repo-source-identity' import { getGeneratedBatch } from '../../utils/skill-generated' import { querySkills } from '../../utils/skills-registry' @@ -68,19 +69,27 @@ const ORG_PROFILE_CACHE_STALE_TTL = 60 * 5 const officialOwners = new Set(officialRepos.map(r => r.owner)) const kindByOwner = new Map(officialRepos.map(r => [r.owner, r.kind])) -async function fetchAndStoreOwner(owner: string, db: D1Database): Promise { - const res = await fetch(`https://api.github.com/users/${owner}`, { - signal: AbortSignal.timeout(GITHUB_PAGE_READ_TIMEOUT_MS), - headers: { - 'User-Agent': 'skilld.dev', - 'Accept': 'application/vnd.github+json', - }, - }).catch(() => { - emitOperationalEvent(createWideEvent({ operation: 'org-profile-fetch', outcome: 'failed' })) - return null - }) +interface GithubOwnerResponse { + name?: string + bio?: string + blog?: string + location?: string + followers?: number + public_repos?: number + type?: string +} + +async function fetchAndStoreOwner(owner: string, db: D1Database, bindings: GithubBindings): Promise { + // Authenticated and conditional: an unchanged profile answers 304 from + // the ETag cache for free. Anonymous, this read shared the Worker IP's 60 + // requests an hour with every other Cloudflare tenant, and failed. + const res = await getGithubJson(`/users/${encodeURIComponent(owner)}`, bindings, { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }) + .catch(() => { + emitOperationalEvent(createWideEvent({ operation: 'org-profile-fetch', outcome: 'failed' })) + return null + }) - if (!res || !res.ok) { + if (!res?.data) { if (res?.status === 404) { await db .prepare( @@ -93,16 +102,7 @@ async function fetchAndStoreOwner(owner: string, db: D1Database): Promise { +async function loadOwner(owner: string, db: D1Database, bindings: GithubBindings): Promise { const cached = await db .prepare( `SELECT kind, name, bio, blog, location, followers, last_synced_at, sync_status @@ -145,7 +145,7 @@ async function loadOwner(owner: string, db: D1Database): Promise { emitOperationalEvent(createWideEvent({ operation, @@ -31,6 +48,44 @@ function githubReadFailed(operation: string, error: unknown): FetchOutcome }): SkillFileScan { + return { + status: tree.truncated ? 'truncated' : 'ok', + skillFiles: tree.tree + .filter(entry => entry.type === 'blob' && isRegistrySkillPath(entry.path)) + .map(entry => entry.path) + .sort(), + } +} + +function profileFrom(meta: RepoMeta, scan: SkillFileScan | null): RepoSourceProfile { + const skillFiles = scan?.skillFiles ?? [] + return { + owner: meta.owner.login, + repo: meta.name, + description: meta.description, + githubUrl: meta.html_url || `https://github.com/${meta.owner.login}/${meta.name}`, + defaultBranch: meta.default_branch, + stars: meta.stargazers_count, + forks: meta.forks_count, + pushedAt: meta.pushed_at, + createdAt: meta.created_at, + archived: Boolean(meta.archived), + fork: Boolean(meta.fork), + skillFileScanStatus: scan?.status ?? 'unavailable', + skillFileCount: skillFiles.length, + skillFiles, + } +} + export default defineCachedEventHandler(async (event) => { const ownerParam = getRouterParam(event, 'owner') const repoParam = getRouterParam(event, 'repo') @@ -39,55 +94,61 @@ export default defineCachedEventHandler(async (event) => { const owner = ownerParam.toLowerCase() const repo = repoParam.toLowerCase() + const edgeCache = useStorage('edge-cache') + if (await readCache(edgeCache, missingRepoKey(owner, repo))) + throw createError({ statusCode: 404, message: 'Repository not found' }) + const db = getDB(event) - const source = await resolveRepoSourceIdentity(db, { owner, repo }) + const row = await db + .prepare(`SELECT source_owner, source_repo, last_tree_sha FROM repos WHERE owner = ? AND repo = ?`) + .bind(owner, repo) + .first<{ source_owner: string | null, source_repo: string | null, last_tree_sha: string | null }>() + const source = resolveRepoSourceIdentityFromRow({ owner, repo }, row) const bindings = resolveGithubBindings(event.context.platform.env) // A read that timed out or dropped is an outage, not a missing repository. const repoRes = await getRepo(source.owner, source.repo, bindings, { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }) .catch((error: unknown) => githubReadFailed('repo-source-profile-repo', error)) - if (repoRes.status === 404) + // The token can read private repositories, and this page is public, so a + // private one answers exactly like one that does not exist. + if (repoRes.status === 404 || repoRes.data?.private) { + await writeCache(edgeCache, missingRepoKey(owner, repo), true, { ttl: MISSING_REPO_TTL_SECONDS }) throw createError({ statusCode: 404, message: 'Repository not found' }) + } if (!repoRes.data) { emitOperationalEvent(createWideEvent({ 'operation': 'repo-source-profile', 'outcome': 'unavailable', 'upstream.status': repoRes.status || null, })) + // Metadata from ungh.cc keeps the page whole while the shared quota is + // spent. The Skill file list needs the tree, which only GitHub serves. + if (githubRefused(repoRes)) { + const fallback = await readUnghRepoMeta(source.owner, source.repo) + if (fallback._tag === 'found') + return profileFrom(fallback.meta, null) + emitOperationalEvent(createWideEvent({ operation: 'repo-source-profile-ungh', outcome: 'unavailable', reason: fallback.reason })) + } return buildUnavailableRepoSourceProfile(owner, repo) satisfies RepoSourceProfile } const meta = repoRes.data - const repoOwner = meta.owner.login - const repoName = meta.name - const treeRes = await getTree(repoOwner, repoName, meta.default_branch, bindings, { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }) - .catch((error: unknown) => githubReadFailed('repo-source-profile-tree', error)) - const skillFiles = (treeRes.data?.tree ?? []) - .filter(entry => entry.type === 'blob' && isRegistrySkillPath(entry.path)) - .map(entry => entry.path) - .sort() - const skillFileScanStatus = !treeRes.data - ? 'unavailable' - : treeRes.data.truncated - ? 'truncated' - : 'ok' + // The sync records the tree it last read. Read at that SHA, the listing + // never changes, so one scan serves every later view. A repository the + // registry has not synced is read at its default branch. + const treeSha = row?.last_tree_sha ?? null + const cachedScan = treeSha + ? await readCache(edgeCache, treeScanKey(meta.owner.login, meta.name, treeSha)) + : null + if (cachedScan) + return profileFrom(meta, cachedScan) satisfies RepoSourceProfile - return { - owner: repoOwner, - repo: repoName, - description: meta.description, - githubUrl: meta.html_url || `https://github.com/${repoOwner}/${repoName}`, - defaultBranch: meta.default_branch, - stars: meta.stargazers_count, - forks: meta.forks_count, - pushedAt: meta.pushed_at, - createdAt: meta.created_at, - archived: Boolean(meta.archived), - fork: Boolean(meta.fork), - skillFileScanStatus, - skillFileCount: skillFiles.length, - skillFiles, - } satisfies RepoSourceProfile + const treeRes = await getTree(meta.owner.login, meta.name, treeSha ?? meta.default_branch, bindings, { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }) + .catch((error: unknown) => githubReadFailed('repo-source-profile-tree', error)) + const scan = treeRes.data ? scanTree(treeRes.data) : null + if (scan && treeSha) + await writeCache(edgeCache, treeScanKey(meta.owner.login, meta.name, treeSha), scan, { ttl: TREE_SCAN_TTL_SECONDS }) + return profileFrom(meta, scan) satisfies RepoSourceProfile }, { maxAge: 60 * 15, swr: true, diff --git a/layers/registry/server/api/skill-related/[...slug].get.ts b/layers/registry/server/api/skill-related/[...slug].get.ts index 4125397b..63163716 100644 --- a/layers/registry/server/api/skill-related/[...slug].get.ts +++ b/layers/registry/server/api/skill-related/[...slug].get.ts @@ -1,12 +1,13 @@ import type { H3Event } from 'h3' import type { EmbeddingNeighbor } from '../../jobs/generate-embeddings' +import type { GithubBindings } from '../../utils/github-client' import type { CoOccurrenceNeighbor } from '../../utils/skill-co-occurrence' import type { SkillCommitSourceRow } from '../../utils/skill-commit-source' import type { CachedRelated } from '../../utils/skill-related' import { readCache, readThroughCache, writeCache } from '#shared/server/cache' import { defineApiHandler } from '#shared/server/handler' import { getEmbeddingNeighbors } from '../../jobs/generate-embeddings' -import { GITHUB_PAGE_READ_TIMEOUT_MS } from '../../utils/github-client' +import { getGithubJson, GITHUB_PAGE_READ_TIMEOUT_MS, resolveGithubBindings } from '../../utils/github-client' import { getCoOccurrenceNeighbors } from '../../utils/skill-co-occurrence' import { skillCommitSourceFromRow } from '../../utils/skill-commit-source' import { @@ -76,7 +77,7 @@ export default defineApiHandler({ const [commits, related, coOccurrenceNeighbors, embeddingNeighbors] = await Promise.all([ source - ? getSkillCommits(source.owner, source.repo, source.path) + ? getSkillCommits(source.owner, source.repo, source.path, resolveGithubBindings(platform.env)) : Promise.resolve([]), findRelatedSkills(event, { owner: skill.owner, repo: skill.repo, excludeName: skill.name, limit: 6 }), getCoOccurrenceNeighbors(platform.db, skill.name), @@ -166,26 +167,32 @@ interface GhCommitResponse { author: { login: string, avatar_url: string } | null } -async function getSkillCommits(owner: string, repo: string, path: string): Promise { +async function getSkillCommits(owner: string, repo: string, path: string, bindings: GithubBindings): Promise { const cacheKey = `skills:commits:v2:${owner}/${repo}:${path}` const cached = await readCache(useStorage('edge-cache'), cacheKey) if (cached) return cached - const data = await $fetch(`https://api.github.com/repos/${owner}/${repo}/commits`, { - query: { path, per_page: 5 }, - headers: { 'Accept': 'application/vnd.github+json', 'User-Agent': 'skilld.dev' }, - // ofetch retries a GET once on its own. The commit list is optional - // decoration, so a slow GitHub costs the list, never the page. - retry: 0, - timeout: GITHUB_PAGE_READ_TIMEOUT_MS, - }).catch(() => { - emitOperationalEvent(createWideEvent({ operation: 'skill-related-commits-fetch', outcome: 'failed' })) + // Authenticated and conditional: an unchanged history answers 304 from the + // ETag cache for free. Anonymous, this read shared the Worker IP's 60 + // requests an hour with every other Cloudflare tenant, and about 330 a day + // failed (2026-10-06). The commit list is optional decoration, so a slow + // GitHub costs the list, never the page. + const query = new URLSearchParams({ path, per_page: '5' }) + const response = await getGithubJson( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits?${query.toString()}`, + bindings, + { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }, + ).catch((error: unknown) => { + emitOperationalEvent(createWideEvent({ operation: 'skill-related-commits-fetch', outcome: 'failed', reason: error instanceof Error ? error.message : String(error) })) return null }) - - if (!Array.isArray(data)) + const data = response?.data + if (!Array.isArray(data)) { + if (response) + emitOperationalEvent(createWideEvent({ 'operation': 'skill-related-commits-fetch', 'outcome': 'failed', 'upstream.status': response.status })) return [] + } const commits: SkillCommit[] = data.map(c => ({ sha: c.sha, diff --git a/layers/registry/server/api/skills/featured.get.ts b/layers/registry/server/api/skills/featured.get.ts index 9c914068..433e6c6c 100644 --- a/layers/registry/server/api/skills/featured.get.ts +++ b/layers/registry/server/api/skills/featured.get.ts @@ -1,8 +1,9 @@ import type { TagPayload } from '../../jobs/generate-tags' +import type { GithubBindings } from '../../utils/github-client' import { defineApiHandler } from '#shared/server/handler' import { officialRepos } from '../../data/official-repos' import { FeaturedSkillsQuery } from '../../schemas/featured-query' -import { GITHUB_PAGE_READ_TIMEOUT_MS } from '../../utils/github-client' +import { getGithubJson, GITHUB_PAGE_READ_TIMEOUT_MS, resolveGithubBindings } from '../../utils/github-client' import { getGeneratedBatch } from '../../utils/skill-generated' import { getFeaturedOfficialSections, getTopReposByCount, getTopReposByStars } from '../../utils/skills-registry' @@ -15,27 +16,30 @@ interface OwnerProfileRow { sync_status: string | null } -async function fetchAndStoreOwner(owner: string, db: D1Database): Promise { - const res = await fetch(`https://api.github.com/users/${owner}`, { - signal: AbortSignal.timeout(GITHUB_PAGE_READ_TIMEOUT_MS), - headers: { 'User-Agent': 'skilld.dev', 'Accept': 'application/vnd.github+json' }, - }).catch(() => { - emitOperationalEvent(createWideEvent({ operation: 'featured-owner-fetch', outcome: 'failed' })) - return null - }) +interface GithubOwnerResponse { + name?: string + bio?: string + blog?: string + location?: string + followers?: number + public_repos?: number + type?: string +} - if (!res?.ok) +async function fetchAndStoreOwner(owner: string, db: D1Database, bindings: GithubBindings): Promise { + // Authenticated and conditional: an unchanged profile answers 304 from + // the ETag cache for free. Anonymous, this read shared the Worker IP's 60 + // requests an hour with every other Cloudflare tenant, and failed. + const res = await getGithubJson(`/users/${encodeURIComponent(owner)}`, bindings, { timeoutMs: GITHUB_PAGE_READ_TIMEOUT_MS }) + .catch(() => { + emitOperationalEvent(createWideEvent({ operation: 'featured-owner-fetch', outcome: 'failed' })) + return null + }) + + if (!res?.data) return null - const data = await res.json() as { - name?: string - bio?: string - blog?: string - location?: string - followers?: number - public_repos?: number - type?: string - } + const data = res.data const kind = data.type === 'Organization' ? 'org' : 'user' await db.prepare( @@ -66,7 +70,7 @@ async function fetchAndStoreOwner(owner: string, db: D1Database): Promise> { +async function loadOwnerProfiles(owners: string[], db: D1Database, bindings: GithubBindings): Promise> { const profiles = new Map() const now = Math.floor(Date.now() / 1000) @@ -84,7 +88,7 @@ async function loadOwnerProfiles(owners: string[], db: D1Database): Promise s.owner), platform.db) + const profileMap = await loadOwnerProfiles(devSections.map(s => s.owner), platform.db, resolveGithubBindings(platform.env)) const enriched = sections.map(enrichSkills) const enrichedDevs = devSections.map((section) => { const profile = profileMap.get(section.owner) diff --git a/layers/registry/server/tasks/sync-github-skills.ts b/layers/registry/server/tasks/sync-github-skills.ts index 403f0717..48c6d06a 100644 --- a/layers/registry/server/tasks/sync-github-skills.ts +++ b/layers/registry/server/tasks/sync-github-skills.ts @@ -8,6 +8,8 @@ import { SUBSCRIBED_REPO_STALE_SECONDS, } from '~~/server/utils/sync-thresholds' import { observedSchedulePolicy } from '#shared/schedule-policy' +import { resolveGithubBindings } from '../utils/github-client' +import { githubSyncPermit } from '../utils/github-sync-control' import { appendRepoReverificationCandidates, DISCOVERY_SYNC_CANDIDATES_SQL, @@ -18,6 +20,7 @@ import { STAGE_HISTORICAL_DISCOVERY_CANDIDATES_SQL, SUBSCRIBED_SYNC_CANDIDATES_SQL, } from '../utils/sync-candidates' +import { prefetchUnchangedRepos } from '../utils/sync-prefetch' const MAX_REPOS_PER_RUN = 250 const MAX_HISTORICAL_CANDIDATES_PER_RUN = 250 @@ -106,26 +109,47 @@ export default defineScheduledTask({ { limit: MAX_REPOS_PER_RUN, maxReverified: MAX_REVERIFIED_REPOS_PER_RUN }, ) - if (queue.length === 0) { + const reverified = queue.length - ordered.length + const candidates = queue.map(({ owner, repo, ownerVerified }) => ({ + owner, + repo, + ownerVerified, + claimDiscovery: discoveryKeys.has(`${owner}/${repo}`), + })) + // While the sync is paused, every job would release itself unread, so + // a prefetch would only spend the quota the pause is saving. + const permit = await githubSyncPermit(db, now) + const prefetch = permit._tag === 'paused' + ? { _tag: 'unread' as const, queue: candidates, reason: 'sync-paused' } + : await prefetchUnchangedRepos({ db, bindings: resolveGithubBindings(env), now }, candidates) + .catch((error: unknown) => { + // The jobs read every repository themselves, as before the + // prefetch existed, so the run still completes. + emitOperationalEvent(createWideEvent({ + operation: 'sync-github-skills-prefetch', + outcome: 'failed', + reason: error instanceof Error ? error.message : String(error), + })) + return { _tag: 'unread' as const, queue: candidates, reason: 'prefetch-threw' } + }) + const prefetched = prefetch._tag === 'prefetched' + ? { unchanged: prefetch.unchanged, graphqlRequests: prefetch.requests } + : { unread: prefetch.reason } + + if (prefetch.queue.length === 0) { await reportJobRun(db, 'sync-github-skills', { cron: CRON, status: 'ok', durationMs: Date.now() - startedAt, }) - return { result: { queued: 0, deferred, stagedHistorical, reverified: 0 } } + return { result: { queued: 0, deferred, stagedHistorical, reverified, prefetched } } } const batch = await createRegistryJobBatch( env as Cloudflare.Env & Record, { name: `registry-sync:${now}`, - jobs: queue.map(({ owner, repo, ownerVerified }) => ({ - operation: 'sync', - owner, - repo, - ownerVerified, - claimDiscovery: discoveryKeys.has(`${owner}/${repo}`), - })), + jobs: prefetch.queue.map(candidate => ({ operation: 'sync' as const, ...candidate })), }, ) const dispatchFailed = batch.dispatched.filter(result => result.status !== 'sent') @@ -145,7 +169,8 @@ export default defineScheduledTask({ queued: batch.jobIds.length, deferred, stagedHistorical, - reverified: queue.length - ordered.length, + reverified, + prefetched, dispatchFailed: dispatchFailed.length, }, } diff --git a/layers/registry/server/utils/github-client.ts b/layers/registry/server/utils/github-client.ts index 565dcc78..9293fb1c 100644 --- a/layers/registry/server/utils/github-client.ts +++ b/layers/registry/server/utils/github-client.ts @@ -1,5 +1,18 @@ /// +import type { + GithubCredential, + GithubCredentialReport, + GithubReadCredential, + InstallationTokenCache, +} from '#shared/server/github-app-credential' +import { + createGithubCredential, + githubDeniedApp, + isolateInstallationTokenCache, + parseGithubCredentialConfig, +} from '#shared/server/github-app-credential' + const API_BASE = 'https://api.github.com' const GRAPHQL_URL = 'https://api.github.com/graphql' @@ -39,10 +52,29 @@ function readInit(options: GithubReadOptions | undefined): RequestInit | undefin export interface GithubBindings { KV_CACHE?: KVNamespace + /** + * The credential every Worker read asks for: the read App's installation + * token, with `GITHUB_TOKEN` only for a read GitHub denied the App. + */ + credential?: GithubCredential + /** A static token, for scripts and tests that pass no `credential`. */ GITHUB_TOKEN?: string + /** Receives each read GitHub denied the App. Defaults to a wide event. */ + reportCredential?: (event: GithubCredentialReport) => void } -type GithubBindingSource = Partial> +type GithubBindingSource = Partial> + +/** What a test or a caller with its own clock passes in place of the isolate defaults. */ +export interface GithubBindingRuntime { + tokenCache: InstallationTokenCache + /** Unix seconds. */ + now: () => number + report: (event: GithubCredentialReport) => void +} /** * Resolve GitHub bindings from a Cloudflare env (Worker runtime) or process.env (local dev). @@ -50,14 +82,62 @@ type GithubBindingSource = Partial = {}, +): GithubBindings { const env = cloudflareEnv ?? {} + const report = runtime.report ?? reportGithubCredential + const config = parseGithubCredentialConfig({ ...env, GITHUB_TOKEN: env.GITHUB_TOKEN ?? process.env.GITHUB_TOKEN }, ['GITHUB_TOKEN']) return { KV_CACHE: env.KV_CACHE, - GITHUB_TOKEN: env.GITHUB_TOKEN ?? process.env.GITHUB_TOKEN, + credential: createGithubCredential(config, { + // Late-bound, so a test that stubs the global fetch reaches the mint too. + fetch: (input, init) => fetch(input, init), + now: runtime.now ?? (() => Math.floor(Date.now() / 1000)), + tokenCache: runtime.tokenCache ?? isolateInstallationTokenCache(), + report, + }), + reportCredential: report, } } +function reportGithubCredential(event: GithubCredentialReport): void { + emitOperationalEvent(createWideEvent({ + 'operation': 'github-credential', + 'outcome': event.outcome, + 'reason': event.reason, + 'github.credential': event.fallback, + })) +} + +async function readCredential(bindings: GithubBindings): Promise { + return bindings.credential ? await bindings.credential.current() : { token: bindings.GITHUB_TOKEN, isApp: false } +} + +/** + * Report a read GitHub denied the App, and answer the token to repeat it + * with, or null when there is none. Every use of the fallback is reported. + */ +function fallbackAfterDenial(bindings: GithubBindings, label: string): string | null { + const fallback = bindings.credential?.fallback ?? null + ;(bindings.reportCredential ?? reportGithubCredential)({ + outcome: 'app-denied', + reason: `GitHub denied the read App for ${label}`, + fallback: fallback?.name ?? 'none', + }) + return fallback?.token ?? null +} + +function withAuthorization(headers: Headers, token: string | undefined): Headers { + const authorized = new Headers(headers) + if (token) + authorized.set('Authorization', `Bearer ${token}`) + else + authorized.delete('Authorization') + return authorized +} + export interface RepoMeta { name: string full_name: string @@ -101,6 +181,12 @@ export interface RateLimitInfo { remaining: number limit: number reset: number + /** + * The bucket GitHub counted the request against: `core` for REST and + * `graphql` for GraphQL. The two are separate quotas. A pause reason that + * said only "163 requests remaining" could not tell which one ran out. + */ + resource: string | null } export interface FetchOutcome { @@ -108,6 +194,11 @@ export interface FetchOutcome { data: T | null rateLimit: RateLimitInfo | null notModified: boolean + /** + * GitHub denied the read App this read, and no fallback token could repeat + * it. The status is 403, but no quota is spent. + */ + denied?: true } /** @@ -149,6 +240,7 @@ function parseRateLimit(headers: Headers): RateLimitInfo | null { remaining: Number(remaining), limit: Number(headers.get('x-ratelimit-limit') ?? 0), reset: Number(headers.get('x-ratelimit-reset') ?? 0), + resource: headers.get('x-ratelimit-resource'), } } @@ -161,8 +253,6 @@ async function ghRequest( headers.set('Accept', 'application/vnd.github+json') headers.set('User-Agent', 'skilld.dev') headers.set('X-GitHub-Api-Version', '2022-11-28') - if (bindings.GITHUB_TOKEN) - headers.set('Authorization', `Bearer ${bindings.GITHUB_TOKEN}`) const cacheKey = etagKey(url) let cached: CachedEntry | null = null @@ -175,7 +265,15 @@ async function ghRequest( headers.set('If-None-Match', cached.etag) } - const res = await fetch(url, { ...init, headers }) + const send = (token: string | undefined) => fetch(url, { ...init, headers: withAuthorization(headers, token) }) + const credential = await readCredential(bindings) + let res = await send(credential.token) + if (credential.isApp && await githubDeniedApp(res)) { + const fallback = fallbackAfterDenial(bindings, new URL(url).pathname) + if (fallback === null) + return { status: 403, data: null, rateLimit: parseRateLimit(res.headers), notModified: false, denied: true } + res = await send(fallback) + } const rateLimit = parseRateLimit(res.headers) if (res.status === 304 && cached) { @@ -197,6 +295,26 @@ async function ghRequest( return { status: res.status, data: body, rateLimit, notModified: false } } +/** + * One authenticated, conditional GET of an API path, such as + * `/users/nuxt`. A 304 answers from the KV ETag cache and costs no quota. + */ +export async function getGithubJson( + path: `/${string}`, + bindings: GithubBindings, + options?: GithubReadOptions, +): Promise> { + return ghRequest(`${API_BASE}${path}`, bindings, readInit(options)) +} + +/** + * Whether GitHub refused a read for want of quota or capacity, as opposed to + * answering it. A timeout or dropped connection reads as status 0. + */ +export function githubRefused(outcome: Pick, 'status'>): boolean { + return outcome.status === 0 || outcome.status === 403 || outcome.status === 429 || outcome.status >= 500 +} + export async function getRepo( owner: string, repo: string, @@ -206,31 +324,52 @@ export async function getRepo( return ghRequest(`${API_BASE}/repos/${owner}/${repo}`, bindings, readInit(options)) } -interface RepoSummaryGqlResponse { - repository: { +interface RepoSummaryGql { + name: string + nameWithOwner: string + url: string + owner: { login: string } + description: string | null + stargazerCount: number + forkCount: number + pushedAt: string + createdAt: string + isArchived: boolean + isFork: boolean + defaultBranchRef: { name: string - nameWithOwner: string - url: string - owner: { login: string } - description: string | null - stargazerCount: number - forkCount: number - pushedAt: string - createdAt: string - isArchived: boolean - isFork: boolean - defaultBranchRef: { - name: string - target: { oid: string, tree: { oid: string } } | null - } | null + target: { oid: string, tree: { oid: string } } | null } | null } +const REPO_SUMMARY_FIELDS = `name nameWithOwner url owner{login} + description stargazerCount forkCount pushedAt createdAt isArchived isFork + defaultBranchRef{name target{... on Commit{oid tree{oid}}}}` + export interface RepoSummary { meta: RepoMeta headTreeSha: string | null } +function repoSummaryFromGql(r: RepoSummaryGql): RepoSummary { + const branch = r.defaultBranchRef?.name || 'main' + const meta: RepoMeta = { + name: r.name, + full_name: r.nameWithOwner, + html_url: r.url, + owner: { login: r.owner.login }, + default_branch: branch, + description: r.description, + stargazers_count: r.stargazerCount, + forks_count: r.forkCount, + pushed_at: r.pushedAt, + created_at: r.createdAt, + archived: r.isArchived, + fork: r.isFork, + } + return { meta, headTreeSha: r.defaultBranchRef?.target?.tree.oid ?? null } +} + /** * GraphQL-backed combo of getRepo + head-tree-SHA. One subrequest replaces * the REST getRepo (1) and lets sync-repo skip the REST getTree call (1) @@ -248,59 +387,96 @@ export async function getRepoSummary( ): Promise> { const query = `query($owner:String!,$repo:String!){ repository(owner:$owner,name:$repo){ - name nameWithOwner url owner{login} - description stargazerCount forkCount pushedAt createdAt isArchived isFork - defaultBranchRef{name target{... on Commit{oid tree{oid}}}} + ${REPO_SUMMARY_FIELDS} } }` - const headers = new Headers() - headers.set('Accept', 'application/vnd.github+json') - headers.set('Content-Type', 'application/json') - headers.set('User-Agent', 'skilld.dev') - if (bindings.GITHUB_TOKEN) - headers.set('Authorization', `Bearer ${bindings.GITHUB_TOKEN}`) - - const res = await fetch(GRAPHQL_URL, { - method: 'POST', - headers, - body: JSON.stringify({ query, variables: { owner, repo } }), - }) - const rateLimit = parseRateLimit(res.headers) - if (!res.ok) - return { status: res.status, data: null, rateLimit, notModified: false } - - const body = await res.json() as { data?: RepoSummaryGqlResponse, errors?: Array<{ type?: string, message?: string }> } + const out = await gqlRequest<{ repository: RepoSummaryGql | null }>(query, { owner, repo }, bindings, { label: `${owner}/${repo}`, retryForbidden: true }) + if (out._tag === 'failed') { + return out.denied + ? { status: out.status, data: null, rateLimit: out.rateLimit, notModified: false, denied: true } + : { status: out.status, data: null, rateLimit: out.rateLimit, notModified: false } + } // GraphQL surfaces NOT_FOUND in errors with HTTP 200. Map it to 404 so // callers preserve their existing rate-limit / broken-repo branching. - if (body.errors?.length) { - const notFound = body.errors.some(e => e.type === 'NOT_FOUND') - return { status: notFound ? 404 : 502, data: null, rateLimit, notModified: false } + if (out.errors.length) { + const notFound = out.errors.some(e => e.type === 'NOT_FOUND') + return { status: notFound ? 404 : 502, data: null, rateLimit: out.rateLimit, notModified: false } } - const r = body.data?.repository + const r = out.data?.repository if (!r) - return { status: 404, data: null, rateLimit, notModified: false } + return { status: 404, data: null, rateLimit: out.rateLimit, notModified: false } + return { status: 200, data: repoSummaryFromGql(r), rateLimit: out.rateLimit, notModified: false } +} - const branch = r.defaultBranchRef?.name || 'main' - const meta: RepoMeta = { - name: r.name, - full_name: r.nameWithOwner, - html_url: r.url, - owner: { login: r.owner.login }, - default_branch: branch, - description: r.description, - stargazers_count: r.stargazerCount, - forks_count: r.forkCount, - pushed_at: r.pushedAt, - created_at: r.createdAt, - archived: r.isArchived, - fork: r.isFork, +/** + * Repositories one batched summary query reads. GitHub prices a query by the + * connections it asks for, and a summary asks for none, so one query of 100 + * costs the same single point as one query of 1. 100 is GitHub's ceiling for + * nodes per connection, and keeps the query under 15 KB. + */ +export const REPO_SUMMARY_BATCH_SIZE = 100 + +export interface RepoSummaryRequest { + owner: string + repo: string +} + +export type RepoSummaryBatchOutcome + = | { + _tag: 'read' + /** + * One entry per request, in request order. Null means GitHub gave no + * summary for that alias, such as NOT_FOUND. The caller leaves those to + * the per-repository sync, which owns the missing and renamed verdicts. + */ + summaries: Array + rateLimit: RateLimitInfo | null + /** GraphQL requests sent. */ + requests: number } - return { - status: 200, - data: { meta, headTreeSha: r.defaultBranchRef?.target?.tree.oid ?? null }, - rateLimit, - notModified: false, + | { _tag: 'failed', status: number, rateLimit: RateLimitInfo | null, requests: number } + +/** + * The summary of many repositories in aliased GraphQL queries of up to + * {@link REPO_SUMMARY_BATCH_SIZE}. + * + * The hourly sync used to send one summary query per repository from its own + * queue job. On 2026-10-06, 1,649 of 1,900 such jobs wrote no Skill row, and + * a job that finds its tree unchanged only advances the freshness cursor. + */ +export async function getRepoSummariesBatch( + requests: RepoSummaryRequest[], + bindings: GithubBindings, +): Promise { + const summaries: Array = [] + let rateLimit: RateLimitInfo | null = null + let sent = 0 + for (const batch of chunk(requests, REPO_SUMMARY_BATCH_SIZE)) { + const varDecls = batch.flatMap((_, i) => [`$o${i}:String!`, `$n${i}:String!`]) + const aliases = batch.map((_, i) => `r${i}:repository(owner:$o${i},name:$n${i}){${REPO_SUMMARY_FIELDS}}`).join(' ') + const variables: Record = {} + batch.forEach((request, i) => { + variables[`o${i}`] = request.owner + variables[`n${i}`] = request.repo + }) + sent++ + const out = await gqlRequest>(`query(${varDecls.join(',')}){${aliases}}`, variables, bindings, { label: 'a summary batch', retryForbidden: false }) + rateLimit = out.rateLimit ?? rateLimit + if (out._tag === 'failed') + return { _tag: 'failed', status: out.status, rateLimit, requests: sent } + // A NOT_FOUND names one alias and leaves the others whole. Any other + // error type, such as RATE_LIMITED, says nothing trustworthy about the + // batch, so it fails as a whole and the per-repository sync takes over. + // A FORBIDDEN alias is a Repository GitHub denies the read App. It reads + // as null here, and the per-repository sync repeats it with the fallback. + if (out.errors.some(error => error.type !== 'NOT_FOUND' && error.type !== 'FORBIDDEN')) + return { _tag: 'failed', status: 502, rateLimit, requests: sent } + for (let i = 0; i < batch.length; i++) { + const r = out.data?.[`r${i}`] + summaries.push(r ? repoSummaryFromGql(r) : null) + } } + return { _tag: 'read', summaries, rateLimit, requests: sent } } export async function getTree( @@ -341,39 +517,87 @@ export async function getCommits( ) } -async function gqlPost( +interface GqlError { type?: string, message?: string } + +type GqlRequestOutcome + = | { _tag: 'answered', data: T | null, errors: GqlError[], rateLimit: RateLimitInfo | null } + | { _tag: 'failed', status: number, rateLimit: RateLimitInfo | null, denied?: true } + +/** + * One GraphQL POST. Partial data and per-alias errors come back together. + * + * A query about one Repository that GitHub denied the read App, with a 403 + * or a FORBIDDEN error, repeats once with the fallback token. A batch over + * many Repositories passes `retryForbidden: false`: one denied alias says + * nothing about the others, and the per-repository sync repeats it. + */ +async function gqlRequest( query: string, variables: Record, bindings: GithubBindings, -): Promise<{ status: number, data: T | null, rateLimit: RateLimitInfo | null }> { + options: { label: string, retryForbidden: boolean }, +): Promise> { + const credential = await readCredential(bindings) + const first = await gqlSend(query, variables, credential.token, credential.isApp) + const deniedApp = first._tag === 'failed' + ? first.denied === true + : options.retryForbidden && first.errors.some(error => error.type === 'FORBIDDEN') + if (!credential.isApp || !deniedApp) + return first + const fallback = fallbackAfterDenial(bindings, options.label) + if (fallback === null) + return { _tag: 'failed', status: 403, rateLimit: first.rateLimit, denied: true } + return await gqlSend(query, variables, fallback, false) +} + +async function gqlSend( + query: string, + variables: Record, + token: string | undefined, + asApp: boolean, +): Promise> { const headers = new Headers() headers.set('Accept', 'application/vnd.github+json') headers.set('Content-Type', 'application/json') headers.set('User-Agent', 'skilld.dev') - if (bindings.GITHUB_TOKEN) - headers.set('Authorization', `Bearer ${bindings.GITHUB_TOKEN}`) + if (token) + headers.set('Authorization', `Bearer ${token}`) const res = await fetch(GRAPHQL_URL, { method: 'POST', headers, body: JSON.stringify({ query, variables }), }) const rateLimit = parseRateLimit(res.headers) - if (!res.ok) - return { status: res.status, data: null, rateLimit } + if (!res.ok) { + const denied = asApp && await githubDeniedApp(res) + return denied ? { _tag: 'failed', status: res.status, rateLimit, denied: true } : { _tag: 'failed', status: res.status, rateLimit } + } // A gateway can answer 200 with a truncated or non-JSON body. Parsing that // eagerly threw `Unexpected end of JSON input` out of the client and reached // the sync summary as an opaque reason with no status attached. const body = await res.json().catch(() => { emitOperationalEvent(createWideEvent({ operation: 'github-graphql-parse', outcome: 'invalid-response' })) return null - }) as { data?: T, errors?: Array<{ type?: string, message?: string }> } | null + }) as { data?: T, errors?: GqlError[] } | null if (!body) - return { status: 502, data: null, rateLimit } - if (body.errors?.length) { - const notFound = body.errors.some(e => e.type === 'NOT_FOUND') - return { status: notFound ? 404 : 502, data: null, rateLimit } + return { _tag: 'failed', status: 502, rateLimit } + return { _tag: 'answered', data: body.data ?? null, errors: body.errors ?? [], rateLimit } +} + +async function gqlPost( + query: string, + variables: Record, + bindings: GithubBindings, + label: string, +): Promise<{ status: number, data: T | null, rateLimit: RateLimitInfo | null, denied?: true }> { + const out = await gqlRequest(query, variables, bindings, { label, retryForbidden: true }) + if (out._tag === 'failed') + return out.denied ? { status: out.status, data: null, rateLimit: out.rateLimit, denied: true } : { status: out.status, data: null, rateLimit: out.rateLimit } + if (out.errors.length) { + const notFound = out.errors.some(e => e.type === 'NOT_FOUND') + return { status: notFound ? 404 : 502, data: null, rateLimit: out.rateLimit } } - return { status: 200, data: body.data ?? null, rateLimit } + return { status: 200, data: out.data, rateLimit: out.rateLimit } } export interface BlobBatchOutcome extends FetchOutcome> { @@ -420,12 +644,12 @@ export async function getBlobsBatch( variables[`expr${i}`] = `${branch}:${p}` }) - const out = await gqlPost<{ repository: Record | null }>(query, variables, bindings) + const out = await gqlPost<{ repository: Record | null }>(query, variables, bindings, `${owner}/${repo}`) rateLimit = out.rateLimit ?? rateLimit // A partial map would look like a repo that lost files, and the caller // would delete skills it simply failed to read. Fail the whole batch. if (!out.data?.repository) - return { status: out.status, data: null, unreadable, rateLimit, notModified: false } + return { status: out.status, data: null, unreadable, rateLimit, notModified: false, ...(out.denied ? { denied: true as const } : {}) } batch.forEach((p, i) => { const blob = out.data!.repository![`b${i}`] if (typeof blob?.text === 'string') @@ -473,11 +697,11 @@ export async function getCommitsBatch( variables[`path${i}`] = p }) - const out = await gqlPost(query, variables, bindings) + const out = await gqlPost(query, variables, bindings, `${owner}/${repo}`) rateLimit = out.rateLimit ?? rateLimit const target = out.data?.repository?.defaultBranchRef?.target if (!target) - return { status: out.status, data: null, rateLimit, notModified: false } + return { status: out.status, data: null, rateLimit, notModified: false, ...(out.denied ? { denied: true as const } : {}) } batch.forEach((p, i) => { const nodes = target[`h${i}`]?.nodes ?? [] map.set(p, nodes.map(c => ({ @@ -507,8 +731,9 @@ export async function getRawFile( ): Promise { const headers = new Headers() headers.set('User-Agent', 'skilld.dev') - if (bindings.GITHUB_TOKEN) - headers.set('Authorization', `Bearer ${bindings.GITHUB_TOKEN}`) + const credential = await readCredential(bindings) + if (credential.token) + headers.set('Authorization', `Bearer ${credential.token}`) const res = await fetch(`${RAW_BASE}/${owner}/${repo}/${ref}/${path}`, { headers }) if (!res.ok) return null diff --git a/layers/registry/server/utils/github-rate-limited.ts b/layers/registry/server/utils/github-rate-limited.ts new file mode 100644 index 00000000..8439a62b --- /dev/null +++ b/layers/registry/server/utils/github-rate-limited.ts @@ -0,0 +1,10 @@ +/** + * Whether GitHub refused a read for want of quota. + * + * A read GitHub denied the read App also answers 403, but it is a fact about + * one Repository: `denied` marks it, and it never pauses the sync. It lives + * apart from the GitHub client, which sync tests replace whole. + */ +export function githubRateLimited(outcome: { status: number, denied?: true }): boolean { + return (outcome.status === 403 || outcome.status === 429) && !outcome.denied +} diff --git a/layers/registry/server/utils/github-sync-control.ts b/layers/registry/server/utils/github-sync-control.ts index 8e28ff28..3ee37942 100644 --- a/layers/registry/server/utils/github-sync-control.ts +++ b/layers/registry/server/utils/github-sync-control.ts @@ -57,11 +57,24 @@ export function githubRatePauseUntil( return Math.min(now + 12 * 60 * 60, Math.max(fallback, (resetAt ?? 0) + 5)) } +/** + * Requests the sync leaves unspent in a GitHub bucket before it pauses. + * + * `GITHUB_TOKEN` is a personal token, and GitHub counts its quota per + * account, so every tool on that account draws on the same bucket. The sync + * spends about 10 REST requests and 130 GraphQL points an hour, and page + * views about 50 REST requests at most. 200 covers page views for the rest + * of any hour once the sync stops. + */ +export const GITHUB_SYNC_RESERVE = 200 + export function githubSyncPauseDecision(input: { owner: string repo: string now: number remaining?: number + /** The bucket `remaining` counts, from `x-ratelimit-resource`. */ + resource?: string resetAt?: number rateLimited: boolean unauthorized: boolean @@ -73,11 +86,12 @@ export function githubSyncPauseDecision(input: { reason: `${input.owner}/${input.repo}: GitHub credential rejected`, } } - if (!input.rateLimited && (input.remaining == null || input.remaining >= 200)) + if (!input.rateLimited && (input.remaining == null || input.remaining >= GITHUB_SYNC_RESERVE)) return { _tag: 'continue' } + const bucket = input.resource ? `${input.resource} ` : '' return { _tag: 'pause', pauseUntil: githubRatePauseUntil(input.now, input.resetAt), - reason: `${input.owner}/${input.repo}: ${input.rateLimited ? 'rate limited' : `${input.remaining} requests remaining`}`, + reason: `${input.owner}/${input.repo}: ${input.rateLimited ? 'rate limited' : `${input.remaining} ${bucket}requests remaining`}`, } } diff --git a/layers/registry/server/utils/sync-prefetch.ts b/layers/registry/server/utils/sync-prefetch.ts new file mode 100644 index 00000000..ec59cf2a --- /dev/null +++ b/layers/registry/server/utils/sync-prefetch.ts @@ -0,0 +1,144 @@ +/// + +import type { GithubBindings, RepoSummary } from './github-client' +import type { ExistingRepo } from './sync-repo' +import { getRepoSummariesBatch } from './github-client' +import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' +import { repoUnchangedStatements, unchangedRepoStatus } from './sync-repo' + +/** One repository the hourly sync is about to queue. */ +export interface SyncCandidate { + owner: string + repo: string + ownerVerified: boolean + claimDiscovery: boolean +} + +export type SyncPrefetchResult + = | { + _tag: 'prefetched' + /** Candidates that still need a sync job. */ + queue: SyncCandidate[] + /** Repositories recorded as unchanged without a job. */ + unchanged: number + /** GraphQL requests the prefetch sent. */ + requests: number + } + | { + _tag: 'unread' + /** Every candidate: the per-repository sync reads each one itself. */ + queue: SyncCandidate[] + reason: string + } + +export interface SyncPrefetchDependencies { + db: D1Database + bindings: GithubBindings + /** Unix seconds. */ + now: number +} + +/** A sync job that touched its progress row this recently may still hold it. */ +const PROGRESS_ACTIVE_SECONDS = 30 * 60 + +/** Repositories whose unchanged writes share one D1 batch. */ +const WRITE_BATCH_REPOSITORIES = 25 + +interface PrefetchRow extends ExistingRepo { + owner: string + repo: string + has_skills: number + in_progress: number +} + +/** + * Read every eligible candidate's summary in batched GraphQL queries, record + * the unchanged ones in place, and return the candidates that still need a job. + * + * Each candidate used to get a queue job whose first act was one GraphQL + * summary read. On 2026-10-06, 1,649 of 1,900 such jobs wrote no Skill row. + * A job that finds its tree unchanged only advances the freshness cursor, and + * the writes here are the same statements `syncRepo` runs on that path. + * + * A discovery claim, a job still holding the repository's progress row, a + * Skill-less repository and any repository GitHub gave no summary for keep + * their job: `syncRepo` owns those verdicts. + */ +export async function prefetchUnchangedRepos( + dependencies: SyncPrefetchDependencies, + candidates: SyncCandidate[], +): Promise { + const eligible = candidates.filter(candidate => !candidate.claimDiscovery) + if (eligible.length === 0) + return { _tag: 'prefetched', queue: candidates, unchanged: 0, requests: 0 } + + const rows = await loadPrefetchRows(dependencies.db, eligible, dependencies.now) + const requests = eligible.map(candidate => resolveRepoSourceIdentityFromRow(candidate, rows.get(repoKey(candidate)))) + const batch = await getRepoSummariesBatch(requests, dependencies.bindings) + if (batch._tag === 'failed') + return { _tag: 'unread', queue: candidates, reason: `graphql-${batch.status}` } + + const unchanged = new Set() + const writes: D1PreparedStatement[][] = [] + eligible.forEach((candidate, index) => { + const row = rows.get(repoKey(candidate)) + const summary = batch.summaries[index] + if (!row || !summary || row.in_progress === 1 || !isUnchanged(row, summary)) + return + unchanged.add(repoKey(candidate)) + writes.push(repoUnchangedStatements(dependencies.db, { + owner: candidate.owner, + repo: candidate.repo, + meta: summary.meta, + checkedAt: dependencies.now, + ownerVerified: candidate.ownerVerified, + })) + }) + for (let offset = 0; offset < writes.length; offset += WRITE_BATCH_REPOSITORIES) + await dependencies.db.batch(writes.slice(offset, offset + WRITE_BATCH_REPOSITORIES).flat()) + + return { + _tag: 'prefetched', + queue: candidates.filter(candidate => !unchanged.has(repoKey(candidate))), + unchanged: unchanged.size, + requests: batch.requests, + } +} + +function isUnchanged(row: PrefetchRow, summary: RepoSummary): boolean { + const pushedAt = Date.parse(summary.meta.pushed_at) + return unchangedRepoStatus({ + existing: row, + hasAdmittedSkills: row.has_skills === 1, + headTreeSha: summary.headTreeSha, + repoPushedAt: Number.isFinite(pushedAt) ? Math.floor(pushedAt / 1000) : null, + }) !== null +} + +function repoKey(identity: { owner: string, repo: string }): string { + return `${identity.owner}/${identity.repo}` +} + +/** One D1 read for every candidate: a JSON array binds as a single parameter. */ +async function loadPrefetchRows( + db: D1Database, + candidates: SyncCandidate[], + now: number, +): Promise> { + const pairs = JSON.stringify(candidates.map(candidate => [candidate.owner, candidate.repo])) + const result = await db.prepare( + `WITH candidates(owner, repo) AS ( + SELECT json_extract(value, '$[0]'), json_extract(value, '$[1]') + FROM json_each(?1) + ) + SELECT r.owner, r.repo, r.last_tree_sha, r.pushed_at, r.source_owner, r.source_repo, + EXISTS (SELECT 1 FROM skills s WHERE s.owner = r.owner AND s.repo = r.repo) AS has_skills, + EXISTS ( + SELECT 1 FROM repo_sync_progress p + WHERE p.owner = r.owner AND p.repo = r.repo AND p.updated_at > ?2 + ) AS in_progress + FROM candidates c + JOIN repos r ON r.owner = c.owner AND r.repo = c.repo`, + ).bind(pairs, now - PROGRESS_ACTIVE_SECONDS).all() + return new Map((result.results ?? []).map(row => [repoKey(row), row])) +} diff --git a/layers/registry/server/utils/sync-repo.ts b/layers/registry/server/utils/sync-repo.ts index 0833df80..dc44917d 100644 --- a/layers/registry/server/utils/sync-repo.ts +++ b/layers/registry/server/utils/sync-repo.ts @@ -5,6 +5,7 @@ import type { SkillTrustTier } from './skill-trust' import { canonicalSkillPaths, isRegistrySkillPath, isSkilldCacheSkill } from '#shared/skill-path' import { isCategoryPinned } from '../data/clusters' import { getBlobsBatch, getCommitsBatch, getRepoSummary, getTree, logRateLimit } from './github-client' +import { githubRateLimited } from './github-rate-limited' import { repoStarObservationStatements } from './repo-history' import { resolveRepoSourceIdentityFromRow } from './repo-source-identity' import { skillContentSha256 } from './skill-content-hash' @@ -25,6 +26,8 @@ export interface SyncRepoStats { revisionsInserted: number activityEmitted: number rateLimitRemaining?: number + /** The GitHub bucket `rateLimitRemaining` counts: `core` or `graphql`. */ + rateLimitResource?: string rateLimitResetAt?: number continuation?: SyncRepoContinuation } @@ -52,7 +55,7 @@ interface ExistingSkillAssets { assets: string } -interface ExistingRepo { +export interface ExistingRepo { last_tree_sha: string | null pushed_at: number | null source_owner: string | null @@ -343,8 +346,8 @@ export async function refreshRepoAssets( owner, repo, reason: `repo fetch ${repoRes.status}`, - retryable: repoRes.status === 403 || repoRes.status === 429 || repoRes.status >= 500, - rateLimited: repoRes.status === 403 || repoRes.status === 429, + retryable: githubRateLimited(repoRes) || repoRes.status >= 500, + rateLimited: githubRateLimited(repoRes), unauthorized: repoRes.status === 401, ...rate, } @@ -358,7 +361,7 @@ export async function refreshRepoAssets( SET source_owner = ?, source_repo = ? WHERE owner = ? AND repo = ? `).bind(sourceOwner, sourceRepo, owner, repo).run() - const treeRes = await getTree(sourceOwner, sourceRepo, branch, bindings) + const treeRes = await getTree(sourceOwner, sourceRepo, repoRes.data.headTreeSha ?? branch, bindings) logRateLimit(`asset-backfill tree ${owner}/${repo}`, treeRes.rateLimit) const rateLimitRemaining = Math.min( rate.rateLimitRemaining ?? Number.POSITIVE_INFINITY, @@ -375,8 +378,8 @@ export async function refreshRepoAssets( owner, repo, reason: `tree fetch ${treeRes.status}`, - retryable: treeRes.status === 403 || treeRes.status === 429 || treeRes.status >= 500, - rateLimited: treeRes.status === 403 || treeRes.status === 429, + retryable: githubRateLimited(treeRes) || treeRes.status >= 500, + rateLimited: githubRateLimited(treeRes), unauthorized: treeRes.status === 401, ...finalRate, } @@ -516,18 +519,62 @@ async function repoHasAdmittedSkills(db: D1Database, owner: string, repo: string return row?.admitted === 1 } -async function markUnchangedOwnerVerified( +/** + * Why a repository needs no content sync this hour, or null when it does. + * + * Only a repository with admitted Skills qualifies: a Skill-less candidate is + * still being discovered, so its tree must be read whatever the cursor says. + */ +export function unchangedRepoStatus(input: { + existing: ExistingRepo | null + hasAdmittedSkills: boolean + headTreeSha: string | null + repoPushedAt: number | null +}): 'skipped-tree-sha' | 'skipped-pushed-at' | null { + const { existing, hasAdmittedSkills, headTreeSha, repoPushedAt } = input + if (!hasAdmittedSkills || !existing?.last_tree_sha) + return null + if (headTreeSha && existing.last_tree_sha === headTreeSha) + return 'skipped-tree-sha' + if (existing.pushed_at != null && repoPushedAt != null && existing.pushed_at >= repoPushedAt) + return 'skipped-pushed-at' + return null +} + +/** + * The writes that record an unchanged repository: fresh metadata, the + * freshness cursor, the day's star count and, for an owner-verified repository, + * the verified flag on every Skill. + */ +export function repoUnchangedStatements( db: D1Database, - owner: string, - repo: string, - meta: RepoMeta, - pushedAt: number | null, - checkedAt: number, -): Promise { - await db.batch([ + input: { + owner: string + repo: string + meta: RepoMeta + checkedAt: number + ownerVerified: boolean + }, +): D1PreparedStatement[] { + const { owner, repo, meta, checkedAt } = input + const pushedAt = epoch(meta.pushed_at) + const statements = [ markRepoSummaryCheckedStatement(db, owner, repo, meta, pushedAt, checkedAt), clearRepoMissingSkillsStatement(db, owner, repo, checkedAt), ...repoStarObservationStatements(db, owner, repo, meta.stargazers_count ?? 0, checkedAt), + ] + return input.ownerVerified + ? [...statements, ...ownerVerifiedStatements(db, owner, repo, checkedAt)] + : statements +} + +function ownerVerifiedStatements( + db: D1Database, + owner: string, + repo: string, + checkedAt: number, +): D1PreparedStatement[] { + return [ db.prepare( `UPDATE skills SET owner_verified = 1 @@ -542,7 +589,7 @@ async function markUnchangedOwnerVerified( queued_at = excluded.queued_at, attempts = 0`, ).bind(checkedAt, owner, repo), - ]) + ] } /** @@ -726,12 +773,13 @@ export async function syncRepo( revisionsInserted: 0, activityEmitted: 0, } - const trackRateLimit = (rateLimit: { remaining: number, reset: number } | null): void => { + const trackRateLimit = (rateLimit: { remaining: number, reset: number, resource: string | null } | null): void => { if (!rateLimit) return - stats.rateLimitRemaining = stats.rateLimitRemaining == null - ? rateLimit.remaining - : Math.min(stats.rateLimitRemaining, rateLimit.remaining) + if (stats.rateLimitRemaining == null || rateLimit.remaining < stats.rateLimitRemaining) { + stats.rateLimitRemaining = rateLimit.remaining + stats.rateLimitResource = rateLimit.resource ?? undefined + } stats.rateLimitResetAt = Math.max(stats.rateLimitResetAt ?? 0, rateLimit.reset) } @@ -750,7 +798,7 @@ export async function syncRepo( return stats } - if (repoRes.status === 403 || repoRes.status === 429) { + if (githubRateLimited(repoRes)) { stats.status = 'rate-limited' stats.reason = `rate-limited (${repoRes.rateLimit?.remaining ?? '?'} remaining)` return stats @@ -760,7 +808,7 @@ export async function syncRepo( if (repoRes.status === 404 || repoRes.status === 410) await markRepoMissing(db, owner, repo, nowSec()) stats.status = 'failed' - stats.reason = `repo fetch ${repoRes.status}` + stats.reason = repoRes.denied ? 'github denied the read App (403)' : `repo fetch ${repoRes.status}` return stats } @@ -775,44 +823,29 @@ export async function syncRepo( const hasAdmittedSkills = await repoHasAdmittedSkills(db, owner, repo) const markUnchanged = async (status: 'skipped-tree-sha' | 'skipped-pushed-at'): Promise => { - if (opts.ownerVerified) { - await markUnchangedOwnerVerified(db, owner, repo, meta, repoPushedAt, checkedAt) - stats.status = 'verified-only' - return stats - } - await db.batch([ - markRepoSummaryCheckedStatement(db, owner, repo, meta, repoPushedAt, checkedAt), - clearRepoMissingSkillsStatement(db, owner, repo, checkedAt), - ...repoStarObservationStatements(db, owner, repo, meta.stargazers_count ?? 0, checkedAt), - ]) - stats.status = status + await db.batch(repoUnchangedStatements(db, { owner, repo, meta, checkedAt, ownerVerified: opts.ownerVerified === true })) + stats.status = opts.ownerVerified ? 'verified-only' : status return stats } // GraphQL gave us the head tree SHA in the same request. If it matches // our cached value, the repo is unchanged and we skip the REST getTree // call entirely. Skill-less candidates deliberately bypass this cursor. - if (!opts.continuation && !opts.forceContent && hasAdmittedSkills && existingRepo?.last_tree_sha && headTreeSha && existingRepo.last_tree_sha === headTreeSha) - return markUnchanged('skipped-tree-sha') - - if ( - !opts.continuation - && !opts.forceContent - && hasAdmittedSkills - && existingRepo?.pushed_at != null - && existingRepo.last_tree_sha != null - && repoPushedAt != null - && existingRepo.pushed_at >= repoPushedAt - ) { - return markUnchanged('skipped-pushed-at') - } - - const treeRes = await getTree(sourceOwner, sourceRepo, branch, bindings) + const unchanged = opts.continuation || opts.forceContent + ? null + : unchangedRepoStatus({ existing: existingRepo, hasAdmittedSkills, headTreeSha, repoPushedAt }) + if (unchanged) + return markUnchanged(unchanged) + + // The head tree SHA names the exact tree the summary saw. A branch name + // can move between the two reads, and a SHA URL is immutable, so its ETag + // cache entry answers 304 for as long as the tree stands. + const treeRes = await getTree(sourceOwner, sourceRepo, headTreeSha ?? branch, bindings) logRateLimit(`tree ${owner}/${repo}`, treeRes.rateLimit) trackRateLimit(treeRes.rateLimit) if (!treeRes.data) { - stats.status = treeRes.status === 403 || treeRes.status === 429 ? 'rate-limited' : 'failed' + stats.status = githubRateLimited(treeRes) ? 'rate-limited' : 'failed' stats.reason = `tree_fetch_failed:${treeRes.status}` return stats } @@ -1078,7 +1111,7 @@ export async function syncRepo( logRateLimit(`blobs ${owner}/${repo}`, blobsRes.rateLimit) trackRateLimit(blobsRes.rateLimit) if (!blobsRes.data) { - stats.status = blobsRes.status === 403 || blobsRes.status === 429 ? 'rate-limited' : 'failed' + stats.status = githubRateLimited(blobsRes) ? 'rate-limited' : 'failed' stats.reason = `blob_batch_failed:${blobsRes.status}` return stats } @@ -1220,7 +1253,7 @@ export async function syncRepo( logRateLimit(`commits-batch ${owner}/${repo}`, commitsRes.rateLimit) trackRateLimit(commitsRes.rateLimit) if (!commitsRes.data) { - stats.status = commitsRes.status === 403 || commitsRes.status === 429 ? 'rate-limited' : 'failed' + stats.status = githubRateLimited(commitsRes) ? 'rate-limited' : 'failed' stats.reason = `commit_batch_failed:${commitsRes.status}` return stats } diff --git a/layers/registry/server/utils/ungh-repo.ts b/layers/registry/server/utils/ungh-repo.ts new file mode 100644 index 00000000..2673261f --- /dev/null +++ b/layers/registry/server/utils/ungh-repo.ts @@ -0,0 +1,72 @@ +import type { RepoMeta } from './github-client' +import { z } from 'zod' +import { GITHUB_PAGE_READ_TIMEOUT_MS } from './github-client' + +const unghRepoSchema = z.object({ + repo: z.object({ + name: z.string().min(1), + repo: z.string().regex(/^[^/]+\/[^/]+$/), + description: z.string().nullable().optional(), + createdAt: z.string(), + pushedAt: z.string(), + stars: z.number().int().nonnegative(), + forks: z.number().int().nonnegative(), + defaultBranch: z.string().min(1), + }), +}) + +export type UnghRepoResult + = | { _tag: 'found', meta: RepoMeta } + | { _tag: 'unavailable', reason: string } + +/** + * Public repository metadata from ungh.cc, the unjs GitHub proxy that caches + * anonymous reads. + * + * A page-view fallback for when `GITHUB_TOKEN` is spent or GitHub is down. It + * answers metadata only. It never serves a tree or file content, and nothing + * built or signed reads it. ungh.cc knows no archived or fork flag, so both + * read false. Its 404 is no verdict about GitHub, so it reads as unavailable. + */ +export async function readUnghRepoMeta( + owner: string, + repo: string, + fetcher: typeof fetch = fetch, +): Promise { + const response = await fetcher(`https://ungh.cc/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`, { + headers: { 'User-Agent': 'skilld.dev', 'Accept': 'application/json' }, + signal: AbortSignal.timeout(GITHUB_PAGE_READ_TIMEOUT_MS), + }).catch((error: unknown) => ({ _tag: 'threw' as const, reason: error instanceof Error ? error.message : String(error) })) + if ('_tag' in response) + return { _tag: 'unavailable', reason: response.reason } + if (!response.ok) { + await response.body?.cancel() + return { _tag: 'unavailable', reason: `ungh ${response.status}` } + } + const parsed = unghRepoSchema.safeParse(await response.json().catch(() => { + // A body that is not JSON fails the schema below, which reports it. + return null + })) + if (!parsed.success) + return { _tag: 'unavailable', reason: 'ungh invalid response' } + const { repo: found } = parsed.data + const [login] = found.repo.split('/') as [string, string] + return { + _tag: 'found', + meta: { + name: found.name, + full_name: found.repo, + html_url: `https://github.com/${found.repo}`, + owner: { login }, + default_branch: found.defaultBranch, + description: found.description ?? null, + stargazers_count: found.stars, + forks_count: found.forks, + pushed_at: found.pushedAt, + created_at: found.createdAt, + archived: false, + fork: false, + private: false, + }, + } +} diff --git a/migrations/0144_artifact_resolution_requesters.sql b/migrations/0144_artifact_resolution_requesters.sql new file mode 100644 index 00000000..246075cd --- /dev/null +++ b/migrations/0144_artifact_resolution_requesters.sql @@ -0,0 +1,13 @@ +-- The signed-in account that asked for a public Resolution, kept only while +-- its build runs. When the shared GitHub credentials are rate limited, the +-- build reads GitHub with this account's own token, for this build only. +-- The build deletes the row when it settles; the daily personal data purge +-- removes any row a dead build left behind. +CREATE TABLE artifact_resolution_requesters ( + resolution_id TEXT PRIMARY KEY REFERENCES artifact_resolutions(id) ON DELETE CASCADE, + account_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + created_at INTEGER NOT NULL +); + +CREATE INDEX idx_artifact_resolution_requesters_created_at + ON artifact_resolution_requesters(created_at); diff --git a/server/jobs/registry/repo-maintenance.ts b/server/jobs/registry/repo-maintenance.ts index dc8d16b9..1559f297 100644 --- a/server/jobs/registry/repo-maintenance.ts +++ b/server/jobs/registry/repo-maintenance.ts @@ -190,6 +190,7 @@ async function applyRateGuard( repo: string now: number remaining?: number + resource?: string resetAt?: number rateLimited: boolean unauthorized: boolean @@ -333,6 +334,7 @@ export async function handleRegistryRepoJob( repo: payload.repo, now, remaining: stats.rateLimitRemaining, + resource: stats.rateLimitResource, resetAt: stats.rateLimitResetAt, rateLimited: stats.status === 'rate-limited', unauthorized: stats.status === 'unauthorized', diff --git a/shared/server/github-app-credential.ts b/shared/server/github-app-credential.ts new file mode 100644 index 00000000..03dd0973 --- /dev/null +++ b/shared/server/github-app-credential.ts @@ -0,0 +1,393 @@ +import { z } from 'zod' + +/** + * The read App's GitHub credential, shared by every Worker caller. + * + * GitHub counts a personal token's quota per account, so every token of one + * account shares one bucket, and the account owner's own tools spend it too. + * The read App (`skilld-dev-registry-reads`, metadata read only) has an + * installation with a bucket of its own. A personal token remains only as the + * fallback for a Repository whose organization denies the App. + */ + +const GITHUB_API = 'https://api.github.com' +const GITHUB_API_VERSION = '2026-03-10' +const GITHUB_REQUEST_TIMEOUT_MS = 15_000 +const MAX_TOKEN_RESPONSE_BYTES = 64 * 1024 + +/** + * Renew an installation token this long before GitHub says it expires, so a + * read that starts with it never sees it lapse part way through. + */ +export const INSTALLATION_TOKEN_RENEW_SECONDS = 5 * 60 + +/** The read App: metadata read only, installed on skilld-dev. */ +export interface ReadAppConfig { + appId: number + installationId: number + privateKeyPkcs8: Uint8Array +} + +/** A personal token a caller may fall back to, by its secret name. */ +export interface FallbackToken { + name: 'ARTIFACT_GITHUB_TOKEN' | 'GITHUB_TOKEN' + token: string +} + +/** Where a caller's GitHub credential comes from, parsed once from the Worker env. */ +export type GithubCredentialConfig + = | { _tag: 'app', app: ReadAppConfig, fallback: FallbackToken | null } + /** App secrets are set but unusable. Reads use the fallback and report why. */ + | { _tag: 'app-misconfigured', reason: string, fallback: FallbackToken | null } + /** No App secret is set, as in local development and tests. */ + | { _tag: 'token', token: FallbackToken } + | { _tag: 'anonymous' } + +export interface GithubCredentialEnv { + SKILLD_READ_APP_ID?: string + SKILLD_READ_APP_INSTALLATION_ID?: string + SKILLD_READ_APP_PRIVATE_KEY_PKCS8?: string + ARTIFACT_GITHUB_TOKEN?: string + GITHUB_TOKEN?: string +} + +/** + * Why a read used a token other than the App's, or found none. It names + * secrets and paths, never a token value. + */ +export interface GithubCredentialReport { + outcome: 'app-misconfigured' | 'app-token-unavailable' | 'app-denied' + reason: string + fallback: FallbackToken['name'] | 'anonymous' | 'none' +} + +interface CachedInstallationToken { + token: string + /** Unix seconds. */ + expiresAt: number +} + +/** + * Installation tokens kept between reads in one isolate. A token lives an + * hour; minting one per read would add a request and a signature to each. + */ +export interface InstallationTokenCache { + tokens: Map + pending: Map> +} + +export function createInstallationTokenCache(): InstallationTokenCache { + return { tokens: new Map(), pending: new Map() } +} + +let isolateTokenCache: InstallationTokenCache | null = null + +/** + * The one installation token cache of this isolate, so the registry and the + * build queue share a token instead of minting one each. + */ +export function isolateInstallationTokenCache(): InstallationTokenCache { + isolateTokenCache ??= createInstallationTokenCache() + return isolateTokenCache +} + +export interface GithubCredentialRuntime { + fetch: typeof globalThis.fetch + /** Unix seconds. */ + now: () => number + tokenCache: InstallationTokenCache + report: (event: GithubCredentialReport) => void +} + +/** The credential for one GitHub read. An undefined token reads anonymously. */ +export interface GithubReadCredential { + token: string | undefined + /** The read App's installation token, which a Repository can deny. */ + isApp: boolean +} + +export interface GithubCredential { + current: () => Promise + /** The token for a read GitHub denied the App, or null. */ + fallback: FallbackToken | null +} + +/** + * Parse the read App secrets and the caller's fallback tokens, in order. + * + * With no App secret at all, the first fallback token becomes the primary + * credential, which is what local development runs on. + */ +export function parseGithubCredentialConfig( + env: GithubCredentialEnv, + fallbackNames: ReadonlyArray, +): GithubCredentialConfig { + const fallback = firstToken(env, fallbackNames) + const appSecrets = [env.SKILLD_READ_APP_ID, env.SKILLD_READ_APP_INSTALLATION_ID, env.SKILLD_READ_APP_PRIVATE_KEY_PKCS8] + if (appSecrets.every(value => !value?.trim())) + return fallback ? { _tag: 'token', token: fallback } : { _tag: 'anonymous' } + const misconfigured = (reason: string): GithubCredentialConfig => ({ _tag: 'app-misconfigured', reason, fallback }) + const appId = positiveInteger(env.SKILLD_READ_APP_ID) + if (appId === null) + return misconfigured('SKILLD_READ_APP_ID is not a GitHub App ID') + const installationId = positiveInteger(env.SKILLD_READ_APP_INSTALLATION_ID) + if (installationId === null) + return misconfigured('SKILLD_READ_APP_INSTALLATION_ID is not an installation ID') + const privateKeyPkcs8 = pkcs8PemBytes(env.SKILLD_READ_APP_PRIVATE_KEY_PKCS8 ?? '') + if (!privateKeyPkcs8) + return misconfigured('SKILLD_READ_APP_PRIVATE_KEY_PKCS8 is not a PKCS #8 PEM private key') + return { _tag: 'app', app: { appId, installationId, privateKeyPkcs8 }, fallback } +} + +/** + * The GitHub credential a caller reads with: the read App's installation + * token, or the fallback when the App gives none. + * + * When the App cannot give a token, the read still runs on the fallback and + * the reason is reported, so a broken key shows up as an event instead of as + * a spent quota. + */ +export function createGithubCredential( + config: GithubCredentialConfig, + runtime: GithubCredentialRuntime, +): GithubCredential { + if (config._tag === 'anonymous') + return { current: async () => ({ token: undefined, isApp: false }), fallback: null } + if (config._tag === 'token') + return { current: async () => ({ token: config.token.token, isApp: false }), fallback: null } + const fallbackName = config.fallback?.name ?? 'anonymous' + const fallback = { token: config.fallback?.token, isApp: false } + if (config._tag === 'app-misconfigured') { + return { + current: async () => { + runtime.report({ outcome: 'app-misconfigured', reason: config.reason, fallback: fallbackName }) + return fallback + }, + fallback: null, + } + } + const app = config.app + const key = `${app.appId}:${app.installationId}` + return { + current: async () => { + const cached = runtime.tokenCache.tokens.get(key) + if (cached && cached.expiresAt - INSTALLATION_TOKEN_RENEW_SECONDS > runtime.now()) + return { token: cached.token, isApp: true } + let pending = runtime.tokenCache.pending.get(key) + if (!pending) { + pending = mintInstallationToken(app, runtime).finally(() => runtime.tokenCache.pending.delete(key)) + runtime.tokenCache.pending.set(key, pending) + } + const minted = await pending + if ('token' in minted) { + runtime.tokenCache.tokens.set(key, minted) + return { token: minted.token, isApp: true } + } + runtime.report({ outcome: 'app-token-unavailable', reason: minted.reason, fallback: fallbackName }) + return fallback + }, + fallback: config.fallback, + } +} + +/** + * Whether GitHub refused a read because it denies the App, as opposed to a + * spent quota. Organizations that restrict Apps answer 403 with quota left, + * and no rate limit message. `neondatabase` did on 2026-10-06. + * + * It reads the body of a 403, so pass a clone or a response whose body is + * not needed. + */ +export async function githubDeniedApp(response: Response): Promise { + if (response.status !== 403) + return false + if (response.headers.get('x-ratelimit-remaining') === '0' || response.headers.get('retry-after')) + return false + // A 403 body that will not read names no rate limit either, so it counts + // as a denial. The status already says the read failed. + const text = await response.text().catch(() => '') + return !/rate limit/i.test(text) +} + +const installationTokenSchema = z.object({ + token: z.string().min(1).max(2048), + expires_at: z.string().datetime(), +}).passthrough() + +export type InstallationTokenResult + = | { _tag: 'created', token: string, expiresAt: string } + /** GitHub answered 401, 403 or 404: the App, its key or the installation is gone. */ + | { _tag: 'refused', status: number } + +/** + * Exchange an App JWT for an installation access token. + * + * A refusal is a value. Any other failure, such as a 5xx or an unreadable + * answer, throws: it says nothing about the App. + */ +export async function requestInstallationToken(input: { + fetch: typeof globalThis.fetch + jwt: string + installationId: number + body: { repository_ids?: number[], permissions: Record } +}): Promise { + const response = await input.fetch(`${GITHUB_API}/app/installations/${input.installationId}/access_tokens`, { + method: 'POST', + headers: { + 'Accept': 'application/vnd.github+json', + 'Authorization': `Bearer ${input.jwt}`, + 'Content-Type': 'application/json', + 'User-Agent': 'skilld.dev', + 'X-GitHub-Api-Version': GITHUB_API_VERSION, + }, + body: JSON.stringify(input.body), + // workerd accepts only `follow` and `manual`. A redirect here is an error. + redirect: 'manual', + signal: AbortSignal.timeout(GITHUB_REQUEST_TIMEOUT_MS), + }) + if (response.status >= 300 && response.status < 400) { + await response.body?.cancel() + throw new Error(`GitHub App request redirected ${response.status}`) + } + if (response.status === 401 || response.status === 403 || response.status === 404) { + await response.body?.cancel() + return { _tag: 'refused', status: response.status } + } + if (!response.ok) + throw new Error(`GitHub App request returned ${response.status}`) + const value = installationTokenSchema.safeParse(await readBoundedJson(response, MAX_TOKEN_RESPONSE_BYTES)) + if (!value.success) + throw new Error('GitHub App returned an invalid response') + return { _tag: 'created', token: value.data.token, expiresAt: value.data.expires_at } +} + +/** + * An RS256 GitHub App JWT. `issuer` is the App's client ID or its App ID; + * GitHub accepts either. + * + * It is valid for ten minutes from a minute in the past, which absorbs clock + * drift between the Worker and GitHub. Ten minutes is GitHub's maximum. + */ +export async function signGithubAppJwt(input: { + issuer: string + privateKeyPkcs8: Uint8Array + /** Unix seconds. */ + now: number +}): Promise { + const issuedAt = input.now - 60 + const header = base64Url(new TextEncoder().encode(JSON.stringify({ alg: 'RS256', typ: 'JWT' }))) + const payload = base64Url(new TextEncoder().encode(JSON.stringify({ + iat: issuedAt, + exp: issuedAt + 600, + iss: input.issuer, + }))) + const unsigned = `${header}.${payload}` + const imported = await crypto.subtle.importKey( + 'pkcs8', + Uint8Array.from(input.privateKeyPkcs8).buffer, + { name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256' }, + false, + ['sign'], + ) + const signature = await crypto.subtle.sign( + 'RSASSA-PKCS1-v1_5', + imported, + new TextEncoder().encode(unsigned), + ) + return `${unsigned}.${base64Url(new Uint8Array(signature))}` +} + +async function mintInstallationToken( + app: ReadAppConfig, + runtime: Pick, +): Promise { + const minted = await signGithubAppJwt({ issuer: String(app.appId), privateKeyPkcs8: app.privateKeyPkcs8, now: runtime.now() }) + .then(jwt => requestInstallationToken({ + fetch: runtime.fetch, + jwt, + installationId: app.installationId, + body: { permissions: { metadata: 'read' } }, + })) + // A network fault, a 5xx or a key WebCrypto rejects: the read uses the + // fallback token, and the reason reaches the report. + .catch((error: unknown) => ({ _tag: 'failed' as const, reason: error instanceof Error ? error.message : String(error) })) + if (minted._tag === 'failed') + return { _tag: 'unavailable', reason: minted.reason.slice(0, 200) } + if (minted._tag === 'refused') + return { _tag: 'unavailable', reason: `GitHub refused the installation token (${minted.status})` } + const expiresAt = Math.floor(Date.parse(minted.expiresAt) / 1000) + return { token: minted.token, expiresAt } +} + +function firstToken(env: GithubCredentialEnv, names: ReadonlyArray): FallbackToken | null { + for (const name of names) { + const token = env[name]?.trim() + if (token) + return { name, token } + } + return null +} + +function positiveInteger(value: string | undefined): number | null { + const trimmed = value?.trim() ?? '' + if (!/^[1-9]\d*$/.test(trimmed)) + return null + const parsed = Number(trimmed) + return Number.isSafeInteger(parsed) ? parsed : null +} + +/** + * The DER bytes of a PKCS #8 PEM private key, or null. + * + * A secret pasted through a dashboard or a JSON file can hold its line breaks + * as the two characters `\n`, so both forms parse. A PKCS #1 key + * (`BEGIN RSA PRIVATE KEY`, as GitHub downloads it) does not: WebCrypto + * imports PKCS #8 only. Convert it with `openssl pkcs8 -topk8 -nocrypt`. + */ +function pkcs8PemBytes(value: string): Uint8Array | null { + const text = value.replaceAll('\\n', '\n').trim() + const match = /^-----BEGIN PRIVATE KEY-----([A-Z0-9+/=\s]+)-----END PRIVATE KEY-----$/i.exec(text) + if (!match) + return null + const base64 = match[1]!.replace(/\s+/g, '') + if (!/^[A-Z0-9+/]+={0,2}$/i.test(base64) || base64.length % 4 !== 0) + return null + const binary = atob(base64) + return Uint8Array.from(binary, character => character.charCodeAt(0)) +} + +function base64Url(bytes: Uint8Array): string { + let binary = '' + for (let offset = 0; offset < bytes.length; offset += 0x8000) + binary += String.fromCharCode(...bytes.subarray(offset, offset + 0x8000)) + return btoa(binary).replaceAll('+', '-').replaceAll('/', '_').replace(/=+$/, '') +} + +async function readBoundedJson(response: Response, maximumBytes: number): Promise { + const declared = Number(response.headers.get('content-length')) + if (Number.isFinite(declared) && declared > maximumBytes) + throw new Error('GitHub App response exceeded the byte limit') + if (!response.body) + throw new Error('GitHub App returned an empty response') + const reader = response.body.getReader() + const chunks: Uint8Array[] = [] + let size = 0 + while (true) { + const next = await reader.read() + if (next.done) + break + size += next.value.byteLength + if (size > maximumBytes) { + await reader.cancel('response too large') + throw new Error('GitHub App response exceeded the byte limit') + } + chunks.push(next.value) + } + const bytes = new Uint8Array(size) + let offset = 0 + for (const chunk of chunks) { + bytes.set(chunk, offset) + offset += chunk.byteLength + } + return JSON.parse(new TextDecoder('utf-8', { fatal: true }).decode(bytes)) as unknown +} diff --git a/test/unit/account-deletion.test.ts b/test/unit/account-deletion.test.ts index 66827b02..e6fc9094 100644 --- a/test/unit/account-deletion.test.ts +++ b/test/unit/account-deletion.test.ts @@ -190,6 +190,14 @@ function seedAccount(raw: DatabaseSync, account: Account, githubToken: string | id, installationId, ) + run( + `INSERT INTO artifact_resolutions ( + id, request_fingerprint, state, requested_owner, requested_repository, selector_type, selector_value, visibility + ) VALUES (?, ?, 'resolving', 'acme', 'skills', 'named-skill', 'tdd', 'public')`, + `public-run-${login}`, + `public-fingerprint-${login}`, + ) + run(`INSERT INTO artifact_resolution_requesters (resolution_id, account_id, created_at) VALUES (?, ?, ?)`, `public-run-${login}`, id, NOW) run(`INSERT INTO artifact_check_results (resolution_id, name, version, outcome, required) VALUES (?, 'skill-structure', '1', 'pass', 1)`, resolutionId) run( `INSERT INTO artifacts (id, content_sha256, content_bytes, format, r2_key) VALUES (?, ?, 128, 'skilld-tar-v1', ?)`, diff --git a/test/unit/artifact-build-reuse.test.ts b/test/unit/artifact-build-reuse.test.ts index 6725b405..67831896 100644 --- a/test/unit/artifact-build-reuse.test.ts +++ b/test/unit/artifact-build-reuse.test.ts @@ -43,6 +43,7 @@ const MIGRATIONS = [ 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0131_artifact_build_reuse.sql', + 'migrations/0144_artifact_resolution_requesters.sql', ] const COMMIT = '0123456789abcdef0123456789abcdef01234567' const NEXT_COMMIT = 'fedcba9876543210fedcba9876543210fedcba98' diff --git a/test/unit/artifact-delivery.test.ts b/test/unit/artifact-delivery.test.ts index 6fc5aaea..ea838500 100644 --- a/test/unit/artifact-delivery.test.ts +++ b/test/unit/artifact-delivery.test.ts @@ -41,6 +41,7 @@ const ARTIFACT_MIGRATIONS = [ 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0144_artifact_resolution_requesters.sql', ] const sourceRequest: SourceRequest = { provider: 'github', diff --git a/test/unit/artifact-github-read-app.test.ts b/test/unit/artifact-github-read-app.test.ts index 95f3a5d4..bcdf950f 100644 --- a/test/unit/artifact-github-read-app.test.ts +++ b/test/unit/artifact-github-read-app.test.ts @@ -1,8 +1,8 @@ import type { SourceRequest } from '../../layers/artifact-delivery/server/schemas/contracts' -import type { ArtifactGithubCredentialReport } from '../../layers/artifact-delivery/server/utils/github-read-credential' +import type { GithubCredentialReport } from '../../shared/server/github-app-credential' import { describe, expect, it } from 'vitest' -import { createInstallationTokenCache } from '../../layers/artifact-delivery/server/utils/github-read-credential' import { createArtifactGithubSource } from '../../layers/artifact-delivery/server/utils/queue' +import { createInstallationTokenCache } from '../../shared/server/github-app-credential' // Builds shared the site token with the hourly registry sync and every `/gh` // page view, so a run failed RATE_LIMITED whenever the sync spent it. The read @@ -90,7 +90,7 @@ describe('the GitHub credential public Artifact builds read with', () => { it('reads with the fallback token, and reports why, when GitHub refuses the installation token', async () => { const keys = await appKeys() const github = fakeGithub({ expiresAt: NOW + 3600, mintStatus: 401 }) - const reports: ArtifactGithubCredentialReport[] = [] + const reports: GithubCredentialReport[] = [] const source = createArtifactGithubSource(appEnv(keys.pem), runtime(github, { report: event => reports.push(event) })) await source.resolve(request) @@ -101,7 +101,7 @@ describe('the GitHub credential public Artifact builds read with', () => { it('reports an unreadable App key and reads with the fallback token', async () => { const github = fakeGithub({ expiresAt: NOW + 3600 }) - const reports: ArtifactGithubCredentialReport[] = [] + const reports: GithubCredentialReport[] = [] const env = { ...appEnv('-----BEGIN RSA PRIVATE KEY-----\nAAAA\n-----END RSA PRIVATE KEY-----'), ARTIFACT_GITHUB_TOKEN: '' } const source = createArtifactGithubSource(env, runtime(github, { report: event => reports.push(event) })) @@ -115,7 +115,7 @@ describe('the GitHub credential public Artifact builds read with', () => { it('never puts a token or the key in a report', async () => { const keys = await appKeys() const github = fakeGithub({ expiresAt: NOW + 3600, mintStatus: 500 }) - const reports: ArtifactGithubCredentialReport[] = [] + const reports: GithubCredentialReport[] = [] await createArtifactGithubSource(appEnv(keys.pem), runtime(github, { report: event => reports.push(event) })).resolve(request) const text = JSON.stringify(reports) @@ -124,6 +124,35 @@ describe('the GitHub credential public Artifact builds read with', () => { expect(text).not.toContain('PRIVATE KEY') expect(reports).toHaveLength(1) }) + + it('repeats a read the Repository denied the read App with the fallback token, and reports it', async () => { + const keys = await appKeys() + const github = fakeGithub({ expiresAt: NOW + 3600, deniesApp: true }) + const reports: GithubCredentialReport[] = [] + const source = createArtifactGithubSource(appEnv(keys.pem), runtime(github, { report: event => reports.push(event) })) + + const result = await source.resolve(request) + + expect(github.readAuthorizations).toEqual(['Bearer ghs_installation_1', 'Bearer runs']) + // The fallback read reached GitHub, which has no such Repository. + expect(result).toMatchObject({ _tag: 'rejected', code: 'SOURCE_NOT_FOUND' }) + expect(reports).toEqual([{ outcome: 'app-denied', reason: 'GitHub denied the read App for skilld-dev/skills', fallback: 'ARTIFACT_GITHUB_TOKEN' }]) + }) + + it('keeps the denial when no token follows the read App', async () => { + const keys = await appKeys() + const github = fakeGithub({ expiresAt: NOW + 3600, deniesApp: true }) + const env = { ...appEnv(keys.pem), ARTIFACT_GITHUB_TOKEN: '', GITHUB_TOKEN: '' } + + const result = await createArtifactGithubSource(env, runtime(github)).resolve(request) + + expect(github.readAuthorizations).toEqual(['Bearer ghs_installation_1']) + expect(result).toMatchObject({ + _tag: 'rejected', + code: 'SOURCE_ACCESS_DENIED', + summary: 'GitHub denies the skilld.dev read App access to this Repository, and no fallback token is set.', + }) + }) }) interface FakeGithub { @@ -132,7 +161,7 @@ interface FakeGithub { readAuthorizations: Array } -function fakeGithub(options: { expiresAt: number, mintStatus?: number }): FakeGithub { +function fakeGithub(options: { expiresAt: number, mintStatus?: number, deniesApp?: boolean }): FakeGithub { const mints: FakeGithub['mints'] = [] const readAuthorizations: FakeGithub['readAuthorizations'] = [] const fetcher = async (input: RequestInfo | URL, init?: RequestInit): Promise => { @@ -144,7 +173,12 @@ function fakeGithub(options: { expiresAt: number, mintStatus?: number }): FakeGi return new Response('{}', { status: options.mintStatus }) return Response.json({ token: `ghs_installation_${mints.length}`, expires_at: new Date(options.expiresAt * 1000).toISOString() }, { status: 201 }) } - readAuthorizations.push(headers.get('authorization')) + const authorization = headers.get('authorization') + readAuthorizations.push(authorization) + // Some organizations answer the read App 403 on a public Repository that + // a personal token reads: neondatabase/agent-skills on 2026-10-06. + if (options.deniesApp && authorization?.startsWith('Bearer ghs_')) + return Response.json({ message: 'Resource not accessible by integration' }, { status: 403 }) return Response.json({ message: 'Not Found' }, { status: 404 }) } return { fetch: fetcher as typeof fetch, mints, readAuthorizations } @@ -152,7 +186,7 @@ function fakeGithub(options: { expiresAt: number, mintStatus?: number }): FakeGi function runtime( github: FakeGithub, - overrides: Partial<{ cache: ReturnType, now: () => number, report: (event: ArtifactGithubCredentialReport) => void }> = {}, + overrides: Partial<{ cache: ReturnType, now: () => number, report: (event: GithubCredentialReport) => void }> = {}, ) { return { fetch: github.fetch, diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 133cec2e..873a817e 100644 --- a/test/unit/artifact-rate-limit.test.ts +++ b/test/unit/artifact-rate-limit.test.ts @@ -22,6 +22,7 @@ const ARTIFACT_MIGRATIONS = [ 'migrations/0111_github_app_delivery.sql', 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', + 'migrations/0144_artifact_resolution_requesters.sql', ] const resolutionRequest: SourceRequest = { provider: 'github', diff --git a/test/unit/artifact-requester-github.test.ts b/test/unit/artifact-requester-github.test.ts new file mode 100644 index 00000000..b4fa30af --- /dev/null +++ b/test/unit/artifact-requester-github.test.ts @@ -0,0 +1,188 @@ +import type { SourceRequest } from '../../layers/artifact-delivery/server/schemas/contracts' +import type { ArtifactSigner } from '../../layers/artifact-delivery/server/utils/attestation' +import type { ArtifactBuildDependencies } from '../../layers/artifact-delivery/server/utils/build' +import type { PublicGithubSourceClient, SourceRejection } from '../../layers/artifact-delivery/server/utils/github-source' +import type { TrustedRoot } from '../../layers/artifact-delivery/server/utils/trusted-root' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { processArtifactBuild } from '../../layers/artifact-delivery/server/utils/build' +import { ARTIFACT_BUILD_QUEUE_NAME, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' +import { requestResolution } from '../../layers/artifact-delivery/server/utils/request-resolution' +import { createRequesterGithubSource } from '../../layers/artifact-delivery/server/utils/requester-github' +import { getResolution } from '../../layers/artifact-delivery/server/utils/state' +import { encryptToken } from '../../layers/identity/server/utils/crypto' +import { purgeRetainedPersonalData } from '../../layers/identity/server/utils/data-retention' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +// The shared GitHub buckets run out each hour. A signed-in run can still read +// GitHub with its own account's token, which has its own quota, for its own +// build only. +const NOW = 1_791_300_000 +const RESET_AT = NOW + 600 +const TOKEN_KEY = btoa(String.fromCharCode(...new Uint8Array(32).fill(7))) +const ALICE = 9101 +const BOB = 9102 +const request: SourceRequest = { + provider: 'github', + owner: 'skilld-dev', + repository: 'skills', + selector: { type: 'path', path: 'skills/demo' }, +} + +describe('a signed-in run when the shared GitHub credential is rate limited', () => { + let fixture: ReturnType + + beforeEach(async () => { + fixture = createSqliteD1(allMigrations()) + const insert = fixture.raw.prepare( + `INSERT INTO users (id, github_id, login, github_token_encrypted, created_at, last_login_at) + VALUES (?, ?, ?, ?, ?, ?)`, + ) + insert.run(ALICE, 900_001, 'alice', await encryptToken('gho_alice', TOKEN_KEY), NOW, NOW) + insert.run(BOB, 900_002, 'bob', await encryptToken('gho_bob', TOKEN_KEY), NOW, NOW) + }) + + afterEach(() => fixture.close()) + + it('reads GitHub with the requester\'s own token', async () => { + const github = userGithub() + const id = await requested('alice-run-0000001', ALICE) + + await processArtifactBuild(dependencies(github), id) + + expect(github.authorizations).toContain('Bearer gho_alice') + expect(github.authorizations).not.toContain('Bearer gho_bob') + // The requester's read reached GitHub, which has no such Repository. + expect(await getResolution(fixture.db, id)).toMatchObject({ state: 'failed', error_code: 'SOURCE_NOT_FOUND' }) + }) + + it('keeps RATE_LIMITED for an anonymous run, and reads with no user token', async () => { + const github = userGithub() + const id = await requested('anonymous-run-001', null) + + await processArtifactBuild(dependencies(github), id) + + expect(github.authorizations).toEqual([]) + expect(await getResolution(fixture.db, id)).toMatchObject({ state: 'failed', error_code: 'RATE_LIMITED' }) + }) + + it('never reads one user\'s build with another user\'s token', async () => { + const github = userGithub() + await requested('alice-run-0000002', ALICE) + const bobRun = await requested('bob-run-000000001', BOB) + const anonymousRun = await requested('anonymous-run-002', null) + + await processArtifactBuild(dependencies(github), bobRun) + await processArtifactBuild(dependencies(github), anonymousRun) + + expect(github.authorizations.length).toBeGreaterThan(0) + expect(github.authorizations.every(value => value === 'Bearer gho_bob')).toBe(true) + }) + + it('keeps the shared credential while it has quota', async () => { + const github = userGithub() + const shared = sharedGithub({ code: 'SOURCE_NOT_FOUND' }) + const id = await requested('alice-run-0000003', ALICE) + + await processArtifactBuild(dependencies(github, shared), id) + + expect(github.authorizations).toEqual([]) + expect(shared.resolve).toHaveBeenCalledOnce() + }) + + it('reads with the requester\'s token when GitHub denies the read App and no fallback token is set', async () => { + const github = userGithub() + const shared = sharedGithub({ code: 'SOURCE_ACCESS_DENIED' }) + const id = await requested('alice-run-0000008', ALICE) + + await processArtifactBuild(dependencies(github, shared), id) + + expect(github.authorizations).toContain('Bearer gho_alice') + expect(await getResolution(fixture.db, id)).toMatchObject({ state: 'failed', error_code: 'SOURCE_NOT_FOUND' }) + }) + + it('reports RATE_LIMITED when the requester has no stored GitHub token', async () => { + fixture.raw.prepare(`UPDATE users SET github_token_encrypted = NULL WHERE id = ?`).run(ALICE) + const github = userGithub() + const id = await requested('alice-run-0000004', ALICE) + + await processArtifactBuild(dependencies(github), id) + + expect(github.authorizations).toEqual([]) + expect(await getResolution(fixture.db, id)).toMatchObject({ state: 'failed', error_code: 'RATE_LIMITED' }) + }) + + it('forgets who asked once the build settles', async () => { + const github = userGithub() + const id = await requested('alice-run-0000005', ALICE) + const message = { id: 'message-1', attempts: 1, body: { version: 1, resolutionId: id }, ack: vi.fn(), retry: vi.fn() } + + await consumeArtifactBuildBatch( + {} as Cloudflare.Env, + { queue: ARTIFACT_BUILD_QUEUE_NAME, messages: [message] } as unknown as Parameters[1], + () => dependencies(github), + ) + + expect(message.ack).toHaveBeenCalledOnce() + expect(requesters()).toEqual([]) + }) + + it('purges a requester a dead build left behind after a day', async () => { + const stale = await requested('alice-run-0000006', ALICE) + fixture.raw.prepare(`UPDATE artifact_resolution_requesters SET created_at = ? WHERE resolution_id = ?`).run(NOW - 86_401, stale) + const fresh = await requested('alice-run-0000007', ALICE) + + await purgeRetainedPersonalData(fixture.db, NOW) + + expect(requesters()).toEqual([fresh]) + }) + + function requesters(): unknown[] { + return fixture.raw.prepare(`SELECT resolution_id FROM artifact_resolution_requesters`).all().map(row => Object.values(row)[0]) + } + + async function requested(idempotencyKey: string, requesterAccountId: number | null): Promise { + const result = await requestResolution({ + db: fixture.db, + lookupAdmitted: async () => null, + enqueue: async () => {}, + now: () => NOW, + }, { source: request, idempotencyKey, access: { visibility: 'public' }, requesterAccountId }) + if (result._tag === 'idempotency-conflict') + throw new Error('Test Resolution conflicted') + return result.row.id + } + + function dependencies(github: UserGithub, shared: PublicGithubSourceClient = sharedGithub({ code: 'RATE_LIMITED' })): ArtifactBuildDependencies { + return { + db: fixture.db, + github: shared, + requesterGithub: createRequesterGithubSource({ db: fixture.db, tokenKey: () => TOKEN_KEY, fetch: github.fetch, now: () => NOW }), + bucket: {} as R2Bucket, + signer: {} as ArtifactSigner, + trustedRoot: {} as TrustedRoot, + now: () => NOW, + } + } +}) + +function sharedGithub(rejection: { code: SourceRejection['code'] }): PublicGithubSourceClient & { resolve: ReturnType } { + const rejected: SourceRejection = rejection.code === 'RATE_LIMITED' + ? { _tag: 'rejected', code: 'RATE_LIMITED', summary: 'GitHub refused the read: its rate limit is spent.', findings: [], retryAfterSeconds: RESET_AT } + : { _tag: 'rejected', code: rejection.code, summary: 'The Repository was not found.', findings: [] } + return { resolve: vi.fn(async () => rejected), load: vi.fn(async () => rejected) } +} + +interface UserGithub { + fetch: typeof fetch + authorizations: Array +} + +/** GitHub as a user token sees it: it has quota, and no such Repository. */ +function userGithub(): UserGithub { + const authorizations: UserGithub['authorizations'] = [] + const fetcher = async (_input: RequestInfo | URL, init?: RequestInit) => { + authorizations.push(new Headers(init?.headers).get('authorization')) + return Response.json({ message: 'Not Found' }, { status: 404 }) + } + return { fetch: fetcher as typeof fetch, authorizations } +} diff --git a/test/unit/github-conditional-read.test.ts b/test/unit/github-conditional-read.test.ts new file mode 100644 index 00000000..1734cab7 --- /dev/null +++ b/test/unit/github-conditional-read.test.ts @@ -0,0 +1,43 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getGithubJson } from '../../layers/registry/server/utils/github-client' + +/** A KV namespace over a Map: the ETag cache the client writes and reads. */ +function memoryKv(): KVNamespace { + const values = new Map() + return { + get: async (key: string, type?: string) => { + const value = values.get(key) + return value === undefined ? null : type === 'json' ? JSON.parse(value) : value + }, + put: async (key: string, value: string) => { + values.set(key, value) + }, + } as unknown as KVNamespace +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('getGithubJson', () => { + it('asks again with the stored ETag and answers a 304 from the cache', async () => { + const seen: Array<{ authorization: string | null, ifNoneMatch: string | null }> = [] + vi.stubGlobal('fetch', vi.fn(async (_url: string, init: RequestInit) => { + const headers = new Headers(init.headers) + seen.push({ authorization: headers.get('authorization'), ifNoneMatch: headers.get('if-none-match') }) + return headers.get('if-none-match') === '"v1"' + ? new Response(null, { status: 304 }) + : new Response(JSON.stringify({ login: 'nuxt', type: 'Organization' }), { status: 200, headers: { etag: '"v1"' } }) + })) + const bindings = { GITHUB_TOKEN: 'registry-token', KV_CACHE: memoryKv() } + + await getGithubJson('/users/nuxt', bindings) + const repeat = await getGithubJson<{ type: string }>('/users/nuxt', bindings) + + expect(repeat).toMatchObject({ status: 304, notModified: true, data: { type: 'Organization' } }) + expect(seen).toEqual([ + { authorization: 'Bearer registry-token', ifNoneMatch: null }, + { authorization: 'Bearer registry-token', ifNoneMatch: '"v1"' }, + ]) + }) +}) diff --git a/test/unit/github-repo-summary-batch.test.ts b/test/unit/github-repo-summary-batch.test.ts new file mode 100644 index 00000000..e300de3b --- /dev/null +++ b/test/unit/github-repo-summary-batch.test.ts @@ -0,0 +1,99 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getRepoSummariesBatch } from '../../layers/registry/server/utils/github-client' + +const bindings = { GITHUB_TOKEN: 'test-token' } + +interface GraphqlBody { + query: string + variables: Record +} + +function repository(owner: string, name: string, tree: string) { + return { + name, + nameWithOwner: `${owner}/${name}`, + url: `https://github.com/${owner}/${name}`, + owner: { login: owner }, + description: null, + stargazerCount: 3, + forkCount: 1, + pushedAt: '2026-10-01T00:00:00Z', + createdAt: '2025-01-01T00:00:00Z', + isArchived: false, + isFork: false, + defaultBranchRef: { name: 'main', target: { oid: 'c'.repeat(40), tree: { oid: tree } } }, + } +} + +/** Answers every alias, except owners named `gone`, which GitHub reports NOT_FOUND. */ +function graphqlAnswer(_input: unknown, init?: RequestInit): Response { + const body = JSON.parse(String(init?.body)) as GraphqlBody + const data: Record = {} + const errors: unknown[] = [] + for (const key of Object.keys(body.variables).filter(name => name.startsWith('o'))) { + const index = key.slice(1) + const owner = body.variables[key]! + const name = body.variables[`n${index}`]! + if (owner === 'gone') { + data[`r${index}`] = null + errors.push({ type: 'NOT_FOUND', path: [`r${index}`], message: 'Could not resolve' }) + } + else { + data[`r${index}`] = repository(owner, name, `tree-${owner}-${name}`) + } + } + return new Response(JSON.stringify({ data, errors: errors.length ? errors : undefined }), { + status: 200, + headers: { 'x-ratelimit-remaining': '4990', 'x-ratelimit-resource': 'graphql' }, + }) +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('getRepoSummariesBatch', () => { + it('reads 230 repositories in three queries, in request order', async () => { + const fetchMock = vi.fn(graphqlAnswer) + vi.stubGlobal('fetch', fetchMock) + const requests = Array.from({ length: 230 }, (_, i) => ({ owner: 'acme', repo: `repo-${i}` })) + + const result = await getRepoSummariesBatch(requests, bindings) + + expect(fetchMock).toHaveBeenCalledTimes(3) + expect(result._tag).toBe('read') + if (result._tag !== 'read') + return + expect(result.summaries).toHaveLength(230) + expect(result.summaries[229]?.headTreeSha).toBe('tree-acme-repo-229') + expect(result.summaries[0]?.meta.full_name).toBe('acme/repo-0') + expect(result.rateLimit?.resource).toBe('graphql') + }) + + it('leaves a repository GitHub cannot find empty and keeps the rest', async () => { + vi.stubGlobal('fetch', vi.fn(graphqlAnswer)) + + const result = await getRepoSummariesBatch([ + { owner: 'acme', repo: 'one' }, + { owner: 'gone', repo: 'two' }, + { owner: 'acme', repo: 'three' }, + ], bindings) + + expect(result._tag === 'read' && result.summaries.map(summary => summary?.headTreeSha ?? null)).toEqual([ + 'tree-acme-one', + null, + 'tree-acme-three', + ]) + }) + + it('fails the whole batch on an error that is not NOT_FOUND', async () => { + vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ + data: null, + errors: [{ type: 'RATE_LIMITED', message: 'API rate limit exceeded' }], + }), { status: 200 }))) + + const result = await getRepoSummariesBatch([{ owner: 'acme', repo: 'one' }], bindings) + + expect(result).toMatchObject({ _tag: 'failed', status: 502 }) + }) +}) diff --git a/test/unit/github-sync-control.test.ts b/test/unit/github-sync-control.test.ts index 42cc26f3..114ecded 100644 --- a/test/unit/github-sync-control.test.ts +++ b/test/unit/github-sync-control.test.ts @@ -41,4 +41,20 @@ describe('github sync pause decision', () => { remaining: 200, })).toEqual({ _tag: 'continue' }) }) + + it('names the GitHub bucket that ran low', () => { + expect(githubSyncPauseDecision({ + now: 1_000, + owner: 'nuxt', + repo: 'ui', + unauthorized: false, + rateLimited: false, + remaining: 163, + resource: 'graphql', + resetAt: 2_000, + })).toMatchObject({ + _tag: 'pause', + reason: 'nuxt/ui: 163 graphql requests remaining', + }) + }) }) diff --git a/test/unit/registry-github-app-credential.test.ts b/test/unit/registry-github-app-credential.test.ts new file mode 100644 index 00000000..dc659c77 --- /dev/null +++ b/test/unit/registry-github-app-credential.test.ts @@ -0,0 +1,195 @@ +import type { GithubCredentialReport } from '../../shared/server/github-app-credential' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + getRepo, + getRepoSummariesBatch, + getRepoSummary, + resolveGithubBindings, +} from '../../layers/registry/server/utils/github-client' +import { githubRateLimited } from '../../layers/registry/server/utils/github-rate-limited' +import { createInstallationTokenCache } from '../../shared/server/github-app-credential' + +// Every Worker GitHub read used `GITHUB_TOKEN`, a personal token whose quota +// the owner's own tools and CI runners also spend. The read App's +// installation has a bucket of its own. `GITHUB_TOKEN` is now read only where +// GitHub denies the App, such as an organization that restricts Apps. +const APP_ID = '5212127' +const INSTALLATION_ID = '168540695' +const NOW = 1_791_300_000 + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('registry GitHub reads with the read App', () => { + it('reads with the installation token, not GITHUB_TOKEN', async () => { + const github = await stubGithub({}) + const bindings = resolveGithubBindings(await appEnv({ GITHUB_TOKEN: 'site' }), runtime()) + + const repo = await getRepo('nuxt', 'ui', bindings) + + expect(repo.status).toBe(200) + expect(github.mints).toBe(1) + expect(github.reads).toEqual(['Bearer ghs_app']) + }) + + it('repeats a read GitHub denied the App with GITHUB_TOKEN, and reports it', async () => { + const github = await stubGithub({ deniesApp: true }) + const reports: GithubCredentialReport[] = [] + const bindings = resolveGithubBindings(await appEnv({ GITHUB_TOKEN: 'site' }), runtime(reports)) + + const repo = await getRepo('neondatabase', 'agent-skills', bindings) + + expect(repo.status).toBe(200) + expect(github.reads).toEqual(['Bearer ghs_app', 'Bearer site']) + expect(reports).toEqual([{ + outcome: 'app-denied', + reason: 'GitHub denied the read App for /repos/neondatabase/agent-skills', + fallback: 'GITHUB_TOKEN', + }]) + }) + + it('answers a denied read as denied, not rate limited, with no GITHUB_TOKEN', async () => { + const github = await stubGithub({ deniesApp: true }) + const reports: GithubCredentialReport[] = [] + const bindings = resolveGithubBindings(await appEnv({}), runtime(reports)) + + const repo = await getRepo('neondatabase', 'agent-skills', bindings) + + expect(github.reads).toEqual(['Bearer ghs_app']) + expect(repo).toMatchObject({ status: 403, denied: true, data: null }) + // The sync pauses every repository for a spent quota. A denial names one. + expect(githubRateLimited(repo)).toBe(false) + expect(reports).toEqual([{ + outcome: 'app-denied', + reason: 'GitHub denied the read App for /repos/neondatabase/agent-skills', + fallback: 'none', + }]) + }) + + it('keeps a secondary rate limit a rate limit, with no second read', async () => { + const github = await stubGithub({ secondaryLimit: true }) + const bindings = resolveGithubBindings(await appEnv({ GITHUB_TOKEN: 'site' }), runtime()) + + const repo = await getRepo('nuxt', 'ui', bindings) + + expect(github.reads).toEqual(['Bearer ghs_app']) + expect(githubRateLimited(repo)).toBe(true) + }) + + it('repeats a GraphQL summary GitHub answered FORBIDDEN with GITHUB_TOKEN', async () => { + const github = await stubGithub({ deniesApp: true }) + const bindings = resolveGithubBindings(await appEnv({ GITHUB_TOKEN: 'site' }), runtime()) + + const summary = await getRepoSummary('neondatabase', 'agent-skills', bindings) + + expect(summary.status).toBe(200) + expect(github.reads).toEqual(['Bearer ghs_app', 'Bearer site']) + }) + + it('leaves a FORBIDDEN alias to the per-repository sync and keeps the rest of the batch', async () => { + const github = await stubGithub({ deniesApp: true }) + const bindings = resolveGithubBindings(await appEnv({ GITHUB_TOKEN: 'site' }), runtime()) + + const batch = await getRepoSummariesBatch([ + { owner: 'nuxt', repo: 'ui' }, + { owner: 'neondatabase', repo: 'agent-skills' }, + ], bindings) + + expect(batch).toMatchObject({ _tag: 'read', summaries: [{ meta: { full_name: 'nuxt/ui' } }, null] }) + expect(github.reads).toEqual(['Bearer ghs_app']) + }) + + it('reads with GITHUB_TOKEN when no App secret is set, as in local development', async () => { + const github = await stubGithub({}) + const bindings = resolveGithubBindings({ GITHUB_TOKEN: 'local' }, runtime()) + + await getRepo('nuxt', 'ui', bindings) + + expect(github.mints).toBe(0) + expect(github.reads).toEqual(['Bearer local']) + }) +}) + +function runtime(reports: GithubCredentialReport[] = []) { + return { tokenCache: createInstallationTokenCache(), now: () => NOW, report: (event: GithubCredentialReport) => reports.push(event) } +} + +async function appEnv(tokens: { GITHUB_TOKEN?: string }) { + 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 + const der = new Uint8Array(await crypto.subtle.exportKey('pkcs8', pair.privateKey) as ArrayBuffer) + const pem = `-----BEGIN PRIVATE KEY-----\n${btoa(String.fromCharCode(...der))}\n-----END PRIVATE KEY-----\n` + return { + SKILLD_READ_APP_ID: APP_ID, + SKILLD_READ_APP_INSTALLATION_ID: INSTALLATION_ID, + SKILLD_READ_APP_PRIVATE_KEY_PKCS8: pem, + ...tokens, + } +} + +/** + * GitHub as the App and a personal token see it. `neondatabase` denies the + * App on every read, as it did on 2026-10-06, and answers a personal token. + */ +async function stubGithub(options: { deniesApp?: boolean, secondaryLimit?: boolean }) { + const state = { mints: 0, reads: [] as Array } + vi.stubGlobal('fetch', vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input) + const headers = new Headers(init?.headers) + if (url.endsWith(`/app/installations/${INSTALLATION_ID}/access_tokens`)) { + state.mints++ + return Response.json({ token: 'ghs_app', expires_at: new Date((NOW + 3600) * 1000).toISOString() }, { status: 201 }) + } + const authorization = headers.get('authorization') + state.reads.push(authorization) + const asApp = authorization === 'Bearer ghs_app' + if (options.secondaryLimit) + return Response.json({ message: 'You have exceeded a secondary rate limit. Please wait a few minutes before you try again.' }, { status: 403 }) + if (url.endsWith('/graphql')) { + const body = JSON.parse(String(init?.body)) as { variables: Record } + const owners = Object.entries(body.variables).filter(([key]) => key === 'owner' || /^o\d+$/.test(key)) + const denied = options.deniesApp && asApp && owners.some(([, owner]) => owner === 'neondatabase') + const repository = (owner: string, name: string) => ({ + name, + nameWithOwner: `${owner}/${name}`, + url: `https://github.com/${owner}/${name}`, + owner: { login: owner }, + description: null, + stargazerCount: 1, + forkCount: 0, + pushedAt: '2026-10-01T00:00:00Z', + createdAt: '2026-01-01T00:00:00Z', + isArchived: false, + isFork: false, + defaultBranchRef: { name: 'main', target: { oid: 'a'.repeat(40), tree: { oid: 'b'.repeat(40) } } }, + }) + if ('owner' in body.variables) { + return denied + ? Response.json({ data: { repository: null }, errors: [{ type: 'FORBIDDEN', message: 'Resource not accessible by integration' }] }) + : Response.json({ data: { repository: repository(body.variables.owner!, body.variables.repo!) } }) + } + const data: Record = {} + const errors: Array<{ type: string, path: string[] }> = [] + for (const [key, owner] of owners) { + const alias = `r${key.slice(1)}` + const name = body.variables[`n${key.slice(1)}`]! + if (owner === 'neondatabase' && denied) { + data[alias] = null + errors.push({ type: 'FORBIDDEN', path: [alias] }) + } + else { + data[alias] = repository(owner, name) + } + } + return Response.json(errors.length ? { data, errors } : { data }) + } + if (options.deniesApp && asApp && url.includes('/neondatabase/')) + return Response.json({ message: 'Resource not accessible by integration' }, { status: 403, headers: { 'x-ratelimit-remaining': '4990' } }) + return Response.json({ name: 'repo', full_name: 'owner/repo', owner: { login: 'owner' }, default_branch: 'main' }, { status: 200 }) + })) + return state +} diff --git a/test/unit/repo-source-profile-quota.test.ts b/test/unit/repo-source-profile-quota.test.ts new file mode 100644 index 00000000..c1597be9 --- /dev/null +++ b/test/unit/repo-source-profile-quota.test.ts @@ -0,0 +1,109 @@ +import type { H3Event } from 'h3' +import type { SqliteD1 } from './helpers/d1-sqlite' +import { createStorage } from 'unstorage' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +const route = { owner: 'acme', repo: 'skills' } +let edgeCache = createStorage() +let harness: SqliteD1 + +function stubRuntime(): void { + vi.stubGlobal('defineCachedEventHandler', (handler: unknown) => handler) + vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) + vi.stubGlobal('getRouterParam', (_event: unknown, key: 'owner' | 'repo') => route[key]) + vi.stubGlobal('useStorage', () => edgeCache) +} + +const TREE_SHA = 'a'.repeat(40) + +function repoMeta(overrides: Record = {}) { + return { + name: 'skills', + full_name: 'acme/skills', + html_url: 'https://github.com/acme/skills', + owner: { login: 'acme' }, + default_branch: 'main', + description: 'Skills', + stargazers_count: 12, + forks_count: 3, + pushed_at: '2026-10-01T00:00:00Z', + created_at: '2025-01-01T00:00:00Z', + private: false, + ...overrides, + } +} + +function json(body: unknown, init: ResponseInit = {}): Response { + return new Response(JSON.stringify(body), { status: 200, ...init }) +} + +async function profile(): Promise> { + const handler = (await import('../../layers/registry/server/api/repos/[owner]/[repo].get')).default as (event: H3Event) => Promise> + return handler({ context: { platform: { db: harness.db, env: { GITHUB_TOKEN: 'token' } } } } as unknown as H3Event) +} + +function calls(fetchMock: ReturnType, fragment: string): number { + return fetchMock.mock.calls.filter(([input]) => String(input).includes(fragment)).length +} + +beforeEach(() => { + edgeCache = createStorage() + harness = createSqliteD1(allMigrations()) + route.repo = 'skills' + stubRuntime() +}) + +// The global wide event stubs come from the setup file, so this file never +// calls `vi.unstubAllGlobals()`: each test replaces the stubs it needs. +afterEach(() => { + harness.close() +}) + +describe('repository source profile GitHub spend', () => { + it('answers 404 for a private repository and never lists its tree', async () => { + const fetchMock = vi.fn(async () => json(repoMeta({ private: true }))) + vi.stubGlobal('fetch', fetchMock) + + await expect(profile()).rejects.toMatchObject({ statusCode: 404 }) + expect(calls(fetchMock, '/git/trees/')).toBe(0) + }) + + it('remembers a repository GitHub cannot find, so a repeat view reads nothing', async () => { + route.repo = 'gone' + const fetchMock = vi.fn(async () => json({ message: 'Not Found' }, { status: 404 })) + vi.stubGlobal('fetch', fetchMock) + + await expect(profile()).rejects.toMatchObject({ statusCode: 404 }) + await expect(profile()).rejects.toMatchObject({ statusCode: 404 }) + + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('reads repository metadata from ungh.cc when GitHub refuses with a spent quota', async () => { + const fetchMock = vi.fn(async (input: unknown) => String(input).startsWith('https://ungh.cc/') + ? json({ repo: { id: 1, name: 'skills', repo: 'acme/skills', description: 'From ungh', createdAt: '2025-01-01T00:00:00Z', pushedAt: '2026-10-01T00:00:00Z', stars: 99, forks: 4, defaultBranch: 'trunk' } }) + : json({ message: 'API rate limit exceeded' }, { status: 403, headers: { 'x-ratelimit-remaining': '0' } })) + vi.stubGlobal('fetch', fetchMock) + + const body = await profile() + + expect(body).toMatchObject({ owner: 'acme', repo: 'skills', stars: 99, description: 'From ungh', defaultBranch: 'trunk', skillFileScanStatus: 'unavailable' }) + expect(calls(fetchMock, '/git/trees/')).toBe(0) + }) + + it('lists a registry repository at its synced tree, once per tree', async () => { + harness.raw.prepare(`INSERT INTO repos (owner, repo, last_tree_sha) VALUES ('acme', 'skills', ?)`).run(TREE_SHA) + const fetchMock = vi.fn(async (input: unknown) => String(input).includes('/git/trees/') + ? json({ sha: TREE_SHA, tree: [{ path: 'skills/one/SKILL.md', type: 'blob', sha: 'b'.repeat(40) }] }) + : json(repoMeta())) + vi.stubGlobal('fetch', fetchMock) + + const first = await profile() + const second = await profile() + + expect(first).toMatchObject({ skillFileScanStatus: 'ok', skillFiles: ['skills/one/SKILL.md'] }) + expect(second).toMatchObject({ skillFiles: ['skills/one/SKILL.md'] }) + expect(fetchMock.mock.calls.filter(([input]) => String(input).includes(`/git/trees/${TREE_SHA}`))).toHaveLength(1) + }) +}) diff --git a/test/unit/repo-source-profile-timeout.test.ts b/test/unit/repo-source-profile-timeout.test.ts index 1e6f4baf..4458ecf6 100644 --- a/test/unit/repo-source-profile-timeout.test.ts +++ b/test/unit/repo-source-profile-timeout.test.ts @@ -1,16 +1,19 @@ import type { H3Event } from 'h3' +import { createStorage } from 'unstorage' import { afterEach, describe, expect, it, vi } from 'vitest' import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' vi.stubGlobal('defineCachedEventHandler', (handler: unknown) => handler) vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) vi.stubGlobal('getRouterParam', (_event: unknown, key: string) => (key === 'owner' ? 'skilld-dev' : 'skills')) +vi.stubGlobal('useStorage', () => createStorage()) afterEach(() => { vi.unstubAllGlobals() vi.stubGlobal('defineCachedEventHandler', (handler: unknown) => handler) vi.stubGlobal('createError', (input: Record) => Object.assign(new Error(String(input.message)), input)) vi.stubGlobal('getRouterParam', (_event: unknown, key: string) => (key === 'owner' ? 'skilld-dev' : 'skills')) + vi.stubGlobal('useStorage', () => createStorage()) }) async function profile() { diff --git a/test/unit/skill-detail-d1-cost.test.ts b/test/unit/skill-detail-d1-cost.test.ts index 152ef08e..eee0d577 100644 --- a/test/unit/skill-detail-d1-cost.test.ts +++ b/test/unit/skill-detail-d1-cost.test.ts @@ -236,17 +236,17 @@ describe('findSkill', () => { describe('uncached skill related render', () => { it('reads D1 at most three times and keeps the commit source', async () => { seed({ maximumQueries: 3 }) - upstreamFetch.mockImplementationOnce(async () => [{ + rawFetch.mockImplementationOnce(async () => new Response(JSON.stringify([{ sha: 'abcdef1234567', html_url: 'https://github.com/c/abcdef1', commit: { message: 'edit alpha', author: { name: 'Eric', date: '2026-09-01T00:00:00Z' } }, author: null, - }] as never) + }]))) const handler = (await import('../../layers/registry/server/api/skill-related/[...slug].get')).default as (event: H3Event) => Promise> const body = await handler({ context: { platform: { db: harness.db, env: {} } }, node: { req: { headers: {} } } } as unknown as H3Event) - expect(upstreamFetch).toHaveBeenCalledWith(`https://api.github.com/repos/${OWNER}/${REPO}/commits`, expect.objectContaining({ query: { path: 'skills/alpha/SKILL.md', per_page: 5 } })) + expect(String(rawFetch.mock.calls[0]?.[0])).toBe(`https://api.github.com/repos/${OWNER}/${REPO}/commits?path=skills%2Falpha%2FSKILL.md&per_page=5`) expect(body.commits.map((commit: { shortSha: string }) => commit.shortSha)).toEqual(['abcdef1']) expect(body.relatedRepoSkills.map((skill: { name: string }) => skill.name)).toEqual(['beta']) }) diff --git a/test/unit/skill-run-sweep.test.ts b/test/unit/skill-run-sweep.test.ts index 55302f16..417c5ecf 100644 --- a/test/unit/skill-run-sweep.test.ts +++ b/test/unit/skill-run-sweep.test.ts @@ -17,6 +17,7 @@ const MIGRATIONS = [ 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0142_artifact_run_checks.sql', 'migrations/0143_artifact_run_check_streak.sql', + 'migrations/0144_artifact_resolution_requesters.sql', ] const NOW = 1_791_000_000 const archify: RunnableSkill = { owner: 'tt-a1i', repository: 'archify', name: 'archify' } diff --git a/test/unit/sync-prefetch.test.ts b/test/unit/sync-prefetch.test.ts new file mode 100644 index 00000000..a8d0f640 --- /dev/null +++ b/test/unit/sync-prefetch.test.ts @@ -0,0 +1,113 @@ +import type { SqliteD1 } from './helpers/d1-sqlite' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { prefetchUnchangedRepos } from '../../layers/registry/server/utils/sync-prefetch' +import { allMigrations, createSqliteD1 } from './helpers/d1-sqlite' + +const NOW = 1_791_300_000 +const bindings = { GITHUB_TOKEN: 'test-token' } + +/** The head tree GitHub reports for each repository. */ +const heads: Record = { + 'acme/unchanged': 'tree-same', + 'acme/changed': 'tree-new', + 'acme/fresh': 'tree-fresh', + 'acme/busy': 'tree-busy', + 'acme/discovered': 'tree-discovered', +} + +function graphqlAnswer(_input: unknown, init?: RequestInit): Response { + const body = JSON.parse(String(init?.body)) as { variables: Record } + const data: Record = {} + const errors: unknown[] = [] + for (const key of Object.keys(body.variables).filter(name => name.startsWith('o'))) { + const index = key.slice(1) + const fullName = `${body.variables[key]}/${body.variables[`n${index}`]}` + const tree = heads[fullName] + if (!tree) { + data[`r${index}`] = null + errors.push({ type: 'NOT_FOUND', path: [`r${index}`] }) + continue + } + const [owner, name] = fullName.split('/') as [string, string] + data[`r${index}`] = { + name, + nameWithOwner: fullName, + url: `https://github.com/${fullName}`, + owner: { login: owner }, + description: 'Skills', + stargazerCount: 77, + forkCount: 2, + pushedAt: '2026-10-01T00:00:00Z', + createdAt: '2025-01-01T00:00:00Z', + isArchived: false, + isFork: false, + defaultBranchRef: { name: 'main', target: { oid: 'c'.repeat(40), tree: { oid: tree } } }, + } + } + return new Response(JSON.stringify({ data, errors: errors.length ? errors : undefined }), { status: 200 }) +} + +function candidate(repo: string, claimDiscovery = false, owner = 'acme') { + return { owner, repo, ownerVerified: false, claimDiscovery } +} + +describe('prefetchUnchangedRepos', () => { + let harness: SqliteD1 + + beforeEach(() => { + harness = createSqliteD1(allMigrations()) + for (const [repo, tree] of [['unchanged', 'tree-same'], ['changed', 'tree-old'], ['fresh', null], ['busy', 'tree-busy'], ['discovered', 'tree-discovered']] as const) { + harness.raw.prepare(`INSERT INTO repos (owner, repo, last_tree_sha, pushed_at, stars) VALUES ('acme', ?, ?, 100, 1)`).run(repo, tree) + if (repo !== 'fresh') + harness.raw.prepare(`INSERT INTO skills (owner, repo, name, display_name, slug) VALUES ('acme', ?, 'one', 'One', 'one')`).run(repo) + } + harness.raw.prepare(`INSERT INTO repo_sync_progress (owner, repo, job_id, tree_sha, checked_at, next_offset, updated_at) VALUES ('acme', 'busy', 'job-1', 'tree-busy', ?, 250, ?)`).run(NOW - 60, NOW - 60) + }) + + afterEach(() => { + harness.close() + vi.unstubAllGlobals() + }) + + it('records an unchanged repository in place and queues only the ones that need a job', async () => { + const fetchMock = vi.fn(graphqlAnswer) + vi.stubGlobal('fetch', fetchMock) + + const result = await prefetchUnchangedRepos({ db: harness.db, bindings, now: NOW }, [ + candidate('unchanged'), + candidate('changed'), + candidate('fresh'), + candidate('busy'), + candidate('discovered', true), + candidate('missing', false, 'gone'), + ]) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(result.queue.map(item => `${item.owner}/${item.repo}`)).toEqual([ + 'acme/changed', + 'acme/fresh', + 'acme/busy', + 'acme/discovered', + 'gone/missing', + ]) + expect(result).toMatchObject({ _tag: 'prefetched', unchanged: 1 }) + expect(harness.raw.prepare(`SELECT stars, repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'unchanged'`).get()) + .toEqual({ stars: 77, repo_meta_synced_at: NOW }) + expect(harness.raw.prepare(`SELECT repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'changed'`).get()) + .toEqual({ repo_meta_synced_at: null }) + }) + + it('queues every candidate when GitHub does not answer the batch', async () => { + vi.stubGlobal('fetch', vi.fn(async () => new Response('bad gateway', { status: 502 }))) + + const result = await prefetchUnchangedRepos({ db: harness.db, bindings, now: NOW }, [ + candidate('unchanged'), + candidate('changed'), + ]) + + expect(result).toMatchObject({ _tag: 'unread', reason: 'graphql-502' }) + expect(result.queue).toHaveLength(2) + expect(harness.raw.prepare(`SELECT repo_meta_synced_at FROM repos WHERE owner = 'acme' AND repo = 'unchanged'`).get()) + .toEqual({ repo_meta_synced_at: null }) + }) +}) diff --git a/test/unit/sync-repo-correctness.test.ts b/test/unit/sync-repo-correctness.test.ts index f9c67dab..c0681727 100644 --- a/test/unit/sync-repo-correctness.test.ts +++ b/test/unit/sync-repo-correctness.test.ts @@ -136,7 +136,7 @@ describe('syncRepo content acknowledgement', () => { const result = await syncRepo('steipete', 'clawdis', {}, db, { ownerVerified: true }) expect(result).toMatchObject({ owner: 'steipete', repo: 'clawdis', status: 'indexed' }) - expect(github.getTree).toHaveBeenCalledWith('openclaw', 'openclaw', 'main', {}) + expect(github.getTree).toHaveBeenCalledWith('openclaw', 'openclaw', 'new-tree', {}) expect(github.getBlobsBatch).toHaveBeenCalledWith( 'openclaw', 'openclaw', diff --git a/test/unit/sync-repo-freshness.test.ts b/test/unit/sync-repo-freshness.test.ts index 966c06e2..8771d110 100644 --- a/test/unit/sync-repo-freshness.test.ts +++ b/test/unit/sync-repo-freshness.test.ts @@ -215,6 +215,7 @@ describe('syncRepo freshness cursor', () => { FROM repos WHERE owner = 'acme' AND repo = 'skills' `).get() + expect(github.getTree).toHaveBeenCalledWith('acme', 'skills', 'moved-tree', {}) expect(result.status).toBe('failed') expect(result.reason).toBe('tree_truncated') expect(row).toEqual({