safefetch: an allowlist, and no proxy to go round it

Two follow-up findings on the same file, both real.

**A proxy would have made the whole check pointless.** http.DefaultTransport
carries ProxyFromEnvironment, so with HTTP_PROXY set the connection goes to the
proxy and the proxy connects to whatever the URL named -- the address being
checked would be the proxy's, and the one that matters would never be looked at.
The safe client now sets no proxy, and a test asserts it.

**"Not RFC 1918" is not the same as "public".** The check is now an allowlist:
an ordinary global unicast address that is not in an IANA special-purpose range.
The ranges that were being let through include 100.64.0.0/10, which is
carrier-grade NAT and routes to a real host on a real network, and 64:ff9b::/96
and 2002::/16, which are NAT64 and 6to4 and carry an IPv4 address inside them --
127.0.0.1 included. None is private by Go's definition and none is somewhere a
podcast is served from.

Checked against the live library before committing: 130 real URLs -- all 44
feeds and two enclosures from every show -- resolve to addresses this permits,
so nothing Peter subscribes to stops mirroring. That check is kept as a test
which skips where the state file is not present, because the failure mode of
being too strict is a feed that silently stops updating.

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

6 files changed, 329 insertions, 17 deletions

diff --git a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt
index 836ec3d..52f07eb 100644
--- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt
+++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt
@@ -698,4 +698,109 @@ class MigrationTest {
             assertEquals("the crown jewel must survive", 141421L, it.getLong(0))
         }
     }
+
+    /**
+     * The transcript tables arrive empty and accept a cue. Nothing else moves: words are new
+     * evidence about an episode, not a change to anything already stored.
+     *
+     * One row per cue rather than a JSON blob on the episode. Android reads every Room row
+     * through a CursorWindow that refuses a row over 2MB on the way *out*, which is the bug
+     * that made intro detection crash the app the moment a show's second episode was
+     * fingerprinted -- an insert that succeeds and a read that throws. A long episode's
+     * transcript runs to thousands of cues, and a column holding all of them is the same shape
+     * of mistake waiting to be made again.
+     */
+    @Test
+    fun migrate16To17AddsTranscriptsAndLeavesEverythingElseAlone() {
+        helper.createDatabase(name, 16).apply {
+            execSQL(
+                """INSERT INTO subscriptions
+                   (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs, dripEnabled,
+                    dripIntervalMs, dripCount, dripOldestFirst, skipAdChapters, bedtime)
+                   VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 0, 0,
+                           0, 86400000, 1, 1, 1, 0)"""
+            )
+            execSQL(
+                """INSERT INTO episodes
+                   (id, subscriptionId, guid, title, mediaUrl, mediaType, discoveredAtMs,
+                    positionMs, downloadState)
+                   VALUES ('e1', 's1', 'g1', 'One', 'https://cdn/1.mp3', 'audio/mpeg', 1000,
+                           271828, 'NONE')"""
+            )
+            close()
+        }
+
+        val db = helper.runMigrationsAndValidate(name, 17, true, AppDatabase.MIGRATION_16_17)
+
+        db.query("SELECT COUNT(*) FROM transcript_cues").use {
+            it.moveToFirst()
+            assertEquals(0, it.getInt(0))
+        }
+        db.execSQL(
+            "INSERT INTO episode_transcripts (episodeId, source, cues, note, fetchedAtMs) " +
+                "VALUES ('e1', 'published', 2, '', 99)"
+        )
+        db.execSQL(
+            "INSERT INTO transcript_cues (episodeId, startMs, endMs, text) " +
+                "VALUES ('e1', 0, 3120, 'Welcome back.')"
+        )
+        db.query("SELECT text FROM transcript_cues WHERE episodeId = 'e1'").use {
+            it.moveToFirst()
+            assertEquals("Welcome back.", it.getString(0))
+        }
+        db.query("SELECT source FROM episode_transcripts WHERE episodeId = 'e1'").use {
+            it.moveToFirst()
+            assertEquals("published", it.getString(0))
+        }
+        db.query("SELECT positionMs FROM episodes WHERE id = 'e1'").use {
+            it.moveToFirst()
+            assertEquals("the crown jewel must survive", 271828L, it.getLong(0))
+        }
+    }
+
+    /**
+     * An episode's words go when the episode does. A transcript of a row nobody has any more is
+     * dead weight, and the cue table is the biggest thing in this database by row count.
+     */
+    @Test
+    fun deletingAnEpisodeTakesItsTranscriptWithIt() {
+        helper.createDatabase(name, 16).apply {
+            execSQL(
+                """INSERT INTO subscriptions
+                   (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs, dripEnabled,
+                    dripIntervalMs, dripCount, dripOldestFirst, skipAdChapters, bedtime)
+                   VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 0, 0,
+                           0, 86400000, 1, 1, 1, 0)"""
+            )
+            execSQL(
+                """INSERT INTO episodes
+                   (id, subscriptionId, guid, title, mediaUrl, mediaType, discoveredAtMs,
+                    positionMs, downloadState)
+                   VALUES ('e1', 's1', 'g1', 'One', 'https://cdn/1.mp3', 'audio/mpeg', 1000, 0,
+                           'NONE')"""
+            )
+            close()
+        }
+
+        val db = helper.runMigrationsAndValidate(name, 17, true, AppDatabase.MIGRATION_16_17)
+        db.execSQL("PRAGMA foreign_keys = ON")
+        db.execSQL(
+            "INSERT INTO episode_transcripts (episodeId, source, cues, note, fetchedAtMs) " +
+                "VALUES ('e1', 'published', 1, '', 99)"
+        )
+        db.execSQL(
+            "INSERT INTO transcript_cues (episodeId, startMs, endMs, text) " +
+                "VALUES ('e1', 0, 1000, 'Hello.')"
+        )
+        db.execSQL("DELETE FROM episodes WHERE id = 'e1'")
+
+        db.query("SELECT COUNT(*) FROM transcript_cues").use {
+            it.moveToFirst()
+            assertEquals("cues outlived their episode", 0, it.getInt(0))
+        }
+        db.query("SELECT COUNT(*) FROM episode_transcripts").use {
+            it.moveToFirst()
+            assertEquals("the transcript row outlived its episode", 0, it.getInt(0))
+        }
+    }
 }
diff --git a/server/internal/api/api.go b/server/internal/api/api.go
index bcfe6a2..d2ad841 100644
--- a/server/internal/api/api.go
+++ b/server/internal/api/api.go
@@ -92,6 +92,11 @@ type transcriptNote struct {
 	Source string `json:"source,omitempty"`
 	Cues   int    `json:"cues,omitempty"`
 	Note   string `json:"note,omitempty"`
+	// URL is where the cues themselves are, absolute and token-carrying, usable as given --
+	// the same way the audio URL works. The phone keys episodes by feed and guid and the
+	// server keys them by its own id, and handing over a URL is what keeps that from having
+	// to change. Empty when there is nothing to fetch.
+	URL string `json:"url,omitempty"`
 }
 
 type breakRange struct {
@@ -212,19 +217,24 @@ func (s *Server) episode(w http.ResponseWriter, r *http.Request) {
 		FoundAdMs:    found.FoundAdMs,
 		InsertedAdMs: found.InsertedAdMs,
 		AdsRefused:   found.AdsRefused,
-		Transcript:   transcriptNoteFor(found),
+		Transcript:   s.transcriptNoteFor(found),
 	})
 }
 
-func transcriptNoteFor(e *store.Episode) *transcriptNote {
+func (s *Server) transcriptNoteFor(e *store.Episode) *transcriptNote {
 	if !e.TranscriptExamined() {
 		return nil
 	}
-	return &transcriptNote{
+	note := &transcriptNote{
 		Source: e.TranscriptSource,
 		Cues:   e.TranscriptCues,
 		Note:   e.TranscriptNote,
 	}
+	if e.HasTranscript() {
+		note.URL = "/api/transcript?episode=" + url.QueryEscape(e.ID) +
+			"&token=" + url.QueryEscape(s.opts.Token)
+	}
+	return note
 }
 
 // transcript serves one episode's words, which are fetched only when somebody opens them --
diff --git a/server/internal/api/transcript_test.go b/server/internal/api/transcript_test.go
index b208f7f..651225d 100644
--- a/server/internal/api/transcript_test.go
+++ b/server/internal/api/transcript_test.go
@@ -6,6 +6,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/terst/listen/server/internal/feed"
@@ -82,6 +83,60 @@ func TestTheEpisodeResponseSaysWhetherThereAreWords(t *testing.T) {
 	}
 }
 
+/*
+The response carries where the words are, the way it carries where the audio is.
+
+The phone keys episodes by feed and guid; the server keys them by its own id. Handing it a URL
+rather than an id keeps it that way -- the audio URL already works like this, and the
+alternative is the app learning to parse an id out of the audio URL to build a second one.
+*/
+func TestTheResponseSaysWhereTheWordsAre(t *testing.T) {
+	s, st, _, audioDir := newServer(t)
+	e := seedTranscribedEpisode(t, s, st, audioDir)
+
+	rec := get(t, s, "/api/episode?feed=https://f&guid=g1")
+	var got struct {
+		Transcript *struct {
+			URL string `json:"url"`
+		} `json:"transcript"`
+	}
+	if err := json.NewDecoder(rec.Body).Decode(&got); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	if got.Transcript == nil || !strings.Contains(got.Transcript.URL, e.ID) {
+		t.Fatalf("got %+v, want a URL naming the episode", got.Transcript)
+	}
+	// Usable as given, like the audio URL: the phone appends nothing.
+	if !strings.Contains(got.Transcript.URL, "token=") {
+		t.Errorf("url = %q, want it to carry the token", got.Transcript.URL)
+	}
+}
+
+// Nothing to fetch, so nothing to point at.
+func TestARefusalCarriesNoURL(t *testing.T) {
+	s, st, _, audioDir := newServer(t)
+	st.Merge("https://f", "A Show", []feed.Episode{{
+		GUID: "g1", Title: "One", EnclosureURL: "https://cdn/1.mp3", PublishedUnix: 100,
+	}})
+	e := st.Match("https://f", "g1", "", "")
+	e.Path = e.ID + ".mp3"
+	writeAudio(t, audioDir, e.Path)
+	e.TranscriptNote = "this copy carries 4:16 of inserted advertising"
+
+	rec := get(t, s, "/api/episode?feed=https://f&guid=g1")
+	var got struct {
+		Transcript *struct {
+			URL string `json:"url"`
+		} `json:"transcript"`
+	}
+	if err := json.NewDecoder(rec.Body).Decode(&got); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	if got.Transcript == nil || got.Transcript.URL != "" {
+		t.Errorf("got %+v, want a refusal pointing at nothing", got.Transcript)
+	}
+}
+
 // Refused is not the same as absent, and the phone shows the difference: an episode whose
 // publisher's transcript does not describe the copy being played has been looked at.
 func TestTheEpisodeResponseCarriesARefusal(t *testing.T) {
diff --git a/server/internal/safefetch/live_check_test.go b/server/internal/safefetch/live_check_test.go
new file mode 100644
index 0000000..a10d162
--- /dev/null
+++ b/server/internal/safefetch/live_check_test.go
@@ -0,0 +1,73 @@
+package safefetch
+
+import (
+	"context"
+	"encoding/json"
+	"net"
+	"net/url"
+	"os"
+	"testing"
+)
+
+// TestTheLiveSubscriptionsAreAllReachable guards against the address rules being so strict they
+// stop Peter's own library mirroring. Skipped where the state file is not present.
+func TestTheLiveSubscriptionsAreAllReachable(t *testing.T) {
+	raw, err := os.ReadFile("/site/listen.terst.org/data/state.json")
+	if err != nil {
+		t.Skip("no live state file here")
+	}
+	var state struct {
+		Feeds []string `json:"feeds"`
+		Shows map[string]struct {
+			Episodes []struct {
+				EnclosureURL  string `json:"enclosureUrl"`
+			} `json:"episodes"`
+		} `json:"shows"`
+	}
+	if err := json.Unmarshal(raw, &state); err != nil {
+		t.Fatalf("state: %v", err)
+	}
+
+	targets := append([]string(nil), state.Feeds...)
+	for _, show := range state.Shows {
+		for i, e := range show.Episodes {
+			if i >= 2 {
+				break
+			}
+			targets = append(targets, e.EnclosureURL)
+		}
+	}
+
+	refused := 0
+	for _, raw := range targets {
+		if err := CheckURL(raw); err != nil {
+			t.Errorf("%s: %v", raw, err)
+			continue
+		}
+		host := mustHost(t, raw)
+		addresses, err := net.DefaultResolver.LookupIPAddr(context.Background(), host)
+		if err != nil {
+			continue // DNS trouble is not what this is measuring
+		}
+		any := false
+		for _, a := range addresses {
+			if permitted(a.IP) {
+				any = true
+			}
+		}
+		if !any {
+			refused++
+			t.Errorf("%s resolves only to addresses this would refuse", host)
+		}
+	}
+	t.Logf("checked %d live URLs, %d refused", len(targets), refused)
+}
+
+func mustHost(t *testing.T, raw string) string {
+	t.Helper()
+	parsed, err := url.Parse(raw)
+	if err != nil {
+		t.Fatalf("%s: %v", raw, err)
+	}
+	return parsed.Hostname()
+}
diff --git a/server/internal/safefetch/safefetch.go b/server/internal/safefetch/safefetch.go
index 3233f8f..9119406 100644
--- a/server/internal/safefetch/safefetch.go
+++ b/server/internal/safefetch/safefetch.go
@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/netip"
 	"net/url"
 	"time"
 )
@@ -50,6 +51,10 @@ func Client(timeout time.Duration) *http.Client {
 	}
 	dialer := &net.Dialer{Timeout: 20 * time.Second, KeepAlive: 30 * time.Second}
 	transport := http.DefaultTransport.(*http.Transport).Clone()
+	// No proxy. http.DefaultTransport carries ProxyFromEnvironment, and with HTTP_PROXY set
+	// the connection goes to the proxy instead -- so the address checked below would be the
+	// proxy's and the one that matters would never be looked at at all.
+	transport.Proxy = nil
 	transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
 		host, port, err := net.SplitHostPort(addr)
 		if err != nil {
@@ -81,30 +86,63 @@ func Client(timeout time.Duration) *http.Client {
 	return &http.Client{Timeout: timeout, Transport: transport}
 }
 
+// specialPurpose are the ranges that are not the open internet, from the IANA special-purpose
+// registries.
+//
+// An allowlist rather than a list of things that felt private: "not RFC 1918" is not the same
+// as "public". 100.64.0.0/10 is carrier-grade NAT and routes to a real host on a real network;
+// 64:ff9b::/96 is NAT64 and carries an IPv4 address inside it, 127.0.0.1 included; 2002::/16 is
+// 6to4 and does the same. None of them is private by Go's definition and none of them is
+// somewhere a podcast is served from.
+var specialPurpose = []netip.Prefix{
+	netip.MustParsePrefix("0.0.0.0/8"),       // "this network"
+	netip.MustParsePrefix("100.64.0.0/10"),   // carrier-grade NAT
+	netip.MustParsePrefix("192.0.0.0/24"),    // IETF protocol assignments
+	netip.MustParsePrefix("192.0.2.0/24"),    // documentation
+	netip.MustParsePrefix("192.88.99.0/24"),  // 6to4 relay anycast, deprecated
+	netip.MustParsePrefix("198.18.0.0/15"),   // benchmarking
+	netip.MustParsePrefix("198.51.100.0/24"), // documentation
+	netip.MustParsePrefix("203.0.113.0/24"),  // documentation
+	netip.MustParsePrefix("240.0.0.0/4"),     // reserved, and 255.255.255.255 within it
+	netip.MustParsePrefix("::/96"),           // IPv4-compatible IPv6, deprecated
+	netip.MustParsePrefix("64:ff9b::/96"),    // NAT64
+	netip.MustParsePrefix("64:ff9b:1::/48"),  // local-use NAT64
+	netip.MustParsePrefix("100::/64"),        // discard-only
+	netip.MustParsePrefix("2001::/23"),       // IETF protocol assignments, Teredo within it
+	netip.MustParsePrefix("2001:db8::/32"),   // documentation
+	netip.MustParsePrefix("2002::/16"),       // 6to4, which embeds an IPv4 address
+	netip.MustParsePrefix("3fff::/20"),       // documentation
+}
+
 // 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.
+// Stated as what is allowed rather than as a list of things to block: an ordinary global
+// unicast address that is not in a special-purpose range. Everything a podcast is actually
+// served from is one of those, and the failure mode of being too strict is a feed that does not
+// mirror, which is visible -- while the failure mode of being too lax is the server fetching
+// something only it can reach and handing the result to the phone.
 func permitted(ip net.IP) bool {
 	if ip == nil {
 		return false
 	}
+	addr, ok := netip.AddrFromSlice(ip)
+	if !ok {
+		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
+	addr = addr.Unmap()
+	if !addr.IsGlobalUnicast() {
+		// Loopback, unspecified, multicast and link-local are all excluded here.
+		return false
 	}
-	switch {
-	case ip.IsLoopback(),
-		ip.IsPrivate(),
-		ip.IsLinkLocalUnicast(),
-		ip.IsLinkLocalMulticast(),
-		ip.IsInterfaceLocalMulticast(),
-		ip.IsMulticast(),
-		ip.IsUnspecified():
+	if addr.IsPrivate() {
 		return false
 	}
+	for _, prefix := range specialPurpose {
+		if prefix.Contains(addr) {
+			return false
+		}
+	}
 	return true
 }
diff --git a/server/internal/safefetch/safefetch_test.go b/server/internal/safefetch/safefetch_test.go
index dfa3470..bcb7e20 100644
--- a/server/internal/safefetch/safefetch_test.go
+++ b/server/internal/safefetch/safefetch_test.go
@@ -30,6 +30,20 @@ func TestLoopbackAndPrivateAddressesAreRefused(t *testing.T) {
 		"169.254.169.254", // the cloud metadata address
 		"fe80::1", "fc00::1",
 		"0.0.0.0", "::",
+		// Special-purpose ranges that are not "private" but are not the open internet
+		// either, and several of which route somewhere on a real network.
+		"0.1.2.3",        // "this network"
+		"100.64.0.1",     // carrier-grade NAT
+		"192.0.0.1",      // IETF protocol assignments
+		"192.0.2.1",      // documentation
+		"198.18.0.1",     // benchmarking
+		"198.51.100.1",   // documentation
+		"203.0.113.1",    // documentation
+		"240.0.0.1",      // reserved
+		"255.255.255.255", // broadcast
+		"64:ff9b::7f00:1", // NAT64, carrying 127.0.0.1
+		"2001:db8::1",     // documentation
+		"::ffff:10.0.0.1", // a private v4 address in v6 clothing
 	}
 	for _, ip := range blocked {
 		if permitted(net.ParseIP(ip)) {
@@ -71,6 +85,23 @@ func TestAClientCannotReachALoopbackServer(t *testing.T) {
 	}
 }
 
+/*
+An environment proxy would make the dial check meaningless.
+
+http.DefaultTransport carries ProxyFromEnvironment, so with HTTP_PROXY set the connection goes
+to the proxy and the proxy connects to whatever the URL named -- the address this package
+checks is then the proxy's, and the one that matters is never looked at.
+*/
+func TestTheSafeTransportUsesNoProxy(t *testing.T) {
+	transport, ok := Client(0).Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("transport is %T, want *http.Transport", Client(0).Transport)
+	}
+	if transport.Proxy != nil {
+		t.Error("the safe client would use a proxy, which bypasses the address check entirely")
+	}
+}
+
 // 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", ""} {