fix(crypto): stop republishing the public key on every login - #256
Merged
Conversation
needsKeySync() never found the stored key, so autoSyncOnLogin() re-uploaded the
browser's local public key on every single login.
Two bugs stacked:
- /api/crypto/public-keys/all returns a bare array of {user_id, public_key}, but
the check read `data.public_keys[currentUserId]` off it. `data.public_keys` is
undefined on an array, so the lookup never resolved.
- Even with the right shape it would still have missed: user_public_keys.user_id
holds the auth user id, while getCurrentUserId() returns the internal users.id.
Same identity domain drift as IA-040, which is what made five RLS policies dead.
The published key is what everyone else encrypts to. Re-uploading it from a
browser whose keypair had been regenerated silently replaced the good key, and
every message sent afterwards was encrypted to a key the recipient could no
longer decrypt -- surfacing as "ChaCha20-Poly1305 decryption failed: invalid tag"
on load, with the odd message decrypting fine because it predated the swap.
Now the service asks for its own key by internal id and lets the server resolve
the identity domain, so there is one id space and no list to mis-index. Both
failure paths also fail closed: publishing is the direction with consequences, so
an unreadable or errored check leaves the stored key alone instead of assuming
the database has nothing.
This stops the ongoing damage. It does not recover history already encrypted to a
lost keypair -- that needs the key-backup restore path, which is written but has
no callers.
505 tests pass; build clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThreatCrush Security Scan91 finding(s) HIGH/CRITICAL: 5 | MEDIUM: 10 | LOW: 76
…and 41 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
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
1 participant
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.
Root cause of the
ChaCha20-Poly1305 decryption failed: invalid tagerrors.What was happening
needsKeySync()never found the stored key, soautoSyncOnLogin()re-uploaded the browser's local public key on every single login. Two bugs stacked:/api/crypto/public-keys/allreturns a bare array of{user_id, public_key}, but the check readdata.public_keys[currentUserId]off it.data.public_keysisundefinedon an array, so the lookup never resolved.user_public_keys.user_idholds the auth user id, whilegetCurrentUserId()returns the internalusers.id. Same identity-domain drift as IA-040 — the thing that made five RLS policies silently dead.The published key is what everyone else encrypts to. Re-uploading it from a browser whose keypair had been regenerated silently replaced the good key, and every message sent afterwards was encrypted to a key the recipient could no longer decrypt. That is exactly the reported symptom, including the detail that the odd message does decrypt — those predate the swap.
Confirmed against production: chovy's
user_public_keysrow was last written 2026-08-16 03:09:51, during the session in the bug report, by this auto-sync.The fix
Ask for our own key by internal id and let the server resolve the identity domain — one id space, no list to mis-index. Both failure paths now fail closed: publishing is the direction with consequences, so an unreadable or errored check leaves the stored key alone rather than assuming the database has nothing.
As a side benefit this stops every client pulling the entire membership list on login just to find one row.
What this does not do
It stops the ongoing damage; it does not recover history already encrypted to a lost keypair. That needs the key-backup restore path —
EncryptedKeyBackup.restoreFromBackup()andMasterKeyDerivation.deriveFromCredentials()are both written and both have zero callers, so backups are being written and can never be restored. chovy has a backup row from 2026-08-10. Worth a follow-up.Verification
pnpm buildclean.🤖 Generated with Claude Code