Detect configs expired from the swarm and re-store them by mpretty-cyro · Pull Request #2151 · session-foundation/session-android

mpretty-cyro · 2026-08-05T05:56:43Z

Config recovery: keep configs alive, and put them back when the swarm loses them

Session Android · feature/config-recovery → dev · 16 commits

What this fixes

Two TTL bugs, both silent. Group config TTL extension was gated on holding an admin key, so a group
whose admins had gone quiet — exactly the group whose configs expire — had nobody extending them. And the
extension asked for 14 days against a 30-day config TTL, at both call sites.

Configs that the swarm has already dropped are never restored. Nothing noticed, and nothing put them
back. A device could hold a perfectly good config the network had forgotten.

A group whose keys expire is unrecoverable, and shows a permanent banner. Nothing on any device could
put a keys message back: a member cannot re-sign one, and no device kept the original bytes to re-send. The
banner was therefore permanent by construction rather than by policy.

How it works

Detection. The expire extension response already says which hashes the swarm still holds; we now read
it. A hash in neither updated nor unchanged is gone. The rules are subtle and each has a vector: a
failed sub-response is excluded, not read as absence; a response that could not be read is inconclusive,
not "everything is missing"; asking about nothing is inconclusive rather than "nothing is missing".

Recovery. Missing hashes are re-stored from local state, byte-identical, so they land on the same hash
and are idempotent. Guarded so recovery can only ever put back what we already had, never create state:
only clean configs, only hashes still current, never a group we have been kicked from. Any member can do
this, not only an admin — a read-only config re-emits the signature it received, so a member's bytes are
identical to the admin's.

Group keys. libSession retains the bytes of keys messages it has loaded, and re-storing those bytes
repairs a group's keys without re-signing them — so a member can repair a group. Groups that predate
retention hold hashes with no bytes behind them, so a backfill re-loads their keys messages to capture
the bytes. Where even that fails, an admin-only force-rekey mints a new generation.

The backfill runs proactively on poll, not when a config is found missing, and that is deliberate rather
than wasteful: it fires when we lack bytes for a hash the swarm still has — the opposite condition to
detection, which fires when the swarm has lost a hash. Moving it behind detection would look tidier and
would repair almost nothing, because by then the message it needs is already gone. It is one small read per
affected group, it records the attempt so a group beyond help stops re-reading, and the condition clears
itself once the bytes are captured.

Reviewing this

The force-rekey can be dropped on its own. One entry point, and no state shared with the backfill in
either direction. With its members, its call site and its tests removed, the backfill's tests still pass.

What stops it firing wrongly. A rekey is the only irreversible, every-member-visible write here. It
refuses unless all of:

the device is an admin — a member holds no signing key, and must not appear to try;
a backfill has already run for this group and the bytes are still absent — checked by the caller, so
"has the other path had its turn" stays the caller's knowledge;
the group's config state is level with the swarm as of the poll that just completed — not merely level
at some point in the session;
it has not already rekeyed this group within 24 hours.

The third matters most, and the risk is worth stating rather than the check. A rekey encrypts the new key
to this device's view of the members config, and this path fires precisely on devices whose config
state is known to be degraded. Issued from a stale view it silently excludes anyone this device hasn't
merged yet — no error anywhere, and the excluded member simply stops being able to read the group.

What holds that off: the "level with the swarm" mark is recorded against the poll that established it, and
the rekey requires that recorded poll to be the one it is running in — not merely that some poll this
session was level. That distinction is the entire guard. A device last fully level yesterday, offered a
members update since that it hasn't merged, still carries a mark, and a check for the mark's presence
would wave it through. The mark is also laid down only when all three configs merged completely, which is
stronger than the members config alone and fails closed: it can decline when the members view was in fact
fine, and the only cost is a rekey deferred to a later poll.

The residual, honestly: this narrows the window to one poll, it does not close it. A members update that
reaches the swarm after our poll completed and before the rekey is issued is still missed, and no
client-side check can fix that — a device cannot account for config it has never been offered.

And plainly, because "irreversible" invites the wrong worry: no config can be lost here. A rekey adds a
generation. It deletes nothing, and every config message this device holds is untouched. The worst case is
a member quietly losing access to the group, recoverable by an admin.

The commits are separable. TTL fixes, detection, recovery, keys backfill and force-rekey land in that
order and each is independently reviewable.

Test names carry vector numbers (V8b, V22c, V24b…) shared with the iOS and Desktop implementations
of the same behaviour, so a disagreement between clients can be pinned to one rule rather than a feature.
They are a shared vocabulary, not a reference to anything outside this repo.

Not done here, deliberately

BatchApiExecutor has no size cap and is re-entrant. Recovery chunks its own requests at the server's
20-sub-request limit, but nothing stops an unrelated request sharing the 100ms batch window, and a retry
re-enters the batcher at an arbitrary later time. The bound belongs in the executor; chunking lower at the
caller narrows the coincidence without removing it.
BatchApiExecutor.kt:176 pairs requests to responses by index. It is fail-safe today because a size
mismatch throws and the catch fails every pending request. If that check is ever "tidied" into a filter,
responses shift by one and detection is misattributed rather than disabled.

Dependency

Requires two libSession changes, both open in libsession-util#123 and not yet merged. The
second is easy to miss:

Keys::active_key_messages() — the accessor the re-store path reads.
the dump flag on insert_key's early-return path (a18b0f08), which records the bytes and marks the
config dirty when a keys message we already hold arrives again. This is what the backfill depends on,
and its absence fails silently: without it the bytes are captured in memory and lost on next launch, so
the backfill appears to work and does not. A pin carrying the accessor but not this ships a no-op.

They are on the same branch and would normally land together, but they are separable, so a pin bump should
be checked for both rather than for the accessor alone.

The wrapper binding is on LibSession-Util-Android feature/group-keys-recovery. Order is
libSession → wrapper re-pin → this. Test numbers here were measured against libSession 1d565e31 plus
those changes, i.e. the head of libsession-util#123.

…re-store them Config messages have a 30-day TTL that is refreshed by the expire bump already piggybacked on every poll. A device offline past the TTL loses its config from the swarm and nothing noticed. This reads the bump's own answer to detect that case and re-stores from local state. Reasoning for each rule is in the code. - Detect missing config hashes from the `unchanged` array the server returns for an extend-only expire. An absent key means detection is unavailable, not that every config is gone. - Re-store on a later poll, only once local state is level with the swarm, so a stale local config can never overwrite a newer remote one. - Chunk the recovery batch at 20 sub-requests; the server rejects the whole sequence above that, and a single config can split into ~66 parts. - Run the obsolete-hash deletes as a separate pass after the stores, built only from restores that fully landed, so a delete can never go out over a config we did not finish storing. - Bar a settled hash for an hour rather than for the session; a backgrounded session can outlive the TTL, which would strand the config it just restored. - Drop the per-store semaphore and let BatchApiExecutor coalesce; the semaphore was bounding batch size by accident, not by design.

… in the repo The comments added by the previous commit cited section numbers from the cross-client design notes they were written against. That document lives in no repo and won't be in the PR, so every citation was a dangling pointer — and a bare "§4.1" is worse than none, because it implies the reasoning lives somewhere the reader is expected to find and can't. The comments were written to carry their reasoning inline, so most citations were pure deletion; the few that leaned on one had the substance written in instead. Vector labels (V13b, V22c) stay in test names: they identify a test within this repo and are shared vocabulary with the other Session clients, so a divergence can be pinned to one rule. Also corrects a comment that still said the chunking fixture "crosses 19" after the sub-request limit moved to 20. Comments only, no behaviour change.

…test checks Test names and assertion messages are the one kind of stale prose nothing contradicts: a wrong comment sits beside code that disagrees with it, but a green test prints its own description as passing on every run. - `V13 ... is put back once` did not advance the clock, so a session-scoped bar and a time-bounded one both satisfied it. The unqualified "once" was a property no assertion there checked; it now says "while the bar holds" and names V13g as the only test that separates the two. - Three vector labels each sat on two different tests (V13b, V16, V22c), in every case because a follow-on test reused the label of the one above it. The continuations are real properties but not vectors, so they are now prose-named. Duplicate labels also defeat a coverage audit by label: an uncovered row can read as covered. Test names and comments only, no assertions changed.

…the guard Both asserted that a response is not read for absence without an extend request, and both used a fixture whose `unchanged` key was absent — which makes the response unreadable on its own grounds. So each passed for a reason unrelated to its name, and would have passed against an implementation that read shorten responses for absence. Verified by mutation rather than by inspection: deleting the extend guard left both green beforehand, and fails both now. The fixtures are readable (`unchanged` present and empty) so the guard is the only thing that can produce Inconclusive. Tests only, no production change.

…factual on purpose Both fixtures send a readable sub-response for a request that a real server would answer without one, so that a single guard is the only thing producing the asserted result. That makes them less accurate than production on purpose, and the previous comments explained the isolation without saying so — leaving the next reader free to restore the realistic shape for accuracy and silently make both tests vacuous again, passing. Where two guards cover two different failures, production usually trips both at once, so isolating one needs a counterfactual, and a counterfactual needs a note or it gets corrected away. Comments only.

… own value detectMissingConfigHashes had three conditions returning one Inconclusive, and AlterTtlApi's decode-failure path returned the same value for a fourth reason. No consumer discriminates on it, so the collapse cost nothing at runtime — it cost every test that touches those guards, because a fixture set up for one condition usually satisfies another too, and the test then passes without reaching the guard it names. Two did exactly that. Inconclusive becomes a sealed interface over ExtendNotRequested, NothingAsked, NoUsableSubResponse and ResponseUnreadable. Behaviour is unchanged and deliberately so: all four still mark nothing missing and authorise no store, and the two sites that discriminate do so on Checked, which a sealed subtype still satisfies. Neither poller names the type. The decode failure is kept separate from NoUsableSubResponse rather than folded in: one means we could not read the answer, the other means the swarm answered and told us nothing. Collapsing them would reintroduce, in the type added to prevent it, the ambiguity this removes. Two assertions that sampled one value for all causes now iterate every cause, so a cause added later cannot quietly acquire the power to flag a group expired or authorise a store.

…mit made false Splitting the inconclusive causes turned this fixture's failure mode from silent to loud: restoring the realistic `unchanged = null` now returns NoUsableSubResponse where the assertion names ExtendNotRequested, so the test fails rather than passing vacuously. The comment still warned about the silent version. The same sentence in AlterTtlApiTest was updated in that commit; this is its twin one file over, which was missed. Comment only.

…d actually is Dirtying a config moves its current hashes into the old set and clears them, so a dirty config usually no longer holds anything the swarm reported missing and the hash-intersection check rejects it before the clean check is consulted — which makes that check look like dead code to anyone auditing it. It is not, and the exception is the reason to keep it: active hashes are the current hashes plus the parts of any pending multipart set, and that second component survives dirtying. A config that went dirty mid-multipart, one of whose part hashes has been lost, reaches the check with a real intersection. Both facts are libsession's rather than ours and cannot be asserted from a JVM unit test, which cannot load the native library — so the reasoning is recorded at the guard with its source location, and the test that reaches the branch now says what it does not establish, since it gets there through a mock that can present a state the real path reaches only one way. Comments only.

…no test reached A sub-response counts as usable only if it is not failed AND carries an unchanged map. Every failed-node fixture in the suite left unchanged absent, so the second condition did all the excluding and the first was never consulted: deleting it passed all 64 tests. The case it alone covers is a snode that reports failure while still carrying unchanged. Read as usable, its empty arrays become authority and every requested hash is reported missing — re-storing configs the swarm still holds, on the word of a node that said it failed. Confirmed rather than assumed: dropping the term alongside a control mutation known to kill two tests left this hole alive while the control fired, proving the build carried both edits. The new test now dies on that same mutation and nothing else does.

…sub-response A sub-response is unusable if it failed or if it omits unchanged, and this vector is about exactly that — but the fixture had no failed node at all, so it exercised the second route twice and was insensitive to the first. It now mixes all three cases and dies under either term being dropped. The failed node claims to hold nothing, which is what makes its exclusion observable: absence is decided by any(), so one snode reporting a hash missing already settles it and a failed node that HELD the hashes could not change the verdict — including it would be inert and the test would survive looking correct. Claiming nothing moves the expected set from {h2} to {h1,h2}.

…s being unrecoverable Four comments and a test name said libsession offers no way to re-serialise a loaded keys config. libsession retains the bytes of active keys messages and exposes them; what is missing is a JNI binding, so the limitation is this platform's and not the format's. The distinction is the whole point of the comments: attributing it to libsession reads as impossible, when it is in fact not yet plumbed — and a member CAN repair a group's keys by pushing retained bytes back, since they land on the same hash without being re-signed. The keys-exclusion test now records that it should invert when the binding lands, rather than be deleted, and no longer asserts the impossibility in its own name.

The keys-exclusion comment justified the expired banner with "only an admin rekey can fix it". That is false once any device holds the bytes: retention happens on LOAD, not on authorship, so an admin immediately after its own rekey holds nothing for the message it just created and is the device least able to repair it. A member holding the bytes can push them back, signature and all. An admin rekey is the remedy only when no device anywhere still holds the bytes, which is what the banner is actually for.

libsession retains the raw bytes of every keys message this device has loaded, and pushing them back lands on the same hash without being re-signed — so a member can repair a group whose keys the swarm has dropped, and an admin immediately after its own rekey holds nothing and is the device least able to. - Keys join the restorable set. Every retained message goes back, not just the reported one: a generation is a rekey plus its supplementals, a member holding part of one cannot derive the key, and the retained map carries no generation field to group by. The superset is bounded and idempotent. - "Expired" now means the keys are gone AND this device cannot put them back. That is one rule, so canRepairKeys is an input to the determinant rather than an override applied to its answer — a lambda, so answering it (which takes the config lock) is deferred until the guards that could make it irrelevant have passed. - The flag is withheld rather than raised and cleared. It drives a visible conversation banner, so correcting it afterwards is a flicker on a group that was never out of reach. - A successful keys re-store announces itself, and ExpiredGroupManager merges that in as another "not expired" event rather than exposing a setter. The reactive clear fires when a keys message is HANDLED, and the device that re-stored it already holds that hash, so it may never handle it again. - One predicate behind both the flag and the re-store. If they disagreed as "flag says repairable, recovery declines", the banner would never appear and nothing would be fixed. Requires the activeKeyMessages() binding on the libsession-util-android branch.

… the flag Detection answered `false` when this device held the keys bytes, which CLEARS the expired flag — and the poller applied that before running the repair. If the re-store then failed nothing raised the flag again: the next poll saw the same missing hashes and the same retained bytes and answered `false` once more, so a device whose stores permanently fail never showed the banner at all. Holding the bytes is not the same answer as having put them back. Detection now defers (null) and the round settles it: repaired, attempted-and-failed, or never attempted. The third state is the one whose absence caused this — recovery declines when backgrounded, backing off, or not level with the swarm, and none of those mean the group is reachable. onRestored becomes onAttempted(restore, landed) for the same reason: a caller told only about successes cannot tell "failed" from "never attempted", and those need opposite treatment of the flag. V23c asserted only that nothing was emitted on keysRestored, so it passed while a different site in the same poll had already cleared the flag. It now asserts the verdict too, and V23a asserts the distinction rather than the value — a test expecting `false` there passes against the broken implementation. Found by cold review (Q2).

Retention records a keys message's bytes when the message is LOADED. A group that existed before retention shipped loaded its keys long ago, so it holds the keys and the hashes and no bytes — and bytes are what a re-store needs. Feeding those same messages back through the ordinary merge fixes it: libsession takes the "we already have this key" early return, which is a no-op for key state and is not a no-op for retention. Proactive, not driven by expiry detection. The two ask opposite questions — detection fires when the swarm has LOST a hash, this fires when WE lack bytes for one the swarm still HAS — so by the time detection fires the message this needs is already gone. It stores nothing. It restores the input the existing re-store path consumes, rather than being that path a second time. The attempt is recorded before the fetch, so a group whose keys are genuinely gone stops re-reading its namespace every poll. That record is a separate per-group map from the re-store bar: sharing the store would bar the very hashes a successful backfill just made recoverable, suppressing the re-store this exists to enable — and a repair that does not happen does not error. Bytes survive a restart because the merge runs inside doWithMutableGroupConfigs and dumpIfNeeded writes storeGroupConfigs. Loading the keys directly would capture them in memory and lose them on next launch. Requires the retention accessor, which is not yet in the pinned libSession, so this is measured against unlanded work.

Last resort for a group whose keys are gone from the swarm and whose bytes no device here holds: mint a new generation so the group can carry on, accepting that content encrypted to the superseded keys stays unreadable. Built to be removed. One entry point, no state shared with the backfill, and the storm guard lives here with its own constant rather than beside the re-store bar — a redundant re-store is a byte-identical no-op, a redundant rekey is an irreversible write every member must process. Deleting this file, its test and its call site leaves the backfill's vectors passing, which is the property that matters if the rekey is dropped. A rekey encrypts the new key to THIS DEVICE'S view of the members config, and this path fires precisely on devices whose config state is degraded, so a member added while we were away would be silently dropped from the group. It therefore refuses unless the members view is level with the swarm AS OF THE POLL THAT JUST COMPLETED — deliberately not the session-scoped level predicate, which means "was level at some point and has not since been withdrawn" and fails open at exactly the moment that matters. Admin-only: a member holds no signing key, so an attempted rekey would fail at signing and surface as an error on a path that was never applicable. The caller owns only the sequencing precondition — a backfill ran and the verdict still says unrepairable. Everything about whether the write is safe lives here and goes when this goes.

…ekey guards - Merge KeysBackfill and ForceRekey into ExpiredConfigRecovery with no behaviour change. GroupPoller takes one dependency where it took three. - Raise the rekey storm guard from 1h to 24h. A redundant re-store is a byte-identical request, so that bar errs short; a redundant rekey makes every member on every version process a new generation, and a group reaching this path has had no retrievable keys message for at least the 30-day config TTL. - Replace the rekey's membersLevelAsOfThisPoll flag with a poll token minted at the top of each poll. The level mark now records which poll made it, so the rekey can demand a mark from the poll it is running in while the re-store keeps the looser "level at some point this session" reading of the same field. Adds V25e (a mark from an earlier poll does not authorise a rekey, and the mark is still present when it refuses) and V25f (another swarm's poll does not make ours stale). Also drops vector references from comments where a reviewer cannot follow them.

…nt one The rekey asked only whether the level mark equalled the token it was handed, which a caller holding a token from an earlier poll satisfies: the mark that poll left is still in the map, so a stale caller was told it was level now. A token names a poll, it does not make that poll current. beginPoll() now takes the swarm key and records that swarm's current poll, and the guard requires the carried token to be that one as well as the mark's. Kept per-swarm so another swarm's poll still cannot age ours out. Not reachable from our own poller, which always passes the token it just minted, but the guard has to hold against its caller rather than because of it. V25e gains the step that catches it: once a later poll supersedes it, the token of the poll that made the mark is not a way back in. iOS hit the same hole.

Drops decorative emoji markers and heavy bold from this feature's comments, and rewrites three comments that narrated a change rather than stating what is true. Emoji appear nowhere else in this repo as comment markers; the base's uses are emoji literals in reaction UI. Bold does exist in the base, 32 spans of it, so this is a density correction rather than removing an import: 70 spans across 15 files against 32 across the other 894. The five pre-existing spans in ConfigFactoryProtocol are left alone. The narration was in KeysBackfillTest, ExpiredConfigRecoveryTest V23c and the force-rekey seam in GroupPoller, each of which described what the code used to do. Content is unchanged; a comment that dates itself against a previous revision stops being true as soon as anything moves.

mpretty-cyro self-assigned this Aug 5, 2026

mpretty-cyro force-pushed the feature/config-recovery branch from 88ca431 to 2d92d84 Compare August 7, 2026 01:20

mpretty-cyro marked this pull request as draft August 16, 2026 23:26

mpretty-cyro added 19 commits September 4, 2026 09:58

mpretty-cyro force-pushed the feature/config-recovery branch from 2d92d84 to 44c18f8 Compare September 8, 2026 06:30

mpretty-cyro marked this pull request as ready for review September 8, 2026 06:33

Navigation Menu

Uh oh!

Detect configs expired from the swarm and re-store them - #2151

Detect configs expired from the swarm and re-store them#2151
mpretty-cyro wants to merge 19 commits into
session-foundation:devfrom
mpretty-cyro:feature/config-recovery

mpretty-cyro commented Aug 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mpretty-cyro commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Config recovery: keep configs alive, and put them back when the swarm loses them

What this fixes

How it works

Reviewing this

Not done here, deliberately

Dependency

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mpretty-cyro commented Aug 5, 2026 •

edited

Loading