diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index b8ba4adb6e..a528906e01 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -8126,3 +8126,25 @@ admission, token binding or removal-block behaviour. Focused proof lives in `internal/monitoring/physical_disk_roundtrip_test.go` (`TestMergeHostAgentSMARTIntoDisks_AgentWearoutDoesNotHideLowPVELife` and `TestMergeHostAgentSMARTIntoDisks_AgentWearoutFillsUnreportedPVELife`). + +### Windows braced MachineGuid does not abort agent startup + +The Windows unified agent resolves host information through gopsutil's combined +`InfoWithContext`, which is fatal if any single field fails. gopsutil v4.26.6 +reads `HKLM\SOFTWARE\Microsoft\Cryptography\MachineGuid` through a fixed 74-byte +registry buffer and rejects any value that is not exactly 36 characters, so a +braced 38-character GUID returns `ERROR_MORE_DATA` ("More data is available") +and the whole call fails. Pulse propagated that error from `newAgent` and the +agent exited at startup (#2125). Host collection now retries through a Windows +recovery path when the host-ID read itself fails: it reads `MachineGuid` +directly with a correctly sized buffer, strips the optional braces, and fills +the remaining fields from gopsutil's individual accessors. A failure of any +other field remains fatal so unrelated faults are not hidden. Machine +identities are canonicalised by removing surrounding braces and lower-casing, +so a braced GUID and its bare form resolve to the same stable agent ID; the +change does not alter enrollment, token binding, removal or command authority. +Focused proof lives in `internal/hostagent/agent_new_test.go` +(`TestNormalizeMachineGUID`, `TestDefaultCollectorHostInfoRecoversWindowsHostIDFailure`, +`TestDefaultCollectorHostInfoKeepsErrorWhenUnrecovered` and +`TestGetReliableMachineIDNormalizesWindowsBraces`); the Windows recovery file is +cross-compiled with `GOOS=windows go build ./internal/hostagent/`. diff --git a/internal/hostagent/agent.go b/internal/hostagent/agent.go index 1ed5bad135..1820eb9328 100644 --- a/internal/hostagent/agent.go +++ b/internal/hostagent/agent.go @@ -2576,6 +2576,15 @@ func GetReliableMachineID(c SystemCollector, gopsutilHostID string, logger zerol } } + // Windows reports MachineGuid with or without surrounding braces depending + // on the source; canonicalise it so a braced GUID resolves to the same + // stable identity as its bare form (issue #2125). + if c.GOOS() == "windows" { + if normalized := normalizeMachineGUID(gopsutilID); normalized != "" { + return normalized + } + } + return gopsutilID } diff --git a/internal/hostagent/agent_new_test.go b/internal/hostagent/agent_new_test.go index bfa4e99348..52650d07fe 100644 --- a/internal/hostagent/agent_new_test.go +++ b/internal/hostagent/agent_new_test.go @@ -1393,3 +1393,75 @@ func TestNormaliseRuntimePlatformUsesBuildTarget(t *testing.T) { }) } } + +func TestNormalizeMachineGUID(t *testing.T) { + cases := []struct { + name string + raw string + want string + }{ + {"bare lowercase", "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab", "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab"}, + {"braced", "{9f2f0c1a-1b2c-3d4e-5f60-1234567890ab}", "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab"}, + {"uppercase and spaces", " {9F2F0C1A-1B2C-3D4E-5F60-1234567890AB} ", "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab"}, + {"empty", "", ""}, + {"braces only", "{}", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := normalizeMachineGUID(tc.raw); got != tc.want { + t.Fatalf("normalizeMachineGUID(%q) = %q, want %q", tc.raw, got, tc.want) + } + }) + } +} + +func TestDefaultCollectorHostInfoRecoversWindowsHostIDFailure(t *testing.T) { + originalInfo := hostInfoWithContext + originalRecover := recoverHostInfo + defer func() { + hostInfoWithContext = originalInfo + recoverHostInfo = originalRecover + }() + + hostInfoWithContext = func(context.Context) (*gohost.InfoStat, error) { + return nil, errors.New("getting host ID: More data is available.") + } + recoverHostInfo = func(context.Context) *gohost.InfoStat { + return &gohost.InfoStat{Hostname: "win-host", HostID: "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab"} + } + + got, err := (&defaultCollector{}).HostInfo(context.Background()) + if err != nil { + t.Fatalf("HostInfo() error = %v, want recovered info", err) + } + if got.Hostname != "win-host" || got.HostID != "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab" { + t.Fatalf("HostInfo() = %+v, want recovered Windows host info", got) + } +} + +func TestDefaultCollectorHostInfoKeepsErrorWhenUnrecovered(t *testing.T) { + originalInfo := hostInfoWithContext + originalRecover := recoverHostInfo + defer func() { + hostInfoWithContext = originalInfo + recoverHostInfo = originalRecover + }() + + wantErr := errors.New("getting hostname: unavailable") + hostInfoWithContext = func(context.Context) (*gohost.InfoStat, error) { + return nil, wantErr + } + recoverHostInfo = func(context.Context) *gohost.InfoStat { return nil } + + if _, err := (&defaultCollector{}).HostInfo(context.Background()); !errors.Is(err, wantErr) { + t.Fatalf("HostInfo() error = %v, want %v", err, wantErr) + } +} + +func TestGetReliableMachineIDNormalizesWindowsBraces(t *testing.T) { + collector := &mockCollector{goos: "windows"} + got := GetReliableMachineID(collector, "{9F2F0C1A-1B2C-3D4E-5F60-1234567890AB}", zerolog.Nop()) + if want := "9f2f0c1a-1b2c-3d4e-5f60-1234567890ab"; got != want { + t.Fatalf("GetReliableMachineID() = %q, want %q", got, want) + } +} diff --git a/internal/hostagent/hostinfo_fallback_other.go b/internal/hostagent/hostinfo_fallback_other.go new file mode 100644 index 0000000000..80a0ecfe0a --- /dev/null +++ b/internal/hostagent/hostinfo_fallback_other.go @@ -0,0 +1,15 @@ +//go:build !windows + +package hostagent + +import ( + "context" + + gohost "github.com/shirou/gopsutil/v4/host" +) + +// recoverWindowsHostInfo has no non-Windows behaviour: the MachineGuid buffer +// and length defect it repairs is specific to the Windows host-ID path. +func recoverWindowsHostInfo(context.Context) *gohost.InfoStat { + return nil +} diff --git a/internal/hostagent/hostinfo_fallback_windows.go b/internal/hostagent/hostinfo_fallback_windows.go new file mode 100644 index 0000000000..a9edb3c2e9 --- /dev/null +++ b/internal/hostagent/hostinfo_fallback_windows.go @@ -0,0 +1,69 @@ +//go:build windows + +package hostagent + +import ( + "context" + "os" + "runtime" + + gohost "github.com/shirou/gopsutil/v4/host" + "golang.org/x/sys/windows/registry" +) + +const windowsMachineGUIDPath = `SOFTWARE\Microsoft\Cryptography` + +// recoverWindowsHostInfo rebuilds host information when gopsutil's combined +// InfoWithContext call fails specifically because the Windows MachineGuid host +// ID cannot be read. +// +// gopsutil reads MachineGuid through a fixed-size registry buffer and rejects +// any value whose length is not exactly 36 characters. A braced GUID (38 +// characters) therefore makes HostIDWithContext return an error, which +// InfoWithContext treats as fatal even though every other host field is +// available. Pulse propagated that error and the agent exited at startup +// (issue #2125). This recovery reads MachineGuid directly with a correctly +// sized buffer, strips the optional braces, and leaves the remaining fields to +// gopsutil's individual accessors, so a malformed GUID no longer aborts the +// agent. +func recoverWindowsHostInfo(ctx context.Context) *gohost.InfoStat { + if _, err := gohost.HostIDWithContext(ctx); err == nil { + // The host ID is readable, so the combined failure came from another + // field and must stay fatal so real problems are not hidden. + return nil + } + + info := &gohost.InfoStat{ + OS: runtime.GOOS, + HostID: readWindowsMachineGUID(), + } + if hostname, err := os.Hostname(); err == nil { + info.Hostname = hostname + } + if platform, family, version, err := gohost.PlatformInformationWithContext(ctx); err == nil { + info.Platform = platform + info.PlatformFamily = family + info.PlatformVersion = version + } + if kernelVersion, err := gohost.KernelVersionWithContext(ctx); err == nil { + info.KernelVersion = kernelVersion + } + if arch, err := gohost.KernelArch(); err == nil { + info.KernelArch = arch + } + return info +} + +func readWindowsMachineGUID() string { + key, err := registry.OpenKey(registry.LOCAL_MACHINE, windowsMachineGUIDPath, registry.QUERY_VALUE|registry.WOW64_64KEY) + if err != nil { + return "" + } + defer key.Close() + + value, _, err := key.GetStringValue("MachineGuid") + if err != nil { + return "" + } + return normalizeMachineGUID(value) +} diff --git a/internal/hostagent/machine_id.go b/internal/hostagent/machine_id.go new file mode 100644 index 0000000000..66a028ee83 --- /dev/null +++ b/internal/hostagent/machine_id.go @@ -0,0 +1,12 @@ +package hostagent + +import "strings" + +// normalizeMachineGUID returns the canonical form of a Windows MachineGuid. +// Windows may report the registry value either bare ("xxxxxxxx-....") or +// wrapped in braces ("{xxxxxxxx-....}"); both must resolve to the same stable +// agent identity, so the optional braces are stripped and the result is +// trimmed and lower-cased. +func normalizeMachineGUID(raw string) string { + return strings.ToLower(strings.Trim(strings.TrimSpace(raw), "{}")) +} diff --git a/internal/hostagent/sysinfo.go b/internal/hostagent/sysinfo.go index 652a077df2..261c0c7274 100644 --- a/internal/hostagent/sysinfo.go +++ b/internal/hostagent/sysinfo.go @@ -64,8 +64,22 @@ type defaultCollector struct { metrics hostmetrics.Collector } +// hostInfoWithContext and recoverHostInfo are seams so the Windows host-ID +// recovery can be exercised from a non-Windows test binary. +var ( + hostInfoWithContext = gohost.InfoWithContext + recoverHostInfo = recoverWindowsHostInfo +) + func (c *defaultCollector) HostInfo(ctx context.Context) (*gohost.InfoStat, error) { - return gohost.InfoWithContext(ctx) + info, err := hostInfoWithContext(ctx) + if err == nil { + return info, nil + } + if recovered := recoverHostInfo(ctx); recovered != nil { + return recovered, nil + } + return nil, err } func (c *defaultCollector) HostUptime(ctx context.Context) (uint64, error) {