Merge branch 'fix/purge-orphaned-attachments' into 'main'

Expired messages no longer leave their plaintext on disk (android#11)

See merge request paulhitt/flamenet-messenger-android!60
This commit was merged in pull request #87.
This commit is contained in:
Paul Hitt
2026-09-04 22:29:55 +00:00
5 changed files with 512 additions and 1 deletions
@@ -3287,7 +3287,16 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) {
// A story arriving must appear without the user pulling
// to refresh; it is filed by `receive()` and read back here.
val received = withContext(Dispatchers.IO) {
runCatching { current.receive() }
val got = runCatching { current.receive() }
// Cached plaintext for anything nothing refers to any
// more — an expired message's photo, a story past its
// lifetime. Throttled inside, and it declines to run at
// all if it cannot tell what is still in use.
runCatching {
current.purgeOrphanedAttachments(
getApplication<Application>().cacheDir)
}
got
}
// 🔴 410 from the poll: this device's registration is gone
// — revoked from another phone, or expired by the sweep.
@@ -416,6 +416,72 @@ class Session(
}
/** Bytes for an attachment, cached on disk after the first fetch. */
/**
* Delete cached attachment bytes that nothing refers to any more.
*
* 🔴 **`attachmentBytes` writes DECRYPTED bytes to the cache and nothing
* ever removed them.** So a disappearing message expired, its bubble went,
* and the plaintext photo or voice note stayed on disk indefinitely — which
* is the exact failure the feature exists to prevent. Same for a story once
* its expiry passed. Spec §6.11 says it plainly: attachments MUST be purged
* with the message that referenced them.
*
* ⚠️ **A sweep, not a purge at the point of deletion, and that is a
* deliberate choice.** Four different paths drop a row — expiry, story
* expiry, delete-for-me and clear-thread — and threading "here is what I
* removed" out of each one would still miss anything a future path drops,
* and would never reach the orphans already on disk. Asking instead "what
* is still referenced?" is complete by construction and self-healing.
*
* The cost is that a file survives until the next sweep rather than going
* at the instant of expiry. Sweeps run on every poll, so that window is a
* poll interval — worth naming rather than implying it is instant.
*
* 🔴 **It fails closed.** If the in-use set cannot be built — an
* unreadable record, anything at all — nothing is deleted. A partial answer
* to "what is still in use" is the one input that turns this from a cleanup
* into data loss.
*
* ⚠️ [graceMillis] protects a file that was only just written, so a sweep
* racing a fetch cannot delete bytes on their way to being shown.
*
* Returns how many files were removed, or 0 when it declined to run.
*/
private var lastAttachmentSweepAt = 0L
fun purgeOrphanedAttachments(
cacheDir: java.io.File,
now: Long = System.currentTimeMillis(),
graceMillis: Long = 60_000L,
minIntervalMillis: Long = 10 * 60_000L,
): Int {
// ⚠️ **Throttled here rather than at the call site**, so it is safe to
// call from the poll loop unconditionally. Building the in-use set
// reads every record in the vault; doing that every few seconds on a
// phone with a lot of history would be a real cost for a cleanup that
// does not need to be prompt.
if (now - lastAttachmentSweepAt < minIntervalMillis) return 0
lastAttachmentSweepAt = now
val inUse = try {
// The SAME transformation `attachmentBytes` applies when it names
// the file — comparing raw ids against mangled filenames would find
// no match and delete everything.
vault.attachmentIdsInUse().map { it.replace('/', '_') }.toSet()
} catch (e: Exception) {
return 0
}
val files = cacheDir.listFiles { f: java.io.File -> f.name.startsWith("att-") }
?: return 0
var removed = 0
for (f in files) {
if (now - f.lastModified() < graceMillis) continue
if (f.name.removePrefix("att-") in inUse) continue
if (f.delete()) removed++
}
return removed
}
fun attachmentBytes(ref: Attachment.Ref, cacheDir: java.io.File): ByteArray {
val cached = java.io.File(cacheDir, "att-" + ref.id.replace('/', '_'))
if (cached.exists()) return cached.readBytes()
@@ -0,0 +1,222 @@
package io.flamenet.messenger.app
import io.flamenet.client.MessengerApi
import io.flamenet.client.RelayClient
import io.flamenet.e2e.Attachment
import io.flamenet.e2e.DeviceIdentity
import io.flamenet.e2e.Vault
import java.io.File
import javax.crypto.KeyGenerator
import javax.crypto.SecretKey
import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
import okhttp3.mockwebserver.MockWebServer
import org.json.JSONObject
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
/**
* Cached plaintext goes when nothing refers to it any more.
*
* 🔴 **`attachmentBytes` writes DECRYPTED bytes and nothing removed them**, so
* a disappearing message expired, the bubble went, and the photo stayed on disk
* indefinitely — the exact failure the feature exists to prevent (android#11,
* spec §6.11).
*
* ⚠️ The dangerous direction here is the opposite of the bug. A sweep that
* deletes too much loses somebody's photo, which is worse than the leak. So
* most of these are about what must SURVIVE.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class AttachmentPurgeTest {
private lateinit var server: MockWebServer
private lateinit var cache: File
@BeforeTest fun start() {
server = MockWebServer().also { it.start() }
cache = File.createTempFile("cache", "").let { it.delete(); it.mkdirs(); it }
}
@AfterTest fun stop() { server.shutdown(); cache.deleteRecursively() }
private class MapStorage : Vault.Storage {
private val records = mutableMapOf<String, ByteArray>()
override fun read(name: String) = records[name]
override fun write(name: String, bytes: ByteArray) { records[name] = bytes }
override fun delete(name: String) { records.remove(name) }
override fun names() = records.keys.toList()
}
private fun key(): SecretKey =
KeyGenerator.getInstance("AES").apply { init(256) }.generateKey()
private lateinit var vault: Vault
private fun session(): Session {
val identity = DeviceIdentity.create()
vault = Vault(MapStorage(), key()).also { it.saveIdentity(identity) }
val base = server.url("/").toString()
return Session("alice", 1, identity, vault,
RelayClient(base, "alice", "app pass word"),
MessengerApi(base, "alice", "app pass word"))
}
private fun ref(id: String) = Attachment.Ref(
kind = Attachment.IMAGE, id = id,
key = ByteArray(32) { 3 }, digest = ByteArray(32) { 4 },
bytes = 9, mime = "image/jpeg", width = 1, height = 1,
)
/** A cache file as `attachmentBytes` would have written it, aged past the grace. */
private fun cached(id: String, ageMillis: Long = 10 * 60_000L): File =
File(cache, "att-" + id.replace('/', '_')).apply {
writeBytes("decrypted".toByteArray())
setLastModified(System.currentTimeMillis() - ageMillis)
}
// ---- The bug ----------------------------------------------------------
@Test
fun `an expired message's cached plaintext is deleted`() {
val s = session()
val orphan = cached("gone-with-the-message")
assertTrue(orphan.exists())
assertEquals(1, s.purgeOrphanedAttachments(cache))
assertFalse(orphan.exists(), "the plaintext of an expired message is still on disk")
}
// ---- What must survive ------------------------------------------------
@Test
fun `an attachment a live message still refers to survives`() {
val s = session()
vault.append("user:2", JSONObject().put("at", 1L).put("text", "")
.put("att", ref("still-shown").toJson()))
val live = cached("still-shown")
assertEquals(0, s.purgeOrphanedAttachments(cache))
assertTrue(live.exists(), "a live message's attachment was deleted")
}
/**
* 🔴 The case that makes a naive sweep destructive: an expired story is
* dropped from the live set and folded into the private archive, which
* keeps its reference.
*/
@Test
fun `an archived story's attachment survives, long after it expired`() {
val s = session()
vault.saveStoryArchive(listOf(Vault.Story(
id = "s1", authorId = 1L, mine = true, ref = ref("archived"),
caption = "", postedAt = 0L, seen = true, expiresAt = 1.0)))
val archived = cached("archived")
assertEquals(0, s.purgeOrphanedAttachments(cache))
assertTrue(archived.exists(), "the archive would now render a blank picture")
}
/** Avatars are attachments, cached the same way, in a store nobody lists. */
@Test
fun `avatars survive`() {
val s = session()
vault.saveAvatar(7L, ref("their-face"))
vault.saveMyAvatar(ref("my-face"))
val a = cached("their-face"); val b = cached("my-face")
assertEquals(0, s.purgeOrphanedAttachments(cache))
assertTrue(a.exists() && b.exists(), "an avatar was deleted as an orphan")
}
/**
* 🔴 **An id containing a slash, which is the reason the transformation
* exists.** `attachmentBytes` writes the file as `att-<id with / -> _>`, so
* membership has to be tested against the SAME mangling. Compare raw ids to
* mangled filenames and nothing matches — every attachment with a slash in
* its id is deleted while still on screen.
*
* ⚠️ Added after a mutation test: removing the transformation broke nothing,
* because every other id here is slash-free. The guard was untested by the
* suite that was meant to be guarding it.
*/
@Test
fun `an id containing a slash is matched against its mangled filename`() {
val s = session()
vault.append("user:2", JSONObject().put("at", 1L).put("text", "")
.put("att", ref("blobs/2026/09/live-one").toJson()))
val live = cached("blobs/2026/09/live-one")
assertEquals("att-blobs_2026_09_live-one", live.name)
assertEquals(0, s.purgeOrphanedAttachments(cache),
"an attachment whose id contains a slash was treated as an orphan")
assertTrue(live.exists())
}
/**
* ⚠️ A file written moments ago is left alone, so a sweep racing a fetch
* cannot delete bytes on their way to being shown.
*/
@Test
fun `a file written just now is left alone`() {
val s = session()
val fresh = cached("just-fetched", ageMillis = 0L)
assertEquals(0, s.purgeOrphanedAttachments(cache))
assertTrue(fresh.exists(), "a file being fetched right now was deleted")
}
/** Nothing outside the attachment cache is touched. */
@Test
fun `unrelated files in the cache directory are never deleted`() {
val s = session()
val other = File(cache, "voice-note.m4a").apply { writeBytes(ByteArray(4)) }
cached("orphan")
assertEquals(1, s.purgeOrphanedAttachments(cache))
assertTrue(other.exists(), "a file that is not an attachment cache entry was deleted")
}
// ---- Failing closed ---------------------------------------------------
/**
* 🔴 If the in-use set cannot be built, NOTHING is deleted. A partial
* answer to "what is still in use" is the one input that turns this from a
* cleanup into data loss.
*/
@Test
fun `an unreadable vault record stops the sweep entirely`() {
val s = session()
val orphan = cached("would-have-gone")
// A record the vault cannot open at all.
vault.append("user:9", JSONObject().put("at", 1L))
val storage = Vault::class.java.getDeclaredField("storage")
.apply { isAccessible = true }.get(vault) as Vault.Storage
storage.write("history/user:9", ByteArray(30) { it.toByte() })
assertEquals(0, s.purgeOrphanedAttachments(cache),
"the sweep ran on a partial view of what is in use")
assertTrue(orphan.exists())
}
// ---- The throttle -----------------------------------------------------
/**
* ⚠️ Building the in-use set reads every vault record, and this is called
* from the poll loop — so a second call moments later does nothing.
*/
@Test
fun `a second sweep moments later does not re-read the vault`() {
val s = session()
cached("first")
assertEquals(1, s.purgeOrphanedAttachments(cache))
cached("second")
assertEquals(0, s.purgeOrphanedAttachments(cache), "the throttle did not hold")
// ...and it does run again once the interval has passed.
assertEquals(1, s.purgeOrphanedAttachments(
cache, now = System.currentTimeMillis() + 11 * 60_000L))
}
}
@@ -1665,6 +1665,65 @@ class Vault(private val storage: Storage, private val key: SecretKey) {
"${m.optBoolean("mine")}|${m.optLong("at")}|${m.optString("text")}|" +
(m.optJSONObject("att")?.optString("id") ?: "")
/**
* Every attachment id this device still has a reason to hold bytes for.
*
* 🔴 **Refs are found by SHAPE, not by looking in the places we remember.**
* An attachment reference serialises with `id`, `key` and `digest`
* together, and nothing else in the vault has all three — so this walks
* every record and matches on that, wherever a ref happens to sit. The
* alternative, a hand-written list of stores, is wrong the day somebody
* adds a store and does not update the list: the sweep that uses this would
* then delete attachments that are still in use, which is a far worse
* failure than the one it exists to fix.
*
* ⚠️ It really is every record, and that is not paranoia. Enumerating by
* hand, the obvious sources are message history and stories — and the vault
* also keeps refs in the story ARCHIVE, in per-contact avatars and in this
* account's own avatar. A sweep built on the obvious two would have deleted
* every avatar on the device the first time it ran.
*
* ⚠️ **Throws rather than returning a partial answer.** A record that
* cannot be opened means this set is incomplete, and an incomplete set of
* "still in use" is exactly the input that turns a cleanup into data loss.
* The caller must treat a throw as "do not sweep".
*/
fun attachmentIdsInUse(): Set<String> {
val found = mutableSetOf<String>()
for (name in storage.names()) {
val record = readJson(name) ?: continue
collectAttachmentIds(record, found)
}
return found
}
/**
* ⚠️ Depth-bounded. The walk is over data this device wrote, but a record
* is a file on disk and a cycle or a pathological nesting must not become
* a stack overflow inside a cleanup.
*/
private fun collectAttachmentIds(node: Any?, into: MutableSet<String>, depth: Int = 0) {
if (depth > 24) return
when (node) {
is JSONObject -> {
val id = node.optString("id")
// The three together are what makes it a reference to bytes.
// A sticker carries `pack` + `id`; a story SET carries
// `id` + `i` + `n` and its id names the sequence, not a blob.
// Neither has a key or a digest, and neither is matched here.
if (id.isNotEmpty() &&
node.optString("key").isNotEmpty() &&
node.optString("digest").isNotEmpty()
) {
into += id
}
for (key in node.keys()) collectAttachmentIds(node.opt(key), into, depth + 1)
}
is JSONArray ->
for (i in 0 until node.length()) collectAttachmentIds(node.opt(i), into, depth + 1)
}
}
/** Every thread this device holds, group or person. */
fun threads(): List<String> =
storage.names().filter { it.startsWith(HISTORY_PREFIX) }
@@ -0,0 +1,155 @@
package io.flamenet.e2e
import org.json.JSONObject
import javax.crypto.KeyGenerator
import javax.crypto.SecretKey
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertFalse
import kotlin.test.assertTrue
/**
* Which attachment ids the device still has a reason to hold bytes for.
*
* 🔴 **This set is the input to a DELETE, so the dangerous error is the one
* that omits something.** A missing id means a cleanup removes a file that is
* still in use — a worse failure than the leak it was written to fix, because
* the leak only wastes space and this loses somebody's photo.
*
* So the cases below are mostly about the stores an enumeration would forget.
*/
class AttachmentsInUseTest {
private class MapStorage : Vault.Storage {
val records = LinkedHashMap<String, ByteArray>()
override fun read(name: String) = records[name]
override fun write(name: String, bytes: ByteArray) { records[name] = bytes }
override fun delete(name: String) { records.remove(name) }
override fun names() = records.keys.toList()
}
private fun key(): SecretKey = KeyGenerator.getInstance("AES").apply { init(256) }.generateKey()
private fun ref(id: String) = Attachment.Ref(
kind = Attachment.IMAGE, id = id,
key = ByteArray(32) { 7 }, digest = ByteArray(32) { 9 },
bytes = 100, mime = "image/jpeg", width = 2, height = 2,
)
private fun story(id: String, attId: String, expiresAt: Double) = Vault.Story(
id = id, authorId = 1L, mine = true, ref = ref(attId),
caption = "", postedAt = 0L, seen = false, expiresAt = expiresAt,
)
// ---- The stores an enumeration forgets --------------------------------
@Test
fun `a message's attachment counts as in use`() {
val storage = MapStorage(); val k = key()
val v = Vault(storage, k)
v.append("user:1", JSONObject().put("text", "").put("at", 1L)
.put("att", ref("in-a-message").toJson()))
assertTrue("in-a-message" in Vault(storage, k).attachmentIdsInUse())
}
@Test
fun `a live story's attachment counts`() {
val storage = MapStorage(); val k = key()
Vault(storage, k).saveStories(listOf(story("s1", "in-a-story", 4_000_000_000.0)), 1.0)
assertTrue("in-a-story" in Vault(storage, k).attachmentIdsInUse())
}
/**
* 🔴 **The archive is the case that makes a naive sweep destructive.**
* An expired story is dropped from the live set AND folded into the private
* archive, which keeps its ref. Purging on expiry without looking here
* would leave somebody's own archive full of blank pictures — the exact
* thing the archive exists to prevent.
*/
@Test
fun `an ARCHIVED story's attachment still counts, long after it expired`() {
val storage = MapStorage(); val k = key()
val v = Vault(storage, k)
// Expired: gone from the live set, kept in the archive.
v.saveStories(listOf(story("s1", "expired-blob", 1.0)), 1_000_000.0)
v.saveStoryArchive(listOf(story("s1", "expired-blob", 1.0)))
val inUse = Vault(storage, k).attachmentIdsInUse()
assertTrue("expired-blob" in inUse,
"an archived story's attachment was reported unused — the archive would go blank")
}
/**
* 🔴 **Avatars are attachments too.** They are fetched and cached by the
* same code path, and they live in a store nobody thinks of when listing
* "places messages are". A sweep that enumerated history and stories would
* have deleted every avatar on the device the first time it ran.
*/
@Test
fun `avatars count — both a contact's and this account's own`() {
val storage = MapStorage(); val k = key()
val v = Vault(storage, k)
v.saveAvatar(42L, ref("their-face"))
v.saveMyAvatar(ref("my-face"))
val inUse = Vault(storage, k).attachmentIdsInUse()
assertTrue("their-face" in inUse, "a contact's avatar was reported unused")
assertTrue("my-face" in inUse, "this account's own avatar was reported unused")
}
// ---- Things that look like refs and are not ---------------------------
/**
* ⚠️ A sticker carries `pack` + `id`, and a story SET carries `id` + `i` +
* `n` where the id names the SEQUENCE, not a blob. Matching on `id` alone
* would pull both in — harmless for a set, but it would keep files alive
* forever under ids that name nothing.
*/
@Test
fun `ids that are not attachment references are not collected`() {
val storage = MapStorage(); val k = key()
val v = Vault(storage, k)
v.append("user:1", JSONObject().put("text", "").put("at", 1L)
.put("sticker", JSONObject().put("pack", "p").put("id", "sticker-id"))
.put("set", JSONObject().put("id", "sequence-id").put("i", 0).put("n", 3)))
val inUse = Vault(storage, k).attachmentIdsInUse()
assertFalse("sticker-id" in inUse, "a sticker id was mistaken for an attachment")
assertFalse("sequence-id" in inUse, "a story SET id was mistaken for an attachment")
}
/** The discriminator is all three fields, so two of them is not enough. */
@Test
fun `an object with an id and a key but no digest is not a reference`() {
val storage = MapStorage(); val k = key()
Vault(storage, k).append("user:1", JSONObject().put("at", 1L)
.put("odd", JSONObject().put("id", "two-of-three").put("key", "x")))
assertFalse("two-of-three" in Vault(storage, k).attachmentIdsInUse())
}
// ---- Failing safe -----------------------------------------------------
/**
* 🔴 **An unreadable record must throw, not be skipped.** Skipping it
* returns a set that is missing whatever it held, and a caller that deletes
* everything outside that set would then delete live attachments. The
* cleanup has to refuse to run rather than run on a partial view.
*/
@Test
fun `a record that cannot be opened stops the whole answer`() {
val storage = MapStorage(); val k = key()
Vault(storage, k).append("user:1", JSONObject().put("at", 1L)
.put("att", ref("real").toJson()))
storage.records["history/user:2"] = byteArrayOf(1, 2, 3, 4, 5, 6, 7, 8,
9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26,
27, 28, 29, 30)
assertFailsWith<Vault.Corrupt> { Vault(storage, k).attachmentIdsInUse() }
}
@Test
fun `an empty vault reports nothing in use rather than failing`() {
assertEquals(emptySet(), Vault(MapStorage(), key()).attachmentIdsInUse())
}
}