From 386fc0415e8236aafd946c9aa97f50e87405d12d Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2026 09:17:23 +0100 Subject: [PATCH] fix(monitoring): reject host-local addresses in agent auto-linking A Docker bridge or link-local address can be unique among monitored PVE nodes but also exist on an unrelated NAS. Counting PVE owners alone then creates a false reciprocal agent association. Exclude host-local IPs and known Docker bridge interfaces from automatic network evidence, preserving management bridges and explicit unicast report IPs. Seven synthetic negative cases fail before this repair and pass afterwards. Focused matcher and host-report tests pass under the race detector. This prevents a reproduced backend misassociation; it does not establish the cause or resolution of issue #1930, whose diagnostic payload remains unavailable. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 18 ++++ .../v6/internal/subsystems/monitoring.md | 21 ++++ internal/monitoring/monitor_agents.go | 30 +++++- .../monitoring/monitor_host_agents_test.go | 97 +++++++++++++++++++ 4 files changed, 163 insertions(+), 3 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 301a0412f..4a0ad216c 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -15,6 +15,24 @@ ## Purpose +### Automatic PVE association identity boundary + +Host ingestion must not create a host-to-PVE or reciprocal PVE-to-agent link +solely from a shared host-local network address. Uniqueness among monitored +PVE nodes does not establish uniqueness across unrelated agent hosts. +Automatic network evidence excludes loopback, unspecified, multicast and +link-local IPs, and interfaces named lo or prefixed docker/br-. This changes +association evidence only: enrollment, token binding, removal, re-enrollment +and command authority remain unchanged. It does not migrate or repair +persisted incorrect links. + +Verification: `TestApplyHostReportDoesNotLinkUnrelatedDockerBridge` in +`internal/monitoring/monitor_host_agents_test.go` ingests repeated synthetic +NAS reports and requires both association directions to remain absent. +The adjacent matcher and filtering tests exercise host-local rejection and +retained management-address evidence. These are local fixture proofs, not +reporter confirmation of #1930 or installed lifecycle qualification. + The internal Patrol request bridge carries explicit execution limits and capability allowlists without a diagnostic report-count budget. Finding writes retain their server-owned scope and cannot enter or satisfy the infrastructure diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 9e1793589..6f5303afe 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -17,6 +17,27 @@ ## Purpose +### Host-local addresses are not PVE identity + +Automatic host/PVE network matching excludes non-global-unicast addresses +and lo/docker*/br-* interfaces from both reported-host and provider-node +address inventories. A Docker bridge address seen on only one PVE node can +still exist on an unrelated NAS; provider-only owner counts cannot make it +machine identity. Management bridges such as vmbr0, unnamed legacy interfaces, +private IPv4 and IPv6/ULA remain eligible. Explicit unicast report-IP hints +remain eligible independently of inferred interface evidence. No blanket +private-subnet exclusion is permitted. + +Verification in `internal/monitoring/monitor_host_agents_test.go`: +`TestFindLinkedProxmoxEntityWithHints_RejectsHostLocalNetworkIdentity` covers +seven previously false associations; `TestAgentLinkNetworkIdentityFiltering` +pins both-side exclusions and positive management controls; +`TestApplyHostReportDoesNotLinkUnrelatedDockerBridge` requires repeated +ingestion to leave both link directions absent. Existing endpoint and +ambiguous-name tests remain required. These checks prevent a reproduced +synthetic misassociation, not all private-address collisions or unknown +custom bridge names, and do not prove the cause of #1930. + Physical disk inventory has an independent collector schedule. The PVE poller carries its default five-minute or configured interval with each disk record, while keeping the last successful observation timestamp on retained records. diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index a4bf3c3af..9b6129c80 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "errors" "fmt" + "net" "slices" "strings" "sync" @@ -4389,18 +4390,38 @@ func (m *Monitor) findLinkedProxmoxEntity(hostname string) (nodeID, vmID, contai return m.findLinkedProxmoxEntityWithHints(hostname, "", nil) } +// Host-local addresses may be unique among the monitored PVE nodes while also +// existing on an unrelated agent host. They are not machine identity evidence. +func normalizeAgentLinkIP(address string) string { + normalized := unifiedresources.NormalizeIP(address) + if ip := net.ParseIP(normalized); ip != nil && ip.IsGlobalUnicast() { + return normalized + } + return "" +} + +// Do not discard all bridges: Proxmox management addresses commonly use vmbr0. +// Explicit report-IP and endpoint hints remain usable for routable addresses. +func isHostLocalAgentLinkInterface(name string) bool { + name = strings.ToLower(strings.TrimSpace(name)) + return name == "lo" || strings.HasPrefix(name, "docker") || strings.HasPrefix(name, "br-") +} + func collectReportedHostIPs( reportIP string, network []agentshost.NetworkInterface, ) map[string]struct{} { ips := make(map[string]struct{}) - if normalized := unifiedresources.NormalizeIP(reportIP); normalized != "" { + if normalized := normalizeAgentLinkIP(reportIP); normalized != "" { ips[normalized] = struct{}{} } for _, nic := range network { + if isHostLocalAgentLinkInterface(nic.Name) { + continue + } for _, address := range nic.Addresses { - if normalized := unifiedresources.NormalizeIP(address); normalized != "" { + if normalized := normalizeAgentLinkIP(address); normalized != "" { ips[normalized] = struct{}{} } } @@ -4412,8 +4433,11 @@ func collectReportedHostIPs( func collectNodeNetworkIPs(network []unifiedresources.NetworkInterface) map[string]struct{} { ips := make(map[string]struct{}) for _, nic := range network { + if isHostLocalAgentLinkInterface(nic.Name) { + continue + } for _, address := range nic.Addresses { - if normalized := unifiedresources.NormalizeIP(address); normalized != "" { + if normalized := normalizeAgentLinkIP(address); normalized != "" { ips[normalized] = struct{}{} } } diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index ef5976097..c5c4b363b 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -5685,3 +5685,100 @@ func TestApplyHostReportPreservesPoolOnlyUnraidCount(t *testing.T) { t.Fatalf("pool-only host raised storage risk: %+v", assessment) } } + +// Uniqueness among PVE nodes does not make host-local addresses machine identity: +// a non-PVE host is not included in that ownership count. +func TestFindLinkedProxmoxEntityWithHints_RejectsHostLocalNetworkIdentity(t *testing.T) { + for _, tc := range []struct{ name, nic, address string }{ + {"docker", "docker0", "172.17.0.1/16"}, + {"custom docker bridge", "br-123abc", "192.0.2.1/24"}, + {"loopback", "lo", "127.0.0.1/8"}, + {"IPv6 loopback", "lo", "::1/128"}, + {"link local", "eth0", "169.254.1.2/16"}, + {"IPv6 link local", "eth0", "fe80::1/64"}, + {"unspecified", "eth0", "0.0.0.0"}, + } { + t.Run(tc.name, func(t *testing.T) { + monitor := &Monitor{state: models.NewState()} + monitor.state.UpdateNodes([]models.Node{{ + ID: "pve-node", Name: "pve", Instance: "cluster", + Host: "https://pve.example:8006", + NetworkInterfaces: []models.HostNetworkInterface{{Name: tc.nic, Addresses: []string{tc.address}}}, + }}) + node, vm, ct := monitor.findLinkedProxmoxEntityWithHints("nas", "", []agentshost.NetworkInterface{ + {Name: tc.nic, Addresses: []string{tc.address}}, + {Name: "eth1", Addresses: []string{"198.51.100.20/24"}}, + }) + if node != "" || vm != "" || ct != "" { + t.Fatalf("unrelated NAS linked by host-local address: node=%q vm=%q ct=%q", node, vm, ct) + } + }) + } +} + +func TestAgentLinkNetworkIdentityFiltering(t *testing.T) { + for _, tc := range []struct { + name, nic, address string + want bool + }{ + {"management bridge", "vmbr0", "192.0.2.10/24", true}, + {"private management", "eth0", "172.17.0.1/16", true}, + {"IPv6 management", "vmbr0", "2001:db8::10/64", true}, + {"ULA management", "eth0", "fd00::10/64", true}, + {"unnamed legacy interface", "", "192.0.2.10", true}, + {"docker", "docker0", "172.17.0.1/16", false}, + {"custom docker", "br-abc", "192.0.2.1/24", false}, + {"loopback", "eth0", "127.1.2.3/8", false}, + {"IPv6 unspecified", "eth0", "::", false}, + {"multicast", "eth0", "224.0.0.1", false}, + {"IPv6 link local", "eth0", "fe90::1/64", false}, + } { + t.Run(tc.name, func(t *testing.T) { + agent := collectReportedHostIPs("", []agentshost.NetworkInterface{{Name: tc.nic, Addresses: []string{tc.address}}}) + node := collectNodeNetworkIPs([]unifiedresources.NetworkInterface{{Name: tc.nic, Addresses: []string{tc.address}}}) + if (len(agent) == 1) != tc.want || (len(node) == 1) != tc.want { + t.Fatalf("agent=%v node=%v want accepted=%v", agent, node, tc.want) + } + }) + } + if got := collectReportedHostIPs("172.17.0.1", nil); len(got) != 1 { + t.Fatalf("explicit private report IP must remain usable: %v", got) + } + if got := collectReportedHostIPs("::1", nil); len(got) != 0 { + t.Fatalf("explicit loopback cannot identify a machine: %v", got) + } +} + +// Exercise ingestion as well as the matcher: no reciprocal PVE link may be +// created from a bridge address shared with an unrelated host. +func TestApplyHostReportDoesNotLinkUnrelatedDockerBridge(t *testing.T) { + monitor := issue1654Monitor() + monitor.state.UpdateNodes([]models.Node{{ + ID: "pve-node", Name: "pve", Instance: "cluster", + Host: "https://pve.example:8006", + NetworkInterfaces: []models.HostNetworkInterface{ + {Name: "vmbr0", Addresses: []string{"192.0.2.10/24"}}, + {Name: "docker0", Addresses: []string{"172.17.0.1/16"}}, + }, + }}) + report := issue1654Report(time.Now().UTC()) + report.Host.Hostname = "nas.example" + report.Network = []agentshost.NetworkInterface{ + {Name: "eth0", Addresses: []string{"198.51.100.20/24"}}, + {Name: "docker0", Addresses: []string{"172.17.0.1/16"}}, + } + for i := 0; i < 2; i++ { + report.Timestamp = report.Timestamp.Add(time.Second) + host, err := monitor.ApplyHostReport(report, nil) + if err != nil { + t.Fatal(err) + } + if host.LinkedNodeID != "" { + t.Fatalf("NAS linked to %q", host.LinkedNodeID) + } + nodes := monitor.state.GetSnapshot().Nodes + if len(nodes) != 1 || nodes[0].LinkedAgentID != "" { + t.Fatalf("PVE node acquired an unrelated agent link: %+v", nodes) + } + } +}