From 6b648a4444e359ddc74f3a106b322a086ddf2ac1 Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Tue, 25 Aug 2026 15:04:09 +0300 Subject: [PATCH 1/3] refactor(lsposed): make the post-write cache set a list, not a procedure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four config-derived caches were refreshed by four open-coded calls whose two invariants — root snapshot first, everyone else with force=false — lived only in a comment. Getting the flag wrong is not a crash, it is a silent extra root shell per cache, so nothing would ever surface the mistake. They are a list now, iterated after the root refresh, with the membership rule written down: a cache belongs here iff its load reads the canonical config. The caches that deliberately do not qualify are named too, since "did someone forget to add this one?" was previously answerable only by reading all nine of them. Skip caches that have never loaded. refreshInPlace bypasses the concrete cache's ensureLoaded, so calling it on a pristine cache runs load() without the inputs that method stashes; the load fails, the cache records the error, and ensure() then early-returns on that error forever. A cache with no value has nothing that can go stale, so there was never a reason to touch it. Replaces the runCatching that was papering over exactly this for RoutingGateCache. --- .../vpnhide/CanonicalConfigRepository.kt | 45 ++++++++++++++----- .../dev/okhsunrog/vpnhide/StateCache.kt | 13 ++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/CanonicalConfigRepository.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/CanonicalConfigRepository.kt index fc073bd8..b3266413 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/CanonicalConfigRepository.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/CanonicalConfigRepository.kt @@ -63,17 +63,42 @@ internal object CanonicalConfigRepository { CanonicalWriteResult(exit, output) } - // Reload each derived cache in place — swap old→new, so no observer sees a null blank - // between the write and the reload (the toggle-flicker fix). Root first: the others - // derive from it and reuse it via force=false. Failures keep the stale value. + /** + * The caches whose value is *derived from the canonical config*, and which a + * write therefore leaves stale. That is the whole membership rule — add a cache + * here if and only if its `load` reads the config (directly, or via the root + * snapshot's config-bearing sections). + * + * Deliberately NOT members: `AppListCache` (an `AppSummary` carries no config + * state — the picker merges target flags in reactively), `UpdateCheckCache` and + * `DiagnosticsCache` (unrelated to the config), and `SystemServerConfigCache` + * (lives in the system_server process, unreachable from here — its own + * `SystemDataFileWatcher` invalidates it). + * + * `RootSnapshotCache` is the shared upstream rather than a member; see + * [refreshDerivedCaches]. + */ + private val derivedCaches: List> = + listOf(TargetsCache, DashboardCache, StatisticsCache, RoutingGateCache) + + /** + * Reload every config-derived cache in place — swap old→new, so no observer sees + * a null blank between the write and the reload (the toggle-flicker fix). + * + * The root snapshot goes first and alone: the others all derive from it, so they + * follow with `force = false` to reuse it. Passing `force = true` here would be a + * silent, invisible cost — each cache would invalidate the snapshot and re-run the + * whole root shell for itself, once per member. Iterating a list instead of + * open-coding the calls is what keeps the order and the flag structural rather + * than a comment someone has to notice. + * + * A failure keeps the stale value (the cache records its own error), so one + * unhappy cache can't abort the rest. + */ internal suspend fun refreshDerivedCaches() { runCatching { RootSnapshotCache.refresh() } - runCatching { TargetsCache.refreshInPlace(force = false) } - runCatching { DashboardCache.refreshInPlace(force = false) } - runCatching { StatisticsCache.refreshInPlace(force = false) } - // Tolerates RoutingGateCache not being initialized yet (load() throws on a - // null appContext before any screen has called ensureLoaded/refresh) — a - // config write racing app startup should not crash the write itself. - runCatching { RoutingGateCache.refreshInPlace(force = false) } + derivedCaches.forEach { cache -> + if (!cache.pristine) runCatching { cache.refreshInPlace(force = false) } + } } } diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/StateCache.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/StateCache.kt index 1bdb7d58..f5e3896a 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/StateCache.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/StateCache.kt @@ -78,6 +78,19 @@ internal abstract class StateCache( reload(force) } + /** + * True while the cache has never held anything — no value, no failed load. + * + * A pristine cache has nothing that can go stale, so a config write can skip + * it: the next [ensure] loads it fresh anyway. Skipping also matters because + * [refreshInPlace] bypasses the concrete cache's `ensureLoaded`, so calling it + * first would run [load] without the inputs that method stashes — the load + * fails, [reload] records the error, and [ensure] then early-returns on that + * error forever. Kept in the error case on purpose: that one is worth retrying. + */ + val pristine: Boolean + get() = _value.value == null && _error.value == null + /** Drop the cached value/error so the next [ensure] reloads. */ open fun invalidate() { _value.value = null From b2820f1d5f2630bf2c45e347d46eb945c1f3328f Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Tue, 25 Aug 2026 15:04:10 +0300 Subject: [PATCH 2/3] refactor(lsposed): say why each @Suppress is there, and stop repeating it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three unrelated things were wearing the same annotation, so the annotation had stopped carrying information. JavaChecks used six per-function DEPRECATION suppressions for one reason: the deprecated ConnectivityManager surface is the point of that file. allNetworks, getNetworkInfo(type), the network-handle calls — those are what a VPN-probing app reaches for, so the checks proving we hid the tunnel must reach for the same ones, and "modernising" them would quietly drop detection coverage. Stated once at file level with a pointer to docs/detection-vectors.md. The file holds no non-probe code for the blanket to hide a real warning in; it also carried 34 imports left behind when the UI moved out, now gone. Motion.kt repeated one identical UNCHECKED_CAST six times to re-type specs cached as Any. One private helper, one suppression, and the soundness argument (these are tweens and thresholdless springs — they never touch a value of T) written down, along with what must not be routed through it. The four casts in the hook process are NOT collapsible the same way, which is worth recording: `as?` against a concrete generic type still checks the raw class at runtime, so a ROM that reshaped a field falls out as null. Behind a helper taking an unbounded T the cast erases and that check silently disappears, turning a clean bail-out into a ClassCastException somewhere later. They keep their own suppressions and now state which AOSP declaration each one trusts. --- .../vpnhide/diagnostics/JavaChecks.kt | 52 +++++-------------- .../dev/okhsunrog/vpnhide/hook/HookEntry.kt | 5 ++ .../vpnhide/hook/PackageVisibilityHooks.kt | 8 +++ .../dev/okhsunrog/vpnhide/ui/theme/Motion.kt | 33 +++++++----- 4 files changed, 46 insertions(+), 52 deletions(-) diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/diagnostics/JavaChecks.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/diagnostics/JavaChecks.kt index 7ce2e193..7be00343 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/diagnostics/JavaChecks.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/diagnostics/JavaChecks.kt @@ -1,32 +1,22 @@ +// The deprecated ConnectivityManager surface is the point of this file, not an +// oversight: `allNetworks`, `getNetworkInfo(type)`, `getNetworkInfo(network)`, +// `activeNetworkInfo` and the network-handle calls are exactly what a VPN-probing +// app reaches for, so the checks that prove we hid the tunnel have to reach for the +// same ones. Migrating them to the modern equivalents would silently drop detection +// coverage — see docs/detection-vectors.md for the vector each one stands in for. +// +// Hence file-level rather than the six per-function suppressions this replaces: the +// whole file is deliberately-legacy probe code, and there is no non-probe code here +// for the blanket to hide a genuine deprecation warning in. +@file:Suppress("DEPRECATION") + package dev.okhsunrog.vpnhide.diagnostics import android.net.ConnectivityManager import android.net.LinkProperties import android.net.Network import android.net.NetworkCapabilities -import android.net.NetworkInfo -import android.net.Uri import android.os.Build -import androidx.activity.compose.rememberLauncherForActivityResult -import androidx.activity.result.contract.ActivityResultContracts -import androidx.compose.foundation.isSystemInDarkTheme -import androidx.compose.foundation.layout.* -import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.shape.RoundedCornerShape -import androidx.compose.foundation.verticalScroll -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.FiberManualRecord -import androidx.compose.material.icons.filled.Stop -import androidx.compose.material3.* -import androidx.compose.runtime.* -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.res.stringResource -import androidx.compose.ui.text.font.FontFamily -import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.unit.dp -import androidx.compose.ui.unit.sp import dev.okhsunrog.vpnhide.LogTags import dev.okhsunrog.vpnhide.R import dev.okhsunrog.vpnhide.VpnHideLog @@ -35,22 +25,10 @@ import dev.okhsunrog.vpnhide.checks.CheckStatus import dev.okhsunrog.vpnhide.checks.NativeProbe import dev.okhsunrog.vpnhide.generated.IfaceLists import dev.okhsunrog.vpnhide.next -import dev.okhsunrog.vpnhide.ui.components.EnhancedButton -import dev.okhsunrog.vpnhide.ui.components.EnhancedCard -import dev.okhsunrog.vpnhide.ui.components.GroupedCard -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext -import java.io.File import java.net.NetworkInterface -import java.text.SimpleDateFormat -import java.util.Date -import java.util.Locale import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicReference -import java.util.zip.ZipEntry -import java.util.zip.ZipOutputStream private const val TAG = LogTags.TEST @@ -327,7 +305,6 @@ private fun checkNetworkInterfaceEnum(name: String): CheckResult = javaCheck(name, null, "${e.message}") } -@Suppress("DEPRECATION") internal fun checkAllNetworksVpn( cm: ConnectivityManager, name: String, @@ -377,7 +354,6 @@ private data class NetworkForTypeResult( val error: String? = null, ) -@Suppress("DEPRECATION") private fun queryNetworkForType( cm: ConnectivityManager, type: Int, @@ -392,7 +368,6 @@ private fun queryNetworkForType( NetworkForTypeResult(error = t.cause?.message ?: t.message ?: t.javaClass.simpleName) } -@Suppress("DEPRECATION") private fun checkNetworkForTypeVpn( cm: ConnectivityManager, name: String, @@ -415,7 +390,6 @@ private fun checkNetworkForTypeVpn( return javaCheck(name, false, detail) } -@Suppress("DEPRECATION") private fun checkActiveNetworkHandle( cm: ConnectivityManager, name: String, @@ -436,7 +410,6 @@ private fun checkActiveNetworkHandle( return javaCheck(name, !leaksVpnHandle, detail) } -@Suppress("DEPRECATION") private fun checkAllNetworksHandles( cm: ConnectivityManager, name: String, @@ -553,7 +526,6 @@ private fun checkLinkPropertiesRoutes( // leak. (Its companion getActiveNetworkInfo() was dropped: .type reports the // underlying transport (WIFI/mobile) for an active VPN, not TYPE_VPN, so it never // surfaced the leak.) -@Suppress("DEPRECATION") private fun checkNetworkInfoVpn( cm: ConnectivityManager, name: String, diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/HookEntry.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/HookEntry.kt index 6fc00ab6..651f9d16 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/HookEntry.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/HookEntry.kt @@ -199,6 +199,9 @@ class HookEntry : IXposedHookLoadPackage { /** Remove routes whose interface is a VPN tunnel. Returns true if any went. */ private fun sanitizeLinkRoutes(copy: LinkProperties): Boolean { try { + // Unchecked only in the element type: AOSP declares LinkProperties.mRoutes as + // ArrayList, and the `as?` still checks List-ness at runtime, so a + // ROM that reshaped the field falls out as null rather than crashing here. @Suppress("UNCHECKED_CAST") val routesField = XposedHelpers.getObjectField(copy, "mRoutes") as? MutableList ?: return false val filtered = @@ -223,6 +226,8 @@ class HookEntry : IXposedHookLoadPackage { private fun sanitizeStackedLinks(copy: LinkProperties): Boolean { var modified = false try { + // Same shape as mRoutes above: AOSP declares mStackedLinks as + // Hashtable; Map-ness is still checked at runtime. @Suppress("UNCHECKED_CAST") val stacked = XposedHelpers.getObjectField(copy, "mStackedLinks") as? MutableMap if (stacked != null && stacked.isNotEmpty()) { diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/PackageVisibilityHooks.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/PackageVisibilityHooks.kt index 3c1f251a..db33a760 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/PackageVisibilityHooks.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/hook/PackageVisibilityHooks.kt @@ -186,6 +186,11 @@ internal object PackageVisibilityHooks { val pls = parceledListSliceClass ?: return if (!pls.isInstance(result)) return + // T is whatever element type the caller hooked for (PackageInfo, + // ApplicationInfo, ResolveInfo…). ParceledListSlice.getList() is + // declared List on the framework side; List-ness is checked at + // runtime, the element type is taken on trust — pkgOf below is the + // only thing that touches an element, and it is null-tolerant. @Suppress("UNCHECKED_CAST") val original = XposedHelpers.callMethod(result, "getList") as? List ?: return val filtered = @@ -336,6 +341,9 @@ internal object PackageVisibilityHooks { object : XC_MethodHook() { override fun afterHookedMethod(param: MethodHookParam) { if (param.hasThrowable()) return + // Unchecked only in element nullability — getNamesForUids returns + // String[] with null holes for unresolved uids. The array class itself + // is still checked, so a ROM returning something else bails out. @Suppress("UNCHECKED_CAST") val names = param.result as? Array ?: return val caller = observerCaller() ?: return diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ui/theme/Motion.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ui/theme/Motion.kt index 8ce1211c..f7c5fedc 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ui/theme/Motion.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ui/theme/Motion.kt @@ -60,6 +60,21 @@ object AppMotion { fun slowEffects(): AnimationSpec = tween(durationMillis = 500, easing = AppEasing.FancyTransition) } +/** + * Hand a spec cached as `Any` back out as one for [T]. + * + * [MotionScheme] asks for a fresh `FiniteAnimationSpec` per call site, but the + * specs are built once and shared, so the erased element type has to be restored on + * the way out. Sound for what is cached here: every spec below is a `tween` or a + * `spring` with no visibility threshold, and neither ever touches a value of `T` — + * they vectorize through the converter supplied at animation time. Do not route a + * spec that carries `T`-typed data (a threshold, a keyframe) through this. + * + * One suppression, stated once, instead of six identical ones down the object. + */ +@Suppress("UNCHECKED_CAST") +private fun AnimationSpec.retyped(): FiniteAnimationSpec = this as FiniteAnimationSpec + /** * Custom [MotionScheme] driving every expressive Material 3 component animation. * @@ -78,21 +93,15 @@ val AppMotionScheme: MotionScheme = private val fastEffects = AppMotion.fastEffects() private val slowEffects = AppMotion.slowEffects() - @Suppress("UNCHECKED_CAST") - override fun defaultSpatialSpec(): FiniteAnimationSpec = defaultSpatial as FiniteAnimationSpec + override fun defaultSpatialSpec(): FiniteAnimationSpec = defaultSpatial.retyped() - @Suppress("UNCHECKED_CAST") - override fun fastSpatialSpec(): FiniteAnimationSpec = fastSpatial as FiniteAnimationSpec + override fun fastSpatialSpec(): FiniteAnimationSpec = fastSpatial.retyped() - @Suppress("UNCHECKED_CAST") - override fun slowSpatialSpec(): FiniteAnimationSpec = slowSpatial as FiniteAnimationSpec + override fun slowSpatialSpec(): FiniteAnimationSpec = slowSpatial.retyped() - @Suppress("UNCHECKED_CAST") - override fun defaultEffectsSpec(): FiniteAnimationSpec = defaultEffects as FiniteAnimationSpec + override fun defaultEffectsSpec(): FiniteAnimationSpec = defaultEffects.retyped() - @Suppress("UNCHECKED_CAST") - override fun fastEffectsSpec(): FiniteAnimationSpec = fastEffects as FiniteAnimationSpec + override fun fastEffectsSpec(): FiniteAnimationSpec = fastEffects.retyped() - @Suppress("UNCHECKED_CAST") - override fun slowEffectsSpec(): FiniteAnimationSpec = slowEffects as FiniteAnimationSpec + override fun slowEffectsSpec(): FiniteAnimationSpec = slowEffects.retyped() } From 50f535063092562034c5b0022461ee2356e89caa Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Tue, 25 Aug 2026 15:04:12 +0300 Subject: [PATCH 3/3] docs(lsposed): glossary for the seven things called "diagnostic" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The word is attached to the check suite, that suite's run state, the canonical report model, the precondition gate, hook attach telemetry, the export, and two bundle section names. None of it is misfiled — they really are all diagnostics — but the name alone no longer tells you which layer you are in, and the pair that actually bites is DiagnosticsCache (a run) versus RoutingGateCache (a precondition, and the only one of the two derived from the canonical config). Also records the derivedCaches membership rule next to the StateCache entry, so someone adding a cache reads it where they are already looking. --- lsposed/AGENTS.md | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lsposed/AGENTS.md b/lsposed/AGENTS.md index feccbd7f..1b7f2f9c 100644 --- a/lsposed/AGENTS.md +++ b/lsposed/AGENTS.md @@ -27,6 +27,27 @@ What stays in the root package is the shared vocabulary both sides use — `DashboardData`, the agent bridge. Moving those buys import churn and nothing else; they belong to no single feature. +## "Diagnostic" names seven different things + +The word got attached to every layer that answers "what is going on", so the +name alone will not tell you which one you are looking at. Nothing here is +misplaced — they genuinely are all diagnostics — but know which is which before +you add to any of them: + +| name | what it actually is | +|---|---| +| `DiagnosticsScreen`, *Detailed diagnostics* | the user-facing check suite | +| `DiagnosticsCache` | the **run state** of that suite (NotRun / Running / Failed) | +| `DiagnosticReport`, `buildDiagnosticReport`, `DiagnosticCheck` | the **canonical model** the screen and the bundle both render — see `docs/diagnostics.md` | +| `DiagnosticGate`, `RoutingGateCache`, `resolveDiagnosticGate` | the **precondition** for a meaningful run (VPN up, this app routed) — not a check | +| `HookDiagnostics`, `ConnectivityAttachDiagnostics`, `KpmDiagnostics` | attach/telemetry for the hooks themselves; **not part of the suite** | +| `writeDiagnosticZip`, `debug/` | the export — the artifact is the *debug bundle* (`docs/debug-bundle.md`) | +| `app_scan_diagnostics`, `kmod_diag` | section names inside that bundle | + +The pair worth keeping straight: `DiagnosticsCache` holds a **run**, +`RoutingGateCache` holds a **precondition**, and only the second is derived from +the canonical config (so only the second is refreshed after a write). + ## Two processes, one APK The single most important thing about this module: `hook/` is loaded by LSPosed @@ -56,7 +77,10 @@ directions, since `internal` is module-wide and the compiler will not. - **`StateCache`** — base for every app-scoped, lazily-loaded cache (loading/error/value flows + single-flight job). A new cache **extends this**; - never hand-roll `inflight`/`loading` again. + never hand-roll `inflight`/`loading` again. If its value is derived from the + canonical config, also add it to `CanonicalConfigRepository.derivedCaches` — + that list, and only that list, is what a config write refreshes. Membership + rule and the deliberate non-members are documented on it. - **`RootSnapshotCache`** — the single batched root read. Need new system state on the Dashboard/Hiding path? Add a section to its shell snapshot; don't add an ad-hoc `suExec` that races the snapshot.