From f9afe81eed78ead291dfd21a3a4af81142e81957 Mon Sep 17 00:00:00 2001 From: Allan Kimmer Jensen Date: Wed, 29 Apr 2026 13:54:29 +0200 Subject: [PATCH] fix(sbom): populate download location for git-sourced dependencies (#11329) * fix(sbom): populate download location for git-sourced dependencies * fix(sbom): avoid double git+ prefix when repo already includes it Address Copilot review on #11329: gitDownloadUrl() would produce git+git+ssh://... when GitResolution.repo already starts with git+. --------- Co-authored-by: Zoltan Kochan --- .changeset/sbom-git-download-location.md | 6 ++ deps/compliance/sbom/package.json | 1 + deps/compliance/sbom/src/collectComponents.ts | 9 ++- deps/compliance/sbom/src/index.ts | 2 +- .../sbom/test/gitDownloadUrl.test.ts | 62 +++++++++++++++++++ .../sbom/test/serializeCycloneDx.test.ts | 27 ++++++++ .../sbom/test/serializeSpdx.test.ts | 32 ++++++++++ deps/compliance/sbom/tsconfig.json | 3 + pnpm-lock.yaml | 3 + 9 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 .changeset/sbom-git-download-location.md create mode 100644 deps/compliance/sbom/test/gitDownloadUrl.test.ts diff --git a/.changeset/sbom-git-download-location.md b/.changeset/sbom-git-download-location.md new file mode 100644 index 0000000000..462829d9df --- /dev/null +++ b/.changeset/sbom-git-download-location.md @@ -0,0 +1,6 @@ +--- +"@pnpm/deps.compliance.sbom": patch +"pnpm": patch +--- + +Populate download location for git-sourced dependencies in SBOM output. Previously `pnpm sbom` emitted `NOASSERTION` (SPDX) and omitted the distribution reference (CycloneDX) for git dependencies. Now emits the git URL with commit hash, e.g. `git+https://github.com/user/repo.git#commit`. diff --git a/deps/compliance/sbom/package.json b/deps/compliance/sbom/package.json index 8c37afcd03..e00331c8da 100644 --- a/deps/compliance/sbom/package.json +++ b/deps/compliance/sbom/package.json @@ -41,6 +41,7 @@ "@pnpm/lockfile.utils": "workspace:*", "@pnpm/lockfile.walker": "workspace:*", "@pnpm/pkg-manifest.reader": "workspace:*", + "@pnpm/resolving.resolver-base": "workspace:*", "@pnpm/store.index": "workspace:*", "@pnpm/store.pkg-finder": "workspace:*", "@pnpm/types": "workspace:*", diff --git a/deps/compliance/sbom/src/collectComponents.ts b/deps/compliance/sbom/src/collectComponents.ts index 2ee6c6ad11..f2bd170e06 100644 --- a/deps/compliance/sbom/src/collectComponents.ts +++ b/deps/compliance/sbom/src/collectComponents.ts @@ -5,6 +5,7 @@ import { lockfileWalkerGroupImporterSteps, type LockfileWalkerStep, } from '@pnpm/lockfile.walker' +import type { Resolution } from '@pnpm/resolving.resolver-base' import { StoreIndex } from '@pnpm/store.index' import type { DependenciesField, ProjectId, Registries } from '@pnpm/types' @@ -110,7 +111,7 @@ async function walkStep ( const integrity = (pkgSnapshot.resolution as TarballResolution).integrity const resolution = pkgSnapshotToResolution(depPath, pkgSnapshot, opts.registries) - const tarballUrl = (resolution as TarballResolution).tarball + const tarballUrl = (resolution as TarballResolution).tarball ?? gitDownloadUrl(resolution) let metadata: { license?: string, description?: string, author?: string, homepage?: string, repository?: string } = {} if (metadataOpts) { @@ -136,3 +137,9 @@ async function walkStep ( ) } +export function gitDownloadUrl (resolution: Resolution): string | undefined { + if (resolution.type !== 'git') return undefined + const needsGitPlusPrefix = resolution.repo.includes('://') && !resolution.repo.startsWith('git+') + const prefix = needsGitPlusPrefix ? 'git+' : '' + return `${prefix}${resolution.repo}#${resolution.commit}` +} diff --git a/deps/compliance/sbom/src/index.ts b/deps/compliance/sbom/src/index.ts index 924460b357..9602e39980 100644 --- a/deps/compliance/sbom/src/index.ts +++ b/deps/compliance/sbom/src/index.ts @@ -1,4 +1,4 @@ -export { collectSbomComponents, type CollectSbomComponentsOptions } from './collectComponents.js' +export { collectSbomComponents, type CollectSbomComponentsOptions, gitDownloadUrl } from './collectComponents.js' export { integrityToHashes } from './integrity.js' export { buildPurl, encodePurlName } from './purl.js' export { type CycloneDxOptions, serializeCycloneDx } from './serializeCycloneDx.js' diff --git a/deps/compliance/sbom/test/gitDownloadUrl.test.ts b/deps/compliance/sbom/test/gitDownloadUrl.test.ts new file mode 100644 index 0000000000..7380728cbd --- /dev/null +++ b/deps/compliance/sbom/test/gitDownloadUrl.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from '@jest/globals' +import { gitDownloadUrl } from '@pnpm/deps.compliance.sbom' +import type { GitResolution, TarballResolution } from '@pnpm/resolving.resolver-base' + +describe('gitDownloadUrl', () => { + it('should construct git+https URL from HTTPS repo', () => { + const resolution: GitResolution = { + type: 'git', + repo: 'https://github.com/stevemao/left-pad.git', + commit: '2fca6157fcca165438e0f9495cf0e5a4e6f71349', + } + + expect(gitDownloadUrl(resolution)).toBe( + 'git+https://github.com/stevemao/left-pad.git#2fca6157fcca165438e0f9495cf0e5a4e6f71349' + ) + }) + + it('should construct git+ssh URL from SSH protocol repo', () => { + const resolution: GitResolution = { + type: 'git', + repo: 'ssh://git@github.com/user/repo.git', + commit: 'abc123', + } + + expect(gitDownloadUrl(resolution)).toBe( + 'git+ssh://git@github.com/user/repo.git#abc123' + ) + }) + + it('should not add git+ prefix for SCP-style SSH URLs', () => { + const resolution: GitResolution = { + type: 'git', + repo: 'git@github.com:user/repo.git', + commit: 'abc123', + } + + expect(gitDownloadUrl(resolution)).toBe( + 'git@github.com:user/repo.git#abc123' + ) + }) + + it('should not double-prefix when repo already starts with git+', () => { + const resolution: GitResolution = { + type: 'git', + repo: 'git+ssh://git@github.com/user/repo.git', + commit: 'abc123', + } + + expect(gitDownloadUrl(resolution)).toBe( + 'git+ssh://git@github.com/user/repo.git#abc123' + ) + }) + + it('should return undefined for non-git resolutions', () => { + const resolution: TarballResolution = { + tarball: 'https://registry.npmjs.org/express/-/express-4.22.1.tgz', + integrity: 'sha512-abc', + } + + expect(gitDownloadUrl(resolution)).toBeUndefined() + }) +}) diff --git a/deps/compliance/sbom/test/serializeCycloneDx.test.ts b/deps/compliance/sbom/test/serializeCycloneDx.test.ts index a96bc9df03..b2139d4db1 100644 --- a/deps/compliance/sbom/test/serializeCycloneDx.test.ts +++ b/deps/compliance/sbom/test/serializeCycloneDx.test.ts @@ -178,6 +178,33 @@ describe('serializeCycloneDx', () => { expect(distRef.hashes[0].content).toBeDefined() }) + it('should include distribution ref with git URL for git dependencies', () => { + const result = makeSbomResult() + result.components[0].tarballUrl = 'git+https://github.com/lodash/lodash.git#abc123' + result.components[0].integrity = undefined + const parsed = JSON.parse(serializeCycloneDx(result)) + + const lodash = parsed.components[0] + const distRef = lodash.externalReferences.find( + (r: { type: string }) => r.type === 'distribution' + ) + expect(distRef).toBeDefined() + expect(distRef.url).toBe('git+https://github.com/lodash/lodash.git#abc123') + expect(distRef.hashes).toBeUndefined() + }) + + it('should omit distribution ref when tarballUrl is absent', () => { + const result = makeSbomResult() + result.components[0].tarballUrl = undefined + const parsed = JSON.parse(serializeCycloneDx(result)) + + const lodash = parsed.components[0] + const distRef = lodash.externalReferences?.find( + (r: { type: string }) => r.type === 'distribution' + ) + expect(distRef).toBeUndefined() + }) + it('should use license.id for known SPDX identifiers', () => { const result = makeSbomResult() const parsed = JSON.parse(serializeCycloneDx(result)) diff --git a/deps/compliance/sbom/test/serializeSpdx.test.ts b/deps/compliance/sbom/test/serializeSpdx.test.ts index 93248d6069..262c5da3c0 100644 --- a/deps/compliance/sbom/test/serializeSpdx.test.ts +++ b/deps/compliance/sbom/test/serializeSpdx.test.ts @@ -181,6 +181,38 @@ describe('serializeSpdx', () => { expect(dependsOn).toHaveLength(1) }) + it('should use tarballUrl as downloadLocation for registry packages', () => { + const result = makeSbomResult() + result.components[0].tarballUrl = 'https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz' + const parsed = JSON.parse(serializeSpdx(result)) + + expect(parsed.packages[1].downloadLocation).toBe('https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz') + }) + + it('should use git URL as downloadLocation for git dependencies', () => { + const result = makeSbomResult() + result.components[0].tarballUrl = 'git+https://github.com/stevemao/left-pad.git#2fca6157' + const parsed = JSON.parse(serializeSpdx(result)) + + expect(parsed.packages[1].downloadLocation).toBe('git+https://github.com/stevemao/left-pad.git#2fca6157') + }) + + it('should preserve SCP-style SSH URLs without git+ prefix', () => { + const result = makeSbomResult() + result.components[0].tarballUrl = 'git@github.com:user/repo.git#abc123' + const parsed = JSON.parse(serializeSpdx(result)) + + expect(parsed.packages[1].downloadLocation).toBe('git@github.com:user/repo.git#abc123') + }) + + it('should use NOASSERTION when no downloadLocation is available', () => { + const result = makeSbomResult() + result.components[0].tarballUrl = undefined + const parsed = JSON.parse(serializeSpdx(result)) + + expect(parsed.packages[1].downloadLocation).toBe('NOASSERTION') + }) + it('should use APPLICATION for application root type', () => { const result = makeSbomResult() result.rootComponent.type = 'application' diff --git a/deps/compliance/sbom/tsconfig.json b/deps/compliance/sbom/tsconfig.json index 269bee0a95..6b92fdfc8a 100644 --- a/deps/compliance/sbom/tsconfig.json +++ b/deps/compliance/sbom/tsconfig.json @@ -30,6 +30,9 @@ { "path": "../../../pkg-manifest/reader" }, + { + "path": "../../../resolving/resolver-base" + }, { "path": "../../../store/cafs" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6a8a109fa2..2179500e6c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3180,6 +3180,9 @@ importers: '@pnpm/pkg-manifest.reader': specifier: workspace:* version: link:../../../pkg-manifest/reader + '@pnpm/resolving.resolver-base': + specifier: workspace:* + version: link:../../../resolving/resolver-base '@pnpm/store.index': specifier: workspace:* version: link:../../../store/index