From b8848a3d758af7ef6e5e1e163e5933b97e3aa629 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:01:00 +0200 Subject: [PATCH 1/4] fix(setup): name the longest part of an update, and stop scanning it twice over An upgrade sat for minutes on "Updating NodePilot" with a motionless bar, then finished quickly. Nothing was wrong; the wizard simply had no vocabulary for the stretch that costs the most. Measured on the 1.2.1 artifact: 2867 files totalling 114 MB, of which 2646 are under 64 KB and 1573 under 8 KB - `knowledge/` alone contributes 2304 files for 25 MB. An update walks that tree six times over (expand to staging, hash against the signed manifest, back up the old install, wipe the install path, copy staging in, hash again), so roughly 17,000 individual file operations, nearly all on tiny files. One full hashing pass measures 2.7 s on a warm NVMe, which rules hashing out as the cost: what makes it minutes is a real-time scanner inspecting every one of those creations. Three changes, none of them to what the update actually does: The updater now announces `Extracting artifact` before it expands and verifies, matching the installer's existing heading, and the progress table gains the matching entry at 10% - ahead of the backup at 20%. The first phase used to be the backup, so everything before it ran silently. The bidirectional drift guard already pins table and Write-Step calls to each other, and Test-SetupAdapter now asserts specifically that extraction is announced and ordered ahead of the backup, so a rename or a reorder cannot quietly restore the silence. The AV hand-off list gains `%TEMP%\nodepilot-artifact-*`. It documented Program Files, ProgramData and the backup folders, but not the staging directory where installer and updater create those ~2900 files first - so the one place that dominates the runtime was the one place not covered. Recorded as recommended rather than mandatory, with the residual risk stated plainly: the directory already carries a restrictive DACL applied atomically at creation, and its contents are verified file by file against the signed manifest immediately afterwards. `Import-NodePilotPkcsTypes` now asks the edition instead of attempting and catching. Add-Type raises a TERMINATING error under the setup's Stop preference and Start-Transcript records it before the catch swallows it, so every setup log carried a red "assembly could not be found" directly above the line confirming the signature had verified. Harmless, and exactly the kind of thing that makes an operator abort a healthy install. --- deploy/ArtifactSecurity.ps1 | 19 +++++++++++++++++-- deploy/SetupContract.ps1 | 6 +++++- deploy/Test-SetupAdapter.ps1 | 11 +++++++++++ deploy/Update-NodePilot.ps1 | 8 ++++++++ docs/av-exclusions.md | 1 + .../content/deployment/av-exclusions.md | 1 + 6 files changed, 43 insertions(+), 3 deletions(-) diff --git a/deploy/ArtifactSecurity.ps1 b/deploy/ArtifactSecurity.ps1 index 54a16d2..c3e8507 100644 --- a/deploy/ArtifactSecurity.ps1 +++ b/deploy/ArtifactSecurity.ps1 @@ -28,9 +28,24 @@ function New-NodePilotRandomBase64 { } function Import-NodePilotPkcsTypes { + <# + The assembly carrying SignedCms is named differently per edition: Windows PowerShell 5.1 + ships it inside System.Security, PowerShell 7 as System.Security.Cryptography.Pkcs. + + Asked by edition rather than attempted and caught. The try/catch this replaces worked, but + Add-Type raises a TERMINATING error under the setup's Stop preference, and Start-Transcript + records it before the catch can swallow it - so every setup log carried a red + "Die Assembly ... konnte nicht gefunden werden" immediately above the line confirming the + signature had verified. An operator reading that in CMTrace has every reason to abort a + perfectly healthy installation. + #> if ('System.Security.Cryptography.Pkcs.SignedCms' -as [type]) { return } - try { Add-Type -AssemblyName System.Security.Cryptography.Pkcs -ErrorAction Stop } - catch { Add-Type -AssemblyName System.Security -ErrorAction Stop } + if ($PSVersionTable.PSEdition -eq 'Core') { + Add-Type -AssemblyName System.Security.Cryptography.Pkcs -ErrorAction Stop + } + else { + Add-Type -AssemblyName System.Security -ErrorAction Stop + } } function ConvertFrom-NodePilotHex { diff --git a/deploy/SetupContract.ps1 b/deploy/SetupContract.ps1 index 6dbabcf..330e7bc 100644 --- a/deploy/SetupContract.ps1 +++ b/deploy/SetupContract.ps1 @@ -366,9 +366,13 @@ $script:NodePilotInstallPhases = @( [pscustomobject]@{ Step = 'Starting service'; Percent = 80; Text = 'Starting the service - this can take up to three minutes' } ) -# The updater's four phases. The probe here waits 60 s, not the installer's 180, so the last +# The updater's five phases. The probe here waits 60 s, not the installer's 180, so the last # caption promises less. $script:NodePilotUpdatePhases = @( + # Ahead of the backup on purpose: expanding ~2900 files to staging and hashing every one of + # them against the signed manifest is the longest part of an update, and it used to happen + # with no phase of its own - the dialog stood on its start caption until the backup began. + [pscustomobject]@{ Step = 'Extracting artifact'; Percent = 10; Text = 'Extracting and verifying the signed artifact - this can take a few minutes' } [pscustomobject]@{ Step = 'Backing up current install'; Percent = 20; Text = 'Backing up the current installation' } [pscustomobject]@{ Step = 'Stopping service'; Percent = 40; Text = 'Stopping the service' } [pscustomobject]@{ Step = 'Installing verified artifact'; Percent = 55; Text = 'Installing the verified artifact' } diff --git a/deploy/Test-SetupAdapter.ps1 b/deploy/Test-SetupAdapter.ps1 index 597d5e9..ea419d0 100644 --- a/deploy/Test-SetupAdapter.ps1 +++ b/deploy/Test-SetupAdapter.ps1 @@ -602,6 +602,7 @@ try { # Every phase either script announces has to be recognised - the reverse of the drift guard, # checked here against the real tables rather than against the scripts. foreach ($sample in @( + '[update] Extracting artifact', '[update] Backing up current install', "[update] Stopping service 'NodePilot'", '[update] Installing verified artifact', @@ -609,6 +610,16 @@ try { Assert-True -Name "the updater phase in '$sample' is recognised" ` -Condition ($null -ne (Get-NodePilotPhaseProgress -Line $sample)) } + # Extraction has to come FIRST, and it has to exist at all. It is the longest part of an + # update - ~2900 files expanded to staging, then hashed one by one against the signed manifest + # - and it used to run with no phase of its own, so the dialog sat on its start caption for + # minutes while an operator reasonably concluded the upgrade had hung. A future edit that + # renames the heading or reorders the table past the backup brings that back. + $extractPhase = Get-NodePilotPhaseProgress -Line '[update] Extracting artifact' + $backupPhase = Get-NodePilotPhaseProgress -Line '[update] Backing up current install' + Assert-True -Name 'the updater announces extraction before the backup' ` + -Condition ($null -ne $extractPhase -and $null -ne $backupPhase -and + [int]$extractPhase.Percent -lt [int]$backupPhase.Percent) Assert-True -Name 'an updater detail line is not a phase' ` -Condition ($null -eq (Get-NodePilotPhaseProgress -Line '[update] Backup: C:\x')) # Ascending percentages are what let the wizard refuse to ever move the bar backwards without diff --git a/deploy/Update-NodePilot.ps1 b/deploy/Update-NodePilot.ps1 index 01857cd..9855dfe 100644 --- a/deploy/Update-NodePilot.ps1 +++ b/deploy/Update-NodePilot.ps1 @@ -189,6 +189,14 @@ try { } # Reject a malformed signed ZIP before stopping the service or touching the installation. + # + # Announced, because this is the longest stretch of the whole update and used to run without a + # single progress line: the first phase below is the backup, so the wizard sat on its start + # caption while ~2900 files were expanded to staging and then hashed one by one against the + # signed manifest. Measured 2.7 s of hashing on a warm NVMe, minutes on a machine whose + # real-time scanner inspects every file created under %TEMP% - see docs/av-exclusions.md. + # An operator watching a motionless dialog reasonably concludes it has hung. + Write-Step 'Extracting artifact' $artifactStage = Expand-NodePilotArtifactToStaging -ArtifactPath $ArtifactPath Write-Info "Verified restricted staging: $artifactStage" diff --git a/docs/av-exclusions.md b/docs/av-exclusions.md index 6583408..952e95e 100644 --- a/docs/av-exclusions.md +++ b/docs/av-exclusions.md @@ -63,6 +63,7 @@ Rolle: Windows-Server mit dem Dienst **`NodePilot`** (Anzeigename `NodePilot Orc | `C:\Program Files\NodePilot\` | Programmverzeichnis: `NodePilot.Api.exe`, ~mehrere hundert verwaltete DLLs, `wwwroot\` (SPA), `PSModules\`, `knowledge\` | Wird beim Update komplett getauscht; ein gehaltenes Scanner-Handle lässt Verschieben/Löschen fehlschlagen. Enthält außerdem `powershell.config.json` — siehe Warnung unter [A.4](#a4-verhaltensregeln-asr-controlled-folder-access) | Pflicht | Schreibrechte hat nur SYSTEM/Administratoren; ein Angreifer mit diesen Rechten hat das System ohnehin. Restrisiko: eine dort abgelegte Fremd-DLL würde nicht mehr gescannt — kompensierbar über Publisher-Regel statt Pfad und über Integritätsüberwachung des Verzeichnisses | | `C:\ProgramData\NodePilot\` | Laufzeitdaten: `logs\`, `archive\`, `jwt-secret.key`, `data-protection-keys\`, `admin-setup.token`, `appsettings.runtime.json`, `install-report.txt`, `postgres-root-ca.pem` | Dauerhaftes Schreiben (Rolling Logs, atomare Config-Writes über `.tmp` + `File.Replace`, gzip-Archive). Scanner-Handles auf der Zieldatei lassen den atomaren Ersetzungsschritt scheitern | Pflicht | Verzeichnis ist per ACL auf das Dienstkonto + Administratoren beschränkt. Es liegen dort **Schlüsselmaterial und Tokens** — der Ausschluss verhindert nicht deren Diebstahl (dafür ist der Dateizugriff zuständig), er reduziert nur die Erkennung einer dort abgelegten Schaddatei | | `C:\Program Files\NodePilot.rollback.*`
`C:\Program Files\NodePilot.backup.*` | Zeitgestempelte Kopien des vorherigen Programmverzeichnisses (drei werden aufbewahrt) | Entstehen nur während Installation/Update; enthalten dieselben Binärdateien wie oben | Empfohlen | Wie Programmverzeichnis. Kann auf ein Wartungsfenster befristet werden | +| `%TEMP%\nodepilot-artifact-*`
(Dienst-Kontext: `C:\Windows\Temp\nodepilot-artifact-*`) | Staging des signierten Artefakts: Installer **und** Updater entpacken das Zip zuerst hierher — rund **2900 Dateien**, davon ~2650 kleiner als 64 KB, und prüfen anschließend jede einzelne gegen das signierte Manifest | Dies ist die **teuerste Stelle des gesamten Updates** und der Grund, warum ein Upgrade minutenlang auf einer Stelle zu stehen scheint: Nicht die Datenmenge (114 MB) kostet, sondern die Dateianzahl. Ein Echtzeit-Scan prüft jede Erzeugung einzeln und vervielfacht die Laufzeit; ein gehaltenes Handle lässt zusätzlich das Aufräumen des Staging-Ordners scheitern | Empfohlen | Der Ordner trägt bereits eine restriktive DACL (SYSTEM + Administratoren + aufrufender Benutzer, atomar bei der Erzeugung gesetzt), und sein Inhalt wird unmittelbar nach dem Entpacken **gegen das signierte Manifest** verifiziert — Datei für Datei, mit Längen- und SHA-256-Vergleich. Der Ausschluss senkt die Erkennung also genau dort, wo bereits kryptografisch geprüft wird. Kann auf ein Wartungsfenster befristet werden | Ausdrücklich **nicht** enthalten: Ordner, in die Workflows schreiben. Siehe [Was nicht ausgeschlossen werden soll](#was-ausdrücklich-nicht-ausgeschlossen-werden-soll). diff --git a/src/nodepilot-docs-ui/content/deployment/av-exclusions.md b/src/nodepilot-docs-ui/content/deployment/av-exclusions.md index bb8bb65..a054023 100644 --- a/src/nodepilot-docs-ui/content/deployment/av-exclusions.md +++ b/src/nodepilot-docs-ui/content/deployment/av-exclusions.md @@ -32,6 +32,7 @@ Ein Dienst: `NodePilot` (Anzeigename `NodePilot Orchestrator`), ausgeführt als | `C:\Program Files\NodePilot` | Programmverzeichnis, wird beim Update vollständig getauscht | Pflicht | Nur SYSTEM und Administratoren haben Schreibrechte. Publisher-Regel statt Pfad bevorzugen | | `C:\ProgramData\NodePilot` | Logs, Archive, Schlüsselmaterial, Laufzeitkonfiguration | Pflicht | Enthält Schlüssel und Tokens; deren Schutz liegt bei der ACL, nicht beim Scanner | | `C:\Program Files\NodePilot.rollback.*`, `…NodePilot.backup.*` | Zeitgestempelte Vorgängerstände (drei werden aufbewahrt) | Empfohlen | Auf das Wartungsfenster befristbar | +| `%TEMP%\nodepilot-artifact-*` (Dienst: `C:\Windows\Temp\…`) | Staging des signierten Artefakts — ~2900 Dateien, davon ~2650 unter 64 KB. Die **teuerste Stelle eines Updates**: nicht die 114 MB kosten, sondern die Dateianzahl, und ein Echtzeit-Scan prüft jede Erzeugung einzeln | Empfohlen | Restriktive DACL bei der Erzeugung; der Inhalt wird direkt danach Datei für Datei gegen das signierte Manifest geprüft. Auf das Wartungsfenster befristbar | ### Prozesse From d8de005086330669ddf171ec1528ba7c0ea0f156 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:01:01 +0200 Subject: [PATCH 2/4] perf(setup): extract the artifact with ZipFile instead of Expand-Archive Measured on the real 1.2.1 artifact (2867 files, 114 MB, 2646 of them under 64 KB), twice, warm: Expand-Archive 25.8 s / 24.7 s ExtractToDirectory 2.2 s / 2.1 s ~12x Expand-Archive pays per-entry pipeline overhead, which dominates a tree of mostly tiny files. On a host whose real-time scanner inspects every creation the absolute saving is proportionally larger, and this runs on both the install and the update path. Equivalence was verified rather than assumed: both extractors produce the same 2867 files across the same 377 directories, with zero differences in relative path, length or SHA-256. Zip-slip is not given up in the trade. .NET refuses an entry whose resolved path leaves the destination, confirmed against a crafted archive carrying '../escaped.txt' - nothing was written outside, same as Expand-Archive. Test-ArtifactSecurity now builds that archive and asserts the rejection, because a later swap to a hand-rolled extraction loop is exactly how this property would be lost quietly. Assert-NodePilotExtracted- Files remains the second line regardless: rooted and dot-dot manifest paths rejected, exact file count required, every file hashed against the signed manifest. Paths are resolved to absolute first - ExtractToDirectory resolves a relative path against the process working directory, which is neither the caller's location nor the staging parent. Import-NodePilotZipTypes loads the assembly by edition for the same reason as the Pkcs helper: a try/catch would write a red terminating error into every setup transcript. --- deploy/ArtifactSecurity.ps1 | 32 +++++++++++++++++++++++++++++++- deploy/Test-ArtifactSecurity.ps1 | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/deploy/ArtifactSecurity.ps1 b/deploy/ArtifactSecurity.ps1 index c3e8507..46fe7f5 100644 --- a/deploy/ArtifactSecurity.ps1 +++ b/deploy/ArtifactSecurity.ps1 @@ -48,6 +48,19 @@ function Import-NodePilotPkcsTypes { } } +function Import-NodePilotZipTypes { + <# + System.IO.Compression.ZipFile lives in a separate assembly that Windows PowerShell 5.1 does + not load on its own; PowerShell 7 has it in the default set. Asked by edition for the same + reason as Import-NodePilotPkcsTypes: a try/catch here would write a red terminating error + into every setup transcript before swallowing it. + #> + if ('System.IO.Compression.ZipFile' -as [type]) { return } + if ($PSVersionTable.PSEdition -ne 'Core') { + Add-Type -AssemblyName System.IO.Compression.FileSystem -ErrorAction Stop + } +} + function ConvertFrom-NodePilotHex { param([Parameter(Mandatory)][string]$Hex) if ($Hex.Length % 2 -ne 0 -or $Hex -notmatch '^[0-9A-Fa-f]+$') { throw "Invalid hexadecimal value in artifact manifest." } @@ -435,9 +448,26 @@ function Expand-NodePilotArtifactToStaging { [string]$ParentPath = [IO.Path]::GetTempPath() ) + # Absolute paths: ExtractToDirectory resolves relative ones against the PROCESS working + # directory, which is not the caller's location and is not the staging parent either. + $resolvedArtifact = (Resolve-Path -LiteralPath $ArtifactPath -ErrorAction Stop).Path $stagingPath = New-NodePilotRestrictedStagingDirectory -ParentPath $ParentPath try { - Expand-Archive -LiteralPath $ArtifactPath -DestinationPath $stagingPath -Force + # ExtractToDirectory rather than Expand-Archive: measured 2.2 s against 25.8 s for this + # artifact (2867 files, 114 MB) - Expand-Archive pays per-entry pipeline overhead that + # dominates a tree of mostly sub-64 KB files. The two were verified to produce identical + # trees: same 2867 files, same 377 directories, zero differences in path, length or + # SHA-256. + # + # Zip-slip protection is NOT lost in the trade. .NET refuses an entry whose resolved path + # leaves the destination ("Durch Extrahieren des Zip-Eintrags wuerde eine Datei ausserhalb + # des angegebenen Zielverzeichnisses erstellt"), verified against a crafted archive + # carrying '../escaped.txt'. Assert-NodePilotExtractedFiles below is the second line + # anyway: it rejects rooted or dot-dot paths in the manifest, requires an exact file + # count, and hashes every file against the signed manifest - so an entry that landed + # somewhere unexpected inside staging would fail there regardless. + Import-NodePilotZipTypes + [IO.Compression.ZipFile]::ExtractToDirectory($resolvedArtifact, $stagingPath) Assert-NodePilotExtractedFiles -RootPath $stagingPath return $stagingPath } diff --git a/deploy/Test-ArtifactSecurity.ps1 b/deploy/Test-ArtifactSecurity.ps1 index 07ef6d0..4477d46 100644 --- a/deploy/Test-ArtifactSecurity.ps1 +++ b/deploy/Test-ArtifactSecurity.ps1 @@ -84,6 +84,36 @@ try { throw 'Extracted artifact tampering was not detected.' } + # Zip-slip. The extractor was swapped from Expand-Archive to ZipFile::ExtractToDirectory for + # speed (2.2 s against 25.8 s on the real artifact), and the whole reason that trade is + # acceptable is that .NET refuses an entry resolving outside the destination. Asserted here + # rather than assumed, because a future swap to a hand-rolled extraction loop would silently + # give it up: this is the one property the staging directory cannot recover from. + $slipZip = Join-Path $testRoot 'zip-slip.zip' + $slipSource = Join-Path $testRoot 'slip-src' + New-Item -ItemType Directory -Path $slipSource -Force | Out-Null + [IO.File]::WriteAllText((Join-Path $slipSource 'harmless.txt'), 'ok', (New-Object Text.UTF8Encoding($false))) + Compress-Archive -Path (Join-Path $slipSource '*') -DestinationPath $slipZip -Force + # Compress-Archive cannot author a traversing entry name, so rewrite one in directly. + Import-NodePilotZipTypes + $slipArchive = [IO.Compression.ZipFile]::Open($slipZip, [IO.Compression.ZipArchiveMode]::Update) + try { + $entry = $slipArchive.CreateEntry('../escaped.txt') + $writer = New-Object IO.StreamWriter($entry.Open()) + try { $writer.Write('escaped') } finally { $writer.Dispose() } + } + finally { $slipArchive.Dispose() } + + $slipBlocked = $false + try { [void](Expand-NodePilotArtifactToStaging -ArtifactPath $slipZip -ParentPath $testRoot) } + catch { $slipBlocked = $true } + if (-not $slipBlocked) { + throw 'A zip entry escaping the staging directory was extracted instead of rejected.' + } + if (Test-Path -LiteralPath (Join-Path $testRoot 'escaped.txt')) { + throw 'Zip-slip wrote a file outside the staging directory.' + } + $secretPath = Join-Path $testRoot 'restricted-settings.json' $secretBytes = [Text.Encoding]::UTF8.GetBytes('{"secret":"first"}') try { From 48ca390fbf276a8fdf75c782f220d88948bf769f Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:01:01 +0200 Subject: [PATCH 3/4] docs(setup): correct the extraction speedup claim to what the lab measured The comment quoted 2.2 s against 25.8 s and the commit before it called that ~12x. That number is real but it is a workstation number, and a Server 2025 VM does not reproduce it: Win 11 workstation (PS 5.1.22621) 24.3-24.8 s -> 2.0-2.7 s ~9-12x Server 2025 VM, 4 cores (26100) 7.3- 7.7 s -> 4.6-5.2 s ~1.6x Same artifact, three and two runs. Notably the workstation is the FASTER machine on the new path and the slower one on the old, so the gap is in what Expand-Archive costs per entry there rather than in raw I/O - which is exactly why a single multiplier does not travel. The change stays: faster on both, identical output, zip-slip still rejected. But a server operator reading "12x" and measuring 1.6x would be right to distrust the rest of the file, and the lab number is the one they will get. --- deploy/ArtifactSecurity.ps1 | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/deploy/ArtifactSecurity.ps1 b/deploy/ArtifactSecurity.ps1 index 46fe7f5..45eaa46 100644 --- a/deploy/ArtifactSecurity.ps1 +++ b/deploy/ArtifactSecurity.ps1 @@ -453,11 +453,20 @@ function Expand-NodePilotArtifactToStaging { $resolvedArtifact = (Resolve-Path -LiteralPath $ArtifactPath -ErrorAction Stop).Path $stagingPath = New-NodePilotRestrictedStagingDirectory -ParentPath $ParentPath try { - # ExtractToDirectory rather than Expand-Archive: measured 2.2 s against 25.8 s for this - # artifact (2867 files, 114 MB) - Expand-Archive pays per-entry pipeline overhead that - # dominates a tree of mostly sub-64 KB files. The two were verified to produce identical - # trees: same 2867 files, same 377 directories, zero differences in path, length or - # SHA-256. + # ExtractToDirectory rather than Expand-Archive, which pays per-entry pipeline overhead + # that dominates a tree of mostly sub-64 KB files. Faster everywhere measured, but the + # margin is strongly environment-dependent, so do not quote a single multiplier: + # + # Win 11 workstation (PS 5.1.22621) 24.3-24.8 s -> 2.0-2.7 s ~9-12x + # Server 2025 VM, 4 cores (26100) 7.3- 7.7 s -> 4.6-5.2 s ~1.6x + # + # Same artifact (2867 files, 114 MB), three and two runs respectively. The workstation is + # the faster machine on the new path and the slower one on the old, so the difference is + # in what Expand-Archive costs per entry there, not in raw I/O. Worth having on both, but + # the lab number is the one to expect on a server. + # + # The two were verified to produce identical trees: same 2867 files, same 377 directories, + # zero differences in relative path, length or SHA-256. # # Zip-slip protection is NOT lost in the trade. .NET refuses an entry whose resolved path # leaves the destination ("Durch Extrahieren des Zip-Eintrags wuerde eine Datei ausserhalb From 7f3480818859af8a2f0cd272453ee77053cad77f Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:01:02 +0200 Subject: [PATCH 4/4] fix(setup): give the prerequisites page room instead of crushing it With all ten checks reporting, the page had nothing left for the field that explains them. Each row is "Title: Detail", so at the default width six of ten wrap to two lines; LayoutReadiness stacks ~238 px of rows into a ~309 px surface and the remediation box gets what remains - about 30 px, one clipped line. That box is where a failed database check has to show a CREATE LOGIN / CREATE USER block, so one line is not a cosmetic problem. Scrolling was the other option and is disproportionate here: Inno's Pascal Script exposes no TScrollBox, the rows are individual controls on the page surface, and a real scroll container would mean rebuilding the list as a TRichEditViewer - losing the per-row click targets and the auto-fix checkboxes. That is a different feature, not a layout fix. So the window grows instead, in both directions and for different reasons. Width does most of the work: +25% puts most rows back on one line, which shortens the stack before height is even considered. Height then buys the explanation real room: ~309 -> ~471 px of surface leaves ~210 px, about twelve lines. At ~560 px including its frame the window still fits the 768 px console of a server VM. An earlier note in [Setup] claimed "every page fits at 100%" and reverted a previous attempt on that basis. That claim is what the ten-check page disproves, so it is replaced with the measurement rather than deleted - otherwise the next reader reverts this for the same stated reason. WizardResizable stays off: those controls are positioned once, at construction, and carry no anchors, so a window dragged open at runtime would grow around a certificate picker that stays put. A fixed larger START size is a different thing and is safe - everything that must grow is already sized from SurfaceWidth. The remediation floor moves 13 px -> 34 px in the same pass. At one line the control reads as a broken edit field rather than an explanation. The larger window means the floor is not reached in practice; it is there for high-DPI scaling and for the day an eleventh check arrives. Rows still win and the box still gives - the other precedence would draw the explanation over the last checks, and a check nobody can see is worse than an explanation that has to scroll. --- deploy/server/NodePilotServer.iss | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/deploy/server/NodePilotServer.iss b/deploy/server/NodePilotServer.iss index 1b1300e..27c1af1 100644 --- a/deploy/server/NodePilotServer.iss +++ b/deploy/server/NodePilotServer.iss @@ -53,12 +53,23 @@ MinVersion=10.0.20348 OutputDir={#OutputDir} OutputBaseFilename=NodePilot-Server-Setup-{#AppVersion} WizardStyle=modern -; Default window size. WizardSizePercent=120 was tried and reverted - every page fits at 100%, and -; the pages that are tight are tight by layout rather than by window size. +; Bigger than default, measured rather than guessed. An earlier note here claimed "every page fits +; at 100%" and reverted a previous attempt on that basis; the prerequisites page disproves it once +; all ten checks report. Each row is "Title: Detail" (see the render loop), so at the default width +; six of ten wrap to two lines, LayoutReadiness stacks ~238 px of rows into a ~309 px surface, and +; the remediation box - which gets whatever is left - lands at ~30 px. That is one clipped line for +; the field that has to show a CREATE LOGIN / CREATE USER block when the database check fails. ; -; WizardResizable stays off regardless of that: the controls on the network and prerequisites pages -; are positioned once, at wizard construction, and carry no anchors. A window the operator drags -; open would grow around a certificate picker that stays where it was. +; Width does most of the work: +25% (497 -> ~621 px) puts most rows back on one line, which shortens +; the stack before height is even considered. Height then buys the explanation real room: +45% +; (360 -> ~522 px, surface ~309 -> ~471 px) leaves ~210 px, about twelve lines. The window is +; ~560 px tall including its frame and fits a 768 px server console. +; +; WizardResizable stays off regardless: the controls on the network and prerequisites pages are +; positioned once, at wizard construction, and carry no anchors. A window the operator drags open +; would grow around a certificate picker that stays where it was. A fixed larger START size is a +; different thing and is safe - every control that must grow is already sized from SurfaceWidth. +WizardSizePercent=125,145 SetupIconFile={#StageDir}\setup-icon.ico LicenseFile={#StageDir}\LICENSE.txt ; A failed setup has to be diagnosable without asking the operator to reproduce it. @@ -614,7 +625,15 @@ begin Available := ButtonTop - ScaleY(6) - RemediationBox.Top; // Never negative, never overlapping the buttons: with every row wrapped and every fix offered // there is little left, and a label with no room is still better than one drawn over them. - if Available < ScaleY(13) then Available := ScaleY(13); + // + // The floor is two lines plus the scrollbar rather than one. At one line the control reads as a + // broken edit field instead of an explanation - which is exactly what it looked like with ten + // checks before the window was enlarged. The larger window means this is not reached in + // practice; it is here for high-DPI scaling and for the day an eleventh check arrives. + // + // Rows still win and the box still gives: the other precedence would draw the explanation over + // the last checks, and a check nobody can see is worse than an explanation that has to scroll. + if Available < ScaleY(34) then Available := ScaleY(34); RemediationBox.Height := Available; end;