kai/web-terminal
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(android): make the terminal actually usable on a device

Browse Source 
The client built to an APK and 617 JVM tests passed, but nothing had ever
run on hardware, and three defects made it unusable the moment it did. All
three were invisible to the JVM suite because none of those tests
instantiate an Android View.

1. Every key press crashed. `RemoteTerminalView` bound the forked emulator
   but never installed a `TerminalViewClient`, and the stock Termux view
   dereferences `mClient` with no null guard on the first line of the paths
   a user actually hits (`onKeyDown` offset 82, `onCreateInputConnection`
   offset 0, `onKeyUp`, `onKeyPreIme`). Installing any client is not enough:
   returning false continues into `mTermSession.write()` at offset 150, and
   `mTermSession` is null forever because `TerminalSession` is final and
   forking a process is what this app must not do. So the client consumes
   the key itself and `KeyRouting` has no defer-to-stock branch at all —
   the crash is unrepresentable, not merely avoided. Termux's `KeyHandler`
   stays the authority for the key tables, so DECCKM still emits `ESC O A`.

2. The PTY never learned the real size. Nothing called
   `RemoteTerminalSession.updateSize`, and the stock fallback is dead
   (`TerminalView.updateSize` early-returns without a session), so every
   full-screen TUI rendered into 80x24. `TerminalResizeDriver` now drives
   it from real cell metrics — read through the public `getFontWidth()` /
   `getFontLineSpacing()` accessors rather than reflection, which would
   break silently under R8, or a re-measured Paint, which would drift from
   what the renderer actually draws (plan risk R5).

3. Bare-LAN connections were impossible. `network_security_config.xml` was
   a self-labelled STUB denying all cleartext while `HostEndpoint` accepts
   http and derives ws://. The format has no CIDR syntax, so the allowlist
   its own comment promised cannot be written; cleartext is permitted in
   base-config, the tunnel domain keeps a TLS block, trust anchors are
   pinned to system-only, and the guard moves to the §5.4 pairing tiers.

Adversarial review then found three more, and one it got wrong:

  - Swipe-scrolling inside an alternate-screen TUI still crashed. The touch
    path bypasses `TerminalViewClient` entirely: `doScroll` turns a scroll
    into `handleKeyCode` whenever the alternate buffer is active, and that
    dereferences `mTermSession`. Claude Code is an alternate-screen TUI, so
    this was a crash during normal use on the app's main screen.
    `TerminalScrollGesture` claims the vertical drag first and reproduces
    all three `doScroll` branches, closing the fling runnable and
    `onGenericMotionEvent` with it.
  - `autofill()` dereferences `mTermSession` unguarded while the view
    advertises itself autofillable, so any password manager crashed it.
    The subtree is excluded from the autofill structure.
  - The review asked for stock text selection to be restored. It cannot be:
    the ActionMode's own Copy and Paste handlers dereference the absent
    session, so a reachable selection UI has an NPE on its Copy button.
    Long press stays consumed and copy-out is recorded as a feature gap.

Also: the WEBTERM_TOKEN cookie is carried on REST and the WS upgrade and
sealed with Tink AEAD under an AndroidKeyStore key (fail-closed — a seal
failure persists nothing rather than falling back to plaintext), and
`allowBackup=false` keeps a 30-day shell credential off Google Drive.

Verified: ./gradlew test :app:assembleDebug -> 757 JVM tests, 0 failures.
Nothing here is device-verified yet; that is the next step.
This commit is contained in:
Yaojia Wang
2026-07-30 08:59:01 +02:00
parent c3613c2fae
commit 3e5cfdc1cf
48 changed files with 5719 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
android/app/src/main/AndroidManifest.xml
View File

@@ -12,13 +12,32 @@
<uses-permission android:name="android.permission.CAMERA" />
<uses-feature android:name="android.hardware.camera.any" android:required="false" />
<!-- Cleartext: @xml/network_security_config is the AUTHORITATIVE policy (minSdk 29, so the
platform always reads it and IGNORES android:usesCleartextTraffic). The attribute is set
to true only so the manifest does not state the opposite of what actually ships: bare-LAN
`ws://` must work (plan §6.9). The config is where the real posture lives — cleartext ON
for user-typed LAN addresses (no CIDR syntax exists to scope it), system-only trust
anchors, and cleartext still BLOCKED for *.terminal.yaojia.wang. Read its header before
changing either value; they must stay in agreement. -->
<!-- Backup: OFF, and deliberately not an exclusion list (plan §8 "app-private, uninstall-wiped,
never in backups/cloud"). Auto Backup defaults to ON and would sweep up EVERY app-private
file: the `webterm` Preferences DataStore (which holds the sealed `webterm_auth` cookie — a
full-shell credential the server keeps alive for 30 days) and the Tink keyset/ciphertext pref
files. `dataExtractionRules`/`fullBackupContent` were rejected because they are allow-by-
default: every store added later is backed up until someone remembers to exclude it, and a
silent miss here is an off-device credential leak. Nothing in this app has restore value that
justifies that risk — re-pairing a host is a QR scan, and the hosts are LAN-local anyway.
NOTE: this attribute is Auto Backup (cloud). Android 12+ DEVICE-TO-DEVICE transfer is governed
by a `dataExtractionRules` <device-transfer> block, which needs an @xml resource (tracked
separately — see the orchestrator note). Do not set this back to true. -->
<application
android:name=".WebTermApp"
android:allowBackup="false"
android:label="@string/app_name"
android:supportsRtl="true"
android:theme="@style/Theme.WebTerm"
android:networkSecurityConfig="@xml/network_security_config"
android:usesCleartextTraffic="false">
android:usesCleartextTraffic="true">
<activity
android:name=".MainActivity"

17
android/app/src/main/java/wang/yaojia/webterm/push/FcmService.kt
View File

@@ -4,7 +4,6 @@ import android.Manifest
import android.content.pm.PackageManager
import android.os.Build
import android.util.Log
import androidx.annotation.RequiresPermission
import androidx.core.app.NotificationManagerCompat
import androidx.core.content.ContextCompat
import com.google.firebase.messaging.FirebaseMessagingService
@@ -56,11 +55,23 @@ public class FcmService : FirebaseMessagingService() {
if (pushTokenSink.isPresent) pushTokenSink.get().onTokenRefreshed(token)
}
@RequiresPermission(Manifest.permission.POST_NOTIFICATIONS)
/**
* NOT `@RequiresPermission`: [hasNotificationPermission] is only a fast path. The user can revoke
* POST_NOTIFICATIONS between that check and this call (a real TOCTOU — a push arrives on a binder
* thread while the settings UI is open), and `notify` then throws [SecurityException]. Throwing out
* of `onMessageReceived` would crash the FCM service, so the loss is absorbed and logged instead:
* a missed notification must never take the push channel down. Declaring the permission as
* *required* here would also be a lie — the method tolerates its absence.
*/
private fun postNotification(payload: PushPayload, plan: NotificationBuilder.NotificationPlan) {
val notification = NotificationBuilder.build(this, payload, plan)
val id = NotificationBuilder.notificationIdFor(payload.sessionId.toString())
NotificationManagerCompat.from(this).notify(id, notification)
try {
NotificationManagerCompat.from(this).notify(id, notification)
} catch (e: SecurityException) {
// Never log the payload — it carries the session id and the decision token (§8).
Log.w(TAG, "POST_NOTIFICATIONS revoked between check and notify; dropping notification", e)
}
}
/** POST_NOTIFICATIONS is a runtime permission on API 33+; below that it is implicitly granted. */

3
android/app/src/main/java/wang/yaojia/webterm/screens/PairingScreen.kt
View File

@@ -199,6 +199,9 @@ private fun QrScanner(onScanned: (String) -> Unit) {
}
}
// QrCodeAnalyzer is @ExperimentalGetImage (it reads ImageProxy.image to feed ML Kit); constructing it
// here propagates that requirement to this call site, so the opt-in has to be declared here too.
@androidx.annotation.OptIn(ExperimentalGetImage::class)
@Composable
private fun CameraPreview(onScanned: (String) -> Unit) {
val context = LocalContext.current

6
android/app/src/main/java/wang/yaojia/webterm/viewmodels/PairingViewModel.kt
View File

@@ -315,7 +315,11 @@ internal object PairingCopy {
"这个端口在运行别的服务，不是 web-terminal。端口对吗？"
is PairingError.OriginRejected -> error.hint // already actionable, endpoint-derived
is PairingError.CleartextBlocked ->
"系统阻止了到 ${error.host} 的明文连接。仅局域网 / Tailscale 地址允许 ws:// 明文。"
// Inverted by the A19 cleartext fix: LAN cleartext is now PERMITTED (the platform has no
// CIDR syntax, so it could not be scoped to RFC1918 — see network_security_config.xml).
// The only host that can still produce this error is the tunnel domain, which
// network_security_config pins to TLS because it always serves a real LE cert.
"${error.host} 必须使用 https / wss —— 该主机由设备证书保护，不允许降级为明文。"
PairingError.TlsFailure ->
// Tunnel hosts present a real LE server cert, so a TLS failure there means OUR client cert
// is the problem (plan §5.4: "TLS failure re-mapped to client cert invalid/revoked").

88
android/app/src/main/res/xml/network_security_config.xml
View File

@@ -1,10 +1,84 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- STUB (plan §8 / A19). Cleartext is OFF globally. A19 will add
<domain-config cleartextTrafficPermitted="true"> entries for the private-LAN
and Tailscale CIDRs that legitimately need ws:// (10/8, 172.16/12,
192.168/16, 100.64/10). Tunnel (*.terminal.yaojia.wang) and Tailscale
MagicDNS hosts present real LE certs and use default system trust (§6.9),
so they stay under this base-config. -->
<!--
Cleartext posture (plan §6.9 + §8 checklist "Cleartext posture"). READ BEFORE EDITING.
── PLATFORM CONSTRAINT (verified against the schema, NOT assumed) ───────────────────────────
The network-security-config format has NO address-range syntax. Its entire vocabulary is
<network-security-config> / <base-config> / <domain-config> / <debug-overrides> / <domain> /
<trust-anchors> / <certificates> / <pin-set> / <pin>, with the attributes src,
includeSubdomains, cleartextTrafficPermitted, expiration and digest — that is the complete
set the validator shipping with our AGP accepts (com.android.tools.lint.checks.
NetworkSecurityConfigDetector, lint 32.2.1). A <domain> element holds exactly ONE hostname or
ONE IP literal, matched by exact / label-suffix comparison; there is no netmask, prefix or
CIDR attribute anywhere in the format, and no wildcard beyond includeSubdomains (which walks
DNS labels and therefore cannot generalise IPv4 literals — 192.168.1.5 is not a "subdomain"
of 192.168).
So the previous STUB comment here ("A19 will add <domain-config> entries for the private-LAN
and Tailscale CIDRs 10/8, 172.16/12, 192.168/16, 100.64/10") described a plan the platform
forbids: those four ranges are ~18.9M addresses and cannot be expressed at all. It has been
replaced by what is actually achievable.
── DECISION: cleartext is PERMITTED in base-config ──────────────────────────────────────────
Bare-LAN `ws://` to a user-typed private address (http://192.168.1.5:3000) is this product's
PRIMARY use case (CLAUDE.md "What This Is"); HostEndpoint.fromBaseUrl accepts `http` and
derives `ws://`, and PairingViewModel treats RFC1918 as a legitimate tier. targetSdk is 35, so
the platform default is deny — with no CIDR syntax available, keeping the denial would make
EVERY LAN connect fail on a real device with UnknownServiceException ("CLEARTEXT communication
to <ip> not permitted by network security policy"), i.e. the app could only ever talk to the
tunnel. There is no narrower posture: the LAN address is user-supplied at runtime and this
file is a build-time resource, so it cannot be scoped to "the IP the user actually typed".
── WHAT STILL GUARDS THE USER (the guard moved into the app; it did not disappear) ──────────
1. §5.4 warning tiers + confirm-before-network (PairingTiers / PairingViewModel): a PUBLIC
host needs an explicit risk acknowledge, PRIVATE_LAN gets the non-blocking `ws://`
cleartext notice, TAILSCALE gets none (WireGuard already encrypts). No dial happens
without a deliberate, warned confirm — that is the real UX guard, and it is unit-tested,
unlike a platform block that can only say "denied" after the fact.
2. Cleartext is DENIED below for the one hostname we control and always serve over TLS.
3. Trust anchors are pinned to SYSTEM certificates only (see <base-config>) — user-installed
CAs are NOT trusted, and there is deliberately NO <debug-overrides> trust-anchor block, so
no build variant of this app can be MITM'd by an added device CA.
── SECURITY TRADEOFF, STATED PLAINLY ────────────────────────────────────────────────────────
Permitting cleartext in base-config means the platform will no longer refuse a plaintext
connection to ANY host — including a public one — if the user pushes past the pairing
warning. On bare `ws://` the traffic (terminal bytes, and the WEBTERM_TOKEN if set) is
readable and replayable by anyone on the path. This mirrors the honest tradeoff already
recorded in CLAUDE.md for the server side: it is a LAN/Tailscale posture, never an
internet-exposure posture. TLS remains the default wherever a hostname is known.
-->
<network-security-config>
<base-config cleartextTrafficPermitted="false" />
<!--
Cleartext ON (rationale above). Trust anchors are stated explicitly rather than inherited:
"system" only is already the default for targetSdk >= 24, but declaring it locks the app
out of user-installed CAs even if that default ever changes. Tunnel and Tailscale MagicDNS
hosts present real Let's Encrypt certs and are verified against exactly these anchors —
default system trust, no custom X509TrustManager, no pinning (§6.9).
-->
<base-config cleartextTrafficPermitted="true">
<trust-anchors>
<certificates src="system" />
</trust-anchors>
</base-config>
<!--
The native tunnel (`*.terminal.yaojia.wang`, PairingTiers.WarningTier.TUNNEL) always
terminates TLS with a real LE cert and is mTLS-gated by the device client cert, so a
plaintext dial there is never legitimate — it can only be a downgrade or a typo. This is
the one place a hostname IS known at build time, so it keeps the platform-level block that
LAN literals cannot have. includeSubdomains covers the apex and every per-host subdomain
(e.g. h7fd8.terminal.yaojia.wang). A blocked attempt surfaces as UnknownServiceException,
which PairingError already maps to CleartextBlocked.
Deliberately NOT listed: *.ts.net (Tailscale MagicDNS). Tailscale carries the connection
inside a WireGuard tunnel, so `http://<host>.<tailnet>.ts.net:3000` is already encrypted
end-to-end and is a supported way to reach a host (HostClassifier tiers it TAILSCALE, "no
cleartext warning"). Blocking cleartext there would break a safe, intended path.
-->
<domain-config cleartextTrafficPermitted="false">
<domain includeSubdomains="true">terminal.yaojia.wang</domain>
</domain-config>
</network-security-config>

112
android/app/src/test/java/wang/yaojia/webterm/NetworkSecurityConfigTest.kt Normal file
View File

@@ -0,0 +1,112 @@
package wang.yaojia.webterm
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Test
import org.w3c.dom.Element
import org.w3c.dom.Document
import java.io.File
import javax.xml.parsers.DocumentBuilderFactory
/**
* Regression lock for the A19 cleartext posture (BLOCKER 3).
*
* The shipped `network_security_config.xml` used to be a STUB that denied cleartext globally, which made
* every bare-LAN `ws://` connect impossible on a real device — while `HostEndpoint.fromBaseUrl` accepts
* `http` and `PairingViewModel` treats RFC1918 as a first-class tier. Bare LAN is this product's PRIMARY
* use case (CLAUDE.md "What This Is"), so that contradiction made the app tunnel-only.
*
* These are resource-shape assertions, deliberately NOT a behavioural test: the behaviour they guard
* (does a socket to 192.168.x.x open?) is only observable on a device and lives in DEVICE_QA_CHECKLIST.
* What CAN regress silently in a code review is the XML, so that is what is pinned here. Each assertion
* corresponds to a way the fix could be undone without any other test going red.
*/
@DisplayName("network_security_config.xml — A19 cleartext posture")
class NetworkSecurityConfigTest {
private fun parse(path: String): Document =
DocumentBuilderFactory.newInstance()
.apply { isNamespaceAware = true }
.newDocumentBuilder()
.parse(File(path))
private fun nsc(): Document = parse("src/main/res/xml/network_security_config.xml")
private fun elements(doc: Document, tag: String): List<Element> {
val nodes = doc.getElementsByTagName(tag)
return (0 until nodes.length).map { nodes.item(it) as Element }
}
@Test
@DisplayName("base-config permits cleartext, so a user-typed LAN address can still connect")
fun baseConfigPermitsCleartext() {
val base = elements(nsc(), "base-config").single()
assertEquals(
"true",
base.getAttribute("cleartextTrafficPermitted"),
"Denying cleartext in base-config makes every ws:// LAN connect fail with " +
"UnknownServiceException. The platform has no CIDR syntax, so this CANNOT be " +
"narrowed to RFC1918 — see the header comment in the resource.",
)
}
@Test
@DisplayName("the tunnel domain keeps its cleartext block, so it can never be downgraded")
fun tunnelDomainDeniesCleartext() {
val denying = elements(nsc(), "domain-config")
.filter { it.getAttribute("cleartextTrafficPermitted") == "false" }
assertTrue(denying.isNotEmpty(), "the tunnel domain-config that pins TLS was removed")
val tunnelDomain = denying
.flatMap { cfg ->
val kids = cfg.getElementsByTagName("domain")
(0 until kids.length).map { kids.item(it) as Element }
}
.singleOrNull { it.textContent.trim() == "terminal.yaojia.wang" }
assertTrue(
tunnelDomain != null,
"terminal.yaojia.wang must stay under a cleartextTrafficPermitted=false domain-config: " +
"it always serves a real LE cert and is mTLS-gated, so a plaintext dial there can only " +
"be a downgrade or a typo.",
)
assertEquals(
"true",
tunnelDomain!!.getAttribute("includeSubdomains"),
"per-host tunnel subdomains (e.g. h7fd8.terminal.yaojia.wang) must be covered too",
)
}
@Test
@DisplayName("no user-installed CA can ever be trusted")
fun trustAnchorsAreSystemOnly() {
val doc = nsc()
assertTrue(
elements(doc, "debug-overrides").isEmpty(),
"a <debug-overrides> trust-anchor block would let an added device CA MITM a build",
)
val certs = elements(doc, "certificates")
assertTrue(certs.isNotEmpty(), "trust anchors must stay declared, not inherited")
certs.forEach {
assertEquals(
"system",
it.getAttribute("src"),
"only system CAs may be trusted (plan §6.9); 'user' would trust an added device CA",
)
}
}
@Test
@DisplayName("the manifest attribute agrees with the config — they can never drift apart")
fun manifestAgreesWithConfig() {
val app = elements(parse("src/main/AndroidManifest.xml"), "application").single()
assertEquals(
"true",
app.getAttributeNS("http://schemas.android.com/apk/res/android", "usesCleartextTraffic"),
"the manifest must not state the opposite of what network_security_config ships. The " +
"config wins at runtime on minSdk 29, so a stale 'false' here is a lie to the reader, " +
"not a second layer of defence.",
)
}
}