Skip to content

Add win-add-temp-user - #152

Open
Marcus Ferreira (mvaferreira) wants to merge 6 commits into
Azure:mainfrom
mvaferreira:rsl-win-add-temp-user
Open

Add win-add-temp-user#152
Marcus Ferreira (mvaferreira) wants to merge 6 commits into
Azure:mainfrom
mvaferreira:rsl-win-add-temp-user

Conversation

@mvaferreira

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

Copy link
Copy Markdown

What this adds

win-add-temp-user: one scenario script and its own map.json entry. Windows creates the
requested local administrator during a nested boot using SYSTEM\Setup\CmdLine; the script
does not edit SAM structures or depend on Group Policy startup scripts.

Success requires a complete guest result for the requested account confirming creation,
administrator membership and enablement, with no contradictory readable SAM observation.
SAM presence alone is never proof that this run created the account. If account creation
fails, the payload does not promote or enable an existing account.

Recovery and safety

  • Cryptographic password generation, percent-safe batch transport and disabled delayed
    expansion. Printable ASCII passwords are supported except double quotes; unsupported
    characters are rejected rather than silently changed by ASCII encoding.
  • Records recovery metadata atomically before arming the hook. A pending operation,
    unreadable manifest or pre-existing nonzero SetupType blocks a new operation.
  • Restores and verifies both recorded Setup values. Incomplete restoration or cleanup returns
    an error and preserves recovery information instead of reporting success.
  • Pins the original disk and Windows drive during rediscovery. Never cleans up through a stale
    drive path after failed rediscovery; recovery instructions require stopping a managed guest.
  • Releases temporary drive letters and flushes helper diagnostics before the final status.

Generated passwords are delivered in run output, not the scenario's desktop log. Azure Run
Command and the CLI can retain that output; handle it as sensitive. A supplied password is
not echoed by the scenario.

Parameters

Parameter Effect
username Account to create; defaults to azrepairadmin. An existing account is not modified.
password Optional compliant password. When omitted, a cryptographic password is generated and printed.
detectOnly Inspect readiness without creating an account or applying a hook. Discovery can stop an unmanaged repair guest, prepare attached disks and assign temporary letters; writable hive reads may perform recovery. This is not a zero-write disk-forensics mode.
revert Restore the recorded Setup hook and remove residual payload/result files. Does not delete an account already created by Windows. Cannot be combined with detectOnly.
windowsDrive Explicit offline Windows drive when automatic selection is inappropriate.
bootTimeoutSeconds Nested heartbeat timeout; defaults to 600 seconds.

Use a repair VM created with az vm repair create --enable-nested, then invoke with
az vm repair run --run-on-repair --run-id win-add-temp-user.

Validation

The remediation was exercised using the real product workflow against deliberately broken
disposable VMs, not just static checks. Windows Server 2016 and 2025 were randomly selected
from the available 2016/2019/2025 choices; both selected images are Gen1.

Check Server 2016, build 14393 Server 2025, build 26100
Native batch transport, actual-main failure paths and manifest/restore regressions 53/53 53/53
Actual create --enable-nested and pinned run --preview detection Passed Passed
Deliberately incomplete Setup hook, preview revert, independent on-disk checks 5/5 5/5
Actual nested account creation and restore to the original VM Passed Passed
Original guest boot, enabled administrator/group membership and cleanup Passed Passed

Preview candidate: 804cd4ae80110bc919e62d9966aadf674d6691b8.
Script SHA256: 19789E8023484613D1B337EC31AA491E96C8EE507A259E40A030195C435DC30B.

Server 2016 required a forced nested power-off after graceful shutdown timed out. The
script re-read the result and SAM and verified Setup restoration rather than trusting
an unflushed guest state; original-VM verification is recorded separately above.
Server 2025 shut down cleanly. These runs do not establish fresh password/RDP authentication,
Gen2, BitLocker or domain-joined coverage.
The final 2016 guest checks were collected after an additional Azure VM restart and correcting
the verifier's transport. No further scenario changes were made. On both original guests,
the new account was enabled and belonged to Administrators/Remote Desktop Users, the original
fixture account remained disabled, and Setup values and all three temporary artifacts were clear.

The September 2 sign-in test and September 10 helper lifecycle tests remain historical
evidence for their then-current code, not additional fresh coverage for this revision.

Review disposition and scope

The maintainer-supplied agent's report on b2814df9f55c0e915423c260e1cd43949f69dea7
identified 3 Critical, 7 Warning and 4 Info items. Confirmed defects were implemented and
exercised; descriptive findings were clarified. The empty SAM Names-key distinction was
not changed: a normal SAM contains built-in accounts, and the conservative unreadable
classification cannot satisfy the new success gate by itself.

First wave of four independent scenario PRs. Helpers from #143, #146 and #147 are already
merged. This PR changes only the new scenario and its catalog entry, not shared helpers or
other scenarios. Private break/verify/regression tooling is not included in the contribution.

Marcus Ferreira and others added 5 commits September 8, 2026 14:19
Adds a local administrator to an offline Windows disk using the Setup CmdLine hook, then boots the disk in the nested Hyper-V guest so the account is really created, and verifies that it exists.
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: b2814df9f55c0e915423c260e1cd43949f69dea7
Agent recommendation: request changes
Finding-table counts: 3 Critical / 7 Warning / 4 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: #152 — Add win-add-temp-user

Generated: 2026-09-10
Target: Azure/repair-script-library → main

PR #152
Head reviewed b2814df9f55c0e915423c260e1cd43949f69dea7
Base / merge-base 3cdb744e1592c1aa0e6c5840bea43e5b8a1911ad
Head repository https://github.com/mvaferreira/repair-script-library
Changed files map.json, src/windows/win-add-temp-user.ps1 (new, 1031 lines)
Review prompt SHA256 5451BA5C325F61E34FE63B9AF99322A17D70E55210CAF23F5CE27E0F839D8F70

Reviewed against the pinned head snapshot only. Helper contracts consulted: src/windows/common/setup/init.ps1, common/helpers/Logger.ps1, common/helpers/README.md, Get-OfflineWindowsDisk.ps1, Use-OfflineRegistryHive.ps1, Use-NestedRepairVm.ps1, doc/adding_new_scripts.md, and the comparable upstream script src/windows/win-create-troubleshooting-user.ps1.

Standards baseline passes. The script sources init.ps1 (line 116), returns $STATUS_SUCCESS/$STATUS_ERROR on every path, uses only Logger functions (no Write-Host), declares a Param() block with defaults, wraps the body in try/catch, and pairs every hive mount through Invoke-WithHive. Its use of Invoke-WithHive, Get-OfflineWindowsDisk, Start-NestedRepairVm, Wait-NestedRepairVmBoot and Stop-NestedRepairVmGraceful matches those helpers' documented parameter names and returned properties. The scope-capture comments at 331-332 and 437-438 are correct: Invoke-WithHive calls & $ScriptBlock with no arguments (Use-OfflineRegistryHive.ps1), the helpers are dot-sourced rather than module-scoped, and Invoke-WithHive declares no local named $command, $setupType, $targetSetupType or $targetCmdLine, so the blocks resolve those from the caller's scope as claimed. The findings below are defects within that otherwise conforming structure.

Findings

Critical

File Line/Context Issue Recommendation
src/windows/win-add-temp-user.ps1 169, 519, 1014 A generated password containing % is silently corrupted, and the run still reports success. $special = '!@#$%^&*()-_=+' (169) includes %. Line 519 emits net user "<user>" "<password>" /add /y into an ASCII .cmd. cmd.exe percent-expansion runs before quote processing, so double quotes do not protect %: a lone % is stripped, and a %…% pair is consumed as a variable reference. net.exe therefore receives a different string than the one printed to the engineer at 1014. net user still returns 0, so add=0, the offline SAM shows the account, and line 1004 reports SUCCESS. The engineer cannot sign in — the exact failure this script exists to fix — after a full nested boot cycle. The generator produces 16 characters (1 seeded special from a 14-character set, 12 drawn from a 71-character set): P(contains %) = 1 − (13/14)(70/71)¹² ≈ 21.7%, roughly one run in five. This is introduced by this PR: the comparator's generator (win-create-troubleshooting-user.ps1, "@zurE" + 8 alphanumerics) cannot emit %. " is the same hazard for an operator-supplied -password; the ValidateScript at 100-108 accepts it via \W. Remove %, ", ^, &, <, >, `
src/windows/win-add-temp-user.ps1 941-942, 968-973, 1004 The success gate accepts offline-SAM presence alone, which cannot distinguish an account this run created from one that already existed. Line 968 requires only one of the two signals, and 1004 explicitly renders the SAM-only case as "confirmed by the offline SAM". But $samSaysCreated (942) is presence, not creation. It is authoritative only when detect proved the account absent (775-782). When detect could not read SAM, the script warns and continues (784-787) — and 786 promises the collision "will surface as a failed payload", which 968 then contradicts: a pre-existing account yields add != 0$payloadSaysCreated = $false, yet SAM presence alone passes the gate, so SUCCESS is printed with a password that was never applied. Precondition chain: SAM unreadable at detect (785) and readable at verify — reachable because a hive too dirty to reg load at detect is reconciled by the guest's boot and clean shutdown. Reliance on the weaker signal is not exotic: $payloadResult.Complete (941) is false whenever the result file is missing or truncated, which the tolerated non-graceful stop at 923-925 makes a normal outcome. Require $payloadSaysCreated for success; accept $samSaysCreated as confirming only when $userState.Known -and -not $userState.Exists at detect. Fail closed when SAM was unreadable at detect rather than proceeding on a signal that cannot answer the question.
src/windows/win-add-temp-user.ps1 519-525, 774-787, 699-762 The payload promotes and enables a pre-existing account unconditionally, with no rollback. Lines 521, 523 and 525 run net localgroup Administrators … /add, net localgroup "Remote Desktop Users" … /add and net user … /active:yes without testing %RC_ADD% from 520. When the requested name already exists on the disk — the case detect cannot rule out when SAM is unreadable (784-787) — the run adds that pre-existing account to local Administrators and Remote Desktop Users and enables it, including a deliberately disabled account. This is a privilege grant to an account the operator never intended to touch (Phase 4, Privilege Escalation), and it occurs even on runs that later report FAILED at 969. -revert cannot undo it: the revert path restores the hook, deletes payload/result and only reports the surviving account (743-748). Gate 521-525 on if "%RC_ADD%"=="0" so the payload only modifies an account it created. Record group membership in the revert manifest, or make an unreadable detect-time SAM a blocking finding rather than a warning.

Warning

File Line/Context Issue Recommendation
src/windows/win-add-temp-user.ps1 670-1030 (no finally), 676, 928 The documented helper caller contract is not met. common/helpers/README.md ("Required caller contract") mandates releasing temporary drive letters and flushing helper logs in finally, "including when discovery fails partway through", and Get-OfflineWindowsDisk's own .OUTPUTS states: "AssignedDriveLetters holds the letters this run assigned; pass each to Remove-OfflineDriveLetter, or call Clear-OfflineDriveLetter, in the caller's finally." The script has no finally and never calls either. Discovery runs twice (676, 928), and each pass assigns letters to EFI System/Recovery partitions via Add-PartitionDriveLetter; none are released on any of the ten return paths or the catch at 1026. The catch also does not call Write-OfflineRepairLog, so helper-buffered diagnostics from the failing call are discarded and only the exception text survives (1027-1028). Adopt the README skeleton: set $status and return it after a finally that calls Clear-OfflineDriveLetter and Write-OfflineRepairLog, each guarded by Get-Command.
src/windows/win-add-temp-user.ps1 928-936, 1026-1029 The highest-consequence failure window is the only one with no remediation guidance. The guard at 931 implies Get-OfflineWindowsDisk can return $null or an object without WindowsPath; it cannot. Every failure inside it throws (Get-OfflineWindowsDisk.ps1: "Could not determine the rescue VM's own system disk number…", "No attached broken OS disk was found…", "No offline Windows installation was found on the attached disk(s)."). So 931-936 is dead code and a failed rediscovery lands in the bare catch at 1026. At that point the Setup hook is armed and, if the payload never ran, Windows\Temp\win-add-temp-user.cmd — containing the plaintext password — is still on the disk, because the cleanup at 959-965 sits after the throwing call. Every other failure path emits recovery text (889, 970, 989); this one emits only an exception and a stack trace. Wrap 928 in its own try/catch that logs the same "-revert true" guidance and states that the payload may remain on the disk. Remove the unreachable guard at 931.
src/windows/win-add-temp-user.ps1 676, 699, 919-921 The documented -revert recovery can itself be blocked by this script's own failure. -revert is handled at 699, after discovery at 676, so revert requires Get-OfflineWindowsDisk to succeed. Start-NestedRepairVm marks the guest with a persistent repair-script-library:nested-repair:v1 Notes line via Set-NestedRepairVmManaged, and per common/helpers/README.md discovery "refuses to proceed while it is active". If Stop-NestedRepairVmGraceful returns Stopped = $false, line 921 returns $STATUS_ERROR with the guest still running and marked — so re-running with -revert true, the remedy offered at 889 and 970, fails at 676. The message at 920 does not tell the operator to stop the guest first. Add to the 920 message that ProblemVM must be stopped (Hyper-V Manager or Stop-VM) before -revert can run; or move the -revert branch ahead of discovery, or let it pass -NestedVmId.
src/windows/win-add-temp-user.ps1; map.json 50, 828-831; map.json:95 detectOnly is described as changing nothing, but it changes rescue-VM and disk state before it reports. Line 50 says "Report what is on the disk and change nothing" and map.json:95 says "Pass detectOnly=true to report only", yet detect runs the full discovery at 676 first. Get-OfflineWindowsDisk unconditionally calls Stop-NestedRepairVm -VmId $NestedVmId before disk preparation — so detectOnly=true powers off a running unmanaged ProblemVM — then Set-OfflineDisksOnline runs diskpart online disk / attributes disk clear readonly, and Add-PartitionDriveLetter may assign letters. Detect also reads through Invoke-WithHive (774, 763), which mounts SYSTEM and SAM writably via reg load; the library exposes a read-only offreg reader for exactly this (common/helpers/README.md, "Read-only hive access"). The source does not establish a zero-write detect. The PR's "Detect-only made no writes" is a recorded historical observation, not something this review can confirm — the packet's validation was static and executed nothing. Reword line 50 and the map.json description to "makes no changes to the Windows installation" and state the guest-stop and disk-online side effects, or route the detect-only reads through the read-only reader.
src/windows/win-add-temp-user.ps1 172-175, 180, 183 An administrator password is generated with Get-Random. Get-Random is backed by System.Random, a non-cryptographic PRNG, and Sort-Object { Get-Random } (183) is a biased shuffle. The supplied Phase 3 checklist classifies "user-created passwords use secure generation" as Critical. Rated Warning here because it matches the comparable upstream script, which also uses Get-Random — so it is not a regression — but this is new code being added, and the "not predictable" claim in the function's own help (164) overstates what Get-Random provides. Draw from [System.Security.Cryptography.RandomNumberGenerator] with rejection sampling, and shuffle with the same source.
src/windows/win-add-temp-user.ps1 443, 457 Restore-OfflineSetupHook verifies only half of what it wrote. Set-OfflineSetupHook reads back and compares both CmdLine (401) and SetupType (406). Restore-OfflineSetupHook compares only SetupType (457) and never inspects $readBack.CmdLine, while the removal at 443 uses -ErrorAction SilentlyContinue. A CmdLine that could not be removed or rewritten is therefore reported as Restored = $true, leaving a stale value pointing at a deleted payload on both the revert path (720) and the finalize path (984). Impact is bounded — SetupType is proven 0, so the stale command does not execute — but the disk is returned in a state the script asserts it cleaned. Mirror 401-404: compare $readBack.CmdLine against the intended value (empty when removed) and fail the restore when it does not match.
src/windows/win-add-temp-user.ps1 981-989 Finalize can restore a non-zero SetupType and then give circular advice. The SetupHookInUse guard at 791-793 blocks only when SetupType != 0 and CmdLine is non-empty, so a disk arriving with a non-zero SetupType and no CmdLine proceeds. Finalize then deliberately restores $hook.PreviousSetupType (984), leaving it non-zero, so 987 is true and 989 warns "SetupType is still N … Re-run with -revert true before restoring the disk" — but -revert writes back the same recorded previous value (720), so the advice cannot resolve it. Line 988 additionally interpolates $restore.Reason, which is $null on this path, producing "The Setup hook could not be cleared: " with an empty reason. Distinguish "restore failed" from "the recorded previous state was itself non-zero", suppress 988 when $restore.Restored, and surface a non-zero pre-existing SetupType as a detect finding at 791.

Info

File Line/Context Suggestion
src/windows/win-add-temp-user.ps1 928 Rediscovery passes -DiskNumber $offline.DiskNumber but -WindowsDrive $windowsDrive — the caller's parameter, normally empty — rather than $offline.WindowsDrive. Get-OfflineWindowsDisk re-scores candidates and subtracts 20 for SetupInProgress, a flag this script itself sets at 981's precondition. On a disk carrying more than one Windows installation the second pass could therefore select a different install and verify the wrong SAM. Not demonstrated, and single-installation disks are unaffected. Passing -WindowsDrive $offline.WindowsDrive would pin it.
src/windows/win-add-temp-user.ps1 1010, 1014-1015 The comment "Printed, never logged" and the message at 1015 are accurate only for the rescue-VM desktop log. Logger.ps1 implements every Log-* function with Write-Output, and doc/adding_new_scripts.md states the logs "are written to stdout which are redirected to file when used with the CLI vm-repair extension", so the password still transits the Run Command output stream and the CLI's local output. This is inherent to the delivery model and strictly better than the comparator, which additionally writes the password to $env:PUBLIC\Desktop. Worth wording so the guarantee is not read as broader than it is.
src/windows/win-add-temp-user.ps1 235-238 & $ScriptBlock unrolls an empty array to $null, so a Names key that exists with zero subkeys is indistinguishable from a missing key and is reported as "the account name key was not present" (256). Harmless in practice — a real SAM always carries Administrator and Guest — but the two cases carry different meanings elsewhere in this script.
map.json 95 The description asserts "The file carrying the password deletes itself, and the setup hook is cleared, so nothing is left on the disk." Both guarantees have failure windows the script itself handles and reports (959-961, 987-989), and one it does not (Warning 2). Softening to "removes the password file and clears the setup hook, and reports when it could not" would match the implementation.

Operational Risk Assessment

Factor Rating Notes
Scope Low One new scenario script plus one appended map.json entry. No helper file is added or modified and no existing scenario is touched, so nothing outside this run-id can regress. Blast radius is confined to operators who invoke win-add-temp-user.
Destructive ops High Writes SYSTEM\Setup\SetupType/CmdLine on a customer OS disk (338-341), writes and deletes files under the customer Windows\Temp (536, 961, 965, 1000), boots the customer disk in a nested guest, and creates — or, per Critical 3, silently promotes and enables — a local administrator. Discovery additionally powers off a running ProblemVM and runs diskpart against attached disks, including under detectOnly.
Rollback possible Partial -revert restores the recorded hook and removes payload/result (699-762), and the manifest merge at 596-625 preserves earlier undo data. But it deliberately does not delete a created account, cannot undo the group membership and enablement of Critical 3, is blocked when the guest is left running and marked (Warning 3), reports success without verifying CmdLine (Warning 6), and cannot resolve a restored non-zero SetupType (Warning 7).
Testing documented Yes, but not reproduced here The PR records a 2 Sept product-path acceptance run via az vm repair run --run-on-repair and a separate revert exercise, and a 10 Sept shared nested-lifecycle suite (87/87 on PS5.1 and PS7, plus 93 native Hyper-V assertions). The PR is candid that the 10 Sept guests were firmware-only. No repair-script-library test harness exists (tests/ is a placeholder), and doc/adding_new_scripts.md notes local --custom-script-file testing cannot pass parameters, so detectOnly/revert/username/password cannot be exercised that way. This review executed nothing — no repair script, no boot, no Azure operation, no hive mount. The packet's validation was likewise static. Critical 1 in particular is a ~1-in-5 probabilistic path that a small number of manual runs would very plausibly miss.
Gen compatibility Unknown The script itself is generation-agnostic: it delegates Gen1/Gen2 handling to Get-OfflineWindowsDisk (partition-style → generation) and Connect-NestedRepairVmDisk (Gen2 boot-order promotion), and only reports $vmState.Generation at 805. It never branches on generation. But the PR explicitly declines to claim a fresh Gen1/Gen2 account-creation matrix, and the SYSTEM\Setup\CmdLine hook is untested here on both firmware types. BitLocker-protected volumes are not detected or reported by this script.

Overall Risk: High 🔴 — driven by Critical 1 (a ~22%-per-run silent wrong-password outcome reported as success), Critical 3 (an unrollbackable privilege grant to an account the operator did not target), and the destructive-operation profile on a customer OS disk.

Validation

Deterministic checks below were performed by the launcher and recorded in the review packet; the source assessment is this review's own.

  • map.json valid: pass (MapJsonValid: true, MapJsonError: null)
  • map.json paths exist and are in-tree: pass (MissingOrUnsafeMapPaths: []) — map.json:94 src/windows/win-add-temp-user.ps1 is present in the head snapshot
  • Duplicate IDs: none (DuplicateIds: [])
  • PowerShell parse errors: none (PowerShellParseErrors: [])
  • New entry placement and JSON shape: passmap.json:92-96, appended after win-create-troubleshooting-user, correct comma placement, every upstream entry preserved; the diff is purely additive (@@ -89,6 +89,11 @@)
  • id naming convention: passwin-add-temp-user matches win-* and equals the file stem
  • Init sourcing: pass. .\src\windows\common\setup\init.ps1 at line 116
  • Status return: pass — every exit returns $STATUS_SUCCESS or $STATUS_ERROR; no bare throw escapes the catch at 1026
  • Logger usage: pass — no Write-Host or bare Write-Output; all output via Log-Output/Log-Warning/Log-Error
  • Helper signature binding: pass — parameter names and consumed properties match Get-OfflineWindowsDisk, Invoke-WithHive, Get-NestedRepairVm, Start-NestedRepairVm, Wait-NestedRepairVmBoot and Stop-NestedRepairVmGraceful
  • Helper caller contract (finally, Clear-OfflineDriveLetter, log flush): fail — see Warning 1
  • Repair scripts executed: no (RepairScriptsExecuted: false). Azure operations performed: no (AzureOperationsPerformed: false). No file was modified and nothing was posted to GitHub during this review.

Recommendation

request changes

The design is a genuine advance on win-create-troubleshooting-user: the SYSTEM\Setup\CmdLine hook is the right mechanism for a domain-joined VM, letting the guest create the account avoids hand-editing SAM, the payload is single-use and self-deleting rather than left on the disk with the password in it, and the script verifies its own outcome instead of asserting success after Start-VM. Those are real improvements and the helper integration is largely correct.

The three Critical findings should be resolved before merge. Critical 1 is the priority: it is unconditional, quantified at roughly one run in five, introduced by this PR relative to the comparator's generator, and silent — both of the script's verification signals still report success. Criticals 2 and 3 share a precondition (the detect-time SAM read failing, which the script treats as a warning at 784-787) and are best fixed together, by gating the payload's group/enable commands on %RC_ADD% and by refusing to treat SAM presence as proof of creation when detect could not establish absence. Warnings 1-3 concern recovery behaviour after a failed run and are cheap to address alongside.

Because Critical 1 is probabilistic, the fixes should be validated by a targeted test — assert the exact password string round-trips through the generated .cmd, with %, ", ^ and & forced into the input — rather than by further end-to-end runs, which can pass repeatedly while the defect remains.

This is an automated review produced by the rsl-pr-review agent from the pinned head snapshot. It is not a human maintainer approval, and no repair, boot or Azure testing was performed as part of it.

…ery state

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

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

Copy link
Copy Markdown
Author

Review findings implemented and exercised

Publication head: ea46b96325dbaea9400131840b03d13c94f7cbe5

The earlier automated report remains a static review of
b2814df9f55c0e915423c260e1cd43949f69dea7. This follow-up records implementation and actual
execution evidence; it is not a relabelled or newly generated agent review.

Finding Disposition
C1: corrupted batch arguments Fixed percent expansion and delayed expansion; verifies payload bytes and rejects lossy/non-ASCII or double-quote input. Valid quoted metacharacters are retained rather than banned unnecessarily.
C2: SAM-only false success Requires a complete matching guest result with successful creation, administrator membership and enablement. Readable SAM must not contradict it. Unreadable SAM is not a blanket blocker when the fresh guest result is authoritative.
C3: promotion after failed creation Group additions and enablement run only after successful account creation. A collision does not modify the existing account.
W1: missing caller cleanup Finally releases temporary letters and flushes diagnostics before the single final status.
W2/W3: incomplete recovery guidance Preserves recovery state, avoids stale drive paths and explicitly requires a stopped managed guest before revert.
W4: detect-only side effects Documents the distinction between no account/hook changes and discovery/disk/hive-read side effects.
W5: password randomness Uses cryptographic random bytes, rejection sampling and Fisher-Yates shuffle.
W6: incomplete hook verification Verifies both CmdLine and SetupType on restore and reports incomplete cleanup as failure.
W7: nonzero initial SetupType Blocks new work on a nonzero initial state; preserves legacy recorded state without circular revert advice.
I1: rediscovery selection Pins both the original disk number and Windows drive.
I2/I4: retention/cleanup claims Documents service-retained output and retained recovery artifacts instead of unconditional cleanup guarantees.
I3: empty SAM Names key Not changed: normal SAMs contain built-in accounts; conservative unreadable handling cannot produce SAM-only success.

Additional confirmed recovery gaps were fixed alongside these findings: strict existing
manifest validation, staged/read-back atomic publication, manifest-before-hook ordering,
pending-operation protection, and rejection of conflicting detect/revert flags.

Actual product testing: randomly selected Windows Server 2016/build 14393 and
2025/build 26100, both Gen1. On each version, 53 targeted checks passed. Real
az vm repair create --enable-nested, pinned run --preview, deliberate partial-hook
revert with 5/5 independent checks, nested administrator creation and restore completed.
Original-guest verification passed on both versions: the new account is enabled, belongs to
Administrators and Remote Desktop Users, and the original fixture account remains disabled.
SetupType is zero, CmdLine is empty, and payload/result/manifest files are absent.

The tested script is byte-identical to candidate
804cd4ae80110bc919e62d9966aadf674d6691b8
(SHA256 19789E8023484613D1B337EC31AA491E96C8EE507A259E40A030195C435DC30B).
Server 2016's graceful nested shutdown timed out and the helper used forced power-off;
result/SAM and Setup cleanup were subsequently rechecked. Server 2025 stopped cleanly.
This is not a fresh Gen2, domain-joined, BitLocker or password/RDP authentication matrix.
The final 2016 guest checks followed an additional Azure VM restart and correction of the
verifier's transport, with no further scenario changes.
Generated-password lines were redacted on the repair hosts before full driver logs were collected.

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

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.

1 participant