Merge pull request #476 from marp-team/enable-html-enum

[v3] `markdown.marp.html` setting to control which HTML elements are rendered

Author: yhatt

CHANGELOG.md

@@ -10,6 +10,14 @@
 - Upgrade Marp Core to [v4.0.1](https://github.com/marp-team/marp-core/releases/v4.0.1) ([#472](https://github.com/marp-team/marp-vscode/pull/472), [#474](https://github.com/marp-team/marp-vscode/pull/474))
 - Upgrade Marp CLI to [v4.0.4](https://github.com/marp-team/marp-cli/releases/v4.0.4) ([#473](https://github.com/marp-team/marp-vscode/pull/473), [#474](https://github.com/marp-team/marp-vscode/pull/474))
 
+### Added
+
+- `markdown.marp.html` setting to control rendering HTML within Marp Markdown ([#476](https://github.com/marp-team/marp-vscode/pull/476))
+
+### Deprecated
+
+- `markdown.marp.enableHtml` setting ([#476](https://github.com/marp-team/marp-vscode/pull/476))
+
 ### Changed
 
 - Upgrade development Node.js and dependent packages to the latest version ([#474](https://github.com/marp-team/marp-vscode/pull/474))

README.md

@@ -205,11 +205,16 @@ Features may be restricted are marked by the shield icon 🛡️ in this documen
 
 [workspace trust]: https://code.visualstudio.com/docs/editor/workspace-trust
 
-#### Enable HTML in Marp Markdown 🛡️
+#### HTML elements in Markdown 🛡️
 
-You can enable previsualization of HTML code within Marp Markdown with the `markdown.marp.enableHtml` option.
+In the trusted workspace, if Marp Markdown was included HTML elements, [only selectivity HTML elements by Marp](https://github.com/marp-team/marp-core/blob/main/src/html/allowlist.ts) can render by default. You can control which HTML elements will be rendered by setting the `markdown.marp.html` option,
 
-It could allow script injection from untrusted Markdown files. Thus, this feature is disabled as a default and will be _always ignored in the untrusted workspace_. Use with caution.
+You can control which HTML elements will be rendered by setting the `markdown.marp.html` option. You can set it as `all` to allow all HTML elements, but could allow script injection from untrusted Markdown files. Use with caution.
+
+In the untrusted workspace, HTML elements in Marp Markdown will be always ignored regardless of the selected option in `markdown.marp.html`.
+
+> [!NOTE]
+> A legacy setting `markdown.marp.enableHtml` is deprecated since v2. Please use `markdown.marp.html` instead.
 
 ## Web extension
 

package.json

@@ -50,6 +50,7 @@
       "supported": "limited",
       "description": "Workspace trust is required for exporting slide deck, and using themes configured in the workspace.",
       "restrictedConfigurations": [
+        "markdown.marp.html",
         "markdown.marp.enableHtml",
         "markdown.marp.themes"
       ]
@@ -119,10 +120,20 @@
           "default": "",
           "description": "Sets the custom path for Chrome or Chromium-based browser to export PDF, PPTX, and image. If it's empty, Marp will find out the installed Google Chrome / Chromium / Microsoft Edge."
         },
-        "markdown.marp.enableHtml": {
-          "type": "boolean",
-          "default": false,
-          "description": "Enables all HTML elements in Marp Markdown. This setting is working only in the trusted workspace."
+        "markdown.marp.html": {
+          "type": "string",
+          "enum": [
+            "off",
+            "default",
+            "all"
+          ],
+          "markdownEnumDescriptions": [
+            "Disable all HTML elements, including originally allowed HTML elements by Marp.",
+            "Enable only selectively enabled HTML elements by Marp. _([See the list of HTML elements](https://github.com/marp-team/marp-core/blob/main/src/html/allowlist.ts))_",
+            "Enable all HTML elements. This setting may become the slide rendering insecure."
+          ],
+          "default": "default",
+          "description": "Sets which HTML elements within Marp Markdown are enabled in rendered slides. If the workspace is not trusted, this setting treats as always \"off\"."
         },
         "markdown.marp.exportType": {
           "type": "string",
@@ -197,6 +208,12 @@
           "items": {
             "type": "string"
           }
+        },
+        "markdown.marp.enableHtml": {
+          "type": "boolean",
+          "default": false,
+          "description": "Enables all HTML elements in Marp Markdown. This setting is working only in the trusted workspace.",
+          "deprecationMessage": "The setting \"markdown.marp.enableHtml\" is deprecated. Please use \"markdown.marp.html\" instead."
         }
       }
     },

src/__mocks__/vscode.ts

@@ -7,6 +7,7 @@ const defaultConf: MockedConf = {
   'markdown.marp.breaks': 'on',
   'markdown.marp.chromePath': '',
   'markdown.marp.enableHtml': false,
+  'markdown.marp.html': 'default',
   'markdown.marp.exportType': 'pdf',
   'markdown.marp.outlineExtension': true,
   'markdown.marp.pdf.noteAnnotations': false,
@@ -227,7 +228,7 @@ export const window = {
   showQuickPick: jest.fn(),
   showSaveDialog: jest.fn(),
   showTextDocument: jest.fn(async () => ({})),
-  showWarningMessage: jest.fn(),
+  showWarningMessage: jest.fn(async () => undefined),
   withProgress: jest.fn(),
 }
 

src/extension.test.ts

@@ -5,7 +5,7 @@ import { Marp } from '@marp-team/marp-core'
 import dedent from 'dedent'
 import markdownIt from 'markdown-it'
 import * as nodeFetch from 'node-fetch'
-import { Memento, Uri, commands, workspace, env } from 'vscode'
+import { Memento, Uri, commands, window, workspace, env } from 'vscode'
 
 jest.mock('node-fetch')
 jest.mock('vscode')
@@ -177,23 +177,37 @@ describe('#extendMarkdownIt', () => {
       })
     })
 
-    describe('markdown.marp.enableHtml', () => {
-      it('does not render not allowed HTML elements when disabled', () => {
-        setConfiguration({ 'markdown.marp.enableHtml': false })
+    describe('markdown.marp.html', () => {
+      it('does not render not allowed HTML elements when the setting is "off"', () => {
+        setConfiguration({ 'markdown.marp.html': 'off' })
 
         const html = md().render(marpMd('<script>console.log("Hi")</script>'))
         expect(html).not.toContain('<script>console.log("Hi")</script>')
       })
 
-      it("allows Marp Core's allowed HTML elements even if disabled", () => {
-        setConfiguration({ 'markdown.marp.enableHtml': false })
+      it(`does not render Marp Core allowed HTML elements when the setting is "off"`, () => {
+        setConfiguration({ 'markdown.marp.html': 'off' })
+
+        const html = md().render(marpMd('line<br>break'))
+        expect(html).not.toContain('line<br />break')
+      })
+
+      it('does not render not allowed HTML elements when the setting is "default"', () => {
+        setConfiguration({ 'markdown.marp.html': 'default' })
+
+        const html = md().render(marpMd('<script>console.log("Hi")</script>'))
+        expect(html).not.toContain('<script>console.log("Hi")</script>')
+      })
+
+      it(`allows Marp Core's allowed HTML elements even if the setting is "default"`, () => {
+        setConfiguration({ 'markdown.marp.html': 'default' })
 
         const html = md().render(marpMd('line<br>break'))
         expect(html).toContain('line<br />break')
       })
 
-      it('renders not allowed HTML elements when enabled', () => {
-        setConfiguration({ 'markdown.marp.enableHtml': true })
+      it('renders not allowed HTML elements when the setting is "all"', () => {
+        setConfiguration({ 'markdown.marp.html': 'all' })
 
         const html = md().render(marpMd('<script>console.log("Hi")</script>'))
         expect(html).toContain('<script>console.log("Hi")</script>')
@@ -210,12 +224,51 @@ describe('#extendMarkdownIt', () => {
 
         afterEach(() => isTrustedMock?.mockRestore())
 
-        it('does not render not allowed HTML elements even if enabled', () => {
-          setConfiguration({ 'markdown.marp.enableHtml': true })
+        it('does not render not allowed HTML elements even the setting is "all"', () => {
+          setConfiguration({ 'markdown.marp.html': 'all' })
+
+          const br = md().render(marpMd('line<br>break'))
+          expect(br).not.toContain('line<br />break')
+
+          const script = md().render(
+            marpMd('<script>console.log("Hi")</script>'),
+          )
+          expect(script).not.toContain('<script>console.log("Hi")</script>')
+        })
+      })
+    })
+
+    describe('[Deprecated] markdown.marp.enableHtml', () => {
+      it('renders not allowed HTML elements when the value is true', () => {
+        setConfiguration({ 'markdown.marp.enableHtml': true })
+
+        const html = md().render(marpMd('<script>console.log("Hi")</script>'))
+        expect(html).toContain('<script>console.log("Hi")</script>')
+      })
 
-          const html = md().render(marpMd('<script>console.log("Hi")</script>'))
-          expect(html).not.toContain('<script>console.log("Hi")</script>')
+      it('prefers explicit "markdown.marp.html" setting than "markdown.marp.enableHtml" setting', () => {
+        setConfiguration({
+          'markdown.marp.html': 'off',
+          'markdown.marp.enableHtml': true,
         })
+
+        const html = md().render(marpMd('<script>console.log("Hi")</script>'))
+        expect(html).not.toContain('<script>console.log("Hi")</script>')
+      })
+
+      it('shows deprecation warning while rendering if "markdown.marp.html" is the default value and "markdown.marp.enableHtml" is enabled', () => {
+        setConfiguration({
+          'markdown.marp.html': 'default',
+          'markdown.marp.enableHtml': true,
+        })
+
+        md().render(marpMd('<script>console.log("Hi")</script>'))
+        expect(window.showWarningMessage).toHaveBeenCalledWith(
+          expect.stringContaining(
+            'The setting "markdown.marp.enableHtml" is deprecated',
+          ),
+          expect.anything(),
+        )
       })
     })
 

src/extension.ts

@@ -17,6 +17,7 @@ import { detectMarpFromMarkdown, marpConfiguration } from './utils'
 const shouldRefreshConfs = [
   'markdown.marp.breaks',
   'markdown.marp.enableHtml',
+  'markdown.marp.html',
   'markdown.marp.mathTypesetting',
   'markdown.marp.outlineExtension',
   'markdown.marp.themes',

src/option.test.ts

@@ -168,9 +168,9 @@ describe('Option', () => {
 
       afterEach(() => isTrustedMock?.mockRestore())
 
-      it('ignores potentially malicious options', async () => {
-        setConfiguration({ 'markdown.marp.enableHtml': true })
-        expect((await subject({ uri: untitledUri })).html).toBeUndefined()
+      it('disallows potentially malicious options', async () => {
+        setConfiguration({ 'markdown.marp.html': 'all' })
+        expect((await subject({ uri: untitledUri })).html).toBe(false)
       })
     })
   })

src/option.ts

@@ -3,7 +3,8 @@ import path from 'node:path'
 import { MarpOptions } from '@marp-team/marp-core'
 import { Options } from 'markdown-it'
 import { nanoid } from 'nanoid'
-import { TextDocument, Uri, workspace } from 'vscode'
+import { TextDocument, Uri, window, workspace } from 'vscode'
+import openExtensionSettings from './commands/open-extension-settings'
 import themes, { ThemeType } from './themes'
 import {
   marpConfiguration,
@@ -30,8 +31,22 @@ const breaks = (inheritedValue: boolean): boolean => {
   }
 }
 
-const enableHtml = () =>
-  marpConfiguration().get<boolean>('enableHtml') && workspace.isTrusted
+const html = () => {
+  if (workspace.isTrusted) {
+    const htmlConf = marpConfiguration().get<string>('html')
+
+    if (htmlConf === 'all') return { value: true }
+    if (htmlConf === 'off') return { value: false }
+
+    // Legacy configuration compatibility
+    const legacyConf = marpConfiguration().get<boolean>('enableHtml')
+    if (legacyConf) return { value: true, legacy: true }
+
+    return { value: undefined }
+  } else {
+    return { value: false }
+  }
+}
 
 const math = () => {
   const conf = mathTypesettingConfiguration()
@@ -55,10 +70,24 @@ export const marpCoreOptionForPreview = (
   baseOption: Options & MarpOptions,
 ): MarpOptions => {
   if (!cachedPreviewOption) {
+    const htmlOption = html()
+
+    if (htmlOption.legacy) {
+      // Show warning for legacy configuration
+      window
+        .showWarningMessage(
+          'The setting "markdown.marp.enableHtml" is deprecated. Please use "markdown.marp.html" instead. Please review your settings JSON to make silence this warning.',
+          'Open Extension Settings',
+        )
+        .then((selected) => {
+          if (selected) void openExtensionSettings()
+        })
+    }
+
     cachedPreviewOption = {
       container: { tag: 'div', id: '__marp-vscode' },
       slideContainer: { tag: 'div', 'data-marp-vscode-slide-wrapper': '' },
-      html: enableHtml() || undefined,
+      html: htmlOption.value,
       inlineSVG: {
         backdropSelector: false,
       },
@@ -87,7 +116,7 @@ export const marpCoreOptionForCLI = async (
     allowLocalFiles,
     pdfNotes,
     pdfOutlines: pdfOutlines(),
-    html: enableHtml() || undefined,
+    html: html().value,
     options: {
       markdown: {
         breaks: breaks(!!confMdPreview.get<boolean>('breaks')),