fix(audit-action): retry, chunk and bound npm advisory requests

action.yaml

@@ -23,6 +23,8 @@ outputs:
     description: 'Rendered vulnerability report (markdown)'
   failed:
     description: 'Set to true when vulnerabilities exceed failure-level'
+  registry-unavailable:
+    description: 'Set to true when the audit could not run because the npm advisory registry was unreachable'
 runs:
   using: 'node24'
   main: 'dist/index.js'

package.json

@@ -1,6 +1,6 @@
 {
   "name": "audit-action",
-  "version": "3.2.1",
+  "version": "3.3.0",
   "private": true,
   "main": "dist/index.js",
   "packageManager": "pnpm@10.33.0",

src/dependency/index.ts

@@ -3,6 +3,8 @@ import type { DependencyAuditOptions } from './types.js'
 import { pnpmBulkAuditor } from './pnpmBulkAuditor.js'
 import { yarnAuditor } from './yarnAuditor.js'
 
+export { RegistryUnavailableError } from './registryClient.js'
+
 const DEPENDENCY_AUDITORS = {
   pnpm: pnpmBulkAuditor,
   yarn: yarnAuditor,

src/dependency/pnpmBulkAuditor.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, vi } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 // -- Inline fixtures --
 
@@ -315,6 +315,10 @@ describe('mapToAuditMetadata', () => {
 })
 
 describe('pnpmBulkAuditor', () => {
+  beforeEach(() => {
+    fetchMock.mockReset()
+  })
+
   it('collects the prod closure from the lockfile and fetches advisories end-to-end', async () => {
     lockfileExists = true
     fetchMock.mockResolvedValueOnce({
@@ -360,17 +364,6 @@ describe('pnpmBulkAuditor', () => {
     }
   })
 
-  it('throws when the registry returns non-200', async () => {
-    lockfileExists = true
-    fetchMock.mockResolvedValueOnce({
-      ok: false,
-      status: 503,
-      text: async () => 'Service Unavailable',
-    })
-
-    await expect(pnpmBulkAuditor()).rejects.toThrow(/Registry returned 503/)
-  })
-
   it('produces a detailed report with direct/indirect flags when options.detailed is true', async () => {
     lockfileExists = true
     fetchMock.mockResolvedValueOnce({

src/dependency/pnpmBulkAuditor.ts

@@ -5,10 +5,11 @@ import type { Severity } from 'audit-types'
 import semver from 'semver'
 import { parse as parseYaml } from 'yaml'
 
+import type { BulkAdvisoryResponse } from './registryClient.js'
 import type { DependencyAuditOptions, DependencyAuditReport, VulnerablePackage } from './types.js'
 
-const BULK_ENDPOINT = 'https://registry.npmjs.org/-/npm/v1/security/advisories/bulk'
-const REQUEST_TIMEOUT_MS = 10_000
+import { fetchBulkAdvisories } from './registryClient.js'
+
 const LOCKFILE_NAME = 'pnpm-lock.yaml'
 
 type DependencyMap = Map<string, Set<string>>
@@ -36,16 +37,6 @@ interface Lockfile {
   snapshots?: Record<string, LockfileSnapshot>
 }
 
-// -- Types for bulk advisory response --
-
-interface BulkAdvisory {
-  severity: string
-  vulnerable_versions: string
-  title: string
-}
-
-type BulkAdvisoryResponse = Record<string, BulkAdvisory[]>
-
 // ---------------------------------------------------------------------------
 // Phase A: Collect the dependency closure from pnpm-lock.yaml
 // ---------------------------------------------------------------------------
@@ -190,22 +181,6 @@ function depsToPayload(deps: DependencyMap): Record<string, string[]> {
   return payload
 }
 
-async function fetchAdvisories(deps: DependencyMap): Promise<BulkAdvisoryResponse> {
-  const res = await fetch(BULK_ENDPOINT, {
-    method: 'POST',
-    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify(depsToPayload(deps)),
-    signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
-  })
-
-  if (!res.ok) {
-    const body = await res.text().catch(() => '')
-    throw new Error(`Registry returned ${res.status}: ${body}`)
-  }
-
-  return res.json()
-}
-
 // ---------------------------------------------------------------------------
 // Phase C: Map bulk advisory response to AuditMetadata
 // ---------------------------------------------------------------------------
@@ -286,7 +261,7 @@ export async function pnpmBulkAuditor(
 ): Promise<DependencyAuditReport> {
   const includeDevDeps = options?.includeDevDeps ?? false
   const deps = collectDependencies(options?.path, includeDevDeps)
-  const advisories = await fetchAdvisories(deps)
+  const advisories = await fetchBulkAdvisories(depsToPayload(deps))
   const directSet = options?.detailed
     ? readDirectDependencies(options?.path, includeDevDeps)
     : undefined

src/dependency/registryClient.test.ts

@@ -0,0 +1,146 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { fetchBulkAdvisories, RegistryUnavailableError } from './registryClient.js'
+
+const fetchMock = vi.fn()
+vi.stubGlobal('fetch', fetchMock)
+
+function response(init: {
+  ok: boolean
+  status?: number
+  body?: unknown
+  headers?: Record<string, string>
+}) {
+  return {
+    ok: init.ok,
+    status: init.status ?? (init.ok ? 200 : 500),
+    headers: new Headers(init.headers),
+    json: async () => init.body ?? {},
+    text: async () => JSON.stringify(init.body ?? ''),
+  }
+}
+
+// Retries sleep between attempts; drive those timers instead of waiting on them.
+async function withFakeTimers<T>(start: () => Promise<T>): Promise<T> {
+  vi.useFakeTimers()
+  try {
+    const pending = start()
+    await vi.runAllTimersAsync()
+    return await pending
+  } finally {
+    vi.useRealTimers()
+  }
+}
+
+function sentPayloadSizes() {
+  return fetchMock.mock.calls.map(
+    (call) => Object.keys(JSON.parse((call[1] as { body: string }).body)).length,
+  )
+}
+
+describe('fetchBulkAdvisories', () => {
+  beforeEach(() => {
+    fetchMock.mockReset()
+  })
+
+  it('returns the advisories of a successful request', async () => {
+    fetchMock.mockResolvedValueOnce(response({ ok: true, body: { axios: [] } }))
+
+    await expect(fetchBulkAdvisories({ axios: ['0.21.1'] })).resolves.toEqual({ axios: [] })
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+  })
+
+  it('retries transient failures and succeeds', async () => {
+    fetchMock
+      .mockResolvedValueOnce(response({ ok: false, status: 503 }))
+      .mockRejectedValueOnce(new DOMException('timeout', 'TimeoutError'))
+      .mockResolvedValueOnce(response({ ok: true, body: { lodash: [] } }))
+
+    const advisories = await withFakeTimers(() => fetchBulkAdvisories({ lodash: ['4.17.20'] }))
+
+    expect(advisories).toEqual({ lodash: [] })
+    expect(fetchMock).toHaveBeenCalledTimes(3)
+  })
+
+  it('gives up after the retry budget and reports the registry as unavailable', async () => {
+    fetchMock.mockResolvedValue(response({ ok: false, status: 503 }))
+
+    const error = await withFakeTimers(() =>
+      fetchBulkAdvisories({ lodash: ['4.17.20'] }).catch((e) => e),
+    )
+
+    expect(error).toBeInstanceOf(RegistryUnavailableError)
+    expect((error as Error).message).toMatch(
+      /unreachable within 4 attempts.*Registry returned 503/s,
+    )
+    expect(fetchMock).toHaveBeenCalledTimes(4)
+  })
+
+  it('does not retry a non-retryable registry response', async () => {
+    fetchMock.mockResolvedValue(response({ ok: false, status: 400 }))
+
+    const error = await fetchBulkAdvisories({ lodash: ['4.17.20'] }).catch((e) => e)
+
+    expect(error).not.toBeInstanceOf(RegistryUnavailableError)
+    expect((error as Error).message).toMatch(/Registry returned 400/)
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+  })
+
+  it('truncates an oversized registry error body', async () => {
+    fetchMock.mockResolvedValue(response({ ok: false, status: 400, body: 'x'.repeat(5000) }))
+
+    const error = await fetchBulkAdvisories({ lodash: ['4.17.20'] }).catch((e) => e)
+
+    expect((error as Error).message.length).toBeLessThan(300)
+    expect((error as Error).message).toMatch(/…$/)
+  })
+
+  it('honours Retry-After for the next attempt', async () => {
+    const attemptedAt: number[] = []
+    fetchMock.mockImplementation(async () => {
+      attemptedAt.push(Date.now())
+      return attemptedAt.length === 1
+        ? response({ ok: false, status: 429, headers: { 'retry-after': '5' } })
+        : response({ ok: true, body: {} })
+    })
+
+    await withFakeTimers(() => fetchBulkAdvisories({ lodash: ['4.17.20'] }))
+
+    expect(attemptedAt).toHaveLength(2)
+    expect(attemptedAt[1] - attemptedAt[0]).toBe(5000)
+  })
+
+  it('splits large dependency sets into several requests and merges the responses', async () => {
+    const payload: Record<string, string[]> = {}
+    for (let i = 0; i < 900; i++) payload[`pkg-${i}`] = ['1.0.0']
+
+    fetchMock
+      .mockResolvedValueOnce(response({ ok: true, body: { 'pkg-1': [] } }))
+      .mockResolvedValueOnce(response({ ok: true, body: { 'pkg-500': [] } }))
+      .mockResolvedValueOnce(response({ ok: true, body: { 'pkg-800': [] } }))
+
+    const advisories = await fetchBulkAdvisories(payload)
+
+    expect(fetchMock).toHaveBeenCalledTimes(3)
+    expect(Object.keys(advisories).sort()).toEqual(['pkg-1', 'pkg-500', 'pkg-800'])
+    expect(sentPayloadSizes().sort((a, b) => b - a)).toEqual([400, 400, 100])
+  })
+
+  it('stops retrying once the whole-run budget is spent', async () => {
+    const payload: Record<string, string[]> = {}
+    for (let i = 0; i < 1600; i++) payload[`pkg-${i}`] = ['1.0.0']
+
+    fetchMock.mockImplementation(
+      () =>
+        new Promise((_resolve, reject) => {
+          setTimeout(() => reject(new DOMException('timeout', 'TimeoutError')), 30_000)
+        }),
+    )
+
+    const error = await withFakeTimers(() => fetchBulkAdvisories(payload).catch((e) => e))
+
+    expect(error).toBeInstanceOf(RegistryUnavailableError)
+    // 4 chunks x 4 attempts would be 16; the shared deadline cuts the run short.
+    expect(fetchMock.mock.calls.length).toBeLessThan(16)
+  })
+})