From adb296c4628564e779a603f74d695f74262a2aa9 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 01:42:40 +1100 Subject: [PATCH 1/4] fix(artifact-delivery): keep healthy runs inside their limits A run polls its Resolution once a second, so one slow build spent the whole guest allowance of 60 requests a minute and failed with RATE_LIMITED. Polls now draw from their own allowance (ADR-0012). GitHub answers a secondary rate limit with 403 or 429 and Retry-After. A 403 failed the run as SOURCE_ACCESS_DENIED, and a 429 threw into the queue retry ladder. Both now fail as RATE_LIMITED with the named wait, and a rate-limited tarball no longer falls back to one read per blob. Every retryable failure now names retryAfterSeconds. The first queue retry waits 5 seconds instead of 60, inside the CLI's 60 second window. --- docs/adr/0012-resolution-poll-allowance.md | 32 ++++ docs/arch/README.md | 2 +- .../artifact-delivery/server/utils/build.ts | 15 +- .../server/utils/github-source.ts | 55 +++++- .../artifact-delivery/server/utils/queue.ts | 16 +- shared/server/api-rate-limit.ts | 22 ++- test/unit/api-rate-limit.test.ts | 36 +++- test/unit/artifact-rate-limit.test.ts | 175 +++++++++++++++++- worker-configuration.d.ts | 3 +- wrangler.jsonc | 4 + 10 files changed, 341 insertions(+), 19 deletions(-) create mode 100644 docs/adr/0012-resolution-poll-allowance.md diff --git a/docs/adr/0012-resolution-poll-allowance.md b/docs/adr/0012-resolution-poll-allowance.md new file mode 100644 index 00000000..6ef929e1 --- /dev/null +++ b/docs/adr/0012-resolution-poll-allowance.md @@ -0,0 +1,32 @@ +# ADR-0012: Resolution poll allowance + +Date: 2026-10-07 + +Amends [ADR-0008](0008-api-rate-limits.md). + +## Context + +ADR-0008 gives every v1 operation one shared allowance. +A guest gets 60 requests per 60 seconds, keyed by `CF-Connecting-IP`. + +`skilld run` polls `GET /api/v1/resolutions/{id}` at the rate the server names in `pollAfterMs`. +The server names 1000 ms. The CLI waits up to 60 seconds for a Resolution. +One build that takes a minute therefore costs about 60 polls. +That spends the whole guest allowance, so the run fails with `RATE_LIMITED`. +A second run from the same network fails the same way. + +## Decision + +A poll of one Resolution draws from a separate binding, `API_RESOLUTION_POLL_RATE_LIMIT`. +It allows 600 requests per 60 seconds. +It uses the ADR-0008 key: the verified account ID, or the guest IP. +Ten runs from one network can poll through a one-minute build. + +A poll is a `GET` of `/api/v1/resolutions/{id}`. +Every other v1 request keeps the ADR-0008 allowance, including the `POST` that starts a Resolution. +An exhausted poll allowance answers like an exhausted shared one: HTTP 429, `RATE_LIMITED`, and `Retry-After: 60`. + +## Consequences + +A poll is one indexed D1 read. 600 polls per minute is 10 reads per second for one key. +The starting `POST` stays in the shared allowance, so the number of builds one network can start does not change. diff --git a/docs/arch/README.md b/docs/arch/README.md index 8d5ce47d..96bc3b7d 100644 --- a/docs/arch/README.md +++ b/docs/arch/README.md @@ -45,7 +45,7 @@ descriptor (skilld-dev/skilld: packages/sdk/src/contract) └─ parity test test/unit/api-v1-parity.test.ts: one route file per operation ``` -- **Operation**: one `namespace.verb` entry in the contract, such as `skills.get`. Its ID never changes. Public handlers never receive caller identity. Rate-limit middleware verifies credentials only to select an allowance. Account operations reuse that identity. V1 responses bypass Workers Cache so every network request reaches the limiter. See [ADR-0008](../adr/0008-api-rate-limits.md). +- **Operation**: one `namespace.verb` entry in the contract, such as `skills.get`. Its ID never changes. Public handlers never receive caller identity. Rate-limit middleware verifies credentials only to select an allowance. Account operations reuse that identity. V1 responses bypass Workers Cache so every network request reaches the limiter. See [ADR-0008](../adr/0008-api-rate-limits.md). Resolution polls draw from their own allowance: [ADR-0012](../adr/0012-resolution-poll-allowance.md). - **defineApiOperation**: the shape of a `/api/v1` route: `{ operation, handler }`. The descriptor parses the input, checks the credential, and sets the status and cache headers. The handler only loads data. The answer passes the strict producer schema, so an unnamed field fails closed with a 500. - **Route file**: lives in the layer that owns the data, at the operation's own path: `GET /api/v1/skills/{owner}/{repository}/{name}` is `layers/registry/server/api/v1/skills/[owner]/[repository]/[name].get.ts`. It calls that layer's utilities, or reads the internal route in process. It never duplicates SQL. - **v1 presenter**: a pure function in `layers//server/presenters/-v1.ts`, typed `OperationResult`. diff --git a/layers/artifact-delivery/server/utils/build.ts b/layers/artifact-delivery/server/utils/build.ts index d07a4672..daea8253 100644 --- a/layers/artifact-delivery/server/utils/build.ts +++ b/layers/artifact-delivery/server/utils/build.ts @@ -662,6 +662,13 @@ async function failWithRejection( ) } +/** + * The wait a retryable failure names when its cause named none. GitHub asks + * for at least a minute after a secondary rate limit, and the build queue + * gives up only after several minutes of failed attempts. + */ +export const DEFAULT_RETRY_AFTER_SECONDS = 60 + export async function failResolution( dependencies: Pick, row: NonNullable>>, @@ -675,9 +682,13 @@ export async function failResolution( // The upstream reset header is an absolute epoch; error_retry_after means a // relative delay in seconds, like HTTP Retry-After. Store the delay a reader // can add to now, and never let a clock skew store zero or a negative one. - const errorRetryAfter = retryAtEpochSeconds === undefined + // A retryable failure whose cause named no time still names one, so the + // CLI never prints "may be retried" without saying when. + const errorRetryAfter = !retryable ? undefined - : Math.max(1, retryAtEpochSeconds - now) + : retryAtEpochSeconds === undefined + ? DEFAULT_RETRY_AFTER_SECONDS + : Math.max(1, retryAtEpochSeconds - now) const advanced = await transitionResolution(dependencies.db, row, 'failed', { errorCode: code, errorRetryable: retryable, diff --git a/layers/artifact-delivery/server/utils/github-source.ts b/layers/artifact-delivery/server/utils/github-source.ts index 6909dcad..2815cda9 100644 --- a/layers/artifact-delivery/server/utils/github-source.ts +++ b/layers/artifact-delivery/server/utils/github-source.ts @@ -156,6 +156,8 @@ interface GithubClientOptions { onReadFailure?: (failure: GithubReadFailure) => void sleep?: (milliseconds: number) => Promise random?: () => number + /** Unix seconds. A Retry-After delay becomes an absolute retry time against it. */ + now?: () => number } export function createPublicGithubSourceClient(options: GithubClientOptions): PublicGithubSourceClient { @@ -164,6 +166,7 @@ export function createPublicGithubSourceClient(options: GithubClientOptions): Pu export function createGithubSourceClient(options: GithubClientOptions): PublicGithubSourceClient { const expectedVisibility = options.visibility ?? 'public' + const now = options.now ?? (() => Math.floor(Date.now() / 1000)) const readRejection = (outcome: FailedReadOutcome) => outcome._tag === 'rate-limited' ? rateLimitRejection(outcome.resetAt) @@ -204,15 +207,15 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi const response = fetched.response if (response.status === 404) return { _tag: 'settled', value: { _tag: 'not-found' } } - if (response.status === 401 || response.status === 403) { - // A spent quota is a fact about this minute, not about the Repository. - // It used to throw, which wrote nothing: the Resolution sat in its - // current state through the 60, 120, 240, 480 second delivery ladder and - // then failed as SERVICE_UNAVAILABLE with no reason on it. - if (response.status === 403 && response.headers.get('x-ratelimit-remaining') === '0') - return { _tag: 'settled', value: { _tag: 'rate-limited', resetAt: epochHeader(response.headers, 'x-ratelimit-reset') } } + // A spent quota is a fact about this minute, not about the Repository. + // It used to throw, which wrote nothing: the Resolution sat in its + // current state through the delivery retry ladder and then failed as + // SERVICE_UNAVAILABLE with no reason on it. + const limited = githubRateLimit(response, now()) + if (limited) + return { _tag: 'settled', value: limited } + if (response.status === 401 || response.status === 403) return { _tag: 'settled', value: { _tag: 'access-denied' } } - } if (!response.ok) { await response.body?.cancel().catch(() => { // The status already decided the outcome. A body that will not close changes nothing. @@ -531,6 +534,10 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi const extracted = await loadFromTarball(source, selected.entries) if (extracted._tag === 'extracted') return { _tag: 'loaded', value: { source, files: extracted.files } } + // The blobs API reads one file per request against the same limit. + // Falling back would spend the most quota exactly when none is left. + if (extracted._tag === 'rate-limited') + return rateLimitRejection(extracted.resetAt) // The tarball is an optimisation, never an authority. Anything it got // wrong, including a file `.gitattributes export-ignore` removed from // the archive, falls through to the blobs API, which serves every blob @@ -551,7 +558,7 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi async function loadFromTarball( source: ResolvedSource, entries: Array, - ): Promise { + ): Promise { const headers = new Headers({ 'Accept': 'application/vnd.github+json', 'User-Agent': 'skilld.dev', @@ -572,6 +579,11 @@ export function createGithubSourceClient(options: GithubClientOptions): PublicGi }) if ('_tag' in response) return { _tag: 'unusable', reason: 'unavailable', findings: [response.message] } + const limited = githubRateLimit(response, now()) + if (limited) { + await response.body?.cancel() + return limited + } if (!response.ok || !response.body) { await response.body?.cancel() return { _tag: 'unusable', reason: 'unavailable', findings: [`GitHub returned ${response.status}`] } @@ -847,6 +859,31 @@ function epochHeader(headers: Headers, name: string): number | null { return Number.isSafeInteger(value) && value > 0 ? value : null } +interface GithubRateLimit { + _tag: 'rate-limited' + /** Unix seconds GitHub allows the next request, when it said so. */ + resetAt: number | null +} + +/** + * GitHub's rate limit answer, or null for any other response. + * + * GitHub answers a spent primary quota with 403 or 429 and no remaining + * requests, and a secondary limit with 403 or 429 and Retry-After. A 429 is + * always a limit. A 403 with neither header denies access. + * https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api + */ +export function githubRateLimit(response: Pick, now: number): GithubRateLimit | null { + if (response.status !== 403 && response.status !== 429) + return null + const retryAfter = epochHeader(response.headers, 'retry-after') + if (retryAfter !== null) + return { _tag: 'rate-limited', resetAt: now + retryAfter } + if (response.headers.get('x-ratelimit-remaining') === '0') + return { _tag: 'rate-limited', resetAt: epochHeader(response.headers, 'x-ratelimit-reset') } + return response.status === 429 ? { _tag: 'rate-limited', resetAt: null } : null +} + function rateLimitRejection(resetAt: number | null): SourceRejection { const rejection = reject('RATE_LIMITED', 'GitHub refused the read: its rate limit is spent.', []) return resetAt === null ? rejection : { ...rejection, retryAfterSeconds: resetAt } diff --git a/layers/artifact-delivery/server/utils/queue.ts b/layers/artifact-delivery/server/utils/queue.ts index 7c2466f0..185de39d 100644 --- a/layers/artifact-delivery/server/utils/queue.ts +++ b/layers/artifact-delivery/server/utils/queue.ts @@ -25,6 +25,17 @@ const artifactBuildMessageSchema = z.object({ resolutionId: z.string().uuid(), }).strict() +/** + * The wait before each retry of a build attempt that threw. A fifth failed + * attempt fails the Resolution as SERVICE_UNAVAILABLE. + * + * The CLI waits 60 seconds for a Resolution, so the first retry comes within + * that window: one transient GitHub or signer error no longer fails the run. + * It used to wait 60 seconds. The later waits still ride out a long GitHub + * network fault, such as the one on 2026-09-30. + */ +export const ARTIFACT_BUILD_RETRY_DELAYS_SECONDS: readonly number[] = [5, 30, 120, 480] + export async function enqueueArtifactBuild(env: Cloudflare.Env, resolutionId: string): Promise { await env.ARTIFACT_BUILD_QUEUE.send({ version: 1, resolutionId }) } @@ -59,8 +70,9 @@ export async function consumeArtifactBuildBatch( attempt: message.attempts, error: outcome.error instanceof Error ? outcome.error.message : String(outcome.error), })) - if (message.attempts < 5) { - message.retry({ delaySeconds: Math.min(3600, 60 * 2 ** Math.max(0, message.attempts - 1)) }) + const delaySeconds = ARTIFACT_BUILD_RETRY_DELAYS_SECONDS[message.attempts - 1] + if (delaySeconds !== undefined) { + message.retry({ delaySeconds }) continue } const row = await getResolution(dependencies.db, parsed.data.resolutionId) diff --git a/shared/server/api-rate-limit.ts b/shared/server/api-rate-limit.ts index 72051f19..ebc8b464 100644 --- a/shared/server/api-rate-limit.ts +++ b/shared/server/api-rate-limit.ts @@ -13,7 +13,10 @@ declare module 'h3' { } } -/** One allowance across v1 operations, keyed by verified account or Cloudflare's client IP. */ +/** + * One allowance across v1 operations, keyed by verified account or Cloudflare's client IP. + * Resolution polls draw from a separate allowance under the same key. + */ export function createApiRateLimitHandler(resolveUser: (event: H3Event) => Promise) { return defineEventHandler(async (event) => { const path = event.path.split('?')[0]! @@ -30,7 +33,9 @@ export function createApiRateLimitHandler(resolveUser: (event: H3Event) => Promi const { env, requestId } = event.context.platform // Do not trust X-Forwarded-For. Missing Cloudflare identity shares a conservative bucket. const key = user ? `account:${user.id}` : `guest:${getHeader(event, 'cf-connecting-ip') ?? 'unknown'}` - const limiter = user ? env.API_ACCOUNT_RATE_LIMIT : env.API_GUEST_RATE_LIMIT + const limiter = isResolutionPoll(event.method, path) + ? env.API_RESOLUTION_POLL_RATE_LIMIT + : user ? env.API_ACCOUNT_RATE_LIMIT : env.API_GUEST_RATE_LIMIT const { success } = await limiter.limit({ key }) if (success) return @@ -52,3 +57,16 @@ export function createApiRateLimitHandler(resolveUser: (event: H3Event) => Promi }) }) } + +const RESOLUTION_POLL_PATH = /^\/api\/v1\/resolutions\/[^/]+$/ + +/** + * A read of one Resolution while its build runs. + * + * The server sets the poll rate through `pollAfterMs`, so a poll is not a + * request the caller chose to make. A one-minute build costs sixty polls, + * which used to spend the whole guest allowance (ADR-0012). + */ +function isResolutionPoll(method: string, path: string): boolean { + return method === 'GET' && RESOLUTION_POLL_PATH.test(path) +} diff --git a/test/unit/api-rate-limit.test.ts b/test/unit/api-rate-limit.test.ts index 6741032b..7afa6f3c 100644 --- a/test/unit/api-rate-limit.test.ts +++ b/test/unit/api-rate-limit.test.ts @@ -5,12 +5,14 @@ import { createApiRateLimitHandler } from '../../shared/server/api-rate-limit' const guest = vi.fn() const account = vi.fn() +const poll = vi.fn() const resolveUser = vi.fn() +const RESOLUTION_ID = '0d6f6c2e-5b1a-4c3e-9f1d-2a7b8c9d0e1f' function serve() { const app = createApp() app.use(eventHandler((event) => { - event.context.platform = { requestId: 'request-test', env: { API_GUEST_RATE_LIMIT: { limit: guest }, API_ACCOUNT_RATE_LIMIT: { limit: account } } } as never + event.context.platform = { requestId: 'request-test', env: { API_GUEST_RATE_LIMIT: { limit: guest }, API_ACCOUNT_RATE_LIMIT: { limit: account }, API_RESOLUTION_POLL_RATE_LIMIT: { limit: poll } } } as never })) app.use(createApiRateLimitHandler(resolveUser)) app.use(eventHandler(() => ({ ok: true }))) @@ -22,6 +24,7 @@ describe('api rate limits', () => { beforeEach(() => { guest.mockReset().mockResolvedValue({ success: true }) account.mockReset().mockResolvedValue({ success: true }) + poll.mockReset().mockResolvedValue({ success: true }) resolveUser.mockReset().mockResolvedValue(null) }) @@ -93,4 +96,35 @@ describe('api rate limits', () => { expect(await response.json()).toMatchObject({ code: 'RATE_LIMITED' }) expect(guest).not.toHaveBeenCalled() }) + + // A run polls its Resolution once a second while it builds. Sixty polls used + // to spend the whole guest allowance, so one slow build failed the run with + // RATE_LIMITED, and so did a second run from the same network. + it('counts Resolution polls against their own allowance, not the shared one', async () => { + const fetch = serve() + for (let attempt = 0; attempt < 3; attempt++) + await fetch(`/api/v1/resolutions/${RESOLUTION_ID}`, { 'CF-Connecting-IP': '203.0.113.8' }) + expect(poll.mock.calls).toEqual(Array.from({ length: 3 }, () => [{ key: 'guest:203.0.113.8' }])) + expect(guest).not.toHaveBeenCalled() + }) + + it('keys an account poll by the account', async () => { + resolveUser.mockResolvedValue({ id: 7, login: 'octo' }) + await serve()(`/api/v1/resolutions/${RESOLUTION_ID}`, { Authorization: 'Bearer valid' }) + expect(poll).toHaveBeenCalledWith({ key: 'account:7' }) + expect(account).not.toHaveBeenCalled() + }) + + it('still counts the request that starts a Resolution against the shared allowance', async () => { + await serve()('/api/v1/resolutions', { 'CF-Connecting-IP': '203.0.113.8' }, 'POST') + expect(guest).toHaveBeenCalledWith({ key: 'guest:203.0.113.8' }) + expect(poll).not.toHaveBeenCalled() + }) + + it('limits polls once their own allowance is spent', async () => { + poll.mockResolvedValue({ success: false }) + const response = await serve()(`/api/v1/resolutions/${RESOLUTION_ID}`, { 'CF-Connecting-IP': '203.0.113.8' }) + expect(response.status).toBe(429) + expect(await response.json()).toMatchObject({ code: 'RATE_LIMITED' }) + }) }) diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 824295be..133cec2e 100644 --- a/test/unit/artifact-rate-limit.test.ts +++ b/test/unit/artifact-rate-limit.test.ts @@ -1,6 +1,7 @@ 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 } from '../../layers/artifact-delivery/server/utils/github-source' import type { TrustedRoot } from '../../layers/artifact-delivery/server/utils/trusted-root' import { describe, expect, it, vi } from 'vitest' import { processArtifactBuild } from '../../layers/artifact-delivery/server/utils/build' @@ -8,7 +9,8 @@ import { createPublicGithubSourceClient, isRetryableProblem, } from '../../layers/artifact-delivery/server/utils/github-source' -import { createResolution, resolutionRequestIdentity } from '../../layers/artifact-delivery/server/utils/state' +import { ARTIFACT_BUILD_QUEUE_NAME, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' +import { createResolution, getResolution, presentResolution, resolutionRequestIdentity } from '../../layers/artifact-delivery/server/utils/state' import { createSqliteD1 } from './helpers/d1-sqlite' const commitSha = '0123456789abcdef0123456789abcdef01234567' @@ -123,6 +125,97 @@ describe('gitHub rate limits as values', () => { }) }) +// GitHub answers a secondary rate limit with 403 or 429 and a Retry-After +// header, and a spent primary quota with 403 or 429 and no remaining requests. +// A 403 with Retry-After used to fail the run as SOURCE_ACCESS_DENIED, which +// nobody retries. A 429 used to throw, so the build sat through the queue +// retry ladder and spent quota on every attempt. +describe('gitHub secondary rate limits as values', () => { + const request: SourceRequest = { + provider: 'github', + owner: 'skilld-dev', + repository: 'skills', + selector: { type: 'path', path: 'skills/demo' }, + } + + it.each([403, 429])('rejects a %i with Retry-After as RATE_LIMITED after the named delay', async (status) => { + const client = createPublicGithubSourceClient({ + fetch: vi.fn(async () => new Response('{}', { + status, + headers: { 'retry-after': '30', 'x-ratelimit-remaining': '4321', 'content-type': 'application/json' }, + })) as unknown as typeof fetch, + now: () => NOW, + }) + + expect(await client.resolve(request)).toMatchObject({ + _tag: 'rejected', + code: 'RATE_LIMITED', + retryAfterSeconds: NOW + 30, + }) + }) + + it('rejects a 429 spent quota with its reset time', async () => { + const client = createPublicGithubSourceClient({ + fetch: vi.fn(async () => new Response('{}', { + status: 429, + headers: { 'x-ratelimit-remaining': '0', 'x-ratelimit-reset': String(RESET_AT), 'content-type': 'application/json' }, + })) as unknown as typeof fetch, + }) + + expect(await client.resolve(request)).toMatchObject({ + _tag: 'rejected', + code: 'RATE_LIMITED', + retryAfterSeconds: RESET_AT, + }) + }) + + it('stops at a rate-limited tarball instead of reading every blob', async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input) + if (url.includes('/tarball/')) { + return new Response('{}', { + status: 403, + headers: { 'retry-after': '60', 'content-type': 'application/json' }, + }) + } + if (url.includes('/git/blobs/')) + return json({ sha: 'd'.repeat(40), size: 12, encoding: 'base64', content: btoa('# Demo skill') }) + if (url.endsWith('/repos/skilld-dev/skills')) + return json({ id: 123, name: 'skills', owner: { login: 'skilld-dev' }, private: false, default_branch: 'main' }) + if (url.endsWith('/git/trees/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')) + return json({ sha: 'a'.repeat(40), tree: [{ path: 'skills', mode: '040000', type: 'tree', sha: 'b'.repeat(40) }] }) + if (url.endsWith('/git/trees/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb')) + return json({ sha: 'b'.repeat(40), tree: [{ path: 'demo', mode: '040000', type: 'tree', sha: 'c'.repeat(40) }] }) + if (url.endsWith('/git/trees/cccccccccccccccccccccccccccccccccccccccc?recursive=1')) { + return json({ + sha: 'c'.repeat(40), + truncated: false, + tree: [{ path: 'SKILL.md', mode: '100644', type: 'blob', sha: 'd'.repeat(40), size: 12 }], + }) + } + return json({}, 404) + }) + const client = createPublicGithubSourceClient({ + fetch: fetchMock as unknown as typeof fetch, + now: () => NOW, + }) + + const result = await client.load({ + provider: 'github', + repositoryId: 123, + owner: 'skilld-dev', + repository: 'skills', + visibility: 'public', + commitSha, + treeSha: 'a'.repeat(40), + skillPath: 'skills/demo', + }) + + expect(result).toMatchObject({ _tag: 'rejected', code: 'RATE_LIMITED', retryAfterSeconds: NOW + 60 }) + expect(fetchMock.mock.calls.some(([input]) => String(input).includes('/git/blobs/'))).toBe(false) + }) +}) + describe('which problems a caller may retry', () => { it.each([ ['RATE_LIMITED', true], @@ -170,6 +263,86 @@ describe('a spent quota persists a delay, not the reset epoch', () => { }) }) +describe('every retryable failure names its wait', () => { + it('names a wait when GitHub sent no reset', async () => { + const harness = await requestedResolution('rate-limit-no-reset-0001') + const dependencies = buildDependencies(harness.sqlite.db, createPublicGithubSourceClient({ + fetch: vi.fn(async () => new Response('{}', { status: 429 })) as unknown as typeof fetch, + })) + + await processArtifactBuild(dependencies, harness.id) + + const row = await getResolution(harness.sqlite.db, harness.id) + expect(presentResolution(row!)).toMatchObject({ state: 'failed', code: 'RATE_LIMITED', retryable: true, retryAfterSeconds: 60 }) + harness.sqlite.close() + }) + + it('names a wait when the build queue gives up', async () => { + const harness = await requestedResolution('queue-gives-up-0001') + const message = queueMessage(harness.id, 5) + + await consumeArtifactBuildBatch({} as Cloudflare.Env, queueBatch([message]), () => buildDependencies(harness.sqlite.db, githubDown())) + + const row = await getResolution(harness.sqlite.db, harness.id) + expect(presentResolution(row!)).toMatchObject({ state: 'failed', code: 'SERVICE_UNAVAILABLE', retryable: true, retryAfterSeconds: 60 }) + harness.sqlite.close() + }) +}) + +// The CLI waits 60 seconds for a Resolution. The first queue retry used to +// wait 60 seconds too, so one transient GitHub or signer error failed every +// run that met it, although the retry then built the Artifact. +describe('a failed build attempt retries inside the run deadline', () => { + it('retries the first failed attempt within ten seconds', async () => { + const harness = await requestedResolution('queue-first-retry-0001') + const message = queueMessage(harness.id, 1) + + await consumeArtifactBuildBatch({} as Cloudflare.Env, queueBatch([message]), () => buildDependencies(harness.sqlite.db, githubDown())) + + expect(message.retry).toHaveBeenCalledOnce() + expect(message.retry.mock.calls[0]![0].delaySeconds).toBeLessThanOrEqual(10) + harness.sqlite.close() + }) +}) + +async function requestedResolution(idempotencyKey: string) { + const sqlite = createSqliteD1(ARTIFACT_MIGRATIONS) + const identity = await resolutionRequestIdentity(resolutionRequest, idempotencyKey) + const created = await createResolution(sqlite.db, resolutionRequest, identity, NOW) + if (created._tag === 'idempotency-conflict') + throw new Error('Test Resolution conflicted') + return { sqlite, id: created.row.id } +} + +function buildDependencies(db: D1Database, github: PublicGithubSourceClient): ArtifactBuildDependencies { + return { + db, + github, + bucket: {} as R2Bucket, + signer: {} as ArtifactSigner, + trustedRoot: {} as TrustedRoot, + now: () => NOW, + } +} + +function githubDown(): PublicGithubSourceClient { + const fault = async () => Promise.reject(new Error('The operation was aborted due to timeout')) + return { resolve: vi.fn(fault), load: vi.fn(fault) } +} + +function queueMessage(resolutionId: string, attempts: number) { + return { + body: { version: 1, resolutionId }, + attempts, + ack: vi.fn(), + retry: vi.fn<(options: { delaySeconds: number }) => void>(), + } +} + +function queueBatch(messages: Array>) { + return { queue: ARTIFACT_BUILD_QUEUE_NAME, messages } as unknown as Parameters[1] +} + function rateLimited(): Response { return rateLimitedAt(RESET_AT) } diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 051b5bc3..3fb5ce73 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --config=wrangler.jsonc --env-file=.dev.vars.example` (hash: 50fee44c1595f78f46cc0523374267df) +// Generated by Wrangler by running `wrangler types --config=wrangler.jsonc --env-file=.dev.vars.example` (hash: a047a1748d71da3be1be11f140a75f7b) // Runtime types generated with workerd@1.20260828.1 2026-08-20 nodejs_compat interface __BaseEnv_Env { KV_CACHE: KVNamespace; @@ -17,6 +17,7 @@ interface __BaseEnv_Env { ARTIFACT_BUILD_QUEUE: Queue; API_GUEST_RATE_LIMIT: RateLimit; API_ACCOUNT_RATE_LIMIT: RateLimit; + API_RESOLUTION_POLL_RATE_LIMIT: RateLimit; TASK_SEARCH_RATE_LIMIT: RateLimit; AI: Ai; CF_VERSION_METADATA: WorkerVersionMetadata; diff --git a/wrangler.jsonc b/wrangler.jsonc index 521662a5..619fb32e 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -13,6 +13,10 @@ "ratelimits": [ { "name": "API_GUEST_RATE_LIMIT", "namespace_id": "2401001", "simple": { "limit": 60, "period": 60 } }, { "name": "API_ACCOUNT_RATE_LIMIT", "namespace_id": "2401002", "simple": { "limit": 600, "period": 60 } }, + // A run polls its Resolution once a second while it builds, at the rate the + // server names. Polls draw from this allowance, keyed like the two above, + // so ten runs from one network can wait out a one-minute build (ADR-0012). + { "name": "API_RESOLUTION_POLL_RATE_LIMIT", "namespace_id": "2401004", "simple": { "limit": 600, "period": 60 } }, // Task search runs a paid model. The longest period a rate limit binding // takes is 60 seconds; the daily ceiling is the spend budget in // runtimeConfig.taskSearch, across every visitor. From e2260b006c6d1945f384a7a415bae8f146d185ed Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 01:47:31 +1100 Subject: [PATCH 2/4] fix(artifact-delivery): read GitHub with a token kept for runs The hourly registry sync spends GITHUB_TOKEN to zero before each GitHub reset. On 2026-10-06, 13 runs failed RATE_LIMITED in those minutes. GitHub counts a personal token's quota per account, so builds now read with ARTIFACT_GITHUB_TOKEN when it is set, and with GITHUB_TOKEN until then. --- .dev.vars.example | 3 +++ layers/artifact-delivery/server/utils/queue.ts | 15 ++++++++++++++- test/unit/artifact-rate-limit.test.ts | 16 +++++++++++++++- worker-configuration.d.ts | 5 +++-- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/.dev.vars.example b/.dev.vars.example index 06e4d999..063a60b9 100644 --- a/.dev.vars.example +++ b/.dev.vars.example @@ -11,6 +11,9 @@ NUXT_OG_IMAGE_SECRET= NUXT_IMAGE_PROXY_KEY= NUXT_AI_READY_RUNTIME_SYNC_SECRET= 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= ARTIFACT_PRIVATE_ACCESS_ENABLED=false GITHUB_APP_ID= GITHUB_APP_CLIENT_ID= diff --git a/layers/artifact-delivery/server/utils/queue.ts b/layers/artifact-delivery/server/utils/queue.ts index 185de39d..2fe4417f 100644 --- a/layers/artifact-delivery/server/utils/queue.ts +++ b/layers/artifact-delivery/server/utils/queue.ts @@ -98,6 +98,19 @@ export function reportGithubReadFailure(failure: GithubReadFailure): void { })) } +/** + * The token public Artifact builds read GitHub with. + * + * GitHub counts a personal token's quota per account, and the registry sync + * spends `GITHUB_TOKEN` to zero before each hourly reset. Every run that needed + * GitHub in those minutes failed RATE_LIMITED. `ARTIFACT_GITHUB_TOKEN`, from + * another account, keeps a quota for runs. Until it is set, builds share + * `GITHUB_TOKEN`. + */ +export function artifactGithubToken(env: Partial>): string | undefined { + return env.ARTIFACT_GITHUB_TOKEN || env.GITHUB_TOKEN +} + export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBuildDependencies { const runtimeFetch = globalThis.fetch.bind(globalThis) const privateDependencies = privateArtifactAccessEnabled(env) @@ -105,7 +118,7 @@ export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBu : {} return { db: env.DB, - github: createPublicGithubSourceClient({ fetch: runtimeFetch, token: env.GITHUB_TOKEN, onReadFailure: reportGithubReadFailure }), + github: createPublicGithubSourceClient({ fetch: runtimeFetch, token: artifactGithubToken(env), onReadFailure: reportGithubReadFailure }), bucket: env.PUBLIC_ARTIFACTS, signer: createArtifactSigner(env.ARTIFACT_SIGNER), trustedRoot: parseTrustedRoot(env.ARTIFACT_TRUSTED_ROOT_JSON, Math.floor(Date.now() / 1000)), diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 133cec2e..4d7437ed 100644 --- a/test/unit/artifact-rate-limit.test.ts +++ b/test/unit/artifact-rate-limit.test.ts @@ -9,7 +9,7 @@ import { createPublicGithubSourceClient, isRetryableProblem, } from '../../layers/artifact-delivery/server/utils/github-source' -import { ARTIFACT_BUILD_QUEUE_NAME, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' +import { ARTIFACT_BUILD_QUEUE_NAME, artifactGithubToken, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' import { createResolution, getResolution, presentResolution, resolutionRequestIdentity } from '../../layers/artifact-delivery/server/utils/state' import { createSqliteD1 } from './helpers/d1-sqlite' @@ -216,6 +216,20 @@ describe('gitHub secondary rate limits as values', () => { }) }) +// The hourly registry sync spends the site token to zero before each GitHub +// reset. On 2026-10-06, 13 runs failed RATE_LIMITED in those minutes. A token +// from another GitHub account carries its own quota, kept for runs. +describe('the token Artifact builds read GitHub with', () => { + it('prefers the token kept for Artifact builds', () => { + expect(artifactGithubToken({ ARTIFACT_GITHUB_TOKEN: 'runs', GITHUB_TOKEN: 'site' })).toBe('runs') + }) + + it('uses the site token while no Artifact token is set', () => { + expect(artifactGithubToken({ ARTIFACT_GITHUB_TOKEN: '', GITHUB_TOKEN: 'site' })).toBe('site') + expect(artifactGithubToken({ GITHUB_TOKEN: 'site' })).toBe('site') + }) +}) + describe('which problems a caller may retry', () => { it.each([ ['RATE_LIMITED', true], diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 3fb5ce73..dd96a883 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --config=wrangler.jsonc --env-file=.dev.vars.example` (hash: a047a1748d71da3be1be11f140a75f7b) +// Generated by Wrangler by running `wrangler types --config=wrangler.jsonc --env-file=.dev.vars.example` (hash: 44fdc49aa25ec9befef6b0cb85d9da7e) // Runtime types generated with workerd@1.20260828.1 2026-08-20 nodejs_compat interface __BaseEnv_Env { KV_CACHE: KVNamespace; @@ -37,6 +37,7 @@ interface __BaseEnv_Env { NUXT_IMAGE_PROXY_KEY: string; NUXT_AI_READY_RUNTIME_SYNC_SECRET: string; GITHUB_TOKEN: string; + ARTIFACT_GITHUB_TOKEN: string; ARTIFACT_PRIVATE_ACCESS_ENABLED: string; GITHUB_APP_ID: string; GITHUB_APP_CLIENT_ID: string; @@ -64,7 +65,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } // Begin runtime types From efe99022f764807a9a1192259183c00de88b1820 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 02:04:28 +1100 Subject: [PATCH 3/4] feat(artifact-delivery): read GitHub as the read App for builds GitHub counts a personal token's quota per account. GITHUB_TOKEN is a personal token, so the registry sync, page views and the account owner's other tools spend the same bucket that builds need, and runs failed RATE_LIMITED in the minutes before each hourly reset. An App installation has its own bucket. Builds now mint an installation token for the read App and fall back to ARTIFACT_GITHUB_TOKEN, then GITHUB_TOKEN, with a reported reason when the App gives none. --- .dev.vars.example | 6 + docs/ops/artifact-delivery-rollout.md | 11 +- .../server/utils/github-app.ts | 90 ++++++-- .../server/utils/github-read-credential.ts | 214 ++++++++++++++++++ .../artifact-delivery/server/utils/queue.ts | 60 ++++- nuxt.config.ts | 3 + test/unit/artifact-github-read-app.test.ts | 204 +++++++++++++++++ test/unit/artifact-rate-limit.test.ts | 16 +- worker-configuration.d.ts | 7 +- 9 files changed, 566 insertions(+), 45 deletions(-) create mode 100644 layers/artifact-delivery/server/utils/github-read-credential.ts create mode 100644 test/unit/artifact-github-read-app.test.ts diff --git a/.dev.vars.example b/.dev.vars.example index 063a60b9..d5a4b14a 100644 --- a/.dev.vars.example +++ b/.dev.vars.example @@ -14,6 +14,12 @@ 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= ARTIFACT_PRIVATE_ACCESS_ENABLED=false GITHUB_APP_ID= GITHUB_APP_CLIENT_ID= diff --git a/docs/ops/artifact-delivery-rollout.md b/docs/ops/artifact-delivery-rollout.md index 3a1214bb..16e2ad09 100644 --- a/docs/ops/artifact-delivery-rollout.md +++ b/docs/ops/artifact-delivery-rollout.md @@ -113,7 +113,16 @@ Preserve these existing values: - `NUXT_TOKEN_KEY` - `GITHUB_TOKEN` -The existing `GITHUB_TOKEN` reads public [GitHub](https://github.com) sources. +The existing `GITHUB_TOKEN` reads public [GitHub](https://github.com) sources for the registry sync and `/gh` pages. + +Public Artifact builds read GitHub with a credential of their own, in this order: + +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`. + +If the App secrets are set but unusable, builds read with the next credential and emit an `artifact-github-credential` 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/utils/github-app.ts b/layers/artifact-delivery/server/utils/github-app.ts index 6fdfd17f..f6a7d541 100644 --- a/layers/artifact-delivery/server/utils/github-app.ts +++ b/layers/artifact-delivery/server/utils/github-app.ts @@ -491,22 +491,16 @@ export function createGithubAppClient(config: GithubAppConfig): GithubAppClient }, async createRepositoryToken(installationId, repositoryId) { - const jwt = await createGithubAppJwt(config) - const result = await request( - `/app/installations/${installationId}/access_tokens`, - jwt, - installationTokenSchema, - { - method: 'POST', - body: { - repository_ids: [repositoryId], - permissions: { contents: 'read', metadata: 'read' }, - }, + const result = await requestInstallationToken({ + fetch: config.fetch, + jwt: await createGithubAppJwt(config), + installationId, + body: { + repository_ids: [repositoryId], + permissions: { contents: 'read', metadata: 'read' }, }, - ) - return result._tag === 'ok' - ? { _tag: 'created', token: result.value.token, expiresAt: result.value.expires_at } - : result + }) + return result._tag === 'created' ? result : { _tag: 'not-found' } }, async userCanAccessRepository(userToken, installationId, repositoryId) { @@ -547,21 +541,81 @@ 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) throw new Error('GitHub App private key is malformed') - const issuedAt = config.now() - 60 + 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: config.clientId, + iss: input.issuer, }))) const unsigned = `${header}.${payload}` const imported = await crypto.subtle.importKey( 'pkcs8', - toArrayBuffer(privateKey), + toArrayBuffer(input.privateKeyPkcs8), { name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256' }, false, ['sign'], diff --git a/layers/artifact-delivery/server/utils/github-read-credential.ts b/layers/artifact-delivery/server/utils/github-read-credential.ts new file mode 100644 index 00000000..6398b471 --- /dev/null +++ b/layers/artifact-delivery/server/utils/github-read-credential.ts @@ -0,0 +1,214 @@ +import type { PublicGithubSourceClient } from './github-source' +import { requestInstallationToken, signGithubAppJwt } from './github-app' + +/** + * 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 + * 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 + } +} + +/** + * A GitHub source client whose every read asks for the current credential, so + * a long-lived client never reads with an expired installation token. + */ +export function withGithubCredential( + credential: ArtifactGithubCredential, + create: (token: string | undefined) => PublicGithubSourceClient, +): PublicGithubSourceClient { + return { + resolve: async request => await create(await credential()).resolve(request), + load: async source => await create(await credential()).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 2fe4417f..cfb0badd 100644 --- a/layers/artifact-delivery/server/utils/queue.ts +++ b/layers/artifact-delivery/server/utils/queue.ts @@ -1,6 +1,11 @@ import type { QueueBatch } from '#cf-jobs/server' import type { ArtifactBuildDependencies } from './build' -import type { GithubReadFailure } from './github-source' +import type { + ArtifactGithubCredentialEnv, + ArtifactGithubCredentialReport, + ArtifactGithubCredentialRuntime, +} from './github-read-credential' +import type { 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' @@ -11,6 +16,12 @@ import { githubAppUserTokenDependenciesFromEnv, loadAccountGithubAppUserToken, } from './github-app' +import { + createArtifactGithubCredential, + createInstallationTokenCache, + parseArtifactGithubCredentialConfig, + withGithubCredential, +} from './github-read-credential' import { createGithubSourceClient, createPublicGithubSourceClient } from './github-source' import { createD1PrivateArtifactKeyProvider, privateArtifactWrappingKeysFromEnv } from './private-crypto' import { privateArtifactAccessEnabled } from './private-feature' @@ -99,16 +110,47 @@ export function reportGithubReadFailure(failure: GithubReadFailure): void { } /** - * The token public Artifact builds read GitHub with. + * 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. * * GitHub counts a personal token's quota per account, and the registry sync - * spends `GITHUB_TOKEN` to zero before each hourly reset. Every run that needed - * GitHub in those minutes failed RATE_LIMITED. `ARTIFACT_GITHUB_TOKEN`, from - * another account, keeps a quota for runs. Until it is set, builds share - * `GITHUB_TOKEN`. + * and `/gh` page views spend `GITHUB_TOKEN` to zero before each hourly + * reset. Every run that needed GitHub in those minutes failed RATE_LIMITED. + * Builds read with the read App's installation token, whose bucket nothing + * else spends, then `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`. */ -export function artifactGithubToken(env: Partial>): string | undefined { - return env.ARTIFACT_GITHUB_TOKEN || env.GITHUB_TOKEN +export function createArtifactGithubSource( + env: ArtifactGithubCredentialEnv, + runtime: ArtifactGithubCredentialRuntime = defaultGithubSourceRuntime(), +): PublicGithubSourceClient { + const credential = createArtifactGithubCredential(parseArtifactGithubCredentialConfig(env), runtime) + return withGithubCredential( + credential, + token => createPublicGithubSourceClient({ fetch: runtime.fetch, token, onReadFailure: reportGithubReadFailure }), + ) +} + +function defaultGithubSourceRuntime(): ArtifactGithubCredentialRuntime { + return { + fetch: globalThis.fetch.bind(globalThis), + now: () => Math.floor(Date.now() / 1000), + tokenCache: installationTokens, + report: reportGithubCredential, + } +} + +function reportGithubCredential(event: ArtifactGithubCredentialReport): void { + emitOperationalEvent(createWideEvent({ + 'operation': 'artifact-github-credential', + 'outcome': event.outcome, + 'reason': event.reason, + 'github.credential': event.fallback, + })) } export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBuildDependencies { @@ -118,7 +160,7 @@ export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBu : {} return { db: env.DB, - github: createPublicGithubSourceClient({ fetch: runtimeFetch, token: artifactGithubToken(env), onReadFailure: reportGithubReadFailure }), + github: createArtifactGithubSource(env), bucket: env.PUBLIC_ARTIFACTS, signer: createArtifactSigner(env.ARTIFACT_SIGNER), trustedRoot: parseTrustedRoot(env.ARTIFACT_TRUSTED_ROOT_JSON, Math.floor(Date.now() / 1000)), diff --git a/nuxt.config.ts b/nuxt.config.ts index 2b45c2e2..78fa4bb3 100644 --- a/nuxt.config.ts +++ b/nuxt.config.ts @@ -142,6 +142,9 @@ export default defineNuxtConfig({ 'error.count', 'failed.count', 'failed.tasks', + // `artifact-github-credential`: which fallback token a build read GitHub + // with after the read App gave none. + 'github.credential', 'github.endpoint', 'github.step', 'item.count', diff --git a/test/unit/artifact-github-read-app.test.ts b/test/unit/artifact-github-read-app.test.ts new file mode 100644 index 00000000..95f3a5d4 --- /dev/null +++ b/test/unit/artifact-github-read-app.test.ts @@ -0,0 +1,204 @@ +import type { SourceRequest } from '../../layers/artifact-delivery/server/schemas/contracts' +import type { ArtifactGithubCredentialReport } from '../../layers/artifact-delivery/server/utils/github-read-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' + +// 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 +// App's installation has its own bucket of 5,000 requests an hour. +const APP_ID = '5212127' +const INSTALLATION_ID = '168540695' +const NOW = 1_791_300_000 +const request: SourceRequest = { + provider: 'github', + owner: 'skilld-dev', + repository: 'skills', + selector: { type: 'path', path: 'skills/demo' }, +} + +describe('the GitHub credential public Artifact builds read with', () => { + it('reads with the read App installation token when the App secrets are set', async () => { + const keys = await appKeys() + const github = fakeGithub({ expiresAt: NOW + 3600 }) + const source = createArtifactGithubSource(appEnv(keys.pem), runtime(github)) + + await source.resolve(request) + + expect(github.mints).toHaveLength(1) + expect(github.mints[0]!.url).toBe(`https://api.github.com/app/installations/${INSTALLATION_ID}/access_tokens`) + expect(github.mints[0]!.body).toEqual({ permissions: { metadata: 'read' } }) + const claims = await verifiedJwtClaims(github.mints[0]!.authorization, keys.publicKey) + expect(claims.iss).toBe(APP_ID) + expect(claims.exp - claims.iat).toBeLessThanOrEqual(600) + expect(github.readAuthorizations).toEqual(['Bearer ghs_installation_1']) + }) + + it('reuses one installation token until five minutes before it expires', async () => { + const keys = await appKeys() + const github = fakeGithub({ expiresAt: NOW + 3600 }) + const clock = { now: NOW } + const cache = createInstallationTokenCache() + const read = () => createArtifactGithubSource(appEnv(keys.pem), runtime(github, { cache, now: () => clock.now })).resolve(request) + + await read() + clock.now = NOW + 3600 - 301 + await read() + expect(github.mints).toHaveLength(1) + + clock.now = NOW + 3600 - 300 + await read() + expect(github.mints).toHaveLength(2) + expect(github.readAuthorizations).toEqual([ + 'Bearer ghs_installation_1', + 'Bearer ghs_installation_1', + 'Bearer ghs_installation_2', + ]) + }) + + it('mints once for reads that start together', async () => { + const keys = await appKeys() + const github = fakeGithub({ expiresAt: NOW + 3600 }) + const source = createArtifactGithubSource(appEnv(keys.pem), runtime(github)) + + await Promise.all([source.resolve(request), source.resolve(request), source.resolve(request)]) + + expect(github.mints).toHaveLength(1) + }) + + it('accepts a PEM whose line breaks were stored as \\n', async () => { + const keys = await appKeys() + const github = fakeGithub({ expiresAt: NOW + 3600 }) + const source = createArtifactGithubSource(appEnv(keys.pem.replaceAll('\n', '\\n')), runtime(github)) + + await source.resolve(request) + + expect(github.readAuthorizations).toEqual(['Bearer ghs_installation_1']) + }) + + it('falls back to ARTIFACT_GITHUB_TOKEN, then GITHUB_TOKEN, without the App secrets', async () => { + const both = fakeGithub({ expiresAt: NOW + 3600 }) + await createArtifactGithubSource({ ARTIFACT_GITHUB_TOKEN: 'runs', GITHUB_TOKEN: 'site' }, runtime(both)).resolve(request) + const siteOnly = fakeGithub({ expiresAt: NOW + 3600 }) + await createArtifactGithubSource({ ARTIFACT_GITHUB_TOKEN: '', GITHUB_TOKEN: 'site' }, runtime(siteOnly)).resolve(request) + + expect(both.mints).toHaveLength(0) + expect(both.readAuthorizations).toEqual(['Bearer runs']) + expect(siteOnly.readAuthorizations).toEqual(['Bearer site']) + }) + + 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 source = createArtifactGithubSource(appEnv(keys.pem), runtime(github, { report: event => reports.push(event) })) + + await source.resolve(request) + + expect(github.readAuthorizations).toEqual(['Bearer runs']) + expect(reports).toEqual([{ outcome: 'app-token-unavailable', reason: 'GitHub refused the installation token (401)', fallback: 'ARTIFACT_GITHUB_TOKEN' }]) + }) + + it('reports an unreadable App key and reads with the fallback token', async () => { + const github = fakeGithub({ expiresAt: NOW + 3600 }) + const reports: ArtifactGithubCredentialReport[] = [] + 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) })) + + await source.resolve(request) + + expect(github.mints).toHaveLength(0) + expect(github.readAuthorizations).toEqual(['Bearer site']) + expect(reports).toEqual([{ outcome: 'app-misconfigured', reason: 'SKILLD_READ_APP_PRIVATE_KEY_PKCS8 is not a PKCS #8 PEM private key', fallback: 'GITHUB_TOKEN' }]) + }) + + 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[] = [] + await createArtifactGithubSource(appEnv(keys.pem), runtime(github, { report: event => reports.push(event) })).resolve(request) + + const text = JSON.stringify(reports) + expect(text).not.toContain('runs') + expect(text).not.toContain('site') + expect(text).not.toContain('PRIVATE KEY') + expect(reports).toHaveLength(1) + }) +}) + +interface FakeGithub { + fetch: typeof fetch + mints: Array<{ url: string, authorization: string, body: unknown }> + readAuthorizations: Array +} + +function fakeGithub(options: { expiresAt: number, mintStatus?: number }): FakeGithub { + const mints: FakeGithub['mints'] = [] + const readAuthorizations: FakeGithub['readAuthorizations'] = [] + const fetcher = async (input: RequestInfo | URL, init?: RequestInit): Promise => { + const url = String(input) + const headers = new Headers(init?.headers) + if (url.includes('/access_tokens')) { + mints.push({ url, authorization: headers.get('authorization') ?? '', body: JSON.parse(String(init?.body)) }) + if (options.mintStatus) + 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')) + return Response.json({ message: 'Not Found' }, { status: 404 }) + } + return { fetch: fetcher as typeof fetch, mints, readAuthorizations } +} + +function runtime( + github: FakeGithub, + overrides: Partial<{ cache: ReturnType, now: () => number, report: (event: ArtifactGithubCredentialReport) => void }> = {}, +) { + return { + fetch: github.fetch, + now: overrides.now ?? (() => NOW), + tokenCache: overrides.cache ?? createInstallationTokenCache(), + report: overrides.report ?? (() => {}), + } +} + +function appEnv(pem: string) { + return { + SKILLD_READ_APP_ID: APP_ID, + SKILLD_READ_APP_INSTALLATION_ID: INSTALLATION_ID, + SKILLD_READ_APP_PRIVATE_KEY_PKCS8: pem, + ARTIFACT_GITHUB_TOKEN: 'runs', + GITHUB_TOKEN: 'site', + } +} + +async function appKeys(): Promise<{ pem: string, publicKey: CryptoKey }> { + 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 base64 = btoa(String.fromCharCode(...der)).match(/.{1,64}/g)!.join('\n') + return { pem: `-----BEGIN PRIVATE KEY-----\n${base64}\n-----END PRIVATE KEY-----\n`, publicKey: pair.publicKey } +} + +async function verifiedJwtClaims(authorization: string, publicKey: CryptoKey): Promise<{ iss: string, iat: number, exp: number }> { + const jwt = authorization.replace(/^Bearer /, '') + const [header, payload, signature] = jwt.split('.') as [string, string, string] + const valid = await crypto.subtle.verify( + 'RSASSA-PKCS1-v1_5', + publicKey, + base64UrlBytes(signature), + new TextEncoder().encode(`${header}.${payload}`), + ) + expect(valid).toBe(true) + expect(JSON.parse(new TextDecoder().decode(base64UrlBytes(header)))).toEqual({ alg: 'RS256', typ: 'JWT' }) + return JSON.parse(new TextDecoder().decode(base64UrlBytes(payload))) +} + +function base64UrlBytes(value: string): Uint8Array { + const normalized = value.replaceAll('-', '+').replaceAll('_', '/') + const binary = atob(normalized + '='.repeat((4 - normalized.length % 4) % 4)) + return Uint8Array.from(binary, character => character.charCodeAt(0)) +} diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 4d7437ed..133cec2e 100644 --- a/test/unit/artifact-rate-limit.test.ts +++ b/test/unit/artifact-rate-limit.test.ts @@ -9,7 +9,7 @@ import { createPublicGithubSourceClient, isRetryableProblem, } from '../../layers/artifact-delivery/server/utils/github-source' -import { ARTIFACT_BUILD_QUEUE_NAME, artifactGithubToken, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' +import { ARTIFACT_BUILD_QUEUE_NAME, consumeArtifactBuildBatch } from '../../layers/artifact-delivery/server/utils/queue' import { createResolution, getResolution, presentResolution, resolutionRequestIdentity } from '../../layers/artifact-delivery/server/utils/state' import { createSqliteD1 } from './helpers/d1-sqlite' @@ -216,20 +216,6 @@ describe('gitHub secondary rate limits as values', () => { }) }) -// The hourly registry sync spends the site token to zero before each GitHub -// reset. On 2026-10-06, 13 runs failed RATE_LIMITED in those minutes. A token -// from another GitHub account carries its own quota, kept for runs. -describe('the token Artifact builds read GitHub with', () => { - it('prefers the token kept for Artifact builds', () => { - expect(artifactGithubToken({ ARTIFACT_GITHUB_TOKEN: 'runs', GITHUB_TOKEN: 'site' })).toBe('runs') - }) - - it('uses the site token while no Artifact token is set', () => { - expect(artifactGithubToken({ ARTIFACT_GITHUB_TOKEN: '', GITHUB_TOKEN: 'site' })).toBe('site') - expect(artifactGithubToken({ GITHUB_TOKEN: 'site' })).toBe('site') - }) -}) - describe('which problems a caller may retry', () => { it.each([ ['RATE_LIMITED', true], diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index dd96a883..cafb6af1 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --config=wrangler.jsonc --env-file=.dev.vars.example` (hash: 44fdc49aa25ec9befef6b0cb85d9da7e) +// Generated by Wrangler by running `wrangler types --config=wrangler.jsonc --env-file=.dev.vars.example` (hash: 91ae1b6a0c6d3e279dd2c18ecea4e0c4) // Runtime types generated with workerd@1.20260828.1 2026-08-20 nodejs_compat interface __BaseEnv_Env { KV_CACHE: KVNamespace; @@ -38,6 +38,9 @@ interface __BaseEnv_Env { NUXT_AI_READY_RUNTIME_SYNC_SECRET: string; GITHUB_TOKEN: string; ARTIFACT_GITHUB_TOKEN: string; + SKILLD_READ_APP_ID: string; + SKILLD_READ_APP_INSTALLATION_ID: string; + SKILLD_READ_APP_PRIVATE_KEY_PKCS8: string; ARTIFACT_PRIVATE_ACCESS_ENABLED: string; GITHUB_APP_ID: string; GITHUB_APP_CLIENT_ID: string; @@ -65,7 +68,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } // Begin runtime types From bbcc3a5b41b24685eff80c23bca2a257779df8d4 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 7 Oct 2026 02:19:41 +1100 Subject: [PATCH 4/4] feat(artifact-delivery): read with the requester's token at a spent quota When every shared GitHub bucket is spent, a signed-in run still has its own: the account's GitHub sign-in token reads public Repositories at that account's quota. The build retries only the refused read with it, and only for the account that asked. The requester row lives while the build runs. The build deletes it when it settles, account deletion removes it, and the daily purge removes one a dead build left behind. --- .../server/api/v1/resolutions/index.post.ts | 1 + .../artifact-delivery/server/utils/build.ts | 15 +- .../artifact-delivery/server/utils/queue.ts | 18 ++ .../server/utils/request-resolution.ts | 19 +- .../server/utils/requester-github.ts | 117 ++++++++++++ .../identity/server/utils/account-deletion.ts | 1 + .../identity/server/utils/data-retention.ts | 9 + .../0143_artifact_resolution_requesters.sql | 13 ++ test/unit/account-deletion.test.ts | 8 + test/unit/artifact-build-reuse.test.ts | 1 + test/unit/artifact-delivery.test.ts | 1 + test/unit/artifact-rate-limit.test.ts | 1 + test/unit/artifact-requester-github.test.ts | 177 ++++++++++++++++++ test/unit/skill-run-sweep.test.ts | 1 + 14 files changed, 379 insertions(+), 3 deletions(-) create mode 100644 layers/artifact-delivery/server/utils/requester-github.ts create mode 100644 migrations/0143_artifact_resolution_requesters.sql create mode 100644 test/unit/artifact-requester-github.test.ts 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 daea8253..79ca3b75 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 } from './github-source' +import { withRequesterFallback } from './requester-github' import { ARTIFACT_POLICY_VERSION, findReadyPublicBuild, @@ -43,6 +44,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: { @@ -703,8 +710,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/queue.ts b/layers/artifact-delivery/server/utils/queue.ts index cfb0badd..5540b406 100644 --- a/layers/artifact-delivery/server/utils/queue.ts +++ b/layers/artifact-delivery/server/utils/queue.ts @@ -26,6 +26,7 @@ import { createGithubSourceClient, createPublicGithubSourceClient } from './gith 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' @@ -70,6 +71,10 @@ export async function consumeArtifactBuildBatch( .then(value => ({ _tag: 'ok' as const, value })) .catch(error => ({ _tag: 'error' as const, error })) 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 } @@ -89,6 +94,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() } } @@ -161,6 +167,18 @@ export function createArtifactBuildDependencies(env: Cloudflare.Env): ArtifactBu return { db: env.DB, github: createArtifactGithubSource(env), + requesterGithub: createRequesterGithubSource({ + db: env.DB, + tokenKey: () => env.NUXT_TOKEN_KEY, + fetch: runtimeFetch, + now: () => Math.floor(Date.now() / 1000), + 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..e82c2319 --- /dev/null +++ b/layers/artifact-delivery/server/utils/requester-github.ts @@ -0,0 +1,117 @@ +import type { GithubReadFailure, PublicGithubSourceClient } 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 + 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, + onReadFailure: dependencies.onReadFailure, + }) + } +} + +/** + * A client that reads with the shared credential, and repeats a read the + * shared quota refused with the requester's own client. + * + * 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 (first._tag !== 'rejected' || first.code !== 'RATE_LIMITED') + return first + const client = await ownClient() + return client ? await client.resolve(request) : first + }, + async load(source) { + const first = await shared.load(source) + if (first._tag !== 'rejected' || first.code !== 'RATE_LIMITED') + return first + const client = await ownClient() + return client ? await client.load(source) : first + }, + } +} 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/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/migrations/0143_artifact_resolution_requesters.sql b/migrations/0143_artifact_resolution_requesters.sql new file mode 100644 index 00000000..246075cd --- /dev/null +++ b/migrations/0143_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/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 5dc57596..2f711843 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/0143_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 03069179..379a51c5 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/0143_artifact_resolution_requesters.sql', ] const sourceRequest: SourceRequest = { provider: 'github', diff --git a/test/unit/artifact-rate-limit.test.ts b/test/unit/artifact-rate-limit.test.ts index 133cec2e..5d1ffe98 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/0143_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..3ef2c0a6 --- /dev/null +++ b/test/unit/artifact-requester-github.test.ts @@ -0,0 +1,177 @@ +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('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/skill-run-sweep.test.ts b/test/unit/skill-run-sweep.test.ts index bd99c8a8..3d5d8874 100644 --- a/test/unit/skill-run-sweep.test.ts +++ b/test/unit/skill-run-sweep.test.ts @@ -15,6 +15,7 @@ const MIGRATIONS = [ 'migrations/0112_private_artifact_keys.sql', 'migrations/0122_artifact_resolution_retry_after.sql', 'migrations/0142_artifact_run_checks.sql', + 'migrations/0143_artifact_resolution_requesters.sql', ] const NOW = 1_791_000_000 const archify: RunnableSkill = { owner: 'tt-a1i', repository: 'archify', name: 'archify' }