fix(javascript): read every document of a pnpm-lock.yaml stream (#5188)

* fix(javascript): read every document of a pnpm-lock.yaml stream

pnpm-lock.yaml can be a multi-document YAML stream. pnpm keeps config
dependencies and the pinned package-manager version in a leading document
and the project's dependency graph in the next one. That layout has
shipped since pnpm 11 for projects using config dependencies, and pnpm 12
records the package-manager pin by default, so two documents is now the
common case.

The cataloger called yaml.Unmarshal, which reads the first document only.
On such a lockfile the SBOM contains pnpm's own release binaries and none
of the project's dependencies. Both documents declare the same
lockfileVersion and the result is well-formed and non-empty, so nothing
signals that it is wrong.

Split the stream and parse each document, merging the results. A
single-document lockfile yields exactly one document, so its behaviour is
unchanged. A document that omits lockfileVersion falls back to the
version from the first one rather than being dropped.

Fixes #5168

Signed-off-by: hamodywe <iosapk.org@gmail.com>

* refactor(javascript): stream pnpm-lock.yaml documents instead of buffering

The multi-document fix read the whole lockfile, decoded it to a `yaml.Node`
tree, re-marshaled every document back to `[]byte` and then re-parsed each one
twice. `yaml.Node` already has a `Decode` method, so the decoder can walk the
stream straight off the reader and hand each document's node to the parser.

On a 4.8MB single-document lockfile that takes total allocations from 1069MB to
250MB, which is also below where things sat before multi-document support. It
drops the `io.ReadAll` and its `noUnboundedReads` suppression, and puts pnpm in
line with every other YAML cataloger in the tree, all of which already stream.

`pnpmLockfileParser.Parse` now takes a `*yaml.Node` rather than `[]byte`. It is
package-private with two implementations and one production call site.

Documents accumulate into a single map keyed by `name@version`, so the stream
follows one collision rule rather than two: the last entry to appear wins,
which is what already happens within a document. That replaces the separate
first-wins-with-fill-in merge pass. A conflicting integrity between documents
is now traced rather than resolved silently.

Three behaviour fixes come with it:

- an empty or comment-only leading document no longer takes down the whole
  file. It decodes to a null node, not a zero-kind one, so the old guard never
  fired and the stream failed with `invalid lockfile version ""`.
- a malformed document no longer discards the documents that already parsed.
  It is reported through `unknown` instead, matching how the rest of the
  cataloger reports partial results.
- parse errors carry the document index, and their line numbers again point at
  the user's file rather than into a re-marshaled buffer.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* chore(javascript): trim the duplicated pnpm multi-document comment

The six-line explanation of why a pnpm lockfile can hold two documents sat
verbatim in both the parser and its test. Keep it next to the code, where
someone debugging will be, and leave the test pointing at it.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: hamodywe <iosapk.org@gmail.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Co-authored-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Hamody We 2026-08-19 21:09:51 +03:00 committed by GitHub
parent 21cca30e5b
commit 8bb3b5eed1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 385 additions and 36 deletions

View File

@ -3,6 +3,7 @@ package javascript
import (
"cmp"
"context"
"errors"
"fmt"
"io"
"iter"
@ -33,8 +34,10 @@ type pnpmPackage struct {
}
// pnpmLockfileParser defines the interface for parsing different versions of pnpm lockfiles.
// Implementations decode into themselves, so a parser is single-use: construct a new one
// per document rather than reusing one across a multi-document stream.
type pnpmLockfileParser interface {
Parse(version float64, data []byte) ([]pnpmPackage, error)
Parse(version float64, doc *yaml.Node) ([]pnpmPackage, error)
}
type pnpmV6PackageEntry struct {
@ -81,8 +84,8 @@ func newGenericPnpmLockAdapter(cfg CatalogerConfig) genericPnpmLockAdapter {
}
// Parse implements the pnpmLockfileParser interface for v6-v8 lockfiles.
func (p *pnpmV6LockYaml) Parse(version float64, data []byte) ([]pnpmPackage, error) {
if err := yaml.Unmarshal(data, p); err != nil {
func (p *pnpmV6LockYaml) Parse(version float64, doc *yaml.Node) ([]pnpmPackage, error) {
if err := doc.Decode(p); err != nil {
return nil, fmt.Errorf("failed to unmarshal pnpm v6 lockfile: %w", err)
}
@ -131,8 +134,8 @@ func (p *pnpmV6LockYaml) Parse(version float64, data []byte) ([]pnpmPackage, err
}
// Parse implements the PnpmLockfileParser interface for v9+ lockfiles.
func (p *pnpmV9LockYaml) Parse(_ float64, data []byte) ([]pnpmPackage, error) {
if err := yaml.Unmarshal(data, p); err != nil {
func (p *pnpmV9LockYaml) Parse(_ float64, doc *yaml.Node) ([]pnpmPackage, error) {
if err := doc.Decode(p); err != nil {
return nil, fmt.Errorf("failed to unmarshal pnpm v9 lockfile: %w", err)
}
@ -183,38 +186,117 @@ func newPnpmLockfileParser(version float64) pnpmLockfileParser {
// parsePnpmLock is the main parser function for pnpm-lock.yaml files.
func (a genericPnpmLockAdapter) parsePnpmLock(ctx context.Context, resolver file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) {
data, err := io.ReadAll(reader) //nolint:gocritic // multi-pass parse requires []byte
if err != nil {
return nil, nil, fmt.Errorf("failed to load pnpm-lock.yaml file: %w", err)
}
// pnpm-lock.yaml can be a multi-document YAML stream: pnpm keeps config dependencies
// and the pinned package-manager version in a leading document and the project's
// dependency graph in the next one. Reading only the first document yields a
// well-formed SBOM that contains pnpm's own release binaries and none of the
// project's dependencies, with nothing to signal that it is wrong.
// See https://github.com/anchore/syft/issues/5168.
//
// Every document is cataloged, not just the project's. The packages pnpm records for
// itself (pnpm and its @pnpm/exe.* release binaries) are really installed on disk, so
// they are reported as components like any other dependency. The alternative, keeping
// only the last document, would drop them from the SBOM entirely.
pnpmPkgs, errs := parsePnpmLockStream(reader)
var lockfile struct {
Version string `yaml:"lockfileVersion"`
}
if err := yaml.Unmarshal(data, &lockfile); err != nil {
return nil, nil, fmt.Errorf("failed to parse pnpm-lock.yaml version: %w", err)
}
version, err := strconv.ParseFloat(lockfile.Version, 64)
if err != nil {
return nil, nil, fmt.Errorf("invalid lockfile version %q: %w", lockfile.Version, err)
}
parser := newPnpmLockfileParser(version)
pnpmPkgs, err := parser.Parse(version, data)
if err != nil {
return nil, nil, fmt.Errorf("failed to parse pnpm-lock.yaml file: %w", err)
}
packages := make([]pkg.Package, 0, len(pnpmPkgs))
for _, p := range pnpmPkgs {
// left nil when nothing parses, so a failed lockfile reports no packages rather than an empty set
var packages []pkg.Package
for _, p := range toSortedSlice(pnpmPkgs) {
if p.Dev && !a.cfg.IncludeDevDependencies {
continue
}
packages = append(packages, newPnpmPackage(ctx, a.cfg, resolver, reader.Location, p.Name, p.Version, p.Integrity, p.Dependencies))
}
return packages, dependency.Resolve(pnpmLockDependencySpecifier, packages), unknown.IfEmptyf(packages, "unable to determine packages")
errs = unknown.Join(errs, unknown.IfEmptyf(packages, "unable to determine packages"))
return packages, dependency.Resolve(pnpmLockDependencySpecifier, packages), errs
}
// parsePnpmLockStream reads every document of the lockfile stream, keyed by name@version.
// A document that fails to parse is reported as unknown rather than discarding the
// documents that did parse.
func parsePnpmLockStream(reader file.LocationReadCloser) (map[string]pnpmPackage, error) {
dec := yaml.NewDecoder(reader)
pnpmPkgs := make(map[string]pnpmPackage)
var firstVersion float64
var errs error
for i := 0; ; i++ {
var doc yaml.Node
if err := dec.Decode(&doc); err != nil {
if errors.Is(err, io.EOF) {
break
}
// a malformed document leaves the decoder unusable, so stop reading and keep
// what earlier documents contributed rather than dropping the whole lockfile
errs = unknown.Appendf(errs, reader, "failed to parse pnpm-lock.yaml document %d: %v", i, err)
break
}
// an empty or comment-only document decodes to a null node; skipping it keeps a
// leading separator from failing the stream on a missing lockfileVersion
if len(doc.Content) == 0 || doc.Content[0].Tag == "!!null" {
continue
}
version, err := pnpmDocumentVersion(&doc, firstVersion)
if err != nil {
// no document has yielded a usable version yet, so there is nothing to parse
return nil, unknown.Join(errs, err)
}
if firstVersion == 0 {
firstVersion = version
}
pkgs, err := newPnpmLockfileParser(version).Parse(version, &doc)
if err != nil {
errs = unknown.Appendf(errs, reader, "failed to parse pnpm-lock.yaml document %d: %v", i, err)
continue
}
mergePnpmPackages(pnpmPkgs, pkgs, i)
}
return pnpmPkgs, errs
}
// pnpmDocumentVersion reads lockfileVersion from a single document of the stream.
// Documents after the first are expected to repeat it, but one that omits it inherits the
// version already established rather than being dropped.
func pnpmDocumentVersion(doc *yaml.Node, established float64) (float64, error) {
var lockfile struct {
Version string `yaml:"lockfileVersion"`
}
if err := doc.Decode(&lockfile); err != nil {
if established != 0 {
return established, nil
}
return 0, fmt.Errorf("failed to parse pnpm-lock.yaml version: %w", err)
}
version, err := strconv.ParseFloat(lockfile.Version, 64)
switch {
case err == nil:
return version, nil
case established != 0:
return established, nil
default:
return 0, fmt.Errorf("invalid lockfile version %q: %w", lockfile.Version, err)
}
}
// mergePnpmPackages folds one document's packages into the running set. The whole stream
// follows a single collision rule, the same one used within a document: the last entry to
// appear wins.
func mergePnpmPackages(into map[string]pnpmPackage, pkgs []pnpmPackage, doc int) {
for _, p := range pkgs {
key := p.Name + "@" + p.Version
if existing, ok := into[key]; ok && existing.Integrity != "" && p.Integrity != "" && existing.Integrity != p.Integrity {
log.WithFields("package", key, "document", doc).Trace("conflicting integrity across pnpm-lock.yaml documents")
}
into[key] = p
}
}
// parseVersionField extracts the version string from a dependency entry.

View File

@ -6,10 +6,13 @@ import (
"net/http"
"net/http/httptest"
"os"
"sort"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.yaml.in/yaml/v3"
"github.com/anchore/syft/syft/artifact"
"github.com/anchore/syft/syft/file"
@ -527,6 +530,15 @@ func Test_corruptPnpmLock(t *testing.T) {
TestParser(t, adapter.parsePnpmLock)
}
// yamlDocument decodes a single YAML document for the parsers, which take the node the
// lockfile stream decoder hands them rather than raw bytes.
func yamlDocument(t *testing.T, data string) *yaml.Node {
t.Helper()
var doc yaml.Node
require.NoError(t, yaml.Unmarshal([]byte(data), &doc))
return &doc
}
func TestParsePnpmLock_DeterministicWithCollidingPeerDeps(t *testing.T) {
// this test verifies that when multiple lockfile keys collapse to the same
// package key after peer-dep stripping (e.g., pkg@1.0.0(peer-a@1) and
@ -535,7 +547,7 @@ func TestParsePnpmLock_DeterministicWithCollidingPeerDeps(t *testing.T) {
// the last key lexicographically wins.
// v9 lockfile with two entries that collapse to the same key
lockfileV9 := []byte(`
lockfileV9 := `
lockfileVersion: '9.0'
packages:
some-pkg@1.0.0(peer-b@2.0.0):
@ -545,12 +557,12 @@ packages:
snapshots:
some-pkg@1.0.0(peer-b@2.0.0): {}
some-pkg@1.0.0(peer-a@1.0.0): {}
`)
`
// run multiple times to catch nondeterminism
for range 10 {
parser := &pnpmV9LockYaml{}
pkgs, err := parser.Parse(9.0, lockfileV9)
pkgs, err := parser.Parse(9.0, yamlDocument(t, lockfileV9))
require.NoError(t, err)
require.Len(t, pkgs, 1, "expected exactly one package after key collision")
@ -561,18 +573,18 @@ snapshots:
}
// v6 lockfile with two entries that collapse to the same key
lockfileV6 := []byte(`
lockfileV6 := `
lockfileVersion: '6.0'
packages:
/some-pkg@1.0.0(peer-b@2.0.0):
resolution: {integrity: sha512-BBB}
/some-pkg@1.0.0(peer-a@1.0.0):
resolution: {integrity: sha512-AAA}
`)
`
for range 10 {
parser := &pnpmV6LockYaml{}
pkgs, err := parser.Parse(6.0, lockfileV6)
pkgs, err := parser.Parse(6.0, yamlDocument(t, lockfileV6))
require.NoError(t, err)
require.Len(t, pkgs, 1, "expected exactly one package after key collision")
@ -618,3 +630,201 @@ func setupNpmRegistry() (mux *http.ServeMux, serverURL string, teardown func())
return mux, server.URL, server.Close
}
// covers a real two-document lockfile end to end; parsePnpmLock explains why every
// document is cataloged
func TestParsePnpmLock_MultiDocument(t *testing.T) {
var expectedRelationships []artifact.Relationship
fixture := "testdata/pnpm-multi-doc/pnpm-lock.yaml"
locationSet := file.NewLocationSet(file.NewLocation(fixture))
expectedPkgs := []pkg.Package{
{
Name: "@pnpm/exe.linux-x64",
Version: "12.0.0-rc.3",
PURL: "pkg:npm/%40pnpm/exe.linux-x64@12.0.0-rc.3",
Locations: locationSet,
Language: pkg.JavaScript,
Type: pkg.NpmPkg,
Metadata: pkg.PnpmLockEntry{
Resolution: pkg.PnpmLockResolution{Integrity: "sha512-/6xWaYfp6MEaJF+7AZIfCMp/fZX2jZlTLh2KVBEH0QOWk1ktaBlKNIJEgT4m6mieOLhNYMzCfLJImMbJDK1IwA=="},
Dependencies: map[string]string{},
},
},
{
// The project's actual dependency, which lives in the second document.
Name: "minimist",
Version: "1.2.0",
PURL: "pkg:npm/minimist@1.2.0",
Locations: locationSet,
Language: pkg.JavaScript,
Type: pkg.NpmPkg,
Metadata: pkg.PnpmLockEntry{
Resolution: pkg.PnpmLockResolution{Integrity: "sha512-7Wl+Jz+IGWuSdgsQEJ4JunV0si/iMhg42MnQQG6h1R6TNeVenp4U9x5CC5v/gYqz/fENLQITAWXidNtVL0NNbw=="},
Dependencies: map[string]string{},
},
},
{
Name: "pnpm",
Version: "12.0.0-rc.3",
PURL: "pkg:npm/pnpm@12.0.0-rc.3",
Locations: locationSet,
Language: pkg.JavaScript,
Type: pkg.NpmPkg,
Metadata: pkg.PnpmLockEntry{
Resolution: pkg.PnpmLockResolution{Integrity: "sha512-JZ9fDGH+WLdRdTEikN3UxeZe6bpDY7dYwV0RX0+OrJ927vc72XbId4IJXeT++ebNFA9cF3IQ1swiXHEd/Maq0Q=="},
Dependencies: map[string]string{},
},
},
}
adapter := newGenericPnpmLockAdapter(CatalogerConfig{IncludeDevDependencies: true})
pkgtest.TestFileParser(t, fixture, adapter.parsePnpmLock, expectedPkgs, expectedRelationships)
}
// pnpmDoc renders a minimal v9 lockfile document holding a single package.
func pnpmDoc(version, name, ver, integrity string) string {
doc := ""
if version != "" {
doc += "lockfileVersion: '" + version + "'\n"
}
return doc + "packages:\n " + name + "@" + ver + ":\n resolution: {integrity: " + integrity + "}\nsnapshots:\n " + name + "@" + ver + ": {}\n"
}
func TestParsePnpmLock_MultiDocumentStream(t *testing.T) {
project := pnpmDoc("9.0", "minimist", "1.2.0", "sha512-AAA")
tests := []struct {
name string
lockfile string
wantPkgs []string
wantErr bool
}{
{
// a stream may open with a separator, which decodes to a null document; it must
// not take down the documents that follow it
name: "leading comment-only document is skipped",
lockfile: "---\n# generated by pnpm\n---\n" + project,
wantPkgs: []string{"minimist@1.2.0"},
},
{
name: "trailing separator is skipped",
lockfile: project + "---\n",
wantPkgs: []string{"minimist@1.2.0"},
},
{
// documents after the first inherit the established lockfileVersion rather than
// being dropped for omitting one
name: "later document inherits lockfileVersion",
lockfile: project + "---\n" + pnpmDoc("", "left-pad", "1.0.0", "sha512-BBB"),
wantPkgs: []string{"left-pad@1.0.0", "minimist@1.2.0"},
},
{
// a broken document is reported, but the documents that already parsed are kept
name: "corrupt later document keeps earlier packages",
lockfile: project + "---\nlockfileVersion: '9.0'\npackages:\n bad@: {oops\n",
wantPkgs: []string{"minimist@1.2.0"},
wantErr: true,
},
{
name: "single document is unchanged",
lockfile: project,
wantPkgs: []string{"minimist@1.2.0"},
},
{
name: "empty file yields no packages",
lockfile: "",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
adapter := newGenericPnpmLockAdapter(CatalogerConfig{IncludeDevDependencies: true})
pkgs, _, err := adapter.parsePnpmLock(context.Background(), nil, nil, file.LocationReadCloser{
Location: file.NewLocation("pnpm-lock.yaml"),
ReadCloser: io.NopCloser(strings.NewReader(tt.lockfile)),
})
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
var got []string
for _, p := range pkgs {
got = append(got, p.Name+"@"+p.Version)
}
sort.Strings(got)
assert.Equal(t, tt.wantPkgs, got)
})
}
}
func Test_mergePnpmPackages(t *testing.T) {
// the whole stream shares one collision rule, the same one used within a document:
// the last entry to appear wins
first := pnpmPackage{Name: "a", Version: "1.0.0", Integrity: "sha512-AAA", Dev: true, Dependencies: map[string]string{"b": "2.0.0"}}
second := pnpmPackage{Name: "a", Version: "1.0.0", Integrity: "sha512-BBB"}
tests := []struct {
name string
docs [][]pnpmPackage
want []pnpmPackage
}{
{
name: "disjoint documents are unioned",
docs: [][]pnpmPackage{{first}, {{Name: "c", Version: "3.0.0"}}},
want: []pnpmPackage{first, {Name: "c", Version: "3.0.0"}},
},
{
name: "later document wins on a colliding name@version",
docs: [][]pnpmPackage{{first}, {second}},
want: []pnpmPackage{second},
},
{
name: "same package in one document only",
docs: [][]pnpmPackage{{first}},
want: []pnpmPackage{first},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := make(map[string]pnpmPackage)
for i, doc := range tt.docs {
mergePnpmPackages(got, doc, i)
}
assert.Equal(t, tt.want, toSortedSlice(got))
})
}
}
func Test_pnpmDocumentVersion(t *testing.T) {
tests := []struct {
name string
doc string
established float64
want float64
wantErr bool
}{
{name: "reads its own version", doc: "lockfileVersion: '9.0'\n", want: 9.0},
{name: "inherits when omitted", doc: "packages: {}\n", established: 9.0, want: 9.0},
{name: "own version beats the established one", doc: "lockfileVersion: '6.0'\n", established: 9.0, want: 6.0},
{name: "first document must carry a version", doc: "packages: {}\n", wantErr: true},
{name: "first document must carry a usable version", doc: "lockfileVersion: 'nope'\n", wantErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := pnpmDocumentVersion(yamlDocument(t, tt.doc), tt.established)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}

View File

@ -0,0 +1,57 @@
---
lockfileVersion: '9.0'
importers:
.:
configDependencies: {}
packageManagerDependencies:
pnpm:
specifier: 12.0.0-rc.3
version: 12.0.0-rc.3
packages:
'@pnpm/exe.linux-x64@12.0.0-rc.3':
resolution: {integrity: sha512-/6xWaYfp6MEaJF+7AZIfCMp/fZX2jZlTLh2KVBEH0QOWk1ktaBlKNIJEgT4m6mieOLhNYMzCfLJImMbJDK1IwA==}
cpu: [x64]
os: [linux]
libc: [glibc]
pnpm@12.0.0-rc.3:
resolution: {integrity: sha512-JZ9fDGH+WLdRdTEikN3UxeZe6bpDY7dYwV0RX0+OrJ927vc72XbId4IJXeT++ebNFA9cF3IQ1swiXHEd/Maq0Q==}
engines: {node: '>=18.*'}
hasBin: true
snapshots:
'@pnpm/exe.linux-x64@12.0.0-rc.3':
optional: true
pnpm@12.0.0-rc.3:
optionalDependencies:
'@pnpm/exe.linux-x64': 12.0.0-rc.3
---
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
dependencies:
minimist:
specifier: 1.2.0
version: 1.2.0
packages:
minimist@1.2.0:
resolution: {integrity: sha512-7Wl+Jz+IGWuSdgsQEJ4JunV0si/iMhg42MnQQG6h1R6TNeVenp4U9x5CC5v/gYqz/fENLQITAWXidNtVL0NNbw==}
snapshots:
minimist@1.2.0: {}