Skip to content

fix(canvas): render inline markdown in table cells and headers (#2771) - #2783

Open
dolho wants to merge 1 commit into
fix/2197-body-horizontal-overflowfrom
fix/2771-table-cell-markdown
Open

dolho wants to merge 1 commit into
fix/2197-body-horizontal-overflowfrom
fix/2771-table-cell-markdown

Conversation

@dolho

@dolho dolho commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Stacked on #2782 (fix/2197-body-horizontal-overflow) → #2781#2780#2778. Part of epic #1430.

Fixes #2771

What was wrong

A table block's cells were plain text. **Deploy**, `done` and [runbook](https://example.com) — what agents actually put in status and reference columns — leaked as literal characters, while the same canvas rendered a GFM pipe table written in markdown prose correctly. Two tables, two behaviours, and the one the MCP tool guide recommends (table = {columns, rows}) was the broken one. That inconsistency is the bug.

Both structured paths are fixed at once, because both delegate to ReportTable: CanvasBlock for a table block, CanvasMarkdown for a ```table fence.

Inline-only, deliberately

marked.parseInline never emits a block element, and the cell policy drops the ones raw HTML in a cell could smuggle in. A heading or a list in a cell degrades to its own text (DOMPurify's KEEP_CONTENT) instead of breaking the row the #2583 gallery pins.

Why the fix is in two files

markdown.js cannot be imported without a DOM — DOMPurify's DOM-less stub has no addHook — and vitest runs environment: 'node'. Anything decided inside it is unreachable by a unit test.

  • utils/inlineMarkdown.js — the decidable half: the parse, the markdown-vs-text decision, the escape, the tag/attr allowlist. Pure, and executed by the spec.
  • markdown.js::renderInlineMarkdown — the sanitising half, on the same DOMPurify instance and hooks, so a cell link inherits the app-wide target="_blank" / rel="noopener noreferrer" hardening and there is no second sanitizer (H-005).

Only strings are parsed

A number, boolean or object keeps exactly its pre-#2771 rendering (JSON.stringify for an object, String() otherwise) and is escaped, not parsed — running a JSON blob through a markdown parser would let its own * and _ italicise a value nobody wrote as prose. null/undefined still render empty.

Headers render on the same terms as cells: an agent that bolds a column name and bolds the values under it should not get two behaviours.

Shared with reports, on purpose

ReportTable is the single renderer for both (ent#537 / #1535), so report tables gain the same rendering — the same defect, fixed once. ReportRenderer.vue is untouched, so the display_hint / shapeOk pins in test_1535_report_prompt_guidance.py are unaffected.

Verification

Unit (17 cases, real configured parser): bold / code / link / em / del; link hardening at the parser; no block markup for any of # heading, lists, quote, fence, pipe table; the markdown-vs-text decision for strings, numbers, booleans, objects, arrays, null, undefined, missing column and empty string; escaping including the ampersand-first rule; and the policy — inline tags only, nothing that could break a row or reach the page (div, table, img, svg, iframe, script, style, form), no on* attribute, no style/class/src, and the object asserted to be the one handed to DOMPurify, frozen.

E2E (the half a unit test structurally cannot reach): a canvas seeded through the real PUT /api/agents/{name}/canvas/{id} route, then the rendered DOM asserted — <strong>, <code>, an <a href target=_blank rel=noopener>, <em>, <del>, the header, the fenced path, no literal **/~~/[…] anywhere in the table, non-strings unchanged, a heading in a cell present as text with zero h1/ul/li/pre/table inside a cell, and the sanitizer: a <script> + <img onerror> payload leaves window.__2771 undefined with zero script/img nodes and no dialog. Plus: the canvas page does not scroll horizontally.

Negative control: with ReportTable.vue reverted the e2e fails (td strong never appears); restored, it passes.

Full frontend unit suite: 133 files, 2933 tests green (includes the raw-color ratchet and the portal report specs).

Also

The MCP set_canvas kind guide now says cells render inline markdown, so agents know the affordance exists.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ

@dolho

dolho commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Review — /review pass

No critical findings. This is the PR in the stack with a real security surface — agent-authored content reaching v-html — so I reviewed the sanitisation path specifically.

The policy is a closed allowlist, not a denylist. INLINE_SANITIZE_CONFIG sets ALLOWED_TAGS/ALLOWED_ATTR, so an element or attribute nobody thought of is dropped by default. That is the right shape (the inverse mistake — naming enemies — is what the #2323 admin-gate note in architecture.md is about), and the spec asserts the absences (script, style, img, iframe, form, on*, style, class, src) rather than only the presences.

It rides the one DOMPurify instance and its hooks, so cell links inherit the app-wide target="_blank" / rel="noopener noreferrer" hardening instead of a second policy to keep in step (H-005). Verified in the browser, not inferred: a <script> + <img onerror> payload written through the real PUT .../canvas/{id} route leaves window.__2771 undefined, with zero script/img nodes in the table and no dialog.

Only strings are parsed. Numbers, booleans and objects keep their pre-#2771 rendering and are escaped rather than fed to a markdown parser — which is the subtle one: JSON.stringify output is full of _ and *, and parsing it would italicise values nobody wrote as prose.

Block markup cannot leak into a cell: parseInlineMarkdown is asserted to emit no h1/ul/ol/li/blockquote/pre/table/div/p for headings, lists, quotes, fences and pipe tables, so a cell cannot break the row the #2583 gallery pins.

[I1] Duplicate column names mis-key cells (pre-existing, not introduced) (confidence 7/10)

cellSource resolves array rows by columns.indexOf(col) and the template keys on :key="col". A payload with two identically-named columns reads the same cell twice and Vue warns on duplicate keys. This PR inherits it rather than causing it — worth a line in the component or its own issue, not a blocker here.

[I2] The MCP guide edit has no parity test (confidence 6/10)

canvas.ts now promises agents that cells render inline markdown. Nothing pins that promise to the renderer — unlike test_1535_report_prompt_guidance.py, which pins the report guide to ReportRenderer's display hints precisely because that drift is silent. If cells ever go back to plain text, the tool guide keeps advertising markdown. A grep-level assertion in the existing e2e would cover it cheaply.

Checked and clean

  • ReportRenderer.vue untouched, so the display_hint / shapeOk pins in test_1535 are unaffected — I verified via the diff rather than claiming it.
  • Report tables get the same rendering, which is the same defect fixed once, and the PR says so rather than hiding a shared-component change.
  • Negative control: with ReportTable.vue reverted the e2e fails (td strong never appears) and passes when restored — so the gate is not vacuous.
  • Both dispatch paths are covered: the table block and the ```table fence inside a markdown block (and the fence case caught a wrong payload key while I wrote it — markdown, not text).
  • [overflow-wrap:anywhere] on cells matches CanvasProse, so a long link or code span wraps instead of widening the column.

@dolho
dolho force-pushed the fix/2197-body-horizontal-overflow branch from 336fb2f to f94b4c0 Compare September 14, 2026 14:42
@dolho
dolho force-pushed the fix/2771-table-cell-markdown branch from 7445683 to 6fb876a Compare September 14, 2026 14:42
@dolho
dolho force-pushed the fix/2197-body-horizontal-overflow branch from f94b4c0 to 14fc78f Compare September 18, 2026 08:48
@dolho
dolho force-pushed the fix/2771-table-cell-markdown branch from 6fb876a to e8a2eaa Compare September 18, 2026 08:48
@dolho
dolho marked this pull request as ready for review September 18, 2026 08:48
@dolho

dolho commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

/review — post-rebase (stacked on fix/2197-body-horizontal-overflow)

Scope: CLEAN. Plan completion vs #2771: 7 done / 1 changed (the <script>/<img onerror> strip is asserted in the @interactive e2e; the node-env unit spec proves the policy object, since markdown.js cannot be imported there — documented).

Critical

None. XSS path verified: every v-html input is renderInlineMarkdown (DOMPurify with INLINE_SANITIZE_CONFIG) or escapeText; INLINE_ATTR excludes class/style/id/on*; href under DOMPurify's default URI regexp; <style> not in INLINE_TAGS; the afterSanitizeAttributes hook sets target/rel.

Informational

Clean

inlineMarkdown.jsmarkedConfig only, markdown.jsinlineMarkdown.js one-way; ReportTable.vue adds only [overflow-wrap:anywhere]; suite 3215/3215 on the tip.

@dolho

dolho commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased the whole stack (#2778#2787) onto dev @ 0c470c76c after #2778's fix (c371b8a21). The failing Build (typecheck) + unit here was the MCP #2807 loop-audit test on a 09-18 head — it passes on current dev (439/439 on the rebased top of stack). One import-only conflict in OverflowTabs.vue (#1925 × #2917's DraftMark) resolved by keeping both. Top of stack: 3377 frontend tests, tokens, build, MCP all green.

@dolho
dolho requested a review from vybe September 21, 2026 11:37
@dolho
dolho force-pushed the fix/2197-body-horizontal-overflow branch from dc0dee1 to 69c677e Compare September 21, 2026 11:52
A `table` block's cells were plain text, so `**Deploy**`, `` `done` `` and
`[runbook](https://example.com)` — what agents routinely put in status and
reference columns — showed their literal characters. The same canvas rendered a
GFM pipe table written in `markdown` prose correctly, so an agent saw two tables
behaving differently, and the one the MCP tool guide recommends was the broken
one. Both structured paths are fixed at once: `CanvasBlock` delegates `table` to
`ReportRenderer`, and a ```table fence inside a `markdown` block delegates to the
same component.

Cells render INLINE markdown only. `marked.parseInline` never emits a block
element, and the cell policy drops the ones raw HTML could smuggle in, so a
heading or a list in a cell degrades to its own text (DOMPurify's `KEEP_CONTENT`)
rather than breaking the row the #2583 gallery pins.

The split is about testability, not taste. `markdown.js` cannot be imported
without a DOM — DOMPurify's DOM-less stub has no `addHook` — and vitest runs
`environment: 'node'`, so anything decided inside it is unreachable by a unit
test. `utils/inlineMarkdown.js` holds the decidable half (the parse, the
markdown-vs-text decision, the escape, the allowlist) and is executed by
`inlineMarkdown.spec.js`; `renderInlineMarkdown` stays in `markdown.js` beside
every other DOMPurify call, on the SAME instance and hooks, so a cell link
inherits the app-wide `target="_blank"` / `rel="noopener noreferrer"` hardening
and there is no second sanitizer (H-005).

Only strings are parsed. A number, boolean or object keeps exactly its
pre-#2771 rendering (`JSON.stringify` for an object, `String()` otherwise) and is
escaped rather than parsed — running a JSON blob through a markdown parser would
let its own `*` and `_` italicise a value nobody wrote as prose.

Headers render on the same terms as cells: an agent that bolds a column name and
bolds the values under it should not get two behaviours.

`ReportTable` is shared with reports (ent#537 / #1535), so report tables gain the
same rendering — the same defect, fixed once. `ReportRenderer.vue` is untouched,
so the `display_hint` / `shapeOk` pins in `test_1535_report_prompt_guidance.py`
are unaffected.

Verified: 17 unit cases over the real configured parser; a new e2e seeds a canvas
through the real `PUT .../canvas/{id}` route and asserts the rendered DOM — bold,
code, a hardened link, em/del, the fenced path, and the half a unit test
structurally cannot reach: a `<script>` + `<img onerror>` payload in a cell
leaves `window.__2771` undefined with zero `script`/`img` nodes in the table.
Red without the fix (the `<strong>` never appears), green with it. Full frontend
unit suite 133 files / 2933 tests green.

Fixes #2771

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui PR touches the frontend UI — triggers Playwright e2e tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant