From b9c0762f236167511632e1aeb50001d9c6dd4b28 Mon Sep 17 00:00:00 2001 From: "Timur Kh." Date: Thu, 22 Oct 2026 17:22:12 +0300 Subject: [PATCH] android: the website guard sees what it promised to see MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four defects in the capture-and-match path, each able to look like "the blocker works incorrectly" on its own. Masks were unenforced exactly while online. The online matcher was built with `blockedDomains = dto.allDomains`, and `all_domains` is concrete hostnames only — a mask is precisely what the backend could *not* expand, and it comes back in `unexpandable`. Overwriting the raw entries emptied `DomainMatcher.rawEntries`, so step 3 of `decide()` — the step a hosts file cannot do — was dead code with the network up. `*gram*` blocked only offline, while the Guard screen and the README both said it worked on the phone. The rules now go in unchanged, with `all_domains` as `expandedHosts`, and `unexpandable` + a newly-read `allowed` folded in so masks and exceptions survive a process that has never read the settings document. The event-text path never matched anything. `event.text` is a list and was joined with a space before being handed over; `normalizeHost` rejects anything containing a space, which is correct of it and left the as-you-type capture dead for every event carrying more than one item. Each item is offered on its own now. Any editable node counted as an address bar — including every text field the *page* renders. Typing a blocked hostname into a search box, a form or a chat ejected you to the launcher, which is worse than a miss: it throws you off a page you are allowed to be on. An unlabelled editable node is now believed only near the top of the tree, a node whose view id names a URL field is believed anywhere and ends the walk, and web-content containers are not descended into at all — which also stops a content-heavy tab from spending the 250-node budget before the toolbar is reached. The guard armed up to 45 seconds late. `active` only changed on the blocker's own poll, which is 45s apart while idle — the exact gap between tapping "Start focus" and opening the thing you blocked. The application now subscribes the guard to the timer, and `onTimerChanged()` applies `:core`'s local verdict at once while the server is asked in the background. The rules are also read once per `start()` rather than only from MainActivity and the Guard screen. 9 new tests. The per-node judgement moved out of the traversal to be reachable from them. Co-Authored-By: Claude Opus 5 --- .../kadence/android/KadenceApplication.kt | 13 +++ .../blocker/BlockerAccessibilityService.kt | 85 +++++++++++++++++-- .../kadence/android/blocker/BlockerEngine.kt | 43 +++++++++- .../tech/kadence/android/data/dto/Dtos.kt | 7 ++ .../tech/kadence/android/BlockerEngineTest.kt | 53 +++++++++++- .../kotlin/tech/kadence/android/UrlBarTest.kt | 77 +++++++++++++++++ .../fixtures/blocker_policy_active.json | 1 + .../fixtures/blocker_policy_inactive.json | 1 + 8 files changed, 267 insertions(+), 13 deletions(-) create mode 100644 android/app/src/test/kotlin/tech/kadence/android/UrlBarTest.kt diff --git a/android/app/src/main/kotlin/tech/kadence/android/KadenceApplication.kt b/android/app/src/main/kotlin/tech/kadence/android/KadenceApplication.kt index 4120072..f9e936b 100644 --- a/android/app/src/main/kotlin/tech/kadence/android/KadenceApplication.kt +++ b/android/app/src/main/kotlin/tech/kadence/android/KadenceApplication.kt @@ -24,6 +24,7 @@ import android.util.Log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import tech.kadence.android.blocker.BlockerEngine @@ -123,6 +124,18 @@ class KadenceApplication : Application() { } } + // Starting or ending an interval arms or releases the guard immediately. Without + // this the blocker only noticed on its own poll — 45s apart while idle, which is + // exactly the window between "Start focus" and opening the thing you blocked. + // `drop(1)` because `blocker.start()` has already asked once. + scope.launch { + engine.state + .map { Triple(it.sessionId, it.status, it.timerEnabled) } + .distinctUntilChanged() + .drop(1) + .collect { blocker.onTimerChanged() } + } + // Re-point the HTTP client's cookie jar when the backend URL changes, so a // session for the old host is not offered to the new one. scope.launch { diff --git a/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerAccessibilityService.kt b/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerAccessibilityService.kt index 3dfa6df..9f781e0 100644 --- a/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerAccessibilityService.kt +++ b/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerAccessibilityService.kt @@ -61,9 +61,21 @@ private val BROWSER_PACKAGES = setOf( /** View ids that name an address bar, for the browsers that expose them. */ private val URL_ID_HINTS = listOf("url", "omnibox", "address", "search_box_text") +/** Containers whose text belongs to the *page*, not to the browser. */ +private val WEB_CONTENT_MARKERS = listOf("webview", "geckoview", "contentview") + private const val MAX_NODES = 250 private const val MAX_DEPTH = 25 +/** + * How deep an unlabelled editable node may be and still be believed. + * + * A toolbar sits a handful of levels under the root. Beyond that an editable field is + * the page's own — a search box, a comment field — and reading it is how typing + * "instagram.com" into Google became a blocked visit. + */ +private const val MAX_BAR_DEPTH = 8 + class BlockerAccessibilityService : AccessibilityService() { private val engine: BlockerEngine? get() = (application as? KadenceApplication)?.blocker @@ -128,7 +140,13 @@ class BlockerAccessibilityService : AccessibilityService() { // The event's own text covers the bar-as-you-type case; the tree walk covers the // bar a second later, once the page has loaded and focus moved to the content. val candidates = mutableListOf() - if (event.text.isNotEmpty()) candidates += event.text.joinToString(" ") + // Each item on its own. Joined with a space they became "YouTube youtube.com", + // and `normalizeHost` rejects anything with a space in it — which is correct of + // it, and quietly made this whole path dead for every multi-item event. + for (piece in event.text) { + val text = piece?.toString()?.trim() + if (!text.isNullOrEmpty()) candidates += text + } candidates += urlCandidates(rootInActiveWindow) for (candidate in candidates) { if (candidate.isNotBlank()) blocker.onUrlSeen(candidate, packageName) @@ -165,8 +183,15 @@ class BlockerAccessibilityService : AccessibilityService() { * Address-bar text, found with a bounded breadth-first walk. * * Bounded because a browser's tree is enormous and this runs on every content - * change; the bar is always near the root, so [MAX_DEPTH] is generous rather than - * tight. + * change; the bar is always near the root, so the walk stops the moment it finds a + * node whose view id names one, and never descends into the page. + * + * The page is the important exclusion. An earlier version accepted any + * `node.isEditable`, which is every text input in the window — including the ones + * belonging to the site. Typing a blocked hostname into a search box, a form or a + * chat then ejected the user to the launcher, which is both wrong and alarming. + * Now an unlabelled editable node is believed only near the top of the tree, and + * never inside a web-content container. */ private fun urlCandidates(root: AccessibilityNodeInfo?): List { root ?: return emptyList() @@ -177,6 +202,10 @@ class BlockerAccessibilityService : AccessibilityService() { while (queue.isNotEmpty() && visited < MAX_NODES) { val (node, depth) = queue.removeFirst() visited++ + // Not into the page, and not spending the node budget on it either: a + // content-heavy tab could exhaust [MAX_NODES] before the toolbar was + // reached, depending on the order the children came back in. + if (isWebContent(node)) continue if (depth < MAX_DEPTH) { for (index in 0 until node.childCount) { node.getChild(index)?.let { queue.addLast(it to depth + 1) } @@ -184,13 +213,53 @@ class BlockerAccessibilityService : AccessibilityService() { } val text = node.text?.toString()?.trim() if (text.isNullOrEmpty()) continue - if (node.isEditable || looksLikeAddressBar(node.viewIdResourceName)) found += text + when (barCandidacy(node.viewIdResourceName, node.isEditable, depth)) { + // The browser told us which node this is. Nothing else is worth reading. + Candidacy.NAMED -> return listOf(text) + Candidacy.PLAUSIBLE -> found += text + Candidacy.NO -> Unit + } } return found } - private fun looksLikeAddressBar(viewId: String?): Boolean { - val id = viewId?.lowercase() ?: return false - return URL_ID_HINTS.any { id.contains(it) } - } + private fun isWebContent(node: AccessibilityNodeInfo): Boolean = + isWebContent(node.className?.toString()) +} + +/** How much a node's text looks like it came from an address bar. */ +internal enum class Candidacy { + /** The view id names a URL field: this is the bar, believe it and stop looking. */ + NAMED, + + /** An editable field near the top of the tree — a toolbar, most likely. */ + PLAUSIBLE, + + /** Anything else, including every editable field inside the page. */ + NO, +} + +/** + * Whether one node's text may be treated as an address bar's. + * + * Extracted from the walk because this, not the traversal, is where the false positives + * came from: accepting any `isEditable` node meant a search box or a comment field in + * the page counted as a visited hostname, and a blocked word typed into Google ejected + * the user to the launcher. + */ +internal fun barCandidacy(viewId: String?, editable: Boolean, depth: Int): Candidacy = when { + looksLikeAddressBar(viewId) -> Candidacy.NAMED + editable && depth <= MAX_BAR_DEPTH -> Candidacy.PLAUSIBLE + else -> Candidacy.NO +} + +internal fun looksLikeAddressBar(viewId: String?): Boolean { + val id = viewId?.lowercase() ?: return false + return URL_ID_HINTS.any { id.contains(it) } +} + +/** Whether a node is the rendered page rather than the browser's own chrome. */ +internal fun isWebContent(className: String?): Boolean { + val name = className?.lowercase() ?: return false + return WEB_CONTENT_MARKERS.any { name.contains(it) } } diff --git a/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerEngine.kt b/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerEngine.kt index 8e67732..b1e4baa 100644 --- a/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerEngine.kt +++ b/android/app/src/main/kotlin/tech/kadence/android/blocker/BlockerEngine.kt @@ -200,6 +200,23 @@ class BlockerEngine( watchJob = null } + /** + * The timer moved, so the verdict may have — decide again at once. + * + * `during_work` is a function of the running interval, but `active` only ever + * changed on a policy poll: fifteen seconds while the guard is on and *forty-five* + * while it is off. Tapping Start therefore bought up to forty-five seconds of + * unguarded time, which is long enough to open the thing you were blocking. + * + * The local verdict is applied immediately — same function and same reason strings + * as the backend's, which is the whole point of `:core`'s [evaluatePolicy] — and the + * server is asked in the background, because it is still the one that owns the answer. + */ + fun onTimerChanged() { + if (rules != null) applyOfflineVerdict(_state.value.connection) + refreshNow() + } + /** On app foreground, and after the screen edits the policy. */ fun refreshNow() = scope.launch { poll() @@ -326,6 +343,11 @@ class BlockerEngine( // ---- polling ------------------------------------------------------------ private suspend fun pollLoop() { + // The rules, not just the verdict, and once per start. Their only other callers + // are `MainActivity` and the Guard screen, so a process Android started to bind + // the accessibility service held no rules at all — leaving the offline fallback, + // which is built from them, enforcing nothing. + loadSettings() while (scope.isActive) { poll() refreshPermissions() @@ -477,12 +499,25 @@ class BlockerEngine( offlineDomains = DomainMatcher(current) offlineApps = AppMatcher(current.blockedApps) - // Online, the server's `all_domains` is the expanded, allow-subtracted list — - // but the allow masks still have to be applied for pattern entries, and the raw - // entries are what step 3 of `decide()` matches patterns against. + // Online, the server's `all_domains` is the expanded, allow-subtracted list — it + // goes in as `expandedHosts`, and the rules go in *unchanged*. + // + // They used to go in as `blockedDomains = dto.allDomains`, which quietly emptied + // `DomainMatcher.rawEntries`: `all_domains` holds concrete hostnames only, + // because a mask is what the backend could not expand and returns separately in + // `unexpandable`. Step 3 of `decide()` — the step a hosts file cannot do — was + // therefore dead code exactly while the phone was online, so `*gram*` blocked + // only when the network was gone, while the Guard screen promised the opposite. + // + // `unexpandable` and `allowed` are folded in so the masks and the exceptions + // survive even before the settings document has been read — which is the state + // every process that Android started for the accessibility service is in. policy?.let { dto -> onlineDomains = DomainMatcher( - rules = current.copy(blockedDomains = dto.allDomains), + rules = current.copy( + blockedDomains = (current.blockedDomains + dto.unexpandable).distinct(), + allowedDomains = (current.allowedDomains + dto.allowed).distinct(), + ), expandedHosts = dto.allDomains, ) onlineApps = AppMatcher(dto.allApps) diff --git a/android/app/src/main/kotlin/tech/kadence/android/data/dto/Dtos.kt b/android/app/src/main/kotlin/tech/kadence/android/data/dto/Dtos.kt index 25ae35c..a94884b 100644 --- a/android/app/src/main/kotlin/tech/kadence/android/data/dto/Dtos.kt +++ b/android/app/src/main/kotlin/tech/kadence/android/data/dto/Dtos.kt @@ -126,6 +126,13 @@ data class BlockerPolicyDto( /** Likewise empty when inactive. */ val apps: List = emptyList(), val unexpandable: List = emptyList(), + /** + * The exceptions, as written. `all_domains` already has them subtracted, but the + * mask step of [tech.kadence.core.DomainMatcher] still has to consult them — and it + * cannot read the settings document when the process was started by the + * accessibility service. + */ + val allowed: List = emptyList(), /** Subdomain expansions the server refused — a bare `*`, say, which is not a label. */ @SerialName("invalid_expansions") val invalidExpansions: List = emptyList(), @SerialName("kill_apps") val killApps: Boolean = true, diff --git a/android/app/src/test/kotlin/tech/kadence/android/BlockerEngineTest.kt b/android/app/src/test/kotlin/tech/kadence/android/BlockerEngineTest.kt index d05dab4..8bec3b0 100644 --- a/android/app/src/test/kotlin/tech/kadence/android/BlockerEngineTest.kt +++ b/android/app/src/test/kotlin/tech/kadence/android/BlockerEngineTest.kt @@ -38,6 +38,14 @@ class BlockerEngineTest { private lateinit var server: MockWebServer private var clock = 1_000_000L + + /** + * Whether `GET /api/settings` answers. + * + * It does not, in the one case that matters most: a process Android started to bind + * the accessibility service, with no Activity to have ever read the document. + */ + private var settingsAvailable = true private var bounces = 0 private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined) @@ -76,7 +84,8 @@ class BlockerEngineTest { val path = request.url.encodedPath val body = when { path.endsWith("/api/blocker/policy") -> fixture("blocker_policy_active.json") - path.endsWith("/api/settings") -> fixture("settings.json") + path.endsWith("/api/settings") -> + if (settingsAvailable) fixture("settings.json") else return error() else -> "{}" } return MockResponse.Builder() @@ -85,6 +94,8 @@ class BlockerEngineTest { .body(body) .build() } + + private fun error() = MockResponse.Builder().code(503).body("nope").build() } } @@ -171,4 +182,44 @@ class BlockerEngineTest { assertEquals(0, bounces) } + + @Test + fun `a mask the server could not expand is enforced while online`() { + // The bug: `all_domains` holds concrete hostnames, because a mask is precisely + // what the backend could not expand — it comes back in `unexpandable`. Building + // the online matcher from `all_domains` therefore dropped every mask, so + // `*gram*` blocked only when the phone was offline, while the Guard screen and + // the README both said it worked. + val engine = engine() + + engine.onUrlSeen("telegram.org", "com.android.chrome") + + assertEquals(1, bounces, "*gram* is on the list and the phone is online") + } + + @Test + fun `the masks survive a process that never read the settings document`() { + settingsAvailable = false + val engine = engine() + + engine.onUrlSeen("telegram.org", "com.android.chrome") + + assertEquals(1, bounces, "the policy's own `unexpandable` is enough") + } + + @Test + fun `an exception beats a mask, with or without the settings document`() { + settingsAvailable = false + val engine = engine() + + // `*gram*` catches `instagram.com`, and `instagram.com` is an exception. The + // expanded host list cannot settle this — a mask is not in it — so the mask step + // has to consult the allow list itself, and with no settings document the only + // place it can read one is the policy's own `allowed`. + engine.onUrlSeen("instagram.com", "com.android.chrome") + assertEquals(0, bounces, "an exception beats the mask that would catch it") + + engine.onUrlSeen("telegram.org", "com.android.chrome") + assertEquals(1, bounces, "and nothing else is let off with it") + } } diff --git a/android/app/src/test/kotlin/tech/kadence/android/UrlBarTest.kt b/android/app/src/test/kotlin/tech/kadence/android/UrlBarTest.kt new file mode 100644 index 0000000..cc18451 --- /dev/null +++ b/android/app/src/test/kotlin/tech/kadence/android/UrlBarTest.kt @@ -0,0 +1,77 @@ +package tech.kadence.android + +import org.junit.Test +import tech.kadence.android.blocker.Candidacy +import tech.kadence.android.blocker.barCandidacy +import tech.kadence.android.blocker.isWebContent +import tech.kadence.android.blocker.looksLikeAddressBar +import tech.kadence.core.normalizeHost +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Which text in a browser's window is the address bar's. + * + * The guard ejects you from a blocked page, so a false positive is worse than a miss: + * it throws you off a page you are allowed to be on, with no explanation beyond a + * notification. The service used to accept any editable node in the window, which + * includes every text field the *page* renders — so typing a blocked word into a search + * box was indistinguishable from visiting the site. + */ +class UrlBarTest { + + @Test + fun `a node whose view id names a url field is the bar`() { + assertTrue(looksLikeAddressBar("com.android.chrome:id/url_bar")) + assertTrue(looksLikeAddressBar("org.mozilla.firefox:id/mozac_browser_toolbar_url_view")) + assertTrue(looksLikeAddressBar("com.android.chrome:id/search_box_text")) + assertEquals( + Candidacy.NAMED, + barCandidacy("com.android.chrome:id/url_bar", editable = false, depth = 40), + "a named bar is believed however deep the tree put it", + ) + } + + @Test + fun `an ordinary view id is not`() { + assertFalse(looksLikeAddressBar("com.android.chrome:id/title")) + assertFalse(looksLikeAddressBar(null)) + assertFalse(looksLikeAddressBar("")) + } + + @Test + fun `an editable field is believed near the toolbar and nowhere else`() { + assertEquals(Candidacy.PLAUSIBLE, barCandidacy(null, editable = true, depth = 3)) + assertEquals( + Candidacy.NO, + barCandidacy(null, editable = true, depth = 14), + "a text input this deep belongs to the page, not the browser", + ) + } + + @Test + fun `a node that is neither named nor editable is never a candidate`() { + assertEquals(Candidacy.NO, barCandidacy("com.android.chrome:id/line_1", false, 2)) + } + + @Test + fun `the rendered page is recognised so the walk can skip it`() { + assertTrue(isWebContent("android.webkit.WebView")) + assertTrue(isWebContent("org.mozilla.geckoview.GeckoView")) + assertTrue(isWebContent("org.chromium.content.browser.ContentView")) + assertFalse(isWebContent("android.widget.EditText")) + assertFalse(isWebContent(null)) + } + + @Test + fun `what a window event actually carries, one item at a time`() { + // `event.text` is a list, and it used to be joined with a space before being + // handed over — which `normalizeHost` rejects outright, so the whole path was + // dead for every event carrying more than one item. + val event = listOf("YouTube", "youtube.com") + assertNull(normalizeHost(event.joinToString(" ")), "the old behaviour: nothing") + assertEquals("youtube.com", event.firstNotNullOf { normalizeHost(it) }) + } +} diff --git a/android/app/src/test/resources/fixtures/blocker_policy_active.json b/android/app/src/test/resources/fixtures/blocker_policy_active.json index 35d4848..b251479 100644 --- a/android/app/src/test/resources/fixtures/blocker_policy_active.json +++ b/android/app/src/test/resources/fixtures/blocker_policy_active.json @@ -7,6 +7,7 @@ "redirect_ip": "127.0.0.1", "domains": ["youtube.com", "www.youtube.com", "m.youtube.com", "reddit.com", "www.reddit.com", "m.reddit.com"], "apps": ["steam*", "*discord*"], + "allowed": ["music.youtube.com", "instagram.com"], "unexpandable": ["*gram*"], "invalid_expansions": ["not a label"], "kill_apps": true, diff --git a/android/app/src/test/resources/fixtures/blocker_policy_inactive.json b/android/app/src/test/resources/fixtures/blocker_policy_inactive.json index d35db0a..16b8b48 100644 --- a/android/app/src/test/resources/fixtures/blocker_policy_inactive.json +++ b/android/app/src/test/resources/fixtures/blocker_policy_inactive.json @@ -7,6 +7,7 @@ "redirect_ip": "127.0.0.1", "domains": [], "apps": [], + "allowed": ["music.youtube.com", "instagram.com"], "unexpandable": ["*gram*"], "kill_apps": true, "notify_on_block": true,