Keep desktop updates, rollback and first-run handoff on the right target

.github/workflows/ci.yml

@@ -172,6 +172,7 @@ jobs:
           .\deploy\Test-DeploymentTemplates.ps1
           .\deploy\Test-ArtifactSecurity.ps1
           .\deploy\Test-SetupAdapter.ps1
+          .\deploy\Test-LocalServerHttp.ps1
           .\deploy\Test-FailoverRecovery.ps1
           .\deploy\Test-MachinePath.ps1
           .\deploy\Test-SwitcherConfig.ps1
@@ -198,6 +199,7 @@ jobs:
         run: |
           .\deploy\Test-ArtifactSecurity.ps1
           .\deploy\Test-SetupAdapter.ps1
+          .\deploy\Test-LocalServerHttp.ps1
           .\deploy\Test-FailoverRecovery.ps1
           .\deploy\Test-MachinePath.ps1
           .\deploy\Test-SwitcherConfig.ps1

CHANGELOG.md

@@ -84,6 +84,22 @@ Planned version: **1.5.4**.
   - The MCP server rejects broken nodes and edges before it merges a workflow change (#726).
   - All MCP requests, including dependency queries, wait until the server connection is ready
     (#727).
+- **Setup and uninstall**
+  - A server uninstall whose cleanup fails stops before the "Apps & features" entry is removed,
+    keeps the installation marker and the deploy scripts and exits with a non-zero code, so the
+    same uninstall can be run again. A failed service deletion stops it before any file is removed.
+    The desktop uninstaller stops the same way and keeps NodePilot installed (#714).
+  - A desktop update keeps a custom data path and custom service names (#709).
+  - When a desktop update fails after its services were removed, the rollback registers both
+    services again with their previous settings and folder permissions, so the old version starts
+    again (#710).
+  - Deleting the data on uninstall (`-PurgeData`, `/PURGEDATA=1`) also works on German Windows. If
+    ownership of an item cannot be taken, nothing under the data directory is deleted (#731).
+  - When a reinstall that changes the service account fails, the rollback gives the previous
+    account its access to the install directory back, so the restored service starts (#735).
+  - The gMSA repair command suggested by the setup check adds this host to the existing principals
+    instead of replacing them. The deploy README no longer suggests delegation on every target
+    (#713).
 - **Monitoring**
   - The observability summary queries the counter names the exporter actually publishes, so
     executions per minute, success rate and failures show values again. The Grafana CPU panels use
@@ -121,6 +137,13 @@ Planned version: **1.5.4**.
 - The CLI, the MCP server and the Python admin scripts under `scripts/` no longer follow redirects
   with the login token, so the token is never sent to another host (#711).
 - Logging out with the CLI also ends the MCP server's session right away (#712).
+- The server setup accepts only the configured certificate, by thumbprint, for its health probe and
+  the first login, does not follow redirects there and no longer turns off certificate validation
+  for the whole process.
+- The desktop setup hands the first-login token to the account that started setup, also under
+  Remote Desktop or a switched user, and to the user signed in at the console when software
+  distribution runs setup as SYSTEM. If the starting account cannot be determined, provisioning stops
+  instead of handing the token to another profile (#732).
 - MCP alerting output shows webhook targets only as scheme, host and port, because the rest of the
   URL can contain tokens (#730).
 - GitHub tokens in step output are masked except for their prefix such as `ghp_`; before, the whole

deploy/ArtifactSecurity.ps1

@@ -697,6 +697,35 @@ function Resolve-SidLabel {
     }
 }
 
+function Get-NodePilotDirectoryAccessSnapshot {
+    <#
+      The DACL of an existing directory, including its protection flag, so a rollback can give a
+      previous service identity back exactly the access it had. $null when the directory is absent.
+    #>
+    param([Parameter(Mandatory)][string]$Path)
+    if (-not (Test-Path -LiteralPath $Path -PathType Container)) { return $null }
+    $access = (Get-Acl -LiteralPath $Path).GetSecurityDescriptorSddlForm(
+        [System.Security.AccessControl.AccessControlSections]::Access)
+    return [pscustomobject]@{ Path = $Path; Access = $access }
+}
+
+function Restore-NodePilotDirectoryAccessSnapshot {
+    <#
+      Writes a DACL captured by Get-NodePilotDirectoryAccessSnapshot back and verifies it. Inheriting
+      children pick it up, so it runs before restored binaries are copied in and started.
+    #>
+    param([Parameter(Mandatory)][object]$Snapshot)
+    $acl = Get-Acl -LiteralPath $Snapshot.Path
+    $acl.SetSecurityDescriptorSddlForm($Snapshot.Access,
+        [System.Security.AccessControl.AccessControlSections]::Access)
+    Set-Acl -LiteralPath $Snapshot.Path -AclObject $acl
+    $restored = (Get-Acl -LiteralPath $Snapshot.Path).GetSecurityDescriptorSddlForm(
+        [System.Security.AccessControl.AccessControlSections]::Access)
+    if ($restored -ne $Snapshot.Access) {
+        throw "The previous access rules on '$($Snapshot.Path)' could not be restored."
+    }
+}
+
 function Assert-ServiceDirectoryAclUsable {
     <#
       Verify, repair once, verify again, then fail instead of handing the service a directory it

deploy/Build-Artifact.ps1

@@ -611,6 +611,7 @@ $deployScriptFiles = @(
     'Uninstall-NodePilot.ps1'            # entry point
     'ArtifactSecurity.ps1'               # dot-sourced by install + update
     'DeploymentProgress.ps1'
+    'LocalServerHttp.ps1'
     'Preflight.ps1'                      # dot-sourced by install
     'ServiceControl.ps1'                 # dot-sourced by install + update
     'MachinePath.ps1'                    # dot-sourced by install + update + uninstall

deploy/Install-NodePilot.ps1

@@ -220,6 +220,7 @@ if (-not (Test-Path -LiteralPath $ArtifactSecurityScript -PathType Leaf)) {
 }
 . $ArtifactSecurityScript
 . (Join-Path $PSScriptRoot 'DeploymentProgress.ps1')
+. (Join-Path $PSScriptRoot 'LocalServerHttp.ps1')
 
 # Readiness checks live in their own file because the setup wizard runs the same set behind a
 # re-check button. Because of that shared use, nothing in Preflight.ps1 may mutate anything.
@@ -250,6 +251,8 @@ $postgresRootCertificatePreviouslyExisted = $false
 $postgresRootCertificateTouched = $false
 $installMutationStarted = $false
 $dataAclApplied = $false
+$previousInstallRootAccess = $null
+$installRootAclApplied = $false
 try {
     if (-not (Test-Path -LiteralPath $ArtifactPath -PathType Leaf)) { throw "Artifact not found: $ArtifactPath" }
     $ArtifactPath = (Resolve-Path -LiteralPath $ArtifactPath).Path
@@ -585,47 +588,19 @@ function Enable-SqlReadCommittedSnapshot {
 function Invoke-HealthProbe {
     param(
         [Parameter(Mandatory)][string]$Url,
+        [Parameter(Mandatory)][string]$CertificateThumbprint,
         [int]$TimeoutSeconds = 60
     )
-    # Trust any cert on localhost - the installer may run before a public hostname resolves,
-    # and the only caller is the installer itself.
-    if ($PSVersionTable.PSVersion.Major -ge 6) {
-        $deadline = (Get-Date).AddSeconds($TimeoutSeconds)
-        while ((Get-Date) -lt $deadline) {
-            try {
-                $r = Invoke-WebRequest -Uri $Url -UseBasicParsing -SkipCertificateCheck -TimeoutSec 5
-                if ($r.StatusCode -eq 200) { return $true }
-            } catch { Start-Sleep -Seconds 2 }
-        }
-        return $false
-    } else {
-        if (-not ('TrustAllCerts' -as [type])) {
-            Add-Type @"
-using System.Net; using System.Security.Cryptography.X509Certificates;
-public class TrustAllCerts : ICertificatePolicy {
-  public bool CheckValidationResult(ServicePoint s, X509Certificate c, WebRequest r, int p) { return true; }
-}
-"@
-        }
-        $previousCertificatePolicy = [System.Net.ServicePointManager]::CertificatePolicy
-        $previousSecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol
+    $deadline = (Get-Date).AddSeconds($TimeoutSeconds)
+    while ((Get-Date) -lt $deadline) {
         try {
-            [System.Net.ServicePointManager]::CertificatePolicy = New-Object TrustAllCerts
-            [System.Net.ServicePointManager]::SecurityProtocol = 'Tls12, Tls13'
-            $deadline = (Get-Date).AddSeconds($TimeoutSeconds)
-            while ((Get-Date) -lt $deadline) {
-                try {
-                    $r = Invoke-WebRequest -Uri $Url -UseBasicParsing -TimeoutSec 5
-                    if ($r.StatusCode -eq 200) { return $true }
-                } catch { Start-Sleep -Seconds 2 }
-            }
-            return $false
-        }
-        finally {
-            [System.Net.ServicePointManager]::CertificatePolicy = $previousCertificatePolicy
-            [System.Net.ServicePointManager]::SecurityProtocol = $previousSecurityProtocol
+            $response = Invoke-NodePilotLocalHttps -Uri $Url -CertificateThumbprint $CertificateThumbprint -TimeoutSeconds 5
+            if ($response.StatusCode -eq 200) { return $true }
         }
+        catch { }
+        Start-Sleep -Seconds 2
     }
+    return $false
 }
 
 function Grant-LogOnAsServiceRight {
@@ -1008,6 +983,10 @@ if (Test-Path -LiteralPath $previousSettingsPath -PathType Leaf) {
     $previousSettingsBytes = [IO.File]::ReadAllBytes($previousSettingsPath)
 }
 
+# The install root grants the service identity execute access. A failed identity change must give
+# the previous identity that access back before its restored binaries start.
+$previousInstallRootAccess = Get-NodePilotDirectoryAccessSnapshot -Path $InstallPath
+
 if ((Test-Path -LiteralPath $InstallPath -PathType Container) -and
     @(Get-ChildItem -LiteralPath $InstallPath -Force).Count -gt 0) {
     Write-Step 'Backing up the existing installation'
@@ -1073,6 +1052,7 @@ New-Item -ItemType Directory -Path (Join-Path $DataPath 'logs') -Force | Out-Nul
 # The binaries are read-only for the service: it executes them, it never rewrites them.
 # Inheritance is dropped here too, so a permissive parent (a custom root on D:\ inherits
 # BUILTIN\Users:(M) from the volume) cannot reach the files the service starts from.
+$installRootAclApplied = $true
 Set-DirectoryAclForService -Path $InstallPath -ServiceAccount $AclIdentity `
     -ReadOnlyForService -SkipServiceRule:$isLocalSystem
 Set-DirectoryAclForService -Path $DataPath -ServiceAccount $AclIdentity -SkipServiceRule:$isLocalSystem
@@ -1438,7 +1418,7 @@ try {
 # spamming "false-positive failed" reports for installs that ARE actually succeeding.
 $probeUrl = "https://localhost:$HttpsPort/healthz/ready"
 Write-Info "  Probing $probeUrl (up to 180s)..."
-if (-not (Invoke-HealthProbe -Url $probeUrl -TimeoutSeconds 180)) {
+if (-not (Invoke-HealthProbe -Url $probeUrl -CertificateThumbprint $NormalizedThumbprint -TimeoutSeconds 180)) {
     Show-NodePilotServiceStartDiagnostics -DataPath $DataPath
     throw "Service did not report /healthz/ready within 180s. Check the NodePilot and Windows Application logs for the startup error."
 }
@@ -1670,6 +1650,12 @@ catch {
                 Remove-ExistingService -Name $ServiceName -InstallPath $InstallPath `
                     -FoundMessage "Removing the service '$ServiceName' that this run registered."
             }
+            # Before the binaries are copied back: restored files inherit this DACL, and the
+            # previous service identity has to be able to execute them.
+            if ($installRootAclApplied -and $previousInstallRootAccess -and
+                (Test-Path -LiteralPath $InstallPath -PathType Container)) {
+                Restore-NodePilotDirectoryAccessSnapshot -Snapshot $previousInstallRootAccess
+            }
             if ($installRollback) {
                 # Refuses a backup whose DACL or files changed since this run made it, before the
                 # install directory is wiped.

deploy/Invoke-NodePilotSetup.ps1

@@ -88,6 +88,7 @@ if ([string]::IsNullOrWhiteSpace($PayloadRoot)) {
 
 . (Join-Path $scriptDirectory 'SetupContract.ps1')
 . (Join-Path $scriptDirectory 'DeploymentProgress.ps1')
+. (Join-Path $scriptDirectory 'LocalServerHttp.ps1')
 . (Join-Path $scriptDirectory 'SetupProvisioning.ps1')
 
 $result = New-NodePilotResultBuffer
@@ -135,6 +136,7 @@ function Invoke-NodePilotFirstLogin {
         Justification = 'Sent as a JSON body and written to the credential file; plaintext throughout by design.')]
     param(
         [Parameter(Mandatory)][int]$HttpsPort,
+        [Parameter(Mandatory)][string]$CertificateThumbprint,
         [Parameter(Mandatory)][string]$Username,
         [Parameter(Mandatory)][string]$Password,
         [Parameter(Mandatory)][string]$Token
@@ -144,31 +146,11 @@ function Invoke-NodePilotFirstLogin {
     $body = @{ username = $Username; password = $Password } | ConvertTo-Json -Compress
     $headers = @{ 'X-Setup-Token' = $Token }
 
-    # The certificate on a loopback call names the public hostname, not localhost, so validation
-    # is relaxed for this request. The previous policy is restored in the finally block so nothing
-    # else in this process inherits it.
-    $previousPolicy = $null
-    $policyChanged = $false
     try {
-        if ($PSVersionTable.PSVersion.Major -lt 6) {
-            if (-not ('TrustAllCertsBootstrap' -as [type])) {
-                Add-Type @"
-using System.Net; using System.Security.Cryptography.X509Certificates;
-public class TrustAllCertsBootstrap : ICertificatePolicy {
-  public bool CheckValidationResult(ServicePoint s, X509Certificate c, WebRequest r, int p) { return true; }
-}
-"@
-            }
-            $previousPolicy = [System.Net.ServicePointManager]::CertificatePolicy
-            [System.Net.ServicePointManager]::CertificatePolicy = New-Object TrustAllCertsBootstrap
-            [System.Net.ServicePointManager]::SecurityProtocol = 'Tls12, Tls13'
-            $policyChanged = $true
-            $response = Invoke-WebRequest -Uri $uri -Method POST -Body $body -Headers $headers `
-                -ContentType 'application/json' -UseBasicParsing -TimeoutSec 30
-        }
-        else {
-            $response = Invoke-WebRequest -Uri $uri -Method POST -Body $body -Headers $headers `
-                -ContentType 'application/json' -UseBasicParsing -TimeoutSec 30 -SkipCertificateCheck
+        $response = Invoke-NodePilotLocalHttps -Uri $uri -CertificateThumbprint $CertificateThumbprint `
+            -Method POST -Body $body -Headers $headers -TimeoutSeconds 30
+        if ($response.StatusCode -lt 200 -or $response.StatusCode -ge 300) {
+            throw "Bootstrap login returned HTTP $($response.StatusCode); redirects are not accepted."
         }
         return [pscustomobject]@{ Status = 'Created'; Detail = "HTTP $($response.StatusCode)" }
     }
@@ -186,9 +168,6 @@ public class TrustAllCertsBootstrap : ICertificatePolicy {
         } catch { }
         return [pscustomobject]@{ Status = 'Failed'; Detail = $detail }
     }
-    finally {
-        if ($policyChanged) { [System.Net.ServicePointManager]::CertificatePolicy = $previousPolicy }
-    }
 }
 
 function Get-NodePilotServiceCrashReason {
@@ -783,6 +762,7 @@ function Invoke-SetupInstall {
         $bootstrapUser = [string]$answers['bootstrap.adminUsername']
         $bootstrapPassword = New-NodePilotBootstrapPassword
         $outcome = Invoke-NodePilotFirstLogin -HttpsPort ([int]$answers['network.httpsPort']) `
+            -CertificateThumbprint $splat['CertThumbprint'] `
             -Username $bootstrapUser -Password $bootstrapPassword -Token $token
 
         Set-NodePilotResult -Buffer $result -Section 'bootstrap' -Name 'status' -Value $outcome.Status

deploy/LocalServerHttp.ps1

@@ -0,0 +1,73 @@
+#requires -Version 5.1
+[CmdletBinding()]
+param()
+
+$ErrorActionPreference = 'Stop'
+Set-StrictMode -Version 3.0
+
+if (-not ('NodePilot.Deployment.LocalServerHttp' -as [type])) {
+    # The Framework-compatible HTTP API is obsolete on PowerShell 7, but its per-request
+    # certificate callback is supported on both runtimes.
+    Add-Type -IgnoreWarnings -TypeDefinition @'
+using System;
+using System.Net;
+using System.Text.RegularExpressions;
+
+namespace NodePilot.Deployment
+{
+    public static class LocalServerHttp
+    {
+        public static HttpWebRequest CreateRequest(Uri uri, string certificateThumbprint, int timeoutMilliseconds)
+        {
+            if (!uri.IsAbsoluteUri || uri.Scheme != Uri.UriSchemeHttps || !uri.IsLoopback || uri.UserInfo.Length != 0)
+                throw new ArgumentException("Setup requests require a loopback HTTPS URL.", "uri");
+            if (certificateThumbprint == null || !Regex.IsMatch(certificateThumbprint, "\\A[0-9a-fA-F]{40}\\z"))
+                throw new ArgumentException("The configured server certificate thumbprint is required.", "certificateThumbprint");
+            if (timeoutMilliseconds <= 0)
+                throw new ArgumentOutOfRangeException("timeoutMilliseconds");
+
+            var request = (HttpWebRequest)WebRequest.Create(uri);
+            request.AllowAutoRedirect = false;
+            request.Proxy = null;
+            request.Timeout = timeoutMilliseconds;
+            request.ReadWriteTimeout = timeoutMilliseconds;
+            // Preflight validates the selected certificate. Loopback uses that exact identity,
+            // even when the certificate names the public hostname or is explicitly self-signed.
+            request.ServerCertificateValidationCallback = (sender, certificate, chain, errors) =>
+                certificate != null && String.Equals(certificate.GetCertHashString(), certificateThumbprint, StringComparison.OrdinalIgnoreCase);
+            return request;
+        }
+    }
+}
+'@
+}
+
+function Invoke-NodePilotLocalHttps {
+    [CmdletBinding()]
+    param(
+        [Parameter(Mandatory)][uri]$Uri,
+        [Parameter(Mandatory)][string]$CertificateThumbprint,
+        [ValidateSet('GET', 'POST')][string]$Method = 'GET',
+        [string]$Body,
+        [hashtable]$Headers = @{},
+        [ValidateRange(1, 300)][int]$TimeoutSeconds = 30
+    )
+
+    $request = [NodePilot.Deployment.LocalServerHttp]::CreateRequest($Uri, $CertificateThumbprint, $TimeoutSeconds * 1000)
+    $request.Method = $Method
+    foreach ($key in $Headers.Keys) { $request.Headers.Add([string]$key, [string]$Headers[$key]) }
+    if ($Method -eq 'POST') {
+        $request.ContentType = 'application/json'
+        $bytes = [Text.Encoding]::UTF8.GetBytes($Body)
+        try {
+            $request.ContentLength = $bytes.Length
+            $stream = $request.GetRequestStream()
+            try { $stream.Write($bytes, 0, $bytes.Length) }
+            finally { $stream.Dispose() }
+        }
+        finally { [Array]::Clear($bytes, 0, $bytes.Length) }
+    }
+    $response = $request.GetResponse()
+    try { return [pscustomobject]@{ StatusCode = [int]$response.StatusCode } }
+    finally { $response.Dispose() }
+}