state: avoid nil deref in registration handlers when old user is missing

Mirror the guard from HandleNodeFromPreAuthKey in HandleNodeFromAuthPath.
Both functions log the old user's name in the "different user" branch
when an existing NodeStore entry under the same machine key belongs to
another user. UserView.Name dereferences the backing User pointer
unconditionally, so when the cached node was loaded with a non-nil
UserID but a nil User (Preload join missed the row, or upstream code
left the snapshot in that shape), the log call panics with a nil-pointer
dereference at hscontrol/types/types_view.go:97.

The panic is caught by the http2 server's runHandler for the noise
control plane, so the process keeps running but every retry produces a
new panic — production has observed bursts of ~1.9k panics per hour
during a tailscaled reconnect loop. The gRPC/OIDC entry has no equivalent
recover and would surface the panic to the caller.

Guard both call sites with oldUser.Valid() and fall back to an empty
old-user name when the pointer is nil. The "Creating new node for
different user" log line still includes the existing node ID, hostname,
machine key, and new user, so operator visibility is preserved.

Add reproduction tests for both handlers seeding the orphan shape
directly into NodeStore via PutNodeInStoreForTest.

Co-Authored-By: Kristoffer Dalby <kristoffer@dalby.cc>

hscontrol/auth_test.go

@@ -3988,3 +3988,116 @@ func TestTaggedNodeWithoutUserToDifferentUser(t *testing.T) {
 		nodeAfterReauth.ID().Uint64(), nodeAfterReauth.Tags().AsSlice(),
 		nodeAfterReauth.IsTagged(), nodeAfterReauth.UserID().Get())
 }
+
+// TestHandleNodeFromPreAuthKey_OldUserNil_NoPanic asserts that
+// HandleNodeFromPreAuthKey does not panic when the in-memory NodeStore
+// holds a non-tagged node whose UserID points at a user but whose User
+// pointer is nil (orphan snapshot, e.g. a Preload("User") join missed
+// the row). Re-registering the same machine key under a different user
+// enters the "different user" branch and would otherwise crash at
+// oldUser.Name() in UserView.Name when the backing pointer is nil.
+func TestHandleNodeFromPreAuthKey_OldUserNil_NoPanic(t *testing.T) {
+	app := createTestApp(t)
+
+	userA := app.state.CreateUserForTest("preauth-orphan-old")
+	userB := app.state.CreateUserForTest("preauth-orphan-new")
+
+	machineKey := key.NewMachine()
+	orphanNodeKey := key.NewNode()
+
+	userIDA := userA.ID
+	orphan := types.Node{
+		ID:             99001,
+		MachineKey:     machineKey.Public(),
+		NodeKey:        orphanNodeKey.Public(),
+		Hostname:       "preauth-orphan",
+		GivenName:      "preauth-orphan",
+		UserID:         &userIDA,
+		User:           nil,
+		RegisterMethod: "authkey",
+	}
+	app.state.PutNodeInStoreForTest(orphan)
+
+	pakB, err := app.state.CreatePreAuthKey(userB.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	newNodeKey := key.NewNode()
+	regReq := tailcfg.RegisterRequest{
+		Auth: &tailcfg.RegisterResponseAuth{
+			AuthKey: pakB.Key,
+		},
+		NodeKey: newNodeKey.Public(),
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname: "preauth-orphan-newuser",
+		},
+		Expiry: time.Now().Add(24 * time.Hour),
+	}
+
+	resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err, "registration must not panic when old user is nil")
+	require.NotNil(t, resp)
+	require.True(t, resp.MachineAuthorized)
+
+	var registered types.NodeView
+
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		var found bool
+
+		registered, found = app.state.GetNodeByNodeKey(newNodeKey.Public())
+		assert.True(c, found, "new node should be available in NodeStore")
+	}, 1*time.Second, 50*time.Millisecond, "waiting for new node")
+
+	assert.NotEqual(t, types.NodeID(99001), registered.ID(), "new node, not orphan")
+	assert.Equal(t, userB.ID, registered.UserID().Get(), "new node belongs to userB")
+}
+
+// TestHandleNodeFromAuthPath_OldUserNil_NoPanic is the parallel guard
+// for the gRPC/OIDC entry point. Same orphan shape as
+// TestHandleNodeFromPreAuthKey_OldUserNil_NoPanic; HandleNodeFromAuthPath
+// has its own oldUser.Name() log line in the existingNodeOwnedByOtherUser
+// branch and panics independently of the noise registration path.
+func TestHandleNodeFromAuthPath_OldUserNil_NoPanic(t *testing.T) {
+	app := createTestApp(t)
+
+	userA := app.state.CreateUserForTest("authpath-orphan-old")
+	userB := app.state.CreateUserForTest("authpath-orphan-new")
+
+	machineKey := key.NewMachine()
+	orphanNodeKey := key.NewNode()
+
+	userIDA := userA.ID
+	orphan := types.Node{
+		ID:             99002,
+		MachineKey:     machineKey.Public(),
+		NodeKey:        orphanNodeKey.Public(),
+		Hostname:       "authpath-orphan",
+		GivenName:      "authpath-orphan",
+		UserID:         &userIDA,
+		User:           nil,
+		RegisterMethod: "oidc",
+	}
+	app.state.PutNodeInStoreForTest(orphan)
+
+	newNodeKey := key.NewNode()
+	authID := types.MustAuthID()
+	regEntry := types.NewRegisterAuthRequest(&types.RegistrationData{
+		MachineKey: machineKey.Public(),
+		NodeKey:    newNodeKey.Public(),
+		Hostname:   "authpath-orphan-newuser",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname: "authpath-orphan-newuser",
+		},
+	})
+	app.state.SetAuthCacheEntry(authID, regEntry)
+
+	node, _, err := app.state.HandleNodeFromAuthPath(
+		authID,
+		types.UserID(userB.ID),
+		nil,
+		"oidc",
+	)
+	require.NoError(t, err, "auth-path registration must not panic on nil old user")
+	require.True(t, node.Valid())
+	assert.NotEqual(t, types.NodeID(99002), node.ID(), "new node, not orphan")
+	assert.Equal(t, userB.ID, node.UserID().Get(), "new node belongs to userB")
+}

hscontrol/state/state.go

@@ -1920,10 +1920,15 @@ func (s *State) HandleNodeFromAuthPath(
 	} else if existingNodeOwnedByOtherUser {
 		oldUser := existingNodeAnyUser.User()
 
+		oldUserName := ""
+		if oldUser.Valid() {
+			oldUserName = oldUser.Name()
+		}
+
 		logger.Info().
 			Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
 			Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
-			Str(zf.OldUser, oldUser.Name()).
+			Str(zf.OldUser, oldUserName).
 			Msg("Creating new node for different user (same machine key exists for another user)")
 
 		finalNode, err = s.createNewNodeFromAuth(
@@ -2223,7 +2228,12 @@ func (s *State) HandleNodeFromPreAuthKey(
 		if belongsToDifferentUser {
 			// Node exists but belongs to a different user.
 			// Create a new node for the new user (do not transfer).
-			oldUserName := existingNodeAnyUser.User().Name()
+			oldUser := existingNodeAnyUser.User()
+
+			oldUserName := ""
+			if oldUser.Valid() {
+				oldUserName = oldUser.Name()
+			}
 
 			log.Info().
 				Caller().