Deep Review: 20260519-100641-pr-4985

<?xml version="1.0" encoding="utf-8"?>
<!-- Windows unattended answer file for Lima guest VMs (amd64).
     Ref: https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/update-windows-settings-and-scripts-create-your-own-answer-file-sxs -->
<unattend xmlns="urn:schemas-microsoft-com:unattend">

  <!-- windowsPE: locale, disk layout, image selection, hw bypass -->

Neither autounattend.xml contains a {{ directive, yet both pass through textutil.ExecuteTemplate in ExecuteTemplateAutounattend (pkg/cidata/template.go:241). The rendered output equals the file on disk byte for byte. TemplateArgs.SSHPubKeys, User, Hostname, and TimeZone are populated but never reach the guest. The PR's claim that "OpenSSH server is installed and started during OOBE so Lima's host-agent can connect" therefore fails — ssh-rsa PLACEHOLDER lands in administrators_authorized_keys and the local account is hardcoded to lima.

	xmlTemplate, err := templateFS.ReadFile(path.Join(root, "autounattend.xml"))
	if err != nil {
		return nil, err
	}

	return textutil.ExecuteTemplate(string(xmlTemplate), args)
}

Fix: render the SSH keys via a PowerShell array literal inside the existing <CommandLine> (careful quoting needed across the XML and PowerShell layers), and use {{ .Hostname | printf "%.15s" }} for <ComputerName> (NetBIOS limit), {{ .User }} for <Name>/<Username>, and {{ .TimeZone }} for <TimeZone> with an IANA→Windows mapping table (Windows expects "Pacific Standard Time", not "America/Los_Angeles").

          <CommandLine>powershell.exe -NoProfile -Command "Set-Content -Path 'C:\ProgramData\ssh\administrators_authorized_keys' -Value 'ssh-rsa PLACEHOLDER'; icacls 'C:\ProgramData\ssh\administrators_authorized_keys' /inheritance:r /grant 'SYSTEM:(F)' /grant 'Administrators:(F)'"</CommandLine>
          <Description>Write authorized_keys with correct ACLs</Description>
        </SynchronousCommand>
        <SynchronousCommand wcm:action="add">
          <Order>5</Order>
          <CommandLine>powershell.exe -NoProfile -Command "$cfg = 'C:\ProgramData\ssh\sshd_config'; (Get-Content $cfg) -replace '#?AuthorizedKeysFile.*', 'AuthorizedKeysFile .ssh/authorized_keys' | Set-Content $cfg; Add-Content $cfg 'Match Group administrators`r`n  AuthorizedKeysFile C:/ProgramData/ssh/administrators_authorized_keys'; Restart-Service sshd"</CommandLine>
          <Description>Configure sshd for admin key auth</Description>
        </SynchronousCommand>
        <SynchronousCommand wcm:action="add">
          <Order>6</Order>
          <CommandLine>reg.exe add "HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Winlogon" /v AutoAdminLogon /t REG_SZ /d 0 /f</CommandLine>

Two compounding problems:

1. PowerShell single-quoted strings are literal, so rn inside Add-Content's argument writes four literal characters (backtick, r, backtick, n) instead of CRLF. sshd parses the resulting one-line Match block as malformed and silently ignores it.
2. The -replace '#?AuthorizedKeysFile.*' regex is unanchored. Windows OpenSSH ships sshd_config with a default Match Group administrators block whose AuthorizedKeysFile line points at __PROGRAMDATA__/ssh/administrators_authorized_keys. The replacement rewrites that line too, so even with problem (1) fixed, sshd no longer consults the path the script claims to populate.

Combined with I1, both halves of the SSH-key plumbing are broken.

Fix: use a double-quoted string for the Add-Content argument (escape XML quotes with ") and anchor the regex with ^. A simpler alternative overwrites sshd_config from a PowerShell array, removing the dependency on the existing file's layout.

	args, err := templateArgs(ctx, true, instDir, name, instConfig, udpDNSLocalPort, tcpDNSLocalPort, vsockPort, virtioPort, noCloudInit, rosettaEnabled, rosettaBinFmt)
	if err != nil {
		return "", err
	}

	// Windows guests use autounattend.xml instead of cloud-init.
	if *instConfig.OS == limatype.WINDOWS {
		var winArch string
		if instConfig.Arch != nil && *instConfig.Arch == limatype.AARCH64 {
			winArch = "arm64"
		} else {
			winArch = "amd64"
		}
		return generateWindowsISO(args, instDir, winArch)
	}

	if err := ValidateTemplateArgs(args); err != nil {
		return "", err
	}

The Windows branch in cidata.go only changes the cidata payload. The QEMU driver still attaches the disk as virtio, the cidata ISO via virtio-scsi, and the NIC as virtio-net-pci, while the amd64 template's own comment says the disk is SATA/AHCI and the arm64 template uses NVMe. Without inbox virtio drivers in WinPE, the installer cannot see the disk during windowsPE, so installation fails before reaching oobeSystem. This is why the PR's testing section invokes QEMU directly with hand-tuned device arguments rather than limactl start.

Fix: add a Windows path in the QEMU driver matching the templates — SATA/AHCI on amd64, NVMe on arm64, usb-storage for both ISOs, USB keyboard/tablet, and ramfb on arm64. The dev tool's command lines in hack/gen-autounattend-iso.go are the right shape to copy. The alternative (embed virtio-win.iso plus a <DriverPaths> block) carries a license question that NVMe avoids.

	GuestAgent          string
	NerdctlArchiveCache string
}

// Prepare ensures the disk, the nerdctl archive, etc.
func Prepare(ctx context.Context, inst *limatype.Instance, guestAgent string) (*Prepared, error) {
	var needsGuestAgent bool
	switch *inst.Config.OS {
	case limatype.DARWIN:
		// macOS guests always need the guest agent for running fake-cloud-init
		needsGuestAgent = true
	case limatype.FREEBSD:
		// guest agent is not implemented for FreeBSD yet
		needsGuestAgent = false
	default:
		needsGuestAgent = !*inst.Config.Plain
	}
	if needsGuestAgent && guestAgent == "" {
		var err error
		guestAgent, err = usrlocal.GuestAgentBinary(*inst.Config.OS, *inst.Config.Arch)
		if err != nil {
			return nil, err
		}
	}
	limaDriver, err := driverutil.CreateConfiguredDriver(inst, 0)
	if err != nil {

Validate now accepts Windows, but the Prepare switch has no Windows case; the default branch sets needsGuestAgent = true (because Plain is false). Prepare then calls usrlocal.GuestAgentBinary("Windows", arch), which searches for lima-guestagent.Windows-x86_64 (or -aarch64). The Makefile only builds Linux and Darwin guest agents — no lima-guestagent.Windows-* exists — so limactl start --os Windows fails inside Prepare with a confusing guest agent binary could not be found error before the driver ever launches.

Fix:

 case limatype.FREEBSD:
     needsGuestAgent = false
+case limatype.WINDOWS:
+    needsGuestAgent = false
 default:
     needsGuestAgent = !*inst.Config.Plain

A more complete fix gates the whole Windows path behind a "not yet startable via limactl" validation error until I3 lands.

		return "", err
	}

	// Windows guests use autounattend.xml instead of cloud-init.
	if *instConfig.OS == limatype.WINDOWS {
		var winArch string
		if instConfig.Arch != nil && *instConfig.Arch == limatype.AARCH64 {
			winArch = "arm64"
		} else {
			winArch = "amd64"
		}
		return generateWindowsISO(args, instDir, winArch)
	}

	if err := ValidateTemplateArgs(args); err != nil {
		return "", err
	}

limatype.ArchTypes includes x86_64, aarch64, armv7l, ppc64le, riscv64, s390x. The PR's validation change accepts any of them for os: Windows. A user writing os: Windows, arch: riscv64 passes validation and silently gets an amd64 answer file; the failure surfaces deep in the install process when Windows refuses to boot the wrong-arch image.

Fix: reject the invalid combination at validation, catching the error before any ISO is generated:

 case limatype.LINUX, limatype.DARWIN, limatype.FREEBSD, limatype.WINDOWS:
+    if *y.OS == limatype.WINDOWS && *y.Arch != limatype.X8664 && *y.Arch != limatype.AARCH64 {
+        errs = errors.Join(errs, fmt.Errorf("field `arch` for `os: Windows` must be %q or %q; got %q", limatype.X8664, limatype.AARCH64, *y.Arch))
+    }

Make ExecuteTemplateAutounattend's arch switch exhaustive afterward so a future caller cannot reintroduce the silent fallthrough.

	args, err := templateArgs(ctx, true, instDir, name, instConfig, udpDNSLocalPort, tcpDNSLocalPort, vsockPort, virtioPort, noCloudInit, rosettaEnabled, rosettaBinFmt)
	if err != nil {
		return "", err
	}

	// Windows guests use autounattend.xml instead of cloud-init.
	if *instConfig.OS == limatype.WINDOWS {
		var winArch string
		if instConfig.Arch != nil && *instConfig.Arch == limatype.AARCH64 {
			winArch = "arm64"
		} else {
			winArch = "amd64"
		}
		return generateWindowsISO(args, instDir, winArch)
	}

	if err := ValidateTemplateArgs(args); err != nil {
		return "", err
	}

The Windows branch returns from GenerateISO9660 immediately. It does not run drv.BootScripts(), does not loop over instConfig.Provision, and does not include any Mount entries. A user copying their existing Linux lima.yaml (with mounts:, provision:, or driver-supplied boot scripts) gets no warning that those blocks are ignored. The trap matters most for provision, where users read their scripts in the YAML and expect them to run.

Fix: reject the unsupported blocks at validation. Name the unsupported field in the error, not the OS, so the user knows what to remove:

+if *y.OS == limatype.WINDOWS {
+    if len(y.Provision) > 0 {
+        errs = errors.Join(errs, errors.New("field `provision` is not yet supported for Windows guests"))
+    }
+    if len(y.Mounts) > 0 {
+        errs = errors.Join(errs, errors.New("field `mounts` is not yet supported for Windows guests"))
+    }
+}

Lift restrictions one at a time as I3's integration work lands.

}

// GenerateISO9660 generates the cidata ISO9660 image (or directory, for noCloudInit)
// in instDir. It returns the instance ID, which changes on every boot.
func GenerateISO9660(ctx context.Context, drv driver.Driver, instDir, name string, instConfig *limatype.LimaYAML, udpDNSLocalPort, tcpDNSLocalPort int, guestAgentBinary, nerdctlArchive string, vsockPort int, virtioPort string, noCloudInit, rosettaEnabled, rosettaBinFmt bool) (string, error) {
	args, err := templateArgs(ctx, true, instDir, name, instConfig, udpDNSLocalPort, tcpDNSLocalPort, vsockPort, virtioPort, noCloudInit, rosettaEnabled, rosettaBinFmt)
	if err != nil {
		return "", err
	}

	// Windows guests use autounattend.xml instead of cloud-init.
	if *instConfig.OS == limatype.WINDOWS {
		var winArch string
		if instConfig.Arch != nil && *instConfig.Arch == limatype.AARCH64 {
			winArch = "arm64"
		} else {
			winArch = "amd64"
		}
		return generateWindowsISO(args, instDir, winArch)
	}

	if err := ValidateTemplateArgs(args); err != nil {
		return "", err
	}

templateArgs() dereferences Linux-specific fields before Windows can peel off: *instConfig.MountType, User.Home/Shell/UID, GuestInstallPrefix, HostResolver.Enabled, the 9p msize parser, sshutil.DefaultPubKeys, osutil.DNSAddresses. None reaches the autounattend ISO; the Windows path uses only Name, Hostname, User, SSHPubKeys, TimeZone, and IID.

Two consequences: a host with no SSH key, an unreadable resolv.conf, or a 9p mount with an unparseable msize aborts Windows ISO generation with an unrelated Linux-pipeline error; and Windows depends on the Unix pipeline staying happy, the opposite of the intended decoupling. The TODO: Split into per-OS template args (Unix vs Windows) at pkg/cidata/template.go:89 already flags this; I7 promotes it from paper TODO to concrete blocker for I1.

	FSType string
	FSArgs []string
}

// TemplateArgs holds all data needed to render provisioning templates.
// TODO: Split into per-OS template args (Unix vs Windows) to avoid carrying irrelevant fields.
type TemplateArgs struct {
	Debug                           bool
	OS                              limatype.OS
	Name                            string // instance name
	Hostname                        string // instance hostname

Fix: short-circuit the OS check before templateArgs, and build a Windows-only windowsTemplateArgs that supplies only the fields the Windows path uses.