fix(marmot): audit fixes for the append log, ratchet record and send states

A review of this branch turned up six real defects, four of which could lose data or key material. Found with the repo's code-review skill; the two most severe were reproduced with throwaway JVM tests before being believed. **The append log went write-only after a torn tail.** decodeFile stopped at a half-written record but left the append position at the end of the file, so every later append landed behind a record parsing always stops at. The log kept accepting writes and never read one back again - permanently, silently. Decoding now rewinds to the torn record's own start and the file is truncated there when the log is opened, so appends resume where reads stop. This is the worst of the set: for Marmot the message log holds the only copy of a decrypted message. **An append marked an entry held before the write succeeded.** A failed appendSegment still added to entries/seen, so the caller's duplicate check skipped that message from then on and its only plaintext copy was gone. The disk write now comes first. **The sender ratchet record was written in place.** A crash mid-write destroyed it, and load then fell back to the position in the full state - the position as of the last COMMIT, behind by every send since. The next send would re-emit all those generations, reusing AEAD key+nonce pairs. The comment even called that fallback "behind but never ahead" as though behind were the safe direction; it is the dangerous one. Now written through a temp file and a rename, so a crash leaves the previous complete record, at worst one send behind - and that send was never published, because publishing waits for this write. **The decrypt contract was not met on Android.** EncryptedAppendLog documents that decrypt returns null for a segment it cannot open; KeyStoreEncryption.decrypt rethrows. One bad segment aborted the whole read, and a caller seeing an empty log can overwrite a history that was merely unreadable. **The fold boundary did not survive a restart.** Held in memory, each session treated whatever it found as already folded and left its own run permanently unfoldable, so segments crept back towards one per message - the per-message cipher round trip the fold exists to prevent. It now lives in the file header. **Two send-state defects.** beginMarmotGroupMessage marked a message sending before the guards that make it return early, leaving a bubble pulsing forever with no failure glyph and so no way to retry; and a DM retry re-ran the publish loop, appending duplicate recipients, which rendered as "0/2" on a 1:1 DM. Retries are also released on success rather than pinned until eviction, and are now spared by eviction, since the window is mostly filled by the UI rendering bubbles and scrolling could otherwise drop a failed message's retry and leave a tappable glyph that did nothing. Known and not addressed: the log's in-memory cache holds every decrypted entry for the process lifetime. It is what makes appends cheap, and the same plaintext is already resident in LocalCache, but it is a second copy and worth bounding later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019R58HADiyhsipTye538fWs
2026-10-06 11:48:24 +00:00 · 2026-09-22 18:56:00 +00:00
parent 44d96d0667
commit e32bdd7f93
6 changed files with 290 additions and 69 deletions
@@ -115,6 +115,19 @@ class AccountMarmotActions(
        nostrGroupId: HexKey,
        innerEvent: Event,
    ) {
+        // The same two guards sendMarmotGroupMessage returns on. Checked here
+        // too, because marking a message as sending and then returning early
+        // would leave the bubble pulsing forever with no failure glyph and
+        // therefore no way to retry it.
+        if (account.marmotManager == null || !account.isWriteable()) {
+            Log.w("MarmotDbg") {
+                "beginMarmotGroupMessage: cannot send in ${nostrGroupId.take(8)}… (no manager, or a read-only account)"
+            }
+            showOwnMessageLocally(nostrGroupId, innerEvent)
+            account.chatDeliveryTracker.markFailed(innerEvent.id)
+            return
+        }
+
        // Marked before the note is indexed, so the bubble never renders a
        // frame without its state. Re-resolving the relay set on retry is the
        // point of taking the id rather than the relays: the commonest reason
@@ -323,7 +323,19 @@ class AndroidMarmotMessageStore(
    private val log =
        EncryptedAppendLog(
            encrypt = { encryption.encrypt(it) },
-            decrypt = { encryption.decrypt(it) },
+            // EncryptedAppendLog requires null, not a throw, for a segment it
+            // cannot open — KeyStoreEncryption.decrypt rethrows. Without this
+            // one bad segment would abort the whole read, and a caller that
+            // then sees an empty log can overwrite a history that was merely
+            // unreadable.
+            decrypt = {
+                try {
+                    encryption.decrypt(it)
+                } catch (e: Exception) {
+                    Log.w(TAG, "a log segment could not be decrypted and was skipped: ${e.message}", e)
+                    null
+                }
+            },
        )

    private fun readAll(nostrGroupId: String): List<String> = readAllFrom(messagesFile(nostrGroupId))
@@ -174,13 +174,15 @@ class AndroidMlsGroupStateStore(
     * The per-send ratchet position, written on its own so a message does not
     * re-encrypt the whole group state to record ~72 bytes.
     *
-     * Deliberately NOT the atomic temp-file-and-rename the state uses. That
-     * costs two file operations plus a directory sync for a record smaller than
-     * a block, and it is not what durability needs here: the record is written
-     * whole, and a half-written one fails to decrypt and is discarded, which
-     * falls back to the full state's position. What matters is that the bytes
-     * are on the device before the message they describe goes out, so the write
-     * is followed by an fsync.
+     * Written atomically, through a temp file and a rename. An in-place write
+     * looked adequate — the record is small and written whole — but the failure
+     * it allows is the one this record exists to prevent. A torn record is
+     * discarded on load, which falls back to the position in the full state:
+     * that is the position as of the last COMMIT, behind by every send since.
+     * The next send would then re-emit every generation in between, reusing
+     * AEAD key+nonce pairs. With a rename, a crash leaves the previous complete
+     * record instead, which is at worst one send behind — and that send's
+     * ciphertext was never published, because publishing waits for this write.
     */
    override suspend fun saveSenderRatchet(
        nostrGroupId: String,
@@ -191,10 +193,17 @@ class AndroidMlsGroupStateStore(
            try {
                file.parentFile?.mkdirs()
                val encrypted = encryption.encrypt(state)
-                FileOutputStream(file).use { out ->
+                val tempFile = File(file.parentFile, "${file.name}.tmp")
+                FileOutputStream(tempFile).use { out ->
                    out.write(encrypted)
                    out.fd.sync()
                }
+                if (!tempFile.renameTo(file)) {
+                    tempFile.copyTo(file, overwrite = true)
+                    if (!tempFile.delete()) {
+                        Log.w(TAG) { "Failed to delete the temp ratchet file: ${tempFile.absolutePath}" }
+                    }
+                }
                true
            } catch (e: Exception) {
                // Returning false sends the caller to a full state write, which
@@ -212,10 +221,11 @@ class AndroidMlsGroupStateStore(
            try {
                encryption.decrypt(file.readBytes())
            } catch (e: Exception) {
-                // A torn or unreadable record says nothing trustworthy about
-                // where the ratchet is. The full state's own position stands,
-                // which is behind but never ahead.
-                Log.w(TAG, "loadSenderRatchet($nostrGroupId) unreadable, ignoring: ${e.message}", e)
+                // Serious: the fallback is the full state's position, which is
+                // BEHIND, and a behind position re-emits used generations. The
+                // atomic write above is what should make this unreachable, so
+                // if it ever fires the group's key material is suspect.
+                Log.e(TAG, "loadSenderRatchet($nostrGroupId) unreadable — the ratchet may rewind: ${e.message}", e)
                null
            }
        }
@@ -179,6 +179,11 @@ class ChatDeliveryTracker(
    /** The send reached the relay pool; relay acceptance takes over from here. */
    fun markSent(displayedNoteId: HexKey) {
        lock.withLock {
+            // The action captured whatever the send needed to repeat itself —
+            // for a DM, every gift wrap. Once the event is with the relay pool
+            // there is nothing to retry, and holding it would pin that graph
+            // for as long as the message stays in the window.
+            retries.remove(displayedNoteId)
            val flow = deliveries[displayedNoteId] ?: return
            flow.value = flow.value?.copy(sendState = ChatSendState.SENT)
        }
@@ -217,11 +222,27 @@ class ChatDeliveryTracker(
            val flow = flowForLocked(displayedNoteId)
            val current = flow.value

+            val existing = current?.recipients.orEmpty()
+            val known = existing.firstOrNull { it.recipient == recipient }
+            // Replace rather than append. A retry re-runs the same publish loop
+            // and would otherwise register every recipient twice, which shows up
+            // as a doubled k/n count ("0/2" on a 1:1 DM) and a detail dialog
+            // listing everyone twice. Acceptances already collected for this
+            // recipient are kept: the wraps are the same events, so relay OKs
+            // from the first attempt still count.
+            val updated =
+                RecipientDelivery(
+                    recipient = recipient,
+                    targetRelays = (known?.targetRelays ?: emptySet()) + targetRelays,
+                    acceptedRelays = known?.acceptedRelays ?: emptySet(),
+                    isSelf = isSelf,
+                )
+
            flow.value =
                ChatDelivery(
                    targetRelays = (current?.targetRelays ?: emptySet()) + targetRelays,
                    acceptedRelays = current?.acceptedRelays ?: emptySet(),
-                    recipients = (current?.recipients ?: emptyList()) + RecipientDelivery(recipient, targetRelays, isSelf = isSelf),
+                    recipients = existing.filterNot { it.recipient == recipient } + updated,
                    // A DM registers one wrap at a time while the remaining
                    // recipients are still being sealed, so the send is not
                    // done just because the first wrap landed here. Only
@@ -325,7 +346,14 @@ class ChatDeliveryTracker(
        knownIds = knownIds + noteId

        while (deliveries.size > MAX_TRACKED) {
-            val evicted = deliveries.keys.first()
+            // Most entries here were created by the UI merely rendering a
+            // bubble, so plain insertion order would let scrolling a long chat
+            // evict a failed message's retry — leaving a tappable warning glyph
+            // that does nothing. Anything still holding a retry is spared until
+            // nothing else is left to drop.
+            val evicted =
+                deliveries.keys.firstOrNull { it !in retries }
+                    ?: deliveries.keys.first()
            deliveries.remove(evicted)
            retries.remove(evicted)
            knownIds = knownIds - evicted
@@ -22,14 +22,14 @@ package com.vitorpamplona.amethyst.commons.marmot

 import java.io.File
 import java.io.FileOutputStream
+import java.io.RandomAccessFile

 /**
 * An encrypted-at-rest log of UTF-8 entries that can be appended to in
 * constant time.
 *
- * The file is a sequence of independently encrypted SEGMENTS:
 * ```
- * file    := MAGIC segment*
+ * file    := MAGIC(8) foldedLength(uint64) segment*
 * segment := uint32 encLen, byte[encLen]        // whatever [encrypt] produces
 * plain   := uint32 count, (uint32 len, byte[len])*
 * ```
@@ -41,14 +41,20 @@ import java.io.FileOutputStream
 * conversation a few thousand messages long was moving hundreds of KB through
 * a hardware-backed cipher to append a couple of hundred bytes.
 *
- * Appending writes one small segment. Loose segments are folded back into one
- * every [compactAfterSegments] appends, which bounds what a read costs: without
- * that, an old conversation would need one cipher round-trip per message ever
- * sent.
+ * Appending writes one small segment. Every [compactAfterSegments] appends the
+ * loose run — everything past `foldedLength` — is folded into a single segment,
+ * which bounds how many cipher round trips a read costs. A fold re-encrypts
+ * only the run it collapses; the already-folded prefix is copied across as
+ * ciphertext, so no append ever pays for the whole history.
+ *
+ * `foldedLength` lives in the header rather than in memory so that bound
+ * survives a restart. Held only in memory, every session would treat whatever
+ * it found as already folded, leave its own run permanently unfoldable, and
+ * segments would creep back towards one per message — which is the cost the
+ * fold exists to prevent.
 *
 * A file written by the older format has no magic prefix and is read as a
- * single legacy blob; the next append rewrites it in this format. Nothing else
- * migrates it, and nothing needs to — reading handles both.
+ * single legacy blob; the next append rewrites it in this format.
 *
 * **Not thread-safe.** Entries are cached in memory so an append never has to
 * read the log back, and that cache assumes one owner. Callers hold their own
@@ -57,8 +63,10 @@ import java.io.FileOutputStream
 *
 * @param encrypt must produce a self-describing blob — it carries its own IV /
 *   nonce, since every segment is encrypted separately.
- * @param decrypt returns null for a segment it cannot open; that segment's
- *   entries are skipped and the rest of the log is still read.
+ * @param decrypt MUST return null rather than throwing for a segment it cannot
+ *   open. One unreadable segment then costs only its own entries; a throw would
+ *   abort the whole read, and a caller that reads an empty log can overwrite a
+ *   history that was merely unreadable.
 */
 class EncryptedAppendLog(
    private val encrypt: (ByteArray) -> ByteArray,
@@ -71,9 +79,9 @@ class EncryptedAppendLog(
        val seen: MutableSet<String>,
        /** False when the file has no header yet: it is new, or still in the old format. */
        var headed: Boolean,
-        /** Byte length of the part that has already been folded; the loose run starts here. */
+        /** Byte offset where the loose run begins; everything before it is one folded prefix. */
        var foldedLength: Long,
-        /** Segments appended since the last fold. */
+        /** Segments sitting past [foldedLength]. */
        var looseSegments: Int,
        /** Entries carried by those segments. */
        var looseEntries: Int,
@@ -83,18 +91,29 @@ class EncryptedAppendLog(

    private fun stateFor(file: File): LogState =
        logs.getOrPut(file.absolutePath) {
-            val (entries, headed) = decodeFile(file)
-            // Everything already on disk counts as folded. Whatever loose
-            // segments a previous session left behind stay where they are —
-            // re-folding them would re-encrypt bytes that are already encrypted,
-            // which is the cost this whole design exists to avoid.
+            val decoded = decodeFile(file)
+
+            // A torn trailing segment is dropped from the FILE, not merely from
+            // what we just read. Leaving it there puts the append position past
+            // a record that parsing always stops at, so every later append is
+            // written and then never read back — the log goes silently
+            // write-only, for good.
+            if (decoded.headed && decoded.validLength < file.length()) {
+                try {
+                    RandomAccessFile(file, "rw").use { it.setLength(decoded.validLength) }
+                } catch (_: Exception) {
+                    // Best effort: the next fold rewrites the file wholesale and
+                    // resolves it anyway.
+                }
+            }
+
            LogState(
-                entries = entries.toMutableList(),
-                seen = entries.toMutableSet(),
-                headed = headed,
-                foldedLength = if (headed) file.length() else 0L,
-                looseSegments = 0,
-                looseEntries = 0,
+                entries = decoded.entries.toMutableList(),
+                seen = decoded.entries.toMutableSet(),
+                headed = decoded.headed,
+                foldedLength = decoded.foldedLength,
+                looseSegments = decoded.looseSegments,
+                looseEntries = decoded.looseEntries,
            )
        }

@@ -107,23 +126,28 @@ class EncryptedAppendLog(
        entry: String,
    ): Boolean = entry in stateFor(file).seen

-    /** Append one entry. Constant time, apart from a periodic compaction. */
+    /** Append one entry. Constant time, apart from a periodic fold. */
    fun append(
        file: File,
        entry: String,
    ) {
        val state = stateFor(file)
-        state.entries.add(entry)
-        state.seen.add(entry)

        // No header to append after — a file that does not exist yet, or one
        // still in the old format. A rewrite lays one down.
        if (!state.headed) {
-            rewrite(file, state.entries.toList())
+            rewrite(file, state.entries + entry)
            return
        }

+        // The disk write comes FIRST. Recording the entry in memory before it is
+        // durable would let a failed write leave it marked as held: the caller's
+        // duplicate check would skip it from then on, and for Marmot this store
+        // holds the only copy of the plaintext.
        appendSegment(file, listOf(entry))
+
+        state.entries.add(entry)
+        state.seen.add(entry)
        state.looseSegments += 1
        state.looseEntries += 1

@@ -149,13 +173,18 @@ class EncryptedAppendLog(
        state: LogState,
    ) {
        if (state.looseEntries == 0) return
-        val prefix = file.readBytes().copyOfRange(0, state.foldedLength.toInt())
-        val folded = encrypt(encodeEntries(state.entries.takeLast(state.looseEntries)))

+        val prefix = ByteArray(state.foldedLength.toInt())
+        RandomAccessFile(file, "r").use { it.readFully(prefix) }
+
+        val folded = encrypt(encodeEntries(state.entries.takeLast(state.looseEntries)))
        val out = ByteArray(prefix.size + 4 + folded.size)
        prefix.copyInto(out, 0)
        lengthPrefix(folded.size).copyInto(out, prefix.size)
        folded.copyInto(out, prefix.size + 4)
+        // The copied prefix still carries the old boundary; the whole file is
+        // folded now.
+        writeLong(out, MAGIC.size, out.size.toLong())
        atomicWrite(file, out)

        state.foldedLength = out.size.toLong()
@@ -178,7 +207,7 @@ class EncryptedAppendLog(
        }
    }

-    /** Replace the whole log with [entries], as a single segment. */
+    /** Replace the whole log with [entries], as a single folded segment. */
    fun rewrite(
        file: File,
        entries: List<String>,
@@ -186,10 +215,11 @@ class EncryptedAppendLog(
        file.parentFile?.mkdirs()

        val segment = encrypt(encodeEntries(entries))
-        val out = ByteArray(MAGIC.size + 4 + segment.size)
+        val out = ByteArray(HEADER_LENGTH + 4 + segment.size)
        MAGIC.copyInto(out, 0)
-        lengthPrefix(segment.size).copyInto(out, MAGIC.size)
-        segment.copyInto(out, MAGIC.size + 4)
+        writeLong(out, MAGIC.size, out.size.toLong())
+        lengthPrefix(segment.size).copyInto(out, HEADER_LENGTH)
+        segment.copyInto(out, HEADER_LENGTH + 4)
        atomicWrite(file, out)

        val state =
@@ -211,34 +241,66 @@ class EncryptedAppendLog(
        logs.remove(file.absolutePath)
    }

-    /** Every entry in [file], and whether the file already carries a header. */
-    private fun decodeFile(file: File): Pair<List<String>, Boolean> {
+    private class Decoded(
+        val entries: List<String>,
+        val headed: Boolean,
+        /** Bytes up to the end of the last segment that parsed cleanly. */
+        val validLength: Long,
+        val foldedLength: Long,
+        val looseSegments: Int,
+        val looseEntries: Int,
+    )
+
+    private fun decodeFile(file: File): Decoded {
        // A file that does not exist yet has no header, so the first append has
        // to write one rather than tack a bare segment onto nothing.
-        if (!file.exists()) return emptyList<String>() to false
+        if (!file.exists()) return Decoded(emptyList(), false, 0L, 0L, 0, 0)
        val bytes = file.readBytes()

-        if (!bytes.startsWithMagic()) {
+        if (!bytes.startsWithMagic() || bytes.size < HEADER_LENGTH) {
            // The older format: the file is one encrypted blob and nothing else.
-            val plain = decrypt(bytes) ?: return emptyList<String>() to false
-            return decodeEntries(plain) to false
+            val plain = decrypt(bytes) ?: return Decoded(emptyList(), false, 0L, 0L, 0, 0)
+            return Decoded(decodeEntries(plain), false, 0L, 0L, 0, 0)
        }

+        val foldedLength = readLong(bytes, MAGIC.size).coerceIn(HEADER_LENGTH.toLong(), bytes.size.toLong())
+
        val result = ArrayList<String>()
-        var offset = MAGIC.size
+        var offset = HEADER_LENGTH
+        var looseSegments = 0
+        var looseEntries = 0
        while (offset + 4 <= bytes.size) {
+            val segmentStart = offset
            val encLen = readInt(bytes, offset)
            offset += 4
            // A truncated tail is a half-finished append (process death between
            // the write and the sync). Everything before it is intact and is
            // what we keep; the torn record is dropped rather than failing the
-            // whole log.
-            if (encLen <= 0 || offset + encLen > bytes.size) break
+            // whole log. Rewinding to the record's own start is what makes
+            // validLength the place a later append may safely resume from.
+            if (encLen <= 0 || offset + encLen > bytes.size) {
+                offset = segmentStart
+                break
+            }
            val plain = decrypt(bytes.copyOfRange(offset, offset + encLen))
            offset += encLen
-            if (plain != null) result.addAll(decodeEntries(plain))
+
+            val entries = if (plain != null) decodeEntries(plain) else emptyList()
+            result.addAll(entries)
+            if (segmentStart >= foldedLength) {
+                looseSegments += 1
+                looseEntries += entries.size
+            }
        }
-        return result to true
+
+        return Decoded(
+            entries = result,
+            headed = true,
+            validLength = offset.toLong(),
+            foldedLength = foldedLength.coerceAtMost(offset.toLong()),
+            looseSegments = looseSegments,
+            looseEntries = looseEntries,
+        )
    }

    private fun encodeEntries(entries: List<String>): ByteArray {
@@ -282,7 +344,10 @@ class EncryptedAppendLog(
        data: ByteArray,
    ) {
        val tempFile = File(target.parentFile, "${target.name}.tmp")
-        tempFile.writeBytes(data)
+        FileOutputStream(tempFile).use { out ->
+            out.write(data)
+            out.fd.sync()
+        }
        if (!tempFile.renameTo(target)) {
            tempFile.copyTo(target, overwrite = true)
            tempFile.delete()
@@ -317,9 +382,29 @@ class EncryptedAppendLog(
            ((source[offset + 2].toInt() and 0xFF) shl 8) or
            (source[offset + 3].toInt() and 0xFF)

+    private fun writeLong(
+        target: ByteArray,
+        offset: Int,
+        value: Long,
+    ) {
+        for (i in 0 until 8) target[offset + i] = (value shr (56 - 8 * i)).toByte()
+    }
+
+    private fun readLong(
+        source: ByteArray,
+        offset: Int,
+    ): Long {
+        var value = 0L
+        for (i in 0 until 8) value = (value shl 8) or (source[offset + i].toLong() and 0xFF)
+        return value
+    }
+
    companion object {
        /** Marks the segmented format; a file without it predates it. */
-        private val MAGIC = "MRMTLOG2".encodeToByteArray()
+        private val MAGIC = "MRMTLOG3".encodeToByteArray()
+
+        /** MAGIC plus the uint64 fold boundary. */
+        private const val HEADER_LENGTH = 16

        private const val COMPACT_AFTER_SEGMENTS = 200

@@ -122,14 +122,18 @@ class EncryptedAppendLogTest {
        (1..4).forEach { log.append(file, "second-run-$it") }
        val afterSecondFold = file.readBytes()

-        // The bytes the first fold produced must survive verbatim. If they were
-        // re-encrypted they would differ, since the stand-in cipher — like
+        // The SEGMENTS the first fold produced must survive verbatim. If they
+        // were re-encrypted they would differ, since the stand-in cipher — like
        // AES-GCM — uses a fresh nonce per call. This is the property that keeps
        // a send from paying for the whole conversation.
+        //
+        // The header is excluded on purpose: a fold moves the boundary it
+        // records, so those eight bytes are expected to change and only the
+        // ciphertext after them is expected to be copied.
        assertContentEquals(
-            afterFirstFold.toList(),
-            afterSecondFold.copyOfRange(0, afterFirstFold.size).toList(),
-            "folding the tail must copy the already-folded prefix as ciphertext",
+            afterFirstFold.copyOfRange(HEADER_LEN, afterFirstFold.size).toList(),
+            afterSecondFold.copyOfRange(HEADER_LEN, afterFirstFold.size).toList(),
+            "folding the tail must copy the already-folded segments as ciphertext",
        )
        assertContentEquals((1..5).map { "first-run-$it" } + (1..4).map { "second-run-$it" }, cipher().readAll(file))
    }
@@ -155,7 +159,7 @@ class EncryptedAppendLogTest {

        assertContentEquals(listOf("old-one", "old-two", "new-one"), cipher().readAll(file))
        // and the upgraded file is in the new format, so the next append is cheap
-        assertTrue(file.readBytes().decodeToString().startsWith("MRMTLOG2"))
+        assertTrue(file.readBytes().decodeToString().startsWith("MRMTLOG3"))
    }

    @Test
@@ -183,12 +187,79 @@ class EncryptedAppendLogTest {

        // Corrupt the first segment's nonce marker so decrypt returns null for it.
        val bytes = file.readBytes()
-        bytes[MAGIC_LEN + 4] = 0
+        bytes[HEADER_LEN + 4] = 0
        file.writeBytes(bytes)

        assertContentEquals(listOf("after"), cipher().readAll(file))
    }

+    @Test
+    fun `a torn tail does not stop the log accepting new entries`() {
+        // The bug this pins: decoding stopped at the torn record but the append
+        // position was left at the end of the file, so every later append landed
+        // behind a record that parsing always stops at. The log kept accepting
+        // writes and never read one back again — silently write-only, for good.
+        val file = tempFile()
+        val log = cipher()
+        log.append(file, "kept-one")
+        log.append(file, "kept-two")
+
+        val intact = file.readBytes()
+        log.append(file, "lost")
+        val torn = file.readBytes()
+        file.writeBytes(torn.copyOfRange(0, intact.size + 6))
+
+        val reopened = cipher()
+        assertContentEquals(listOf("kept-one", "kept-two"), reopened.readAll(file))
+
+        reopened.append(file, "three")
+        assertContentEquals(
+            listOf("kept-one", "kept-two", "three"),
+            cipher().readAll(file),
+            "an append after a torn tail must be readable by the next reader",
+        )
+    }
+
+    @Test
+    fun `the fold boundary survives a restart`() {
+        // Held only in memory, each session treated whatever it found as already
+        // folded and left its own run permanently unfoldable — segments crept
+        // back towards one per message, which is the per-message cipher round
+        // trip the fold exists to prevent.
+        val file = tempFile()
+        repeat(12) { session ->
+            // A fresh instance per session, as a cold process gets.
+            val log = cipher(compactAfter = 4)
+            repeat(3) { log.append(file, "s$session-m$it") }
+        }
+
+        val expected = (0 until 12).flatMap { session -> (0 until 3).map { "s$session-m$it" } }
+        assertContentEquals(expected, cipher().readAll(file))
+
+        // 36 entries at a fold every 4 appends: a handful of segments, not 36.
+        assertTrue(
+            file.length() < expected.size * SEGMENT_OVERHEAD_CEILING,
+            "segments should still be folding across restarts, file was ${file.length()} bytes",
+        )
+    }
+
+    @Test
+    fun `a failed write does not mark an entry as held`() {
+        // The duplicate check is what keeps a redelivered message from being
+        // written twice. If a failed append still registered the entry, the
+        // check would skip it forever and the only copy of that plaintext would
+        // be gone.
+        val file = tempFile()
+        val log =
+            EncryptedAppendLog(
+                encrypt = { throw IllegalStateException("cipher unavailable") },
+                decrypt = { null },
+            )
+
+        runCatching { log.append(file, "never-written") }
+        assertFalse(log.contains(file, "never-written"), "a write that failed must not count as persisted")
+    }
+
    @Test
    fun `entries keep their bytes through the round trip`() {
        val file = tempFile()
@@ -222,7 +293,9 @@ class EncryptedAppendLogTest {

    companion object {
        private const val NONCE_MARK: Byte = 0x7F
-        private const val MAGIC_LEN = 8
+
+        /** MAGIC plus the uint64 fold boundary. */
+        private const val HEADER_LEN = 16
        private const val SEGMENT_OVERHEAD_CEILING = 64
    }
 }