OverviewFilesCommitsIssuesDocsReleasesPeers

atproto-room commits / 01f9a42

state-base: projectMembers must MERGE __devices__, not wipe it
__devices__ maps writerKey -> memberKey and is written at ADMISSION time (_applySet, while a joiner still carries an `admit` proof). projectMembers rebuilt it from the roster's per-member `devices` arrays and OVERWROTE the view — but hpp's roster is [{key, role}] with no devices at all, so every projection replaced the map with {}. The production trigger is RESTART: _membersProjected is in-memory, so each boot resets the latch and device-node.js:156 calls projectMembers(roster) again. An already-admitted writer never re-sends an admit proof, so nothing repopulates the map — it stays empty forever. Confirmed live: BOTH hosted DIDs report devicesInView {}. Why that matters: ban()/REMOVE_DEVICE walks __devices__ to removeWriter a banned member's devices. With it empty a ban still records the identity in __removed__ (so re-admission is blocked) but removes NO existing writer — revocation was silently inert. Three changes, all non-destructive: - MERGE __devices__ instead of overwriting (additive; never drops an entry). - SELF-HEAL: record this node's own base.local.key -> selfKey, so the map repopulates organically instead of needing a migration (mirrors atproto-base base.js:313). Stopping the wipe alone would have left the live maps empty forever. - REFUSE an empty roster over a populated __members__ (admission verifies every proof against it, so wiping it denies all future admission — the wj62 fork signature), and skip appends when nothing changed (these run on a timer AND every swarm connection). ban.test.js could not catch this: it projects repeatedly BEFORE admission and never again, and never restarts. New test/devices-projection.test.js reproduces the real sequence — control run with the fix stashed fails 8 assertions, including "the admitted peer device SURVIVED the re-projection". Suite 40/40 tests, 170/170 asserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
01f9a4258e277d444a080fb092eefde09f445c44
by plankatron <fleeky@gmail.com> · 2026-07-25T00:24:42.000Z

Changed files (3) +139 -2

lib/state-base.js +40 -2
……@@ -83,6 +83,15 @@ function normKeyHex (k) {
8383 const s = String(k)
8484 return /^[0-9a-fA-F]{64}$/.test(s) ? s.toLowerCase() : s
8585}
86// Order-insensitive equality for the small hex->value maps projected into the view (__members__ /
87// __devices__), so a re-projection only APPENDS when the view would actually change. Every append
88// grows the log forever, and these run on a timer + on every swarm connection.
89function sameMap (a, b) {
90 const ka = Object.keys(a || {}).sort(); const kb = Object.keys(b || {}).sort()
91 if (ka.length !== kb.length) return false
92 for (let i = 0; i < ka.length; i++) { if (ka[i] !== kb[i] || String(a[ka[i]]) !== String(b[kb[i]])) return false }
93 return true
94}
8695function deriveGsKeyPair (roomSeed, ns = GS_NAMESPACE) { return crypto.keyPair(crypto.hash([ns, toBuf(roomSeed)])) }
8796function deriveBaseKey (gsPublicKey) { return Hypercore.key({ version: 1, signers: [{ publicKey: gsPublicKey }] }) }
8897
……@@ -975,8 +984,37 @@ export class StateBaseV3 {
975984 } else if (members && typeof members === 'object') {
976985 for (const id of Object.keys(members)) { const h = normKeyHex(id); if (h) map[h] = Number(members[id]) || 3 }
977986 }
978 await this.base.append(encode('@state/set-entry', { key: '__members__', value: JSON.stringify(map), ts: Date.now() }))
979 await this.base.append(encode('@state/set-entry', { key: '__devices__', value: JSON.stringify(devmap), ts: Date.now() }))
987 // SELF-HEAL: record THIS node's own writer->identity mapping. __devices__ is otherwise written
988 // only at ADMISSION time (_applySet, while a writer still carries an `admit` proof), so once an
989 // entry is lost it is never rewritten — a device that is already admitted never re-announces. Each
990 // node knows its own pair authoritatively, so projecting it here repopulates the map organically.
991 // Mirrors atproto-base's projectMembers (base.js:313). ctx.node.from.key in _applySet IS this
992 // base.local.key, so the two writers agree on the key space.
993 if (this._selfKey && this.base.local && this.base.local.key) {
994 devmap[this.base.local.key.toString('hex')] = this._selfKey.toString('hex')
995 }
996
997 // MERGE __devices__, never overwrite. hpp's roster is [{key, role}] with NO `devices` array
998 // (device-node.js), so devmap was always {} — and an unconditional overwrite stamped with a fresh
999 // ts therefore WIPED every entry admission had accumulated. ban()/REMOVE_DEVICE iterates this map
1000 // to removeWriter a banned member's devices, so an empty map made revocation silently remove
1001 // NOTHING (observed on both live DIDs: devicesInView {}). Additive only — never drops an entry.
1002 const prevDev = (await this.getNow('__devices__').catch(() => null)) || {}
1003 const mergedDev = { ...prevDev, ...devmap }
1004
1005 // Never write an EMPTY roster over a populated one. Admission verifies every joiner's proof against
1006 // __members__ (_verifyAdmissionInApply), so wiping it denies every future admission — that is the
1007 // wj62 fork signature. A caller with no roster must be a no-op, not a wipe.
1008 const prevMembers = (await this.getNow('__members__').catch(() => null)) || {}
1009 if (Object.keys(map).length === 0 && Object.keys(prevMembers).length > 0) {
1010 console.log('[state-base-v3] room=%s projectMembers REFUSED: empty roster would wipe %d existing member(s)',
1011 this._roomKeyHex.slice(0, 12), Object.keys(prevMembers).length)
1012 return false
1013 }
1014
1015 // Append only what actually changes — these run on the promote timer AND every swarm connection.
1016 if (!sameMap(prevMembers, map)) await this.base.append(encode('@state/set-entry', { key: '__members__', value: JSON.stringify(map), ts: Date.now() }))
1017 if (!sameMap(prevDev, mergedDev)) await this.base.append(encode('@state/set-entry', { key: '__devices__', value: JSON.stringify(mergedDev), ts: Date.now() }))
9801018 await this.base.update()
9811019 this._membersProjected = true // __members__ appended + drained into our view → stop re-projecting (early-returns cheaply)
9821020 // P3 (#103): a re-projection is the moment a role change becomes visible. If THIS
test/all.js +1 -0
……@@ -11,6 +11,7 @@ import './signer-conformance.test.js'
1111import './membership.test.js'
1212import './doorbell-durable-write.test.js'
1313import './admission-members-projection.test.mjs'
14import './devices-projection.test.js'
1415import './phase2-nonblocking.test.js'
1516import './multi-did.test.js'
1617import './non-indexer-default.test.js'
test/devices-projection.test.js +98 -0
……@@ -0,0 +1,98 @@
1// __devices__ must SURVIVE a re-projection (regression).
2//
3// __devices__ maps writerKey -> memberKey and is written at ADMISSION time (_applySet, while a joiner
4// still carries an `admit` proof). projectMembers used to OVERWRITE it from the roster's per-member
5// `devices` arrays — and hpp's roster is [{key, role}] with no devices at all (device-node.js), so the
6// map was replaced by {} on every projection.
7//
8// The production trigger is RESTART: _membersProjected is in-memory, so each boot resets the latch and
9// device-node calls projectMembers(roster) again. An already-admitted writer never re-sends an admit
10// proof, so nothing repopulates the map — it stays empty forever. Observed live on both hosted DIDs
11// (devicesInView {}), which matters because ban()/REMOVE_DEVICE walks this map to removeWriter a
12// banned member's devices: with it empty, revocation silently removes NOTHING.
13//
14// ban.test.js cannot catch this: it projects repeatedly BEFORE admission and never again.
15//
16// Run: bare test/devices-projection.test.js
17import '../lib/bare-globals.js'
18import test from 'brittle'
19import crypto from 'hypercore-crypto'
20import fs from 'bare-fs'
21import b4a from 'b4a'
22import { StateBaseV3 } from '../lib/state-base.js'
23
24const tmp = () => { const d = '/tmp/atproom-devproj-' + Math.floor(Math.random() * 1e9); fs.mkdirSync(d, { recursive: true }); return d }
25const hex = (b) => b4a.toString(b, 'hex')
26
27async function mk (selfKp, members) {
28 const sb = new StateBaseV3(tmp(), crypto.randomBytes(32), {
29 roomSeed: crypto.randomBytes(32),
30 encryptionKey: crypto.randomBytes(32),
31 network: false,
32 selfKey: selfKp.publicKey,
33 selfKeyPair: selfKp,
34 getMembers: async () => members
35 })
36 await sb.ready()
37 return sb
38}
39
40test('projectMembers MERGES __devices__ and never wipes an admitted device', async (t) => {
41 const self = crypto.keyPair()
42 const members = [{ key: hex(self.publicKey), role: 1 }]
43 const sb = await mk(self, members)
44 t.teardown(async () => { try { await sb.close() } catch {} })
45 t.ok(sb.base.writable, 'creator is writable')
46
47 t.ok(await sb.projectMembers(members), 'first projection lands')
48 const dev1 = await sb.get('__devices__')
49 t.ok(dev1 && Object.keys(dev1).length === 1, 'projection recorded THIS node\'s own writer->identity mapping (self-heal)')
50 t.is(dev1[hex(sb.base.local.key)], hex(self.publicKey), 'the recorded pair is base.local.key -> selfKey')
51
52 // Simulate an ADMISSION by another device: _applySet records writerKey -> memberKey for any
53 // admit-carrying op. Seed that entry directly, which is what the apply path leaves behind.
54 const peerWriter = hex(crypto.randomBytes(32))
55 const peerIdentity = hex(crypto.randomBytes(32))
56 await sb.set('__devices__', { ...dev1, [peerWriter]: peerIdentity })
57 t.is((await sb.get('__devices__'))[peerWriter], peerIdentity, 'peer device recorded (as admission would)')
58
59 // RESTART / reproject: the latch is in-memory, so a reboot re-runs projectMembers with the SAME
60 // deviceless roster. Before the fix this replaced the map with {} and the peer entry was lost.
61 sb._membersProjected = false
62 t.ok(await sb.projectMembers(members), 're-projection lands')
63
64 const dev2 = await sb.get('__devices__')
65 t.is(dev2[peerWriter], peerIdentity, 'the admitted peer device SURVIVED the re-projection (was wiped before the fix)')
66 t.is(dev2[hex(sb.base.local.key)], hex(self.publicKey), 'our own mapping is still present too')
67 t.is(Object.keys(dev2).length, 2, 'merge is additive — both entries retained, none dropped')
68})
69
70test('projectMembers REFUSES an empty roster that would wipe __members__', async (t) => {
71 const self = crypto.keyPair()
72 const members = [{ key: hex(self.publicKey), role: 1 }]
73 const sb = await mk(self, members)
74 t.teardown(async () => { try { await sb.close() } catch {} })
75
76 await sb.projectMembers(members)
77 const before = await sb.get('__members__')
78 t.is(Object.keys(before).length, 1, '__members__ populated')
79
80 // Admission verifies every joiner's proof against __members__, so wiping it denies all future
81 // admission — the wj62 fork signature. An empty roster must be a no-op, not a wipe.
82 sb._membersProjected = false
83 t.absent(await sb.projectMembers([]), 'empty roster projection is REFUSED (returns false)')
84 t.alike(await sb.get('__members__'), before, '__members__ is untouched by the refused projection')
85})
86
87test('an unchanged re-projection appends NOTHING (the log does not grow on a timer tick)', async (t) => {
88 const self = crypto.keyPair()
89 const members = [{ key: hex(self.publicKey), role: 1 }]
90 const sb = await mk(self, members)
91 t.teardown(async () => { try { await sb.close() } catch {} })
92
93 await sb.projectMembers(members)
94 const len = sb.base.local.length
95 sb._membersProjected = false
96 await sb.projectMembers(members) // identical roster + identical devices → nothing to write
97 t.is(sb.base.local.length, len, 'no append for an identical re-projection (runs on a timer + every connection)')
98})