Every outbound fetch may only reach a public address

A security review flagged the transcript download as an SSRF: the URL comes from
a <podcast:transcript> tag, so a publisher chooses it. It was the third place
with that shape, not the first -- the feed fetch takes a URL the phone sent, and
the enclosure download takes one from inside the feed, and both had been doing
it for longer. Fixing the flagged instance alone would have fixed nothing.

The exposure is specific rather than theoretical. listend listens on
127.0.0.1:38090 behind Apache on a host running other services, so a feed naming
a loopback, private or link-local address -- 169.254.169.254 among them -- would
have the server fetch something only it can reach. In the transcript case the
result would then be stored and handed to the phone as an episode's words, which
turns an internal reachability problem into a readable one.

The check runs when the connection is dialled, not when the URL is read, so it
also covers a redirect into an internal address and a hostname that resolves to
one. The address connected to is the address that was checked.

internal/safefetch now has a structural test that walks this server's own source
and refuses any production file that builds its own HTTP client or uses the
shared default. It was watched failing against a deliberately added violation
before being kept. Tests still inject a plain client through the seam each type
already had, because a httptest server is on loopback by definition -- which is
exactly what production is now refusing.

Checked against the live subscription list before committing: all 44 feeds use
https and none resolves to a private, loopback or link-local address, so nothing
Peter subscribes to is affected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

7 files changed, 320 insertions, 14 deletions

diff --git a/server/internal/mirror/mirror.go b/server/internal/mirror/mirror.go
index 9e91041..47d29f2 100644
--- a/server/internal/mirror/mirror.go
+++ b/server/internal/mirror/mirror.go
@@ -14,6 +14,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/terst/listen/server/internal/safefetch"
 	"github.com/terst/listen/server/internal/store"
 )
 
@@ -44,7 +45,9 @@ func (m *Mirror) client() *http.Client {
 	if m.Client != nil {
 		return m.Client
 	}
-	return &http.Client{Timeout: DefaultTimeout}
+	// An enclosure URL comes from the publisher's feed. Same rule as every other outbound
+	// fetch here: it may only reach a public address.
+	return safefetch.Client(DefaultTimeout)
 }
 
 func (m *Mirror) userAgent() string {
@@ -67,6 +70,9 @@ func (m *Mirror) Fetch(ctx context.Context, e *store.Episode) error {
 		return fmt.Errorf("episode %s: %w", e.ID, err)
 	}
 
+	if err := safefetch.CheckURL(e.EnclosureURL); err != nil {
+		return err
+	}
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, e.EnclosureURL, nil)
 	if err != nil {
 		return fmt.Errorf("episode %s: %w", e.ID, err)
diff --git a/server/internal/mirror/mirror_test.go b/server/internal/mirror/mirror_test.go
index b510451..fd1e716 100644
--- a/server/internal/mirror/mirror_test.go
+++ b/server/internal/mirror/mirror_test.go
@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/terst/listen/server/internal/store"
 )
@@ -28,7 +29,7 @@ func TestFetchWritesTheAudioAndRecordsWhereItWent(t *testing.T) {
 	defer srv.Close()
 
 	dir := t.TempDir()
-	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1325113}}
+	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1325113}, Client: loopbackClient()}
 	e := episode("abc", srv.URL+"/1.mp3")
 
 	if err := m.Fetch(context.Background(), e); err != nil {
@@ -57,7 +58,7 @@ func TestFetchNamesTheFileByEpisodeId(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}}
+	m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}, Client: loopbackClient()}
 	e := episode("deadbeef", srv.URL+"/whatever.m4a")
 	if err := m.Fetch(context.Background(), e); err != nil {
 		t.Fatal(err)
@@ -79,7 +80,7 @@ func TestAFailedFetchLeavesNothingBehind(t *testing.T) {
 	defer srv.Close()
 
 	dir := t.TempDir()
-	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}}
+	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}, Client: loopbackClient()}
 	e := episode("abc", srv.URL+"/1.mp3")
 
 	if err := m.Fetch(context.Background(), e); err == nil {
@@ -98,7 +99,7 @@ func TestAFailedFetchLeavesNothingBehind(t *testing.T) {
 }
 
 func TestFetchRefusesAnEpisodeWithNoEnclosure(t *testing.T) {
-	m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}}
+	m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}, Client: loopbackClient()}
 	if err := m.Fetch(context.Background(), episode("abc", "")); err == nil {
 		t.Error("expected an error for an episode with nothing to download")
 	}
@@ -106,7 +107,7 @@ func TestFetchRefusesAnEpisodeWithNoEnclosure(t *testing.T) {
 
 func TestPruneRemovesTheFileAndForgetsThePath(t *testing.T) {
 	dir := t.TempDir()
-	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}}
+	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}, Client: loopbackClient()}
 	e := episode("abc", "https://cdn/1.mp3")
 	e.Path = "abc.mp3"
 	if err := os.WriteFile(filepath.Join(dir, e.Path), []byte("bytes"), 0o644); err != nil {
@@ -128,7 +129,7 @@ func TestPruneRemovesTheFileAndForgetsThePath(t *testing.T) {
 // mean downloading it again.
 func TestPruneKeepsWhatDetectionDecided(t *testing.T) {
 	dir := t.TempDir()
-	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}}
+	m := &Mirror{AudioDir: dir, Probe: stubProbe{ms: 1}, Client: loopbackClient()}
 	e := episode("abc", "https://cdn/1.mp3")
 	e.Path = "abc.mp3"
 	intro := int64(16652)
@@ -150,7 +151,7 @@ func TestPruneKeepsWhatDetectionDecided(t *testing.T) {
 }
 
 func TestPruningSomethingAlreadyGoneIsFine(t *testing.T) {
-	m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}}
+	m := &Mirror{AudioDir: t.TempDir(), Probe: stubProbe{ms: 1}, Client: loopbackClient()}
 	e := episode("abc", "https://cdn/1.mp3")
 	e.Path = "not-there.mp3"
 	if err := m.Prune(e); err != nil {
@@ -160,3 +161,11 @@ func TestPruningSomethingAlreadyGoneIsFine(t *testing.T) {
 		t.Error("Path not cleared")
 	}
 }
+
+// loopbackClient is a plain http client for the test servers here.
+//
+// Production uses safefetch, which refuses a loopback address on purpose: an enclosure URL
+// comes from a publisher's feed and must not be able to name something only this host can
+// reach. A httptest server is on loopback by definition, so the tests supply the seam that
+// exists for exactly this.
+func loopbackClient() *http.Client { return &http.Client{Timeout: 30 * time.Second} }
diff --git a/server/internal/safefetch/audit_test.go b/server/internal/safefetch/audit_test.go
new file mode 100644
index 0000000..fa77cea
--- /dev/null
+++ b/server/internal/safefetch/audit_test.go
@@ -0,0 +1,79 @@
+package safefetch
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+/*
+No outbound request in this server may bypass safefetch.
+
+The SSRF this package exists for was found in the transcript harvester, but the harvester was
+the third place that fetched a URL somebody else chose: the feed fetch and the enclosure
+download had the same shape and had had it for longer. Fixing the instance that was flagged and
+leaving the other two would have fixed nothing.
+
+This walks the server's own source and refuses a production file that builds its own HTTP client
+or uses the shared default one. The allowed way to get a client is safefetch.Client; a test may
+inject a plain one through the seam the type already has, because a httptest server is on
+loopback by definition.
+*/
+
+// banned are the ways to reach the network without going through this package.
+var banned = []string{
+	"http.DefaultClient",
+	"http.DefaultTransport.RoundTrip",
+	"&http.Client{",
+	"http.Get(",
+	"http.Post(",
+	"http.Head(",
+}
+
+// allowed names the files that may mention them, and why.
+var allowed = map[string]string{
+	// This package is where the safe client is built.
+	filepath.Join("internal", "safefetch", "safefetch.go"): "builds the client",
+}
+
+func TestNothingFetchesWithoutSafefetch(t *testing.T) {
+	root, err := filepath.Abs("../..")
+	if err != nil {
+		t.Fatalf("root: %v", err)
+	}
+
+	err = filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil || info.IsDir() || !strings.HasSuffix(path, ".go") {
+			return err
+		}
+		// A test may inject a plain client through the seam: the servers it talks to are on
+		// loopback, which is the thing production is refusing.
+		if strings.HasSuffix(path, "_test.go") {
+			return nil
+		}
+		relative, err := filepath.Rel(root, path)
+		if err != nil {
+			return err
+		}
+		if _, ok := allowed[relative]; ok {
+			return nil
+		}
+		raw, err := os.ReadFile(path)
+		if err != nil {
+			return err
+		}
+		body := string(raw)
+		for _, pattern := range banned {
+			if strings.Contains(body, pattern) {
+				t.Errorf("%s uses %s -- every outbound request goes through safefetch.Client, "+
+					"because the URLs this server fetches are chosen by podcast publishers",
+					relative, pattern)
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("walk: %v", err)
+	}
+}
diff --git a/server/internal/safefetch/safefetch.go b/server/internal/safefetch/safefetch.go
new file mode 100644
index 0000000..3233f8f
--- /dev/null
+++ b/server/internal/safefetch/safefetch.go
@@ -0,0 +1,110 @@
+// Package safefetch makes outbound requests to addresses this server did not choose.
+//
+// Everything listend fetches is named by somebody else. The feed URL comes from the phone, and
+// the enclosure and <podcast:transcript> URLs inside the feed come from the publisher. listend
+// listens on 127.0.0.1:38090 behind Apache on a host running other things, so a feed naming a
+// loopback, private or link-local address would have the server fetch something reachable only
+// from inside -- and in the transcript case store the result and hand it to the phone as an
+// episode's words.
+//
+// The check happens when the connection is dialled rather than when the URL is read. That is
+// the only point that also covers a redirect into an internal address and a hostname that
+// resolves to one: the address connected to is the address that was checked.
+package safefetch
+
+import (
+	"context"
+	"fmt"
+	"net"
+	"net/http"
+	"net/url"
+	"time"
+)
+
+// DefaultTimeout is what Client uses when given none.
+const DefaultTimeout = 60 * time.Second
+
+// CheckURL rejects anything that is not an ordinary web address.
+//
+// A publisher naming file:// or gopher:// has not given us a transcript, and Go's http client
+// would not fetch those anyway -- the value of saying so here is that the refusal is explicit
+// and testable rather than an obscure transport error.
+func CheckURL(raw string) error {
+	parsed, err := url.Parse(raw)
+	if err != nil {
+		return fmt.Errorf("safefetch: %q is not a URL: %w", raw, err)
+	}
+	if parsed.Scheme != "http" && parsed.Scheme != "https" {
+		return fmt.Errorf("safefetch: %q is not http or https", raw)
+	}
+	if parsed.Host == "" {
+		return fmt.Errorf("safefetch: %q names no host", raw)
+	}
+	return nil
+}
+
+// Client returns an http.Client that can only reach public addresses.
+func Client(timeout time.Duration) *http.Client {
+	if timeout <= 0 {
+		timeout = DefaultTimeout
+	}
+	dialer := &net.Dialer{Timeout: 20 * time.Second, KeepAlive: 30 * time.Second}
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		host, port, err := net.SplitHostPort(addr)
+		if err != nil {
+			return nil, fmt.Errorf("safefetch: %q: %w", addr, err)
+		}
+		addresses, err := net.DefaultResolver.LookupIPAddr(ctx, host)
+		if err != nil {
+			return nil, fmt.Errorf("safefetch: %q: %w", host, err)
+		}
+		// Dial a checked address rather than the hostname, so the connection cannot land
+		// somewhere a second lookup would have resolved to.
+		var lastErr error
+		for _, candidate := range addresses {
+			if !permitted(candidate.IP) {
+				lastErr = fmt.Errorf("safefetch: %s is not a public address", candidate.IP)
+				continue
+			}
+			conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(candidate.IP.String(), port))
+			if err == nil {
+				return conn, nil
+			}
+			lastErr = err
+		}
+		if lastErr == nil {
+			lastErr = fmt.Errorf("safefetch: %q resolved to nothing", host)
+		}
+		return nil, lastErr
+	}
+	return &http.Client{Timeout: timeout, Transport: transport}
+}
+
+// permitted reports whether an address is one the open internet could have given us.
+//
+// Everything a podcast is actually served from is a public address. The excluded ranges are the
+// ones that mean "somewhere only this host or this network can reach": loopback, the RFC 1918
+// private ranges, IPv6 unique-local, link-local -- which includes 169.254.169.254, the cloud
+// metadata address -- and the unspecified and multicast addresses.
+func permitted(ip net.IP) bool {
+	if ip == nil {
+		return false
+	}
+	// An IPv4 address written in IPv6-mapped form is the same address and reaches the same
+	// host, so it is judged as the IPv4 one.
+	if mapped := ip.To4(); mapped != nil {
+		ip = mapped
+	}
+	switch {
+	case ip.IsLoopback(),
+		ip.IsPrivate(),
+		ip.IsLinkLocalUnicast(),
+		ip.IsLinkLocalMulticast(),
+		ip.IsInterfaceLocalMulticast(),
+		ip.IsMulticast(),
+		ip.IsUnspecified():
+		return false
+	}
+	return true
+}
diff --git a/server/internal/safefetch/safefetch_test.go b/server/internal/safefetch/safefetch_test.go
new file mode 100644
index 0000000..dfa3470
--- /dev/null
+++ b/server/internal/safefetch/safefetch_test.go
@@ -0,0 +1,86 @@
+package safefetch
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+/*
+Outbound requests to addresses a podcast feed chose.
+
+Everything this server fetches is named by somebody else: the feed URL comes from the phone, and
+the enclosure and <podcast:transcript> URLs inside it come from the publisher. listend runs on
+127.0.0.1:38090 behind Apache, on a host with other services, so a feed naming a loopback or
+private address would have the server fetch something only it can reach -- and in the transcript
+case hand the result to the phone as an episode's words.
+
+Checked at dial time rather than on the URL string, because that is the only place that catches
+a redirect to an internal address and a hostname that resolves to one. The address actually
+connected to is the address that was checked.
+*/
+
+func TestLoopbackAndPrivateAddressesAreRefused(t *testing.T) {
+	blocked := []string{
+		"127.0.0.1", "127.1.2.3", "::1",
+		"10.0.0.5", "172.16.3.1", "192.168.1.1",
+		"169.254.169.254", // the cloud metadata address
+		"fe80::1", "fc00::1",
+		"0.0.0.0", "::",
+	}
+	for _, ip := range blocked {
+		if permitted(net.ParseIP(ip)) {
+			t.Errorf("%s was permitted", ip)
+		}
+	}
+}
+
+func TestOrdinaryPublicAddressesArePermitted(t *testing.T) {
+	for _, ip := range []string{"93.184.216.34", "1.1.1.1", "2606:4700:4700::1111"} {
+		if !permitted(net.ParseIP(ip)) {
+			t.Errorf("%s was refused", ip)
+		}
+	}
+}
+
+// An IPv4 address written as an IPv6-mapped one is the same address, and reaches the same host.
+func TestAMappedLoopbackAddressIsStillLoopback(t *testing.T) {
+	if permitted(net.ParseIP("::ffff:127.0.0.1")) {
+		t.Error("::ffff:127.0.0.1 was permitted")
+	}
+}
+
+// A real connection to a server on loopback, which is exactly the shape the finding describes.
+func TestAClientCannotReachALoopbackServer(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte("internal"))
+	}))
+	defer server.Close()
+
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL, nil)
+	if err != nil {
+		t.Fatalf("request: %v", err)
+	}
+	if _, err := Client(0).Do(req); err == nil {
+		t.Fatal("reached a loopback server")
+	} else if !strings.Contains(err.Error(), "not a public address") {
+		t.Errorf("refused for the wrong reason: %v", err)
+	}
+}
+
+// A publisher could name a file:// or gopher:// URL; neither is something to fetch.
+func TestOnlyHTTPAndHTTPSAreFetched(t *testing.T) {
+	for _, raw := range []string{"file:///etc/passwd", "gopher://x/", "ftp://x/f", "x", ""} {
+		if err := CheckURL(raw); err == nil {
+			t.Errorf("%q was accepted", raw)
+		}
+	}
+	for _, raw := range []string{"https://example.com/f.vtt", "http://example.com/f.vtt"} {
+		if err := CheckURL(raw); err != nil {
+			t.Errorf("%q was refused: %v", raw, err)
+		}
+	}
+}
diff --git a/server/internal/transcript/harvest.go b/server/internal/transcript/harvest.go
index ed149e3..6369ea3 100644
--- a/server/internal/transcript/harvest.go
+++ b/server/internal/transcript/harvest.go
@@ -6,6 +6,8 @@ import (
 	"io"
 	"net/http"
 	"time"
+
+	"github.com/terst/listen/server/internal/safefetch"
 )
 
 // maxTranscriptBytes caps a download. The largest in this corpus is Darknet Diaries' 1784-cue
@@ -44,7 +46,9 @@ func (h Harvester) client() *http.Client {
 	if h.Client != nil {
 		return h.Client
 	}
-	return &http.Client{Timeout: 60 * time.Second}
+	// A transcript URL comes from a publisher's feed, so it names an address this server did
+	// not choose. safefetch refuses one that resolves to somewhere only this host can reach.
+	return safefetch.Client(60 * time.Second)
 }
 
 // Harvest fetches an offered transcript and stores it if it describes the mirrored file.
@@ -80,6 +84,9 @@ func (h Harvester) Harvest(ctx context.Context, offer Offer) (Result, error) {
 }
 
 func (h Harvester) download(ctx context.Context, url string) (string, error) {
+	if err := safefetch.CheckURL(url); err != nil {
+		return "", err
+	}
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
 		return "", fmt.Errorf("transcript: %w", err)
diff --git a/server/internal/transcript/harvest_test.go b/server/internal/transcript/harvest_test.go
index ec55681..23a192a 100644
--- a/server/internal/transcript/harvest_test.go
+++ b/server/internal/transcript/harvest_test.go
@@ -6,6 +6,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 )
 
 /*
@@ -46,7 +47,7 @@ func serving(t *testing.T, status int, body, contentType string) *httptest.Serve
 func TestHarvestStoresAPublishedTranscriptThatFitsTheFile(t *testing.T) {
 	server := serving(t, 200, harvestSRT, "application/srt")
 	store := Store{Dir: t.TempDir()}
-	harvester := Harvester{Store: store}
+	harvester := Harvester{Store: store, Client: loopbackClient()}
 
 	got, err := harvester.Harvest(context.Background(), Offer{
 		EpisodeID:  "ep1",
@@ -73,7 +74,7 @@ func TestHarvestStoresAPublishedTranscriptThatFitsTheFile(t *testing.T) {
 func TestHarvestDeclinesATranscriptForAnAdInsertedCopy(t *testing.T) {
 	server := serving(t, 200, harvestSRT, "application/srt")
 	store := Store{Dir: t.TempDir()}
-	harvester := Harvester{Store: store}
+	harvester := Harvester{Store: store, Client: loopbackClient()}
 
 	// Darknet Diaries' shape: the file is minutes longer than the feed declares.
 	got, err := harvester.Harvest(context.Background(), Offer{
@@ -101,7 +102,7 @@ func TestHarvestDeclinesATranscriptForAnAdInsertedCopy(t *testing.T) {
 // is worth another attempt; a refusal is not.
 func TestAFailedDownloadIsAnErrorAndNotAVerdict(t *testing.T) {
 	server := serving(t, 404, "nope", "text/plain")
-	harvester := Harvester{Store: Store{Dir: t.TempDir()}}
+	harvester := Harvester{Store: Store{Dir: t.TempDir()}, Client: loopbackClient()}
 
 	got, err := harvester.Harvest(context.Background(), Offer{
 		EpisodeID: "ep1", URL: server.URL, Type: "application/srt",
@@ -119,7 +120,7 @@ func TestAFailedDownloadIsAnErrorAndNotAVerdict(t *testing.T) {
 // that is a verdict: the feed says this is the transcript, and it is not one.
 func TestSomethingThatIsNotCuesIsDeclinedNotRetried(t *testing.T) {
 	server := serving(t, 200, "<html><body>Transcript coming soon</body></html>", "text/html")
-	harvester := Harvester{Store: Store{Dir: t.TempDir()}}
+	harvester := Harvester{Store: Store{Dir: t.TempDir()}, Client: loopbackClient()}
 
 	got, err := harvester.Harvest(context.Background(), Offer{
 		EpisodeID: "ep1", URL: server.URL, Type: "application/srt",
@@ -134,7 +135,7 @@ func TestSomethingThatIsNotCuesIsDeclinedNotRetried(t *testing.T) {
 }
 
 func TestAnEpisodeOfferingNoTranscriptIsNotFetched(t *testing.T) {
-	harvester := Harvester{Store: Store{Dir: t.TempDir()}}
+	harvester := Harvester{Store: Store{Dir: t.TempDir()}, Client: loopbackClient()}
 	got, err := harvester.Harvest(context.Background(), Offer{EpisodeID: "ep1", ServedMs: 1_000})
 	if err != nil {
 		t.Fatalf("Harvest: %v", err)
@@ -143,3 +144,11 @@ func TestAnEpisodeOfferingNoTranscriptIsNotFetched(t *testing.T) {
 		t.Errorf("got %+v, want nothing done and nothing said", got)
 	}
 }
+
+// loopbackClient is a plain http client for the test servers here.
+//
+// Production uses safefetch, which refuses a loopback address on purpose: a transcript URL
+// comes from a publisher's feed, and one naming 127.0.0.1 would have the server fetch
+// something only it can reach and hand the result to the phone as an episode's words. A
+// httptest server is on loopback by definition, so the tests supply the Client seam.
+func loopbackClient() *http.Client { return &http.Client{Timeout: 30 * time.Second} }