From a4f90e48757c18284fd04438f547fcd2362e101e Mon Sep 17 00:00:00 2001
From: Tristen Pierson <tpierson@bitconcepts.tech>
Date: Mon, 4 May 2026 09:16:33 -0400
Subject: [PATCH] security: resolve 17 CodeQL alerts (XSS /
 attribute-sanitization / insecure-randomness / shell-quote)

This is a defence-in-depth pass that closes every open CodeQL finding
on the develop branch. No user-visible behaviour changes.

media/session.js
- esc() now also escapes " and ' so it is safe to use in HTML attribute
  context, not just element text. This fixes the underlying cause of
  the five js/incomplete-html-attribute-sanitization alerts at
  lines 104, 166, 172, 218, 299.
- addU / addA: pass customTs (history-replay timestamps) through esc()
  before interpolating into innerHTML (was bare).
- addTStart: escape lbl and the args.path branch of hint (was bare).
  These were the user-controlled values flagged by js/xss at line 78.
- addImg: escape u (the data: URL / file path) before interpolating
  into <img src=...>. Closes the js/xss alert at line 298.
- vcs_state additions/deletions span: coerce to integers via
  Number(x)|0 before HTML concat; closes the js/xss alert at line 510.
- The Report Bug + View Full + Crash buttons used inline
  onclick="rptTool(this,'X','Y')" with brittle JS-string escaping
  via cleanR.replace(/'/g,"\\'") -> CodeQL flagged the regex pair as
  js/identity-replacement and the surrounding template as
  js/incomplete-html-attribute-sanitization. Replaced with
  data-action / data-tool / data-output / data-text / data-title /
  data-detail / data-repo attributes (HTML-escaped via esc()) plus
  one delegated document.addEventListener('click',...) handler that
  routes by btn.dataset.action. The brittle regex is gone; values
  cross the JS boundary as plain DOM attributes.

src/ChatStreamConsumer.ts
- generateSessionId() now uses crypto.randomUUID() instead of
  Math.random(); the 12-char hex slice is preserved so log lines
  remain readable. Closes both js/insecure-randomness alerts.

src/extension.ts (specsmith.runPreflight) and src/GovernancePanel.ts
(case 'agentTask')
- The double-quoted shell argument was previously built with
  utterance.replace(/"/g,'\\"') which leaves a trailing backslash able
  to escape the closing quote. Now backslashes are escaped first, then
  double quotes. Closes both js/incomplete-sanitization alerts.

src/test/session-logic.test.ts
- The two integrity tests that scan the bundle for stray escaped
  backticks used /<script>...<\/script>/g which CodeQL flags as
  js/bad-tag-filter because it does not match upper-case <SCRIPT>.
  Switched to /<script\b[^>]*>...<\/script\s*>/gi. We never emit
  uppercase tags, so behaviour is unchanged; the alert is closed.

Verification
- npm run lint: clean (tsc --noEmit).
- npm run build: 290.9 KB bundle.
- npm test: 144 passing.

Co-Authored-By: Oz <oz-agent@warp.dev>
---
 CHANGELOG.md                   |  2 ++
 media/session.js               | 42 ++++++++++++++++++++++++----------
 src/ChatStreamConsumer.ts      |  9 ++++----
 src/GovernancePanel.ts         |  5 +++-
 src/extension.ts               |  6 +++--
 src/test/session-logic.test.ts |  7 ++++--
 6 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 14d703c..0472253 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,8 @@
 All notable changes to the VS Code extension are documented here.
 
 ## [Unreleased]
+### Security
+- **Resolved 17 CodeQL alerts in `media/session.js`, `src/ChatStreamConsumer.ts`, `src/extension.ts`, `src/GovernancePanel.ts` and `src/test/session-logic.test.ts`.** Hardened the chat-webview HTML escaping (`esc()` now also escapes `"` and `'`), rewrote the inline `onclick="rptTool(...)"` / `onclick="rptCrash(...)"` / `onclick="viewFull(...)"` buttons to use `data-action` + a delegated click listener (eliminates the brittle `replace(/'/g,"\\'")` JS-string smuggling and the matching `js/identity-replacement` finding), escaped LLM-controlled values flowing into `addImg` `src=` and the VCS additions/deletions span, swapped `Math.random()` session-id generation for `crypto.randomUUID()`, and made the shell-quote helpers in the preflight + agent-task paths escape backslashes before quotes. Also tightened the `<script>` discovery regex in the build-output integrity test so it matches uppercase tags. No behaviour change for end users.
 ### Removed
 - **Cloud Runs sidebar tree retired.** The `specsmith.cloud` view (`Cloud Runs`) and the `CloudTree` provider have been removed. The CLI-side `specsmith cloud spawn` / `specsmith cloud-serve` commands they fronted are no longer shipped.
 ## [0.8.0] — 2026-05-01
diff --git a/media/session.js b/media/session.js
index b762888..1edc1c6 100644
--- a/media/session.js
+++ b/media/session.js
@@ -3,7 +3,7 @@ let curMdl='',busy=false,warned=false,lastU='',proposalCount=0;
 const CTX={claude:200000,'gpt-4o':128000,o1:200000,o3:200000,gemini:1000000,mistral:128000};
 function csize(m){const l=(m||'').toLowerCase();for(const[k,v]of Object.entries(CTX))if(l.includes(k))return v;return 128000}
 function ts(){return new Date().toLocaleString([],{month:'short',day:'numeric',hour:'2-digit',minute:'2-digit',second:'2-digit'})}
-function esc(s){return String(s||'').replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;')}
+function esc(s){return String(s||'').replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;').replace(/"/g,'&quot;').replace(/'/g,'&#39;')}
 function rmd(r){
   var s=esc(r);
   s=s.replace(/```(\S*)\n([\s\S]*?)```/g,function(_,_l,c){return '<pre><code>'+c+'</code></pre>';});
@@ -17,13 +17,13 @@ const C=document.getElementById('chat');
 function sb2(){C.scrollTop=C.scrollHeight}
 function addU(t,customTs){
   lastU=t;const d=document.createElement('div');d.className='mu';d.dataset.raw=t;
-  d.innerHTML=`<div class="bbl">${esc(t)}</div><div class="mt">${customTs||ts()}</div>
+  d.innerHTML=`<div class="bbl">${esc(t)}</div><div class="mt">${esc(customTs||ts())}</div>
   <div class="mact"><button class="ab" title="Copy" onclick="cp(this)">⎘</button>
   <button class="ab" title="Edit" onclick="ed(this)">✏</button></div>`;
   C.appendChild(d);sb2()}
 function addA(t,customTs){const d=document.createElement('div');d.className='ma';d.dataset.raw=t;
   d.innerHTML=`<div class="rtag">🧠 AEE Agent</div><div class="bbl">${rmd(t)}</div>
-  <div class="mt">${customTs||ts()}</div><div class="mact">
+  <div class="mt">${esc(customTs||ts())}</div><div class="mact">
   <button class="ab" title="Copy" onclick="cp(this)">⎘</button>
   <button class="ab" title="Regenerate" onclick="regen()">&#x21BA;</button></div>`;
   C.appendChild(d);sb2()}
@@ -73,9 +73,9 @@ function addTStart(n,args){
     const cmd=String(args.command);
     hint=' \u2192 '+esc(cmd.length>80?cmd.slice(0,80)+'\u2026':cmd);
   }else if(args&&args.fix==='true'){hint=' (auto-fix)';}
-  else if(args&&args.path){hint=' \u2014 '+String(args.path).split(/[\/]/).pop();}
+  else if(args&&args.path){hint=' \u2014 '+esc(String(args.path).split(/[\/]/).pop());}
   else if(args&&args.content&&n==='write_file'){hint=' \u2014 writing';}
-  d.innerHTML=`<span style="color:var(--teal)">\u23f3 ${lbl}${hint}\u2026</span><span class="mts">${ts()}</span>`;
+  d.innerHTML=`<span style="color:var(--teal)">\u23f3 ${esc(lbl)}${hint}\u2026</span><span class="mts">${esc(ts())}</span>`;
   C.appendChild(d);sb2()}
 /* REQ-132 — Warp-style command block.
  * Shows the command in a header, the output in an expandable body,
@@ -163,13 +163,16 @@ function addT(n,r,e,args){
     // Add a Report Bug button for Python-level crashes (Traceback, ImportError, etc.)
     const isPyCrash=/Traceback \(most recent call last\)|ImportError|ModuleNotFoundError|AttributeError:|TypeError: |RuntimeError:/i.test(r);
     const rptBtn=isPyCrash
-      ?`<button onclick="rptTool(this,'${esc(n)}','${esc(r.slice(0,2000))}')"
+      ?`<button data-action="rptTool" data-tool="${esc(n)}" data-output="${esc(r.slice(0,2000))}"
           style="margin-top:6px;background:var(--red);color:#fff;border:none;border-radius:3px;padding:3px 8px;cursor:pointer;font-size:10px">\uD83D\uDC1B Report Bug</button>`
       :'';
+    const vfBtn=cleanR.length>3000
+      ?`<button class="ab" style="margin-top:4px;font-size:10px;color:var(--dim)" data-action="viewFull" data-text="${esc(cleanR.slice(0,50000))}">\uD83D\uDCC4 View Full</button>`
+      :'';
     d.innerHTML=`<div class="thdr">\u274c ${esc(lbl)}</div>
       <details><summary class="tres" style="cursor:pointer;list-style:none">
         ${esc(summary)}<span style="font-size:9px;margin-left:4px;opacity:.6">(click for details)</span>
-      </summary><pre class="err-detail" style="margin-top:4px;font-size:10px">${esc(cleanR.slice(0,3000))}${cleanR.length>3000?'\n\u2026(truncated)':''}</pre>${rptBtn}${cleanR.length>3000?'<button class="ab" style="margin-top:4px;font-size:10px;color:var(--dim)" onclick="vscode.postMessage({command:\'viewFull\',text:\''+esc(cleanR.replace(/'/g,"\'").slice(0,50000))+'\'})">' + '\uD83D\uDCC4 View Full</button>':''}</details>`;
+      </summary><pre class="err-detail" style="margin-top:4px;font-size:10px">${esc(cleanR.slice(0,3000))}${cleanR.length>3000?'\n\u2026(truncated)':''}</pre>${rptBtn}${vfBtn}</details>`;
   }else if(e){
     d.innerHTML=`<div class="thdr">\u274c ${esc(lbl)}</div><div class="tres">${esc(cleanR.slice(0,200))}</div>`;
   }else{
@@ -215,9 +218,9 @@ function addToolCrash(data){
     <div style="font-size:11px;color:var(--dim);margin-bottom:8px">${esc(detail)}</div>
     <div style="font-size:11px;color:var(--dim);margin-bottom:8px">The session has stopped. This is an unexpected error — not something you did wrong.</div>
     <div style="display:flex;gap:8px;flex-wrap:wrap">
-      <button onclick="rptCrash(this,'${esc(title)}','${esc(fullDetail)}','${esc(repo)}')"
+      <button data-action="rptCrash" data-title="${esc(title)}" data-detail="${esc(fullDetail)}" data-repo="${esc(repo)}"
         style="background:var(--red);color:#fff;border:none;border-radius:4px;padding:4px 10px;cursor:pointer;font-size:11px">🐛 Report Bug</button>
-      <button onclick="this.closest('div[style]').remove()"
+      <button data-action="dismissCrash"
         style="background:none;border:1px solid var(--br);border-radius:4px;padding:4px 10px;cursor:pointer;font-size:11px;color:var(--dim)">Dismiss</button>
     </div>`;
   C.appendChild(d);sb2();}
@@ -296,7 +299,7 @@ function rptBug(btn){
   btn.textContent='✓ Reported';btn.disabled=true;}
 function addImg(u,l){const d=document.createElement('div');d.className='mu';
   d.innerHTML=`<div class="bbl"><div style="font-size:11px;color:var(--dim);margin-bottom:4px">📎 ${esc(l)}</div>
-  <img class="iprev" src="${u}" alt="${esc(l)}"></div><div class="mt">${ts()}</div>`;
+  <img class="iprev" src="${esc(u)}" alt="${esc(l)}"></div><div class="mt">${esc(ts())}</div>`;
   C.appendChild(d);sb2()}
 function updTok(i,o,c){const t=i+o,sz=csize(curMdl),p=Math.min(100,Math.round(t/sz*100));
   const f=document.getElementById('cfil');f.style.width=p+'%';
@@ -506,7 +509,7 @@ window.addEventListener('message',({data})=>{switch(data.type){
     if(vb)vb.textContent=data.branch||'\u2014';
     if(vc){const n=data.changes||0;vc.textContent=n>0?n+' change'+(n!==1?'s':''):'clean';vc.className=n>0?'vc':'';}
     if(vd){
-      const a=data.additions||0,d=data.deletions||0;
+      const a=Number(data.additions)|0,d=Number(data.deletions)|0;
       if(a||d){vd.innerHTML='<span style="color:var(--grn);font-weight:600;font-size:9px">+'+a+'</span> <span style="color:var(--red);font-weight:600;font-size:9px">-'+d+'</span>';}
       else{vd.textContent='';}
     }
@@ -582,4 +585,19 @@ window.addEventListener('message',({data})=>{switch(data.type){
     C.appendChild(pd);sb2();
     break;}
 }});
-vscode.postMessage({command:'ready'});
\ No newline at end of file
+/* Delegated click handler for data-action buttons (avoids inline onclick with user-controlled data) */
+document.addEventListener('click',function(e){
+  const btn=e.target&&e.target.closest&&e.target.closest('[data-action]');
+  if(!btn)return;
+  const action=btn.dataset.action;
+  if(action==='rptTool'){
+    rptTool(btn,btn.dataset.tool||'?',btn.dataset.output||'');
+  }else if(action==='rptCrash'){
+    rptCrash(btn,btn.dataset.title||'',btn.dataset.detail||'',btn.dataset.repo||'specsmith');
+  }else if(action==='viewFull'){
+    vscode.postMessage({command:'viewFull',text:btn.dataset.text||''});
+  }else if(action==='dismissCrash'){
+    const card=btn.closest('div[style]');if(card)card.remove();
+  }
+});
+vscode.postMessage({command:'ready'});
diff --git a/src/ChatStreamConsumer.ts b/src/ChatStreamConsumer.ts
index c7cbbdc..35b3c23 100644
--- a/src/ChatStreamConsumer.ts
+++ b/src/ChatStreamConsumer.ts
@@ -18,6 +18,7 @@
 //   4. Tests — the routing and persistence logic live in pure modules
 //      and are exercised by Mocha tests under ``src/test/``.
 import * as cp from 'child_process';
+import * as crypto from 'crypto';
 import * as path from 'path';
 import * as vscode from 'vscode';
 
@@ -49,10 +50,10 @@ let _bridgeInstalled = false;
 const _replayedProjects = new Set<string>();
 
 function generateSessionId(): string {
-  return (
-    'sess_' +
-    Array.from({ length: 12 }, () => Math.floor(Math.random() * 16).toString(16)).join('')
-  );
+  // Use a cryptographically-secure source so session ids are not predictable.
+  // randomUUID() yields a 36-char string; we keep the legacy 'sess_' prefix +
+  // 12 hex chars by stripping dashes and slicing.
+  return 'sess_' + crypto.randomUUID().replace(/-/g, '').slice(0, 12);
 }
 
 function defaultProjectDir(): string | undefined {
diff --git a/src/GovernancePanel.ts b/src/GovernancePanel.ts
index 709202d..8a5e770 100644
--- a/src/GovernancePanel.ts
+++ b/src/GovernancePanel.ts
@@ -523,9 +523,12 @@ async function _handleMsg(msg: GovMsg): Promise<void> {
       if (!task) { break; }
       const exec6 = _specsmithExec();
       const term6 = _getSpecsmithTerminal();
+      // Escape backslashes first so trailing `\` cannot escape the closing
+      // quote (CodeQL js/incomplete-sanitization).
+      const taskQuoted = task.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
       term6.sendText(
         _execCall(exec6) + ' agent ' + sub
-        + ' "' + task.replace(/"/g, '\\"') + '"'
+        + ' "' + taskQuoted + '"'
         + ' --project-dir "' + _projectDir + '"'
       );
       term6.show();
diff --git a/src/extension.ts b/src/extension.ts
index c821c62..61cf36a 100644
--- a/src/extension.ts
+++ b/src/extension.ts
@@ -623,8 +623,10 @@ export function activate(context: vscode.ExtensionContext): void {
         name: 'specsmith preflight',
         shellPath: _shellPath(),
       });
-      // Quote utterance so spaces survive the shell.
-      const quoted = `"${utterance.replace(/"/g, '\\"')}"`;
+      // Quote utterance so spaces survive the shell. Escape backslashes
+      // first so a trailing `\` cannot escape the closing quote (CodeQL
+      // js/incomplete-sanitization).
+      const quoted = `"${utterance.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
       term.sendText(`${exec} preflight ${quoted} --project-dir "${projectDir}" --json --verbose`);
       term.show();
     }),
diff --git a/src/test/session-logic.test.ts b/src/test/session-logic.test.ts
index af609ca..efbe84a 100644
--- a/src/test/session-logic.test.ts
+++ b/src/test/session-logic.test.ts
@@ -235,7 +235,10 @@ suite('Built Extension Webview Blocks', () => {
   });
 
   test('GovernancePanel script block has no escaped backticks', () => {
-    const blocks = extensionJs.match(/<script>[\s\S]*?<\/script>/g) || [];
+    // Case-insensitive flag avoids the `js/bad-tag-filter` CodeQL alert; the
+    // strings we emit are always lowercase but a defensive scanner pattern is
+    // strictly safer.
+    const blocks = extensionJs.match(/<script\b[^>]*>[\s\S]*?<\/script\s*>/gi) || [];
     for (const block of blocks) {
       if (block.includes('scanToolsNow')) {
         // This is the GovernancePanel script
@@ -246,7 +249,7 @@ suite('Built Extension Webview Blocks', () => {
   });
 
   test('SettingsPanel script block has no escaped backticks', () => {
-    const blocks = extensionJs.match(/<script>[\s\S]*?<\/script>/g) || [];
+    const blocks = extensionJs.match(/<script\b[^>]*>[\s\S]*?<\/script\s*>/gi) || [];
     for (const block of blocks) {
       if (block.includes('INST_VER') || block.includes('loadModels')) {
         const bt = (block.match(/\\`/g) || []).length;