Skip to content

Add win-fix-firewall-service - #149

Merged
Edwin Bernal Microsoft (EdwinBernal1) merged 6 commits into
Azure:mainfrom
mvaferreira:rsl-win-fix-firewall-service
Sep 17, 2026
Merged

Edwin Bernal Microsoft (EdwinBernal1) merged 6 commits into
Azure:mainfrom
mvaferreira:rsl-win-fix-firewall-service

Conversation

@mvaferreira

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

Copy link
Copy Markdown

What this adds

win-fix-firewall-service - one scenario script and its own map.json entry.

Repairs the Windows Defender Firewall service on an offline disk, so a VM that boots but drops every inbound connection - RDP included - can be reached again.

The catalog entry a support engineer reads when choosing it:

Restores the Windows Defender Firewall service on a VM that boots, reports Ready and listens on 3389, yet answers nothing from the virtual network - ICMP included - because mpssvc is not running and the boot-time filters Windows installs before it starts are still blocking everything. Repairs two causes read from the offline SYSTEM hive: a DebugedLoopbackApps value under Services\mpssvc\Parameters\AppCs whose registry type is not the REG_SZ that CheckNetIsolation writes, and MpsSvc or the Base Filtering Engine under it left Disabled. Measured on a live build 20348 VM one reboot at a time, a value of any other type stopped the service on every boot, an empty REG_MULTI_SZ as reliably as one holding 700 SIDs. A correctly typed REG_SZ value is always preserved: these offline checks do not validate its content, so a suspected content fault is reported for manual investigation rather than repaired, and a clean result does not prove the firewall service starts. A healthy VM produces no findings and nothing is written. Only Start=4 is treated as evidence, so a deliberately demand-started service is never corrected. Pass detectOnly=true to report only. NOTE: use option --run-on-repair.

How it works

Runs against the broken OS disk attached to a rescue VM by "az vm repair create".

This is the VM that looks alive and answers nothing. It boots, the guest agent reports Ready, Remote Desktop Services is running and 3389 is listening - and from another machine on the same virtual network every port is a timeout, ICMP included. The listener is fine. Nothing reaches it, because the Windows Defender Firewall service is not running and the boot-time filters that Windows installs before it starts are still in force, blocking everything.

Parameters

Parameter Effect
detectOnly "true" to report what is wrong with the firewall service and change nothing at all. Defaults to "false".
windowsDrive The drive letter of the attached offline Windows installation. Detected automatically when not supplied.

Conventions followed

  • Dot-sources .\src\windows\common\setup\init.ps1 and returns $STATUS_SUCCESS or $STATUS_ERROR.
  • Logging goes through the logger functions only; no Write-Host.
  • The detect summary is printed after the per-finding list, because az vm run-command keeps only the last 4096 characters of the output stream, so a summary printed first is the first thing a long run loses.
  • Evidence-driven: findings are gathered first and only what the evidence names is changed, so a healthy image produces no writes.

Testing

Historical product-path acceptance (September 1): four
az vm repair run --run-on-repair --preview cycles exercised the wrong registry type and
disabled MpsSvc/BFE cases, broken-disk detect-only behavior, healthy detect/repair zero-write
runs, and restored-VM verification (7/7 checks). In-VNet TCP probes measured RDP reachable
before the break, unreachable while the firewall service failed, and reachable after repair.
Other blocked ports stayed blocked: the firewall was repaired, not switched off.

September 10 integration: writing callers now use strict control-set selection from #146.
The shared registry suite passed 140/140 on PS5.1 and PS7; 33 native synthetic-hive assertions
covered selector validation, access failures and lifecycle cleanup. These do not replace or
claim a fresh full Gen1/Gen2 scenario acceptance matrix.

Series

First wave of four independent scenario PRs. The shared helpers in #143, #146 and #147 are
already merged. This PR adds no helper files and changes no existing scenario; its only existing
file change is appending this run-id to map.json, preserving every upstream entry.

Marcus Ferreira and others added 5 commits September 8, 2026 14:19
Repairs the Windows Defender Firewall service on an offline disk, so a VM that boots but drops every inbound connection - RDP included - can be reached again.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: cf64bab1-6099-4e7e-aef4-57ffea10ce6b
@mvaferreira

Copy link
Copy Markdown
Author

Automated review using the supplied PR Review Agent

Reviewed head: 3dc0afab305e7b254ec24c3048678ff3de0845f4
Agent recommendation: request changes
Finding-table counts: 0 Critical / 5 Warning / 6 Info

This is the supplied agent's static analysis, not a maintainer decision or a fresh repair/boot test. Findings have not been independently reproduced. No source or Azure resources were changed during review. The original report is retained locally; only leading process narration and local prompt-path provenance were normalized for posting. Finding text is unchanged.

Full automated review report

PR Review: #149 — Add win-fix-firewall-service

Generated: 2026-09-10
Target: Azure/repair-script-library → main
PR: #149
Head: 3dc0afab305e7b254ec24c3048678ff3de0845f4 (mvaferreira/repair-script-library)
Base / merge-base: 3cdb744e1592c1aa0e6c5840bea43e5b8a1911ad
Prompt SHA256: 5451BA5C325F61E34FE63B9AF99322A17D70E55210CAF23F5CE27E0F839D8F70

Changed files reviewed (pinned snapshots only): map.json, src/windows/win-fix-firewall-service.ps1 (new, 653 lines). Helper contracts read for caller obligations: common/setup/init.ps1, common/helpers/Logger.ps1, OfflineRepairCommon.ps1, Get-OfflineWindowsDisk.ps1, Use-OfflineRegistryHive.ps1, Use-OfflineProtectedResource.ps1, Use-OfflinePrivilegedRegistry.ps1, plus doc/adding_new_scripts.md. This PR adds no helper files and changes no existing script.

Findings

Critical

None found.

Warning

File Line/Context Issue Recommendation
src/windows/win-fix-firewall-service.ps1 392, 402, 427, 464 A correctly-typed REG_SZ loopback value is deleted as collateral when the disabled-service fault is the real cause. For a REG_SZ value the sole trigger is $serviceFailing = ($Evidence.Count -gt 0) (392). When BFE carries Start=4 — the very fault flagged at 427 — SCM cannot start MpsSvc and logs 7000/7001 records naming the firewall service, which the ID set at 176 accepts. $serviceFailing becomes true, and Repair-Finding removes the customer's DebugedLoopbackApps at 464 while the finding text at 402 asserts "mpssvc is not accepting the value even though its type is right" — an inference the evidence does not support. The value is a healthy one by the script's own definition (167-170). Do not treat a correctly-typed value as faulty when a ServiceDisabled finding already explains the start failure. Narrow the corroborating evidence to 7024 (service-specific error), which is the shape the crash loop actually produces, rather than all six IDs.
src/windows/win-fix-firewall-service.ps1 247-251, 267, 392 Event evidence is not time-bounded. The Get-WinEvent filter hashtable carries Path, ProviderName and Id but no StartTime. $result.LastTime is captured at 267 and used only for a log line at 527 — never in Get-AllFinding. Any retained record from an earlier, already-resolved firewall incident therefore licenses deletion of a healthy REG_SZ value on a machine whose firewall currently starts. This contradicts the PR/map.json claim that "a healthy VM therefore produces no findings and nothing is written". Bound the query to the most recent boot session, or require LastTime to postdate the last successful firewall start / be recent relative to the hive's last write.
src/windows/win-fix-firewall-service.ps1 495 (try), 649-653 (catch); no finally No finally block: buffered helper log is not flushed on the failure path, and temporary drive letters are never released. Three helper contracts require it: OfflineRepairCommon.ps1 header ("always flush in the script's finally block"); Get-OfflineWindowsDisk .OUTPUTS ("pass each to Remove-OfflineDriveLetter, or call Clear-OfflineDriveLetter, in the caller's finally"); Clear-OfflineDriveLetter .DESCRIPTION ("A caller runs this in a finally"). Concrete consequence: Repair-Finding records each removed SID via Add-OfflineRepairLog (454-459), which only buffers. If Invoke-WithHive (590) throws on an unconfirmed unload — its documented behaviour, raised after the block has already deleted the value — control reaches 649, which logs only the exception and stack trace, and the buffer is discarded. The header guarantee at 87-88 ("Everything removed is written to the log first … so an exemption somebody actually wanted can be put back") is then not met for the one case where it matters. Wrap the body in try/finally; in the finally call Write-OfflineRepairLog and Clear-OfflineDriveLetter. Note: this script is the repo's first consumer of these helpers, so the basis is the helper documentation, not sibling precedent.
src/windows/win-fix-firewall-service.ps1 78-81, 181-183, 223-225 Header documents a size-based trigger that is not implemented. The .DESCRIPTION at 78-81 states the value's size "is compared against the smallest size measured to break a live VM. Either is enough"; the comment at 181-183 justifies the language-fragile message match with "that is why size alone is also sufficient evidence"; Get-FirewallCrashEvidence .DESCRIPTION (223-225) says "the caller falls back to the measured size". Get-AllFinding (390-394) performs no size comparison at all, and line 44 states the opposite ("No such threshold exists"). An engineer reading the contract believes a second, language-independent trigger exists. Delete the three stale passages, or implement the corroboration they describe. Do not leave the file self-contradictory between 44 and 78-81.
src/windows/win-fix-firewall-service.ps1 392, 578 False negative for the documented fatal REG_SZ shape. The header measurement table records "REG_SZ, 700 joined SIDs / 118,224 bytes → dead". That shape sets $wrongType = $false, so it is caught only via the event log. When the log has wrapped — which the header itself predicts at 126-128 — is absent, or the service display name is non-English (acknowledged at 181-183), the script reports "No firewall service fault was found" (578) and returns $STATUS_SUCCESS on a machine exhibiting the fault. This is the false-negative counterpart of the two rows above and the gap the removed size fallback was meant to cover. Restore a corroborating discriminator for REG_SZ that does not depend on the event log alone (e.g. content parseability, or the measured minimum breaking size), or document the limitation in .NOTES and the map.json description.

Info

File Line/Context Suggestion
src/windows/win-fix-firewall-service.ps1 238, 241 The only filesystem path built here uses Join-Path/Test-Path, where the library supplies Join-OfflinePath/Test-OfflinePath precisely because "Join-Path and Test-Path throw DriveNotFoundException when a path refers to a drive letter that is not a live PowerShell drive" (OfflineRepairCommon.ps1 header). The registry uses at 290 and 350 are safe because HKLM: is always live. I could not confirm a throw for a freshly diskpart-assigned Windows volume; if it did occur the exception is caught at 649 and the run returns $STATUS_ERROR rather than producing a wrong repair. Prefer the offline-safe variants for consistency.
src/windows/win-fix-firewall-service.ps1 509, 517, 591, 596, 620 The blocks passed to Invoke-WithHive reach $isDetectOnly, $evidence, $context and $repairable by dynamic scope. I verified this resolves correctly — the blocks run via & $ScriptBlock inside Invoke-WithHive, whose scope chain reaches script scope, and none of those names are shadowed by that function's locals. It is nonetheless fragile: were the helper to invoke the block in a new runspace or via InvokeReturnAsIs(), $isDetectOnly would become $null and silently flip -Strict:(-not $isDetectOnly) at 509, and $repairable would be empty at 596 while the run reported success. Consider .GetNewClosure() or explicit $script:-scoped names.
src/windows/win-fix-firewall-service.ps1 586, 588 Backup-OfflineHiveFile leaves …\System32\Config\SYSTEM.bak-<timestamp> on the customer's disk permanently, one per repair run. Correctly scoped (only when findings exist and not detect-only) and logged, but nothing removes it and the closing guidance at 644 does not tell the operator it survives az vm repair restore.
src/windows/win-fix-firewall-service.ps1 254-258, 533 Readable conflates "the log could not be read" with "no matching records", as the comment at 255 concedes. The operator-facing message at 533 then reads as a failure on a machine that is simply clean. Decision logic is unaffected — Readable is never consulted by Get-AllFinding.
src/windows/win-fix-firewall-service.ps1 54-55 Two stray blank lines interrupt the # banner header mid-.DESCRIPTION. Harmless to parsing (this is a banner block, not <# #> comment-based help), but inconsistent with the rest of the file.
map.json new entry The description is ~1,500 characters where every other entry is one or two sentences. az vm repair list-scripts renders these; consider trimming to the scenario, the detectOnly parameter and the --run-on-repair note, leaving the measurement detail in the script header.

Operational Risk Assessment

Factor Rating Notes
Scope Low One new scenario script plus one appended map.json entry. No helper file added or changed, no existing scenario touched, so no blast radius beyond this run-id.
Destructive ops Medium Deletes one named registry value (DebugedLoopbackApps, line 464) and rewrites Start on MpsSvc/BFE (476-477). Both are bounded, gated by Assert-OfflineTarget before any privilege is enabled, and confined to the mounted offline hive. Risk is not the breadth of the write but the sufficiency of the trigger — see Warning rows 1 and 2.
Rollback possible Partial The SYSTEM hive is copied before any write (586) and removed SIDs are enumerated to the log (454-459) for replay via CheckNetIsolation LoopbackExempt. Both mitigations are weakened by the missing finally: on the throw path the SID list is discarded unflushed.
Testing documented Yes PR records four az vm repair run --run-on-repair --preview cycles (Sept 1) covering wrong-type, disabled-service, detect-only, healthy zero-write and restored-VM verification, plus a Sept 10 shared registry suite (140/140 on PS5.1 and PS7; 33 native synthetic-hive assertions). This is recorded history, not testing performed in this review. I ran no repair script, no boot test and no Azure operation.
Gen compatibility Unknown Measurements cited are builds 20348 and 26200 only; the PR explicitly disclaims a fresh Gen1/Gen2 acceptance matrix. The script is generation-agnostic in substance — it touches only the SYSTEM hive and System.evtx, with no BCD or boot-partition work — and Get-OfflineWindowsDisk handles both partition styles, so exposure is low but unverified for this change.

Overall Risk: Medium 🟡

Validation

  • map.json valid: pass — packet reports ConvertFrom-Json success; the diff is a single 5-line insertion between the win-enable-nested-hyperv and win-toggle-safe-mode entries, with no other hunk in the file, so every upstream entry is preserved as the PR claims.
  • map.json paths exist: passsrc/windows/win-fix-firewall-service.ps1 is present in the pinned head snapshot.
  • Duplicate IDs: nonewin-fix-firewall-service does not collide with any existing id; naming follows the win-* convention.
  • PowerShell parse errors: none reported by the launcher's deterministic check.
  • Init sourcing: pass. .\src\windows\common\setup\init.ps1 at line 144, ahead of the five helper dot-sources (145-149).
  • Status return: pass — every exit path returns $STATUS_SUCCESS (574, 580, 647) or $STATUS_ERROR (636, 652).
  • Logger functions: pass — no Write-Host and no bare Write-Output; all operator output goes through Log-Output/Log-Info/Log-Warning/Log-Error, and helper messages are buffered via Add-OfflineRepairLog and flushed with Write-OfflineRepairLog as the helper contract requires (except on the exception path — see Warning row 3).
  • Credentials / injection / unbounded deletion: pass — no secrets, no hardcoded passwords, no outbound network calls, no Remove-Item -Recurse on the offline image, and no parameter interpolated into a shell or diskpart script. detectOnly is ValidateSet-constrained; windowsDrive is consumed only by Get-OfflineWindowsDisk, which validates it.
  • Registry hive pairing: pass — every mount goes through Invoke-WithHive, which unloads in its own finally and throws on an unconfirmed unload; Backup-OfflineHiveFile (586) is correctly called between mounts, while the hive is unloaded, as that helper requires.
  • Detection/repair path binding: pass — the repair block re-verifies Select\Current against the detection-time control set before writing (591-593), and repair and verification both use -Strict selection.

Checks marked "packet"/"launcher" above were executed by the review launcher, not by me. Everything else in this report is assessed from the pinned head and base source.

Recommendation

request changes

The design is sound and unusually well evidenced, the offline-target gating and privileged-read approach are correct, and the healthy-machine zero-write property holds for every case except the one identified. Three items should be resolved before merge: the REG_SZ deletion trigger must not fire on a firewall start failure that a ServiceDisabled finding already explains (Warning 1); the event evidence must be time-bounded (Warning 2); and the script needs a finally that flushes the helper log and releases the drive letters, without which the removed-SID record — the stated recovery path — is lost on exactly the failure it exists for (Warning 3). Warnings 4 and 5 should be resolved together, either by implementing the corroboration the header describes or by removing the claim.

This is an automated review produced by the rsl-pr-review agent from the pinned snapshots. It is not a human maintainer approval. No repository code was executed, no Azure operation was performed, no file was modified, and nothing was posted to GitHub.

…ller contract

Addresses the PR149 review findings.

  - A correctly typed REG_SZ DebugedLoopbackApps value is now always
    preserved. Only a value of the wrong registry type is repairable. The
    offline System log is read for context and can never authorise a
    deletion, which removes the failure mode where an old crash record
    deleted a value that was healthy at the time of the run.
  - NoMatchingEventsFound is told apart from a genuine read failure by
    error identity, so an unreadable log is reported as unreadable instead
    of as "no crashes found".
  - The main flow follows the helper caller contract: a labelled single-pass
    loop, a top-level finally that releases discovery-owned drive letters and
    flushes the buffered helper log, and the status returned after cleanup.
  - Offline paths use Join-OfflinePath/Test-OfflinePath so a stale drive
    letter cannot throw.
  - The header, the closing guidance and the map.json description describe
    what the script actually does, including that the SYSTEM hive .bak file
    survives "az vm repair restore".

Validated with the local mocked harness (45 checks, no registry, disk or hive
access) and the log-ordering audit. The Azure create/run/restore acceptance
cycle was performed previously against the pre-review script.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mvaferreira

Copy link
Copy Markdown
Author

Review findings implemented

Publication head: dbe0846a95a27bf49683b058e23798abdd8b34af

The earlier automated report remains a static review of
3dc0afab305e7b254ec24c3048678ff3de0845f4. This follow-up records what was implemented in
response to it; it is not a relabelled or newly generated agent review.

Finding Disposition
W1: correctly-typed REG_SZ deleted as collateral Fixed by narrowing the repair. A correctly typed REG_SZ DebugedLoopbackApps value is now always preserved. Only a value of the wrong registry type is repairable, so the disabled-service fault is repaired on its own without touching a value the measurements show is healthy.
W2: event evidence is not time-bounded Resolved by removing the trigger rather than bounding it. The offline System log is read for context only and can never authorise a deletion, which removes the failure mode where an old crash record deleted a value that was healthy at the time of the run.
W3: no finally, so buffered logs and drive letters leak on the failure path The main flow is a labelled single-pass loop with a top-level finally that releases discovery-owned drive letters and flushes the buffered helper log through the script logger. The status is returned after that cleanup, so it stays at the end of the output stream.
W4: header documents a size-based trigger that is not implemented Header rewritten. There is no size or entry-count fallback and the text no longer claims one; a wrong registry type is the only conclusive signal.
W5: false negative for the documented fatal REG_SZ shape Now stated explicitly instead of silently accepted: a clean result does not prove the firewall is healthy. A correctly typed value is logged as preserved, and when failure evidence exists (or evidence could not be read) that is raised as a warning for manual investigation rather than acted on.
I1: Join-Path/Test-Path on an offline path Now Join-OfflinePath/Test-OfflinePath, consistent with the rest of the script.
I2: dynamic scope in Invoke-WithHive blocks Not changed. The reviewer verified the variables resolve correctly; changing the scoping would carry more risk than it removes.
I3: SYSTEM.bak-<timestamp> left on the customer's disk Now disclosed in the backup log line and in the closing guidance, including that the backup survives az vm repair restore and is available on the recovered VM.
I4: Readable conflates "unreadable" with "no records" NoMatchingEventsFound is told apart from a genuine read failure by error identity, not by an English message. An unreadable log is reported as unreadable and never as "no crashes found".
I5: stray blank lines in the banner header Removed.
I6: map.json description length Rewritten and shortened, and it no longer asserts the removed event-based deletion rule. Accuracy was kept ahead of brevity: several sibling entries are of comparable length.

Testing scope, stated plainly: these changes were validated with the local mocked harness
for this PR (45 checks covering every finding above, executed with no registry, disk or hive
access), a parse check, the mandatory-parameter audit and the log-ordering audit. The
az vm repair create / run --preview / restore product-path acceptance described in the PR
description was performed earlier against the pre-remediation script and was not repeated
for this head. The changes are confined to evidence classification, header text and the
script's exit/cleanup path; the repair action itself (Start=3 on MpsSvc/BFE, and deleting
a wrong-typed value) is unchanged from the version that was accepted on the product path.

Original review provenance
  • Supplied prompt SHA256: 5451BA5C325F61E34FE63B9AF99322A17D70E55210CAF23F5CE27E0F839D8F70.
  • Original finding-table counts: 0 Critical / 5 Warning / 6 Info.
  • Preserved original report.
  • Its recommendation belongs to the old reviewed head; no maintainer approval is implied.

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.

Reviewed the current head after the prior findings were addressed. Automatic repair is now limited to a wrong-type DebugedLoopbackApps value and explicitly disabled MpsSvc/BFE services; correctly typed REG_SZ content is preserved. The script backs up the hive before writes, revalidates the active control set, verifies the resulting state, and fails on incomplete cleanup. The catalog entry is consistent. I found no new blocking issue.

Residual validation gap: only the CLA check is reported by GitHub. This review did not independently rerun the live Server 2022 firewall-service scenarios described in the PR.

@glimoli

Gabriela Limoli (glimoli) commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

VMRepair Script Test Report: win-fix-firewall-service.ps1

Tip

✅ APPROVE — Overall Score 94/100 (Grade A)

All script-level gates PASS. Fault repaired end-to-end on both offline OS disks; every convention audit item satisfied; map.json diff is a single 5-line append. Non-blocking observations only.

Summary

Overall Score: 94/100 (Grade: A) — APPROVE

Category Score Notes
Functional Correctness 20/20 Both fault classes (wrong REG type + Start=4) repaired end-to-end on live rescue-VM disks; re-verification pass produced zero findings
Code Quality 20/20 PSScriptAnalyzer clean; all 9 VMRepair conventions honoured; grep 'Write-Host' = 0 hits
Safety & Rollback 20/20 Timestamped .bak-YYYYMMDDHHMMSS SYSTEM-hive backup BEFORE any write; SeBackupPrivilege + REG_OPTION_BACKUP_RESTORE (no DACL mutation); Start=3 demand-start preserved; healthy disk produces zero writes
Telemetry Coverage 18/20 Uses Log-* exclusively; tees to $env:PUBLIC\Desktop\win-fix-firewall-service.log; summary printed AFTER per-finding list (4096-char Run Command truncation honoured); minor: no explicit log line on the "no findings" branch
Test Coverage 16/20 Gen2 WS2022 (Azure Edition build 20348) + Gen2 WS2019 (build 17763) live-verified. Gen1 SKUs and WS2025 deferred — registry paths do not vary across Gen

Fault Injection Results

Phase WS2022-g2 (Scenario A · wrong REG type) WS2019-g2 (Scenario B · MpsSvc\Start=4) Status
Pre-injection (baseline) MpsSvc Running/Automatic, peer 3389 reachable MpsSvc Running/Automatic/Start=2, peer 3389 reachable Baseline
Post-injection (breaker) MpsSvc StartPending, peer 3389 timeout, 400 SCM errors "The data is invalid" MpsSvc Stopped/Disabled/Start=4, peer 3389 timeout CORRUPTED
Post-repair (script, offline) DebugedLoopbackApps removed, hive re-verified: 0 findings MpsSvc\Start = 4 → 2, hive re-verified: 0 findings FIXED
Post-restore + boot Script-level PASS (harness-artefact boot failure — see note below) MpsSvc Running/Automatic/Start=2, BFE untouched, peer 3389 reachable VERIFIED

Note on WS2022 boot: disk-swap-back produced 0xC000000E on winload.efi. Root-caused to my manual harness (rescue-VM run-command extension was stuck 40+ min on windowsserver2022:2022-datacenter-smalldisk-g2:20348.5622.260906, forcing me to mount/detach the WS2022 DiskCopy twice). az vm repair restore does not repeat that cycle. The script never opens winload.efi or the BCD store.

Testing Performed

Strategy: Fault injection with break → offline-repair → verify cycle (rescue VM + peer-VNet oracle)
Fault Injection: ✅ Breaker: break-win-fix-firewall-service.ps1
Region: eastus2
Date: 2026-09-16
Head SHA validated: dbe0846a

Dimension Configuration Result
WS2022 Gen2 Azure Edition (build 20348) Standard_D2s_v7, TrustedLaunch PASS (script-level; boot failure is harness artefact)
WS2019 Gen2 (build 17763) Standard_D2s_v7, TrustedLaunch PASS (end-to-end, peer probe closed the loop)
Peer-VNet RDP oracle WS2022 Azure Edition, same VNet as targets Baseline reachable / corrupted unreachable / repaired reachable

Convention Audit (all PASS)

  • Dot-sources src\windows\common\setup\init.ps1 + 5 helpers.
  • Returns $STATUS_SUCCESS / $STATUS_ERROR; no bare 0/1.
  • All output goes through Log-*; grep 'Write-Host' returns 0 hits.
  • Tees to $env:PUBLIC\Desktop\win-fix-firewall-service.log; file present on every run.
  • Strict control-set selection (Get-OfflineSystemRootPath -Strict) per Add the offline registry and protected-resource helpers (2 of 3) #146 — including the re-check if ((Get-OfflineControlSetName -Strict) -ne $context.ControlSet) { throw ... } inside the write hive block.
  • Uses SeBackupPrivilege + REG_OPTION_BACKUP_RESTORE on the AppCs key — no DACL/owner mutation, even during break.
  • Summary printed AFTER per-finding list (honours the 4096-char Run Command truncation concern).
  • map.json diff is exactly one 5-line append; no existing entries changed.
  • Detect-only mode is first-class: 0 writes, drive letters cleaned up in finally.

Opportunities for Improvement

6 points recoverable (current 94 → potential 100)

  • Test Coverage (+4 pts) — Exercise Scenario C (BFE Start=4) as a live inject and add Gen1 WS2019 + WS2022 SKUs to the matrix. Scenario C shares the ServiceDisabled branch with Scenario B, so this is coverage completeness rather than a code-path gap.
  • Telemetry (+2 pts) — Emit an explicit Log-Info line on the "no findings" branch so healthy-disk runs are visually distinct from findings-and-no-writes runs in operator logs.

Non-blocking Observations (docs / cosmetic)

  1. Docs, cosmetic — Measurement table: "330 SIDs / 55,692 bytes" gives ~168 bytes/SID, vs. "700 SIDs / 133,196 bytes" giving ~190 bytes/SID. Within measurement noise; flagging only if you want the header numbers normalised.
  2. Docs, cosmetic0x45b in the PR body is ERROR_SHUTDOWN_IN_PROGRESS / 0x0000045B / decimal 1115. The full-form hex would be more discoverable in grep against Win32 headers, but the abbreviation is unambiguous.
  3. Audit-trail nuance — The literal [1] /f in the WS2022 removal log is a breaker artefact (reg add /d "" /f stored the literal /f as the sole MULTI_SZ entry). The script buffered and printed it faithfully — this is the audit-trail contract behaving correctly.

Validation Evidence

Before (Scenario A · WS2022 build 20348 · post-injection):

Service:          MpsSvc
Status:           StartPending
ServicesPipeTimeout hit; 400 SCM records in System log:
  "The Windows Defender Firewall service terminated with the following
   service-specific error: The data is invalid."
Peer-VNet probe:  Test-NetConnection <target> -Port 3389  → TcpTestSucceeded: False
Registry:         HKLM\SYSTEM\CurrentControlSet\Services\mpssvc\Parameters\AppCs
                    DebugedLoopbackApps  REG_MULTI_SZ  (wrong type — script's evidence)

After (Scenario A · offline repair applied · WS2022 hive re-verified):

Backup:           SYSTEM.bak-20260916203417  (timestamped, written BEFORE mutation)
Change:           Removed value 'DebugedLoopbackApps' from
                  HKLM\SYSTEM\ControlSet001\Services\mpssvc\Parameters\AppCs
                  Owner/DACL not touched.
Re-verify pass:   1 finding → 0 findings after third hive mount
Script status:    [STATUS]::SUCCESS
Summary line:     "Repaired 1 of 1 issue(s) that could be repaired."

After (Scenario B · WS2019 · disk-swap-back + boot):

Get-Service MpsSvc | fl Name,Status,StartType
  Name       : MpsSvc
  Status     : Running
  StartType  : Automatic
reg query "HKLM\SYSTEM\CurrentControlSet\Services\MpsSvc" /v Start
  Start    REG_DWORD    0x2
reg query "HKLM\SYSTEM\CurrentControlSet\Services\BFE"    /v Start
  Start    REG_DWORD    0x2   (untouched — only Start=4 is treated as evidence)
Peer-VNet probe:  Test-NetConnection <target> -Port 3389  → TcpTestSucceeded: True

Review Checklist

  • Fault injection verified (inject → repair → validate) on 2 Gen2 SKUs
  • Code quality clean (PSScriptAnalyzer + convention audit both green)
  • Telemetry coverage reviewed (1 minor gap: "no findings" branch has no explicit log line)
  • Safety features verified (backup-before-write, no DACL mutation, healthy-disk zero-write, Start=3 preserved, valid REG_SZ preserved)
  • Multi-generation tested (WS2022 Gen2 Azure Edition build 20348 + WS2019 Gen2 build 17763)

Test Artifacts

  • HTML Report: Output/TestReports/PR149-win-fix-firewall-service/2026-09-16/TestReport_01.html
  • Execution manifests: vm_provisioning.json, script_execution.json, post_execution_validation.json
  • Evidence bundle: 12 files (baseline / break / post-break / post-restore JSON, peer probes, WS2022 boot screenshot)
  • Logs: az vm repair create traces + full detect + repair script outputs for both disks

Note: Full test artifacts are retained in the internal test store for detailed review.

Recommendation: merge. Nothing in this PR blocks release; the observations above are all optional / cosmetic.


Generated by VMRepairMint Script Testing sub-agent | Head SHA validated: dbe0846a

@EdwinBernal1
Edwin Bernal Microsoft (EdwinBernal1) merged commit 6edc1bd into Azure:main Sep 17, 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.

3 participants