From 02e5c45cd4cfd6282e3fc690f8265725f4d91198 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Tue, 25 Aug 2026 17:13:59 -0400 Subject: [PATCH] A test that could actually have caught it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `a retired session is kept and still opens its chain` passed while the fix was broken, because it retires a PRISTINE session — which is not what the receive path holds. By the time that path decides to retire anything it has just proved the peer is on another chain by handing this session an envelope it could not open, and E2ESession is a class, so that attempt consumed skipped keys and stepped the chain. The new test reproduces that state and asserts both halves: the damaged instance must NOT still open its chain (or the test proves nothing), and the copy re-read from the vault must. It fails against the first version of the fix and passes against this one. 13 VaultTest cases, 112 engine tests, 0 failures. Both platforms compile. Co-Authored-By: Claude Opus 5 --- .../test/kotlin/io/flamenet/e2e/VaultTest.kt | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt index de858a0..003acc3 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt @@ -118,6 +118,64 @@ class VaultTest { fresh.loadRetiredSession(bob.deviceId)!!.decrypt(reply)) } + /** + * 🔴 **A failed decrypt damages the session object it was handed, so the + * thing that gets retired must be re-read from the vault.** + * + * This is the bug the first version of the retirement fix shipped with, and + * the reason `a retired session is kept and still opens its chain` did not + * catch it: that test retires a PRISTINE session, which is not what the + * receive path has in its hand. The real path has just proved the peer is on + * another chain — by handing this session an envelope it could not open — + * and `E2ESession` is a class, so that attempt consumed skipped keys and + * stepped the chain. Retiring THAT instance stores something that can no + * longer open anything, which looks exactly like not retiring at all. + * + * Asserts both halves: the mutated instance fails, the re-read one works. + */ + @Test + fun `the session retired after a failed decrypt must be re-read, not reused`() { + val storage = MapStorage() + val secret = key() + val alice = DeviceIdentity.create() + val bob = DeviceIdentity.create() + + // Bob's own chain, stored, with a message Alice will later answer on it. + val bobsOwn = E2ESession.initiate(bob, bundleFor(alice)) + val bobsFirst = bobsOwn.encrypt("from bob") + Vault(storage, secret).saveSession(bob.deviceId, bobsOwn) + + val alicesReplyOnBobsChain = E2ESession.respond(alice, bobsFirst).let { + it.decrypt(bobsFirst) + it.encrypt("answering on the chain bob started") + } + + // The receive path: Bob's stored session is handed a foreign envelope + // and fails. THIS is the instance the buggy version retired. + val foreign = E2ESession.initiate(alice, bundleFor(bob)).encrypt("different chain") + val damaged = Vault(storage, secret).loadSession(bob.deviceId)!! + assertNull(runCatching { damaged.decrypt(foreign) }.getOrNull(), + "the foreign envelope must NOT open, or this test proves nothing") + + // Retiring the damaged instance loses the chain. + val wrong = MapStorage().also { storage.records.forEach(it.records::put) } + Vault(wrong, secret).saveRetiredSession(bob.deviceId, damaged) + assertNull( + runCatching { + Vault(wrong, secret).loadRetiredSession(bob.deviceId)!! + .decrypt(alicesReplyOnBobsChain) + }.getOrNull(), + "a session damaged by a failed decrypt should NOT still open its chain — " + + "if this ever starts passing, the hazard is gone and this test can go") + + // Re-reading the vault copy keeps it. This is the fix. + val pristine = Vault(storage, secret).loadSession(bob.deviceId)!! + Vault(storage, secret).saveRetiredSession(bob.deviceId, pristine) + assertEquals("answering on the chain bob started", + Vault(storage, secret).loadRetiredSession(bob.deviceId)!! + .decrypt(alicesReplyOnBobsChain)) + } + /** Signing out must take the retired chains too — they hold chain keys. */ @Test fun `forgetting sessions forgets the retired ones as well`() {