DataTable: add Table.CopyAsMarkdownButton with hardened markdown serializer#7857
Open
ianwinsemius wants to merge 2 commits into
Open
DataTable: add Table.CopyAsMarkdownButton with hardened markdown serializer#7857ianwinsemius wants to merge 2 commits into
ianwinsemius wants to merge 2 commits into
Conversation
…alizer
Adds an opt-in slot button for copying the visible rows to the clipboard
as a GitHub-flavoured Markdown table.
Security model (audit hand-off):
- Cell values are projected via `Column.getExportValue` (preferred) or
the field value via dotted-path traversal. `renderCell` is explicitly
NEVER called — React/JSX output cannot reach the clipboard.
- Escaping rules: `\` → `\\`, `|` → `\|`, CR/LF → single space,
ASCII / Unicode control characters (including TAB and DEL) stripped.
Backticks, brackets, asterisks intentionally not escaped — they cannot
break the table cell layout, and downstream Markdown renderers may
legitimately style them.
- Output is always `text/plain`. No HTML, no inline-HTML escape hatches.
- Clipboard write prefers the async Clipboard API, falls back to
`document.execCommand('copy')` via a hidden `<textarea>` for
non-secure contexts.
API:
- New: `<Table.CopyAsMarkdownButton rows columns ... />` slot.
- New: `Column.getExportValue?: (data) => string` — declare a plain-text
projection for columns whose `renderCell` emits React nodes.
- New helpers in `./clipboard.ts`: `escapeMarkdownCell`,
`getExportableCellValue`, `rowsToMarkdown`, `writeTextToClipboard`.
Tests (33 cases):
- Escape rules: pipes, backslashes, CR/LF, ASCII/C1 controls, DEL,
TAB, leading/trailing whitespace.
- Hostile input (table-breakout attempt) cannot break out of the cell.
- `getExportableCellValue` precedence, field traversal, dotted paths,
arrays, plain objects, null/undefined.
- `renderCell` is never invoked by the serializer (verified via spy).
- `rowsToMarkdown`: header/separator/body shape, empty columns/data,
header pipe escaping, callable `header` fallback to column id.
- `writeTextToClipboard`: Clipboard API path, execCommand fallback when
Clipboard API rejects.
- Component: default label, click writes serialized markdown to
clipboard, transient success label, onCopy callback, custom children.
Stories: `WithCopyAsMarkdown`.
Docs: `DataTable.docs.json` updated with `Table.CopyAsMarkdownButton` and
`Column.getExportValue`.
Changeset: minor.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 5c4a830 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a DataTable slot action for copying tabular data as Markdown, plus serializer helpers and documentation/tests around the export behavior.
Changes:
- Adds
Table.CopyAsMarkdownButtonand exports clipboard helpers from the DataTable barrel. - Introduces Markdown serialization, cell escaping, export-value projection, and clipboard write fallback logic.
- Adds docs, Storybook example, tests, and a minor changeset for the new API.
Show a summary per file
| File | Description |
|---|---|
packages/react/src/DataTable/index.ts |
Exports the new slot component, props type, and clipboard helpers. |
packages/react/src/DataTable/DataTable.features.stories.tsx |
Adds a Storybook feature example for Markdown copying. |
packages/react/src/DataTable/DataTable.docs.json |
Documents the new button and Column.getExportValue. |
packages/react/src/DataTable/CopyAsMarkdownButton.tsx |
Implements the copy button UI/state and clipboard interaction. |
packages/react/src/DataTable/CopyAsMarkdownButton.module.css |
Adds stable sizing for transient button labels. |
packages/react/src/DataTable/column.ts |
Adds the optional getExportValue column projection hook. |
packages/react/src/DataTable/clipboard.ts |
Implements Markdown escaping, row serialization, and clipboard writing. |
packages/react/src/DataTable/__tests__/clipboard.test.tsx |
Adds serializer, clipboard, and component tests. |
.changeset/datatable-copy-as-markdown.md |
Adds a minor changeset for the new DataTable API. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 2
Comment on lines
+33
to
+39
| export function escapeMarkdownCell(value: string): string { | ||
| return value | ||
| .replace(/\\/g, '\\\\') | ||
| .replace(/\|/g, '\\|') | ||
| .replace(/\r\n|\r|\n/g, ' ') | ||
| .replace(CONTROL_CHARACTERS, '') | ||
| .trim() |
Comment on lines
+76
to
+78
| const handleClick = useCallback(async () => { | ||
| const markdown = rowsToMarkdown(rows, columns) | ||
| const ok = await writeTextToClipboard(markdown) |
- Add clipboard.test.tsx to check-classname-tests.mjs via a manual className-forwarding test inside the file (the implementsClassName helper renders the component with no other props, but CopyAsMarkdownButton needs rows/columns to render anything). - escapeMarkdownCell now backslash-escapes < and > so a cell containing raw HTML such as <img src=x onerror=...> is rendered as literal text by GFM/CommonMark renderers instead of being interpreted as HTML. This is the most important hardening — paste destinations like GitHub comments DO interpret HTML inside markdown. - Architectural fix for the rows-source issue: introduce a small publish/subscribe snapshot store provided by Table.Container and populated by DataTable with its visible rows (post sort + filter + pagination). CopyAsMarkdownButton consumes the snapshot via useSyncExternalStore so it copies what the user actually sees, not the original `data` prop. Explicit `rows` / `columns` props still override the snapshot for standalone usage. - Tests: angle-bracket escape verified; 3 new context tests cover context source, sorted-order reflection, and explicit-props override. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
13 tasks
Author
|
Note The failing |
Member
|
Hi there @ianwinsemius! 👋 For these types of feature additions it'd be helpful to collaborate on an issue first, either through https://github.com/primer/react/issues or https://github.com/github/primer/issues to align before implementation begins. I think this use-case might be something that would work well either local in a project or github-ui first and then upstreamed as the use-case became more broadly used. Let me know what you're thinking though! |
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.
Closes #
Adds an opt-in slot button for copying the visible rows of a
DataTableto the system clipboard as a GitHub-flavoured Markdown table.Why review this PR carefully
A markdown-export feature can become a vulnerability surface if cell content can break out of a table cell, smuggle in HTML, or escape into the surrounding markdown. The implementation here is intentionally conservative — please scrutinise the escape rules and security model.
Security model (for review)
text/plain. There is no HTML and no inline-HTML escape hatch (e.g. no<br>).renderCellis never called by the serializer. Cell projection order is:column.getExportValue(row)— explicit, type-safe, audited per-columncolumn.field(dotted path traversal)A spy-based test verifies
renderCellis not invoked.escapeMarkdownCell:\→\\(done first so escaped pipes survive)|→\|\u0000–\u001fand Unicode C1\u007f–\u009f→ stripped (includes TAB, DEL)navigator.clipboard.writeTextand falls back to a hidden-textareadocument.execCommand('copy')for non-secure contexts and browsers that reject the async API.Hostile-input test
There's a test case that feeds the serializer an explicit table-breakout attempt:
and asserts:
Changelog
New
Table.CopyAsMarkdownButtonslot component (designed forTable.Actions).Column.getExportValue?: (data: Data) => string— opt-in plain-text projection per column.escapeMarkdownCell,rowsToMarkdown,writeTextToClipboard.Changed
DataTable.docs.jsonupdated withTable.CopyAsMarkdownButtonandColumn.getExportValuedocs.Removed
(none)
Rollout strategy
Testing & Reviewing
packages/react/src/DataTable/__tests__/clipboard.test.tsxcover escape rules, hostile input, projection precedence,renderCellexclusion (spy-verified), array/object stringification, dotted field paths, well-formed table shape, empty data, header pipe escaping, callableheaderfallback, both clipboard write paths, and the component's transient label /onCopycallback / custom children.npm run build,npm run type-check,npm run lint,npm run lint:css, andnpm test -- --run packages/react/src/DataTable/all pass locally.Merge checklist