diff --git a/README.md b/README.md index a67d5cb..9c99cd1 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,8 @@ - [Overview](#overview) - [Viewing the results](#viewing-the-results) - [Installation](#installation) - - [Installation (standard)](#installation-standard) - - [Installation (GitHub Enterprise Server)](#installation-github-enterprise-server) + - [Installation (standard)](#installation-standard) + - [Installation (GitHub Enterprise Server)](#installation-github-enterprise-server) - [Configuration](#configuration) - [Configuration options](#configuration-options) - [Configuration methods](#configuration-methods) @@ -130,6 +130,7 @@ All configuration options are optional. | `warn-only`+ | When set to `true`, the action will log all vulnerabilities as warnings regardless of the severity, and the action will complete with a `success` status. This overrides the `fail-on-severity` option. | `true`, `false` | `false` | | `show-openssf-scorecard` | When set to `true`, the action will output information about all the known OpenSSF Scorecard scores for the dependencies changed in this pull request. | `true`, `false` | `true` | | `warn-on-openssf-scorecard-level` | When `show-openssf-scorecard-levels` is set to `true`, this option lets you configure the threshold for when a score is considered too low and gets a :warning: warning in the CI. | Any positive integer | 3 | +| `show-patched-versions`\* | When set to `true`, the vulnerability summary table will include an additional column showing the first patched version for each vulnerability. This requires additional API calls to fetch advisory data. | `true`, `false` | `false` | > [!NOTE] > @@ -215,6 +216,7 @@ You can use an external configuration file to specify settings for this action. 3. Create the configuration file in the path you specified for `config-file`. 4. In the configuration file, specify your chosen settings. + ```yaml fail-on-severity: 'critical' allow-licenses: diff --git a/__tests__/summary.test.ts b/__tests__/summary.test.ts index fdfd758..eec1f4b 100644 --- a/__tests__/summary.test.ts +++ b/__tests__/summary.test.ts @@ -1,12 +1,25 @@ -import {expect, jest, test} from '@jest/globals' +import {expect, jest, test, beforeEach} from '@jest/globals' import {Changes, ConfigurationOptions, Scorecard} from '../src/schemas' import * as summary from '../src/summary' import * as core from '@actions/core' import {createTestChange} from './fixtures/create-test-change' import {createTestVulnerability} from './fixtures/create-test-vulnerability' +import * as utils from '../src/utils' + +const mockOctokitRequest = jest.fn() + +beforeEach(() => { + jest.spyOn(utils, 'octokitClient').mockReturnValue({ + request: mockOctokitRequest + } as any) + + mockOctokitRequest.mockResolvedValue({ + data: {vulnerabilities: []} + }) +}) afterEach(() => { - jest.clearAllMocks() + jest.restoreAllMocks() core.summary.emptyBuffer() }) @@ -34,7 +47,8 @@ const defaultConfig: ConfigurationOptions = { retry_on_snapshot_warnings_timeout: 120, warn_only: false, warn_on_openssf_scorecard_level: 3, - show_openssf_scorecard: false + show_openssf_scorecard: false, + show_patched_versions: false } const changesWithEmptyManifests: Changes = [ @@ -315,19 +329,19 @@ test('uses checkmarks for vulnerabilities if only license issues were found', () expect(text).toContain('✅ 0 package(s) with unknown licenses') }) -test('addChangeVulnerabilitiesToSummary() - only includes section if any vulnerabilities found', () => { - summary.addChangeVulnerabilitiesToSummary(emptyChanges, 'low') +test('addChangeVulnerabilitiesToSummary() - only includes section if any vulnerabilities found', async () => { + await summary.addChangeVulnerabilitiesToSummary(emptyChanges, 'low') const text = core.summary.stringify() expect(text).toEqual('') }) -test('addChangeVulnerabilitiesToSummary() - includes all vulnerabilities', () => { +test('addChangeVulnerabilitiesToSummary() - includes all vulnerabilities', async () => { const changes = [ createTestChange({name: 'lodash'}), createTestChange({name: 'underscore', package_url: 'test-url'}) ] - summary.addChangeVulnerabilitiesToSummary(changes, 'low') + await summary.addChangeVulnerabilitiesToSummary(changes, 'low') const text = core.summary.stringify() expect(text).toContain('

Vulnerabilities

') @@ -335,7 +349,7 @@ test('addChangeVulnerabilitiesToSummary() - includes all vulnerabilities', () => expect(text).toContain('underscore') }) -test('addChangeVulnerabilitiesToSummary() - includes advisory url if available', () => { +test('addChangeVulnerabilitiesToSummary() - includes advisory url if available', async () => { const changes = [ createTestChange({ name: 'underscore', @@ -348,14 +362,14 @@ test('addChangeVulnerabilitiesToSummary() - includes advisory url if available', }) ] - summary.addChangeVulnerabilitiesToSummary(changes, 'low') + await summary.addChangeVulnerabilitiesToSummary(changes, 'low') const text = core.summary.stringify() expect(text).toContain('lodash') expect(text).toContain('test-summary') }) -test('addChangeVulnerabilitiesToSummary() - groups vulnerabilities of a single package', () => { +test('addChangeVulnerabilitiesToSummary() - groups vulnerabilities of a single package', async () => { const changes = [ createTestChange({ name: 'package-with-multiple-vulnerabilities', @@ -366,7 +380,7 @@ test('addChangeVulnerabilitiesToSummary() - groups vulnerabilities of a single p }) ] - summary.addChangeVulnerabilitiesToSummary(changes, 'low') + await summary.addChangeVulnerabilitiesToSummary(changes, 'low') const text = core.summary.stringify() expect(text.match('package-with-multiple-vulnerabilities')).toHaveLength(1) @@ -374,10 +388,10 @@ test('addChangeVulnerabilitiesToSummary() - groups vulnerabilities of a single p expect(text).toContain('test-summary-2') }) -test('addChangeVulnerabilitiesToSummary() - prints severity statement if above low', () => { +test('addChangeVulnerabilitiesToSummary() - prints severity statement if above low', async () => { const changes = [createTestChange()] - summary.addChangeVulnerabilitiesToSummary(changes, 'medium') + await summary.addChangeVulnerabilitiesToSummary(changes, 'medium') const text = core.summary.stringify() expect(text).toContain( @@ -385,15 +399,79 @@ test('addChangeVulnerabilitiesToSummary() - prints severity statement if above l ) }) -test('addChangeVulnerabilitiesToSummary() - does not print severity statement if it is set to "low"', () => { +test('addChangeVulnerabilitiesToSummary() - does not print severity statement if it is set to "low"', async () => { const changes = [createTestChange()] - summary.addChangeVulnerabilitiesToSummary(changes, 'low') + await summary.addChangeVulnerabilitiesToSummary(changes, 'low') const text = core.summary.stringify() expect(text).not.toContain('Only included vulnerabilities') }) +test('addChangeVulnerabilitiesToSummary() - does not include patched version column by default', async () => { + const changes = [createTestChange()] + + await summary.addChangeVulnerabilitiesToSummary(changes, 'low') + + const text = core.summary.stringify() + expect(text).not.toContain('Patched Version') +}) + +test('addChangeVulnerabilitiesToSummary() - includes patched version column when enabled', async () => { + const changes = [createTestChange()] + + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', true) + + const text = core.summary.stringify() + expect(text).toContain('Patched Version') +}) + +test('addChangeVulnerabilitiesToSummary() - skips patched version on GHES even when enabled', async () => { + const originalUrl = process.env.GITHUB_SERVER_URL + process.env.GITHUB_SERVER_URL = 'https://ghes.example.com' + const warnSpy = jest.spyOn(core, 'warning') + + const changes = [createTestChange()] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', true) + + const text = core.summary.stringify() + expect(text).not.toContain('Patched Version') + expect(warnSpy).toHaveBeenCalledWith( + 'show-patched-versions is not supported on GitHub Enterprise Server. The Patched Version column will be omitted.' + ) + expect(mockOctokitRequest).not.toHaveBeenCalled() + + process.env.GITHUB_SERVER_URL = originalUrl +}) + +test('addChangeVulnerabilitiesToSummary() - works normally on GHES when patched versions disabled', async () => { + const originalUrl = process.env.GITHUB_SERVER_URL + process.env.GITHUB_SERVER_URL = 'https://ghes.example.com' + + const changes = [createTestChange()] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', false) + + const text = core.summary.stringify() + expect(text).not.toContain('Patched Version') + expect(mockOctokitRequest).not.toHaveBeenCalled() + + process.env.GITHUB_SERVER_URL = originalUrl +}) + +test('addChangeVulnerabilitiesToSummary() - works normally on GHES with default (no third arg)', async () => { + const originalUrl = process.env.GITHUB_SERVER_URL + process.env.GITHUB_SERVER_URL = 'https://ghes.example.com' + + const changes = [createTestChange()] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low') + + const text = core.summary.stringify() + expect(text).not.toContain('Patched Version') + expect(mockOctokitRequest).not.toHaveBeenCalled() + + process.env.GITHUB_SERVER_URL = originalUrl +}) + test('addLicensesToSummary() - does not include entire section if no license issues found', () => { summary.addLicensesToSummary(emptyInvalidLicenseChanges, defaultConfig) const text = core.summary.stringify() @@ -508,3 +586,293 @@ test('addLicensesToSummary() - includes allowed dependency licences', () => { '
Excluded from license check: MIT, Apache-2.0
' ) }) + +test('addChangeVulnerabilitiesToSummary() - handles multiple version ranges for same package', async () => { + // Simulates GHSA-gwq6-fmvp-qp68 scenario with multiple version ranges + const pkg8 = createTestChange({ + ecosystem: 'nuget', + name: 'Microsoft.NetCore.App.Runtime.linux-arm', + version: '8.0.1', + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: 'GHSA-test-multi', + advisory_summary: 'Test Multi-Range Advisory', + severity: 'high' + }) + ] + }) + + const pkg9 = createTestChange({ + ecosystem: 'nuget', + name: 'Microsoft.NetCore.App.Runtime.linux-arm', + version: '9.0.1', + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: 'GHSA-test-multi', + advisory_summary: 'Test Multi-Range Advisory', + severity: 'high' + }) + ] + }) + + // Mock API response with multiple version ranges for same package + mockOctokitRequest.mockResolvedValueOnce({ + data: { + vulnerabilities: [ + { + package: { + ecosystem: 'NuGet', + name: 'Microsoft.NetCore.App.Runtime.linux-arm' + }, + vulnerable_version_range: '>= 8.0.0, <= 8.0.20', + first_patched_version: '8.0.21' + }, + { + package: { + ecosystem: 'NuGet', + name: 'Microsoft.NetCore.App.Runtime.linux-arm' + }, + vulnerable_version_range: '>= 9.0.0, <= 9.0.9', + first_patched_version: '9.0.10' + } + ] + } + }) + + const changes = [pkg8, pkg9] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', true) + + const text = core.summary.stringify() + + // Both packages should have correct patched versions based on their version ranges + expect(text).toContain('8.0.21') + expect(text).toContain('9.0.10') + expect(mockOctokitRequest).toHaveBeenCalledWith('GET /advisories/{ghsa_id}', { + ghsa_id: 'GHSA-test-multi' + }) +}) + +test('addChangeVulnerabilitiesToSummary() - handles RestSharp GHSA-4rr6-2v9v-wcpc case', async () => { + const pkg = createTestChange({ + ecosystem: 'nuget', + name: 'RestSharp', + version: '111.4.1', + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: 'GHSA-4rr6-2v9v-wcpc', + advisory_summary: + "CRLF Injection in RestSharp's `RestRequest.AddHeader` method", + severity: 'moderate' + }) + ] + }) + + // Mock API response matching actual GitHub Advisory Database response + mockOctokitRequest.mockResolvedValueOnce({ + data: { + vulnerabilities: [ + { + package: { + ecosystem: 'nuget', + name: 'RestSharp' + }, + vulnerable_version_range: '>= 107.0.0-preview.1, < 112.0.0', + first_patched_version: '112.0.0' + } + ] + } + }) + + const changes = [pkg] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', true) + + const text = core.summary.stringify() + + // Should show the correct patched version + expect(text).toContain('112.0.0') + expect(text).not.toContain('N/A') + expect(mockOctokitRequest).toHaveBeenCalledWith('GET /advisories/{ghsa_id}', { + ghsa_id: 'GHSA-4rr6-2v9v-wcpc' + }) +}) + +test('addChangeVulnerabilitiesToSummary() - handles version coercion for non-strict semver versions', async () => { + // Test that versions like "8.0" (without patch version) can be coerced to "8.0.0" + // for successful range matching in fail-open mode (patch selection) + const pkg = createTestChange({ + ecosystem: 'npm', + name: 'test-package', + version: '8.0', // Non-strict semver version + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: 'GHSA-test-1234', + advisory_summary: 'Test vulnerability', + severity: 'high' + }) + ] + }) + + mockOctokitRequest.mockResolvedValueOnce({ + data: { + vulnerabilities: [ + { + package: { + ecosystem: 'npm', + name: 'test-package' + }, + vulnerable_version_range: '>= 8.0.0, < 9.0.0', + first_patched_version: '9.0.0' + } + ] + } + }) + + const changes = [pkg] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', true) + + const text = core.summary.stringify() + + // Should coerce "8.0" to "8.0.0" and successfully match the range, + // showing the patched version instead of N/A + expect(text).toContain('9.0.0') + expect(text).not.toContain('N/A') +}) + +test('addChangeVulnerabilitiesToSummary() - handles invalid versions in fail-open mode', async () => { + // Test that completely invalid versions that can't be coerced + // still return N/A gracefully in fail-open mode + const pkg = createTestChange({ + ecosystem: 'npm', + name: 'test-package', + version: 'invalid-version-string', + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: 'GHSA-test-5678', + advisory_summary: 'Test vulnerability', + severity: 'high' + }) + ] + }) + + mockOctokitRequest.mockResolvedValueOnce({ + data: { + vulnerabilities: [ + { + package: { + ecosystem: 'npm', + name: 'test-package' + }, + vulnerable_version_range: '>= 1.0.0, < 2.0.0', + first_patched_version: '2.0.0' + } + ] + } + }) + + const changes = [pkg] + await summary.addChangeVulnerabilitiesToSummary(changes, 'low', true) + + const text = core.summary.stringify() + + // Should show N/A since version can't be coerced or matched + expect(text).toContain('N/A') +}) + +test('addChangeVulnerabilitiesToSummary() - respects concurrency limit for API calls', async () => { + // Create 15 packages with different vulnerabilities to test concurrency limiting + const packages = Array.from({length: 15}, (_, i) => + createTestChange({ + ecosystem: 'npm', + name: `package-${i}`, + version: '1.0.0', + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: `GHSA-test-${i.toString().padStart(4, '0')}`, + advisory_summary: `Vulnerability ${i}`, + severity: 'high' + }) + ] + }) + ) + + // Track concurrent calls + let maxConcurrent = 0 + let currentConcurrent = 0 + + mockOctokitRequest.mockImplementation(async () => { + currentConcurrent++ + maxConcurrent = Math.max(maxConcurrent, currentConcurrent) + + // Simulate async API call with a small deterministic delay + await new Promise(resolve => setTimeout(resolve, 5)) + + currentConcurrent-- + + return { + data: { + vulnerabilities: [ + { + package: {ecosystem: 'npm', name: 'test'}, + vulnerable_version_range: '>= 1.0.0, < 2.0.0', + first_patched_version: '2.0.0' + } + ] + } + } + }) + + await summary.addChangeVulnerabilitiesToSummary(packages, 'low', true) + + // Verify that concurrency limit (10) was respected + expect(maxConcurrent).toBeLessThanOrEqual(10) + // Verify all 15 unique advisories were fetched + expect(mockOctokitRequest).toHaveBeenCalledTimes(15) +}) + +test('addChangeVulnerabilitiesToSummary() - completes all tasks even with varying durations', async () => { + // Test that promise pool doesn't lose tasks when some complete faster than others + const packages = Array.from({length: 20}, (_, i) => + createTestChange({ + ecosystem: 'npm', + name: `package-${i}`, + version: '1.0.0', + vulnerabilities: [ + createTestVulnerability({ + advisory_ghsa_id: `GHSA-vary-${i.toString().padStart(4, '0')}`, + advisory_summary: `Vulnerability ${i}`, + severity: 'high' + }) + ] + }) + ) + + const completedAdvisories = new Set() + + mockOctokitRequest.mockImplementation( + async (path: string, params: {ghsa_id: string}) => { + // Variable delay to simulate real-world API response times + const delay = Math.random() * 50 + await new Promise(resolve => setTimeout(resolve, delay)) + + completedAdvisories.add(params.ghsa_id) + + return { + data: { + vulnerabilities: [ + { + package: {ecosystem: 'npm', name: 'test'}, + vulnerable_version_range: '>= 1.0.0, < 2.0.0', + first_patched_version: '2.0.0' + } + ] + } + } + } + ) + + await summary.addChangeVulnerabilitiesToSummary(packages, 'low', true) + + // Verify all 20 unique advisories were fetched and completed + expect(completedAdvisories.size).toBe(20) + expect(mockOctokitRequest).toHaveBeenCalledTimes(20) +}) diff --git a/action.yml b/action.yml index f62ad1e..9571681 100644 --- a/action.yml +++ b/action.yml @@ -76,6 +76,9 @@ inputs: warn-on-openssf-scorecard-level: description: Numeric threshold for the OpenSSF Scorecard score. If the score is below this threshold, the action will warn you. required: false + show-patched-versions: + description: When set to `true`, the vulnerability summary table will include a column showing the first patched version for each vulnerability. + required: false outputs: comment-content: description: Prepared dependency report comment diff --git a/package-lock.json b/package-lock.json index b35cdc6..243c8ff 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,7 @@ "got": "^14.4.7", "jest": "^29.7.0", "octokit": "^3.1.2", + "semver": "^7.7.4", "spdx-expression-parse": "^4.0.0", "spdx-satisfies": "^6.0.0", "ts-jest": "^29.4.1", @@ -8285,9 +8286,9 @@ } }, "node_modules/semver": { - "version": "7.7.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.2.tgz", - "integrity": "sha512-RF0Fw+rO5AMf9MAyaRXI4AV0Ulj5lMHqVxxdSgiVbixSCXoEmmX/jk0CuJw4+3SqroYO9VoUh+HcuJivvtJemA==", + "version": "7.7.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.4.tgz", + "integrity": "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA==", "license": "ISC", "bin": { "semver": "bin/semver.js" diff --git a/package.json b/package.json index 15903bd..9e61ba9 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "got": "^14.4.7", "jest": "^29.7.0", "octokit": "^3.1.2", + "semver": "^7.7.4", "spdx-expression-parse": "^4.0.0", "spdx-satisfies": "^6.0.0", "ts-jest": "^29.4.1", diff --git a/src/config.ts b/src/config.ts index b2198ff..9bba050 100644 --- a/src/config.ts +++ b/src/config.ts @@ -52,6 +52,7 @@ function readInlineConfig(): ConfigurationOptionsPartial { const warn_on_openssf_scorecard_level = getOptionalNumber( 'warn-on-openssf-scorecard-level' ) + const show_patched_versions = getOptionalBoolean('show-patched-versions') validateLicenses('allow-licenses', allow_licenses) validateLicenses('deny-licenses', deny_licenses) @@ -74,7 +75,8 @@ function readInlineConfig(): ConfigurationOptionsPartial { retry_on_snapshot_warnings_timeout, warn_only, show_openssf_scorecard, - warn_on_openssf_scorecard_level + warn_on_openssf_scorecard_level, + show_patched_versions } return Object.fromEntries( diff --git a/src/main.ts b/src/main.ts index cc74c72..0f64554 100644 --- a/src/main.ts +++ b/src/main.ts @@ -205,7 +205,11 @@ async function run(): Promise { if (config.vulnerability_check) { core.setOutput('vulnerable-changes', JSON.stringify(vulnerableChanges)) - summary.addChangeVulnerabilitiesToSummary(vulnerableChanges, minSeverity) + await summary.addChangeVulnerabilitiesToSummary( + vulnerableChanges, + minSeverity, + config.show_patched_versions + ) issueFound ||= await printVulnerabilitiesBlock( vulnerableChanges, minSeverity, diff --git a/src/schemas.ts b/src/schemas.ts index d1f81d4..077377d 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -115,6 +115,7 @@ export const ConfigurationOptionsSchema = z retry_on_snapshot_warnings_timeout: z.number().default(120), show_openssf_scorecard: z.boolean().optional().default(true), warn_on_openssf_scorecard_level: z.number().default(3), + show_patched_versions: z.boolean().default(false), comment_summary_in_pr: z .union([ z.preprocess( diff --git a/src/summary.ts b/src/summary.ts index a49cf4e..892b38e 100644 --- a/src/summary.ts +++ b/src/summary.ts @@ -2,7 +2,14 @@ import * as core from '@actions/core' import {SummaryTableRow} from '@actions/core/lib/summary' import {InvalidLicenseChanges, InvalidLicenseChangeTypes} from './licenses' import {Change, Changes, ConfigurationOptions, Scorecard} from './schemas' -import {groupDependenciesByManifest, getManifestsSet, renderUrl} from './utils' +import { + groupDependenciesByManifest, + getManifestsSet, + renderUrl, + octokitClient, + isEnterprise +} from './utils' +import * as semver from 'semver' const icons = { check: '✅', @@ -11,6 +18,109 @@ const icons = { } const MAX_SCANNED_FILES_BYTES = 1048576 +const API_CONCURRENCY_LIMIT = 10 // Limit concurrent API requests to avoid rate limiting + +/** + * Helper to check if a version falls within a vulnerable range. + * Uses the `semver` library for proper prerelease handling and range parsing. + * + * @param version - The version to check (can be pre-trimmed). + * @param range - The version range to check against (can be pre-trimmed and/or pre-normalized). + * @param options - Configuration options. + * @param options.preTrimmed - If true, assumes inputs are already trimmed (optimization). + * @param options.preNormalized - If true, assumes range is already normalized (comma-to-space conversion done). + * @param options.failClosed - If true, returns true (vulnerable) on errors; if false, returns false (no match). + * @returns `true` if the version is considered within the vulnerable range (or on fail-closed), otherwise `false`. + */ +function versionInRange( + version: string | undefined, + range: string | undefined, + options: { + preTrimmed?: boolean + preNormalized?: boolean + failClosed?: boolean + } = {} +): boolean { + const {preTrimmed = false, preNormalized = false, failClosed = true} = options + + // Trim inputs if not pre-trimmed + const trimmedVersion = preTrimmed ? version : version?.trim() || '' + const trimmedRange = preTrimmed ? range : range?.trim() || '' + + if (!trimmedVersion) { + if (failClosed) { + core.debug( + `Empty or missing version for range "${range}". Treating as vulnerable (fail closed).` + ) + } + return failClosed + } + if (!trimmedRange) { + if (failClosed) { + core.debug( + `Empty or missing version range for version "${version}". Treating as vulnerable (fail closed).` + ) + } + return failClosed + } + + // Convert GitHub API range format to semver-compatible format if not already normalized + // GitHub uses: ">= 8.0.0, <= 8.0.20" + // Semver accepts: ">= 8.0.0 <= 8.0.20" (operators may be followed by a space) + const semverRange = preNormalized + ? trimmedRange + : trimmedRange.replace(/,\s*/g, ' ') + + // Validate version and range explicitly to enforce fail-closed semantics + // semver.satisfies() typically returns false for invalid inputs without throwing + let validVersion = semver.valid(trimmedVersion) + const validRange = semver.validRange(semverRange) + + // For fail-open mode (patch selection), try coercing invalid versions + // to handle common real-world formats like "8.0", date-based versions, etc. + if (!validVersion && !failClosed) { + const coerced = semver.coerce(trimmedVersion) + if (coerced) { + validVersion = coerced.version + core.debug( + `Coerced version "${trimmedVersion}" to "${validVersion}" for range matching` + ) + } + } + + if (!validVersion || !validRange) { + if (failClosed) { + const issues: string[] = [] + if (!validVersion) issues.push('version') + if (!validRange) issues.push('version range') + core.debug( + `Invalid ${issues.join(' and ')}: version="${version}", range="${range}". Treating as vulnerable (fail closed).` + ) + } + return failClosed + } + + // Both version and range are valid; perform the satisfies check + // Only include prereleases when the version being checked is itself a prerelease + // to avoid changing range semantics globally + const isPrerelease = semver.prerelease(validVersion) !== null + return semver.satisfies(validVersion, validRange, { + includePrerelease: isPrerelease + }) +} + +function extractPatchVersionId(patchData: unknown): string | null { + // Handle string format (current API response) + if (typeof patchData === 'string') return patchData + + // Handle object format with identifier field (for backward compatibility) + if (patchData && typeof patchData === 'object' && 'identifier' in patchData) { + const id = (patchData as {identifier: unknown}).identifier + return typeof id === 'string' ? id : null + } + + return null +} // generates the DR report summary and caches it to the Action's core.summary. // returns the DR summary string, ready to be posted as a PR comment if the @@ -132,21 +242,142 @@ function countScorecardWarnings( ) } -export function addChangeVulnerabilitiesToSummary( +/** + * Execute promises with a concurrency limit to avoid overwhelming APIs. + * @param tasks - Array of functions that return promises + * @param limit - Maximum number of concurrent promises + */ +async function promisePool( + tasks: (() => Promise)[], + limit: number +): Promise { + const executing: Set> = new Set() + + for (const task of tasks) { + // Execute task and clean up + const wrappedPromise = (async () => { + await task() + })() + + executing.add(wrappedPromise) + + // When promise completes, remove it from the executing set + wrappedPromise.finally(() => { + executing.delete(wrappedPromise) + }) + + // Wait if we've hit the concurrency limit + if (executing.size >= limit) { + await Promise.race(executing) + } + } + + // Wait for all remaining promises + await Promise.all(executing) +} + +export async function addChangeVulnerabilitiesToSummary( vulnerableChanges: Changes, - severity: string -): void { + severity: string, + showPatchedVersions = false +): Promise { if (vulnerableChanges.length === 0) { return } - const rows: SummaryTableRow[] = [] - const manifests = getManifestsSet(vulnerableChanges) + // Build set of unique advisories to query + const advisorySet = new Set() + if (showPatchedVersions) { + if (isEnterprise()) { + core.warning( + 'show-patched-versions is not supported on GitHub Enterprise Server. The Patched Version column will be omitted.' + ) + showPatchedVersions = false + } else { + for (const pkg of vulnerableChanges) { + for (const vuln of pkg.vulnerabilities) { + advisorySet.add(vuln.advisory_ghsa_id) + } + } + } + } + + // Query GitHub API for patch info with concurrency limiting + // Store all vulnerability entries (may be multiple per package with different ranges) + // Pre-normalize ecosystem, package name, and range to avoid repeated work in rendering + const patchInfo: Record< + string, + { + eco: string + pkg: string + range: string + patch: string + ecoLower: string + pkgLower: string + normalizedRange: string + }[] + > = {} + const apiClient = octokitClient() + + // Create tasks for promise pool + const tasks = Array.from(advisorySet).map(advId => async () => { + try { + core.debug(`Fetching advisory data for ${advId}`) + const apiResult = await apiClient.request('GET /advisories/{ghsa_id}', { + ghsa_id: advId + }) + + patchInfo[advId] = [] + const vulnList = apiResult.data.vulnerabilities || [] + core.debug(`Found ${vulnList.length} vulnerability entries for ${advId}`) + + for (const v of vulnList) { + if (v.package && v.package.ecosystem) { + const normalizedEco = v.package.ecosystem.toLowerCase() + const pkgName = v.package.name || '' + const vulnRange = v.vulnerable_version_range || '' + const patchVerId = extractPatchVersionId(v.first_patched_version) + if (patchVerId) { + // Pre-normalize and cache values to avoid repeated work in rendering loop + const trimmedRange = vulnRange.trim() + const normalizedRange = trimmedRange.replace(/,\s*/g, ' ') + patchInfo[advId].push({ + eco: normalizedEco, + pkg: pkgName, + range: vulnRange, + patch: patchVerId, + ecoLower: normalizedEco, // Ecosystem already normalized to lowercase + pkgLower: pkgName.toLowerCase(), + normalizedRange + }) + core.debug( + `Added patch info for ${pkgName} (${normalizedEco}): ${patchVerId} for range ${vulnRange}` + ) + } else { + core.debug( + `No patch version found for ${pkgName} (${normalizedEco}) in ${advId}` + ) + } + } + } + } catch (e) { + const errorMessage = e instanceof Error ? e.message : String(e) + core.debug(`API call failed for ${advId}: ${errorMessage}`) + patchInfo[advId] = [] + } + }) + + // Execute API calls with concurrency limit + await promisePool(tasks, API_CONCURRENCY_LIMIT) + core.summary.addHeading('Vulnerabilities', 2) for (const manifest of manifests) { + // Create fresh rows array for each manifest to avoid accumulation + const rows: SummaryTableRow[] = [] + for (const change of vulnerableChanges.filter( pkg => pkg.manifest === manifest )) { @@ -157,33 +388,100 @@ export function addChangeVulnerabilitiesToSummary( previous_package === change.name && previous_version === change.version + // Look up patch version by matching package name, ecosystem, and version range + let patchVer = 'N/A' + const advisoryEntries = patchInfo[vuln.advisory_ghsa_id] + if (advisoryEntries && advisoryEntries.length > 0) { + const ecoLowercase = change.ecosystem.toLowerCase() + const packageLowercase = change.name.toLowerCase() + const normalizedChangeVersion = change.version.trim() + core.debug( + `Looking up patch for ${change.name}@${change.version} (${ecoLowercase}) in ${vuln.advisory_ghsa_id}` + ) + + // Find matching entry by ecosystem, package name (case-insensitive), and version range + // Use pre-normalized values from cache to avoid repeated lowercasing and range conversion + let foundEntry: + | {eco: string; pkg: string; range: string; patch: string} + | undefined + for (const vulnEntry of advisoryEntries) { + if (vulnEntry.ecoLower !== ecoLowercase) continue + if (vulnEntry.pkgLower !== packageLowercase) continue + + // Use fail-open (failClosed: false) for patch selection to avoid + // incorrectly matching on invalid ranges + // Use preTrimmed and preNormalized optimizations since we've done both + const isInRange = versionInRange( + normalizedChangeVersion, + vulnEntry.normalizedRange, + {preTrimmed: true, preNormalized: true, failClosed: false} + ) + + if (isInRange) { + foundEntry = vulnEntry + break + } + } + + if (foundEntry) { + patchVer = foundEntry.patch + core.debug( + `Found patch version ${patchVer} for ${change.name}@${change.version}` + ) + } else { + const maxLoggedEntries = 5 + const entriesPreview = advisoryEntries + .slice(0, maxLoggedEntries) + .map( + entry => + `${entry.eco}:${entry.pkg} ${entry.range} -> ${entry.patch}` + ) + core.debug( + `No matching patch found for ${change.name}@${change.version}. Available entries (showing up to ${Math.min(advisoryEntries.length, maxLoggedEntries)} of ${advisoryEntries.length}): ${entriesPreview.join('; ')}` + ) + } + } else { + core.debug(`No advisory data available for ${vuln.advisory_ghsa_id}`) + } + if (!sameAsPrevious) { - rows.push([ + const row: SummaryTableRow = [ renderUrl(change.source_repository_url, change.name), change.version, renderUrl(vuln.advisory_url, vuln.advisory_summary), vuln.severity - ]) + ] + if (showPatchedVersions) { + row.push(patchVer) + } + rows.push(row) } else { - rows.push([ + const row: SummaryTableRow = [ {data: '', colspan: '2'}, renderUrl(vuln.advisory_url, vuln.advisory_summary), vuln.severity - ]) + ] + if (showPatchedVersions) { + row.push(patchVer) + } + rows.push(row) } previous_package = change.name previous_version = change.version } } - core.summary.addHeading(`${manifest}`, 4).addTable([ - [ - {data: 'Name', header: true}, - {data: 'Version', header: true}, - {data: 'Vulnerability', header: true}, - {data: 'Severity', header: true} - ], - ...rows - ]) + const headerRow: SummaryTableRow = [ + {data: 'Name', header: true}, + {data: 'Version', header: true}, + {data: 'Vulnerability', header: true}, + {data: 'Severity', header: true} + ] + if (showPatchedVersions) { + headerRow.push({data: 'Patched Version', header: true}) + } + core.summary + .addHeading(`${manifest}`, 4) + .addTable([headerRow, ...rows]) } if (severity !== 'low') { diff --git a/src/utils.ts b/src/utils.ts index 4646511..a3b6e70 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -33,7 +33,7 @@ export function renderUrl(url: string | null, text: string): string { } } -function isEnterprise(): boolean { +export function isEnterprise(): boolean { const serverUrl = new URL( process.env['GITHUB_SERVER_URL'] ?? 'https://github.com' )