diff --git a/README.md b/README.md index ca4eec4..98a8692 100644 --- a/README.md +++ b/README.md @@ -67,8 +67,11 @@ native desktop app enforces them on your machine: `*` to that list and "every" means every, answered by the DNS stub rather than by guessed hostnames. A hosts file has no wildcards at all, so `*gram*` is reported honestly as unenforceable on the machine rather than silently ignored. - Exceptions take masks too, so `*.google.com` blocked plus `*.docs.google.com` - allowed does what you would expect. + Exceptions read exactly like blocks: a plain one covers its subdomains + (`youtube.com` allowed spares `music.youtube.com`, the same way blocking it + catches them), and a masked one is a mask — so `*.google.com` blocked plus + `*.docs.google.com` allowed does what you would expect, and `docs.*` allowed + is how you spare something narrower than a whole subtree. * **Applications** — `/proc` is watched and blocked apps are closed (or just reported, if you prefer a warning). Entries here are globs as well: `steam*`, `*discord*`, matched against the process name and its command line. diff --git a/android/core/src/main/kotlin/tech/kadence/core/Blocker.kt b/android/core/src/main/kotlin/tech/kadence/core/Blocker.kt index 528b610..1061f66 100644 --- a/android/core/src/main/kotlin/tech/kadence/core/Blocker.kt +++ b/android/core/src/main/kotlin/tech/kadence/core/Blocker.kt @@ -85,9 +85,20 @@ fun expandEntry(raw: String, labels: List, wildcard: Boolean = false): P return listOf(entry, "www.$entry", "m.$entry") to null } -/** Whether an allow-list entry — itself maskable — covers this hostname. */ -fun isAllowed(host: String, allowMasks: List): Boolean = - allowMasks.any { globMatches(it, host) } +/** + * Whether an allow-list entry — itself maskable — covers this hostname. + * + * A plain entry covers itself and its subdomains, matching what a plain entry does on + * the block side (`youtube.com` blocked catches `music.youtube.com`). An exception used + * to be glob-matched whatever it looked like, so the two halves of one list read + * identically and behaved differently. A masked entry is glob-matched as before, which + * is still how you allow something narrower than a whole subtree. + * + * Parity with `blocker_service.is_allowed`; `BlockerTest` pins it. + */ +fun isAllowed(host: String, allowMasks: List): Boolean = allowMasks.any { entry -> + if (isMask(entry)) globMatches(entry, host) else host == entry || host.endsWith(".$entry") +} /** Normalised, de-duplicated allow masks, the way `expand_domains()` prepares them. */ fun allowMasksOf(rules: BlockerRules): List = rules.allowedDomains diff --git a/android/core/src/test/kotlin/tech/kadence/core/BlockerTest.kt b/android/core/src/test/kotlin/tech/kadence/core/BlockerTest.kt index 716f9ab..c4353af 100644 --- a/android/core/src/test/kotlin/tech/kadence/core/BlockerTest.kt +++ b/android/core/src/test/kotlin/tech/kadence/core/BlockerTest.kt @@ -275,4 +275,32 @@ class BlockerTest { assertEquals(ZoneId.systemDefault(), zoneOrDefault(null)) assertEquals(ZoneId.systemDefault(), zoneOrDefault("")) } + + @Test + fun `a plain exception covers its subdomains, like a plain block does`() { + // Parity with `blocker_service.is_allowed`. `youtube.com` blocked catches + // `music.youtube.com`; `youtube.com` allowed now spares it, instead of sparing + // only the literal string while reading exactly like the block entry. + assertTrue(isAllowed("music.youtube.com", listOf("youtube.com"))) + assertTrue(isAllowed("youtube.com", listOf("youtube.com"))) + assertFalse(isAllowed("notyoutube.com", listOf("youtube.com"))) + } + + @Test + fun `a masked exception is still a glob`() { + assertTrue(isAllowed("docs.google.com", listOf("docs.*"))) + assertTrue(isAllowed("a.docs.google.com", listOf("*.docs.google.com"))) + assertFalse(isAllowed("sheets.google.com", listOf("docs.*"))) + } + + @Test + fun `an exception subtracts its subtree from the expanded list`() { + val rules = BlockerRules( + blockedDomains = listOf("*.youtube.com", "reddit.com"), + allowedDomains = listOf("youtube.com"), + subdomainExpansions = listOf("www", "m", "music"), + ) + + assertEquals(listOf("reddit.com", "www.reddit.com", "m.reddit.com"), expandDomains(rules)) + } } diff --git a/backend/app/services/blocker_service.py b/backend/app/services/blocker_service.py index 2633c8d..5cd8edf 100644 --- a/backend/app/services/blocker_service.py +++ b/backend/app/services/blocker_service.py @@ -74,9 +74,34 @@ def parse_expansions(raw: list[str]) -> Expansion: return Expansion(labels=labels, wildcard=wildcard, rejected=rejected) +#: What makes an entry a mask rather than a name, `fnmatch`'s own metacharacters. +_WILDCARDS = "*?[" + + +def is_mask(entry: str) -> bool: + return any(char in entry for char in _WILDCARDS) + + def is_allowed(host: str, allow_masks: list[str]) -> bool: - """Whether an allow-list entry (plain or masked) covers this hostname.""" - return any(fnmatch(host, mask) for mask in allow_masks) + """Whether an allow-list entry (plain or masked) covers this hostname. + + A plain entry covers itself *and its subdomains*, which is the same rule a + plain entry follows on the block side — `youtube.com` blocked catches + `music.youtube.com`. It did not used to: an exception was glob-matched, so a + plain one meant exactly one hostname while a plain block meant a subtree, and + the two halves of one list read the same and behaved differently. + + A masked entry is glob-matched as it always was, so `docs.*` and + `*.docs.google.com` are unchanged — and still the way to allow something + *narrower* than a subtree. + """ + for mask in allow_masks: + if is_mask(mask): + if fnmatch(host, mask): + return True + elif host == mask or host.endswith(f".{mask}"): + return True + return False def expand_entry(raw: str, expansion: Expansion) -> tuple[list[str], str | None]: diff --git a/backend/tests/test_blocker.py b/backend/tests/test_blocker.py index 9c398a6..a9116be 100644 --- a/backend/tests/test_blocker.py +++ b/backend/tests/test_blocker.py @@ -70,6 +70,32 @@ def test_masked_subdomains_are_expanded(session, settings): assert hosts == ["google.com", "www.google.com", "mail.google.com"] +def test_a_plain_exception_covers_its_subdomains_like_a_plain_block_does(session, settings): + """The two halves of one list have to read the same way. + + `youtube.com` on the block list catches `music.youtube.com`. For a long time + `youtube.com` on the *allow* list caught only itself, because exceptions were + glob-matched — so the same text meant a subtree in one box and one hostname + in the other, with nothing saying so. + """ + settings.blocker.blocked_domains = ["*.youtube.com", "reddit.com"] + settings.blocker.subdomain_expansions = ["www", "m", "music"] + settings.blocker.allowed_domains = ["youtube.com"] + + hosts = blocker_service.expand_domains(settings) + + assert hosts == ["reddit.com", "www.reddit.com", "m.reddit.com"] + assert blocker_service.is_allowed("music.youtube.com", ["youtube.com"]) + assert not blocker_service.is_allowed("notyoutube.com", ["youtube.com"]) + + +def test_a_masked_exception_is_still_a_glob(session, settings): + """Which is how you allow something narrower than a whole subtree.""" + assert blocker_service.is_allowed("docs.google.com", ["docs.*"]) + assert blocker_service.is_allowed("a.docs.google.com", ["*.docs.google.com"]) + assert not blocker_service.is_allowed("sheets.google.com", ["docs.*"]) + + def test_allow_masks_carve_holes_in_block_masks(session, settings): settings.blocker.blocked_domains = ["*.google.com", "reddit.com"] settings.blocker.subdomain_expansions = ["www", "docs"] diff --git a/desktop/kadence_desktop/blocking/resolver.py b/desktop/kadence_desktop/blocking/resolver.py index ac4ddb4..5c02b5c 100644 --- a/desktop/kadence_desktop/blocking/resolver.py +++ b/desktop/kadence_desktop/blocking/resolver.py @@ -116,6 +116,23 @@ def _matches(name: str, patterns: list[str]) -> bool: return any(fnmatch.fnmatchcase(name, pattern) for pattern in patterns) +def _allowed(name: str, entries: list[str]) -> bool: + """Whether an exception covers this name. + + Not [_matches]: a plain exception covers its subdomains, the same way a plain + block-list entry does. Parity with `blocker_service.is_allowed` and `:core`'s + `isAllowed` — the stub is the third place that decides this, and an exception + that works in the browser but not at the resolver is worse than neither. + """ + for entry in entries: + if any(char in entry for char in "*?["): + if fnmatch.fnmatchcase(name, entry): + return True + elif name == entry or name.endswith(f".{entry}"): + return True + return False + + # ========================================================================== # The DNS wire format, as little of it as a blocker needs # ========================================================================== @@ -228,7 +245,7 @@ class _Rules: return ( self.active and bool(self.patterns) - and not _matches(name, list(self.allow)) + and not _allowed(name, list(self.allow)) and _matches(name, list(self.patterns)) ) diff --git a/frontend/src/pages/BlockerPage.tsx b/frontend/src/pages/BlockerPage.tsx index 2805ae8..63cab83 100644 --- a/frontend/src/pages/BlockerPage.tsx +++ b/frontend/src/pages/BlockerPage.tsx @@ -256,7 +256,8 @@ export default function BlockerPage({ onToast }: { onToast: (message: string) => sx={{ "& textarea": { fontFamily: "monospace", fontSize: 13, lineHeight: 1.7 } }} /> - Masks allowed here too — an exception wins over a block. + Masks allowed here too — an exception wins over a block, and a plain + entry covers its subdomains just as a blocked one does.