The player sends the token in a header too, not just the downloader

The last thing standing between the token and the query string it should not
be in. The downloader moved to an Authorization header a while back; the player
never did, and the player is how the car plays anything not already on disk.
Withdrawing the query parameter without this would have broken streaming in the
car silently -- a 401 arriving as "playback failed".

ExoPlayer resolves a URL on its own loading thread with no coroutine to read
DataStore in, so ServerAudioAuth caches the configuration and the session
callback refreshes it before handing over any stream URL. Nothing is cached per
URL: the same player fetches publisher audio, and the whole point of
ServerBearer is that those get nothing. Attached through a ResolvingDataSource
rather than as a default request property for the same reason -- a default
would send Peter's server credentials to every podcast CDN he subscribes to.

Before the first refresh it sends nothing, which is the safe direction: a
stream that should have carried the token fails visibly, where one that should
not have carried it would have leaked it.

Verified against the live server, not a fake: the player reaches READY on
listen.terst.org audio with a header and no token in the URL, and the same URL
without credentials is still refused. The token is supplied as an
instrumentation argument and is not in the repository -- a test file carrying
it would be the same mistake in a different file -- so the test skips unless
one is passed.

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

6 files changed, 268 insertions, 1 deletions

diff --git a/app/src/androidTest/java/org/terst/listen/playback/StreamedServerAudioTest.kt b/app/src/androidTest/java/org/terst/listen/playback/StreamedServerAudioTest.kt
new file mode 100644
index 0000000..6255a42
--- /dev/null
+++ b/app/src/androidTest/java/org/terst/listen/playback/StreamedServerAudioTest.kt
@@ -0,0 +1,135 @@
+package org.terst.listen.playback
+
+import android.content.ComponentName
+import androidx.media3.common.MediaItem
+import androidx.media3.common.Player
+import androidx.media3.session.MediaBrowser
+import androidx.media3.session.SessionToken
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.guava.await
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withContext
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Assume.assumeFalse
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.terst.listen.data.Graph
+import java.net.HttpURLConnection
+import java.net.URL
+
+/**
+ * The server's audio plays when it is streamed, not only when it is downloaded.
+ *
+ * The token used to ride in the audio URL's query string, which wrote a copy of the secret into
+ * Apache's access log on every fetch. The downloader moved to an `Authorization` header; the
+ * player had not, and the player is how the car plays anything not already on disk. Withdrawing
+ * the query parameter without this would have broken streaming in the car silently, as a 401
+ * surfacing as "playback failed".
+ *
+ * Skipped unless a real token is supplied, because the thing being tested is a live server's
+ * answer and a fake one proves nothing about Apache, the vhost or the token:
+ *
+ * ```
+ * ... -Pandroid.testInstrumentationRunnerArguments.listenToken=<token>
+ * ```
+ *
+ * The token is never checked in. It arrives as an instrumentation argument for exactly that
+ * reason -- a test file carrying it would put the secret back in the repository, which is the
+ * same mistake in a different file.
+ */
+@RunWith(AndroidJUnit4::class)
+class StreamedServerAudioTest {
+
+    private val args = InstrumentationRegistry.getArguments()
+    private val token: String = args.getString("listenToken").orEmpty()
+    private val baseUrl: String = args.getString("listenServer") ?: "https://listen.terst.org"
+    private val episodeId: String = args.getString("listenEpisode").orEmpty()
+
+    /**
+     * Asks the server for the audio exactly as the player now does: header, no query.
+     *
+     * Driven through the app's own [ServerAudioAuth] rather than a hand-written header, so what
+     * is proved is the decision the player actually makes -- including that it decides to send
+     * anything at all for this URL.
+     */
+    @Test
+    fun theServerServesItsAudioToAHeaderWithNoTokenInTheUrl() = runBlocking {
+        assumeFalse("no listenToken supplied", token.isEmpty())
+        assumeFalse("no listenEpisode supplied", episodeId.isEmpty())
+
+        val context = InstrumentationRegistry.getInstrumentation().targetContext
+        Graph.get(context).preferences.setServer(baseUrl, token)
+        val auth = Graph.get(context).serverAudioAuth.also { it.refresh() }
+
+        val url = "$baseUrl/audio/$episodeId"
+        val header = auth.headerFor(url)
+        assertEquals("the player must send the token to the server's own audio", "Bearer $token", header)
+
+        assertEquals("server should serve the audio to a header alone", 200, statusOf(url, header))
+        assertEquals("and must still refuse an unauthenticated fetch", 401, statusOf(url, null))
+    }
+
+    /**
+     * The player itself streams it, which is the half the check above cannot reach.
+     *
+     * That one proves the app decides to send a token and the server accepts one. This proves
+     * ExoPlayer actually puts it on the wire: the header is attached by a `ResolvingDataSource`
+     * wrapped around the HTTP source, and if that wiring were wrong the decision would be made
+     * correctly and thrown away. A 401 surfaces here as a source error rather than READY.
+     */
+    @Test
+    fun thePlayerReachesReadyOnTheServersAudio() = runBlocking {
+        assumeFalse("no listenToken supplied", token.isEmpty())
+        assumeFalse("no listenEpisode supplied", episodeId.isEmpty())
+
+        val context = InstrumentationRegistry.getInstrumentation().targetContext
+        Graph.get(context).preferences.setServer(baseUrl, token)
+
+        val token2 = SessionToken(context, ComponentName(context, PlaybackService::class.java))
+        val browser = withContext(Dispatchers.Main) {
+            MediaBrowser.Builder(context, token2).buildAsync().await()
+        }
+        try {
+            withContext(Dispatchers.Main) {
+                browser.setMediaItem(
+                    MediaItem.Builder().setUri("$baseUrl/audio/$episodeId").build()
+                )
+                browser.prepare()
+
+                val giveUpAt = System.currentTimeMillis() + 30_000
+                while (browser.playbackState != Player.STATE_READY &&
+                    browser.playerError == null &&
+                    System.currentTimeMillis() < giveUpAt
+                ) {
+                    delay(200)
+                }
+                assertNull("streaming the server's audio errored", browser.playerError)
+                assertEquals(
+                    "player should be ready on the server's stream",
+                    Player.STATE_READY,
+                    browser.playbackState,
+                )
+            }
+        } finally {
+            withContext(Dispatchers.Main) {
+                browser.stop()
+                browser.release()
+            }
+        }
+    }
+
+    private fun statusOf(url: String, header: String?): Int {
+        val connection = URL(url).openConnection() as HttpURLConnection
+        return try {
+            connection.requestMethod = "HEAD"
+            header?.let { connection.setRequestProperty("Authorization", it) }
+            connection.responseCode
+        } finally {
+            connection.disconnect()
+        }
+    }
+}
diff --git a/app/src/main/java/org/terst/listen/data/Repositories.kt b/app/src/main/java/org/terst/listen/data/Repositories.kt
index f08c484..0817178 100644
--- a/app/src/main/java/org/terst/listen/data/Repositories.kt
+++ b/app/src/main/java/org/terst/listen/data/Repositories.kt
@@ -10,6 +10,7 @@ import org.terst.listen.download.WaitingDownloads
 import org.terst.listen.server.LabelPusher
 import org.terst.listen.server.ServerSkips
 import org.terst.listen.server.SkipRefresher
+import org.terst.listen.playback.ServerAudioAuth
 import org.terst.listen.server.SkipServer
 import org.terst.listen.server.SkipServerConfig
 import org.terst.listen.server.StoredRefusals
@@ -227,6 +228,15 @@ class AppGraph(private val context: Context) : UiGraph {
      * WorkManager's ten-minute ceiling allows, and the attempt burned the battery for weeks
      * while confirming nothing.
      */
+    /**
+     * The token the player sends, shared so the service and the session callback hold one cache.
+     *
+     * The callback refreshes it; the player reads it from ExoPlayer's loading thread, which has
+     * no coroutine to suspend in. Two instances would mean the one doing the reading was never
+     * the one that had been refreshed.
+     */
+    val serverAudioAuth: ServerAudioAuth by lazy { ServerAudioAuth { skipServer.configured() } }
+
     val skipServer: SkipServer by lazy {
         SkipServer(
             config = {
diff --git a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt
index d8e1856..758fb91 100644
--- a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt
+++ b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt
@@ -199,6 +199,9 @@ class ListenSessionCallback(
         controller: MediaSession.ControllerInfo,
         mediaItems: MutableList<MediaItem>,
     ): ListenableFuture<MutableList<MediaItem>> = scope.future {
+        // Before any stream URL is handed to the player: the resolver reads this cache from
+        // ExoPlayer's loading thread, where there is no coroutine to read DataStore in.
+        graph.serverAudioAuth.refresh()
         mediaItems.mapNotNull { item ->
             val episodeId = BrowseIds.episodeId(item.mediaId) ?: return@mapNotNull item
             val episode = graph.episodes.getById(episodeId) ?: return@mapNotNull null
diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
index a2ca31c..d8ff882 100644
--- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
+++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
@@ -8,7 +8,11 @@ import androidx.media3.common.C
 import androidx.media3.common.MediaItem
 import androidx.media3.common.PlaybackParameters
 import androidx.media3.common.Player
+import androidx.media3.datasource.DefaultDataSource
+import androidx.media3.datasource.DefaultHttpDataSource
+import androidx.media3.datasource.ResolvingDataSource
 import androidx.media3.exoplayer.ExoPlayer
+import androidx.media3.exoplayer.source.DefaultMediaSourceFactory
 import androidx.media3.session.MediaSession
 import androidx.media3.session.MediaLibraryService
 import kotlinx.coroutines.CoroutineScope
@@ -54,7 +58,21 @@ class PlaybackService : MediaLibraryService() {
 
     override fun onCreate() {
         super.onCreate()
+        val graph = Graph.get(this)
+        // The server's audio needs the token in a header. It used to ride in the URL's query,
+        // which wrote a copy of the secret into Apache's access log on every fetch; the
+        // downloader moved to a header and streaming had to follow, because streaming is how the
+        // car plays anything not already downloaded. Resolved per URL rather than set as a
+        // default request property: the same player fetches publisher audio, and that gets none.
+        val audioSource = ResolvingDataSource.Factory(
+            DefaultDataSource.Factory(this, DefaultHttpDataSource.Factory()),
+        ) { spec ->
+            graph.serverAudioAuth.headerFor(spec.uri.toString())
+                ?.let { spec.withAdditionalHeaders(mapOf("Authorization" to it)) }
+                ?: spec
+        }
         val exo = ExoPlayer.Builder(this)
+            .setMediaSourceFactory(DefaultMediaSourceFactory(audioSource))
             .setAudioAttributes(
                 AudioAttributes.Builder()
                     .setContentType(C.AUDIO_CONTENT_TYPE_SPEECH)
@@ -75,7 +93,6 @@ class PlaybackService : MediaLibraryService() {
 
         // Read before the session is built: the session's callback needs the reporter, because
         // a "this is an ad" press is answered from the player's position.
-        val graph = Graph.get(this)
         reporter = AdReporter(AdReportGesture(), graph.adReportStore)
 
         // A library session rather than a plain one: Android Auto only treats an app as
diff --git a/app/src/main/java/org/terst/listen/playback/ServerAudioAuth.kt b/app/src/main/java/org/terst/listen/playback/ServerAudioAuth.kt
new file mode 100644
index 0000000..ec8b4b5
--- /dev/null
+++ b/app/src/main/java/org/terst/listen/playback/ServerAudioAuth.kt
@@ -0,0 +1,43 @@
+package org.terst.listen.playback
+
+import org.terst.listen.server.ServerBearer
+import org.terst.listen.server.SkipServerConfig
+
+/**
+ * The token the *player* sends, as opposed to the one the downloader sends.
+ *
+ * Both ask [ServerBearer] the same question and must answer it the same way, but they ask it from
+ * different places. The downloader is in a coroutine and can read the configuration when it needs
+ * it; ExoPlayer resolves a URL on its own loading thread with no coroutine in sight, so the
+ * configuration has to be sitting there already. Hence a cache and an explicit [refresh], rather
+ * than blocking the loading thread on a DataStore read before every stream starts.
+ *
+ * Why the player needs a token at all: the server's audio URL used to carry it in the query
+ * string, which put a copy of the secret in Apache's access log on every fetch. The downloader
+ * moved to a header; streaming did not, and streaming is how the car plays anything it has not
+ * already downloaded. Until both send it, the query parameter cannot be withdrawn.
+ *
+ * Nothing is cached per URL. The decision depends on the URL and must be made again every time:
+ * the same player fetches publisher audio too, and the point of [ServerBearer] is that those get
+ * nothing.
+ */
+class ServerAudioAuth(private val config: suspend () -> SkipServerConfig?) {
+
+    @Volatile
+    private var cached: SkipServerConfig? = null
+
+    /** Reads the current configuration, so [headerFor] can answer without suspending. */
+    suspend fun refresh() {
+        cached = config()
+    }
+
+    /**
+     * The `Authorization` value for [url], or null when it is not the server's to send to.
+     *
+     * Null before the first [refresh], which is the safe direction: a stream that should have
+     * carried the token fails visibly with a 401, where one that should not have carried it
+     * would hand Peter's credentials to a podcast host.
+     */
+    fun headerFor(url: String): String? =
+        ServerBearer.forUrl(cached, url)?.let { "Bearer $it" }
+}
diff --git a/app/src/test/java/org/terst/listen/playback/ServerAudioAuthTest.kt b/app/src/test/java/org/terst/listen/playback/ServerAudioAuthTest.kt
new file mode 100644
index 0000000..fa5c5bc
--- /dev/null
+++ b/app/src/test/java/org/terst/listen/playback/ServerAudioAuthTest.kt
@@ -0,0 +1,59 @@
+package org.terst.listen.playback
+
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+import org.terst.listen.server.SkipServerConfig
+
+class ServerAudioAuthTest {
+
+    private val server = SkipServerConfig(baseUrl = "https://listen.example.org", token = "s3cret")
+
+    private fun auth(config: SkipServerConfig? = server) = ServerAudioAuth { config }
+
+    @Test
+    fun sendsNothingBeforeTheConfigurationHasBeenRead() {
+        assertNull(auth().headerFor("https://listen.example.org/audio/abc"))
+    }
+
+    @Test
+    fun sendsTheTokenToTheServersOwnAudio() = runBlocking {
+        val auth = auth().also { it.refresh() }
+
+        assertEquals("Bearer s3cret", auth.headerFor("https://listen.example.org/audio/abc"))
+    }
+
+    /**
+     * The failure this exists to prevent: the same player fetches whatever a publisher nominated,
+     * and a token sent there is Peter's server credentials handed to a podcast host.
+     */
+    @Test
+    fun sendsNothingToAPublisher() = runBlocking {
+        val auth = auth().also { it.refresh() }
+
+        assertNull(auth.headerFor("https://cdn.megaphone.fm/episode.mp3"))
+        assertNull(auth.headerFor("https://listen.example.org.evil.test/audio/abc"))
+        assertNull(auth.headerFor("http://listen.example.org/audio/abc"))
+    }
+
+    @Test
+    fun sendsNothingWhenNoServerIsConfigured() = runBlocking {
+        val auth = auth(config = null).also { it.refresh() }
+
+        assertNull(auth.headerFor("https://listen.example.org/audio/abc"))
+    }
+
+    @Test
+    fun picksUpAServerConfiguredAfterItStarted() = runBlocking {
+        var current: SkipServerConfig? = null
+        val auth = ServerAudioAuth { current }
+        auth.refresh()
+        assertNull(auth.headerFor("https://listen.example.org/audio/abc"))
+
+        current = server
+        auth.refresh()
+
+        assertEquals("Bearer s3cret", auth.headerFor("https://listen.example.org/audio/abc"))
+    }
+}