From fe6948418b44e6534fe62f842074f82aadaf471e Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Sun, 16 Aug 2026 00:47:37 +0200 Subject: [PATCH 1/2] Point Add/Remove Programs at the installation, and clear the entry a ZIP install invalidates Two consequences of {app} not following the answer file's installPath. Both are verified on the lab server, not only in contract tests. Apps & Features showed InstallLocation = {app}, which is where the uninstaller ended up rather than where NodePilot was installed: /ANSWERFILE skips the directory page, so {app} keeps DefaultDirName while the adapter installs to installPath. On a lab install to C:\Program Files\NodePilotC the entry pointed at C:\Program Files\NodePilot, a directory holding the uninstaller and nothing else. ssPostInstall now corrects it from the installation marker, the same source the uninstaller reads. Best-effort by construction: an exception in ssPostInstall does not change the exit code, so a failed write simply leaves Inno's own value. The GUI setup keeps its uninstaller inside the install directory, so running the ZIP installer over a setup installation - both are documented ways to install the same product - deleted it and left an Add/Remove Programs entry Windows can neither run nor clear. That is how the lab machine came to carry two dead entries. Sparing the file is not available: Assert-NodePilotExtractedFiles requires the install directory to hold exactly the signed artifact and nothing else, which is what stops a binary being swapped before the service executes it - the first attempt at this fix spared unins*/deploy and broke that check immediately. The now-dead entry is removed instead, and only when its uninstaller lived in the directory just emptied and is really gone. Known limitation, unchanged: the installation marker is a single key, so a host running more than one instance keeps only the most recently installed one, and uninstalling any instance clears it for all. The uninstaller falls back to the old defaults when it is absent. Tests: contracts for the corrected ARP write and its marker source, and for the stale-entry removal including both narrowing conditions - without them the check would pass on a script that deletes every uninstall entry on the machine. --- deploy/Install-NodePilot.ps1 | 29 +++++++++++++++++++++++++++++ deploy/Test-DeploymentTemplates.ps1 | 24 ++++++++++++++++++++++++ deploy/server/NodePilotServer.iss | 24 ++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/deploy/Install-NodePilot.ps1 b/deploy/Install-NodePilot.ps1 index f7be3f4..062881f 100644 --- a/deploy/Install-NodePilot.ps1 +++ b/deploy/Install-NodePilot.ps1 @@ -1202,6 +1202,35 @@ Assert-SafeInstallRoot -Path $InstallPath if (Test-Path $InstallPath) { # Empty install path but do NOT touch DataPath. Get-ChildItem $InstallPath -Force -ErrorAction SilentlyContinue | Remove-Item -Recurse -Force + + # The GUI setup keeps its uninstaller in this directory, so emptying it invalidates whatever + # Add/Remove Programs entry pointed at that uninstaller. Both the setup and this script are + # documented ways to install the same product and an operator may well mix them - and an entry + # whose uninstaller no longer exists is one Windows can neither run nor clear, so it sits there + # until somebody edits the registry. + # + # Sparing the uninstaller instead is not an option: Assert-NodePilotExtractedFiles a few steps + # below requires this directory to hold exactly the signed artifact and nothing else, which is + # what stops a binary being swapped before the service executes it. + # + # Deliberately narrow - an entry is removed only when its uninstaller lived in the directory + # just emptied AND is really gone. One that still works belongs to somebody else. + $deadEntryPrefix = $InstallPath.TrimEnd('\') + '\' + foreach ($uninstallRoot in @( + 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall', + 'HKLM:\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall')) { + foreach ($entry in @(Get-ChildItem -LiteralPath $uninstallRoot -ErrorAction SilentlyContinue)) { + $uninstallString = [string]$entry.GetValue('UninstallString') + if ([string]::IsNullOrWhiteSpace($uninstallString)) { continue } + # Strip the quoting an uninstaller path normally carries, plus any trailing switches. + $uninstallTarget = ($uninstallString -replace '^\s*"([^"]+)".*$', '$1').Trim() + if (-not $uninstallTarget.StartsWith($deadEntryPrefix, [StringComparison]::OrdinalIgnoreCase)) { continue } + if (Test-Path -LiteralPath $uninstallTarget) { continue } + $deadEntryName = [string]$entry.GetValue('DisplayName') + Remove-Item -LiteralPath $entry.PSPath -Recurse -Force -ErrorAction SilentlyContinue + Write-Info " Removed the stale uninstall entry '$deadEntryName' - its uninstaller was in $InstallPath." + } + } } else { New-Item -ItemType Directory -Path $InstallPath -Force | Out-Null } diff --git a/deploy/Test-DeploymentTemplates.ps1 b/deploy/Test-DeploymentTemplates.ps1 index bb12097..a0a8696 100644 --- a/deploy/Test-DeploymentTemplates.ps1 +++ b/deploy/Test-DeploymentTemplates.ps1 @@ -307,6 +307,20 @@ Assert-TextMatches -Name 'installer renders the trusted-proxy placeholder' ` -Text $installer -Pattern "Replace\('\{\{KNOWN_PROXIES_JSON\}\}'" Assert-TextMatches -Name 'installer validates trusted proxy addresses' ` -Text $installer -Pattern 'IPAddress\]::TryParse\(\$proxyIp' +# Emptying the install directory kills the GUI setup's uninstaller, which lives in it. Sparing that +# file is not an option - Assert-NodePilotExtractedFiles requires the directory to hold exactly the +# signed artifact - so the Add/Remove Programs entry it leaves behind is removed instead, or Windows +# keeps an entry it can neither run nor clear. +Assert-TextMatches -Name 'a ZIP install clears the uninstall entry it just invalidated' ` + -Text $installer ` + -Pattern '(?s)CurrentVersion\\Uninstall[\s\S]{0,1400}Remove-Item -LiteralPath \$entry\.PSPath' +# Both halves matter: an entry pointing somewhere else, or one whose uninstaller still exists, is +# somebody else's. Without either test this silently becomes "delete every uninstall entry". +Assert-TextMatches -Name 'and only one whose uninstaller lived in the emptied directory' ` + -Text $installer -Pattern '\$uninstallTarget\.StartsWith\(\$deadEntryPrefix' +Assert-TextMatches -Name 'and only one whose uninstaller is really gone' ` + -Text $installer -Pattern '(?s)if \(Test-Path -LiteralPath \$uninstallTarget\) \{ continue \}' + Assert-TextMatches -Name 'installer keeps the Postgres secret in the service-scoped environment' ` -Text $installer -Pattern 'ConnectionStrings__Postgres=\$postgresServiceConnStr' Assert-TextMatches -Name 'installer protects the service registry key before writing the Postgres secret' ` @@ -1072,6 +1086,16 @@ Assert-TextDoesNotMatch -Name 'the uninstall must not pass the wizard default as Assert-TextDoesNotMatch -Name 'the uninstall must not pass {app} as the install path' ` -Text $serverIssCode -Pattern '-InstallPath[\s\S]{0,24}ExpandConstant' +# Apps & Features must name the directory NodePilot is in, not the one the uninstaller landed in. +# With /ANSWERFILE the directory page never runs, so {app} keeps DefaultDirName while the adapter +# installs to the answer file's installPath - and Inno fills InstallLocation with {app}. +Assert-TextMatches -Name 'the ARP entry is corrected to the real install path' ` + -Text $serverIssCode ` + -Pattern '(?s)ssPostInstall[\s\S]{0,900}RegWriteStringValue\(HKLM64,[\s\S]{0,200}InstallLocation' +Assert-TextMatches -Name 'and it takes that path from the installer marker' ` + -Text $serverIssCode ` + -Pattern '(?s)ssPostInstall[\s\S]{0,600}RegQueryStringValue\(HKLM64,[^)]*InstallPath' + # /ANSWERFILE skips the mode page, so IsUpdateSelected() reads ModePage's hard default of 0 # ('update') and AnswerMode contradicts a file that says "mode": "install". Gating the silent # provisioning run on AnswerMode alone therefore dropped every provisioning key - database, login, diff --git a/deploy/server/NodePilotServer.iss b/deploy/server/NodePilotServer.iss index 222f895..cb8be1e 100644 --- a/deploy/server/NodePilotServer.iss +++ b/deploy/server/NodePilotServer.iss @@ -1833,6 +1833,30 @@ begin UninstallPurgeData := Response = IDNO; end; +procedure CurStepChanged(CurStep: TSetupStep); +var + InstalledInstallPath: String; +begin + if CurStep = ssPostInstall then + begin + // Apps & Features shows InstallLocation, and Inno fills it with {app} - which is where the + // uninstaller ended up, not where NodePilot was installed. /ANSWERFILE skips the directory + // page, so {app} keeps DefaultDirName while the adapter installs to the answer file's + // installPath; the entry then points an operator at a directory holding the uninstaller and + // nothing else. Corrected from the marker the installer has just written, which is the same + // source the uninstaller reads. + // + // Best-effort on purpose. An exception in ssPostInstall does NOT change the exit code (see the + // note in [Files]), so nothing load-bearing may live here - a failed write leaves Inno's own + // value in place, which is exactly today's behaviour. + if RegQueryStringValue(HKLM64, 'SOFTWARE\NodePilot\Server', 'InstallPath', InstalledInstallPath) and + (InstalledInstallPath <> '') then + RegWriteStringValue(HKLM64, + 'SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\{03EAD540-1472-4A1B-9F06-9CB3D358E202}_is1', + 'InstallLocation', AddBackslash(InstalledInstallPath)); + end; +end; + procedure CurUninstallStepChanged(CurUninstallStep: TUninstallStep); var ResultCode: Integer; From db5135f422e46c2017b26922b5ad5e54d84ade03 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Sun, 16 Aug 2026 08:46:33 +0200 Subject: [PATCH 2/2] Refresh the installation marker on update Only Install-NodePilot.ps1 wrote HKLM\SOFTWARE\NodePilot\Server, so its Version kept naming the last INSTALL and every script-driven update was invisible in it. That value is what the setup wizard puts on its mode page ("NodePilot is already installed in ") and the obvious thing for an inventory to read. Measured in the lab before the fix: binaries updated from 1.2.6-rc1 to the 1.2.5 artifact - ProductVersion really changed - while the marker still claimed 1.2.6-rc1. After: marker and binary move together. Version only. Path, service name, provider and port are not changed by an update and are already correct. Guarded on InstallPath because the marker is a single machine-wide key: on a host running more than one instance it describes whichever was installed last, and stamping this update's version onto another instance's marker is worse than leaving it stale. A write failure stays a warning - it costs discoverability, not a working installation. --- deploy/Test-DeploymentTemplates.ps1 | 11 +++++++++++ deploy/Update-NodePilot.ps1 | 29 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/deploy/Test-DeploymentTemplates.ps1 b/deploy/Test-DeploymentTemplates.ps1 index a0a8696..55fdc9c 100644 --- a/deploy/Test-DeploymentTemplates.ps1 +++ b/deploy/Test-DeploymentTemplates.ps1 @@ -520,6 +520,17 @@ $successCode = ($successPath -split "`n" | Where-Object { $_.TrimStart() -notmat Assert-TextDoesNotMatch -Name 'a successful update must not stop the service again' ` -Text $successCode -Pattern 'Stop-ServiceAndVerify' +# Only the installer used to write the marker, so its Version kept naming the last INSTALL and a +# script-driven update was invisible in it - including on the setup wizard's own mode page, which +# reads exactly this value to tell the operator what is already installed. +Assert-TextMatches -Name 'a successful update refreshes the installation marker' ` + -Text $successCode ` + -Pattern "(?s)SOFTWARE\\NodePilot\\Server[\s\S]{0,700}New-ItemProperty[^\r\n]*-Name 'Version'" +# The marker is one machine-wide key, so on a host with a second instance it may well describe a +# different installation. Stamping this update's version onto that one is worse than leaving it stale. +Assert-TextMatches -Name 'and only when the marker describes this installation' ` + -Text $successCode -Pattern '\$markerInstallPath\.TrimEnd\([^)]*\) -eq \$InstallPath\.TrimEnd\(' + # The rollback path is the one place the prior state still governs. $rollbackPath = $updateScript.Substring($catchStart) Assert-TextMatches -Name 'a failed update still restores the pre-update state' ` diff --git a/deploy/Update-NodePilot.ps1 b/deploy/Update-NodePilot.ps1 index 9855dfe..8774f61 100644 --- a/deploy/Update-NodePilot.ps1 +++ b/deploy/Update-NodePilot.ps1 @@ -328,6 +328,35 @@ public class TrustAllCertsUpdate : ICertificatePolicy { catch { Write-Host "[update] Update succeeded, but old backup pruning failed: $($_.Exception.Message)" -ForegroundColor Yellow } + + # Only Install-NodePilot.ps1 used to write this, so the marker kept the version of the last + # INSTALL and every script-driven update was invisible in it. That value is what the setup + # wizard puts on its mode page ("NodePilot is already installed in ") and + # the obvious thing for an inventory to read - measured in the lab: binaries updated from + # 1.2.6-rc1 to the 1.2.5 artifact, marker still claiming 1.2.6-rc1. + # + # Version only: path, service name, provider and port are not changed by an update and are + # already correct. Guarded on InstallPath because the marker is a single machine-wide key - + # on a host running more than one instance it describes whichever was installed last, and + # stamping this update's version onto another instance's marker is worse than leaving it + # stale. A failure here is a warning: it costs discoverability, not a working installation. + try { + $markerPath = 'HKLM:\SOFTWARE\NodePilot\Server' + $marker = Get-ItemProperty -LiteralPath $markerPath -ErrorAction Stop + $markerInstallPath = [string]$marker.InstallPath + if ($markerInstallPath -and + $markerInstallPath.TrimEnd('\') -eq $InstallPath.TrimEnd('\')) { + New-ItemProperty -LiteralPath $markerPath -Name 'Version' ` + -Value ([string]$verifiedArtifact.Version) -PropertyType String -Force | Out-Null + Write-Info "Installation marker updated to version $($verifiedArtifact.Version)." + } + else { + Write-Info 'Installation marker describes another installation; left untouched.' + } + } catch { + Write-Warn "Could not update the installation marker: $($_.Exception.Message)" + } + Write-Ok 'Update complete.' } catch {