Registry restore deletes the key subtree before rewriting, with no rollback on partial failure (data loss)

[HetCreep]

Summary: The restore-from-backup path deletes the live registry subtree BEFORE rewriting it, with no rollback. If the rewrite throws partway, the subtree is permanently deleted or left half-populated -- in the one operation a user runs specifically to RECOVER state.

Where: Scripts/Features/RestoreRegistryApplyState.ps1, function Restore-RegistryKeySnapshot. When the snapshot has subkeys (forceFullTree), Remove-RegistrySubKeyTreeIfExists (DeleteSubKeyTree) runs BEFORE CreateSubKey + the value-restore loop + the recursive subkey restore. None of that recreation is wrapped in a try/catch that re-creates the deleted data, and the caller Restore-RegistryBackupState loops over roots with no try/catch either. Reachable from the GUI Restore Backup flow (Show-RestoreBackupWindow).

Triggers (neither needs malice):

1. A corrupted or hand-edited backup .json whose Data does not fit its Kind, e.g. Kind=DWord with Data=4294967296. Load-time validation (Test-RegistryValueKindNameSupported) checks the Kind NAME parses but never checks Data against Kind, so it passes validation and then throws an OverflowException at the [uint32]Data cast in Convert-RegistryValueDataFromBackup -- AFTER the delete.
2. A runtime SetValue failure (AccessDenied / IO / permission drift between backup and restore) mid-loop.

In both cases the live subtree is already gone; the user sees a 'restore failed' message but their registry data is now lost or half-restored (including any live values outside this backup's captured scope).

Impact: Permanent registry data loss in a restore feature. Edge-triggered (corrupted backup, or permission/state drift) but severe when it fires.

Proposed fix (design, not a blind PR -- see disclosure): There is no atomic 'replace key subtree' primitive in the .NET Microsoft.Win32.RegistryKey API (RegRenameKey is not exposed; PowerShell registry transactions would require rewriting the path onto provider cmdlets), so no design is fully airtight -- this is a mitigation-tier decision for the maintainer. The least-breakable shape:

• Build the restored subtree under a temporary sibling key first and confirm it fully populates WITHOUT error (this catches the corrupted-Data case BEFORE touching the live key), AND
• reg export the live subtree to a temp file before the delete, as a rollback target, AND
• wrap the delete + swap so any throw re-imports the export.
• Add a Data-vs-Kind validation at load time so case 1 is rejected before any restore begins.

Critical caveat (related gap): any rollback built on the PowerShell registry-writer fallback Invoke-RegistryOperationsFromRegFile (ApplyRegistryRegFile.ps1) inherits a silent-partial-failure behavior: that helper only throws when ALL operations are access-denied; on PARTIAL denial it returns normally and the caller prints success without incrementing the failure counter. On a normal elevated consumer machine this is not reachable (all shipped .reg keys are admin-writable and the tool runs elevated), but a rollback or restore that relies on it could silently half-succeed on a GPO-hardened/MDM machine. Worth gating that success message on the actual access-denied count.

Testing disclosure: I could not exercise a destructive registry restore on my machine; this is code-level analysis, cross-checked against Microsoft's documented DeleteSubKeyTree / SetValue / RegRenameKey behavior (the registry has no atomic key-replace, which is why a design decision is needed). Distinct from PR #613, which adds rollback on the APPLY path, not this RESTORE path.

Found via a multi-perspective code audit.

[HetCreep]

Update: #704 addresses the corrupted/hand-edited-backup trigger (case 1 above -- Data/Kind mismatch) by rejecting it at validation time, before Restore-RegistryBackupState ever runs. The runtime SetValue failure trigger (case 2 -- AccessDenied/IO/permission drift mid-restore) is still open; per the issue body, closing that fully needs a real mitigation design (temp-key staging + reg export rollback, or similar), since there's no atomic subtree-replace primitive in the RegistryKey API to lean on. Also relevant: #613's rollback-on-failure path calls into this same restore function, so it inherits this same residual exposure (flagged by an independent reviewer on that PR).

[Raphire]

This should be fixed with #710

[HetCreep]

Agreed -- verified against current master.

#710's Test-RegistryKeySnapshotCanBeRestored dry-runs the entire snapshot tree (every value's Kind/Data conversion + subkey-path hierarchy) before any live key is deleted, which closes every trigger that can be detected up front -- that was the practically-reachable failure class here.

The only residual is a mid-write runtime failure (SetValue access-denied / IO during the rewrite itself), which by nature can't be pre-validated -- but on a tool-generated backup under an elevated run that window is narrow enough that I think this is reasonable to close. Thanks for the thorough fix.