From 343f1edd57898f379045df67c74be1eda2fe2607 Mon Sep 17 00:00:00 2001 From: martbost Date: Fri, 28 Aug 2026 16:01:53 -0500 Subject: [PATCH] Beta-tester safety: preview caps on the level override, and close a race in stop() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things needed before testers get level overrides. 1. The override decided what a position could SPEND as well as what it could SEE. A tester at Scintilla overridden to Corona would have had 150,000 real network impressions instead of 2,500, and a 200,000-impression grant pool to hand to real members — real inventory, spent for real. Ad allowances and split-test launches are now metered on trueLevel, and grant pools are capped to small preview amounts (5,000 impressions / 25 copy / 2 pages / 5 ad batches) when a position is overridden: enough to exercise the whole flow, trivial to lose. 2. suite-traffic.stop() read the ledger, made TWO network round trips to the ad network, then wrote back the object it had read seconds earlier — silently discarding any campaign another member launched in that window, with the impressions already spent on the network. It now re-reads after the awaits and mutates the fresh copy. Worth recording that this was the ONLY such race: Node's single thread makes a synchronous read-modify-write atomic, so the meter, grants and split ledgers were never at risk. The danger was only ever the await gap. Co-Authored-By: Claude Fable 5 --- server.js | 26 ++++++++++++++++---------- suite-grants.js | 20 ++++++++++++++------ suite-traffic.js | 23 +++++++++++++++++++---- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/server.js b/server.js index 84ea477..68ba1c1 100644 --- a/server.js +++ b/server.js @@ -692,6 +692,12 @@ async function handleApi(req,res,pathname){ return {d:dd,inOrg,beta,allowed}; } + // A level override decides what a position can SEE. It must never decide what + // a position can SPEND: ad impressions and grant pools are real inventory on + // a real network. So anything that costs something is metered on trueLevel. + function spendLevel(e){ return (e.d && e.d.trueLevel != null) ? e.d.trueLevel : (e.d ? e.d.level : 1); } + function isPreview(e){ return !!(e.d && e.d.levelOverridden); } + // ── Page Builder ───────────────────────────────────────────────────────── // Level 6 extra pages: /p//. Falls back to the member's main page // rather than 404ing, for the same reason /p/ falls back to /join/. @@ -817,7 +823,7 @@ async function handleApi(req,res,pathname){ const e=await suiteEntitlement(req).catch(()=>({error:'Chain read hiccup.',code:500})); if(e.error)return json(res,e.code||500,{error:e.error}); if(!e.inOrg||!e.allowed)return json(res,403,{error:'Not available for this position yet.'}); - const st=suiteTraffic.status(e.d.id,e.d.level); + const st=suiteTraffic.status(e.d.id,spendLevel(e)); let live=[]; try{ live=await suiteTraffic.stats(st.campaigns.map(c=>c.adId)); }catch(err){} // Tell the UI whether a personal page actually exists, so it can't be @@ -836,14 +842,14 @@ async function handleApi(req,res,pathname){ try{ const pgRec=suitePages.load(e.d.id); const entry=await suiteTraffic.launch({ - id:e.d.id, level:e.d.level, size:b.size, creative:b.creative, + id:e.d.id, level:spendLevel(e), size:b.size, creative:b.creative, kind:b.kind==='text'?'text':'banner', subject:b.subject, lines:b.lines, impressions:b.impressions, target:b.target, angle:b.angle, hasPage:!!(pgRec&&pgRec.copy), name:String(b.name||'').slice(0,60) }); - return json(res,200,{campaign:entry,status:suiteTraffic.status(e.d.id,e.d.level)}); + return json(res,200,{campaign:entry,status:suiteTraffic.status(e.d.id,spendLevel(e))}); }catch(err){ return json(res,400,{error:String(err.message||err)}); } } @@ -890,7 +896,7 @@ async function handleApi(req,res,pathname){ let r=[]; try{ r=await suiteSplit.results(e.d.id); }catch(err){} const pg=suitePages.load(e.d.id); return json(res,200,{tests:r,level:e.d.level,id:e.d.id, - traffic:suiteTraffic.status(e.d.id,e.d.level),hasPage:!!(pg&&pg.copy), + traffic:suiteTraffic.status(e.d.id,spendLevel(e)),hasPage:!!(pg&&pg.copy), minPerArm:suiteSplit.MIN_PER_ARM,maxArms:suiteSplit.MAX_ARMS, textAngles:suiteTextAds.angles(), textMeter:suiteMeter.check(e.d.id,e.d.level,'textad')}); @@ -903,9 +909,9 @@ async function handleApi(req,res,pathname){ } const pg=suitePages.load(e.d.id); try{ - const t=await suiteSplit.launch({id:e.d.id,level:e.d.level,arms:b.arms, + const t=await suiteSplit.launch({id:e.d.id,level:spendLevel(e),arms:b.arms, impressions:b.impressions,target:b.target,angle:b.angle,hasPage:!!(pg&&pg.copy)}); - return json(res,200,{test:t,traffic:suiteTraffic.status(e.d.id,e.d.level)}); + return json(res,200,{test:t,traffic:suiteTraffic.status(e.d.id,spendLevel(e))}); }catch(err){ return json(res,400,{error:String(err.message||err)}); } } } @@ -1031,7 +1037,7 @@ async function handleApi(req,res,pathname){ if(req.method==='GET'){ // Recipients see what they were given even below level 7 - that is the // half of the feature that makes being helped visible. - return json(res,200,{status:suiteGrants.status(e.d.id,e.d.level),tools:suiteGrants.tools(), + return json(res,200,{status:suiteGrants.status(e.d.id,e.d.level,isPreview(e)),tools:suiteGrants.tools(), minLevel:suiteGrants.MIN_LEVEL,level:e.d.level,id:e.d.id}); } if(req.method==='POST'){ @@ -1044,8 +1050,8 @@ async function handleApi(req,res,pathname){ try{ inTeam=chain.isInTeam(to,Number(e.d.id)); }catch(err){} if(!inTeam)return json(res,400,{error:'Position #'+to+' is not in your organisation. You can only grant to people below you.'}); try{ - const rec=suiteGrants.grant({by:e.d.id,to:to,tool:String(b.tool||''),n:b.n,level:e.d.level}); - return json(res,200,{granted:rec,status:suiteGrants.status(e.d.id,e.d.level)}); + const rec=suiteGrants.grant({by:e.d.id,to:to,tool:String(b.tool||''),n:b.n,level:e.d.level,preview:isPreview(e)}); + return json(res,200,{granted:rec,status:suiteGrants.status(e.d.id,e.d.level,isPreview(e))}); }catch(err){ return json(res,400,{error:String(err.message||err)}); } } } @@ -1101,7 +1107,7 @@ async function handleApi(req,res,pathname){ const b=await bodyJson(req)||{}; try{ const r=await suiteTraffic.stop(e.d.id,Number(b.ad_id)); - return json(res,200,{stopped:r,status:suiteTraffic.status(e.d.id,e.d.level)}); + return json(res,200,{stopped:r,status:suiteTraffic.status(e.d.id,spendLevel(e))}); }catch(err){ return json(res,400,{error:String(err.message||err)}); } } diff --git a/suite-grants.js b/suite-grants.js index 03ecd97..f06b1d4 100644 --- a/suite-grants.js +++ b/suite-grants.js @@ -47,11 +47,19 @@ function tools() { }); } -function poolFor(tool, level) { +// A tester on a level override can SEE every tier, but must not be able to +// hand out a real Corona pool — 200,000 network impressions given to real +// members is real inventory, spent for real. Preview pools are big enough to +// exercise the whole flow and small enough that losing them costs nothing. +const PREVIEW_POOL = { traffic: 5000, copy: 25, page: 2, textad: 5 }; + +function poolFor(tool, level, preview) { const t = POOLS[tool]; if (!t) return 0; const lv = Math.max(1, Math.min(8, Number(level) || 1)); - return t.pool[lv - 1] || 0; + const base = t.pool[lv - 1] || 0; + if (preview) return Math.min(base, PREVIEW_POOL[tool] || 0); + return base; } function rows(mk) { @@ -86,10 +94,10 @@ function receivedDetail(memberId) { return out; } -function status(memberId, level) { +function status(memberId, level, preview) { const t = {}; Object.keys(POOLS).forEach(function (k) { - const pool = poolFor(k, level); + const pool = poolFor(k, level, preview); const given = givenBy(memberId, k); t[k] = { label: POOLS[k].label, unit: POOLS[k].unit, step: POOLS[k].step, max: POOLS[k].max, @@ -97,7 +105,7 @@ function status(memberId, level) { }; }); return { - level: Number(level) || 0, canGrant: Number(level) >= MIN_LEVEL, + level: Number(level) || 0, canGrant: Number(level) >= MIN_LEVEL, preview: !!preview, tools: t, resets: monthKey(), history: rows().filter(function (r) { return Number(r.by) === Number(memberId); }).slice(-40).reverse(), received: receivedDetail(memberId) @@ -119,7 +127,7 @@ function grant(opts) { if (n <= 0) throw new Error('Choose how much to give.'); if (n > POOLS[tool].max) throw new Error('The most you can give one person at a time is ' + POOLS[tool].max.toLocaleString() + '.'); - const pool = poolFor(tool, level); + const pool = poolFor(tool, level, opts.preview); const given = givenBy(by, tool); if (given + n > pool) { throw new Error('That is more than you have left to give this month — ' + diff --git a/suite-traffic.js b/suite-traffic.js index 125cc75..18ee804 100644 --- a/suite-traffic.js +++ b/suite-traffic.js @@ -189,9 +189,9 @@ async function stop(memberId, adId) { const all = readLedger(); const mk = monthKey(); const rows = ((all[mk] || {})[String(memberId)]) || []; - const row = rows.find(function (r) { return Number(r.adId) === Number(adId); }); - if (!row) throw new Error('That banner is not one of yours from this month.'); - if (row.stopped) throw new Error('That banner is already stopped.'); + const pre = rows.find(function (r) { return Number(r.adId) === Number(adId); }); + if (!pre) throw new Error('That banner is not one of yours from this month.'); + if (pre.stopped) throw new Error('That banner is already stopped.'); let served = 0, unserved = 0; try { @@ -205,6 +205,21 @@ async function stop(memberId, adId) { await callNas({ action: 'deactivate', ad_id: Number(adId) }); + // RE-READ before writing. Everything above this point took two network round + // trips to the ad network, and the ledger object read at the top of this + // function is now seconds stale. Writing it back would silently discard any + // campaign another member launched in the meantime — impressions spent on the + // network with no record that they were. Node's single thread makes the block + // below atomic; the danger was only ever the await gap. + const fresh = readLedger(); + const freshRows = ((fresh[mk] || {})[String(memberId)]) || []; + const row2 = freshRows.find(function (r) { return Number(r.adId) === Number(adId); }); + if (!row2) throw new Error('That banner is no longer in this month\'s ledger.'); + if (row2.stopped) return { adId: Number(adId), served: row2.served || 0, refunded: row2.refunded || 0 }; + + const all2 = fresh; + const row = row2; + // Charge only what actually served; the rest returns to the allowance. // `bought` preserves the original order size so the member's history still // shows what they launched, not just what it ended up costing them. @@ -220,7 +235,7 @@ async function stop(memberId, adId) { row.served = served; row.refunded = unserved; row.impressions = served; // what this campaign counts against the month - writeLedger(all); + writeLedger(all2); return { adId: Number(adId), served: served, refunded: unserved }; }