Skip to content

Commit 35d905d

Browse files
fallback logic
Signed-off-by: prateek-stepsecurity <prateek@stepsecurity.io>
1 parent ce70bf6 commit 35d905d

2 files changed

Lines changed: 106 additions & 41 deletions

File tree

internal/detector/nodescan.go

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -203,48 +203,61 @@ func (s *NodeScanner) scanPnpmGlobal(ctx context.Context) (model.NodeScanResult,
203203
return model.NodeScanResult{}, false
204204
}
205205

206-
// pnpm v11 hard-fails any `-g` invocation when its user-local bin dir is
207-
// not on PATH. Two execution paths to handle:
208-
// - In-process (exec.Command): prepend the bin dir to this Go process's
209-
// PATH so the child inherits it.
210-
// - Root → user delegation (sudo -u): inline `PATH='…':$PATH` into the
211-
// shell command itself. This works regardless of sudo's env policy
212-
// (macOS keeps PATH via env_keep; Linux strips it via secure_path),
213-
// because the env-prefix is honored by the user's shell AFTER sudo
214-
// has already done whatever it does.
215-
extra := defaultPnpmBinDir(s.exec)
216-
if extra != "" {
217-
oldPath := os.Getenv("PATH")
218-
_ = os.Setenv("PATH", extra+string(os.PathListSeparator)+oldPath)
219-
defer os.Setenv("PATH", oldPath)
220-
}
221-
222-
// For the delegation path, embed `PATH='extra':$PATH` in the command name.
223-
// runCmd's delegation branch space-joins name+args into the shell command
224-
// string, so the env-prefix flows through to the user's shell intact.
225-
// Applied to every pnpm `-g` call, since v11 enforces the bin-dir check
226-
// on each one.
227206
pnpmCmd := "pnpm"
228-
if s.shouldRunAsUser() && extra != "" {
229-
pnpmCmd = "PATH=" + platformShellQuote(s.exec, extra) + ":$PATH pnpm"
230-
}
231207

232208
versionOut, _, _, verErr := s.runCmd(ctx, 10*time.Second, pnpmCmd, "--version")
233209
version := strings.TrimSpace(versionOut)
234210
if verErr != nil || version == "" {
235211
version = "unknown"
236212
}
237213

238-
rootOut, _, _, _ := s.runCmd(ctx, 10*time.Second, pnpmCmd, "root", "-g")
214+
rootOut, _, rootExit, _ := s.runCmd(ctx, 10*time.Second, pnpmCmd, "root", "-g")
239215
globalDir := strings.TrimSpace(rootOut)
240-
if globalDir == "" {
241-
s.log.Warn("pnpm found but `pnpm root -g` returned empty — skipping pnpm global scan")
216+
217+
// fallback logic in case `pnpm root -g` returns empty
218+
var extra string
219+
if rootExit != 0 || globalDir == "" {
220+
extra = defaultPnpmBinDir(s.exec)
221+
if extra != "" {
222+
oldPath := os.Getenv("PATH")
223+
_ = os.Setenv("PATH", extra+string(os.PathListSeparator)+oldPath)
224+
defer os.Setenv("PATH", oldPath)
225+
226+
// For the delegation path, embed `PATH='extra':$PATH` in the command name.
227+
// runCmd's delegation branch space-joins name+args into the shell command
228+
// string, so the env-prefix flows through to the user's shell intact.
229+
if s.shouldRunAsUser() {
230+
pnpmCmd = "PATH=" + platformShellQuote(s.exec, extra) + ":$PATH pnpm"
231+
}
232+
233+
s.log.Debug("pnpm root -g returned empty; retrying with bin dir %q prepended to PATH", extra)
234+
rootOut, _, _, _ = s.runCmd(ctx, 10*time.Second, pnpmCmd, "root", "-g")
235+
globalDir = strings.TrimSpace(rootOut)
236+
}
237+
}
238+
239+
if globalDir != "" {
240+
globalDir = filepath.Dir(globalDir)
241+
} else if extra != "" {
242+
// Both attempts failed; use the bin dir itself as last-resort
243+
// ProjectPath so we still produce a result rather than dropping the
244+
// scan entirely.
245+
s.log.Debug("pnpm root -g still empty after PATH workaround; using defaultPnpmBinDir=%q", extra)
246+
globalDir = extra
247+
} else {
248+
s.log.Warn("pnpm found but `pnpm root -g` returned empty and no fallback available — skipping pnpm global scan")
242249
return model.NodeScanResult{}, false
243250
}
244-
globalDir = filepath.Dir(globalDir)
245251

252+
// Try with --depth=3 first for transitive coverage (works on pnpm v10).
253+
// Fall back to no --depth on non-zero exit — pnpm v11 hard-fails any
254+
// --depth>=1 on -g and pnpm itself recommends omitting --depth.
246255
start := time.Now()
247-
stdout, stderr, exitCode, err := s.runCmd(ctx, 60*time.Second, pnpmCmd, "list", "-g", "--json")
256+
stdout, stderr, exitCode, err := s.runCmd(ctx, 60*time.Second, pnpmCmd, "list", "-g", "--json", "--depth=3")
257+
if exitCode != 0 {
258+
s.log.Debug("pnpm list -g --depth=3 failed (exit=%d) — retrying without --depth (v11 path)", exitCode)
259+
stdout, stderr, exitCode, err = s.runCmd(ctx, 60*time.Second, pnpmCmd, "list", "-g", "--json")
260+
}
248261
duration := time.Since(start).Milliseconds()
249262

250263
errMsg := ""

internal/detector/nodescan_test.go

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,12 @@ func TestNodeScanner_ScanPnpmGlobal_Windows(t *testing.T) {
121121
mock.SetCommand("9.1.0\n", "", 0, "pnpm", "--version")
122122
// pnpm root -g returns the global node_modules dir. The code calls
123123
// filepath.Dir on it. Since filepath.Dir is host-OS dependent, we use
124-
// forward slashes here so the test works on macOS hosts too.
124+
// forward slashes here so the test works on macOS hosts too. First
125+
// attempt succeeds so the PATH workaround is skipped on this path.
125126
mock.SetCommand("C:/Users/dev/AppData/Local/pnpm/global/5/node_modules\n", "", 0, "pnpm", "root", "-g")
127+
// Production tries `--depth=3` first (v10 transitive), falls back to no --depth
128+
// on non-zero exit (v11 path). Stub both legs so the fallback is verified.
129+
mock.SetCommand("", "ERR_PNPM_GLOBAL_LS_DEPTH_NOT_SUPPORTED", 1, "pnpm", "list", "-g", "--json", "--depth=3")
126130
mock.SetCommand(`{"dependencies":{"typescript":{"version":"5.4.0"}}}`, "", 0, "pnpm", "list", "-g", "--json")
127131

128132
scanner := newTestScanner(mock)
@@ -151,11 +155,13 @@ func TestNodeScanner_ScanPnpmGlobal_Windows(t *testing.T) {
151155
}
152156

153157
// TestNodeScanner_ScanPnpmGlobal_Delegated exercises the root → user delegation
154-
// path (macOS-as-root or Linux-as-root with a logged-in user). Verifies that
155-
// the inline `PATH='…':$PATH pnpm <args>` shell prefix reaches the delegated
156-
// command intact. Without this prefix, pnpm v11 hard-fails each `-g` call on
157-
// hosts where sudo strips the caller's PATH (Linux `secure_path`, hardened
158-
// macOS sudoers).
158+
// path (macOS-as-root or Linux-as-root with a logged-in user). Verifies the
159+
// lazy-fallback flow:
160+
// - `pnpm --version` runs plainly (doesn't need bin dir on PATH).
161+
// - First `pnpm root -g` runs plainly; on failure the scanner applies the
162+
// inline `PATH='…':$PATH pnpm` workaround and retries.
163+
// - `pnpm list -g` then uses the same prefixed pnpmCmd, so it survives sudo's
164+
// env policy (Linux `secure_path` or hardened macOS sudoers).
159165
func TestNodeScanner_ScanPnpmGlobal_Delegated(t *testing.T) {
160166
mock := executor.NewMock()
161167
mock.SetGOOS("darwin")
@@ -166,12 +172,19 @@ func TestNodeScanner_ScanPnpmGlobal_Delegated(t *testing.T) {
166172
// the Mock dispatches as `bash -c "<cmd>"`.
167173
mock.SetCommand("/opt/homebrew/bin/pnpm\n", "", 0, "bash", "-c", "which pnpm")
168174

169-
// All pnpm calls in scanPnpmGlobal are built as
170-
// PATH='<extra>':$PATH pnpm <args>
171-
// then sent through runCmd → RunAsUser → bash -c "<cmd>".
175+
// `pnpm --version` is called plainly (no prefix) — it doesn't need the
176+
// bin dir on PATH.
177+
mock.SetCommand("11.1.2\n", "", 0, "bash", "-c", "pnpm --version")
178+
179+
// First `pnpm root -g` attempt runs plainly; v11 errors when bin dir not on PATH.
180+
mock.SetCommand("", "ERR_PNPM_GLOBAL_LS_DEPTH_NOT_SUPPORTED", 1, "bash", "-c", "pnpm root -g")
181+
182+
// Production then applies the workaround and retries with the inline PATH= prefix.
172183
prefix := `PATH='/Users/testuser/Library/pnpm/bin':$PATH pnpm`
173-
mock.SetCommand("11.1.2\n", "", 0, "bash", "-c", prefix+" --version")
174184
mock.SetCommand("/Users/testuser/Library/pnpm/global/v11/node_modules\n", "", 0, "bash", "-c", prefix+" root -g")
185+
186+
// `pnpm list -g` tries with --depth=3 first; v11 path errors → fall back to no --depth.
187+
mock.SetCommand("", "ERR_PNPM_GLOBAL_LS_DEPTH_NOT_SUPPORTED", 1, "bash", "-c", prefix+" list -g --json --depth=3")
175188
mock.SetCommand(`{"dependencies":{"typescript":{"version":"5.4.0"}}}`, "", 0, "bash", "-c", prefix+" list -g --json")
176189

177190
log := progress.NewLogger(progress.LevelInfo)
@@ -190,16 +203,55 @@ func TestNodeScanner_ScanPnpmGlobal_Delegated(t *testing.T) {
190203
t.Fatal("expected pnpm in delegated scan results")
191204
}
192205
if pnpm.PMVersion != "11.1.2" {
193-
t.Errorf("PMVersion = %q, want 11.1.2 — PATH= prefix likely missing from `pnpm --version` invocation", pnpm.PMVersion)
206+
t.Errorf("PMVersion = %q, want 11.1.2 — `pnpm --version` should run plainly without PATH prefix", pnpm.PMVersion)
194207
}
195208
if pnpm.ProjectPath != "/Users/testuser/Library/pnpm/global/v11" {
196-
t.Errorf("ProjectPath = %q, want /Users/testuser/Library/pnpm/global/v11 — PATH= prefix likely missing from `pnpm root -g` invocation", pnpm.ProjectPath)
209+
t.Errorf("ProjectPath = %q, want /Users/testuser/Library/pnpm/global/v11 — PATH= prefix likely missing from `pnpm root -g` retry", pnpm.ProjectPath)
197210
}
198211
if pnpm.ExitCode != 0 {
199212
t.Errorf("ExitCode = %d, want 0 — PATH= prefix likely missing from `pnpm list -g` invocation", pnpm.ExitCode)
200213
}
201214
}
202215

216+
// TestNodeScanner_ScanPnpmGlobal_RootGFallback verifies that when BOTH
217+
// `pnpm root -g` attempts fail (plain + with PATH workaround), the scan does
218+
// not bail out — it falls back to the platform-default bin dir
219+
// (defaultPnpmBinDir) as ProjectPath so the result is still produced.
220+
func TestNodeScanner_ScanPnpmGlobal_RootGFallback(t *testing.T) {
221+
mock := executor.NewMock()
222+
mock.SetGOOS("darwin")
223+
mock.SetEnv("HOME", "/Users/foo")
224+
mock.SetPath("pnpm", "/opt/homebrew/bin/pnpm")
225+
mock.SetCommand("11.1.2\n", "", 0, "pnpm", "--version")
226+
// pnpm root -g errors on every attempt — both the plain first call AND
227+
// the retry use the same plain `pnpm root -g` command, because
228+
// shouldRunAsUser is false on this in-process path (IsRoot not set).
229+
mock.SetCommand("", "ERR_PNPM_GLOBAL_LS_DEPTH_NOT_SUPPORTED", 1, "pnpm", "root", "-g")
230+
mock.SetCommand("", "ERR_PNPM_GLOBAL_LS_DEPTH_NOT_SUPPORTED", 1, "pnpm", "list", "-g", "--json", "--depth=3")
231+
mock.SetCommand(`{"dependencies":{"jest":{"version":"30.4.2"}}}`, "", 0, "pnpm", "list", "-g", "--json")
232+
233+
scanner := newTestScanner(mock)
234+
results := scanner.ScanGlobalPackages(context.Background())
235+
236+
var pnpm *model.NodeScanResult
237+
for i, r := range results {
238+
if r.PackageManager == "pnpm" {
239+
pnpm = &results[i]
240+
break
241+
}
242+
}
243+
if pnpm == nil {
244+
t.Fatal("expected pnpm in results — fallback should have prevented an early return")
245+
}
246+
// ProjectPath falls back to defaultPnpmBinDir on darwin = $HOME/Library/pnpm/bin.
247+
if pnpm.ProjectPath != "/Users/foo/Library/pnpm/bin" {
248+
t.Errorf("ProjectPath = %q, want /Users/foo/Library/pnpm/bin (defaultPnpmBinDir fallback)", pnpm.ProjectPath)
249+
}
250+
if pnpm.ExitCode != 0 {
251+
t.Errorf("ExitCode = %d, want 0 — `pnpm list -g` should have succeeded via the no-depth fallback", pnpm.ExitCode)
252+
}
253+
}
254+
203255
// TestDefaultPnpmBinDir pins pnpm's per-platform global bin-dir layout. macOS
204256
// uses a /bin subdirectory; Linux and Windows place global binaries directly
205257
// in PNPM_HOME (no /bin). This asymmetry matches pnpm's own `pnpm setup`.

0 commit comments

Comments
 (0)