From e9fd6c197ba69ab6eb29aff3ecb2d60406353911 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 17 Dec 2025 13:28:06 +0000 Subject: [PATCH] fix(config): avoid deadlock saving empty nodes config --- internal/config/persistence.go | 22 +++++++++++++-- internal/config/persistence_deadlock_test.go | 28 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 internal/config/persistence_deadlock_test.go diff --git a/internal/config/persistence.go b/internal/config/persistence.go index 58ac9f1..225911b 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -912,8 +912,26 @@ func (c *ConfigPersistence) saveNodesConfig(pveInstances []PVEInstance, pbsInsta // CRITICAL: Never save empty nodes configuration // This prevents data loss from accidental wipes if !allowEmpty && len(pveInstances) == 0 && len(pbsInstances) == 0 && len(pmgInstances) == 0 { - // Check if we're replacing existing non-empty config - if existing, err := c.LoadNodesConfig(); err == nil && existing != nil { + // If we're replacing an existing non-empty config, block the wipe. + // We must not call LoadNodesConfig here because it acquires c.mu again. + if _, err := os.Stat(c.nodesFile); err == nil { + data, err := os.ReadFile(c.nodesFile) + if err != nil { + return fmt.Errorf("refusing to save empty nodes config: failed to read existing nodes config: %w", err) + } + if c.crypto != nil { + decrypted, err := c.crypto.Decrypt(data) + if err != nil { + return fmt.Errorf("refusing to save empty nodes config: existing nodes config is not decryptable: %w", err) + } + data = decrypted + } + + var existing NodesConfig + if err := json.Unmarshal(data, &existing); err != nil { + return fmt.Errorf("refusing to save empty nodes config: existing nodes config is not parseable: %w", err) + } + if len(existing.PVEInstances) > 0 || len(existing.PBSInstances) > 0 || len(existing.PMGInstances) > 0 { log.Error(). Int("existing_pve", len(existing.PVEInstances)). diff --git a/internal/config/persistence_deadlock_test.go b/internal/config/persistence_deadlock_test.go new file mode 100644 index 0000000..140e0f6 --- /dev/null +++ b/internal/config/persistence_deadlock_test.go @@ -0,0 +1,28 @@ +package config + +import ( + "testing" + "time" +) + +func TestSaveNodesConfig_EmptyDoesNotDeadlockWhenExistingNodesPresent(t *testing.T) { + cp := NewConfigPersistence(t.TempDir()) + + if err := cp.SaveNodesConfig([]PVEInstance{{Name: "pve1", Host: "https://example.invalid:8006", User: "root@pam"}}, nil, nil); err != nil { + t.Fatalf("SaveNodesConfig seed: %v", err) + } + + done := make(chan error, 1) + go func() { + done <- cp.SaveNodesConfig(nil, nil, nil) + }() + + select { + case err := <-done: + if err == nil { + t.Fatalf("expected error when saving empty over existing nodes") + } + case <-time.After(2 * time.Second): + t.Fatalf("SaveNodesConfig appears to have deadlocked") + } +}