Skip to content

Add the offline removal, BCD and nested-VM helpers (3 of 3) - #147

Merged
Edwin Bernal Microsoft (EdwinBernal1) merged 5 commits into
Azure:mainfrom
mvaferreira:rsl-helpers-repair
Sep 10, 2026
Merged

Add the offline removal, BCD and nested-VM helpers (3 of 3)#147
Edwin Bernal Microsoft (EdwinBernal1) merged 5 commits into
Azure:mainfrom
mvaferreira:rsl-helpers-repair

Conversation

@mvaferreira

@mvaferreira Marcus Ferreira (mvaferreira) commented Sep 8, 2026

Copy link
Copy Markdown

What this adds

The three helpers that perform destructive or boot-affecting repairs.

Helper Used by Purpose
Use-OfflineFileRemoval.ps1 2 Removes a set of files from an offline disk with a verified backup, a proof that only the intended files went, and automatic rollback when that proof fails
Get-OfflineBcdStore.ps1 2 Reads and modifies the Boot Configuration Data store of an offline installation
Use-NestedRepairVm.ps1 1 Drives the nested Hyper-V guest that az vm repair create --enable-nested provides

No map.json entries 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-BcdEdit built a command string and ran it through cmd.exe /c. Any value that reached it
from the offline store — a description field, a device path — was re-parsed by the shell.

It now takes [string[]]$Arguments and invokes & bcdedit.exe /store $StorePath @Arguments
directly. 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 of
which 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\config use case that made SYSTEM.LOG1, SOFTWARE.LOG2 and the transaction
.blf files removable — deleting a hive's log alongside the hive is how a recoverable image becomes
an 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/.TxR infix, so their base name
never equals a hive name and win-fix-transaction-logs is unaffected.

Rollback was also silently ineffective: it enumerated the backup with -ErrorAction SilentlyContinue, so a rollback that recovered nothing reported Restored = 0; Failed = 0 and read
as 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-OfflineHiveFile now parses through the shared in-memory offreg reader from #143, not a
scratch registry mount. The before/after Loads field remains a comparison result, not a promise
that Windows will boot. The subsequent review fixes below change removal, rollback and VM coordination.

Capacity, hashes and rollback

Get-OfflineFreeSpace now uses GetDiskFreeSpaceExW on the actual directory's volume, without
depending 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-OfflineRemovalResult did not contain a null-equals-null hash comparison. The gap was later
restoration: verified original hashes now survive in BackupRecord, and rollback checks both
the backup before copying and the destination afterward. A missing comparison hash is a failure.
Consumers must persist these records and pass them as -FileRecord on 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 records
without a Hash field explicitly warn that they verify the copy against the current backup,
not against a recorded historical original.

The nested guest

Start-NestedRepairVm passed the wrong variable to the attach call, so the disk it took offline was
not the disk it handed to the guest. Four early returns left host disks offline on failure. An
already-running guest short-circuited to Started = $true without checking whether the requested
disk 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 was
already 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 Stopped before rediscovery. An unconfirmed/failed forced stop is not success.

First consumer and end-to-end scope

The first planned upstream consumer of Invoke-OfflineRemovalPlan is
win-fix-transaction-logs
in wave 1. win-fix-pending-servicing also 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 the
original 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/restore acceptance matrix. That distinction is retained
explicitly; scenario acceptance belongs to the consuming PRs.

Conventions followed

  • Every repair is evidence-driven: nothing is written unless a specific fault is detected, so a
    healthy image produces no changes.
  • No security protection is disabled as a workaround.
  • Logging is ordered so the verdict is last, because az vm run-command keeps only the final 4096
    characters of the output stream.

Testing

  • PSScriptAnalyzer: 0 findings on all three files, at every severity.
  • 66/66 removal regressions and 87/87 nested lifecycle regressions pass on PS5.1 and PS7.
  • 57/57 consumer regressions pass on both engines: verified records survive JSON/merged
    manifests, failed or partial restores keep retry data and return error, and successful retries
    complete without misleading rollback claims.
  • Defect-level tests covering bcdedit argument injection, the hive base-name veto (including real
    transaction-log filenames that must stay removable), the nested-VM disk hand-off, and -WhatIf
    safety on every function that changes host state.
  • No map.json run-id resolves to a helper, so these files cannot be exercised through
    az vm repair run on their own. They are validated by the test suite here; the scenario scripts
    that consume them are validated end-to-end in their own pull requests. win-fix-bcd.ps1 is the
    one existing script whose call sites change (Invoke-BcdEdit -Command-Arguments, 3 sites),
    and it ships in its own pull request with that validation.
  • Earlier nested Hyper-V lab validation is recorded in the
    September 8 follow-up.
  • This round adds 93 native Hyper-V assertions with disposable Gen1/Gen2 guests and actual
    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.

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
Marcus Ferreira and others added 2 commits September 10, 2026 11:28
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


Comment thread src/windows/common/helpers/Get-OfflineBcdStore.ps1
Comment thread src/windows/common/helpers/Use-OfflineFileRemoval.ps1
Comment thread src/windows/common/helpers/Use-NestedRepairVm.ps1
Marcus Ferreira and others added 2 commits September 10, 2026 13:52
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
@mvaferreira

Copy link
Copy Markdown
Author

Updated the description and answered all three inline threads. The first planned upstream consumer of Invoke-OfflineRemovalPlan is win-fix-transaction-logs (wave 1); win-fix-pending-servicing also consumes it (wave 3). Their updated record/restore handling is in the fork, with 57/57 consumer regressions passing on PS5.1 and PS7.

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 az vm repair create/run --preview/restore scenario acceptance matrix; that remains explicitly scoped to the consuming PRs. Private lab/test tooling is not being added to this helper PR.

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.

@EdwinBernal1
Edwin Bernal Microsoft (EdwinBernal1) merged commit 3cdb744 into Azure:main Sep 10, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants