diff --git a/.gitignore b/.gitignore index e22ce1e..41943b8 100644 --- a/.gitignore +++ b/.gitignore @@ -20,11 +20,18 @@ !docs/**/*.html !images/**/*.html -# Go build artifacts — never commit compiled binaries -**/stepsecurity-dev-machine-guard +# Go build artifacts — never commit compiled binaries. +# Two explicit paths: the intended root output, and the stray output +# `go build` produces when run from inside the source dir. +/stepsecurity-dev-machine-guard +cmd/stepsecurity-dev-machine-guard/stepsecurity-dev-machine-guard *.exe dist/ stepsecurity-dev-machine-guard-linux # Temporary files todo-remove/ + +# Agent runtime state — never tracked. Tests run from a subpkg can +# drop config.json / agent.error.log / etc. here. +**/.stepsecurity/ diff --git a/.goreleaser.yml b/.goreleaser.yml index 863127f..40fe5e6 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -20,6 +20,8 @@ builds: - -X github.com/step-security/dev-machine-guard/internal/buildinfo.GitCommit={{.FullCommit}} - -X github.com/step-security/dev-machine-guard/internal/buildinfo.ReleaseTag={{.Tag}} - -X github.com/step-security/dev-machine-guard/internal/buildinfo.ReleaseBranch={{.Branch}} + # Windows-only: GUI subsystem suppresses Task Scheduler console flash. + - '{{ if eq .Os "windows" }}-H windowsgui{{ end }}' env: - CGO_ENABLED=0 diff --git a/Makefile b/Makefile index 52aaf97..4e248ea 100644 --- a/Makefile +++ b/Makefile @@ -14,11 +14,13 @@ LDFLAGS := -s -w \ build: go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY) ./cmd/stepsecurity-dev-machine-guard +# -H windowsgui prevents Task Scheduler from allocating a console. +# AttachParentConsole at startup restores stdio for interactive use. build-windows: - GOOS=windows GOARCH=amd64 go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY).exe ./cmd/stepsecurity-dev-machine-guard + GOOS=windows GOARCH=amd64 go build -trimpath -ldflags "$(LDFLAGS) -H windowsgui" -o $(BINARY).exe ./cmd/stepsecurity-dev-machine-guard build-windows-arm64: - GOOS=windows GOARCH=arm64 go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY)-arm64.exe ./cmd/stepsecurity-dev-machine-guard + GOOS=windows GOARCH=arm64 go build -trimpath -ldflags "$(LDFLAGS) -H windowsgui" -o $(BINARY)-arm64.exe ./cmd/stepsecurity-dev-machine-guard build-linux: GOOS=linux GOARCH=amd64 go build -trimpath -ldflags "$(LDFLAGS)" -o $(BINARY)-linux ./cmd/stepsecurity-dev-machine-guard diff --git a/cmd/stepsecurity-dev-machine-guard/console_other.go b/cmd/stepsecurity-dev-machine-guard/console_other.go new file mode 100644 index 0000000..317d2a0 --- /dev/null +++ b/cmd/stepsecurity-dev-machine-guard/console_other.go @@ -0,0 +1,6 @@ +//go:build !windows + +package main + +// AttachParentConsole is a no-op on non-Windows. +func AttachParentConsole() {} diff --git a/cmd/stepsecurity-dev-machine-guard/console_windows.go b/cmd/stepsecurity-dev-machine-guard/console_windows.go new file mode 100644 index 0000000..1118d3b --- /dev/null +++ b/cmd/stepsecurity-dev-machine-guard/console_windows.go @@ -0,0 +1,41 @@ +//go:build windows + +package main + +import ( + "os" + "syscall" + + "golang.org/x/sys/windows" +) + +// AttachParentConsole re-wires os.Std* to the parent's console when +// one exists. The agent is GUI-subsystem (-H windowsgui) so Task +// Scheduler launches don't allocate a console — the no-flash property. +// The cost is no inherited stdio; this restores it for interactive +// runs from cmd.exe / PowerShell. Under Task Scheduler the parent has +// no console and this no-ops. Must run before any logging. +// +// Quirks for interactive use (also documented in README): +// - Parent shell doesn't wait for GUI-subsystem children; output +// streams async below the prompt. Use `Start-Process -Wait`. +// - Pipes don't work — stdout is a console handle, not a pipe. +// - $LASTEXITCODE / %ERRORLEVEL% unreliable without -Wait. +func AttachParentConsole() { + const ATTACH_PARENT_PROCESS uint32 = 0xFFFFFFFF + + attach := windows.NewLazySystemDLL("kernel32.dll").NewProc("AttachConsole") + if r1, _, _ := attach.Call(uintptr(ATTACH_PARENT_PROCESS)); r1 == 0 { + return // no parent console; expected under Task Scheduler + } + + if h, err := syscall.Open("CONOUT$", syscall.O_RDWR, 0); err == nil { + os.Stdout = os.NewFile(uintptr(h), "/dev/stdout") + } + if h, err := syscall.Open("CONOUT$", syscall.O_RDWR, 0); err == nil { + os.Stderr = os.NewFile(uintptr(h), "/dev/stderr") + } + if h, err := syscall.Open("CONIN$", syscall.O_RDWR, 0); err == nil { + os.Stdin = os.NewFile(uintptr(h), "/dev/stdin") + } +} diff --git a/cmd/stepsecurity-dev-machine-guard/main.go b/cmd/stepsecurity-dev-machine-guard/main.go index d23442a..2c76ee0 100644 --- a/cmd/stepsecurity-dev-machine-guard/main.go +++ b/cmd/stepsecurity-dev-machine-guard/main.go @@ -38,6 +38,11 @@ import ( const hookReconcileTimeout = 30 * time.Second func main() { + // Windows GUI-subsystem build needs this to restore stdio for + // interactive runs. No-op under Task Scheduler / non-Windows. + // Must run before any logging. + AttachParentConsole() + // Hook hot path. Agents invoke `_hook` on every event and any non-zero // exit is treated as a hook failure / block — so we MUST exit 0 even on // malformed args. Skip every line below this branch (CLI parsing, diff --git a/internal/aiagents/enrich/npm/registry.go b/internal/aiagents/enrich/npm/registry.go index 7c3b8b8..4a1b7d4 100644 --- a/internal/aiagents/enrich/npm/registry.go +++ b/internal/aiagents/enrich/npm/registry.go @@ -7,6 +7,8 @@ import ( "os/exec" "path/filepath" "strings" + + "github.com/step-security/dev-machine-guard/internal/winproc" ) // Source identifies which command produced the resolution. Empty when @@ -25,6 +27,7 @@ var runFunc = execRun func execRun(ctx context.Context, cwd, bin string, args ...string) (string, error) { cmd := exec.CommandContext(ctx, bin, args...) + winproc.HideWindow(cmd) if cwd != "" { cmd.Dir = cwd } diff --git a/internal/config/config_windows.go b/internal/config/config_windows.go index 9696576..9b08257 100644 --- a/internal/config/config_windows.go +++ b/internal/config/config_windows.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" + "github.com/step-security/dev-machine-guard/internal/winproc" "golang.org/x/sys/windows" ) @@ -48,7 +49,9 @@ func hardenMachineConfigACL(path string) error { "/grant:r", "*S-1-5-32-545:R", // BUILTIN\Users = Read "/Q", } - output, err := exec.Command("icacls", args...).CombinedOutput() + cmd := exec.Command("icacls", args...) + winproc.HideWindow(cmd) + output, err := cmd.CombinedOutput() if err != nil { fmt.Fprintf(os.Stderr, "warning: icacls hardening of %q failed: %v\nicacls output:\n%s\n", diff --git a/internal/detector/ide.go b/internal/detector/ide.go index b5c5a49..0fa91c2 100644 --- a/internal/detector/ide.go +++ b/internal/detector/ide.go @@ -238,7 +238,7 @@ func (d *IDEDetector) detectDarwin(ctx context.Context, spec ideSpec) (model.IDE // Fallback: product-info.json (JetBrains IDEs) if version == "unknown" { - version = readProductInfoVersion(d.exec, filepath.Join(spec.AppPath, "Contents", "Resources", "product-info.json")) + version = readJSONVersion(d.exec, filepath.Join(spec.AppPath, "Contents", "Resources", "product-info.json")) } // Fallback: Info.plist @@ -328,7 +328,7 @@ func (d *IDEDetector) resolveLinuxVersion(ctx context.Context, spec ideSpec, ins } // product-info.json at the root of the install dir (JetBrains, some Electron apps) - if v := readProductInfoVersion(d.exec, filepath.Join(installDir, "product-info.json")); v != "unknown" { + if v := readJSONVersion(d.exec, filepath.Join(installDir, "product-info.json")); v != "unknown" { return v } @@ -378,9 +378,14 @@ func (d *IDEDetector) resolveWindowsVersion(ctx context.Context, spec ideSpec, i return version } -// resolveWindowsVersionFromDir tries binary, product-info.json, and .eclipseproduct. -// Does NOT query the registry (caller handles that to avoid redundant queries). +// resolveWindowsVersionFromDir tries package.json, the binary, +// product-info.json, .eclipseproduct (in order). package.json first +// avoids the bin\*.cmd shell-out for VS Code-family Electron IDEs. func (d *IDEDetector) resolveWindowsVersionFromDir(ctx context.Context, spec ideSpec, installDir string) string { + if v := readJSONVersion(d.exec, filepath.Join(installDir, "resources", "app", "package.json")); v != "unknown" { + return v + } + version := "unknown" if spec.WinBinary != "" && spec.VersionFlag != "" { @@ -391,7 +396,7 @@ func (d *IDEDetector) resolveWindowsVersionFromDir(ctx context.Context, spec ide } if version == "unknown" { - version = readProductInfoVersion(d.exec, filepath.Join(installDir, "product-info.json")) + version = readJSONVersion(d.exec, filepath.Join(installDir, "product-info.json")) } if version == "unknown" { @@ -487,9 +492,10 @@ func runVersionCmd(ctx context.Context, exec executor.Executor, binary, flag str return "unknown" } -// readProductInfoVersion reads the "version" field from a JetBrains product-info.json file. -// Returns "unknown" if the file does not exist or cannot be parsed. -func readProductInfoVersion(exec executor.Executor, filePath string) string { +// readJSONVersion reads top-level "version" from a JSON file (used +// for JetBrains product-info.json and VS Code-family package.json). +// Returns "unknown" if the file is missing or unparseable. +func readJSONVersion(exec executor.Executor, filePath string) string { data, err := exec.ReadFile(filePath) if err != nil { return "unknown" diff --git a/internal/detector/ide_test.go b/internal/detector/ide_test.go index b764aea..a409473 100644 --- a/internal/detector/ide_test.go +++ b/internal/detector/ide_test.go @@ -382,6 +382,33 @@ func TestIDEDetector_Windows_FindsEclipse_UserProfile_Glob(t *testing.T) { } } +// Version must come from package.json without shelling out to +// bin\code.cmd. No command is mocked for the binary; falling through +// fails the test. +func TestIDEDetector_Windows_VSCode_PackageJSONFastPath(t *testing.T) { + mock := executor.NewMock() + mock.SetGOOS("windows") + mock.SetEnv("LOCALAPPDATA", `C:\Users\testuser\AppData\Local`) + mock.SetEnv("PROGRAMFILES", `C:\Program Files`) + + vscodePath := `C:\Program Files\Microsoft VS Code` + mock.SetDir(vscodePath) + mock.SetFile(vscodePath+`/bin\code.cmd`, []byte{}) + mock.SetFile(vscodePath+`/resources/app/package.json`, + []byte(`{"name":"Code","version":"1.115.0"}`)) + + det := NewIDEDetector(mock) + results := det.Detect(context.Background()) + + found := findIDE(results, "vscode") + if found == nil { + t.Fatal("expected VS Code to be detected") + } + if found.Version != "1.115.0" { + t.Errorf("version should come from package.json (1.115.0), got %s", found.Version) + } +} + func TestIDEDetector_Windows_VSCode_StillWorks(t *testing.T) { mock := executor.NewMock() mock.SetGOOS("windows") @@ -441,7 +468,7 @@ func TestReadProductInfoVersion(t *testing.T) { mock.SetFile("/test/product-info.json", []byte(`{"name":"GoLand","version":"2025.1.3","buildNumber":"251.26927.50"}`)) - v := readProductInfoVersion(mock, "/test/product-info.json") + v := readJSONVersion(mock, "/test/product-info.json") if v != "2025.1.3" { t.Errorf("expected 2025.1.3, got %s", v) } @@ -449,7 +476,7 @@ func TestReadProductInfoVersion(t *testing.T) { func TestReadProductInfoVersion_MissingFile(t *testing.T) { mock := executor.NewMock() - v := readProductInfoVersion(mock, "/nonexistent/product-info.json") + v := readJSONVersion(mock, "/nonexistent/product-info.json") if v != "unknown" { t.Errorf("expected unknown, got %s", v) } @@ -459,7 +486,7 @@ func TestReadProductInfoVersion_InvalidJSON(t *testing.T) { mock := executor.NewMock() mock.SetFile("/test/product-info.json", []byte(`not json`)) - v := readProductInfoVersion(mock, "/test/product-info.json") + v := readJSONVersion(mock, "/test/product-info.json") if v != "unknown" { t.Errorf("expected unknown, got %s", v) } diff --git a/internal/executor/executor.go b/internal/executor/executor.go index ebe9a77..ace107e 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -12,6 +12,8 @@ import ( "strings" "sync" "time" + + "github.com/step-security/dev-machine-guard/internal/winproc" ) // Executor defines the interface for all OS interactions. @@ -82,6 +84,7 @@ func NewReal() *Real { return &Real{} } func (r *Real) Run(ctx context.Context, name string, args ...string) (string, string, int, error) { cmd := exec.CommandContext(ctx, name, args...) + winproc.HideWindow(cmd) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -111,6 +114,7 @@ func (r *Real) RunInDir(ctx context.Context, dir string, timeout time.Duration, ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() cmd := exec.CommandContext(ctx, name, args...) + winproc.HideWindow(cmd) cmd.Dir = dir var stdout, stderr bytes.Buffer cmd.Stdout = &stdout @@ -266,6 +270,7 @@ func (r *Real) IsAppleCLTStub(_ context.Context, binPath string) bool { probeCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() cmd := exec.CommandContext(probeCtx, "xcode-select", "-p") + winproc.HideWindow(cmd) var stdout bytes.Buffer cmd.Stdout = &stdout err := cmd.Run() diff --git a/internal/progress/filelog/filelog.go b/internal/progress/filelog/filelog.go index bc6ec2a..42627ae 100644 --- a/internal/progress/filelog/filelog.go +++ b/internal/progress/filelog/filelog.go @@ -33,7 +33,6 @@ package filelog import ( "errors" "fmt" - "io" "os" "path/filepath" "sync" @@ -221,14 +220,16 @@ func (c *Capture) Stop() error { func (c *Capture) teeLoop() { defer close(c.done) - dst := io.MultiWriter(c.origErr, c.file) buf := make([]byte, 4096) for { n, err := c.pipeRead.Read(buf) if n > 0 { - // Best-effort: a failed write to dst (file full, disk - // removed) must not stall the agent. - _, _ = dst.Write(buf[:n]) + // File first, origErr second. io.MultiWriter aborts on the + // first error, so an invalid origErr (GUI-subsystem agent + // with no parent console) used to drop the file write too. + // Both ignored — neither failure should stall the agent. + _, _ = c.file.Write(buf[:n]) + _, _ = c.origErr.Write(buf[:n]) } if err != nil { return diff --git a/internal/progress/filelog/filelog_test.go b/internal/progress/filelog/filelog_test.go index 960a585..5c046cf 100644 --- a/internal/progress/filelog/filelog_test.go +++ b/internal/progress/filelog/filelog_test.go @@ -81,6 +81,54 @@ func TestStartStopWritesFile(t *testing.T) { } } +// Regression guard: a broken origErr (closed/invalid handle, as +// GUI-subsystem agents get under Task Scheduler) must not block the +// file write. Previously io.MultiWriter aborted the loop, leaving +// agent.error.log empty despite a successful scan. +func TestStartWritesFileEvenWhenOrigStderrIsBroken(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "agent.error.log") + + origStderr := os.Stderr + t.Cleanup(func() { os.Stderr = origStderr }) + + // Simulate the invalid-handle state by opening then immediately + // closing a file, then assigning the closed handle as os.Stderr. + // Writes through that handle will return os.ErrClosed — analogous + // to ERROR_INVALID_HANDLE on Windows under GUI-subsystem. + f, err := os.Open(os.DevNull) + if err != nil { + t.Fatalf("open devnull: %v", err) + } + if err := f.Close(); err != nil { + t.Fatalf("close devnull: %v", err) + } + os.Stderr = f + + cap, err := Start(path, DefaultMaxBytes) + if err != nil { + t.Fatalf("Start: %v", err) + } + + // Write through the now-piped os.Stderr. The data flows: pipe -> + // teeLoop -> file (must succeed) + origErr (will fail, ignored). + if _, err := fmt.Fprintln(os.Stderr, "hello via broken origErr"); err != nil { + t.Fatalf("Fprintln: %v", err) + } + + if err := cap.Stop(); err != nil { + t.Fatalf("Stop: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !bytes.Contains(data, []byte("hello via broken origErr")) { + t.Errorf("file missing payload despite broken origErr: %q", data) + } +} + func TestRotateIfOverCap_OverwritesExistingPrev(t *testing.T) { // Regression guard: on Windows, os.Rename fails when the destination // already exists, so a stale .prev would block all subsequent diff --git a/internal/schtasks/schtasks.go b/internal/schtasks/schtasks.go index f1eb68c..cf96091 100644 --- a/internal/schtasks/schtasks.go +++ b/internal/schtasks/schtasks.go @@ -55,14 +55,9 @@ func Install(exec executor.Executor, log *progress.Logger) error { return fmt.Errorf("creating log directory: %w", err) } - // For admin installs the log dir lives at C:\ProgramData\StepSecurity, which - // inherits ACLs from C:\ProgramData and only grants non-admin users - // Read & Execute on the files inside. The /ru INTERACTIVE task fires under - // whatever user is logged on — typically a non-admin developer — and - // cmd.exe's `>>` redirect to agent.log would fail with Access Denied, which - // aborts the whole task action. Grant BUILTIN\Users (SID 545) Modify rights - // on the log dir, propagated to files and subfolders, so any logged-in - // user can append to the log files. + // Admin install's ProgramData log dir is Users-read-only by default. + // /ru INTERACTIVE fires as the logged-in user and filelog needs to + // append agent.error.log — grant BUILTIN\Users (SID 545) Modify. if exec.IsRoot() { _, _, _, icaclsErr := exec.Run(ctx, "icacls", logDir, "/grant", "*S-1-5-32-545:(OI)(CI)M", "/Q") if icaclsErr != nil { @@ -70,21 +65,13 @@ func Install(exec executor.Executor, log *progress.Logger) error { } } - // Bake STEPSECURITY_HOME into the task command. Admin/INTERACTIVE - // tasks (machine-wide installs) anchor at C:\ProgramData\StepSecurity - // so the scheduled task and the MSI's earlier write_config agree on - // the dir, regardless of which interactive user is logged on at fire - // time. Non-admin tasks track paths.Home() (whatever the user's own - // install dir resolves to). - var stepHome string - if exec.IsRoot() { - stepHome = `C:\ProgramData\StepSecurity` - } else { - stepHome = paths.Home() - } + // Reuse logDir: its fallback chain ends at ProgramData so it's + // never empty. paths.Home() directly could yield "", which the CLI + // treats as opt-out (disables file logging). + stepHome := logDir - args := buildCreateArgs(binaryPath, logDir, stepHome, hours, exec.IsRoot()) - log.Debug("schtasks create: binary=%q log_dir=%q step_home=%q hours=%d is_admin=%v", binaryPath, logDir, stepHome, hours, exec.IsRoot()) + args := buildCreateArgs(binaryPath, stepHome, hours, exec.IsRoot()) + log.Debug("schtasks create: binary=%q install_dir=%q hours=%d is_admin=%v", binaryPath, stepHome, hours, exec.IsRoot()) _, stderr, exitCode, err := exec.Run(ctx, "schtasks", args...) log.Debug("schtasks /create: exit_code=%d err=%v", exitCode, err) @@ -136,16 +123,11 @@ func isConfigured(ctx context.Context, exec executor.Executor) bool { return exitCode == 0 } -func buildCreateArgs(binaryPath, logDir, stepHome string, hours int, isAdmin bool) []string { - // `set "VAR=value"` is the safe cmd.exe form when the value may - // contain spaces or `&` / `|` / `<` / `>` metacharacters — without - // the quotes, cmd.exe would split on the first space and the - // remainder would be treated as a separate command. Inside the outer - // `cmd /c "..."` quotes the inner double-quotes are escaped as `""`. - // stepHome ultimately comes from $HOME (e.g. `C:\Users\John Doe`), - // so quoting is load-bearing here. - taskCmd := fmt.Sprintf(`cmd /c "set ""STEPSECURITY_HOME=%s"" && \"%s\" send-telemetry >> \"%s\agent.log\" 2>> \"%s\agent.error.log\""`, - stepHome, binaryPath, logDir, logDir) +// buildCreateArgs returns the schtasks /create args. Task invokes the +// binary directly — the previous `cmd /c` wrapper flashed a console +// on every fire; --install-dir + filelog handle what it did. +func buildCreateArgs(binaryPath, stepHome string, hours int, isAdmin bool) []string { + taskCmd := fmt.Sprintf(`"%s" send-telemetry --install-dir="%s"`, binaryPath, stepHome) args := []string{"/create", "/tn", taskName, "/tr", taskCmd, "/sc", "HOURLY", "/mo", strconv.Itoa(hours), "/f"} if isAdmin { diff --git a/internal/schtasks/schtasks_test.go b/internal/schtasks/schtasks_test.go index 6df61b0..ad0f74f 100644 --- a/internal/schtasks/schtasks_test.go +++ b/internal/schtasks/schtasks_test.go @@ -2,6 +2,7 @@ package schtasks import ( "context" + "strings" "testing" "github.com/step-security/dev-machine-guard/internal/executor" @@ -103,7 +104,7 @@ func TestResolveLogDir_Admin(t *testing.T) { } func TestBuildCreateArgs_CustomFrequency(t *testing.T) { - args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, `C:\logs`, 6, false) + args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, 6, false) // Find the /mo argument and check its value foundMo := false @@ -121,7 +122,7 @@ func TestBuildCreateArgs_CustomFrequency(t *testing.T) { } func TestBuildCreateArgs_Admin(t *testing.T) { - args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, `C:\ProgramData\StepSecurity`, 4, true) + args := buildCreateArgs(`C:\agent.exe`, `C:\ProgramData\StepSecurity`, 4, true) foundRU := false for i, a := range args { @@ -138,7 +139,7 @@ func TestBuildCreateArgs_Admin(t *testing.T) { } func TestBuildCreateArgs_NonAdmin(t *testing.T) { - args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, `C:\logs`, 4, false) + args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, 4, false) for _, a := range args { if a == "/ru" { @@ -146,3 +147,36 @@ func TestBuildCreateArgs_NonAdmin(t *testing.T) { } } } + +// Regression guard: no `cmd /c` wrapper, no `>>` redirection, +// --install-dir present, binary path quoted. +func TestBuildCreateArgs_TaskCommandFormat(t *testing.T) { + args := buildCreateArgs(`C:\agent.exe`, `C:\ProgramData\StepSecurity`, 4, true) + + taskCmd := "" + for i, a := range args { + if a == "/tr" && i+1 < len(args) { + taskCmd = args[i+1] + break + } + } + if taskCmd == "" { + t.Fatal("no /tr argument found") + } + + if strings.Contains(strings.ToLower(taskCmd), "cmd /c") || strings.Contains(strings.ToLower(taskCmd), "cmd.exe") { + t.Errorf("task command must not wrap binary in cmd: %q", taskCmd) + } + if !strings.Contains(taskCmd, "send-telemetry") { + t.Errorf("task command missing send-telemetry: %q", taskCmd) + } + if !strings.Contains(taskCmd, `--install-dir="C:\ProgramData\StepSecurity"`) { + t.Errorf("task command missing --install-dir flag: %q", taskCmd) + } + if !strings.HasPrefix(taskCmd, `"C:\agent.exe"`) { + t.Errorf("task command must start with quoted binary path: %q", taskCmd) + } + if strings.Contains(taskCmd, ">>") || strings.Contains(taskCmd, "STEPSECURITY_HOME=") { + t.Errorf("task command must not redirect output or set env vars: %q", taskCmd) + } +} diff --git a/internal/winproc/winproc.go b/internal/winproc/winproc.go new file mode 100644 index 0000000..49a155a --- /dev/null +++ b/internal/winproc/winproc.go @@ -0,0 +1,10 @@ +//go:build !windows + +// Package winproc hides the console window of a child process on +// Windows. No-op on every other platform so call sites stay portable. +package winproc + +import "os/exec" + +// HideWindow is a no-op on non-Windows platforms. +func HideWindow(_ *exec.Cmd) {} diff --git a/internal/winproc/winproc_test.go b/internal/winproc/winproc_test.go new file mode 100644 index 0000000..17342d6 --- /dev/null +++ b/internal/winproc/winproc_test.go @@ -0,0 +1,18 @@ +package winproc + +import ( + "os/exec" + "testing" +) + +func TestHideWindow_NilSafe(t *testing.T) { + HideWindow(nil) +} + +func TestHideWindow_ZeroValueCmd(t *testing.T) { + cmd := exec.Command("true") + HideWindow(cmd) + if cmd.Path == "" { + t.Fatal("HideWindow corrupted cmd.Path") + } +} diff --git a/internal/winproc/winproc_windows.go b/internal/winproc/winproc_windows.go new file mode 100644 index 0000000..3a1a4b0 --- /dev/null +++ b/internal/winproc/winproc_windows.go @@ -0,0 +1,24 @@ +//go:build windows + +package winproc + +import ( + "os/exec" + "syscall" +) + +const createNoWindow uint32 = 0x08000000 + +// HideWindow sets HideWindow + CREATE_NO_WINDOW on cmd. Without this, +// powershell/cmd/.cmd subprocesses Go spawns under Task Scheduler +// flash a console window. Merges with existing SysProcAttr; idempotent. +func HideWindow(cmd *exec.Cmd) { + if cmd == nil { + return + } + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.HideWindow = true + cmd.SysProcAttr.CreationFlags |= createNoWindow +} diff --git a/internal/winproc/winproc_windows_test.go b/internal/winproc/winproc_windows_test.go new file mode 100644 index 0000000..e50326f --- /dev/null +++ b/internal/winproc/winproc_windows_test.go @@ -0,0 +1,56 @@ +//go:build windows + +package winproc + +import ( + "os/exec" + "syscall" + "testing" +) + +func TestHideWindow_SetsAttrs(t *testing.T) { + cmd := exec.Command("cmd.exe") + HideWindow(cmd) + + if cmd.SysProcAttr == nil { + t.Fatal("SysProcAttr was not allocated") + } + if !cmd.SysProcAttr.HideWindow { + t.Error("HideWindow flag not set") + } + if cmd.SysProcAttr.CreationFlags&createNoWindow == 0 { + t.Errorf("CREATE_NO_WINDOW not OR'd into CreationFlags (got 0x%x)", cmd.SysProcAttr.CreationFlags) + } +} + +// Existing SysProcAttr fields must survive the merge. +func TestHideWindow_MergesExistingAttrs(t *testing.T) { + const otherFlag uint32 = 0x00000200 // CREATE_NEW_PROCESS_GROUP + cmd := exec.Command("cmd.exe") + cmd.SysProcAttr = &syscall.SysProcAttr{ + CreationFlags: otherFlag, + Token: 42, + } + + HideWindow(cmd) + + if cmd.SysProcAttr.CreationFlags&otherFlag == 0 { + t.Error("pre-existing CreationFlags bit was clobbered") + } + if cmd.SysProcAttr.CreationFlags&createNoWindow == 0 { + t.Error("CREATE_NO_WINDOW not OR'd in") + } + if cmd.SysProcAttr.Token != 42 { + t.Errorf("Token field clobbered (want 42, got %d)", cmd.SysProcAttr.Token) + } +} + +func TestHideWindow_Idempotent(t *testing.T) { + cmd := exec.Command("cmd.exe") + HideWindow(cmd) + flags1 := cmd.SysProcAttr.CreationFlags + HideWindow(cmd) + if cmd.SysProcAttr.CreationFlags != flags1 { + t.Errorf("second call mutated CreationFlags (0x%x -> 0x%x)", flags1, cmd.SysProcAttr.CreationFlags) + } +}