fix(security): harden remaining inline onclick handlers against XSS double-context

Extends PR #132 (ultracode handlers) to the rest of the frontend. The same
JS-string-in-HTML-attribute pattern — '${escapeHtml(value)}' — remained in 32
more inline handlers across app.js, panels-ui.js, session-ui.js,
subagent-windows.js, and notification-manager.js. The browser HTML-decodes the
attribute value before parsing the handler source, so escapeHtml's ' reverts
to ' and a quote-bearing id/path/name breaks out of the JS string literal into
executable code.

Switch all to escapeHtml(JSON.stringify(value)): JSON.stringify JS-encodes and
quote-wraps first, then escapeHtml handles the HTML-attribute layer, so the
value round-trips as one inert string argument.

Also fixes two non-escapeHtml variants of the same class:
- panels-ui.js: mux-session `sid` was pre-escaped with escapeHtml() then dropped
  into a single-quoted JS string (selectSession / killMuxSession). Now
  JSON.stringify'd at the source.
- orchestrator-panel.js: phase.id was interpolated raw (no escaping at all) into
  orchestratorSkipPhase / orchestratorRetryPhase. Now escapeHtml(JSON.stringify()).

The most realistic vector here is file paths (panels-ui openLogViewerWindow) —
filenames can legally contain a single quote.

Numeric interpolations (${i+1}, ${index}, ${item.version}) and the
developer-literal ${onclick} in orchestrator-panel are not user data and are
left as-is. Verified: 0 vulnerable patterns remain, all 22 frontend files parse
(check:frontend-syntax + node --check), and a runtime round-trip confirms the
injection that fired under the old pattern is now an inert string argument.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-06-19 15:26:42 +02:00
parent d8da1bd3ff
commit a8782b364f
6 changed files with 33 additions and 33 deletions
+4 -4
View File
@@ -33,10 +33,10 @@ Object.assign(CodemanApp.prototype, {
const truncatedName = displayName.length > 25 ? displayName.substring(0, 25) + '…' : displayName;
const statusClass = agent?.status || 'idle';
agentItems.push(`
<div class="subagent-dropdown-item" onclick="event.stopPropagation(); app.restoreMinimizedSubagent('${escapeHtml(agentId)}', '${escapeHtml(sessionId)}')" title="Click to restore">
<div class="subagent-dropdown-item" onclick="event.stopPropagation(); app.restoreMinimizedSubagent(${escapeHtml(JSON.stringify(agentId))}, ${escapeHtml(JSON.stringify(sessionId))})" title="Click to restore">
<span class="subagent-dropdown-status ${statusClass}"></span>
<span class="subagent-dropdown-name">${escapeHtml(truncatedName)}</span>
<span class="subagent-dropdown-close" onclick="event.stopPropagation(); app.permanentlyCloseMinimizedSubagent('${escapeHtml(agentId)}', '${escapeHtml(sessionId)}')" title="Dismiss">&times;</span>
<span class="subagent-dropdown-close" onclick="event.stopPropagation(); app.permanentlyCloseMinimizedSubagent(${escapeHtml(JSON.stringify(agentId))}, ${escapeHtml(JSON.stringify(sessionId))})" title="Dismiss">&times;</span>
</div>
`);
}
@@ -699,7 +699,7 @@ Object.assign(CodemanApp.prototype, {
parentSessionId && parentSessionName
? `<div class="subagent-window-parent" data-parent-session="${parentSessionId}">
<span class="parent-label">from</span>
<span class="parent-name" onclick="app.selectSession('${escapeHtml(parentSessionId)}')">${escapeHtml(parentSessionName)}</span>
<span class="parent-name" onclick="app.selectSession(${escapeHtml(JSON.stringify(parentSessionId))})">${escapeHtml(parentSessionName)}</span>
</div>`
: '';
@@ -720,7 +720,7 @@ Object.assign(CodemanApp.prototype, {
<span class="status ${agent.status}">${agent.status}</span>
</div>
<div class="subagent-window-actions">
<button onclick="app.closeSubagentWindow('${escapeHtml(agentId)}')" title="Minimize to tab">─</button>
<button onclick="app.closeSubagentWindow(${escapeHtml(JSON.stringify(agentId))})" title="Minimize to tab">─</button>
</div>
</div>
${parentHeader}