diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 42b67dc1e..6e09d9180 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -9,6 +9,53 @@ const val STRAW_SDK_TARGET = 35 // Sulkta fork — Straw // +// vc=91 / 0.1.0-CY — reliability + security fix sprint (straw audit 2026-07-04): +// * The in-app updater no longer self-bricks on a routine cert renewal. It +// used to pin the fdroid.sulkta.com leaf SPKI + the LE "E7" intermediate, +// but the leaf rotates every ~90-day renewal and LE rotates intermediates +// from a pool (they explicitly say don't pin them), so a normal renewal was +// GUARANTEED to miss both pins — silently killing the ONLY update path, +// with the fix reachable only through the channel the break just killed. +// Dropped pinning: default system-CA validation still blocks a MITM, and a +// swapped APK can't install over Straw anyway (the package installer +// verifies it against our signing key). Also: the check now distinguishes +// "couldn't reach the index" from "up to date" (no more "checked just now / +// up to date" while actually blind), added a 30s call timeout + a bounded +// index read, guarded the API-26 NotificationChannel (was a crash on 7.x), +// and setOnlyAlertOnce so a pending update doesn't re-nag every tick. +// * Android 13+ update/media notifications actually appear now: +// POST_NOTIFICATIONS was manifest-declared but never requested at runtime, +// so nm.notify() was a silent no-op on 13+. Requested once on cold start. +// * The published APK is no longer debuggable (isDebuggable=false on the +// shipped debug variant) — closes an ADB/USB `run-as` dump of watch/search +// history + the subscription list. No package-id cutover; updates in place. +// * Crash fixes: (a) opening a video whose channel repeats a videoId in its +// first ~20 entries hard-crashed Compose on a duplicate LazyColumn key — +// moreFromChannel + related now distinctBy url; (b) in-app back died +// permanently after one back-to-Home on Android 12+ (moveTaskToBack doesn't +// destroy the activity, so the disabled callback never re-enabled) — the +// callback is now driven by live nav depth instead of disable-and-redispatch; +// (c) the local-playlists store was uncapped, so a hostile NewPipe import +// could persist a huge blob that ANR'd every cold start — added per-store +// caps (playlists / items / string length) + off-main hydration like +// ResumePositionsStore; (d) a SponsorBlock fetch that resolves after you +// skip to another video no longer hijacks the now-playing item (staleness +// fence). +// * Cache-size caps that first-launch defaulted to "Unlimited" (100k rows, +// because nearest() was exact-match-or-Unlimited over a set the defaults +// weren't in) now snap up to the nearest finite cap. +// * The RYD + SponsorBlock FFI shims wrap the uniffi call, so a core panic +// (now catchable — see below) is swallowed to null/empty per their contract +// instead of crashing video-detail load. +// * Rust FFI wrapper: release profile is panic="unwind" (was "abort" — abort +// defeated UniFFI's catch_unwind + tokio's spawn_blocking JoinError capture, +// turning ANY panic in the extractor core into a whole-app SIGABRT), and +// every blocking extractor call is wrapped in a 60s wall-clock timeout so a +// wedged JS/HTTP path unblocks the caller instead of spinning forever and +// piling detached threads toward tokio's 512 blocking-pool cap. Pairs with +// the strawcore-core reliability fix (bounded rquickjs 64MiB/2MiB/5s + +// invalidate-on-failure player.js self-heal). +// // vc=90 / 0.1.0-CX — cold-start hydration + status-bar bleed fix + RYD FFI slim: // * ResumePositionsStore + EnrichmentStore no longer JSON-decode on the main // thread in Application.onCreate. Resume was the heaviest cold-start cost in @@ -312,6 +359,6 @@ const val STRAW_SDK_TARGET = 35 // vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via // strawcore-core (Sulkta-OSS/strawcore) via the UniFFI wrapper; no // NewPipeExtractor in the runtime path. -const val STRAW_VERSION_CODE = 90 -const val STRAW_VERSION_NAME = "0.1.0-CX" +const val STRAW_VERSION_CODE = 91 +const val STRAW_VERSION_NAME = "0.1.0-CY" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 2f1ce3924..a35ae1124 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -21,7 +21,14 @@ repository = "https://git.sulkta.com/Sulkta-OSS/straw" strip = true lto = "thin" codegen-units = 1 -panic = "abort" +# unwind (NOT abort): panic=abort defeats UniFFI's catch_unwind AND tokio's +# spawn_blocking JoinError capture, turning ANY panic in the extractor core +# (which chews weekly-rotating, attacker-influenced YouTube payloads) into an +# instant whole-app SIGABRT. unwind restores both safety nets so a core panic +# surfaces as a caught StrawcoreException instead of crashing the app. The +# unwind-table size cost is a rounding error against the 4-ABI jniLibs payload. +# (straw + FFI-wrapper audits, 2026-07-04.) +panic = "unwind" opt-level = "z" # `url` crate for video-id extraction in stream.rs. diff --git a/rust/strawcore/src/channel.rs b/rust/strawcore/src/channel.rs index 6f9554f66..71cf43610 100644 --- a/rust/strawcore/src/channel.rs +++ b/rust/strawcore/src/channel.rs @@ -33,11 +33,8 @@ pub async fn channel_info(input: String) -> Result log::info!("strawcore::channel_info input_len={}", input.len()); crate::runtime::ensure_initialized(); let identifier = resolve_channel_identifier(&input)?; - let core = tokio::task::spawn_blocking(move || core_channel_info(identifier)) - .await - .map_err(|e| StrawcoreError::Extractor { - msg: format!("join: {e}"), - })??; + let core = crate::runtime::run_extract("channel_info", move || core_channel_info(identifier)) + .await?; Ok(map_channel(core)) } @@ -63,11 +60,10 @@ pub async fn channel_videos_continuation(token: String) -> Result(what: &'static str, f: F) -> Result +where + F: FnOnce() -> Result + Send + 'static, + T: Send + 'static, + E: Send + 'static, + StrawcoreError: From, +{ + match tokio::time::timeout(EXTRACT_TIMEOUT, tokio::task::spawn_blocking(f)).await { + // Timed out waiting on the blocking task. + Err(_elapsed) => Err(StrawcoreError::Extractor { + msg: format!("timeout: {what} exceeded {}s", EXTRACT_TIMEOUT.as_secs()), + }), + // Blocking task finished within the deadline but the join failed — + // panic (only under a non-abort profile) or cancellation. + Ok(Err(join)) => Err(StrawcoreError::Extractor { + msg: format!("join: {join}"), + }), + // Blocking task returned; propagate its own error via the existing + // `From` mapping. + Ok(Ok(inner)) => inner.map_err(StrawcoreError::from), + } +} diff --git a/rust/strawcore/src/search.rs b/rust/strawcore/src/search.rs index 308bacdda..726019a96 100644 --- a/rust/strawcore/src/search.rs +++ b/rust/strawcore/src/search.rs @@ -84,13 +84,10 @@ pub async fn search(query: String) -> Result { // the hot entry points. Now every extractor entry re-asserts // — cheap when INITIALIZED is true (single Acquire load). crate::runtime::ensure_initialized(); - let result = tokio::task::spawn_blocking(move || { + let result = crate::runtime::run_extract("search", move || { search_extractor::search(&query, SearchFilter::Videos) }) - .await - .map_err(|e| StrawcoreError::Extractor { - msg: format!("join: {e}"), - })??; + .await?; // Page 1 carries the first continuation token (None once YT stops // handing them out). The Kotlin SearchViewModel passes it back to // `search_continuation` on scroll. @@ -106,10 +103,9 @@ pub async fn search(query: String) -> Result { pub async fn search_continuation(token: String) -> Result { log::info!("strawcore::search_continuation token_len={}", token.len()); crate::runtime::ensure_initialized(); - let page = tokio::task::spawn_blocking(move || search_extractor::search_continuation(&token)) - .await - .map_err(|e| StrawcoreError::Extractor { - msg: format!("join: {e}"), - })??; + let page = crate::runtime::run_extract("search_continuation", move || { + search_extractor::search_continuation(&token) + }) + .await?; Ok(page_from_core(page)) } diff --git a/rust/strawcore/src/stream.rs b/rust/strawcore/src/stream.rs index e95c313ea..09eb19cfd 100644 --- a/rust/strawcore/src/stream.rs +++ b/rust/strawcore/src/stream.rs @@ -78,11 +78,10 @@ pub async fn stream_info(input: String) -> Result { crate::runtime::ensure_initialized(); let video_id = resolve_video_id(&input)?; let video_id_for_call = video_id.clone(); - let core = tokio::task::spawn_blocking(move || core_stream_info(&video_id_for_call)) - .await - .map_err(|e| StrawcoreError::Extractor { - msg: format!("join: {e}"), - })??; + let core = crate::runtime::run_extract("stream_info", move || { + core_stream_info(&video_id_for_call) + }) + .await?; Ok(map_stream_info(video_id, core)) } @@ -95,11 +94,10 @@ pub async fn stream_metadata(input: String) -> Result<(i64, i64), StrawcoreError log::info!("strawcore::stream_metadata input_len={}", input.len()); crate::runtime::ensure_initialized(); let video_id = resolve_video_id(&input)?; - let core = tokio::task::spawn_blocking(move || core_stream_metadata(&video_id)) - .await - .map_err(|e| StrawcoreError::Extractor { - msg: format!("join: {e}"), - })??; + let core = crate::runtime::run_extract("stream_metadata", move || { + core_stream_metadata(&video_id) + }) + .await?; Ok((clamp_nonneg(core.view_count), core.duration_seconds.max(0))) } diff --git a/strawApp/build.gradle.kts b/strawApp/build.gradle.kts index bf1bdbed2..ed3b5df02 100644 --- a/strawApp/build.gradle.kts +++ b/strawApp/build.gradle.kts @@ -66,7 +66,15 @@ configure { // strawApp/proguard-rules.pro cover UniFFI + JNA + // kotlinx-serialization companions. debug { - isDebuggable = true + // NOT debuggable, even though this is the `debug` buildType: the + // debug variant is what we PUBLISH to fdroid.sulkta.com, and a + // shipped debuggable APK lets any ADB/USB attacker `run-as` the + // package and dump watch/search history + the full subscription + // list — defeating allowBackup=false / dataExtractionRules on a + // privacy-focused client. Flipping this needs no package-id + // cutover: signing + the `.debug` suffix below are unchanged, so + // in-place fdroid updates keep working. + isDebuggable = false // Keep the `.debug` applicationId suffix (package stays // com.sulkta.straw.debug) so in-place fdroid updates + the in-app // auto-updater keep working and nobody loses their subs/history. diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt b/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt index fbc39d27c..d69e2b918 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt @@ -5,12 +5,16 @@ package com.sulkta.straw +import android.Manifest import android.content.Intent +import android.content.pm.PackageManager +import android.os.Build import android.os.Bundle import androidx.activity.ComponentActivity import androidx.activity.OnBackPressedCallback import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge +import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.isSystemInDarkTheme import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize @@ -20,6 +24,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -27,6 +32,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.core.content.ContextCompat import androidx.media3.common.util.UnstableApi import com.sulkta.straw.data.Settings import com.sulkta.straw.data.ThemeMode @@ -62,10 +68,23 @@ class StrawActivity : ComponentActivity() { */ private val pendingDeepLink = MutableStateFlow(null) + /** + * Android 13+ gates every `notify()` (media controls + the + * update-available notice) behind a runtime POST_NOTIFICATIONS grant. + * Registered here (before the activity is STARTED, as the API requires) + * and launched once on cold start. The result is intentionally ignored: + * Android won't re-prompt after a permanent choice, and both playback + * and the update surface degrade gracefully without it (the passive + * Settings "Update available" text still shows). + */ + private val notificationPermissionLauncher = + registerForActivityResult(ActivityResultContracts.RequestPermission()) { } + @OptIn(UnstableApi::class) override fun onCreate(savedInstanceState: Bundle?) { enableEdgeToEdge() super.onCreate(savedInstanceState) + maybeRequestNotificationPermission() val startUrl = pickYouTubeUrl(intent) @@ -100,25 +119,39 @@ class StrawActivity : ComponentActivity() { expanded = true } - DisposableEffect(nav) { - val cb = object : OnBackPressedCallback(true) { + // Intercept back ONLY when there's an in-app action to take: + // collapse an expanded player, or pop the browse stack. At + // the browse root with nothing open the callback is disabled + // so the system handles back natively (moveTaskToBack on + // A12+). The previous version disabled the callback and + // re-dispatched to the system at root — but on A12+ + // moveTaskToBack does NOT destroy the activity, so the + // composition (and the now-disabled callback) survived and + // DisposableEffect(nav) never re-ran to re-enable it, killing + // all in-app back permanently after one back-to-Home. + // Driving isEnabled from live nav/expanded state fixes that. + val canGoBack = expanded || nav.stack.size > 1 + val backCallback = remember { + object : OnBackPressedCallback(canGoBack) { override fun handleOnBackPressed() { - // Back order: the true-fullscreen Player pops - // first; an expanded player collapses to the - // minibar; otherwise pop the browse stack (or - // exit at root). + // The true-fullscreen Player is a nav screen, so + // it pops via the stack branch; an expanded + // (non-fullscreen) player collapses first. if (nav.current !is Screen.Player && expanded) { expanded = false - return - } - if (!nav.pop()) { - isEnabled = false - this@StrawActivity.onBackPressedDispatcher.onBackPressed() + } else { + // Guaranteed poppable: enabled only when + // expanded || stack.size > 1, and the + // expanded case is handled above. + nav.pop() } } } - onBackPressedDispatcher.addCallback(cb) - onDispose { cb.remove() } + } + SideEffect { backCallback.isEnabled = canGoBack } + DisposableEffect(onBackPressedDispatcher) { + onBackPressedDispatcher.addCallback(backCallback) + onDispose { backCallback.remove() } } // Open the deep-linked video into the expandable player on @@ -173,6 +206,18 @@ class StrawActivity : ComponentActivity() { pickYouTubeUrl(intent)?.let { pendingDeepLink.value = it } } + /** Ask for POST_NOTIFICATIONS on API 33+ if it isn't already granted. */ + private fun maybeRequestNotificationPermission() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) return + val granted = ContextCompat.checkSelfPermission( + this, + Manifest.permission.POST_NOTIFICATIONS, + ) == PackageManager.PERMISSION_GRANTED + if (!granted) { + notificationPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS) + } + } + @Composable private fun ScreenContent( nav: Navigator, diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt index 22b381868..05fd20840 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/PlaylistsStore.kt @@ -20,6 +20,7 @@ import android.content.SharedPreferences import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.flow.updateAndGet import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json @@ -45,6 +46,24 @@ data class Playlist( private const val PREFS = "straw_playlists" private const val KEY = "playlists_v1" +// Caps so a hostile/oversized NewPipe import (SettingsImport permitted up to +// 256 playlists × 5000 items with no length bound) can't persist a +// hundreds-of-MB blob that then re-decodes on every cold start → ANR/OOM +// crash-loop recoverable only by clearing app data. Realistic use is a +// handful of playlists with tens–hundreds of short-string items, so these +// bound the pathological case while leaving normal use untouched. +private const val MAX_PLAYLISTS = 100 +private const val MAX_ITEMS_PER_PLAYLIST = 1000 +private const val MAX_NAME_LEN = 100 +private const val MAX_TITLE_LEN = 300 +private const val MAX_URL_LEN = 512 +private const val MAX_UPLOADER_LEN = 100 + +// Refuse to even decode a persisted blob larger than this (chars). A capped +// legitimate export is a few MB at most; anything past this is corrupt or a +// pre-fix poison blob, and we'd rather start empty than OOM decoding it. +private const val MAX_BLOB_CHARS = 8_000_000 + class PlaylistsStore(context: Context) { private val sp: SharedPreferences = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } @@ -53,17 +72,43 @@ class PlaylistsStore(context: Context) { // list so disk converges to the latest in-memory state. private val writer = PrefsWriter(sp) - private val _playlists = MutableStateFlow(load()) + // Seed empty + hydrate OFF the main thread (mirrors ResumePositionsStore). + // The old MutableStateFlow(load()) decoded the whole playlists blob + // synchronously in the constructor, which runs in StrawApp.onCreate on + // Main — a large (even capped) blob blocked cold start. The hydrate shares + // the PrefsWriter FIFO dispatcher (enqueued before this store is published) + // so it always runs before any write's persist, and mutations that arrive + // before it finishes defer themselves (see the `hydrated` guard below). + private val _playlists = MutableStateFlow>(emptyList()) val playlists: StateFlow> = _playlists.asStateFlow() + @Volatile private var hydrated = false + + init { + writer.serial { + try { + val loaded = load() + if (loaded.isNotEmpty()) { + _playlists.update { live -> + // Deferral keeps `live` empty at hydrate time, so this + // is normally just `loaded`; merge-by-id (live wins) is + // the belt-and-suspenders path if a create slipped in. + if (live.isEmpty()) loaded else capPlaylists(mergeById(loaded, live)) + } + } + } finally { + hydrated = true + } + } + } + fun create(name: String): Playlist { val pl = Playlist( id = UUID.randomUUID().toString(), - name = name.trim().ifBlank { "Untitled" }, + name = clampName(name).ifBlank { "Untitled" }, createdAt = System.currentTimeMillis(), ) - _playlists.updateAndGet { it + pl } - persist() + insert(pl) return pl } @@ -76,32 +121,42 @@ class PlaylistsStore(context: Context) { */ fun importPlaylist(name: String, items: List): Playlist { val stampNow = System.currentTimeMillis() - // Dedup within the import + stamp addedAt once. + // Dedup within the import, clamp string lengths, stamp addedAt once, + // and stop at the per-playlist item cap. val seen = HashSet() - val deduped = ArrayList(items.size) - for (it in items) { - if (it.streamUrl.isBlank()) continue - if (!seen.add(it.streamUrl)) continue - deduped.add(it.copy(addedAt = if (it.addedAt == 0L) stampNow else it.addedAt)) + val deduped = ArrayList(minOf(items.size, MAX_ITEMS_PER_PLAYLIST)) + for (raw in items) { + if (raw.streamUrl.isBlank()) continue + val clamped = clampItem(raw).let { if (it.addedAt == 0L) it.copy(addedAt = stampNow) else it } + if (!seen.add(clamped.streamUrl)) continue + deduped.add(clamped) + if (deduped.size >= MAX_ITEMS_PER_PLAYLIST) break } val pl = Playlist( id = UUID.randomUUID().toString(), - name = name.trim().ifBlank { "Untitled" }, + name = clampName(name).ifBlank { "Untitled" }, createdAt = stampNow, items = deduped, ) - _playlists.updateAndGet { it + pl } - persist() + insert(pl) return pl } + private fun insert(pl: Playlist) { + if (!hydrated) { writer.serial { insert(pl) }; return } + _playlists.updateAndGet { capPlaylists(it + pl) } + persist() + } + fun delete(id: String) { + if (!hydrated) { writer.serial { delete(id) }; return } _playlists.updateAndGet { cur -> cur.filterNot { it.id == id } } persist() } fun rename(id: String, newName: String) { - val trimmed = newName.trim().ifBlank { return } + val trimmed = clampName(newName).ifBlank { return } + if (!hydrated) { writer.serial { rename(id, newName) }; return } _playlists.updateAndGet { cur -> cur.map { if (it.id == id) it.copy(name = trimmed) else it } } @@ -109,11 +164,14 @@ class PlaylistsStore(context: Context) { } fun addItem(playlistId: String, item: PlaylistItem) { - val stamped = item.copy(addedAt = System.currentTimeMillis()) + val stamped = clampItem(item).copy(addedAt = System.currentTimeMillis()) + if (stamped.streamUrl.isBlank()) return + if (!hydrated) { writer.serial { addItem(playlistId, item) }; return } _playlists.updateAndGet { cur -> cur.map { pl -> if (pl.id != playlistId) pl else if (pl.items.any { it.streamUrl == stamped.streamUrl }) pl + else if (pl.items.size >= MAX_ITEMS_PER_PLAYLIST) pl else pl.copy(items = pl.items + stamped) } } @@ -121,6 +179,7 @@ class PlaylistsStore(context: Context) { } fun removeItem(playlistId: String, streamUrl: String) { + if (!hydrated) { writer.serial { removeItem(playlistId, streamUrl) }; return } _playlists.updateAndGet { cur -> cur.map { pl -> if (pl.id != playlistId) pl @@ -136,9 +195,37 @@ class PlaylistsStore(context: Context) { writer.write { putString(KEY, json.encodeToString(_playlists.value)) } } + private fun clampName(s: String): String = s.trim().take(MAX_NAME_LEN) + + private fun clampItem(item: PlaylistItem): PlaylistItem = item.copy( + streamUrl = item.streamUrl.take(MAX_URL_LEN), + title = item.title.take(MAX_TITLE_LEN), + thumbnail = item.thumbnail?.take(MAX_URL_LEN), + uploader = item.uploader.take(MAX_UPLOADER_LEN), + ) + + /** Keep the newest MAX_PLAYLISTS (new ones are appended). */ + private fun capPlaylists(list: List): List = + if (list.size <= MAX_PLAYLISTS) list else list.takeLast(MAX_PLAYLISTS) + + /** live entries win on id collision; loaded's remainder is appended. */ + private fun mergeById(loaded: List, live: List): List { + val liveIds = live.mapTo(HashSet()) { it.id } + return live + loaded.filterNot { it.id in liveIds } + } + private fun load(): List = runCatching { val s = sp.getString(KEY, null) ?: return emptyList() + // Don't decode an implausibly huge (corrupt/hostile/legacy-poison) blob. + if (s.length > MAX_BLOB_CHARS) return emptyList() json.decodeFromString>(s) + .takeLast(MAX_PLAYLISTS) + .map { pl -> + pl.copy( + name = clampName(pl.name), + items = pl.items.take(MAX_ITEMS_PER_PLAYLIST).map { clampItem(it) }, + ) + } }.getOrDefault(emptyList()) } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt index 86b3643f6..9fe25bee0 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SettingsStore.kt @@ -83,8 +83,19 @@ enum class CacheCap(val label: String, val value: Int) { Unlimited("Unlimited", Int.MAX_VALUE); companion object { + /** + * Resolve a stored/raw cap value to an enum. Snaps UP to the + * smallest offered cap that is >= target, so a value that isn't an + * exact enum member (a legacy default like 20/30/500, or a stored + * value from a build with different granularity) resolves to a real + * FINITE cap that covers it — not silently to Unlimited. Only a + * target larger than every finite cap resolves to Unlimited (which + * is exactly what a stored Int.MAX_VALUE "Unlimited" selection is). + * The prior exact-match-or-Unlimited version turned every non-enum + * default into an effective no-cap (100k rows) on fresh installs. + */ fun nearest(target: Int): CacheCap = - entries.firstOrNull { it.value == target } ?: Unlimited + entries.filter { it.value >= target }.minByOrNull { it.value } ?: Unlimited } } @@ -277,26 +288,29 @@ class SettingsStore(context: Context) { * takes effect immediately (next write trims to the new cap; reads * are unbounded since they're already in memory). * - * Defaults match the earlier hardcoded constants so first-launch - * behavior is unchanged from prior versions. + * Defaults are real enum values (not the old 20/30/500 non-enum + * literals, which — because nearest() used to be exact-match-or-Unlimited + * — silently resolved to Unlimited on every fresh install and made the + * effective cap the hard ceiling). nearest() now snaps up, so even a + * legacy stored non-enum value lands on a finite cap. */ private val _historyWatchesCap = MutableStateFlow( - CacheCap.nearest(sp.getInt(KEY_CACHE_HISTORY_WATCHES, 50)), + CacheCap.nearest(sp.getInt(KEY_CACHE_HISTORY_WATCHES, CacheCap.Tiny.value)), ) val historyWatchesCap: StateFlow = _historyWatchesCap.asStateFlow() private val _historySearchesCap = MutableStateFlow( - loadCap(KEY_CACHE_HISTORY_SEARCHES, default = 20), + loadCap(KEY_CACHE_HISTORY_SEARCHES, default = CacheCap.Tiny.value), ) val historySearchesCap: StateFlow = _historySearchesCap.asStateFlow() private val _resumePositionsCap = MutableStateFlow( - loadCap(KEY_CACHE_RESUME_POSITIONS, default = 500), + loadCap(KEY_CACHE_RESUME_POSITIONS, default = CacheCap.Medium.value), ) val resumePositionsCap: StateFlow = _resumePositionsCap.asStateFlow() private val _searchCacheCap = MutableStateFlow( - loadCap(KEY_CACHE_SEARCH, default = 30), + loadCap(KEY_CACHE_SEARCH, default = CacheCap.Tiny.value), ) val searchCacheCap: StateFlow = _searchCacheCap.asStateFlow() diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/dataimport/SettingsImport.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/dataimport/SettingsImport.kt index 979c4dcbc..39657b91e 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/dataimport/SettingsImport.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/dataimport/SettingsImport.kt @@ -321,8 +321,11 @@ object SettingsImport { openDb(dbFile).use { db -> val playlistRows = mutableListOf>() // Hard caps so a malicious export with millions of rows - // doesn't walk an unbounded cursor into memory. - db.rawQuery("SELECT uid, name FROM playlists LIMIT 256", null).use { c -> + // doesn't walk an unbounded cursor into memory. Matched to + // PlaylistsStore's MAX_PLAYLISTS/MAX_ITEMS_PER_PLAYLIST (the store + // is the real enforcement point; this just avoids materializing + // rows it would drop anyway). + db.rawQuery("SELECT uid, name FROM playlists LIMIT 100", null).use { c -> while (c.moveToNext()) { val uid = c.getLong(0) val name = c.getString(1) ?: "Untitled" @@ -338,7 +341,7 @@ object SettingsImport { JOIN streams s ON s.uid = j.stream_id WHERE j.playlist_id = ? ORDER BY j.join_index - LIMIT 5000 + LIMIT 1000 """.trimIndent(), arrayOf(uid.toString()), ).use { c -> 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 dbc8d8e5b..cb67041c4 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 @@ -197,18 +197,24 @@ class VideoDetailViewModel : ViewModel() { rydDeferred.await() to sbDeferred.await() } - val related = info.related.map { r -> - StreamItem( - url = r.url, - title = r.title.ifBlank { "(no title)" }, - uploader = r.uploader, - uploaderUrl = r.uploaderUrl, - thumbnail = r.thumbnail, - durationSeconds = r.durationSeconds, - viewCount = r.viewCount, - uploadDateRelative = r.uploadDateRelative, - ) - } + // Dedup by url: the related list is keyed "rel:"+url in the + // LazyColumn, so a duplicate would hard-crash Compose. Empty + // today (core returns no related), but arm the path now so it + // can't regress the moment core starts populating it. + val related = info.related + .distinctBy { it.url } + .map { r -> + StreamItem( + url = r.url, + title = r.title.ifBlank { "(no title)" }, + uploader = r.uploader, + uploaderUrl = r.uploaderUrl, + thumbnail = r.thumbnail, + durationSeconds = r.durationSeconds, + viewCount = r.viewCount, + uploadDateRelative = r.uploadDateRelative, + ) + } // More from this channel via strawcore.channelInfo — one // Rust round-trip returns the channel's Videos tab pre-mapped. @@ -262,6 +268,13 @@ class VideoDetailViewModel : ViewModel() { subscriberCount = ch.subscriberCount, videos = ch.videos .filter { it.url != streamUrl } + // A repeated videoId within the first 20 + // channel-tab entries would produce a + // duplicate LazyColumn key ("mfc:"+url) and + // hard-crash Compose on opening the video. + // Core doesn't dedup channel videos, so gate + // it here (ChannelViewModel does the same). + .distinctBy { it.url } .take(20) .map { v -> StreamItem( diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt index a1f8bb619..be7b6e5c9 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt @@ -75,6 +75,26 @@ object NowPlaying { } } + /** + * Attach richer data (e.g. late-arriving SponsorBlock segments) to the + * currently-playing item ONLY if it is still `item`'s streamUrl. Unlike + * [claim], this NEVER replaces a *different* current — so an async fetch + * that resolves after the user skipped to another video can't hijack + * NowPlaying back to the old one (which would apply stale segments, + * flip the collapsed-follow video, and pollute the resume store). No-op + * if playback has moved on. Race-free: only ever CAS-swaps a same-url + * item, so a concurrent writer flipping `current` to a new url makes the + * next loop's guard bail instead of clobbering it. + */ + fun refreshIfCurrent(item: NowPlayingItem) { + while (true) { + val cur = _current.value ?: return + if (cur.streamUrl != item.streamUrl) return + if (cur == item) return + if (_current.compareAndSet(cur, item)) return + } + } + fun clear() { _current.value = null } 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 8bd570751..3df7e4ff1 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 @@ -258,7 +258,12 @@ class PlaybackService : MediaSessionService() { SponsorBlockClient.fetch(videoId, cats) } if (segments.isNotEmpty()) { - NowPlaying.claim(item.copy(segments = segments)) + // Staleness fence: this SB fetch is a network RTT during + // which the player may have skipped to another track. + // refreshIfCurrent only attaches the segments if `item` is + // STILL the playing video — it will not hijack NowPlaying + // back to this (now-previous) one the way claim() would. + NowPlaying.refreshIfCurrent(item.copy(segments = segments)) } } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/AppUpdateClient.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/AppUpdateClient.kt index 2800500cf..06160d238 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/AppUpdateClient.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/AppUpdateClient.kt @@ -21,12 +21,10 @@ 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" @@ -46,38 +44,56 @@ private val APK_NAME_RE = Regex("""^/[A-Za-z0-9._-]+\.apk$""") */ private const val MAX_PLAUSIBLE_VC = 10_000_000L +/** + * Hard cap on the index body we'll buffer. The straw repo's index-v2.json + * is a few KB; 8 MiB is unreachable organically but stops a ballooned or + * hostile body from OOMing the update check. + */ +private const val MAX_INDEX_BYTES = 8L * 1024 * 1024 + data class UpdateInfo( val versionCode: Long, val versionName: String, val apkUrl: String, ) -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. - * - * - 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() +/** + * Outcome of an index check. Distinguishes "couldn't reach/parse the index" + * (indeterminate — must NOT be reported as up-to-date) from "reached it and + * there is genuinely nothing newer". + */ +sealed interface UpdateResult { + /** Reached + parsed the highest published entry for this package. */ + data class Found(val info: UpdateInfo) : UpdateResult + /** Reached + parsed, but no usable published entry exists. */ + object None : UpdateResult + + /** Could not reach or parse the index — retry without lying "up to date". */ + object Failed : UpdateResult +} + +object AppUpdateClient { + // No certificate pinning — deliberately. We used to pin the + // fdroid.sulkta.com leaf SPKI plus the Let's Encrypt "E7" intermediate, + // but that inverted the risk: the leaf rotates on every ~90-day certbot + // renewal and LE rotates issuing intermediates from a pool (and + // explicitly documents "don't pin our intermediates"), so a routine + // renewal was guaranteed to miss both pins — silently bricking the ONLY + // in-app update path, with the fix reachable only THROUGH the channel the + // break just killed. The threat pinning defended (a CA misissue serving a + // forged index) is already backstopped: a swapped APK cannot install over + // Straw because the Android package installer verifies it against our + // signing key (bb9ca96b…), and the index only names an APK URL. Default + // system-CA validation still blocks a plain MITM. So we drop the + // self-brick and keep the real integrity control (the signature gate). private val http: OkHttpClient = OkHttpClient.Builder() .connectTimeout(15, TimeUnit.SECONDS) .readTimeout(15, TimeUnit.SECONDS) - .certificatePinner(pinner) + // Absolute ceiling on the whole call (connect + write + read + + // redirects) so a slow-drip server can't keep the request — and the + // WorkManager job behind it — alive indefinitely. + .callTimeout(30, TimeUnit.SECONDS) .build() private val json = Json { ignoreUnknownKeys = true } @@ -86,19 +102,27 @@ object AppUpdateClient { * for THIS app's package. Returns null on network/parse failure (the * caller treats null as "no update available, try again later"). */ - suspend fun fetchLatest(): UpdateInfo? = withContext(Dispatchers.IO) { + suspend fun fetchLatest(): UpdateResult = withContext(Dispatchers.IO) { runCatchingCancellable { val req = Request.Builder().url(INDEX_URL).build() val raw = http.newCall(req).execute().use { resp -> - if (!resp.isSuccessful) return@runCatchingCancellable null + if (!resp.isSuccessful) return@runCatchingCancellable UpdateResult.Failed + // Bounded read: request() buffers up to the cap without + // consuming it, so a within-cap body still string()s in full, + // but an attacker-/misconfig-ballooned index is refused + // instead of OOMing the process. + if (resp.body.source().request(MAX_INDEX_BYTES + 1)) { + strawLogW("StrawUpdate") { "index exceeds $MAX_INDEX_BYTES-byte cap — refusing" } + return@runCatchingCancellable UpdateResult.Failed + } resp.body.string() } val index = json.decodeFromString(raw) val pkg = index.packages[BuildConfig.APPLICATION_ID] - ?: return@runCatchingCancellable null + ?: return@runCatchingCancellable UpdateResult.None val best = pkg.versions.values .maxByOrNull { it.manifest.versionCode } - ?: return@runCatchingCancellable null + ?: return@runCatchingCancellable UpdateResult.None // Reject implausible versionCodes outright — see // MAX_PLAUSIBLE_VC. if (best.manifest.versionCode <= 0 || @@ -106,7 +130,7 @@ object AppUpdateClient { strawLogW("StrawUpdate") { "rejecting implausible versionCode=${best.manifest.versionCode}" } - return@runCatchingCancellable null + return@runCatchingCancellable UpdateResult.None } // Strict APK-basename match before we hand this off to // ACTION_VIEW. Anything else gets logged + dropped. @@ -115,14 +139,16 @@ object AppUpdateClient { strawLogW("StrawUpdate") { "rejecting unsafe file.name=${fileName.take(80)}" } - return@runCatchingCancellable null + return@runCatchingCancellable UpdateResult.None } - UpdateInfo( - versionCode = best.manifest.versionCode, - versionName = best.manifest.versionName.orEmpty(), - apkUrl = "$REPO_BASE$fileName", + UpdateResult.Found( + UpdateInfo( + versionCode = best.manifest.versionCode, + versionName = best.manifest.versionName.orEmpty(), + apkUrl = "$REPO_BASE$fileName", + ) ) - }.getOrNull() + }.getOrElse { UpdateResult.Failed } } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/UpdateCheckWorker.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/UpdateCheckWorker.kt index 758fdabdc..55ab485fc 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/UpdateCheckWorker.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/update/UpdateCheckWorker.kt @@ -24,6 +24,7 @@ import android.app.PendingIntent import android.content.Context import android.content.Intent import android.net.Uri +import android.os.Build import androidx.core.app.NotificationCompat import androidx.work.CoroutineWorker import androidx.work.WorkerParameters @@ -36,23 +37,37 @@ import com.sulkta.straw.util.strawLogI * Touched by both the scheduled worker AND the "Check now" Settings * button so behavior stays identical regardless of trigger. */ -suspend fun runUpdateCheck(context: Context): UpdateInfo? { - val info = AppUpdateClient.fetchLatest() - Settings.get().setLastUpdateCheck(System.currentTimeMillis()) - if (info == null) { - strawLogI("update", "check: network/parse failure, will retry") - return null +suspend fun runUpdateCheck(context: Context): UpdateInfo? = + when (val result = AppUpdateClient.fetchLatest()) { + // Indeterminate — do NOT advance the last-check timestamp, or a + // permanently-failing checker would read "checked just now / up to + // date" while it's actually blind. Retry on the next tick. + is UpdateResult.Failed -> { + strawLogI("update", "check: network/parse failure, will retry (timestamp not advanced)") + null + } + // Reached the repo; it has nothing usable for this package. + is UpdateResult.None -> { + Settings.get().setLastUpdateCheck(System.currentTimeMillis()) + Settings.get().setLatestKnownVersion(0L, "") + strawLogI("update", "check: no published build for this package") + null + } + is UpdateResult.Found -> { + val info = result.info + Settings.get().setLastUpdateCheck(System.currentTimeMillis()) + if (info.versionCode <= BuildConfig.VERSION_CODE) { + strawLogI("update", "check: up to date (latest=${info.versionCode})") + Settings.get().setLatestKnownVersion(0L, "") + null + } else { + strawLogI("update", "check: ${BuildConfig.VERSION_CODE} → ${info.versionCode} available") + Settings.get().setLatestKnownVersion(info.versionCode, info.versionName) + postUpdateNotification(context, info) + info + } + } } - if (info.versionCode <= BuildConfig.VERSION_CODE) { - strawLogI("update", "check: up to date (latest=${info.versionCode})") - Settings.get().setLatestKnownVersion(0L, "") - return null - } - strawLogI("update", "check: ${BuildConfig.VERSION_CODE} → ${info.versionCode} available") - Settings.get().setLatestKnownVersion(info.versionCode, info.versionName) - postUpdateNotification(context, info) - return info -} class UpdateCheckWorker( context: Context, @@ -74,14 +89,19 @@ private const val NOTIF_ID = 23 private fun postUpdateNotification(context: Context, info: UpdateInfo) { val nm = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager - val channel = NotificationChannel( - NOTIF_CHANNEL_ID, - "Straw updates", - NotificationManager.IMPORTANCE_DEFAULT, - ).apply { - description = "Notifies when a newer Straw build is on fdroid.sulkta.com." + // NotificationChannel is API 26+; minSdk is 24, so guard it or a 7.x + // device crashes (NoClassDefFoundError) the moment an update is found. + // Pre-O, NotificationCompat.Builder ignores the channel id and posts fine. + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + val channel = NotificationChannel( + NOTIF_CHANNEL_ID, + "Straw updates", + NotificationManager.IMPORTANCE_DEFAULT, + ).apply { + description = "Notifies when a newer Straw build is on fdroid.sulkta.com." + } + nm.createNotificationChannel(channel) } - nm.createNotificationChannel(channel) // ACTION_VIEW on the APK URL — Chrome / system browser fetches it // via DownloadManager and the user taps it to install. No @@ -102,6 +122,10 @@ private fun postUpdateNotification(context: Context, info: UpdateInfo) { .setContentTitle("Straw $name available") .setContentText("Tap to download from fdroid.sulkta.com.") .setAutoCancel(true) + // Only sound/vibrate the first time we surface a given version — the + // check re-runs on every cadence tick and would otherwise re-nag for + // the same pending update until the user installs it. + .setOnlyAlertOnce(true) .setContentIntent(pending) .build() nm.notify(NOTIF_ID, notif) diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/net/RydClient.kt b/strawApp/src/main/kotlin/com/sulkta/straw/net/RydClient.kt index 400127de3..e47fe8d13 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/net/RydClient.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/net/RydClient.kt @@ -10,6 +10,7 @@ package com.sulkta.straw.net +import com.sulkta.straw.util.runCatchingCancellable import kotlinx.serialization.Serializable @Serializable @@ -23,13 +24,18 @@ object RydClient { /** Suspends on the Rust async runtime; call from a coroutine. Returns * null on any failure (transport / non-2xx / bad JSON), same contract * the old blocking client had. Only likes/dislikes are carried — RYD's - * rating/viewCount were dead, redundant overlay data (audit #2 L-9). */ + * rating/viewCount were dead, redundant overlay data (audit #2 L-9). + * The uniffi call is wrapped: a core panic now unwinds to a UniFFI + * InternalException (panic=unwind), which this shim's "null on failure" + * contract must swallow rather than crash the detail screen. */ suspend fun fetch(videoId: String): RydVotes? = - uniffi.strawcore.fetchRydVotes(videoId)?.let { v -> - RydVotes( - id = v.id, - likes = v.likes, - dislikes = v.dislikes, - ) - } + runCatchingCancellable { + uniffi.strawcore.fetchRydVotes(videoId)?.let { v -> + RydVotes( + id = v.id, + likes = v.likes, + dislikes = v.dislikes, + ) + } + }.getOrNull() } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/net/SponsorBlockClient.kt b/strawApp/src/main/kotlin/com/sulkta/straw/net/SponsorBlockClient.kt index f15a6ef6d..662db3eea 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/net/SponsorBlockClient.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/net/SponsorBlockClient.kt @@ -12,6 +12,7 @@ package com.sulkta.straw.net +import com.sulkta.straw.util.runCatchingCancellable import kotlinx.serialization.Serializable @Serializable @@ -28,17 +29,22 @@ data class SbSegment( object SponsorBlockClient { /** Suspends on the Rust async runtime; call from a coroutine. Returns * an empty list on any failure, same contract the old blocking client - * had. `categories` are the SB category keys to request. */ + * had. `categories` are the SB category keys to request. The uniffi + * call is wrapped so a core panic (now unwinding to a UniFFI + * InternalException under panic=unwind) is swallowed to an empty list + * rather than crashing video-detail load. */ suspend fun fetch( videoId: String, categories: List = listOf("sponsor"), ): List = - uniffi.strawcore.fetchSponsorSegments(videoId, categories).map { s -> - SbSegment( - UUID = s.uuid, - category = s.category, - segment = listOf(s.startSec, s.endSec), - actionType = s.actionType, - ) - } + runCatchingCancellable { + uniffi.strawcore.fetchSponsorSegments(videoId, categories).map { s -> + SbSegment( + UUID = s.uuid, + category = s.category, + segment = listOf(s.startSec, s.endSec), + actionType = s.actionType, + ) + } + }.getOrDefault(emptyList()) }