Verify the data directory the way the service will, before starting it

deploy/Install-NodePilot.ps1

@@ -375,6 +375,143 @@ function Set-DirectoryAclForService {
     Set-Acl -Path $Path -AclObject $acl
 }
 
+function Test-ServiceDirectoryAclTrust {
+    <#
+      Answers the one question the service will ask itself seconds later, and answers it the same
+      way: RestrictedFileWriter.ValidateWindowsDirectoryAcl refuses to read the JWT signing key
+      when the directory holding it has an owner it does not trust, or grants mutation rights to a
+      principal outside a deliberately tiny set - SYSTEM, Administrators, TrustedInstaller,
+      CreatorOwner, OwnerRights, and the identity the service is RUNNING as.
+
+      That last one is the trap this exists for. An ACE for a service account is only harmless
+      while the service runs as that account; install once as A and again as B, and A's leftover
+      ACE is a stranger with write access next to the signing key. The service then refuses to
+      start with "grants mutation rights to an untrusted principal", after the installer has
+      already replaced the binaries - so the failure lands in the rollback path instead of in a
+      check. Same rule, evaluated here, turns that into something the installer can fix.
+
+      Returns @{ IsSecure = bool; Reason = string } - never throws, so a directory this account
+      cannot read its ACL from is reported rather than crashing the install.
+    #>
+    param(
+        [Parameter(Mandatory)][string]$Path,
+        [Parameter(Mandatory)][string]$ServiceAccount,
+        [switch]$SkipServiceRule
+    )
+
+    try {
+        $acl = Get-Acl -LiteralPath $Path -ErrorAction Stop
+    } catch {
+        return @{ IsSecure = $false; Reason = "the ACL of '$Path' could not be read: $($_.Exception.Message)" }
+    }
+
+    # Mirrors BuildTrustedSids() on the API side. Kept as SIDs, not names, because a localised
+    # Windows calls these groups something else and a domain account resolves differently.
+    $trusted = [System.Collections.Generic.HashSet[string]]::new(
+        [string[]]@(
+            'S-1-5-18',                                                       # LocalSystem
+            'S-1-5-32-544',                                                   # Administrators
+            'S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464', # TrustedInstaller
+            'S-1-3-0',                                                        # CreatorOwner
+            'S-1-3-4'                                                         # OwnerRights
+        ),
+        [System.StringComparer]::OrdinalIgnoreCase)
+
+    # The account the service will run as. LocalSystem is already in the set above; for anything
+    # else the SID has to be resolved, and a name that no longer resolves is itself a finding.
+    if (-not $SkipServiceRule) {
+        try {
+            $svcSid = (New-Object System.Security.Principal.NTAccount($ServiceAccount)).Translate(
+                [System.Security.Principal.SecurityIdentifier]).Value
+            $null = $trusted.Add($svcSid)
+        } catch {
+            return @{ IsSecure = $false; Reason = "the service account '$ServiceAccount' could not be resolved to a SID: $($_.Exception.Message)" }
+        }
+    }
+
+    $ownerSid = $null
+    try {
+        $ownerSid = $acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value
+    } catch {
+        return @{ IsSecure = $false; Reason = "the owner of '$Path' could not be read: $($_.Exception.Message)" }
+    }
+    if (-not $trusted.Contains($ownerSid)) {
+        return @{ IsSecure = $false; Reason = "'$Path' is owned by $(Resolve-SidLabel $ownerSid), which the service does not trust" }
+    }
+
+    # Same right mask the API applies to the immediate parent of a secret.
+    $dangerous = [System.Security.AccessControl.FileSystemRights]::Delete `
+        -bor [System.Security.AccessControl.FileSystemRights]::DeleteSubdirectoriesAndFiles `
+        -bor [System.Security.AccessControl.FileSystemRights]::ChangePermissions `
+        -bor [System.Security.AccessControl.FileSystemRights]::TakeOwnership `
+        -bor [System.Security.AccessControl.FileSystemRights]::CreateFiles `
+        -bor [System.Security.AccessControl.FileSystemRights]::CreateDirectories `
+        -bor [System.Security.AccessControl.FileSystemRights]::WriteAttributes `
+        -bor [System.Security.AccessControl.FileSystemRights]::WriteExtendedAttributes
+
+    foreach ($rule in $acl.GetAccessRules($true, $true, [System.Security.Principal.SecurityIdentifier])) {
+        if ($rule.AccessControlType -ne [System.Security.AccessControl.AccessControlType]::Allow) { continue }
+        if ($rule.PropagationFlags.HasFlag([System.Security.AccessControl.PropagationFlags]::InheritOnly)) { continue }
+        $sid = $rule.IdentityReference.Value
+        if ($trusted.Contains($sid)) { continue }
+        if (($rule.FileSystemRights -band $dangerous) -ne 0) {
+            return @{ IsSecure = $false; Reason = "'$Path' grants write access to $(Resolve-SidLabel $sid), which the service does not trust" }
+        }
+    }
+
+    return @{ IsSecure = $true; Reason = '' }
+}
+
+function Resolve-SidLabel {
+    <#
+      "DOMAIN\account (S-1-5-21-...)" where the SID still resolves, the bare SID otherwise. An
+      orphaned SID is the common case here - a decommissioned service account keeps its ACE - and
+      it is precisely the value icacls needs to remove it, so translation failure is an answer,
+      not an error.
+    #>
+    param([Parameter(Mandatory)][string]$Sid)
+    try {
+        $name = (New-Object System.Security.Principal.SecurityIdentifier($Sid)).Translate(
+            [System.Security.Principal.NTAccount]).Value
+        return "$name ($Sid)"
+    } catch {
+        return "$Sid (account no longer exists)"
+    }
+}
+
+function Assert-ServiceDirectoryAclUsable {
+    <#
+      Verify, repair once, verify again - then give up loudly instead of handing the service a
+      directory it will refuse. The repair is not a second mechanism: it is the same
+      Set-DirectoryAclForService the install already ran, which drops inheritance, wipes every
+      explicit ACE and forces the owner back to Administrators. Running it again is what clears a
+      stranger's ACE that predates this installation.
+    #>
+    param(
+        [Parameter(Mandatory)][string]$Path,
+        [Parameter(Mandatory)][string]$ServiceAccount,
+        [switch]$SkipServiceRule,
+        [Parameter(Mandatory)][string]$Label
+    )
+
+    $verdict = Test-ServiceDirectoryAclTrust -Path $Path -ServiceAccount $ServiceAccount -SkipServiceRule:$SkipServiceRule
+    if ($verdict.IsSecure) { return }
+
+    Write-Warn "  $Label is not usable by the service yet: $($verdict.Reason)"
+    Write-Info '  Repairing it (owner, inheritance and ACEs) and re-checking.'
+    Set-DirectoryAclForService -Path $Path -ServiceAccount $ServiceAccount -SkipServiceRule:$SkipServiceRule
+
+    $verdict = Test-ServiceDirectoryAclTrust -Path $Path -ServiceAccount $ServiceAccount -SkipServiceRule:$SkipServiceRule
+    if ($verdict.IsSecure) {
+        Write-Info '  Repaired.'
+        return
+    }
+
+    throw ("$Label cannot be made usable by the service: $($verdict.Reason). " +
+           'The service would refuse to start with "JWT signing-key file security validation failed". ' +
+           "Remove that entry (icacls '$Path' /remove:g '<account>'), then run the installer again.")
+}
+
 function Assert-SafeInstallRoot {
     <#
       H-18. The install directory holds the service binaries and is registered as the image path
@@ -1092,6 +1229,13 @@ foreach ($identityBoundSecret in @('jwt-secret.key', 'admin-setup.token')) {
     }
 }
 
+# Applying an ACL and assuming it landed is what let a leftover ACE from an earlier installation
+# survive all the way to the first service start, where the API - not the installer - discovered
+# it and refused to read the JWT key. Ask the same question here, while the only thing that has
+# happened is that two directories exist: the binaries are not extracted until below.
+Assert-ServiceDirectoryAclUsable -Path $DataPath -ServiceAccount $AclIdentity `
+    -SkipServiceRule:$isLocalSystem -Label "The data directory '$DataPath'"
+
 # Provisioning seed: copied in rather than referenced where the operator left it, because the API
 # reads it as the service account at first start and a deployment share is not reachable then.
 # It carries credentials, so it lands with the same restricted ACL as the configuration itself and

deploy/README.md

@@ -348,7 +348,7 @@ Der Installer macht alles Weitere:
 5. `appsettings.Production.json` aus Template erzeugen
 6. ACLs setzen (Service-Identität = gMSA bzw. `NT AUTHORITY\SYSTEM` bei LocalSystem):
    - InstallPath: Service = **ReadAndExecute**, Admins/SYSTEM = Full, Vererbung aus. Der Dienst führt die Binaries aus, er überschreibt sie nie — Schreibrecht dort wäre Code-Ausführung als Dienstkonto (H-18). Der Pfad wird vorher validiert (lokal, NTFS/ReFS, keine Reparse Points) und nach dem Kopieren erneut geprüft.
-   - DataPath: Service = Modify, Admins/SYSTEM = Full, sonst nichts. Bei LocalSystem deckt die SYSTEM-Full-ACE den Dienst bereits ab — keine zusätzliche ACE.
+   - DataPath: Service = Modify, Admins/SYSTEM = Full, sonst nichts. Bei LocalSystem deckt die SYSTEM-Full-ACE den Dienst bereits ab — keine zusätzliche ACE. **Direkt danach wird gegengeprüft**, und zwar mit derselben Regel, die der Dienst gleich anlegt (`Test-ServiceDirectoryAclTrust` spiegelt `RestrictedFileWriter.BuildTrustedSids`): Owner vertrauenswürdig, kein fremder Allow-ACE mit Mutationsrechten. Schlägt das an, repariert der Installer einmal und prüft erneut; erst dann geht es weiter. Grund: Eine ACE für ein Dienstkonto ist nur so lange harmlos, wie der Dienst **genau unter diesem Konto** läuft — ein Rest aus einer Installation mit anderer Identität überlebte sonst bis zum ersten Dienststart und kippte ihn dort mit „grants mutation rights to an untrusted principal", also erst im Rollback-Pfad. Die Prüfung liegt bewusst **vor** dem Entpacken des Artefakts, damit ein Aufgeben nichts kostet.
    - `appsettings.Production.json`: Service = Read, Admins/SYSTEM = Full (bei LocalSystem analog von SYSTEM-Full abgedeckt)
    - Cert Private Key: gMSA = Read; bei LocalSystem übersprungen (SYSTEM hat Read auf MachineKeys per Default)
     - PostgreSQL: `ConnectionStrings:Postgres` bleibt in JSON leer; der vollständig gequotete

deploy/Test-DeploymentTemplates.ps1

@@ -582,6 +582,37 @@ Assert-TextMatches -Name 'the data directory gets a trusted owner, not just trus
     -Text $installerScript.Substring($aclFunctionStart, $aclFunctionEnd - $aclFunctionStart) `
     -Pattern 'SetOwner\('
 
+# Applying the ACL and assuming it landed is not the same as the service being able to use it. A
+# leftover ACE from an installation that ran under a different service identity survives into the
+# new install, and the API - not the installer - discovers it, at the first service start, with
+# "grants mutation rights to an untrusted principal" and a rollback that the comment at the catch
+# block records as capable of failing on its own. Reported from the field on 1.2.3. The installer
+# therefore asks the same question itself, and it has to ask it BEFORE the artifact is extracted,
+# so a give-up costs nothing.
+Assert-TextMatches -Name 'the installer verifies the data directory the way the service will' `
+    -Text $installerScript `
+    -Pattern '(?s)Set-DirectoryAclForService -Path \$DataPath[\s\S]{0,2000}Assert-ServiceDirectoryAclUsable -Path \$DataPath'
+Assert-TextMatches -Name 'that verification runs before the artifact is extracted' `
+    -Text $installerScript `
+    -Pattern '(?s)Assert-ServiceDirectoryAclUsable -Path \$DataPath[\s\S]{0,4000}Write-Step "Extracting artifact"'
+# Repair-then-recheck, not repair-and-hope: the second verdict is what decides.
+$assertFunctionStart = $installerScript.IndexOf('function Assert-ServiceDirectoryAclUsable')
+$assertFunctionEnd = $installerScript.IndexOf('function Assert-SafeInstallRoot', $assertFunctionStart)
+if ($assertFunctionStart -lt 0 -or $assertFunctionEnd -le $assertFunctionStart) {
+    throw 'Deployment template check failed: could not delimit Assert-ServiceDirectoryAclUsable in the installer.'
+}
+$assertFunction = $installerScript.Substring($assertFunctionStart, $assertFunctionEnd - $assertFunctionStart)
+Assert-TextMatches -Name 'the ACL repair is re-verified and gives up loudly' `
+    -Text $assertFunction `
+    -Pattern '(?s)Set-DirectoryAclForService[\s\S]{0,600}Test-ServiceDirectoryAclTrust[\s\S]{0,600}throw'
+# The trusted set has to match BuildTrustedSids() in RestrictedFileWriter.cs, or the installer
+# blesses a directory the service then rejects - the exact failure this whole check exists for.
+foreach ($trustedSid in @('S-1-5-18', 'S-1-5-32-544', 'S-1-3-0', 'S-1-3-4')) {
+    Assert-TextMatches -Name "the installer's trusted set carries $trustedSid, like the API's" `
+        -Text $installerScript `
+        -Pattern ([regex]::Escape($trustedSid))
+}
+
 # H-18. The install directory is the image path of a service running as LocalSystem or a gMSA, so
 # write access to it is code execution as that account. Only DataPath used to be hardened;
 # InstallPath was created with a plain New-Item -Force and inherited whatever the parent allowed -

docs/deployment-guide.md

@@ -365,6 +365,7 @@ Logs: `C:\ProgramData\NodePilot\logs\` (CMTrace-formatted). Firewall rule:
 | Boot log repeats `Waiting for the database to accept connections (n/120s)` | the database is not answering yet — a remote SQL Server still recovering, a DC not yet reachable for Kerberos, or a wrong host | let it finish; it proceeds either way and then reports the real connection error. Raise `Database:StartupWaitSeconds` (max 600) if the database routinely needs longer |
 | Event log 7000 *the service did not start due to a logon failure*, gMSA identity, only on boot | the service tried to log on before Netlogon could fetch the gMSA password from a DC | current builds set `depend= Netlogon` for gMSA services; on older ones `sc.exe config NodePilot depend= Netlogon` fixes it in place |
 | `admin-setup.token` → *Access to the path is denied* | intentional owner-only ACL for the service account | read via `robocopy /B` as shown in [Step 4](#step-4--first-login) instead of editing the ACL |
+| Install fails with `JWT signing-key file security validation failed: parent directory 'C:\ProgramData\NodePilot' grants mutation rights to an untrusted principal`, then rolls back | an ACE on the data directory belongs to a principal the service does not trust — in practice the service account of an **earlier** installation, because an ACE is only trusted while the service actually runs as that account. Not a version problem; the check has existed since 1.0.0 | installers from 2026-08-12 on verify the directory with the service's own rule after applying the ACL, repair it, and only then start the service — so this no longer reaches the service. On older builds: `icacls C:\ProgramData\NodePilot` names the stranger, `icacls C:\ProgramData\NodePilot /remove:g "<account>"` removes it. **`Jwt:RotateInsecureKeyFile=true` does not help here** — it replaces the key file, and the directory is what was rejected |
 | Every `runScript` step fails with `The term 'Write-Output' is not recognized` | artifact built with a pre-2026-08 `Build-Artifact.ps1` that did not stage the PowerShell built-in modules — `$PSHOME\Modules` is missing in the install dir | rebuild with the current build script; or hot-fix in place: `Copy-Item 'C:\Program Files\NodePilot\runtimes\win\lib\net10.0\Modules' 'C:\Program Files\NodePilot\Modules' -Recurse` and restart the service |
 | Installer prints `FAILED: ... Restoring the previous installation` | any error after mutation began rolls back to the previous state | fix the reported cause and re-run; note the diagnostics tail the shared log file, so lines from the *previous* installation can appear — check timestamps |
 | Browser shows *Not secure* / `Invoke-RestMethod` trust error | self-signed Kestrel certificate not trusted on the client | import it into `LocalMachine\Root` on that machine |

src/NodePilot.Api/Security/RestrictedFileWriter.cs

@@ -309,7 +309,7 @@ private static ExistingSecretFileSecurity ValidateWindowsDirectoryAcl(
         var owner = acl.GetOwner(typeof(SecurityIdentifier)) as SecurityIdentifier;
         if (owner is null || !trustedSids.Contains(owner))
             return ExistingSecretFileSecurity.Invalid(
-                $"parent directory '{directory.FullName}' has an untrusted owner");
+                $"parent directory '{directory.FullName}' has an untrusted owner ({Describe(owner)})");
 
         var dangerous = FileSystemRights.Delete
                         | FileSystemRights.DeleteSubdirectoriesAndFiles
@@ -339,12 +339,40 @@ private static ExistingSecretFileSecurity ValidateWindowsDirectoryAcl(
 
             if ((rule.FileSystemRights & dangerous) != 0)
                 return ExistingSecretFileSecurity.Invalid(
-                    $"parent directory '{directory.FullName}' grants mutation rights to an untrusted principal");
+                    $"parent directory '{directory.FullName}' grants mutation rights to an untrusted principal "
+                    + $"({Describe(sid)})");
         }
 
         return ExistingSecretFileSecurity.Valid();
     }
 
+    /// <summary>
+    /// Renders a SID for an operator: the account name where it still resolves, the raw SID
+    /// otherwise — and both when they differ, because the SID is what <c>icacls</c> needs.
+    ///
+    /// <para>Naming the principal is the whole point. "grants mutation rights to an untrusted
+    /// principal" is true and useless: the usual cause is a leftover ACE from an earlier
+    /// installation that ran under a different service identity, and without the name there is
+    /// nothing to search for. An orphaned SID — the account was deleted, which is exactly what a
+    /// decommissioned service account looks like — cannot be translated, and that failure is
+    /// itself the answer, so it must never turn into an exception on a boot path.</para>
+    /// </summary>
+    private static string Describe(SecurityIdentifier? sid)
+    {
+        if (sid is null) return "no owner could be read";
+        try
+        {
+            var name = ((NTAccount)sid.Translate(typeof(NTAccount))).Value;
+            return $"{name}, {sid.Value}";
+        }
+        catch (Exception ex) when (ex is IdentityNotMappedException or SystemException)
+        {
+            // Unresolvable SIDs are the interesting case, not an error: a deleted account still
+            // holds its ACE, and the raw SID is what removes it.
+            return $"{sid.Value}, unresolvable — the account no longer exists";
+        }
+    }
+
     private static HashSet<SecurityIdentifier> BuildTrustedSids()
     {
         var currentSid = WindowsIdentity.GetCurrent().User