From d7b333d2aa0465106a850c3f2984da5a1ce2c93d Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Fri, 2 Oct 2026 09:00:17 +0000
Subject: [PATCH] mapper: send an empty SSH policy when compiling it fails

Fail closed; nil kept the client's previous rules.

Updates #3508
---
 hscontrol/mapper/batcher_test.go | 51 ++++++++++++++++++++++++++++++++
 hscontrol/mapper/builder.go      | 11 ++++---
 hscontrol/state/state.go         |  7 +++++
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/hscontrol/mapper/batcher_test.go b/hscontrol/mapper/batcher_test.go
index db343333b..55b4a555a 100644
--- a/hscontrol/mapper/batcher_test.go
+++ b/hscontrol/mapper/batcher_test.go
@@ -1,6 +1,7 @@
 package mapper
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net/netip"
@@ -13,6 +14,7 @@ import (
 
 	"github.com/juanfont/headscale/hscontrol/db"
 	"github.com/juanfont/headscale/hscontrol/derp"
+	"github.com/juanfont/headscale/hscontrol/policy"
 	"github.com/juanfont/headscale/hscontrol/state"
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/types/change"
@@ -2409,6 +2411,55 @@ func TestAddWorkFullUpdateUnaffectedByEmpty(t *testing.T) {
 	}
 }
 
+var errInjectedSSHPolicy = errors.New("injected SSH policy failure")
+
+type failingSSHPolicyManager struct {
+	policy.PolicyManager
+}
+
+func (failingSSHPolicyManager) SSHPolicy(string, types.NodeView) (*tailcfg.SSHPolicy, error) {
+	return nil, errInjectedSSHPolicy
+}
+
+// TestSSHPolicyEmptyOnWire pins that a node without SSH rules, or whose SSH
+// policy fails to compile, gets "SSHPolicy":{"rules":[]}. A nil SSHPolicy
+// leaves the client's previous rules in force.
+// https://github.com/juanfont/headscale/issues/3508
+func TestSSHPolicyEmptyOnWire(t *testing.T) {
+	for name, failCompile := range map[string]bool{
+		"no ssh rules":  false,
+		"compile error": true,
+	} {
+		t.Run(name, func(t *testing.T) {
+			testData, cleanup := setupBatcherWithTestData(t, NewBatcherAndMapper, 1, 2, normalBufferSize)
+			defer cleanup()
+
+			if failCompile {
+				testData.State.WrapPolicyManagerForTest(
+					func(pm policy.PolicyManager) policy.PolicyManager {
+						return failingSSHPolicyManager{PolicyManager: pm}
+					},
+				)
+			}
+
+			self := testData.Nodes[0].n.ID
+			mc := newMockNodeConnection(self)
+
+			require.NoError(t, handleNodeChange(mc, testData.Batcher.mapper, change.PolicyChange()))
+			require.NoError(t, handleNodeChange(mc, testData.Batcher.mapper, change.FullSelf(self)))
+
+			sent := mc.getSent()
+			require.Len(t, sent, 2)
+
+			for _, resp := range sent {
+				wire, err := json.Marshal(resp)
+				require.NoError(t, err)
+				assert.Contains(t, string(wire), `"SSHPolicy":{"rules":[]}`)
+			}
+		})
+	}
+}
+
 // TestAddWorkPeersRemovedNotTreatedAsEmpty ensures the empty filter does
 // not swallow a PeersRemoved change — deletion cleanup must still run, and
 // surviving recipients must still see the removal.
diff --git a/hscontrol/mapper/builder.go b/hscontrol/mapper/builder.go
index 4f56a93a3..68f03161e 100644
--- a/hscontrol/mapper/builder.go
+++ b/hscontrol/mapper/builder.go
@@ -151,16 +151,15 @@ func (b *MapResponseBuilder) WithSSHPolicy() *MapResponseBuilder {
 
 	sshPolicy, err := b.mapper.state.SSHPolicy(node)
 	if err != nil {
-		// SSH policy is optional for a node to function. Rather than fail the
-		// whole map (leaving the node unable to connect), log and continue
-		// without it; the node still receives a usable netmap.
-		log.Warn().Caller().
+		// Don't fail the whole map over SSH, but fail closed: a nil
+		// SSHPolicy would leave the client's previous rules in force.
+		log.Error().Caller().
 			Err(err).
 			Uint64(zf.NodeID, node.ID().Uint64()).
 			Str(zf.NodeHostname, node.Hostname()).
-			Msg("building map response: skipping SSH policy for node; node will receive a map without SSH rules")
+			Msg("building map response: compiling SSH policy failed; sending empty SSH policy")
 
-		return b
+		sshPolicy = &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{}}
 	}
 
 	b.resp.SSHPolicy = sshPolicy
diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go
index e57a849d7..6185701df 100644
--- a/hscontrol/state/state.go
+++ b/hscontrol/state/state.go
@@ -1624,6 +1624,13 @@ func (s *State) DeleteNodeFromStoreForTest(id types.NodeID) {
 	s.nodeStore.DeleteNode(id)
 }
 
+// WrapPolicyManagerForTest replaces the policy manager with wrap(current) so
+// tests can fail a single method. The [NodeStore] keeps the original manager,
+// so wrap must delegate to its argument. Call before any map is served.
+func (s *State) WrapPolicyManagerForTest(wrap func(policy.PolicyManager) policy.PolicyManager) {
+	s.polMan = wrap(s.polMan)
+}
+
 // CreateRegisteredNodeForTest creates a test node with allocated IPs. This is a convenience wrapper around the database layer.
 func (s *State) CreateRegisteredNodeForTest(user *types.User, hostname ...string) *types.Node {
 	return s.db.CreateRegisteredNodeForTest(user, hostname...)