Security audit: fix 8 findings across crypto, vault, and widget

An adversarial audit (five independent reviewers over crypto/protocol,
the vault, the widget, relay flows, and doc honesty) found real bugs in
code this repo shipped. Fixed here, worst first:

CRITICAL - the widget worker ignored application_binding.origin on
vaultRevealNwc, so any caller naming a connection id could pull a
wallet credential the user granted to a DIFFERENT site. The worker now
derives its own origin (self.location) instead of trusting a payload
field -- which also closes binding forgery via vaultSaveNwc /
vaultSetBinding / vaultOfferCheck, where a host could bind another
site's wallet to itself and inherit its one-tap approval.

CRITICAL - VaultSession.destroy() left the vault root live (the root
regenerates every derived key) and, worse, left the session USABLE with
a zeroed prk: deriveRecordKey/deriveVaultSigningKey over 32 zero bytes
are deterministic and world-computable, so a stale reference could seal
a wallet secret under a key any observer can derive and report success.
Now wipes the root and fails loudly.

CRITICAL - changePassword published the tombstone that DESTROYS the old
capsule before checking that the new one reached quorum: a failed
rotation could leave neither password working, phrase-only recovery.
Reordered to fail closed with the old password intact, plus a
regression test that drives it with a partially-hostile relay set.

HIGH - enableConnectionVault never checked quorum on the recovery read,
so one stale relay could make it mint a SECOND vault root and orphan
every stored credential -- the outcome its own header calls
unforgivable.

HIGH - the rollback high-water mark used strict `<` and never compared
the event id it stored, so a same-second sibling of a revocation could
resurrect a revoked credential forever. Ties now break on event id.

HIGH - §11.10 claimed derived secrets are wiped; wipe() had two callers
and neither was on the password or recovery path. The Argon2id output,
both PRKs, and the 64-byte BIP-39 seed now die at their derivation
site.

MEDIUM - one malformed record threw out of the listing loop and made
every OTHER connection unlistable; recovery lacked the
already-registered-at-this-locator guard that registration and rotation
both carry; the recovery chain check silently passed on gaps and forks.
Synthetic (untrusted) clicks and submits no longer drive consent UI.

Docs corrected where they overstated: the README's "private keys never
leave the worker" now says plainly that a same-origin host can reach
exportIdentity, and connection-vault.md gains §19 recording six OPEN
limitations -- no CAS on capsule writes, per-device-only rollback
state, tombstones not destroying prior ciphertext, the sudo key
shipping with the root, fetches not failing closed, and a plaintext
mark store.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMr9Yt8SYmZcfgrg3CV2G1

README.md

@@ -58,9 +58,23 @@ the site root.
 
 Once a user signs in, `window.nostr` is installed with the de facto NIP-07
 shape (`getPublicKey`, `signEvent`, `getRelays`, `nip44.encrypt/decrypt`), so
-existing Nostr web apps work against it unmodified. Private keys never leave
-the worker; the main thread only ever sees public keys, signed events, and
-ciphertext. See `packages/demo/public/docs.html` for the full integration
+existing Nostr web apps work against it unmodified. Private keys are held in
+the worker and are not handed to the main thread by the signing APIs — those
+return public keys, signed events, and ciphertext only.
+
+**Be precise about what that does and does not buy you.** The worker protects
+keys from *accidental* exposure through the normal API surface; it is not a
+boundary against the host page itself. A page that embeds `<bitlogin-auth>`
+runs same-origin with it and can reach the element's worker client directly,
+so a malicious or XSS-compromised host can call `exportIdentity` and obtain
+the `nsec`, and can synthesize UI events (the widget rejects untrusted ones,
+and can still be visually overlaid by host CSS). This is inherent to an
+embedded same-origin component — the same reason `docs/connection-vault.md`
+§12.3 says wallet credentials are *revealed*, not brokered. Treat the host
+page as trusted; embed BitLogin only in sites you control, and prefer the
+recovery phrase (never the exported nsec) as the durable backup. A
+cross-origin or extension build is what would change this, and is not
+shipped. See `packages/demo/public/docs.html` for the full integration
 guide, and `packages/demo/public/account.html` for a working example
 (create → confirm recovery phrase → sign an event → rotate password →
 export identity).

docs/connection-vault.md

@@ -618,3 +618,41 @@ Rollback resistance (§9) is per record: a local high-water mark stores the
 newest accepted `created_at` per connection id, and `fetchConnectionRecordEvents`
 withholds and reports anything older instead of serving it — a stale replica
 must never silently resurrect a revoked credential.
+
+
+## 19. Known limitations (audit, 2026-07-31)
+
+An adversarial audit of the implementation produced fixes for the worst
+findings (origin binding now enforced worker-side; `destroy()` wipes the root
+and poisons the session; equal-timestamp rollback tie-break; per-record
+listing isolation; quorum enforced before the migration's mint decision).
+These remain OPEN and are the honest limits of the current build:
+
+1. **No compare-and-swap on capsule writes.** Two devices enabling the vault
+   concurrently can each mint a root and write it, leaving the credential and
+   recovery capsules disagreeing. Mitigation today: enable the vault once,
+   from one device. A `previous_recovery_event_id` precondition check on
+   write is the fix.
+2. **Rollback protection is per-device local state.** A fresh install has no
+   high-water marks, so the oldest record a hostile relay serves is accepted
+   and becomes the mark. A record-level generation counter inside the
+   encrypted payload would close this; the local mark cannot.
+3. **A tombstone does not destroy the prior ciphertext.** Record keys are
+   derived from the immutable `connection_id`, so the superseded event — which
+   still contains the credential — remains decryptable by anyone who later
+   obtains the vault root, and relays may retain it regardless of NIP-09.
+   Deleting a connection in BitLogin is therefore NOT revocation: revoke at
+   the wallet or provider, which §11 already says and which this makes
+   cryptographically concrete.
+4. **The sudo key ships with the vault root at every login**, because both
+   live in the same capsule under the same password-derived key. The
+   personal tier's separation is real against a stolen *record*, but the
+   "fresh Argon2id run to reveal" ceremony is a host-side convention, not a
+   cryptographic gate. Personal-tier UI should not ship until this is either
+   fixed (separate capsule) or the claim is weakened.
+5. **Fetches do not fail closed on quorum failure**, and `limit: 500`
+   truncates silently — a single relay can shape a new device's view of the
+   vault.
+6. **The high-water-mark store is plaintext and unauthenticated**, leaking the
+   derived vault pubkey and the connection inventory to same-origin storage
+   access, and is tamperable to disable rollback detection.

packages/core/src/account/account.e2e.test.ts

@@ -394,6 +394,35 @@ describe("BitLogin Phase 0 end-to-end scenarios (§32)", () => {
     ARGON2_TIMEOUT * 2
   );
 
+  it(
+    "a rotation that cannot publish the new capsule leaves the OLD password working (§18.1 ordering)",
+    async () => {
+      // The tombstone DESTROYS the old capsule. Publishing it before the new
+      // capsule is known durable can leave an account with neither password --
+      // recoverable only by phrase. Failing closed keeps the old one alive.
+      const loginName = "ordering";
+      const oldPassword = "old ordering password 41";
+      const newPassword = "new ordering password 42";
+      await registerAccount({ loginName, password: oldPassword, vaultRelayUrls });
+
+      // Every relay refuses the NEW locator's writes while still accepting
+      // reads and the tombstone -- the partially-hostile quorum case.
+      const newLocator = getPublicKeyHex(
+        (await derivePasswordKeys(newPassword, normalizeLoginName(loginName))).locatorPrivateKey
+      );
+      for (const relay of relays) relay.refusePublishFrom.add(newLocator);
+      await expect(
+        changePassword({ loginName, oldPassword, newPassword, vaultRelayUrls })
+      ).rejects.toThrow(/still works|quorum/i);
+      for (const relay of relays) relay.refusePublishFrom.clear();
+
+      // The old password must still sign in, and the new one must not exist.
+      const login = await loginWithPassword({ loginName, password: oldPassword, vaultRelayUrls });
+      expect(login.everydayPublicKey).toMatch(/^[0-9a-f]{64}$/);
+    },
+    45_000
+  );
+
   it(
     "rotating to a new password that collides with another account under the same login name is refused (§18.1)",
     async () => {

packages/core/src/account/changePassword.ts

@@ -140,6 +140,21 @@ export async function changePassword(params: ChangePasswordParams): Promise<Chan
   });
   newPool.closeAll();
 
+  // ORDERING IS A SAFETY PROPERTY, not a style choice. The tombstone below
+  // DESTROYS the old capsule; publishing it before confirming the new one is
+  // durable can leave an account with NEITHER password working -- the new
+  // capsule missing quorum while the tombstone lands, recoverable only by
+  // phrase. A partially hostile relay set can induce that deliberately
+  // (accept the tombstone, drop the new capsule). Failing here instead leaves
+  // the OLD password working and the rotation simply retryable, which is the
+  // strictly safer end state.
+  if (!newCredentialPublish.success) {
+    throw new RegistrationFailedError(
+      "The new credential capsule did not reach the required relay acknowledgement and readback quorum. " +
+        "Your existing password still works; nothing was changed. Please retry."
+    );
+  }
+
   // §18.1 steps 4-5: mandatory tombstone + NIP-09 deletion at the OLD locator address.
   const tombstoneEvent = buildCredentialTombstoneEvent({
     oldLocatorPrivateKey: oldKeys.locatorPrivateKey,
@@ -159,12 +174,6 @@ export async function changePassword(params: ChangePasswordParams): Promise<Chan
   ]);
   oldPool.closeAll();
 
-  if (!newCredentialPublish.success) {
-    throw new RegistrationFailedError(
-      "The new credential capsule did not reach the required relay acknowledgement and readback quorum. Please retry."
-    );
-  }
-
   await raiseHighWaterMark(store, oldPayload.operational_public_key, { generation: newGeneration });
 
   return {

packages/core/src/account/enableVault.ts

@@ -98,6 +98,12 @@ export async function enableConnectionVault(
   } finally {
     recoveryPool.closeAll();
   }
+  // QUORUM IS LOAD-BEARING HERE, not decoration. The mint decision below is
+  // "did I see a root?", so a single stale or hostile relay answering with a
+  // pre-vault generation makes this flow mint a SECOND root and permanently
+  // orphan every existing connectable record -- the spendable NWC strings.
+  // The credential read above already enforces quorum; this one must too.
+  if (!recoveryRead.quorumMet) throw new AccountNotFoundError("quorum-not-met");
   if (!recoveryRead.best) throw new AccountNotFoundError("no-valid-candidate");
   const recoveryPayload = recoveryRead.best.payload!;
   if (recoveryPayload.account_id !== credentialPayload.account_id) {

packages/core/src/account/normalize.ts

@@ -1,6 +1,7 @@
 /** Login name normalization (§8.2) and password-derived key hierarchy (§7.3, §11.3, §11.4). */
 import { sha256 } from "@noble/hashes/sha2";
 import { hkdfExtract, hkdfExpand, labelSalt } from "../crypto/hkdf.js";
+import { wipe } from "../crypto/memory.js";
 import { scalarExpand } from "../crypto/scalarExpand.js";
 import { deriveArgon2id, normalizePasswordToBytes } from "../crypto/argon2id.js";
 import { concatBytes, utf8ToBytes } from "../crypto/encoding.js";
@@ -82,6 +83,12 @@ export async function derivePasswordKeys(password: string, normalizedLoginName:
   const passwordPrk = hkdfExtract(labelSalt("bitlogin/password-root/v1"), passwordRoot);
   const locator = scalarExpand(passwordPrk, "bitlogin/password-locator-signing/v1");
   const capsuleKey = hkdfExpand(passwordPrk, "bitlogin/password-capsule-encryption/v1", 32);
+  // §11.10 -- the INTERMEDIATES die here, not at GC's convenience. passwordPrk
+  // alone re-derives both returned keys, and passwordRoot re-derives it, so a
+  // heap snapshot taken after this call must not contain either. (The returned
+  // keys are the caller's to wipe; the password string itself is immutable and
+  // unwipeable by construction, which is why it is converted to bytes once.)
+  wipe(passwordBytes, passwordRoot, passwordPrk);
   return { locatorPrivateKey: locator.scalar, capsuleKey };
 }
 
@@ -100,5 +107,8 @@ export function deriveRecoveryKeys(bip39Seed: Uint8Array): RecoveryDerivedKeys {
   const recoveryPrk = hkdfExtract(labelSalt("bitlogin/recovery-root/v1"), bip39Seed);
   const signing = scalarExpand(recoveryPrk, "bitlogin/recovery-signing/v1");
   const capsuleKey = hkdfExpand(recoveryPrk, "bitlogin/recovery-capsule-encryption/v1", 32);
+  // §11.10, as above: recoveryPrk re-derives both returned keys. The caller
+  // owns the bip39Seed it passed in (recover.ts wipes it after this returns).
+  wipe(recoveryPrk);
   return { recoveryPrivateKey: signing.scalar, capsuleKey };
 }

packages/core/src/account/recover.ts

@@ -1,5 +1,6 @@
 /** Phrase recovery flow (§17), including the mandatory recovery-capsule refresh (§17.5). */
 import { getPublicKeyHex } from "../crypto/secp256k1.js";
+import { wipe } from "../crypto/memory.js";
 import { base64urlToBytes, bytesToBase64url } from "../crypto/encoding.js";
 import { isValidRecoveryPhrase, recoveryPhraseToSeed } from "../crypto/bip39.js";
 import { deriveRecoveryKeys, derivePasswordKeys, normalizeLoginName } from "./normalize.js";
@@ -14,7 +15,12 @@ import {
   SCHEMA_CREDENTIAL_V1,
   SCHEMA_RECOVERY_V1
 } from "../nostr/kinds.js";
-import { readRecoveryCapsule, checkRecoveryChainAcrossCandidates, type Candidate } from "./capsuleReader.js";
+import {
+  readCredentialCapsule,
+  readRecoveryCapsule,
+  checkRecoveryChainAcrossCandidates,
+  type Candidate
+} from "./capsuleReader.js";
 import { buildRecoveryCapsuleEvent, decryptRecoveryCapsuleEvent } from "../capsules/recoveryCapsule.js";
 import { buildCredentialCapsuleEvent } from "../capsules/credentialCapsule.js";
 import { PROTOCOL_CAPSULE_ENCRYPTION, PROTOCOL_PASSWORD_KDF, PROTOCOL_RECOVERY_DERIVATION } from "../capsules/types.js";
@@ -66,6 +72,10 @@ export async function recoverWithPhrase(params: RecoverWithPhraseParams): Promis
   }
   const bip39Seed = await recoveryPhraseToSeed(params.phrase);
   const { recoveryPrivateKey, capsuleKey } = deriveRecoveryKeys(bip39Seed);
+  // §11.10 -- the 64-byte master seed re-derives every recovery key; it must
+  // not outlive this line. The phrase STRING itself is unwipeable (JS strings
+  // are immutable), which is exactly why the caller is told to hold it briefly.
+  wipe(bip39Seed);
   const recoveryPublicKey = getPublicKeyHex(recoveryPrivateKey);
 
   const vaultPool = new RelayPool(params.vaultRelayUrls, { authPrivateKey: recoveryPrivateKey });
@@ -215,6 +225,36 @@ export async function completeRecoveryWithNewCredentials(params: CompleteRecover
 
   const { locatorPrivateKey, capsuleKey } = await derivePasswordKeys(params.newPassword, normalizedLoginName);
   const locatorPublicKey = getPublicKeyHex(locatorPrivateKey);
+
+  // §15.6 -- the SAME guard registration and rotation already carry, and it
+  // was missing only here. The new locator address is fully determined by
+  // (login name, new password); if a DIFFERENT account already lives there,
+  // publishing would replace and destroy it. Recovery is the one flow where
+  // the user is picking fresh credentials under stress, so it is exactly
+  // where a collision is most likely to be shrugged past.
+  const collisionPool = new RelayPool(params.vaultRelayUrls, { authPrivateKey: locatorPrivateKey });
+  let existingAtLocator;
+  try {
+    existingAtLocator = await readCredentialCapsule(
+      collisionPool,
+      locatorPublicKey,
+      capsuleKey,
+      params.timeoutMs
+    );
+  } finally {
+    collisionPool.closeAll();
+  }
+  if (!existingAtLocator.quorumMet) {
+    throw new RecoveryFailedError(
+      "Couldn't verify the new login name and password aren't already registered. Please retry, or add more vault relays."
+    );
+  }
+  if (existingAtLocator.candidates.length > 0) {
+    throw new RecoveryFailedError(
+      "Another account is already registered with that login name and password. Pick a different one."
+    );
+  }
+
   const credentialPayload: CredentialPayload = {
     schema: SCHEMA_CREDENTIAL_V1,
     account_id: recovered.accountId,

packages/core/src/capsules/validation.ts

@@ -139,20 +139,49 @@ export function validateRecoveryPayload(payload: RecoveryPayload): void {
 export function checkRecoveryChainConsistency(
   generations: Array<{ eventId: string; recoveryGeneration: number; previousRecoveryEventId: string | null }>
 ): { consistent: boolean; warning?: string } {
-  const byGeneration = new Map(generations.map((g) => [g.recoveryGeneration, g]));
+  // §12.3 claims this detects gaps AND replays; the first version detected
+  // neither. A Map keyed by generation SILENTLY COLLAPSED two capsules
+  // claiming the same generation -- a fork, i.e. the exact replay signal --
+  // and a missing generation made the link check `if (prior && ...)` skip
+  // entirely, so {1, 3} with 2 absent reported "consistent".
+  const byGeneration = new Map<number, { eventId: string }>();
+  for (const entry of generations) {
+    const existing = byGeneration.get(entry.recoveryGeneration);
+    if (existing && existing.eventId !== entry.eventId) {
+      return {
+        consistent: false,
+        warning: `Two different recovery capsules both claim generation ${entry.recoveryGeneration}: possible fork, replay, or relay misbehavior.`
+      };
+    }
+    byGeneration.set(entry.recoveryGeneration, entry);
+  }
+
   const sorted = [...generations].sort((a, b) => a.recoveryGeneration - b.recoveryGeneration);
   for (let i = 1; i < sorted.length; i++) {
     const current = sorted[i]!;
-    const prior = byGeneration.get(current.recoveryGeneration - 1);
     if (current.previousRecoveryEventId === null) {
-      return { consistent: false, warning: `Generation ${current.recoveryGeneration} has a null previous-event link but is not the first generation.` };
-    }
-    if (prior && current.previousRecoveryEventId !== prior.eventId) {
       return {
         consistent: false,
-        warning: `Recovery generation chain is broken between generation ${prior.recoveryGeneration} and ${current.recoveryGeneration}: possible replay or relay misbehavior.`
+        warning: `Generation ${current.recoveryGeneration} has a null previous-event link but is not the first generation.`
+      };
+    }
+    const prior = byGeneration.get(current.recoveryGeneration - 1);
+    if (!prior) {
+      // A gap is not proof of misbehavior -- relays legitimately drop old
+      // generations -- but it means the chain CANNOT be verified across it,
+      // and reporting that as "consistent" was the lie worth fixing.
+      return {
+        consistent: false,
+        warning: `Recovery generation ${current.recoveryGeneration - 1} is missing, so the chain up to generation ${current.recoveryGeneration} cannot be verified.`
+      };
+    }
+    if (current.previousRecoveryEventId !== prior.eventId) {
+      return {
+        consistent: false,
+        warning: `Recovery generation chain is broken between generation ${current.recoveryGeneration - 1} and ${current.recoveryGeneration}: possible replay or relay misbehavior.`
       };
     }
   }
   return { consistent: true };
 }
+

packages/core/src/test-support/mockRelay.ts

@@ -29,6 +29,10 @@ export class MockRelay {
   private events = new Map<string, NostrEvent>();
   private plainEvents: NostrEvent[] = [];
   public requireAuthForKinds: Set<number> = new Set();
+  /** Pubkeys whose publishes are refused. Models the partially-hostile relay
+   *  that accepts some writes and drops others -- the case that makes write
+   *  ORDERING a safety property rather than a style choice. */
+  public refusePublishFrom: Set<string> = new Set();
 
   private constructor(wss: WebSocketServer, port: number) {
     this.wss = wss;
@@ -69,6 +73,10 @@ export class MockRelay {
           socket.send(JSON.stringify(["OK", event.id, false, "auth-required: publish requires NIP-42 AUTH"]));
           return;
         }
+        if (this.refusePublishFrom.has(event.pubkey)) {
+          socket.send(JSON.stringify(["OK", event.id, false, "blocked: refused by test relay"]));
+          return;
+        }
         this.store(event);
         socket.send(JSON.stringify(["OK", event.id, true, ""]));
         return;

packages/core/src/vault/session.ts

@@ -47,10 +47,16 @@ export interface CreateConnectionParams {
 export class VaultSession {
   private vaultPrk: Uint8Array;
   private personalPrk: Uint8Array | null = null;
+  private destroyed = false;
+  private readonly connectionVaultRoot: Uint8Array;
   readonly vaultPublicKey: string;
 
-  constructor(private readonly connectionVaultRoot: Uint8Array) {
-    this.vaultPrk = deriveVaultPrk(connectionVaultRoot);
+  constructor(connectionVaultRoot: Uint8Array) {
+    // A private COPY: the caller's buffer is theirs to wipe on their own
+    // schedule, and destroy() must be able to erase this session's copy
+    // without reaching into memory it does not own.
+    this.connectionVaultRoot = connectionVaultRoot.slice();
+    this.vaultPrk = deriveVaultPrk(this.connectionVaultRoot);
     this.vaultPublicKey = deriveVaultPublicKey(this.vaultPrk);
   }
 
@@ -71,13 +77,29 @@ export class VaultSession {
     this.personalPrk = null;
   }
 
-  /** Wipes everything this session holds. The session is unusable after. */
+  /**
+   * Wipes everything this session holds, permanently.
+   *
+   * The ROOT is wiped too, not just the derived prk: the root regenerates the
+   * prk and (with a sudo key) the personal prk, so leaving it live made
+   * "wipes everything" false. And the session is marked destroyed rather than
+   * merely zeroed, because a zeroed prk is not inert -- deriveRecordKey and
+   * deriveVaultSigningKey over 32 zero bytes are deterministic, WORLD-
+   * COMPUTABLE values. A stale reference calling createConnection() after
+   * teardown would otherwise seal a wallet secret under a key any observer
+   * can derive, sign it with a publicly known identity, and report success.
+   * Failing loudly is the only safe behaviour.
+   */
   destroy(): void {
     this.endSudo();
-    wipe(this.vaultPrk);
+    wipe(this.vaultPrk, this.connectionVaultRoot);
+    this.destroyed = true;
   }
 
   private prkForTier(tier: ConnectionTier): Uint8Array {
+    if (this.destroyed) {
+      throw new Error("This vault session was destroyed; create a new one from the account's root.");
+    }
     if (tier === "connectable") return this.vaultPrk;
     if (!this.personalPrk) {
       throw new Error("Personal-tier records require an open sudo window (enableSudo).");
@@ -155,6 +177,9 @@ export class VaultSession {
 
   /** Trial-decrypts one event; personal-tier records resolve only inside a sudo window. */
   decryptEvent(event: NostrEvent): Promise<DecryptedConnectionRecord | null> {
+    if (this.destroyed) {
+      return Promise.reject(new Error("This vault session was destroyed; create a new one from the account's root."));
+    }
     return decryptConnectionRecordEvent(event, this.vaultPrk, this.personalPrk ?? undefined);
   }
 
@@ -162,6 +187,9 @@ export class VaultSession {
    *  signed by the vault identity. Best-effort by contract — the encrypted
    *  tombstone is the durable part of a deletion, this is the courtesy ask. */
   buildDeletionRequest(eventIdToDelete: string, now?: number): NostrEvent {
+    if (this.destroyed) {
+      throw new Error("This vault session was destroyed; create a new one from the account's root.");
+    }
     return buildDeletionRequest({
       privateKey: deriveVaultSigningKey(this.vaultPrk),
       eventIdToDelete,
@@ -198,14 +226,35 @@ export class VaultSession {
     relayUrls: string[];
     store: KeyValueStore;
     timeoutMs?: number;
-  }): Promise<{ connections: DecryptedConnectionRecord[]; rollbackWarnings: string[]; quorumMet: boolean }> {
+  }): Promise<{
+    connections: DecryptedConnectionRecord[];
+    rollbackWarnings: string[];
+    /** Event ids that failed to decrypt or validate. Surfaced, never silent. */
+    unreadable: string[];
+    quorumMet: boolean;
+  }> {
     const fetched = await this.fetchEvents(params);
     const connections: DecryptedConnectionRecord[] = [];
+    const unreadable: string[] = [];
     for (const event of fetched.events.values()) {
-      const decrypted = await this.decryptEvent(event);
-      if (decrypted) connections.push(decrypted);
+      // Per-record isolation. decryptConnectionRecordEvent THROWS on a bad
+      // signature, wrong author, malformed d-tag, id mismatch, or tier
+      // mismatch -- all correct strictness -- but without this catch a single
+      // malformed or future-version record made every OTHER connection,
+      // including working wallet credentials, unlistable on every device.
+      try {
+        const decrypted = await this.decryptEvent(event);
+        if (decrypted) connections.push(decrypted);
+      } catch {
+        unreadable.push(event.id);
+      }
     }
     connections.sort((a, b) => b.record.updated_at - a.record.updated_at);
-    return { connections, rollbackWarnings: fetched.rollbackWarnings, quorumMet: fetched.quorumMet };
+    return {
+      connections,
+      rollbackWarnings: fetched.rollbackWarnings,
+      unreadable,
+      quorumMet: fetched.quorumMet
+    };
   }
 }

packages/core/src/vault/sync.ts

@@ -46,7 +46,13 @@ export async function raiseRecordHighWaterMark(
   observed: RecordHighWaterMark
 ): Promise<void> {
   const current = await getRecordHighWaterMark(store, vaultPubkey, connectionId);
-  if (current && current.createdAt >= observed.createdAt) return;
+  if (current && current.createdAt > observed.createdAt) return;
+  // Equal timestamps are NOT a no-op: two devices editing the same record in
+  // the same second both compute nextCreatedAt(T) = T+1, so a benign edit can
+  // land at the exact timestamp of a REVOCATION. Ties break on event id (the
+  // addressable-event convention) so the mark advances deterministically
+  // rather than depending on which write arrived first.
+  if (current && current.createdAt === observed.createdAt && current.eventId <= observed.eventId) return;
   await store.set(hwmKey(vaultPubkey, connectionId), JSON.stringify(observed));
 }
 
@@ -128,7 +134,15 @@ export async function fetchConnectionRecordEvents(params: {
   const rollbackWarnings: string[] = [];
   for (const [connectionId, event] of newestById) {
     const hwm = await getRecordHighWaterMark(params.store, vaultPubkey, connectionId);
-    if (hwm && event.created_at < hwm.createdAt) {
+    // Strict `<` alone let a same-second sibling of a revocation through
+    // forever (the relay may keep both events at one address). An equal
+    // timestamp is only acceptable from the SAME event, or a later-sorting
+    // id under the same tie-break the mark itself uses.
+    const stale =
+      hwm !== null &&
+      (event.created_at < hwm.createdAt ||
+        (event.created_at === hwm.createdAt && event.id < hwm.eventId));
+    if (stale) {
       rollbackWarnings.push(connectionId);
       continue;
     }

packages/widget/src/element.ts

@@ -278,7 +278,7 @@ export class BitLoginAuthElement extends HTMLElement {
     const vaultStatus = await this.worker.vaultStatus();
     if (!vaultStatus.enabled) return "unavailable";
     const origin = window.location.origin;
-    const check = await this.worker.vaultOfferCheck({ uri, origin });
+    const check = await this.worker.vaultOfferCheck({ uri });
     if (check.duplicate) return "already-saved";
 
     const appName = options.appName?.trim() || window.location.hostname || "This app";
@@ -311,7 +311,7 @@ export class BitLoginAuthElement extends HTMLElement {
     this.setBusy(true);
     try {
       const label = this.field("vaultOfferLabel").trim() || offer.label;
-      await this.worker.vaultSaveNwc({ uri: offer.uri, label, origin: window.location.origin });
+      await this.worker.vaultSaveNwc({ uri: offer.uri, label });
       this.setBusy(false);
       this.finishVaultOffer("saved");
       this.flashSuccess(this.screen, "Wallet saved");
@@ -353,7 +353,7 @@ export class BitLoginAuthElement extends HTMLElement {
       return;
     }
     this.vaultUnsaved = false;
-    const found = await this.worker.vaultFindForOrigin({ origin: request.origin });
+    const found = await this.worker.vaultFindForOrigin();
     if (found.connection) {
       this.vaultCandidate = found.connection;
       this.goto("vault-consent");
@@ -416,7 +416,7 @@ export class BitLoginAuthElement extends HTMLElement {
       return;
     }
     const label = labelDraft.trim() || `${request.appName} wallet`;
-    await this.worker.vaultSaveNwc({ uri, label, origin: request.origin });
+    await this.worker.vaultSaveNwc({ uri, label });
     this.setBusy(false);
     this.finishVaultRequest(uri);
     this.flashSuccess(this.screen, "Wallet connected");
@@ -623,6 +623,14 @@ export class BitLoginAuthElement extends HTMLElement {
   }
 
   private async onClick(e: Event): Promise<void> {
+    // A synthetic click is not consent. The shadow root is open (by design,
+    // for host theming), so a host page can find and .click() any button in
+    // here -- including "approve this wallet" and "reveal my nsec". isTrusted
+    // is false for anything script-dispatched. This does NOT make the widget
+    // a security boundary against its host (see connection-vault.md §12.3;
+    // the host can still style the real button under its own), but it closes
+    // the zero-effort version where no human ever sees a screen.
+    if (!e.isTrusted) return;
     const target = (e.target as HTMLElement).closest<HTMLElement>("[data-action]");
     if (!target) return;
     const action = target.dataset.action!;
@@ -752,6 +760,8 @@ export class BitLoginAuthElement extends HTMLElement {
 
   private async onSubmit(e: Event): Promise<void> {
     e.preventDefault();
+    if (!e.isTrusted) return; // see onClick: synthetic submits are not consent
+
     const form = e.target as HTMLFormElement;
     const formName = form.dataset.form;
     try {
@@ -1431,7 +1441,7 @@ export class BitLoginAuthElement extends HTMLElement {
         return `
           <h2>Signed in</h2>
           ${this.renderWarnings()}
-          <p class="pubkey">${this.session?.npub ?? ""}</p>
+          <p class="pubkey">${escapeHtml(this.session?.npub ?? "")}</p>
           ${this.renderError()}
           <button class="secondary" type="button" data-action="sign-test-event">Sign a test event</button>
           ${

packages/widget/src/worker/cryptoWorker.ts

@@ -149,6 +149,24 @@ async function persistSession(): Promise<void> {
   });
 }
 
+/**
+ * The origin this worker actually runs in, derived rather than accepted.
+ *
+ * Every vault call used to take `origin` from the message payload, which made
+ * the per-origin binding -- the vault's ENTIRE authorization model -- advisory:
+ * a caller could claim any origin, bind another site's wallet to itself, or
+ * plant a wallet under a victim origin. The worker is same-origin with its
+ * page and can simply look. A dedicated worker's self.location.origin is the
+ * origin of the script's URL, which is the embedding page's origin.
+ */
+function selfOrigin(): string {
+  try {
+    return new URL(self.location.href).origin;
+  } catch {
+    return "";
+  }
+}
+
 function requireVault(): VaultSession {
   if (!session.signer) throw new Error("No identity is unlocked in this session.");
   if (!session.connectionVaultRoot) {
@@ -482,7 +500,8 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
     }
 
     case "vaultSaveNwc": {
-      const p = payload as { uri: string; label: string; origin: string | null };
+      const p = payload as { uri: string; label: string };
+      const origin = selfOrigin();
       const vault = requireVault();
       const credential = parseNwcUri(p.uri);
       const label = p.label.trim().slice(0, 120) || "Wallet connection";
@@ -491,7 +510,7 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
         tier: "connectable",
         label,
         credential: credential as unknown as { schema: string } & Record<string, unknown>,
-        application_binding: { origin: p.origin, app_pubkey: null }
+        application_binding: { origin, app_pubkey: null }
       });
       const publish = await vault.publish({
         event,
@@ -509,13 +528,13 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
     }
 
     case "vaultFindForOrigin": {
-      const p = payload as { origin: string };
+      const origin = selfOrigin();
       const { connections } = await listLiveConnections();
       const match = connections.find(
         (c) =>
           c.record.connection_type === "nwc" &&
           c.record.state === "active" &&
-          c.record.application_binding.origin === p.origin
+          c.record.application_binding.origin === origin
       );
       return { connection: match ? summarize(match) : null };
     }
@@ -526,6 +545,16 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
       if (connection.record.connection_type !== "nwc" || connection.record.state !== "active") {
         throw new Error("That connection is not an active wallet connection.");
       }
+      // THE authorization check. Reveal mode concedes that the requesting
+      // origin receives the credential IT was granted (connection-vault.md
+      // §12.3) -- it concedes nothing about OTHER origins' grants. Without
+      // this, any caller that can name a connection id (they appear in the
+      // management UI's markup) could pull a wallet the user connected to a
+      // different site entirely.
+      const boundOrigin = connection.record.application_binding.origin;
+      if (boundOrigin !== null && boundOrigin !== selfOrigin()) {
+        throw new Error("That connection belongs to a different site.");
+      }
       const credential = connection.record.credential as unknown as NwcCredential;
       validateNwcCredential(credential);
       return { uri: toNwcUri(credential) };
@@ -535,7 +564,7 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
       const p = payload as { connectionId: string; origin: string | null };
       const { vault, connection } = await findConnection(p.connectionId);
       const { record, event } = await vault.updateConnection(connection, {
-        application_binding: { origin: p.origin, app_pubkey: null }
+        application_binding: { origin, app_pubkey: null }
       });
       const publish = await vault.publish({
         event,
@@ -577,7 +606,8 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
       // is warranted. The origin binding is refreshed silently — the offering
       // origin already HOLDS the credential (it supplied it), so binding is
       // bookkeeping for later one-tap approvals, not a new grant.
-      const p = payload as { uri: string; origin: string | null };
+      const p = payload as { uri: string };
+      const origin = selfOrigin();
       const credential = parseNwcUri(p.uri);
       const { vault, connections } = await listLiveConnections();
       const existing = connections.find(
@@ -586,9 +616,9 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
           sameNwcCredential(c.record.credential as unknown as NwcCredential, credential)
       );
       if (!existing) return { duplicate: false };
-      if (p.origin !== null && existing.record.application_binding.origin !== p.origin) {
+      if (existing.record.application_binding.origin !== origin) {
         const { record, event } = await vault.updateConnection(existing, {
-          application_binding: { origin: p.origin, app_pubkey: null }
+          application_binding: { origin, app_pubkey: null }
         });
         const publish = await vault.publish({
           event,

packages/widget/src/worker/protocol.ts

@@ -197,12 +197,10 @@ export interface VaultListResult {
 export interface VaultSaveNwcPayload {
   uri: string;
   label: string;
-  origin: string | null;
 }
 
-export interface VaultFindForOriginPayload {
-  origin: string;
-}
+/** No origin field: the worker derives its own (see selfOrigin). */
+export type VaultFindForOriginPayload = Record<string, never>;
 
 export interface VaultRevealNwcPayload {
   connectionId: string;
@@ -222,7 +220,6 @@ export interface VaultDeletePayload {
 
 export interface VaultOfferCheckPayload {
   uri: string;
-  origin: string | null;
 }
 /** duplicate=true means the same wallet+secret is already stored; the worker
  *  has silently refreshed its origin binding, and no consent UI is needed —

packages/widget/src/worker/workerClient.ts

@@ -130,8 +130,8 @@ export class WorkerClient {
   vaultSaveNwc(payload: VaultSaveNwcPayload) {
     return this.call("vaultSaveNwc", payload);
   }
-  vaultFindForOrigin(payload: VaultFindForOriginPayload) {
-    return this.call("vaultFindForOrigin", payload);
+  vaultFindForOrigin() {
+    return this.call("vaultFindForOrigin", {});
   }
   vaultRevealNwc(payload: VaultRevealNwcPayload) {
     return this.call("vaultRevealNwc", payload);