From d73a4b53aa4aba6e7eedbca166bdea7cc2851beb Mon Sep 17 00:00:00 2001 From: Kayos Date: Tue, 26 May 2026 21:55:29 -0700 Subject: [PATCH] vc=70: audit-fix sprint round 3 (wrapper) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 audit (on vc=69) caught three real HIGHs plus three worth-fixing MEDs. All in the same family: 'isAllowedYtUrl gate missing at consumer side.' HIGH-1 — ChannelScreen ignored the round-2 loadedUrl gate Round 2 added loadedUrl to ChannelUiState but ChannelScreen never read it. Channel A → back → Channel B showed A's data for one frame before vm.load(B) cleared it. Same shape as the VideoDetailScreen fix; matching gate added. HIGH-2 — PlaybackService.tryAutoplay missing allowlist Both the SameChannel path (channelInfo(uploaderUrl)) and the candidate-resolve path (streamInfo(candidateUrl)) hit strawcore with extractor-derived URLs. Gate added on both call sites, plus the picker's intermediate uploaderUrl check. HIGH-3 — VideoActionsSheet.enqueue missing allowlist Long-press on a poisoned related-card / channel video → 'Add to queue' invoked uniffi.strawcore.streamInfo on the raw target.url. Bail with Toast before launching the resolve coroutine. MED-1 — FeedRefreshWorker doesn't retry on RequiresLogin reCAPTCHA challenges clear minutes-to-hours later. Treating them as permanent ate a full refresh cycle. Catch StrawcoreException.RequiresLogin → Result.retry(). MED-3 — VideoDetail.uploaderUrl persisted raw extractor value Round-2 added safeFresh for the avatar but the uploaderUrl saved to VideoDetail.detail was still info.uploaderUrl. NowPlaying → PlaybackService picked up the raw value. Validate once and persist the SAFE value so the whole downstream chain inherits it. MED-4 — enrich-job filter rebuilt Set per iteration .filter { it.url in channelsSnapshot.map { c -> c.url }.toSet() } was O(N²). Hoist via mapTo(HashSet()) once. Bonus sweep — gated two more uniffi.strawcore.* sites the round-3 agent's category prediction caught: * SubscriptionFeedViewModel.enrichVisibleItems → enrichFeedItem now skips items failing isAllowedYtUrl. * PlaybackService autoplay candidate-resolve already covered under HIGH-2 above. --- buildSrc/src/main/kotlin/ProjectConfig.kt | 4 ++-- .../straw/feature/channel/ChannelScreen.kt | 10 ++++++++++ .../feature/detail/VideoDetailViewModel.kt | 18 +++++++++++++++--- .../straw/feature/feed/FeedRefreshWorker.kt | 7 +++++++ .../feature/feed/SubscriptionFeedViewModel.kt | 16 ++++++++++++++-- .../straw/feature/player/PlaybackService.kt | 12 ++++++++++++ .../straw/feature/playlist/VideoActions.kt | 9 +++++++++ 7 files changed, 69 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 8889f44a1..c09cbd2e2 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -55,6 +55,6 @@ const val NEWPIPE_APPLICATION_ID_NEW = "net.newpipe.app" // vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via // strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no // NewPipeExtractor in the runtime path. -const val STRAW_VERSION_CODE = 69 -const val STRAW_VERSION_NAME = "0.1.0-CC" +const val STRAW_VERSION_CODE = 70 +const val STRAW_VERSION_NAME = "0.1.0-CD" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt index 1618fc2a9..1b1ab29ce 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/channel/ChannelScreen.kt @@ -76,6 +76,16 @@ fun ChannelScreen( } when { + // Stale-state gate: activity-scoped VM, so when we navigate A → B + // the screen recomposes once with A's state before vm.load(B) + // resets it. Without this branch we'd render channel A's banner / + // name / videos under URL B. Same shape as VideoDetailScreen's + // gate. Round-69 audit HIGH-1. + state.loadedUrl != channelUrl -> Box( + modifier = Modifier.fillMaxSize().statusBarsPadding(), + contentAlignment = Alignment.Center, + ) { CircularProgressIndicator() } + state.loading -> Box( modifier = Modifier.fillMaxSize().statusBarsPadding(), contentAlignment = Alignment.Center, diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt index 6f00358a3..df48c4fb4 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt @@ -217,14 +217,23 @@ class VideoDetailViewModel : ViewModel() { // we apply to imports: a poisoned uploaderUrl from the // extractor would otherwise trigger an arbitrary-host // network call. Round-4 audit HIGH-4. - val uploaderUrl = info.uploaderUrl + // + // Round-69 audit MED-3: validate once and persist the + // SAFE value into VideoDetail.uploaderUrl so downstream + // consumers (NowPlaying → PlaybackService autoplay, + // queue, etc.) inherit the validated string instead + // of the raw extractor value. + val rawUploaderUrl = info.uploaderUrl + val uploaderUrl = if (!rawUploaderUrl.isNullOrBlank() && isAllowedYtUrl(rawUploaderUrl)) { + rawUploaderUrl + } else null data class ChannelExtras( val avatar: String?, val subscriberCount: Long, val videos: List, ) val channelExtras: ChannelExtras = - if (uploaderUrl.isNullOrBlank() || !isAllowedYtUrl(uploaderUrl)) { + if (uploaderUrl == null) { ChannelExtras(null, -1, emptyList()) } else runCatchingCancellable { val ch = uniffi.strawcore.channelInfo(uploaderUrl) @@ -288,7 +297,10 @@ class VideoDetailViewModel : ViewModel() { id = videoId, title = title, uploader = uploader, - uploaderUrl = info.uploaderUrl, + // Use the allowlist-validated value, not + // the raw extractor field. Round-69 audit + // MED-3. + uploaderUrl = uploaderUrl, uploaderAvatar = channelExtras.avatar, uploaderSubscriberCount = channelExtras.subscriberCount, viewCount = info.viewCount, diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/FeedRefreshWorker.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/FeedRefreshWorker.kt index 0d816db2d..40bfd117a 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/FeedRefreshWorker.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/FeedRefreshWorker.kt @@ -56,6 +56,13 @@ class FeedRefreshWorker( } catch (e: uniffi.strawcore.StrawcoreException.Network) { strawLogW("FeedRefresh") { "transient network failure, retrying: ${e.message}" } return Result.retry() + } catch (e: uniffi.strawcore.StrawcoreException.RequiresLogin) { + // reCAPTCHA challenges clear on their own minutes-to-hours + // later. Treating these as permanent eats a full refresh + // cycle the same way the pre-fix IOException catch did. + // Round-69 audit MED-1. + strawLogW("FeedRefresh") { "YT challenge, retrying: ${e.message}" } + return Result.retry() } catch (e: Throwable) { strawLogW("FeedRefresh") { "non-transient failure, giving up this cycle: ${e.message}" } return Result.success() diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt index e1a24a32e..e0864ebb1 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt @@ -371,6 +371,15 @@ class SubscriptionFeedViewModel : ViewModel() { gate.withPermit { val videoId = com.sulkta.straw.feature.detail.extractYtVideoId(item.url) ?: return@withPermit + // Defense in depth: enrichFeedItem calls + // strawcore.stream_info which expects a + // canonical YT URL. A poisoned cached + // item.url shouldn't be able to reach the + // extractor either. Round-69 audit + // family — uniffi.strawcore.* sites that + // take a user-influenced URL all get the + // gate. + if (!com.sulkta.straw.util.isAllowedYtUrl(item.url)) return@withPermit if (FeedEnrichment.get().get(videoId) != null) return@withPermit val md = runCatchingCancellable { withContext(Dispatchers.IO) { @@ -396,9 +405,12 @@ class SubscriptionFeedViewModel : ViewModel() { // include channels the user has since unsubscribed from // in the ~2s enrich window. Intersect so a freshly- // unsubscribed channel doesn't briefly re-appear in the - // feed after the enrich emit. + // feed after the enrich emit. Round-69 audit MED-4: + // hoist the snapshot-URL set once instead of rebuilding + // it per filter iteration. + val snapshotUrls = channelsSnapshot.mapTo(HashSet()) { it.url } val mergeChannels = Subscriptions.get().subs.value - .filter { it.url in channelsSnapshot.map { c -> c.url }.toSet() } + .filter { it.url in snapshotUrls } val merged = withContext(Dispatchers.Default) { mergeFromCache(mergeChannels) } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt index ea0e89f50..e9ca5ab3c 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt @@ -54,6 +54,7 @@ import com.sulkta.straw.feature.detail.resolveStreamPlayback import com.sulkta.straw.net.IosSafeHttpDataSource import com.sulkta.straw.net.STRAW_USER_AGENT import com.sulkta.straw.net.SponsorBlockClient +import com.sulkta.straw.util.isAllowedYtUrl import com.sulkta.straw.util.runCatchingCancellable import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -271,6 +272,12 @@ class PlaybackService : MediaSessionService() { val candidateUrl = withContext(Dispatchers.IO) { pickAutoplayCandidate(mode, current.streamUrl, uploaderUrl) } ?: return@runCatchingCancellable + // Final allowlist gate before we hit strawcore with a + // URL whose origin was the extractor. Same defense as + // VideoDetailViewModel.load. Round-69 audit HIGH-2 / + // HIGH-3 family — every uniffi.strawcore.* site that + // takes a user-influenced URL needs this gate. + if (!isAllowedYtUrl(candidateUrl)) return@runCatchingCancellable // Resolve + enqueue + auto-play. Because the queue is // currently empty (we just ended), enqueueLast routes // through setPlayingFrom (auto-starts). @@ -310,6 +317,11 @@ class PlaybackService : MediaSessionService() { AutoplayMode.Off -> null AutoplayMode.SameChannel -> { if (uploaderUrl.isNullOrBlank()) return null + // uploaderUrl came from the extractor and flows + // through NowPlaying without revalidation. Same + // gate as the inline channelInfo path. Round-69 + // audit HIGH-2. + if (!isAllowedYtUrl(uploaderUrl)) return null val ch = uniffi.strawcore.channelInfo(uploaderUrl) ch.videos .asSequence() diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/playlist/VideoActions.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/playlist/VideoActions.kt index 51138d738..f90bf0918 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/playlist/VideoActions.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/playlist/VideoActions.kt @@ -104,6 +104,15 @@ fun VideoActionsSheet( Toast.makeText(context, "player not ready yet", Toast.LENGTH_SHORT).show() return } + // The action-sheet bypasses VideoDetailViewModel.load's + // allowlist gate (round-4 audit HIGH-4) — a long-press on a + // poisoned related-card otherwise hits strawcore directly. + // Round-69 audit HIGH-3. + if (!com.sulkta.straw.util.isAllowedYtUrl(target.streamUrl)) { + Toast.makeText(context, "unsupported URL", Toast.LENGTH_SHORT).show() + onDismiss() + return + } Toast.makeText(context, "Resolving…", Toast.LENGTH_SHORT).show() val appContext = context.applicationContext onDismiss()