Add the offline removal, BCD and nested-VM helpers (3 of 3) - #147
Conversation
e887aeb to
63a6069
Compare
Three helpers that perform the destructive part of an offline repair, all of them gated on the offline root that Assert-OfflineTarget binds, so none can act on the rescue VM own Windows installation. Get-OfflineBcdStore.ps1 runs bcdedit.exe against the offline store directly with an argument array, instead of building a command string and handing it to cmd.exe /c. A store path or a boot entry identifier could otherwise carry a shell metacharacter into a SYSTEM context. It also checks the bcdedit exit code, so an empty boot inventory is no longer indistinguishable from a failed enumeration. Use-OfflineFileRemoval.ps1 removes files from the attached disk with a backup and a rollback that is verified rather than assumed. Its documented use case is System32\config, so it vetoes a removal whose base name matches a registry hive - SYSTEM.LOG1 and the transaction-log side files were previously removable because the allow list only ever inspected the extension. Rollback now restores through the protected-file copy path and replays the recorded security descriptor, and reports its own failure as a distinct fatal outcome. Use-NestedRepairVm.ps1 boots the offline installation as a nested Hyper-V guest for repairs only the running OS can perform. Every disk it takes offline is restored in a finally block; a guest is never reported as started until its disk is confirmed attached; and a failed firmware change is a failure rather than a warning that leaves a Gen2 guest to PXE boot into a ten minute timeout. All three files report zero PSScriptAnalyzer findings at every severity. Depends on the helpers in the foundation and registry pull requests. No map.json run-id resolves to any helper, so a helper whose dependency has not merged yet is unreachable rather than broken. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 518d6d32-f227-4d1a-ae36-1e0be233b09b
63a6069 to
3abd65b
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Edwin Bernal Microsoft (EdwinBernal1)
left a comment
There was a problem hiding this comment.
Info — scope and testing
Same structural point as #143: nine helpers, no consuming script, no tests, and tests/ is still a placeholder. For read-mostly helpers that is a manageable risk. For file removal and BCD editing on a customer's boot volume it is a larger one — these are the helpers where a defect is unrecoverable for the customer, and they are landing with no in-repo exercise at all.
Not asking for a test harness in this PR. But it would help reviewers a lot if the description said which scenario will consume Invoke-OfflineRemovalPlan (Use-OfflineFileRemoval.ps1 L785) first, and whether it has been run end-to-end against a real attached disk (Gen1 and Gen2, since Get-BcdStorePath at Get-OfflineBcdStore.ps1 L139 branches on generation).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cf64bab1-6099-4e7e-aef4-57ffea10ce6b
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cf64bab1-6099-4e7e-aef4-57ffea10ce6b
|
Updated the description and answered all three inline threads. The first planned upstream consumer of For the generation question: this round ran 51 native assertions on real attached MBR/Gen1 and GPT/Gen2 volumes, including FAT32 EFI, covering capacity, protected removal/restore, tamper refusal, exact ACL/attribute replay, automatic rollback and BCD create/edit/read-back/restore through both generation-specific paths. Another 22 assertions ran both actual scenario revert entry points in four processes against synthetic attached-volume fixtures. The native Hyper-V lifecycle run passed 93 assertions. These are real native helper/entry-point operations, but the hives/BCD stores were synthetic and the nested guests had no Windows OS. They are not a fresh full Gen1/Gen2 The remaining work stays in #146 and #147 only. #143 is already merged and unchanged; #146 carries the shared support used here; #148 is closed. Both remaining merge orders are clean, and scenario PRs wait for both. Threads are left open for reviewer confirmation. |
3cdb744
into
Azure:main
What this adds
The three helpers that perform destructive or boot-affecting repairs.
Use-OfflineFileRemoval.ps1Get-OfflineBcdStore.ps1Use-NestedRepairVm.ps1az vm repair create --enable-nestedprovidesNo
map.jsonentries are added. Helpers are not run-ids, so nothing new is invocable yet.Part of the #143 split
This came from the three-way split of #143. #143 is now merged and stays unchanged.
#146 carries the additional shared state/discovery/coordination support used here. The briefly
opened #148 split is closed; all remaining fixes stay in #146 and #147, which merge cleanly
in either order. Consuming scenario PRs must wait for both.
Command injection in the BCD helper
Invoke-BcdEditbuilt a command string and ran it throughcmd.exe /c. Any value that reached itfrom the offline store — a description field, a device path — was re-parsed by the shell.
It now takes
[string[]]$Argumentsand invokes& bcdedit.exe /store $StorePath @Argumentsdirectly. No shell, no re-parsing. This was verified rather than assumed: under the old shape a
payload embedded in an argument executed; under the new shape the identical payload arrives as a
single literal argument.
The signature change touches exactly one scenario,
win-fix-bcd.ps1, at three call sites, all ofwhich already build proper argument arrays.
Removing files from an offline disk
The review found that the "two-layer" allow-list was extension-only on both layers. In the
documented
System32\configuse case that madeSYSTEM.LOG1,SOFTWARE.LOG2and the transaction.blffiles removable — deleting a hive's log alongside the hive is how a recoverable image becomesan unbootable one.
There is now a base-name veto: a file whose name without its extension matches a hive name is
refused, ordinal and case-insensitive. This was checked against the real scenarios rather than
assumed safe — genuine transaction files carry a GUID and a
.TM/.TxRinfix, so their base namenever equals a hive name and
win-fix-transaction-logsis unaffected.Rollback was also silently ineffective: it enumerated the backup with
-ErrorAction SilentlyContinue, so a rollback that recovered nothing reportedRestored = 0; Failed = 0and readas success. It now fails loudly when the recovered count does not match, routes restores through the
protected-file path so an ACL cannot block them, and reports a distinct fatal outcome when rollback
itself fails.
The initial September 10 dependency follow-up updated hive-comparison documentation to match #146:
Test-OfflineHiveFilenow parses through the shared in-memory offreg reader from #143, not ascratch registry mount. The before/after
Loadsfield remains a comparison result, not a promisethat Windows will boot. The subsequent review fixes below change removal, rollback and VM coordination.
Capacity, hashes and rollback
Get-OfflineFreeSpacenow usesGetDiskFreeSpaceExWon the actual directory's volume, withoutdepending on the PowerShell drive list. Real volume-GUID and boot-volume paths are exercised.
Unknown, invalid or insufficient capacity refuses removal before any backup copy or deletion;
zero bytes remains a valid capacity result, not an unknown one.
The existing backup comparison already rejected missing hashes, and
Test-OfflineRemovalResultdid not contain a null-equals-null hash comparison. The gap was laterrestoration: verified original hashes now survive in
BackupRecord, and rollback checks boththe backup before copying and the destination afterward. A missing comparison hash is a failure.
Consumers must persist these records and pass them as
-FileRecordon later revert.Native testing also found that applying an ACL after attributes set the NTFS Archive bit.
Rollback now restores security first, then reapplies and reads back exact attributes. Failed
metadata replay is a failed restore, not a warning alongside
Succeeded = true. Legacy recordswithout a Hash field explicitly warn that they verify the copy against the current backup,
not against a recorded historical original.
The nested guest
Start-NestedRepairVmpassed the wrong variable to the attach call, so the disk it took offline wasnot the disk it handed to the guest. Four early returns left host disks offline on failure. An
already-running guest short-circuited to
Started = $truewithout checking whether the requesteddisk was attached at all.
All three are fixed, and a fourth case that the review did not raise is now refused at both ends: if
every requested disk is skipped, the guest would previously start with no disk, report success,
and leave the caller waiting out the full heartbeat timeout.
Disks are restored in a
finally, and only the disks this call took offline — a disk that wasalready offline at entry was offline for a reason this helper does not own.
The new coordination fix marks started/adopted guests in Notes before reporting success.
The shared support in #146 recognizes that marker across processes and refuses discovery while
the guest is active. It also protects unrelated guests and serializes lifecycle transitions.
The temporary-user caller no longer invokes the broad stopper after its explicit shutdown;
it confirms
Stoppedbefore rediscovery. An unconfirmed/failed forced stop is not success.First consumer and end-to-end scope
The first planned upstream consumer of
Invoke-OfflineRemovalPlaniswin-fix-transaction-logsin wave 1.
win-fix-pending-servicingalso consumes it, in wave 3.This review follow-up exercised real attached MBR/Gen1 and GPT/Gen2 VHD volumes, including a
FAT32 EFI partition: native capacity checks, protected file deletion/restoration, same-size
backup tampering, original ACL/attribute restoration, automatic rollback, and BCD
create/edit/read-back/restore through each generation-specific path. 51 assertions passed.
The originals were preserved and the fixtures removed.
An additional 22 native assertions exercised the real manifest writers and both scenario
revert entry points in four separate PowerShell processes. Each refused a same-size modified
backup with
STATUS_ERROR, retained its manifest, and completed a verified retry after theoriginal backup was restored. Pending servicing also kept the pending.xml rename retryable.
These used synthetic files/hives on a disposable attached VHD, not a complete Windows repair.
These are full helper-level operations on real volumes, not bootable customer-image fixtures.
They do not establish a new full Gen1/Gen2 Windows scenario
az vm repair create/run --preview/restoreacceptance matrix. That distinction is retainedexplicitly; scenario acceptance belongs to the consuming PRs.
Conventions followed
healthy image produces no changes.
az vm run-commandkeeps only the final 4096characters of the output stream.
Testing
manifests, failed or partial restores keep retry data and return error, and successful retries
complete without misleading rollback claims.
transaction-log filenames that must stay removable), the nested-VM disk hand-off, and
-WhatIfsafety on every function that changes host state.
map.jsonrun-id resolves to a helper, so these files cannot be exercised throughaz vm repair runon their own. They are validated by the test suite here; the scenario scriptsthat consume them are validated end-to-end in their own pull requests.
win-fix-bcd.ps1is theone existing script whose call sites change (
Invoke-BcdEdit -Command→-Arguments, 3 sites),and it ships in its own pull request with that validation.
September 8 follow-up.
passthrough-disk hand-offs. A separate PowerShell process cannot interrupt a helper-owned guest.
Five existing guests and seven original disks were preserved. These guests have no Windows OS;
their role is lifecycle/disk coordination, not boot or payload validation.