fix(files): allowlist the schemes a markdown link may target

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts

@@ -76,21 +76,24 @@ export function applyFrontmatter(frontmatter: string, body: string): string {
   return frontmatter + body
 }
 
-/** A leading `scheme://` URL (network protocol). */
-const SCHEME_URL = /^([a-z][a-z0-9+.-]*):\/\//i
 /** A leading `scheme:` token (per the URL grammar). */
 const HAS_SCHEME = /^[a-z][a-z0-9+.-]*:/i
 /** A bare `host:port` (digits after the colon) — looks scheme-like but is really a domain. */
 const HOST_PORT = /^[a-z0-9.-]+:\d+(?:[/?#]|$)/i
 
+/**
+ * The only schemes a document link may target — an allowlist, because `scheme://` is well-formed for
+ * every scheme: rejecting just the ones known to be dangerous leaves the next one through, and
+ * `javascript://…` is a valid URL whose `//` run is merely a comment.
+ */
+const SAFE_SCHEME = /^(?:(?:https?|ftps?):\/\/|(?:mailto|tel):)/i
+
 /**
  * Normalize a user-entered link target: prefix a bare domain with `https://` so it doesn't resolve
  * as an in-app relative URL, while leaving already-qualified, relative (`./other.md`, `../doc.md`), and
- * protocol-relative URLs intact. Dangerous schemes are rejected outright rather than trusted or mangled:
- * any `scheme:` without `//` other than `mailto:`/`tel:` (so `javascript:`, `data:`, `vbscript:`,
- * `blob:`, …), and `file://` (local file access). Other network `scheme://` URLs (`http(s)`, `ftp`, …)
- * pass through. A bare `host:port` (digits after the colon) is a domain, not a scheme, so it still gets
- * the `https://` prefix.
+ * protocol-relative URLs intact. A scheme is kept only when {@link SAFE_SCHEME} matches; every other
+ * one is dropped to `''`, which callers render as inert text rather than a link. A bare `host:port`
+ * (digits after the colon) is a domain, not a scheme, so it still gets the `https://` prefix.
  */
 export function normalizeLinkHref(href: string): string {
   const trimmed = href.trim()
@@ -99,9 +102,7 @@ export function normalizeLinkHref(href: string): string {
   if (trimmed.startsWith('//')) return `https:${trimmed}`
   if (trimmed.startsWith('/')) return trimmed
   if (trimmed.startsWith('./') || trimmed.startsWith('../')) return trimmed
-  if (/^(?:mailto|tel):/i.test(trimmed)) return trimmed
-  const schemed = trimmed.match(SCHEME_URL)
-  if (schemed) return /^file$/i.test(schemed[1]) ? '' : trimmed
+  if (SAFE_SCHEME.test(trimmed)) return trimmed
   if (HAS_SCHEME.test(trimmed) && !HOST_PORT.test(trimmed)) return ''
   return `https://${trimmed}`
 }

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/link-editing.test.ts

@@ -0,0 +1,46 @@
+import type { ChainedCommands } from '@tiptap/core'
+import { describe, expect, it, vi } from 'vitest'
+import { applyLink } from './link-editing'
+
+function chainSpy() {
+  const calls: string[] = []
+  const chain = {
+    extendMarkRange: vi.fn(() => chain),
+    setLink: vi.fn(({ href }: { href: string }) => {
+      calls.push(`setLink:${href}`)
+      return chain
+    }),
+    unsetLink: vi.fn(() => {
+      calls.push('unsetLink')
+      return chain
+    }),
+    run: vi.fn(() => true),
+  }
+  return { chain: chain as unknown as ChainedCommands, calls }
+}
+
+describe('applyLink', () => {
+  it('sets a link for a target that survives normalization', () => {
+    const { chain, calls } = chainSpy()
+    applyLink(chain, '  sim.ai  ')
+    expect(calls).toEqual(['setLink:https://sim.ai'])
+  })
+
+  it('removes the link when the field is cleared', () => {
+    const { chain, calls } = chainSpy()
+    applyLink(chain, '   ')
+    expect(calls).toEqual(['unsetLink'])
+  })
+
+  /**
+   * The field is seeded with the raw href, so committing one untouched must not be read as "remove".
+   * Dropping an unsafe target is a refusal to link, not an instruction to delete what is already there.
+   */
+  it('leaves the existing link untouched when the target normalizes away', () => {
+    for (const target of ['javascript://%0aalert(1)', 'customproto://host/path']) {
+      const { chain, calls } = chainSpy()
+      applyLink(chain, target)
+      expect(calls).toEqual([])
+    }
+  })
+})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/link-editing.tsx

@@ -4,11 +4,17 @@ import { normalizeLinkHref } from '../markdown-fidelity'
 
 /**
  * Applies a link to the chain's current selection: normalizes `rawHref`, expands to the full link
- * mark, and sets it — or removes the link when the href is empty/unsafe. The caller supplies a chain
- * already focused with the target selection (the captured bubble-menu range / the hovered link range).
+ * mark, and sets it. Clearing the field removes the link; a target that survives normalization
+ * replaces it. A target that normalizes away is neither set nor removed — the editor seeds this field
+ * with the raw href, so committing an untouched one would otherwise delete a link the user only
+ * opened, and dropping an unsafe target is not the same instruction as "remove this link". The
+ * caller supplies a chain already focused with the target selection (the captured bubble-menu range /
+ * the hovered link range).
  */
 export function applyLink(chain: ChainedCommands, rawHref: string): void {
-  const href = normalizeLinkHref(rawHref.trim())
+  const trimmed = rawHref.trim()
+  const href = normalizeLinkHref(trimmed)
+  if (!href && trimmed) return
   chain.extendMarkRange('link')
   if (href) chain.setLink({ href })
   else chain.unsetLink()

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts

@@ -5,6 +5,7 @@
  * be idempotent (a second pass changes nothing) so autosave never churns. Mirrors the exact
  * pipeline the editor uses: split frontmatter out, serialize the body, re-attach + clean up.
  */
+import type { JSONContent } from '@tiptap/core'
 import { Editor } from '@tiptap/core'
 import { afterEach, describe, expect, it } from 'vitest'
 import { createMarkdownContentExtensions } from './extensions'
@@ -14,6 +15,7 @@ import {
   postProcessSerializedMarkdown,
   splitFrontmatter,
 } from './markdown-fidelity'
+import { parseMarkdownToDoc } from './markdown-parse'
 
 let editor: Editor | null = null
 
@@ -126,6 +128,66 @@ describe('markdown-fidelity utils', () => {
     expect(normalizeLinkHref('blob:https://x.com/uuid')).toBe('')
     expect(normalizeLinkHref('vbscript:msgbox(1)')).toBe('')
     expect(normalizeLinkHref('localhost:3000/path')).toBe('https://localhost:3000/path')
+    // Adding `//` doesn't make a scheme safe, and an unknown scheme is dropped rather than trusted —
+    // the allowlist is the whole rule.
+    expect(normalizeLinkHref('javascript://%0aalert(1)')).toBe('')
+    expect(normalizeLinkHref('customproto://host/path')).toBe('')
+  })
+
+  /**
+   * The property that matters, stated over the spellings a browser collapses before it resolves a
+   * scheme: whatever comes back must not be executable. Padding and interior tabs/newlines are the
+   * usual way a blocked scheme is smuggled past a matcher that only reads the literal text.
+   */
+  it('never returns a target that resolves to an executable scheme', () => {
+    const tab = String.fromCharCode(9)
+    const lf = String.fromCharCode(10)
+    const nbsp = String.fromCharCode(160)
+    const inputs = [
+      'javascript://%0aalert(1)',
+      'javascript:alert(1)',
+      'JAVASCRIPT://x',
+      ' javascript:alert(1) ',
+      `${nbsp}javascript:alert(1)`,
+      `java${tab}script://alert(1)`,
+      `java${lf}script:alert(1)`,
+      'data://text/html,<script>',
+      'vbscript://x',
+      'blob://x',
+      'file://x',
+    ]
+
+    const executable = inputs.filter((input) =>
+      /^(?:javascript|data|vbscript|blob|file):/.test(
+        normalizeLinkHref(input)
+          .replace(/[\t\n\r]/g, '')
+          .toLowerCase()
+      )
+    )
+    expect(executable).toEqual([])
+  })
+
+  /**
+   * A linked image carries its target in a node attribute rather than a link mark, so the mark's own
+   * URI validation never sees it and the raw target survives parsing — which is correct, since the
+   * document must serialize back verbatim. `image.tsx` builds its anchor from
+   * `normalizeLinkHref(attrs.href)` and omits the anchor entirely when that is empty, so this is the
+   * step that decides whether the target ever reaches the DOM.
+   */
+  it('drops a dangerous linked-image target before it can reach an anchor', () => {
+    const doc = parseMarkdownToDoc('[![a](https://x.example/i.png)](javascript://%0aalert(1))')
+    const hrefs: string[] = []
+    const walk = (node: JSONContent) => {
+      if (node.type === 'image' && typeof node.attrs?.href === 'string') hrefs.push(node.attrs.href)
+      node.content?.forEach(walk)
+    }
+    walk(doc)
+
+    // The parser preserves the authored target — serialization round-trips it verbatim.
+    expect(hrefs).toHaveLength(1)
+    expect(hrefs[0]).toContain('javascript://')
+    // …and the renderer refuses to build an anchor out of it.
+    expect(normalizeLinkHref(hrefs[0])).toBe('')
   })
 
   it('collapses trailing blank lines and preserves leading whitespace', () => {