fix(security): escape HTML in agent chat before markdown substitution

[notSumit25]

The problem

AgentView renders assistant replies through dangerouslySetInnerHTML, and the function feeding it escaped nothing:

function boldify(text) {
  return text
    .replace(/\*\*(.*?)\*\*/g, "<strong>$1</strong>")
    .replace(/`([^`]+)`/g, "<code>$1</code>");
}

Confirmed by executing the shipped function, not reading it:

IN : <img src=x onerror=alert(document.cookie)>
OUT: <img src=x onerror=alert(document.cookie)>     byte for byte

Corrected after hands-on QA: an earlier draft of this PR claimed **a" onmouseover="alert(1)** produced attribute injection. Tested in a browser, it does not — both builds render <strong>a" onmouseover="alert(1)</strong>, with the quotes in the element's text content, not its attribute list ([onmouseover] count is 0 on both). That example was wrong. The <img onerror> vectors below did genuinely execute, so the fix stands.

Why it mattered

Stored, not reflected. agent_conversation.transcript is a JSONB column replayed into this renderer on load, so a payload fires on every visit.

The input isn't trusted. The agent echoes database content. A real stored transcript in this install reads:

No — **DANGER**. It rewrites the whole `t` table, takes an **AccessExclusiveLock** …

where `t` is a table name read from the database — the same channel an injected identifier arrives on.

Severity capped, not removed. httpOnly cookies mean the token can't be read — but client.js sends withCredentials, so injected script acts as the reader against the API (queries, results, admin endpoints if the reader is an admin).

One nuance worth recording: <script> via innerHTML does not execute (HTML spec). The live vector is an event-handler attribute. Asserting only on <script> would prove nothing — the tests assert on onerror.

Browser proof

	Before (shipped)	After (fixed)
onerror executed	true ✗	false ✓
Real <img> elements in DOM	1	0
Rendered as	live element	escaped, visible text

imgTagsBefore: 1 is the smoking gun — Chromium parsed the payload into a real DOM element and fired its handler. After the fix the payload appears as readable text, which is the correct behaviour: a table name containing angle brackets should be legible, not executed.

The fix

export function boldify(text, codeTag = PLAIN_CODE_TAG) {
  if (!text) return "";
  return escapeHtml(text)
    .replace(/\*\*(.*?)\*\*/g, "<strong>$1</strong>")
    .replace(/`([^`]+)`/g, codeTag);
}

Order is load-bearing. Escaping after would also escape the <strong>/<code> tags this function emits, printing literal tag text. That's the obvious thing to "simplify" later, so both halves are pinned by tests.

Why not swap in the safe renderer that already exists? AgentChat/AgentMarkdown.jsx (ReactMarkdown + remarkGfm, no rehype-raw) is the right long-term answer, but it's a complete renderer with its own CSS module, DownloadableTable and link handling, while AgentView has bespoke inline styles. Swapping it in makes this a visual redesign inside a security fix. Four lines of escaping changes no pixels.

Why not escape everything? The real transcript above shows how heavily the agent uses this formatting. Rendering **DANGER** as literal asterisks would be rejected by its users.

The sibling sink

Brain/AgentArtifacts.jsx:136 had the identical bug with a styled <code> tag. Currently unreachable (AGENTS_ENABLED = false) — but dead-code-adjacent, not dead. Both renderers now share one escaping implementation with only presentation passed in, so they can't drift the way two copies would.

Verification

Step	Result
Tests vs. the real shipped boldify (RED)	6 fail, 5 pass — the 5 are formatting cases, so not vacuous
After the fix (GREEN)	12 pass
escapeHtml removed (mutation)	7 fail — tests guard the fix
Browser, old path	handler executed, 1 real <img>
Browser, fixed path	handler did not execute, 0 <img>, payload visible as text
npm run build	clean
npm run lint	41 errors on main, 41 with this change — unchanged; 0 errors in the 4 files touched
Frontend tests	22 pass, 0 fail

Residual work (deliberately not here)

• No Content-Security-Policy header. docker/nginx/default.conf sets X-Frame-Options, X-Content-Type-Options and Referrer-Policy but no CSP — so nothing stands behind an escaping bug if another is introduced. A script-src without unsafe-inline is the defence in depth this sink deserves; separate deployment change, own blast radius.
• Migrating AgentView to AgentMarkdown as a deliberate UI change.

Write-up: docs/security/2026-09-16-agent-chat-stored-xss.md

🤖 Generated with Claude Code

[notSumit25]

Hands-on QA against the live stack — 11/11 scenarios PASS, with one correction to my own claim

QA'd against the real running stack (live Postgres, live backend, patched build served on :3099 vs. the unpatched main build on :3000). The payload was planted directly into agent_conversation.transcript — which is more faithful to the threat model than typing into chat, since the real vector is a poisoned DB value the agent echoes.

⚠️ Correction: I overstated one example

The PR claims **a" onmouseover="alert(1)** produces attribute injection. It does not. Tested on both builds — each renders:

<strong>a" onmouseover="alert(1)</strong>

The quotes land in the element's text content, not its attribute list, because the ** delimiters are replaced with complete <strong>/</strong> tags leaving no open attribute position. Verified: querySelectorAll('[onmouseover]').length === 0 on both builds.

That example was wrong and I'm correcting the record rather than leaving an inflated claim. The fix remains necessary and correct — the real vectors below genuinely executed.

Browser evidence (real Chrome, payload from the live DB)

Check	UNPATCHED (main)	PATCHED (#114)
<img onerror> handler fired	true ✗	false ✓
Nested-in-bold onerror fired	true ✗	false ✓
Real <img> elements in DOM	2	0
<script> fired	false	false

<script> reporting false on both builds confirms the spec behaviour the PR documents — innerHTML-inserted <script> never executes, which is exactly why the tests assert on onerror instead.

All 11 scenarios

ID	Scenario	Result
S1	<img onerror> escaped, handler blocked	PASS
S2	Baseline executes (proves S1 not vacuous)	PASS
S3	Real formatting intact — 3 <strong>, 1 <code>	PASS
S4	Quote handling (claim corrected — not injection on either build)	PASS
S5	Payload nested inside bold	PASS
S6	<script> escaped	PASS
S7	Bullet <span> sink (AgentView.jsx:901) — 0 raw tags	PASS
S8	AgentArtifacts styled <code> — full style preserved, 0 <img>	PASS
S9	Full page reload — still escaped	PASS
S10	null/undefined/empty/whitespace — no crash	PASS
S11	Idempotent, no double-escaping (&amp;lt; absent)	PASS

S8 confirms the shared-escape design works: AgentArtifacts keeps its full inline style (background:#f3f4f6;padding:1px 5px;…) while the payload is escaped — presentation differs, escaping doesn't.

Environment

• Health confirmed via logs (Started DbaAgentApplication) and real requests (200 health / 401 unauthenticated), not docker ps.
• Verified '&amp;','<':'&lt;' is present in the patched bundle and absent from the running main build — the difference under test is real.
• DB restored to baseline: 16 rows before, 16 after, 0 scratch rows remaining.

Verdict: READY — zero blocking issues. The PR body should be amended to drop the attribute-injection example.