Harden ultracode inline onclick handlers against XSS#132
Merged
Conversation
The ultracode run/agent cards and minimized-tab badges built inline onclick
handlers by interpolating escapeHtml(value) inside single-quoted JavaScript
strings within an HTML attribute:
onclick="app.openUltracodeAgentWindow('${escapeHtml(agentId)}', ...)"
escapeHtml maps ' -> &Ark0N#39;, but the browser HTML-decodes the attribute value
before the handler source is parsed, so &Ark0N#39; becomes a literal ' again and a
quote in a run/agent/session id breaks out of the string literal into
executable JS. escapeHtml alone is insufficient for the JS-string-within-HTML-
attribute double context.
Switch each handler to escapeHtml(JSON.stringify(value)): JSON.stringify
JS-encodes and quote-wraps the value, then escapeHtml handles the HTML
attribute layer, so the value round-trips as an inert string argument. This
matches the encoding already used by other handlers in these files.
Affected:
- ultracode-panel.js: selectWorkflowRun, openUltracodeAgentWindow
- ultracode-windows.js: restore/dismiss for minimized run and agent tabs
Ark0N
pushed a commit
that referenced
this pull request
Jun 19, 2026
…ouble-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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The ultracode run/agent cards and minimized-tab badges build inline
onclickhandlers by interpolatingescapeHtml(value)inside single-quoted JavaScript strings within an HTML attribute, e.g.:escapeHtmlmaps'→', but the browser HTML-decodes the attribute value before the handler source is parsed, so'becomes a literal'again and a quote in a run/agent/session id breaks out of the string literal into executable JS.escapeHtmlalone is insufficient for the JS-string-within-HTML-attribute double context.Fix
Switch each handler to
escapeHtml(JSON.stringify(value)):JSON.stringifyJS-encodes and quote-wraps the value first, thenescapeHtmlhandles the HTML-attribute layer, so the value round-trips as an inert string argument. This matches the encoding convention already used by other handlers in these files.Affected handlers (6):
ultracode-panel.js—selectWorkflowRun,openUltracodeAgentWindowultracode-windows.js—restoreUltracodeRunFromTab,dismissMinimizedUltracodeRun,restoreUltracodeAgentFromTab,dismissMinimizedUltracodeAgentVerification
escapeHtmlinline handler remains.node --checkparses both files cleanly.');window.__PWNED=1;(') parses as a separate executable statement under the old pattern, but round-trips as a single inert string argument under the fix — nothing executes.