wgengine/magicsock: fix js/wasm crash regression loading non-existent portmapper

Thanks for the report, @Need-an-AwP!

Fixes #17681
Updates #9394

Change-Id: I2e0b722ef9b460bd7e79499192d1a315504ca84c
Signed-off-by: Brad Fitzpatrick <[email protected]>

client/local/local.go

@@ -596,6 +596,19 @@ func (lc *Client) DebugResultJSON(ctx context.Context, action string) (any, erro
 	return x, nil
 }
 
+// QueryOptionalFeatures queries the optional features supported by the Tailscale daemon.
+func (lc *Client) QueryOptionalFeatures(ctx context.Context) (*apitype.OptionalFeatures, error) {
+	body, err := lc.send(ctx, "POST", "/localapi/v0/debug-optional-features", 200, nil)
+	if err != nil {
+		return nil, fmt.Errorf("error %w: %s", err, body)
+	}
+	var x apitype.OptionalFeatures
+	if err := json.Unmarshal(body, &x); err != nil {
+		return nil, err
+	}
+	return &x, nil
+}
+
 // SetDevStoreKeyValue set a statestore key/value. It's only meant for development.
 // The schema (including when keys are re-read) is not a stable interface.
 func (lc *Client) SetDevStoreKeyValue(ctx context.Context, key, value string) error {

client/tailscale/apitype/apitype.go

@@ -94,3 +94,13 @@ type DNSQueryResponse struct {
 	// Resolvers is the list of resolvers that the forwarder deemed able to resolve the query.
 	Resolvers []*dnstype.Resolver
 }
+
+// OptionalFeatures describes which optional features are enabled in the build.
+type OptionalFeatures struct {
+	// Features is the map of optional feature names to whether they are
+	// enabled.
+	//
+	// Disabled features may be absent from the map. (That is, false values
+	// are not guaranteed to be present.)
+	Features map[string]bool
+}

feature/feature.go

@@ -13,6 +13,12 @@ var ErrUnavailable = errors.New("feature not included in this build")
 
 var in = map[string]bool{}
 
+// Registered reports the set of registered features.
+//
+// The returned map should not be modified by the caller,
+// not accessed concurrently with calls to Register.
+func Registered() map[string]bool { return in }
+
 // Register notes that the named feature is linked into the binary.
 func Register(name string) {
 	if _, ok := in[name]; ok {

feature/portmapper/portmapper.go

@@ -6,6 +6,7 @@
 package portmapper
 
 import (
+	"tailscale.com/feature"
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/portmapper"
 	"tailscale.com/net/portmapper/portmappertype"
@@ -14,6 +15,7 @@ import (
 )
 
 func init() {
+	feature.Register("portmapper")
 	portmappertype.HookNewPortMapper.Set(newPortMapper)
 }
 

ipn/localapi/debug.go

@@ -19,6 +19,7 @@ import (
 	"sync"
 	"time"
 
+	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/feature"
 	"tailscale.com/feature/buildfeatures"
 	"tailscale.com/ipn"
@@ -39,6 +40,7 @@ func init() {
 	Register("debug-packet-filter-matches", (*Handler).serveDebugPacketFilterMatches)
 	Register("debug-packet-filter-rules", (*Handler).serveDebugPacketFilterRules)
 	Register("debug-peer-endpoint-changes", (*Handler).serveDebugPeerEndpointChanges)
+	Register("debug-optional-features", (*Handler).serveDebugOptionalFeatures)
 }
 
 func (h *Handler) serveDebugPeerEndpointChanges(w http.ResponseWriter, r *http.Request) {
@@ -463,3 +465,11 @@ func (h *Handler) serveDebugLog(w http.ResponseWriter, r *http.Request) {
 
 	w.WriteHeader(http.StatusNoContent)
 }
+
+func (h *Handler) serveDebugOptionalFeatures(w http.ResponseWriter, r *http.Request) {
+	of := &apitype.OptionalFeatures{
+		Features: feature.Registered(),
+	}
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(of)
+}

tstest/integration/integration_test.go

@@ -175,6 +175,28 @@ func TestControlKnobs(t *testing.T) {
 	}
 }
 
+func TestExpectedFeaturesLinked(t *testing.T) {
+	tstest.Shard(t)
+	tstest.Parallel(t)
+	env := NewTestEnv(t)
+	n1 := NewTestNode(t, env)
+
+	d1 := n1.StartDaemon()
+	n1.AwaitResponding()
+	lc := n1.LocalClient()
+	got, err := lc.QueryOptionalFeatures(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !got.Features["portmapper"] {
+		t.Errorf("optional feature portmapper unexpectedly not found: got %v", got.Features)
+	}
+
+	d1.MustCleanShutdown(t)
+
+	t.Logf("number of HTTP logcatcher requests: %v", env.LogCatcher.numRequests())
+}
+
 func TestCollectPanic(t *testing.T) {
 	flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/15865")
 	tstest.Shard(t)

wgengine/magicsock/magicsock.go

@@ -719,9 +719,13 @@ func NewConn(opts Options) (*Conn, error) {
 		newPortMapper, ok := portmappertype.HookNewPortMapper.GetOk()
 		if ok {
 			c.portMapper = newPortMapper(portmapperLogf, opts.EventBus, opts.NetMon, disableUPnP, c.onlyTCP443.Load)
-		} else if !testenv.InTest() {
-			panic("unexpected: HookNewPortMapper not set")
 		}
+		// If !ok, the HookNewPortMapper hook is not set (so feature/portmapper
+		// isn't linked), but the build tag to explicitly omit the portmapper
+		// isn't set either. This should only happen to js/wasm builds, where
+		// the portmapper is a no-op even if linked (but it's no longer linked,
+		// since the move to feature/portmapper), or if people are wiring up
+		// their own Tailscale build from pieces.
 	}
 
 	c.netMon = opts.NetMon