diff --git a/src/commands/scan/create-scan-from-github.mts b/src/commands/scan/create-scan-from-github.mts index 83bb3ec428..9007e60723 100644 --- a/src/commands/scan/create-scan-from-github.mts +++ b/src/commands/scan/create-scan-from-github.mts @@ -19,6 +19,11 @@ import constants from '../../constants.mts' import { apiFetch } from '../../utils/api.mts' import { debugApiRequest, debugApiResponse } from '../../utils/debug.mts' import { formatErrorWithDetail } from '../../utils/errors.mts' +import { + classifyGitHubResponse, + githubApiRequest, + isGitHubBlockingError, +} from '../../utils/github-errors.mts' import { isReportSupportedFile } from '../../utils/glob.mts' import { fetchListAllRepos } from '../repository/fetch-list-all-repos.mts' @@ -92,26 +97,91 @@ export async function createScanFromGithub({ } } - let scansCreated = 0 - for (const repoSlug of targetRepos) { - // eslint-disable-next-line no-await-in-loop - const scanCResult = await scanRepo(repoSlug, { + return await runGithubScanLoop(targetRepos, repoSlug => + scanRepo(repoSlug, { githubApiUrl, githubToken, orgSlug, orgGithub, outputKind, repos, - }) + }), + ) +} + +/** + * Drive the per-repo scan loop and decide the overall run result. + * + * The loop stops early on a blocking GitHub error (rate limit / auth / abuse + * detection) because every remaining repo would fail the same way. Previously + * a rate-limited token made every repo fail its API calls, the loop swallowed + * each failure, and the final "N repos / 0 manifests" summary misled users and + * CI into thinking the scan succeeded when nothing was uploaded. A run where + * every attempted repo failed for a non-blocking reason is also surfaced as an + * error rather than a silent ok:true. + * + * `scanRepoFn` is injected so this decision logic can be tested without the + * GitHub network path. + */ +export async function runGithubScanLoop( + targetRepos: string[], + scanRepoFn: (repoSlug: string) => Promise>, +): Promise> { + let scansCreated = 0 + let reposScanned = 0 + let blockingError: CResult | undefined + const perRepoFailures: Array<{ repo: string; message: string }> = [] + for (const repoSlug of targetRepos) { + reposScanned += 1 + // eslint-disable-next-line no-await-in-loop + const scanCResult = await scanRepoFn(repoSlug) if (scanCResult.ok) { const { scanCreated } = scanCResult.data if (scanCreated) { scansCreated += 1 } + continue + } + perRepoFailures.push({ repo: repoSlug, message: scanCResult.message }) + // Stop on rate-limit / auth / abuse failures: every remaining repo will + // fail for the same reason, and continuing only burns more quota while + // delaying the real error. + if (isGitHubBlockingError(scanCResult.message)) { + blockingError = { + ok: false, + message: scanCResult.message, + cause: scanCResult.cause, + } + break + } + } + + if (blockingError) { + logger.fail(blockingError.message) + return blockingError + } + + // If every attempted repo failed (but not for a known blocking reason), + // treat the run as an error so scripts do not infer success from an + // ok:true with zero scans created. Checked before the success lines below + // so an all-failed run never prints green "processed" output that a script + // reading log lines could mistake for success. + if ( + reposScanned > 0 && + scansCreated === 0 && + perRepoFailures.length === reposScanned + ) { + const firstFailure = perRepoFailures[0]! + return { + ok: false, + message: 'All repos failed to scan', + cause: + `All ${reposScanned} repos failed to scan. First failure for ${firstFailure.repo}: ${firstFailure.message}. ` + + 'See the log above for per-repo details.', } } - logger.success(targetRepos.length, 'GitHub repos detected') + logger.success(reposScanned, 'GitHub repos processed') logger.success(scansCreated, 'with supported Manifest files') return { @@ -321,8 +391,20 @@ async function testAndDownloadManifestFiles({ if (result.data.isManifest) { fileCount += 1 } - } else if (!firstFailureResult) { - firstFailureResult = result + } else { + // A blocking error (rate limit / auth / abuse detection) is token-wide: + // every remaining download would fail the same way, and an earlier + // successful download must not mask it into an ok:true scan of a + // partial manifest set. Surface it immediately, the same way the repo + // loop short-circuits on blocking errors. + if (isGitHubBlockingError(result.message)) { + logger.groupEnd() + logger.fail(result.message) + return result + } + if (!firstFailureResult) { + firstFailureResult = result + } } } logger.groupEnd() @@ -406,23 +488,22 @@ async function downloadManifestFile({ const fileUrl = `${repoApiUrl}/contents/${file}?ref=${defaultBranch}` debugDir('inspect', { fileUrl }) - debugApiRequest('GET', fileUrl) - let downloadUrlResponse: Response - try { - downloadUrlResponse = await apiFetch(fileUrl, { + const reqResult = await githubApiRequest( + fileUrl, + { method: 'GET', headers: { Authorization: `Bearer ${githubToken}`, }, - }) - debugApiResponse('GET', fileUrl, downloadUrlResponse.status) - } catch (e) { - debugApiResponse('GET', fileUrl, undefined, e) - throw e + }, + `fetching the download URL for ${file}`, + ) + if (!reqResult.ok) { + return reqResult } debugFn('notice', 'complete: request') - const downloadUrlText = await downloadUrlResponse.text() + const downloadUrlText = reqResult.data.bodyText debugFn('inspect', 'response: raw download url', downloadUrlText) let downloadUrl @@ -477,6 +558,20 @@ async function streamDownloadWithFetch( debugApiResponse('GET', downloadUrl, response.status) if (!response.ok) { + // Surface rate-limit / auth failures on the raw download host too, so a + // bulk run that trips the limit mid-download fails loudly instead of + // being counted as just another skipped file. Header/status-only check; + // the stream body is left unconsumed. + const blocking = classifyGitHubResponse( + response.status, + response.headers, + '', + 'downloading a manifest file', + ) + if (blocking) { + logger.fail(blocking.message) + return blocking + } const errorMsg = `Download failed due to bad server response: ${response.status} ${response.statusText} for ${downloadUrl}` logger.fail(errorMsg) return { ok: false, message: 'Download Failed', cause: errorMsg } @@ -571,21 +666,20 @@ async function getLastCommitDetails({ const commitApiUrl = `${repoApiUrl}/commits?sha=${defaultBranch}&per_page=1` debugFn('inspect', 'url: commit', commitApiUrl) - debugApiRequest('GET', commitApiUrl) - let commitResponse: Response - try { - commitResponse = await apiFetch(commitApiUrl, { + const reqResult = await githubApiRequest( + commitApiUrl, + { headers: { Authorization: `Bearer ${githubToken}`, }, - }) - debugApiResponse('GET', commitApiUrl, commitResponse.status) - } catch (e) { - debugApiResponse('GET', commitApiUrl, undefined, e) - throw e + }, + `fetching the latest commit for ${orgGithub}/${repoSlug}`, + ) + if (!reqResult.ok) { + return reqResult } - const commitText = await commitResponse.text() + const commitText = reqResult.data.bodyText debugFn('inspect', 'response: commit', commitText) let lastCommit @@ -683,23 +777,22 @@ async function getRepoDetails({ const repoApiUrl = `${githubApiUrl}/repos/${orgGithub}/${repoSlug}` debugDir('inspect', { repoApiUrl }) - let repoDetailsResponse: Response - try { - debugApiRequest('GET', repoApiUrl) - repoDetailsResponse = await apiFetch(repoApiUrl, { + const reqResult = await githubApiRequest( + repoApiUrl, + { method: 'GET', headers: { Authorization: `Bearer ${githubToken}`, }, - }) - debugApiResponse('GET', repoApiUrl, repoDetailsResponse.status) - } catch (e) { - debugApiResponse('GET', repoApiUrl, undefined, e) - throw e + }, + `fetching repo details for ${orgGithub}/${repoSlug}`, + ) + if (!reqResult.ok) { + return reqResult } logger.success(`Request completed.`) - const repoDetailsText = await repoDetailsResponse.text() + const repoDetailsText = reqResult.data.bodyText debugFn('inspect', 'response: repo', repoDetailsText) let repoDetails @@ -747,22 +840,21 @@ async function getRepoBranchTree({ const treeApiUrl = `${repoApiUrl}/git/trees/${defaultBranch}?recursive=1` debugFn('inspect', 'url: tree', treeApiUrl) - let treeResponse: Response - try { - debugApiRequest('GET', treeApiUrl) - treeResponse = await apiFetch(treeApiUrl, { + const reqResult = await githubApiRequest( + treeApiUrl, + { method: 'GET', headers: { Authorization: `Bearer ${githubToken}`, }, - }) - debugApiResponse('GET', treeApiUrl, treeResponse.status) - } catch (e) { - debugApiResponse('GET', treeApiUrl, undefined, e) - throw e + }, + `fetching the file tree for ${orgGithub}/${repoSlug}`, + ) + if (!reqResult.ok) { + return reqResult } - const treeText = await treeResponse.text() + const treeText = reqResult.data.bodyText debugFn('inspect', 'response: tree', treeText) let treeDetails diff --git a/src/commands/scan/create-scan-from-github.test.mts b/src/commands/scan/create-scan-from-github.test.mts new file mode 100644 index 0000000000..817c7468e2 --- /dev/null +++ b/src/commands/scan/create-scan-from-github.test.mts @@ -0,0 +1,103 @@ +import { describe, expect, it } from 'vitest' + +import { runGithubScanLoop } from './create-scan-from-github.mts' +import { + GITHUB_ERR_AUTH_FAILED, + GITHUB_ERR_RATE_LIMIT, +} from '../../utils/github-errors.mts' + +import type { CResult } from '../../types.mts' + +type ScanResult = CResult<{ scanCreated: boolean }> + +// Build a scanRepoFn from a map of repo -> canned result, recording the +// order of calls so we can assert the loop stops early on blocking errors. +// No module mocking — this is a plain function injected into the loop. +function fakeScanner(results: Record): { + fn: (repoSlug: string) => Promise + calls: string[] +} { + const calls: string[] = [] + return { + calls, + fn: (repoSlug: string) => { + calls.push(repoSlug) + return Promise.resolve(results[repoSlug]!) + }, + } +} + +const scanned: ScanResult = { ok: true, data: { scanCreated: true } } +const emptyRepo: ScanResult = { ok: true, data: { scanCreated: false } } +const noManifests: ScanResult = { + ok: false, + message: 'No manifest files found', + cause: 'No supported manifest files were found.', +} +const rateLimited: ScanResult = { + ok: false, + message: GITHUB_ERR_RATE_LIMIT, + cause: 'GitHub API rate limit exceeded on the supplied token.', +} +const authFailed: ScanResult = { + ok: false, + message: GITHUB_ERR_AUTH_FAILED, + cause: 'GitHub authentication failed.', +} + +describe('runGithubScanLoop GitHub blocking-error handling', () => { + it('stops and returns ok:false on a GitHub rate limit', async () => { + const scanner = fakeScanner({ + 'repo-a': rateLimited, + 'repo-b': scanned, + 'repo-c': scanned, + }) + const result = await runGithubScanLoop( + ['repo-a', 'repo-b', 'repo-c'], + scanner.fn, + ) + expect(result.ok).toBe(false) + expect(result.ok ? '' : result.message).toBe(GITHUB_ERR_RATE_LIMIT) + // Loop stopped after the first repo — no quota burned on the rest. + expect(scanner.calls).toEqual(['repo-a']) + }) + + it('stops and returns ok:false on a GitHub auth failure', async () => { + const scanner = fakeScanner({ 'repo-a': authFailed, 'repo-b': scanned }) + const result = await runGithubScanLoop(['repo-a', 'repo-b'], scanner.fn) + expect(result.ok).toBe(false) + expect(result.ok ? '' : result.message).toBe(GITHUB_ERR_AUTH_FAILED) + expect(scanner.calls).toEqual(['repo-a']) + }) + + it('carries the rate-limit cause through so the CLI can print it', async () => { + const scanner = fakeScanner({ 'repo-a': rateLimited }) + const result = await runGithubScanLoop(['repo-a'], scanner.fn) + expect(result.ok ? undefined : result.cause).toContain('rate limit') + }) + + it('succeeds when a repo has no manifests but the tree was empty', async () => { + const scanner = fakeScanner({ 'repo-a': emptyRepo, 'repo-b': emptyRepo }) + const result = await runGithubScanLoop(['repo-a', 'repo-b'], scanner.fn) + expect(result.ok).toBe(true) + expect(scanner.calls).toEqual(['repo-a', 'repo-b']) + }) + + it('succeeds when at least one repo produced a scan', async () => { + const scanner = fakeScanner({ 'repo-a': noManifests, 'repo-b': scanned }) + const result = await runGithubScanLoop(['repo-a', 'repo-b'], scanner.fn) + expect(result.ok).toBe(true) + // Both repos are attempted; a non-blocking failure does not stop the loop. + expect(scanner.calls).toEqual(['repo-a', 'repo-b']) + }) + + it('fails when every repo errored for a non-blocking reason', async () => { + const scanner = fakeScanner({ + 'repo-a': noManifests, + 'repo-b': noManifests, + }) + const result = await runGithubScanLoop(['repo-a', 'repo-b'], scanner.fn) + expect(result.ok).toBe(false) + expect(result.ok ? '' : result.message).toBe('All repos failed to scan') + }) +}) diff --git a/src/utils/github-errors.mts b/src/utils/github-errors.mts new file mode 100644 index 0000000000..ce4217c59a --- /dev/null +++ b/src/utils/github-errors.mts @@ -0,0 +1,289 @@ +/** + * GitHub API error detection for the raw-fetch `socket scan github` flow. + * + * The `socket scan github` command talks to the GitHub REST API directly + * through `apiFetch` (see utils/api.mts) rather than Octokit. That path used + * to read every response body and JSON-parse it without ever inspecting the + * HTTP status, so a rate-limit response (`403` with `x-ratelimit-remaining: 0`, + * `429`, or a secondary-limit body) was misread as "repo has no default + * branch / no manifests" and the run reported a silent success. + * + * This module centralizes: + * - Classifying a GitHub response as a blocking error (rate limit / abuse + * detection / auth) with a clear, actionable message. + * - A bounded-retry request wrapper that respects `Retry-After` / + * `x-ratelimit-reset` for short reset windows and retries transient 5xx / + * network failures with capped exponential backoff. + */ + +import { debugFn } from '@socketsecurity/registry/lib/debug' +import { logger } from '@socketsecurity/registry/lib/logger' + +import { apiFetch } from './api.mts' +import { debugApiRequest, debugApiResponse } from './debug.mts' +import { formatErrorWithDetail } from './errors.mts' +import { + HTTP_STATUS_FORBIDDEN, + HTTP_STATUS_INTERNAL_SERVER_ERROR, + HTTP_STATUS_UNAUTHORIZED, +} from '../constants.mts' + +import type { ApiFetchInit } from './api.mts' +import type { CResult } from '../types.mts' + +// GitHub returns 429 for some secondary rate limits; there is no shared +// constant for it in constants.mts, so define it locally. +const HTTP_STATUS_TOO_MANY_REQUESTS = 429 + +// Retry at most this many times for transient (5xx / network) failures, +// counting the initial attempt. +const MAX_TRANSIENT_ATTEMPTS = 3 + +// Cap for exponential backoff between transient retries. +const MAX_BACKOFF_MS = 10_000 + +// Only wait-and-retry a rate-limited response when the reset window is at +// most this many seconds. The usual primary-limit reset is up to an hour +// away, which is not worth blocking a CLI run on — those surface immediately. +const CHEAP_RATE_LIMIT_WAIT_MAX_SECONDS = 30 + +// Canonical `message` values returned for blocking conditions. Exported so +// the scan loop can short-circuit on them without matching free-form strings. +export const GITHUB_ERR_ABUSE_DETECTION = 'GitHub abuse detection triggered' +export const GITHUB_ERR_AUTH_FAILED = 'GitHub authentication failed' +export const GITHUB_ERR_RATE_LIMIT = 'GitHub rate limit exceeded' + +// A blocking error means every subsequent repo will fail for the same +// reason, so the scan loop should stop and surface it rather than silently +// reporting "0 manifests". +const BLOCKING_ERROR_MESSAGES = new Set([ + GITHUB_ERR_ABUSE_DETECTION, + GITHUB_ERR_AUTH_FAILED, + GITHUB_ERR_RATE_LIMIT, +]) + +/** + * Whether a CResult `message` is one of the blocking GitHub conditions + * (rate limit / abuse detection / auth) that should stop a multi-repo scan. + */ +export function isGitHubBlockingError(message: string): boolean { + return BLOCKING_ERROR_MESSAGES.has(message) +} + +/** + * Seconds to wait before a rate-limited request could succeed, derived from + * the `Retry-After` header (seconds) or the `x-ratelimit-reset` header (an + * epoch-seconds timestamp). Returns undefined when neither is usable. + */ +export function getRateLimitWaitSeconds(headers: Headers): number | undefined { + const retryAfter = headers.get('retry-after') + if (retryAfter) { + const seconds = Number.parseInt(retryAfter, 10) + if (Number.isFinite(seconds) && seconds >= 0) { + return seconds + } + } + const reset = headers.get('x-ratelimit-reset') + if (reset) { + const resetEpochSeconds = Number.parseInt(reset, 10) + if (Number.isFinite(resetEpochSeconds)) { + return Math.max(0, resetEpochSeconds - Math.floor(Date.now() / 1000)) + } + } + return undefined +} + +/** + * Classify a GitHub API response as a blocking error (rate limit / abuse + * detection / auth). Returns undefined when the response is not one of those + * conditions, so the caller can continue its normal parsing (including its + * own handling of 404s, empty repos, etc.). + */ +export function classifyGitHubResponse( + status: number, + headers: Headers, + bodyText: string, + context: string, +): CResult | undefined { + const lowerBody = bodyText.toLowerCase() + + // Secondary / abuse rate limit. Check first since it is more specific than + // the standard rate limit and shares the 403 status. + if ( + status === HTTP_STATUS_FORBIDDEN && + (lowerBody.includes('secondary rate limit') || + lowerBody.includes('abuse detection')) + ) { + return { + ok: false, + message: GITHUB_ERR_ABUSE_DETECTION, + cause: + `GitHub abuse detection triggered while ${context}. ` + + 'This happens when too many requests are made in a short period. ' + + 'Wait a minute before retrying, and reduce the number of repos ' + + 'scanned at once.', + } + } + + // Standard rate limit: 429, or 403 with the quota exhausted + // (x-ratelimit-remaining: 0) or a rate-limit message in the body. + const remaining = headers.get('x-ratelimit-remaining') + if ( + status === HTTP_STATUS_TOO_MANY_REQUESTS || + (status === HTTP_STATUS_FORBIDDEN && + (remaining === '0' || lowerBody.includes('rate limit'))) + ) { + const waitSeconds = getRateLimitWaitSeconds(headers) + const resetHint = + waitSeconds === undefined + ? 'Try again in a few minutes.' + : `Try again in ${waitSeconds} second${waitSeconds === 1 ? '' : 's'}.` + return { + ok: false, + message: GITHUB_ERR_RATE_LIMIT, + cause: + `GitHub API rate limit exceeded on the supplied token while ${context}. ` + + `${resetHint} ` + + 'Authenticated requests get a far higher rate limit than ' + + 'unauthenticated ones — set a valid GITHUB_TOKEN (or pass ' + + '--github-token) if you have not already.', + } + } + + // Authentication failure. The token is invalid/expired or lacks scopes; + // retrying with the same token will not help. + if (status === HTTP_STATUS_UNAUTHORIZED) { + return { + ok: false, + message: GITHUB_ERR_AUTH_FAILED, + cause: + `GitHub authentication failed while ${context}. ` + + 'The token may be invalid, expired, or missing required scopes ' + + '(read access to repository contents). Provide a valid GITHUB_TOKEN ' + + '(or pass --github-token) and retry.', + } + } + + return undefined +} + +function backoffMs(attempt: number): number { + return Math.min(1000 * 2 ** (attempt - 1), MAX_BACKOFF_MS) +} + +function sleep(ms: number): Promise { + return new Promise(resolve => { + setTimeout(resolve, ms) + }) +} + +/** + * Perform a GitHub REST request through `apiFetch`, detecting rate-limit / + * auth / abuse-detection responses up front and applying bounded retries. + * + * On success returns the response together with its already-read body text + * (the body stream can only be consumed once). On a blocking or exhausted + * transient failure returns a typed CResult error. Non-blocking non-2xx + * responses (e.g. 404, or GitHub's "empty repository" 200) are returned as + * successes so the caller keeps its existing body-parsing logic. + * + * Retry policy: + * - Rate limit / abuse: wait once for the reset window, but only when it is + * short (<= CHEAP_RATE_LIMIT_WAIT_MAX_SECONDS); otherwise surface the error + * immediately. Long primary-limit resets are not worth blocking on. + * - Auth: never retried. + * - 5xx / network: capped exponential backoff, MAX_TRANSIENT_ATTEMPTS total. + */ +export async function githubApiRequest( + url: string, + init: ApiFetchInit, + context: string, + // Injectable request implementation. Defaults to the real `apiFetch`; + // tests pass a fake so the retry/backoff logic can be exercised without + // touching the network. + fetchImpl: (url: string, init: ApiFetchInit) => Promise = apiFetch, +): Promise> { + const method = init.method || 'GET' + let rateLimitWaitUsed = false + for (let attempt = 1; ; attempt += 1) { + debugApiRequest(method, url) + let response: Response + try { + // eslint-disable-next-line no-await-in-loop + response = await fetchImpl(url, init) + debugApiResponse(method, url, response.status) + } catch (e) { + debugApiResponse(method, url, undefined, e) + // Network-level failure (DNS, connection reset, timeout). Retry a few + // times with bounded backoff before giving up. + if (attempt < MAX_TRANSIENT_ATTEMPTS) { + debugFn('notice', `retry: network error while ${context}`, attempt) + // eslint-disable-next-line no-await-in-loop + await sleep(backoffMs(attempt)) + continue + } + return { + ok: false, + message: 'Network error connecting to GitHub', + cause: formatErrorWithDetail(`Network error while ${context}`, e), + } + } + + // eslint-disable-next-line no-await-in-loop + const bodyText = await response.text() + + const blocking = classifyGitHubResponse( + response.status, + response.headers, + bodyText, + context, + ) + if (blocking) { + // Auth failures never succeed on retry. + if (blocking.message === GITHUB_ERR_AUTH_FAILED) { + return blocking + } + // Rate limit / abuse: retry once, but only when the reset window is + // short enough to be worth waiting on. + const waitSeconds = getRateLimitWaitSeconds(response.headers) + if ( + !rateLimitWaitUsed && + waitSeconds !== undefined && + waitSeconds <= CHEAP_RATE_LIMIT_WAIT_MAX_SECONDS + ) { + rateLimitWaitUsed = true + logger.info( + `GitHub rate limit hit while ${context}; waiting ${waitSeconds}s before one retry...`, + ) + // Add a second of slack so we retry just past the reset boundary. + // eslint-disable-next-line no-await-in-loop + await sleep((waitSeconds + 1) * 1000) + continue + } + return blocking + } + + // Transient server errors: retry with bounded backoff, then surface. + if (response.status >= HTTP_STATUS_INTERNAL_SERVER_ERROR) { + if (attempt < MAX_TRANSIENT_ATTEMPTS) { + debugFn( + 'notice', + `retry: GitHub ${response.status} while ${context}`, + attempt, + ) + // eslint-disable-next-line no-await-in-loop + await sleep(backoffMs(attempt)) + continue + } + return { + ok: false, + message: 'GitHub server error', + cause: + `GitHub server error (${response.status}) while ${context}. ` + + 'GitHub may be experiencing issues; try again shortly.', + } + } + + return { ok: true, data: { response, bodyText } } + } +} diff --git a/src/utils/github-errors.test.mts b/src/utils/github-errors.test.mts new file mode 100644 index 0000000000..24fc058968 --- /dev/null +++ b/src/utils/github-errors.test.mts @@ -0,0 +1,254 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { + GITHUB_ERR_ABUSE_DETECTION, + GITHUB_ERR_AUTH_FAILED, + GITHUB_ERR_RATE_LIMIT, + classifyGitHubResponse, + getRateLimitWaitSeconds, + githubApiRequest, + isGitHubBlockingError, +} from './github-errors.mts' + +function makeResponse( + status: number, + headers: Record = {}, + body = '', +): Response { + return new Response(body, { status, headers }) +} + +type ResponseSpec = { + status: number + headers?: Record + body?: string +} + +// A fake apiFetch that returns a fresh Response per call from the queued +// specs (a Response body can only be read once, and real apiFetch returns a +// new Response each time). Runs past the end by repeating the last spec. No +// module mocking — passed directly into githubApiRequest's injectable param. +function fakeFetch(specs: ResponseSpec[]): { + fetchImpl: (url: string, init: unknown) => Promise + calls: () => number +} { + let index = 0 + let calls = 0 + return { + fetchImpl: () => { + calls += 1 + const spec = specs[Math.min(index, specs.length - 1)]! + index += 1 + return Promise.resolve( + makeResponse(spec.status, spec.headers ?? {}, spec.body ?? ''), + ) + }, + calls: () => calls, + } +} + +describe('classifyGitHubResponse', () => { + const ctx = 'fetching repo details for acme/widgets' + + it('flags a 403 with x-ratelimit-remaining: 0 as a rate limit', () => { + const result = classifyGitHubResponse( + 403, + new Headers({ 'x-ratelimit-remaining': '0' }), + '{"message":"API rate limit exceeded"}', + ctx, + ) + expect(result?.ok).toBe(false) + expect(result?.message).toBe(GITHUB_ERR_RATE_LIMIT) + }) + + it('flags a 429 as a rate limit', () => { + const result = classifyGitHubResponse(429, new Headers(), '', ctx) + expect(result?.message).toBe(GITHUB_ERR_RATE_LIMIT) + }) + + it('flags a 403 with a rate-limit body message as a rate limit', () => { + const result = classifyGitHubResponse( + 403, + new Headers(), + '{"message":"API rate limit exceeded for user 123."}', + ctx, + ) + expect(result?.message).toBe(GITHUB_ERR_RATE_LIMIT) + }) + + it('flags a 403 secondary rate limit as abuse detection', () => { + const result = classifyGitHubResponse( + 403, + new Headers(), + '{"message":"You have exceeded a secondary rate limit."}', + ctx, + ) + expect(result?.message).toBe(GITHUB_ERR_ABUSE_DETECTION) + }) + + it('flags a 401 as an auth failure', () => { + const result = classifyGitHubResponse(401, new Headers(), '', ctx) + expect(result?.message).toBe(GITHUB_ERR_AUTH_FAILED) + }) + + it('surfaces the reset window in the rate-limit cause', () => { + const result = classifyGitHubResponse( + 429, + new Headers({ 'retry-after': '42' }), + '', + ctx, + ) + expect(result?.cause).toContain('42 seconds') + }) + + it('returns undefined for a healthy 200', () => { + expect( + classifyGitHubResponse(200, new Headers(), '{}', ctx), + ).toBeUndefined() + }) + + it('returns undefined for a 404 (handled by the caller)', () => { + expect( + classifyGitHubResponse(404, new Headers(), '{}', ctx), + ).toBeUndefined() + }) + + it('returns undefined for a 403 permission denial with quota remaining', () => { + const result = classifyGitHubResponse( + 403, + new Headers({ 'x-ratelimit-remaining': '4999' }), + '{"message":"Must have admin rights to Repository."}', + ctx, + ) + expect(result).toBeUndefined() + }) +}) + +describe('getRateLimitWaitSeconds', () => { + it('prefers retry-after (seconds)', () => { + expect(getRateLimitWaitSeconds(new Headers({ 'retry-after': '30' }))).toBe( + 30, + ) + }) + + it('falls back to x-ratelimit-reset (epoch seconds)', () => { + const resetEpoch = Math.floor(Date.now() / 1000) + 25 + const seconds = getRateLimitWaitSeconds( + new Headers({ 'x-ratelimit-reset': String(resetEpoch) }), + ) + expect(seconds).toBeGreaterThan(20) + expect(seconds).toBeLessThanOrEqual(25) + }) + + it('returns undefined when neither header is present', () => { + expect(getRateLimitWaitSeconds(new Headers())).toBeUndefined() + }) +}) + +describe('isGitHubBlockingError', () => { + it('is true for rate limit / auth / abuse', () => { + expect(isGitHubBlockingError(GITHUB_ERR_RATE_LIMIT)).toBe(true) + expect(isGitHubBlockingError(GITHUB_ERR_AUTH_FAILED)).toBe(true) + expect(isGitHubBlockingError(GITHUB_ERR_ABUSE_DETECTION)).toBe(true) + }) + + it('is false for a benign no-manifest result', () => { + expect(isGitHubBlockingError('No manifest files found')).toBe(false) + }) +}) + +describe('githubApiRequest', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('returns the response and body text on success', async () => { + const fake = fakeFetch([{ status: 200, body: '{"ok":true}' }]) + const result = await githubApiRequest( + 'https://api.github.com/x', + {}, + 'x', + fake.fetchImpl, + ) + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.data.bodyText).toBe('{"ok":true}') + expect(result.data.response.status).toBe(200) + } + expect(fake.calls()).toBe(1) + }) + + it('surfaces a long-window rate limit immediately without retrying', async () => { + // No retry-after / reset header => unknown window => surface now. + const fake = fakeFetch([ + { + status: 403, + headers: { 'x-ratelimit-remaining': '0' }, + body: '{"message":"API rate limit exceeded"}', + }, + ]) + const result = await githubApiRequest( + 'https://api.github.com/x', + {}, + 'x', + fake.fetchImpl, + ) + expect(result.ok).toBe(false) + expect(result.ok ? '' : result.message).toBe(GITHUB_ERR_RATE_LIMIT) + expect(fake.calls()).toBe(1) + }) + + it('never retries an auth failure', async () => { + const fake = fakeFetch([{ status: 401 }]) + const result = await githubApiRequest( + 'https://api.github.com/x', + {}, + 'x', + fake.fetchImpl, + ) + expect(result.ok ? '' : result.message).toBe(GITHUB_ERR_AUTH_FAILED) + expect(fake.calls()).toBe(1) + }) + + it('waits and retries once on a short rate-limit window, then succeeds', async () => { + vi.useFakeTimers() + const fake = fakeFetch([ + { + status: 403, + headers: { 'x-ratelimit-remaining': '0', 'retry-after': '1' }, + body: '{"message":"API rate limit exceeded"}', + }, + { status: 200, body: '{"recovered":true}' }, + ]) + const promise = githubApiRequest( + 'https://api.github.com/x', + {}, + 'x', + fake.fetchImpl, + ) + await vi.runAllTimersAsync() + const result = await promise + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.data.bodyText).toBe('{"recovered":true}') + } + expect(fake.calls()).toBe(2) + }) + + it('retries transient 5xx with bounded backoff, then surfaces a server error', async () => { + vi.useFakeTimers() + const fake = fakeFetch([{ status: 503, body: 'unavailable' }]) + const promise = githubApiRequest( + 'https://api.github.com/x', + {}, + 'x', + fake.fetchImpl, + ) + await vi.runAllTimersAsync() + const result = await promise + expect(result.ok).toBe(false) + expect(result.ok ? '' : result.message).toBe('GitHub server error') + // Initial attempt + 2 retries = 3 total. + expect(fake.calls()).toBe(3) + }) +})