chore(windows): address relevant comments

Signed-off-by: Swarit Pandey <swarit@stepsecurity.io>

Author: swarit-stepsecurity

internal/detector/aicli.go

@@ -164,7 +164,9 @@ func (d *AICLIDetector) getVersion(ctx context.Context, spec cliToolSpec, binary
 }
 
 // cleanVersionString strips a leading tool name prefix from version output.
-// e.g. "codex-cli 0.118.0" -> "0.118.0", "aider 0.86.2" -> "0.86.2"
+// It finds the first token that looks like a version number (starts with a digit
+// or "v" followed by a digit) and returns it, preserving any "v" prefix.
+// e.g. "codex-cli 0.118.0" -> "0.118.0", "aider 0.86.2" -> "0.86.2", "v1.2.3" -> "v1.2.3"
 func cleanVersionString(v string) string {
 	parts := strings.Fields(v)
 	for _, p := range parts {

internal/schtasks/schtasks.go

@@ -42,20 +42,14 @@ func Install(exec executor.Executor, log *progress.Logger) error {
 		return fmt.Errorf("creating log directory: %w", err)
 	}
 
-	// Build the task command with output redirection
-	taskCmd := fmt.Sprintf(`cmd /c "\"%s\" send-telemetry >> \"%s\agent.log\" 2>> \"%s\agent.error.log\""`,
-		binaryPath, logDir, logDir)
-
-	// Build schtasks arguments
-	args := []string{"/create", "/tn", taskName, "/tr", taskCmd,
-		"/sc", "HOURLY", "/mo", strconv.Itoa(hours), "/f"}
-	if exec.IsRoot() {
-		args = append(args, "/ru", "SYSTEM")
-	}
+	args := buildCreateArgs(binaryPath, logDir, hours, exec.IsRoot())
 
 	_, stderr, exitCode, err := exec.Run(ctx, "schtasks", args...)
-	if err != nil || exitCode != 0 {
-		return fmt.Errorf("failed to create scheduled task: %s", stderr)
+	if err != nil {
+		return fmt.Errorf("failed to create scheduled task: %w", err)
+	}
+	if exitCode != 0 {
+		return fmt.Errorf("failed to create scheduled task (exit code %d): %s", exitCode, stderr)
 	}
 
 	log.Progress("Windows Task Scheduler configuration completed successfully")
@@ -81,8 +75,11 @@ func Uninstall(exec executor.Executor, log *progress.Logger) error {
 
 func doUninstall(ctx context.Context, exec executor.Executor, log *progress.Logger) error {
 	_, stderr, exitCode, err := exec.Run(ctx, "schtasks", "/delete", "/tn", taskName, "/f")
-	if err != nil || exitCode != 0 {
-		return fmt.Errorf("failed to delete scheduled task: %s", stderr)
+	if err != nil {
+		return fmt.Errorf("failed to delete scheduled task: %w", err)
+	}
+	if exitCode != 0 {
+		return fmt.Errorf("failed to delete scheduled task (exit code %d): %s", exitCode, stderr)
 	}
 
 	log.Progress("Removed scheduled task: %s", taskName)
@@ -95,6 +92,17 @@ func isConfigured(ctx context.Context, exec executor.Executor) bool {
 	return exitCode == 0
 }
 
+func buildCreateArgs(binaryPath, logDir string, hours int, isAdmin bool) []string {
+	taskCmd := fmt.Sprintf(`cmd /c "\"%s\" send-telemetry >> \"%s\agent.log\" 2>> \"%s\agent.error.log\""`,
+		binaryPath, logDir, logDir)
+	args := []string{"/create", "/tn", taskName, "/tr", taskCmd,
+		"/sc", "HOURLY", "/mo", strconv.Itoa(hours), "/f"}
+	if isAdmin {
+		args = append(args, "/ru", "SYSTEM")
+	}
+	return args
+}
+
 func resolveLogDir(exec executor.Executor) string {
 	if exec.IsRoot() {
 		return `C:\ProgramData\StepSecurity`

internal/schtasks/schtasks_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/step-security/dev-machine-guard/internal/config"
 	"github.com/step-security/dev-machine-guard/internal/executor"
 	"github.com/step-security/dev-machine-guard/internal/progress"
 )
@@ -98,21 +97,47 @@ func TestResolveLogDir_Admin(t *testing.T) {
 	}
 }
 
-func TestInstall_CustomFrequency(t *testing.T) {
-	origFreq := config.ScanFrequencyHours
-	t.Cleanup(func() { config.ScanFrequencyHours = origFreq })
-	config.ScanFrequencyHours = "6"
+func TestBuildCreateArgs_CustomFrequency(t *testing.T) {
+	args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, 6, false)
+
+	// Find the /mo argument and check its value
+	foundMo := false
+	for i, a := range args {
+		if a == "/mo" && i+1 < len(args) {
+			foundMo = true
+			if args[i+1] != "6" {
+				t.Errorf("expected /mo 6, got /mo %s", args[i+1])
+			}
+		}
+	}
+	if !foundMo {
+		t.Error("expected /mo argument in schtasks create args")
+	}
+}
 
-	mock := executor.NewMock()
-	mock.SetGOOS("windows")
-	mock.SetIsRoot(false)
-	mock.SetHomeDir(`C:\Users\testuser`)
-	// Task doesn't exist
-	mock.SetCommand("", "ERROR: not found", 1, "schtasks", "/query", "/tn", taskName)
+func TestBuildCreateArgs_Admin(t *testing.T) {
+	args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, 4, true)
+
+	foundRU := false
+	for i, a := range args {
+		if a == "/ru" && i+1 < len(args) {
+			foundRU = true
+			if args[i+1] != "SYSTEM" {
+				t.Errorf("expected /ru SYSTEM, got /ru %s", args[i+1])
+			}
+		}
+	}
+	if !foundRU {
+		t.Error("expected /ru SYSTEM for admin install")
+	}
+}
+
+func TestBuildCreateArgs_NonAdmin(t *testing.T) {
+	args := buildCreateArgs(`C:\agent.exe`, `C:\logs`, 4, false)
 
-	// Install will fail at os.Executable or schtasks create, but we're testing
-	// that the frequency is parsed correctly via the resolveLogDir and config paths.
-	// A full integration test requires the real binary on Windows.
-	_ = Install(mock, newTestLogger())
-	// If we got past the config parsing without panic, the frequency was handled correctly.
+	for _, a := range args {
+		if a == "/ru" {
+			t.Error("expected no /ru argument for non-admin install")
+		}
+	}
 }

scripts/deploy-windows.sh

@@ -113,7 +113,14 @@ scp_cmd() {
     if [[ -n "$KEY" ]]; then
         scp $SSH_OPTS -i "$KEY" "$src" "${USER}@${HOST}:${dst}"
     elif [[ -n "$PASSWORD" ]]; then
+        if ! command -v sshpass &>/dev/null; then
+            echo "Error: sshpass required for password auth (brew install sshpass / apt install sshpass)" >&2
+            exit 1
+        fi
         SSHPASS="$PASSWORD" sshpass -e scp $SSH_OPTS "$src" "${USER}@${HOST}:${dst}"
+    else
+        echo "Error: provide --key or set DEPLOY_PASSWORD" >&2
+        exit 1
     fi
 }