Skip to content

Support Mermaid without Prism and safe local wiki links#520

Draft
leevi2010-cursor wants to merge 1 commit into
schuyler:mainfrom
leevi2010-cursor:claude/mermaid-wikilink-preview-019f4aa8
Draft

Support Mermaid without Prism and safe local wiki links#520
leevi2010-cursor wants to merge 1 commit into
schuyler:mainfrom
leevi2010-cursor:claude/mermaid-wikilink-preview-019f4aa8

Conversation

@leevi2010-cursor

Copy link
Copy Markdown

Summary

Fixes two main-preview compatibility gaps for Markdown knowledge-base documents:

  • Mermaid scripts now load when Mermaid is enabled, independently of Prism syntax highlighting
  • The Mermaid preference is no longer disabled in the UI when syntax highlighting is off
  • Basic [[note]] and [[note|label]] wiki links render in prose and tables
  • Wiki links resolve only to existing .md files in the current document directory
  • Missing, unsafe, traversal, and non-Markdown targets remain readable text without a link
  • Fenced code blocks preserve literal wiki-link syntax

Quick Look intentionally remains unchanged: it disables JavaScript and explicitly excludes Mermaid for sandbox, startup-cost, and security reasons. This PR fixes the main document preview without conflating that separate product boundary.

Testing

  • MPMermaidRenderingTests: 17 passed
  • MPMarkdownRenderingTests: 71 passed
  • 88 focused tests passed in total
  • Full Debug build succeeded with Xcode 26.6
  • Interface Builder validation passed for MPHtmlPreferencesViewController.xib

The wiki-link implementation is intentionally narrow. It does not add vault indexing, title lookup, backlinks, embeds, headings, or graph behavior.

@schuyler schuyler left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @leevi2010-cursor!

Issues

  • Reinvents path-safety instead of reusing MPURLSecurityPolicy: the wiki-link resolver uses an inline character blacklist that doesn't resolve symlinks, rather than the existing CVE-motivated MPURLSecurityPolicy containment check, so the two "safe local file" definitions can drift and a symlinked .md target isn't caught.
  • XSS-escaping of link labels is unverified: labels are escaped only for Markdown metacharacters, not HTML, and a payload like [[note|<script>…]] re-enters Hoedown, so a test confirming the output is entity-escaped would close this (and reveal a real issue if the assumption is wrong).
  • Fragile dependency on preview base-URL internals: wiki-link resolution recovers the document directory from previewSafeBaseURL:'s WebKit sentinel path, an accidental coupling that isn't tested against the real document delegate and would break silently if that scheme changes.
  • Export path untested: the Mermaid-without-Prism change was applied identically to the HTML export path but only the preview path is tested, leaving the duplicated branch open to silent drift.
  • Fence detection can misparse: matching fences by hasPrefix: against a fixed three-backtick marker collapses different backtick counts, so a 4-backtick block can be closed by a 3-backtick line, differing from how Hoedown parses and risking linkification inside code blocks.
  • Inline code spans aren't preserved: wiki syntax inside single-backtick spans such as `[[note]]` still gets linkified, contradicting the "code preserves literal syntax" intent that fenced blocks honor.

Suggestions

  • Empty or whitespace-only targets ([[]], [[ |x]]) currently vanish from the output instead of staying as plain text.
  • Coverage is missing for several defensible-but-unverified cases: absolute-path targets, a nil base URL (unsaved document), ~/percent-encoded traversal, and the "neither Mermaid nor Prism" combination.
  • The rendererBaseURL: doc comment and the Quick Look "Excluded features" comment are now out of date given wiki links, and the [[note]] syntax has no user-facing documentation.
  • A resolved link's label gets a second Markdown pass (so [[note|a*b*]] italicizes), and documenting that as intended would settle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants