From b1b3230df3f11fbd47e606641c464d5208b46480 Mon Sep 17 00:00:00 2001 From: Baptiste Leproux Date: Tue, 13 Oct 2026 16:51:42 +0200 Subject: [PATCH 1/2] fix(security): sanitize frontmatter, tag selectors and data URLs --- AGENTS.md | 4 +- docs/content/4.plugins/1.built-in/security.md | 43 ++++- .../{props-validation.ts => sanitize.ts} | 172 ++++++++++++++---- .../src/internal/stringify/attributes.ts | 2 +- packages/comark/src/plugins/security.ts | 40 ++-- packages/comark/src/utils/index.ts | 4 + packages/comark/test/plugins/security.test.ts | 74 ++++++++ ...ps-validation.test.ts => sanitize.test.ts} | 135 +++++++++++++- packages/comark/test/utils/mdc-compiler.ts | 2 +- test/bundle.test.ts | 2 +- 10 files changed, 417 insertions(+), 61 deletions(-) rename packages/comark/src/internal/{props-validation.ts => sanitize.ts} (67%) rename packages/comark/test/{props-validation.test.ts => sanitize.test.ts} (83%) diff --git a/AGENTS.md b/AGENTS.md index 7be82c61..baa599b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,7 +94,7 @@ packages/comark/ │ ├── shiki.ts # Shared Shiki runtime used by both entry points │ ├── frontmatter.ts # parseFrontmatter() / renderFrontmatter() │ ├── yaml.ts # YAML helpers -│ ├── props-validation.ts # Component props validation +│ ├── sanitize.ts # Component props validation and frontmatter sanitization │ ├── parse/ # Parsing pipeline │ │ ├── token-processor.ts # markdown-exit tokens → document nodes │ │ ├── auto-close/ # Self-healing for incomplete markdown @@ -481,7 +481,7 @@ import { renderMarkdown } from 'comark/render' // Document model types and utilities import type { MarkdownDocument, Node, ElementNode, TextNode, CommentNode } from 'comark' -import { textContent, visit, visitAsync, escapeHtml, isMarkdownDocument } from 'comark/utils' +import { textContent, visit, visitAsync, escapeHtml, isMarkdownDocument, sanitizeData } from 'comark/utils' import { noopTracer, withSpan } from 'comark/utils/trace' // Core plugins — use when calling parseMarkdown() directly (framework-agnostic) diff --git a/docs/content/4.plugins/1.built-in/security.md b/docs/content/4.plugins/1.built-in/security.md index 33c9cb9f..890cb74b 100644 --- a/docs/content/4.plugins/1.built-in/security.md +++ b/docs/content/4.plugins/1.built-in/security.md @@ -115,9 +115,11 @@ Object and array prop values (`:links='[{"to": "/docs", "label": "Docs"}]'`, YAM URL props are decoded (URL-encoded and HTML entity variants included) and checked against a hard-coded block list. A prop is a URL prop when its name is `href`, `src`, `to`, `action`, `poster`, `cite`, `background` or `ping`, or ends in `href`, `src`, `url`, `uri` or `link` (optionally plural), such as `xlink:href`, `imageUrl`, or `avatar-src`. Other props are never parsed as URLs, so prose such as `title="JavaScript: the good parts"` is kept. -These protocols are **always** blocked, even if `allowedProtocols: ['*']` is set: +These URLs are **always** blocked, even if `allowedProtocols: ['*']` is set: -`javascript:` · `vbscript:` · `data:text/html` · `data:text/javascript` · `data:text/vbscript` · `data:text/css` · `data:text/plain` · `data:text/xml` +- `javascript:` and `vbscript:` URLs +- `data:` URLs in link props (`href`, `to`, …) +- `data:` URLs in image props, unless their MIME type is `image/*` (see [`allowDataImages`](#options-allowdataimages)) A `data:` URL is checked the way browsers read its MIME type, which ignores spaces after `data:`. So `data: text/html,…` is blocked like `data:text/html,…`. @@ -137,6 +139,24 @@ The same check applies to `:href` and `:src` bindings twice: on the JSON-decoded :: +### Frontmatter + +Renderers spread frontmatter into components, so it is sanitized with the same rules as props, at any depth: + +- event handler and markup keys are removed, with the same name checks +- URLs in URL keys (`to`, `link`, `imageSrc`, …) are held to the protocol floor and the URL options +- an `as` or `is` value naming a blocked, unsafe or not-allowed tag is removed + +Dropped array items are removed without leaving a hole. Set [`frontmatter: false`](#options-frontmatter) to opt out. + +To apply the same rules to objects that don't come through the parser, such as a YAML or JSON file you load yourself, use `sanitizeData()` from `comark/utils`. It cleans the data in place and returns the dotted paths it removed: + +```typescript +import { sanitizeData } from 'comark/utils' + +const removed = sanitizeData(data, { blockedTags: ['script'], allowDataImages: false }) +``` + --- ## API @@ -165,6 +185,7 @@ Returns a `ComarkPlugin` that sanitizes the parsed AST. | [`allowedImagePrefixes`](#options-allowedimageprefixes) | `string[]` | `['*']` | URL prefixes permitted in image props (`src`, `*src`, `poster`) | | [`defaultOrigin`](#options-defaultorigin) | `string` | `undefined` | Rewrite disallowed URLs to this origin instead of stripping | | [`allowDataImages`](#options-allowdataimages) | `boolean` | `true` | Allow `data:image/*` URIs in image props | +| [`frontmatter`](#options-frontmatter) | `boolean` | `true` | Sanitize frontmatter as well as the AST | ### `blockedTags` @@ -197,7 +218,9 @@ security({ }) ``` -The `as` prop (which makes framework renderers resolve a different component than the element's own tag) is held to the same filters: an `as` value naming a blocked or not-allowed tag is stripped, and the element falls back to its own tag. +The `as` and `is` props (which make framework renderers resolve a different component than the element's own tag) are held to the same filters: a value naming a blocked or not-allowed tag, or `object`, is stripped, and the element falls back to its own tag. + +Their bindings are checked too. A JSON binding (`:as='"section"'`) is checked decoded, as renderers decode it. A dot-path binding (`:as="frontmatter.tag"`) only resolves at render time, after every check, so it is always stripped. ### `tagFallback` @@ -227,7 +250,7 @@ security({ ``` ::warning -The hard-coded unsafe protocols (`javascript:`, `vbscript:`, `data:text/*`) are a floor that cannot be overridden. Even `allowedProtocols: ['javascript']` will not unblock `javascript:` URLs. +The hard-coded floor (`javascript:`, `vbscript:`, and `data:` outside images) cannot be overridden. Even `allowedProtocols: ['javascript']` will not unblock `javascript:` URLs. :: ### `allowedLinkPrefixes` @@ -277,9 +300,19 @@ security({ ``` ::tip -`data:text/*` variants in `href` are always blocked by the hard-coded protocol list regardless of this setting. +`data:` URLs in link props, and non-image `data:` URLs anywhere, are always blocked by the hard-coded floor regardless of this setting. :: +### `frontmatter` + +Controls whether frontmatter is sanitized along with the AST (see [Frontmatter](#frontmatter)). Set to `false` when your frontmatter is trusted and never spread into components. + +```typescript +security({ + frontmatter: false +}) +``` + --- ## Examples diff --git a/packages/comark/src/internal/props-validation.ts b/packages/comark/src/internal/sanitize.ts similarity index 67% rename from packages/comark/src/internal/props-validation.ts rename to packages/comark/src/internal/sanitize.ts index bbe64dcf..9edaed31 100644 --- a/packages/comark/src/internal/props-validation.ts +++ b/packages/comark/src/internal/sanitize.ts @@ -14,24 +14,20 @@ export const unsafeTags = ['object'] // path via the `html` plugin and does not need these). export const unsafeAttributes = ['srcdoc', 'formaction', 'innerhtml', 'dangerouslysetinnerhtml', 'textcontent'] -export const unsafeLinkPrefix = [ - 'javascript:', - 'data:text/html', - 'vbscript:', - 'data:text/javascript', - 'data:text/vbscript', - 'data:text/css', - 'data:text/plain', - 'data:text/xml', -] +export const unsafeProtocols = ['javascript:', 'vbscript:'] /** - * Whether `url` uses a hard-floor unsafe scheme. + * Whether `url` is a known-unsafe protocol. */ -function hasUnsafeScheme(url: URL): boolean { - let href = url.href.toLowerCase() - if (url.protocol === 'data:') href = `data:${href.slice(5).trimStart()}` - return unsafeLinkPrefix.some((prefix) => href.startsWith(prefix)) +function isUnsafeProtocol(url: URL, allowDataImage: boolean): boolean { + if (unsafeProtocols.includes(url.protocol)) return true + if (url.protocol !== 'data:') return false + if (!allowDataImage) return true + + const body = url.href.slice('data:'.length) + let end = 0 + while (end < body.length && body[end] !== ',' && body[end] !== ';') end++ + return !body.slice(0, end).trim().toLowerCase().startsWith('image/') } export interface PropsValidationOptions { @@ -143,13 +139,7 @@ function validateUrl( url = resolved } - // Block known-unsafe protocols — hard floor, not overrideable by options - if (hasUnsafeScheme(url)) { - return REJECTED_PROP - } - - // Block data: images when allowDataImages is false - if (mode === 'image' && !allowDataImages && url.protocol === 'data:') { + if (isUnsafeProtocol(url, mode === 'image' && allowDataImages)) { return REJECTED_PROP } @@ -201,12 +191,11 @@ function matchesAllowedPrefix(url: URL, prefix: string): boolean { } /** - * Hard-floor check: does this string resolve to a known-unsafe URL scheme - * (`javascript:`, `data:text/html`, …)? Applied to binding-resolved values at - * render time so dot-path data (frontmatter/meta/data) cannot smuggle an - * unsafe URL past parse-time validation. Relative URLs are never unsafe here. + * Whether `value` is a known-unsafe URL. */ export function isUnsafeUrlValue(value: string): boolean { + if (!value.includes(':') && !value.includes('&') && !value.includes('%')) return false + let decoded = value try { decoded = decodeURIComponent(value) @@ -221,7 +210,8 @@ export function isUnsafeUrlValue(value: string): boolean { } catch { return false } - return hasUnsafeScheme(url) + // No prop mode here: a data image passes, as it cannot run script. + return isUnsafeProtocol(url, true) } // Attributes browsers navigate to or load as a URL, beyond the `*href` / @@ -254,11 +244,50 @@ function normalizePropName(attribute: string): string { .replace(/[:_-]/g, '') } -export function validateProp(attribute: string, value: unknown, options: PropsValidationOptions = {}): unknown { - const name = normalizePropName(attribute) +/** Whether a normalized prop name is an event handler or a markup sink, whatever its value. */ +function isUnsafePropName(name: string): boolean { // `*Html` / `*-html` props (`innerHTML`, Nuxt UI's `labelHtml`, `v-html`) // are markup sinks: components render them with `v-html` or equivalents. - if (name.startsWith('on') || name.endsWith('html') || unsafeAttributes.includes(name)) { + return name.startsWith('on') || name.endsWith('html') || unsafeAttributes.includes(name) +} + +/** Props that pick the element or component a renderer creates. */ +const tagSelectors = ['as', 'is'] + +/** Whether prop `key` is a tag selector: `as`, `is`, or one of their bindings. */ +export function isTagSelector(key: string): boolean { + return tagSelectors.includes(normalizePropName(key)) +} + +/** A filter for tag selectors: `unsafeTags` and `blockedTags`, or any tag outside `allowedTags` when set. */ +export function createTagFilter(blockedTags: string[] = [], allowedTags: string[] = []): (tag: string) => boolean { + const blocked = new Set([...blockedTags, ...unsafeTags].map((tag) => tag.toLowerCase())) + const allowed = new Set(allowedTags.map((tag) => tag.toLowerCase())) + return (tag) => { + const name = tag.toLowerCase() + return blocked.has(name) || (allowed.size > 0 && !allowed.has(name)) + } +} + +/** + * Whether tag selector `key` can make a renderer create a tag `isFilteredTag` rejects. + */ +export function isUnsafeTagSelector(key: string, value: unknown, isFilteredTag: (tag: string) => boolean): boolean { + if (typeof value !== 'string') return false + if (!/^(?::|v-bind:)/.test(key)) return isFilteredTag(value) + + let decoded: unknown + try { + decoded = JSON.parse(value) + } catch { + return true + } + return typeof decoded === 'string' && isFilteredTag(decoded) +} + +export function validateProp(attribute: string, value: unknown, options: PropsValidationOptions = {}): unknown { + const name = normalizePropName(attribute) + if (isUnsafePropName(name)) { return REJECTED_PROP } @@ -286,16 +315,18 @@ export function validateProp(attribute: string, value: unknown, options: PropsVa return validateValue(name, value, options) } -function validateValue(name: string, value: unknown, options: PropsValidationOptions): unknown { - if (typeof value === 'string') { - // SVG `` / `` targeting `href` can rewrite a link to the - // script URL in `values` / `from` / `to` / `by` once the animation runs. - if (name === 'attributename') { - return decodeHtmlEntities(value).toLowerCase().includes('href') ? REJECTED_PROP : value - } - if (!isUrlProp(name)) return value - return validateUrl(value, name.endsWith('src') || name === 'poster' ? 'image' : 'link', options) +function validateString(name: string, value: string, options: PropsValidationOptions): string | typeof REJECTED_PROP { + // SVG `` / `` targeting `href` can rewrite a link to the + // script URL in `values` / `from` / `to` / `by` once the animation runs. + if (name === 'attributename') { + return decodeHtmlEntities(value).toLowerCase().includes('href') ? REJECTED_PROP : value } + if (!isUrlProp(name)) return value + return validateUrl(value, name.endsWith('src') || name === 'poster' ? 'image' : 'link', options) +} + +function validateValue(name: string, value: unknown, options: PropsValidationOptions): unknown { + if (typeof value === 'string') return validateString(name, value, options) // A non-string href/src can reach here as an array/object from the YAML // block-props JSON round-trip, and is stringified when set as an attribute. @@ -371,3 +402,66 @@ export function validateProps( return props } + +export interface SanitizeDataOptions extends PropsValidationOptions { + /** Tags an `as` or `is` value may not name. */ + blockedTags?: string[] + /** When set, the only tags an `as` or `is` value may name. */ + allowedTags?: string[] +} + +/** + * Remove unsafe entries from `data`, such as frontmatter, in place, at any depth. + */ +export function sanitizeData(data: unknown, options: SanitizeDataOptions = {}): string[] { + const isFilteredTag = createTagFilter(options.blockedTags, options.allowedTags) + const removed: string[] = [] + + const seen = new WeakMap>() + + // `key` is undefined for an array item: it inherits its array's name, as in `validateProp()`. + const sanitizeEntry = (name: string, key: string | undefined, value: unknown): unknown => { + if (key !== undefined) { + if (isUnsafePropName(name)) return REJECTED_PROP + if (tagSelectors.includes(name) && isUnsafeTagSelector(key, value, isFilteredTag)) return REJECTED_PROP + } + return typeof value === 'string' ? validateString(name, value, options) : value + } + + const walk = (value: object, path: string, name: string): void => { + const isArray = Array.isArray(value) + const seenAs = isArray ? name : '' + const names = seen.get(value) + if (names?.has(seenAs)) return + if (names) names.add(seenAs) + else seen.set(value, new Set([seenAs])) + + let deleted = false + for (const [key, child] of Object.entries(value)) { + const at = path ? `${path}.${key}` : key + const childName = isArray ? name : normalizePropName(key) + const result = sanitizeEntry(childName, isArray ? undefined : key, child) + if (result === REJECTED_PROP) { + Reflect.deleteProperty(value, key) + removed.push(at) + deleted = true + } else if (result !== child) { + // `defaultOrigin` rewrote a URL. + ;(value as Record)[key] = result + } else if (child !== null && typeof child === 'object') { + walk(child, at, childName) + } + } + + if (deleted && isArray) { + let length = 0 + for (let index = 0; index < value.length; index++) { + if (index in value) value[length++] = value[index] + } + value.length = length + } + } + if (data !== null && typeof data === 'object') walk(data, '', '') + + return removed +} diff --git a/packages/comark/src/internal/stringify/attributes.ts b/packages/comark/src/internal/stringify/attributes.ts index 2ed7472f..9bc0a0c3 100644 --- a/packages/comark/src/internal/stringify/attributes.ts +++ b/packages/comark/src/internal/stringify/attributes.ts @@ -1,6 +1,6 @@ import { stringifyYaml } from '../yaml.ts' import { escapeHtml, get } from '../../utils/index.ts' -import { isUnsafeUrlValue } from '../props-validation.ts' +import { isUnsafeUrlValue } from '../sanitize.ts' import { pickFence } from './fence.ts' import type { NodeRenderData } from '../../types.ts' diff --git a/packages/comark/src/plugins/security.ts b/packages/comark/src/plugins/security.ts index 19584d3c..4cf34035 100644 --- a/packages/comark/src/plugins/security.ts +++ b/packages/comark/src/plugins/security.ts @@ -1,8 +1,14 @@ import type { ElementNode, Node } from 'comark' import { defineComarkPlugin } from 'comark' import { visitAsync } from 'comark/utils' -import { validateProps } from '../internal/props-validation.ts' -import type { PropsValidationOptions } from '../internal/props-validation.ts' +import { + createTagFilter, + isTagSelector, + isUnsafeTagSelector, + sanitizeData, + validateProps, +} from '../internal/sanitize.ts' +import type { PropsValidationOptions } from '../internal/sanitize.ts' interface SecurityOptions extends PropsValidationOptions { /** @@ -22,6 +28,12 @@ interface SecurityOptions extends PropsValidationOptions { * @default undefined */ tagFallback?: (element: ElementNode) => false | Node | Promise + + /** + * Sanitize frontmatter too: renderers spread it into components, so it gets the prop rules at any depth. + * @default true + */ + frontmatter?: boolean } export default defineComarkPlugin((options: SecurityOptions = {}) => { @@ -29,6 +41,7 @@ export default defineComarkPlugin((options: SecurityOptions = {}) => { blockedTags = [], allowedTags = [], tagFallback = undefined, + frontmatter = true, allowedLinkPrefixes, allowedImagePrefixes, allowedProtocols, @@ -38,6 +51,7 @@ export default defineComarkPlugin((options: SecurityOptions = {}) => { const dropSet = new Set(blockedTags.map((t) => t.toLowerCase())) const allowSet = new Set(allowedTags.map((t) => t.toLowerCase())) + const isFilteredTag = createTagFilter(blockedTags, allowedTags) const propsOptions: PropsValidationOptions = { allowedLinkPrefixes, @@ -69,15 +83,12 @@ export default defineComarkPlugin((options: SecurityOptions = {}) => { return false } - // The `as` prop makes renderers resolve a different component than - // the element's own tag — hold it to the same tag filters, otherwise - // `[x]{as="AdminPanel"}` bypasses allowedTags/blockedTags. - const asValue = element[1].as - if (typeof asValue === 'string') { - const asTag = asValue.toLowerCase() - if (dropSet.has(asTag) || (allowSet.size > 0 && !allowSet.has(asTag))) { - console.warn(`[comark/plugins/security] removing unsafe attribute: as="${asValue}"`) - delete element[1].as + // `as` and `is` make renderers resolve a different component than the element's own tag. + // Hold them, bindings included, to the same tag filters: otherwise `[x]{as="AdminPanel"}` bypasses them. + for (const key of Object.keys(element[1])) { + if (isTagSelector(key) && isUnsafeTagSelector(key, element[1][key], isFilteredTag)) { + console.warn(`[comark/plugins/security] removing unsafe attribute: ${key}="${element[1][key]}"`) + delete element[1][key] } } @@ -91,6 +102,13 @@ export default defineComarkPlugin((options: SecurityOptions = {}) => { } } ) + + if (frontmatter) { + const removed = sanitizeData(state.tree.frontmatter, { ...propsOptions, blockedTags, allowedTags }) + for (const path of removed) { + console.warn(`[comark/plugins/security] removing unsafe frontmatter: ${path}`) + } + } }, } }) diff --git a/packages/comark/src/utils/index.ts b/packages/comark/src/utils/index.ts index 34332f3c..dc020e7e 100644 --- a/packages/comark/src/utils/index.ts +++ b/packages/comark/src/utils/index.ts @@ -303,3 +303,7 @@ export function get(data: unknown, key: string): unknown { // same `:prefix` semantics as the HTML/ANSI handlers without duplicating logic. export { resolveAttributes, resolveAttribute } from '../internal/stringify/attributes.ts' export type { ResolveAttributesOptions } from '../internal/stringify/attributes.ts' + +// The security plugin's frontmatter rules, for objects the parser never sees. +export { sanitizeData } from '../internal/sanitize.ts' +export type { SanitizeDataOptions } from '../internal/sanitize.ts' diff --git a/packages/comark/test/plugins/security.test.ts b/packages/comark/test/plugins/security.test.ts index c6f6c2e9..203ffc10 100644 --- a/packages/comark/test/plugins/security.test.ts +++ b/packages/comark/test/plugins/security.test.ts @@ -354,6 +354,52 @@ home: javascript:alert(1) }) }) +describe('security plugin — frontmatter', () => { + const md = `--- +title: Page +onclick: alert(1) +hero: + ^onmouseover: alert(1) + as: script + is: script +links: + - label: Docs + to: /docs + - label: Evil + url: "javascript:alert(1)" +image: + src: "data: text/html," +description: "javascript: tips and tricks" +--- + +# Page` + + it('strips unsafe keys, URLs and `as` or `is` values at any depth', async () => { + const tree = await parseWithSecurity(md, { blockedTags: ['script'] }) + + expect(tree.frontmatter).toEqual({ + title: 'Page', + hero: {}, + links: [{ label: 'Docs', to: '/docs' }, { label: 'Evil' }], + image: {}, + // Not a URL key: prose stays, as for props. + description: 'javascript: tips and tricks', + }) + }) + + it('leaves frontmatter alone with `frontmatter: false`', async () => { + const tree = await parseWithSecurity(md, { blockedTags: ['script'], frontmatter: false }) + + expect(tree.frontmatter).toMatchObject({ onclick: 'alert(1)', hero: { as: 'script' } }) + }) + + it('checks an aliased array under its URL key too', async () => { + const tree = await parseWithSecurity(`---\ntags: &x\n - "javascript:alert(1)"\nlinks: *x\n---\n`) + + expect(tree.frontmatter).toEqual({ tags: [], links: [] }) + }) +}) + describe('security plugin — component props beyond href/src', () => { const untrusted = { blockedTags: ['script', 'iframe', 'embed', 'form', 'base', 'meta', 'link', 'style'], @@ -585,6 +631,34 @@ describe('security plugin — as prop', () => { const el = tree.nodes[0] as [string, any] expect(el[1].as).toBe('Badge') }) + + it('strips is like as', async () => { + const tree = makeTree([['span', { is: 'script' }, 'x']]) + await runPlugin(tree, { blockedTags: ['script'] }) + const el = tree.nodes[0] as [string, any] + expect(el[1].is).toBeUndefined() + }) + + it('checks a JSON binding decoded, as renderers decode it', async () => { + const tree = makeTree([['span', { ':as': '"script"', ':is': '"section"' }, 'x']]) + await runPlugin(tree, { blockedTags: ['script'] }) + const el = tree.nodes[0] as [string, any] + expect(el[1]).toEqual({ ':is': '"section"' }) + }) + + it('strips a binding resolved from data at render time, as no check can follow it', async () => { + const tree = makeTree([['span', { ':as': 'frontmatter.tag', 'v-bind:is': 'props.tag' }, 'x']]) + await runPlugin(tree) + const el = tree.nodes[0] as [string, any] + expect(el[1]).toEqual({}) + }) + + it("strips as naming object, whose props were checked against the element's own tag", async () => { + const tree = makeTree([['span', { as: 'object', data: '/x' }, 'x']]) + await runPlugin(tree) + const el = tree.nodes[0] as [string, any] + expect(el[1].as).toBeUndefined() + }) }) describe('security plugin — prop sanitization', () => { diff --git a/packages/comark/test/props-validation.test.ts b/packages/comark/test/sanitize.test.ts similarity index 83% rename from packages/comark/test/props-validation.test.ts rename to packages/comark/test/sanitize.test.ts index c33c140e..76baeb72 100644 --- a/packages/comark/test/props-validation.test.ts +++ b/packages/comark/test/sanitize.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' -import { REJECTED_PROP, validateProp, validateProps } from '../src/internal/props-validation' +import { REJECTED_PROP, isUnsafeUrlValue, validateProp, validateProps } from '../src/internal/sanitize' +import { sanitizeData } from '../src/utils/index.ts' describe('validateProp', () => { describe('event handlers', () => { @@ -619,3 +620,135 @@ describe('validateProps – unsafe tag case bypass', () => { expect(validateProps('Object', { data: '/foo' })).toEqual({}) }) }) + +describe('data: URLs', () => { + it('blocks every non-image MIME type, not only `text/*`', () => { + expect(validateProp('src', 'data:application/xhtml+xml,')).toBe(REJECTED_PROP) + expect(isUnsafeUrlValue('data:application/xhtml+xml,')).toBe(true) + }) + + it('only allows a data image as a `src`', () => { + expect(validateProp('src', 'data: image/png;base64,abc')).not.toBe(REJECTED_PROP) + expect(validateProp('href', 'data:image/png;base64,abc')).toBe(REJECTED_PROP) + expect(isUnsafeUrlValue('data:image/png;base64,abc')).toBe(false) + }) +}) + +describe('sanitizeData', () => { + it('drops event handler and markup keys at any depth, whatever their case or prefix', () => { + const data = { + title: 'Page', + onclick: 'x()', + '^onMouseOver': 'x()', + links: [{ label: 'Docs', '.innerHTML': 'x', TextContent: 'x' }], + hero: { card: { srcdoc: 'x', '@click': 'x()' } }, + } + + expect(sanitizeData(data)).toEqual([ + 'onclick', + '^onMouseOver', + 'links.0..innerHTML', + 'links.0.TextContent', + 'hero.card.srcdoc', + 'hero.card.@click', + ]) + expect(data).toEqual({ title: 'Page', links: [{ label: 'Docs' }], hero: { card: {} } }) + }) + + it('drops unsafe URLs in URL keys, at any depth', () => { + const data = { + home: { to: 'javascript:alert(1)' }, + course: { url: ' JavaScript:alert(1)' }, + agency: { link: 'java\tscript:x', website: 'https://comark.dev' }, + coverSrc: 'data: text/html,x', + } + + expect(sanitizeData(data)).toEqual(['home.to', 'course.url', 'agency.link', 'coverSrc']) + expect(data).toEqual({ home: {}, course: {}, agency: { website: 'https://comark.dev' } }) + }) + + it('keeps safe URLs, and prose under other keys, like `validateProp()`', () => { + const data = { + title: 'JavaScript: the good parts', + note: 'Note: none', + price: '100%', + to: '/docs', + href: 'mailto:hello@comark.dev', + imageSrc: 'data:image/png;base64,abc', + } + const copy = structuredClone(data) + + expect(sanitizeData(data)).toEqual([]) + expect(data).toEqual(copy) + }) + + it('passes the URL options through', () => { + expect(sanitizeData({ imageSrc: 'data:image/png;base64,abc' }, { allowDataImages: false })).toEqual(['imageSrc']) + + const data = { link: 'https://evil.com/path' } + expect( + sanitizeData(data, { allowedLinkPrefixes: ['https://comark.dev'], defaultOrigin: 'https://comark.dev' }) + ).toEqual([]) + expect(data.link).toBe('https://comark.dev/path') + }) + + it('drops an `as` or `is` naming a blocked or unsafe tag, or one outside `allowedTags`', () => { + const data = { hero: { as: 'SCRIPT' }, card: { is: 'object' }, section: { as: 'section', is: 'section' } } + + expect(sanitizeData(data, { blockedTags: ['script'] })).toEqual(['hero.as', 'card.is']) + expect(data.section).toEqual({ as: 'section', is: 'section' }) + expect(sanitizeData({ hero: { is: 'aside' } }, { allowedTags: ['section'] })).toEqual(['hero.is']) + }) + + it('drops an `as` or `is` binding no check can follow', () => { + const data = { hero: { ':as': 'frontmatter.tag', ':is': '"script"' }, card: { ':as': '"section"' } } + + expect(sanitizeData(data, { blockedTags: ['script'] })).toEqual(['hero.:as', 'hero.:is']) + expect(data.card).toEqual({ ':as': '"section"' }) + }) + + it('checks array items against their array name, closing the hole a dropped one leaves', () => { + const data = { links: ['/docs', 'javascript:alert(1)', '/blog'], tags: ['javascript: tips'] } + + expect(sanitizeData(data)).toEqual(['links.1']) + expect(data).toEqual({ links: ['/docs', '/blog'], tags: ['javascript: tips'] }) + }) + + it('walks a shared or cyclic object once', () => { + const link: Record = { label: 'Docs', onclick: 'x()' } + link.self = link + + expect(sanitizeData({ links: [link, link] })).toEqual(['links.0.onclick']) + }) + + it('checks a shared array under each name, as a YAML alias can reuse it', () => { + const shared = ['/docs', 'javascript:alert(1)'] + const data = { tags: shared, links: shared } + + expect(sanitizeData(data)).toEqual(['links.1']) + expect(data).toEqual({ tags: ['/docs'], links: ['/docs'] }) + }) + + it('compacts a large array without overflowing the stack', () => { + const data = { links: [...Array.from({ length: 300_000 }, () => 0), 'javascript:alert(1)'] } + + expect(sanitizeData(data)).toEqual(['links.300000']) + expect(data.links).toHaveLength(300_000) + }) + + it('leaves scalars and safe data untouched', () => { + const data = { + title: 'Installation', + navigation: { icon: 'i-lucide-play' }, + enabled: false, + count: 3, + date: '2026-10-13', + } + const copy = structuredClone(data) + + expect(sanitizeData(data)).toEqual([]) + expect(data).toEqual(copy) + expect(sanitizeData('hello')).toEqual([]) + expect(sanitizeData(null)).toEqual([]) + }) +}) diff --git a/packages/comark/test/utils/mdc-compiler.ts b/packages/comark/test/utils/mdc-compiler.ts index 6d127a0c..46ca95e2 100644 --- a/packages/comark/test/utils/mdc-compiler.ts +++ b/packages/comark/test/utils/mdc-compiler.ts @@ -4,7 +4,7 @@ import type { Root, RootContent, MDCNode, MDCRoot, NodePosition } from '@nuxtjs/mdc' import Slugger from 'github-slugger' import { toString } from 'hast-util-to-string' -import { validateProps } from '../../src/internal/props-validation' +import { validateProps } from '../../src/internal/sanitize' export interface CompilerOptions { keepPosition?: boolean diff --git a/test/bundle.test.ts b/test/bundle.test.ts index bd7e87ff..a15a1adb 100644 --- a/test/bundle.test.ts +++ b/test/bundle.test.ts @@ -67,7 +67,7 @@ describe('package bundle size', { timeout: 60_000 }, () => { "@comark/react": "38.0k (76 files)", "@comark/svelte": "45.9k (84 files)", "@comark/vue": "56.2k (80 files)", - "comark": "377k (160 files)", + "comark": "382k (160 files)", } `) }) From 1bfd6a841911784f129890f154cc3078b160db62 Mon Sep 17 00:00:00 2001 From: Baptiste Leproux Date: Tue, 13 Oct 2026 17:36:04 +0200 Subject: [PATCH 2/2] reviews --- packages/comark/src/internal/sanitize.ts | 5 +++++ .../comark/src/internal/stringify/attributes.ts | 12 ++++-------- packages/comark/test/plugins/security.test.ts | 16 ++++++++++++++++ packages/comark/test/resolve-attributes.test.ts | 15 +++++++++++++++ 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/comark/src/internal/sanitize.ts b/packages/comark/src/internal/sanitize.ts index 9edaed31..2babe9e2 100644 --- a/packages/comark/src/internal/sanitize.ts +++ b/packages/comark/src/internal/sanitize.ts @@ -244,6 +244,11 @@ function normalizePropName(attribute: string): string { .replace(/[:_-]/g, '') } +/** Whether prop `key` carries a URL, by the same names as `validateProp()`. */ +export function isUrlPropKey(key: string): boolean { + return isUrlProp(normalizePropName(key)) +} + /** Whether a normalized prop name is an event handler or a markup sink, whatever its value. */ function isUnsafePropName(name: string): boolean { // `*Html` / `*-html` props (`innerHTML`, Nuxt UI's `labelHtml`, `v-html`) diff --git a/packages/comark/src/internal/stringify/attributes.ts b/packages/comark/src/internal/stringify/attributes.ts index 9bc0a0c3..8bf7fe36 100644 --- a/packages/comark/src/internal/stringify/attributes.ts +++ b/packages/comark/src/internal/stringify/attributes.ts @@ -1,6 +1,6 @@ import { stringifyYaml } from '../yaml.ts' import { escapeHtml, get } from '../../utils/index.ts' -import { isUnsafeUrlValue } from '../sanitize.ts' +import { isUnsafeUrlValue, isUrlPropKey } from '../sanitize.ts' import { pickFence } from './fence.ts' import type { NodeRenderData } from '../../types.ts' @@ -87,16 +87,12 @@ export function resolveAttributes( outValue = value } - // Hard floor: a binding must never resolve href/src to an unsafe scheme + // Hard floor: a binding must never resolve a URL prop to an unsafe scheme // (javascript:, data:text/html, …). Parse-time validation only sees the // literal path, so the resolved value is checked here — even when the // security plugin is not enabled. - if ( - isBinding && - (lowerOutKey === 'href' || lowerOutKey === 'src' || lowerOutKey === 'xlink:href' || lowerOutKey === 'to') && - typeof outValue === 'string' && - isUnsafeUrlValue(outValue) - ) { + // The URL props are the ones `validateProp()` checks: `:link="frontmatter.x"` counts too. + if (isBinding && isUrlPropKey(outKey) && typeof outValue === 'string' && isUnsafeUrlValue(outValue)) { continue } diff --git a/packages/comark/test/plugins/security.test.ts b/packages/comark/test/plugins/security.test.ts index 203ffc10..aa63ffd9 100644 --- a/packages/comark/test/plugins/security.test.ts +++ b/packages/comark/test/plugins/security.test.ts @@ -335,6 +335,22 @@ home: javascript:alert(1) expect(html).not.toContain('javascript:') }) + it('blocks javascript: URLs bound from a non-URL frontmatter key to any URL prop', async () => { + const md = `--- +home: javascript:alert(1) +--- + +::my-card{:link="frontmatter.home" :image-url="frontmatter.home"} +::` + + const tree = await parseWithSecurity(md) + // `home` is not a URL key, so frontmatter sanitizing keeps it… + expect(tree.frontmatter.home).toBe('javascript:alert(1)') + // …and the render-time floor checks the same URL props as parse time. + const html = await render(tree, { format: 'text/html', blockSeparator: '\n' }) + expect(html).not.toContain('javascript:') + }) + it('strips framework HTML sink props from components', async () => { const tree = await parseWithSecurity( `\ diff --git a/packages/comark/test/resolve-attributes.test.ts b/packages/comark/test/resolve-attributes.test.ts index e234dd12..184878dc 100644 --- a/packages/comark/test/resolve-attributes.test.ts +++ b/packages/comark/test/resolve-attributes.test.ts @@ -176,6 +176,21 @@ describe('unsafe URL bindings (render-time hard floor)', () => { expect(result).toEqual({}) }) + it('drops any URL prop binding resolving to javascript:, by the same names as validateProp()', () => { + const result = resolveAttributes( + { + ':link': 'frontmatter.home', + ':image-url': 'frontmatter.home', + ':avatarSrc': 'frontmatter.home', + ':xlink:href': 'frontmatter.home', + ':title': 'frontmatter.home', + }, + makeRenderData({ frontmatter: { home: 'javascript:alert(1)' } }) + ) + // `title` is not a URL prop: prose stays, as for props. + expect(result).toEqual({ title: 'javascript:alert(1)' }) + }) + it('drops bindings whose resolved value is entity-encoded javascript:', () => { const result = resolveAttributes( { ':href': 'frontmatter.home' },