fix(windows): replace shell-sensitive commands with explicit PS equivalents

[kke]

All Windows command execs that relied on cmd.exe being the default shell are now wrapped with cmd.PS() using PowerShell-native cmdlets. This fixes breakage when the SSH default shell on a Windows host is PowerShell.

initsystem/winscm.go:

• StartService/StopService: sc.exe start/stop → Start-Service/Stop-Service
• EnableService: sc.exe config start=enabled → Set-Service -StartupType Automatic (also fixes a latent bug: "enabled" is not a valid sc.exe start= value)
• DisableService: sc.exe config start=disabled → Set-Service -StartupType Disabled
• ServiceIsRunning: sc.exe query | findstr → Get-Service status check

remotefs/winfs.go:

• Chmod: bare attrib ±R → Get-Item / .Attributes FileAttributes bitmask via PS
• Reboot: schtasks /create|run|delete → Register/Start/Unregister-ScheduledTask (preserves SYSTEM-principal scheduled-task approach for UAC token bypass)

cmd.exe /c del, rmdir, mkdir remain unchanged — those explicitly invoke cmd.exe and are unaffected by the default-shell setting.

Tests updated/added for all changed methods.

[Copilot]

Pull request overview

This PR updates Windows remote execution paths to be robust when the remote host’s default shell is PowerShell (e.g., SSH default shell), by explicitly wrapping affected commands with cmd.PS() and using PowerShell-native cmdlets for service management, attribute changes, and reboot scheduling.

Changes:

• Switch Windows SCM operations from sc.exe/findstr to Start-Service/Stop-Service/Set-Service/Get-Service via cmd.PS().
• Rework Windows Chmod to modify FileAttributes via PowerShell instead of attrib ±R.
• Rework Windows reboot to use Register/Start/Unregister-ScheduledTask instead of schtasks and add/update tests accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
remotefs/winfs.go	Replace attrib-based chmod and schtasks reboot flow with PowerShell-native implementations via cmd.PS().
remotefs/winfs_test.go	Update chmod tests to expect PowerShell invocation rather than attrib.
remotefs/reboot_test.go	Update reboot tests to validate encoded PowerShell scripts (create/run/delete) and avoid schtasks.
initsystem/winscm.go	Replace sc.exe service management with PowerShell cmdlets via cmd.PS().
initsystem/winscm_test.go	Add unit tests asserting the new PowerShell-based SCM behavior.

Comments suppressed due to low confidence (4)

remotefs/winfs.go:751

• Start-ScheduledTask / Unregister-ScheduledTask failures may not translate into a non-zero powershell.exe exit code unless errors are made terminating. That can make Reboot report success even when the reboot task didn’t start or cleanup didn’t happen. Add -ErrorAction Stop / $ErrorActionPreference='Stop' and explicit exit 1 on catch to make error handling reliable.

	runScript := "Start-ScheduledTask -TaskName " + ps.DoubleQuote(taskName)
	deleteScript := fmt.Sprintf("Unregister-ScheduledTask -TaskName %s -Confirm:$false", ps.DoubleQuote(taskName))

	if err := s.ExecContext(ctx, runScript, cmd.PS(), cmd.AllowWinStderr()); err != nil {
		// Best-effort delete in all error paths to prevent the task from
		// firing later. Ignore the delete error: if the host is already
		// rebooting it will fail anyway.
		_ = s.ExecContext(ctx, deleteScript, cmd.PS(), cmd.AllowWinStderr())

initsystem/winscm.go:30

• Stop-Service can emit non-terminating errors while powershell.exe exits 0, which would make StopService incorrectly return nil. Consider adding -ErrorAction Stop and/or wrapping in try/catch { exit 1 } (or setting $ErrorActionPreference='Stop') to ensure ExecContext returns an error on failure.

// StopService stops a service.
func (c WinSCM) StopService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, "Stop-Service "+ps.DoubleQuote(s), cmd.PS()); err != nil {
		return fmt.Errorf("failed to stop service %s: %w", s, err)
	}

initsystem/winscm.go:52

• Set-Service errors (invalid service name, access denied, etc.) may be non-terminating and still result in a 0 exit code from powershell.exe, causing EnableService to incorrectly report success. Add -ErrorAction Stop / $ErrorActionPreference='Stop' plus try/catch { exit 1 } so failures propagate reliably.

func (c WinSCM) EnableService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, fmt.Sprintf("Set-Service -Name %s -StartupType Automatic", ps.DoubleQuote(s)), cmd.PS()); err != nil {
		return fmt.Errorf("failed to enable service %s: %w", s, err)
	}

initsystem/winscm.go:72

• Set-Service failures can be non-terminating and may not cause powershell.exe to exit non-zero, which would make DisableService incorrectly return nil. Add -ErrorAction Stop / $ErrorActionPreference='Stop' and explicit failure exit handling (e.g., try/catch { exit 1 }) so ExecContext reports errors reliably.

func (c WinSCM) DisableService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, fmt.Sprintf("Set-Service -Name %s -StartupType Disabled", ps.DoubleQuote(s)), cmd.PS()); err != nil {
		return fmt.Errorf("failed to disable service %s: %w", s, err)
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

initsystem/winscm.go:50

• Set-Service -Name is currently given a double-quoted string (ps.DoubleQuote), which can mis-handle service names containing '$' due to PowerShell interpolation. Prefer a literal-safe quoting strategy (e.g. ps.SingleQuote) for the service name to avoid breaking services like MSSQL$.

	if err := h.ExecContext(ctx, fmt.Sprintf("Set-Service -Name %s -StartupType Automatic -ErrorAction Stop", ps.DoubleQuote(s)), cmd.PS()); err != nil {
		return fmt.Errorf("failed to enable service %s: %w", s, err)

initsystem/winscm.go:78

• ServiceIsRunning builds a PowerShell snippet using ps.DoubleQuote(s). In PowerShell double quotes interpolate '$', so services such as MSSQL$SQLEXPRESS can cause false negatives. Quote the service name in a non-interpolating way (e.g. ps.SingleQuote) and consider using Get-Service -Name for clarity.

func (c WinSCM) ServiceIsRunning(ctx context.Context, h cmd.ContextRunner, s string) bool {
	return h.ExecContext(ctx, fmt.Sprintf(`if ((Get-Service %s -ErrorAction SilentlyContinue).Status -ne 'Running') { exit 1 }`, ps.DoubleQuote(s)), cmd.PS()) == nil

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

remotefs/winfs_test.go:340

• This test similarly only checks that PowerShell ran, but doesn’t assert that the script sets the ReadOnly attribute when owner-write is unset. Strengthen the assertion by checking the decoded PowerShell script (or exact generated command) contains the expected -bor [IO.FileAttributes]::ReadOnly behavior.

		mr.AddCommandSuccess(rigtest.HasPrefix("powershell.exe"))
		fsys := remotefs.NewWindowsFS(mr)
		// 0o444 has no owner-write bit → should set read-only.
		require.NoError(t, fsys.Chmod(`C:\file.txt`, fs.FileMode(0o444)))
		require.NoError(t, mr.Received(rigtest.HasPrefix("powershell.exe")))

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (5)

initsystem/winscm.go:29

• Same issue as StartService: this will likely be emitted as powershell.exe ... -Command "..." and can break under an outer PowerShell default shell due to $ProgressPreference interpolation. Ensure this invocation is forced through -EncodedCommand (or otherwise escape $ for outer-shell safety).

// StopService stops a service.
func (c WinSCM) StopService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, "Stop-Service "+ps.SingleQuote(s)+" -ErrorAction Stop", cmd.PS()); err != nil {
		return fmt.Errorf("failed to stop service %s: %w", s, err)

initsystem/winscm.go:42

• Same outer-PowerShell interpolation risk as StartService: with the current powershell.Cmd heuristic, this one-liner will usually use -Command and may be corrupted by $ProgressPreference expansion when the remote default shell is PowerShell. Force -EncodedCommand for this call as well.

// RestartService restarts a service.
func (c WinSCM) RestartService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, "Restart-Service "+ps.SingleQuote(s)+" -ErrorAction Stop", cmd.PS()); err != nil {
		return fmt.Errorf("failed to restart service %s: %w", s, err)

initsystem/winscm.go:50

• Same outer-PowerShell interpolation risk as StartService: this command is likely to be sent via -Command "..." and can be corrupted by $ProgressPreference expansion when the SSH default shell is PowerShell. Force -EncodedCommand (or adjust powershell.Cmd to encode more aggressively) to make this robust.

// EnableService enables a service by setting its startup type to Automatic.
func (c WinSCM) EnableService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, fmt.Sprintf("Set-Service -Name %s -StartupType Automatic -ErrorAction Stop", ps.SingleQuote(s)), cmd.PS()); err != nil {
		return fmt.Errorf("failed to enable service %s: %w", s, err)

initsystem/winscm.go:71

• Same outer-PowerShell interpolation risk as StartService: this command will likely use -Command and can be corrupted by $ProgressPreference expansion under a PowerShell default shell. Force -EncodedCommand (or update powershell.Cmd/cmd.PS) here too.

// DisableService disables a service by setting its startup type to Disabled.
func (c WinSCM) DisableService(ctx context.Context, h cmd.ContextRunner, s string) error {
	if err := h.ExecContext(ctx, fmt.Sprintf("Set-Service -Name %s -StartupType Disabled -ErrorAction Stop", ps.SingleQuote(s)), cmd.PS()); err != nil {
		return fmt.Errorf("failed to disable service %s: %w", s, err)

initsystem/winscm.go:79

• This script is short enough that powershell.Cmd will typically choose -Command "...". When the remote SSH DefaultShell is PowerShell, the outer shell can expand $ProgressPreference injected by powershell.Cmd, corrupting the script. Force -EncodedCommand for reliability in the PowerShell-default-shell scenario this PR targets.

// ServiceIsRunning returns true if a service is running.
func (c WinSCM) ServiceIsRunning(ctx context.Context, h cmd.ContextRunner, s string) bool {
	return h.ExecContext(ctx, fmt.Sprintf(`if ((Get-Service %s -ErrorAction SilentlyContinue).Status -ne 'Running') { exit 1 }`, ps.SingleQuote(s)), cmd.PS()) == nil
}

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.