Scope removed host-agent blocks by identity

Refs #1495

docs/release-control/v6/internal/subsystems/monitoring.md

@@ -23,6 +23,9 @@ Monitoring also owns the distinction between Proxmox VM power state and QEMU
 guest-agent reachability: fresh or never-healthy VMs with an enabled but
 unavailable guest agent stay `not-running`, while only VMs with recent healthy
 guest-agent evidence may become `expected-unreachable`.
+Removed host-agent reconnect blocks are identity-scoped: matching may use the
+canonical host ID or token-qualified machine/hostname continuity, but must never
+block a distinct live host by hostname alone.
 
 ## Canonical Files
 
@@ -1193,10 +1196,13 @@ before snapshots are returned to diagnostics consumers, not only when new
 snapshots are first recorded.
 The same canonical identity rule now applies when removed host agents are
 blocked from re-reporting. `ApplyHostReport` must resolve the final canonical
-host identifier for the `(token, machine-id, hostname)` tuple before it checks
-`removedHostAgents` or emits the reconnect-blocking error, so removing one
-token-bound host cannot poison a different host that shared the same raw
-machine identifier.
+host identifier before it checks `removedHostAgents` or emits the
+reconnect-blocking error, and removed-host records must carry machine and token
+identity so the block is scoped to the retired host. Hostname equivalence may
+only participate when it is qualified by the same token and compatible machine
+identity; removing one stale duplicate must not poison a different live host
+that shares the same hostname or raw machine identifier through a different
+token.
 Docker host re-identification now shares the same hostname-equivalence rule:
 monitoring may treat `qnap` and `qnap.local` as the same host when the token
 or machine identity already points at one canonical runtime, but it must not

internal/models/models.go

@@ -824,6 +824,8 @@ type RemovedHostAgent struct {
 	ID                string    `json:"id"`
 	Hostname          string    `json:"hostname,omitempty"`
 	DisplayName       string    `json:"displayName,omitempty"`
+	MachineID         string    `json:"machineId,omitempty"`
+	TokenID           string    `json:"tokenId,omitempty"`
 	LinkedVMID        string    `json:"linkedVmId,omitempty"`
 	LinkedContainerID string    `json:"linkedContainerId,omitempty"`
 	RemovedAt         time.Time `json:"removedAt"`

internal/monitoring/canonical_guardrails_test.go

@@ -2048,7 +2048,9 @@ func TestHostAgentRemovalGuardUsesResolvedIdentifier(t *testing.T) {
 	}
 	source := string(data)
 	requiredSnippets := []string{
-		"removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname)",
+		"removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname, report.Host.MachineID, tokenID)",
+		"func removedHostAgentMatchesReport(entry models.RemovedHostAgent, identifier, hostname, machineID, tokenID string) bool {",
+		"entryTokenID == \"\" || tokenID == \"\" || entryTokenID != tokenID",
 		`Str("hostID", identifier)`,
 		`fmt.Errorf("host agent %q had monitoring stopped at %v and cannot report again.`,
 	}
@@ -2060,6 +2062,9 @@ func TestHostAgentRemovalGuardUsesResolvedIdentifier(t *testing.T) {
 	if strings.Contains(source, "m.lookupRemovedHostAgent(baseIdentifier, hostname)") {
 		t.Fatal("monitor_agents.go must not check removed host-agent state against pre-resolution baseIdentifier")
 	}
+	if strings.Contains(source, "if hostAgentHostnamesMatch(entry.Hostname, hostname) {") {
+		t.Fatal("monitor_agents.go must not block host-agent re-enroll solely by removed hostname")
+	}
 }
 
 func TestAlertLifecycleCanonicalChangesRemainWritable(t *testing.T) {

internal/monitoring/monitor_agents.go

@@ -219,6 +219,8 @@ func (m *Monitor) RemoveHostAgent(hostID string) (models.Host, error) {
 		ID:                hostID,
 		Hostname:          host.Hostname,
 		DisplayName:       host.DisplayName,
+		MachineID:         host.MachineID,
+		TokenID:           host.TokenID,
 		LinkedVMID:        host.LinkedVMID,
 		LinkedContainerID: host.LinkedContainerID,
 		RemovedAt:         removedAt,
@@ -282,8 +284,10 @@ func (m *Monitor) AllowHostAgentReenroll(hostID string) error {
 	return nil
 }
 
-func (m *Monitor) lookupRemovedHostAgent(identifier, hostname string) (time.Time, bool) {
+func (m *Monitor) lookupRemovedHostAgent(identifier, hostname, machineID, tokenID string) (time.Time, bool) {
 	identifier = strings.TrimSpace(identifier)
+	machineID = sanitizeDockerHostSuffix(machineID)
+	tokenID = strings.TrimSpace(tokenID)
 
 	m.mu.RLock()
 	removedAt, wasRemoved := m.removedHostAgents[identifier]
@@ -293,10 +297,7 @@ func (m *Monitor) lookupRemovedHostAgent(identifier, hostname string) (time.Time
 	}
 
 	for _, entry := range m.state.GetRemovedHostAgents() {
-		if strings.TrimSpace(entry.ID) == identifier {
-			return entry.RemovedAt, true
-		}
-		if hostAgentHostnamesMatch(entry.Hostname, hostname) {
+		if removedHostAgentMatchesReport(entry, identifier, hostname, machineID, tokenID) {
 			return entry.RemovedAt, true
 		}
 	}
@@ -304,6 +305,32 @@ func (m *Monitor) lookupRemovedHostAgent(identifier, hostname string) (time.Time
 	return time.Time{}, false
 }
 
+func removedHostAgentMatchesReport(entry models.RemovedHostAgent, identifier, hostname, machineID, tokenID string) bool {
+	if strings.TrimSpace(entry.ID) == strings.TrimSpace(identifier) {
+		return true
+	}
+
+	entryMachineID := sanitizeDockerHostSuffix(entry.MachineID)
+	entryTokenID := strings.TrimSpace(entry.TokenID)
+	if entryTokenID != "" || tokenID != "" {
+		if entryTokenID == "" || tokenID == "" || entryTokenID != tokenID {
+			return false
+		}
+		if entryMachineID != "" && machineID != "" && entryMachineID == machineID {
+			return true
+		}
+		if !hostAgentHostnamesMatch(entry.Hostname, hostname) {
+			return false
+		}
+		return entryMachineID == "" || machineID == "" || entryMachineID == machineID
+	}
+
+	return entryMachineID != "" &&
+		machineID != "" &&
+		entryMachineID == machineID &&
+		hostAgentHostnamesMatch(entry.Hostname, hostname)
+}
+
 // LinkHostAgent manually links a host agent to a specific PVE node.
 // This is used when auto-linking can't disambiguate (e.g., multiple nodes with hostname "pve").
 // After linking, the host agent's temperature/sensor data will appear on the correct node.
@@ -1726,7 +1753,11 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config.
 		}
 	}
 
-	removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname)
+	tokenID := ""
+	if tokenRecord != nil {
+		tokenID = strings.TrimSpace(tokenRecord.ID)
+	}
+	removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname, report.Host.MachineID, tokenID)
 	if wasRemoved {
 		log.Info().
 			Str("hostID", identifier).

internal/monitoring/monitor_host_agents_test.go

@@ -971,6 +971,121 @@ func TestApplyHostReportDisambiguatesCollidingIdentifiersAcrossTokens(t *testing
 	}
 }
 
+func TestRemoveHostAgent_DoesNotBlockDistinctLiveHostWithSameHostname(t *testing.T) {
+	t.Helper()
+
+	monitor := &Monitor{
+		state:             models.NewState(),
+		alertManager:      alerts.NewManager(),
+		hostTokenBindings: make(map[string]string),
+		config:            &config.Config{},
+		rateTracker:       NewRateTracker(),
+	}
+	t.Cleanup(func() { monitor.alertManager.Stop() })
+
+	now := time.Now().UTC()
+	staleReport := agentshost.Report{
+		Agent: agentshost.AgentInfo{
+			ID:              "old-agent",
+			Version:         "1.0.0",
+			IntervalSeconds: 30,
+		},
+		Host: agentshost.HostInfo{
+			ID:        "shared-machine",
+			MachineID: "shared-machine",
+			Hostname:  "pve-node",
+			Platform:  "linux",
+		},
+		Timestamp: now,
+	}
+	liveReport := agentshost.Report{
+		Agent: agentshost.AgentInfo{
+			ID:              "new-agent",
+			Version:         "1.0.0",
+			IntervalSeconds: 30,
+		},
+		Host: agentshost.HostInfo{
+			ID:        "shared-machine",
+			MachineID: "shared-machine",
+			Hostname:  "pve-node",
+			Platform:  "linux",
+		},
+		Timestamp: now.Add(30 * time.Second),
+	}
+
+	staleHost, err := monitor.ApplyHostReport(staleReport, &config.APITokenRecord{ID: "old-token"})
+	if err != nil {
+		t.Fatalf("ApplyHostReport stale host: %v", err)
+	}
+	liveHost, err := monitor.ApplyHostReport(liveReport, &config.APITokenRecord{ID: "new-token"})
+	if err != nil {
+		t.Fatalf("ApplyHostReport live host: %v", err)
+	}
+	if liveHost.ID == staleHost.ID {
+		t.Fatalf("expected distinct duplicate host IDs, got %q", liveHost.ID)
+	}
+
+	if _, err := monitor.RemoveHostAgent(staleHost.ID); err != nil {
+		t.Fatalf("RemoveHostAgent stale host: %v", err)
+	}
+
+	liveReport.Timestamp = now.Add(time.Minute)
+	liveHostAfterRemoval, err := monitor.ApplyHostReport(liveReport, &config.APITokenRecord{ID: "new-token"})
+	if err != nil {
+		t.Fatalf("live host with same hostname should not be blocked by stale removal: %v", err)
+	}
+	if liveHostAfterRemoval.ID != liveHost.ID {
+		t.Fatalf("expected live host ID %q to remain stable, got %q", liveHost.ID, liveHostAfterRemoval.ID)
+	}
+}
+
+func TestRemoveHostAgent_BlocksRemovedTokenMachineAfterIdentifierChurn(t *testing.T) {
+	t.Helper()
+
+	monitor := &Monitor{
+		state:             models.NewState(),
+		alertManager:      alerts.NewManager(),
+		hostTokenBindings: make(map[string]string),
+		config:            &config.Config{},
+		rateTracker:       NewRateTracker(),
+	}
+	t.Cleanup(func() { monitor.alertManager.Stop() })
+
+	now := time.Now().UTC()
+	hostID := "agent-id-before-reinstall"
+	monitor.state.UpsertHost(models.Host{
+		ID:        hostID,
+		Hostname:  "blocked-node",
+		MachineID: "machine-stable",
+		TokenID:   "host-token",
+		Status:    "offline",
+		LastSeen:  now.Add(-time.Minute),
+	})
+
+	if _, err := monitor.RemoveHostAgent(hostID); err != nil {
+		t.Fatalf("RemoveHostAgent: %v", err)
+	}
+
+	report := agentshost.Report{
+		Agent: agentshost.AgentInfo{
+			ID:              "agent-id-after-reinstall",
+			Version:         "1.0.0",
+			IntervalSeconds: 30,
+		},
+		Host: agentshost.HostInfo{
+			ID:        "machine-stable",
+			MachineID: "machine-stable",
+			Hostname:  "blocked-node",
+			Platform:  "linux",
+		},
+		Timestamp: now,
+	}
+
+	if _, err := monitor.ApplyHostReport(report, &config.APITokenRecord{ID: "host-token"}); err == nil {
+		t.Fatal("expected stable token and machine identity to remain blocked after host ID changed")
+	}
+}
+
 func TestRemoveHostAgentUnbindsToken(t *testing.T) {
 	t.Helper()