Harden agent-contact CRM (contrarian gate): serialize writes, stable letter ids, XSS guards

ed5770bdf620007ab29bfe86247c06d8fa9bbbc4 · 2026-07-17 09:04:59 -0700 · Steve

- Concurrency (HIGH #1): every read-modify-write of agent-contacts.json now runs through a
  promise-chain lock (withContacts) so interleaved requests cant last-writer-win and drop data.
  Verified: 8 parallel saves -> all 8 persisted.
- Stable letter ids (HIGH #2): each letter gets a monotonic id at creation; delete addresses by
  id (index fallback for legacy), and send-draft records its own id-stamped letter instead of the
  fragile match-by-20k-char-body that could push a ghost + shift indices. Verified delete-by-id.
- XSS: safeUrl() blocks javascript:/data: URLs in p.source hrefs; esc() applied to qwen.thesis,
  qwen.risks[], and upside_note (were raw innerHTML).

Verified headless: parallel-save no-loss, delete-by-id, UI 2->1 delete, 0 errors, still fail-closed.

Files touched

M public/index.html
M scripts/serve.js

Diff

commit ed5770bdf620007ab29bfe86247c06d8fa9bbbc4
Author: Steve <steve@designerwallcoverings.com>
Date:   Fri Jul 17 09:04:59 2026 -0700

    Harden agent-contact CRM (contrarian gate): serialize writes, stable letter ids, XSS guards
    
    - Concurrency (HIGH #1): every read-modify-write of agent-contacts.json now runs through a
      promise-chain lock (withContacts) so interleaved requests cant last-writer-win and drop data.
      Verified: 8 parallel saves -> all 8 persisted.
    - Stable letter ids (HIGH #2): each letter gets a monotonic id at creation; delete addresses by
      id (index fallback for legacy), and send-draft records its own id-stamped letter instead of the
      fragile match-by-20k-char-body that could push a ghost + shift indices. Verified delete-by-id.
    - XSS: safeUrl() blocks javascript:/data: URLs in p.source hrefs; esc() applied to qwen.thesis,
      qwen.risks[], and upside_note (were raw innerHTML).
    
    Verified headless: parallel-save no-loss, delete-by-id, UI 2->1 delete, 0 errors, still fail-closed.
---
 public/index.html | 22 ++++++++--------
 scripts/serve.js  | 77 ++++++++++++++++++++++++++++++++++---------------------
 2 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/public/index.html b/public/index.html
index ae52207..4d1785c 100644
--- a/public/index.html
+++ b/public/index.html
@@ -808,7 +808,7 @@ function renderList(rows){
   const head='<th scope="col" class="stick sortable" data-sortk="addr" title="Sort by address">Property'+arrow('addr')+'</th>'+cols.map(c=>`<th scope="col" class="sortable" data-sortk="${c.k}" title="Sort by ${esc(c.label)}">${esc(c.label)}${arrow(c.k)}</th>`).join('');
   const colspan=cols.length+1;
   const body=rows.map(p=>`<tr class="lrow" data-id="${p.id}" title="Click the row to open agent contact + deeper searches">`
-    +`<td class="stick"><span class="exptog" aria-hidden="true">▸</span><button class="starbtn ${SHORT.has(p.id)?'on':''}" data-star="${p.id}" aria-label="${SHORT.has(p.id)?'Remove from':'Add to'} shortlist" aria-pressed="${SHORT.has(p.id)}" title="Shortlist">★</button> <a class="la" href="${p.source}" target="_blank" rel="noopener noreferrer">${esc(p.address)}</a><div class="lc">${esc(p.city)}, CA ${p.zip||''}</div></td>`
+    +`<td class="stick"><span class="exptog" aria-hidden="true">▸</span><button class="starbtn ${SHORT.has(p.id)?'on':''}" data-star="${p.id}" aria-label="${SHORT.has(p.id)?'Remove from':'Add to'} shortlist" aria-pressed="${SHORT.has(p.id)}" title="Shortlist">★</button> <a class="la" href="${safeUrl(p.source)}" target="_blank" rel="noopener noreferrer">${esc(p.address)}</a><div class="lc">${esc(p.city)}, CA ${p.zip||''}</div></td>`
     +cols.map(c=>`<td>${colCell(c.k,p)}</td>`).join('')+`</tr>`
     +`<tr class="lexp" data-for="${p.id}"><td colspan="${colspan}"><div class="lexpbody" id="exp-${p.id}"></div></td></tr>`).join('');
   return `<div class="listwrap"><table class="listtbl"><thead><tr>${head}</tr></thead><tbody>${body}</tbody></table></div>`;
@@ -819,7 +819,7 @@ function renderList(rows){
 function expandBody(p){
   const a=agentInfo(p), firm=a.firm, id=p.id;
   const deep=[];
-  deep.push(`<a href="${p.source}" target="_blank" rel="noopener noreferrer">View listing ↗</a>`);
+  deep.push(`<a href="${safeUrl(p.source)}" target="_blank" rel="noopener noreferrer">View listing ↗</a>`);
   deep.push(loopnetLink(p));
   deep.push(`<button class="lcbtn" data-id="${id}" title="Pull fresh comps from LoopNet via a cloud browser (~$0.03)">🔄 Live comps</button>`);
   deep.push(`<button class="histbtn" data-id="${id}" data-address="${(p.address||'').replace(/"/g,'&quot;')}" data-city="${(p.city||'').replace(/"/g,'&quot;')}" data-zip="${p.zip||''}" title="LA County assessor + permit + film history (free)">📜 Property history</button>`);
@@ -955,6 +955,8 @@ function agentInfo(p){
            firm:c.firm||firmLabel(p), contacted_at:c.contacted_at||null, letters:(c.letters||[]) };
 }
 function gsearch(q){ return 'https://www.google.com/search?q='+encodeURIComponent(q); }
+// Only http(s) URLs are safe in an href — blocks a javascript:/data: scheme sneaking in via p.source.
+function safeUrl(u){ u=String(u==null?'':u); return /^https?:\/\//i.test(u)?u:'#'; }
 // Listing-firm cell → scoped web search for that brokerage + its agents.
 function firmCell(p){ const n=firmLabel(p); if(!n||n==='Unknown') return esc(n||'—');
   return `<a class="flink" href="${gsearch('"'+n+'" commercial real estate brokerage Los Angeles')}" target="_blank" rel="noopener noreferrer" title="Find ${esc(n)} — brokerage & its agents (opens Google)">${esc(n)} ↗</a>`; }
@@ -1051,12 +1053,12 @@ function card(p){
       ${vis('cashNeeded')?`<div><span class="k">Cash needed</span><span class="v ${f.affordable?'pos':'neg'}">${fmt(f.cashNeeded)}</span></div>`:''}
       ${vis('cashFlow')?`<div><span class="k">Cash flow/yr</span><span class="v ${cls(f.cashFlow,1,0)}">${fmt(f.cashFlow)}</span></div>`:''}
     </div>`:''}
-    ${vis('thesis')&&p.qwen?.thesis?`<div class="thesis">${p.qwen.thesis}</div>`:''}
-    ${vis('risks')&&p.qwen?.risks?.length?`<div><span class="small">Risks</span><ul class="lst">${p.qwen.risks.map(r=>`<li>${r}</li>`).join('')}</ul></div>`:''}
-    ${vis('upside')&&p.upside_note?`<div class="small">💡 ${p.upside_note}</div>`:''}
+    ${vis('thesis')&&p.qwen?.thesis?`<div class="thesis">${esc(p.qwen.thesis)}</div>`:''}
+    ${vis('risks')&&p.qwen?.risks?.length?`<div><span class="small">Risks</span><ul class="lst">${p.qwen.risks.map(r=>`<li>${esc(r)}</li>`).join('')}</ul></div>`:''}
+    ${vis('upside')&&p.upside_note?`<div class="small">💡 ${esc(p.upside_note)}</div>`:''}
     ${vis('demo')?demoLine(p):''}
     ${vis('rent')?rentLine(p):''}
-    ${vis('links')?`<div class="src"><a href="${p.source}" target="_blank" rel="noopener noreferrer">View listing ↗</a> · ${loopnetLink(p)} · <button class="lcbtn" data-id="${p.id}" title="Pull fresh comps from LoopNet via a cloud browser (~$0.03)">🔄 Live comps</button> · <button class="histbtn" data-id="${p.id}" data-address="${(p.address||'').replace(/"/g,'&quot;')}" data-city="${(p.city||'').replace(/"/g,'&quot;')}" data-zip="${p.zip||''}" title="LA County assessor + permit + film history (free)">📜 Property history</button></div>`:''}
+    ${vis('links')?`<div class="src"><a href="${safeUrl(p.source)}" target="_blank" rel="noopener noreferrer">View listing ↗</a> · ${loopnetLink(p)} · <button class="lcbtn" data-id="${p.id}" title="Pull fresh comps from LoopNet via a cloud browser (~$0.03)">🔄 Live comps</button> · <button class="histbtn" data-id="${p.id}" data-address="${(p.address||'').replace(/"/g,'&quot;')}" data-city="${(p.city||'').replace(/"/g,'&quot;')}" data-zip="${p.zip||''}" title="LA County assessor + permit + film history (free)">📜 Property history</button></div>`:''}
     ${anyAssessorVisible()?`<div class="assessline small" data-addr="${(p.address||'').replace(/"/g,'&quot;')}" data-city="${(p.city||'').replace(/"/g,'&quot;')}">📋 assessor…</div>`:''}
     ${cardAgentBlock(p)}
     <div class="lcout" id="lc-${p.id}"></div>
@@ -1594,7 +1596,7 @@ async function saveAgentInfo(id){
   catch(e){ toast('⚠ '+e.message); }
 }
 async function deleteLetter(id,idx){
-  try{ const j=await postJSON('/api/contacts/'+encodeURIComponent(id)+'/letter/'+idx+'/delete',{}); if(j.contact){ window.CONTACTS[id]=j.contact; refreshContactUI(id); repaintModal(id); toast('letter deleted'); } }
+  try{ const j=await postJSON('/api/contacts/'+encodeURIComponent(id)+'/letter/'+encodeURIComponent(idx)+'/delete',{}); if(j.contact){ window.CONTACTS[id]=j.contact; refreshContactUI(id); repaintModal(id); toast('letter deleted'); } }
   catch(e){ toast('⚠ '+e.message); }
 }
 function agentDraft(p,a){ return `Hi ${a.name||'there'},\n\nI'm interested in ${p.address||'your listing'}${p.city?' in '+p.city:''}${p.price?' (listed '+fmt(p.price)+')':''}. Could you share the setup — current rents / in-place NOI, any financing in place, and whether an offer near list would be entertained?\n\nThanks,\nFrank`; }
@@ -1603,7 +1605,7 @@ function contactModalBody(p){
   const a=agentInfo(p), id=p.id, firm=a.firm;
   const web=a.name?` <a class="flink" href="${gsearch('"'+a.name+'"'+(firm&&firm!=='Unknown'?' "'+firm+'"':'')+' real estate agent')}" target="_blank" rel="noopener noreferrer">find agent ↗</a>`:'';
   const letters=a.letters.length?`<div class="crm-letters"><div class="deeplbl">Saved letters (${a.letters.length}) — saved, not sent</div>`
-    +a.letters.map((l,i)=>`<div class="lett"><div class="letth"><b>${esc(l.subject||'(no subject)')}</b>${l.draft_id?`<span class="achip done" title="Gmail draft created${l.to?' to '+esc(l.to):''} — review & send in Gmail">📤 drafted</span>`:''}<span class="letts">${fmtWhen(l.ts)}</span><button class="letdel" data-letdel="${id}" data-idx="${i}" title="Delete this saved letter">✕</button></div><div class="lettb">${esc(l.body||'')}</div></div>`).join('')+`</div>`:'';
+    +a.letters.map((l,i)=>`<div class="lett"><div class="letth"><b>${esc(l.subject||'(no subject)')}</b>${l.draft_id?`<span class="achip done" title="Gmail draft created${l.to?' to '+esc(l.to):''} — review & send in Gmail">📤 drafted</span>`:''}<span class="letts">${fmtWhen(l.ts)}</span><button class="letdel" data-letdel="${id}" data-idx="${l.id||i}" title="Delete this saved letter">✕</button></div><div class="lettb">${esc(l.body||'')}</div></div>`).join('')+`</div>`:'';
   return `<div class="composeCard">
     <div class="composeH">✎ Agent contact — <b>${esc(p.address||'listing')}</b>${p.city?', '+esc(p.city):''}${web}<button class="composeX" title="Close">✕</button></div>
     <div class="cm-info">
@@ -1657,7 +1659,7 @@ function openContact(id){
     }
     if(e.target.closest('[data-saveinfo]')){ saveAgentInfo(id); return; }
     if(e.target.closest('[data-mark]')){ markContacted(id); return; }
-    const ld=e.target.closest('[data-letdel]'); if(ld){ deleteLetter(id,+ld.dataset.idx); return; }
+    const ld=e.target.closest('[data-letdel]'); if(ld){ deleteLetter(id, ld.dataset.idx); return; }
   };
 }
 // Repaint the open modal in place (after mark/letter changes) — preserves it being open.
@@ -1768,7 +1770,7 @@ fetch('data/ranked.json').then(r=>r.json()).then(async d=>{
     const ct=e.target.closest('[data-contact]'); if(ct){ openCompose(ct.dataset.contact); return; }
     const mk=e.target.closest('[data-mark]'); if(mk){ markContacted(mk.dataset.mark); return; }
     const si=e.target.closest('[data-saveinfo]'); if(si){ saveAgentInfo(si.dataset.saveinfo); return; }
-    const ld=e.target.closest('[data-letdel]'); if(ld){ deleteLetter(ld.dataset.letdel,+ld.dataset.idx); return; }
+    const ld=e.target.closest('[data-letdel]'); if(ld){ deleteLetter(ld.dataset.letdel, ld.dataset.idx); return; }
     const row=e.target.closest('.listtbl tr.lrow'); if(row && !e.target.closest('a,button,input,textarea,select')){ toggleExpand(row); return; } };
   $('#shortbtn').onclick=()=>{ F.shortlist=!F.shortlist; render(); };
   $('#cmpbtn').onclick=openCompare;
diff --git a/scripts/serve.js b/scripts/serve.js
index 2a3a115..670bf7d 100644
--- a/scripts/serve.js
+++ b/scripts/serve.js
@@ -76,46 +76,64 @@ function contactRec(map, key) {
   return map[key];
 }
 const clip = (s, n) => String(s == null ? '' : s).slice(0, n);
+let _letterSeq = 0;
+const letterId = () => Date.now().toString(36) + '-' + (++_letterSeq).toString(36); // stable, monotonic
+// Serialize every read-modify-write. readContacts→mutate→writeContacts is not atomic across the
+// pair, so two interleaved requests would last-writer-win and silently drop data. This promise
+// chain makes every mutation run to completion before the next starts. (Contrarian HIGH #1.)
+let _cq = Promise.resolve();
+function withContacts(mutator) {
+  const run = _cq.then(() => { const map = readContacts(); const out = mutator(map); writeContacts(map); return out; });
+  _cq = run.then(() => {}, () => {}); // keep the chain alive even if a mutator throws
+  return run;
+}
+const jsonErr = (res) => (e) => res.status(500).json({ error: String(e && e.message || e) });
 // Whole store — front-end loads once at boot, keyed by listing id.
 app.get('/api/contacts', (req, res) => res.json({ contacts: readContacts() }));
 // Save/override the agent's contact info (name/phone/email) for a listing.
 app.post('/api/contacts/:key/info', (req, res) => {
   const key = clip(req.params.key, 120); if (!key) return res.status(400).json({ error: 'no key' });
-  const map = readContacts(); const rec = contactRec(map, key);
   const b = req.body || {};
-  if (b.name != null) rec.name = clip(b.name, 200);
-  if (b.phone != null) rec.phone = clip(b.phone, 60);
-  if (b.email != null) rec.email = clip(b.email, 200);
-  if (b.firm != null) rec.firm = clip(b.firm, 200);
-  if (b.address != null) rec.address = clip(b.address, 300);
-  rec.updated_at = new Date().toISOString();
-  writeContacts(map); res.json({ ok: true, contact: rec });
+  withContacts(map => { const rec = contactRec(map, key);
+    if (b.name != null) rec.name = clip(b.name, 200);
+    if (b.phone != null) rec.phone = clip(b.phone, 60);
+    if (b.email != null) rec.email = clip(b.email, 200);
+    if (b.firm != null) rec.firm = clip(b.firm, 200);
+    if (b.address != null) rec.address = clip(b.address, 300);
+    rec.updated_at = new Date().toISOString(); return rec;
+  }).then(rec => res.json({ ok: true, contact: rec })).catch(jsonErr(res));
 });
 // Stamp (or clear) the contacted date. Body {date} optional (ISO); defaults to now.
 app.post('/api/contacts/:key/contacted', (req, res) => {
   const key = clip(req.params.key, 120); if (!key) return res.status(400).json({ error: 'no key' });
-  const map = readContacts(); const rec = contactRec(map, key);
   const b = req.body || {};
-  rec.contacted_at = (b.clear ? null : (b.date ? clip(b.date, 40) : new Date().toISOString()));
-  writeContacts(map); res.json({ ok: true, contact: rec });
+  withContacts(map => { const rec = contactRec(map, key);
+    rec.contacted_at = (b.clear ? null : (b.date ? clip(b.date, 40) : new Date().toISOString())); return rec;
+  }).then(rec => res.json({ ok: true, contact: rec })).catch(jsonErr(res));
 });
-// Append a saved letter {subject, body, channel}. SAVE only — does not send anything.
+// Append a saved letter {subject, body, channel}. SAVE only — never sends. Each letter gets a
+// stable `id` so it can be addressed without relying on its array position (Contrarian HIGH #2).
 app.post('/api/contacts/:key/letter', (req, res) => {
   const key = clip(req.params.key, 120); if (!key) return res.status(400).json({ error: 'no key' });
   const b = req.body || {}; const body = clip(b.body, 20000);
   if (!body.trim() && !clip(b.subject, 400).trim()) return res.status(400).json({ error: 'empty letter' });
-  const map = readContacts(); const rec = contactRec(map, key);
-  const letter = { ts: new Date().toISOString(), subject: clip(b.subject, 400), body, channel: clip(b.channel || 'note', 20) };
-  rec.letters.push(letter);
-  if (!rec.contacted_at) rec.contacted_at = letter.ts;   // first saved letter counts as first contact
-  writeContacts(map); res.json({ ok: true, contact: rec, letter });
+  withContacts(map => { const rec = contactRec(map, key);
+    const letter = { id: letterId(), ts: new Date().toISOString(), subject: clip(b.subject, 400), body, channel: clip(b.channel || 'note', 20) };
+    rec.letters.push(letter);
+    if (!rec.contacted_at) rec.contacted_at = letter.ts;
+    return { rec, letter };
+  }).then(o => res.json({ ok: true, contact: o.rec, letter: o.letter })).catch(jsonErr(res));
 });
-// Delete a saved letter by index (reversible correction of a mistaken save).
+// Delete a saved letter by stable id (preferred) or legacy array index. Addressing by id removes
+// the TOCTOU race where an appended letter shifted positions and the wrong one got deleted.
 app.post('/api/contacts/:key/letter/:idx/delete', (req, res) => {
-  const key = clip(req.params.key, 120); const idx = parseInt(req.params.idx, 10);
-  const map = readContacts(); const rec = contactRec(map, key);
-  if (idx >= 0 && idx < rec.letters.length) rec.letters.splice(idx, 1);
-  writeContacts(map); res.json({ ok: true, contact: rec });
+  const key = clip(req.params.key, 120); const ref = String(req.params.idx);
+  withContacts(map => { const rec = contactRec(map, key);
+    let i = rec.letters.findIndex(l => l.id && l.id === ref);
+    if (i < 0) { const n = parseInt(ref, 10); if (String(n) === ref && n >= 0 && n < rec.letters.length) i = n; }
+    if (i >= 0) rec.letters.splice(i, 1);
+    return rec;
+  }).then(rec => res.json({ ok: true, contact: rec })).catch(jsonErr(res));
 });
 // "Send via George" — creates a Gmail DRAFT (never a live send) via the George gmail-agent on
 // this same box (127.0.0.1:9850 /api/drafts). GATED THREE WAYS: (1) it only ever creates a DRAFT,
@@ -138,13 +156,14 @@ app.post('/api/contacts/:key/send-draft', async (req, res) => {
     });
     const gj = await gr.json().catch(() => ({}));
     if (!gr.ok || !gj.success) return res.status(502).json({ error: 'george_error', detail: gj.error || ('HTTP ' + gr.status) });
-    // Attach the draft to the matching saved letter (or record a new one).
-    const map = readContacts(); const rec = contactRec(map, key);
-    let letter = rec.letters.find(l => l.body === body && !l.draft_id);
-    if (!letter) { letter = { ts: new Date().toISOString(), subject, body, channel: 'letter' }; rec.letters.push(letter); }
-    letter.status = 'gmail_draft'; letter.draft_id = gj.draftId; letter.drafted_at = new Date().toISOString(); letter.to = to;
-    if (!rec.contacted_at) rec.contacted_at = letter.drafted_at;
-    writeContacts(map);
+    // Record the drafted letter as its own id-stamped entry (no fragile body-matching).
+    const now = new Date().toISOString();
+    const rec = await withContacts(map => {
+      const r = contactRec(map, key);
+      r.letters.push({ id: letterId(), ts: now, subject, body, channel: 'letter', status: 'gmail_draft', draft_id: gj.draftId, drafted_at: now, to });
+      if (!r.contacted_at) r.contacted_at = now;
+      return r;
+    });
     res.json({ ok: true, draftId: gj.draftId, account: gj.account, contact: rec, gmailDrafts: 'https://mail.google.com/mail/u/0/#drafts' });
   } catch (e) { res.status(502).json({ error: 'george_unreachable', detail: String(e.message).split('\n')[0] }); }
 });

← 5a19595 serve.js: load project .env via native process.loadEnvFile ( · back to Commercialrealestate · Agent CRM: all-inline (no outside links) + inline Notes fiel 4d0bb9f →