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 <z@kochan.io>

.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`.

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:*",

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}`
+}

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'

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()
+  })
+})

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))

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'

deps/compliance/sbom/tsconfig.json

@@ -30,6 +30,9 @@
     {
       "path": "../../../pkg-manifest/reader"
     },
+    {
+      "path": "../../../resolving/resolver-base"
+    },
     {
       "path": "../../../store/cafs"
     },

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