From 799c155d94c501133ed300efcbe8107d34f58fba Mon Sep 17 00:00:00 2001 From: jpetree331 Date: Sun, 29 Mar 2026 08:22:18 -0400 Subject: [PATCH] docs: C-1 Windows-specific bugs audit Comprehensive audit of Windows agent code: winget detection, Windows Update ghost updates, service wrapper, HWID, and vendored windowsupdate package. Key findings: - F-C1-1 HIGH: Winget not found as SYSTEM (PATH-only search) - F-C1-3 HIGH: No post-install verification (ghost updates) - F-C1-5 HIGH: Windows service has duplicated polling loop missing B-2 fixes (jitter cap, exponential backoff) - F-C1-2 MEDIUM: Fragile winget text parser - F-C1-4 MEDIUM: No service auto-restart on crash 9 findings total. See docs/C1_Windows_Audit.md for details. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/C1_Windows_Audit.md | 205 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 docs/C1_Windows_Audit.md diff --git a/docs/C1_Windows_Audit.md b/docs/C1_Windows_Audit.md new file mode 100644 index 0000000..9c7eac9 --- /dev/null +++ b/docs/C1_Windows_Audit.md @@ -0,0 +1,205 @@ +# C-1 Windows-Specific Bugs Audit + +**Date:** 2026-03-29 +**Branch:** culurien +**Scope:** Winget detection, ghost updates, service wrapper, HWID, vendored package + +--- + +## 1. WINDOWS-SPECIFIC FILES + +| File | Build Tag | Purpose | +|------|-----------|---------| +| `scanner/winget.go` | none (cross-platform) | Winget package update scanning | +| `scanner/windows_wua.go` | `//go:build windows` | WUA COM API scanning | +| `scanner/windows_override.go` | `//go:build windows` | Type alias for WUA scanner | +| `scanner/windows.go` | `//go:build !windows` | Stub (non-Windows) | +| `service/windows.go` | `//go:build windows` | Windows service wrapper | +| `system/windows.go` | `//go:build windows` | System info collection | +| `system/windows_stub.go` | (no tag — always compiles) | Stub for non-Windows | +| `installer/windows.go` | none (cross-platform) | Windows Update installer | +| `installer/winget.go` | none (cross-platform) | Winget package installer | +| `pkg/windowsupdate/*` | none (20 files) | Vendored WUA COM bindings | + +--- + +## 2. WINGET DETECTION & SCANNING + +### 2a. Winget Location + +`scanner/winget.go:41`: `exec.LookPath("winget")` — searches PATH only. + +**F-C1-1 HIGH: Winget not found when running as SYSTEM service.** +`exec.LookPath` searches the PATH environment variable. When the agent runs as SYSTEM via the Windows service, the PATH does not include the per-user `%LOCALAPPDATA%\Microsoft\WindowsApps\` directory where winget is typically installed. The scanner will always report "winget is not available" when running as a service. + +Known winget install locations NOT checked: +- `%LOCALAPPDATA%\Microsoft\WindowsApps\winget.exe` (per-user) +- `%PROGRAMFILES%\WindowsApps\Microsoft.DesktopAppInstaller_*\winget.exe` (system-wide) + +### 2b. Command Used + +`scanner/winget.go:90`: `winget list --outdated --accept-source-agreements --output json` + +Fallback: `winget list --outdated --accept-source-agreements` (text output, line 126) + +### 2c. Output Parsing + +- **Primary**: JSON parsing via `json.Unmarshal` into `[]WingetPackage` (line 104) +- **Fallback**: Text parsing via `strings.Fields` (line 149) + +**F-C1-2 MEDIUM: Fragile text parser.** The fallback text parser at line 149 uses `strings.Fields(line)` and assumes `fields[0]` = name, `fields[1]` = version, `fields[2]` = available. Winget table output has variable-width columns with spaces IN package names (e.g., "Microsoft Visual Studio Code"). This parser will split "Microsoft Visual Studio Code 1.85.0 1.86.0" into 6 fields, misidentifying the name as just "Microsoft". + +### 2d. Data Structure + +`WingetPackage` struct (line 15-23): Name, ID, Version, Available, Source, IsPinned, PinReason. + +### 2e. Edge Cases + +- **No updates**: JSON returns `[]` → empty result, correct. +- **Format change**: JSON output change would cause `json.Unmarshal` error → falls back to text parser → likely misparses. +- **UAC prompt**: `--accept-source-agreements` flag suppresses most prompts. But `--output json` flag was added in winget v1.6+ — older versions will fail. +- **SYSTEM account**: See F-C1-1. + +### 2f. Tests + +No winget parsing tests exist in the codebase. + +--- + +## 3. WINDOWS UPDATE SCANNING (GHOST UPDATES) + +### 3a-3c. COM Interfaces + +Uses vendored `pkg/windowsupdate/` package (originally by Zheng Dayu, Apache 2.0 license). + +- `IUpdateSession` → `CreateUpdateSearcher()` +- `IUpdateSearcher` → `Search("IsInstalled=0 AND IsHidden=0")` +- `ISearchResult` → `Updates` collection +- Each `IUpdate` has: Title, Description, Identity, MsrcSeverity, Categories, KBArticleIDs, SecurityBulletinIDs, IsInstalled, IsHidden, IsMandatory, etc. + +### 3d. Installation Flow + +`installer/windows.go` uses PowerShell (`Install-WindowsUpdate`) or `wuauclt /detectnow` + `wuauclt /installnow`. + +**F-C1-3 HIGH: No post-install re-scan with state verification.** +After installation, the agent does NOT re-scan to verify `IsInstalled=1`. The next scan cycle uses `IsInstalled=0 AND IsHidden=0` which may still return the update if Windows hasn't committed the install state yet (common after reboot-pending updates). + +### 3e-3f. Timing Issue + +The ghost update bug is a timing issue: +1. Agent installs update via PowerShell/wuauclt +2. Agent immediately re-scans on next polling cycle (5 seconds in rapid mode) +3. Windows Update has not yet committed the install state +4. `IsInstalled=0` still returns true for the just-installed update +5. Agent reports it as "available" again + +**Root cause**: No delay or state verification between install and next scan. No `IsInstalled` check post-install. + +### 3g. IsInstalled / IsHidden + +The search criteria `IsInstalled=0 AND IsHidden=0` is correct for finding available updates. But after installation, the `IsInstalled` flag transitions asynchronously — especially for updates requiring a reboot. During the reboot-pending window, `IsInstalled` may still be `0`. + +### 3h. Vendored Package Modifications + +The vendored package appears to be the original Zheng Dayu library with additions: +- `QueryHistoryAll()` was added (not in the original) +- Additional fields on `IUpdate` (SecurityBulletinIDs, MsrcSeverity, etc.) +- No modifications to core COM interaction logic + +--- + +## 4. WINDOWS SERVICE WRAPPER + +### 4a. Framework + +`golang.org/x/sys/windows/svc` — official Go Windows service package. Uses `svc.Run()` for service lifecycle. + +### 4b. Service Account + +Runs as SYSTEM (default for `sc.exe` created services). The install function at `service/windows.go` uses `mgr.Config{StartType: mgr.StartAutomatic}` without specifying a user account. + +### 4c. Permission Issues + +- **Windows Update COM**: SYSTEM CAN access WUA APIs — this works. +- **Winget**: SYSTEM CANNOT access per-user winget installation — see F-C1-1. +- **C:\ProgramData\RedFlag\**: SYSTEM has full access — this works. + +### 4d. Service Installation + +`service/windows.go` contains `InstallService()` and `RemoveService()` functions using `mgr.Connect()` → `mgr.CreateService()`. Agent binary provides `--install-service` and `--remove-service` CLI flags. + +### 4e. Crash Recovery + +**F-C1-4 MEDIUM: No auto-restart on service crash.** The service is created with `mgr.Config{StartType: mgr.StartAutomatic}` but no recovery options (FailureActions). If the service crashes, it stays stopped until manually restarted or system rebooted. + +### F-C1-5 HIGH: Windows service has duplicated polling loop. + +`service/windows.go:138-178` contains a COMPLETE COPY of the agent polling loop (`runAgent()`). This is a separate implementation from `cmd/agent/main.go`. The B-2 fixes (proportional jitter F-B2-5, exponential backoff F-B2-7) were applied to `cmd/agent/main.go` but NOT to `service/windows.go:runAgent()`. The Windows service still has the old fixed 30-second jitter (line 178) and no exponential backoff. + +--- + +## 5. WINDOWS MACHINE ID (HWID) + +### 5a. HWID Source + +`system/machine_id.go:80-88`: Uses `machineid.ID()` from `denisbrodbeck/machineid` library. On Windows, this reads `HKLM\SOFTWARE\Microsoft\Cryptography\MachineGuid` from the registry. + +### 5b. Hashing + +YES — `hashMachineID(id)` returns SHA256 hex (line 37-39). Same as Linux. + +### 5c. HWID Change + +If MachineGuid changes (VM clone, sysprep, registry corruption), the agent gets a different machine ID → MachineBindingMiddleware rejects it → agent must re-register. + +### 5d. WMI Unavailability + +Not applicable — the library uses registry, not WMI. If the registry key is missing, `machineid.ID()` fails → falls back to `generateGenericMachineID()` (hostname-based). + +### 5e. Cross-Platform Consistency + +Uses the same `GetMachineID()` function. Windows fallback is simpler than Linux (just retries `machineid.ID()`, no additional registry keys tried). Same issue flagged in DEV-024. + +--- + +## 6. CROSS-PLATFORM CONSISTENCY + +### 6a. Update Schema + +Both Windows and Linux produce `client.UpdateReportItem` with the same struct. Package type differentiators: `"winget"`, `"windows_update"`, `"apt"`, `"dnf"`. + +### 6b. Machine ID Format + +Both produce SHA256 hex strings (64 chars). Consistent. + +### 6c. OS Detection + +`Agent.OSType` is set during registration from `runtime.GOOS`. Server stores it in `agents.os_type` column with CHECK constraint: `('windows', 'linux', 'macos')`. + +### 6d. Config Paths + +`constants/paths.go`: Windows uses `C:\ProgramData\RedFlag\`, Linux uses `/etc/redflag/`. Handled via `runtime.GOOS` switch. Correct. + +--- + +## 7. ETHOS VIOLATIONS + +- **ETHOS #1**: Winget scanner uses `fmt.Printf` for error output (lines 59, 67, 72-77, etc.) instead of structured logging. Not using `[TAG] [system] [component]` format. +- **ETHOS #1**: Windows service `runAgent()` uses emojis in log messages (lines 139-144). +- **ETHOS #3**: `installViaWuauclt` (installer/windows.go:127) runs `wuauclt /detectnow` followed by `wuauclt /installnow` with a fixed 10-second sleep between them, assuming detection completes in 10 seconds. + +--- + +## FINDINGS SUMMARY + +| ID | Severity | Finding | Location | +|----|----------|---------|----------| +| F-C1-1 | HIGH | Winget not found when running as SYSTEM (searches PATH only, not known install locations) | scanner/winget.go:41 | +| F-C1-2 | MEDIUM | Winget text fallback parser splits on whitespace, breaks on package names with spaces | scanner/winget.go:149 | +| F-C1-3 | HIGH | No post-install state verification for Windows Updates — causes ghost updates | scanner/windows_wua.go:58, installer/windows.go | +| F-C1-4 | MEDIUM | Windows service has no auto-restart on crash (no FailureActions set) | service/windows.go (InstallService) | +| F-C1-5 | HIGH | Windows service runAgent() is a duplicated polling loop missing B-2 fixes (jitter, backoff) | service/windows.go:138-178 | +| F-C1-6 | LOW | Winget scanner uses fmt.Printf instead of structured logging (ETHOS #1) | scanner/winget.go:59,67,72 | +| F-C1-7 | LOW | Windows service runAgent() uses emojis in log messages (ETHOS #1) | service/windows.go:139-144 | +| F-C1-8 | LOW | No winget parsing tests in codebase | scanner/winget.go | +| F-C1-9 | LOW | Windows HWID fallback is simpler than Linux (only retries machineid.ID, no registry key exploration) | system/machine_id.go:80-88 |