hub: a browser key is never replaced by a second tab, and is written durably (K5a2)

hub/docs/PRD_COMMITMENTS.md

@@ -296,6 +296,7 @@ K5 adds four things.
 3. **Where the key lives.** It lives in IndexedDB as a **non-extractable** `CryptoKey`. The page can use the key to sign, but no script, including the page's own, can read the private half.
    - This deliberately differs from `/client`, which keeps an extractable JWK in `localStorage`, where any script on the origin can read it. K5a fixes `/client` first (below).
    - Non-extractable stops theft, not misuse: a script injected into the page can still sign with the key in place. So every page that holds a key (`/client`, `/c/`, the join page, `/me`) is served with a strict Content-Security-Policy, and loses its inline `onclick` handlers. Today no hub page sends a CSP.
+     - **The protection is origin-wide, not page-wide (Legion, #960).** IndexedDB belongs to the origin, so a script injected into ANY page on it can open the store and sign, even though it can't extract the key. `/discuss` and the admin-listener pages (`/`, `/record`, `/admin/*`) still send no CSP, and the admin pages carry inline `<script>`. Until those pages are strict too, or keys move to an origin of their own, the CSP on the key pages narrows the risk without closing it. Tracked as a follow-up; it is not a regression.
    - The page calls `navigator.storage.persist()`. It also shows "open this in your browser" when it detects an in-app webview: a key made inside a mail app's webview stays there, and a later `/me` in the real browser finds nothing.
    - If the browser is lost, so is the key. Safari also evicts script-written storage after 7 days without interaction, which is exactly a volunteer nudged monthly. Recovery is a fresh invite: a new LCT, which the operator links to the old one with Slice E's `attest-predecessor`. The member's history then follows the lineage, and the old key is never needed. **Expect this path to run routinely, not rarely**, so it must be one click for the operator.
    - A browser key is priced at web4-core's default anchor level, a software key, and claims nothing more.
@@ -305,6 +306,12 @@ K5 adds four things.
 **Order of work:**
 - **K5a: `/client` and CSP.** On first load, `/client` imports its stored JWK once as a non-extractable `CryptoKey` into IndexedDB, then deletes the `localStorage` entry. The key and the LCT stay the same, so no identity is orphaned. The same PR adds the strict CSP to the key-holding pages.
 - **K5b: invite, join, `ContactBound`, `InviteRevoked`, the admission notice.**
+- **K5a2: Legion's #960 follow-ups.**
+  - A key is stored with `add`, never `put`. A second tab that also found no key gets the stored key back instead of replacing one that may already be admitted. A migration that loses that race leaves the old JWK alone.
+    - The losing tab's error is read from the failed request: IndexedDB fires the request's error before the abort sets the transaction's (Legion, #963).
+    - **Known gap:** if a fresh key wins against a migration, the old JWK stays in `localStorage` and is never read again. That is an extractable key that may be admitted, the exposure K5a exists to remove. The race is rare. Recovering that person goes through the organiser.
+  - Both key writes ask for `{durability:'strict'}`, so the write is on disk before the old JWK is deleted.
+  - `navigator.storage.persisted()` is checked on every load and persistence is requested again. When the browser refuses, `/client` and the join page say so. Unverified: Firefox may ask the person again on each `persist()` call while storage is not persisted.
 - **K5b2: commitments follow the binding.** A commitment that names the contact reads its role for the member: role checks, nudge targets and acknowledgements. Split from K5b so each PR stays reviewable.
 - **K5c: `/me`.**
 

hub/hub-daemon/js-tests/client_keystore.js

@@ -8,7 +8,8 @@ const assert = require('assert');
 
 function makeIdb(failPut) {
   const dbs = {};
-  return {
+  const opts = [];
+  const idbApi = {
     open(name) {
       const req = {};
       setTimeout(() => {
@@ -17,17 +18,36 @@ function makeIdb(failPut) {
         const db = {
           createObjectStore(n) { stores[n] = new Map(); },
           close() {},
-          transaction(n, mode) {
+          transaction(n, mode, o) {
             const tx = {};
+            if (mode === 'readwrite') opts.push(o || {});
+            let failed = failPut;
             const store = {
-              get(k) { const r = { result: stores[n].get(k) }; return r; },
-              put(v, k) { if (failPut) { setTimeout(() => tx.onabort && tx.onabort()); return {}; }
-                          stores[n].set(k, v); return { result: k }; },
+              // hideNextGet: the read a tab made just before another tab stored a key.
+              get(k) { if (idbApi.hideNextGet) { idbApi.hideNextGet = false; return { result: undefined }; }
+                       return { result: stores[n].get(k) }; },
+              add(v, k) {
+                if (failed) { setTimeout(() => tx.onabort && tx.onabort()); return {}; }
+                if (stores[n].has(k)) {
+                  // The IndexedDB order (Legion, #963): the request's error event reaches
+                  // tx.onerror first, while tx.error is still null; only the abort sets it.
+                  failed = true;
+                  const req = { error: Object.assign(new Error('key exists'), { name: 'ConstraintError' }) };
+                  tx.error = null;
+                  setTimeout(() => {
+                    tx.onerror && tx.onerror({ target: req });
+                    tx.error = req.error;
+                    tx.onabort && tx.onabort();
+                  });
+                  return req;
+                }
+                stores[n].set(k, v); return { result: k };
+              },
               delete(k) { stores[n].delete(k); return { result: undefined }; },
             };
             tx.objectStore = () => store;
             tx.error = new Error('quota');
-            setTimeout(() => { if (!failPut || mode === 'readonly') tx.oncomplete && tx.oncomplete(); });
+            setTimeout(() => { if (!failed || mode === 'readonly') tx.oncomplete && tx.oncomplete(); });
             return tx;
           },
         };
@@ -38,20 +58,23 @@ function makeIdb(failPut) {
       return req;
     },
     dbs,
+    opts,
+    hideNextGet: false,
   };
+  return idbApi;
 }
 
-function makeEnv(failPut) {
+function makeEnv(failPut, { idb, persisted = true, persistGrant = true } = {}) {
   const ls = new Map();
   const el = () => ({ innerHTML: '', textContent: '', className: '', value: '', href: '',
     classList: { add() {}, remove() {} } });
   const els = {};
   const ctx = {
     crypto: globalThis.crypto, TextEncoder, Uint8Array, JSON, Promise, String, Object, Array, Error, setTimeout,
     console,
-    indexedDB: makeIdb(failPut),
+    indexedDB: idb || makeIdb(failPut),
     localStorage: { getItem: k => (ls.has(k) ? ls.get(k) : null), setItem: (k, v) => ls.set(k, String(v)), removeItem: k => ls.delete(k) },
-    navigator: { storage: { persist: () => Promise.resolve(true) } },
+    navigator: { storage: { persisted: () => Promise.resolve(persisted), persist: () => Promise.resolve(persistGrant) } },
     document: { querySelector: s => (els[s] || (els[s] = el())), addEventListener() {} },
     fetch: () => Promise.reject(new Error('offline')),
   };
@@ -120,4 +143,33 @@ const run = (ctx, code) => vm.runInContext(code, ctx);
     assert.strictEqual(await run(c.ctx, 'idbGet()'), undefined, 'and nothing half-moved');
   }
   console.log('ok 6 a corrupt old key is named and kept');
+
+  // 7. Two tabs that both found no key: the second never replaces the first one's key.
+  const shared = makeIdb(false);
+  const tabA = makeEnv(false, { idb: shared }), tabB = makeEnv(false, { idb: shared });
+  const b = await run(tabB.ctx, 'newID()');
+  const a = await run(tabA.ctx, 'newID()');
+  assert.strictEqual(a.lct_id, b.lct_id, 'the later tab gets the stored key back');
+  assert.strictEqual((await run(tabA.ctx, 'loadID()')).lct_id, b.lct_id);
+  // A migration that loses the same race keeps the old JWK where it was.
+  tabA.ls.set('hestia_hub_id', JSON.stringify(old));
+  await run(tabA.ctx, 'idbDel()');
+  await run(tabB.ctx, 'newID()');
+  const c2 = makeEnv(false, { idb: shared }); c2.ls.set('hestia_hub_id', JSON.stringify(old));
+  shared.hideNextGet = true;
+  const kept = await run(c2.ctx, 'loadID()');
+  assert.strictEqual(shared.hideNextGet, false, 'the migration really read an empty store');
+  assert.notStrictEqual(kept.lct_id, 'lct-1'); assert.ok(c2.ls.has('hestia_hub_id'), 'the JWK is not deleted for a key it did not store');
+  console.log('ok 7 a second tab never replaces a stored key');
+
+  // 8. Every key write asks for strict durability.
+  assert.ok(shared.opts.filter(o => o.durability !== undefined).length >= 3, JSON.stringify(shared.opts));
+  assert.ok(shared.opts.filter(o => o.durability !== undefined).every(o => o.durability === 'strict'));
+  console.log('ok 8 key writes are strict-durable');
+
+  // 9. Persistence is asked for on each load and reported when refused.
+  assert.strictEqual(await run(makeEnv(false).ctx, 'ensurePersisted()'), true);
+  assert.strictEqual(await run(makeEnv(false, { persisted: false, persistGrant: true }).ctx, 'ensurePersisted()'), true);
+  assert.strictEqual(await run(makeEnv(false, { persisted: false, persistGrant: false }).ctx, 'ensurePersisted()'), false);
+  console.log('ok 9 persistence is checked and its refusal is visible');
 })().catch(e => { console.error('FAIL', e); process.exit(1); });

hub/hub-daemon/src/rest.rs

@@ -8415,6 +8415,7 @@ const CLIENT_HTML: &str = r##"<!doctype html><meta charset="utf-8">
  never leaves this device, and not even this page can read it back; the hub pins only your public
  half when you join.</p>
  <div id="idbody"></div>
+ <p id="persistnote" class="m warn hidden"></p>
 </div>
 
 <div class="card hidden" id="joincard">
@@ -8488,18 +8489,38 @@ function idb(){
     r.onsuccess=()=>ok(r.result); r.onerror=()=>no(r.error);
   });
 }
-async function idbDo(mode,f){
+async function idbDo(mode,f,opts){
   const db=await idb();
   return new Promise((ok,no)=>{
-    const tx=db.transaction(STORE,mode), r=f(tx.objectStore(STORE));
+    const tx=db.transaction(STORE,mode,opts), r=f(tx.objectStore(STORE));
     tx.oncomplete=()=>{ db.close(); ok(r.result); };
-    tx.onerror=tx.onabort=()=>{ db.close(); no(tx.error); };
+    // A failed request reaches onerror before the abort sets tx.error (Legion, #963).
+    tx.onerror=ev=>{ db.close(); no(ev.target.error||tx.error); };
+    tx.onabort=()=>{ db.close(); no(tx.error); };
   });
 }
 const idbGet=()=>idbDo('readonly',s=>s.get(SLOT));
-const idbPut=v=>idbDo('readwrite',s=>s.put(v,SLOT));
 const idbDel=()=>idbDo('readwrite',s=>s.delete(SLOT));
+// A key is stored with add, never put: a second tab that also found no key must not replace one
+// this browser has since made and perhaps had admitted (Legion, #960). The loser gets the stored
+// key back. Strict durability: the write is on disk before anything is deleted on its account.
+async function idbAdd(v){
+  try{ await idbDo('readwrite',s=>s.add(v,SLOT),{durability:'strict'}); return v; }
+  catch(e){
+    if(e&&e.name==='ConstraintError'){ const had=await idbGet(); if(had)return had; }
+    throw e;
+  }
+}
 function persist(){ if(navigator.storage&&navigator.storage.persist)navigator.storage.persist().catch(()=>{}); }
+// Whether this browser has agreed to keep the key store when it runs short of space or the
+// site goes unvisited. Asked on every load, since a browser that refused once may agree later.
+// null: the browser can't say.
+async function ensurePersisted(){
+  const st=navigator.storage;
+  if(!st||!st.persisted)return null;
+  try{ return (await st.persisted())||(st.persist?await st.persist():false); }catch(e){ return false; }
+}
+const NOT_PERSISTED='This browser hasn\'t agreed to keep your key for good. If you don\'t come back for a few weeks, it may be cleared; if that happens, ask your organiser for a new invite.';
 
 async function loadID(){
   const id=await idbGet(); if(id)return id;
@@ -8513,7 +8534,10 @@ async function loadID(){
     priv=await crypto.subtle.importKey('jwk',o.privJwk,{name:'Ed25519'},false,['sign']);
   }catch(e){ throw new Error('LEGACY_UNREADABLE'); }
   const moved={lct_id:o.lct_id,pubHex:o.pubHex,priv};
-  await idbPut(moved); localStorage.removeItem(LEGACY); persist();
+  const kept=await idbAdd(moved);
+  // Another tab stored a key first: that one is in use, and the old JWK stays as it was.
+  if(kept!==moved)return kept;
+  localStorage.removeItem(LEGACY); persist();
   return moved;
 }
 // A fresh identity, stored before it is returned. Throws NO_ED25519 where the browser
@@ -8524,8 +8548,8 @@ async function newID(){
   catch(e){ throw new Error('NO_ED25519'); }
   const pub=await crypto.subtle.exportKey('raw',kp.publicKey);
   const id={lct_id:crypto.randomUUID(),pubHex:hex(pub),priv:kp.privateKey};
-  await idbPut(id); persist();
-  return id;
+  const kept=await idbAdd(id); persist();
+  return kept;
 }
 async function forgetID(){ await idbDel(); localStorage.removeItem(LEGACY); }
 
@@ -8646,6 +8670,7 @@ document.addEventListener('click',ev=>{
     ?'this browser holds an older key that can\'t be read; it has been left as it was. Ask your organiser.'
     :'could not open this browser\'s key store: '+esc(e.message))+'</span>'; }
   if(ID||!$('#idbody').textContent)renderID();
+  if(ID&&await ensurePersisted()===false){ $('#persistnote').textContent=NOT_PERSISTED; $('#persistnote').classList.remove('hidden'); }
   loadTopics();
 })();
 "##;

hub/hub-daemon/src/rest/commitments.rs

@@ -2071,6 +2071,6 @@ mod tests {
         };
         let said = format!("{}{}", String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr));
         assert!(out.status.success(), "{said}");
-        assert_eq!(said.matches("\nok ").count() + usize::from(said.starts_with("ok ")), 6, "{said}");
+        assert_eq!(said.matches("\nok ").count() + usize::from(said.starts_with("ok ")), 9, "{said}");
     }
 }

hub/hub-daemon/src/rest/commitments/invites.rs

@@ -380,7 +380,8 @@ async function join(){
     const r=await fetch('/v1/hubs/'+w.hub_lct_id+'/members/join',{method:'POST',
       headers:{'content-type':'application/json'},body:JSON.stringify(await signEnvelope(id,p))});
     const out=await r.json().catch(()=>({}));
-    if(r.status===200){ b.hidden=true; say('ok','You\'re in. Keep using this browser: your key lives here.'); }
+    if(r.status===200){ b.hidden=true;
+      say('ok','You\'re in. Keep using this browser: your key lives here.'+(await ensurePersisted()===false?' '+NOT_PERSISTED:'')); }
     else if(r.status===202){ b.hidden=true; say('muted','Your organiser will confirm you. Once they have, open this link again.'); }
     else { say('err',out.error||('Something went wrong ('+r.status+').')); b.disabled=false; }
   }catch(e){