From cc2255dfcf619488a5f79f4c5e2b40eaeec695be Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sun, 1 Feb 2026 17:50:58 +0000 Subject: [PATCH 1/2] perf: use osv batch api for dep analysis --- server/utils/dependency-analysis.ts | 127 +++++-- shared/types/dependency-analysis.ts | 24 ++ .../server/utils/dependency-analysis.spec.ts | 325 ++++++++++++------ 3 files changed, 334 insertions(+), 142 deletions(-) diff --git a/server/utils/dependency-analysis.ts b/server/utils/dependency-analysis.ts index 7be88a081e..b60bdc7742 100644 --- a/server/utils/dependency-analysis.ts +++ b/server/utils/dependency-analysis.ts @@ -1,5 +1,6 @@ import type { OsvQueryResponse, + OsvBatchResponse, OsvVulnerability, OsvSeverityLevel, VulnerabilitySummary, @@ -10,29 +11,68 @@ import type { } from '#shared/types/dependency-analysis' import { resolveDependencyTree } from './dependency-resolver' -/** Result of a single OSV query */ -type OsvQueryResult = { status: 'ok'; data: PackageVulnerabilityInfo | null } | { status: 'error' } +/** Package info needed for OSV queries */ +interface PackageQueryInfo { + name: string + version: string + depth: DependencyDepth + path: string[] +} /** - * Query OSV for vulnerabilities in a package + * Query OSV batch API to find which packages have vulnerabilities. + * Returns indices of packages that have vulnerabilities (for follow-up detailed queries). + * @see https://google.github.io/osv.dev/post-v1-querybatch/ */ -async function queryOsv( - name: string, - version: string, - depth: DependencyDepth, - path: string[], -): Promise { +async function queryOsvBatch( + packages: PackageQueryInfo[], +): Promise<{ vulnerableIndices: number[]; failed: boolean }> { + if (packages.length === 0) return { vulnerableIndices: [], failed: false } + + try { + const response = await $fetch('https://api.osv.dev/v1/querybatch', { + method: 'POST', + body: { + queries: packages.map(pkg => ({ + package: { name: pkg.name, ecosystem: 'npm' }, + version: pkg.version, + })), + }, + }) + + // Find indices of packages that have vulnerabilities + const vulnerableIndices: number[] = [] + for (let i = 0; i < response.results.length; i++) { + const result = response.results[i] + if (result?.vulns && result.vulns.length > 0) { + vulnerableIndices.push(i) + } + } + + return { vulnerableIndices, failed: false } + } catch (error) { + // oxlint-disable-next-line no-console -- log OSV API failures for debugging + console.warn(`[dep-analysis] OSV batch query failed:`, error) + return { vulnerableIndices: [], failed: true } + } +} + +/** + * Query OSV for full vulnerability details for a single package. + * Only called for packages known to have vulnerabilities. + */ +async function queryOsvDetails(pkg: PackageQueryInfo): Promise { try { const response = await $fetch('https://api.osv.dev/v1/query', { method: 'POST', body: { - package: { name, ecosystem: 'npm' }, - version, + package: { name: pkg.name, ecosystem: 'npm' }, + version: pkg.version, }, }) const vulns = response.vulns || [] - if (vulns.length === 0) return { status: 'ok', data: null } + if (vulns.length === 0) return null const counts = { total: vulns.length, critical: 0, high: 0, moderate: 0, low: 0 } const vulnerabilities: VulnerabilitySummary[] = [] @@ -65,11 +105,18 @@ async function queryOsv( }) } - return { status: 'ok', data: { name, version, depth, path, vulnerabilities, counts } } + return { + name: pkg.name, + version: pkg.version, + depth: pkg.depth, + path: pkg.path, + vulnerabilities, + counts, + } } catch (error) { // oxlint-disable-next-line no-console -- log OSV API failures for debugging - console.warn(`[dep-analysis] OSV query failed for ${name}@${version}:`, error) - return { status: 'error' } + console.warn(`[dep-analysis] OSV detail query failed for ${pkg.name}@${pkg.version}:`, error) + return null } } @@ -110,17 +157,24 @@ function getSeverityLevel(vuln: OsvVulnerability): OsvSeverityLevel { /** * Analyze entire dependency tree for vulnerabilities and deprecated packages. + * Uses OSV batch API for efficient vulnerability discovery, then fetches + * full details only for packages with known vulnerabilities. */ export const analyzeDependencyTree = defineCachedFunction( async (name: string, version: string): Promise => { // Resolve all packages in the tree with depth tracking const resolved = await resolveDependencyTree(name, version, { trackDepth: true }) - // Convert to array for OSV querying - const packages = [...resolved.values()] + // Convert to array with query info + const packages: PackageQueryInfo[] = [...resolved.values()].map(pkg => ({ + name: pkg.name, + version: pkg.version, + depth: pkg.depth!, + path: pkg.path || [], + })) // Collect deprecated packages (no API call needed - already in packument data) - const deprecatedPackages: DeprecatedPackageInfo[] = packages + const deprecatedPackages: DeprecatedPackageInfo[] = [...resolved.values()] .filter(pkg => pkg.deprecated) .map(pkg => ({ name: pkg.name, @@ -135,22 +189,27 @@ export const analyzeDependencyTree = defineCachedFunction( return depthOrder[a.depth] - depthOrder[b.depth] }) - // Query OSV for all packages in parallel batches - const vulnerablePackages: PackageVulnerabilityInfo[] = [] - let failedQueries = 0 - const batchSize = 10 + // Step 1: Use batch API to find which packages have vulnerabilities + // This is much faster than individual queries - one request for all packages + const { vulnerableIndices, failed: batchFailed } = await queryOsvBatch(packages) + + let vulnerablePackages: PackageVulnerabilityInfo[] = [] + let failedQueries = batchFailed ? packages.length : 0 + + if (!batchFailed && vulnerableIndices.length > 0) { + // Step 2: Fetch full vulnerability details only for packages with vulns + // This is typically a small fraction of total packages + const vulnerablePackageInfos = vulnerableIndices.map(i => packages[i]!) - for (let i = 0; i < packages.length; i += batchSize) { - const batch = packages.slice(i, i + batchSize) - const results = await Promise.all( - batch.map(pkg => queryOsv(pkg.name, pkg.version, pkg.depth!, pkg.path || [])), + const detailResults = await Promise.all( + vulnerablePackageInfos.map(pkg => queryOsvDetails(pkg)), ) - for (const result of results) { - if (result.status === 'error') { + for (const result of detailResults) { + if (result) { + vulnerablePackages.push(result) + } else { failedQueries++ - } else if (result.data) { - vulnerablePackages.push(result.data) } } } @@ -175,11 +234,11 @@ export const analyzeDependencyTree = defineCachedFunction( totalCounts.low += pkg.counts.low } - // Log critical failures (>50% of queries failed) - if (failedQueries > 0 && failedQueries > packages.length / 2) { + // Log if batch query failed entirely + if (batchFailed) { // oxlint-disable-next-line no-console -- critical error logging console.error( - `[dep-analysis] Critical: ${failedQueries}/${packages.length} OSV queries failed for ${name}@${version}`, + `[dep-analysis] Critical: OSV batch query failed for ${name}@${version} (${packages.length} packages)`, ) } @@ -197,6 +256,6 @@ export const analyzeDependencyTree = defineCachedFunction( maxAge: 60 * 60, swr: true, name: 'dependency-analysis', - getKey: (name: string, version: string) => `v1:${name}@${version}`, + getKey: (name: string, version: string) => `v2:${name}@${version}`, }, ) diff --git a/shared/types/dependency-analysis.ts b/shared/types/dependency-analysis.ts index 5924110ed3..2f768132a6 100644 --- a/shared/types/dependency-analysis.ts +++ b/shared/types/dependency-analysis.ts @@ -64,6 +64,30 @@ export interface OsvQueryResponse { next_page_token?: string } +/** + * Single result from OSV batch query (minimal info - just ID and modified) + */ +export interface OsvBatchVulnRef { + id: string + modified: string +} + +/** + * Single result in OSV batch response + */ +export interface OsvBatchResult { + vulns?: OsvBatchVulnRef[] + next_page_token?: string +} + +/** + * OSV batch query response + * @see https://google.github.io/osv.dev/post-v1-querybatch/ + */ +export interface OsvBatchResponse { + results: OsvBatchResult[] +} + /** * Simplified vulnerability info for display */ diff --git a/test/unit/server/utils/dependency-analysis.spec.ts b/test/unit/server/utils/dependency-analysis.spec.ts index 309008fe5c..a70abad4e5 100644 --- a/test/unit/server/utils/dependency-analysis.spec.ts +++ b/test/unit/server/utils/dependency-analysis.spec.ts @@ -14,6 +14,31 @@ vi.mock('../../../../server/utils/dependency-resolver', () => ({ const { resolveDependencyTree } = await import('../../../../server/utils/dependency-resolver') +/** + * Helper to create mock $fetch that handles the two-step OSV API pattern: + * 1. Batch query to /v1/querybatch - returns which packages have vulns (just IDs) + * 2. Individual queries to /v1/query - returns full vuln details for affected packages + * + * @param batchResults - Array of results for batch query (vulns array per package, in order) + * @param detailResults - Map of package key to full vuln details for individual queries + */ +function mockOsvApi( + batchResults: Array<{ vulns?: Array<{ id: string; modified: string }> }>, + detailResults: Map> }> = new Map(), +) { + vi.mocked($fetch).mockImplementation(async (url: string, options?: { body?: unknown }) => { + if (url === 'https://api.osv.dev/v1/querybatch') { + return { results: batchResults } + } + if (url === 'https://api.osv.dev/v1/query') { + const body = options?.body as { package: { name: string }; version: string } + const key = `${body.package.name}@${body.version}` + return detailResults.get(key) || { vulns: [] } + } + throw new Error(`Unexpected fetch to ${url}`) + }) +} + describe('dependency-analysis', () => { beforeEach(() => { vi.clearAllMocks() @@ -36,8 +61,8 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - // Mock OSV API returning no vulnerabilities - vi.mocked($fetch).mockResolvedValue({ vulns: [] }) + // Mock batch API returning no vulnerabilities + mockOsvApi([{ vulns: [] }]) const result = await analyzeDependencyTree('test-pkg', '1.0.0') @@ -49,7 +74,7 @@ describe('dependency-analysis', () => { expect(result.totalCounts).toEqual({ total: 0, critical: 0, high: 0, moderate: 0, low: 0 }) }) - it('tracks failed queries when OSV API fails', async () => { + it('tracks failed queries when OSV batch API fails', async () => { const mockResolved = new Map([ [ 'test-pkg@1.0.0', @@ -76,14 +101,13 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - // First call succeeds, second fails - vi.mocked($fetch) - .mockResolvedValueOnce({ vulns: [] }) - .mockRejectedValueOnce(new Error('OSV API error')) + // Batch query fails entirely + vi.mocked($fetch).mockRejectedValue(new Error('OSV API error')) const result = await analyzeDependencyTree('test-pkg', '1.0.0') - expect(result.failedQueries).toBe(1) + // When batch fails, all packages are counted as failed + expect(result.failedQueries).toBe(2) expect(result.totalPackages).toBe(2) }) @@ -103,15 +127,31 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - // Mock OSV API returning vulnerabilities with different severities - vi.mocked($fetch).mockResolvedValue({ - vulns: [ - { id: 'GHSA-1', summary: 'Critical vuln', database_specific: { severity: 'CRITICAL' } }, - { id: 'GHSA-2', summary: 'High vuln', database_specific: { severity: 'HIGH' } }, - { id: 'GHSA-3', summary: 'Moderate vuln', database_specific: { severity: 'MODERATE' } }, - { id: 'GHSA-4', summary: 'Low vuln', database_specific: { severity: 'LOW' } }, - ], - }) + // Mock batch API returns vuln IDs, then detail query returns full info + mockOsvApi( + [{ vulns: [{ id: 'GHSA-1', modified: '2024-01-01' }] }], + new Map([ + [ + 'vuln-pkg@1.0.0', + { + vulns: [ + { + id: 'GHSA-1', + summary: 'Critical vuln', + database_specific: { severity: 'CRITICAL' }, + }, + { id: 'GHSA-2', summary: 'High vuln', database_specific: { severity: 'HIGH' } }, + { + id: 'GHSA-3', + summary: 'Moderate vuln', + database_specific: { severity: 'MODERATE' }, + }, + { id: 'GHSA-4', summary: 'Low vuln', database_specific: { severity: 'LOW' } }, + ], + }, + ], + ]), + ) const result = await analyzeDependencyTree('vuln-pkg', '1.0.0') @@ -119,10 +159,10 @@ describe('dependency-analysis', () => { expect(result.totalCounts).toEqual({ total: 4, critical: 1, high: 1, moderate: 1, low: 1 }) const pkg = result.vulnerablePackages[0] - expect(pkg.counts.critical).toBe(1) - expect(pkg.counts.high).toBe(1) - expect(pkg.counts.moderate).toBe(1) - expect(pkg.counts.low).toBe(1) + expect(pkg?.counts.critical).toBe(1) + expect(pkg?.counts.high).toBe(1) + expect(pkg?.counts.moderate).toBe(1) + expect(pkg?.counts.low).toBe(1) }) it('includes dependency path in vulnerable packages', async () => { @@ -152,20 +192,27 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch) - .mockResolvedValueOnce({ vulns: [] }) // root has no vulns - .mockResolvedValueOnce({ - vulns: [ - { id: 'GHSA-test', summary: 'Test vuln', database_specific: { severity: 'HIGH' } }, + // Batch: root has no vulns, vuln-dep has vulns + mockOsvApi( + [{ vulns: [] }, { vulns: [{ id: 'GHSA-test', modified: '2024-01-01' }] }], + new Map([ + [ + 'vuln-dep@2.0.0', + { + vulns: [ + { id: 'GHSA-test', summary: 'Test vuln', database_specific: { severity: 'HIGH' } }, + ], + }, ], - }) // vuln-dep has vuln + ]), + ) const result = await analyzeDependencyTree('root', '1.0.0') expect(result.vulnerablePackages).toHaveLength(1) const vulnPkg = result.vulnerablePackages[0] - expect(vulnPkg.path).toEqual(['root@1.0.0', 'middle@1.5.0', 'vuln-dep@2.0.0']) - expect(vulnPkg.depth).toBe('transitive') + expect(vulnPkg?.path).toEqual(['root@1.0.0', 'middle@1.5.0', 'vuln-dep@2.0.0']) + expect(vulnPkg?.depth).toBe('transitive') }) it('sorts vulnerable packages by depth then severity', async () => { @@ -206,35 +253,56 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - // All have vulnerabilities - vi.mocked($fetch) - .mockResolvedValueOnce({ - vulns: [ - { id: 'GHSA-root', summary: 'Root vuln', database_specific: { severity: 'LOW' } }, + // All packages have vulnerabilities + mockOsvApi( + [ + { vulns: [{ id: 'GHSA-root', modified: '2024-01-01' }] }, + { vulns: [{ id: 'GHSA-direct', modified: '2024-01-01' }] }, + { vulns: [{ id: 'GHSA-trans', modified: '2024-01-01' }] }, + ], + new Map([ + [ + 'root@1.0.0', + { + vulns: [ + { id: 'GHSA-root', summary: 'Root vuln', database_specific: { severity: 'LOW' } }, + ], + }, ], - }) - .mockResolvedValueOnce({ - vulns: [ + [ + 'direct-dep@1.0.0', { - id: 'GHSA-direct', - summary: 'Direct vuln', - database_specific: { severity: 'CRITICAL' }, + vulns: [ + { + id: 'GHSA-direct', + summary: 'Direct vuln', + database_specific: { severity: 'CRITICAL' }, + }, + ], }, ], - }) - .mockResolvedValueOnce({ - vulns: [ - { id: 'GHSA-trans', summary: 'Trans vuln', database_specific: { severity: 'HIGH' } }, + [ + 'transitive-dep@1.0.0', + { + vulns: [ + { + id: 'GHSA-trans', + summary: 'Trans vuln', + database_specific: { severity: 'HIGH' }, + }, + ], + }, ], - }) + ]), + ) const result = await analyzeDependencyTree('root', '1.0.0') expect(result.vulnerablePackages).toHaveLength(3) // Should be sorted: root first, then direct, then transitive - expect(result.vulnerablePackages[0].name).toBe('root') - expect(result.vulnerablePackages[1].name).toBe('direct-dep') - expect(result.vulnerablePackages[2].name).toBe('transitive-dep') + expect(result.vulnerablePackages[0]?.name).toBe('root') + expect(result.vulnerablePackages[1]?.name).toBe('direct-dep') + expect(result.vulnerablePackages[2]?.name).toBe('transitive-dep') }) it('generates correct vulnerability URLs for GHSA', async () => { @@ -253,19 +321,27 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ - vulns: [ - { - id: 'GHSA-xxxx-yyyy-zzzz', - summary: 'Test vuln', - database_specific: { severity: 'HIGH' }, - }, - ], - }) + mockOsvApi( + [{ vulns: [{ id: 'GHSA-xxxx-yyyy-zzzz', modified: '2024-01-01' }] }], + new Map([ + [ + 'pkg@1.0.0', + { + vulns: [ + { + id: 'GHSA-xxxx-yyyy-zzzz', + summary: 'Test vuln', + database_specific: { severity: 'HIGH' }, + }, + ], + }, + ], + ]), + ) const result = await analyzeDependencyTree('pkg', '1.0.0') - expect(result.vulnerablePackages[0].vulnerabilities[0].url).toBe( + expect(result.vulnerablePackages[0]?.vulnerabilities[0]?.url).toBe( 'https://github.com/advisories/GHSA-xxxx-yyyy-zzzz', ) }) @@ -286,20 +362,28 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ - vulns: [ - { - id: 'OSV-2024-001', - summary: 'Test vuln', - aliases: ['CVE-2024-12345'], - database_specific: { severity: 'HIGH' }, - }, - ], - }) + mockOsvApi( + [{ vulns: [{ id: 'OSV-2024-001', modified: '2024-01-01' }] }], + new Map([ + [ + 'pkg@1.0.0', + { + vulns: [ + { + id: 'OSV-2024-001', + summary: 'Test vuln', + aliases: ['CVE-2024-12345'], + database_specific: { severity: 'HIGH' }, + }, + ], + }, + ], + ]), + ) const result = await analyzeDependencyTree('pkg', '1.0.0') - expect(result.vulnerablePackages[0].vulnerabilities[0].url).toBe( + expect(result.vulnerablePackages[0]?.vulnerabilities[0]?.url).toBe( 'https://nvd.nist.gov/vuln/detail/CVE-2024-12345', ) }) @@ -320,15 +404,27 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ - vulns: [ - { id: 'PYSEC-2024-001', summary: 'Test vuln', database_specific: { severity: 'HIGH' } }, - ], - }) + mockOsvApi( + [{ vulns: [{ id: 'PYSEC-2024-001', modified: '2024-01-01' }] }], + new Map([ + [ + 'pkg@1.0.0', + { + vulns: [ + { + id: 'PYSEC-2024-001', + summary: 'Test vuln', + database_specific: { severity: 'HIGH' }, + }, + ], + }, + ], + ]), + ) const result = await analyzeDependencyTree('pkg', '1.0.0') - expect(result.vulnerablePackages[0].vulnerabilities[0].url).toBe( + expect(result.vulnerablePackages[0]?.vulnerabilities[0]?.url).toBe( 'https://osv.dev/vulnerability/PYSEC-2024-001', ) }) @@ -349,18 +445,26 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ - vulns: [ - { - id: 'GHSA-1', - summary: 'Critical (9.5)', - severity: [{ score: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H/9.5' }], - }, - { id: 'GHSA-2', summary: 'High (7.5)', severity: [{ score: '7.5' }] }, - { id: 'GHSA-3', summary: 'Moderate (5.0)', severity: [{ score: '5.0' }] }, - { id: 'GHSA-4', summary: 'Low (2.0)', severity: [{ score: '2.0' }] }, - ], - }) + mockOsvApi( + [{ vulns: [{ id: 'GHSA-1', modified: '2024-01-01' }] }], + new Map([ + [ + 'pkg@1.0.0', + { + vulns: [ + { + id: 'GHSA-1', + summary: 'Critical (9.5)', + severity: [{ score: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H/9.5' }], + }, + { id: 'GHSA-2', summary: 'High (7.5)', severity: [{ score: '7.5' }] }, + { id: 'GHSA-3', summary: 'Moderate (5.0)', severity: [{ score: '5.0' }] }, + { id: 'GHSA-4', summary: 'Low (2.0)', severity: [{ score: '2.0' }] }, + ], + }, + ], + ]), + ) const result = await analyzeDependencyTree('pkg', '1.0.0') @@ -397,7 +501,7 @@ describe('dependency-analysis', () => { ], ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ vulns: [] }) + mockOsvApi([{ vulns: [] }, { vulns: [] }]) const result = await analyzeDependencyTree('root', '1.0.0') @@ -426,7 +530,7 @@ describe('dependency-analysis', () => { ], ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ vulns: [] }) + mockOsvApi([{ vulns: [] }]) const result = await analyzeDependencyTree('root', '1.0.0') @@ -473,17 +577,17 @@ describe('dependency-analysis', () => { ], ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - vi.mocked($fetch).mockResolvedValue({ vulns: [] }) + mockOsvApi([{ vulns: [] }, { vulns: [] }, { vulns: [] }]) const result = await analyzeDependencyTree('root', '1.0.0') expect(result.deprecatedPackages).toHaveLength(3) - expect(result.deprecatedPackages[0].name).toBe('root') - expect(result.deprecatedPackages[0].depth).toBe('root') - expect(result.deprecatedPackages[1].name).toBe('direct-dep') - expect(result.deprecatedPackages[1].depth).toBe('direct') - expect(result.deprecatedPackages[2].name).toBe('transitive-dep') - expect(result.deprecatedPackages[2].depth).toBe('transitive') + expect(result.deprecatedPackages[0]?.name).toBe('root') + expect(result.deprecatedPackages[0]?.depth).toBe('root') + expect(result.deprecatedPackages[1]?.name).toBe('direct-dep') + expect(result.deprecatedPackages[1]?.depth).toBe('direct') + expect(result.deprecatedPackages[2]?.name).toBe('transitive-dep') + expect(result.deprecatedPackages[2]?.depth).toBe('transitive') }) it('returns both vulnerabilities and deprecated packages together', async () => { @@ -525,26 +629,31 @@ describe('dependency-analysis', () => { ]) vi.mocked(resolveDependencyTree).mockResolvedValue(mockResolved) - // root and deprecated-pkg have no vulns, vuln-pkg has one - vi.mocked($fetch) - .mockResolvedValueOnce({ vulns: [] }) // root - .mockResolvedValueOnce({ - vulns: [ + // Batch: root and deprecated-pkg have no vulns, vuln-pkg has one + mockOsvApi( + [{ vulns: [] }, { vulns: [{ id: 'GHSA-vuln', modified: '2024-01-01' }] }, { vulns: [] }], + new Map([ + [ + 'vuln-pkg@1.0.0', { - id: 'GHSA-vuln', - summary: 'A vulnerability', - database_specific: { severity: 'HIGH' }, + vulns: [ + { + id: 'GHSA-vuln', + summary: 'A vulnerability', + database_specific: { severity: 'HIGH' }, + }, + ], }, ], - }) // vuln-pkg - .mockResolvedValueOnce({ vulns: [] }) // deprecated-pkg + ]), + ) const result = await analyzeDependencyTree('root', '1.0.0') expect(result.vulnerablePackages).toHaveLength(1) - expect(result.vulnerablePackages[0].name).toBe('vuln-pkg') + expect(result.vulnerablePackages[0]?.name).toBe('vuln-pkg') expect(result.deprecatedPackages).toHaveLength(1) - expect(result.deprecatedPackages[0].name).toBe('deprecated-pkg') + expect(result.deprecatedPackages[0]?.name).toBe('deprecated-pkg') expect(result.totalPackages).toBe(3) }) }) From 0ec3e236af67bf75dc2275f665df930c1d417a96 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sun, 1 Feb 2026 19:26:43 +0000 Subject: [PATCH 2/2] fix: limit concurrent requests --- app/composables/useNpmRegistry.ts | 74 ++++++++++------------ server/utils/dependency-analysis.ts | 21 ++++-- server/utils/dependency-resolver.ts | 90 +++++++++++++------------- shared/utils/async.ts | 37 +++++++++++ test/unit/shared/utils/async.spec.ts | 95 ++++++++++++++++++++++++++++ 5 files changed, 228 insertions(+), 89 deletions(-) create mode 100644 shared/utils/async.ts create mode 100644 test/unit/shared/utils/async.spec.ts diff --git a/app/composables/useNpmRegistry.ts b/app/composables/useNpmRegistry.ts index 62903d5c16..80d1eea7d1 100644 --- a/app/composables/useNpmRegistry.ts +++ b/app/composables/useNpmRegistry.ts @@ -9,6 +9,7 @@ import type { PackageVersionInfo, } from '#shared/types' import type { ReleaseType } from 'semver' +import { mapWithConcurrency } from '#shared/utils/async' import { maxSatisfying, prerelease, major, minor, diff, gt, compare } from 'semver' import { isExactVersion } from '~/utils/versions' import { extractInstallScriptsInfo } from '~/utils/install-scripts' @@ -546,34 +547,28 @@ export function useOrgPackages(orgName: MaybeRefOrGetter) { // Fetch packuments and downloads in parallel const [packuments, downloads] = await Promise.all([ - // Fetch packuments in parallel (with concurrency limit) + // Fetch packuments with concurrency limit (async () => { - const concurrency = 10 - const results: MinimalPackument[] = [] - for (let i = 0; i < packageNames.length; i += concurrency) { - const batch = packageNames.slice(i, i + concurrency) - const batchResults = await Promise.all( - batch.map(async name => { - try { - const encoded = encodePackageName(name) - const { data: pkg } = await cachedFetch( - `${NPM_REGISTRY}/${encoded}`, - { signal }, - ) - return pkg - } catch { - return null - } - }), - ) - for (const pkg of batchResults) { - // Filter out any unpublished packages (missing dist-tags) - if (pkg && pkg['dist-tags']) { - results.push(pkg) + const results = await mapWithConcurrency( + packageNames, + async name => { + try { + const encoded = encodePackageName(name) + const { data: pkg } = await cachedFetch( + `${NPM_REGISTRY}/${encoded}`, + { signal }, + ) + return pkg + } catch { + return null } - } - } - return results + }, + 10, + ) + // Filter out any unpublished packages (missing dist-tags) + return results.filter( + (pkg): pkg is MinimalPackument => pkg !== null && !!pkg['dist-tags'], + ) })(), // Fetch downloads in bulk fetchBulkDownloads(packageNames, { signal }), @@ -772,23 +767,20 @@ export function useOutdatedDependencies( return } - const results: Record = {} const entries = Object.entries(deps) - const batchSize = 5 - - for (let i = 0; i < entries.length; i += batchSize) { - const batch = entries.slice(i, i + batchSize) - const batchResults = await Promise.all( - batch.map(async ([name, constraint]) => { - const info = await checkDependencyOutdated(cachedFetch, name, constraint) - return [name, info] as const - }), - ) + const batchResults = await mapWithConcurrency( + entries, + async ([name, constraint]) => { + const info = await checkDependencyOutdated(cachedFetch, name, constraint) + return [name, info] as const + }, + 5, + ) - for (const [name, info] of batchResults) { - if (info) { - results[name] = info - } + const results: Record = {} + for (const [name, info] of batchResults) { + if (info) { + results[name] = info } } diff --git a/server/utils/dependency-analysis.ts b/server/utils/dependency-analysis.ts index b60bdc7742..fcd109c1bf 100644 --- a/server/utils/dependency-analysis.ts +++ b/server/utils/dependency-analysis.ts @@ -9,8 +9,12 @@ import type { VulnerabilityTreeResult, DeprecatedPackageInfo, } from '#shared/types/dependency-analysis' +import { mapWithConcurrency } from '#shared/utils/async' import { resolveDependencyTree } from './dependency-resolver' +/** Maximum concurrent requests for fetching vulnerability details */ +const OSV_DETAIL_CONCURRENCY = 25 + /** Package info needed for OSV queries */ interface PackageQueryInfo { name: string @@ -47,6 +51,15 @@ async function queryOsvBatch( if (result?.vulns && result.vulns.length > 0) { vulnerableIndices.push(i) } + // Warn if pagination token present (>1000 vulns for single query or >3000 total) + // This is extremely unlikely for npm packages but log for visibility + if (result?.next_page_token) { + // oxlint-disable-next-line no-console -- warn about paginated results + console.warn( + `[dep-analysis] OSV batch result has pagination token for package index ${i} ` + + `(${packages[i]?.name}@${packages[i]?.version}) - some vulnerabilities may be missing`, + ) + } } return { vulnerableIndices, failed: false } @@ -199,10 +212,10 @@ export const analyzeDependencyTree = defineCachedFunction( if (!batchFailed && vulnerableIndices.length > 0) { // Step 2: Fetch full vulnerability details only for packages with vulns // This is typically a small fraction of total packages - const vulnerablePackageInfos = vulnerableIndices.map(i => packages[i]!) - - const detailResults = await Promise.all( - vulnerablePackageInfos.map(pkg => queryOsvDetails(pkg)), + const detailResults = await mapWithConcurrency( + vulnerableIndices, + i => queryOsvDetails(packages[i]!), + OSV_DETAIL_CONCURRENCY, ) for (const result of detailResults) { diff --git a/server/utils/dependency-resolver.ts b/server/utils/dependency-resolver.ts index 47eaa559bc..1b007e636c 100644 --- a/server/utils/dependency-resolver.ts +++ b/server/utils/dependency-resolver.ts @@ -1,6 +1,10 @@ import type { Packument, PackumentVersion, DependencyDepth } from '#shared/types' +import { mapWithConcurrency } from '#shared/utils/async' import { maxSatisfying } from 'semver' +/** Concurrency limit for fetching packuments during dependency resolution */ +const PACKUMENT_FETCH_CONCURRENCY = 20 + /** * Target platform for dependency resolution. * We resolve for linux-x64 with glibc as a representative platform. @@ -142,63 +146,61 @@ export async function resolveDependencyTree( seen.add(name) } - // Process current level in batches + // Process current level with concurrency limit const entries = [...currentLevel.entries()] - for (let i = 0; i < entries.length; i += 20) { - const batch = entries.slice(i, i + 20) - - await Promise.all( - batch.map(async ([name, { range, optional, path }]) => { - const packument = await fetchPackument(name) - if (!packument) return + await mapWithConcurrency( + entries, + async ([name, { range, optional, path }]) => { + const packument = await fetchPackument(name) + if (!packument) return - const versions = Object.keys(packument.versions) - const version = resolveVersion(range, versions) - if (!version) return + const versions = Object.keys(packument.versions) + const version = resolveVersion(range, versions) + if (!version) return - const versionData = packument.versions[version] - if (!versionData) return + const versionData = packument.versions[version] + if (!versionData) return - if (!matchesPlatform(versionData)) return + if (!matchesPlatform(versionData)) return - const size = (versionData.dist as { unpackedSize?: number })?.unpackedSize ?? 0 - const key = `${name}@${version}` + const size = (versionData.dist as { unpackedSize?: number })?.unpackedSize ?? 0 + const key = `${name}@${version}` - // Build path for this package (path to parent + this package with version) - const currentPath = [...path, `${name}@${version}`] + // Build path for this package (path to parent + this package with version) + const currentPath = [...path, `${name}@${version}`] - if (!resolved.has(key)) { - const pkg: ResolvedPackage = { name, version, size, optional } - if (options.trackDepth) { - pkg.depth = level === 0 ? 'root' : level === 1 ? 'direct' : 'transitive' - pkg.path = currentPath - } - if (versionData.deprecated) { - pkg.deprecated = versionData.deprecated - } - resolved.set(key, pkg) + if (!resolved.has(key)) { + const pkg: ResolvedPackage = { name, version, size, optional } + if (options.trackDepth) { + pkg.depth = level === 0 ? 'root' : level === 1 ? 'direct' : 'transitive' + pkg.path = currentPath } - - // Collect dependencies for next level - if (versionData.dependencies) { - for (const [depName, depRange] of Object.entries(versionData.dependencies)) { - if (!seen.has(depName) && !nextLevel.has(depName)) { - nextLevel.set(depName, { range: depRange, optional: false, path: currentPath }) - } + if (versionData.deprecated) { + pkg.deprecated = versionData.deprecated + } + resolved.set(key, pkg) + } + + // Collect dependencies for next level + if (versionData.dependencies) { + for (const [depName, depRange] of Object.entries(versionData.dependencies)) { + if (!seen.has(depName) && !nextLevel.has(depName)) { + nextLevel.set(depName, { range: depRange, optional: false, path: currentPath }) } } + } - // Collect optional dependencies - if (versionData.optionalDependencies) { - for (const [depName, depRange] of Object.entries(versionData.optionalDependencies)) { - if (!seen.has(depName) && !nextLevel.has(depName)) { - nextLevel.set(depName, { range: depRange, optional: true, path: currentPath }) - } + // Collect optional dependencies + if (versionData.optionalDependencies) { + for (const [depName, depRange] of Object.entries(versionData.optionalDependencies)) { + if (!seen.has(depName) && !nextLevel.has(depName)) { + nextLevel.set(depName, { range: depRange, optional: true, path: currentPath }) } } - }), - ) - } + } + }, + PACKUMENT_FETCH_CONCURRENCY, + ) currentLevel = nextLevel level++ diff --git a/shared/utils/async.ts b/shared/utils/async.ts new file mode 100644 index 0000000000..b7cf051d08 --- /dev/null +++ b/shared/utils/async.ts @@ -0,0 +1,37 @@ +/** + * Async utilities for controlled concurrency and parallel execution. + */ + +/** + * Map over an array with limited concurrency. + * Similar to Promise.all but limits how many promises run simultaneously. + * + * @param items - Array of items to process + * @param fn - Async function to apply to each item + * @param concurrency - Maximum number of concurrent operations (default: 10) + * @returns Array of results in the same order as input + * + * @example + * const results = await mapWithConcurrency(urls, fetchUrl, 5) + */ +export async function mapWithConcurrency( + items: T[], + fn: (item: T, index: number) => Promise, + concurrency = 10, +): Promise { + const results: R[] = Array.from({ length: items.length }) as R[] + let currentIndex = 0 + + async function worker(): Promise { + while (currentIndex < items.length) { + const index = currentIndex++ + results[index] = await fn(items[index]!, index) + } + } + + // Start workers up to concurrency limit + const workers = Array.from({ length: Math.min(concurrency, items.length) }, () => worker()) + await Promise.all(workers) + + return results +} diff --git a/test/unit/shared/utils/async.spec.ts b/test/unit/shared/utils/async.spec.ts new file mode 100644 index 0000000000..227460781c --- /dev/null +++ b/test/unit/shared/utils/async.spec.ts @@ -0,0 +1,95 @@ +import { describe, expect, it, vi } from 'vitest' +import { mapWithConcurrency } from '../../../../shared/utils/async' + +describe('mapWithConcurrency', () => { + it('processes all items and returns results in order', async () => { + const items = [1, 2, 3, 4, 5] + const results = await mapWithConcurrency(items, async x => x * 2) + + expect(results).toEqual([2, 4, 6, 8, 10]) + }) + + it('respects concurrency limit', async () => { + let concurrent = 0 + let maxConcurrent = 0 + + const items = Array.from({ length: 10 }, (_, i) => i) + + await mapWithConcurrency( + items, + async () => { + concurrent++ + maxConcurrent = Math.max(maxConcurrent, concurrent) + await new Promise(resolve => setTimeout(resolve, 10)) + concurrent-- + }, + 3, + ) + + expect(maxConcurrent).toBe(3) + }) + + it('handles empty array', async () => { + const results = await mapWithConcurrency([], async x => x) + expect(results).toEqual([]) + }) + + it('handles single item', async () => { + const results = await mapWithConcurrency([42], async x => x * 2) + expect(results).toEqual([84]) + }) + + it('passes index to callback', async () => { + const items = ['a', 'b', 'c'] + const results = await mapWithConcurrency(items, async (item, index) => `${item}${index}`) + + expect(results).toEqual(['a0', 'b1', 'c2']) + }) + + it('propagates errors', async () => { + const items = [1, 2, 3] + const fn = vi.fn(async (x: number) => { + if (x === 2) throw new Error('test error') + return x + }) + + await expect(mapWithConcurrency(items, fn)).rejects.toThrow('test error') + }) + + it('uses default concurrency of 10', async () => { + let concurrent = 0 + let maxConcurrent = 0 + + const items = Array.from({ length: 20 }, (_, i) => i) + + await mapWithConcurrency(items, async () => { + concurrent++ + maxConcurrent = Math.max(maxConcurrent, concurrent) + await new Promise(resolve => setTimeout(resolve, 5)) + concurrent-- + }) + + expect(maxConcurrent).toBe(10) + }) + + it('handles fewer items than concurrency limit', async () => { + let concurrent = 0 + let maxConcurrent = 0 + + const items = [1, 2, 3] + + await mapWithConcurrency( + items, + async () => { + concurrent++ + maxConcurrent = Math.max(maxConcurrent, concurrent) + await new Promise(resolve => setTimeout(resolve, 10)) + concurrent-- + }, + 10, + ) + + // Should only have 3 concurrent since we only have 3 items + expect(maxConcurrent).toBe(3) + }) +})