Skip to content

Commit 8055c59

Browse files
fix: address Copilot PR review feedback
- Gate systemd install/uninstall to runtime.GOOS == "linux" with unsupported-platform error for other Unix targets - Fix lock_windows.go: treat ERROR_ACCESS_DENIED as process alive - Fix systemd.go: check daemon-reload exit code, escape paths with spaces in unit files - Fix syspkg.go: check exit codes on all RunWithTimeout calls - Fix ide.go resolveInstallDirFromBinary: correct control flow for resources/app/bin layout - Fix ide.go parseDesktopExec: handle wrapper commands (env VAR=...) by scanning for first absolute path - Fix ide.go resolveLinuxVersion: prefer binary inside installDir over PATH to avoid version mismatch
1 parent b2593a0 commit 8055c59

5 files changed

Lines changed: 81 additions & 40 deletions

File tree

cmd/stepsecurity-dev-machine-guard/main.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,14 @@ func main() {
108108
log.Error("%v", err)
109109
os.Exit(1)
110110
}
111-
default:
111+
case "linux":
112112
if err := systemd.Install(exec, log); err != nil {
113113
log.Error("%v", err)
114114
os.Exit(1)
115115
}
116+
default:
117+
log.Error("Scheduled installation is not supported on %s", runtime.GOOS)
118+
os.Exit(1)
116119
}
117120
log.Progress("Sending initial telemetry...")
118121
fmt.Println()
@@ -134,11 +137,14 @@ func main() {
134137
log.Error("%v", err)
135138
os.Exit(1)
136139
}
137-
default:
140+
case "linux":
138141
if err := systemd.Uninstall(exec, log); err != nil {
139142
log.Error("%v", err)
140143
os.Exit(1)
141144
}
145+
default:
146+
log.Error("Scheduled installation is not supported on %s", runtime.GOOS)
147+
os.Exit(1)
142148
}
143149

144150
default:

internal/detector/ide.go

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,24 @@ func (d *IDEDetector) detectLinux(ctx context.Context, spec ideSpec) (model.IDE,
302302
}
303303

304304
// resolveLinuxVersion determines the IDE version on Linux.
305-
// Tries: binary --version, then product-info.json (flat layout), then .eclipseproduct.
305+
// Prefers the binary within installDir to avoid version mismatch with a different
306+
// install found via PATH. Falls back to PATH lookup, then metadata files.
306307
func (d *IDEDetector) resolveLinuxVersion(ctx context.Context, spec ideSpec, installDir string) string {
307-
// Try binary --version from PATH first (most reliable on Linux)
308308
if spec.LinuxBinary != "" && spec.VersionFlag != "" {
309+
// Try binary inside the detected install directory first
310+
for _, relBin := range []string{
311+
filepath.Join("bin", spec.LinuxBinary),
312+
spec.LinuxBinary,
313+
} {
314+
localBin := filepath.Join(installDir, relBin)
315+
if d.exec.FileExists(localBin) {
316+
if v := runVersionCmd(ctx, d.exec, localBin, spec.VersionFlag); v != "unknown" {
317+
return v
318+
}
319+
}
320+
}
321+
322+
// Fall back to PATH (may be a different install, but better than unknown)
309323
if binPath, err := d.exec.LookPath(spec.LinuxBinary); err == nil {
310324
if v := runVersionCmd(ctx, d.exec, binPath, spec.VersionFlag); v != "unknown" {
311325
return v
@@ -572,46 +586,55 @@ func (d *IDEDetector) discoverViaDesktopFile(spec ideSpec, homeDir string) (stri
572586
}
573587

574588
// parseDesktopExec extracts the binary path from the first Exec= line in a .desktop file.
575-
// Strips field codes (%F, %U, etc.) and flags (--new-window, etc.).
589+
// Handles wrapper commands like "env VAR=val /snap/bin/code %U" by scanning tokens
590+
// for the first absolute path.
576591
func parseDesktopExec(content string) string {
577592
for _, line := range strings.Split(content, "\n") {
578593
line = strings.TrimSpace(line)
579594
if !strings.HasPrefix(line, "Exec=") {
580595
continue
581596
}
582597
execValue := strings.TrimPrefix(line, "Exec=")
583-
// The first token is the binary path; the rest are arguments
584598
parts := strings.Fields(execValue)
585-
if len(parts) == 0 {
586-
continue
587-
}
588-
binPath := parts[0]
589-
// Skip entries that are just bare command names (no path)
590-
if !strings.Contains(binPath, "/") {
591-
continue
599+
600+
// Find the first token that looks like an absolute path
601+
for _, token := range parts {
602+
// Skip field codes (%F, %U, etc.) and flags (--foo)
603+
if strings.HasPrefix(token, "%") || strings.HasPrefix(token, "-") {
604+
continue
605+
}
606+
// Skip env var assignments (VAR=val)
607+
if strings.Contains(token, "=") && !strings.HasPrefix(token, "/") {
608+
continue
609+
}
610+
if strings.HasPrefix(token, "/") {
611+
return token
612+
}
592613
}
593-
return binPath
594614
}
595615
return ""
596616
}
597617

598618
// resolveInstallDirFromBinary walks up from a binary path to find the app root directory.
599-
// Strips known bin subdirectories: /bin/, /resources/app/bin/.
619+
// Handles two common layouts:
620+
// - /usr/share/code/bin/code -> /usr/share/code
621+
// - /usr/share/cursor/resources/app/bin/cursor -> /usr/share/cursor
600622
func resolveInstallDirFromBinary(binPath string) string {
601623
dir := filepath.Dir(binPath)
602624
base := filepath.Base(dir)
603625

604-
// /usr/share/code/bin/code -> /usr/share/code
605-
// /opt/Windsurf/bin/windsurf -> /opt/Windsurf
606-
if base == "bin" {
607-
return filepath.Dir(dir)
626+
if base != "bin" {
627+
return dir
608628
}
629+
630+
parent := filepath.Dir(dir)
631+
632+
// Check for resources/app/bin layout (Electron apps like Cursor)
609633
// /usr/share/cursor/resources/app/bin/cursor -> /usr/share/cursor
610-
if base == "bin" || filepath.Base(filepath.Dir(dir)) == "app" {
611-
candidate := filepath.Dir(filepath.Dir(filepath.Dir(dir)))
612-
if candidate != "" && candidate != "." {
613-
return candidate
614-
}
634+
if filepath.Base(parent) == "app" && filepath.Base(filepath.Dir(parent)) == "resources" {
635+
return filepath.Dir(filepath.Dir(parent))
615636
}
616-
return dir
637+
638+
// Simple /bin/ layout: /usr/share/code/bin/code -> /usr/share/code
639+
return parent
617640
}

internal/detector/syspkg.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ func (d *SystemPkgDetector) Detect(ctx context.Context) *model.PkgManager {
7272
}
7373

7474
version := "unknown"
75-
stdout, _, _, err := d.exec.RunWithTimeout(ctx, 10*time.Second, spec.Binary, spec.VersionCmd...)
76-
if err == nil {
75+
stdout, _, exitCode, err := d.exec.RunWithTimeout(ctx, 10*time.Second, spec.Binary, spec.VersionCmd...)
76+
if err == nil && exitCode == 0 {
7777
if line := strings.TrimSpace(strings.SplitN(stdout, "\n", 2)[0]); line != "" {
7878
version = line
7979
}
@@ -101,8 +101,8 @@ func (d *SystemPkgDetector) ListPackages(ctx context.Context) []model.SystemPack
101101
continue
102102
}
103103

104-
stdout, _, _, err := d.exec.RunWithTimeout(ctx, 60*time.Second, spec.Binary, spec.ListCmd...)
105-
if err != nil {
104+
stdout, _, exitCode, err := d.exec.RunWithTimeout(ctx, 60*time.Second, spec.Binary, spec.ListCmd...)
105+
if err != nil || exitCode != 0 {
106106
return nil
107107
}
108108

@@ -158,8 +158,8 @@ func (d *SystemPkgDetector) DetectAdditionalManagers(ctx context.Context) []mode
158158
}
159159

160160
version := "unknown"
161-
stdout, _, _, err := d.exec.RunWithTimeout(ctx, 10*time.Second, pm.binary, pm.versionCmd...)
162-
if err == nil {
161+
stdout, _, exitCode, err := d.exec.RunWithTimeout(ctx, 10*time.Second, pm.binary, pm.versionCmd...)
162+
if err == nil && exitCode == 0 {
163163
if line := strings.TrimSpace(strings.SplitN(stdout, "\n", 2)[0]); line != "" {
164164
version = line
165165
}
@@ -181,8 +181,8 @@ func (d *SystemPkgDetector) ListSnapPackages(ctx context.Context) []model.System
181181
return nil
182182
}
183183

184-
stdout, _, _, err := d.exec.RunWithTimeout(ctx, 30*time.Second, "snap", "list")
185-
if err != nil {
184+
stdout, _, exitCode, err := d.exec.RunWithTimeout(ctx, 30*time.Second, "snap", "list")
185+
if err != nil || exitCode != 0 {
186186
return nil
187187
}
188188

@@ -209,9 +209,9 @@ func (d *SystemPkgDetector) ListFlatpakPackages(ctx context.Context) []model.Sys
209209
return nil
210210
}
211211

212-
stdout, _, _, err := d.exec.RunWithTimeout(ctx, 30*time.Second,
212+
stdout, _, exitCode, err := d.exec.RunWithTimeout(ctx, 30*time.Second,
213213
"flatpak", "list", "--app", "--columns=application,version")
214-
if err != nil {
214+
if err != nil || exitCode != 0 {
215215
return nil
216216
}
217217

internal/lock/lock_windows.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ var lockFilePath = filepath.Join(os.TempDir(), "stepsecurity-dev-machine-guard.l
1313

1414
// isProcessAlive checks if a process with the given PID exists by attempting
1515
// to open a handle to it. This avoids shelling out to tasklist.
16+
// Access-denied means the process exists but is owned by another user/session.
1617
func isProcessAlive(pid int) bool {
1718
h, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
1819
if err != nil {
19-
return false
20+
// ERROR_ACCESS_DENIED means the process exists but we can't open it
21+
return err == windows.ERROR_ACCESS_DENIED
2022
}
2123
_ = windows.CloseHandle(h)
2224
return true

internal/systemd/systemd.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ func Install(exec executor.Executor, log *progress.Logger) error {
5252
}
5353

5454
data := unitTemplateData{
55-
BinaryPath: binaryPath,
56-
LogDir: logDir,
55+
BinaryPath: systemdEscape(binaryPath),
56+
LogDir: systemdEscape(logDir),
5757
Hours: hours,
5858
}
5959

@@ -70,9 +70,13 @@ func Install(exec executor.Executor, log *progress.Logger) error {
7070
}
7171

7272
// Reload and enable
73-
if _, _, _, err := exec.Run(ctx, "systemctl", "--user", "daemon-reload"); err != nil {
73+
_, daemonStderr, daemonExitCode, err := exec.Run(ctx, "systemctl", "--user", "daemon-reload")
74+
if err != nil {
7475
return fmt.Errorf("daemon-reload failed: %w", err)
7576
}
77+
if daemonExitCode != 0 {
78+
return fmt.Errorf("daemon-reload failed (exit code %d): %s", daemonExitCode, daemonStderr)
79+
}
7680

7781
_, stderr, exitCode, err := exec.Run(ctx, "systemctl", "--user", "enable", "--now", unitName+".timer")
7882
if err != nil {
@@ -149,11 +153,17 @@ func writeTemplate(path, tmplStr string, data unitTemplateData) error {
149153
}
150154

151155
type unitTemplateData struct {
152-
BinaryPath string
156+
BinaryPath string // systemd-escaped (spaces replaced with \x20)
153157
LogDir string
154158
Hours int
155159
}
156160

161+
// systemdEscape escapes a file path for use in systemd unit files.
162+
// Spaces must be escaped as \x20 in ExecStart and related directives.
163+
func systemdEscape(path string) string {
164+
return strings.ReplaceAll(path, " ", `\x20`)
165+
}
166+
157167
const serviceTmpl = `[Unit]
158168
Description=StepSecurity Dev Machine Guard scan
159169

0 commit comments

Comments
 (0)