Fix revoke-rebinds bug; correct the website's custody and revoke claims

The audit's most important finding was not a doc issue at all. In vaultSetBinding the identifier `origin` resolved to the WORKER GLOBAL self.origin rather than the intended value, so the dashboard's "Revoke app access" REBOUND the record to the calling page instead of clearing it -- the precise cross-origin adoption the binding guard exists to prevent, wearing the label of the button meant to prevent it. It typechecked because the DOM lib declares that global. I introduced this hours ago: a string replace intended for one call site silently matched two. Fixed, with the reasoning written where the next reader will hit it. Relay/account audit fixes: - Phrase recovery permanently tripped the rollback alarm. The mark is keyed by the everyday pubkey (unchanged by recovery) while recovery legitimately restarts at generation 0, and raiseHighWaterMark keeps the maximum -- so anyone who had ever rotated a password met "possibly revoked credentials" on EVERY later login, forever, with no way to clear it. The phrase is the master authority; it now resets the mark. - quorumMet counted TCP connections, not answers: a query timeout RESOLVED, so a connectable-but-silent relay counted as responding and every "not enough relays answered" guard was near-vacuous. Only EOSE is an answer now. - Relay responses were never checked against the address requested, so one junk event from one relay blocked registration, rotation, and recovery for every login name and password a user tried. Events must now match author, kind, and d-tag. Website honesty (the surface a stranger reads first, and the one that had drifted furthest from the spec): - docs.html and llms.txt still carried "private keys never leave the worker" -- false on its face, since the widget ships a reveal-nsec button. Both now state the same limit the README does. - "you revoke" became honest everywhere: removing a connection deletes the stored copy; spending authority is revoked at the wallet. - index.html dropped a pre-softening recovery claim, now says an account lives only while relays retain its capsule, and says "independently unaudited" out loud. - llms.txt gains the two warnings that were missing entirely: losing BOTH password and phrase is permanent, and removal is not revocation. - Stale headers: nwc-connections.md no longer claims "Not implemented", docs/README no longer says the authorization layer is unbuilt, the test count and spec version are current, and the demo domain points at bitlogin.network. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01CMr9Yt8SYmZcfgrg3CV2G1
2026-10-27 08:31:21 +00:00 · 2026-07-31 09:47:01 -04:00
1 parent d040c2f5af
commit efd994c4de
@@ -7,7 +7,7 @@ password that unlocks a portable Nostr identity, plus an independent 12-word
 recovery phrase. Encrypted account material lives on public Nostr relays —
 no account server, no password-reset email, no database.

-This repo implements the protocol from `docs/spec.md` (v0.4) as a modular
+This repo implements the protocol from `docs/spec.md` (v0.6) as a modular
 system any static site can embed for login and Nostr signing.

 ## Layout
@@ -312,7 +312,7 @@ point of vendoring is that this is a reviewed step, not an automatic one.
 This build covers the protocol's Phase 0 (cryptographic core + test vectors)
 and the load-bearing parts of Phase 1 (account MVP), verified two ways:

- **Protocol-level tests** (`npm test`, 79 tests): Argon2id/ScalarExpand/JCS/
+- **Protocol-level tests** (`npm test`, 120 tests): Argon2id/ScalarExpand/JCS/
  padding/timestamp/NIP-44 unit tests with the spec's exact byte layouts
  (§11), and full end-to-end scenarios — create → clean-device login, phrase
  recovery → new credentials → clean-device login, password change with
@@ -36,8 +36,10 @@ identity and recovery model.
 The Connection Vault stores portable application credentials outside the core
 identity capsules as independently encrypted records. Its protocol layer
 (records, key hierarchy, relay sync, migration, tiers) is **implemented in
-`@bitlogin/core`'s `src/vault/`**; the widget authorization layer (consent
-UI, grants, Connected apps) is still to come.
+`@bitlogin/core`'s `src/vault/`**; the widget authorization layer shipped 2026-07-30 in REVEAL mode
+(`requestNwcConnection`, `offerNwcConnection`, and a Wallet-connections
+screen). Durable per-origin grants and a Connected-apps page are still to
+come; see `connection-vault.md` §19 for the open limitations.

 - [`connection-vault.md`](./connection-vault.md) — overall vault architecture,
  key hierarchy, relay storage, authorization, recovery, and deletion; §18
@@ -3,7 +3,11 @@
 **Status:** Experimental design
 **Document version:** 0.1
 **Date:** July 22, 2026
-**Implementation status:** Not implemented
+**Implementation status:** PARTIALLY implemented. The parser/exporter and the
+canonical credential shipped in `packages/core/src/vault/nwc.ts`, and the
+request/offer flows shipped in the widget. NOT implemented: declared
+capabilities, budget and expiry fields, `get_info` refresh, the suspend state,
+broker mode, and the `wallet.*` API in §12 — those sections remain design.

 ## 1. Summary

@@ -4,7 +4,7 @@
 * recovery, and password-change (which must read the capsule it replaces).
 */
 import { RelayPool } from "../nostr/pool.js";
-import { verifyNostrEvent, type NostrEvent } from "../nostr/event.js";
+import { verifyNostrEvent, findTagValue, type NostrEvent } from "../nostr/event.js";
 import { KIND_APP_DATA, D_TAG_PASSWORD_CAPSULE, D_TAG_RECOVERY_CAPSULE } from "../nostr/kinds.js";
 import { decryptCredentialCapsuleEvent } from "../capsules/credentialCapsule.js";
 import { decryptRecoveryCapsuleEvent } from "../capsules/recoveryCapsule.js";
@@ -28,9 +28,28 @@ export interface CapsuleReadResult<TPayload> {
  relayDisagreement: boolean;
 }

-function dedupeAndSort(events: NostrEvent[]): NostrEvent[] {
+/**
+ * Keeps only events that are actually AT the address we asked for.
+ *
+ * A relay's response is a claim, not an answer: nothing stops it returning a
+ * validly-signed event by some other author, of some other kind, or under a
+ * different `d` tag. Signature verification alone accepted all of those.
+ * `best` was never at risk (the AEAD binds the ciphertext to the event's own
+ * pubkey, so a foreign event cannot decrypt) -- but `candidates.length` is
+ * load-bearing in the registration, rotation, and recovery collision guards,
+ * so ONE junk event from ONE relay was enough to block all three, for every
+ * login name and password the user tried, at zero cost.
+ */
+function dedupeAndSort(
+  events: NostrEvent[],
+  authorPubkeyHex: string,
+  dTag: string
+): NostrEvent[] {
  const byId = new Map<string, NostrEvent>();
  for (const event of events) {
+    if (event.pubkey !== authorPubkeyHex) continue;
+    if (event.kind !== KIND_APP_DATA) continue;
+    if (findTagValue(event, "d") !== dTag) continue;
    if (verifyNostrEvent(event)) byId.set(event.id, event);
  }
  return [...byId.values()].sort((a, b) => b.created_at - a.created_at);
@@ -49,7 +68,7 @@ async function readAddressableCapsule<TPayload>(
  );

  const allEvents = quorum.outcomes.flatMap((o) => o.events);
-  const sortedEvents = dedupeAndSort(allEvents);
+  const sortedEvents = dedupeAndSort(allEvents, authorPubkeyHex, dTag);

  const candidates: Array<Candidate<TPayload>> = [];
  for (const event of sortedEvents) {
@@ -63,8 +82,12 @@ async function readAddressableCapsule<TPayload>(
  const best = candidates.find((c) => c.payload !== null) ?? null;

  const latestIdPerRelay = quorum.outcomes
-    .filter((o) => o.responded && o.events.length > 0)
-    .map((o) => o.events.slice().sort((a, b) => b.created_at - a.created_at)[0]!.id);
+    .map((outcome) => ({
+      responded: outcome.responded,
+      events: dedupeAndSort(outcome.events, authorPubkeyHex, dTag)
+    }))
+    .filter((outcome) => outcome.responded && outcome.events.length > 0)
+    .map((outcome) => outcome.events[0]!.id);
  const relayDisagreement = new Set(latestIdPerRelay).size > 1;

  return {
@@ -35,3 +35,25 @@ export async function raiseHighWaterMark(
  await store.set(key(everydayPublicKeyHex), JSON.stringify(merged));
  return merged;
 }
+
+/**
+ * Resets the mark after a phrase recovery.
+ *
+ * The recovered account restarts at generation 0 at a BRAND NEW locator
+ * address, but the mark is keyed by the everyday public key -- which recovery
+ * deliberately preserves -- so a user who had ever rotated a password was met
+ * with "Refusing to log in with older, possibly-revoked credentials" on every
+ * subsequent login, on every device that had seen the higher generation, with
+ * no way to clear it (raiseHighWaterMark keeps the maximum). The recovery
+ * phrase is the master authority in this protocol (§SF9); a phrase-driven
+ * reset is exactly the authority it is supposed to carry, and leaving the
+ * alarm permanently lit trains users to click through the one control that
+ * detects a real replay.
+ */
+export async function resetHighWaterMark(
+  store: KeyValueStore,
+  everydayPublicKeyHex: string,
+  generations: HighWaterMark = { generation: 0, recoveryGeneration: -1 }
+): Promise<void> {
+  await store.set(key(everydayPublicKeyHex), JSON.stringify(generations));
+}
@@ -28,6 +28,8 @@ import type { CredentialPayload, RecoveryPayload } from "../capsules/types.js";
 import { parseDmRelayListEvent, parseRelayListEvent } from "./profileEvents.js";
 import { publishAndVerify, type PublishVerificationResult } from "./publish.js";
 import { AccountNotFoundError, RecoveryFailedError } from "./errors.js";
+import { resetHighWaterMark } from "./highWaterMark.js";
+import type { KeyValueStore } from "../storage/interface.js";

 export interface RecoverWithPhraseParams {
  phrase: string;
@@ -160,6 +162,10 @@ export interface CompleteRecoveryParams {
  minAcknowledgements?: number;
  timeoutMs?: number;
  now?: number;
+  /** Where this device's rollback high-water mark lives. Passing it is what
+   *  stops the recovered account from tripping the rollback alarm on every
+   *  later login (the new capsule legitimately restarts at generation 0). */
+  store?: KeyValueStore;
 }

 export interface CompleteRecoveryResult {
@@ -285,5 +291,16 @@ export async function completeRecoveryWithNewCredentials(params: CompleteRecover
    throw new RecoveryFailedError("Could not publish the refreshed recovery and credential capsules to enough relays. Please retry.");
  }

+  // Only after both capsules are durable: the recovered account is now
+  // authoritatively at generation 0, so the mark must follow it down rather
+  // than keep flagging every future login as a rollback (see
+  // resetHighWaterMark for why the phrase is entitled to do this).
+  if (params.store) {
+    await resetHighWaterMark(params.store, recovered.everydayPublicKey, {
+      generation: 0,
+      recoveryGeneration: recovered.currentRecoveryPayload.recovery_generation + 1
+    });
+  }
+
  return { normalizedLoginName, locatorPublicKey, credentialEvent, refreshedRecoveryEvent, credentialPublish, recoveryPublish };
 }
@@ -184,16 +184,27 @@ export class RelayConnection {
      content: JSON.stringify(filter) + Math.random()
    }).slice(0, 16);

-    return new Promise<NostrEvent[]>((resolve) => {
+    return new Promise<NostrEvent[]>((resolve, reject) => {
      const events: NostrEvent[] = [];
-      const finish = () => {
+      const settle = (fail?: Error) => {
        clearTimeout(timer);
        this.subs.delete(subId);
        this.send(["CLOSE", subId]);
-        resolve(events);
+        if (fail) reject(fail);
+        else resolve(events);
      };
-      const timer = setTimeout(finish, timeoutMs);
-      this.subs.set(subId, { events, onEose: finish });
+      // A relay that opened a socket and then said NOTHING for the whole
+      // window has not answered, and must not be counted toward quorum:
+      // quorumMet was measuring reachability, not participation, which made
+      // every "not enough relays answered" guard -- login, recovery,
+      // rotation, and the registration collision check -- near-vacuous
+      // against a set of connectable-but-silent relays. Only EOSE is an
+      // answer; the timeout is a failure.
+      const timer = setTimeout(
+        () => settle(new Error("relay did not answer the subscription before the timeout")),
+        timeoutMs
+      );
+      this.subs.set(subId, { events, onEose: () => settle() });
      this.send(["REQ", subId, filter]);
    });
  }
@@ -88,9 +88,7 @@ const signed = await window.nostr.signEvent({
 const cipher = await window.nostr.nip44.encrypt(peerPubkeyHex, 'hi there');
 const plain  = await window.nostr.nip44.decrypt(peerPubkeyHex, cipher);</code></pre>
      <div class="callout">
-        <strong>Private keys never leave a dedicated Web Worker.</strong> The main thread — your page's JS — only ever
-        sees public keys, signed events, and ciphertext. <code>window.nostr</code> calls are proxied into the worker
-        and back.
+        <strong>Private keys live in a dedicated Web Worker.</strong> The signing APIs return only public keys, signed events, and ciphertext — the main thread never receives a private key through them. Be precise about the limit, though: an embedded widget runs <em>same-origin</em> with the page hosting it, so a malicious or XSS-compromised host can reach the worker directly (including <code>exportIdentity</code>, which backs the widget's own "reveal private key" button) and can visually overlay the consent UI. Treat the host page as trusted and embed BitLogin only in sites you control.
      </div>
    </li>
    <li>
@@ -104,7 +104,7 @@
    </div>
    <div class="feature">
      <h3>Recovery phrase</h3>
-      <p>A 12-word phrase derives a completely independent recovery identity. A cracked password can never sign, replace, or delete the recovery capsule — it has no authority over that path at all.</p>
+      <p>A 12-word phrase derives a completely independent recovery identity. A cracked password cannot sign, replace, or delete the recovery capsule. Recovery availability still depends on relays retaining your capsule and on your own recovery export — quorum and retention are best-effort, not guarantees.</p>
    </div>
    <div class="feature">
      <h3>One everyday identity</h3>
@@ -145,7 +145,7 @@
    </div>
    <div class="feature">
      <h3>No single point of failure</h3>
-      <p>Encrypted capsules are published to multiple independent relays with quorum reads and readback verification, so losing any one of them doesn't lock anyone out.</p>
+      <p>Encrypted capsules are published to multiple independent relays with quorum reads and readback verification, so losing any one of them doesn't lock anyone out. An account still lives only as long as some relay retains its capsule — keep the recovery export.</p>
    </div>
    <div class="feature">
      <h3>Generated by default, never weak</h3>
@@ -165,7 +165,7 @@
    </div>
    <div class="feature">
      <h3>Wallets that follow you</h3>
-      <p>The Connection Vault stores wallet connections (NWC) as independently encrypted records on your account. Connect a wallet to an app once; on a new device it's one tap, not another paste. Apps ask with <code>requestNwcConnection()</code>, you approve, you revoke — and the budget your wallet enforces stays the real limit.</p>
+      <p>The Connection Vault stores wallet connections (NWC) as independently encrypted records on your account. Connect a wallet to an app once; on a new device it's one tap, not another paste. Apps ask with <code>requestNwcConnection()</code>, you approve, and you remove the stored copy any time (revoking spending authority happens at your wallet) — and the budget your wallet enforces stays the real limit.</p>
    </div>
  </div>
 </section>
@@ -179,7 +179,7 @@
 </section>

 <footer>
-  BitLogin is an experimental alpha protocol, built on the open Nostr network. This demo is a reference implementation, not a production account service.
+  BitLogin is an experimental, independently unaudited alpha protocol, built on the open Nostr network. This demo is a reference implementation, not a production account service.
  <div class="footer-links"><a href="https://github.com/PR0M3TH3AN/bitlogin">Source on GitHub</a> · MIT licensed — <a href="https://github.com/PR0M3TH3AN/bitlogin/blob/main/LICENSE">LICENSE</a></div>
 </footer>

@@ -10,7 +10,7 @@
 ## Integrate in one snippet

 ```html
-<script type="module" src="https://bitlogin.vercel.app/vendor/bitlogin/bitlogin.js"></script>
+<script type="module" src="https://bitlogin.network/vendor/bitlogin/bitlogin.js"></script>
 <bitlogin-auth
  vault-relays="wss://relay-one.example,wss://relay-two.example,wss://relay-three.example"
  discovery-relays="wss://discovery-one.example,wss://discovery-two.example">
@@ -67,8 +67,16 @@ deployment above.
  (the last also fires on `window` when triggered via
  `window.bitlogin.releaseSigner()`), `bitlogin-connection-granted`
  (`detail.origin`).
- Private keys never leave a dedicated Web Worker; the main thread only ever
-  sees public keys, signed events, and ciphertext.
+- Private keys live in a dedicated Web Worker and the signing APIs return only
+  public keys, signed events, and ciphertext. LIMIT: an embedded widget is
+  same-origin with its host page, so a malicious or XSS-compromised host can
+  reach the worker directly (including exportIdentity) and can overlay the
+  consent UI. Treat the host page as trusted; embed only in sites you control.
+- Removing a wallet connection from BitLogin deletes the stored copy only. It
+  is NOT revocation -- spending authority is revoked at the wallet.
+- Losing BOTH the password and the recovery phrase is permanent: there is no
+  custodial recovery, by design. An account also depends on relays retaining
+  its encrypted capsule; download the recovery export as an offline fallback.

 ## Key facts for integration

@@ -92,7 +100,7 @@ deployment above.
 - [Live account manager demo](/account.html) — create/login/recover, sign a
  test event, rotate password, export identity.
 - [Protocol specification](https://github.com/PR0M3TH3AN/bitlogin/blob/main/docs/spec.md) —
-  the full BitLogin v0.3 protocol (crypto construction, capsule formats,
+  the full BitLogin protocol (crypto construction, capsule formats,
  relay architecture, threat model).
 - [Second-factor / recovery options design notes](https://github.com/PR0M3TH3AN/bitlogin/blob/main/docs/second-factor.md)
  and [future recovery-path options](https://github.com/PR0M3TH3AN/bitlogin/blob/main/docs/future-recovery-options.md) —
@@ -103,6 +111,8 @@ deployment above.

 ## Status

-Experimental alpha. Verified with an in-memory mock-relay test suite and
-real-browser end-to-end runs, but not independently audited. See the README
+Experimental alpha, NOT independently audited. Verified with an in-memory
+mock-relay test suite, real-browser end-to-end runs, and an internal
+adversarial audit (2026-07-31) whose findings are recorded in
+docs/connection-vault.md §19 and the CHANGELOG. See the README
 in the source repository for the full implemented/deferred feature list.
@@ -333,7 +333,8 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
        recovered: session.pendingRecovery,
        newLoginName: p.newLoginName,
        newPassword: p.newPassword,
-        vaultRelayUrls
+        vaultRelayUrls,
+        store
      });
      session.activeCredentialEvent = result.credentialEvent;
      session.activeRecoveryEvent = result.refreshedRecoveryEvent;
@@ -561,10 +562,21 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
    }

    case "vaultSetBinding": {
+      // Two transitions only, and the difference is the whole feature:
+      // origin === null is UNBIND (the dashboard's "Revoke app access"), and
+      // anything else binds to THIS origin -- never a caller-named one, or a
+      // host could adopt a wallet the user bound to another site.
+      //
+      // The bare identifier `origin` sat here and silently resolved to the
+      // WORKER GLOBAL self.origin, so revoke rebound the record to the
+      // calling page instead of clearing it -- the exact adoption this guard
+      // exists to prevent, wearing the label of the button that prevents it.
+      // It typechecked because the DOM lib declares that global.
      const p = payload as { connectionId: string; origin: string | null };
      const { vault, connection } = await findConnection(p.connectionId);
+      const nextOrigin = p.origin === null ? null : selfOrigin();
      const { record, event } = await vault.updateConnection(connection, {
-        application_binding: { origin, app_pubkey: null }
+        application_binding: { origin: nextOrigin, app_pubkey: null }
      });
      const publish = await vault.publish({
        event,