hscontrol: skip garbage collecting a node that is no longer ephemeral

hscontrol/app.go

@@ -183,6 +183,15 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) {
 			return
 		}
 
+		// Ephemerality can change after the timer is armed: re-registration
+		// with a permanent key, or a user claiming a tagged node.
+		if !node.IsEphemeral() {
+			log.Debug().Caller().EmbedObject(node).
+				Msg("node is no longer ephemeral, skipping garbage collection")
+
+			return
+		}
+
 		changes, err := app.state.DeleteNode(node)
 		app.Change(changes...)
 

hscontrol/poll_test.go

@@ -359,6 +359,63 @@ func TestFailedReconnectDoesNotCancelEphemeralGC(t *testing.T) {
 		"failed reconnect must not cancel the ephemeral GC timer (issue #3382)")
 }
 
+func TestStaleGCTimerSparesPermanentNode(t *testing.T) {
+	t.Parallel()
+	app := createTestApp(t)
+	app.cfg.Node.Ephemeral.InactivityTimeout = time.Hour
+	app.StartEphemeralGCForTest(t)
+	user := app.state.CreateUserForTest("gc-permanent-user")
+	ephemeral, err := app.state.CreatePreAuthKey(user.TypedID(), true, true, nil, nil)
+	require.NoError(t, err)
+	permanent, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine().Public()
+	req := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: ephemeral.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "gc-permanent-node"},
+	}
+	_, err = app.handleRegisterWithAuthKey(req, machineKey)
+	require.NoError(t, err)
+
+	node, ok := app.state.GetNodeByNodeKey(req.NodeKey)
+	require.True(t, ok)
+	require.True(t, node.IsEphemeral())
+	session := app.newMapSession(t.Context(), tailcfg.MapRequest{},
+		&recordingResponseWriter{}, node)
+	_, epoch := app.state.Connect(node.ID())
+	app.ephemeralGC.Cancel(node.ID())
+
+	req.Auth.AuthKey = permanent.Key
+	_, err = app.handleRegisterWithAuthKey(req, machineKey)
+	require.NoError(t, err)
+
+	fresh, ok := app.state.GetNodeByID(node.ID())
+	require.True(t, ok)
+	require.False(t, fresh.IsEphemeral())
+
+	_, err = app.state.Disconnect(node.ID(), epoch)
+	require.NoError(t, err)
+
+	app.cfg.Node.Ephemeral.InactivityTimeout = 25 * time.Millisecond
+
+	session.afterServeLongPoll()
+	require.True(t, app.ephemeralGC.IsScheduled(node.ID()))
+
+	require.Eventually(t, func() bool {
+		return !app.ephemeralGC.IsScheduled(node.ID())
+	}, time.Second, 5*time.Millisecond)
+	assert.Never(t, func() bool {
+		_, ok := app.state.GetNodeByID(node.ID())
+		return !ok
+	}, 2*time.Second, 5*time.Millisecond,
+		"a stale session must not collect a node that became permanent")
+
+	_, err = app.state.DB().GetNodeByID(node.ID())
+	require.NoError(t, err)
+}
+
 // TestEphemeralGCDoesNotDeleteReconnectedNode replays, in order, the
 // interleaving of two sessions from
 // https://github.com/juanfont/headscale/issues/3535: the old session takes the