fix: don't leak ancestor props into elements whose bindings are all omitted

packages/comark-angular/src/components/markdown-node.component.ts

@@ -138,8 +138,15 @@ export class MarkdownNode implements OnChanges {
       // Resolve attributes (:binding support)
       const resolved = resolveAttributes(nodeProps, this.renderData, { parseJson: true })
 
-      // Build childrenRenderData - only shadow parent scope when element has own attrs
-      const hasOwnAttrs = Object.keys(resolved).length > 0
+      // Build childrenRenderData - only shadow parent scope when element has own attrs.
+      // The check uses the raw attributes: resolving can omit all of them.
+      let hasOwnAttrs = false
+      for (const key in nodeProps) {
+        if (key !== '$') {
+          hasOwnAttrs = true
+          break
+        }
+      }
       const childrenRenderData: NodeRenderData = hasOwnAttrs ? { ...this.renderData, props: resolved } : this.renderData
 
       if ((customComponent as StructuralComponent | undefined)?.ɵcomarkIf) {
@@ -373,7 +380,13 @@ export class MarkdownNode implements OnChanges {
           }
         } else {
           const resolved = resolveAttributes(childProps, renderData, { parseJson: true })
-          const hasOwnAttrs = Object.keys(resolved).length > 0
+          let hasOwnAttrs = false
+          for (const key in childProps) {
+            if (key !== '$') {
+              hasOwnAttrs = true
+              break
+            }
+          }
           const childRenderData: NodeRenderData = hasOwnAttrs ? { ...renderData, props: resolved } : renderData
           this.renderNativeEl(parentEl, childTag, resolved, grandChildren, childRenderData, child)
         }

packages/comark-angular/test/props-scope.test.ts

@@ -0,0 +1,69 @@
+import '@angular/compiler'
+import { describe, expect, it } from 'vitest'
+import { Component, provideZonelessChangeDetection, type Type } from '@angular/core'
+import { bootstrapApplication } from '@angular/platform-browser'
+import { renderApplication } from '@angular/platform-server'
+import { parseMarkdown, type MarkdownDocument as MarkdownDocumentType } from 'comark'
+import { MarkdownDocument } from '../src/components/markdown-document.component.ts'
+
+class Probe {
+  text = ''
+}
+Component({
+  selector: 'app-probe',
+  standalone: true,
+  inputs: ['text'],
+  template: '<i class="probe">[{{ text }}]</i>',
+})(Probe)
+
+async function renderMarkdown(markdown: string): Promise<string> {
+  const document = await parseMarkdown(markdown)
+  const components: Record<string, Type<unknown>> = { probe: Probe as Type<unknown> }
+
+  class App {
+    document: MarkdownDocumentType = document
+    components = components
+    data = { bad: 'javascript:alert(1)' }
+  }
+  Component({
+    selector: 'app-root',
+    standalone: true,
+    imports: [MarkdownDocument],
+    template: `
+      <comark-markdown-document
+        [value]="document"
+        [components]="components"
+        [data]="data"
+      />
+    `,
+  })(App)
+
+  return renderApplication(
+    (context) => bootstrapApplication(App, { providers: [provideZonelessChangeDetection()] }, context),
+    { document: '<!DOCTYPE html><html><head></head><body><app-root></app-root></body></html>' }
+  )
+}
+
+describe('MarkdownDocument: props scope when every binding is omitted', () => {
+  it('hides the grandparent props from children of an element with all attributes omitted', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad"}\n::probe{:text="props.label"}\n::\n:::\n::::'
+    )
+    expect(html).toContain('class="probe"')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('exposes the resolved attributes of the middle element to its children', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad" id="keep"}\n::probe{:text="props.id"}\n::\n:::\n::::'
+    )
+    expect(html).toContain('[keep]')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('keeps the parent props visible through bare wrappers', async () => {
+    const html = await renderMarkdown('::::wrap{label="Outer"}\n- :probe{:text="props.label"}\n::::')
+    expect(html).toContain('<ul>')
+    expect(html).toContain('[Outer]')
+  })
+})

packages/comark-html/test/props-scope.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from 'vitest'
+import { parseMarkdown } from 'comark'
+import { renderHtmlFromDocument } from '../src/index'
+
+const UNSAFE = { bad: 'javascript:alert(1)' }
+
+async function render(markdown: string, data: Record<string, unknown> = UNSAFE): Promise<string> {
+  return renderHtmlFromDocument(await parseMarkdown(markdown), { data })
+}
+
+describe('props scope when every binding is omitted', () => {
+  it('does not leak the parent props into children of an element whose bindings are all omitted', async () => {
+    const html = await render('::card{title="T"}\n:::div{:href="data.bad"}\n:span[x]{:data-x="props.title"}\n:::\n::')
+    expect(html).not.toContain('javascript:')
+    expect(html).not.toContain('data-x="T"')
+  })
+
+  it('exposes the resolved attributes of the middle element to its children', async () => {
+    const html = await render(
+      '::card{title="T"}\n:::div{:href="data.bad" id="keep"}\n:span[x]{:data-x="props.id"}\n:::\n::'
+    )
+    expect(html).toContain('data-x="keep"')
+    expect(html).not.toContain('data-x="T"')
+  })
+
+  it('keeps the parent props visible through bare wrappers', async () => {
+    const html = await render('::card{title="T"}\n- :span[x]{:data-x="props.title"}\n::')
+    expect(html).toContain('<ul>')
+    expect(html).toContain('data-x="T"')
+    expect(html).not.toMatch(/<(ul|li)[^>]* title=/)
+  })
+})

packages/comark-react/src/components/MarkdownDocument.tsx

@@ -176,8 +176,15 @@ function renderNode(
 
     // Only shadow the parent's `props` scope when the current element has its
     // own attributes. Bare wrappers (`<p>`, `<ul>`, `<li>`, …) must keep the
-    // parent's scope so bindings like `{{ props.x }}` reach across them.
-    const hasOwnAttrs = Object.keys(resolved).length > 0
+    // parent's scope so bindings like `{{ props.x }}` reach across them. The
+    // check uses the raw attributes: resolving can omit all of them.
+    let hasOwnAttrs = false
+    for (const key in nodeProps) {
+      if (key !== '$') {
+        hasOwnAttrs = true
+        break
+      }
+    }
     const childrenRenderData: NodeRenderData = hasOwnAttrs ? { ...renderData, props: scopeProps } : renderData
     // Separate template elements (slots) from regular children
     const slots: Record<string, React.ReactNode[]> = {}

packages/comark-react/test/props-scope.test.tsx

@@ -0,0 +1,46 @@
+import { describe, expect, it } from 'vitest'
+import React from 'react'
+import { renderToString } from 'react-dom/server'
+import { parseMarkdown } from 'comark'
+import { MarkdownDocument } from '../src/components/MarkdownDocument'
+
+function Probe({ text = '' }: { text?: string }) {
+  return <i className="probe">{`[${text}]`}</i>
+}
+
+async function renderMarkdown(markdown: string, props: Record<string, any> = {}) {
+  const tree = await parseMarkdown(markdown)
+  return renderToString(
+    <MarkdownDocument
+      value={tree}
+      components={{ probe: Probe }}
+      {...props}
+    />
+  )
+}
+
+describe('MarkdownDocument: props scope when every binding is omitted', () => {
+  it('hides the grandparent props from children of an element with all attributes omitted', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad"}\n::probe{:text="props.label"}\n::\n:::\n::::',
+      { data: { bad: 'javascript:alert(1)' } }
+    )
+    expect(html).toContain('class="probe"')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('exposes the resolved attributes of the middle element to its children', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad" id="keep"}\n::probe{:text="props.id"}\n::\n:::\n::::',
+      { data: { bad: 'javascript:alert(1)' } }
+    )
+    expect(html).toContain('[keep]')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('keeps the parent props visible through bare wrappers', async () => {
+    const html = await renderMarkdown('::::wrap{label="Outer"}\n- :probe{:text="props.label"}\n::::')
+    expect(html).toContain('<ul>')
+    expect(html).toContain('[Outer]')
+  })
+})

packages/comark-svelte/src/components/MarkdownNode.svelte

@@ -191,11 +191,20 @@ naturally appears inline after the deepest trailing text node.
 
   // Only shadow the parent's `props` scope when the current element has its
   // own attributes. Bare wrappers (`<p>`, `<ul>`, `<li>`, …) must keep the
-  // parent's scope so bindings like `{{ props.x }}` reach across them.
+  // parent's scope so bindings like `{{ props.x }}` reach across them. The
+  // check uses the raw attributes: resolving can omit all of them.
   let nativeProps = $derived(tag ? toNativeAttributes(tag, node, mappedProps) : mappedProps)
 
+  let hasOwnAttrs = $derived.by(() => {
+    const attrs: Record<string, any> = (Array.isArray(node) && node.length >= 2 ? node[1] : null) ?? {}
+    for (const key in attrs) {
+      if (key !== '$') return true
+    }
+    return false
+  })
+
   let childrenRenderData = $derived<NodeRenderData>(
-    Object.keys(mappedProps).length > 0
+    hasOwnAttrs
       ? { ...renderData, props: mappedProps }
       : renderData,
   )

packages/comark-svelte/test/props-scope.test.ts

@@ -0,0 +1,34 @@
+import { describe, expect, it } from 'vitest'
+import { render } from 'svelte/server'
+import { parseMarkdown } from 'comark'
+import MarkdownDocument from '../src/components/MarkdownDocument.svelte'
+import Probe from './test-components/Probe.svelte'
+
+async function renderMarkdown(markdown: string, data: Record<string, unknown> = { bad: 'javascript:alert(1)' }) {
+  const value = await parseMarkdown(markdown)
+  return render(MarkdownDocument, { props: { value, components: { probe: Probe }, data } }).body
+}
+
+describe('MarkdownDocument: props scope when every binding is omitted', () => {
+  it('hides the grandparent props from children of an element with all attributes omitted', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad"}\n::probe{:text="props.label"}\n::\n:::\n::::'
+    )
+    expect(html).toContain('class="probe"')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('exposes the resolved attributes of the middle element to its children', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad" id="keep"}\n::probe{:text="props.id"}\n::\n:::\n::::'
+    )
+    expect(html).toContain('[keep]')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('keeps the parent props visible through bare wrappers', async () => {
+    const html = await renderMarkdown('::::wrap{label="Outer"}\n- :probe{:text="props.label"}\n::::')
+    expect(html).toContain('<ul>')
+    expect(html).toContain('[Outer]')
+  })
+})

packages/comark-svelte/test/test-components/Probe.svelte

@@ -0,0 +1,5 @@
+<script lang="ts">
+  let { text = '' }: { text?: string } = $props()
+</script>
+
+<i class="probe">[{text}]</i>

packages/comark-vue/src/components/MarkdownDocument.ts

@@ -171,8 +171,15 @@ function renderNode(
 
     // Only shadow the parent's `props` scope when the current element has its
     // own attributes. Bare wrappers (`<p>`, `<ul>`, `<li>`, …) must keep the
-    // parent's scope so bindings like `{{ props.x }}` reach across them.
-    const hasOwnAttrs = Object.keys(resolved).length > 0
+    // parent's scope so bindings like `{{ props.x }}` reach across them. The
+    // check uses the raw attributes: resolving can omit all of them.
+    let hasOwnAttrs = false
+    for (const key in nodeProps) {
+      if (key !== '$') {
+        hasOwnAttrs = true
+        break
+      }
+    }
     const childrenRenderData = hasOwnAttrs ? { ...renderData, props: scopeProps } : renderData
     // Separate template elements (slots) from regular children
     const slots: Record<string, () => (VNode | string)[]> = {}

packages/comark-vue/test/props-scope.test.ts

@@ -0,0 +1,46 @@
+import { describe, expect, it } from 'vitest'
+import { createSSRApp, defineComponent, h } from 'vue'
+import { renderToString } from '@vue/server-renderer'
+import { parseMarkdown } from 'comark'
+import { MarkdownDocument } from '../src/components/MarkdownDocument'
+
+const Probe = defineComponent({
+  props: { text: { type: String, default: '' } },
+  setup(props) {
+    return () => h('i', { class: 'probe' }, `[${props.text}]`)
+  },
+})
+
+async function renderMarkdown(markdown: string, props: Record<string, any> = {}) {
+  const tree = await parseMarkdown(markdown)
+  const app = createSSRApp({
+    setup: () => () => h(MarkdownDocument, { value: tree, components: { probe: Probe }, ...props }),
+  })
+  return renderToString(app as any)
+}
+
+describe('MarkdownDocument: props scope when every binding is omitted', () => {
+  it('hides the grandparent props from children of an element with all attributes omitted', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad"}\n::probe{:text="props.label"}\n::\n:::\n::::',
+      { data: { bad: 'javascript:alert(1)' } }
+    )
+    expect(html).toContain('class="probe"')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('exposes the resolved attributes of the middle element to its children', async () => {
+    const html = await renderMarkdown(
+      '::::wrap{label="Outer"}\n:::div{:href="data.bad" id="keep"}\n::probe{:text="props.id"}\n::\n:::\n::::',
+      { data: { bad: 'javascript:alert(1)' } }
+    )
+    expect(html).toContain('[keep]')
+    expect(html).not.toContain('[Outer]')
+  })
+
+  it('keeps the parent props visible through bare wrappers', async () => {
+    const html = await renderMarkdown('::::wrap{label="Outer"}\n- :probe{:text="props.label"}\n::::')
+    expect(html).toContain('<ul>')
+    expect(html).toContain('[Outer]')
+  })
+})

packages/comark/src/internal/stringify/state.ts

@@ -58,12 +58,22 @@ export async function one(node: Node, state: State, parent?: ElementNode, atLine
   // element's values, regardless of which handler (html / ansi / user) runs.
   // Elements with no attributes (e.g. an auto-generated `<p>` wrapper) must
   // NOT shadow the parent's scope, otherwise `{{ props.* }}` inside them would
-  // resolve to nothing.
+  // resolve to nothing. The check uses the raw attributes, because resolving
+  // can drop all of them and the parent's props must not leak back in.
   const prevRenderData = state.renderData
   if (state.renderData && node[1]) {
-    const resolved = resolveAttributes(node[1] as Record<string, unknown>, prevRenderData)
-    if (Object.keys(resolved).length > 0) {
-      state.renderData = { ...prevRenderData, props: resolved }
+    let hasOwnAttrs = false
+    for (const key in node[1]) {
+      if (key !== '$') {
+        hasOwnAttrs = true
+        break
+      }
+    }
+    if (hasOwnAttrs) {
+      state.renderData = {
+        ...prevRenderData,
+        props: resolveAttributes(node[1] as Record<string, unknown>, prevRenderData),
+      }
     }
   }
 

test/bundle.test.ts

@@ -60,13 +60,13 @@ describe('package bundle size', { timeout: 60_000 }, () => {
 
     expect(report).toMatchInlineSnapshot(`
       {
-        "@comark/angular": "56.2k (72 files)",
+        "@comark/angular": "56.5k (72 files)",
         "@comark/ansi": "37.3k (98 files)",
         "@comark/html": "16.5k (58 files)",
         "@comark/nuxt": "11.9k (58 files)",
-        "@comark/react": "38.5k (76 files)",
-        "@comark/svelte": "46.5k (84 files)",
-        "@comark/vue": "56.2k (80 files)",
+        "@comark/react": "38.6k (76 files)",
+        "@comark/svelte": "46.8k (84 files)",
+        "@comark/vue": "56.4k (80 files)",
         "comark": "385k (152 files)",
       }
     `)