vc=70: audit-fix sprint round 3 (wrapper)
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.
This commit is contained in:
parent
23fb6f52b0
commit
d73a4b53aa
7 changed files with 69 additions and 7 deletions
|
|
@ -55,6 +55,6 @@ const val NEWPIPE_APPLICATION_ID_NEW = "net.newpipe.app"
|
||||||
// vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via
|
// vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via
|
||||||
// strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no
|
// strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no
|
||||||
// NewPipeExtractor in the runtime path.
|
// NewPipeExtractor in the runtime path.
|
||||||
const val STRAW_VERSION_CODE = 69
|
const val STRAW_VERSION_CODE = 70
|
||||||
const val STRAW_VERSION_NAME = "0.1.0-CC"
|
const val STRAW_VERSION_NAME = "0.1.0-CD"
|
||||||
const val STRAW_APPLICATION_ID = "com.sulkta.straw"
|
const val STRAW_APPLICATION_ID = "com.sulkta.straw"
|
||||||
|
|
|
||||||
|
|
@ -76,6 +76,16 @@ fun ChannelScreen(
|
||||||
}
|
}
|
||||||
|
|
||||||
when {
|
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(
|
state.loading -> Box(
|
||||||
modifier = Modifier.fillMaxSize().statusBarsPadding(),
|
modifier = Modifier.fillMaxSize().statusBarsPadding(),
|
||||||
contentAlignment = Alignment.Center,
|
contentAlignment = Alignment.Center,
|
||||||
|
|
|
||||||
|
|
@ -217,14 +217,23 @@ class VideoDetailViewModel : ViewModel() {
|
||||||
// we apply to imports: a poisoned uploaderUrl from the
|
// we apply to imports: a poisoned uploaderUrl from the
|
||||||
// extractor would otherwise trigger an arbitrary-host
|
// extractor would otherwise trigger an arbitrary-host
|
||||||
// network call. Round-4 audit HIGH-4.
|
// 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(
|
data class ChannelExtras(
|
||||||
val avatar: String?,
|
val avatar: String?,
|
||||||
val subscriberCount: Long,
|
val subscriberCount: Long,
|
||||||
val videos: List<StreamItem>,
|
val videos: List<StreamItem>,
|
||||||
)
|
)
|
||||||
val channelExtras: ChannelExtras =
|
val channelExtras: ChannelExtras =
|
||||||
if (uploaderUrl.isNullOrBlank() || !isAllowedYtUrl(uploaderUrl)) {
|
if (uploaderUrl == null) {
|
||||||
ChannelExtras(null, -1, emptyList())
|
ChannelExtras(null, -1, emptyList())
|
||||||
} else runCatchingCancellable {
|
} else runCatchingCancellable {
|
||||||
val ch = uniffi.strawcore.channelInfo(uploaderUrl)
|
val ch = uniffi.strawcore.channelInfo(uploaderUrl)
|
||||||
|
|
@ -288,7 +297,10 @@ class VideoDetailViewModel : ViewModel() {
|
||||||
id = videoId,
|
id = videoId,
|
||||||
title = title,
|
title = title,
|
||||||
uploader = uploader,
|
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,
|
uploaderAvatar = channelExtras.avatar,
|
||||||
uploaderSubscriberCount = channelExtras.subscriberCount,
|
uploaderSubscriberCount = channelExtras.subscriberCount,
|
||||||
viewCount = info.viewCount,
|
viewCount = info.viewCount,
|
||||||
|
|
|
||||||
|
|
@ -56,6 +56,13 @@ class FeedRefreshWorker(
|
||||||
} catch (e: uniffi.strawcore.StrawcoreException.Network) {
|
} catch (e: uniffi.strawcore.StrawcoreException.Network) {
|
||||||
strawLogW("FeedRefresh") { "transient network failure, retrying: ${e.message}" }
|
strawLogW("FeedRefresh") { "transient network failure, retrying: ${e.message}" }
|
||||||
return Result.retry()
|
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) {
|
} catch (e: Throwable) {
|
||||||
strawLogW("FeedRefresh") { "non-transient failure, giving up this cycle: ${e.message}" }
|
strawLogW("FeedRefresh") { "non-transient failure, giving up this cycle: ${e.message}" }
|
||||||
return Result.success()
|
return Result.success()
|
||||||
|
|
|
||||||
|
|
@ -371,6 +371,15 @@ class SubscriptionFeedViewModel : ViewModel() {
|
||||||
gate.withPermit {
|
gate.withPermit {
|
||||||
val videoId = com.sulkta.straw.feature.detail.extractYtVideoId(item.url)
|
val videoId = com.sulkta.straw.feature.detail.extractYtVideoId(item.url)
|
||||||
?: return@withPermit
|
?: 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
|
if (FeedEnrichment.get().get(videoId) != null) return@withPermit
|
||||||
val md = runCatchingCancellable {
|
val md = runCatchingCancellable {
|
||||||
withContext(Dispatchers.IO) {
|
withContext(Dispatchers.IO) {
|
||||||
|
|
@ -396,9 +405,12 @@ class SubscriptionFeedViewModel : ViewModel() {
|
||||||
// include channels the user has since unsubscribed from
|
// include channels the user has since unsubscribed from
|
||||||
// in the ~2s enrich window. Intersect so a freshly-
|
// in the ~2s enrich window. Intersect so a freshly-
|
||||||
// unsubscribed channel doesn't briefly re-appear in the
|
// 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
|
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) {
|
val merged = withContext(Dispatchers.Default) {
|
||||||
mergeFromCache(mergeChannels)
|
mergeFromCache(mergeChannels)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -54,6 +54,7 @@ import com.sulkta.straw.feature.detail.resolveStreamPlayback
|
||||||
import com.sulkta.straw.net.IosSafeHttpDataSource
|
import com.sulkta.straw.net.IosSafeHttpDataSource
|
||||||
import com.sulkta.straw.net.STRAW_USER_AGENT
|
import com.sulkta.straw.net.STRAW_USER_AGENT
|
||||||
import com.sulkta.straw.net.SponsorBlockClient
|
import com.sulkta.straw.net.SponsorBlockClient
|
||||||
|
import com.sulkta.straw.util.isAllowedYtUrl
|
||||||
import com.sulkta.straw.util.runCatchingCancellable
|
import com.sulkta.straw.util.runCatchingCancellable
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.Job
|
import kotlinx.coroutines.Job
|
||||||
|
|
@ -271,6 +272,12 @@ class PlaybackService : MediaSessionService() {
|
||||||
val candidateUrl = withContext(Dispatchers.IO) {
|
val candidateUrl = withContext(Dispatchers.IO) {
|
||||||
pickAutoplayCandidate(mode, current.streamUrl, uploaderUrl)
|
pickAutoplayCandidate(mode, current.streamUrl, uploaderUrl)
|
||||||
} ?: return@runCatchingCancellable
|
} ?: 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
|
// Resolve + enqueue + auto-play. Because the queue is
|
||||||
// currently empty (we just ended), enqueueLast routes
|
// currently empty (we just ended), enqueueLast routes
|
||||||
// through setPlayingFrom (auto-starts).
|
// through setPlayingFrom (auto-starts).
|
||||||
|
|
@ -310,6 +317,11 @@ class PlaybackService : MediaSessionService() {
|
||||||
AutoplayMode.Off -> null
|
AutoplayMode.Off -> null
|
||||||
AutoplayMode.SameChannel -> {
|
AutoplayMode.SameChannel -> {
|
||||||
if (uploaderUrl.isNullOrBlank()) return null
|
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)
|
val ch = uniffi.strawcore.channelInfo(uploaderUrl)
|
||||||
ch.videos
|
ch.videos
|
||||||
.asSequence()
|
.asSequence()
|
||||||
|
|
|
||||||
|
|
@ -104,6 +104,15 @@ fun VideoActionsSheet(
|
||||||
Toast.makeText(context, "player not ready yet", Toast.LENGTH_SHORT).show()
|
Toast.makeText(context, "player not ready yet", Toast.LENGTH_SHORT).show()
|
||||||
return
|
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()
|
Toast.makeText(context, "Resolving…", Toast.LENGTH_SHORT).show()
|
||||||
val appContext = context.applicationContext
|
val appContext = context.applicationContext
|
||||||
onDismiss()
|
onDismiss()
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue