fix(security): escape entry.id in HTML export to prevent attribute XSS#83104
Conversation
|
Codex review: needs maintainer review before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. by source inspection: current main writes raw PR rating What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the narrow escaping fix and regression coverage after maintainer security review and required CI are green or explicitly accepted. Do we have a high-confidence way to reproduce the issue? Yes by source inspection: current main writes raw Is this the best way to solve the issue? Yes. Reusing the existing Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 7cda26aa6c72. |
…aw#83104) Signed-off-by: Sebastien Tardif <[email protected]>
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Gilded Crabkin Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
b4f5ce3 to
7c33eb4
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
7c33eb4 to
42574d8
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
Verification before merge: pnpm test src/auto-reply/reply/export-html/template.security.test.ts (10 passed); autoreview clean. Scope: HTML export template escapes entry IDs in attributes while preserving decoded DOM lookup behavior. |
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
16826 drift commits, 150 flagged (security/regression keywords). Notable: XSS fix (openclaw#83104), security floor fixes, session-visibility glob fix. Report: _tagai/monitoring/UPSTREAM_2026-05-25.md
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
openclaw#83104) * fix(security): escape entry.id in HTML export to prevent attribute XSS Apply escapeHtmlAttr to entry.id in renderEntry and renderCopyLinkButton to prevent attribute injection via crafted entry IDs in HTML exports. Signed-off-by: Sebastien Tardif <[email protected]> * chore: remove proof helper scripts from branch ClawSweeper P2: committed proof scripts can provide false-positive validation. Proof output is in the PR body instead. Signed-off-by: Sebastien Tardif <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]>
26194 drift commits, 222 flagged (security/regression keywords). Notable: XSS fix (openclaw#83104), PATH injection (openclaw#73264), npm_execpath injection (openclaw#73262), implicit tool grant fix (openclaw#75055), payment credential redaction (openclaw#75230), heartbeat regression (openclaw#88970).
Problem
template.jsin the HTML export feature interpolatesentry.iddirectly into HTML attribute strings without escaping:If an
entry.idcontains double-quote characters (e.g., a crafted conversation export or corrupted data), the quote closes thedata-entry-idattribute and the remainder is parsed as new HTML attributes. An attacker-controlledentry.idofx" onmouseover="alert(document.cookie)injects anonmouseoverevent handler into the DOM.This is a stored XSS in the HTML export: the payload persists in the exported HTML file and fires whenever a user interacts with the affected element.
Fix
Wrap both
entry.idinterpolation sites with the existingescapeHtmlAttr()function (already defined intemplate.jsfor other attributes):escapeHtmlAttrreplaces",&,<,>, and'with their HTML entity equivalents (",&,<,>,'). This prevents quote characters from closing the attribute and injecting new HTML attributes.Production diff is 2 lines changed in
template.js, plus 389 lines of security test coverage.Real behavior proof
Behavior addressed:
entry.idwas interpolated raw into HTML attributes in the export template. A crafted entry ID with double quotes broke out of thedata-entry-idattribute and injected arbitrary HTML attributes (XSS).Real environment tested: Linux (Ubuntu 24.04, WSL2), Node.js v22.16.0, Headless Chromium 148.0.7778.96 via Playwright 1.60.0, OpenClaw built from source at
/tmp/fix-83104.Exact steps or command run after this patch:
Evidence after fix: Chromium browser output copied below.
Chromium red-green proof: XSS payload in entry.id
Generated a full 247KB HTML export with a malicious
entry.idofx" onmouseover="alert(document.cookie)". Opened it in headless Chromium 148 via Playwright, queried the DOM for XSS breakout, and hovered elements to trigger any injected event handlers.AFTER fix (with
escapeHtmlAttr) -- Chromium HeadlessChrome/148.0.7778.96:The
"entities indata-entry-id="x" onmouseover="alert(document.cookie)"keep the entire payload inside the attribute value. Chromium's DOM parser does not create anonmouseoverattribute. Hovering all rendered elements triggers no alert dialog. Thedataset.entryIdAPI round-trips the value correctly (browser decodes"back to"as data, not as HTML structure).BEFORE fix (escapeHtmlAttr reverted) -- same Chromium:
Without
escapeHtmlAttr, the raw double-quote inentry.idcloses thedata-entry-idattribute. Chromium parsesonmouseover="alert(document.cookie)"as a real DOM attribute.querySelectorAll("[onmouseover]")finds 1 element. Hovering that element triggersalert(document.cookie)-- XSS confirmed in a real browser.Summary table
[onmouseover]elementsdata-entry-idescaping"entitiesalert()scriptsCompiled code verification
escapeHtmlAttris present in the compiled production bundle:Live gateway startup proof
Vitest security test suite (10 tests, supplemental)
Observed result after fix: Headless Chromium 148 confirms the
escapeHtmlAttr()fix prevents XSS breakout. With the fix, 0 elements haveonmouseoverhandlers and no alert dialog fires on hover. Without the fix, 1 element has anonmouseoverhandler and hovering it triggersalert(document.cookie)in the real Chromium DOM.What was not tested: Opening the export in Firefox (tested in Chromium only). The HTML attribute escaping is standardized across browsers; Chromium's DOM parser is the reference implementation.