Skip to content

Harden server setup rollback, uninstall and local TLS - #783

Merged
Sev7eNup merged 3 commits into
mainfrom
fix/audit-server-setup
Nov 3, 2026
Merged

Sev7eNup merged 3 commits into
mainfrom
fix/audit-server-setup

Conversation

@Sev7eNup

@Sev7eNup Sev7eNup commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

First of three setup/switcher PRs from the 2026-10-28 audit. The desktop and switcher PRs are stacked on this one, so merge this first.

What changes

  • Local TLS in setup (D01): the install health probe and the wizard's first login go through a new LocalServerHttp.ps1 helper. It accepts only the configured server certificate (by thumbprint), refuses redirects and proxies, and only talks to loopback HTTPS. The process-wide TrustAllCerts policy is gone. Nothing changes for the operator: the certificate may still name the public host or be self-signed, TLS 1.2 and 1.3 are still offered on Windows PowerShell 5.1 as the old probe did, and a rejected first login still shows the server's own message.
  • Identity change rollback (Restore install-directory ACLs when server identity changes roll back #735): the installer snapshots the install root DACL before it changes it. A failed reinstall from gMSA A to B restores that DACL before the old binaries are copied back, so A can start them again.
  • Repeatable uninstall, server part (Stop installer uninstallation when service cleanup fails #714): a failed sc.exe delete stops the script before files are removed. The installation marker and deploy\ are removed last, only after everything else is gone. The Inno uninstaller aborts (non-zero exit, Apps & features entry kept) when the cleanup fails, cannot start, or its script is missing while the marker still exists. Because the script runs hidden, it now keeps a transcript at %TEMP%\nodepilot-server-uninstall.log (deleted after a clean run), and the abort message names that file.
  • PurgeData on German Windows (Make Windows data purge ownership recovery locale independent #731): ownership is taken with takeown /A item by item instead of takeown /R /D Y, which German Windows rejects. If any native step fails, nothing under the data directory is deleted.
  • gMSA repair advice (Preserve existing principals in gMSA and RBCD repair guidance #713): the suggested command reads the existing PrincipalsAllowedToRetrieveManagedPassword and adds this host instead of replacing the list. The deploy README drops the blanket RBCD recipe, in line with the guidance Remove blanket delegation grants from direct service-identity WinRM guidance #707 already put on the website.

The wizard loads the new helper only in install mode, so its other steps do not compile C#. CHANGELOG and the website's uninstall section are updated. The new harness runs in both CI PowerShell steps.

Testing

Windows PowerShell 5.1, locally:

  • Test-LocalServerHttp.ps1 (21 checks), Test-ArtifactSecurity.ps1, Test-DeploymentTemplates.ps1, server/Test-ServerWizardCompile.ps1: pass.
  • Test-SetupAdapter.ps1 stops on this machine at the unrelated "occupied port" check, the same way on main. I ran the new gMSA remediation case on its own against Preflight.ps1, including a quote in the account name: pass.
  • docs-ui: 227 tests pass.

Lab (signed 1.5.4-audit1 build of the same changes, GW1 and the German host):

  • D30: failed gMSA A→B reinstall leaves the install root DACL as before and the service runs as A again (/healthz/ready 200); an update with an unchanged identity also passes.
  • D09: with service deletion denied via sc sdset, the uninstall exits 1 and keeps the service, the files, deploy\, the marker and the Apps & features entry; the retry removes everything.
  • D26: old takeown /R /D Y fails on German Windows; the new helper unlocks and deletes a SYSTEM-owned folder.

PowerShell 7 runs and the rest of Test-SetupAdapter.ps1 are covered by CI. The full release lab matrix runs before the next tag as usual.

Closes #713
Closes #731
Closes #735
Refs #714 (closed by the desktop PR)

🤖 Generated with Claude Code

- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
@Sev7eNup
Sev7eNup merged commit 6f26bf7 into main Nov 3, 2026
16 checks passed
@Sev7eNup
Sev7eNup deleted the fix/audit-server-setup branch November 3, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant