fix(phase4): track Windows acceptance host provisioning and repair bootstrap - #73
fix(phase4): track Windows acceptance host provisioning and repair bootstrap#73gitcommit90 wants to merge 1 commit into
Conversation
…otstrap The Windows acceptance host bootstrap existed only as a pre-built ISO on the hypervisor, so it could not be reviewed, reproduced, or corrected. Track it and fix two defects found while provisioning a fresh host. setup.ps1 installed the VirtIO network driver but never the VirtIO serial driver. The QEMU guest agent then starts and reports Running while the host side (qm agent <vmid> ping) stays dead, leaving no guest-exec channel to provision through. setup.ps1 also skipped firewall setup whenever an OpenSSH-Server-In-TCP rule already existed. Add-WindowsCapability creates that rule scoped to the Private profile only, and a freshly bridged VM is categorized Public, so port 22 stayed unreachable even though sshd reported Running. The operator public key is no longer embedded. This repository is public, so publishing which key is authorized as Administrator on the acceptance host is needless disclosure; it is supplied at build time and staged onto the media. Also records two host constraints that are easy to get wrong: Proxmox always creates the TPM state volume as raw, and a raw volume on directory storage blocks snapshots for the whole VM, which the acceptance lane depends on for restoring its accepted clean baseline; and Windows 11 25H2 no longer auto-applies an answer file from secondary media. No release, tag, version bump, website deploy, or production change. Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds a Windows 11 acceptance-host bootstrap ISO, unattended installation configuration, first-boot provisioning script, and documentation for SSH access, readiness signaling, VM snapshots, and Windows 11 25H2 installation constraints. ChangesWindows acceptance host
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant build-unattend-iso.sh
participant WindowsSetup
participant setup.ps1
participant ready.json
Operator->>build-unattend-iso.sh: Provide operator SSH public key
build-unattend-iso.sh->>WindowsSetup: Create bootstrap ISO
WindowsSetup->>setup.ps1: Run first-boot provisioning
setup.ps1->>ready.json: Write readiness metadata
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ops/platform-acceptance/windows-host/.gitignore`:
- Line 2: Update the Windows host .gitignore entry containing authorized_key.pub
to also ignore the generated 1helm-windows-unattend.iso file, ensuring the
bootstrap ISO is not tracked in the worktree.
In `@ops/platform-acceptance/windows-host/Autounattend.xml`:
- Around line 10-11: Update the Autounattend.xml VirtIO driver paths in the
PathAndCredentials entries to match the documented three-medium layout: Windows
on D:, VirtIO media on E:, and the bootstrap ISO on F:. Also update the
associated build or operator instructions to require and document that drive
order, or remove the fixed drive-letter dependency while preserving driver
discovery.
In `@ops/platform-acceptance/windows-host/setup.ps1`:
- Around line 13-21: Update the installation checks in setup.ps1, including the
NetKVM, VirtIO serial, and QEMU guest agent commands, so exit code 3010 persists
a restart-required state, stops provisioning, and reboots the host instead of
continuing. Ensure setup.ps1 is scheduled to rerun after reboot and writes
ready.json only after all required drivers and services complete successfully
without a pending restart.
- Around line 37-42: Update the sshd_config handling in setup.ps1 to append
PasswordAuthentication no and PubkeyAuthentication yes when either directive is
absent, while preserving replacement of existing or commented directives.
Validate the resulting configuration with sshd -t before the Start-Service sshd
call, and only proceed when validation succeeds.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b73666ad-bd65-4a3d-8c15-9f40456fa557
📒 Files selected for processing (6)
docs/phase4-platform-acceptance.mdops/platform-acceptance/windows-host/.gitignoreops/platform-acceptance/windows-host/Autounattend.xmlops/platform-acceptance/windows-host/README.mdops/platform-acceptance/windows-host/build-unattend-iso.shops/platform-acceptance/windows-host/setup.ps1
| @@ -0,0 +1,2 @@ | |||
| # The per-site operator public key is never committed to this public repository. | |||
| authorized_key.pub | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
git check-ignore -v --no-index \
ops/platform-acceptance/windows-host/1helm-windows-unattend.isoRepository: gitcommit90/1Helm
Length of output: 155
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- relevant files ---'
git ls-files 'ops/platform-acceptance/windows-host/*' | sed -n '1,120p'
printf '%s\n' '--- ignore rules ---'
cat -n ops/platform-acceptance/windows-host/.gitignore
printf '%s\n' '--- ISO/key references ---'
rg -n -C 4 --glob '!.git' \
'1helm-windows-unattend\.iso|authorized_key\.pub|build-unattend-iso' \
ops/platform-acceptance/windows-hostRepository: gitcommit90/1Helm
Length of output: 5986
Information Disclosure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Internal
Ignore the generated bootstrap ISO.
The default 1helm-windows-unattend.iso contains authorized_key.pub for Administrator SSH access but is not ignored. Add the ISO to .gitignore or write it outside the worktree.
Proposed fix
authorized_key.pub
+1helm-windows-unattend.iso📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| authorized_key.pub | |
| authorized_key.pub | |
| 1helm-windows-unattend.iso |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ops/platform-acceptance/windows-host/.gitignore` at line 2, Update the
Windows host .gitignore entry containing authorized_key.pub to also ignore the
generated 1helm-windows-unattend.iso file, ensuring the bootstrap ISO is not
tracked in the worktree.
| <PathAndCredentials wcm:action="add" wcm:keyValue="1"><Path>E:\vioscsi\w11\amd64</Path></PathAndCredentials> | ||
| <PathAndCredentials wcm:action="add" wcm:keyValue="2"><Path>E:\NetKVM\w11\amd64</Path></PathAndCredentials> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Define the required VirtIO media and drive order.
Lines 10-11 require the VirtIO driver tree on E:. The bootstrap ISO contains only the answer file, setup.ps1, and the key. The build script instructs operators to attach it as the second CD-ROM beside Windows media. In that layout, E: is the bootstrap ISO and WinPE cannot load the VirtIO storage and network drivers.
Require and document a third medium with Windows on D:, VirtIO on E:, and the bootstrap ISO on F:. Alternatively, remove the fixed drive-letter dependency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ops/platform-acceptance/windows-host/Autounattend.xml` around lines 10 - 11,
Update the Autounattend.xml VirtIO driver paths in the PathAndCredentials
entries to match the documented three-medium layout: Windows on D:, VirtIO media
on E:, and the bootstrap ISO on F:. Also update the associated build or operator
instructions to require and document that drive order, or remove the fixed
drive-letter dependency while preserving driver discovery.
| if ($LASTEXITCODE -notin 0, 3010) { throw "NetKVM driver install failed: $LASTEXITCODE" } | ||
| # The QEMU guest agent reaches the host over a VirtIO serial port. Without | ||
| # this driver the QEMU-GA service still starts and reports Running, while the | ||
| # host side ("qm agent <vmid> ping") stays dead and there is no guest-exec | ||
| # channel to provision the host with. | ||
| & pnputil.exe /add-driver "$drive\vioserial\w11\amd64\*.inf" /subdirs /install | ||
| if ($LASTEXITCODE -notin 0, 3010) { throw "VirtIO serial driver install failed: $LASTEXITCODE" } | ||
| & msiexec.exe /i "$drive\guest-agent\qemu-ga-x86_64.msi" /qn /norestart | ||
| if ($LASTEXITCODE -notin 0, 3010) { throw "QEMU guest agent install failed: $LASTEXITCODE" } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='ops/platform-acceptance/windows-host/setup.ps1'
rg -n -C 4 '3010|/norestart|ready\.json|Restart-Computer|shutdown\.exe' "$file"
if ! rg -qE 'Restart-Computer|shutdown\.exe.*[/-]r' "$file"; then
echo 'No explicit restart exists for an accepted 3010 result.' >&2
exit 1
fiRepository: gitcommit90/1Helm
Length of output: 2426
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='ops/platform-acceptance/windows-host/setup.ps1'
printf '%s\n' '--- setup.ps1 ---'
cat -n "$file" | sed -n '1,75p'
printf '%s\n' '--- references to setup.ps1 and ready.json ---'
rg -n -C 3 'setup\.ps1|ready\.json|bootstrap\.log|QEMU-GA|Start-Service' . \
-g '!node_modules' -g '!dist' -g '!build'Repository: gitcommit90/1Helm
Length of output: 15357
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
setup = Path("ops/platform-acceptance/windows-host/setup.ps1").read_text()
unattend = Path("ops/platform-acceptance/windows-host/Autounattend.xml").read_text()
accepted_3010 = [
line for line in setup.splitlines()
if "3010" in line and "-notin" in line
]
restart_calls = [
line for line in setup.splitlines()
if "Restart-Computer" in line or "shutdown.exe" in line
]
ready_writes = [
(i + 1, line) for i, line in enumerate(setup.splitlines())
if "ready.json" in line and "Set-Content" in line
]
print(f"accepted_3010_checks={len(accepted_3010)}")
print(f"explicit_restart_calls={len(restart_calls)}")
print(f"ready_writes={ready_writes}")
print(f"unattend_will_reboot_never={'<WillReboot>Never</WillReboot>' in unattend}")
print("accepted_3010_checks:")
for line in accepted_3010:
print(line.strip())
PYRepository: gitcommit90/1Helm
Length of output: 670
Do not publish readiness after exit code 3010.
When any installation returns 3010, stop provisioning, persist the restart-required state, and reboot the host. Ensure setup.ps1 runs again after reboot and writes ready.json only after the required drivers and services are ready.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ops/platform-acceptance/windows-host/setup.ps1` around lines 13 - 21, Update
the installation checks in setup.ps1, including the NetKVM, VirtIO serial, and
QEMU guest agent commands, so exit code 3010 persists a restart-required state,
stops provisioning, and reboots the host instead of continuing. Ensure setup.ps1
is scheduled to rerun after reboot and writes ready.json only after all required
drivers and services complete successfully without a pending restart.
| if (Test-Path $config) { | ||
| $text = Get-Content -Raw $config | ||
| $text = [regex]::Replace($text, '(?m)^\s*#?\s*PasswordAuthentication\s+.*$', 'PasswordAuthentication no') | ||
| $text = [regex]::Replace($text, '(?m)^\s*#?\s*PubkeyAuthentication\s+.*$', 'PubkeyAuthentication yes') | ||
| Set-Content -Encoding ascii -Path $config -Value $text | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='ops/platform-acceptance/windows-host/setup.ps1'
rg -n -C 3 'PasswordAuthentication|PubkeyAuthentication|sshd_config|sshd -t' "$file"
# Expected after the fix: explicit fallback insertion for absent directives
# and an `sshd -t` validation before Start-Service sshd.
rg -n 'sshd -t' "$file"Repository: gitcommit90/1Helm
Length of output: 1007
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='ops/platform-acceptance/windows-host/setup.ps1'
printf '%s\n' '--- setup.ps1 ---'
nl -ba "$file" | sed -n '1,130p'
printf '%s\n' '--- relevant call sites and configuration setup ---'
rg -n -C 3 'Add-WindowsCapability|OpenSSH|sshd_config|New-NetFirewallRule|Start-Service|Set-Service|ready\.json|3010|reboot' \
ops/platform-acceptance/windows-hostRepository: gitcommit90/1Helm
Length of output: 213
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
file='ops/platform-acceptance/windows-host/setup.ps1'
printf '%s\n' '--- setup.ps1 ---'
awk '{ printf "%5d %s\n", NR, $0 }' "$file" | sed -n '1,130p'
printf '%s\n' '--- relevant call sites and configuration setup ---'
rg -n -C 3 'Add-WindowsCapability|OpenSSH|sshd_config|New-NetFirewallRule|Start-Service|Set-Service|ready\.json|3010|reboot' \
ops/platform-acceptance/windows-hostRepository: gitcommit90/1Helm
Length of output: 12548
🌐 Web query:
Microsoft Windows OpenSSH Server default sshd_config PasswordAuthentication PubkeyAuthentication Windows 11
💡 Result:
When you first install and start the OpenSSH Server on Windows 11, the sshd_config file is automatically generated by the service at %ProgramData%\ssh\sshd_config [1][2][3]. By default, the OpenSSH Server is configured to allow both password-based authentication and public key authentication [4]. Specifically, the generated configuration file defaults to the following settings for these authentication methods: PubkeyAuthentication yes [4] PasswordAuthentication yes [4] To modify these settings, you can edit the %ProgramData%\ssh\sshd_config file with a text editor (run as an administrator) and change the values to no [5][4]. After modifying the file, you must restart the OpenSSH SSH Server service (using the Services app or PowerShell) for the changes to take effect [5][6]. For security, especially if you intend to disable password authentication in favor of key-based authentication, ensure you have correctly configured the authorized_keys files. Note that Windows OpenSSH enforces specific Access Control List (ACL) permissions on these files; they must be owned by the user or the SYSTEM account, and other users must not have write access to them [3][7]. Additionally, if you are using an administrator account, the server looks for authorized keys in a specific location at %ProgramData%\ssh\administrators_authorized_keys by default, rather than the user's home directory [3][8].
Citations:
- 1: https://learn.microsoft.com/en-us/windows-server/administration/openssh/openssh-server-configuration
- 2: https://github.com/PowerShell/Win32-OpenSSH/wiki/sshd_config
- 3: https://winscp.net/eng/docs/guide%5Fwindows%5Fopenssh%5Fserver
- 4: https://woshub.com/connect-to-windows-via-ssh/
- 5: https://serverfault.com/questions/888350/windows-10-ssh-disable-password-authentication
- 6: https://learn.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse
- 7: https://learn.microsoft.com/en-us/windows-server/administration/openssh/openssh_keymanagement
- 8: https://learn.microsoft.com/en-us/troubleshoot/windows-server/system-management-components/upgrade-in-box-openssh-to-latest-openssh-release
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("ops/platform-acceptance/windows-host/setup.ps1")
text = path.read_text()
assert "if (Test-Path $config)" in text
assert "Start-Service sshd" in text
assert text.index("if (Test-Path $config)") < text.index("Start-Service sshd")
assert "PasswordAuthentication no" in text
assert "PubkeyAuthentication yes" in text
# Model both replacements with a configuration that omits both directives.
config = "Host *\n AuthorizedKeysFile .ssh/authorized_keys\n"
updated = re.sub(
r"(?m)^\s*#?\s*PasswordAuthentication\s+.*$",
"PasswordAuthentication no",
config,
)
updated = re.sub(
r"(?m)^\s*#?\s*PubkeyAuthentication\s+.*$",
"PubkeyAuthentication yes",
updated,
)
assert "PasswordAuthentication no" not in updated
assert "PubkeyAuthentication yes" not in updated
print("Absent directives remain absent; service start follows the conditional rewrite.")
PYRepository: gitcommit90/1Helm
Length of output: 235
Broken Authentication (CWE-287): Improper Authentication
Reachability: External · Exploitability: Moderate
Append the SSH authentication directives.
When sshd_config omits either directive, the replacements add nothing. Start-Service sshd can then retain the Windows default PasswordAuthentication yes, while the later firewall rule allows TCP/22 on every profile. Append missing directives and run sshd -t before starting sshd.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ops/platform-acceptance/windows-host/setup.ps1` around lines 37 - 42, Update
the sshd_config handling in setup.ps1 to append PasswordAuthentication no and
PubkeyAuthentication yes when either directive is absent, while preserving
replacement of existing or commented directives. Validate the resulting
configuration with sshd -t before the Start-Service sshd call, and only proceed
when validation succeeds.
|
Folded unchanged into #80 with the final Mac and Windows acceptance fixes so one merge produces one end-to-end proof run. |
Why
While provisioning a fresh Windows acceptance host for Phase 4, two defects in the host bootstrap cost hours of debugging. Both will recur on every future acceptance host.
The bootstrap was also not in this repository at all — it existed only as a pre-built ISO on the hypervisor, so it could not be reviewed, reproduced, or fixed. This PR brings it under version control along with the corrections.
The two defects
1. Missing VirtIO serial driver.
setup.ps1installed the NetKVM network driver but nevervioserial. The QEMU guest agent then installs and its service reportsRunning, while the host side (qm agent <vmid> ping) stays dead — so there is no guest-exec channel, and the failure looks like a healthy service.2. SSH firewall rule scoped to the wrong profile.
setup.ps1skipped firewall setup wheneverOpenSSH-Server-In-TCPalready existed.Add-WindowsCapabilitycreates exactly that rule, scoped to the Private profile only. A freshly bridged VM is categorized Public, so the guard short-circuited and port 22 refused connections despitesshdreportingRunning.Both are now handled explicitly, and the firewall rule is ensured to cover every profile whether or not it pre-exists.
Security change
The operator SSH public key is no longer embedded in the script. This repository is public, and publishing which key is authorized as Administrator on the acceptance host is needless disclosure. It is now passed at build time (or via
HELM_ACCEPTANCE_AUTHORIZED_KEY), staged onto the bootstrap media asauthorized_key.pub, andsetup.ps1refuses to continue without a valid OpenSSH public key. A.gitignorekeeps local copies out of git.Two host constraints now documented
qm snapshot <vmid> probebefore provisioning.setuphost.exe) does not auto-applyAutounattend.xmlfrom secondary media and ignoressetup.exe /unattend:. The answer file is readable from WinPE, just not consumed. The README documents the offlinedism /apply-imagepath, which still runs the specialize, oobeSystem and auditUser passes.Verification
node --test test/phase4-platform-acceptance.mjs— 8/8 passbash -n build-unattend-iso.sh— cleanScope
No release, tag, version bump, website deploy, Stable promotion, or production change.
HELM_PHASE4_WINDOWS_ENABLEDremains unset, so the Windows lane stays skipped and recorded as blocked.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation