From dc0e230e4a611d7fc9db7e6dd54f91ad77846a36 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:31:38 +0100 Subject: [PATCH 1/3] Harden server setup rollback, uninstall and local TLS - Setup health probe and first login pin the configured server certificate per request and reject redirects, instead of switching off certificate validation for the whole process (D01). - A failed reinstall that changes the service account restores the install root DACL before the old binaries come back (#735). - Uninstall stops when the service cannot be deleted, keeps the marker and deploy\ until everything else is gone, and the Inno uninstaller aborts on a failed cleanup so it can be repeated (#714, server part). - PurgeData takes ownership with takeown /A item by item, so it works on German Windows, and deletes nothing when a native step fails (#731). - The gMSA repair command reads the existing retrieval principals and adds this host; the deploy README drops the blanket RBCD recipe (#713). Closes #713 Closes #731 Closes #735 Refs #714 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 + CHANGELOG.md | 15 ++ deploy/ArtifactSecurity.ps1 | 29 +++ deploy/Build-Artifact.ps1 | 1 + deploy/Install-NodePilot.ps1 | 60 ++---- deploy/Invoke-NodePilotSetup.ps1 | 34 +-- deploy/LocalServerHttp.ps1 | 73 +++++++ deploy/Preflight.ps1 | 15 +- deploy/README.md | 45 ++-- deploy/Test-DeploymentTemplates.ps1 | 156 +++++++++++++- deploy/Test-LocalServerHttp.ps1 | 81 +++++++ deploy/Test-SetupAdapter.ps1 | 23 ++ deploy/Uninstall-NodePilot.ps1 | 201 ++++++++++++------ deploy/server/Build-ServerInstaller.ps1 | 1 + deploy/server/NodePilotServer.iss | 32 ++- .../content/de/deployment/production.md | 4 + .../content/en/deployment/production.md | 4 + 17 files changed, 614 insertions(+), 162 deletions(-) create mode 100644 deploy/LocalServerHttp.ps1 create mode 100644 deploy/Test-LocalServerHttp.ps1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55ace603..fef74826 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a760ccc..ff9d8719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,18 @@ 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 + (#714). + - 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 +133,9 @@ 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. - 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 diff --git a/deploy/ArtifactSecurity.ps1 b/deploy/ArtifactSecurity.ps1 index 6ad35e5e..26433606 100644 --- a/deploy/ArtifactSecurity.ps1 +++ b/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 diff --git a/deploy/Build-Artifact.ps1 b/deploy/Build-Artifact.ps1 index 866fd3a8..5c847978 100644 --- a/deploy/Build-Artifact.ps1 +++ b/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 diff --git a/deploy/Install-NodePilot.ps1 b/deploy/Install-NodePilot.ps1 index 7d7b6194..691d0449 100644 --- a/deploy/Install-NodePilot.ps1 +++ b/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. diff --git a/deploy/Invoke-NodePilotSetup.ps1 b/deploy/Invoke-NodePilotSetup.ps1 index cc710d25..e302135d 100644 --- a/deploy/Invoke-NodePilotSetup.ps1 +++ b/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 diff --git a/deploy/LocalServerHttp.ps1 b/deploy/LocalServerHttp.ps1 new file mode 100644 index 00000000..5633e88d --- /dev/null +++ b/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() } +} diff --git a/deploy/Preflight.ps1 b/deploy/Preflight.ps1 index d4c36f56..ef252329 100644 --- a/deploy/Preflight.ps1 +++ b/deploy/Preflight.ps1 @@ -1078,6 +1078,18 @@ function Get-NodePilotGmsaRetrievalState { catch { return & $unknown $_.Exception.Message } } +function Get-NodePilotGmsaRetrievalRemediation { + param([Parameter(Mandatory)][string]$Sam, [Parameter(Mandatory)][string]$ComputerName) + $accountLiteral = $Sam.Replace("'", "''") + $computerLiteral = $ComputerName.Replace("'", "''") + @( + ('$account = Get-ADServiceAccount -Identity ''{0}'' -Properties PrincipalsAllowedToRetrieveManagedPassword -ErrorAction Stop' -f $accountLiteral) + '$principals = @($account.PrincipalsAllowedToRetrieveManagedPassword | ForEach-Object { if ($_ -is [string]) { $_ } else { $_.DistinguishedName } })' + ('$principals += (Get-ADComputer -Identity ''{0}'' -ErrorAction Stop).DistinguishedName' -f $computerLiteral) + 'Set-ADServiceAccount -Identity $account -PrincipalsAllowedToRetrieveManagedPassword @($principals | Sort-Object -Unique) -ErrorAction Stop' + ) -join "`r`n" +} + function New-NodePilotGmsaRetrievalResult { <# The verdict for the LDAP answer, separated from the directory call so every branch is @@ -1110,8 +1122,7 @@ function New-NodePilotGmsaRetrievalResult { -RemediationHint ('Allow this host to retrieve the password, then RESTART it - a computer ' + "account's group membership only reaches the machine through a new " + 'Kerberos ticket. As Domain Admin:') ` - -Remediation ("Set-ADServiceAccount -Identity $Sam -PrincipalsAllowedToRetrieveManagedPassword " + - "(Get-ADComputer '$env:COMPUTERNAME')") ` + -Remediation (Get-NodePilotGmsaRetrievalRemediation -Sam $Sam -ComputerName $env:COMPUTERNAME) ` -AbortMessage "$env:COMPUTERNAME may not retrieve the managed password of '$Sam'." } New-NodePilotPreflightResult -Id 'gmsa' -Title $Title -Status 'Pass' ` diff --git a/deploy/README.md b/deploy/README.md index 678a60d0..a632a0af 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -257,24 +257,19 @@ The shipped default profile leaves all external providers disabled and sets loca `BreakGlassOnly`. SCIM tokens are rotated without interruption by keeping the old value briefly under `Authentication:Scim:PreviousBearerToken` and deleting it after the IdP switch. -### 5. Kerberos constrained delegation (for WinRM targets) +### 5. Authorize the WinRM execution identity -> This section concerns the **integrated (credential-less) WinRM path only**. If NodePilot uses credentials stored per machine, no delegation is needed — the service's identity is then irrelevant. Under **LocalSystem** the identity to delegate is the **computer account** of the NodePilot server (`Get-ADComputer `) instead of the gMSA — otherwise the procedure is identical. +The integrated (credential-less) WinRM path authenticates directly as the NodePilot +process identity: the gMSA, or the host computer account when running as LocalSystem. +Grant that identity permission to the target's WinRM endpoint and the resources used +by the workflow. For LocalSystem HA installations, authorize each NodePilot host. +This first hop needs no resource-based constrained delegation and does not impersonate +the browser user. Stored per-machine credentials use their own account permissions. -For the gMSA to reach target machines via implicit Kerberos, **resource-based constrained delegation** -has to be configured on every target server: - -```powershell -# On a DC: -$gmsa = Get-ADServiceAccount -Identity svc-nodepilot -foreach ($target in $targetHosts) { - $computer = Get-ADComputer -Identity $target - Set-ADComputer -Identity $target ` - -PrincipalsAllowedToDelegateToAccount ( - (Get-ADComputer $target).PrincipalsAllowedToDelegateToAccount + $gmsa - ) -} -``` +Delegated-user or second-hop access is a separate topology requiring an explicit design; +do not enable delegation on every target as part of this setup. Existing AD retrieval +or delegation grants must be preserved when an administrator changes them. The setup's +gMSA repair snippet reads existing password-retrieval principals before adding this host. WinRM endpoint on the targets (if not already active): @@ -562,8 +557,14 @@ in the lab: 31 seconds). If the service is deleted in that window, the process i longer reachable through the SCM — and the subsequent deletion pulls the DLLs out from under a running application. The script therefore waits up to `-ProcessExitTimeoutSeconds` (default 90) for processes from `InstallPath` and otherwise aborts **with the service still registered**, so that a supported way -to stop it remains. If files are left behind afterwards (virus scanner, search indexer), the script -still runs to completion, names the leftovers together with the holding process, and exits with code 1. +to stop it remains. A failed `sc.exe delete` stops it the same way, before any file is removed. If files +are left behind afterwards (virus scanner, search indexer), the script still runs to completion, names +the leftovers together with the holding process, and exits with code 1. + +**A failed uninstall can be repeated.** The installation marker and `\deploy\` (which holds +this script) are removed only after everything else is gone. Any non-zero exit makes the GUI and +`/SILENT` uninstaller abort before Inno removes a file or the "Apps & features" entry, and the +uninstaller itself exits non-zero; fix the reported cause and uninstall again. Two things are **deliberately** left in place, because both can be shared with something else on the host and blindly revoking them would break exactly that: the gMSA's "Log on as a service" right and the @@ -573,9 +574,11 @@ uninstall whether it is still there. **`-PurgeData` takes ownership first.** The installer writes `jwt-secret.key` and `admin-setup.token` owner-only to the *service account*; otherwise not even an administrator can delete them and the run -aborts halfway. The script therefore calls `takeown` and `icacls` with the well-known SID -`S-1-5-32-544` — **without** `(OI)(CI)`: those are container inheritance flags that are silently -discarded on a leaf file while `icacls` still reports success. +aborts halfway. The script therefore takes ownership item by item with `takeown /A` (the Administrators +group, without the localized confirmation answer `takeown /R /D` expects) and grants the well-known SID +`S-1-5-32-544` with `icacls` — **without** `(OI)(CI)`: those are container inheritance flags that are +silently discarded on a leaf file while `icacls` still reports success. If either tool fails for any +item, nothing under the data directory is deleted and the run exits with code 1. ## Backup & disaster recovery diff --git a/deploy/Test-DeploymentTemplates.ps1 b/deploy/Test-DeploymentTemplates.ps1 index 9cefb1fc..644e3bbb 100644 --- a/deploy/Test-DeploymentTemplates.ps1 +++ b/deploy/Test-DeploymentTemplates.ps1 @@ -1023,6 +1023,54 @@ Assert-TextMatches -Name 'a failed first install is narrated as an undo' ` # wording written for an upgrade. Assert-TextMatches -Name 'the rollback names the service it registered itself' ` -Text $rollbackBlock -Pattern "FoundMessage `"Removing the service '\`$ServiceName' that this run registered\.`"" +# The install root carries the execute grant for the service identity. A failed gMSA A-to-B +# reinstall restored A's binaries under B's DACL, so A could not start them. +Assert-TextMatches -Name 'the installer snapshots the install root DACL before changing it' ` + -Text $installerScript -Pattern '(?s)\$previousInstallRootAccess = Get-NodePilotDirectoryAccessSnapshot -Path \$InstallPath.*?\$installRootAclApplied = \$true\s*\r?\nSet-DirectoryAclForService -Path \$InstallPath' +Assert-TextMatches -Name 'the rollback restores install root access before the binaries and the service' ` + -Text $rollbackBlock -Pattern '(?s)Restore-NodePilotDirectoryAccessSnapshot -Snapshot \$previousInstallRootAccess.*?Restore-NodePilotBinaryBackup -Backup \$installRollback.*?Restore-ServiceRollbackSnapshot -Snapshot \$previousService' +& { + . $ArtifactSecurityPath + $root = Join-Path ([IO.Path]::GetTempPath()) ('nodepilot-installroot-' + [Guid]::NewGuid().ToString('N')) + New-Item -ItemType Directory -Path $root | Out-Null + # LocalService and NetworkService stand in for gMSA A and gMSA B. + $identityA = New-Object System.Security.Principal.SecurityIdentifier('S-1-5-19') + $identityB = New-Object System.Security.Principal.SecurityIdentifier('S-1-5-20') + $me = [System.Security.Principal.WindowsIdentity]::GetCurrent().User + $setRootAcl = { + param($ServiceIdentity) + $acl = New-Object System.Security.AccessControl.DirectorySecurity + $acl.SetAccessRuleProtection($true, $false) + foreach ($grant in @(@($me, 'FullControl'), @($ServiceIdentity, 'ReadAndExecute'))) { + $acl.AddAccessRule((New-Object System.Security.AccessControl.FileSystemAccessRule( + $grant[0], $grant[1], 'ContainerInherit,ObjectInherit', 'None', 'Allow'))) + } + Set-Acl -LiteralPath $root -AclObject $acl + } + $grantsTo = { + param($Path, $Sid) + @((Get-Acl -LiteralPath $Path).Access | Where-Object { + $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]) -eq $Sid -and + ($_.FileSystemRights -band [System.Security.AccessControl.FileSystemRights]::ExecuteFile) + }).Count -gt 0 + } + try { + & $setRootAcl $identityA + $snapshot = Get-NodePilotDirectoryAccessSnapshot -Path $root + & $setRootAcl $identityB + Restore-NodePilotDirectoryAccessSnapshot -Snapshot $snapshot + [IO.File]::WriteAllText((Join-Path $root 'NodePilot.Api.exe'), 'restored binary') + $restored = Join-Path $root 'NodePilot.Api.exe' + if (-not (& $grantsTo $restored $identityA) -or (& $grantsTo $restored $identityB)) { + throw 'Deployment template check failed: a restored binary must be executable by the previous identity only.' + } + if ($null -ne (Get-NodePilotDirectoryAccessSnapshot -Path (Join-Path $root 'absent'))) { + throw 'Deployment template check failed: an absent install root has no access snapshot.' + } + } + finally { Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue } + Write-Host ' OK a failed identity change gives the previous identity its install root access back' -ForegroundColor DarkGray +} Assert-TextMatches -Name 'the rollback hands the secrets back to the previous identity too' ` -Text $rollbackBlock -Pattern '(?s)Set-NodePilotServiceOwnedFileAcl[\s\S]{0,200}\$previousAclIdentity' # Guarded, so a failure before the ACL was ever touched does not rewrite a directory this run had @@ -1203,17 +1251,105 @@ if ($purgeBlockStart -lt 0) { } $purgeBlock = $uninstallScript.Substring($purgeBlockStart) Assert-TextMatches -Name 'purging takes ownership before deleting owner-only files' ` - -Text $purgeBlock -Pattern 'takeown\.exe' -# "BUILTIN\Administrators" does not resolve on a non-English Windows. + -Text $purgeBlock -Pattern '(?s)Grant-NodePilotPurgeAccess -Path \$DataPath.*?Remove-Item -LiteralPath \$DataPath' +$purgeAccessStart = $uninstallScript.IndexOf('function Grant-NodePilotPurgeAccess') +if ($purgeAccessStart -lt 0) { + throw 'Deployment template check failed: could not locate the purge ownership helper in the uninstaller.' +} +$purgeAccess = $uninstallScript.Substring($purgeAccessStart, + $uninstallScript.IndexOf("`nWrite-Step", $purgeAccessStart) - $purgeAccessStart) +# "BUILTIN\Administrators" does not resolve on a non-English Windows, and takeown /D expects a +# localized answer (Y on English, J on German Windows). /A names the group without either. +Assert-TextMatches -Name 'ownership goes to Administrators without a localized name' ` + -Text $purgeAccess -Pattern 'takeown\.exe /F \$ItemPath /A' +Assert-TextDoesNotMatch -Name 'takeown is never given a localized confirmation answer' ` + -Text $uninstallScript -Pattern 'takeown[^\r\n]*/D\b' Assert-TextMatches -Name 'the ownership grant uses the well-known SID, not a localised group name' ` - -Text $purgeBlock -Pattern 'S-1-5-32-544' + -Text $purgeAccess -Pattern 'S-1-5-32-544' Assert-TextDoesNotMatch -Name 'the ownership grant must not use a localised group name' ` - -Text $purgeBlock -Pattern 'BUILTIN\\Administrators' + -Text $purgeAccess -Pattern 'BUILTIN\\Administrators' # (OI) and (CI) are CONTAINER inheritance flags. Applied to a leaf file, icacls drops them, reports # "Successfully processed 1 files" and adds no ACE whatsoever - so the grant looks like it worked # and the file stays undeletable. Measured on the lab host against jwt-secret.key. Assert-TextDoesNotMatch -Name 'the grant must not carry container inheritance flags' ` - -Text $purgeBlock -Pattern 'icacls[^\r\n]*\(OI\)' + -Text $purgeAccess -Pattern 'icacls[^\r\n]*\(OI\)' + +# A failed native step leaves the data in place and fails the uninstall instead of deleting part +# of a tree it could not open. +Assert-TextMatches -Name 'ownership failures stop the purge before anything is deleted' ` + -Text $purgeBlock -Pattern '(?s)if \(\$ownershipFailures\.Count -gt 0\) \{.*?\$dataPathRemaining = \$true.*?\}\s*else \{\s*try \{\s*Remove-Item -LiteralPath \$DataPath' + +# The helper against a real tree with a folder this account cannot list until its "ownership" is +# taken, which is what an owner-only directory looks like to an administrator. +& { + $uninstallErrors = $null + $uninstallAst = [System.Management.Automation.Language.Parser]::ParseFile( + $UninstallScriptPath, [ref]$null, [ref]$uninstallErrors) + if ($uninstallErrors.Count) { throw 'Uninstaller contains PowerShell syntax errors.' } + $helper = $uninstallAst.Find({ param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and + $node.Name -eq 'Grant-NodePilotPurgeAccess' + }, $false) + . ([scriptblock]::Create($helper.Extent.Text)) + $fixture = Join-Path ([IO.Path]::GetTempPath()) ('nodepilot-purge-' + [Guid]::NewGuid().ToString('N')) + $locked = Join-Path $fixture 'keys' + New-Item -ItemType Directory -Path (Join-Path $locked 'inner') -Force | Out-Null + foreach ($file in @('jwt-secret.key', 'keys\key-1.xml', 'keys\inner\key-2.xml')) { + [IO.File]::WriteAllText((Join-Path $fixture $file), 'fixture') + } + $me = [System.Security.Principal.WindowsIdentity]::GetCurrent().User + $deny = New-Object System.Security.AccessControl.FileSystemAccessRule($me, 'ListDirectory', 'Deny') + $lockedAcl = Get-Acl -LiteralPath $locked + $lockedAcl.AddAccessRule($deny) + Set-Acl -LiteralPath $locked -AclObject $lockedAcl + $calls = New-Object 'System.Collections.Generic.List[string]' + $unlock = { + param($ItemPath) + $calls.Add("own:$ItemPath") + if ($ItemPath -eq $locked) { + $acl = Get-Acl -LiteralPath $locked + [void]$acl.RemoveAccessRule($deny) + Set-Acl -LiteralPath $locked -AclObject $acl + } + 0 + } + try { + $failures = @(Grant-NodePilotPurgeAccess -Path $fixture -TakeOwnership $unlock ` + -GrantAdministrators { param($ItemPath) $calls.Add("grant:$ItemPath"); 0 }) + $expected = @($fixture, (Join-Path $fixture 'jwt-secret.key'), $locked, (Join-Path $locked 'inner'), + (Join-Path $locked 'key-1.xml'), (Join-Path $locked 'inner\key-2.xml')) + $missing = @($expected | Where-Object { + $calls.IndexOf("own:$_") -lt 0 -or $calls.IndexOf("grant:$_") -lt $calls.IndexOf("own:$_") }) + if ($failures.Count -ne 0 -or $missing.Count -ne 0) { + throw "Deployment template check failed: purge ownership must reach every item behind an unlistable folder (missing: $($missing -join ', '))." + } + + $calls.Clear() + $failures = @(Grant-NodePilotPurgeAccess -Path $fixture ` + -TakeOwnership { param($ItemPath) $calls.Add("own:$ItemPath"); if ($ItemPath -like '*jwt-secret.key') { 1 } else { 0 } } ` + -GrantAdministrators { param($ItemPath) $calls.Add("grant:$ItemPath"); 0 }) + if ($failures.Count -ne 1 -or $failures[0] -notmatch 'takeown exited with 1' -or + $calls.Contains("grant:$(Join-Path $fixture 'jwt-secret.key')")) { + throw 'Deployment template check failed: a failed takeown must be reported and must skip the grant.' + } + } + finally { + $acl = Get-Acl -LiteralPath $locked + [void]$acl.RemoveAccessRule($deny) + Set-Acl -LiteralPath $locked -AclObject $acl + Remove-Item -LiteralPath $fixture -Recurse -Force -ErrorAction SilentlyContinue + } + Write-Host ' OK purge ownership is taken item by item without a localized answer and reports native failures' -ForegroundColor DarkGray +} + +# A failed cleanup must stay repeatable: the marker and deploy\ are removed only after everything +# else is gone, and both setup uninstallers abort before Inno removes anything. +Assert-TextMatches -Name 'the uninstaller keeps the marker and its scripts while anything is left' ` + -Text $uninstallScript -Pattern '(?s)if \(\$installPathRemaining -or \$backupsRemaining -or \$dataPathRemaining\) \{.*?exit 1\s*\}.*?Remove-Item -LiteralPath \$recoveryHelperPath.*?Remove-Item -LiteralPath \$markerPath' +Assert-TextMatches -Name 'the uninstaller defers deploy\ while it empties the install directory' ` + -Text $uninstallScript -Pattern "\`$entry\.Name -eq \`$recoveryHelperName\) \{ continue \}" +Assert-TextMatches -Name 'a failed service deletion stops the uninstall before files are removed' ` + -Text $uninstallScript -Pattern '(?s)\$deleteExit -ne 0 -and \$deleteExit -ne 1072\) \{\s*throw' # --- server setup wizard contracts ------------------------------------------------------------- # The Pascal side has no unit-test story at all, so what CAN be pinned statically is pinned here @@ -1225,6 +1361,16 @@ Assert-TextDoesNotMatch -Name 'the server setup must not use a [Run] section' ` -Text $serverIss -Pattern '(?mi)^\s*\[Run\]' Assert-TextMatches -Name 'every Exec result is inspected' ` -Text $serverIss -Pattern '(?m)ResultCode\s*<>\s*0' +# Abort in usUninstall is the one place Inno stops an uninstall before it removes files and the +# Apps & Features entry, with a non-zero exit code for /SILENT callers. +$serverUninstallStep = [regex]::Match($serverIss, + '(?s)procedure CurUninstallStepChanged\(CurUninstallStep: TUninstallStep\);.*?\r?\nend;').Value +Assert-TextMatches -Name 'a failed server cleanup aborts the uninstall' ` + -Text $serverUninstallStep -Pattern '(?s)else if ResultCode <> 0 then\s*begin.*?Abort;\s*end;' +Assert-TextMatches -Name 'a server cleanup that cannot start aborts the uninstall' ` + -Text $serverUninstallStep -Pattern "(?s)if not Exec\('powershell\.exe', Arguments.*?then\s*begin.*?Abort;\s*end" +Assert-TextMatches -Name 'a missing server cleanup script aborts while the installation marker exists' ` + -Text $serverUninstallStep -Pattern "(?s)if not FileExists\(ScriptPath\) then\s*begin\s*//[^\r\n]*\r?\n\s*//[^\r\n]*\r?\n\s*if not RegKeyExists\(HKLM64, 'SOFTWARE\\NodePilot\\Server'\) then Exit;.*?Abort;" # Windows Server 2022 is build 20348. Copying the desktop installer's 22000 would make the SERVER # setup refuse to run on the only operating system it targets. diff --git a/deploy/Test-LocalServerHttp.ps1 b/deploy/Test-LocalServerHttp.ps1 new file mode 100644 index 00000000..22152845 --- /dev/null +++ b/deploy/Test-LocalServerHttp.ps1 @@ -0,0 +1,81 @@ +#requires -Version 5.1 +[CmdletBinding()] +param() + +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version 3.0 +. (Join-Path $PSScriptRoot 'LocalServerHttp.ps1') + +$script:passed = 0 +function Assert-LocalHttps { + param([bool]$Condition, [string]$Message) + if (-not $Condition) { throw $Message } + $script:passed++ +} + +function Import-SetupFunction { + param([string]$FileName, [string]$Name) + $tokens = $null; $errors = $null + $ast = [Management.Automation.Language.Parser]::ParseFile((Join-Path $PSScriptRoot $FileName), [ref]$tokens, [ref]$errors) + if ($errors.Count) { throw "Could not parse $FileName" } + $function = $ast.Find({ param($node) $node -is [Management.Automation.Language.FunctionDefinitionAst] -and $node.Name -eq $Name }, $true) + if (-not $function) { throw "Missing function $Name" } + return [scriptblock]::Create($function.Extent.Text) +} + +$rsa = [Security.Cryptography.RSA]::Create(2048) +$certificate = $null; $otherCertificate = $null +try { + $certificateRequest = [Security.Cryptography.X509Certificates.CertificateRequest]::new( + 'CN=setup.synthetic.invalid', $rsa, [Security.Cryptography.HashAlgorithmName]::SHA256, [Security.Cryptography.RSASignaturePadding]::Pkcs1) + $certificate = $certificateRequest.CreateSelfSigned([DateTimeOffset]::UtcNow.AddDays(-1), [DateTimeOffset]::UtcNow.AddDays(1)) + $otherCertificate = $certificateRequest.CreateSelfSigned([DateTimeOffset]::UtcNow.AddDays(-2), [DateTimeOffset]::UtcNow.AddDays(2)) + $policyProperty = [Net.ServicePointManager].GetProperty('CertificatePolicy') + $previousPolicy = if ($policyProperty) { $policyProperty.GetValue($null) } else { $null } + $previousCallback = [Net.ServicePointManager]::ServerCertificateValidationCallback + $request = [NodePilot.Deployment.LocalServerHttp]::CreateRequest([uri]'https://localhost:12345/healthz/ready', $certificate.Thumbprint, 1000) + $errors = [Net.Security.SslPolicyErrors]::RemoteCertificateNameMismatch -bor [Net.Security.SslPolicyErrors]::RemoteCertificateChainErrors + Assert-LocalHttps ($request.ServerCertificateValidationCallback.Invoke($request, $certificate, $null, $errors)) 'The selected self-signed public-host certificate must identify the loopback server.' + Assert-LocalHttps (-not $request.ServerCertificateValidationCallback.Invoke($request, $otherCertificate, $null, [Net.Security.SslPolicyErrors]::None)) 'A different certificate must fail even with a valid chain and hostname.' + Assert-LocalHttps (-not $request.ServerCertificateValidationCallback.Invoke($request, $null, $null, $errors)) 'A missing certificate must fail.' + Assert-LocalHttps (-not $request.AllowAutoRedirect) 'Bootstrap credentials must not follow redirects.' + Assert-LocalHttps ($null -eq $request.Proxy) 'Loopback setup must not use a configured proxy.' + if ($policyProperty) { + Assert-LocalHttps ([object]::ReferenceEquals($previousPolicy, $policyProperty.GetValue($null))) 'The global certificate policy changed.' + } + Assert-LocalHttps ([object]::ReferenceEquals($previousCallback, [Net.ServicePointManager]::ServerCertificateValidationCallback)) 'The global validation callback changed.' + foreach ($uri in @('http://localhost:12345/', 'https://server.synthetic.invalid/', 'https://user@localhost/')) { + $rejected = $false + try { [NodePilot.Deployment.LocalServerHttp]::CreateRequest([uri]$uri, $certificate.Thumbprint, 1000) | Out-Null } + catch { $rejected = $true } + Assert-LocalHttps $rejected "Unsafe setup URI accepted: $uri" + } + $rejected = $false + try { [NodePilot.Deployment.LocalServerHttp]::CreateRequest([uri]'https://localhost/', '', 1000) | Out-Null } + catch { $rejected = $true } + Assert-LocalHttps $rejected 'Missing certificate pin was accepted.' + + . (Import-SetupFunction -FileName 'Install-NodePilot.ps1' -Name 'Invoke-HealthProbe') + . (Import-SetupFunction -FileName 'Invoke-NodePilotSetup.ps1' -Name 'Invoke-NodePilotFirstLogin') + $script:requests = @() + $script:statusCode = 200 + function Invoke-NodePilotLocalHttps { + param($Uri, $CertificateThumbprint, $Method = 'GET', $Body, $Headers, $TimeoutSeconds) + $script:requests += [pscustomobject]@{Uri=$Uri; Pin=$CertificateThumbprint; Method=$Method; Body=$Body; Headers=$Headers} + return [pscustomobject]@{StatusCode=$script:statusCode} + } + Assert-LocalHttps (Invoke-HealthProbe -Url 'https://localhost:12345/healthz/ready' -CertificateThumbprint $certificate.Thumbprint) 'Pinned readiness response was rejected.' + $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' + Assert-LocalHttps ($result.Status -eq 'Created') 'Pinned bootstrap response was rejected.' + Assert-LocalHttps (@($script:requests | Where-Object Pin -ne $certificate.Thumbprint).Count -eq 0) 'A caller lost the configured certificate pin.' + Assert-LocalHttps ($script:requests[1].Headers['X-Setup-Token'] -eq 'synthetic-token') 'Bootstrap token was not sent through the pinned transport.' + $script:statusCode = 302 + $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' + Assert-LocalHttps ($result.Status -eq 'Failed') 'Redirect was treated as successful bootstrap.' +} +finally { + if ($certificate) { $certificate.Dispose() } + if ($otherCertificate) { $otherCertificate.Dispose() } + $rsa.Dispose() +} +Write-Host "Local server HTTPS checks passed: $script:passed" diff --git a/deploy/Test-SetupAdapter.ps1 b/deploy/Test-SetupAdapter.ps1 index d90f5386..44f18d66 100644 --- a/deploy/Test-SetupAdapter.ps1 +++ b/deploy/Test-SetupAdapter.ps1 @@ -1801,6 +1801,29 @@ try { -State ([pscustomobject]@{ Determined = $true; Allowed = $true; Detail = '' }) Assert-True -Name 'a host that may fetch the password passes' -Condition ($permitted.Status -eq 'Pass') + & { + # Execute the generated advice against in-memory AD boundaries only. + function Get-ADServiceAccount { + param($Identity, $Properties, $ErrorAction) + if ($Properties -ne 'PrincipalsAllowedToRetrieveManagedPassword') { throw 'Existing grants were not requested.' } + [pscustomobject]@{ PrincipalsAllowedToRetrieveManagedPassword = @( + [pscustomobject]@{ DistinguishedName = 'CN=ExistingGroup,DC=test' }, 'CN=ExistingHost,DC=test') } + } + function Get-ADComputer { + param($Identity, $ErrorAction) + [pscustomobject]@{ DistinguishedName = 'CN=NewHost,DC=test' } + } + function Set-ADServiceAccount { + param($Identity, $PrincipalsAllowedToRetrieveManagedPassword, $ErrorAction) + Assert-True -Name 'gMSA advice preserves existing grants while adding the host' -Condition ( + @($PrincipalsAllowedToRetrieveManagedPassword).Count -eq 3 -and + $PrincipalsAllowedToRetrieveManagedPassword -contains 'CN=ExistingGroup,DC=test' -and + $PrincipalsAllowedToRetrieveManagedPassword -contains 'CN=ExistingHost,DC=test' -and + $PrincipalsAllowedToRetrieveManagedPassword -contains 'CN=NewHost,DC=test') + } + & ([scriptblock]::Create($denied.Remediation)) + } + # Not knowing is not evidence: an unreachable directory must not refuse an install. $unknown = New-NodePilotGmsaRetrievalResult -Sam 'q-sdvorch2' -Title $gmsaTitle ` -State ([pscustomobject]@{ Determined = $false; Allowed = $false; Detail = 'no directory could be reached' }) diff --git a/deploy/Uninstall-NodePilot.ps1 b/deploy/Uninstall-NodePilot.ps1 index 87a7f4fe..76b6c0f4 100644 --- a/deploy/Uninstall-NodePilot.ps1 +++ b/deploy/Uninstall-NodePilot.ps1 @@ -147,6 +147,60 @@ function Remove-NodePilotBinaryBackups { return $failed } +function Grant-NodePilotPurgeAccess { + <# + Takes ownership of $Path and everything below it for Administrators (takeown /A), then grants + that group full control, item by item. takeown /R prompts with a localized answer on folders + it cannot list, so the recursion happens here: a folder unlocked in one pass exposes its + children to the next. A failed takeown skips the grant for that item. Returns the failures. + #> + param( + [Parameter(Mandatory)][string]$Path, + [scriptblock]$TakeOwnership = { + param($ItemPath) + & takeown.exe /F $ItemPath /A 2>&1 | Out-Null + $LASTEXITCODE + }, + # No (OI)(CI): those are container flags, and on a leaf file icacls drops them and adds no + # ACE at all while still reporting success. + [scriptblock]$GrantAdministrators = { + param($ItemPath) + & icacls.exe $ItemPath /grant '*S-1-5-32-544:(F)' /C /Q 2>&1 | Out-Null + $LASTEXITCODE + }, + [int]$MaxPasses = 16 + ) + # Native tools report on stderr; under 'Stop' Windows PowerShell turns that into an exception. + $ErrorActionPreference = 'Continue' + $done = New-Object 'System.Collections.Generic.HashSet[string]' ([StringComparer]::OrdinalIgnoreCase) + $failures = @{} + for ($pass = 1; $pass -le $MaxPasses; $pass++) { + $items = @(Get-Item -LiteralPath $Path -Force -ErrorAction SilentlyContinue) + + @(Get-ChildItem -LiteralPath $Path -Recurse -Force -ErrorAction SilentlyContinue) + $pending = @($items | Where-Object { -not $done.Contains($_.FullName) }) + if ($pending.Count -eq 0) { break } + $progress = $false + # Parents first, so their children become listable for the next pass. + foreach ($item in @($pending | Sort-Object { $_.FullName.Length })) { + $ownerExit = & $TakeOwnership $item.FullName + if ($ownerExit -ne 0) { + $failures[$item.FullName] = "takeown exited with $ownerExit for $($item.FullName)" + continue + } + $grantExit = & $GrantAdministrators $item.FullName + if ($grantExit -ne 0) { + $failures[$item.FullName] = "icacls exited with $grantExit for $($item.FullName)" + continue + } + $failures.Remove($item.FullName) + [void]$done.Add($item.FullName) + $progress = $true + } + if (-not $progress) { break } + } + return @($failures.Values | Sort-Object) +} + Write-Step "Stopping and removing service '$ServiceName'" $svc = Get-Service -Name $ServiceName -ErrorAction SilentlyContinue if ($svc) { @@ -188,7 +242,14 @@ if ($svc) { } & sc.exe delete $ServiceName | Out-Null - Write-Info " sc.exe delete returned exit $LASTEXITCODE" + $deleteExit = $LASTEXITCODE + Write-Info " sc.exe delete returned exit $deleteExit" + # 1072: already marked for deletion. Anything else leaves the service registered, so stop + # before removing the files it runs from. + if ($deleteExit -ne 0 -and $deleteExit -ne 1072) { + throw ("sc.exe delete '$ServiceName' failed with exit $deleteExit. Nothing else was removed; " + + 'fix the cause and run the uninstall again.') + } # sc.exe delete normally removes the whole service key, including the Environment MULTI_SZ # that holds ConnectionStrings__Postgres - the database password. While anything still holds @@ -204,24 +265,6 @@ if ($svc) { Write-Info " Service not present." } -Write-Step "Removing the installation marker" -# Without this, a fresh install after an uninstall is misdetected as an upgrade forever - the -# setup wizard reads this key to choose between the two. -$markerPath = 'HKLM:\SOFTWARE\NodePilot\Server' -if (Test-Path -LiteralPath $markerPath) { - Remove-Item -LiteralPath $markerPath -Recurse -Force -ErrorAction SilentlyContinue - Write-Info " Removed: $markerPath" -} else { - Write-Info " No installation marker present." -} -# Drop the parent only when this uninstall emptied it; something else may live under it. -$markerParent = 'HKLM:\SOFTWARE\NodePilot' -if ((Test-Path -LiteralPath $markerParent) -and - -not (Get-ChildItem -LiteralPath $markerParent -ErrorAction SilentlyContinue) -and - -not (Get-Item -LiteralPath $markerParent -ErrorAction SilentlyContinue).GetValueNames()) { - Remove-Item -LiteralPath $markerParent -Force -ErrorAction SilentlyContinue -} - Write-Step "Removing the CLI from the machine PATH" # Install-NodePilot.ps1 appends \tools\np so operators can just type `np`. Leaving it # behind would point PATH at a directory this uninstall is about to delete, and repeated @@ -260,10 +303,13 @@ if (Test-Path $InstallPath) { # arbitrary remainder behind. Skipping Inno's files lets it finish the job itself; run # standalone there are no such files and everything goes. $uninstallerPattern = 'unins*' + # deploy\ holds this script. It goes last, once everything else is gone, so a failed run can + # be repeated from the same place. + $recoveryHelperName = 'deploy' for ($attempt = 1; $attempt -le 3; $attempt++) { $failed = $false foreach ($entry in Get-ChildItem -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue) { - if ($entry.Name -like $uninstallerPattern) { continue } + if ($entry.Name -like $uninstallerPattern -or $entry.Name -eq $recoveryHelperName) { continue } try { Remove-Item -LiteralPath $entry.FullName -Recurse -Force -ErrorAction Stop } catch { $failed = $true @@ -277,23 +323,17 @@ if (Test-Path $InstallPath) { if ($attempt -lt 3) { Start-Sleep -Seconds 2 } } - # Only when nothing is left. Inno removes its own uninstaller and the empty directory after - # this script returns. - $remaining = @(Get-ChildItem -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue) - if ($remaining.Count -eq 0) { - Remove-Item -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue - } - elseif (@($remaining | Where-Object { $_.Name -notlike $uninstallerPattern }).Count -eq 0) { - Write-Info ' Leaving the setup uninstaller to remove itself and the directory.' - } - - # Anything still here that is not the setup's own uninstaller is a genuine leftover. + # Anything still here that is not the setup's own uninstaller or deploy\ is a genuine leftover. + $recoveryHelperPrefix = (Join-Path $InstallPath $recoveryHelperName).TrimEnd('\') + '\' $leftovers = @(Get-ChildItem -LiteralPath $InstallPath -Recurse -Force -File -ErrorAction SilentlyContinue | - Where-Object { $_.Name -notlike $uninstallerPattern }) + Where-Object { + $_.Name -notlike $uninstallerPattern -and + -not $_.FullName.StartsWith($recoveryHelperPrefix, [StringComparison]::OrdinalIgnoreCase) + }) if ($leftovers.Count -gt 0) { - # Do not abort here. The service and the marker are already gone; stopping now would skip - # the data notice and the residue report. Report the leftovers, carry on, and exit - # non-zero at the end. + # Do not abort here. The service is already gone; stopping now would skip the data notice + # and the residue report. Report the leftovers, carry on, and exit non-zero at the end, + # with the marker and deploy\ kept for the retry. $installPathRemaining = $true Write-Warn " $($leftovers.Count) file(s) could not be deleted under $InstallPath." $holders = @(Get-ProcessesUnderPath -Path $InstallPath) @@ -324,32 +364,30 @@ if ($PurgeData) { # account, so an administrator can neither read nor delete them and a plain Remove-Item # gets partway through the directory and then throws. # - # Ownership is taken first, using the well-known Administrators SID rather than the group - # name: "BUILTIN\Administrators" does not resolve on a non-English Windows, the same - # reason the ACL helpers in ArtifactSecurity.ps1 use SIDs. - # - # The grant carries no inheritance flags. (OI)(CI) are container flags; applied to a leaf - # file icacls drops them silently and adds no ACE at all. /T visits every item, so a flat - # grant is sufficient for a tree that is about to be deleted anyway. - $previousPreference = $ErrorActionPreference - $ErrorActionPreference = 'Continue' - try { - & takeown.exe /F $DataPath /R /D Y 2>&1 | Out-Null - & icacls.exe $DataPath /grant '*S-1-5-32-544:(F)' /T /C 2>&1 | Out-Null - } - finally { $ErrorActionPreference = $previousPreference } - - try { - Remove-Item -LiteralPath $DataPath -Recurse -Force -ErrorAction Stop - Write-Info " Deleted: $DataPath" - } - catch { - # Report and continue. Aborting here would skip the closing report, and the operator - # would be left with a half-purged directory and no idea which parts survived. + # Ownership is taken first, for the Administrators group via takeown /A and the + # well-known SID: neither the group name nor a takeown answer is the same on every + # Windows language. + $ownershipFailures = @(Grant-NodePilotPurgeAccess -Path $DataPath) + if ($ownershipFailures.Count -gt 0) { + # Nothing is deleted from a tree that is only partly accessible, so a retry starts from + # the same state. $dataPathRemaining = $true - Write-Warn " Could not fully delete $DataPath : $($_.Exception.Message)" - $survivors = @(Get-ChildItem -LiteralPath $DataPath -Recurse -Force -File -ErrorAction SilentlyContinue) - Write-Warn " $($survivors.Count) file(s) remain. Delete the folder by hand, or reboot and retry." + foreach ($failure in $ownershipFailures) { Write-Warn " $failure" } + Write-Warn " $DataPath was not deleted. Resolve the failures above and run the uninstall again." + } + else { + try { + Remove-Item -LiteralPath $DataPath -Recurse -Force -ErrorAction Stop + Write-Info " Deleted: $DataPath" + } + catch { + # Report and continue. Aborting here would skip the closing report, and the operator + # would be left with a half-purged directory and no idea which parts survived. + $dataPathRemaining = $true + Write-Warn " Could not fully delete $DataPath : $($_.Exception.Message)" + $survivors = @(Get-ChildItem -LiteralPath $DataPath -Recurse -Force -File -ErrorAction SilentlyContinue) + Write-Warn " $($survivors.Count) file(s) remain. Delete the folder by hand, or reboot and retry." + } } } else { Write-Info " Data path not present." @@ -401,7 +439,48 @@ if ($installedThumbprint -and $isManagedAccount) { Write-Host "" if ($installPathRemaining -or $backupsRemaining -or $dataPathRemaining) { - Write-Warn 'Uninstall finished with leftovers (see above).' + # The marker and deploy\ stay, so the setup uninstaller and this script can be run again. + Write-Warn 'Uninstall finished with leftovers (see above). The installation marker and the deploy scripts were kept; run the uninstall again once the cause is fixed.' exit 1 } + +Write-Step "Removing the deploy scripts and the installation marker" +$recoveryHelperPath = Join-Path $InstallPath 'deploy' +if (Test-Path -LiteralPath $recoveryHelperPath) { + try { Remove-Item -LiteralPath $recoveryHelperPath -Recurse -Force -ErrorAction Stop } + catch { + Write-Warn " Could not delete $recoveryHelperPath : $($_.Exception.Message)" + Write-Warn 'Uninstall finished with leftovers. The installation marker was kept; run the uninstall again.' + exit 1 + } + Write-Info " Deleted: $recoveryHelperPath" +} +# Only when nothing is left. Inno removes its own uninstaller and the empty directory after this +# script returns. +if (Test-Path -LiteralPath $InstallPath) { + $remaining = @(Get-ChildItem -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue) + if ($remaining.Count -eq 0) { + Remove-Item -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue + Write-Info " Deleted: $InstallPath" + } + else { + Write-Info ' Leaving the setup uninstaller to remove itself and the directory.' + } +} +# Without this, a fresh install after an uninstall is misdetected as an upgrade forever - the +# setup wizard reads this key to choose between the two. +$markerPath = 'HKLM:\SOFTWARE\NodePilot\Server' +if (Test-Path -LiteralPath $markerPath) { + Remove-Item -LiteralPath $markerPath -Recurse -Force -ErrorAction SilentlyContinue + Write-Info " Removed: $markerPath" +} else { + Write-Info " No installation marker present." +} +# Drop the parent only when this uninstall emptied it; something else may live under it. +$markerParent = 'HKLM:\SOFTWARE\NodePilot' +if ((Test-Path -LiteralPath $markerParent) -and + -not (Get-ChildItem -LiteralPath $markerParent -ErrorAction SilentlyContinue) -and + -not (Get-Item -LiteralPath $markerParent -ErrorAction SilentlyContinue).GetValueNames()) { + Remove-Item -LiteralPath $markerParent -Force -ErrorAction SilentlyContinue +} Write-Ok "Uninstall complete." diff --git a/deploy/server/Build-ServerInstaller.ps1 b/deploy/server/Build-ServerInstaller.ps1 index 4dfd3ac4..64cbae46 100644 --- a/deploy/server/Build-ServerInstaller.ps1 +++ b/deploy/server/Build-ServerInstaller.ps1 @@ -144,6 +144,7 @@ Write-Step 'Staging the deployment scripts' $deployScripts = @( 'ArtifactSecurity.ps1' 'DeploymentProgress.ps1' + 'LocalServerHttp.ps1' 'MachinePath.ps1' 'SwitcherConfig.ps1' 'Preflight.ps1' diff --git a/deploy/server/NodePilotServer.iss b/deploy/server/NodePilotServer.iss index 8d4845ec..ce01bd7a 100644 --- a/deploy/server/NodePilotServer.iss +++ b/deploy/server/NodePilotServer.iss @@ -2234,11 +2234,16 @@ begin ScriptPath := ExpandConstant('{app}\deploy\Uninstall-NodePilot.ps1'); if not FileExists(ScriptPath) then begin + // The script removes the marker only after a complete cleanup, so without the marker there + // is nothing left for it to do and Inno may finish. + if not RegKeyExists(HKLM64, 'SOFTWARE\NodePilot\Server') then Exit; SuppressibleMsgBox('The NodePilot uninstall script is missing:' + #13#10 + ScriptPath + #13#10#13#10 + - 'The Windows service and its firewall rules have NOT been removed. Run ' + - 'Uninstall-NodePilot.ps1 by hand, or reinstall and uninstall again.', + 'The Windows service and its firewall rules have NOT been removed, and this uninstall ' + + 'was stopped. Reinstall NodePilot Server, then uninstall again.', mbCriticalError, MB_OK, IDOK); - Exit; + // Abort in usUninstall ends the uninstaller with a non-zero exit code before Inno removes + // any file or registry entry, so the installation stays retryable. + Abort; end; // Built HERE, at uninstall time. This is the whole reason the [UninstallRun] section is gone. @@ -2273,15 +2278,24 @@ begin Arguments := Arguments + Switches; if not Exec('powershell.exe', Arguments, '', SW_HIDE, ewWaitUntilTerminated, ResultCode) then - SuppressibleMsgBox('Could not start PowerShell to remove the NodePilot service.', - mbCriticalError, MB_OK, IDOK) + begin + SuppressibleMsgBox('Could not start PowerShell to remove the NodePilot service. ' + + 'Nothing was removed; run the uninstall again.', + mbCriticalError, MB_OK, IDOK); + Abort; + end else if ResultCode <> 0 then - // Reported, not swallowed. The script exits non-zero when it had to leave files behind, and - // an operator who is told that can act on it. - SuppressibleMsgBox('The NodePilot uninstall reported problems (exit code ' + + begin + // The script exits non-zero while something is left. It keeps the marker and deploy\ in + // that case, and the abort keeps the uninstaller and its Apps & Features entry, so the + // same uninstall can be repeated once the cause is fixed. + SuppressibleMsgBox('The NodePilot uninstall could not finish (exit code ' + IntToStr(ResultCode) + ').' + #13#10#13#10 + - 'Check %TEMP% for details; some files or the Windows service may still be present.', + 'The uninstall was stopped and NodePilot Server stays registered. Fix the problem ' + + 'reported by Uninstall-NodePilot.ps1, then uninstall again.', mbError, MB_OK, IDOK); + Abort; + end; end; if CurUninstallStep = usPostUninstall then diff --git a/src/nodepilot-docs-ui/content/de/deployment/production.md b/src/nodepilot-docs-ui/content/de/deployment/production.md index 4fef5126..c85adddf 100644 --- a/src/nodepilot-docs-ui/content/de/deployment/production.md +++ b/src/nodepilot-docs-ui/content/de/deployment/production.md @@ -678,6 +678,10 @@ Nach einer Installation über das GUI-Setup geht dasselbe über „Apps & Featur & 'C:\Program Files\NodePilot\unins000.exe' /VERYSILENT /SUPPRESSMSGBOXES /PURGEDATA=1 ``` +Lässt sich etwas nicht entfernen, bricht die Deinstallation mit einem Exit-Code ungleich 0 ab und +behält den Installations-Marker, die Deploy-Skripte und den Eintrag unter „Apps & Features". Die +gemeldete Ursache beheben und dieselbe Deinstallation erneut ausführen. + **Die Datenbank wird nie entfernt, und es gibt dafür keine Option.** NodePilot legt sie nicht an — sie wurde in Kapitel 2 separat bereitgestellt, hat oft ein eigenes Backup- und Replikationsregime, und in einem Active/Passive-Cluster teilen sich beide Knoten dieselbe. Aus diff --git a/src/nodepilot-docs-ui/content/en/deployment/production.md b/src/nodepilot-docs-ui/content/en/deployment/production.md index 8f9d929c..c4e7ba45 100644 --- a/src/nodepilot-docs-ui/content/en/deployment/production.md +++ b/src/nodepilot-docs-ui/content/en/deployment/production.md @@ -676,6 +676,10 @@ After an installation through the GUI setup, the same is possible through "Apps & 'C:\Program Files\NodePilot\unins000.exe' /VERYSILENT /SUPPRESSMSGBOXES /PURGEDATA=1 ``` +If something cannot be removed, the uninstall stops with a non-zero exit code and keeps the +installation marker, the deploy scripts and the "Apps & features" entry. Fix the cause it reports +and run the same uninstall again. + **The database is never removed, and there is no option for it.** NodePilot does not create it — it was provisioned separately in chapter 2, often has its own backup and replication regime, and in an active/passive cluster both nodes share it. For the same reason, the gMSA's "Log on as a service" From 93c409397199e647ba0c8e2a6f1e58b2b68ef3f1 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:41:55 +0100 Subject: [PATCH 2/3] Keep TLS 1.2 and the server's login message in the pinned setup calls The old setup probe set TLS 1.2/1.3 for Windows PowerShell explicitly. Without the system-default switch, PowerShell 5.1 offers only TLS 1.0, Kestrel refuses it and the install rolls back after the 180 s probe. The helper now adds TLS 1.2 for the request and restores the previous process-wide setting afterwards. A rejected first login surfaces as a WebException wrapped in a MethodInvocationException, so the setup reported only "(400) Bad Request". It now unwraps it and reports the server's own message again. Co-Authored-By: Claude Opus 5.5 --- deploy/Invoke-NodePilotSetup.ps1 | 8 +++--- deploy/LocalServerHttp.ps1 | 34 +++++++++++++++++------- deploy/Test-LocalServerHttp.ps1 | 45 ++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/deploy/Invoke-NodePilotSetup.ps1 b/deploy/Invoke-NodePilotSetup.ps1 index e302135d..5cb51c13 100644 --- a/deploy/Invoke-NodePilotSetup.ps1 +++ b/deploy/Invoke-NodePilotSetup.ps1 @@ -156,10 +156,12 @@ function Invoke-NodePilotFirstLogin { } catch { # Report the server's own message: a password-policy rejection or a username mismatch is - # actionable, a generic failure is not. - $detail = $_.Exception.Message + # actionable, a generic failure is not. The .NET call wraps the WebException that carries it. + $webError = $_.Exception + while ($webError -and -not ($webError -is [Net.WebException])) { $webError = $webError.InnerException } + $detail = if ($webError) { $webError.Message } else { $_.Exception.Message } try { - $errorResponse = $_.Exception.Response + $errorResponse = if ($webError) { $webError.Response } else { $null } if ($errorResponse) { $reader = New-Object IO.StreamReader($errorResponse.GetResponseStream()) $payload = $reader.ReadToEnd() diff --git a/deploy/LocalServerHttp.ps1 b/deploy/LocalServerHttp.ps1 index 5633e88d..84d65283 100644 --- a/deploy/LocalServerHttp.ps1 +++ b/deploy/LocalServerHttp.ps1 @@ -42,6 +42,14 @@ namespace NodePilot.Deployment '@ } +function Get-NodePilotLocalHttpsProtocol { + # Windows PowerShell without the system-default switch offers only SSL 3 and TLS 1.0, which + # Kestrel refuses. 0 is SystemDefault: the OS already negotiates TLS 1.2 or later. + param([Parameter(Mandatory)][Net.SecurityProtocolType]$Current) + if ([int]$Current -eq 0) { return $Current } + return $Current -bor [Net.SecurityProtocolType]::Tls12 +} + function Invoke-NodePilotLocalHttps { [CmdletBinding()] param( @@ -56,18 +64,24 @@ function Invoke-NodePilotLocalHttps { $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() } + # The protocol setting is process-wide, so it only applies for this request. + $previousProtocol = [Net.ServicePointManager]::SecurityProtocol + [Net.ServicePointManager]::SecurityProtocol = Get-NodePilotLocalHttpsProtocol -Current $previousProtocol + try { + 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) } } - finally { [Array]::Clear($bytes, 0, $bytes.Length) } + $response = $request.GetResponse() } - $response = $request.GetResponse() + finally { [Net.ServicePointManager]::SecurityProtocol = $previousProtocol } try { return [pscustomobject]@{ StatusCode = [int]$response.StatusCode } } finally { $response.Dispose() } } diff --git a/deploy/Test-LocalServerHttp.ps1 b/deploy/Test-LocalServerHttp.ps1 index 22152845..5a2adc15 100644 --- a/deploy/Test-LocalServerHttp.ps1 +++ b/deploy/Test-LocalServerHttp.ps1 @@ -55,13 +55,35 @@ try { catch { $rejected = $true } Assert-LocalHttps $rejected 'Missing certificate pin was accepted.' + # Windows PowerShell without the system-default switch would offer only TLS 1.0 to Kestrel. + $legacy = [Net.SecurityProtocolType]::Tls + $offered = Get-NodePilotLocalHttpsProtocol -Current $legacy + Assert-LocalHttps (($offered -band [Net.SecurityProtocolType]::Tls12) -and ($offered -band $legacy)) 'TLS 1.2 must be added to a legacy protocol setting.' + Assert-LocalHttps ([int](Get-NodePilotLocalHttpsProtocol -Current ([Net.SecurityProtocolType]0)) -eq 0) 'The system default must be left to the OS.' + $previousProtocol = [Net.ServicePointManager]::SecurityProtocol + $closedProbe = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback, 0) + $closedProbe.Start() + $closedPort = ([System.Net.IPEndPoint]$closedProbe.LocalEndpoint).Port + $closedProbe.Stop() + try { + [Net.ServicePointManager]::SecurityProtocol = $legacy + $failed = $false + try { Invoke-NodePilotLocalHttps -Uri "https://localhost:$closedPort/healthz/ready" -CertificateThumbprint $certificate.Thumbprint -TimeoutSeconds 2 | Out-Null } + catch { $failed = $true } + Assert-LocalHttps $failed 'A closed port must fail the request.' + Assert-LocalHttps ([Net.ServicePointManager]::SecurityProtocol -eq $legacy) 'The process-wide protocol setting must be restored after the request.' + } + finally { [Net.ServicePointManager]::SecurityProtocol = $previousProtocol } + . (Import-SetupFunction -FileName 'Install-NodePilot.ps1' -Name 'Invoke-HealthProbe') . (Import-SetupFunction -FileName 'Invoke-NodePilotSetup.ps1' -Name 'Invoke-NodePilotFirstLogin') $script:requests = @() $script:statusCode = 200 + $script:failWith = $null function Invoke-NodePilotLocalHttps { param($Uri, $CertificateThumbprint, $Method = 'GET', $Body, $Headers, $TimeoutSeconds) $script:requests += [pscustomobject]@{Uri=$Uri; Pin=$CertificateThumbprint; Method=$Method; Body=$Body; Headers=$Headers} + if ($script:failWith) { throw $script:failWith } return [pscustomobject]@{StatusCode=$script:statusCode} } Assert-LocalHttps (Invoke-HealthProbe -Url 'https://localhost:12345/healthz/ready' -CertificateThumbprint $certificate.Thumbprint) 'Pinned readiness response was rejected.' @@ -72,6 +94,29 @@ try { $script:statusCode = 302 $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' Assert-LocalHttps ($result.Status -eq 'Failed') 'Redirect was treated as successful bootstrap.' + + # A rejected login reaches the caller as a WebException wrapped by the .NET method call; the + # server's message must still be reported. + if (-not ('NodePilot.Deployment.Tests.SyntheticErrorResponse' -as [type])) { + Add-Type -IgnoreWarnings -TypeDefinition @' +namespace NodePilot.Deployment.Tests +{ + public sealed class SyntheticErrorResponse : System.Net.WebResponse + { + private readonly byte[] _body; + public SyntheticErrorResponse(string body) { _body = System.Text.Encoding.UTF8.GetBytes(body); } + public override System.IO.Stream GetResponseStream() { return new System.IO.MemoryStream(_body); } + } +} +'@ + } + $serverMessage = '{"error":"synthetic password policy rejection"}' + $webError = New-Object Net.WebException('The remote server returned an error: (400) Bad Request.', $null, + [Net.WebExceptionStatus]::ProtocolError, (New-Object NodePilot.Deployment.Tests.SyntheticErrorResponse($serverMessage))) + $script:failWith = New-Object Management.Automation.MethodInvocationException('Exception calling "GetResponse" with "0" argument(s)', $webError) + $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' + Assert-LocalHttps ($result.Status -eq 'Failed' -and $result.Detail -eq $serverMessage) 'A rejected bootstrap login must report the server message.' + $script:failWith = $null } finally { if ($certificate) { $certificate.Dispose() } From 2772d7ba9375653954bf81ae1aad3630a31911ff Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:14:07 +0100 Subject: [PATCH 3/3] Make a stopped server uninstall diagnosable and keep setup TLS as before - Uninstall-NodePilot.ps1 writes a transcript to %TEMP%\nodepilot-server-uninstall.log and deletes it after a clean run. The setup uninstaller runs the script hidden and now stops on any failure, so its message names that file, as the desktop uninstaller already does. - The pinned setup calls offer TLS 1.3 next to 1.2 on Windows PowerShell, as the old probe did, so hosts with TLS 1.2 disabled still pass the health probe and the first login. - The wizard loads LocalServerHttp.ps1 only in install mode. Loading it compiles C#, and the other wizard modes never use it. Refs #714 Co-Authored-By: Claude Opus 5.5 --- deploy/Invoke-NodePilotSetup.ps1 | 4 +++- deploy/LocalServerHttp.ps1 | 5 +++-- deploy/README.md | 3 ++- deploy/Test-DeploymentTemplates.ps1 | 5 +++++ deploy/Test-LocalServerHttp.ps1 | 3 ++- deploy/Uninstall-NodePilot.ps1 | 6 ++++++ deploy/server/NodePilotServer.iss | 3 ++- src/nodepilot-docs-ui/content/de/deployment/production.md | 3 ++- src/nodepilot-docs-ui/content/en/deployment/production.md | 4 ++-- 9 files changed, 27 insertions(+), 9 deletions(-) diff --git a/deploy/Invoke-NodePilotSetup.ps1 b/deploy/Invoke-NodePilotSetup.ps1 index 5cb51c13..5adbe5ab 100644 --- a/deploy/Invoke-NodePilotSetup.ps1 +++ b/deploy/Invoke-NodePilotSetup.ps1 @@ -88,7 +88,6 @@ 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 @@ -675,6 +674,9 @@ function Invoke-SetupInstall { $answers = Read-NodePilotAnswerFile -Path $AnswerFile Set-NodePilotResult -Buffer $result -Section 'result' -Name 'mode' -Value 'install' . (Join-Path $scriptDirectory 'ArtifactSecurity.ps1') + # Only the install mode talks to the local server; loading it compiles C#, so the wizard's + # other modes skip it. + . (Join-Path $scriptDirectory 'LocalServerHttp.ps1') $splat = ConvertTo-NodePilotInstallParameters -Answers $answers diff --git a/deploy/LocalServerHttp.ps1 b/deploy/LocalServerHttp.ps1 index 84d65283..e07be4b4 100644 --- a/deploy/LocalServerHttp.ps1 +++ b/deploy/LocalServerHttp.ps1 @@ -44,10 +44,11 @@ namespace NodePilot.Deployment function Get-NodePilotLocalHttpsProtocol { # Windows PowerShell without the system-default switch offers only SSL 3 and TLS 1.0, which - # Kestrel refuses. 0 is SystemDefault: the OS already negotiates TLS 1.2 or later. + # Kestrel refuses; TLS 1.3 covers hosts with TLS 1.2 disabled. 0 is SystemDefault: the OS + # already negotiates TLS 1.2 or later. param([Parameter(Mandatory)][Net.SecurityProtocolType]$Current) if ([int]$Current -eq 0) { return $Current } - return $Current -bor [Net.SecurityProtocolType]::Tls12 + return $Current -bor [Net.SecurityProtocolType]::Tls12 -bor [Net.SecurityProtocolType]::Tls13 } function Invoke-NodePilotLocalHttps { diff --git a/deploy/README.md b/deploy/README.md index a632a0af..b461cc64 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -564,7 +564,8 @@ the leftovers together with the holding process, and exits with code 1. **A failed uninstall can be repeated.** The installation marker and `\deploy\` (which holds this script) are removed only after everything else is gone. Any non-zero exit makes the GUI and `/SILENT` uninstaller abort before Inno removes a file or the "Apps & features" entry, and the -uninstaller itself exits non-zero; fix the reported cause and uninstall again. +uninstaller itself exits non-zero; fix the cause named in `%TEMP%\nodepilot-server-uninstall.log` +(the script's transcript, deleted after a clean run) and uninstall again. Two things are **deliberately** left in place, because both can be shared with something else on the host and blindly revoking them would break exactly that: the gMSA's "Log on as a service" right and the diff --git a/deploy/Test-DeploymentTemplates.ps1 b/deploy/Test-DeploymentTemplates.ps1 index 644e3bbb..1ee0a1d1 100644 --- a/deploy/Test-DeploymentTemplates.ps1 +++ b/deploy/Test-DeploymentTemplates.ps1 @@ -1350,6 +1350,9 @@ Assert-TextMatches -Name 'the uninstaller defers deploy\ while it empties the in -Text $uninstallScript -Pattern "\`$entry\.Name -eq \`$recoveryHelperName\) \{ continue \}" Assert-TextMatches -Name 'a failed service deletion stops the uninstall before files are removed' ` -Text $uninstallScript -Pattern '(?s)\$deleteExit -ne 0 -and \$deleteExit -ne 1072\) \{\s*throw' +# The setup runs the script hidden, so a stopped uninstall is diagnosable only from its transcript. +Assert-TextMatches -Name 'the uninstaller keeps a transcript until a clean run deletes it' ` + -Text $uninstallScript -Pattern '(?s)\$TranscriptPath = Join-Path \$env:TEMP ''nodepilot-server-uninstall\.log''\s*\r?\ntry \{ Start-Transcript -Path \$TranscriptPath.*?Write-Ok "Uninstall complete\."\s*\r?\ntry \{ Stop-Transcript \| Out-Null \} catch \{ \}\s*\r?\nRemove-Item -LiteralPath \$TranscriptPath' # --- server setup wizard contracts ------------------------------------------------------------- # The Pascal side has no unit-test story at all, so what CAN be pinned statically is pinned here @@ -1367,6 +1370,8 @@ $serverUninstallStep = [regex]::Match($serverIss, '(?s)procedure CurUninstallStepChanged\(CurUninstallStep: TUninstallStep\);.*?\r?\nend;').Value Assert-TextMatches -Name 'a failed server cleanup aborts the uninstall' ` -Text $serverUninstallStep -Pattern '(?s)else if ResultCode <> 0 then\s*begin.*?Abort;\s*end;' +Assert-TextMatches -Name 'a failed server cleanup names the uninstall transcript' ` + -Text $serverUninstallStep -Pattern "(?s)else if ResultCode <> 0 then\s*begin.*?GetEnv\('TEMP'\) \+ '\\nodepilot-server-uninstall\.log'.*?Abort;" Assert-TextMatches -Name 'a server cleanup that cannot start aborts the uninstall' ` -Text $serverUninstallStep -Pattern "(?s)if not Exec\('powershell\.exe', Arguments.*?then\s*begin.*?Abort;\s*end" Assert-TextMatches -Name 'a missing server cleanup script aborts while the installation marker exists' ` diff --git a/deploy/Test-LocalServerHttp.ps1 b/deploy/Test-LocalServerHttp.ps1 index 5a2adc15..a74b5fb5 100644 --- a/deploy/Test-LocalServerHttp.ps1 +++ b/deploy/Test-LocalServerHttp.ps1 @@ -58,7 +58,8 @@ try { # Windows PowerShell without the system-default switch would offer only TLS 1.0 to Kestrel. $legacy = [Net.SecurityProtocolType]::Tls $offered = Get-NodePilotLocalHttpsProtocol -Current $legacy - Assert-LocalHttps (($offered -band [Net.SecurityProtocolType]::Tls12) -and ($offered -band $legacy)) 'TLS 1.2 must be added to a legacy protocol setting.' + Assert-LocalHttps (($offered -band [Net.SecurityProtocolType]::Tls12) -and ($offered -band [Net.SecurityProtocolType]::Tls13) -and + ($offered -band $legacy)) 'TLS 1.2 and 1.3 must be added to a legacy protocol setting.' Assert-LocalHttps ([int](Get-NodePilotLocalHttpsProtocol -Current ([Net.SecurityProtocolType]0)) -eq 0) 'The system default must be left to the OS.' $previousProtocol = [Net.ServicePointManager]::SecurityProtocol $closedProbe = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback, 0) diff --git a/deploy/Uninstall-NodePilot.ps1 b/deploy/Uninstall-NodePilot.ps1 index 76b6c0f4..14f2a1eb 100644 --- a/deploy/Uninstall-NodePilot.ps1 +++ b/deploy/Uninstall-NodePilot.ps1 @@ -44,6 +44,10 @@ param( $ErrorActionPreference = 'Stop' Set-StrictMode -Version 3.0 +# The setup uninstaller runs this hidden and names this file when it stops; a clean run deletes it. +$TranscriptPath = Join-Path $env:TEMP 'nodepilot-server-uninstall.log' +try { Start-Transcript -Path $TranscriptPath -Force | Out-Null } catch { } + function Write-Step { param([string]$Text) Write-Host "[uninstall] $Text" -ForegroundColor Cyan } function Write-Info { param([string]$Text) Write-Host "[uninstall] $Text" -ForegroundColor Gray } function Write-Ok { param([string]$Text) Write-Host "[uninstall] $Text" -ForegroundColor Green } @@ -484,3 +488,5 @@ if ((Test-Path -LiteralPath $markerParent) -and Remove-Item -LiteralPath $markerParent -Force -ErrorAction SilentlyContinue } Write-Ok "Uninstall complete." +try { Stop-Transcript | Out-Null } catch { } +Remove-Item -LiteralPath $TranscriptPath -Force -ErrorAction SilentlyContinue diff --git a/deploy/server/NodePilotServer.iss b/deploy/server/NodePilotServer.iss index ce01bd7a..abd0aadb 100644 --- a/deploy/server/NodePilotServer.iss +++ b/deploy/server/NodePilotServer.iss @@ -2292,7 +2292,8 @@ begin SuppressibleMsgBox('The NodePilot uninstall could not finish (exit code ' + IntToStr(ResultCode) + ').' + #13#10#13#10 + 'The uninstall was stopped and NodePilot Server stays registered. Fix the problem ' + - 'reported by Uninstall-NodePilot.ps1, then uninstall again.', + 'named in the log, then uninstall again. The log is at:' + #13#10 + + GetEnv('TEMP') + '\nodepilot-server-uninstall.log', mbError, MB_OK, IDOK); Abort; end; diff --git a/src/nodepilot-docs-ui/content/de/deployment/production.md b/src/nodepilot-docs-ui/content/de/deployment/production.md index c85adddf..dcc48c57 100644 --- a/src/nodepilot-docs-ui/content/de/deployment/production.md +++ b/src/nodepilot-docs-ui/content/de/deployment/production.md @@ -680,7 +680,8 @@ Nach einer Installation über das GUI-Setup geht dasselbe über „Apps & Featur Lässt sich etwas nicht entfernen, bricht die Deinstallation mit einem Exit-Code ungleich 0 ab und behält den Installations-Marker, die Deploy-Skripte und den Eintrag unter „Apps & Features". Die -gemeldete Ursache beheben und dieselbe Deinstallation erneut ausführen. +Ursache steht in `%TEMP%\nodepilot-server-uninstall.log`; sie beheben und dieselbe Deinstallation +erneut ausführen. **Die Datenbank wird nie entfernt, und es gibt dafür keine Option.** NodePilot legt sie nicht an — sie wurde in Kapitel 2 separat bereitgestellt, hat oft ein eigenes Backup- und diff --git a/src/nodepilot-docs-ui/content/en/deployment/production.md b/src/nodepilot-docs-ui/content/en/deployment/production.md index c4e7ba45..f76edd2c 100644 --- a/src/nodepilot-docs-ui/content/en/deployment/production.md +++ b/src/nodepilot-docs-ui/content/en/deployment/production.md @@ -677,8 +677,8 @@ After an installation through the GUI setup, the same is possible through "Apps ``` If something cannot be removed, the uninstall stops with a non-zero exit code and keeps the -installation marker, the deploy scripts and the "Apps & features" entry. Fix the cause it reports -and run the same uninstall again. +installation marker, the deploy scripts and the "Apps & features" entry. The cause is in +`%TEMP%\nodepilot-server-uninstall.log`; fix it and run the same uninstall again. **The database is never removed, and there is no option for it.** NodePilot does not create it — it was provisioned separately in chapter 2, often has its own backup and replication regime, and in an