Feature/xray 138688 add pnpm support for jf ca

[gauriy-tech]

• The pull request is targeting the dev branch.
• The code has been validated to compile successfully by running go vet ./....
• The code has been formatted properly using go fmt ./....
• All static analysis checks passed.
• All tests have passed. If this feature is not already covered by the tests, new tests have been added.
• Updated the Contributing page / ReadMe page / CI Workflow files if needed.
• All changes are detailed at the description. if not already covered at JFrog Documentation, new documentation have been added.

---

What: Added pnpm support to jf ca (curation audit) using lockfile-based dependency resolution
Why: jf ca can't run pnpm ls (requires node_modules) — lockfile parsing avoids any tarball downloads
How: pnpm install --lockfile-only generates/refreshes the lockfile, then pnpm-lock.yaml is parsed directly. jf audit/jf scan pnpm workflow is unchanged.
Scope: Only pnpm v10.x supported. jf audit path untouched.
Test plan: new unit tests in pnpm_test.go, pnpmlock_test.go, curationaudit_test.go

Detailed Test execution plan is here https://jfrog-int.atlassian.net/browse/XRAY-144540

[gineshkumar]

ensureLockfile writes the regenerated lockfile back to the user's project directory (workingDir), but the log.Debug at the top of the function says "running in a temporary directory" — that's misleading, since the result ends up in the real project. For CI workspaces with read-only checkouts this causes a confusing permission error rather than a useful scan failure. More generally, a security scanning tool shouldn't silently modify project files.

Consider parsing the lockfile from tmpDir directly rather than copying it back:

Suggested change

	return os.WriteFile(filepath.Join(workingDir, "pnpm-lock.yaml"), generatedLock, 0644)
	// parsePnpmLockFile reads from tmpDir — no write-back to the user's project needed.
	return parsePnpmLockFile(tmpDir)

If the write-back is intentional (to cache for future runs), at minimum add a log.Info so users know their lockfile was updated.

[gineshkumar]

The comment says "Anything >= 6.0 shares the same structure" — this is incorrect. Lockfile versions 6.x/7.x/8.x (pnpm 7/8 era) use a packages block; the snapshots block only exists in v9.0+. The pnpmLockFile struct only maps Snapshots, so a v6/v7/v8 lockfile silently yields an empty lf.Snapshots. walkSnapshot returns nil for every dependency, and all transitive dependencies are lost without any error or warning.

A user with pnpm 10 installed but a pre-upgrade lockfile in v6 format will pass both the version check and this check, and get an incomplete dependency tree.

Suggested change

	if strings.HasPrefix(v, "5.") {
	// Only lockfileVersion '9.0' and later include the 'snapshots' block this parser uses.
	// Lockfile versions 5.x–8.x used a 'packages' block with a different structure.
	major, _, _ := strings.Cut(v, ".")
	majorNum, _ := strconv.Atoi(major)
	if majorNum < 9 {
	return fmt.Errorf("pnpm-lock.yaml lockfileVersion %q is not supported; run 'pnpm install --lockfile-only' to regenerate with a compatible lockfile format", v)
	}

[gineshkumar]

major != supportedPnpmMajorVersion rejects both older and newer versions. If pnpm 11 ships with the same lockfileVersion: '9.0' format (which is very likely — pnpm has kept that schema since v8), the lockfile parser will work correctly, but jf ca will refuse to run. Users who upgrade pnpm will be blocked until the codebase is updated.

The lockfile validator (validateLockfileVersion) already handles compatibility at the format level — the version guard should only reject versions below the minimum:

Suggested change

	if major != supportedPnpmMajorVersion {
	if major < supportedPnpmMajorVersion {
	return fmt.Errorf("resolving pnpm dependencies from Artifactory requires pnpm %d.x or later. The current pnpm version is: %s", supportedPnpmMajorVersion, versionStr)
	}

If there's a specific reason pnpm 11 is expected to break the format, a log.Warn for major > supportedPnpmMajorVersion ("untested version, results may vary") would be safer than a hard reject.

[gineshkumar]

This branch is dead code — rawRef is never a scoped-package key. In both callers (buildDepsMap and walkSnapshot), rawRef is a version string such as "2.0.0(@peer/dep@1.0.0)" or "1.0.0", which never starts with @. Scoped package names (e.g. @scope/pkg) go in the name parameter, not rawRef. The function comment above ("rawRef is already a full key like @scope/pkg@2.0.0") is also incorrect.

Suggested change

	if strings.HasPrefix(rawRef, "@") {
	// buildSnapshotKey constructs the key used to look up an entry in the snapshots map.
	// name is the bare package name (e.g. "xml" or "@scope/pkg"); rawRef is the version
	// string from the importers or snapshots block (e.g. "1.0.1" or "2.0.0(@peer@1.0)").
	func buildSnapshotKey(name, rawRef string) string {
	return name + "@" + rawRef
	}

[gineshkumar]

Nit / code quality: pnpm --version runs twice per curation audit. getPnpmExecPath (called at line 52) already invokes it for logging. validatePnpmVersionForCuration runs it again here for the version check. That's two subprocess spawns before any scanning starts.

Consider returning the version from getPnpmExecPath so it can be reused:

func getPnpmExecPath() (pnpmExecPath, version string, err error) {
    // ...
    versionOut, versionErr := getPnpmCmd(pnpmExecPath, "", "--version").RunWithOutput()
    // ...
    version = strings.TrimSpace(string(versionOut))
    log.Debug("Pnpm version:", version)
    return
}

// validatePnpmVersionForCuration(version string) — no subprocess needed

[gineshkumar]

Error strings should not contain newlines — they compose in chains (e.g. fmt.Errorf("outer: %w", err)) and an embedded \n breaks the chain readability. The "Ensure the registry…" guidance belongs in a log.Warn, not in the error value.

Suggested change

	return fmt.Errorf("pnpm: failed to read Artifactory details from .npmrc: %w\nEnsure the registry is configured in .npmrc (e.g. registry=https://<host>/artifactory/api/npm/<repo>/)", err)
	log.Warn("Ensure the pnpm registry is configured in .npmrc (e.g. registry=https://<host>/artifactory/api/npm/<repo>/)")
	return fmt.Errorf("pnpm: failed to read Artifactory details from .npmrc: %w", err)

[gineshkumar]

Go 1.24 introduced t.Chdir(dir), which automatically restores the working directory after the test (including on t.Fatal / panic). The manual save-and-restore pattern here can miss cleanup on early exits.

This project is on Go 1.25 — t.Chdir is available:

Suggested change

	require.NoError(t, os.Chdir(sub))
	t.Chdir(sub)

The origDir variable and the defer can be removed entirely.

[gineshkumar]

The log.Info here logs which URL and repo are being used, but not which auth source was chosen. When a user has both a .npmrc token and a jf c server configured and the .npmrc token is expired or invalid, they'll get a 401 from Artifactory with no diagnostic indicating that the .npmrc token was preferred over the jf c credential.

Consider logging the auth source so failures are self-diagnosable:

Suggested change

	log.Info(fmt.Sprintf("pnpm: using Artifactory URL %q and repository %q from .npmrc", registryConfig.ArtifactoryUrl, registryConfig.RepoName))
	if registryConfig.AuthToken != "" {
	log.Debug("pnpm: using auth token from .npmrc")
	} else {
	log.Debug("pnpm: no token in .npmrc — using jf c server credentials")
	}
	log.Info(fmt.Sprintf("pnpm: using Artifactory URL %q and repository %q from .npmrc", registryConfig.ArtifactoryUrl, registryConfig.RepoName))

[gineshkumar]

Suggestion: add a real-world lockfile fixture

The two existing fixtures (pnpm-lock.yaml, pnpm-lock-complex.yaml) are hand-crafted and share the same shape. Neither contains patchedDependencies, overrides, or packageExtensionsChecksum — top-level blocks that real pnpm 10 projects commonly have. If the parser is ever changed to use strict YAML decoding (KnownFields(true)), the existing tests won't catch the breakage.

Proposed addition — two new files:

tests/testdata/projects/package-managers/pnpm/vite/package.json

{
  "name": "@vitejs/vite-monorepo",
  "version": "0.0.0",
  "private": true
}

tests/testdata/projects/package-managers/pnpm/vite/pnpm-lock.yaml (trimmed copy of vitejs/vite — 42 lines, structure taken verbatim):

lockfileVersion: '9.0'

settings:
  autoInstallPeers: false
  dedupePeers: true
  excludeLinksFromLockfile: false

overrides:
  rolldown: 1.0.3

packageExtensionsChecksum: sha256-Tldxs3DhJEw/FFBonUidqhCBqApA0zxQnop3Y+BTO3U=

patchedDependencies:
  chokidar@3.6.0:
    hash: 8a4f9e2b397e6034b91a0508faae3cecb97f222313faa129d7cb0eb71e9d0e84
    path: patches/chokidar@3.6.0.patch
  dotenv-expand@13.0.0:
    hash: 49330a663821151418e003e822a82a6a61d2f0f8a6e3cab00c1c94815a112889
    path: patches/dotenv-expand@13.0.0.patch
  sirv@3.0.2:
    hash: c07c56eb72faea34341d465cde2314e89db472106ed378181e3447893af6bf95
    path: patches/sirv@3.0.2.patch

importers:
  .:
    devDependencies:
      '@types/node':
        specifier: ^24.12.4
        version: 24.12.4
      eslint:
        specifier: ^9.39.4
        version: 9.39.4(jiti@2.6.1)(ms@2.1.3)
  packages/vite:
    dependencies:
      rolldown:
        specifier: 1.0.3
        version: 1.0.3
  packages/create-vite:
    devDependencies:
      cross-spawn:
        specifier: ^7.0.6
        version: 7.0.6

snapshots:
  cross-spawn@7.0.6: {}
  rolldown@1.0.3: {}
  '@types/node@24.12.4': {}
  eslint@9.39.4(jiti@2.6.1)(ms@2.1.3): {}

Test in pnpmlock_test.go:

func viteFixtureDir(t *testing.T) string {
    t.Helper()
    dir, err := filepath.Abs("../../../../..")
    require.NoError(t, err)
    return filepath.Join(dir, "tests", "testdata", "projects", "package-managers", "pnpm", "vite")
}

// TestParsePnpmLockFileVite is a smoke test using a trimmed copy of vitejs/vite's
// pnpm-lock.yaml. The fixture contains top-level blocks absent from the hand-crafted
// fixtures (patchedDependencies, overrides, packageExtensionsChecksum) and verifies
// the parser handles them without error. It would catch, for example, adding strict
// YAML decoding (KnownFields(true)) that would reject these unknown fields.
func TestParsePnpmLockFileVite(t *testing.T) {
    projects, err := parsePnpmLockFile(viteFixtureDir(t))
    require.NoError(t, err)
    // Fixture has 3 importers: root (.), packages/vite, packages/create-vite.
    assert.Greater(t, len(projects), 2)
}

Passes in 0ms. Happy to share the full diff if useful.