vc=68: audit-fix sprint round 1 (11 HIGH + MED batch)

Block B — enrichment lifecycle drift:
  * SubscriptionFeedViewModel tracks enrichJob, cancelled in refresh
    + clearInMemoryCache so spam-refresh and cache-toggle no longer
    leave a globalScope coroutine writing to a destroyed _ui
  * Enrich now runs on viewModelScope, channels snapshotted at job
    start so the terminal merge doesn't read a stale subs list
  * mergeFromCache moved off Main on both the refresh path AND the
    init-hydration path — 750-item flatMap+sort+regex no longer
    blocks the UI thread
  * VideoDetailViewModel dual loadedUrl bookkeeping collapsed to
    the UiState field only; the rejected-URL path also stamps
    loadedUrl so the gate reads coherently

Block A — auto-update authenticity:
  * AppUpdateClient pins the fdroid.sulkta.com leaf SPKI + the
    Let's Encrypt E7 intermediate via OkHttp CertificatePinner
  * file.name accepted only when matching ^/[A-Za-z0-9._-]+\.apk$
  * versionCode clamped to (0, 10_000_000] before we trust the
    'update available' notification — a hostile index can no longer
    pin us to MAX_VALUE

Block C — captureResumePosition perf:
  * ResumePositionsStore.record short-circuits when the existing
    entry matches position+duration so the 5s poll's
    before !== next guard actually skips the SP write
  * JSON encode + SP write off Main via globalScope IO

Block D — Rust feed.rs hardening:
  * Shared reqwest Client via OnceLock — 50 channels no longer
    pay 50 TLS handshakes
  * Response body capped at 2 MiB via bytes_stream — adversarial
    feeds can't OOM the JVM
  * parse_rss returns partial results on quick-xml errors instead
    of nuking everything already parsed
  * extract_channel_id widened (m./www./http(s)?/trailing path)
    and validates exact 24-char UC<22 base64-ish>
  * Skip entries with empty title/published
  * iso_to_relative future dates → 'just now' (clock skew
    no longer pins items to top)
  * civil_to_days year clamp 1970..=2200 before the i64 arithmetic
  * Redirect chain capped at 3
  * Dropped the broken lexicographic sort on upload_date_relative
  * Cap parsed entries at 50 per channel

MED batch:
  * ThumbnailProgressOverlay uses derivedStateOf so only rows
    whose specific entry changed recompose on the 5s positions tick
  * EnrichmentStore.put short-circuits on identical view+duration
    so re-enrich within TTL doesn't write SP
  * EnrichmentStore.load prunes TTL-expired entries on hydration
  * FeedRefreshWorker distinguishes transient (Result.retry) from
    parse (Result.success) failures
  * WorkManager interval coerceAtLeast(15L) on both schedulers
This commit is contained in:
Kayos 2026-05-26 20:53:25 -07:00
parent 796244e065
commit c960a1f424
11 changed files with 385 additions and 83 deletions

View file

@ -76,6 +76,17 @@ class EnrichmentStore(context: Context) {
)
val before = _entries.value
val next = _entries.updateAndGet { current ->
// Round-67 audit HIGH-4: short-circuit when the cached
// value is already the same view+duration — re-enriching
// within TTL otherwise allocates a new Map every call
// and the `before !== next` guard never triggers, so a
// refresh-after-refresh hammers the SP file.
val existing = current[videoId]
if (existing != null &&
existing.viewCount == entry.viewCount &&
existing.durationSeconds == entry.durationSeconds) {
return@updateAndGet current
}
val withEntry = current + (videoId to entry)
if (withEntry.size > MAX_ENRICHMENTS) {
withEntry.entries
@ -98,7 +109,14 @@ class EnrichmentStore(context: Context) {
private fun load(): Map<String, Enrichment> = runCatching {
val s = sp.getString(KEY, null) ?: return emptyMap()
json.decodeFromString<Map<String, Enrichment>>(s)
val loaded = json.decodeFromString<Map<String, Enrichment>>(s)
// Round-67 audit MED-6: prune TTL-expired entries on load
// so the store doesn't accumulate dead weight up to
// MAX_ENRICHMENTS over time. `Forever` TTL skips the prune.
val ttl = Settings.get().cacheTtl.value
if (ttl.isForever) return loaded
val cutoff = System.currentTimeMillis() - ttl.ms
loaded.filterValues { it.fetchedAt >= cutoff }
}.getOrDefault(emptyMap())
}

View file

@ -17,10 +17,13 @@ package com.sulkta.straw.data
import android.content.Context
import android.content.SharedPreferences
import com.sulkta.straw.StrawApp
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.updateAndGet
import kotlinx.coroutines.launch
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
@ -94,7 +97,27 @@ class ResumePositionsStore(context: Context) {
)
val before = _positions.value
val next = _positions.updateAndGet { current ->
// Round-67 audit HIGH-6: short-circuit value-equality —
// a 5s poll tick that finds the same (position, duration,
// wall-time) for an existing entry returns `current`
// unchanged so the outer `next !== before` guard
// actually short-circuits the SP write.
//
// lastWatchedAt updates every tick by definition, but
// ResumePosition equality on position+duration alone is
// ALL we care about for "did anything meaningful change."
// We re-stamp lastWatchedAt only when the player position
// actually advances.
val existing = current[videoId]
if (existing != null &&
existing.positionMs == entry.positionMs &&
existing.durationMs == entry.durationMs) {
return@updateAndGet current
}
val withEntry = current + (videoId to entry)
// Skip sort+associate when we're under the cap (the
// common case at default 500). Sort is O(n log n);
// associate allocates another map. Round-67 audit HIGH-6.
if (withEntry.size > maxResumes()) {
// Drop oldest by lastWatchedAt — newcomers always land
// because the entry we just added is by definition the
@ -108,7 +131,13 @@ class ResumePositionsStore(context: Context) {
}
}
if (next !== before) {
sp.edit().putString(KEY_POSITIONS, json.encodeToString(next)).apply()
// JSON encode + SP write off Main — encoding 100k entries
// would be ~50-100 ms on a low-end device, and the 5s
// captureResumePosition poll runs on Main. Round-67
// audit HIGH-6.
StrawApp.globalScope.launch(Dispatchers.IO) {
sp.edit().putString(KEY_POSITIONS, json.encodeToString(next)).apply()
}
}
}
@ -125,13 +154,20 @@ class ResumePositionsStore(context: Context) {
if (videoId !in current) current else current - videoId
}
if (next !== before) {
sp.edit().putString(KEY_POSITIONS, json.encodeToString(next)).apply()
StrawApp.globalScope.launch(Dispatchers.IO) {
sp.edit().putString(KEY_POSITIONS, json.encodeToString(next)).apply()
}
}
}
fun clearAll() {
val before = _positions.value
_positions.updateAndGet { emptyMap() }
sp.edit().putString(KEY_POSITIONS, json.encodeToString(emptyMap<String, ResumePosition>())).apply()
if (before.isNotEmpty()) {
StrawApp.globalScope.launch(Dispatchers.IO) {
sp.edit().putString(KEY_POSITIONS, json.encodeToString(emptyMap<String, ResumePosition>())).apply()
}
}
}
private fun load(): Map<String, ResumePosition> = runCatching {

View file

@ -109,8 +109,6 @@ class VideoDetailViewModel : ViewModel() {
private val _ui = MutableStateFlow(VideoDetailUiState())
val ui: StateFlow<VideoDetailUiState> = _ui.asStateFlow()
private var loadedUrl: String? = null
// Track the active load coroutine so a rapid tap to a different video
// cancels the prior fetch; otherwise a slow-to-finish older load
// overwrites the newer state and the player ends up streaming A while
@ -120,23 +118,31 @@ class VideoDetailViewModel : ViewModel() {
fun load(streamUrl: String) {
// viewModel() is activity-scoped, so the same VM is reused across
// navigations. Skip the refetch if the requested URL already has
// a resolved state.
if (loadedUrl == streamUrl && _ui.value.detail != null) return
// a resolved state. Snapshot _ui once so the two reads agree.
val snap = _ui.value
if (snap.loadedUrl == streamUrl && snap.detail != null) return
// Same YT-host gate as ChannelViewModel — covers the case
// where a tap on a poisoned related-card lands here.
// Round-5 audit MED-3. Round-6 audit HIGH-1: cancel any
// in-flight load on rejection too — otherwise the
// late-arriving prior-job's fence still PASSES (loadedUrl
// wasn't moved) and clobbers the "Unsupported URL" error
// banner.
// banner. round-67 audit HIGH-7: also set loadedUrl on this
// path so the gate reads coherently for any caller that
// checks _ui.value.loadedUrl on the rejected path.
if (!isAllowedYtUrl(streamUrl)) {
inFlight?.cancel()
inFlight = null
_ui.update { VideoDetailUiState(loading = false, error = "Unsupported URL") }
_ui.update {
VideoDetailUiState(
loading = false,
error = "Unsupported URL",
loadedUrl = streamUrl,
)
}
return
}
inFlight?.cancel()
loadedUrl = streamUrl
_ui.update { VideoDetailUiState(loading = true, loadedUrl = streamUrl) }
inFlight = viewModelScope.launch {
try {
@ -261,8 +267,10 @@ class VideoDetailViewModel : ViewModel() {
// loads: if a subsequent load(B) cancelled this one but
// we resolved past the suspension point, drop our
// result rather than clobber B's state. Round-4 audit
// HIGH-2.
if (loadedUrl != streamUrl) return@launch
// HIGH-2. Round-67 audit HIGH-7: single source of
// truth — read loadedUrl from _ui rather than a
// shadowing field.
if (_ui.value.loadedUrl != streamUrl) return@launch
_ui.update {
VideoDetailUiState(
loading = false,
@ -288,7 +296,7 @@ class VideoDetailViewModel : ViewModel() {
}
} catch (t: Throwable) {
if (t is CancellationException) throw t
if (loadedUrl != streamUrl) return@launch
if (_ui.value.loadedUrl != streamUrl) return@launch
_ui.update {
VideoDetailUiState(
loading = false,

View file

@ -29,8 +29,10 @@ object FeedRefreshScheduler {
wm.cancelUniqueWork(WORK_NAME)
return
}
// WorkManager 15-minute periodic floor — see UpdateScheduler.
// Round-67 audit MED-4.
val request = PeriodicWorkRequestBuilder<FeedRefreshWorker>(
s.bgFeedRefreshInterval.value.minutes,
s.bgFeedRefreshInterval.value.minutes.coerceAtLeast(15L),
TimeUnit.MINUTES,
).setConstraints(
Constraints.Builder()

View file

@ -27,6 +27,8 @@ import com.sulkta.straw.data.Settings
import com.sulkta.straw.data.Subscriptions
import com.sulkta.straw.feature.search.StreamItem
import com.sulkta.straw.util.strawLogI
import com.sulkta.straw.util.strawLogW
import java.io.IOException
class FeedRefreshWorker(
context: Context,
@ -41,9 +43,21 @@ class FeedRefreshWorker(
// One bulk call via the Rust subscriptionFeed fan-out. Returns
// a flat list; we group by uploaderUrl to rebuild the per-
// channel cache shape FeedCacheStore expects.
val flat = runCatching {
//
// Round-67 audit MED-5: distinguish transient failures
// (network down, timeout) from parse failures. The former
// wants Result.retry() so WorkManager re-attempts within the
// current window with exponential backoff; without this, a
// 30-second offline blip eats a full 6-hour refresh cycle.
val flat = try {
uniffi.strawcore.subscriptionFeed(subs.map { it.url })
}.getOrNull() ?: return Result.success()
} catch (e: IOException) {
strawLogW("FeedRefresh") { "transient network failure, retrying: ${e.message}" }
return Result.retry()
} catch (e: Throwable) {
strawLogW("FeedRefresh") { "non-transient failure, giving up this cycle: ${e.message}" }
return Result.success()
}
val now = System.currentTimeMillis()
val grouped: Map<String, FeedCacheEntry> = flat

View file

@ -91,12 +91,17 @@ class SubscriptionFeedViewModel : ViewModel() {
if (channels.isNotEmpty()) {
pruneCacheToSubs(channels)
val savedTs = saved.values.maxOfOrNull { it.fetchedAt } ?: 0L
// Compute the merge off-Main first (round-67 audit
// HIGH-1) — flatMap + regex + sort on hydration was
// running on Main and could add ~10-20 ms to cold
// start on a slow phone.
val hydrated = withContext(Dispatchers.Default) { mergeFromCache(channels) }
// _ui.update so a concurrent refresh()'s state write
// doesn't race with this copy. vc=37 round-3 audit
// HIGH-4. Only advance lastFetchedAt — never regress.
_ui.update {
it.copy(
items = mergeFromCache(channels),
items = hydrated,
lastFetchedAt = maxOf(it.lastFetchedAt, savedTs),
)
}
@ -134,6 +139,16 @@ class SubscriptionFeedViewModel : ViewModel() {
/** Live refresh job, so spam-tapping Refresh doesn't fan out racing fetches. */
private var inFlight: Job? = null
/**
* The background enrichment job runs on StrawApp.globalScope so it
* outlives the VM's viewModelScope but a refresh-cancel must
* still kill the *previous* enrichment so we don't pile up
* overlapping fan-outs (8-wide × N overlapping refreshes blows the
* concurrency budget). Tracked here, cancelled in the same places
* `inFlight` is. Round-67 audit HIGH-2/3/8.
*/
private var enrichJob: Job? = null
fun refreshIfStale() {
// Skip if a refresh is already in flight. vc=36 audit CRIT-R1:
// SubsPane's LaunchedEffect(subs) re-fires every time
@ -160,8 +175,11 @@ class SubscriptionFeedViewModel : ViewModel() {
// channelCache when the user unsubscribes from the last
// channel; we'd clear() then immediately repopulate with
// phantom entries when the prior fetchChannelInto resolved.
// vc=37 round-3 audit HIGH-3.
// vc=37 round-3 audit HIGH-3. Also kill any in-flight
// enrichment fan-out so we don't end up with N overlapping
// enrich jobs piling up under spam-refresh — round-67 HIGH-8.
inFlight?.cancel()
enrichJob?.cancel()
val channels = Subscriptions.get().subs.value
if (channels.isEmpty()) {
_ui.update { it.copy(loading = false, items = emptyList(), error = null) }
@ -193,7 +211,11 @@ class SubscriptionFeedViewModel : ViewModel() {
.awaitAll()
}
pruneCacheToSubs(channels)
val freshItems = mergeFromCache(channels)
// Move flatMap + per-item regex + sort off Main —
// viewModelScope.launch runs on Main by default and
// mergeFromCache is non-trivial on a 500-item merge.
// Round-67 audit HIGH-1.
val freshItems = withContext(Dispatchers.Default) { mergeFromCache(channels) }
_ui.update {
SubscriptionFeedUiState(
loading = false,
@ -205,7 +227,11 @@ class SubscriptionFeedViewModel : ViewModel() {
// viewCount=0 + durationSeconds=0; kick a bounded
// background job that calls enrichFeedItem for the
// top items and pumps a fresh _ui emit when done.
enrichVisibleItems(freshItems)
// Pass the channels snapshot so the enrich job's
// terminal mergeFromCache uses what was current at
// job start, not whatever the user's subs are by
// the time enrichment finishes ~2s later.
enrichVisibleItems(freshItems, channels)
// Persist what we just freshened. Off the main thread —
// JSON encode on 30 subs * 30 items is small but not
// free, and SharedPreferences.apply is async anyway.
@ -317,15 +343,19 @@ class SubscriptionFeedViewModel : ViewModel() {
* complete in ~2s. Skipped per-item when FeedEnrichment already
* has a fresh hit (TTL controlled by Settings.cacheTtl).
*
* Runs OFF viewModelScope so a refresh-cancel doesn't kill an
* enrichment that's almost done the background fill is for
* NEXT-open paint, no rush. Uses StrawApp.globalScope.
* Runs on viewModelScope (round-67 audit HIGH-2): outliving the VM
* would mean a destroyed _ui can still receive a stale emit (and
* mergeFromCache reads a now-cleared channelCache). The next
* VM instance does its own enrichment on next refresh; nothing
* is lost by not finishing the prior one. Tracked in enrichJob so
* refresh + clearInMemoryCache can cancel it.
*/
private fun enrichVisibleItems(items: List<StreamItem>) {
private fun enrichVisibleItems(items: List<StreamItem>, channelsSnapshot: List<ChannelRef>) {
val take = items.take(ENRICH_HEAD_COUNT)
.filter { it.viewCount <= 0L && it.durationSeconds <= 0L }
if (take.isEmpty()) return
com.sulkta.straw.StrawApp.globalScope.launch {
enrichJob?.cancel()
enrichJob = viewModelScope.launch {
val gate = Semaphore(ENRICH_PARALLELISM)
coroutineScope {
take.map { item ->
@ -348,11 +378,14 @@ class SubscriptionFeedViewModel : ViewModel() {
}
}.awaitAll()
}
// Pump a fresh emit so the UI picks up the overlay.
withContext(Dispatchers.Main) {
val channels = Subscriptions.get().subs.value
_ui.update { it.copy(items = mergeFromCache(channels)) }
// Compute the merge off-Main — flatMap + per-item regex
// + sort over up to 500 items is too much for the UI
// thread. Then hop to Main only for the StateFlow emit.
// Round-67 audit HIGH-1.
val merged = withContext(Dispatchers.Default) {
mergeFromCache(channelsSnapshot)
}
_ui.update { it.copy(items = merged) }
}
}
@ -385,8 +418,13 @@ class SubscriptionFeedViewModel : ViewModel() {
fun clearInMemoryCache() {
// Cancel any in-flight refresh — without this, fetchChannelInto
// coroutines mid-execution would re-populate the cache after
// the clear. Round-3 audit function MED-3.
// the clear. Round-3 audit function MED-3. Also cancel any
// enrichment fan-out (lives on globalScope, NOT viewModelScope)
// — otherwise a still-running enrichment would write to
// FeedEnrichment + then push a merged emit reading the empty
// channelCache. Round-67 audit HIGH-3.
inFlight?.cancel()
enrichJob?.cancel()
channelCache.clear()
// Use _ui.update for atomicity vs concurrent refresh writes
// (round-3 audit HIGH-4).

View file

@ -26,7 +26,9 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
@ -57,10 +59,20 @@ fun BoxScope.ThumbnailProgressOverlay(videoId: String?) {
// scroll jank (vc=67). The Lifecycle pause optimization doesn't
// matter for a foreground feed that's only collected while the
// composable is on screen anyway.
val positions by Resume.get().positions.collectAsState()
val entry = positions[videoId] ?: return
if (entry.durationMs <= 0L) return
val fraction = (entry.positionMs.toFloat() / entry.durationMs.toFloat())
//
// Round-67 audit MED-2: derivedStateOf isolates each row's
// dependency to ONLY its own videoId's entry. Without this, the
// 5s captureResumePosition tick re-emits the entire positions
// map → every visible thumbnail recomposes. With it, only rows
// whose specific entry changed recompose.
val positionsFlow = Resume.get().positions
val positions by positionsFlow.collectAsState()
val entry by remember(videoId) {
derivedStateOf { positions[videoId] }
}
val resolved = entry ?: return
if (resolved.durationMs <= 0L) return
val fraction = (resolved.positionMs.toFloat() / resolved.durationMs.toFloat())
.coerceIn(0f, 1f)
Box(
modifier = Modifier

View file

@ -16,17 +16,37 @@ package com.sulkta.straw.feature.update
import com.sulkta.straw.BuildConfig
import com.sulkta.straw.util.runCatchingCancellable
import com.sulkta.straw.util.strawLogW
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import okhttp3.CertificatePinner
import okhttp3.OkHttpClient
import okhttp3.Request
import java.util.concurrent.TimeUnit
private const val INDEX_HOST = "fdroid.sulkta.com"
private const val INDEX_URL = "https://fdroid.sulkta.com/fdroid/repo/index-v2.json"
private const val REPO_BASE = "https://fdroid.sulkta.com/fdroid/repo"
/**
* Only accept file names that look like a plain APK basename. The index
* controls a string we substitute into an `ACTION_VIEW` intent; without
* sanitization a hostile or compromised index could ship `..//host/x.apk`
* or worse. Round-67 audit HIGH-5.
*/
private val APK_NAME_RE = Regex("""^/[A-Za-z0-9._-]+\.apk$""")
/**
* Sanity-cap on parsed versionCode. Straw vc is currently low double
* digits; ten million is a horizon we won't hit organically but blocks
* a hostile index from latching us to Long.MAX_VALUE and burying every
* legitimate update behind a "you're already up to date" check.
* Round-67 audit HIGH-5.
*/
private const val MAX_PLAUSIBLE_VC = 10_000_000L
data class UpdateInfo(
val versionCode: Long,
val versionName: String,
@ -34,9 +54,31 @@ data class UpdateInfo(
)
object AppUpdateClient {
/**
* Pin two Subject-Public-Key-Info SHA-256 hashes against
* fdroid.sulkta.com so an off-tree CA misissue can't ship the
* user an attacker-signed index. Round-67 audit HIGH-5.
*
* - sha256/8ofd... current leaf SPKI. Rotates every ~90 days
* with each Let's Encrypt renewal; an app update before the
* next rotation refreshes this pin.
* - sha256/y7xV... Let's Encrypt E7 intermediate SPKI. Stable
* for years; serves as the rotation-safety pin while we push
* a new leaf hash.
*
* When the leaf pin no longer matches (post-rotation), OkHttp
* still accepts the chain because the E7 intermediate pin
* matches. The next app release rolls the leaf forward.
*/
private val pinner: CertificatePinner = CertificatePinner.Builder()
.add(INDEX_HOST, "sha256/8ofdiPS6TAiUx9zb2O7Qa9IKZQ3D2i+18teKCrz/MqA=")
.add(INDEX_HOST, "sha256/y7xVm0TVJNahMr2sZydE2jQH8SquXV9yLF9seROHHHU=")
.build()
private val http: OkHttpClient = OkHttpClient.Builder()
.connectTimeout(15, TimeUnit.SECONDS)
.readTimeout(15, TimeUnit.SECONDS)
.certificatePinner(pinner)
.build()
private val json = Json { ignoreUnknownKeys = true }
@ -58,10 +100,29 @@ object AppUpdateClient {
val best = pkg.versions.values
.maxByOrNull { it.manifest.versionCode }
?: return@runCatchingCancellable null
// Reject implausible versionCodes outright — see
// MAX_PLAUSIBLE_VC. Round-67 audit HIGH-5.
if (best.manifest.versionCode <= 0 ||
best.manifest.versionCode > MAX_PLAUSIBLE_VC) {
strawLogW("StrawUpdate") {
"rejecting implausible versionCode=${best.manifest.versionCode}"
}
return@runCatchingCancellable null
}
// Strict APK-basename match before we hand this off to
// ACTION_VIEW. Anything else gets logged + dropped.
// Round-67 audit HIGH-5.
val fileName = best.file.name
if (!APK_NAME_RE.matches(fileName)) {
strawLogW("StrawUpdate") {
"rejecting unsafe file.name=${fileName.take(80)}"
}
return@runCatchingCancellable null
}
UpdateInfo(
versionCode = best.manifest.versionCode,
versionName = best.manifest.versionName.orEmpty(),
apkUrl = "$REPO_BASE${best.file.name}",
apkUrl = "$REPO_BASE$fileName",
)
}.getOrNull()
}

View file

@ -34,8 +34,12 @@ object UpdateScheduler {
wm.cancelUniqueWork(WORK_NAME)
return
}
// WorkManager floors periodic intervals at 15 minutes.
// coerceAtLeast(15) future-proofs against a smaller enum case
// landing without anyone noticing the silent clamp. Round-67
// audit MED-4.
val request = PeriodicWorkRequestBuilder<UpdateCheckWorker>(
interval.minutes,
interval.minutes.coerceAtLeast(15L),
TimeUnit.MINUTES,
).setConstraints(
Constraints.Builder()