feat(windows): add --exec mode to GUI launcher#113
Open
swarit-stepsecurity wants to merge 2 commits into
Open
Conversation
The launcher was hardcoded to spawn the sibling agent .exe, which is exactly what the MSI install layout needs but doesn't work for the PowerShell loader. The PS task action has to run `powershell.exe -File loader.ps1 send-telemetry` on every tick (loader owns auto-update), and powershell.exe is console-subsystem — Task Scheduler firing it directly allocates a console and flashes a window before -WindowStyle Hidden takes effect. Same root cause PR step-security#104 fixed for the agent, just at the powershell layer. This adds an --exec mode so the PS loader's scheduled task can wrap powershell.exe in the launcher's no-console envelope: task.exe --exec powershell.exe -ExecutionPolicy Bypass ... When --exec is absent the launcher falls through to the existing sibling-agent behaviour, so MSI installs see no change. Target resolution lives in internal/launcher so it gets unit-test coverage on the macOS CI runner (the launcher binary itself stays Windows-only).
There was a problem hiding this comment.
Pull request overview
This PR extends the Windows GUI-subsystem scheduled-task launcher (stepsecurity-dev-machine-guard-task.exe) to support a new --exec mode so it can wrap arbitrary console-subsystem executables (notably powershell.exe) under CREATE_NO_WINDOW, avoiding visible console flashes when Task Scheduler triggers the PowerShell loader.
Changes:
- Added
internal/launcher.ResolveTargetto resolve the child executable/argv in either default (sibling agent) mode or--execmode. - Added unit tests for target resolution behavior across platforms.
- Updated the Windows launcher binary to delegate target selection to
internal/launcherand to support--exec.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/launcher/launcher.go | Introduces platform-agnostic child-process target resolution logic (default vs --exec). |
| internal/launcher/launcher_test.go | Adds unit tests covering argument forwarding, resolution errors, and sibling discovery behavior. |
| cmd/stepsecurity-dev-machine-guard-task/main.go | Wires the Windows-only GUI launcher to ResolveTarget and enables --exec mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+66
| // stderr is normally unreachable for a GUI-subsystem process | ||
| // launched by Task Scheduler, but it's visible to interactive | ||
| // invocations and to test runs — both worth surfacing the | ||
| // concrete misuse string for. | ||
| fmt.Fprintln(os.Stderr, err) | ||
| return 2 |
Comment on lines
+54
to
+56
| // launcher was deployed without its companion; matches the previous | ||
| // behaviour (return error → exit 1 → no console output) so existing | ||
| // MSI installs see no behaviour change. |
Comment on lines
+103
to
+117
| // Don't clobber a real agent that might be sitting next to the test | ||
| // binary in a developer's checkout — restore whatever was there. | ||
| prev, hadPrev := os.ReadFile(siblingPath) | ||
| _ = prev | ||
|
|
||
| if err := os.WriteFile(siblingPath, []byte("fake-agent"), 0o644); err != nil { | ||
| t.Fatalf("seeding fake agent at %q: %v", siblingPath, err) | ||
| } | ||
| t.Cleanup(func() { | ||
| if hadPrev == nil { | ||
| _ = os.WriteFile(siblingPath, prev, 0o644) | ||
| } else { | ||
| _ = os.Remove(siblingPath) | ||
| } | ||
| }) |
Three coordinated fixes from Copilot review on step-security#113: - main.go: previous patch unified all ResolveTarget errors to "stderr + exit 2". For the default (sibling-agent) mode, that's a behavior change MSI installs would inherit — Task Scheduler's LastTaskResult column flips from 1 to 2 when the sibling is absent, which would silently break any downstream alerting that keys on the result code. Split: --exec errors still get stderr + exit 2 (caller asked for a feature, give them diagnostics), default-mode errors stay silent with exit 1 (pre-1.11.5 contract preserved). - launcher.go: docstring now reflects the actual split — the "matches the previous behaviour" line was stale once main.go did the unified mapping above; moved the contract into the docstring with a clear note about which mode produces which exit code. - launcher_test.go: TestResolveTarget_DefaultMode_FindsSibling's cleanup was buggy. The original code always overwrote siblingPath in cleanup even if the initial os.ReadFile failed for reasons other than non-existence (locked file, perm error), and it restored with a hardcoded 0o644 mode. Now: capture the original mode via os.Stat, treat any non-IsNotExist read error as a Skip (so we don't destroy real developer state we can't restore), restore both bytes AND mode on cleanup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The launcher was hardcoded to spawn the sibling agent .exe, which is exactly what the MSI install layout needs but doesn't work for the PowerShell loader. The PS task action has to run
powershell.exe -File loader.ps1 send-telemetryon every tick (loader owns auto-update), and powershell.exe is console-subsystem — Task Scheduler firing it directly allocates a console and flashes a window before -WindowStyle Hidden takes effect. Same root cause PR #104 fixed for the agent, just at the powershell layer.This adds an --exec mode so the PS loader's scheduled task can wrap powershell.exe in the launcher's no-console envelope:
task.exe --exec powershell.exe -ExecutionPolicy Bypass ...
When --exec is absent the launcher falls through to the existing sibling-agent behaviour, so MSI installs see no change. Target resolution lives in internal/launcher so it gets unit-test coverage on the macOS CI runner (the launcher binary itself stays Windows-only).
What does this PR do?
Type of change
Testing
./stepsecurity-dev-machine-guard --verbose./stepsecurity-dev-machine-guard --json | python3 -m json.toolmake lintmake testRelated Issues