Skip to content

Commit bd373c5

Browse files
fix(windows): use RunInDir for project package scanning
On Windows, Go's os/exec escapes double quotes with backslashes when passing arguments to cmd.exe, but cmd.exe treats \ as a path separator. This caused every "cd <path> && npm ls" command to fail with "The filename, directory name, or volume label syntax is incorrect." Result: all project-level NPM packages were missing from telemetry on Windows (only global packages were collected). Fix: Add RunInDir to the Executor interface which sets exec.Cmd.Dir directly, bypassing the shell entirely. Update scanProject() and scanYarnGlobal() to use RunInDir instead of shell cd.
1 parent 76b4e51 commit bd373c5

5 files changed

Lines changed: 48 additions & 24 deletions

File tree

internal/detector/nodescan.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,15 @@ func (s *NodeScanner) runCmd(ctx context.Context, timeout time.Duration, name st
7070

7171
// runShellCmd runs a shell command string, delegating to the logged-in user when running as root.
7272
// Falls through to the platform-aware free function for the normal (non-delegation) path.
73-
func (s *NodeScanner) runShellCmd(ctx context.Context, timeout time.Duration, shellCmd string) (string, string, int, error) {
73+
func (s *NodeScanner) runInDir(ctx context.Context, dir string, timeout time.Duration, name string, args ...string) (string, string, int, error) {
7474
if s.shouldRunAsUser() {
7575
ctx, cancel := context.WithTimeout(ctx, timeout)
7676
defer cancel()
77-
stdout, err := s.exec.RunAsUser(ctx, s.loggedInUser, shellCmd)
77+
cmd := "cd " + platformShellQuote(s.exec, dir) + " && " + name
78+
for _, a := range args {
79+
cmd += " " + a
80+
}
81+
stdout, err := s.exec.RunAsUser(ctx, s.loggedInUser, cmd)
7882
if err != nil {
7983
if ctx.Err() == context.DeadlineExceeded {
8084
return stdout, "", 124, fmt.Errorf("command timed out after %s", timeout)
@@ -83,9 +87,10 @@ func (s *NodeScanner) runShellCmd(ctx context.Context, timeout time.Duration, sh
8387
}
8488
return stdout, "", 0, nil
8589
}
86-
return runShellCmd(ctx, s.exec, timeout, shellCmd)
90+
return s.exec.RunInDir(ctx, dir, timeout, name, args...)
8791
}
8892

93+
8994
// checkPath checks if a binary is available, using the logged-in user's PATH when running as root.
9095
func (s *NodeScanner) checkPath(ctx context.Context, name string) error {
9196
if s.shouldRunAsUser() {
@@ -166,8 +171,8 @@ func (s *NodeScanner) scanYarnGlobal(ctx context.Context) (model.NodeScanResult,
166171
}
167172

168173
start := time.Now()
169-
shellCmd := "cd " + platformShellQuote(s.exec, globalDir) + " && yarn list --json --depth=0"
170-
stdout, stderr, exitCode, _ := s.runShellCmd(ctx, 60*time.Second, shellCmd)
174+
// Run directly in the global dir instead of shell cd (avoids Windows quoting issues).
175+
stdout, stderr, exitCode, _ := s.runInDir(ctx, globalDir, 60*time.Second, "yarn", "list", "--json", "--depth=0")
171176
duration := time.Since(start).Milliseconds()
172177

173178
errMsg := ""
@@ -343,11 +348,10 @@ func (s *NodeScanner) scanProject(ctx context.Context, projectDir string) model.
343348
}
344349

345350
start := time.Now()
346-
cmdStr := "cd " + platformShellQuote(s.exec, projectDir) + " && " + cmd
347-
for _, a := range args {
348-
cmdStr += " " + a
349-
}
350-
stdout, stderr, exitCode, _ := s.runShellCmd(ctx, 30*time.Second, cmdStr)
351+
// Run the package manager command directly in the project directory.
352+
// Avoids shell cd + quoting issues on Windows where cmd.exe misinterprets
353+
// Go's backslash-escaped quotes in paths.
354+
stdout, stderr, exitCode, _ := s.runInDir(ctx, projectDir, 30*time.Second, cmd, args...)
351355
duration := time.Since(start).Milliseconds()
352356

353357
errMsg := ""

internal/detector/nodescan_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,9 @@ func TestNodeScanner_ScanProject_Windows(t *testing.T) {
154154
// DetectProjectPM uses filepath.Join which is host-dependent;
155155
// construct the mock file path the same way the code will.
156156
mock.SetFile(filepath.Join(`C:\Users\dev\myapp`, "package-lock.json"), []byte{})
157+
// RunInDir calls Run(name, args...) directly — no shell cd needed
157158
mock.SetCommand(`{"dependencies":{"lodash":{"version":"4.17.21"}}}`, "", 0,
158-
"cmd", "/c", `cd "C:\Users\dev\myapp" && npm ls --json --depth=3`)
159+
"npm", "ls", "--json", "--depth=3")
159160

160161
scanner := newTestScanner(mock)
161162
result := scanner.scanProject(context.Background(), `C:\Users\dev\myapp`)
@@ -187,8 +188,9 @@ func TestNodeScanner_ScanProject_YarnBerry_Windows(t *testing.T) {
187188
projectDir := `C:\Users\dev\myapp`
188189
mock.SetFile(filepath.Join(projectDir, "yarn.lock"), []byte{})
189190
mock.SetFile(filepath.Join(projectDir, ".yarnrc.yml"), []byte{})
191+
// RunInDir calls Run(name, args...) directly — no shell cd needed
190192
mock.SetCommand(`{"name":"myapp","children":[]}`, "", 0,
191-
"cmd", "/c", `cd "C:\Users\dev\myapp" && yarn info --all --json`)
193+
"yarn", "info", "--all", "--json")
192194

193195
scanner := newTestScanner(mock)
194196
result := scanner.scanProject(context.Background(), projectDir)

internal/detector/shellcmd.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,11 @@
11
package detector
22

33
import (
4-
"context"
54
"strings"
6-
"time"
75

86
"github.com/step-security/dev-machine-guard/internal/executor"
97
)
108

11-
// runShellCmd runs a shell command string using the platform-appropriate shell.
12-
// On Unix: bash -c "command"
13-
// On Windows: cmd /c "command"
14-
func runShellCmd(ctx context.Context, exec executor.Executor, timeout time.Duration, command string) (string, string, int, error) {
15-
if exec.GOOS() == "windows" {
16-
return exec.RunWithTimeout(ctx, timeout, "cmd", "/c", command)
17-
}
18-
return exec.RunWithTimeout(ctx, timeout, "bash", "-c", command)
19-
}
20-
219
// platformShellQuote quotes a string for use in a shell command.
2210
// On Unix: single quotes with escaping.
2311
// On Windows: double quotes with escaping.

internal/executor/executor.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ type Executor interface {
2020
Run(ctx context.Context, name string, args ...string) (stdout, stderr string, exitCode int, err error)
2121
// RunWithTimeout executes a command with a timeout.
2222
RunWithTimeout(ctx context.Context, timeout time.Duration, name string, args ...string) (stdout, stderr string, exitCode int, err error)
23+
// RunInDir executes a command with a working directory and timeout.
24+
// Avoids shell quoting issues with cd on Windows.
25+
RunInDir(ctx context.Context, dir string, timeout time.Duration, name string, args ...string) (stdout, stderr string, exitCode int, err error)
2326
// RunAsUser runs a shell command as a specific user (for root -> user delegation).
2427
RunAsUser(ctx context.Context, username, command string) (string, error)
2528
// LookPath searches for an executable in PATH.
@@ -87,6 +90,29 @@ func (r *Real) RunWithTimeout(ctx context.Context, timeout time.Duration, name s
8790
return stdout, stderr, code, err
8891
}
8992

93+
func (r *Real) RunInDir(ctx context.Context, dir string, timeout time.Duration, name string, args ...string) (string, string, int, error) {
94+
ctx, cancel := context.WithTimeout(ctx, timeout)
95+
defer cancel()
96+
cmd := exec.CommandContext(ctx, name, args...)
97+
cmd.Dir = dir
98+
var stdout, stderr bytes.Buffer
99+
cmd.Stdout = &stdout
100+
cmd.Stderr = &stderr
101+
err := cmd.Run()
102+
exitCode := 0
103+
if err != nil {
104+
if exitErr, ok := err.(*exec.ExitError); ok {
105+
exitCode = exitErr.ExitCode()
106+
} else {
107+
return stdout.String(), stderr.String(), -1, err
108+
}
109+
}
110+
if ctx.Err() == context.DeadlineExceeded {
111+
return stdout.String(), stderr.String(), 124, fmt.Errorf("command timed out after %s", timeout)
112+
}
113+
return stdout.String(), stderr.String(), exitCode, nil
114+
}
115+
90116
func (r *Real) LookPath(name string) (string, error) {
91117
return exec.LookPath(name)
92118
}

internal/executor/mock.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ func (m *Mock) RunWithTimeout(ctx context.Context, _ time.Duration, name string,
167167
return m.Run(ctx, name, args...)
168168
}
169169

170+
func (m *Mock) RunInDir(ctx context.Context, _ string, _ time.Duration, name string, args ...string) (string, string, int, error) {
171+
return m.Run(ctx, name, args...)
172+
}
173+
170174
func (m *Mock) RunAsUser(ctx context.Context, _ string, command string) (string, error) {
171175
stdout, _, _, err := m.Run(ctx, "bash", "-c", command)
172176
return stdout, err

0 commit comments

Comments
 (0)