From f02377c42cda50944480aaaad45cfcb1acf2991f Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Fri, 14 Aug 2026 08:05:38 +0300 Subject: [PATCH 1/2] build(lsposed): drop the Kotlin source-size budget check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checkKotlinSourceSize task capped per-file line counts with a hand-maintained budget map. In practice it produced churn without catching anything: an edit that grew a file failed the build, and the budget entry got bumped to match — so the budget just trailed the file size instead of bounding it. Remove the task, its budget map, and the detekt dependency; drop the now-stale references in AGENTS.md and detekt.yml. detekt's LongMethod/complexity rules still bound function size. --- lsposed/AGENTS.md | 4 --- lsposed/app/build.gradle.kts | 62 -------------------------------- lsposed/config/detekt/detekt.yml | 6 ++-- 3 files changed, 2 insertions(+), 70 deletions(-) diff --git a/lsposed/AGENTS.md b/lsposed/AGENTS.md index 52cb3dd..b78c1d5 100644 --- a/lsposed/AGENTS.md +++ b/lsposed/AGENTS.md @@ -83,10 +83,6 @@ reinvent them.** `grep` for an existing helper before writing a new one. `@Suppress("RuleName")` + a one-line reason — visible in the code, not hidden in a baseline. Only disable a rule in `detekt.yml` (with a comment) when it doesn't fit the codebase at all. -- **Kotlin source-size budget** — `:app:detekt` also runs - `checkKotlinSourceSize`. New source files are capped at 700 lines; existing - oversized files have explicit shrink-only budgets in `app/build.gradle.kts`. - Split responsibilities instead of raising a budget. - **CPD** (copy-paste detector) — finds cross-file duplicated blocks that detekt can't (re-implemented parsers / save-builders are the classic AI-duplication smell). CI-enforced (`./gradlew cpdCheck`); report at diff --git a/lsposed/app/build.gradle.kts b/lsposed/app/build.gradle.kts index 65dd7b6..97622f5 100644 --- a/lsposed/app/build.gradle.kts +++ b/lsposed/app/build.gradle.kts @@ -1,43 +1,8 @@ import java.io.FileInputStream import java.util.Properties -import org.gradle.api.DefaultTask -import org.gradle.api.file.ConfigurableFileCollection -import org.gradle.api.provider.MapProperty -import org.gradle.api.provider.Property import org.gradle.api.tasks.Exec -import org.gradle.api.tasks.Input -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.PathSensitivity -import org.gradle.api.tasks.PathSensitive -import org.gradle.api.tasks.TaskAction import org.jetbrains.kotlin.gradle.dsl.JvmTarget -abstract class CheckKotlinSourceSize : DefaultTask() { - @get:InputFiles - @get:PathSensitive(PathSensitivity.RELATIVE) - abstract val sources: ConfigurableFileCollection - - @get:Input - abstract val budgets: MapProperty - - @get:Input - abstract val defaultLimit: Property - - @TaskAction - fun verify() { - val limits = budgets.get() - val violations = - sources.files.mapNotNull { source -> - val lineCount = source.readLines().size - val limit = limits[source.name] ?: defaultLimit.get() - if (lineCount > limit) "${source.name}: $lineCount lines (limit $limit)" else null - } - check(violations.isEmpty()) { - "Kotlin source-size budget exceeded:\n${violations.joinToString("\n")}" - } - } -} - plugins { alias(libs.plugins.android.application) // kotlin-android removed: AGP 9+ has built-in Kotlin support. @@ -59,33 +24,6 @@ detekt { source.setFrom(files("src/main/kotlin", "src/test/kotlin")) } -// Top-level Compose functions evade detekt's LargeClass rule, so keep a -// separate file-size budget. Existing oversized files are shrink-only debt: -// their exact current sizes are explicit here, while every new source gets the -// default ceiling. Remove an entry once a file drops below the default. -val kotlinSourceLineBudgets = - mapOf( - "DashboardData.kt" to 1730, - "DashboardScreen.kt" to 1335, - "HookEntry.kt" to 1276, - "SettingsScreen.kt" to 1201, - "StatisticsScreen.kt" to 1102, - "AppPickerScreen.kt" to 940, - "AgentControl.kt" to 888, - "MainActivity.kt" to 817, - ) - -val checkKotlinSourceSize = - tasks.register("checkKotlinSourceSize") { - group = "verification" - description = "Rejects new Kotlin god-files and growth in existing oversized sources." - sources.from(fileTree("src/main/kotlin") { include("**/*.kt") }) - budgets.set(kotlinSourceLineBudgets) - defaultLimit.set(700) - } - -tasks.named("detekt").configure { dependsOn(checkKotlinSourceSize) } - tasks.withType().configureEach { // Codegen output (IfaceLists) and UniFFI bindings aren't hand-written. exclude("**/generated/**") diff --git a/lsposed/config/detekt/detekt.yml b/lsposed/config/detekt/detekt.yml index 272fb7f..92bd5db 100644 --- a/lsposed/config/detekt/detekt.yml +++ b/lsposed/config/detekt/detekt.yml @@ -4,8 +4,7 @@ # Goal: catch the smell ktlint can't — god-functions, high complexity, # dead code, common bug patterns — without bikeshedding AI-written Compose # code to death. There is no baseline: inherent exceptions stay visible at the -# call site, and a separate Gradle source-size budget covers top-level Compose -# files that LargeClass cannot see. +# call site. build: # Any finding not in the baseline fails the build. @@ -14,8 +13,7 @@ build: complexity: LongMethod: # The whole reason for this gate: loadDashboardState was 830 lines. - # @Composable UI builders are exempt here; checkKotlinSourceSize bounds - # top-level god-screens that detekt's class-oriented rules cannot see. + # @Composable UI builders are exempt here. threshold: 60 ignoreAnnotated: ['Composable'] CyclomaticComplexMethod: From fa33f5c7b42ddd2a5f1e2f048ed24d9e58c54f18 Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Fri, 14 Aug 2026 08:05:38 +0300 Subject: [PATCH 2/2] refactor(diagnostics): make a report's verdict reachable only through the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A diagnostic run that the gate blocked (VPN off, self not routed, needs restart) still builds LayerReports carrying a placeholder Active(0, 0). The old LayerReport.verdict folded those zero counts into Verdict.Ok, and the debug-bundle renderers read it unconditionally — so a gated run's diagnostics.txt/json could print "Native verdict: ok" for a device with the VPN off. The contract that "verdict must not be consulted unless gate == ROUTED" was a doc comment, not a type. Encode it in the type instead: remove the ungated LayerReport.verdict and expose the verdict only through DiagnosticReport.nativeVerdict / javaVerdict, which return null unless the run was measured (ROUTED). The renderers take that gate-checked value and print "not-measured" for a gated layer. The raw LayerStatus.Active.verdict stays for the dashboard, which already gates it type-safely via ProtectionCheck.Checked. Adds a regression test that a blocked gate exposes no verdict and never renders a placeholder ok. --- .../dev/okhsunrog/vpnhide/DebugExport.kt | 4 +-- .../dev/okhsunrog/vpnhide/DiagnosticReport.kt | 32 +++++++++++++------ .../vpnhide/DiagnosticReportRender.kt | 24 ++++++++------ .../okhsunrog/vpnhide/DiagnosticReportTest.kt | 26 ++++++++++++--- 4 files changed, 59 insertions(+), 27 deletions(-) diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DebugExport.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DebugExport.kt index b5adb00..377eb35 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DebugExport.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DebugExport.kt @@ -163,8 +163,8 @@ internal fun buildDiagnosticSummaryText( appendLine("Diagnostics: not run") } else { appendLine("Diagnostics gate: ${report.gate.name.lowercase()}") - appendLine("Native verdict: ${report.native.verdictLabel()}") - appendLine("Java verdict: ${report.java.verdictLabel()}") + appendLine("Native verdict: ${report.native.verdictLabel(report.nativeVerdict)}") + appendLine("Java verdict: ${report.java.verdictLabel(report.javaVerdict)}") appendLine("Outcomes: ${report.outcomeTally()}") } appendLine("selfNeedsRestart: $selfNeedsRestart") diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReport.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReport.kt index b87c83f..3fabaeb 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReport.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReport.kt @@ -58,7 +58,10 @@ internal data class DiagnosticCheck( val owned: Boolean, ) -/** Per-layer rollup: presence/verdict plus the classified checks that produced it. */ +/** Per-layer rollup: presence plus the classified checks that produced it. The + * Ok/Partial/Broken verdict is deliberately NOT exposed here — it is only valid + * for a measured run, so it is reachable only through the gate-checked + * [DiagnosticReport.nativeVerdict] / [DiagnosticReport.javaVerdict]. */ internal data class LayerReport( val layer: CheckLayer, val backend: NativeBackendId?, @@ -67,12 +70,7 @@ internal data class LayerReport( // a warning, never counted against the tile verdict. Always 0 for the Java layer. val unownedLeaks: Int, val checks: List, -) { - /** Ok/Partial/Broken when the layer is [LayerStatus.Active]; null otherwise. - * Only meaningful when the report's [DiagnosticReport.gate] is - * [DiagnosticGate.ROUTED] — a gated report carries presence only. */ - val verdict: Verdict? get() = (status as? LayerStatus.Active)?.verdict -} +) /** * The single canonical diagnostic snapshot. @@ -91,15 +89,29 @@ internal data class DiagnosticReport( // False after the fast core phase, true once the slow Java probes have filled in. val complete: Boolean, val schema: Int = DIAGNOSTIC_REPORT_SCHEMA, -) +) { + /** Per-layer Ok/Partial/Broken — the ONLY way to read a report's verdict. + * Null unless the run was actually measured ([DiagnosticGate.ROUTED]); a + * gated report's layers carry a placeholder [LayerStatus.Active] with zero + * counts, so returning its verdict would render a false "Ok". */ + val nativeVerdict: Verdict? get() = native.verdictFor(gate) + val javaVerdict: Verdict? get() = java.verdictFor(gate) +} + +/** Verdict of a layer, but only for a measured run — the gate is required so no + * caller can obtain a verdict without acknowledging whether the run measured + * anything. */ +private fun LayerReport.verdictFor(gate: DiagnosticGate): Verdict? = + if (gate == DiagnosticGate.ROUTED) (status as? LayerStatus.Active)?.verdict else null /** * Fold the raw check run into the canonical [DiagnosticReport]. Pure: same inputs * → same report, no Android or IO dependency, so it is unit-tested directly. * * [results] is null when the gate blocked the run ([DiagnosticGate.ROUTED] is the - * only gate that carries measurements); the layers then report presence only and - * their [LayerReport.verdict] must not be consulted. + * only gate that carries measurements); the layers then report presence only, and + * [DiagnosticReport.nativeVerdict] / [DiagnosticReport.javaVerdict] return null so + * a gated verdict can never be rendered. */ internal fun buildDiagnosticReport( gate: DiagnosticGate, diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportRender.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportRender.kt index 445c299..4bee09d 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportRender.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportRender.kt @@ -17,10 +17,13 @@ private fun LayerStatus.presenceToken(): String = is LayerStatus.Active -> "active" } -/** One-line verdict/presence summary for a layer, e.g. `broken (hidden 0, leaks 2, unowned 1)`. */ -internal fun LayerReport.verdictLabel(): String { +/** One-line verdict/presence summary for a layer, e.g. `broken (hidden 0, leaks 2, + * unowned 1)`. [verdict] comes from the report's gate-checked accessor, so a gated + * run (verdict == null) renders `not-measured`, never a placeholder `ok`. */ +internal fun LayerReport.verdictLabel(verdict: Verdict?): String { val active = status as? LayerStatus.Active ?: return status.presenceToken() - return "${active.verdict.name.lowercase()} (hidden ${active.hidden}, leaks ${active.leaks}, unowned $unownedLeaks)" + val head = verdict?.name?.lowercase() ?: "not-measured" + return "$head (hidden ${active.hidden}, leaks ${active.leaks}, unowned $unownedLeaks)" } /** Count of every check by outcome token across both layers — the honest headline @@ -43,18 +46,19 @@ internal fun DiagnosticReport.toDiagnosticsText(): String = appendLine("complete: $complete") appendLine("outcomes: ${outcomeTally()}") appendLine() - appendLayer(native, "Native", native.backend?.name?.lowercase() ?: "none") + appendLayer(native, "Native", native.backend?.name?.lowercase() ?: "none", nativeVerdict) appendLine() - appendLayer(java, "Java", "lsposed") + appendLayer(java, "Java", "lsposed", javaVerdict) }.trimEnd() private fun StringBuilder.appendLayer( layer: LayerReport, title: String, backendLabel: String, + verdict: Verdict?, ) { appendLine("--- $title layer ($backendLabel) ---") - appendLine(layer.verdictLabel()) + appendLine(layer.verdictLabel(verdict)) if (layer.checks.isEmpty()) { appendLine("(no checks — gated run)") return @@ -113,17 +117,17 @@ private fun DiagnosticReport.toReportJson(): ReportJson = schema = schema, gate = gate.name.lowercase(), complete = complete, - native = native.toLayerJson(), - java = java.toLayerJson(), + native = native.toLayerJson(nativeVerdict), + java = java.toLayerJson(javaVerdict), ) -private fun LayerReport.toLayerJson(): LayerJson { +private fun LayerReport.toLayerJson(verdict: Verdict?): LayerJson { val active = status as? LayerStatus.Active return LayerJson( layer = layer.name.lowercase(), backend = backend?.name?.lowercase(), presence = status.presenceToken(), - verdict = active?.verdict?.name?.lowercase(), + verdict = verdict?.name?.lowercase(), hidden = active?.hidden, leaks = active?.leaks, unownedLeaks = unownedLeaks, diff --git a/lsposed/app/src/test/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportTest.kt b/lsposed/app/src/test/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportTest.kt index f77c1b5..7d86780 100644 --- a/lsposed/app/src/test/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportTest.kt +++ b/lsposed/app/src/test/kotlin/dev/okhsunrog/vpnhide/DiagnosticReportTest.kt @@ -1,6 +1,8 @@ package dev.okhsunrog.vpnhide import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -45,19 +47,19 @@ class DiagnosticReportTest { @Test fun `native verdict is Broken when an owned vector leaks and nothing hid`() { val r = report(results = CheckResults(native = nativeResults("ioctl_flags" to CheckOutcome.Leak))) - assertEquals(Verdict.Broken, r.native.verdict) + assertEquals(Verdict.Broken, r.nativeVerdict) } @Test fun `native verdict is Partial when it hides some but an owned vector leaks`() { val native = nativeResults("ioctl_flags" to CheckOutcome.Leak, "getifaddrs" to CheckOutcome.HiddenByBackend) - assertEquals(Verdict.Partial, report(results = CheckResults(native = native)).native.verdict) + assertEquals(Verdict.Partial, report(results = CheckResults(native = native)).nativeVerdict) } @Test fun `native verdict is Ok when nothing owned leaks`() { val native = nativeResults("ioctl_flags" to CheckOutcome.HiddenByBackend) - assertEquals(Verdict.Ok, report(results = CheckResults(native = native)).native.verdict) + assertEquals(Verdict.Ok, report(results = CheckResults(native = native)).nativeVerdict) } // ── unowned leaks are surfaced separately, never against the verdict ──── @@ -67,7 +69,7 @@ class DiagnosticReportTest { // netlink_getrule (fib_nl_fill_rule) has no zygisk hook → out of scope for // the zygisk tile, so the verdict stays Ok and the leak is counted as unowned. val r = report(results = CheckResults(native = nativeResults("netlink_getrule" to CheckOutcome.Leak))) - assertEquals(Verdict.Ok, r.native.verdict) + assertEquals(Verdict.Ok, r.nativeVerdict) assertEquals(1, r.native.unownedLeaks) } @@ -119,7 +121,7 @@ class DiagnosticReportTest { val r = report(results = results) val javaCheck = r.java.checks.single() assertEquals(CheckOutcome.Leak, javaCheck.outcome) - assertEquals(Verdict.Broken, r.java.verdict) + assertEquals(Verdict.Broken, r.javaVerdict) } // ── gate ─────────────────────────────────────────────────────────────── @@ -132,6 +134,20 @@ class DiagnosticReportTest { assertTrue(r.java.checks.isEmpty()) } + @Test + fun `a blocked gate exposes no verdict and never renders a placeholder ok`() { + // A gated run's active layers carry a placeholder Active(0,0). The + // gate-checked accessors must return null and the renderers must show + // "not-measured", never a false "ok" folded off the zero counts. + val r = report(gate = DiagnosticGate.VPN_OFF, results = null) + assertNull(r.nativeVerdict) + assertNull(r.javaVerdict) + val text = r.toDiagnosticsText() + assertTrue("gated layers render not-measured", text.contains("not-measured")) + assertFalse("no placeholder ok verdict", text.contains("ok (hidden 0, leaks 0")) + assertFalse("json verdict stays null for a gated run", r.toJson().contains("\"verdict\": \"ok\"")) + } + @Test fun `gate folds the three signals worst-first`() { assertEquals(DiagnosticGate.VPN_OFF, resolveDiagnosticGate(vpnActive = false, selfRouted = true, selfNeedsRestart = false))