feature,client: add serviceclientprefs for desktop client service launch (#20501)

Add serviceclientprefs, an optional feature that stores and loads the desktop clients' saved service launch preferences, one file per login profile. - Add GET|POST /localapi/v0/prefs/service-clients to load and save the current profile's service client prefs. - Add local client GetServiceClientPrefs and SetServiceClientPref that call the new local api endpoint. - Store the prefs with the ipn/store FileStore at TailscaleVarRoot()/profile-data/<profileID>/service-client-prefs/<hex-encoded-key>, so DeleteProfile cleans them up for free. Fall back to an in-memory store when there's no var root. - Register the feature and its local api route from build tagged files so the whole thing drops out under ts_omit_serviceclientprefs. - Add the serviceclient package holding Pref and Prefs (saved client, username, database name, and last used time), so the local api client and desktop apps can import the types without the feature machinery. Change-Id: I340a99c1b332d181fb1556fbf3e8003bb3b95a08 Updates: https://github.com/tailscale/tailscale/issues/20429 Signed-off-by: Rollie Ma <rollie@tailscale.com>
2026-07-22 20:36:05 -07:00
parent 66bb4ac61f
commit f4978b4b9b
20 changed files with 597 additions and 3 deletions
@@ -0,0 +1,13 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Code generated by gen.go; DO NOT EDIT.
+
+//go:build ts_omit_serviceclientprefs
+
+package buildfeatures
+
+// HasServiceClientPrefs is whether the binary was built with support for modular feature "Desktop client service launch preferences".
+// Specifically, it's whether the binary was NOT built with the "ts_omit_serviceclientprefs" build tag.
+// It's a const so it can be used for dead code elimination.
+const HasServiceClientPrefs = false
@@ -0,0 +1,13 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Code generated by gen.go; DO NOT EDIT.
+
+//go:build !ts_omit_serviceclientprefs
+
+package buildfeatures
+
+// HasServiceClientPrefs is whether the binary was built with support for modular feature "Desktop client service launch preferences".
+// Specifically, it's whether the binary was NOT built with the "ts_omit_serviceclientprefs" build tag.
+// It's a const so it can be used for dead code elimination.
+const HasServiceClientPrefs = true
@@ -0,0 +1,8 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_serviceclientprefs
+
+package condregister
+
+import _ "tailscale.com/feature/serviceclientprefs"
@@ -229,8 +229,9 @@ var Features = map[FeatureTag]FeatureMeta{
 		Desc: "Linux NetworkManager integration",
 		Deps: []FeatureTag{"dbus"},
 	},
-	"qrcodes":     {Sym: "QRCodes", Desc: "QR codes in tailscale CLI"},
-	"relayserver": {Sym: "RelayServer", Desc: "Relay server"},
+	"serviceclientprefs": {Sym: "ServiceClientPrefs", Desc: "Desktop client service launch preferences"},
+	"qrcodes":            {Sym: "QRCodes", Desc: "QR codes in tailscale CLI"},
+	"relayserver":        {Sym: "RelayServer", Desc: "Relay server"},
 	"remoteconfig": {
 		Sym:  "RemoteConfig",
 		Desc: "Full remote configuration of this node by the tailnet admin, opting out of Tailscale's per-feature double opt-in in favor of a single client-side trust decision",
@@ -0,0 +1,68 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package serviceclientprefs
+
+import (
+	"encoding/json"
+	"errors"
+	"net/http"
+
+	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/feature/serviceclientprefs/serviceclient"
+	"tailscale.com/ipn/ipnlocal"
+	"tailscale.com/ipn/localapi"
+	"tailscale.com/util/httpm"
+)
+
+func init() {
+	localapi.Register("prefs/service-clients", serveServiceClientPrefs)
+}
+
+// serveServiceClientPrefs handles GET and POST /localapi/v0/prefs/service-clients. GET returns all
+// of the current profile's service client prefs. POST merges one [apitype.ServiceClientPrefRequest]
+// into the saved service client prefs and returns the full updated set.
+func serveServiceClientPrefs(h *localapi.Handler, w http.ResponseWriter, r *http.Request) {
+	if !h.PermitRead {
+		http.Error(w, "service-clients access denied", http.StatusForbidden)
+		return
+	}
+	ext, ok := ipnlocal.GetExt[*extension](h.LocalBackend())
+	if !ok {
+		http.Error(w, "misconfigured serviceclientprefs extension", http.StatusInternalServerError)
+		return
+	}
+	var out serviceclient.Prefs
+	switch r.Method {
+	case httpm.GET:
+		var err error
+		if out, err = ext.serviceClientPrefs(); err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+	case httpm.POST:
+		if !h.PermitWrite {
+			http.Error(w, "service-clients write access denied", http.StatusForbidden)
+			return
+		}
+		var req apitype.ServiceClientPrefRequest
+		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+		var err error
+		if out, err = ext.setServiceClientPref(req); err != nil {
+			status := http.StatusInternalServerError
+			if errors.Is(err, errInvalidServiceClientPref) {
+				status = http.StatusBadRequest
+			}
+			http.Error(w, err.Error(), status)
+			return
+		}
+	default:
+		http.Error(w, "use GET or POST", http.StatusMethodNotAllowed)
+		return
+	}
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(out)
+}
@@ -0,0 +1,98 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package serviceclientprefs
+
+import (
+	"encoding/json"
+	"errors"
+
+	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/feature/serviceclientprefs/serviceclient"
+	"tailscale.com/ipn"
+)
+
+// errInvalidServiceClientPref is returned by [extension.setServiceClientPref] for a request that
+// fails validation, such as a missing key. The handler maps it to a 400, unlike backend failures.
+var errInvalidServiceClientPref = errors.New("service client pref key is required")
+
+// serviceClientPrefs returns the saved service client prefs for the current profile. It returns an
+// empty, non-nil map when nothing has been saved yet, or when there's no current profile (a logged
+// out node simply has no service client prefs).
+func (e *extension) serviceClientPrefs() (serviceclient.Prefs, error) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	return e.loadLocked()
+}
+
+// setServiceClientPref merges the non-empty fields from [apitype.ServiceClientPrefRequest] into the
+// saved pref for its key, stamps [serviceclient.Pref.LastUsed] with the current time, and returns
+// the full updated service client prefs for the current profile.
+func (e *extension) setServiceClientPref(req apitype.ServiceClientPrefRequest) (serviceclient.Prefs, error) {
+	if req.Key == "" {
+		return nil, errInvalidServiceClientPref
+	}
+
+	// Hold mu across the whole read, change, and write so two writers can't clobber each other,
+	// since each writes back the full prefs map.
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
+	prefs, err := e.loadLocked()
+	if err != nil {
+		return nil, err
+	}
+	pref := prefs[req.Key]
+	if req.Client != "" {
+		pref.Client = req.Client
+	}
+	if req.Username != "" {
+		pref.Username = req.Username
+	}
+	if req.DatabaseName != "" {
+		pref.DatabaseName = req.DatabaseName
+	}
+	pref.LastUsed = e.sb.Clock().Now()
+	prefs[req.Key] = pref
+
+	if err := e.saveLocked(prefs); err != nil {
+		return nil, err
+	}
+	return prefs, nil
+}
+
+// loadLocked reads the current profile's prefs, returning an empty map when there's no store (no
+// current profile) or nothing saved yet. The caller must hold e.mu.
+func (e *extension) loadLocked() (serviceclient.Prefs, error) {
+	if e.store == nil {
+		return serviceclient.Prefs{}, nil
+	}
+	data, err := e.store.ReadState(storeKey)
+	if errors.Is(err, ipn.ErrStateNotExist) {
+		return serviceclient.Prefs{}, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	var prefs serviceclient.Prefs
+	if err := json.Unmarshal(data, &prefs); err != nil {
+		return nil, err
+	}
+	if prefs == nil {
+		prefs = serviceclient.Prefs{}
+	}
+	return prefs, nil
+}
+
+// saveLocked writes prefs to the current profile's store. It returns an error when there's no
+// store to persist to (no current profile, or the store failed to build). The caller must hold e.mu.
+func (e *extension) saveLocked(prefs serviceclient.Prefs) error {
+	if e.store == nil {
+		return errors.New("no store available to persist service client prefs")
+	}
+	data, err := json.Marshal(prefs)
+	if err != nil {
+		return err
+	}
+	return e.store.WriteState(storeKey, data)
+}
@@ -0,0 +1,116 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package serviceclientprefs
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/feature/serviceclientprefs/serviceclient"
+	"tailscale.com/ipn"
+)
+
+func TestServiceClientPrefsSetAndGet(t *testing.T) {
+	tests := []struct {
+		name     string
+		requests []apitype.ServiceClientPrefRequest
+		want     serviceclient.Prefs
+	}{
+		{
+			name:     "records_a_single_launch",
+			requests: []apitype.ServiceClientPrefRequest{{Key: "ssh:22", Client: "terminal", Username: "rollie"}},
+			want:     serviceclient.Prefs{"ssh:22": {Client: "terminal", Username: "rollie"}},
+		},
+		{
+			name: "partial_update_preserves_existing_fields",
+			requests: []apitype.ServiceClientPrefRequest{
+				{Key: "ssh:22", Client: "terminal", Username: "rollie"},
+				{Key: "ssh:22", Client: "iterm2"},
+			},
+			want: serviceclient.Prefs{"ssh:22": {Client: "iterm2", Username: "rollie"}},
+		},
+		{
+			name: "independent_services_kept_separate",
+			requests: []apitype.ServiceClientPrefRequest{
+				{Key: "ssh:22", Client: "terminal"},
+				{Key: "db:5432", Client: "psql", DatabaseName: "prod"},
+			},
+			want: serviceclient.Prefs{
+				"ssh:22":  {Client: "terminal"},
+				"db:5432": {Client: "psql", DatabaseName: "prod"},
+			},
+		},
+	}
+
+	ignoreLastUsed := cmpopts.IgnoreFields(serviceclient.Pref{}, "LastUsed")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := newTestExtension(t, "pid")
+			for _, req := range tt.requests {
+				if _, err := e.setServiceClientPref(req); err != nil {
+					t.Fatal(err)
+				}
+			}
+			got, err := e.serviceClientPrefs()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if diff := cmp.Diff(tt.want, got, ignoreLastUsed, cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("(-want +got):\n%s", diff)
+			}
+			for key := range tt.want {
+				if got[key].LastUsed.IsZero() {
+					t.Errorf("%s: LastUsed was not stamped", key)
+				}
+			}
+		})
+	}
+}
+
+func TestServiceClientPrefsEmptyKeyRejected(t *testing.T) {
+	e := newTestExtension(t, "pid")
+	_, err := e.setServiceClientPref(apitype.ServiceClientPrefRequest{Client: "terminal"})
+	if !errors.Is(err, errInvalidServiceClientPref) {
+		t.Errorf("want errInvalidServiceClientPref for empty key, got %v", err)
+	}
+}
+
+func TestServiceClientPrefsNoCurrentProfileReturnsEmpty(t *testing.T) {
+	e := newTestExtension(t, "") // no current profile
+	got, err := e.serviceClientPrefs()
+	if err != nil {
+		t.Fatalf("want nil error for no current profile, got %v", err)
+	}
+	if got == nil || len(got) != 0 {
+		t.Errorf("want empty non-nil map, got %#v", got)
+	}
+}
+
+func TestServiceClientPrefsRepopulatedOnProfileSwitch(t *testing.T) {
+	e := newTestExtension(t, "pid0")
+
+	// Save a pref for pid0.
+	if _, err := e.setServiceClientPref(apitype.ServiceClientPrefRequest{Key: "ssh:22", Client: "terminal"}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Switch to a different profile; it must not see pid0's data.
+	e.onChangeProfile((&ipn.LoginProfile{ID: "pid1"}).View(), ipn.PrefsView{}, false)
+	if got, _ := e.serviceClientPrefs(); len(got) != 0 {
+		t.Fatalf("pid1 unexpectedly saw pid0's prefs: %v", got)
+	}
+
+	// Switch back to pid0; its saved pref should be read back from disk into the cache.
+	e.onChangeProfile((&ipn.LoginProfile{ID: "pid0"}).View(), ipn.PrefsView{}, false)
+	got, err := e.serviceClientPrefs()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got["ssh:22"].Client != "terminal" {
+		t.Errorf("after switching back, got %v, want Client=terminal", got["ssh:22"])
+	}
+}
@@ -0,0 +1,35 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package serviceclient holds the types for the desktop clients' saved service launch preferences.
+// They're shared by the serviceclientprefs feature and the LocalAPI client.
+package serviceclient
+
+import (
+	"time"
+)
+
+// Prefs maps a service key to the user's saved preferences for that service.
+// Keys are "<serviceName>:<port>" where serviceName is a [tailcfg.ServiceName]
+// (e.g. key "svc:my-db:5432" for service "svc:my-db" on port 5432).
+type Prefs map[string]Pref
+
+// Pref captures the saved preferences for one service.
+type Pref struct {
+	// Client is the saved client identifier the user picked in the last service launch,
+	// for example, terminal/putty/iterm2/... for SSH, and dbeaver/psql/mycli/... for
+	// database. Empty means no client has been saved for this service.
+	Client string `json:",omitzero"`
+
+	// Username is the saved login name for SSH and future services that require username.
+	// Empty means none saved.
+	Username string `json:",omitzero"`
+
+	// DatabaseName is the saved DB name (database service types). Empty means none saved.
+	DatabaseName string `json:",omitzero"`
+
+	// LastUsed is when this service was last launched. Zero means never.
+	// When the macOS and Windows apps generate a "recently used" list, they sort by
+	// LastUsed descending.
+	LastUsed time.Time `json:",omitzero"`
+}
@@ -0,0 +1,102 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package serviceclientprefs stores the desktop clients' saved service launch preferences, in a
+// file per login profile.
+package serviceclientprefs
+
+import (
+	"encoding/hex"
+	"path/filepath"
+	"sync"
+
+	"tailscale.com/feature"
+	"tailscale.com/ipn"
+	"tailscale.com/ipn/ipnext"
+	"tailscale.com/ipn/ipnlocal"
+	"tailscale.com/ipn/store"
+	"tailscale.com/ipn/store/mem"
+	"tailscale.com/types/logger"
+)
+
+const featureName = "serviceclientprefs"
+
+// storeKey is the key under which a profile's service client prefs are stored.
+const storeKey = "service-client-prefs"
+
+func init() {
+	feature.Register(featureName)
+	ipnext.RegisterExtension(featureName, newExtension)
+}
+
+// newExtension is the [ipnext.NewExtensionFn] for this feature.
+func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) {
+	return &extension{
+		logf: logger.WithPrefix(logf, featureName+": "),
+		sb:   sb,
+	}, nil
+}
+
+// extension stores the service client prefs for the current login profile.
+type extension struct {
+	logf logger.Logf
+	sb   ipnext.SafeBackend
+	host ipnext.Host // from Init
+
+	// mu guards store and curPID, and the read/change/write in setServiceClientPref.
+	mu     sync.Mutex
+	store  ipn.StateStore // for the current profile
+	curPID ipn.ProfileID  // the profile store is built for
+}
+
+// Name implements [ipnext.Extension].
+func (e *extension) Name() string { return featureName }
+
+// Init implements [ipnext.Extension]. It subscribes to profile changes and sets up storage for the
+// current profile.
+func (e *extension) Init(h ipnext.Host) error {
+	e.host = h
+	h.Hooks().ProfileStateChange.Add(e.onChangeProfile)
+	profile, prefs := h.Profiles().CurrentProfileState()
+	e.onChangeProfile(profile, prefs, false)
+	return nil
+}
+
+// Shutdown implements [ipnext.Extension].
+func (e *extension) Shutdown() error { return nil }
+
+// onChangeProfile builds the store for the profile we switched to. The prefs live under
+// profile-data/<id>/, which [ipnlocal.LocalBackend] removes when the profile is deleted.
+func (e *extension) onChangeProfile(profile ipn.LoginProfileView, _ ipn.PrefsView, sameNode bool) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
+	pid := profile.ID()
+	if sameNode && e.curPID == pid && e.store != nil {
+		// Same profile, just a prefs or metadata update; keep the store we already built.
+		return
+	}
+	e.curPID = pid
+	e.store = nil
+	if pid == "" {
+		return
+	}
+	varRoot := e.sb.TailscaleVarRoot()
+	if varRoot == "" {
+		// No writable storage (ephemeral node, or a non-file [ipn.StateStore] like Kubernetes).
+		// Service client prefs are only used by the desktop clients, so an in-memory store that
+		// doesn't survive restarts is fine here.
+		e.store = new(mem.Store)
+		return
+	}
+	// Store the file under a dir plus a hex-encoded filename with no extension, matching
+	// netmapcache.Store's layout so a shared store could read these files if we consolidate later.
+	hexStoreKey := hex.EncodeToString([]byte(storeKey))
+	path := filepath.Join(varRoot, ipnlocal.ProfileDataDir, string(pid), storeKey, hexStoreKey)
+	st, err := store.NewFileStore(e.logf, path)
+	if err != nil {
+		e.logf("building store for profile %q: %v", pid, err)
+		return
+	}
+	e.store = st
+}
@@ -0,0 +1,64 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package serviceclientprefs
+
+import (
+	"testing"
+	"time"
+
+	"tailscale.com/client/tailscale/apitype"
+	"tailscale.com/ipn"
+	"tailscale.com/tsd"
+	"tailscale.com/tstest"
+	"tailscale.com/tstime"
+	"tailscale.com/types/logger"
+)
+
+// fakeBackend is a minimal [ipnext.SafeBackend] for tests. Only Clock and TailscaleVarRoot are
+// used by the extension; Sys is never called.
+type fakeBackend struct {
+	clock   tstime.Clock
+	varRoot string
+}
+
+func (f *fakeBackend) Sys() *tsd.System         { return nil }
+func (f *fakeBackend) Clock() tstime.Clock      { return f.clock }
+func (f *fakeBackend) TailscaleVarRoot() string { return f.varRoot }
+
+// newTestExtension returns an extension backed by a temp var root and a fixed test clock, with its
+// current profile set to pid.
+func newTestExtension(t *testing.T, pid ipn.ProfileID) *extension {
+	t.Helper()
+	e := &extension{
+		logf: logger.Discard,
+		sb: &fakeBackend{
+			clock:   tstest.NewClock(tstest.ClockOpts{Start: time.Now()}),
+			varRoot: t.TempDir(),
+		},
+	}
+	e.onChangeProfile((&ipn.LoginProfile{ID: pid}).View(), ipn.PrefsView{}, false)
+	return e
+}
+
+func TestNoVarRootUsesMemory(t *testing.T) {
+	e := &extension{
+		logf: logger.Discard,
+		sb: &fakeBackend{
+			clock:   tstest.NewClock(tstest.ClockOpts{Start: time.Now()}),
+			varRoot: "", // force the in-memory fallback
+		},
+	}
+	e.onChangeProfile((&ipn.LoginProfile{ID: "pid"}).View(), ipn.PrefsView{}, false)
+
+	if _, err := e.setServiceClientPref(apitype.ServiceClientPrefRequest{Key: "ssh:22", Client: "terminal"}); err != nil {
+		t.Fatal(err)
+	}
+	got, err := e.serviceClientPrefs()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got["ssh:22"].Client != "terminal" {
+		t.Errorf("in-memory store: got %v, want Client=terminal", got["ssh:22"])
+	}
+}