diff --git a/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md b/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md new file mode 100644 index 0000000..e340cae --- /dev/null +++ b/docs/superpowers/specs/2026-08-13-windows-agent-parity-design.md @@ -0,0 +1,285 @@ +# Windows agent parity: OS updates and workloads + +Date: 2026-08-13 + +## Goal + +Bring the Windows agent up to the Linux agent on two subsystems: OS update +check/apply, and the workload registry (collection, control, logs). Everything +else about the Windows agent stays as it is. + +Out of scope, deliberately: + +- **Package inventory and CVE findings.** `trivy-db` carries no Windows feed, so + a Windows finding needs a different source, a different matcher and a + different version comparison. That is its own project, and until it exists a + Windows host correctly reports `status: unsupported` rather than "0 findings". +- **SSH key management on Windows.** `administrators_authorized_keys` is a real + possibility but a separate decision. +- **winget.** Third-party app upgrades are a different question from OS + patching, and winget is absent on Server Core and older builds. + +## Current state + +The Windows agent registers, heartbeats, reports inventory, runs workflow steps +through PowerShell, relays console connections and self-updates via MSI. Four +gates stop it doing more: + +| Gate | Location | +| --- | --- | +| `runtime.GOOS != "linux"` early return | `agent/internal/workloads/workloads.go`, `agent/internal/sync/workloads.go` (twice) | +| package-manager detection finds nothing | `agent/internal/updates/updates.go` (`detectPM`) | +| hard error | `agent/internal/packages/packages.go` (out of scope here) | +| `authorized_keys` write skipped | `agent/internal/sync/sync.go` (out of scope here) | + +## Approach + +The platform split moves into the agent, expressed as build tags following the +existing `inventory/collect_linux.go` / `collect_windows.go` / +`collect_other.go` precedent. The control plane stays OS-blind: `ReportWorkloads`, +`ControlWorkload`, `WorkloadLogs` and `ApplyUpdates` need no changes at all, +because a Windows service is reported as the same `unit` kind a systemd service +is. + +Build tags rather than `runtime.GOOS` switches so PowerShell command strings do +not ship in the Linux binary, and so a platform left unimplemented is a compile +error rather than a silent no-op at runtime. + +## Updates + +### Layout + +``` +agent/internal/updates/ + updates.go # PackageUpdate; CheckAvailable/ApplyAll declared once + updates_linux.go # existing detectPM, checkApt/DnfYum/Pacman/Zypper/Apk, ApplyAll + updates_windows.go # Windows Update COM, driven through PowerShell + updates_other.go # //go:build !linux && !windows — no-ops +``` + +`updates_other.go` carries the build constraint for the same reason +`inventory/collect_other.go` does: `_other` is not a GOOS suffix, so without the +constraint the file compiles everywhere and collides. + +### Checking + +One PowerShell invocation, `-NoProfile -NonInteractive`, emitting JSON: + +```powershell +$searcher = (New-Object -ComObject Microsoft.Update.Session).CreateUpdateSearcher() +$result = $searcher.Search("IsInstalled=0 and Type='Software' and IsHidden=0") +``` + +The Windows Update COM API is used rather than the `PSWindowsUpdate` module +because it is present on every supported Windows, needs no PowerShell Gallery +install, and works unchanged against a WSUS server on an air-gapped fleet. The +agent runs as `LocalSystem`, which has the rights the API requires. + +Each result maps to a `PackageUpdate`: + +| Field | Value | +| --- | --- | +| `Name` | the update Title | +| `CurrentVersion` | empty | +| `NewVersion` | the KB article ID, e.g. `KB5034123` | + +`CurrentVersion` is empty because a Windows update is not a version bump of a +named package, and inventing a current version would put a wrong string in front +of an operator. The KB ID goes in `NewVersion` because it is the identifier +people actually search for. + +Timeout: 10 minutes. The first search after a boot contacts Microsoft Update and +is routinely slow. + +### Applying + +The same COM session: `CreateUpdateDownloader` then `CreateUpdateInstaller`, +over the updates returned by the search above, skipping any that require user +input. EULAs are accepted programmatically; an update whose EULA cannot be +accepted is skipped rather than failing the batch. + +The operation fails when the installer's `ResultCode` is not 2 (succeeded) or 3 +(succeeded with errors). + +Timeout: 60 minutes. A patch-Tuesday cumulative genuinely takes that long, and +the Linux path's existing 5-minute cap is already tight. + +**The agent never reboots the host.** A control plane silently restarting a +production server is unrecoverable from the UI, and the reboot is a decision a +person or a workflow makes. Instead the need for one is reported. + +### Reboot required + +A new field `reboot_required` on `InventoryReport`, added to +`proto/vantage/v1/vantage.proto` and to both hand-written `pb` copies +(`agent/internal/grpc/pb`, `server/internal/grpc/pb`) in the same commit. + +It travels on the inventory report rather than the update report because it is a +host property like the kernel version, and inventory refreshes every 30 seconds +with a full static snapshot every 15 minutes — so a host rebooted by hand clears +the flag promptly instead of showing it for up to an hour. + +Both platforms set it, since parity is free here: + +- Linux: `/var/run/reboot-required` exists, or `dnf needs-restarting -r` exits + non-zero. +- Windows: the `Microsoft.Update.SystemInfo` COM object's `RebootRequired` + property, falling back to the pending-reboot registry keys + (`Component Based Servicing\RebootPending`, + `WindowsUpdate\Auto Update\RebootRequired`, + `Session Manager\PendingFileRenameOperations`). + +`services.ReportInventory` stores it on `servers.inventory`. + +## Workloads + +### Layout + +``` +agent/internal/workloads/ + workloads.go # Result, Collect, Hash — Collect calls collectUnits + docker.go # unchanged, shared: shells to the docker binary + systemd_linux.go # was systemd.go + services_windows.go # new: Win32_Service collection + control.go # shared validation; platform halves split out + control_linux.go # docker/systemctl, /proc/self/cgroup own-container check + control_windows.go # Start/Stop/Restart-Service, VantageAgent protection + logs.go # shared: capLog, MaxLogLines, MaxLogBytes + logs_linux.go # docker logs / journalctl + logs_windows.go # docker logs / Get-WinEvent +``` + +`Collect` loses its `runtime.GOOS != "linux"` return and calls +`collectUnits(ctx)`, which is the systemd collector on Linux and the service +collector on Windows. `runWorkloads` and `reportWorkloads` in +`agent/internal/sync/workloads.go` lose all three of their platform returns. + +`docker.go` stays shared and ungated. It shells to the `docker` binary, which +behaves identically on Windows, so a Docker Desktop or Mirantis host reports its +containers with no new code. `DockerOK` / `DockerError` keep their existing +three-state meaning: not installed (the common case, not a fault), installed but +not responding, and running nothing. + +### Collecting Windows services + +`Get-CimInstance Win32_Service` converted to JSON — not `Get-Service`, which +exposes neither `PathName` nor `StartMode`, and the filter needs both. + +A service is reported when its executable does **not** resolve under +`%SystemRoot%\System32`, and it is running, failed, or has `StartMode=Auto` +while stopped. This mirrors the systemd collector's intent: show what an +operator installed, and show what is meant to be up but is not. + +Path parsing strips surrounding quotes and trailing arguments before the +`%SystemRoot%` comparison. `"C:\Program Files\X\x.exe" -service` is one path +with one argument, and splitting naively on whitespace misfiles a substantial +share of a real fleet. + +Field mapping: + +| Workload field | Source | +| --- | --- | +| `Kind` | `"unit"` | +| `ID` | `Name` (the service name) | +| `Name` | `DisplayName` | +| `State` | `running` / `stopped` / `failed`, from `State` plus `ExitCode` | +| `Health`, `Image`, `Stack`, `Ports`, `Restarts` | unset | + +`Kind: "unit"` and the existing `systemd_ok` / `systemd_error` fields are reused +rather than a `service` kind and `services_ok` fields being added. That would +cost a proto change, both pb copies, the server model, the service layer and the +web client, and would teach every existing consumer a second kind — to describe +the same thing. The naming is corrected where it is read, in the UI, which knows +the server's OS. + +`State` values match the ones the UI already colours, so no web change is needed +for the rows themselves. + +### Protection + +The protected set stays computed and enforced agent-side, as it is on Linux: the +control plane may name a target, but the agent decides what it will do to +itself. + +On Windows the protected workload is the `VantageAgent` service — the NSSM +service name written by `installer/setup.ps1` — matched case-insensitively, +because Windows service names are. `detectOwnContainer` and its +`/proc/self/cgroup` read move to `control_linux.go`; the Windows build returns +no own-container ID. + +`ErrProtected` still surfaces as HTTP 409 from the API, and the reported +`Protected` flag remains a courtesy that greys the button rather than the +boundary. + +### Control + +`Start-Service`, `Stop-Service -Force`, `Restart-Service -Force`, under the same +90-second `controlTimeout`, with the error text taken from PowerShell's stderr. + +`sc.exe` is avoided because it returns before the operation completes, which +turns a timeout into a false success. `-Force` is required because +`Stop-Service` without it refuses when other services depend on the target, and +that refusal reads to an operator as a silent no-op. + +### Logs + +`Get-WinEvent` with a filter hashtable over the `System` and `Application` logs, +provider names matching the service name, its display name, and +`Service Control Manager`, newest first, capped by the requested tail. + +Each event is formatted as ` `, which is +the same shape `journalctl --output=short-iso` produces, so the log dialog needs +no per-platform rendering. + +Service Control Manager logs every service on the host under one provider, so +its events are filtered client-side to those naming the target service. + +`capLog` is shared and unchanged: 500 lines and 256KB, whichever binds first, +trimmed from the front. There is still no follow mode. + +An empty result returns an empty string and no error. A service that has logged +nothing is normal, and an error there would read as a broken feature. + +## Server and web + +The server changes in one place: `services.ReportInventory` persists +`reboot_required`. + +The web changes in three, all keyed on `server.os_type`, which is already stored +on the server document and already serialised, but currently unread by `web/`: + +1. `web/components/workloads/WorkloadList.tsx` — the systemd status lines become + platform-worded. On Windows the error line reads "Windows services could not + be read" and the "systemd is not in use on this server" line is not rendered + at all. The empty-state line drops "on Linux only". The Docker lines are + unchanged. +2. Server detail — a `Reboot required` pill beside the update count when the + flag is set, placed with the update panel because that is what caused it. +3. The Updates panel's Windows copy describes a list of KB articles rather than + package upgrades, since `current_version` is empty on that platform. + +## Testing + +The Windows collectors are, in substance, parsers of PowerShell output. Parsing +is separated from invocation and table-tested against captured real output, +following `agent/internal/packages/parse.go`: + +- `Win32_Service` JSON, including a quoted path with arguments, a + `%SystemRoot%\System32` service that must be filtered out, a stopped + `StartMode=Auto` service that must be kept, and a failed service with a + non-zero `ExitCode`. +- Update searcher JSON, including an update with no KB ID. +- `Get-WinEvent` JSON, including a Service Control Manager event for another + service that must be filtered out. +- Pending-reboot detection from registry key presence. + +`capLog` and `Hash` are unchanged and gain no tests. + +The invocation halves are verified by hand on a Windows host: check, apply, +service start/stop/restart, a protected refusal on `VantageAgent`, and logs on +both a chatty service and a silent one. + +`GOOS=windows go build ./...` and `GOOS=linux go build ./...` both belong in the +implementation plan as explicit steps — a build-tag split is exactly the change +that compiles on the machine you are sitting at and nowhere else. CI already +cross-builds the agent on release, so no workflow change is needed.