state: reject renames whose FQDN exceeds the hostname limit

A valid label can still overflow 255 chars under a long base_domain; gate
RenameNode with the new types.ValidateGivenName.

Updates #3346

hscontrol/state/rename_test.go

@@ -0,0 +1,40 @@
+package state
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/juanfont/headscale/hscontrol/db"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRenameNodeRejectsNameExceedingFQDNLimit proves RenameNode rejects a name
+// that is a valid DNS label but whose FQDN, under the configured base_domain,
+// exceeds MaxHostnameLength. Without the FQDN-length gate such a name persists
+// and then breaks map generation for the node and its peers (issue #3346):
+// admin-facing writes must not be able to introduce an unmappable name.
+func TestRenameNodeRejectsNameExceedingFQDNLimit(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+	// A long base domain so a 63-char label overflows the 255-char FQDN bound.
+	cfg.BaseDomain = strings.Repeat("b", 200) + ".example.com"
+
+	database, err := db.NewHeadscaleDatabase(cfg)
+	require.NoError(t, err)
+
+	user := database.CreateUserForTest("rename-user")
+	node := database.CreateRegisteredNodeForTest(user, "rename-node")
+	require.NoError(t, database.Close())
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	// Valid 63-char DNS label, but the resulting FQDN exceeds 255 chars.
+	_, _, err = s.RenameNode(node.ID, strings.Repeat("a", 63))
+	require.Error(t, err, "rename to a name whose FQDN exceeds the limit must be rejected")
+
+	// A short, valid name is still accepted.
+	_, _, err = s.RenameNode(node.ID, "short")
+	require.NoError(t, err)
+}

hscontrol/state/state.go

@@ -1032,7 +1032,10 @@ func (s *State) SetApprovedRoutes(nodeID types.NodeID, routes []netip.Prefix) (t
 // auto-sanitisation) and collisions error out rather than silently
 // bumping a user-facing label. See HOSTNAME.md for the CLI contract.
 func (s *State) RenameNode(nodeID types.NodeID, newName string) (types.NodeView, change.Change, error) {
-	err := dnsname.ValidLabel(newName)
+	// Validate the label AND that the resulting FQDN fits MaxHostnameLength:
+	// a valid 63-char label can still overflow under a long base_domain, and
+	// an unmappable name would break this node and its peers (issue #3346).
+	err := types.ValidateGivenName(newName, s.cfg.BaseDomain)
 	if err != nil {
 		return types.NodeView{}, change.Change{}, fmt.Errorf("%w: %w", ErrGivenNameInvalid, err)
 	}

hscontrol/types/node.go

@@ -19,6 +19,7 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/views"
+	"tailscale.com/util/dnsname"
 )
 
 var (
@@ -517,6 +518,28 @@ func (node *Node) GetFQDN(baseDomain string) (string, error) {
 	return hostname, nil
 }
 
+// ValidateGivenName reports whether givenName is usable as a node's DNS label:
+// a valid DNS label that, combined with baseDomain, yields an FQDN within
+// MaxHostnameLength. Admin-facing write paths (e.g. node rename) reject names
+// that fail this, since the mapper cannot build a map for a node — or any of
+// its peers — whose GetFQDN fails. Derived paths sanitise/coerce instead.
+func ValidateGivenName(givenName, baseDomain string) error {
+	err := dnsname.ValidLabel(givenName)
+	if err != nil {
+		return fmt.Errorf("%q is not a valid DNS label: %w", givenName, err)
+	}
+
+	// Reuse GetFQDN so the length bound stays identical to what the mapper
+	// enforces; a valid 63-char label can still overflow under a long
+	// base_domain.
+	_, err = (&Node{GivenName: givenName}).GetFQDN(baseDomain)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // AnnouncedRoutes returns the list of routes the node announces, as
 // reported by the client in [tailcfg.Hostinfo.RoutableIPs]. Announcement alone
 // does not grant visibility — see [Node.SubnetRoutes] for approval-gated

hscontrol/types/node_test.go

@@ -417,6 +417,33 @@ func TestNodeFQDN(t *testing.T) {
 	}
 }
 
+func TestValidateGivenName(t *testing.T) {
+	tests := []struct {
+		name       string
+		givenName  string
+		baseDomain string
+		wantErr    bool
+	}{
+		{"valid", "test", "example.com", false},
+		{"empty", "", "example.com", true},
+		{"invalid label chars", "not valid", "example.com", true},
+		{"label too long", strings.Repeat("a", 64), "example.com", true},
+		// A valid 63-char label whose FQDN overflows only because the base
+		// domain is long: ValidLabel passes, the FQDN-length bound rejects it.
+		{"fqdn too long under long base domain", strings.Repeat("a", 63), strings.Repeat("b", 200) + ".example.com", true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := ValidateGivenName(tc.givenName, tc.baseDomain)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("ValidateGivenName(%q, %q) error = %v, wantErr %v",
+					tc.givenName, tc.baseDomain, err, tc.wantErr)
+			}
+		})
+	}
+}
+
 func TestPeerChangeFromMapRequest(t *testing.T) {
 	nKeys := []key.NodePublic{
 		key.NewNode().Public(),