policy/v2: evaluate policy tests before taking the write lock

SetPolicy ran the policy's tests under the lock every map response and
node write needs. Run them on a snapshot of users and nodes first.

hscontrol/policy/v2/policy.go

@@ -673,10 +673,11 @@ func (pm *PolicyManager) SetPolicy(polB []byte) (bool, error) {
 		return false, fmt.Errorf("parsing policy: %w", err)
 	}
 
-	pm.mu.Lock()
-	defer pm.mu.Unlock()
+	pm.mu.RLock()
+	users, nodes := pm.users, pm.nodes
+	pm.mu.RUnlock()
 
-	err = validateUserReferences(pol, pm.users)
+	err = validateUserReferences(pol, users)
 	if err != nil {
 		return false, fmt.Errorf("validating policy user references: %w", err)
 	}
@@ -686,17 +687,25 @@ func (pm *PolicyManager) SetPolicy(polB []byte) (bool, error) {
 	// they fail, return without mutating the live PolicyManager so the
 	// failed write does not knock the running config offline.
 	//
+	// They run before the write lock is taken: how long they take is up
+	// to the policy author, and every map response and node write needs
+	// the lock. A user or node that changes meanwhile counts as changed
+	// right after the write.
+	//
 	// Aggregate ACL and SSH test failures via multierr so operators
 	// see both classes in a single response instead of having to
 	// fix-and-retry to discover the second one.
 	testErr := multierr.New(
-		evaluateTests(pol, pm.users, pm.nodes),
-		evaluateSSHTests(pol, pm.users, pm.nodes),
+		evaluateTests(pol, users, nodes),
+		evaluateSSHTests(pol, users, nodes),
 	)
 	if testErr != nil {
 		return false, testErr
 	}
 
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
+
 	// Log policy metadata for debugging
 	log.Debug().
 		Int("policy.bytes", len(polB)).

hscontrol/policy/v2/test_test.go

@@ -6,6 +6,7 @@ import (
 	"slices"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/stretchr/testify/assert"
@@ -439,6 +440,48 @@ func TestRunTestsManyAssertionsAndDestinations(t *testing.T) {
 	require.NoError(t, evaluateTests(pol, users, nodes.ViewSlice()))
 }
 
+// TestSetPolicyEvaluatesTestsUnlocked asserts that a policy's tests run
+// before SetPolicy takes the write lock, so map generation and node writes
+// do not wait for them: a reader holding the lock does not hold up the
+// evaluation, and a rejected policy never needs the write lock at all.
+func TestSetPolicyEvaluatesTestsUnlocked(t *testing.T) {
+	users := policyTestUsers()
+	nodes := policyTestNodes(users)
+
+	pm, err := NewPolicyManager([]byte(`{}`), users, nodes.ViewSlice())
+	require.NoError(t, err)
+
+	bad := `{
+		"tagOwners": { "tag:server": ["alice@headscale.net"] },
+		"acls": [{
+			"action": "accept",
+			"src": ["alice@headscale.net"],
+			"dst": ["tag:server:22"]
+		}],
+		"tests": [{
+			"src": "bob@headscale.net",
+			"accept": ["tag:server:22"]
+		}]
+	}`
+
+	pm.mu.RLock()
+	defer pm.mu.RUnlock()
+
+	done := make(chan error, 1)
+
+	go func() {
+		_, err := pm.SetPolicy([]byte(bad))
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		require.ErrorIs(t, err, errPolicyTestsFailed)
+	case <-time.After(10 * time.Second):
+		t.Fatal("SetPolicy waited for the write lock before evaluating the tests")
+	}
+}
+
 // TestSetPolicyRejectsFailingTests asserts that SetPolicy is the user-write
 // boundary: a policy whose tests fail must be rejected without mutating the
 // live PolicyManager. NewPolicyManager (boot path) does not run tests.