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

2026-09-04T22:29:32Z

paulhitt commented

2026-09-04 22:29:32 +00:00

(Migrated from gitlab.com)

attachmentBytes wrote DECRYPTED bytes to cacheDir/att-<id> and nothing
ever removed them. So a disappearing message expired, the bubble went, and
the photo or voice note stayed on disk indefinitely — the exact failure the
feature exists to prevent. Spec §6.11: attachments MUST be purged with the
message that referenced them.

A sweep, not a purge at each deletion point, and that changed what the
fix had to be. FOUR paths drop a row — message expiry, story expiry,
delete-for-me and clear-thread — so threading "here is what I removed" out
of each 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.

Refs are found by SHAPE. An attachment reference serialises with
id, key and digest together and nothing else in the vault has all
three, so attachmentIdsInUse walks every record and matches on that. A
hand-written list of stores is wrong the day somebody adds a store and
does not update it — and the sweep would then delete attachments still in
use, which is far worse than the leak.

That is not hypothetical: enumerating by hand gets it wrong today.
The obvious sources are history and stories. The vault ALSO holds 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, and blanked the user's own story
archive — which is the one thing that archive exists to prevent.

Three more safety properties, each with a test that fails without it:
fails closed when the in-use set cannot be built (a partial answer is what
turns a cleanup into data loss); a grace period so a sweep racing a fetch
cannot delete bytes on their way to the screen; and a throttle inside the
function, since it reads every vault record and is called from the poll
loop.

The cost is that a file survives to the next sweep rather than going at
the instant of expiry — named in the code rather than left implied.

17 tests. Every guard proved by breaking it. One did NOT hold at first:
removing the / -> _ filename mangling broke nothing, because every id
in the suite was slash-free — the guard was untested by the tests meant to
be guarding it. Added the case; it now fails as it should.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

`attachmentBytes` wrote DECRYPTED bytes to `cacheDir/att-<id>` and nothing ever removed them. So a disappearing message expired, the bubble went, and the photo or voice note stayed on disk indefinitely — the exact failure the feature exists to prevent. Spec §6.11: attachments MUST be purged with the message that referenced them. **A sweep, not a purge at each deletion point, and that changed what the fix had to be.** FOUR paths drop a row — message expiry, story expiry, delete-for-me and clear-thread — so threading "here is what I removed" out of each 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. **Refs are found by SHAPE.** An attachment reference serialises with `id`, `key` and `digest` together and nothing else in the vault has all three, so `attachmentIdsInUse` walks every record and matches on that. A hand-written list of stores is wrong the day somebody adds a store and does not update it — and the sweep would then delete attachments still in use, which is far worse than the leak. **That is not hypothetical: enumerating by hand gets it wrong today.** The obvious sources are history and stories. The vault ALSO holds 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, and blanked the user's own story archive — which is the one thing that archive exists to prevent. Three more safety properties, each with a test that fails without it: fails closed when the in-use set cannot be built (a partial answer is what turns a cleanup into data loss); a grace period so a sweep racing a fetch cannot delete bytes on their way to the screen; and a throttle inside the function, since it reads every vault record and is called from the poll loop. The cost is that a file survives to the next sweep rather than going at the instant of expiry — named in the code rather than left implied. 17 tests. Every guard proved by breaking it. One did NOT hold at first: removing the `/` -> `_` filename mangling broke nothing, because every id in the suite was slash-free — the guard was untested by the tests meant to be guarding it. Added the case; it now fails as it should. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

paulhitt commented

2026-09-04 22:29:32 +00:00

(Migrated from gitlab.com)

assigned to @paulhitt

paulhitt commented

2026-09-04 22:29:56 +00:00

(Migrated from gitlab.com)

mentioned in commit 93dd278434

mentioned in commit 93dd2784340ffcd0f04a7ff57c8ac06676fa146c

paulhitt (Migrated from gitlab.com) merged commit 93dd278434 into main

2026-09-04 22:29:56 +00:00

paulhitt commented

2026-09-04 22:29:57 +00:00

(Migrated from gitlab.com)

mentioned in issue #11

paulhitt commented

2026-09-09 19:01:22 +00:00

(Migrated from gitlab.com)

changed the description

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: extant2000/flamenet-messenger-android#87