fix(web): keep Windows drive paths as chat file links - #7182
Conversation
rehype-sanitize treats D: as a URL protocol and strips the href before urlTransform can rewrite it. Normalize Windows drive/UNC destinations and bare paths to file:// URLs first so existing file-link chips still resolve.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| serverConfig?.availableEditors ?? [], | ||
| ); | ||
| const diffThemeName = resolveDiffThemeName(resolvedTheme); | ||
| const markdownSource = useMemo(() => normalizeWindowsMarkdownFileLinks(text), [text]); |
There was a problem hiding this comment.
🟡 Medium components/ChatMarkdown.tsx:1386
normalizeWindowsMarkdownFileLinks(text) rewrites Windows paths inside code that CommonMark renders literally, so multi-backtick inline spans, fenced blocks containing mid-line ```/~~~ sequences, and indented code blocks can display Markdown link syntax instead of the original path. mapMarkdownOutsideCode does not recognize these code boundaries reliably; make the normalization parser-aware (or otherwise preserve all CommonMark code blocks) before passing the source to `ReactMarkdown`.
Also found in 2 other location(s)
apps/web/src/markdown-links.ts:261
FENCED_CODE_SEGMENT_PATTERNtreats any```or~~~sequence as a fence terminator, without requiring it to be on a closing-fence line. If fenced code content contains three backticks/tilde characters mid-line, protection ends there and later Windows paths inside the same code block are rewritten into links, corrupting the code sample.
apps/web/src/markdown-links.ts:273
mapMarkdownOutsideCodeprotects only fenced and inline code, so Markdown indented code blocks are passed throughtransform. For example,D:\tmp\file.mdis rewritten into link syntax even though CommonMark renders the original line as code, changing code samples and potentially causing the rewritten line to render differently instead of preserving the code block.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around line 1386:
`normalizeWindowsMarkdownFileLinks(text)` rewrites Windows paths inside code that CommonMark renders literally, so multi-backtick inline spans, fenced blocks containing mid-line [code fence]/~~~ sequences, and indented code blocks can display Markdown link syntax instead of the original path. `mapMarkdownOutsideCode` does not recognize these code boundaries reliably; make the normalization parser-aware (or otherwise preserve all CommonMark code blocks) before passing the source to `ReactMarkdown`.
Also found in 2 other location(s):
- apps/web/src/markdown-links.ts:261 -- `FENCED_CODE_SEGMENT_PATTERN` treats any ` [code fence] ` or `~~~` sequence as a fence terminator, without requiring it to be on a closing-fence line. If fenced code content contains three backticks/tilde characters mid-line, protection ends there and later Windows paths inside the same code block are rewritten into links, corrupting the code sample.
- apps/web/src/markdown-links.ts:273 -- `mapMarkdownOutsideCode` protects only fenced and inline code, so Markdown indented code blocks are passed through `transform`. For example, ` D:\tmp\file.md` is rewritten into link syntax even though CommonMark renders the original line as code, changing code samples and potentially causing the rewritten line to render differently instead of preserving the code block.
There was a problem hiding this comment.
Fixed in 72ff697. mapMarkdownOutsideCode now walks lines: indented code (4 spaces / tab) is left alone, and a fence only closes on its own line, so mid-line ``` no longer ends the block.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR adds substantial new markdown parsing logic for Windows path normalization. There is an unresolved Medium severity finding about potential issues with code block handling - while the author claims to have fixed it, the fix should be verified by human review. You can customize Macroscope's approvability policy. Learn more. |
Task checkbox offsets were taken from the rewritten markdown source, so toggles could miss the original file. Bare-path autolinks also ate emphasis stars and rewrote indented or mid-line-fenced code. Search markers in the original text, stop paths at *, and skip indented code plus fence closers that are not on their own line.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 72ff697. Configure here.
Windows path rewrites stay out of CommonMark fences that start after > or a list marker. Plain ``` fences still protect their bodies.
What Changed
Fixes #6418. Markdown destinations like
D:/...are rewritten tofile:///before sanitize, and bare Windows paths are autolinked. Workspace vs external-editor behavior is unchanged.Why
On Windows,
[Open](D:/tmp/file.md)survives Markdown parse as a link, butrehype-sanitizetreatsD:as an unknown protocol and strips the href beforeurlTransform/rewriteMarkdownFileUriHrefcan keep it. Bare paths such asM:\batches\...\prompt.mdnever become links because GFM does not autolink filesystem paths.This preprocesses those destinations (and conservative bare drive/UNC paths) into
file:///URLs so sanitize keeps them. ExistingrewriteMarkdownFileUriHref/resolveMarkdownFileLinkMetastill decide chips, tooltips, and workspace vs external-editor behavior. Drive-letter schemes are not added to the sanitize allowlist.Checklist
Note
Medium Risk
Adds a large regex-based markdown preprocessor on the chat render path; behavior is heavily tested but edge cases in mixed Windows paths, emphasis, and code fences could still mis-link or mis-map task checkboxes.
Overview
Fixes Windows chat markdown where
D:/...link destinations were stripped by rehype-sanitize (drive letter treated as a protocol) and bare paths never became links.normalizeWindowsMarkdownFileLinkspreprocesses the message before parse/render: rewrites inline/link-definition/angle-bracket Windows destinations tofile:///URLs, conservatively autolinks bare drive and UNC paths, and skips fenced code, indented code, and inline code.ChatMarkdownrenders and resolves file-link metadata from this rewrittenmarkdownSourcewhile task-checkbox edits still target the originaltext.rewriteMarkdownFileUriHrefandparseFileUrlHrefnow handle UNCfile://URLs and pass leftover bareD:// UNC hrefs through sourlTransformdoes not drop them.findTaskListMarkerOffsetmoves tomarkdown-task-list.tsand maps AST offsets from the rewritten source back to the raw string by line when earlier autolinking shifts byte positions.Reviewed by Cursor Bugbot for commit 18b22ae. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix Windows drive and UNC paths to render as clickable file links in chat markdown
normalizeWindowsMarkdownFileLinksin markdown-links.ts to preprocess chat markdown before rendering, converting bare Windows drive paths (e.g.C:\...) and UNC paths (e.g.\\server\share) intofile://links while skipping code blocks and existing links.parseFileUrlHrefandrewriteMarkdownFileUriHrefto correctly round-trip Windows drive and UNCfile://URLs back to their path forms instead of dropping them.findTaskListMarkerOffsetin markdown-task-list.ts to correctly map checkbox marker positions back to the original (pre-normalized) source when toggling task list items.Macroscope summarized 18b22ae.