fix(groups): keep the encrypted latch off the wire, as documented
src/lib/groupRecord.js describes the latch as "Non-secret, and deliberately LOCAL-only, never appended to a view", with a good argument: the view of an encrypted group is itself encrypted, so the one device that needs to read the flag is the one device that cannot. Only the code did not do it (TODO #147). appendGroupWithAvatarSplit destructured out `encryptionKey` and let `encrypted` ride along in `...rest` on every append, and resolveGroupEncryptedFlag ORs `incomingEncrypted` back in on the way out through mirrorToLocal. No live consequence was found, and the item said so when filed: a keyless device cannot decrypt a keyed group's view to receive the flag, and a legitimately unencrypted group has no keyed peer to originate one. So classifyKeylessGroup's `certain` verdict was sound - but sound by accident rather than by the stated invariant, and that verdict is load-bearing. It is what tells a device it is the broken one rather than a legacy unencrypted group, which is the whole reason the latch exists (#124, #142). Stripping it turned out to need more than one destructure. Both view-to-local merge sites compare before writing, and a merged record without the latch never equals a local record with it - so every mirror and every resync would have rewritten the group and emitted a groupChanged that changed nothing. Both now carry the latch across from local state, exactly as they already do for the key. putGroupRecord would preserve it regardless; the point is that the comparison runs first. 336 unit tests, 2 new: the append strips it, and both merge sites carry it. Verified against the real worklet, twice: a group created and resynced still reports the latch, and a fresh process opened against the same data dir finds it still set with the group opening on its derived encrypted topic - so the latch now survives on local state alone, which is what "local-only" has to mean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HG8ayyquJuDMPSVLKDQKVh
This commit is contained in:
co-authored by
Claude Opus 5
parent
76b2eece92
commit
25886a8af2
+24
-1
@@ -565,7 +565,19 @@ async function appendGroupWithAvatarSplit (base, groupValue) {
|
||||
// the (encrypted) view. Members receive it via the invite; the local group
|
||||
// record + personal-base fan-out carry it across restarts, and mirrorToLocal
|
||||
// preserves it when this view record replays back into the local mirror.
|
||||
const { encryptionKey: _ek, ...rest } = groupValue
|
||||
//
|
||||
// TODO #147 - `encrypted` is stripped for the same reason and was not. The
|
||||
// latch is documented in src/lib/groupRecord.js as "Non-secret, and
|
||||
// deliberately LOCAL-only, never appended to a view", with a good argument:
|
||||
// the view of an encrypted group is itself encrypted, so the one device that
|
||||
// needs to read the flag is the one device that cannot. Only the code did not
|
||||
// do it - `encrypted` rode along in `...rest` on every append. No live
|
||||
// consequence was found (a keyless device cannot decrypt a keyed group's view
|
||||
// to receive it, and a legitimately unencrypted group has no keyed peer to
|
||||
// originate one), so classifyKeylessGroup's `certain` verdict was sound - but
|
||||
// sound by accident rather than by the stated invariant, and that verdict is
|
||||
// load-bearing: it is what tells a device it is the broken one.
|
||||
const { encryptionKey: _ek, encrypted: _enc, ...rest } = groupValue
|
||||
const value = { ...rest, members, updatedAt: groupValue.updatedAt || Date.now() }
|
||||
await safeAppend(base, { op: 'put', type: 'group', key: NS.groups + groupValue.id, value })
|
||||
}
|
||||
@@ -5486,6 +5498,11 @@ async function resyncGroup (groupId) {
|
||||
// repair (App.jsx:1346), so it destroyed the key moments after the
|
||||
// repair had restored it, which is why the banner kept coming back.
|
||||
encryptionKey: ev?.encryptionKey || value.encryptionKey,
|
||||
// Carried for the same reason as the key, and specifically so the
|
||||
// equality check below still works now that the view record no longer
|
||||
// holds the latch (TODO #147). Without it every resync would rewrite
|
||||
// the group and emit a change that changed nothing.
|
||||
encrypted: ev?.encrypted ?? value.encrypted,
|
||||
removedMembers: [...removedMap.values()],
|
||||
members: splitMembers,
|
||||
}
|
||||
@@ -6712,6 +6729,12 @@ async function mirrorToLocal (type, key, value, groupId) {
|
||||
// base won't reopen encrypted after a restart. Fallback to value for
|
||||
// safety (e.g. a future path that legitimately carries it).
|
||||
encryptionKey: existing?.value?.encryptionKey || value.encryptionKey,
|
||||
// Same treatment for the same reason (TODO #147). putGroupRecord would
|
||||
// preserve the latch anyway, but the comparison below runs BEFORE that:
|
||||
// without carrying it here, a view record (which no longer has the flag)
|
||||
// never equals the local one (which does), so every mirror would rewrite
|
||||
// the record and fire a groupChanged event that changed nothing.
|
||||
encrypted: existing?.value?.encrypted ?? value.encrypted,
|
||||
removedMembers: [...removedMap.values()],
|
||||
members: splitMembers,
|
||||
updatedAt: value.updatedAt || Date.now()
|
||||
|
||||
@@ -156,3 +156,26 @@ test('the view append still strips the key it is stripping for', () => {
|
||||
assert.ok(fn, 'appendGroupWithAvatarSplit not found')
|
||||
assert.match(fn.body, /encryptionKey:\s*_ek/, 'the key must be destructured out before the append')
|
||||
})
|
||||
|
||||
test('the view append also strips the encrypted latch (TODO #147)', () => {
|
||||
// groupRecord.js documents the latch as "deliberately LOCAL-only, never
|
||||
// appended to a view", and for a while only the comment said so. The verdict
|
||||
// that rests on it - classifyKeylessGroup returning 'certain' - is what tells
|
||||
// a device it is the broken one, so it should hold by design rather than by
|
||||
// the accident that a keyless device cannot decrypt the view to receive it.
|
||||
const fn = FUNCTIONS.find(f => f.name === 'appendGroupWithAvatarSplit')
|
||||
assert.match(fn.body, /encrypted:\s*_enc/, 'the latch must be destructured out before the append')
|
||||
})
|
||||
|
||||
test('every view-to-local merge carries the latch across, not just the key', () => {
|
||||
// Stripping the latch from the view means a merged record no longer has it
|
||||
// while the local one does. Both merge sites compare before writing, so
|
||||
// without carrying it they would never compare equal: every mirror and every
|
||||
// resync would rewrite the record and emit a change that changed nothing.
|
||||
for (const name of ['mirrorToLocal', 'resyncGroup']) {
|
||||
const fn = FUNCTIONS.find(f => f.name === name)
|
||||
assert.ok(fn, name + ' not found')
|
||||
assert.match(fn.body, /encrypted:\s*(existing\?\.value\?\.encrypted|ev\?\.encrypted)/,
|
||||
name + ' must carry the local latch into its merged record')
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user