fix(pnpm): resolve v11 global-scan regression

Signed-off-by: prateek-stepsecurity <prateek@stepsecurity.io>

Author: Prateek-stepsecurity

internal/detector/nodescan.go

@@ -199,27 +199,60 @@ func (s *NodeScanner) scanYarnGlobal(ctx context.Context) (model.NodeScanResult,
 
 func (s *NodeScanner) scanPnpmGlobal(ctx context.Context) (model.NodeScanResult, bool) {
 	if err := s.checkPath(ctx, "pnpm"); err != nil {
+		s.log.Warn("pnpm not found on PATH — skipping pnpm global scan: %v", err)
 		return model.NodeScanResult{}, false
 	}
 
-	version := s.getVersion(ctx, "pnpm", "--version")
-	globalDir := s.getOutput(ctx, "pnpm", "root", "-g")
+	// pnpm v11 hard-fails any `-g` invocation when its user-local bin dir is
+	// not on PATH. Two execution paths to handle:
+	// - In-process (exec.Command): prepend the bin dir to this Go process's
+	//   PATH so the child inherits it.
+	// - Root → user delegation (sudo -u): inline `PATH='…':$PATH` into the
+	//   shell command itself. This works regardless of sudo's env policy
+	//   (macOS keeps PATH via env_keep; Linux strips it via secure_path),
+	//   because the env-prefix is honored by the user's shell AFTER sudo
+	//   has already done whatever it does.
+	extra := defaultPnpmBinDir(s.exec)
+	if extra != "" {
+		oldPath := os.Getenv("PATH")
+		_ = os.Setenv("PATH", extra+string(os.PathListSeparator)+oldPath)
+		defer os.Setenv("PATH", oldPath)
+	}
+
+	// For the delegation path, embed `PATH='extra':$PATH` in the command name.
+	// runCmd's delegation branch space-joins name+args into the shell command
+	// string, so the env-prefix flows through to the user's shell intact.
+	// Applied to every pnpm `-g` call, since v11 enforces the bin-dir check
+	// on each one.
+	pnpmCmd := "pnpm"
+	if s.shouldRunAsUser() && extra != "" {
+		pnpmCmd = "PATH=" + platformShellQuote(s.exec, extra) + ":$PATH pnpm"
+	}
+
+	versionOut, _, _, verErr := s.runCmd(ctx, 10*time.Second, pnpmCmd, "--version")
+	version := strings.TrimSpace(versionOut)
+	if verErr != nil || version == "" {
+		version = "unknown"
+	}
+
+	rootOut, _, _, _ := s.runCmd(ctx, 10*time.Second, pnpmCmd, "root", "-g")
+	globalDir := strings.TrimSpace(rootOut)
 	if globalDir == "" {
+		s.log.Warn("pnpm found but `pnpm root -g` returned empty — skipping pnpm global scan")
 		return model.NodeScanResult{}, false
 	}
 	globalDir = filepath.Dir(globalDir)
 
 	start := time.Now()
-	// pnpm v11 exits non-zero on `--depth=3` for global scans; use `--depth=Infinity` there.
-	stdout, stderr, exitCode, _ := s.runCmd(ctx, 60*time.Second, "pnpm", "list", "-g", "--json", pnpmDepthArg(version))
+	stdout, stderr, exitCode, err := s.runCmd(ctx, 60*time.Second, pnpmCmd, "list", "-g", "--json")
 	duration := time.Since(start).Milliseconds()
 
 	errMsg := ""
 	if exitCode != 0 {
 		errMsg = "pnpm list -g command failed"
 		s.log.Warn("pnpm list -g failed (exit_code=%d, %dms) — results may be incomplete", exitCode, duration)
 	}
-	s.log.Debug("pnpm global scan: version=%s global_dir=%s exit_code=%d stdout_bytes=%d duration=%dms", version, globalDir, exitCode, len(stdout), duration)
+	s.log.Debug("pnpm global scan: version=%s global_dir=%s exit_code=%d stdout_bytes=%d duration=%dms err=%v", version, globalDir, exitCode, len(stdout), duration, err)
 
 	return model.NodeScanResult{
 		ProjectPath:      globalDir,
@@ -234,6 +267,26 @@ func (s *NodeScanner) scanPnpmGlobal(ctx context.Context) (model.NodeScanResult,
 	}, true
 }
 
+// defaultPnpmBinDir returns the default pnpm global bin directory for the current OS
+// based on environment variables.
+func defaultPnpmBinDir(exec executor.Executor) string {
+	switch exec.GOOS() {
+	case model.PlatformDarwin:
+		if home := exec.Getenv("HOME"); home != "" {
+			return filepath.Join(home, "Library", "pnpm", "bin")
+		}
+	case model.PlatformLinux:
+		if home := exec.Getenv("HOME"); home != "" {
+			return filepath.Join(home, ".local", "share", "pnpm")
+		}
+	case model.PlatformWindows:
+		if localAppData := exec.Getenv("LOCALAPPDATA"); localAppData != "" {
+			return filepath.Join(localAppData, "pnpm")
+		}
+	}
+	return ""
+}
+
 // projectEntry holds a discovered package.json with its modification time for sorting.
 type projectEntry struct {
 	dir     string
@@ -405,19 +458,3 @@ func isInsideNodeModules(projectDir string) bool {
 	normalized := strings.ReplaceAll(projectDir, "\\", "/")
 	return strings.Contains(normalized, "/node_modules/")
 }
-
-// pnpmDepthArg picks the `--depth` arg for `pnpm list -g` based on the pnpm
-// version. pnpm v11 exits non-zero when `--depth=3` is passed to a global
-// scan; `--depth=Infinity` works on v11 and preserves transitive depth.
-// Falls back to `--depth=3` for older / unparseable versions to preserve
-// existing behavior.
-func pnpmDepthArg(version string) string {
-	v := strings.TrimSpace(version)
-	v = strings.TrimPrefix(v, "v")
-	major, _, _ := strings.Cut(v, ".")
-	n, err := strconv.Atoi(major)
-	if err == nil && n >= 11 {
-		return "--depth=Infinity"
-	}
-	return "--depth=3"
-}

internal/detector/nodescan_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/step-security/dev-machine-guard/internal/executor"
+	"github.com/step-security/dev-machine-guard/internal/model"
 	"github.com/step-security/dev-machine-guard/internal/progress"
 )
 
@@ -122,7 +123,7 @@ func TestNodeScanner_ScanPnpmGlobal_Windows(t *testing.T) {
 	// filepath.Dir on it. Since filepath.Dir is host-OS dependent, we use
 	// forward slashes here so the test works on macOS hosts too.
 	mock.SetCommand("C:/Users/dev/AppData/Local/pnpm/global/5/node_modules\n", "", 0, "pnpm", "root", "-g")
-	mock.SetCommand(`{"dependencies":{"typescript":{"version":"5.4.0"}}}`, "", 0, "pnpm", "list", "-g", "--json", "--depth=3")
+	mock.SetCommand(`{"dependencies":{"typescript":{"version":"5.4.0"}}}`, "", 0, "pnpm", "list", "-g", "--json")
 
 	scanner := newTestScanner(mock)
 	results := scanner.ScanGlobalPackages(context.Background())
@@ -139,13 +140,128 @@ func TestNodeScanner_ScanPnpmGlobal_Windows(t *testing.T) {
 			if r.PMVersion != "9.1.0" {
 				t.Errorf("expected PMVersion 9.1.0, got %s", r.PMVersion)
 			}
+			if r.ExitCode != 0 {
+				t.Errorf("expected ExitCode 0 (mock stub matched), got %d — check that production args still match the SetCommand stub", r.ExitCode)
+			}
 		}
 	}
 	if !pnpmFound {
 		t.Fatal("expected pnpm in global scan results on Windows")
 	}
 }
 
+// TestNodeScanner_ScanPnpmGlobal_Delegated exercises the root → user delegation
+// path (macOS-as-root or Linux-as-root with a logged-in user). Verifies that
+// the inline `PATH='…':$PATH pnpm <args>` shell prefix reaches the delegated
+// command intact. Without this prefix, pnpm v11 hard-fails each `-g` call on
+// hosts where sudo strips the caller's PATH (Linux `secure_path`, hardened
+// macOS sudoers).
+func TestNodeScanner_ScanPnpmGlobal_Delegated(t *testing.T) {
+	mock := executor.NewMock()
+	mock.SetGOOS("darwin")
+	mock.SetIsRoot(true)
+	mock.SetEnv("HOME", "/Users/testuser")
+
+	// checkPath in delegation mode runs `which pnpm` through RunAsUser, which
+	// the Mock dispatches as `bash -c "<cmd>"`.
+	mock.SetCommand("/opt/homebrew/bin/pnpm\n", "", 0, "bash", "-c", "which pnpm")
+
+	// All pnpm calls in scanPnpmGlobal are built as
+	//   PATH='<extra>':$PATH pnpm <args>
+	// then sent through runCmd → RunAsUser → bash -c "<cmd>".
+	prefix := `PATH='/Users/testuser/Library/pnpm/bin':$PATH pnpm`
+	mock.SetCommand("11.1.2\n", "", 0, "bash", "-c", prefix+" --version")
+	mock.SetCommand("/Users/testuser/Library/pnpm/global/v11/node_modules\n", "", 0, "bash", "-c", prefix+" root -g")
+	mock.SetCommand(`{"dependencies":{"typescript":{"version":"5.4.0"}}}`, "", 0, "bash", "-c", prefix+" list -g --json")
+
+	log := progress.NewLogger(progress.LevelInfo)
+	scanner := NewNodeScanner(mock, log, "testuser")
+
+	results := scanner.ScanGlobalPackages(context.Background())
+
+	var pnpm *model.NodeScanResult
+	for i, r := range results {
+		if r.PackageManager == "pnpm" {
+			pnpm = &results[i]
+			break
+		}
+	}
+	if pnpm == nil {
+		t.Fatal("expected pnpm in delegated scan results")
+	}
+	if pnpm.PMVersion != "11.1.2" {
+		t.Errorf("PMVersion = %q, want 11.1.2 — PATH= prefix likely missing from `pnpm --version` invocation", pnpm.PMVersion)
+	}
+	if pnpm.ProjectPath != "/Users/testuser/Library/pnpm/global/v11" {
+		t.Errorf("ProjectPath = %q, want /Users/testuser/Library/pnpm/global/v11 — PATH= prefix likely missing from `pnpm root -g` invocation", pnpm.ProjectPath)
+	}
+	if pnpm.ExitCode != 0 {
+		t.Errorf("ExitCode = %d, want 0 — PATH= prefix likely missing from `pnpm list -g` invocation", pnpm.ExitCode)
+	}
+}
+
+// TestDefaultPnpmBinDir pins pnpm's per-platform global bin-dir layout. macOS
+// uses a /bin subdirectory; Linux and Windows place global binaries directly
+// in PNPM_HOME (no /bin). This asymmetry matches pnpm's own `pnpm setup`.
+func TestDefaultPnpmBinDir(t *testing.T) {
+	tests := []struct {
+		name string
+		goos string
+		envs map[string]string
+		want string
+	}{
+		{
+			name: "darwin with HOME → bin subdir",
+			goos: "darwin",
+			envs: map[string]string{"HOME": "/Users/foo"},
+			want: "/Users/foo/Library/pnpm/bin",
+		},
+		{
+			name: "darwin without HOME → empty",
+			goos: "darwin",
+			envs: map[string]string{},
+			want: "",
+		},
+		{
+			name: "linux with HOME → no bin suffix",
+			goos: "linux",
+			envs: map[string]string{"HOME": "/home/foo"},
+			want: "/home/foo/.local/share/pnpm",
+		},
+		{
+			name: "linux without HOME → empty",
+			goos: "linux",
+			envs: map[string]string{},
+			want: "",
+		},
+		{
+			name: "windows without LOCALAPPDATA → empty",
+			goos: "windows",
+			envs: map[string]string{},
+			want: "",
+		},
+		{
+			name: "unrecognized OS → empty",
+			goos: "freebsd",
+			envs: map[string]string{"HOME": "/home/foo"},
+			want: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mock := executor.NewMock()
+			mock.SetGOOS(tt.goos)
+			for k, v := range tt.envs {
+				mock.SetEnv(k, v)
+			}
+			got := defaultPnpmBinDir(mock)
+			if got != tt.want {
+				t.Errorf("defaultPnpmBinDir() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestNodeScanner_ScanProject_Windows(t *testing.T) {
 	mock := executor.NewMock()
 	mock.SetGOOS("windows")
@@ -206,89 +322,6 @@ func TestNodeScanner_ScanProject_YarnBerry_Windows(t *testing.T) {
 	}
 }
 
-func TestPnpmDepthArg(t *testing.T) {
-	tests := []struct {
-		version string
-		want    string
-	}{
-		{"", "--depth=3"},
-		{"unknown", "--depth=3"},
-		{"9.1.0", "--depth=3"},
-		{"10.12.4", "--depth=3"},
-		{"11.0.0", "--depth=Infinity"},
-		{"11.1.1", "--depth=Infinity"},
-		{"v12.0.0\n", "--depth=Infinity"},
-	}
-	for _, tt := range tests {
-		t.Run(tt.version, func(t *testing.T) {
-			got := pnpmDepthArg(tt.version)
-			if got != tt.want {
-				t.Errorf("pnpmDepthArg(%q) = %q, want %q", tt.version, got, tt.want)
-			}
-		})
-	}
-}
-
-func TestNodeScanner_ScanPnpmGlobal_V11(t *testing.T) {
-	mock := executor.NewMock()
-	mock.SetPath("pnpm", "/opt/homebrew/bin/pnpm")
-	mock.SetCommand("11.1.1\n", "", 0, "pnpm", "--version")
-	mock.SetCommand("/Users/dev/Library/pnpm/global/v11/abc/node_modules\n", "", 0, "pnpm", "root", "-g")
-	// v11 must use --depth=Infinity; --depth=3 would fail in real life.
-	mock.SetCommand(`{"dependencies":{"jest":{"version":"29.7.0"}}}`, "", 0, "pnpm", "list", "-g", "--json", "--depth=Infinity")
-
-	scanner := newTestScanner(mock)
-	results := scanner.ScanGlobalPackages(context.Background())
-
-	pnpmFound := false
-	for _, r := range results {
-		if r.PackageManager == "pnpm" {
-			pnpmFound = true
-			if r.PMVersion != "11.1.1" {
-				t.Errorf("expected PMVersion 11.1.1, got %s", r.PMVersion)
-			}
-			if r.ExitCode != 0 {
-				t.Errorf("expected ExitCode 0, got %d", r.ExitCode)
-			}
-			decoded, _ := base64.StdEncoding.DecodeString(r.RawStdoutBase64)
-			if len(decoded) == 0 {
-				t.Error("expected non-empty RawStdoutBase64")
-			}
-		}
-	}
-	if !pnpmFound {
-		t.Fatal("expected pnpm in global scan results for v11")
-	}
-}
-
-func TestNodeScanner_ScanPnpmGlobal_V10_StillUsesDepth3(t *testing.T) {
-	mock := executor.NewMock()
-	mock.SetPath("pnpm", "/usr/local/bin/pnpm")
-	mock.SetCommand("10.12.4\n", "", 0, "pnpm", "--version")
-	mock.SetCommand("/Users/dev/.local/share/pnpm/global/5/node_modules\n", "", 0, "pnpm", "root", "-g")
-	// v10 must still use --depth=3 (proves we didn't break the legacy path).
-	mock.SetCommand(`{"dependencies":{"typescript":{"version":"5.4.0"}}}`, "", 0, "pnpm", "list", "-g", "--json", "--depth=3")
-
-	scanner := newTestScanner(mock)
-	results := scanner.ScanGlobalPackages(context.Background())
-
-	pnpmFound := false
-	for _, r := range results {
-		if r.PackageManager == "pnpm" {
-			pnpmFound = true
-			if r.PMVersion != "10.12.4" {
-				t.Errorf("expected PMVersion 10.12.4, got %s", r.PMVersion)
-			}
-			if r.ExitCode != 0 {
-				t.Errorf("expected ExitCode 0, got %d", r.ExitCode)
-			}
-		}
-	}
-	if !pnpmFound {
-		t.Fatal("expected pnpm in global scan results for v10")
-	}
-}
-
 func TestNodeScanner_ScanGlobalPackages_NoneInstalled(t *testing.T) {
 	mock := executor.NewMock()
 	scanner := newTestScanner(mock)