feat(android): close the audit's remaining parity gaps
Seven items the earlier repair waves deliberately left alone, so that the
blocker fixes could land with a working safety net first.
CERTIFICATE LIFECYCLE (the audit's only "high"). iOS ships a
CertificateRotationScheduler; Android had no counterpart, so a device
certificate simply expired and needed a manual re-enroll. The decision is a
pure total function over five answers, with RE_ENROLL_REQUIRED checked first so
it outranks any backoff. Renewal is single-flight — a second trigger coalesces,
and a cancelled leader releases the slot so cancellation cannot wedge rotation
for the process lifetime. Failure leaves the prior identity fully live (the
existing atomic ping-pong flip already guaranteed that) and is published rather
than swallowed.
Two real defects surfaced while building it:
renew() decoded its 201 with EnrollResponseDto, whose deviceId is required —
but /device/:id/renew returns only {cert, caChain, notAfter}; only
/device/enroll echoes deviceId. Every silent renewal would have thrown
MalformedResponse. Fixed with a RenewResponseDto plus the deviceId from the
path segment we addressed.
And because no renew 201 carries renewAfter, a client that persisted it would
rotate exactly once and then report not-due until the cert died. That is why
renewAfter is re-derived from the live leaf as notBefore + 2/3·lifetime — the
control plane's own formula. This is a latent iOS defect that Android now
avoids rather than inherits.
/device/:id/recover was confirmed real (control-plane/src/api/renew.ts:443,
contract pinned by that suite's CP6e) and is now wired. It goes over a separate
NON-mTLS client that throws rather than falling back, because an expired client
certificate cannot authenticate its own recovery — the deadlock a previous
session already hit and recorded.
Asked explicitly about step-up: rotation is unaffected. stepUp appears nowhere
in control-plane/src; it lives only on the relay's WebSocket upgrade. On a
step-up host the SESSION is denied, not the rotation, so the certificate still
stays alive and that principal gap is orthogonal.
FOLLOW-UP QUEUE (W2). The queue frame decoded but the three routes managing it
were missing, so Android could see "N queued" and not queue anything. One
outcome union covers both guarded writes and distinguishes full / too-large /
disabled / session-gone / rate-limited, because they need different copy. 429 is
surfaced and never auto-retried — POST and DELETE share one 20/min bucket.
Queued text is raw shell input and travels verbatim, proven with control
characters and CJK.
UNREAD WATERMARK now persists, so the unread state survives process death —
which is exactly when it matters, since the product premise is walking away and
coming back. It stores a plain map rather than an UnreadLedger: the reducer
lives in :session-core, which :host-registry may not depend on, so the logic is
not restated anywhere.
COPY-OUT. Text selection was a recorded feature gap: the stock ActionMode's own
Copy handler dereferences the absent TerminalSession, so making it reachable
puts an NPE on the button — worse than no selection. The refusal was
re-verified from the bytecode, and the same disassembly showed the way out:
TerminalBuffer.getSelectedText touches only mLines/TerminalRow, no session and
no view. A first-party selection layer now builds on that, with a pure
row-major model so a backwards drag normalises correctly. Terminal content
reaches the clipboard only on an explicit user copy; OSC 52 stays declined.
HOST REMOVAL. PushRegistrar.unregisterHost had zero call sites, so a removed
host kept receiving pushes. Removal is now confirm-gated and cleans everything
keyed by host id — a half-removed host is worse than none.
/config/ui is finally honoured. It was implemented and never called, so
allowAutoMode was ignored and the plan gate offered "approve + auto" even where
the operator had disabled it. It fails CLOSED: an unfetchable config treats auto
as disabled, because the permissive default is the dangerous one.
Verified: ./gradlew test :app:assembleDebug :app:assembleDebugAndroidTest
:macrobenchmark:assembleBenchmark koverVerify -> BUILD SUCCESSFUL, 1150 JVM
tests, 0 failures (903 -> 1150). Instrumented suite re-run on emulator-5554
against live servers: 67 tests, 0 failures, 0 skipped — no device regression.
This commit is contained in:
@@ -0,0 +1,77 @@
|
||||
package wang.yaojia.webterm.hostregistry
|
||||
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
/**
|
||||
* Contract tests for the in-`main` [InMemorySessionWatermarkStore] double (the same role
|
||||
* [InMemoryHostStore] / [InMemoryAuthCookieStore] play: it stands in for the DataStore
|
||||
* store in the AW4 ViewModel tests). The double must enforce the SAME at-rest rules as
|
||||
* the real store, otherwise a ViewModel test can pass against behaviour the device never
|
||||
* exhibits.
|
||||
*/
|
||||
class InMemorySessionWatermarkStoreTest {
|
||||
|
||||
private val idA = "3f2504e0-4f89-41d3-9a0c-0305e82c3301"
|
||||
private val idB = "3f2504e0-4f89-41d3-9a0c-0305e82c3302"
|
||||
|
||||
@Test
|
||||
fun `an empty store loads an empty map`() = runTest {
|
||||
assertTrue(InMemorySessionWatermarkStore().loadAll().isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `replaceAll then loadAll returns the same watermarks`() = runTest {
|
||||
val store = InMemorySessionWatermarkStore()
|
||||
|
||||
store.replaceAll(mapOf(idA to 10L, idB to 20L))
|
||||
|
||||
assertEquals(mapOf(idA to 10L, idB to 20L), store.loadAll())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `replaceAll returns exactly what was stored`() = runTest {
|
||||
val store = InMemorySessionWatermarkStore()
|
||||
|
||||
val stored = store.replaceAll(mapOf(idA to 10L, "garbage" to 20L))
|
||||
|
||||
assertEquals(mapOf(idA to 10L), stored)
|
||||
assertEquals(stored, store.loadAll())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `replaceAll is a wholesale swap, not a merge`() = runTest {
|
||||
val store = InMemorySessionWatermarkStore(mapOf(idA to 10L))
|
||||
|
||||
store.replaceAll(mapOf(idB to 20L))
|
||||
|
||||
assertEquals(mapOf(idB to 20L), store.loadAll())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an initial map is sanitized too`() = runTest {
|
||||
val store = InMemorySessionWatermarkStore(mapOf(idA to 10L, "garbage" to 20L))
|
||||
|
||||
assertEquals(mapOf(idA to 10L), store.loadAll())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `remove drops one session and returns the new map`() = runTest {
|
||||
val store = InMemorySessionWatermarkStore(mapOf(idA to 10L, idB to 20L))
|
||||
|
||||
val remaining = store.remove(idA)
|
||||
|
||||
assertEquals(mapOf(idB to 20L), remaining)
|
||||
assertEquals(remaining, store.loadAll())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `removing an unknown session is a no-op`() = runTest {
|
||||
val store = InMemorySessionWatermarkStore(mapOf(idA to 10L))
|
||||
|
||||
assertEquals(mapOf(idA to 10L), store.remove(idB))
|
||||
assertEquals(mapOf(idA to 10L), store.loadAll())
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
package wang.yaojia.webterm.hostregistry
|
||||
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
/**
|
||||
* At-rest codec tests (mirrors `HostCodecTest`): encode is total, decode is defensive —
|
||||
* the blob is untrusted at rest, so anything undecodable reads back as "no watermarks"
|
||||
* rather than crashing the session list on cold start.
|
||||
*/
|
||||
class SessionWatermarkCodecTest {
|
||||
|
||||
private val idA = "3f2504e0-4f89-41d3-9a0c-0305e82c3301"
|
||||
private val idB = "3f2504e0-4f89-41d3-9a0c-0305e82c3302"
|
||||
|
||||
@Test
|
||||
fun `round-trips a watermark map`() {
|
||||
val map = mapOf(idA to 1_700_000_000_000L, idB to 42L)
|
||||
|
||||
assertEquals(map, SessionWatermarkCodec.decode(SessionWatermarkCodec.encode(map)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an empty map round-trips to an empty map`() {
|
||||
assertTrue(SessionWatermarkCodec.decode(SessionWatermarkCodec.encode(emptyMap())).isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `null reads back as empty`() {
|
||||
assertTrue(SessionWatermarkCodec.decode(null).isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `blank reads back as empty`() {
|
||||
assertTrue(SessionWatermarkCodec.decode(" ").isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a corrupt blob reads back as empty instead of throwing`() {
|
||||
assertTrue(SessionWatermarkCodec.decode("{not json").isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a wrong-shaped blob reads back as empty`() {
|
||||
// A JSON array where a map was written (e.g. a hand-edited or superseded format).
|
||||
assertTrue(SessionWatermarkCodec.decode("""["$idA"]""").isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a non-numeric instant reads back as empty`() {
|
||||
assertTrue(SessionWatermarkCodec.decode("""{"$idA":"yesterday"}""").isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `decode does not itself validate ids - the store transforms do`() {
|
||||
// Keeping the codec dumb keeps ONE place (sanitizedWatermarks) responsible for trust.
|
||||
assertEquals(mapOf("garbage" to 5L), SessionWatermarkCodec.decode("""{"garbage":5}"""))
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,150 @@
|
||||
package wang.yaojia.webterm.hostregistry
|
||||
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertFalse
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
/**
|
||||
* Pure-transform tests for the watermark map shared by [InMemorySessionWatermarkStore] and
|
||||
* [DataStoreSessionWatermarkStore] (mirrors `HostStoreTransformsTest` /
|
||||
* `AuthCookieStoreTransformsTest`). These are the JVM-testable half of the storage split
|
||||
* (plan §3: "the logic half of :host-registry" is in the Kover gate).
|
||||
*
|
||||
* Covers the three at-rest defences and the GC bound:
|
||||
* - every stored session id is re-validated with the frozen `Validation.isValidSessionId`;
|
||||
* - non-positive instants are dropped (a watermark of 0 means "never seen" anyway);
|
||||
* - case-variant spellings of one id collapse via max (they are the same session);
|
||||
* - the map is capped at [MAX_WATERMARKS], newest kept — what bounds growth.
|
||||
*/
|
||||
class SessionWatermarkTransformsTest {
|
||||
|
||||
private val idA = "3f2504e0-4f89-41d3-9a0c-0305e82c3301"
|
||||
private val idB = "3f2504e0-4f89-41d3-9a0c-0305e82c3302"
|
||||
|
||||
// ── sanitizedWatermarks() ─────────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
fun `a valid map survives sanitizing unchanged`() {
|
||||
val map = mapOf(idA to 10L, idB to 20L)
|
||||
|
||||
assertEquals(map, map.sanitizedWatermarks())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a non-v4 session id is dropped`() {
|
||||
// UUID v1 (version nibble 1) parses as a UUID but is not a valid wire session id.
|
||||
val v1 = "3f2504e0-4f89-11d3-9a0c-0305e82c3301"
|
||||
val map = mapOf(idA to 10L, v1 to 20L, "abc123" to 30L, "" to 40L)
|
||||
|
||||
assertEquals(mapOf(idA to 10L), map.sanitizedWatermarks())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `non-positive instants are dropped`() {
|
||||
val map = mapOf(idA to 0L, idB to -5L)
|
||||
|
||||
assertTrue(map.sanitizedWatermarks().isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `case-variant spellings of one id collapse to the newest`() {
|
||||
val map = mapOf(idA to 10L, idA.uppercase() to 99L)
|
||||
|
||||
assertEquals(mapOf(idA to 99L), map.sanitizedWatermarks())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `sanitizing preserves encounter order`() {
|
||||
val map = linkedMapOf(idB to 20L, idA to 10L)
|
||||
|
||||
assertEquals(listOf(idB, idA), map.sanitizedWatermarks().keys.toList())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `sanitizing never mutates the receiver`() {
|
||||
val map = linkedMapOf(idA to 10L, "abc123" to 20L)
|
||||
|
||||
map.sanitizedWatermarks()
|
||||
|
||||
assertEquals(linkedMapOf(idA to 10L, "abc123" to 20L), map)
|
||||
}
|
||||
|
||||
// ── cappedWatermarks() ───────────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
fun `a map at the cap is returned unchanged`() {
|
||||
val map = watermarks(MAX_WATERMARKS)
|
||||
|
||||
assertEquals(map, map.cappedWatermarks())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `over the cap the newest entries are kept`() {
|
||||
val map = watermarks(MAX_WATERMARKS + 10)
|
||||
|
||||
val capped = map.cappedWatermarks()
|
||||
|
||||
assertEquals(MAX_WATERMARKS, capped.size)
|
||||
// Instants ascend with the index, so the 10 oldest (lowest) must be the dropped ones.
|
||||
assertFalse(capped.containsKey(sessionId(0)))
|
||||
assertTrue(capped.containsKey(sessionId(MAX_WATERMARKS + 9)))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `equal instants break the tie deterministically by session id`() {
|
||||
val map = (0 until MAX_WATERMARKS + 2).associate { sessionId(it) to 7L }
|
||||
|
||||
val capped = map.cappedWatermarks()
|
||||
|
||||
assertEquals(MAX_WATERMARKS, capped.size)
|
||||
assertEquals(map.cappedWatermarks().keys, capped.keys) // stable across runs
|
||||
// Lowest ids win the tie-break, so the two highest ids are the dropped ones.
|
||||
assertFalse(capped.containsKey(sessionId(MAX_WATERMARKS + 1)))
|
||||
}
|
||||
|
||||
// ── removingWatermark() ──────────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
fun `removing drops just that session`() {
|
||||
val map = mapOf(idA to 10L, idB to 20L)
|
||||
|
||||
assertEquals(mapOf(idB to 20L), map.removingWatermark(idA))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `removing an unknown session is a no-op`() {
|
||||
val map = mapOf(idA to 10L)
|
||||
|
||||
assertEquals(map, map.removingWatermark(idB))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `removing is case-insensitive like the id itself`() {
|
||||
val map = mapOf(idA to 10L)
|
||||
|
||||
assertTrue(map.removingWatermark(idA.uppercase()).isEmpty())
|
||||
}
|
||||
|
||||
// ── forStorage() = sanitize + cap ────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
fun `forStorage sanitizes and caps in one pass`() {
|
||||
val map = watermarks(MAX_WATERMARKS + 1) + mapOf("garbage" to 999_999L)
|
||||
|
||||
val stored = map.forStorage()
|
||||
|
||||
assertEquals(MAX_WATERMARKS, stored.size)
|
||||
assertFalse(stored.containsKey("garbage"))
|
||||
}
|
||||
|
||||
// ── helpers ─────────────────────────────────────────────────────────────
|
||||
|
||||
/** A distinct valid v4 id per [index] (the last 4 hex digits carry the index). */
|
||||
private fun sessionId(index: Int): String =
|
||||
"3f2504e0-4f89-41d3-9a0c-0305e82c%04x".format(index)
|
||||
|
||||
/** [count] valid watermarks whose instants ascend with the index (1-based, so all positive). */
|
||||
private fun watermarks(count: Int): Map<String, Long> =
|
||||
(0 until count).associate { sessionId(it) to (it + 1).toLong() }
|
||||
}
|
||||
Reference in New Issue
Block a user