A serial number off the bus is data, not code

src/web/assets/index_html.h

@@ -165,6 +165,13 @@ const $=s=>document.querySelector(s[0]==='#'||s[0]==='.'?s:'#'+s);
 const esc=s=>String(s??'').replace(/[<>&"']/g,c=>({'<':'&lt;','>':'&gt;','&':'&amp;','"':'&quot;',"'":'&#39;'}[c]));
 // null must never render as 0 -- that is why the firmware sends null in the first place.
 const fmt=(v,d=1)=>v===null||v===undefined?'—':Number(v).toFixed(d);
+// esc() makes a URL safe to SIT in an attribute; it does not make it safe to FOLLOW.
+// javascript: and data: survive HTML-escaping intact and run on click. Only http(s)
+// is linked; anything else renders as plain text, which is the honest failure -- the
+// reader still sees what the feed offered without the page offering to run it.
+const safeUrl=u=>{if(!u)return null;  // String(null) would resolve to a link reading "null"
+  try{const p=new URL(String(u),location.href).protocol;
+  return p==='http:'||p==='https:'?String(u):null}catch(e){return null}};
 const sp=n=>String(n).replace(/\B(?=(\d{3})+(?!\d))/g,'\u2009');
 const up=s=>s<3600?Math.floor(s/60)+' m':s<86400?(s/3600).toFixed(1)+' h':Math.floor(s/86400)+' d '+Math.round(s%86400/3600)+' h';
 const kb=b=>b===null||b===undefined?'—':Math.round(b/1024)+' kB';
@@ -187,6 +194,31 @@ function goTab(name){
 /// gets typed into the wrong section.
 function togglePanel(id){panel=panel===id?null:id;devDraft=null;paint()}
 
+// Attribute-borne DATA, never attribute-borne CODE. The browser HTML-decodes an attribute
+// value before the JS parser compiles an inline handler, so a quote that esc() turned into
+// &#39; is a quote again by the time it runs -- and a device id here is driverId + '-' +
+// the serial number the inverter reported over RS485, filtered only to printable ASCII.
+// An apostrophe in that serial used to close the string and run whatever followed, in the
+// admin's authenticated session, with the Basic-auth token sitting in sessionStorage.
+//
+// data-* values are read back with getAttribute, which yields the decoded string and never
+// executable text, so the same bytes are inert. One delegated listener on document also
+// survives every innerHTML repaint, which per-element handlers do not.
+//
+// tools/check_web_js.py fails the build if ${...} ever appears inside an on* attribute
+// again -- the shape is what was wrong, so the shape is what is banned.
+document.addEventListener('click',e=>{
+  const t=e.target.closest('[data-act]');
+  if(!t)return;
+  const a=t.getAttribute('data-act'), v=t.getAttribute('data-val')||'';
+  if(a==='panel')togglePanel(v);
+  else if(a==='tab-panel'){goTab(t.getAttribute('data-tab'));togglePanel(v)}
+  else if(a==='remove-extra')removeExtraAt(Number(v));
+  else if(a==='save-device')saveDevice(Number(v));
+  else if(a==='log-filter'){logFilter=v;loadLogs(true)}
+  else if(a==='wiz-pick')wizPick(v,JSON.parse(t.getAttribute('data-opts')||'{}'));
+});
+
 // ---------------- admin auth ----------------
 // fetch() never raises the browser's Basic-auth dialog: a 401 is just a 401. So ask once, keep
 // it for the tab only, and send the header ourselves.
@@ -485,7 +517,7 @@ function firstRunCard(here){
     <div class="hint" style="margin-top:8px">Wire A, B and ground to the inverter first, then let
       the bridge look for it. Discovery listens at each driver's own line speed and reports what
       answered; nothing is written to the inverter and nothing is changed until you confirm.</div>
-    <div class="acts"><button onclick="${here?"togglePanel('wiz')":"goTab('inv');togglePanel('wiz')"}">Find my inverter</button>
+    <div class="acts"><button data-act="${here?'panel':'tab-panel'}" data-tab="inv" data-val="wiz">Find my inverter</button>
       ${here?'':'<button class="alt" onclick="goTab(\'inv\')">Open the Inverters tab</button>'}</div>
   </div>`;
 }
@@ -738,8 +770,8 @@ function paintInverters(){
       </div>
       ${!dev.label?`<div class="hint" style="margin-top:10px">Id <code>${esc(id)}</code> — this is what the API, the MQTT topics and the Modbus unit mapping use.</div>`:''}
       <div class="acts">
-        <button class="alt sm" onclick="togglePanel('m:${esc(id)}')">${panel==='m:'+id?'Hide readings':`All ${esc(count)} reading${count===1?'':'s'}`}</button>
-        <button class="alt sm" onclick="togglePanel('s:${esc(id)}')">${panel==='s:'+id?'Close settings':'Name, driver and address'}</button>
+        <button class="alt sm" data-act="panel" data-val="m:${esc(id)}">${panel==='m:'+id?'Hide readings':`All ${esc(count)} reading${count===1?'':'s'}`}</button>
+        <button class="alt sm" data-act="panel" data-val="s:${esc(id)}">${panel==='s:'+id?'Close settings':'Name, driver and address'}</button>
       </div>
       ${panel==='m:'+id?`<div class="hair">
         <div class="hint" style="margin:0 0 10px">Everything this driver declares it can read. A channel is listed because the inverter <i>has</i> it; an em dash means it has it but is not reporting a value right now.
@@ -775,12 +807,12 @@ function paintInverters(){
       h+=`<div class="card" style="border-color:var(--warn)">
         <div class="between"><div><b>${esc(e.label||'Inverter '+(i+2))}</b>
           <span class="tag warn">starts after a restart</span></div>
-          ${panel==='x:'+i?'':`<button class="dangerAlt sm" onclick="removeExtraAt(${i})">Remove</button>`}</div>
+          ${panel==='x:'+i?'':`<button class="dangerAlt sm" data-act="remove-extra" data-val="${i}">Remove</button>`}</div>
         <div class="hint">${esc((drv&&drv.display_name)||e.driver_id||'no driver chosen')}${
           addr?' · address '+esc(addr):''} — added to the configuration, not polled yet. Correct it
           here before the restart if anything is off, or remove it.</div>
         <div class="acts">
-          <button class="alt sm" onclick="togglePanel('x:${i}')">${panel==='x:'+i?'Close settings':'Name, driver and address'}</button>
+          <button class="alt sm" data-act="panel" data-val="x:${i}">${panel==='x:'+i?'Close settings':'Name, driver and address'}</button>
         </div>
         ${panel==='x:'+i?deviceForm(i+1):''}
       </div>`;
@@ -851,10 +883,10 @@ function deviceForm(slot){
     ${drvId!==storedDrvId?'<div class="hint" style="color:var(--warn)">A different driver from the one running. Its options below start at this driver\u2019s own defaults, not the stored ones.</div>':''}`;
   h+=optionFields(drv,stored,'dv_o_');
   h+=`<div class="acts">
-      <button onclick="saveDevice(${slot})">Save</button>
+      <button data-act="save-device" data-val="${slot}">Save</button>
       <button class="alt" onclick="togglePanel(null)">Cancel</button>
       <span style="flex:1"></span>
-      ${primary?'':`<button class="dangerAlt" onclick="removeExtraAt(${slot-1})">Remove this inverter</button>`}
+      ${primary?'':`<button class="dangerAlt" data-act="remove-extra" data-val="${slot-1}">Remove this inverter</button>`}
     </div>
     <div class="hint">${primary?'This is the first inverter, which every build has. Point it at a different driver rather than removing it.'
       :'Removing it does not remove what it already published: the old entities stay in Home Assistant, available, showing their last value.'}</div>
@@ -1046,7 +1078,7 @@ function paintHealth(){
       <div style="display:flex;gap:8px;align-items:center;flex-wrap:wrap">
         <span class="hint" style="margin:0">show</span>
         ${[['all','everything'],['warn','warnings & errors'],['bus','RS485 only']].map(([k,n])=>
-          `<button class="pill${logFilter===k?'':' off'}" onclick="logFilter='${k}';loadLogs(true)">${esc(n)}</button>`).join('')}
+          `<button class="pill${logFilter===k?'':' off'}" data-act="log-filter" data-val="${k}">${esc(n)}</button>`).join('')}
         <span style="width:1px;height:18px;background:var(--line)"></span>
         <span class="hint" style="margin:0">level</span>
         <select class="tiny" onchange="setLogLevel(this.value)">${['error','warn','info','debug','trace'].map(l=>
@@ -1740,7 +1772,7 @@ function wizardHtml(){
         <tr><td class="dim">Serial number</td><td>${esc(x.serial_number||'—')}</td></tr>
         <tr><td class="dim">Evidence</td><td>${(x.evidence||[]).map(e=>'· '+esc(e)).join('<br>')||'—'}</td></tr>
         </table>
-        <div class="acts"><button onclick='wizPick(${JSON.stringify(x.driver_id)},${JSON.stringify(x.options||{})})'>Choose this device</button></div>
+        <div class="acts"><button data-act="wiz-pick" data-val="${esc(x.driver_id)}" data-opts="${esc(JSON.stringify(x.options||{}))}">Choose this device</button></div>
       </div>`;
     }
     h+=`<div class="acts"><button class="alt" onclick="wizStep=1;paintInverters()">Back</button></div>`;
@@ -2177,7 +2209,7 @@ function updRender(){
     return;
   }
   box.innerHTML=`<div class="msg ok" style="display:block"><b>${esc(updLatest.version)}</b> is available. You are running ${esc(cur)}.
-      ${updLatest.notes_url?`<a href="${esc(updLatest.notes_url)}" target="_blank" rel="noopener noreferrer">Release notes</a>`:''}</div>
+      ${safeUrl(updLatest.notes_url)?`<a href="${esc(safeUrl(updLatest.notes_url))}" target="_blank" rel="noopener noreferrer">Release notes</a>`:''}</div>
     <div class="hint">${Math.round((asset.size||0)/1024)} kB. Your browser downloads it and hands it to the bridge, which checks it against the checksum from the release before writing anything. That proves the image arrived intact — it is not a signature, and does not prove who built it.</div>
     <div class="acts"><button id="upd_btn" onclick="installUpdate()">Install ${esc(updLatest.version)}</button></div>
     <div id="upd_prog" class="prog"><div></div></div>

tools/check_dashboard_layout.py

@@ -242,7 +242,7 @@
     if(!inv.textContent.includes('starts after a restart')){
       say('a configured row that has not started is nowhere on the page');
     }
-    if(![...inv.querySelectorAll('button')].some(b=>/removeExtraAt/.test(b.getAttribute('onclick')||''))){
+    if(![...inv.querySelectorAll('button')].some(b=>b.getAttribute('data-act')==='remove-extra')){
       say('a configured row that has not started offers no way to remove it');
     }
 
@@ -267,9 +267,9 @@
       if(!pend[0].textContent.includes('Refused')){
         say('the pending card names a running inverter: "'+pend[0].querySelector('b').textContent+'"');
       }
-      const btn=pend[0].querySelector('button[onclick^="removeExtraAt"]');
-      if(!btn||btn.getAttribute('onclick')!=='removeExtraAt(1)'){
-        say('the remove button points at the wrong configuration row: '+(btn&&btn.getAttribute('onclick')));
+      const btn=pend[0].querySelector('button[data-act="remove-extra"]');
+      if(!btn||btn.getAttribute('data-val')!=='1'){
+        say('the remove button points at the wrong configuration row: '+(btn&&btn.getAttribute('data-val')));
       }
     }
 
@@ -288,7 +288,7 @@
     const pcard=[...document.querySelectorAll('#inv .card')]
       .find(c=>c.textContent.includes('starts after a restart'));
     const open=pcard&&[...pcard.querySelectorAll('button')]
-      .find(b=>/togglePanel\('x:/.test(b.getAttribute('onclick')||''));
+      .find(b=>b.getAttribute('data-act')==='panel'&&(b.getAttribute('data-val')||'').startsWith('x:'));
     if(!open) say('a pending row offers no way to correct it, only to delete it');
     else{
       open.click();
@@ -699,6 +699,67 @@ def report(label: str, verdict: str) -> int:
     return 1
 
 
+# A serial number is bytes off the RS485 bus, and the only filter on them is "printable ASCII"
+# -- which includes the apostrophe. The dashboard used to build its per-device buttons as
+# onclick="togglePanel('m:${esc(id)}')", and esc() does NOT save that: the browser HTML-decodes
+# an attribute value before compiling the inline handler, so ' is an apostrophe again by the
+# time it runs. A device reporting the serial below closed the string and ran what followed, in
+# the admin's authenticated session, where sessionStorage holds the Basic-auth token.
+#
+# BOTH halves are asserted, because either alone passes for the wrong reason: the payload must
+# not run, AND the button must still open its panel. Escaping harder satisfies the first and
+# breaks the second; removing the button satisfies both and ships a dead dashboard.
+HOSTILE_SERIAL = "x');window.__pwned=1;//"
+
+HOSTILE_SERIAL_JS = r"""
+(function(){
+const fail=[];
+const say=m=>fail.push(m);
+const done=()=>{document.title=fail.length?'LAYOUT-FAIL '+fail.join(' || '):'LAYOUT-OK'};
+let tries=0;
+// The per-device cards live on the Inverters tab, so this has to go there first -- and wait for
+// loadInverters() to have filled the caches the cards render from.
+const tick=setInterval(()=>{
+  if(typeof goTab==='function' && tab!=='inv') goTab('inv');
+  const b=document.querySelector('[data-act="panel"][data-val^="m:"]');
+  if(!b && ++tries<=120) return;
+  clearInterval(tick);
+  try{
+    if(!b){say('no readings button rendered at all');done();return}
+    if(window.__pwned){say('the payload ran while the page was rendering');done();return}
+    // The defence that does not depend on how the source was written. A shape check in
+    // check_web_js.py can always be evaded by building the same string another way; this
+    // asks the RENDERED page whether any handler attribute ended up carrying bus bytes.
+    for(const el of document.querySelectorAll('*')){
+      for(const a of el.attributes){
+        if(/^on/i.test(a.name) && a.value.indexOf("x');")>=0)
+          say('a rendered '+a.name+' attribute carries the device serial: '+a.value.slice(0,60));
+      }
+    }
+    const want=b.getAttribute('data-val');
+    // If the fixture ever stops carrying the payload this whole check is vacuous, so it says so
+    // rather than passing quietly.
+    if(want.indexOf("');")<0){say('the fixture lost its payload, so nothing was tested: '+want);done();return}
+    b.click();
+    setTimeout(()=>{
+      if(window.__pwned) say('the payload ran when the readings button was clicked');
+      if(panel!==want) say('the readings button did not open its panel: panel='+panel);
+      // The settings button carries the identical payload and was equally exploitable, so a
+      // regression reintroduced in only that path must not pass here either.
+      const sb=document.querySelector('[data-act="panel"][data-val^="s:"]');
+      if(!sb){say('no settings button rendered');done();return}
+      sb.click();
+      setTimeout(()=>{
+        if(window.__pwned) say('the payload ran when the settings button was clicked');
+        if(panel!==sb.getAttribute('data-val')) say('the settings button did not open its panel');
+        done();
+      },250);
+    },250);
+  }catch(e){say('threw: '+e.message);done()}
+},25);})();
+"""
+
+
 def main() -> int:
     stripped = build_web.served_page()
     stub = (ROOT / "tools" / "demo_fleet.js").read_text(encoding="utf-8")
@@ -770,6 +831,29 @@ def main() -> int:
         verdict, _ = render(chrome, page, 1000, scratch, "int")
         status |= report("integrations still reports what changed", verdict)
 
+        # The one device on this fleet whose id came off the bus rather than out of a config.
+        hostile = stub.replace(
+            "'eversolar_legacy-EU00T112345678'",
+            '"eversolar_legacy-' + HOSTILE_SERIAL + '"',
+        ).replace("'EU00T112345678'", '"' + HOSTILE_SERIAL + '"')
+        if "__pwned" not in hostile:
+            print(
+                "hostile serial: FAIL (the stub no longer carries the id this substitutes)"
+            )
+            status |= 1
+        else:
+            page = build_page(
+                stripped, hostile, "{soc:68,power:-1240}", HOSTILE_SERIAL_JS
+            )
+            verdict, _ = render(chrome, page, 1000, scratch, "hostile")
+            status |= report("a hostile serial number cannot run script", verdict)
+
+    # A verdict, for the same reason check_web_js.py grew one: a failing check prints its
+    # own FAIL line and then the failure detail, and every check AFTER it prints OK -- so any
+    # tail of this output reads as green. That is not hypothetical in either tool. It was
+    # read as green here on 2026-08-29, on a branch whose whole point was that a gate which
+    # cannot fail is worse than no gate.
+    print(f"RESULT: {'PASS' if status == 0 else 'FAIL'}")
     return status