From 033dcdf43bacd0c1d454c4aa74e7585964a34140 Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@dalby.cc>
Date: Sat, 17 Oct 2026 13:16:48 +0000
Subject: [PATCH] state: serialise policy reloads

Load and install the stored policy as one step, so an older
reload cannot install its document over a newer one.
---
 hscontrol/state/persist_test.go | 58 +++++++++++++++++++++++++++++++++
 hscontrol/state/state.go        |  7 ++++
 2 files changed, 65 insertions(+)

diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go
index 7d3a283a2..7cc241dbb 100644
--- a/hscontrol/state/persist_test.go
+++ b/hscontrol/state/persist_test.go
@@ -1252,6 +1252,64 @@ func TestReloadPolicyReturnsChangesOnAutoApproveFailure(t *testing.T) {
 		"the reload must return its policy change with the error: %v", cs)
 }
 
+// TestReloadPolicyOverlappingWrites has a second policy write complete after
+// the reload of the first one loaded its document and before it installs it.
+// The first reload must not put the older document back over the newer one.
+func TestReloadPolicyOverlappingWrites(t *testing.T) {
+	_, s, _ := persistTestSetup(t)
+	t.Cleanup(func() { _ = s.Close() })
+
+	const (
+		first  = `{"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}]}`
+		second = `{"acls": [{"action": "accept", "src": ["persist-user@"], "dst": ["persist-user@:22"]}]}`
+	)
+
+	_, err := s.SetPolicyInDB(second)
+	require.NoError(t, err)
+	_, err = s.ReloadPolicy()
+	require.NoError(t, err)
+
+	want, _ := s.Filter()
+
+	_, err = s.SetPolicyInDB(first)
+	require.NoError(t, err)
+
+	var reads atomic.Int32
+
+	written := make(chan struct{})
+
+	require.NoError(t, s.db.DB.Callback().Query().After("gorm:query").
+		Register("write_after_load", func(tx *gorm.DB) {
+			if tx.Statement.Table != "policies" || reads.Add(1) != 1 {
+				return
+			}
+
+			go func() {
+				defer close(written)
+
+				_, err := s.SetPolicyInDB(second)
+				assert.NoError(t, err)
+				_, err = s.ReloadPolicy()
+				assert.NoError(t, err)
+			}()
+
+			// Give the second write time to finish, unless the reload
+			// blocks it.
+			select {
+			case <-written:
+			case <-time.After(200 * time.Millisecond):
+			}
+		}))
+	t.Cleanup(func() { _ = s.db.DB.Callback().Query().Remove("write_after_load") })
+
+	_, err = s.ReloadPolicy()
+	require.NoError(t, err)
+	<-written
+
+	got, _ := s.Filter()
+	assert.Equal(t, want, got, "the policy in force must be the one stored last")
+}
+
 // TestNodeWriteChangeWhenPolicyRefreshFails fails both the NodeStore
 // writer's SetNodes and the caller's. The write still reached the
 // NodeStore, so the change returned with the error must resend the node to
diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go
index 3be9ce24e..2e98981ec 100644
--- a/hscontrol/state/state.go
+++ b/hscontrol/state/state.go
@@ -182,6 +182,10 @@ type State struct {
 	// polMan handles policy evaluation and management
 	polMan policy.PolicyManager
 
+	// policyMu makes loading the stored policy and installing it one step,
+	// so of two overlapping reloads the older document cannot land last.
+	policyMu sync.Mutex
+
 	// authCache holds pending Web, OIDC and SSH authentication requests.
 	// Admission preserves unexpired entries at capacity. Expiry resolves the
 	// verdict with ErrRegistrationExpired so waiting goroutines wake.
@@ -465,6 +469,9 @@ func (s *State) DERPMap() tailcfg.DERPMapView {
 // Returns the resulting [change.Change] slice when the policy or routes changed,
 // also alongside an error once the policy is swapped.
 func (s *State) ReloadPolicy() ([]change.Change, error) {
+	s.policyMu.Lock()
+	defer s.policyMu.Unlock()
+
 	pol, err := hsdb.PolicyBytes(s.db.DB, s.cfg)
 	if err != nil {
 		return nil, fmt.Errorf("loading policy: %w", err)