peer: fix nil-pointer panic when UPnP discovery fails

Two changes to make the failure-on-discover path actually surface its
underlying error instead of crashing the IPC handler.

  1. peer/peer.go's default cfg.NewForwarder wrapped
     portforward.NewForwarder with a bare `return portforward.NewForwarder(ctx)`.
     When discovery failed, that collapsed the `(*Forwarder)(nil), err`
     pair into a typed-nil interface — `if fwd != nil` in the deferred
     cleanup passed (the interface has a type), `fwd.UnmapPort(...)`
     dispatched to a nil receiver, and `f.mu.Lock()` panicked. The
     wrapper now returns a clean `nil, err` so the caller sees
     ErrNoPortForwarding (or whatever the discoverer returned) and the
     deferred cleanup short-circuits on the interface nil-check.

  2. portforward.UnmapPort grew a defensive `if f == nil { return nil }`
     at the top. Belt-and-suspenders for any future caller that lands
     here through an interface and bypasses the inline nil-check —
     teardown should be idempotent on a nil receiver, not a panic.

Reproduced live on macOS 26.x with `Share My Connection` toggled on
when UPnP discovery returned ErrNoPortForwarding; the http2 IPC
goroutine panicked instead of rolling back the toggle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 2 people

peer/peer.go

@@ -101,7 +101,18 @@ func NewClient(cfg Config) (*Client, error) {
 	}
 	if cfg.NewForwarder == nil {
 		cfg.NewForwarder = func(ctx context.Context) (portForwarder, error) {
-			return portforward.NewForwarder(ctx)
+			// Explicitly return a nil interface on error — `return
+			// portforward.NewForwarder(ctx)` collapses the (*Forwarder, error)
+			// pair into a typed-nil interface on failure, which then panics
+			// inside the deferred cleanup's `if fwd != nil { fwd.UnmapPort... }`
+			// because the nil-check passes (interface has a type) but the
+			// receiver is nil. Surfacing the underlying error here lets the
+			// caller see ErrNoPortForwarding instead of a runtime panic.
+			fwd, err := portforward.NewForwarder(ctx)
+			if err != nil {
+				return nil, err
+			}
+			return fwd, nil
 		}
 	}
 	if cfg.BuildBoxService == nil {

portforward/portforward.go

@@ -113,6 +113,14 @@ func (f *Forwarder) MapPort(ctx context.Context, internalPort uint16, descriptio
 // about a router rule that's actually still live and the user would have
 // to wait for the UPnP lease to expire.
 func (f *Forwarder) UnmapPort(ctx context.Context) error {
+	// Defensive: callers that pass a *Forwarder through an interface (see
+	// peer.Client's portForwarder shim) can land here with f == nil if a
+	// failed construction collapsed `(*Forwarder)(nil), err` into a
+	// non-nil-but-typed-nil interface. A bare `f.mu.Lock()` would panic;
+	// this guard makes the cleanup path idempotent against that race.
+	if f == nil {
+		return nil
+	}
 	f.mu.Lock()
 	defer f.mu.Unlock()
 	if f.cancel != nil {