Repository navigation
ref(cli): Escape Markdown with the shared toolkit-core builders - #1805
Merged
Merged
Conversation
The CLI's escapeMarkdownInline, escapeMarkdownCell, and safeCodeSpan are now toolkit-core's text, tableCell (new), and inlineCode, so the CLI and MCP escape untrusted values the same way, verified by property tests against the same marked version the CLI renders with. Behavior changes in CLI output: - Cells escape emphasis, links, and the rest of inline syntax, not only pipes and angle brackets. - Code spans keep backticks and pipes instead of replacing them with look-alike characters. - Underscores inside words stay unescaped. - URLs in escaped text render as plain text rather than autolinks. escapeMarkdownLines keeps line breaks for the multi-line values (blockquoted messages, fix instructions, credential errors). mdKvTable no longer shows the backslash of an escaped pipe. Replay tags and breadcrumb labels use code spans instead of escaped text inside backticks, where the backslashes showed. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…yntax tableCell escaped pipes in a second replace after text. That was correct, since text had already doubled every backslash, but CodeQL flagged it as incomplete escaping. Both helpers now share one escape pass, and tableCell adds the pipe to the pattern.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
mdKvTable dropped any backslash before a pipe, including a literal one inside a code span (culprit, location) or in a raw value. It now scans escapes, code spans, and pipes in order: only an escaped pipe loses its backslash, and code-span content keeps every backslash. Log custom attribute names and values are now escaped like other API data.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3f8880d. Configure here.
The extraFields fallback in log details now escapes attribute names and values like the allAttributes path. mdKvTable now runs labels through the same cell sanitizer as values, so a pipe in a label cannot split the row. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
BYK
requested changes
Nov 2, 2026
| lines.push( | ||
| `| **${label}** | ${value.replace(/\n/g, " ").replace(/\|/g, "\u2502")} |`, | ||
| ); | ||
| lines.push(`| **${kvTableCell(label)}** | ${kvTableCell(value)} |`); |
Member
There was a problem hiding this comment.
I really feel like we should consider using helpers and AST to construct proper markdown outputs at this point. Is this not feasible?
Contributor
Author
There was a problem hiding this comment.
Feasible, yes. I'd do it as a follow-up rather than in this PR. I tried it quickly:
- It works with our renderer. I built an mdast tree (a table with untrusted text, strong, and inline code containing
|,`,*,[,<,\, and a URL), serialized it withmdast-util-to-markdown+gfmToMarkdown(), and parsed it with the CLI'smarked. Every cell came back as the literal input. The serializer escapes pipes properly, sokvTableCelland the│replacement could go away. - Cost: both packages are already in the lockfile (
mdast-util-gfmis a toolkit-core devDependency). Bundled and minified they add about 63 KB (21 KB gzipped) to the CLI. - Scope: about 60 CLI source files and roughly 430 call sites build Markdown from strings today, and
colorTag(<red>…</red>) would need a custom node orhtmlnodes. That's too much to fold into a PR that's meant to share escaping.
Suggested path: add mdast-backed builders (kv table, table row, paragraph) to @sentry/toolkit-core/markdown, cover them with the existing marked + micromark property tests, then move formatters over in a few PRs, with mdKvTable first since it has the most hand-written escaping. Want me to open that as a stacked follow-up?
Drop the escapeMarkdownCell, escapeMarkdownInline, and safeCodeSpan aliases. Call sites now import tableCell, text, and inlineCode from @sentry/toolkit-core/markdown, as MCP does. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
BYK
approved these changes
Nov 2, 2026
BYK
enabled auto-merge (squash)
November 2, 2026 22:11
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 2, 2026
Resolve import conflicts with #1805 (shared toolkit-core Markdown builders) by keeping both the valibot schema imports and the new markdown helper imports. In release create's dry-run output, use inlineCode() from main on the now string-typed version. Co-Authored-By: Junior (Claude) <noreply@anthropic.com>
This branch was previously deployed
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
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.
Follow-up to #1795. The CLI's three escape helpers are now the shared builders in
@sentry/toolkit-core/markdown, so CLI and MCP escape untrusted values the same way, checked by property tests against the samemarkedversion the CLI renders with.escapeMarkdownInlinetext\ * _[ ] < >`escapeMarkdownCelltableCell(new):textplus escaped pipes\ | < >and newlines onlysafeCodeSpaninlineCode`→ˋ,|→│The names stay, so the ~220 call sites don't change. (
textwould also shadow a lot of localtextvariables.)tableCellescapes each|aftertexthas doubled every backslash, so every\|is an escaped pipe, never an escaped backslash followed by a cell delimiter. New property tests put it in a two-column GFM table and check that, with bothmarkedand micromark, the value comes back intact and the next cell is untouched. They fail on"|"if the pipe escape is removed.Behavior changes in CLI output
*prod*no longer renders in italics.│:mdKvTabledoes that for table structure.hello_world).renderOneInline's escape-stripping for autolinks, because raw Markdown from other sources can still reach it.texttrims a value and puts it on one line. For the four multi-line callers there's a newescapeMarkdownLines, which keeps line breaks and up to three spaces of indent: blockquoted issue and log messages,cli fixinstructions, and credential errors.Fixes along the way
mdKvTableturned an escaped\|into\│, so a backslash showed. It now drops the escape. Even runs of backslashes are kept.safeCodeSpan." (offline, from cache)"through the escaper, andtextwould trim its leading space. It's a plain literal now.Verification: toolkit-core 372 tests, mcp-core 1897 (6 skipped), CLI 13002 passed.
tscandlint(including the workspace dependency check) are clean. Six CLI tests fail in my sandbox with or without this change: permission/readonly cases (EACCES, EPERM, readonly DB, unreadable file, lock) that need a non-privileged user. CI runs them as one.via @BYK.
--
View Session in Sentry