fix: don't panic on pnpm lockfiles with an invalid path (#280)

Author: G-Rath

pkg/lockfile/fixtures/pnpm/invalid-package-path.yaml

@@ -0,0 +1,92 @@
+lockfileVersion: 5.4
+
+specifiers:
+  '@types/jsdom': ^20.0.1
+  axios: ^1.2.5
+  pinia: ^2.0.28
+  stream: 0.0.2
+  typescript: ~4.7.4
+
+dependencies:
+  axios: 1.2.5
+  pinia: 2.0.28_e7lp6ggkpgyi5vqd44m2kxvk6i
+  stream: 0.0.2
+
+devDependencies:
+  '@types/jsdom': 20.0.1
+  npm-run-all: 4.1.5
+
+packages:
+
+  /@babel/helper-string-parser/7.19.4:
+    resolution: {integrity: sha512-nHtDoQcuqFmwYNYPz3Rah5ph2p8PFeFCsZk9A/48dPc/rGocJ5J3hAAZ7pb76VWX3fZKu+uEr/FhH5jLx7umrw==}
+    engines: {node: '>=6.9.0'}
+
+  /@babel/helper-validator-identifier/7.19.1:
+    resolution: {integrity: sha512-awrNfaMtnHUr653GgGEs++LlAvW6w+DcPrOliSMXWCKo597CwL5Acf/wWdNkf/tfEQE3mjkeD1YOVZOUV/od1w==}
+    engines: {node: '>=6.9.0'}
+
+  /@babel/parser/7.20.7:
+    resolution: {integrity: sha512-T3Z9oHybU+0vZlY9CiDSJQTD5ZapcW18ZctFMi0MOAl/4BjFF4ul7NVSARLdbGO5vDqy9eQiGTV0LtKfvCYvcg==}
+    engines: {node: '>=6.0.0'}
+    hasBin: true
+    dependencies:
+      '@babel/types': 7.20.7
+
+  /@types/jsdom/20.0.1:
+    resolution: {integrity: sha512-d0r18sZPmMQr1eG35u12FZfhIXNrnsPU/g5wvRKCUf/tOGilKKwYMYGqh33BNR6ba+2gkHw1EUiHoN3mn7E5IQ==}
+    dependencies:
+      '@types/node': 18.11.18
+      '@types/tough-cookie': 4.0.2
+      parse5: 7.1.2
+    dev: true
+
+  [email protected]:
+    resolution: {integrity: sha512-9pU/8mmjSSOb4CXVsvGIevN+MlO/t9OWtKadTaLuN85Gge3HGorUckgp8A/2FH4V4hJ7JuQ3LIeI7KAV9ITZrQ==}
+    dependencies:
+      follow-redirects: 1.15.2
+      form-data: 4.0.0
+      proxy-from-env: 1.1.0
+    transitivePeerDependencies:
+      - debug
+    dev: false
+
+  /stream/0.0.2:
+    resolution: {integrity: sha512-gCq3NDI2P35B2n6t76YJuOp7d6cN/C7Rt0577l91wllh0sY9ZBuw9KaSGqH/b0hzn3CWWJbpbW0W0WvQ1H/Q7g==}
+    dependencies:
+      emitter-component: 1.1.1
+    dev: false
+
+  /typescript/4.7.4:
+    resolution: {integrity: sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ==}
+    engines: {node: '>=4.2.0'}
+    hasBin: true
+
+  /pinia/2.0.28_e7lp6ggkpgyi5vqd44m2kxvk6i:
+    resolution: {integrity: sha512-YClq9DkqCblq9rlyUual7ezMu/iICWdBtfJrDt4oWU9Zxpijyz7xB2xTwx57DaBQ96UGvvTMORzALr+iO5PVMw==}
+    peerDependencies:
+      '@vue/composition-api': ^1.4.0
+      typescript: '>=4.4.4'
+      vue: ^2.6.14 || ^3.2.0
+    peerDependenciesMeta:
+      '@vue/composition-api':
+        optional: true
+      typescript:
+        optional: true
+    dependencies:
+      '@vue/devtools-api': 6.4.5
+      typescript: 4.7.4
+      vue: 3.2.45
+      vue-demi: [email protected]
+    dev: false
+
+  /http-proxy-agent/5.0.0:
+    resolution: {integrity: sha512-n2hY8YdoRE1i7r6M0w9DIw5GgZN0G25P8zLCRQ8rjXtTU3vsNFBI/vWK/UIeE6g5MUUz6avwAPXmL6Fy9D/90w==}
+    engines: {node: '>= 6'}
+    dependencies:
+      '@tootallnate/once': 2.0.0
+      agent-base: 6.0.2
+      debug: 4.3.4
+    transitivePeerDependencies:
+      - supports-color
+    dev: true

pkg/lockfile/parse-pnpm-lock.go

@@ -1,6 +1,7 @@
 package lockfile
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"strconv"
@@ -10,6 +11,8 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+var errInvalidPackagePath = errors.New("invalid package path")
+
 type PnpmLockPackageResolution struct {
 	Tarball string `yaml:"tarball"`
 	Commit  string `yaml:"commit"`
@@ -62,12 +65,12 @@ func startsWithNumber(str string) bool {
 
 // extractPnpmPackageNameAndVersion parses a dependency path, attempting to
 // extract the name and version of the package it represents
-func extractPnpmPackageNameAndVersion(dependencyPath string, lockfileVersion float64) (string, string) {
+func extractPnpmPackageNameAndVersion(dependencyPath string, lockfileVersion float64) (string, string, error) {
 	// file dependencies must always have a name property to be installed,
 	// and their dependency path never has the version encoded, so we can
 	// skip trying to extract either from their dependency path
 	if strings.HasPrefix(dependencyPath, "file:") {
-		return "", ""
+		return "", "", nil
 	}
 
 	// v9.0 specifies the dependencies as <package>@<version> rather than as a path
@@ -81,10 +84,15 @@ func extractPnpmPackageNameAndVersion(dependencyPath string, lockfileVersion flo
 			name = "@" + name
 		}
 
-		return name, version
+		return name, version, nil
 	}
 
 	parts := strings.Split(dependencyPath, "/")
+
+	if len(parts) == 1 {
+		return "", "", errInvalidPackagePath
+	}
+
 	var name string
 
 	parts = parts[1:]
@@ -108,14 +116,14 @@ func extractPnpmPackageNameAndVersion(dependencyPath string, lockfileVersion flo
 	}
 
 	if version == "" || !startsWithNumber(version) {
-		return "", ""
+		return "", "", nil
 	}
 
 	// peer dependencies in v5 lockfiles are attached to the end of the version
 	// with an "_", so we always want the first element if an "_" is present
 	version, _, _ = strings.Cut(version, "_")
 
-	return name, version
+	return name, version, nil
 }
 
 func parseNameAtVersion(value string) (name string, version string) {
@@ -129,11 +137,15 @@ func parseNameAtVersion(value string) (name string, version string) {
 	return matches[1], matches[2]
 }
 
-func parsePnpmLock(lockfile PnpmLockfile) []PackageDetails {
+func parsePnpmLock(lockfile PnpmLockfile) ([]PackageDetails, error) {
 	packages := make([]PackageDetails, 0, len(lockfile.Packages))
 
 	for s, pkg := range lockfile.Packages {
-		name, version := extractPnpmPackageNameAndVersion(s, lockfile.Version)
+		name, version, err := extractPnpmPackageNameAndVersion(s, lockfile.Version)
+
+		if err != nil {
+			return nil, err
+		}
 
 		// "name" is only present if it's not in the dependency path and takes
 		// priority over whatever name we think we've extracted (if any)
@@ -171,7 +183,7 @@ func parsePnpmLock(lockfile PnpmLockfile) []PackageDetails {
 		})
 	}
 
-	return packages
+	return packages, nil
 }
 
 func ParsePnpmLock(pathToLockfile string) ([]PackageDetails, error) {
@@ -194,5 +206,11 @@ func ParsePnpmLock(pathToLockfile string) ([]PackageDetails, error) {
 		parsedLockfile = &PnpmLockfile{}
 	}
 
-	return parsePnpmLock(*parsedLockfile), nil
+	packageDetails, err := parsePnpmLock(*parsedLockfile)
+
+	if err != nil {
+		return []PackageDetails{}, fmt.Errorf("could not parse %s: %w", pathToLockfile, err)
+	}
+
+	return packageDetails, nil
 }

pkg/lockfile/parse-pnpm-lock_test.go

@@ -615,6 +615,15 @@ func TestParsePnpmLock_Commits(t *testing.T) {
 	})
 }
 
+func TestParsePnpmLock_InvalidPackagePath(t *testing.T) {
+	t.Parallel()
+
+	packages, err := lockfile.ParsePnpmLock("fixtures/pnpm/invalid-package-path.yaml")
+
+	expectErrContaining(t, err, "invalid package path")
+	expectPackages(t, packages, []lockfile.PackageDetails{})
+}
+
 func TestParsePnpmLock_Files(t *testing.T) {
 	t.Parallel()