mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
[nostr-core] Pin the NIP-46 multi-account lifecycle contract (TODO #33)
The 2026-06-25 audit blamed "logout logs out everyone" / broken profile
switching / unknown-user-after-refresh on the single shared nip46Client and
single-slot session storage. Re-auditing the current code shows every named
suspect has since been fixed piecemeal: per-account v2 session map with v1
last-connected default, keepStored logout, scoped no-arg clear (v1 slot only),
reuseStored+expectPubkey switch wired end-to-end into
useStoredRemoteSigner({pubkey}), and a validator-enforced boot restore.
None of that was pinned at the manager level, so any layer could regress
silently. tests/nip46-signer-lifecycle.test.mjs now exercises the real
SignerManager against the real signer registry and real (polyfilled) storage:
- logging out the active NIP-46 account destroys its live client exactly once
while BOTH accounts' stored sessions and the other account's registry signer
survive
- a failed targeted restore (passphrase-locked or missing session) fails typed
and deletes nothing
- disconnectRemoteSigner with and without keepStored never touches the
per-account map
Mutation-checked: reinjecting the historical bugs (logout without keepStored;
no-arg clear wiping the v2 map) fails 2/6; real code passes 6/6.
TODO #33 updated: remaining work is the on-phone Amber verification the item
already gates on — and if that still misbehaves, the failure is now known to
live ABOVE this contract (UI/state reconciliation on restore failure), which
narrows the search considerably.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01LHH3wPBsoruwCaJo8Rpf6a
This commit is contained in:
1 parent
2eb7d09865
commit
975b996f4c
2 files changed
+233
-3
No files matched your search
@@ -0,0 +1,212 @@
|
|||||||
|
// SignerManager lifecycle scoping for NIP-46 multi-account (pre-launch TODO #33).
|
||||||
|
//
|
||||||
|
// The 2026-06-25 audit blamed "logout logs out everyone" on the single shared
|
||||||
|
// nip46Client being torn down by any scoped logout. Since then the layers under
|
||||||
|
// it were fixed piecemeal (per-account v2 session map, scoped clears, targeted
|
||||||
|
// restore) — but nothing pinned the MANAGER-level contract, so any of those
|
||||||
|
// fixes could regress silently. These tests exercise the real SignerManager
|
||||||
|
// against the real signer registry and real (polyfilled) storage.
|
||||||
|
//
|
||||||
|
// test_integrity_note:
|
||||||
|
// change_type: ["new_tests"]
|
||||||
|
// scenarios:
|
||||||
|
// - id: SCN-nip46-logout-scope
|
||||||
|
// given: "account A active with a live remote-signer client; A and B both have stored sessions; B has a registered signer"
|
||||||
|
// when: "SignerManager.logout() logs out A"
|
||||||
|
// then: "A's live client is destroyed exactly once; BOTH stored sessions survive; B's registry signer survives"
|
||||||
|
// - id: SCN-nip46-restore-failure-scope
|
||||||
|
// given: "stored sessions for A and B"
|
||||||
|
// when: "a targeted restore of A fails (locked session / missing session)"
|
||||||
|
// then: "the failure is typed and NO stored session is deleted"
|
||||||
|
// observable_outcomes:
|
||||||
|
// - "fake client's destroy() call count"
|
||||||
|
// - "readStoredNip46Session(pubkey) after the operation"
|
||||||
|
// - "resolveActiveSigner(pubkey) for the OTHER account"
|
||||||
|
// determinism_controls:
|
||||||
|
// - "in-memory localStorage polyfill; fake in-process client object; no network"
|
||||||
|
// anti_cheat_rationale:
|
||||||
|
// prevents: ["over-mocking internal logic", "hard-coded return value"]
|
||||||
|
// relaxation:
|
||||||
|
// did_relax_any_assertion: false
|
||||||
|
|
||||||
|
import "./test-helpers/setup-localstorage.mjs";
|
||||||
|
import test, { beforeEach } from "node:test";
|
||||||
|
import { strict as assert } from "node:assert";
|
||||||
|
import { SignerManager } from "../js/nostr/managers/SignerManager.js";
|
||||||
|
import {
|
||||||
|
writeStoredNip46SessionSync,
|
||||||
|
readStoredNip46Session,
|
||||||
|
} from "../js/nostr/nip46Client.js";
|
||||||
|
import {
|
||||||
|
registerSigner,
|
||||||
|
setActiveSigner,
|
||||||
|
clearActiveSigner,
|
||||||
|
resolveActiveSigner,
|
||||||
|
} from "../js/nostrClientRegistry.js";
|
||||||
|
|
||||||
|
const USER_A =
|
||||||
|
"00000000000000000000000000000000000000000000000000000000000000a1";
|
||||||
|
const USER_B =
|
||||||
|
"00000000000000000000000000000000000000000000000000000000000000b2";
|
||||||
|
|
||||||
|
function storedSession(userPubkey, remotePubkey) {
|
||||||
|
return {
|
||||||
|
version: 1,
|
||||||
|
clientPublicKey:
|
||||||
|
"3333333333333333333333333333333333333333333333333333333333333333",
|
||||||
|
remotePubkey,
|
||||||
|
relays: ["wss://relay.example"],
|
||||||
|
encryption: "nip44.v2",
|
||||||
|
userPubkey,
|
||||||
|
lastConnectedAt: 1700000000000,
|
||||||
|
// Sessions at rest carry encrypted secrets; restoring without the
|
||||||
|
// passphrase must fail typed, not fall through to a broken connect.
|
||||||
|
encryptedSecrets: "cipher",
|
||||||
|
keyEncryption: {
|
||||||
|
salt: "c2FsdA==",
|
||||||
|
iv: "aXY=",
|
||||||
|
iterations: 1000,
|
||||||
|
hash: "SHA-256",
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeManager() {
|
||||||
|
// SignerManager only needs the app client for pool access on connect paths;
|
||||||
|
// the scoping contracts under test never reach the network.
|
||||||
|
return new SignerManager({
|
||||||
|
relays: ["wss://relay.example"],
|
||||||
|
ensurePool: async () => {
|
||||||
|
throw new Error("network must not be touched by these tests");
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
localStorage.clear();
|
||||||
|
clearActiveSigner();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("logging out the active NIP-46 account keeps every other account intact", () => {
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_A, "1".repeat(64)));
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_B, "2".repeat(64)));
|
||||||
|
|
||||||
|
// B is a signed-in account in the registry (e.g. logged in earlier this
|
||||||
|
// session); A is the ACTIVE account with the live remote-signer client.
|
||||||
|
const signerB = { pubkey: USER_B, signEvent: async (e) => e };
|
||||||
|
registerSigner(USER_B, signerB);
|
||||||
|
|
||||||
|
let destroyCalls = 0;
|
||||||
|
const liveClient = {
|
||||||
|
remotePubkey: "1".repeat(64),
|
||||||
|
userPubkey: USER_A,
|
||||||
|
destroy() {
|
||||||
|
destroyCalls += 1;
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const manager = makeManager();
|
||||||
|
manager.pubkey = USER_A;
|
||||||
|
manager.nip46Client = liveClient;
|
||||||
|
|
||||||
|
manager.logout();
|
||||||
|
|
||||||
|
// The departing account's LIVE connection is torn down exactly once…
|
||||||
|
assert.equal(destroyCalls, 1);
|
||||||
|
assert.equal(manager.nip46Client, null);
|
||||||
|
assert.equal(manager.pubkey, null);
|
||||||
|
|
||||||
|
// …but its stored session survives (logout keeps sessions reconnectable —
|
||||||
|
// that is what makes "switch back later" possible)…
|
||||||
|
const storedA = readStoredNip46Session(USER_A);
|
||||||
|
assert.ok(storedA, "A's stored session must survive A's own logout");
|
||||||
|
assert.equal(storedA.remotePubkey, "1".repeat(64));
|
||||||
|
|
||||||
|
// …and account B is completely untouched: stored session AND registry signer.
|
||||||
|
const storedB = readStoredNip46Session(USER_B);
|
||||||
|
assert.ok(storedB, "B's stored session must survive A's logout");
|
||||||
|
assert.equal(storedB.remotePubkey, "2".repeat(64));
|
||||||
|
assert.equal(
|
||||||
|
resolveActiveSigner(USER_B),
|
||||||
|
signerB,
|
||||||
|
"B's registered signer must survive A's logout"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("logout is safe when no remote client is live (nsec/nip07 active)", () => {
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_B, "2".repeat(64)));
|
||||||
|
|
||||||
|
const manager = makeManager();
|
||||||
|
manager.pubkey = USER_A; // active via some non-NIP-46 method
|
||||||
|
|
||||||
|
assert.doesNotThrow(() => manager.logout());
|
||||||
|
assert.ok(
|
||||||
|
readStoredNip46Session(USER_B),
|
||||||
|
"an unrelated account's NIP-46 session must survive a non-NIP-46 logout"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a failed targeted restore deletes nothing", async () => {
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_A, "1".repeat(64)));
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_B, "2".repeat(64)));
|
||||||
|
|
||||||
|
const manager = makeManager();
|
||||||
|
|
||||||
|
// A's session is passphrase-locked and none was provided: typed failure.
|
||||||
|
await assert.rejects(
|
||||||
|
manager.useStoredRemoteSigner({ pubkey: USER_A, silent: true }),
|
||||||
|
(error) => error?.code === "passphrase-required"
|
||||||
|
);
|
||||||
|
|
||||||
|
assert.ok(readStoredNip46Session(USER_A), "A's session survives its own failed restore");
|
||||||
|
assert.ok(readStoredNip46Session(USER_B), "B's session survives A's failed restore");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("restoring an account with no stored session fails typed and touches nothing", async () => {
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_B, "2".repeat(64)));
|
||||||
|
|
||||||
|
const manager = makeManager();
|
||||||
|
|
||||||
|
await assert.rejects(
|
||||||
|
manager.useStoredRemoteSigner({ pubkey: USER_A, silent: true }),
|
||||||
|
(error) => error?.code === "no-stored-session"
|
||||||
|
);
|
||||||
|
|
||||||
|
assert.ok(
|
||||||
|
readStoredNip46Session(USER_B),
|
||||||
|
"an unrelated stored session must survive a missing-session restore attempt"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("disconnectRemoteSigner({keepStored:true}) never clears stored sessions", async () => {
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_A, "1".repeat(64)));
|
||||||
|
|
||||||
|
const manager = makeManager();
|
||||||
|
manager.nip46Client = { destroy() {} };
|
||||||
|
|
||||||
|
await manager.disconnectRemoteSigner({ keepStored: true });
|
||||||
|
|
||||||
|
assert.ok(readStoredNip46Session(USER_A));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("disconnectRemoteSigner() without keepStored clears only the last-connected default", async () => {
|
||||||
|
// The no-arg clear historically wiped the whole v2 map — "after switching
|
||||||
|
// away you could no longer switch back". Pin the scoped behavior: the v1
|
||||||
|
// default slot goes, the per-account map survives.
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_A, "1".repeat(64)));
|
||||||
|
writeStoredNip46SessionSync(storedSession(USER_B, "2".repeat(64)));
|
||||||
|
|
||||||
|
const manager = makeManager();
|
||||||
|
manager.nip46Client = { destroy() {} };
|
||||||
|
|
||||||
|
await manager.disconnectRemoteSigner();
|
||||||
|
|
||||||
|
assert.ok(
|
||||||
|
readStoredNip46Session(USER_A),
|
||||||
|
"per-account session A must survive a bare disconnect"
|
||||||
|
);
|
||||||
|
assert.ok(
|
||||||
|
readStoredNip46Session(USER_B),
|
||||||
|
"per-account session B must survive a bare disconnect"
|
||||||
|
);
|
||||||
|
});
|
||||||
@@ -103,13 +103,31 @@ Likely a shared root cause in session/identity persistence + multi-account handl
|
|||||||
`SignerManager.logout` (previousPubkey/hadRemoteClient), and
|
`SignerManager.logout` (previousPubkey/hadRemoteClient), and
|
||||||
`authService.switchProfile` (from/to + whether requestLogin resolved the expected
|
`authService.switchProfile` (from/to + whether requestLogin resolved the expected
|
||||||
pubkey). **Remove these once the fix lands.**
|
pubkey). **Remove these once the fix lands.**
|
||||||
|
- [x] **CODE-LEVEL SUSPECTS RE-AUDITED + PINNED (2026-07-30).** Every root cause the
|
||||||
|
2026-06-25 audit named has since been fixed by later work, verified end-to-end
|
||||||
|
by reading the current code:
|
||||||
|
- Per-account NIP-46 session storage exists (`bitvid:nip46:sessions:v2` map,
|
||||||
|
v1 slot = last-connected default, migration on read).
|
||||||
|
- `logout()` → `disconnectRemoteSigner({ keepStored: true })` — stored
|
||||||
|
sessions survive; the no-arg `clearStoredNip46Session()` clears ONLY the
|
||||||
|
v1 default slot, never the map (the wipe-all was fixed earlier).
|
||||||
|
- Switch is fully wired: coordinator sets `reuseStored: true` for nip46
|
||||||
|
targets → `switchProfile` passes `expectPubkey` → provider forwards it →
|
||||||
|
`useStoredRemoteSigner({ pubkey })` restores the TARGET account's session,
|
||||||
|
and forgets only the attempted account on access-denial.
|
||||||
|
- Boot restore (`scheduleStoredRemoteSignerRestore`) enforces the
|
||||||
|
access-control validator and fails scoped (nothing else cleared).
|
||||||
|
NEW: `tests/nip46-signer-lifecycle.test.mjs` pins the manager-level contract
|
||||||
|
(logout scoping, restore-failure scoping, bare-disconnect scoping) — mutation-
|
||||||
|
checked: reinjecting the historical keepStored/wipe-all bugs fails 2/6.
|
||||||
- [ ] **VERIFY / REPRODUCE (user action):** on the phone with Amber on
|
- [ ] **VERIFY / REPRODUCE (user action):** on the phone with Amber on
|
||||||
unstable.bitvid.network, exercise (1) fresh load, (2) switch profiles, (3) logout,
|
unstable.bitvid.network, exercise (1) fresh load, (2) switch profiles, (3) logout,
|
||||||
and capture the `[nip46-diag]` console lines for each — restore succeeded/FAILED,
|
and capture the `[nip46-diag]` console lines for each — restore succeeded/FAILED,
|
||||||
switchProfile `matchesTarget`, and disconnect `keepStored`/`hadClient`/caller stack.
|
switchProfile `matchesTarget`, and disconnect `keepStored`/`hadClient`/caller stack.
|
||||||
- [ ] **THEN fix** based on the captured trace (likely: per-profile NIP-46 session keyed
|
If all three behave, close this item and REMOVE the `[nip46-diag]` diagnostics;
|
||||||
by pubkey; don't tear the shared client on a scoped logout; reconnect on
|
if not, the trace now points at whatever the unit contract does NOT cover
|
||||||
switch/restore) and REMOVE the temporary `[nip46-diag]` diagnostics.
|
(likely UI/state reconciliation, e.g. restore-failure leaving "unknown user"
|
||||||
|
rendered instead of a reconnect prompt).
|
||||||
|
|
||||||
### 1. Delete is not fully working — tombstoned videos still show in the UI
|
### 1. Delete is not fully working — tombstoned videos still show in the UI
|
||||||
- [x] **Root cause found + fixed** (`afb6200b`): deletes published only to the CAPPED
|
- [x] **Root cause found + fixed** (`afb6200b`): deletes published only to the CAPPED
|
||||||
|
|||||||
Reference in new issue
Block a user