fix(yarn): stop a dependency named "version" from clobbering the entry version - #76
Open
shoemoney wants to merge 1 commit into
Open
Conversation
…y version parseYarnLock scans every indented line of an entry with no first-wins guard, so a line inside a dependencies:/peerDependencies: block whose package is literally named "version" (a real npm package) matches the version-field prefix and overwrites the already-captured entry version. A Classic entry for board2d@1.0.0 that depends on version@^0.1.0 was inventoried as version "^0.1.0"; the Berry form emitted "npm:^0.1.0". Since exposure matching is exact name+version, a compromised package at the pinned version was silently not flagged. Guard the capture with first-wins, the same way parsePnpmPackages in internal/ecosystem/pnpm/pnpm.go already guards the same field (strings.HasPrefix(trim, "version:") && cur.version == ""); the yarn twin never got the guard. Safe because yarn Classic and Berry always emit the entry's own version as the first field of an entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
parseYarnLock captures the version field from every indented line of an entry with no first-wins guard (internal/ecosystem/yarn/yarn.go:160 on main). A line inside a dependencies:/peerDependencies:/optionalDependencies: block whose package is literally named "version" (a real npm package, ~19K weekly downloads) matches the same prefix and overwrites the already-captured entry version.
Concretely, a Classic entry for board2d@1.0.0 that depends on version@^0.1.0 was inventoried as version "^0.1.0", and the Berry form as "npm:^0.1.0". Exposure-catalog matching is exact name+version, so a compromised board2d@1.0.0 on the endpoint would silently not be flagged, and the inventory record carries a range spec instead of a version.
Fix: guard the capture with
&& cur.version == "", mirroring parsePnpmPackages in internal/ecosystem/pnpm/pnpm.go:328, which already guards the same field. This is safe because yarn (Classic and Berry) always emits the entry's own version as the first field of an entry.Added TestScanLockfile_DependencyNamedVersion with classic and berry subtests; both fail on main with exactly the clobbered values above and pass with the guard.
go test ./...,go test -race,go vet ./...,gofmt -l ., and./bumblebee selftestare clean.One thing I deliberately did not do: require the version line to sit at exactly 2 spaces of indent. First-wins alone is sufficient given yarn's emission order, and an indent requirement would be a second behaviour change with its own edge cases (tab-indented hand-edited lockfiles), better considered separately if wanted.