fix(ios,android): close the acceptance gaps the review found, add Android CI
- T-iOS-34's stated acceptance is "最大字号不破版" and it was failing: the key bar froze its height at 52pt while an AX5 keycap needs 108.24pt, so caps clipped — recorded as a withKnownIssue rather than fixed. Height now derives from the content size category (and tracks live changes via registerForTraitChanges); the known-issue marker is gone, replaced by positive assertions including a 12-category no-clip sweep and a guard that stays red if anyone writes the constant back. Honest tradeoff: the keycap font is clamped at .accessibility2, the same policy the design system already applies to dense content, because an unclamped AX5 bar would eat the terminal. A test pins the clamp so the two cannot drift. - Thumbnails silently 401'd on a token-gated host: the pipeline built its own transport with no token source, and by design never throws, so every preview degraded to a placeholder with no signal. Assembled from AppEnvironment now. - Android had zero CI while the token wave shipped 24 files of secret-handling code. The instrumented leg is workflow_dispatch-only and says why in the file: no one has ever seen it green on a runner, and a required leg nobody trusts just produces a false green. - Android persisted a validated token before the host probe succeeded, stranding a secret for a host that never paired. App bundle 534 -> 550 on both simulators, zero known issues. Android 687 -> 691.
This commit is contained in:
@@ -3,10 +3,12 @@ package wang.yaojia.webterm.viewmodels
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.NonCancellable
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import wang.yaojia.webterm.api.pairing.AuthProbeResult
|
||||
import wang.yaojia.webterm.api.pairing.PairingError
|
||||
import wang.yaojia.webterm.api.pairing.PairingProbeResult
|
||||
@@ -56,6 +58,14 @@ import java.util.UUID
|
||||
* continues (never treated as "authenticated"). A wrong token / rate-limit / malformed token keeps the
|
||||
* user on the confirm step with a typed [TokenError] and NEVER reaches the network probe.
|
||||
*
|
||||
* ### RULE 6 — a pairing that does not end in [PairingUiState.Paired] strands no secret (F2)
|
||||
* Storing before probing means the store can be left holding a token for a host that never paired (wrong
|
||||
* port, unreachable, user backs out). So every non-paired exit — probe failure, a throw, and cancellation
|
||||
* by [reset] or by a superseding attempt — rolls the store back to exactly what it held before this
|
||||
* attempt ([TokenAttempt]): the previous token for a re-pair, nothing at all for a first pairing. The
|
||||
* rollback runs under [NonCancellable] so an abandoned attempt still cleans up, and the next attempt
|
||||
* `join`s the one it supersedes so the two never race on the same key.
|
||||
*
|
||||
* The token itself never enters [PairingUiState] — it lives in the screen's own field and is passed in
|
||||
* per call, so no state snapshot, log or crash report can carry it.
|
||||
*
|
||||
@@ -161,8 +171,12 @@ public class PairingViewModel(
|
||||
return
|
||||
}
|
||||
val scope = scope ?: return
|
||||
probeJob?.cancel()
|
||||
val superseded = probeJob
|
||||
superseded?.cancel()
|
||||
probeJob = scope.launch {
|
||||
// Wait out the attempt this one replaces: its RULE 6 rollback must finish before we read or
|
||||
// write the same store key, or the two attempts race and the loser's undo wins.
|
||||
superseded?.join()
|
||||
// RULE 4 — tunnel cert-gate: refuse BEFORE any network I/O; retry re-hits this same guard.
|
||||
if (tier.isCertGated && !hasDeviceCert()) {
|
||||
_uiState.value = PairingUiState.CertRequired(candidate.endpoint, candidate.name, tier)
|
||||
@@ -170,20 +184,36 @@ public class PairingViewModel(
|
||||
}
|
||||
_uiState.value = PairingUiState.Probing(candidate.endpoint, candidate.name, tier)
|
||||
// RULE 5 — validate + store the token BEFORE probing (the probe authenticates from the store).
|
||||
if (accessToken.isNotBlank() && !establishToken(candidate, accessToken)) return@launch
|
||||
when (val result = prober.probe(candidate.endpoint)) {
|
||||
is PairingProbeResult.Success -> onProbeSuccess(candidate)
|
||||
is PairingProbeResult.Failure -> onProbeFailure(candidate, result.error)
|
||||
val attempt = when {
|
||||
accessToken.isBlank() -> TokenAttempt.NONE
|
||||
else -> establishToken(candidate, accessToken) ?: return@launch
|
||||
}
|
||||
// RULE 6 — anything short of Paired puts the store back the way this attempt found it.
|
||||
val paired = try {
|
||||
when (val result = prober.probe(candidate.endpoint)) {
|
||||
is PairingProbeResult.Success -> onProbeSuccess(candidate)
|
||||
is PairingProbeResult.Failure -> {
|
||||
onProbeFailure(candidate, result.error)
|
||||
false
|
||||
}
|
||||
}
|
||||
} catch (error: Throwable) {
|
||||
// Cancelled (reset / a newer attempt) or a transport throw — the rule is the same, and
|
||||
// NonCancellable is what lets an already-cancelled attempt still clean up after itself.
|
||||
withContext(NonCancellable) { rollBackToken(candidate, attempt) }
|
||||
throw error
|
||||
}
|
||||
if (!paired) rollBackToken(candidate, attempt)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate [token] with `POST /auth` and, if the server accepted it, persist it for this host.
|
||||
* Returns true when pairing may continue (accepted-and-stored, OR the host has no auth at all);
|
||||
* false after publishing the matching [TokenError] / failure state.
|
||||
* Returns the [TokenAttempt] to undo should pairing not complete (RULE 6) — [TokenAttempt.NONE] when
|
||||
* the host turned out to have no auth at all — or **null** after publishing the matching
|
||||
* [TokenError] / failure state, meaning "stop, do not probe".
|
||||
*/
|
||||
private suspend fun establishToken(candidate: Candidate, token: String): Boolean {
|
||||
private suspend fun establishToken(candidate: Candidate, token: String): TokenAttempt? {
|
||||
val outcome = try {
|
||||
authProber.validate(candidate.endpoint, token)
|
||||
} catch (cancel: CancellationException) {
|
||||
@@ -191,13 +221,13 @@ public class PairingViewModel(
|
||||
} catch (error: Throwable) {
|
||||
// A network-level failure is not a token problem — classify it like any other probe error.
|
||||
onProbeFailure(candidate, PairingError.classify(error, candidate.endpoint))
|
||||
return false
|
||||
return null
|
||||
}
|
||||
return when (outcome) {
|
||||
AuthProbeResult.Accepted -> storeToken(candidate, token)
|
||||
// 204 with no Set-Cookie: the host has no auth. Store NOTHING (never "assume authenticated")
|
||||
// and carry on — an open host pairs exactly as it did before this feature.
|
||||
AuthProbeResult.AuthDisabled -> true
|
||||
AuthProbeResult.AuthDisabled -> TokenAttempt.NONE
|
||||
AuthProbeResult.InvalidToken -> failToken(candidate, TokenError.INVALID)
|
||||
AuthProbeResult.RateLimited -> failToken(candidate, TokenError.TOO_MANY_ATTEMPTS)
|
||||
AuthProbeResult.Malformed -> failToken(candidate, TokenError.MALFORMED)
|
||||
@@ -205,26 +235,49 @@ public class PairingViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun storeToken(candidate: Candidate, token: String): Boolean {
|
||||
val stored = runCatching { tokenStore.put(candidate.endpoint, token) }
|
||||
return when {
|
||||
stored.getOrNull() == true -> true
|
||||
/** Persist [token], remembering what the store held before so RULE 6 can put it back. */
|
||||
private suspend fun storeToken(candidate: Candidate, token: String): TokenAttempt? {
|
||||
return try {
|
||||
val previous = tokenStore.tokenFor(candidate.endpoint)
|
||||
// put() == false ⇒ the token failed the client-side rule (unreachable after a server ACCEPT).
|
||||
stored.isSuccess -> failToken(candidate, TokenError.MALFORMED)
|
||||
if (!tokenStore.put(candidate.endpoint, token)) failToken(candidate, TokenError.MALFORMED)
|
||||
else TokenAttempt(stored = true, previous = previous)
|
||||
} catch (cancel: CancellationException) {
|
||||
throw cancel
|
||||
} catch (_: Throwable) {
|
||||
// A throw ⇒ encrypted storage failed; never continue with a token we could not keep.
|
||||
else -> failToken(candidate, TokenError.STORE_FAILED)
|
||||
failToken(candidate, TokenError.STORE_FAILED)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Undo [attempt]'s write: restore the previous token (a failed re-pair must not cost the user the
|
||||
* one that was working) or drop the key entirely when this host had none. Never publishes state —
|
||||
* the caller has already published the reason pairing stopped.
|
||||
*/
|
||||
private suspend fun rollBackToken(candidate: Candidate, attempt: TokenAttempt) {
|
||||
if (!attempt.stored) return
|
||||
try {
|
||||
val previous = attempt.previous
|
||||
if (previous == null) tokenStore.remove(candidate.endpoint)
|
||||
else tokenStore.put(candidate.endpoint, previous)
|
||||
} catch (cancel: CancellationException) {
|
||||
throw cancel
|
||||
} catch (_: Throwable) {
|
||||
// Best effort: encrypted storage just failed on the undo. There is no recovery beyond this,
|
||||
// and surfacing it would replace the pairing error the user actually needs to read.
|
||||
}
|
||||
}
|
||||
|
||||
/** Publish a token-specific error, keeping the user ON the confirm step (the field is there). */
|
||||
private fun failToken(candidate: Candidate, error: TokenError): Boolean {
|
||||
private fun failToken(candidate: Candidate, error: TokenError): TokenAttempt? {
|
||||
_uiState.value = PairingUiState.Confirming(
|
||||
endpoint = candidate.endpoint,
|
||||
name = candidate.name,
|
||||
tier = candidate.tier,
|
||||
tokenError = error,
|
||||
)
|
||||
return false
|
||||
return null
|
||||
}
|
||||
|
||||
private fun onProbeFailure(candidate: Candidate, error: PairingError) {
|
||||
@@ -242,7 +295,8 @@ public class PairingViewModel(
|
||||
)
|
||||
}
|
||||
|
||||
private suspend fun onProbeSuccess(candidate: Candidate) {
|
||||
/** Persist the paired host. Returns false when it could not be paired (so RULE 6 undoes the token). */
|
||||
private suspend fun onProbeSuccess(candidate: Candidate): Boolean {
|
||||
val host = Host.create(id = newId(), name = candidate.name.ifBlank { defaultName(candidate.endpoint) }, baseUrl = candidate.endpoint.baseUrl)
|
||||
if (host == null) {
|
||||
// Unreachable in practice (the endpoint already validated), but never persist a null host.
|
||||
@@ -253,10 +307,11 @@ public class PairingViewModel(
|
||||
error = PairingError.HttpOkButNotWebTerminal,
|
||||
message = PairingCopy.describe(PairingError.HttpOkButNotWebTerminal, candidate.tier),
|
||||
)
|
||||
return
|
||||
return false
|
||||
}
|
||||
hostStore.upsert(host)
|
||||
_uiState.value = PairingUiState.Paired(host)
|
||||
return true
|
||||
}
|
||||
|
||||
private fun defaultName(endpoint: HostEndpoint): String =
|
||||
@@ -428,6 +483,18 @@ public enum class TokenError {
|
||||
/** The endpoint+name+tier a probe runs against, recovered from whatever state currently holds one. */
|
||||
private data class Candidate(val endpoint: HostEndpoint, val name: String, val tier: WarningTier)
|
||||
|
||||
/**
|
||||
* What one pairing attempt wrote to the access-token store, and therefore what has to be put back if the
|
||||
* attempt does not end in [PairingUiState.Paired] (RULE 6). [previous] is what the store held for this
|
||||
* host beforehand — non-null only for a re-pair, whose working token a failed attempt must not cost.
|
||||
*/
|
||||
private data class TokenAttempt(val stored: Boolean, val previous: String?) {
|
||||
companion object {
|
||||
/** No token was written (none typed, or the host has no auth) — nothing to undo. */
|
||||
val NONE: TokenAttempt = TokenAttempt(stored = false, previous = null)
|
||||
}
|
||||
}
|
||||
|
||||
private fun PairingUiState.candidate(): Candidate? = when (this) {
|
||||
is PairingUiState.Confirming -> Candidate(endpoint, name, tier)
|
||||
is PairingUiState.Probing -> Candidate(endpoint, name, tier)
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
package wang.yaojia.webterm.viewmodels
|
||||
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertInstanceOf
|
||||
@@ -25,6 +27,9 @@ import java.io.IOException
|
||||
* - a wrong token / rate-limit keeps the user ON the confirm step with a typed [TokenError] (the field
|
||||
* is right there) instead of dropping them into a generic failure;
|
||||
* - a host that answers 401 to the probe asks for a token ([TokenError.REQUIRED]);
|
||||
* - **no stranded secret** (F2): the flip side of storing before probing is that a pairing which does
|
||||
* not reach [PairingUiState.Paired] — failed probe, or abandoned mid-probe — must roll the store back
|
||||
* to exactly what it held before, restoring a re-paired host's previous token rather than clobbering it;
|
||||
* - the token NEVER appears in the UI state (no accidental leak into a state dump / crash report).
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
@@ -33,6 +38,12 @@ class PairingTokenViewModelTest {
|
||||
private companion object {
|
||||
const val LAN = "http://10.0.0.5:3000"
|
||||
const val TOKEN = "0123456789abcdefTOKEN"
|
||||
|
||||
/** The token an already-paired host holds, so a failed re-pair can be shown not to clobber it. */
|
||||
const val PREVIOUS_TOKEN = "0123456789abcdefOLD1"
|
||||
|
||||
/** A cert-gated native-tunnel host (`WarningTier.TUNNEL`). */
|
||||
const val TUNNEL = "https://star.terminal.yaojia.wang"
|
||||
}
|
||||
|
||||
/** Records every `/auth` validation so ordering vs the pairing probe is directly assertable. */
|
||||
@@ -238,6 +249,102 @@ class PairingTokenViewModelTest {
|
||||
assertEquals(TOKEN, tokens.tokenFor(endpoint()))
|
||||
}
|
||||
|
||||
// ── a pairing that does not succeed must not strand the secret (F2) ─────────────────────────
|
||||
//
|
||||
// The token is stored BEFORE the probe on purpose (the probe's own HTTP+WS legs read it back out
|
||||
// of the store to authenticate). The flip side is that every path which does NOT end in `Paired`
|
||||
// has to put the store back the way it found it — otherwise a mistyped port leaves a live token
|
||||
// on disk for a host that was never paired, and nothing ever removes it.
|
||||
|
||||
@Test
|
||||
fun `a probe failure after the token was accepted leaves no stranded token`() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val tokens = InMemoryAccessTokenStore()
|
||||
var tokenVisibleToProbe: String? = null
|
||||
val prober = PairingProber { endpoint ->
|
||||
tokenVisibleToProbe = tokens.tokenFor(endpoint)
|
||||
PairingProbeResult.Failure(PairingError.HostUnreachable("connect refused"))
|
||||
}
|
||||
val vm = newVm(prober, FakeAuthProber { AuthProbeResult.Accepted }, tokens)
|
||||
vm.bind(backgroundScope)
|
||||
|
||||
vm.onManualEntry(LAN)
|
||||
vm.confirm(accessToken = TOKEN)
|
||||
|
||||
assertEquals(TOKEN, tokenVisibleToProbe, "the probe's legs must still authenticate from the store")
|
||||
assertInstanceOf(PairingUiState.Failed::class.java, vm.uiState.value)
|
||||
assertNull(tokens.tokenFor(endpoint()), "a host that never paired must not keep a stored token")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failed re-pair restores the token the host was already paired with`() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val tokens = InMemoryAccessTokenStore()
|
||||
tokens.put(endpoint(), PREVIOUS_TOKEN)
|
||||
val prober = FakeProber { PairingProbeResult.Failure(PairingError.HttpOkButNotWebTerminal) }
|
||||
val vm = newVm(prober, FakeAuthProber { AuthProbeResult.Accepted }, tokens)
|
||||
vm.bind(backgroundScope)
|
||||
|
||||
vm.onManualEntry(LAN)
|
||||
vm.confirm(accessToken = TOKEN)
|
||||
|
||||
assertEquals(
|
||||
PREVIOUS_TOKEN,
|
||||
tokens.tokenFor(endpoint()),
|
||||
"a failed re-pair rolls back to the token that was already working, it does not clobber it",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The one test that needs a REAL suspension point mid-probe, so it runs on the default
|
||||
* [kotlinx.coroutines.test.StandardTestDispatcher] rather than the unconfined dispatcher the rest of
|
||||
* the file uses. Note `runCurrent()`, not `advanceUntilIdle()`: the probe runs in `backgroundScope`,
|
||||
* and `advanceUntilIdle` stops as soon as no FOREGROUND work is left — it would never run the probe
|
||||
* at all (verified: the coroutine did not start).
|
||||
*/
|
||||
@Test
|
||||
fun `abandoning an in-flight probe rolls the freshly stored token back`() = runTest {
|
||||
val probeReached = CompletableDeferred<Unit>()
|
||||
val neverAnswers = CompletableDeferred<PairingProbeResult>()
|
||||
val tokens = InMemoryAccessTokenStore()
|
||||
val vm = newVm(
|
||||
prober = { probeReached.complete(Unit); neverAnswers.await() },
|
||||
authProber = FakeAuthProber { AuthProbeResult.Accepted },
|
||||
tokenStore = tokens,
|
||||
)
|
||||
vm.bind(backgroundScope)
|
||||
|
||||
vm.onManualEntry(LAN)
|
||||
vm.confirm(accessToken = TOKEN)
|
||||
runCurrent()
|
||||
|
||||
assertTrue(probeReached.isCompleted, "the probe must be in flight for this to test anything")
|
||||
assertEquals(TOKEN, tokens.tokenFor(endpoint()), "the in-flight probe reads the candidate token")
|
||||
|
||||
vm.reset() // the user backs out while the probe hangs
|
||||
runCurrent()
|
||||
|
||||
assertNull(tokens.tokenFor(endpoint()), "an abandoned pairing must not leave the secret behind")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cert-gated tunnel host is refused before the token is validated or stored`() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val tokens = InMemoryAccessTokenStore()
|
||||
val authProber = FakeAuthProber { AuthProbeResult.Accepted }
|
||||
val prober = FakeProber { PairingProbeResult.Success(it) }
|
||||
val vm = newVm(prober, authProber, tokens) // hasDeviceCert = false
|
||||
|
||||
vm.bind(backgroundScope)
|
||||
|
||||
vm.onManualEntry(TUNNEL)
|
||||
vm.confirm(accessToken = TOKEN)
|
||||
|
||||
assertInstanceOf(PairingUiState.CertRequired::class.java, vm.uiState.value)
|
||||
assertTrue(authProber.calls.isEmpty(), "the cert-gate refuses BEFORE any network I/O")
|
||||
assertNull(tokens.tokenFor(endpoint(TUNNEL)), "nothing is stored for a host that was never probed")
|
||||
}
|
||||
|
||||
// ── secrecy ─────────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user