Repository navigation
A removed MLS device stays removed (GRYT-1555) - #249
Merged
Merged
Conversation
Removing a device used to delete its row, and nothing else stopped it. If it was still signed in, its next sync said registered: false, the driver published fresh KeyPackages under the same id, and whoever sent next added it back to the DM. So removing a lost phone that was still online did nothing for long. mls:device:remove now writes the id to mls_removed_devices as well. From then on every mls:* call from that device id gets device_removed: sync, publish, claim, commit, send and welcome:ack. touchMlsDevice refuses it too, so a publish racing the removal can't bring the row back. Nothing it published is left to claim, since its KeyPackages go with the removal and it can't publish more. A new device id registers as usual, so setting the app up again makes a new device. The rows are kept for good. Any window we'd pick is one a phone left in a drawer can wait out, and a row is a few dozen bytes. There's no other per-device limit to hit either. A guest merged into an account brings its removed ids along, like it brings its devices. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removing a device deleted its row and nothing else. If the device was still signed in, its next
mls:syncsaidregistered: false, the driver published fresh KeyPackages under the same id, and whoever sent next added it back to the DM. Removing a lost phone that was still online only lasted until it reconnected.What changes
mls:device:removealso writes the id to a new table,mls_removed_devices (server_user_id, device_id, removed_at), in the same transaction.ownDevice()in the handler checks that table first. So everymls:*call naming a removed id getsdevice_removed: sync, KeyPackage publish, claim, commit, send and welcome ack.touchMlsDevicerefuses a removed id inside its transaction too. A publish that races the removal can't write the row back.How long they're kept
For good. A phone left in a drawer can wait out any window we'd pick, and waiting it out is exactly the case this is for. A row is a few dozen bytes, and it only grows when somebody removes a device by hand, which is rate limited with the KeyPackage publish bucket. Users aren't deleted on this server except by a guest merge, which carries the rows over.
What to look at
src/db/sqlite/**, which is review-required. The diff there is about 30 lines.mls:device:removeon an id that's already removed now answersdevice_removedinstead ofok. The client's retire-after-clear path treatsunknown_deviceas done and needs to treatdevice_removedthe same way. That's in client#713.device_removed, wipes that server's MLS state and history, and waits for a sign-in or a recovery-key restore before it makes a new device.Tests
device_removed. Ivar's claim only gets Hana's laptop, andmls:devicesno longer lists the phone. A new device id publishes fine.yarn testpasses exceptvoiceRecovery.test.ts, which timed out once under the full run and passes alone.eslint,buildand the comment check are clean.Docs: Gryt-chat/docs#148.
🤖 Generated with Claude Code