From 67979d7d299865169c0d8b76ec3284860337c130 Mon Sep 17 00:00:00 2001 From: ravi-shah Date: Sun, 13 Sep 2026 09:23:44 -0400 Subject: [PATCH] fix(runner): hide Windows background bridge consoles --- cmd/enbor-runner/internal/runtime/bridge.go | 14 +- .../internal/runtime/bridge_windows_test.go | 27 +++ .../internal/sys/processtree/process_unix.go | 7 + .../sys/processtree/process_windows.go | 52 ++++- .../sys/processtree/process_windows_test.go | 182 ++++++++++++++++++ 5 files changed, 271 insertions(+), 11 deletions(-) create mode 100644 cmd/enbor-runner/internal/runtime/bridge_windows_test.go diff --git a/cmd/enbor-runner/internal/runtime/bridge.go b/cmd/enbor-runner/internal/runtime/bridge.go index 0e084888..78f490ec 100644 --- a/cmd/enbor-runner/internal/runtime/bridge.go +++ b/cmd/enbor-runner/internal/runtime/bridge.go @@ -223,7 +223,7 @@ func (b Bridge) bridgeRequest(ctx context.Context, requestID string, request any if err != nil { return nil, err } - process, err := processtree.Start(cmd) + process, err := processtree.StartBackground(cmd) if err != nil { return nil, err } @@ -253,9 +253,7 @@ func resolveNodeExecutable(ctx context.Context, workDir string) (string, error) probeCtx, cancel := context.WithTimeout(ctx, nodeExecutableProbeTimeout) defer cancel() - probe := exec.CommandContext(probeCtx, nodePath, "-p", "process.execPath") - probe.Dir = workDir - probe.Env = os.Environ() + probe := nodeExecutableProbeCommand(probeCtx, nodePath, workDir) output, err := probe.Output() if err != nil { return "", fmt.Errorf("resolve node executable: %w", err) @@ -274,6 +272,14 @@ func resolveNodeExecutable(ctx context.Context, workDir string) (string, error) return resolved, nil } +func nodeExecutableProbeCommand(ctx context.Context, nodePath string, workDir string) *exec.Cmd { + probe := exec.CommandContext(ctx, nodePath, "-p", "process.execPath") + probe.Dir = workDir + probe.Env = os.Environ() + processtree.HideConsoleWindow(probe) + return probe +} + func commandEnvironment(request Request) ([]string, error) { env, err := sandbox.ProcessCommandEnvironment(request.WorkDir) if err != nil { diff --git a/cmd/enbor-runner/internal/runtime/bridge_windows_test.go b/cmd/enbor-runner/internal/runtime/bridge_windows_test.go new file mode 100644 index 00000000..7c905808 --- /dev/null +++ b/cmd/enbor-runner/internal/runtime/bridge_windows_test.go @@ -0,0 +1,27 @@ +//go:build windows + +package runtime + +import ( + "context" + "testing" + + "golang.org/x/sys/windows" +) + +func TestNodeExecutableProbeCommandHidesWindowsConsoleAndKeepsWorkDir(t *testing.T) { + workDir := t.TempDir() + cmd := nodeExecutableProbeCommand(context.Background(), "node.exe", workDir) + if cmd.Dir != workDir { + t.Fatalf("expected probe work dir %q, got %q", workDir, cmd.Dir) + } + if cmd.SysProcAttr == nil { + t.Fatal("expected Windows process attributes") + } + if !cmd.SysProcAttr.HideWindow { + t.Fatal("node executable probe must hide its console window") + } + if cmd.SysProcAttr.CreationFlags&windows.CREATE_NO_WINDOW == 0 { + t.Fatalf("expected CREATE_NO_WINDOW; flags=%#x", cmd.SysProcAttr.CreationFlags) + } +} diff --git a/cmd/enbor-runner/internal/sys/processtree/process_unix.go b/cmd/enbor-runner/internal/sys/processtree/process_unix.go index fd71a75b..f5a77573 100644 --- a/cmd/enbor-runner/internal/sys/processtree/process_unix.go +++ b/cmd/enbor-runner/internal/sys/processtree/process_unix.go @@ -20,6 +20,13 @@ func Start(cmd *exec.Cmd) (*Process, error) { return &Process{cmd: cmd}, nil } +func StartBackground(cmd *exec.Cmd) (*Process, error) { + return Start(cmd) +} + +func HideConsoleWindow(_ *exec.Cmd) { +} + func (p *Process) Wait() error { return p.cmd.Wait() } diff --git a/cmd/enbor-runner/internal/sys/processtree/process_windows.go b/cmd/enbor-runner/internal/sys/processtree/process_windows.go index 8d0c91dd..f0302e1f 100644 --- a/cmd/enbor-runner/internal/sys/processtree/process_windows.go +++ b/cmd/enbor-runner/internal/sys/processtree/process_windows.go @@ -13,12 +13,35 @@ import ( ) type Process struct { - cmd *exec.Cmd - job windows.Handle - once sync.Once + cmd *exec.Cmd + job windows.Handle + background bool + once sync.Once } +var ( + generateConsoleCtrlEvent = windows.GenerateConsoleCtrlEvent + terminateJobObject = windows.TerminateJobObject +) + func Start(cmd *exec.Cmd) (*Process, error) { + return start(cmd, false) +} + +// StartBackground hides console windows for non-interactive probes. Hidden +// children do not share the caller console for CTRL_BREAK; Stop cleans them up +// through the Job Object instead of claiming graceful console delivery. +func StartBackground(cmd *exec.Cmd) (*Process, error) { + return start(cmd, true) +} + +func HideConsoleWindow(cmd *exec.Cmd) { + attr := sysProcAttr(cmd) + attr.CreationFlags |= windows.CREATE_NO_WINDOW + attr.HideWindow = true +} + +func start(cmd *exec.Cmd, background bool) (*Process, error) { job, err := windows.CreateJobObject(nil, nil) if err != nil { return nil, err @@ -34,7 +57,11 @@ func Start(cmd *exec.Cmd) (*Process, error) { _ = windows.CloseHandle(job) return nil, err } - cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_NEW_PROCESS_GROUP} + attr := sysProcAttr(cmd) + attr.CreationFlags |= windows.CREATE_NEW_PROCESS_GROUP + if background { + HideConsoleWindow(cmd) + } if err := cmd.Start(); err != nil { _ = windows.CloseHandle(job) return nil, err @@ -56,7 +83,14 @@ func Start(cmd *exec.Cmd) (*Process, error) { _ = windows.CloseHandle(job) return nil, err } - return &Process{cmd: cmd, job: job}, nil + return &Process{cmd: cmd, job: job, background: background}, nil +} + +func sysProcAttr(cmd *exec.Cmd) *syscall.SysProcAttr { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + return cmd.SysProcAttr } func (p *Process) Wait() error { @@ -67,11 +101,15 @@ func (p *Process) Stop(grace time.Duration) { if p == nil || p.cmd == nil || p.cmd.Process == nil { return } - _ = windows.GenerateConsoleCtrlEvent(windows.CTRL_BREAK_EVENT, uint32(p.cmd.Process.Pid)) + // CREATE_NO_WINDOW children do not share the caller's console, so CTRL_BREAK + // delivery is only a graceful path for visible process groups. + if !p.background { + _ = generateConsoleCtrlEvent(windows.CTRL_BREAK_EVENT, uint32(p.cmd.Process.Pid)) + } if grace > 0 { time.Sleep(grace) } - _ = windows.TerminateJobObject(p.job, 1) + _ = terminateJobObject(p.job, 1) } func (p *Process) Close() error { diff --git a/cmd/enbor-runner/internal/sys/processtree/process_windows_test.go b/cmd/enbor-runner/internal/sys/processtree/process_windows_test.go index 60eca21d..f224f771 100644 --- a/cmd/enbor-runner/internal/sys/processtree/process_windows_test.go +++ b/cmd/enbor-runner/internal/sys/processtree/process_windows_test.go @@ -3,11 +3,113 @@ package processtree import ( + "os" "os/exec" + "path/filepath" + "strconv" + "strings" "testing" "time" + + "golang.org/x/sys/windows" ) +func TestStartKeepsInteractiveWindowsLaunchVisible(t *testing.T) { + cmd := exec.Command("cmd.exe", "/d", "/s", "/c", "ping -n 30 127.0.0.1 >nul") + process, err := Start(cmd) + if err != nil { + t.Fatal(err) + } + defer process.Close() + defer func() { + process.Stop(0) + _ = process.Wait() + }() + if cmd.SysProcAttr == nil { + t.Fatal("expected Windows process attributes") + } + if cmd.SysProcAttr.HideWindow { + t.Fatal("interactive process launch must not hide its console window") + } + if cmd.SysProcAttr.CreationFlags&windows.CREATE_NO_WINDOW != 0 { + t.Fatalf("interactive process launch must not set CREATE_NO_WINDOW; flags=%#x", cmd.SysProcAttr.CreationFlags) + } + if cmd.SysProcAttr.CreationFlags&windows.CREATE_NEW_PROCESS_GROUP == 0 { + t.Fatalf("expected CREATE_NEW_PROCESS_GROUP; flags=%#x", cmd.SysProcAttr.CreationFlags) + } + if process.background { + t.Fatal("interactive process launch must keep graceful console cancellation enabled") + } +} + +func TestStartBackgroundHidesWindowsConsoleAndKeepsProcessGroup(t *testing.T) { + cmd := exec.Command("cmd.exe", "/d", "/s", "/c", "ping -n 30 127.0.0.1 >nul") + process, err := StartBackground(cmd) + if err != nil { + t.Fatal(err) + } + defer process.Close() + defer func() { + process.Stop(0) + _ = process.Wait() + }() + if cmd.SysProcAttr == nil { + t.Fatal("expected Windows process attributes") + } + if !cmd.SysProcAttr.HideWindow { + t.Fatal("background process launch must hide its console window") + } + if cmd.SysProcAttr.CreationFlags&windows.CREATE_NO_WINDOW == 0 { + t.Fatalf("expected CREATE_NO_WINDOW; flags=%#x", cmd.SysProcAttr.CreationFlags) + } + if cmd.SysProcAttr.CreationFlags&windows.CREATE_NEW_PROCESS_GROUP == 0 { + t.Fatalf("expected CREATE_NEW_PROCESS_GROUP; flags=%#x", cmd.SysProcAttr.CreationFlags) + } + if !process.background { + t.Fatal("background process launch must record force-cleanup cancellation semantics") + } +} + +func TestStopAttemptsCtrlBreakOnlyForVisibleWindowsProcesses(t *testing.T) { + originalCtrlBreak := generateConsoleCtrlEvent + originalTerminate := terminateJobObject + defer func() { + generateConsoleCtrlEvent = originalCtrlBreak + terminateJobObject = originalTerminate + }() + + var ctrlBreaks []uint32 + var terminations int + generateConsoleCtrlEvent = func(_ uint32, processGroupID uint32) error { + ctrlBreaks = append(ctrlBreaks, processGroupID) + return nil + } + terminateJobObject = func(_ windows.Handle, _ uint32) error { + terminations++ + return nil + } + + visible := &Process{cmd: &exec.Cmd{Process: &os.Process{Pid: 42}}, job: windows.Handle(7)} + visible.Stop(0) + if len(ctrlBreaks) != 1 || ctrlBreaks[0] != 42 { + t.Fatalf("expected visible process to receive CTRL_BREAK attempt, got %#v", ctrlBreaks) + } + if terminations != 1 { + t.Fatalf("expected visible process to terminate job once, got %d", terminations) + } + + ctrlBreaks = nil + terminations = 0 + background := &Process{cmd: &exec.Cmd{Process: &os.Process{Pid: 84}}, job: windows.Handle(9), background: true} + background.Stop(0) + if len(ctrlBreaks) != 0 { + t.Fatalf("background process must not claim shared-console CTRL_BREAK delivery, got %#v", ctrlBreaks) + } + if terminations != 1 { + t.Fatalf("expected background process to terminate job once, got %d", terminations) + } +} + func TestStopTerminatesWindowsJob(t *testing.T) { cmd := exec.Command("cmd.exe", "/d", "/s", "/c", "ping -n 30 127.0.0.1 >nul") process, err := Start(cmd) @@ -24,3 +126,83 @@ func TestStopTerminatesWindowsJob(t *testing.T) { t.Fatal("Windows Job Object did not stop its process tree") } } + +func TestStopCloseTerminatesBackgroundWindowsProcessTree(t *testing.T) { + dir := t.TempDir() + readyPath := filepath.Join(dir, "ready") + childPIDPath := filepath.Join(dir, "child.pid") + cmd := exec.Command("powershell.exe", "-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", ` +$ErrorActionPreference = 'Stop' +$child = Start-Process -FilePath $env:ComSpec -ArgumentList @('/d','/s','/c','ping -n 30 127.0.0.1 >nul') -PassThru +Set-Content -LiteralPath $env:ENBOR_TEST_CHILD_PID -Value $child.Id -NoNewline +Set-Content -LiteralPath $env:ENBOR_TEST_READY -Value ready -NoNewline +Wait-Process -Id $child.Id +`) + cmd.Env = append(os.Environ(), + "ENBOR_TEST_READY="+readyPath, + "ENBOR_TEST_CHILD_PID="+childPIDPath, + ) + process, err := StartBackground(cmd) + if err != nil { + t.Fatal(err) + } + defer func() { + process.Stop(0) + _ = process.Close() + }() + waitForFile(t, readyPath, 5*time.Second) + childPIDData, err := os.ReadFile(childPIDPath) + if err != nil { + t.Fatal(err) + } + childPID, err := strconv.Atoi(strings.TrimSpace(string(childPIDData))) + if err != nil { + t.Fatalf("expected child pid, got %q: %v", childPIDData, err) + } + child, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(childPID)) + if err != nil { + t.Fatalf("expected descendant process %d to exist before Stop: %v", childPID, err) + } + defer windows.CloseHandle(child) + + process.Stop(0) + if err := process.Close(); err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { done <- process.Wait() }() + select { + case err := <-done: + if err == nil { + t.Fatal("expected terminated root process to report an exit error") + } + case <-time.After(5 * time.Second): + t.Fatal("background Windows Job Object did not stop its root process") + } + waitForHandleExit(t, child, 5*time.Second, "descendant process") +} + +func waitForFile(t *testing.T, path string, timeout time.Duration) { + t.Helper() + deadline := time.Now().Add(timeout) + for { + if _, err := os.Stat(path); err == nil { + return + } + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for %s", path) + } + time.Sleep(50 * time.Millisecond) + } +} + +func waitForHandleExit(t *testing.T, handle windows.Handle, timeout time.Duration, label string) { + t.Helper() + event, err := windows.WaitForSingleObject(handle, uint32(timeout/time.Millisecond)) + if err != nil { + t.Fatalf("wait for %s failed: %v", label, err) + } + if event != windows.WAIT_OBJECT_0 { + t.Fatalf("%s did not exit; wait result=%#x", label, event) + } +}