portforward: ManualForwarder.ExternalIP returns "" again

The original manualPortForwarder (introduced in b81b6b0) returned
"", nil from ExternalIP, leaning on lantern-cloud peer_handler's
"external_ip empty → use observed" fall-through to fill the IP from
the /v1/peer/register call's RemoteAddr. The refactor into
portforward.ManualForwarder regressed this path by actively probing
publicip.Detect, which fails on the only deployment where it
matters: machines with Lantern's own VPN tunnel up.

When the tunnel is up, outbound traffic routes through it. The
publicip discovery endpoints (api.iantem.io etc) either time out or
return the tunnel exit's IP rather than the user's WAN IP — neither
is what we want. Worse, the failure surfaces as a hard error so
peer.Client.Start aborts before /v1/peer/register is even called,
breaking peer-share for every user with a manual port + an active
tunnel.

Empty is also more correct: peer_handler uses the IP that will
actually receive inbound traffic on the manually-forwarded port (the
register-call's RemoteAddr), which is by definition the right answer.

Add a regression test pinning the empty-string contract.

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

Author: 2 people

portforward/manual.go

@@ -4,9 +4,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"time"
-
-	"github.com/getlantern/publicip"
 )
 
 // ManualForwarder is a no-op port forwarder for users who can't (or won't)
@@ -78,25 +75,16 @@ func (f *ManualForwarder) UnmapPort(_ context.Context) error {
 // the user removes them, with no UPnP lease to renew.
 func (f *ManualForwarder) StartRenewal(_ context.Context) {}
 
-// ExternalIP probes a public-IP discovery service (the radiance default
-// methods, hitting api.iantem.io etc.) since there's no UPnP gateway to
-// ask. Cached for the duration of the forwarder's life — the user's
-// public IP shouldn't change mid-session, and a stale cache yields a
-// clean re-register on a heartbeat 404 if it does.
-func (f *ManualForwarder) ExternalIP(ctx context.Context) (string, error) {
-	probeCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
-	defer cancel()
-	result, err := publicip.Detect(probeCtx, &publicip.Config{
-		Timeout:      5 * time.Second,
-		MinConsensus: 1,
-	})
-	if err != nil {
-		return "", fmt.Errorf("detect public ip: %w", err)
-	}
-	if result.IP == nil {
-		return "", fmt.Errorf("public-ip detection returned no result")
-	}
-	return result.IP.String(), nil
+// ExternalIP returns "" so the server fills in the observed IP from the
+// register call's RemoteAddr (peer_handler's "external_ip empty → use
+// observed" path). Probing publicip.Detect from here regresses on
+// machines where Lantern's own tunnel is up — outbound goes through the
+// tunnel and the discovery endpoints either time out or report the
+// tunnel exit's IP rather than the user's WAN IP. Letting the server
+// use the observed RemoteAddr is also more correct: it's the IP that
+// will actually receive inbound traffic on the manually-forwarded port.
+func (f *ManualForwarder) ExternalIP(_ context.Context) (string, error) {
+	return "", nil
 }
 
 // ParseManualPort is a small helper for callers that want to read a

portforward/manual_test.go

@@ -72,6 +72,23 @@ func TestManualForwarder_StartRenewal_NoOp(t *testing.T) {
 	f.StartRenewal(context.Background()) // must not panic
 }
 
+// TestManualForwarder_ExternalIP_ReturnsEmpty pins the contract that
+// ExternalIP returns "" with no error so the lantern-cloud peer_handler
+// fills the IP from the register call's RemoteAddr. A previous revision
+// regressed this to call publicip.Detect, which fails on machines where
+// Lantern's own VPN tunnel is up — outbound traffic gets routed through
+// the tunnel and the discovery endpoints return the tunnel exit's IP
+// (or time out entirely), breaking peer registration silently.
+func TestManualForwarder_ExternalIP_ReturnsEmpty(t *testing.T) {
+	t.Parallel()
+	f, err := NewManualForwarder(5698)
+	require.NoError(t, err)
+	ip, err := f.ExternalIP(context.Background())
+	require.NoError(t, err)
+	assert.Empty(t, ip,
+		"ManualForwarder.ExternalIP must return \"\" so server uses observed RemoteAddr")
+}
+
 func TestParseManualPort(t *testing.T) {
 	t.Parallel()