integration: add test for tagged→user-owned conversion panic

Add TestTagsAuthKeyConvertToUserViaCLIRegister that reproduces the
exact panic from #3038: register a node with a tags-only PreAuthKey
(no user), force reauth with empty tags, then register via CLI with
a user. The mapper panics on node.Owner().Model().ID when User is nil.

The critical detail is using a tags-only PreAuthKey (User: nil). When
the key is created under a user, the node inherits the User pointer
from createAndSaveNewNode and the bug is masked.

Also add Owner() validity assertions to the existing unit test
TestTaggedNodeWithoutUserToDifferentUser to catch the nil pointer
at the unit test level.

Updates #3038

.github/workflows/test-integration.yaml

@@ -247,6 +247,7 @@ jobs:
           - TestTagsUserLoginReauthWithEmptyTagsRemovesAllTags
           - TestTagsAuthKeyWithoutUserInheritsTags
           - TestTagsAuthKeyWithoutUserRejectsAdvertisedTags
+          - TestTagsAuthKeyConvertToUserViaCLIRegister
     uses: ./.github/workflows/integration-test-template.yml
     secrets: inherit
     with:

hscontrol/auth_tags_test.go

@@ -625,6 +625,152 @@ func TestTaggedNodeReauthPreservesDisabledExpiry(t *testing.T) {
 		"Tagged node should have expiry PRESERVED as disabled after re-auth")
 }
 
+// TestExpiryDuringPersonalToTaggedConversion tests that when a personal node
+// is converted to tagged via reauth with RequestTags, the expiry is cleared to nil.
+// BUG #3048: Previously expiry was NOT cleared because expiry handling ran
+// BEFORE processReauthTags.
+func TestExpiryDuringPersonalToTaggedConversion(t *testing.T) {
+	app := createTestApp(t)
+	user := app.state.CreateUserForTest("expiry-test-user")
+
+	// Update policy to allow user to own tags
+	err := app.state.UpdatePolicyManagerUsersForTest()
+	require.NoError(t, err)
+
+	policy := `{
+		"tagOwners": {
+			"tag:server": ["expiry-test-user@"]
+		},
+		"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}]
+	}`
+	_, err = app.state.SetPolicy([]byte(policy))
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	nodeKey1 := key.NewNode()
+
+	// Step 1: Create user-owned node WITH expiry set
+	clientExpiry := time.Now().Add(24 * time.Hour)
+	registrationID1 := types.MustRegistrationID()
+	regEntry1 := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(),
+		NodeKey:    nodeKey1.Public(),
+		Hostname:   "personal-to-tagged",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "personal-to-tagged",
+			RequestTags: []string{}, // No tags - user-owned
+		},
+		Expiry: &clientExpiry,
+	})
+	app.state.SetRegistrationCacheEntry(registrationID1, regEntry1)
+
+	node, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID1, types.UserID(user.ID), nil, "webauth",
+	)
+	require.NoError(t, err)
+	require.False(t, node.IsTagged(), "Node should be user-owned initially")
+	require.True(t, node.Expiry().Valid(), "User-owned node should have expiry set")
+
+	// Step 2: Re-auth with tags (Personal → Tagged conversion)
+	nodeKey2 := key.NewNode()
+	registrationID2 := types.MustRegistrationID()
+	regEntry2 := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(),
+		NodeKey:    nodeKey2.Public(),
+		Hostname:   "personal-to-tagged",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "personal-to-tagged",
+			RequestTags: []string{"tag:server"}, // Adding tags
+		},
+		Expiry: &clientExpiry, // Client still sends expiry
+	})
+	app.state.SetRegistrationCacheEntry(registrationID2, regEntry2)
+
+	nodeAfter, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID2, types.UserID(user.ID), nil, "webauth",
+	)
+	require.NoError(t, err)
+	require.True(t, nodeAfter.IsTagged(), "Node should be tagged after conversion")
+
+	// CRITICAL ASSERTION: Tagged nodes should NOT have expiry
+	assert.False(t, nodeAfter.Expiry().Valid(),
+		"Tagged node should have expiry cleared to nil")
+}
+
+// TestExpiryDuringTaggedToPersonalConversion tests that when a tagged node
+// is converted to personal via reauth with empty RequestTags, expiry is set
+// from the client request.
+// BUG #3048: Previously expiry was NOT set because expiry handling ran
+// BEFORE processReauthTags (node was still tagged at check time).
+func TestExpiryDuringTaggedToPersonalConversion(t *testing.T) {
+	app := createTestApp(t)
+	user := app.state.CreateUserForTest("expiry-test-user2")
+
+	// Update policy to allow user to own tags
+	err := app.state.UpdatePolicyManagerUsersForTest()
+	require.NoError(t, err)
+
+	policy := `{
+		"tagOwners": {
+			"tag:server": ["expiry-test-user2@"]
+		},
+		"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}]
+	}`
+	_, err = app.state.SetPolicy([]byte(policy))
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	nodeKey1 := key.NewNode()
+
+	// Step 1: Create tagged node (expiry should be nil)
+	registrationID1 := types.MustRegistrationID()
+	regEntry1 := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(),
+		NodeKey:    nodeKey1.Public(),
+		Hostname:   "tagged-to-personal",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "tagged-to-personal",
+			RequestTags: []string{"tag:server"}, // Tagged node
+		},
+	})
+	app.state.SetRegistrationCacheEntry(registrationID1, regEntry1)
+
+	node, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID1, types.UserID(user.ID), nil, "webauth",
+	)
+	require.NoError(t, err)
+	require.True(t, node.IsTagged(), "Node should be tagged initially")
+	require.False(t, node.Expiry().Valid(), "Tagged node should have nil expiry")
+
+	// Step 2: Re-auth with empty tags (Tagged → Personal conversion)
+	nodeKey2 := key.NewNode()
+	clientExpiry := time.Now().Add(48 * time.Hour)
+	registrationID2 := types.MustRegistrationID()
+	regEntry2 := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(),
+		NodeKey:    nodeKey2.Public(),
+		Hostname:   "tagged-to-personal",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "tagged-to-personal",
+			RequestTags: []string{}, // Empty tags - convert to user-owned
+		},
+		Expiry: &clientExpiry, // Client requests expiry
+	})
+	app.state.SetRegistrationCacheEntry(registrationID2, regEntry2)
+
+	nodeAfter, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID2, types.UserID(user.ID), nil, "webauth",
+	)
+	require.NoError(t, err)
+	require.False(t, nodeAfter.IsTagged(), "Node should be user-owned after conversion")
+
+	// CRITICAL ASSERTION: User-owned nodes should have expiry from client
+	assert.True(t, nodeAfter.Expiry().Valid(),
+		"User-owned node should have expiry set")
+	assert.WithinDuration(t, clientExpiry, nodeAfter.Expiry().Get(), 5*time.Second,
+		"Expiry should match client request")
+}
+
 // TestReAuthWithDifferentMachineKey tests the edge case where a node attempts
 // to re-authenticate with the same NodeKey but a DIFFERENT MachineKey.
 // This scenario should be handled gracefully (currently creates a new node).

hscontrol/auth_test.go

@@ -3832,3 +3832,98 @@ func TestDeletedPreAuthKeyNotRecreatedOnNodeUpdate(t *testing.T) {
 
 	t.Log("SUCCESS: PreAuthKey remained deleted after node update")
 }
+
+// TestTaggedNodeWithoutUserToDifferentUser tests that a node registered with a
+// tags-only PreAuthKey (no user) can be re-registered to a different user
+// without panicking. This reproduces the issue reported in #3038.
+func TestTaggedNodeWithoutUserToDifferentUser(t *testing.T) {
+	t.Parallel()
+
+	app := createTestApp(t)
+
+	// Step 1: Create a tags-only PreAuthKey (no user, only tags)
+	// This is valid for tagged nodes where ownership is defined by tags, not users
+	tags := []string{"tag:server", "tag:prod"}
+	pak, err := app.state.CreatePreAuthKey(nil, true, false, nil, tags)
+	require.NoError(t, err, "Failed to create tags-only pre-auth key")
+	require.Nil(t, pak.User, "Tags-only PAK should have nil User")
+
+	machineKey := key.NewMachine()
+	nodeKey1 := key.NewNode()
+
+	// Step 2: Register node with tags-only PreAuthKey
+	regReq := tailcfg.RegisterRequest{
+		Auth: &tailcfg.RegisterResponseAuth{
+			AuthKey: pak.Key,
+		},
+		NodeKey: nodeKey1.Public(),
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname: "tagged-orphan-node",
+		},
+		Expiry: time.Now().Add(24 * time.Hour),
+	}
+
+	resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err, "Initial registration should succeed")
+	require.True(t, resp.MachineAuthorized, "Node should be authorized")
+
+	// Verify initial state: node is tagged with no UserID
+	node, found := app.state.GetNodeByNodeKey(nodeKey1.Public())
+	require.True(t, found, "Node should be found")
+	require.True(t, node.IsTagged(), "Node should be tagged")
+	require.ElementsMatch(t, tags, node.Tags().AsSlice(), "Node should have tags from PAK")
+	require.False(t, node.UserID().Valid(), "Node should NOT have a UserID (tags-only PAK)")
+	require.False(t, node.User().Valid(), "Node should NOT have a User (tags-only PAK)")
+
+	t.Logf("Initial registration complete - Node ID: %d, Tags: %v, IsTagged: %t, UserID valid: %t",
+		node.ID().Uint64(), node.Tags().AsSlice(), node.IsTagged(), node.UserID().Valid())
+
+	// Step 3: Create a new user (alice) to re-register the node to
+	alice := app.state.CreateUserForTest("alice")
+	require.NotNil(t, alice, "Alice user should be created")
+
+	// Step 4: Re-register the node to alice via HandleNodeFromAuthPath
+	// This is what happens when running: headscale nodes register --user alice --key ...
+	nodeKey2 := key.NewNode()
+	registrationID := types.MustRegistrationID()
+	regEntry := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(), // Same machine key as the tagged node
+		NodeKey:    nodeKey2.Public(),
+		Hostname:   "tagged-orphan-node",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "tagged-orphan-node",
+			RequestTags: []string{}, // Empty - transition to user-owned
+		},
+	})
+	app.state.SetRegistrationCacheEntry(registrationID, regEntry)
+
+	// This should NOT panic - before the fix, this would panic with:
+	// panic: runtime error: invalid memory address or nil pointer dereference
+	// at UserView.Name() because the existing node has no User
+	nodeAfterReauth, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID,
+		types.UserID(alice.ID),
+		nil,
+		"cli",
+	)
+	require.NoError(t, err, "Re-registration to alice should succeed without panic")
+
+	// Verify the existing tagged node was converted to be owned by alice (same node ID)
+	require.True(t, nodeAfterReauth.Valid(), "Node should be valid")
+	require.True(t, nodeAfterReauth.UserID().Valid(), "Node should have a UserID")
+	require.Equal(t, alice.ID, nodeAfterReauth.UserID().Get(), "Node should be owned by alice")
+	require.Equal(t, node.ID(), nodeAfterReauth.ID(), "Should be the same node (converted, not new)")
+	require.False(t, nodeAfterReauth.IsTagged(), "Node should no longer be tagged")
+	require.Empty(t, nodeAfterReauth.Tags().AsSlice(), "Node should have no tags")
+
+	// Verify Owner() works without panicking - this is what the mapper's
+	// generateUserProfiles calls, and it would panic with a nil pointer
+	// dereference if node.User was not set during the tag→user conversion.
+	owner := nodeAfterReauth.Owner()
+	require.True(t, owner.Valid(), "Owner should be valid after conversion (mapper would panic if nil)")
+	require.Equal(t, alice.ID, owner.Model().ID, "Owner should be alice")
+
+	t.Logf("Re-registration complete - Node ID: %d, Tags: %v, IsTagged: %t, UserID: %d",
+		nodeAfterReauth.ID().Uint64(), nodeAfterReauth.Tags().AsSlice(),
+		nodeAfterReauth.IsTagged(), nodeAfterReauth.UserID().Get())
+}

hscontrol/mapper/mapper.go

@@ -77,11 +77,22 @@ func generateUserProfiles(
 	userMap := make(map[uint]*types.UserView)
 	ids := make([]uint, 0, len(userMap))
 	user := node.Owner()
+	if !user.Valid() {
+		log.Error().
+			Uint64("node.id", node.ID().Uint64()).
+			Str("node.name", node.Hostname()).
+			Msg("node has no valid owner, skipping user profile generation")
+
+		return nil
+	}
 	userID := user.Model().ID
 	userMap[userID] = &user
 	ids = append(ids, userID)
 	for _, peer := range peers.All() {
 		peerUser := peer.Owner()
+		if !peerUser.Valid() {
+			continue
+		}
 		peerUserID := peerUser.Model().ID
 		userMap[peerUserID] = &peerUser
 		ids = append(ids, peerUserID)

hscontrol/state/state.go

@@ -22,6 +22,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/types/change"
 	"github.com/juanfont/headscale/hscontrol/util"
+	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"golang.org/x/sync/errgroup"
 	"gorm.io/gorm"
@@ -135,6 +136,7 @@ func NewState(cfg *types.Config) (*State, error) {
 	for _, node := range nodes {
 		node.IsOnline = ptr.To(false)
 	}
+
 	users, err := db.ListUsers()
 	if err != nil {
 		return nil, fmt.Errorf("loading users: %w", err)
@@ -156,6 +158,7 @@ func NewState(cfg *types.Config) (*State, error) {
 	if batchSize == 0 {
 		batchSize = defaultNodeStoreBatchSize
 	}
+
 	batchTimeout := cfg.Tuning.NodeStoreBatchTimeout
 	if batchTimeout == 0 {
 		batchTimeout = defaultNodeStoreBatchTimeout
@@ -189,7 +192,8 @@ func NewState(cfg *types.Config) (*State, error) {
 func (s *State) Close() error {
 	s.nodeStore.Stop()
 
-	if err := s.db.Close(); err != nil {
+	err := s.db.Close()
+	if err != nil {
 		return fmt.Errorf("closing database: %w", err)
 	}
 
@@ -572,12 +576,14 @@ func (s *State) ListNodes(nodeIDs ...types.NodeID) views.Slice[types.NodeView] {
 
 	// Filter nodes by the requested IDs
 	allNodes := s.nodeStore.ListNodes()
+
 	nodeIDSet := make(map[types.NodeID]struct{}, len(nodeIDs))
 	for _, id := range nodeIDs {
 		nodeIDSet[id] = struct{}{}
 	}
 
 	var filteredNodes []types.NodeView
+
 	for _, node := range allNodes.All() {
 		if _, exists := nodeIDSet[node.ID()]; exists {
 			filteredNodes = append(filteredNodes, node)
@@ -600,12 +606,14 @@ func (s *State) ListPeers(nodeID types.NodeID, peerIDs ...types.NodeID) views.Sl
 
 	// For specific peerIDs, filter from all nodes
 	allNodes := s.nodeStore.ListNodes()
+
 	nodeIDSet := make(map[types.NodeID]struct{}, len(peerIDs))
 	for _, id := range peerIDs {
 		nodeIDSet[id] = struct{}{}
 	}
 
 	var filteredNodes []types.NodeView
+
 	for _, node := range allNodes.All() {
 		if _, exists := nodeIDSet[node.ID()]; exists {
 			filteredNodes = append(filteredNodes, node)
@@ -618,6 +626,7 @@ func (s *State) ListPeers(nodeID types.NodeID, peerIDs ...types.NodeID) views.Sl
 // ListEphemeralNodes retrieves all ephemeral (temporary) nodes in the system.
 func (s *State) ListEphemeralNodes() views.Slice[types.NodeView] {
 	allNodes := s.nodeStore.ListNodes()
+
 	var ephemeralNodes []types.NodeView
 
 	for _, node := range allNodes.All() {
@@ -749,7 +758,8 @@ func (s *State) SetApprovedRoutes(nodeID types.NodeID, routes []netip.Prefix) (t
 
 // RenameNode changes the display name of a node.
 func (s *State) RenameNode(nodeID types.NodeID, newName string) (types.NodeView, change.Change, error) {
-	if err := util.ValidateHostname(newName); err != nil {
+	err := util.ValidateHostname(newName)
+	if err != nil {
 		return types.NodeView{}, change.Change{}, fmt.Errorf("renaming node: %w", err)
 	}
 
@@ -1079,6 +1089,7 @@ func preserveNetInfo(existingNode types.NodeView, nodeID types.NodeID, validHost
 	if existingNode.Valid() {
 		existingHostinfo = existingNode.Hostinfo().AsStruct()
 	}
+
 	return netInfoFromMapRequest(nodeID, existingHostinfo, validHostinfo)
 }
 
@@ -1101,6 +1112,167 @@ type newNodeParams struct {
 	ExistingNodeForNetinfo types.NodeView
 }
 
+// authNodeUpdateParams contains parameters for updating an existing node during auth.
+type authNodeUpdateParams struct {
+	// Node to update; must be valid and in NodeStore.
+	ExistingNode types.NodeView
+	// Client data: keys, hostinfo, endpoints.
+	RegEntry *types.RegisterNode
+	// Pre-validated hostinfo; NetInfo preserved from ExistingNode.
+	ValidHostinfo *tailcfg.Hostinfo
+	// Hostname from hostinfo, or generated from keys if client omits it.
+	Hostname string
+	// Auth user; may differ from ExistingNode.User() on conversion.
+	User *types.User
+	// Overrides RegEntry.Node.Expiry; ignored for tagged nodes.
+	Expiry *time.Time
+	// Only used when IsConvertFromTag=true.
+	RegisterMethod string
+	// Set true for tagged->user conversion. Affects RegisterMethod and expiry.
+	IsConvertFromTag bool
+}
+
+// applyAuthNodeUpdate applies common update logic for re-authenticating or converting
+// an existing node. It updates the node in NodeStore, processes RequestTags, and
+// persists changes to the database.
+func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView, error) {
+	// Log the operation type
+	if params.IsConvertFromTag {
+		log.Info().
+			Str("node.name", params.ExistingNode.Hostname()).
+			Uint64("node.id", params.ExistingNode.ID().Uint64()).
+			Strs("old.tags", params.ExistingNode.Tags().AsSlice()).
+			Msg("Converting tagged node to user-owned node")
+	} else {
+		log.Info().
+			Str("node.name", params.ExistingNode.Hostname()).
+			Uint64("node.id", params.ExistingNode.ID().Uint64()).
+			Interface("hostinfo", params.RegEntry.Node.Hostinfo).
+			Msg("Updating existing node registration via reauth")
+	}
+
+	// Process RequestTags during reauth (#2979)
+	// Due to json:",omitempty", we treat empty/nil as "clear tags"
+	var requestTags []string
+	if params.RegEntry.Node.Hostinfo != nil {
+		requestTags = params.RegEntry.Node.Hostinfo.RequestTags
+	}
+
+	oldTags := params.ExistingNode.Tags().AsSlice()
+
+	// Validate tags BEFORE calling UpdateNode to ensure we don't modify NodeStore
+	// if validation fails. This maintains consistency between NodeStore and database.
+	rejectedTags := s.validateRequestTags(params.ExistingNode, requestTags)
+	if len(rejectedTags) > 0 {
+		return types.NodeView{}, fmt.Errorf(
+			"%w %v are invalid or not permitted",
+			ErrRequestedTagsInvalidOrNotPermitted,
+			rejectedTags,
+		)
+	}
+
+	// Update existing node in NodeStore - validation passed, safe to mutate
+	updatedNodeView, ok := s.nodeStore.UpdateNode(params.ExistingNode.ID(), func(node *types.Node) {
+		node.NodeKey = params.RegEntry.Node.NodeKey
+		node.DiscoKey = params.RegEntry.Node.DiscoKey
+		node.Hostname = params.Hostname
+
+		// Preserve NetInfo from existing node when re-registering
+		node.Hostinfo = params.ValidHostinfo
+		node.Hostinfo.NetInfo = preserveNetInfo(
+			params.ExistingNode,
+			params.ExistingNode.ID(),
+			params.ValidHostinfo,
+		)
+
+		node.Endpoints = params.RegEntry.Node.Endpoints
+		node.IsOnline = ptr.To(false)
+		node.LastSeen = ptr.To(time.Now())
+
+		// Set RegisterMethod - for conversion this is the new method,
+		// for reauth we preserve the existing one from regEntry
+		if params.IsConvertFromTag {
+			node.RegisterMethod = params.RegisterMethod
+		} else {
+			node.RegisterMethod = params.RegEntry.Node.RegisterMethod
+		}
+
+		// Track tagged status BEFORE processing tags
+		wasTagged := node.IsTagged()
+
+		// Process tags - may change node.Tags and node.UserID
+		// Tags were pre-validated, so this will always succeed (no rejected tags)
+		_ = s.processReauthTags(node, requestTags, params.User, oldTags)
+
+		// Handle expiry AFTER tag processing, based on transition
+		// This ensures expiry is correctly set/cleared based on the NEW tagged status
+		isTagged := node.IsTagged()
+
+		switch {
+		case wasTagged && !isTagged:
+			// Tagged → Personal: set expiry from client request
+			if params.Expiry != nil {
+				node.Expiry = params.Expiry
+			} else {
+				node.Expiry = params.RegEntry.Node.Expiry
+			}
+		case !wasTagged && isTagged:
+			// Personal → Tagged: clear expiry (tagged nodes don't expire)
+			node.Expiry = nil
+		case params.IsConvertFromTag:
+			// Explicit conversion from tagged to user-owned: set expiry from client request
+			if params.Expiry != nil {
+				node.Expiry = params.Expiry
+			} else {
+				node.Expiry = params.RegEntry.Node.Expiry
+			}
+		case !isTagged:
+			// Personal → Personal: update expiry from client
+			if params.Expiry != nil {
+				node.Expiry = params.Expiry
+			} else {
+				node.Expiry = params.RegEntry.Node.Expiry
+			}
+		}
+		// Tagged → Tagged: keep existing expiry (nil) - no action needed
+	})
+
+	if !ok {
+		return types.NodeView{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, params.ExistingNode.ID())
+	}
+
+	// Persist to database
+	// Omit AuthKeyID/AuthKey to prevent stale PreAuthKey references from causing FK errors.
+	_, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
+		err := tx.Omit("AuthKeyID", "AuthKey").Updates(updatedNodeView.AsStruct()).Error
+		if err != nil {
+			return nil, fmt.Errorf("failed to save node: %w", err)
+		}
+
+		return nil, nil //nolint:nilnil // side-effect only write
+	})
+	if err != nil {
+		return types.NodeView{}, err
+	}
+
+	// Log completion
+	if params.IsConvertFromTag {
+		log.Trace().
+			Str("node.name", updatedNodeView.Hostname()).
+			Uint64("node.id", updatedNodeView.ID().Uint64()).
+			Str("node.key", updatedNodeView.NodeKey().ShortString()).
+			Msg("Tagged node converted to user-owned")
+	} else {
+		log.Trace().
+			Str("node.name", updatedNodeView.Hostname()).
+			Uint64("node.id", updatedNodeView.ID().Uint64()).
+			Str("node.key", updatedNodeView.NodeKey().ShortString()).
+			Msg("Node re-authorized")
+	}
+
+	return updatedNodeView, nil
+}
+
 // createAndSaveNewNode creates a new node, allocates IPs, saves to DB, and adds to NodeStore.
 // It preserves netinfo from an existing node if one is provided (for faster DERP connectivity).
 func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, error) {
@@ -1148,6 +1320,7 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 			nodeToRegister.User = params.PreAuthKey.User
 			nodeToRegister.Tags = nil
 		}
+
 		nodeToRegister.AuthKey = params.PreAuthKey
 		nodeToRegister.AuthKeyID = &params.PreAuthKey.ID
 	} else {
@@ -1166,21 +1339,14 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 	// Process RequestTags (from tailscale up --advertise-tags) ONLY for non-PreAuthKey registrations.
 	// Validate early before IP allocation to avoid resource leaks on failure.
 	if params.PreAuthKey == nil && params.Hostinfo != nil && len(params.Hostinfo.RequestTags) > 0 {
-		var approvedTags, rejectedTags []string
+		// Validate all tags before applying - reject if any tag is not permitted
-
+		rejectedTags := s.validateRequestTags(nodeToRegister.View(), params.Hostinfo.RequestTags)
-		for _, tag := range params.Hostinfo.RequestTags {
-			if s.polMan.NodeCanHaveTag(nodeToRegister.View(), tag) {
-				approvedTags = append(approvedTags, tag)
-			} else {
-				rejectedTags = append(rejectedTags, tag)
-			}
-		}
-
-		// Reject registration if any requested tags are unauthorized
 		if len(rejectedTags) > 0 {
 			return types.NodeView{}, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, rejectedTags)
 		}
 
+		// All tags are approved - apply them
+		approvedTags := params.Hostinfo.RequestTags
 		if len(approvedTags) > 0 {
 			nodeToRegister.Tags = approvedTags
 			slices.Sort(nodeToRegister.Tags)
@@ -1217,12 +1383,14 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 		if err != nil {
 			return types.NodeView{}, fmt.Errorf("failed to ensure unique given name: %w", err)
 		}
+
 		nodeToRegister.GivenName = givenName
 	}
 
 	// New node - database first to get ID, then NodeStore
 	savedNode, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
-		if err := tx.Save(&nodeToRegister).Error; err != nil {
+		err := tx.Save(&nodeToRegister).Error
+		if err != nil {
 			return nil, fmt.Errorf("failed to save node: %w", err)
 		}
 
@@ -1243,6 +1411,26 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 	return s.nodeStore.PutNode(*savedNode), nil
 }
 
+// validateRequestTags validates that the requested tags are permitted for the node.
+// This should be called BEFORE UpdateNode to ensure we don't modify NodeStore
+// if validation fails. Returns the list of rejected tags (empty if all valid).
+func (s *State) validateRequestTags(node types.NodeView, requestTags []string) []string {
+	// Empty tags = clear tags, always permitted
+	if len(requestTags) == 0 {
+		return nil
+	}
+
+	var rejectedTags []string
+
+	for _, tag := range requestTags {
+		if !s.polMan.NodeCanHaveTag(node, tag) {
+			rejectedTags = append(rejectedTags, tag)
+		}
+	}
+
+	return rejectedTags
+}
+
 // processReauthTags handles tag changes during node re-authentication.
 // It processes RequestTags from the client and updates node tags accordingly.
 // Returns rejected tags (if any) for post-validation error handling.
@@ -1276,6 +1464,7 @@ func (s *State) processReauthTags(
 
 			node.Tags = []string{}
 			node.UserID = &user.ID
+			node.User = user
 		}
 
 		return nil
@@ -1368,140 +1557,75 @@ func (s *State) HandleNodeFromAuthPath(
 		regEntry.Node.Hostinfo,
 	)
 
+	// Lookup existing nodes
+	machineKey := regEntry.Node.MachineKey
+	existingNodeSameUser, _ := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(user.ID))
+	existingNodeAnyUser, _ := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
+
+	// Named conditions - describe WHAT we found, not HOW we check it
+	nodeExistsForSameUser := existingNodeSameUser.Valid()
+	nodeExistsForAnyUser := existingNodeAnyUser.Valid()
+	existingNodeIsTagged := nodeExistsForAnyUser && existingNodeAnyUser.IsTagged()
+	existingNodeOwnedByOtherUser := nodeExistsForAnyUser &&
+		!existingNodeIsTagged &&
+		existingNodeAnyUser.UserID().Get() != user.ID
+
+	// Create logger with common fields for all auth operations
+	logger := log.With().
+		Str("registration_id", registrationID.String()).
+		Str("user.name", user.Name).
+		Str("machine.key", machineKey.ShortString()).
+		Str("method", registrationMethod).
+		Logger()
+
+	// Common params for update operations
+	updateParams := authNodeUpdateParams{
+		RegEntry:       regEntry,
+		ValidHostinfo:  validHostinfo,
+		Hostname:       hostname,
+		User:           user,
+		Expiry:         expiry,
+		RegisterMethod: registrationMethod,
+	}
+
 	var finalNode types.NodeView
 
-	// Check if node already exists with same machine key for this user
+	if nodeExistsForSameUser {
-	existingNodeSameUser, existsSameUser := s.nodeStore.GetNodeByMachineKey(regEntry.Node.MachineKey, types.UserID(user.ID))
+		updateParams.ExistingNode = existingNodeSameUser
 
-	// If this node exists for this user, update the node in place.
+		finalNode, err = s.applyAuthNodeUpdate(updateParams)
-	if existsSameUser && existingNodeSameUser.Valid() {
-		log.Info().
-			Caller().
-			Str("registration_id", registrationID.String()).
-			Str("user.name", user.Name).
-			Str("registrationMethod", registrationMethod).
-			Str("node.name", existingNodeSameUser.Hostname()).
-			Uint64("node.id", existingNodeSameUser.ID().Uint64()).
-			Interface("hostinfo", regEntry.Node.Hostinfo).
-			Msg("Updating existing node registration via reauth")
-
-		// Process RequestTags during reauth (#2979)
-		// Due to json:",omitempty", we treat empty/nil as "clear tags"
-		var requestTags []string
-		if regEntry.Node.Hostinfo != nil {
-			requestTags = regEntry.Node.Hostinfo.RequestTags
-		}
-
-		oldTags := existingNodeSameUser.Tags().AsSlice()
-
-		var rejectedTags []string
-
-		// Update existing node - NodeStore first, then database
-		updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
-			node.NodeKey = regEntry.Node.NodeKey
-			node.DiscoKey = regEntry.Node.DiscoKey
-			node.Hostname = hostname
-
-			// TODO(kradalby): We should ensure we use the same hostinfo and node merge semantics
-			// when a node re-registers as we do when it sends a map request (UpdateNodeFromMapRequest).
-
-			// Preserve NetInfo from existing node when re-registering
-			node.Hostinfo = validHostinfo
-			node.Hostinfo.NetInfo = preserveNetInfo(existingNodeSameUser, existingNodeSameUser.ID(), validHostinfo)
-
-			node.Endpoints = regEntry.Node.Endpoints
-			node.RegisterMethod = regEntry.Node.RegisterMethod
-			node.IsOnline = ptr.To(false)
-			node.LastSeen = ptr.To(time.Now())
-
-			// Tagged nodes keep their existing expiry (disabled).
-			// User-owned nodes update expiry from the provided value or registration entry.
-			if !node.IsTagged() {
-				if expiry != nil {
-					node.Expiry = expiry
-				} else {
-					node.Expiry = regEntry.Node.Expiry
-				}
-			}
-
-			rejectedTags = s.processReauthTags(node, requestTags, user, oldTags)
-		})
-
-		if !ok {
-			return types.NodeView{}, change.Change{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, existingNodeSameUser.ID())
-		}
-
-		if len(rejectedTags) > 0 {
-			return types.NodeView{}, change.Change{}, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, rejectedTags)
-		}
-
-		_, err = hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
-			// Use Updates() to preserve fields not modified by UpdateNode.
-			// Omit AuthKeyID/AuthKey to prevent stale PreAuthKey references from causing FK errors.
-			err := tx.Omit("AuthKeyID", "AuthKey").Updates(updatedNodeView.AsStruct()).Error
-			if err != nil {
-				return nil, fmt.Errorf("failed to save node: %w", err)
-			}
-			return nil, nil
-		})
 		if err != nil {
 			return types.NodeView{}, change.Change{}, err
 		}
+	} else if existingNodeIsTagged {
+		updateParams.ExistingNode = existingNodeAnyUser
+		updateParams.IsConvertFromTag = true
 
-		log.Trace().
+		finalNode, err = s.applyAuthNodeUpdate(updateParams)
-			Caller().
+		if err != nil {
-			Str("node.name", updatedNodeView.Hostname()).
+			return types.NodeView{}, change.Change{}, err
-			Uint64("node.id", updatedNodeView.ID().Uint64()).
-			Str("machine.key", regEntry.Node.MachineKey.ShortString()).
-			Str("node.key", updatedNodeView.NodeKey().ShortString()).
-			Str("user.name", user.Name).
-			Msg("Node re-authorized")
-
-		finalNode = updatedNodeView
-	} else {
-		// Node does not exist for this user with this machine key
-		// Check if node exists with this machine key for a different user (for netinfo preservation)
-		existingNodeAnyUser, existsAnyUser := s.nodeStore.GetNodeByMachineKeyAnyUser(regEntry.Node.MachineKey)
-
-		if existsAnyUser && existingNodeAnyUser.Valid() && existingNodeAnyUser.UserID().Get() != user.ID {
-			// Node exists but belongs to a different user
-			// Create a NEW node for the new user (do not transfer)
-			// This allows the same machine to have separate node identities per user
-			oldUser := existingNodeAnyUser.User()
-			log.Info().
-				Caller().
-				Str("existing.node.name", existingNodeAnyUser.Hostname()).
-				Uint64("existing.node.id", existingNodeAnyUser.ID().Uint64()).
-				Str("machine.key", regEntry.Node.MachineKey.ShortString()).
-				Str("old.user", oldUser.Name()).
-				Str("new.user", user.Name).
-				Str("method", registrationMethod).
-				Msg("Creating new node for different user (same machine key exists for another user)")
 		}
+	} else if existingNodeOwnedByOtherUser {
+		oldUser := existingNodeAnyUser.User()
 
-		// Create a completely new node
+		logger.Info().
-		log.Debug().
+			Str("existing.node.name", existingNodeAnyUser.Hostname()).
-			Caller().
+			Uint64("existing.node.id", existingNodeAnyUser.ID().Uint64()).
-			Str("registration_id", registrationID.String()).
+			Str("old.user", oldUser.Name()).
-			Str("user.name", user.Name).
+			Msg("Creating new node for different user (same machine key exists for another user)")
-			Str("registrationMethod", registrationMethod).
-			Str("expiresAt", fmt.Sprintf("%v", expiry)).
-			Msg("Registering new node from auth callback")
 
-		// Create and save new node
+		finalNode, err = s.createNewNodeFromAuth(
-		var err error
+			logger, user, regEntry, hostname, validHostinfo,
-		finalNode, err = s.createAndSaveNewNode(newNodeParams{
+			expiry, registrationMethod, existingNodeAnyUser,
-			User:                   *user,
+		)
-			MachineKey:             regEntry.Node.MachineKey,
+		if err != nil {
-			NodeKey:                regEntry.Node.NodeKey,
+			return types.NodeView{}, change.Change{}, err
-			DiscoKey:               regEntry.Node.DiscoKey,
+		}
-			Hostname:               hostname,
+	} else {
-			Hostinfo:               validHostinfo,
+		finalNode, err = s.createNewNodeFromAuth(
-			Endpoints:              regEntry.Node.Endpoints,
+			logger, user, regEntry, hostname, validHostinfo,
-			Expiry:                 cmp.Or(expiry, regEntry.Node.Expiry),
+			expiry, registrationMethod, types.NodeView{},
-			RegisterMethod:         registrationMethod,
+		)
-			ExistingNodeForNetinfo: cmp.Or(existingNodeAnyUser, types.NodeView{}),
-		})
 		if err != nil {
 			return types.NodeView{}, change.Change{}, err
 		}
@@ -1534,6 +1658,37 @@ func (s *State) HandleNodeFromAuthPath(
 	return finalNode, c, nil
 }
 
+// createNewNodeFromAuth creates a new node during auth callback.
+// This is used for both new registrations and when a machine already has a node
+// for a different user.
+func (s *State) createNewNodeFromAuth(
+	logger zerolog.Logger,
+	user *types.User,
+	regEntry *types.RegisterNode,
+	hostname string,
+	validHostinfo *tailcfg.Hostinfo,
+	expiry *time.Time,
+	registrationMethod string,
+	existingNodeForNetinfo types.NodeView,
+) (types.NodeView, error) {
+	logger.Debug().
+		Interface("expiry", expiry).
+		Msg("Registering new node from auth callback")
+
+	return s.createAndSaveNewNode(newNodeParams{
+		User:                   *user,
+		MachineKey:             regEntry.Node.MachineKey,
+		NodeKey:                regEntry.Node.NodeKey,
+		DiscoKey:               regEntry.Node.DiscoKey,
+		Hostname:               hostname,
+		Hostinfo:               validHostinfo,
+		Endpoints:              regEntry.Node.Endpoints,
+		Expiry:                 cmp.Or(expiry, regEntry.Node.Expiry),
+		RegisterMethod:         registrationMethod,
+		ExistingNodeForNetinfo: existingNodeForNetinfo,
+	})
+}
+
 // HandleNodeFromPreAuthKey handles node registration using a pre-authentication key.
 func (s *State) HandleNodeFromPreAuthKey(
 	regReq tailcfg.RegisterRequest,
@@ -1753,6 +1908,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 		}
 
 		var err error
+
 		finalNode, err = s.createAndSaveNewNode(newNodeParams{
 			User:                   pakUser,
 			MachineKey:             machineKey,
@@ -1890,7 +2046,9 @@ func (s *State) autoApproveNodes() ([]change.Change, error) {
 				}
 
 				mu.Lock()
+
 				cs = append(cs, c)
+
 				mu.Unlock()
 			}
 
@@ -1960,6 +2118,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 		if hi := req.Hostinfo; hi != nil {
 			hasNewRoutes = len(hi.RoutableIPs) > 0
 		}
+
 		needsRouteApproval = hostinfoChanged && (routesChanged(currentNode.View(), req.Hostinfo) || (hasNewRoutes && len(currentNode.ApprovedRoutes) == 0))
 		if needsRouteApproval {
 			// Extract announced routes from request
@@ -2112,6 +2271,7 @@ func hostinfoEqual(oldNode types.NodeView, newHI *tailcfg.Hostinfo) bool {
 	if !oldNode.Valid() || newHI == nil {
 		return false
 	}
+
 	old := oldNode.AsStruct().Hostinfo
 
 	return old.Equal(newHI)

integration/tags_test.go

@@ -3116,3 +3116,122 @@ func TestTagsAuthKeyWithoutUserRejectsAdvertisedTags(t *testing.T) {
 		t.Fail()
 	}
 }
+
+// =============================================================================
+// Test Suite 6: Tagged→User Conversion via CLI Register (#3038)
+// =============================================================================
+
+// TestTagsAuthKeyConvertToUserViaCLIRegister reproduces the panic from
+// issue #3038: register a node with a tags-only preauthkey (no user), then
+// convert it to a user-owned node via "headscale nodes register --user <user> --key ...".
+// The crash happens in the mapper's generateUserProfiles when node.User is nil
+// after the tag→user conversion in processReauthTags.
+//
+// The key detail is using a tags-only PreAuthKey (User: nil). When created under
+// a user, the node inherits User from the PreAuthKey and the bug is masked.
+func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
+	IntegrationSkip(t)
+
+	policy := tagsTestPolicy()
+
+	spec := ScenarioSpec{
+		NodesPerUser: 0,
+		Users:        []string{tagTestUser},
+	}
+
+	scenario, err := NewScenario(spec)
+
+	require.NoError(t, err)
+	defer scenario.ShutdownAssertNoPanics(t)
+
+	err = scenario.CreateHeadscaleEnvWithLoginURL(
+		[]tsic.Option{},
+		hsic.WithACLPolicy(policy),
+		hsic.WithTestName("tags-authkey-to-user-cli-3038"),
+		hsic.WithTLS(),
+	)
+	requireNoErrHeadscaleEnv(t, err)
+
+	headscale, err := scenario.Headscale()
+	requireNoErrGetHeadscale(t, err)
+
+	// Step 1: Create a tags-only preauthkey WITHOUT a user.
+	// This is the critical detail: when PreAuthKey.UserID is nil, the node
+	// enters the NodeStore with node.User == nil. The processReauthTags
+	// conversion then sets UserID but not User, leaving it nil for the mapper.
+	authKey, err := scenario.CreatePreAuthKeyWithOptions(hsic.AuthKeyOptions{
+		User:      nil,
+		Reusable:  false,
+		Ephemeral: false,
+		Tags:      []string{"tag:valid-owned"},
+	})
+	require.NoError(t, err)
+	t.Logf("Created tags-only PreAuthKey (no user) with tags: %v", authKey.GetAclTags())
+
+	client, err := scenario.CreateTailscaleNode(
+		"head",
+		tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]),
+	)
+	require.NoError(t, err)
+
+	err = client.Login(headscale.GetEndpoint(), authKey.GetKey())
+	require.NoError(t, err)
+
+	err = client.WaitForRunning(120 * time.Second)
+	require.NoError(t, err)
+
+	// Verify initial state: node is tagged
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		nodes, err := headscale.ListNodes()
+		assert.NoError(c, err)
+		assert.Len(c, nodes, 1)
+
+		if len(nodes) == 1 {
+			assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
+			t.Logf("Initial state - Node ID: %d, Tags: %v", nodes[0].GetId(), nodes[0].GetTags())
+		}
+	}, 30*time.Second, 500*time.Millisecond, "node should be tagged initially")
+
+	// Step 2: Force reauth with empty tags (triggers web auth flow)
+	command := []string{
+		"tailscale", "up",
+		"--login-server=" + headscale.GetEndpoint(),
+		"--hostname=" + client.Hostname(),
+		"--advertise-tags=",
+		"--force-reauth",
+	}
+
+	stdout, stderr, _ := client.Execute(command)
+	t.Logf("Reauth command output: stdout=%s stderr=%s", stdout, stderr)
+
+	loginURL, err := util.ParseLoginURLFromCLILogin(stdout + stderr)
+	require.NoError(t, err, "Failed to parse login URL from reauth command")
+
+	body, err := doLoginURL(client.Hostname(), loginURL)
+	require.NoError(t, err)
+
+	// Step 3: Register via CLI with user (this is the exact step that triggers the panic)
+	err = scenario.runHeadscaleRegister(tagTestUser, body)
+	require.NoError(t, err)
+
+	err = client.WaitForRunning(120 * time.Second)
+	require.NoError(t, err)
+
+	// Step 4: Verify node is now user-owned and the mapper didn't panic.
+	// The panic would occur when the mapper builds the MapResponse and calls
+	// node.Owner().Model().ID with a nil User pointer.
+	// ShutdownAssertNoPanics in the defer catches any panics in headscale logs.
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		nodes, err := headscale.ListNodes()
+		assert.NoError(c, err)
+		assert.Len(c, nodes, 1)
+
+		if len(nodes) == 1 {
+			assertNodeHasNoTagsWithCollect(c, nodes[0])
+			assert.Equal(c, tagTestUser, nodes[0].GetUser().GetName(),
+				"Node ownership should be returned to user after untagging")
+			t.Logf("After conversion - Node ID: %d, Tags: %v, User: %s",
+				nodes[0].GetId(), nodes[0].GetTags(), nodes[0].GetUser().GetName())
+		}
+	}, 60*time.Second, 1*time.Second, "node should be user-owned after conversion via CLI register")
+}