Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
11 changes: 8 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 21 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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."
}
}
},
Expand Down
3 changes: 2 additions & 1 deletion src/__mocks__/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
}

Expand Down
77 changes: 65 additions & 12 deletions src/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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>')
Expand All @@ -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(),
)
})
})

Expand Down
1 change: 1 addition & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
6 changes: 3 additions & 3 deletions src/option.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
Expand Down
39 changes: 34 additions & 5 deletions src/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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,
},
Expand Down Expand Up @@ -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')),
Expand Down