A failed revoke on block is shown, and a failed revoke on sign-out is finished next time

Two security operations discarded their result, the same two the iPhone
had. One commit because both wires live in the view model.

Block: the token revoke was a nested runCatching whose value nobody read.
The block must not fail because the follow-up did, and that reasoning was
right; making the follow-up silent was not. A revoke that fails leaves the
blocked person holding capabilities that deliver sealed to this inbox, the
opposite of what "Blocked" now says. Kept the block, surfaced the failure,
and said that blocking again retries it.

Sign-out: the device revoke was discarded on the line before the keys that
authorise it are destroyed, so a failure was unrecoverable and invisible.
What it leaves is a dead device row the relay fans every envelope to and
every sender spends a prekey on; one was found this morning with 42
undelivered. The device id is now kept in preferences, not the vault,
because the vault is destroyed a line later and a note there would not
survive. Keyed by account, retried on the next sign-in to that account,
cleared only when the relay accepts.

The helper's tests round-trip the note, keep it account-scoped, and wipe
filesDir, which is what destroy does to storage, to prove the note lives
elsewhere. The view-model wiring is covered by compile and the suite;
there is no view-model harness, so the two wires are verified by reading.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-17 06:24:29 -04:00
co-authored by Claude Fable 5.1
parent 6c4289ee42
commit 4cfc044198
3 changed files with 139 additions and 11 deletions
@@ -329,7 +329,7 @@ class MessengerViewModel(
api =
MessengerApi(credential.relayBaseUrl, credential.username, credential.appPassword),
)
session?.let { wireSealingPolicy(it) }
session?.let { wireSealingPolicy(it); retireLeftoverDevice(it) }
val fetchedBuddies = runCatching { session!!.roster() }
val fetchedGroups = runCatching { session!!.namedGroups() }
stored to
@@ -608,7 +608,7 @@ class MessengerViewModel(
relay = relay,
api = io.flamenet.client.MessengerApi(config.baseUrl, account.username, credential),
)
session?.let { wireSealingPolicy(it) }
session?.let { wireSealingPolicy(it); retireLeftoverDevice(it) }
// Our return address, before the first federated send.
this.accountKeySeed = accountKeySeed
@@ -2033,16 +2033,31 @@ class MessengerViewModel(
withContext(Dispatchers.IO) {
runCatching {
current.api.setBlocked(userId, blocked)
// And take their sealed capabilities with it.
if (blocked) {
runCatching { current.conversations.revokeDeliveryTokens(userId) }
}
current.api.blocked().toSet() to current.roster()
// And take their sealed capabilities with it. Never allowed
// to fail the block, which is the safety action, and never
// silent either: a revoke that fails leaves the blocked
// person still holding capabilities that deliver sealed to
// this inbox, the opposite of what the screen now says.
val revoke =
if (blocked) runCatching { current.conversations.revokeDeliveryTokens(userId) } else null
Triple(current.api.blocked().toSet(), current.roster(), revoke?.exceptionOrNull())
}
}
done
.onSuccess { (blockedIds, list) ->
update { it.copy(busy = false, blocked = blockedIds, buddies = list) }
.onSuccess { (blockedIds, list, revokeFailure) ->
update {
it.copy(
busy = false,
blocked = blockedIds,
buddies = list,
error =
revokeFailure?.let { f ->
"Blocked. Their sealed-delivery capability could not be revoked right now " +
"(${describe(f)}), so a message they already prepared may still arrive. " +
"Blocking again retries it."
},
)
}
}.onFailure { failure -> update { it.copy(busy = false, error = describe(failure)) } }
}
@@ -3212,6 +3227,19 @@ class MessengerViewModel(
/**
* Wire the sealing policy and counters into a fresh session (ios#16).
*/
/**
* Finish a revoke a previous sign-out could not. Best effort and off the
* launch path; a relay still unreachable leaves the note for next time.
*/
private fun retireLeftoverDevice(current: Session) {
val old = UnretiredDevice.read(getApplication(), current.userId) ?: return
if (old == current.identity.deviceId) return
viewModelScope.launch(Dispatchers.IO) {
runCatching { current.revokeDevice(old) }
.onSuccess { UnretiredDevice.clear(getApplication(), current.userId) }
}
}
private fun wireSealingPolicy(session: Session) {
val settings = MediaSettings(getApplication())
session.conversations.requireSealed = { settings.requireSealedSending }
@@ -4054,8 +4082,14 @@ class MessengerViewModel(
if (erasingDevice) {
withContext(Dispatchers.IO) {
// Tell the relay BEFORE destroying the keys that authorise
// saying it.
if (myDevice != null) runCatching { current?.revokeDevice(myDevice) }
// saying it. And keep the answer: a revoke that fails here
// used to be unrecoverable, because the next line destroys
// the keys, and invisible, because nothing read the result.
if (myDevice != null && current != null) {
runCatching { current.revokeDevice(myDevice) }
.onSuccess { UnretiredDevice.clear(getApplication(), current.userId) }
.onFailure { UnretiredDevice.remember(getApplication(), current.userId, myDevice) }
}
KeystoreVault.destroy(getApplication())
}
identity = null
@@ -0,0 +1,46 @@
package io.flamenet.messenger.app
import android.content.Context
/**
* A device a sign-out could not revoke on the relay, remembered so the next
* sign-in to the same account finishes the job.
*
* Sign-out asked the relay to revoke the device and discarded the answer, then
* destroyed the keys that authorise asking. A revoke that failed was therefore
* unrecoverable and invisible: the relay went on listing a device nobody
* held, fanning every envelope out to it and letting every sender spend a
* prekey on it. One was found this way with 42 undelivered.
*
* Kept in preferences, not the vault, because the vault is destroyed a line
* after the revoke is attempted and a note left there would not survive.
* Keyed by account, so the note is only ever acted on by the account that
* can act on it.
*/
object UnretiredDevice {
private const val PREFS = "flamenet-unretired-devices"
private fun prefs(context: Context) = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE)
private fun key(userId: Long) = "u$userId"
fun remember(
context: Context,
userId: Long,
deviceId: String,
) {
prefs(context).edit().putString(key(userId), deviceId).apply()
}
fun read(
context: Context,
userId: Long,
): String? = prefs(context).getString(key(userId), null)
fun clear(
context: Context,
userId: Long,
) {
prefs(context).edit().remove(key(userId)).apply()
}
}
@@ -0,0 +1,48 @@
package io.flamenet.messenger.app
import androidx.test.core.app.ApplicationProvider
import io.flamenet.e2e.Vault
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNull
/**
* The note about a device a sign-out could not revoke outlives the vault.
*
* Sign-out destroys the vault a line after attempting the revoke, so a note
* kept there would not survive to be read. This one is kept in preferences,
* keyed by account, and read back by the account that can act on it.
*/
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34])
class UnretiredDeviceTest {
private val context get() = ApplicationProvider.getApplicationContext<android.content.Context>()
@Test
fun `a remembered device is read back for the same account and no other`() {
UnretiredDevice.clear(context, 7)
UnretiredDevice.clear(context, 8)
UnretiredDevice.remember(context, 7, "dead-device-1")
assertEquals("dead-device-1", UnretiredDevice.read(context, 7))
assertNull(UnretiredDevice.read(context, 8), "another account saw a device it cannot revoke")
UnretiredDevice.clear(context, 7)
assertNull(UnretiredDevice.read(context, 7))
}
@Test
fun `the note survives the vault being destroyed`() {
UnretiredDevice.clear(context, 7)
UnretiredDevice.remember(context, 7, "dead-device-1")
// What sign-out does next, minus the keystore entry Robolectric has no
// keystore for: `KeystoreVault.destroy` deletes the vault's files
// under `filesDir`. Wiping all of `filesDir` is that and more, so a
// note that survives this survives sign-out. Preferences live in a
// sibling directory, which is the whole reason they are used here.
context.filesDir.deleteRecursively()
assertEquals("dead-device-1", UnretiredDevice.read(context, 7), "the note died with the vault")
UnretiredDevice.clear(context, 7)
}
}