From 6a70c6f2346c4715e662625d8153e6724c70f192 Mon Sep 17 00:00:00 2001 From: Snider Date: Mon, 30 Mar 2026 21:17:43 +0100 Subject: [PATCH] fix(node): surface corrupted persisted state during load Co-Authored-By: Virgil --- node/identity.go | 12 +++++++++--- node/identity_test.go | 16 ++++++++++++++++ node/peer.go | 12 +++++++++--- node/peer_test.go | 16 ++++++++++++++++ 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/node/identity.go b/node/identity.go index 38bf5e1..2f82de3 100644 --- a/node/identity.go +++ b/node/identity.go @@ -104,6 +104,9 @@ func NewNodeManager() (*NodeManager, error) { // NewNodeManagerFromPaths loads or creates a node identity store at explicit paths. // +// Missing files are treated as a fresh install; malformed or partial identity +// state is returned as an error so callers can handle it explicitly. +// // nodeManager, err := NewNodeManagerFromPaths("/srv/p2p/private.key", "/srv/p2p/node.json") func NewNodeManagerFromPaths(keyPath, configPath string) (*NodeManager, error) { nm := &NodeManager{ @@ -111,12 +114,15 @@ func NewNodeManagerFromPaths(keyPath, configPath string) (*NodeManager, error) { configPath: configPath, } - // Try to load existing identity - if err := nm.loadIdentity(); err != nil { - // Identity doesn't exist yet, that's ok + // Missing files indicate a first run; anything else is a load failure. + if !fsExists(keyPath) && !fsExists(configPath) { return nm, nil } + if err := nm.loadIdentity(); err != nil { + return nil, err + } + return nm, nil } diff --git a/node/identity_test.go b/node/identity_test.go index 87c03b4..9693716 100644 --- a/node/identity_test.go +++ b/node/identity_test.go @@ -170,6 +170,22 @@ func TestIdentity_NodeIdentity_Good(t *testing.T) { }) } +func TestIdentity_NodeManagerFromPaths_CorruptIdentity_Bad(t *testing.T) { + tmpDir := t.TempDir() + keyPath, configPath := testNodeManagerPaths(tmpDir) + + testWriteFile(t, configPath, []byte(`{"id":"node-1","name":"broken","publicKey":"not-json"`), 0o600) + + nm, err := NewNodeManagerFromPaths(keyPath, configPath) + if err == nil { + t.Fatal("expected error when loading a corrupted node identity") + } + + if nm != nil { + t.Fatal("expected nil node manager when identity data is corrupted") + } +} + func TestIdentity_NodeRoles_Good(t *testing.T) { tests := []struct { role NodeRole diff --git a/node/peer.go b/node/peer.go index d344c39..60a09c4 100644 --- a/node/peer.go +++ b/node/peer.go @@ -136,6 +136,9 @@ func NewPeerRegistry() (*PeerRegistry, error) { // NewPeerRegistryFromPath loads or creates a peer registry at an explicit path. // +// Missing files are treated as an empty registry; malformed registry files are +// returned as errors so callers can repair the persisted state. +// // peerRegistry, err := NewPeerRegistryFromPath("/srv/p2p/peers.json") func NewPeerRegistryFromPath(peersPath string) (*PeerRegistry, error) { pr := &PeerRegistry{ @@ -146,13 +149,16 @@ func NewPeerRegistryFromPath(peersPath string) (*PeerRegistry, error) { allowedPublicKeys: make(map[string]bool), } - // Try to load existing peers - if err := pr.load(); err != nil { - // No existing peers, that's ok + // Missing files indicate a first run; any existing file must parse cleanly. + if !fsExists(peersPath) { pr.rebuildKDTree() return pr, nil } + if err := pr.load(); err != nil { + return nil, err + } + pr.rebuildKDTree() return pr, nil } diff --git a/node/peer_test.go b/node/peer_test.go index ed4bd70..9b0d506 100644 --- a/node/peer_test.go +++ b/node/peer_test.go @@ -27,6 +27,22 @@ func TestPeer_Registry_NewPeerRegistry_Good(t *testing.T) { } } +func TestPeer_Registry_NewPeerRegistryFromPath_CorruptFile_Bad(t *testing.T) { + tmpDir := t.TempDir() + peersPath := testJoinPath(tmpDir, "peers.json") + + testWriteFile(t, peersPath, []byte(`{"id":"peer-1"`), 0o600) + + pr, err := NewPeerRegistryFromPath(peersPath) + if err == nil { + t.Fatal("expected error when loading a corrupted peer registry") + } + + if pr != nil { + t.Fatal("expected nil peer registry when persisted data is corrupted") + } +} + func TestPeer_Registry_AddPeer_Good(t *testing.T) { pr, cleanup := setupTestPeerRegistry(t) defer cleanup()