Enable pinch-to-zoom in Hermes WebUI - #111
Conversation
Review at
|
|
Addressed in d1e4704. I added an execution-time origin guard around every script in I also added coverage for:
The updated CI run is fully green, including unit tests, lint, debug APK build, injected-JS checks, and Android instrumentation on API 35 and API 36: |
…tructor The runtime-script guard resolved its trusted origin with `new URL(...)`, but `URL` is a page-controlled global. A foreign origin can replace it before the asynchronously-evaluated script runs, return its own origin from the constructor, and satisfy the check — so the guarded payload executes off-origin, which is the exact hole the guard was added to close. Reproduced in a sandbox: with `window.URL` shadowed, a hostile page ran the guarded payload. Compare `window.location.origin` against a natively-canonicalized string literal instead, so nothing the page controls participates in the decision. The literal must match what a browser reports, which is NOT the WebViewCompat allow-rule: `documentStartOriginRule` keeps an explicitly-specified default port (`http://host:80`) while the browser drops it. Add `UrlOrigins.pageOrigin` for the browser-shaped origin and use it at the guard's call site. Tests: an instrumentation case that replaces `window.URL` on a provider page and asserts the viewport is untouched; unit cases covering default-port dropping, non-default ports, IPv6 bracketing, host lowercasing, and non-web scheme rejection. Mutation-verified — reverting the guard to the `new URL(...)` shape makes the bypass case regress. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
The literal the origin guard compares against must be byte-identical to what the page reports in `window.location.origin`, or the guard never matches and every runtime shim is silently suppressed for that server. `java.net.URI` does not apply WHATWG host parsing, so two configurable forms diverged: a numeric host (`http://2130706433` and `http://0x7f000001`, which browsers resolve to `http://127.0.0.1`) and an expanded IPv6 literal (`http://[0:0:0:0:0:0:0:1]`, which browsers compress to `http://[::1]`). Canonicalize the host natively before building the literal: WHATWG numeric-IPv4 parsing (decimal/octal/hex, with a short last part filling the remaining bytes) and RFC 5952 IPv6 compression. A host we cannot canonicalize confidently returns null, so the caller skips injection rather than emitting an unmatchable literal. IPv6 compression is deliberately pure string handling rather than InetAddress: this runs on the main thread during script injection and must never risk a name-resolution call. Verified by porting the implementation to JS and differential-testing it against the platform's own WHATWG URL parser: 21 hand-picked cases plus 2600 fuzzed inputs (random IPv6 in exploded and compressed spellings, random IPv4 in dotted/integer/hex forms, DNS names, default and non-default ports) — zero divergences. Rejected hosts also match: every input we return null for is one the browser's URL parser throws on. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
The assertion dropped the `:9000` port when transcribing from the differential oracle, so it expected `http://[2001:db8::1428:57ab]` where both a real browser and the implementation produce `http://[2001:db8::1428:57ab]:9000`. The test was wrong, not the code — CI caught it (169/170 passing). Re-checked EVERY pageOrigin assertion in this file against the platform's own WHATWG URL parser; all 13 expected values now agree with the browser, and each null-case is one the browser's parser also rejects. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
Differential-testing the canonicalizer against a real headless Chromium (not Node's URL) surfaced four remaining divergences. Each would emit a literal the guard can never match, silently suppressing every runtime shim: - `http://0x` — an empty hex payload is zero, so Chromium yields `http://0.0.0.0`. A bare `0` prefix is likewise octal-with-empty-payload. - `http://2130706433.` / `http://127.0.0.1.` — a NUMERIC host drops one trailing dot. A DNS name does NOT: Chromium keeps `http://hermes.example.com.` verbatim, so the two cases are handled separately. - `http://[::ffff:127.0.0.010]` — the embedded dotted-quad uses the same radix-aware part parsing as a bare IPv4 host, so octal `010` is 8 (`…:7f00:8`), not decimal 10 (`…:7f00:a`). Chromium also accepts components longer than three characters when they carry leading zeroes. - `http://999.1.1.1`, `http://1.2.3.4.5` — these are numeric candidates the browser REJECTS, not DNS names. Previously they fell through and returned the raw host. They now fail closed via an INVALID_NUMERIC_HOST sentinel so the caller skips injection. Also adds the mutation-coverage case the reviewer asked for: `[1:2:3:4:5:6:7:8:9]` is admitted by java.net.URI, so it actually reaches canonicalBrowserHost and the assertion fails if canonicalization is reduced to `return host`. Verified against real Chromium over CDP: 2627 probes (hand-picked edge cases plus fuzzed IPv6 in exploded/compressed spellings and IPv4 in dotted/integer/hex forms) with zero divergences, and all 23 value assertions in UrlPolicyTest re-checked against Chromium's own output. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
Running the compiled canonicalizer against a real headless Chromium surfaced a class java.net.URI silently drops: it is RFC 2396-strict and returns a null host (and port -1) for spellings a browser accepts — a trailing-dot IPv4 (`127.0.0.1.`, `2130706433.:8080`) and `0x.0x.0x.0x`. Those hit `?: return null` and disabled every runtime shim for such a configured server. Add a raw-authority fallback used only when URI yields no host: read the host (and port) straight from the authority substring, stripping userinfo to match a browser's origin (`user:pass@host` → `host`), then run the same WHATWG canonicalization. Anything unreadable still fails closed. Verification tightened to remove the CI round-trips that caught my earlier test typos: the production Kotlin is now compiled locally (kotlinc + JDK 21) and run against real Chromium over CDP across 2750 probes with ZERO divergences and zero wrong-value results (every remaining difference is a fail-closed null on a host Chromium also rejects). All 37 assertions in UrlPolicyTest were generated from the compiled Kotlin's own output, so the suite matches the implementation which matches the browser. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
…fallback Two ways the raw-authority fallback could synthesize a literal that disagrees with a browser's location.origin (both found by attacking the compiled Kotlin against real Chromium): - Ports were parsed with toIntOrNull, which accepts signs and overflows. `:-1`, `:+80`, `:65536`, `:8_7` all produced a valid-looking origin where Chromium rejects the URL. Parse ports as ASCII-digits-only in 0..65535 on the fallback path AND range-check uri.port on the normal branch; fail closed otherwise. - The fallback passed a host through verbatim, but it only exists to recover the numeric/ASCII hosts java.net.URI wrongly rejects — it does not implement WHATWG percent-decoding or IDNA. `foo%2ebar` and `münchen.de` would emit an un-decoded/un-punycoded literal that never matches the browser's origin. The fallback now returns null on any host carrying `%` or a non-ASCII character. (A real self-hosted server URL is an IP or an ASCII hostname, both of which java.net.URI already accepts on the normal path — so this loses no legitimate case; it only refuses to guess where a browser would decode.) Every residual difference from a browser is now a fail-closed null (skip injection), never a wrong literal. Verified against real Chromium via the compiled production Kotlin across 2770 probes (base + fuzz + adversarial authorities + IDNA/percent/port edge cases): zero wrong-value results. All 45 UrlPolicyTest assertions were regenerated from the compiled Kotlin's own output. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
…losed The canonicalizer treated a host as a DNS name whenever IPv4 parsing failed, so `http://foo.1`, `http://example.99`, `http://09`, `http://1..2.3`, and `http://1.2.3.09` were passed through verbatim — but a browser REJECTS all of them, so the emitted literal never matched location.origin (a wrong-value, not a fail-closed null). WHATWG's rule: if a host's last label ends in a number, the whole host MUST parse as a valid IPv4 address or the host is invalid. Implement that directly — detect "ends in a number" first (last label all-digits or a parseable IPv4 part), and if so, any parse/range/empty-label failure returns INVALID_NUMERIC_HOST (caller fails closed) instead of a DNS pass-through. A host that does not end in a number stays an ordinary DNS name. Also reject an empty IPv4 part so interior empty labels (`1..2.3`) fail rather than parse. Verified against real Chromium via the compiled production Kotlin across 3379 probes (base + fuzz + numeric-ending edge cases + DNS names ending in digits): ZERO wrong-value AND zero conservative-null — the implementation now matches Chromium exactly on every input. All 54 UrlPolicyTest assertions regenerated from the compiled Kotlin's output. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
Two more wrong-value cases the compiled-Kotlin-vs-Chromium diff surfaced: - Overflowing hex like `http://0x8000000000000000` was mistaken for a DNS name: the "ends in a number" test went through parseIpv4Part, which returned null on Long overflow, so the host fell through to a pass-through. Split the concern: `ipv4PartLooksNumeric` is a pure SYNTAX test (a digit run, or `0x`+valid-hex) that decides "ends in a number" independent of magnitude and octal validity; parseIpv4Part still enforces value validity. An overflowing-but-numeric host now fails closed. This also fixes `09` (looks numeric, is an invalid octal → the browser rejects it → we must too), which the previous round regressed. - The raw-authority fallback emitted browser-divergent literals for hosts with a character a browser percent-encodes (`foo*bar` → `foo%2Abar`). The fallback does not encode, so it now accepts a host verbatim ONLY when every character is one a browser also keeps verbatim: the LDH set plus `_` and `~` (derived by probing all 94 printable-ASCII chars against Chromium). This RECOVERS real hostnames java.net.URI wrongly rejects (`my_host.local`, `foo_bar` — common on Docker/internal networks) while failing closed on the exotic-char hosts no self-hosted server uses. Verified against real Chromium via the compiled production Kotlin across 4074 probes (base + fuzz + numeric-ending + overflow + all-ASCII-char host probes + underscore/tilde recovery): ZERO wrong-value results. All 62 UrlPolicyTest assertions regenerated from the compiled Kotlin's own output. Co-authored-by: sacgsxr <sacgsxr@users.noreply.github.com>
Re-review at
|
2 participants
Summary
maximum-scale=1, user-scalable=noviewport directives on the trusted WebUI originWhy
Hermes WebUI currently disables browser scaling in its viewport metadata. On Android this prevents users from pinch-zooming chat content, which is useful for accessibility and for inspecting dense content on a phone.
The script remains scoped to the configured Hermes WebUI origin; OAuth/provider pages are not modified.
Verification
./gradlew --no-daemon testDebugUnitTest lintDebug assembleDebugpython3 -m unittest discover -s tools/tests -p "test_*.py" -vpython3 tools/check_markdown.pypython3 tools/extract_webui_scripts.pynpx --yes eslint@^10 --no-config-lookup -c eslint.runtime-guard.config.mjs "build/webui-scripts/**/*.js"