[Maintenance] Fix: replace showdown with markdown-it to remediate CVE-2024-1899

[sumukhswamy]

Description

Showdown <= 2.1.0 is vulnerable to a ReDoS in the anchors subparser
(CVE-2024-1899) and has no upstream patch. Replace it with markdown-it,
which provides equivalent feature support for report header/footer
markdown rendering: tables, autolink (linkify), and strikethrough.

The legacy tasklists showdown option is dropped to avoid an extra
plugin dependency; checkboxes in report header/footer text now render
as plain list items. Output continues to be sanitized via DOMPurify
at every call site, and the converter API surface (converter.makeHtml)
is preserved so all four call sites remain unchanged.

Signed-off-by: Sumukh Swamy sumukhhs@amazon.com

Issues Resolved

[List any issues this PR will resolve]

Check List

• New functionality includes testing.
  • All tests pass, including unit test, integration test and doctest
• New functionality has been documented.
  • New functionality has javadoc added
  • New functionality has user manual doc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[joshuali925]

@joshuali925-osdbot 1. does the original CVE have meaningful impact to OSD? 2. is there any difference in how showdown and markdown-it handles parsing differently, that switching over would break user experience?

[joshuali925-osdbot]

1. Does CVE-2024-1899 have meaningful impact to OSD?

Low practical impact, but still worth fixing.

The CVE is a ReDoS (Regular Expression Denial of Service) in showdown's anchors subparser — CVSS 3.1 score 5.3 Medium (AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L). It requires crafted markdown input containing malicious link/anchor patterns that trigger catastrophic backtracking.

In dashboards-reporting, showdown is used in 4 places:

• Report header/footer markdown preview (report_settings.tsx) — user types markdown in a text editor, rendered as preview via converter.makeHtml()
• Report generation (generate_report.ts) — header/footer markdown converted to HTML for PDF/PNG reports
• Delivery description (delivery.tsx) — converts email body text to HTML

The markdown input comes from authenticated OSD users (report creators/editors), not from unauthenticated external sources. An attacker would need valid OSD credentials to supply malicious markdown. The impact would be client-side browser tab freeze (for the preview) or server-side report generation hang (for PDF generation via headless Chromium). It would not crash the OSD server process itself.

Bottom line: Exploitability is limited to authenticated users, and the blast radius is a single browser tab or report generation job. It's a low-risk DoS vector in practice, but showdown has no upstream fix and the library appears unmaintained, so replacing it is the right long-term move regardless.

---

2. Parsing differences between showdown and markdown-it that could affect UX

Both libraries support the features used here (tables, autolinks, strikethrough), but there are some behavioral differences:

Feature	Showdown	markdown-it (this PR)
Auto-linking	simplifiedAutoLink: true — bare URLs become links	linkify: true — same behavior
Tables	GFM-style tables	GFM-style tables (built-in)
Strikethrough	~~text~~ → <del>	Built-in, enabled by default — same behavior
Task lists	- [x] item → checkbox HTML	Dropped — renders as plain list items
Header IDs	noHeaderId: true suppresses auto IDs	Does not generate header IDs by default — equivalent
Inline HTML	Allowed by default	html: false — raw HTML in markdown is now stripped
Line breaks	Requires double-newline for paragraph break	breaks: false — same behavior

Potential UX differences:

1. Task lists removed — Users who put - [x] Done / - [ ] Todo in report headers/footers will now see plain bullet items instead of checkboxes. The PR description acknowledges this tradeoff.

2. Inline HTML disabled — If users previously embedded raw HTML in their report header/footer markdown (e.g., <span style="color:red">text</span>), it will now be stripped at the markdown-it level. However, DOMPurify was already sanitizing the output at every call site, so dangerous HTML would have been stripped anyway.

3. Minor rendering edge cases — markdown-it follows the CommonMark spec more strictly than showdown (which follows the original Markdown.pl behavior). Edge cases in nested lists, blockquotes, and emphasis parsing may render slightly differently, but for typical report header/footer content (headings, bold, links, tables) the output should be identical.

Overall risk: Low. The typical use case is simple markdown in report headers/footers. The task list removal is the only clearly visible behavioral change, and it's a rarely-used feature in this context.

[joshuali925]

@joshuali925-osdbot can you start the OSD before and after this PR, test the markdown rendering in reporting plugin with commonly used and potentially breaking syntax, and post the comparison screenshots?

[joshuali925-osdbot]

Markdown Rendering Comparison: Showdown (before) vs markdown-it (after)

Tested with the following markdown in the report header preview:

# Heading 1
## Heading 2
### Heading 3

**Bold text** and *italic text* and ~~strikethrough~~

A paragraph with a [link](https://example.com) and an auto-link: https://opensearch.org

> This is a blockquote
> with multiple lines

- Unordered item 1
- Unordered item 2
  - Nested item

1. Ordered item 1
2. Ordered item 2

- [x] Task completed
- [ ] Task pending

| Column 1 | Column 2 | Column 3 |
|-----------|----------|----------|
| Cell 1    | Cell 2   | Cell 3   |
| Cell 4    | Cell 5   | Cell 6   |

Inline `code` and a code block:

function hello() {
return "world";
}

---

<b>HTML bold</b> and <em>HTML italic</em>

Before (showdown on main branch)

After (markdown-it on PR branch)

Differences Found

Feature	Before (showdown)	After (markdown-it)	Impact
Strikethrough	<del> tag	<s> tag	✅ No visual difference
Headings	✅ Works	✅ Works	None
Bold/Italic	✅ Works	✅ Works	None
Links	✅ Works	✅ Works	None
Auto-links	✅ Works	✅ Works	None
Blockquotes	✅ Works	✅ Works	None
Unordered lists	Flat (nested items not indented)	Properly nested <ul> inside <li>	✅ Improvement
Ordered lists	✅ Works	✅ Works	None
Task lists	Rendered as checkboxes (<input type="checkbox">)	Rendered as plain text [x] / [ ]	⚠️ Behavioral change (acknowledged in PR)
Tables	✅ Works	✅ Works	None
Inline code	✅ Works	✅ Works	None
Code blocks	✅ Works	✅ Works	None
Horizontal rule	✅ Works	✅ Works	None
Inline HTML	Rendered as HTML (bold/italic applied)	Escaped as text (<b> shown literally)	⚠️ Behavioral change (html: false in config)

Summary

The rendering is functionally equivalent for all commonly used markdown syntax. Two behavioral differences:

1. Task lists — now render as plain bullet items with [x]/[ ] text instead of interactive checkboxes. Low impact since task lists are rarely used in report headers/footers.

2. Inline HTML — raw HTML tags are now escaped instead of rendered. This is actually a security improvement (defense-in-depth alongside DOMPurify), though users who relied on <b>, <span style="..."> etc. in their report headers will see the raw tags displayed as text.

3. Nested lists — markdown-it correctly nests sub-items inside parent <li> elements, which is actually an improvement over showdown's flat rendering.

Overall the PR is safe to merge for typical report header/footer usage.

[joshuali925]

@joshuali925-osdbot compare with OUI/EUI, create a PR to use EuiMarkdownFormat instead if it is less breaking

[joshuali925-osdbot]

Investigated EuiMarkdownFormat as an alternative. It would be more breaking than markdown-it, so I don't recommend switching.

EuiMarkdownFormat Analysis

EuiMarkdownFormat (from @elastic/eui) is a React component that uses unified + remark-parse (CommonMark) internally. It renders markdown as React elements, not HTML strings.

Feature Comparison

Feature	showdown (current)	markdown-it (PR)	EuiMarkdownFormat
Tables	✅	✅	❌ No GFM support
Strikethrough	✅	✅	❌ No GFM support
Auto-links (linkify)	✅	✅	❌ No linkify plugin
Task lists / Checkboxes	✅	⚠️ Plain text	✅ Custom plugin
Inline HTML	✅	❌ Escaped	✅ allowDangerousHtml
Emoji	❌	❌	✅ remark-emoji
Code highlighting	❌	❌	✅ PrismJS
Tooltips	❌	❌	✅ Custom plugin

Architectural Issues with EuiMarkdownFormat

1. Output format mismatch — EuiMarkdownFormat returns React elements, but 2 of 4 call sites need HTML strings:

   • generate_report.ts injects HTML via innerHTML for PDF/PNG report generation
   • delivery.tsx sends htmlDescription to the backend for notification emails
   • Would require wrapping with ReactDOMServer.renderToStaticMarkup() at those sites

2. Missing GFM extensions — The bundled remark-parse (v8) is CommonMark only. No remark-gfm is available in the OSD dependency tree, so tables, strikethrough, and autolinks would all break.

3. No remark-gfm available — Adding it would mean a new dependency, and the remark ecosystem version (v8/v9 in OUI) predates the unified remark-gfm plugin (which targets remark v13+), making it incompatible.

Verdict

markdown-it is the better choice for this use case:

• Supports all GFM features out of the box (tables, strikethrough, linkify)
• Produces HTML strings directly (matches existing architecture)
• Zero new dependencies beyond the single markdown-it package
• Only regressions vs showdown are task lists (minor) and inline HTML escaping (security improvement)

EuiMarkdownFormat is designed for interactive editor/preview UIs within OSD (like the query assist summary), not for generating HTML strings for report injection. Using it here would lose table/strikethrough/autolink support and add architectural complexity.

[ps48]

Added some minor comments to change few things.

[ps48]

The converter is defined identically in public/components/report_definitions/utils/index.ts and public/components/visual_report/constants.ts. Consider consolidating into a single shared module (e.g., common/utils/markdown.ts) to avoid drift. Not blocking for a security fix.

[ps48]

Suggestion: Change html: false to html: true

Showdown rendered inline HTML by default, so html: false is a user-facing regression anyone using <b>, <br>, <span style="..."> etc. in report headers/footers will see raw tags displayed as text. This is safe because:

• All DOM-rendering paths already wrap output in DOMPurify.sanitize() (generate_report.ts:126,129 and report_settings.tsx:363,386)
• The delivery path sends HTML to a notification channel, not rendered in-browser
• The CVE is in showdown's regex parser, not in HTML pass-through html: true doesn't reintroduce the vulnerability

[joshuali925-osdbot]

Thanks for the review @ps48! Both suggestions make sense — I've pushed a fix:

Changes (PR to author's branch):

1. Consolidated converter — extracted into public/components/utils/markdown.ts, both original locations now re-export from it.
2. html: true — inline HTML passes through since DOMPurify already sanitizes at every call site (generate_report.ts:126,129 and report_settings.tsx:363,386). No regression for users with <b>, <br>, <span style="..."> in headers/footers.
3. Unit tests — 6 tests covering tables, strikethrough, linkify, HTML pass-through, and null input.

@sumukhswamy please merge sumukhswamy#1 into your branch to pick up these changes.