fix(windows): make tightvnc startup deterministic

internal/cli/bootstrap.go

@@ -1,3 +1,4 @@
+// Builds provider bootstrap scripts that prepare disposable Crabbox machines for SSH and desktop access.
 package cli
 
 import (
@@ -474,32 +475,69 @@ func windowsDesktopBootstrapPowerShell() string {
     "SET_ACCEPTHTTPCONNECTIONS=1", "VALUE_OF_ACCEPTHTTPCONNECTIONS=0"
   ) -Wait
 }
+function Enable-CrabboxTightVNCFirewallRule {
+  $vncExecutable = "C:\Program Files\TightVNC\tvnserver.exe"
+  if (-not (Test-Path -LiteralPath $vncExecutable)) { return }
+  $ruleName = "Crabbox-TightVNC-Loopback"
+  $existing = Get-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue
+  if ($existing) {
+    Set-NetFirewallRule -Name $ruleName -Enabled True -Direction Inbound -Action Allow -Profile Any | Out-Null
+  } else {
+    New-NetFirewallRule -Name $ruleName -DisplayName "Crabbox TightVNC loopback" -Enabled True -Direction Inbound -Program $vncExecutable -Protocol TCP -LocalPort 5900 -Action Allow -Profile Any | Out-Null
+  }
+}
+Enable-CrabboxTightVNCFirewallRule
 $userVNCStartup = @'
-$ErrorActionPreference = "SilentlyContinue"
+$ErrorActionPreference = "Stop"
 $base = "C:\ProgramData\crabbox"
 $password = (Get-Content -Raw -LiteralPath (Join-Path $base "vnc.password")).Trim()
 $serverKey = "HKCU:\Software\TightVNC\Server"
 $serviceKey = "HKLM:\Software\TightVNC\Server"
 $serviceConfig = Get-ItemProperty -Path $serviceKey -ErrorAction SilentlyContinue
-function Set-TightVNCBinaryValue($Name) {
+function Get-TightVNCBinaryHex($Name) {
   $hex = ""
+  $regOutput = & reg.exe query "HKLM\Software\TightVNC\Server" /v $Name 2>$null
+  if ($LASTEXITCODE -eq 0) {
+    foreach ($line in $regOutput) {
+      if ($line -match ("^\s*" + [regex]::Escape($Name) + "\s+REG_BINARY\s+([0-9A-Fa-f]+)\s*$")) {
+        return $Matches[1]
+      }
+    }
+  }
   if ($serviceConfig -and $serviceConfig.$Name) {
     $bytes = [byte[]]$serviceConfig.$Name
     if ($bytes -and $bytes.Length -gt 0) {
       $hex = -join ($bytes | ForEach-Object { $_.ToString("X2") })
     }
   }
-  if ($hex) {
-    & reg.exe add "HKCU\Software\TightVNC\Server" /v $Name /t REG_BINARY /d $hex /f | Out-Null
+  return $hex
+}
+function Set-TightVNCBinaryValue($Name) {
+  $hex = Get-TightVNCBinaryHex $Name
+  if (-not $hex) {
+    return $false
+  }
+  & reg.exe add "HKCU\Software\TightVNC\Server" /v $Name /t REG_BINARY /d $hex /f | Out-Null
+  return ($LASTEXITCODE -eq 0)
+}
+$vncPasswordReady = $false
+for ($i = 1; $i -le 60; $i++) {
+  New-Item -Force -Path $serverKey | Out-Null
+  New-ItemProperty -Force -Path $serverKey -Name UseVncAuthentication -PropertyType DWord -Value 1 | Out-Null
+  $passwordReady = Set-TightVNCBinaryValue "Password"
+  New-ItemProperty -Force -Path $serverKey -Name UseControlAuthentication -PropertyType DWord -Value 1 | Out-Null
+  $controlReady = Set-TightVNCBinaryValue "ControlPassword"
+  New-ItemProperty -Force -Path $serverKey -Name AllowLoopback -PropertyType DWord -Value 1 | Out-Null
+  New-ItemProperty -Force -Path $serverKey -Name AcceptHttpConnections -PropertyType DWord -Value 0 | Out-Null
+  if ($passwordReady -and $controlReady) {
+    $vncPasswordReady = $true
+    break
   }
+  Start-Sleep -Seconds 1
+}
+if (-not $vncPasswordReady) {
+  throw "TightVNC HKCU password values were not copied"
 }
-New-Item -Force -Path $serverKey | Out-Null
-New-ItemProperty -Force -Path $serverKey -Name UseVncAuthentication -PropertyType DWord -Value 1 | Out-Null
-Set-TightVNCBinaryValue "Password"
-New-ItemProperty -Force -Path $serverKey -Name UseControlAuthentication -PropertyType DWord -Value 1 | Out-Null
-Set-TightVNCBinaryValue "ControlPassword"
-New-ItemProperty -Force -Path $serverKey -Name AllowLoopback -PropertyType DWord -Value 1 | Out-Null
-New-ItemProperty -Force -Path $serverKey -Name AcceptHttpConnections -PropertyType DWord -Value 0 | Out-Null
 $exe = "C:\Program Files\TightVNC\tvnserver.exe"
 Get-Process tvnserver -ErrorAction SilentlyContinue | Where-Object { $_.SessionId -eq (Get-Process -Id $PID).SessionId } | Stop-Process -Force -ErrorAction SilentlyContinue
 Start-Sleep -Milliseconds 500
@@ -509,8 +547,11 @@ Set-Content -Encoding UTF8 -LiteralPath $userVNCStartupPath -Value $userVNCStart
 New-Item -ItemType Directory -Force -Path (Split-Path -Parent $userVNCStartupCommandPath) | Out-Null
 Set-Content -Encoding ASCII -LiteralPath $userVNCStartupCommandPath -Value ('@echo off' + [Environment]::NewLine + 'powershell.exe -NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File "' + $userVNCStartupPath + '"' + [Environment]::NewLine)
 $startupTask = "CrabboxUserVNC"
-cmd.exe /c "schtasks.exe /Delete /TN $startupTask /F 2>NUL" | Out-Null
-schtasks.exe /Create /TN $startupTask /SC ONLOGON /TR "powershell.exe -NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File $userVNCStartupPath" /RU $user /IT /F | Out-Null
+Unregister-ScheduledTask -TaskName $startupTask -Confirm:$false -ErrorAction SilentlyContinue
+$startupAction = New-ScheduledTaskAction -Execute "powershell.exe" -Argument ('-NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File "' + $userVNCStartupPath + '"')
+$startupTrigger = New-ScheduledTaskTrigger -AtLogOn -User $user
+$startupPrincipal = New-ScheduledTaskPrincipal -UserId $user -LogonType Interactive -RunLevel Highest
+Register-ScheduledTask -TaskName $startupTask -Action $startupAction -Trigger $startupTrigger -Principal $startupPrincipal -Force | Out-Null
 Get-Service -Name tvnserver -ErrorAction SilentlyContinue | Set-Service -StartupType Disabled
 Stop-Service -Name tvnserver -Force -ErrorAction SilentlyContinue
 $winlogon = "HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Winlogon"

internal/cli/bootstrap_test.go

@@ -568,17 +568,30 @@ func TestAWSUserDataWindowsProfile(t *testing.T) {
 		tightVNCMSISHA256,
 		"VALUE_OF_PASSWORD=$vncPassword",
 		"VALUE_OF_ALLOWLOOPBACK=1",
+		"Enable-CrabboxTightVNCFirewallRule",
+		"Crabbox-TightVNC-Loopback",
+		`New-NetFirewallRule -Name $ruleName -DisplayName "Crabbox TightVNC loopback"`,
+		"-Protocol TCP -LocalPort 5900",
 		"CrabboxUserVNC",
 		"crabbox-user-vnc.cmd",
 		`AppData\Roaming\Microsoft\Windows\Start Menu\Programs\Startup`,
 		"start-user-vnc.ps1",
+		`$ErrorActionPreference = "Stop"`,
+		"Get-TightVNCBinaryHex",
 		"Set-TightVNCBinaryValue",
+		`reg.exe query "HKLM\Software\TightVNC\Server" /v $Name`,
 		`reg.exe add "HKCU\Software\TightVNC\Server"`,
+		`return ($LASTEXITCODE -eq 0)`,
+		`if (-not $vncPasswordReady)`,
+		`throw "TightVNC HKCU password values were not copied"`,
 		`$hex = -join ($bytes | ForEach-Object { $_.ToString("X2") })`,
 		"-run",
 		"NewNetworkWindowOff",
 		"DoNotOpenServerManagerAtLogon",
-		"/SC ONLOGON",
+		"New-ScheduledTaskAction",
+		"New-ScheduledTaskTrigger -AtLogOn -User $user",
+		"New-ScheduledTaskPrincipal -UserId $user -LogonType Interactive -RunLevel Highest",
+		"Register-ScheduledTask",
 		"Set-Service -StartupType Disabled",
 		"Stop-Service -Name tvnserver",
 		"New-CrabboxPassword",
@@ -605,6 +618,9 @@ func TestAWSUserDataWindowsProfile(t *testing.T) {
 	if strings.Contains(got, "/SC ONCE") {
 		t.Fatalf("windows user data should not schedule user VNC as a one-shot task")
 	}
+	if strings.Contains(got, "/SC ONLOGON") {
+		t.Fatalf("windows user data should create the user VNC task with ScheduledTasks cmdlets")
+	}
 	if strings.Contains(got, "Set-Service -StartupType Manual") {
 		t.Fatalf("windows user data should not keep the service VNC fallback enabled")
 	}
@@ -622,6 +638,18 @@ func TestAWSUserDataWindowsProfile(t *testing.T) {
 			t.Fatalf("windows bootstrap must run %q before %q", pair[0], pair[1])
 		}
 	}
+	firewallIndex := strings.Index(got, "Enable-CrabboxTightVNCFirewallRule")
+	userStartupIndex := strings.Index(got, "$userVNCStartup = @'")
+	taskIndex := strings.Index(got, "CrabboxUserVNC")
+	if firewallIndex < 0 || userStartupIndex < 0 || taskIndex < 0 {
+		t.Fatalf("windows user data missing VNC startup markers")
+	}
+	if firewallIndex > userStartupIndex {
+		t.Fatalf("windows user data must create the TightVNC firewall rule before user startup script")
+	}
+	if firewallIndex > taskIndex {
+		t.Fatalf("windows user data must create the TightVNC firewall rule before scheduling VNC")
+	}
 	mirrorIndex := strings.Index(got, "$credentialPaths += $passwordMirrorPath")
 	aclIndex := strings.Index(got, "foreach ($credentialPath in $credentialPaths)")
 	if mirrorIndex < 0 || aclIndex < 0 || mirrorIndex > aclIndex {

worker/src/bootstrap.ts

@@ -1,3 +1,4 @@
+// Builds provider bootstrap scripts that prepare disposable Crabbox machines for SSH and desktop access.
 import { sshPorts, type LeaseConfig } from "./config";
 
 const tightVNCMSIURL =
@@ -486,30 +487,67 @@ function windowsDesktopBootstrapPowerShell(): string {
     "SET_ACCEPTHTTPCONNECTIONS=1", "VALUE_OF_ACCEPTHTTPCONNECTIONS=0"
   ) -Wait
 }
+function Enable-CrabboxTightVNCFirewallRule {
+  $vncExecutable = "C:\\Program Files\\TightVNC\\tvnserver.exe"
+  if (-not (Test-Path -LiteralPath $vncExecutable)) { return }
+  $ruleName = "Crabbox-TightVNC-Loopback"
+  $existing = Get-NetFirewallRule -Name $ruleName -ErrorAction SilentlyContinue
+  if ($existing) {
+    Set-NetFirewallRule -Name $ruleName -Enabled True -Direction Inbound -Action Allow -Profile Any | Out-Null
+  } else {
+    New-NetFirewallRule -Name $ruleName -DisplayName "Crabbox TightVNC loopback" -Enabled True -Direction Inbound -Program $vncExecutable -Protocol TCP -LocalPort 5900 -Action Allow -Profile Any | Out-Null
+  }
+}
+Enable-CrabboxTightVNCFirewallRule
 $userVNCStartup = @'
-$ErrorActionPreference = "SilentlyContinue"
+$ErrorActionPreference = "Stop"
 $serverKey = "HKCU:\\Software\\TightVNC\\Server"
 $serviceKey = "HKLM:\\Software\\TightVNC\\Server"
 $serviceConfig = Get-ItemProperty -Path $serviceKey -ErrorAction SilentlyContinue
-function Set-TightVNCBinaryValue($Name) {
+function Get-TightVNCBinaryHex($Name) {
   $hex = ""
+  $regOutput = & reg.exe query "HKLM\\Software\\TightVNC\\Server" /v $Name 2>$null
+  if ($LASTEXITCODE -eq 0) {
+    foreach ($line in $regOutput) {
+      if ($line -match ("^\\s*" + [regex]::Escape($Name) + "\\s+REG_BINARY\\s+([0-9A-Fa-f]+)\\s*$")) {
+        return $Matches[1]
+      }
+    }
+  }
   if ($serviceConfig -and $serviceConfig.$Name) {
     $bytes = [byte[]]$serviceConfig.$Name
     if ($bytes -and $bytes.Length -gt 0) {
       $hex = -join ($bytes | ForEach-Object { $_.ToString("X2") })
     }
   }
-  if ($hex) {
-    & reg.exe add "HKCU\\Software\\TightVNC\\Server" /v $Name /t REG_BINARY /d $hex /f | Out-Null
+  return $hex
+}
+function Set-TightVNCBinaryValue($Name) {
+  $hex = Get-TightVNCBinaryHex $Name
+  if (-not $hex) {
+    return $false
+  }
+  & reg.exe add "HKCU\\Software\\TightVNC\\Server" /v $Name /t REG_BINARY /d $hex /f | Out-Null
+  return ($LASTEXITCODE -eq 0)
+}
+$vncPasswordReady = $false
+for ($i = 1; $i -le 60; $i++) {
+  New-Item -Force -Path $serverKey | Out-Null
+  New-ItemProperty -Force -Path $serverKey -Name UseVncAuthentication -PropertyType DWord -Value 1 | Out-Null
+  $passwordReady = Set-TightVNCBinaryValue "Password"
+  New-ItemProperty -Force -Path $serverKey -Name UseControlAuthentication -PropertyType DWord -Value 1 | Out-Null
+  $controlReady = Set-TightVNCBinaryValue "ControlPassword"
+  New-ItemProperty -Force -Path $serverKey -Name AllowLoopback -PropertyType DWord -Value 1 | Out-Null
+  New-ItemProperty -Force -Path $serverKey -Name AcceptHttpConnections -PropertyType DWord -Value 0 | Out-Null
+  if ($passwordReady -and $controlReady) {
+    $vncPasswordReady = $true
+    break
   }
+  Start-Sleep -Seconds 1
+}
+if (-not $vncPasswordReady) {
+  throw "TightVNC HKCU password values were not copied"
 }
-New-Item -Force -Path $serverKey | Out-Null
-New-ItemProperty -Force -Path $serverKey -Name UseVncAuthentication -PropertyType DWord -Value 1 | Out-Null
-Set-TightVNCBinaryValue "Password"
-New-ItemProperty -Force -Path $serverKey -Name UseControlAuthentication -PropertyType DWord -Value 1 | Out-Null
-Set-TightVNCBinaryValue "ControlPassword"
-New-ItemProperty -Force -Path $serverKey -Name AllowLoopback -PropertyType DWord -Value 1 | Out-Null
-New-ItemProperty -Force -Path $serverKey -Name AcceptHttpConnections -PropertyType DWord -Value 0 | Out-Null
 $exe = "C:\\Program Files\\TightVNC\\tvnserver.exe"
 Get-Process tvnserver -ErrorAction SilentlyContinue | Where-Object { $_.SessionId -eq (Get-Process -Id $PID).SessionId } | Stop-Process -Force -ErrorAction SilentlyContinue
 Start-Sleep -Milliseconds 500
@@ -519,8 +557,11 @@ Set-Content -Encoding UTF8 -LiteralPath $userVNCStartupPath -Value $userVNCStart
 New-Item -ItemType Directory -Force -Path (Split-Path -Parent $userVNCStartupCommandPath) | Out-Null
 Set-Content -Encoding ASCII -LiteralPath $userVNCStartupCommandPath -Value ('@echo off' + [Environment]::NewLine + 'powershell.exe -NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File "' + $userVNCStartupPath + '"' + [Environment]::NewLine)
 $startupTask = "CrabboxUserVNC"
-cmd.exe /c "schtasks.exe /Delete /TN $startupTask /F 2>NUL" | Out-Null
-schtasks.exe /Create /TN $startupTask /SC ONLOGON /TR "powershell.exe -NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File $userVNCStartupPath" /RU $user /IT /F | Out-Null
+Unregister-ScheduledTask -TaskName $startupTask -Confirm:$false -ErrorAction SilentlyContinue
+$startupAction = New-ScheduledTaskAction -Execute "powershell.exe" -Argument ('-NoProfile -WindowStyle Hidden -ExecutionPolicy Bypass -File "' + $userVNCStartupPath + '"')
+$startupTrigger = New-ScheduledTaskTrigger -AtLogOn -User $user
+$startupPrincipal = New-ScheduledTaskPrincipal -UserId $user -LogonType Interactive -RunLevel Highest
+Register-ScheduledTask -TaskName $startupTask -Action $startupAction -Trigger $startupTrigger -Principal $startupPrincipal -Force | Out-Null
 Get-Service -Name tvnserver -ErrorAction SilentlyContinue | Set-Service -StartupType Disabled
 Stop-Service -Name tvnserver -Force -ErrorAction SilentlyContinue
 $winlogon = "HKLM:\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Winlogon"

worker/test/bootstrap.test.ts

@@ -490,19 +490,43 @@ describe("cloud-init bootstrap", () => {
     expect(got).toContain("DoNotOpenServerManagerAtLogon");
     expect(got).toContain("VALUE_OF_PASSWORD=$vncPassword");
     expect(got).toContain("VALUE_OF_ALLOWLOOPBACK=1");
+    expect(got).toContain("Enable-CrabboxTightVNCFirewallRule");
+    expect(got).toContain("Crabbox-TightVNC-Loopback");
+    expect(got).toContain(
+      'New-NetFirewallRule -Name $ruleName -DisplayName "Crabbox TightVNC loopback"',
+    );
+    expect(got).toContain("-Protocol TCP -LocalPort 5900");
     expect(got).toContain("CrabboxUserVNC");
     expect(got).toContain("crabbox-user-vnc.cmd");
     expect(got).toContain("AppData\\Roaming\\Microsoft\\Windows\\Start Menu\\Programs\\Startup");
     expect(got).toContain("start-user-vnc.ps1");
+    expect(got).toContain('$ErrorActionPreference = "Stop"');
+    expect(got).toContain("Get-TightVNCBinaryHex");
     expect(got).toContain("Set-TightVNCBinaryValue");
+    expect(got).toContain('reg.exe query "HKLM\\Software\\TightVNC\\Server" /v $Name');
     expect(got).toContain('reg.exe add "HKCU\\Software\\TightVNC\\Server"');
+    expect(got).toContain("return ($LASTEXITCODE -eq 0)");
+    expect(got).toContain("if (-not $vncPasswordReady)");
+    expect(got).toContain('throw "TightVNC HKCU password values were not copied"');
     expect(got).toContain('$hex = -join ($bytes | ForEach-Object { $_.ToString("X2") })');
-    expect(got).toContain("/SC ONLOGON");
+    expect(got).toContain("New-ScheduledTaskAction");
+    expect(got).toContain("New-ScheduledTaskTrigger -AtLogOn -User $user");
+    expect(got).toContain(
+      "New-ScheduledTaskPrincipal -UserId $user -LogonType Interactive -RunLevel Highest",
+    );
+    expect(got).toContain("Register-ScheduledTask");
     expect(got).toContain("Set-Service -StartupType Disabled");
     expect(got).toContain("Stop-Service -Name tvnserver");
     expect(got).not.toContain("/SC ONCE");
+    expect(got).not.toContain("/SC ONLOGON");
     expect(got).not.toContain("Set-Service -StartupType Manual");
     expect(got).not.toContain("Start-Service -Name tvnserver");
+    expect(got.indexOf("Enable-CrabboxTightVNCFirewallRule")).toBeLessThan(
+      got.indexOf("$userVNCStartup = @'"),
+    );
+    expect(got.indexOf("Enable-CrabboxTightVNCFirewallRule")).toBeLessThan(
+      got.indexOf("CrabboxUserVNC"),
+    );
     expect(got).toContain("New-CrabboxPassword");
     expect(got).toContain("${userSID}:F");
     expect(got).toContain("$credentialPaths = @($passwordPath)");