From ec434402eb725644d14fbcf7ad0dd81a3fe692ec Mon Sep 17 00:00:00 2001 From: Gxrvish Date: Mon, 10 Aug 2026 23:46:22 +0530 Subject: [PATCH] feat(csp): drop inline style attributes from editor markup The editor builds most of its chrome by assigning HTML strings, so every `style="..."` literal in a template is parsed as an inline style attribute and blocked by a strict `style-src-attr` policy. `setAttribute('style', ...)` has the same problem. CSSOM writes (`el.style.prop = value`) are not covered by CSP, so they are the way to apply values only known at runtime. Static styles move to utility classes, adding `gjs-pointer-events-all` and `gjs-clear-float` next to the existing `gjs-hidden` and `gjs-no-pointer-events`: canvas and frame tools, frame remove icon, modal collector, device add button and the float clearers in the asset manager, file uploader, modal and style manager. Runtime values move to the CSSOM through a new `setStyleText` helper, which applies a declaration string property by property. It splits only on top-level `;`, so data URLs and quoted values survive, and it keeps `!important` and custom properties: asset preview background, navigator indentation, style manager layer preview, select option styles in the style and trait managers, RTE action `style` attributes, the canvas iframe component and the color picker swatches. Elements whose visibility is toggled at runtime by resetting the inline display keep using the CSSOM for their initial state, so the toggles still work. The style manager clear button instead switches to toggling `gjs-hidden`, since its update is debounced and a class avoids a flash on render. Out of scope: the `style` attribute of user components (`ComponentView`), which is the content the editor exists to author, and the SVG image placeholder, which is serialized to a base64 data URL and governed by `img-src`. Both are recorded in the allowlist of the new guard spec. --- .../src/asset_manager/view/AssetImageView.ts | 15 ++++- .../core/src/asset_manager/view/AssetsView.ts | 4 +- .../src/asset_manager/view/FileUploader.ts | 3 +- packages/core/src/canvas/view/CanvasView.ts | 6 +- .../core/src/canvas/view/FrameWrapView.ts | 10 +-- packages/core/src/commands/view/Preview.ts | 4 +- .../src/device_manager/view/DevicesView.ts | 2 +- .../dom_components/view/ComponentFrameView.ts | 6 +- .../core/src/modal_dialog/view/ModalView.ts | 4 +- packages/core/src/navigator/view/ItemView.ts | 9 ++- .../rich_text_editor/model/RichTextEditor.ts | 6 +- .../selector_manager/view/ClassTagsView.ts | 4 +- .../core/src/style_manager/view/LayerView.ts | 13 ++-- .../style_manager/view/PropertyFileView.ts | 4 +- .../style_manager/view/PropertySelectView.ts | 11 +++- .../src/style_manager/view/PropertyView.ts | 10 +-- .../styles/scss/_gjs_category_general.scss | 8 +++ packages/core/src/styles/scss/spectrum.scss | 1 + .../src/trait_manager/view/TraitSelectView.ts | 14 +++-- packages/core/src/utils/ColorPicker.ts | 24 ++++--- packages/core/src/utils/dom.ts | 61 ++++++++++++++++++ .../core/test/specs/commands/view/Preview.ts | 1 + .../style_manager/view/PropertySelectView.ts | 5 +- .../specs/utils/noInlineStyleAttributes.ts | 62 ++++++++++++++++++ .../core/test/specs/utils/setStyleText.ts | 63 +++++++++++++++++++ 25 files changed, 293 insertions(+), 57 deletions(-) create mode 100644 packages/core/test/specs/utils/noInlineStyleAttributes.ts create mode 100644 packages/core/test/specs/utils/setStyleText.ts diff --git a/packages/core/src/asset_manager/view/AssetImageView.ts b/packages/core/src/asset_manager/view/AssetImageView.ts index 0853f9d4f..78f9a9be4 100644 --- a/packages/core/src/asset_manager/view/AssetImageView.ts +++ b/packages/core/src/asset_manager/view/AssetImageView.ts @@ -5,14 +5,23 @@ import html from '../../utils/html'; export default class AssetImageView extends AssetView { getPreview() { - const { pfx, ppfx, model } = this; - const src = model.get('src'); + const { pfx, ppfx } = this; return html` -
+
`; } + render() { + super.render(); + // Set through the CSSOM, a `style` attribute would be blocked by a strict + // `style-src-attr` policy + const previewEl = this.el.querySelector('[data-preview]') as HTMLElement; + const src = this.model.get('src'); + previewEl && previewEl.style.setProperty('background-image', src ? `url(${JSON.stringify(src)})` : ''); + return this; + } + getInfo() { const { pfx, model } = this; let name = model.get('name'); diff --git a/packages/core/src/asset_manager/view/AssetsView.ts b/packages/core/src/asset_manager/view/AssetsView.ts index a62638c00..b4dbc77f7 100644 --- a/packages/core/src/asset_manager/view/AssetsView.ts +++ b/packages/core/src/asset_manager/view/AssetsView.ts @@ -20,7 +20,7 @@ export default class AssetsView extends View { -
+
`; } @@ -31,7 +31,7 @@ export default class AssetsView extends View { ${form}
-
+
`; } diff --git a/packages/core/src/asset_manager/view/FileUploader.ts b/packages/core/src/asset_manager/view/FileUploader.ts index 900a19113..8744d9aa6 100644 --- a/packages/core/src/asset_manager/view/FileUploader.ts +++ b/packages/core/src/asset_manager/view/FileUploader.ts @@ -48,6 +48,7 @@ export default class FileUploaderView extends View { uploadForm?: HTMLFormElement | null; template({ pfx, title, uploadId, disabled, multiUpload }: FileUploaderTemplateProps) { + const { ppfx } = this; return html`
${title}
@@ -60,7 +61,7 @@ export default class FileUploaderView extends View { ${disabled ? 'disabled' : ''} ${multiUpload ? 'multiple' : ''} /> -
+
`; } diff --git a/packages/core/src/canvas/view/CanvasView.ts b/packages/core/src/canvas/view/CanvasView.ts index 02ed35e4e..4f2712a4b 100644 --- a/packages/core/src/canvas/view/CanvasView.ts +++ b/packages/core/src/canvas/view/CanvasView.ts @@ -668,16 +668,16 @@ export default class CanvasView extends ModuleView { const toolsWrp = $el.find('[data-tools]'); this.toolsWrapper = toolsWrp.get(0); toolsWrp.append(` -
+
-
+
${config.extHl ? `
` : ''}
-
+
diff --git a/packages/core/src/canvas/view/FrameWrapView.ts b/packages/core/src/canvas/view/FrameWrapView.ts index 1c9f97347..595a366cb 100644 --- a/packages/core/src/canvas/view/FrameWrapView.ts +++ b/packages/core/src/canvas/view/FrameWrapView.ts @@ -204,7 +204,7 @@ export default class FrameWrapView extends ModuleView { ${model.get('name') || ''}
- @@ -218,8 +218,7 @@ export default class FrameWrapView extends ModuleView { const elTools = createEl( 'div', { - class: `${ppfx}tools`, - style: 'pointer-events:none; display: none', + class: `${ppfx}tools ${ppfx}no-pointer-events`, }, `
@@ -228,7 +227,7 @@ export default class FrameWrapView extends ModuleView {
-
+
@@ -247,6 +246,9 @@ export default class FrameWrapView extends ModuleView {
`, ); + // Kept on the CSSOM instead of a class, `toggleToolsEl` shows it back by + // resetting the inline display + elTools.style.display = 'none'; this.elTools = elTools; const twrp = cv?.toolsWrapper; twrp && twrp.appendChild(elTools); // TODO remove on frame remove diff --git a/packages/core/src/commands/view/Preview.ts b/packages/core/src/commands/view/Preview.ts index 877753925..6335067f0 100644 --- a/packages/core/src/commands/view/Preview.ts +++ b/packages/core/src/commands/view/Preview.ts @@ -102,7 +102,9 @@ export default class CommandPreview extends CommandAbstract { panels.forEach((panel) => panel.set('visible', true)); const canvas = editor.Canvas.getElement(); - canvas.setAttribute('style', ''); + // Removing beats writing an empty `style`, which a strict `style-src-attr` + // policy would still report + canvas.removeAttribute('style'); selected && editor.select(selected); delete this.selected; diff --git a/packages/core/src/device_manager/view/DevicesView.ts b/packages/core/src/device_manager/view/DevicesView.ts index 4ec0c5a67..f826ef1c3 100644 --- a/packages/core/src/device_manager/view/DevicesView.ts +++ b/packages/core/src/device_manager/view/DevicesView.ts @@ -25,7 +25,7 @@ export default class DevicesView extends View {
- + `; } diff --git a/packages/core/src/dom_components/view/ComponentFrameView.ts b/packages/core/src/dom_components/view/ComponentFrameView.ts index 7efc9e49e..70a3a0b3a 100644 --- a/packages/core/src/dom_components/view/ComponentFrameView.ts +++ b/packages/core/src/dom_components/view/ComponentFrameView.ts @@ -1,5 +1,5 @@ import ComponentView from './ComponentView'; -import { createEl, find, attrUp } from '../../utils/dom'; +import { createEl, find, attrUp, setStyleText } from '../../utils/dom'; import ComponentFrame from '../model/ComponentFrame'; export default class ComponentFrameView extends ComponentView { @@ -21,9 +21,11 @@ export default class ComponentFrameView extends ComponentView { super.render(); const frame = createEl('iframe', { class: `${this.ppfx}no-pointer`, - style: 'width: 100%; height: 100%; border: none', src: this.__getSrc(), }); + // Set through the CSSOM, a `style` attribute would be blocked by a strict + // `style-src-attr` policy + setStyleText(frame, 'width: 100%; height: 100%; border: none'); this.el.appendChild(frame); return this; } diff --git a/packages/core/src/modal_dialog/view/ModalView.ts b/packages/core/src/modal_dialog/view/ModalView.ts index 225a9720a..ea48b38a9 100644 --- a/packages/core/src/modal_dialog/view/ModalView.ts +++ b/packages/core/src/modal_dialog/view/ModalView.ts @@ -10,10 +10,10 @@ export default class ModalView extends ModuleView {
${content}
-
+
- `; +
`; } events() { diff --git a/packages/core/src/navigator/view/ItemView.ts b/packages/core/src/navigator/view/ItemView.ts index 9faa1f7de..8247f7942 100644 --- a/packages/core/src/navigator/view/ItemView.ts +++ b/packages/core/src/navigator/view/ItemView.ts @@ -49,8 +49,6 @@ export default class ItemView extends View { const clsTitle = `${this.clsTitle} ${addClass}`; const clsTitleC = `${this.clsTitleC}`; const clsInput = `${this.inputNameCls} ${clsNoEdit} ${ppfx}no-app`; - const level = opt.level || 0; - const gut = `${level * 10}px`; const name = model.getName(); const icon = model.getIcon(); const clsBase = `${pfx}layer`; @@ -69,7 +67,7 @@ export default class ItemView extends View { : '' }
-
+
${chevron} ${icon ? `${icon}` : ''} @@ -434,6 +432,11 @@ export default class ItemView extends View { el.find(`.${this.clsChildren}`).append(children); } + // Set through the CSSOM, a `style` attribute would be blocked by a strict + // `style-src-attr` policy + const titleEl = this.el.querySelector('[data-title-indent]') as HTMLElement; + titleEl && (titleEl.style.paddingLeft = `${(opt.level || 0) * 10}px`); + !module.isVisible(model) && (this.className += ` ${pfx}hide`); hidden && (this.className += ` ${ppfx}hidden`); el.attr('class', this.className!); diff --git a/packages/core/src/rich_text_editor/model/RichTextEditor.ts b/packages/core/src/rich_text_editor/model/RichTextEditor.ts index bbb22c2d4..7a30035c7 100644 --- a/packages/core/src/rich_text_editor/model/RichTextEditor.ts +++ b/packages/core/src/rich_text_editor/model/RichTextEditor.ts @@ -4,7 +4,7 @@ import { isString } from 'underscore'; import RichTextEditorModule from '..'; import EditorModel from '../../editor/model/Editor'; -import { getPointerEvent, off, on } from '../../utils/dom'; +import { getPointerEvent, off, on, setStyleText } from '../../utils/dom'; import { getComponentModel } from '../../utils/mixins'; export interface RichTextEditorAction { @@ -369,7 +369,9 @@ export default class RichTextEditor { action.btn = btn; for (let key in attr) { - btn.setAttribute(key, attr[key]); + // `style` goes through the CSSOM, writing the attribute would be + // blocked by a strict `style-src-attr` policy + key === 'style' ? setStyleText(btn, attr[key]) : btn.setAttribute(key, attr[key]); } if (typeof icon == 'string') { diff --git a/packages/core/src/selector_manager/view/ClassTagsView.ts b/packages/core/src/selector_manager/view/ClassTagsView.ts index da84e91f1..6849b8206 100644 --- a/packages/core/src/selector_manager/view/ClassTagsView.ts +++ b/packages/core/src/selector_manager/view/ClassTagsView.ts @@ -32,7 +32,7 @@ export default class ClassTagsView extends View {
$${iconAdd} - + $${iconSync}
${labelInfo}:
@@ -436,6 +436,8 @@ export default class ClassTagsView extends View { this.$classes = $el.find('#' + pfx + 'tags-c'); this.$btnSyncEl = $el.find('[data-sync-style]'); this.$input.hide(); + // Hidden through the CSSOM, `updateSelector` brings it back with `show()` + this.$btnSyncEl.hide(); this.renderStates(); this.renderClasses(); $el.attr('class', `${this.className} ${ppfx}one-bg ${ppfx}two-color`); diff --git a/packages/core/src/style_manager/view/LayerView.ts b/packages/core/src/style_manager/view/LayerView.ts index ffe551fbf..2b36d2d66 100644 --- a/packages/core/src/style_manager/view/LayerView.ts +++ b/packages/core/src/style_manager/view/LayerView.ts @@ -36,7 +36,7 @@ export default class LayerView extends View { ${iconMove}
-
diff --git a/packages/core/src/style_manager/view/PropertySelectView.ts b/packages/core/src/style_manager/view/PropertySelectView.ts index 0a3c4eb7e..95c734279 100644 --- a/packages/core/src/style_manager/view/PropertySelectView.ts +++ b/packages/core/src/style_manager/view/PropertySelectView.ts @@ -1,3 +1,4 @@ +import { setStyleText } from '../../utils/dom'; import PropertySelect from '../model/PropertySelect'; import PropertyView from './PropertyView'; @@ -31,19 +32,23 @@ export default class PropertySelectView extends PropertyView { if (!this.input) { const optionsRes: string[] = []; + const optionsStyle: string[] = []; options.forEach((option) => { const id = model.getOptionId(option); const name = model.getOptionLabel(id); - const style = option.style ? option.style.replace(/"/g, '"') : ''; - const styleAttr = style ? `style="${style}"` : ''; const value = id.replace(/"/g, '"'); - optionsRes.push(``); + optionsStyle.push(option.style || ''); + optionsRes.push(``); }); const inputH = this.el.querySelector(`#${pfx}input-holder`)!; inputH.innerHTML = ``; this.input = inputH.firstChild as HTMLInputElement; + // Option styles are applied through the CSSOM, a `style` attribute would + // be blocked by a strict `style-src-attr` policy + const optionEls = this.input.querySelectorAll('option'); + optionsStyle.forEach((style, i) => style && setStyleText(optionEls[i] as HTMLElement, style)); } } diff --git a/packages/core/src/style_manager/view/PropertyView.ts b/packages/core/src/style_manager/view/PropertyView.ts index 802a3be6b..f6fa0bd0d 100644 --- a/packages/core/src/style_manager/view/PropertyView.ts +++ b/packages/core/src/style_manager/view/PropertyView.ts @@ -82,7 +82,7 @@ export default class PropertyView extends View { } templateLabel(model: Property) { - const { pfx, em } = this; + const { pfx, ppfx, em } = this; const { parent } = model; const { icon = '', info = '' } = model.attributes; const icons = em?.getConfig().icons; @@ -92,7 +92,7 @@ export default class PropertyView extends View { ${model.getLabel()} - ${!parent ? `` : ''} + ${!parent ? `
${iconClose}
` : ''} `; } @@ -123,13 +123,13 @@ export default class PropertyView extends View { const computedCls = `${ppfx}color-warn`; const labelEl = this.$el.children(`.${pfx}label`); const clearStyleEl = this.getClearEl(); - const clearStyle = clearStyleEl ? clearStyleEl.style : ({} as CSSStyleDeclaration); + const hiddenCls = `${ppfx}hidden`; labelEl.removeClass(`${updatedCls} ${computedCls}`); - clearStyle.display = 'none'; + clearStyleEl?.classList.add(hiddenCls); if (model.hasValue({ noParent: true }) && config.highlightChanged) { labelEl.addClass(updatedCls); - config.clearProperties && (clearStyle.display = ''); + config.clearProperties && clearStyleEl?.classList.remove(hiddenCls); } else if (model.hasValue() && config.highlightComputed) { labelEl.addClass(computedCls); } diff --git a/packages/core/src/styles/scss/_gjs_category_general.scss b/packages/core/src/styles/scss/_gjs_category_general.scss index 6e77397e5..f2ff79fd7 100644 --- a/packages/core/src/styles/scss/_gjs_category_general.scss +++ b/packages/core/src/styles/scss/_gjs_category_general.scss @@ -57,6 +57,14 @@ pointer-events: none; } +.#{gjs_vars.$app-prefix}pointer-events-all { + pointer-events: all; +} + +.#{gjs_vars.$app-prefix}clear-float { + clear: both; +} + .no-select { @include gjs_main_mixins.user-select(none); } diff --git a/packages/core/src/styles/scss/spectrum.scss b/packages/core/src/styles/scss/spectrum.scss index 9e17b1f1d..75ec52dfd 100644 --- a/packages/core/src/styles/scss/spectrum.scss +++ b/packages/core/src/styles/scss/spectrum.scss @@ -603,6 +603,7 @@ See http://bgrins.github.io/spectrum/themes/ for instructions. } .sp-clear-display { + background-color: transparent; background-repeat: no-repeat; background-position: center; background-image: url(data:image/gif;base64,R0lGODlhFAAUAPcAAAAAAJmZmZ2dnZ6enqKioqOjo6SkpKWlpaampqenp6ioqKmpqaqqqqurq/Hx8fLy8vT09PX19ff39/j4+Pn5+fr6+vv7+wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAEAAP8ALAAAAAAUABQAAAihAP9FoPCvoMGDBy08+EdhQAIJCCMybCDAAYUEARBAlFiQQoMABQhKUJBxY0SPICEYHBnggEmDKAuoPMjS5cGYMxHW3IiT478JJA8M/CjTZ0GgLRekNGpwAsYABHIypcAgQMsITDtWJYBR6NSqMico9cqR6tKfY7GeBCuVwlipDNmefAtTrkSzB1RaIAoXodsABiZAEFB06gIBWC1mLVgBa0AAOw==); diff --git a/packages/core/src/trait_manager/view/TraitSelectView.ts b/packages/core/src/trait_manager/view/TraitSelectView.ts index 44af3e70d..570bc7df2 100644 --- a/packages/core/src/trait_manager/view/TraitSelectView.ts +++ b/packages/core/src/trait_manager/view/TraitSelectView.ts @@ -1,5 +1,6 @@ import { isString, isUndefined } from 'underscore'; import { $ } from '../../common'; +import { setStyleText } from '../../utils/dom'; import TraitView from './TraitView'; export default class TraitSelectView extends TraitView { @@ -31,8 +32,9 @@ export default class TraitSelectView extends TraitView { const values: string[] = []; let input = ''; this.$input = $(input); + // Option styles are applied through the CSSOM, a `style` attribute would + // be blocked by a strict `style-src-attr` policy + const optionEls = this.$input!.get(0)!.querySelectorAll('option'); + styles.forEach((style, i) => style && setStyleText(optionEls[i] as HTMLElement, style)); const val = model.getTargetValue(); const valResult = values.indexOf(val) >= 0 ? val : model.get('default'); !isUndefined(valResult) && this.$input!.val(valResult); diff --git a/packages/core/src/utils/ColorPicker.ts b/packages/core/src/utils/ColorPicker.ts index d6a8aa49e..8d5b29609 100644 --- a/packages/core/src/utils/ColorPicker.ts +++ b/packages/core/src/utils/ColorPicker.ts @@ -5,6 +5,7 @@ // https://github.com/bgrins/spectrum // Author: Brian Grinstead // License: MIT +import { setStyleText } from './dom'; import { hasWin } from './mixins'; export interface ColorPickerOptions { @@ -159,6 +160,16 @@ export default function ($, undefined?: any) { ].join(''); })(); + // Swatch colors are known only at runtime, so they are carried by a data + // attribute and moved to the CSSOM once in the DOM: writing them as a `style` + // attribute would be blocked by a strict `style-src-attr` policy. + function applySwatchStyles($container) { + $container.find('[data-swatch-style]').each(function (i, el) { + setStyleText(el, el.getAttribute('data-swatch-style')); + el.removeAttribute('data-swatch-style'); + }); + } + function paletteTemplate(p, color, className, opts) { var html = []; for (var i = 0; i < p.length; i++) { @@ -176,20 +187,15 @@ export default function ($, undefined?: any) { tiny.toRgbString() + '" class="' + c + - '">', + '">', ); } else { var cls = 'sp-clear-display'; html.push( $('
') - .append( - $('').attr( - 'title', - opts.noColorSelectedText, - ), - ) + .append($('').attr('title', opts.noColorSelectedText)) .html(), ); } @@ -598,6 +604,7 @@ export default function ($, undefined?: any) { } paletteContainer.html(html.join('')); + applySwatchStyles(paletteContainer); } function drawInitial() { @@ -605,6 +612,7 @@ export default function ($, undefined?: any) { var initial = colorOnShow; var current = get(); initialColorContainer.html(paletteTemplate([initial, current], current, 'sp-palette-row-initial', opts)); + applySwatchStyles(initialColorContainer); } } diff --git a/packages/core/src/utils/dom.ts b/packages/core/src/utils/dom.ts index b45fea3d8..89e443826 100644 --- a/packages/core/src/utils/dom.ts +++ b/packages/core/src/utils/dom.ts @@ -105,6 +105,67 @@ export const createStyleEl = (css = '', nonce?: string, attributes: ObjectAny = return el; }; +/** + * Split a CSS declaration string on the top-level `;`, ignoring the ones nested + * in functions or strings (eg. `background: url(data:image/png;base64,...)`). + */ +const splitDeclarations = (style: string) => { + const result: string[] = []; + let current = ''; + let depth = 0; + let quote = ''; + + for (let i = 0; i < style.length; i++) { + const char = style[i]; + + if (quote) { + char === quote && (quote = ''); + } else if (char === '"' || char === "'") { + quote = char; + } else if (char === '(') { + depth++; + } else if (char === ')') { + depth = Math.max(0, depth - 1); + } else if (char === ';' && !depth) { + result.push(current); + current = ''; + continue; + } + + current += char; + } + + result.push(current); + + return result; +}; + +const IMPORTANT_RE = /!\s*important\s*$/i; + +/** + * Apply a CSS declaration string to an element through the CSSOM, replacing any + * style previously set on it. + * Unlike writing the `style` attribute, CSSOM updates are not subject to the + * `style-src-attr` CSP directive, so this keeps the editor usable on pages + * served with a strict policy. + */ +export const setStyleText = (el: T, style?: string) => { + el.removeAttribute('style'); + + splitDeclarations(style || '').forEach((declaration) => { + const index = declaration.indexOf(':'); + if (index < 0) return; + const prop = declaration.slice(0, index).trim(); + if (!prop) return; + let value = declaration.slice(index + 1).trim(); + const important = IMPORTANT_RE.test(value); + important && (value = value.replace(IMPORTANT_RE, '').trim()); + el.style.setProperty(prop, value, important ? 'important' : ''); + }); + + return el; +}; + // Unfortunately just creating `KeyboardEvent(e.type, e)` is not enough, // the keyCode/which will be always `0`. Even if it's an old/deprecated // property keymaster (and many others) still use it... using `defineProperty` diff --git a/packages/core/test/specs/commands/view/Preview.ts b/packages/core/test/specs/commands/view/Preview.ts index 2af00e34b..ce160e1f9 100644 --- a/packages/core/test/specs/commands/view/Preview.ts +++ b/packages/core/test/specs/commands/view/Preview.ts @@ -43,6 +43,7 @@ describe('Preview command', () => { getElement: jest.fn().mockReturnValue({ style: {}, setAttribute: jest.fn(), + removeAttribute: jest.fn(), }), }, diff --git a/packages/core/test/specs/style_manager/view/PropertySelectView.ts b/packages/core/test/specs/style_manager/view/PropertySelectView.ts index 71f955bbf..55fa5901a 100644 --- a/packages/core/test/specs/style_manager/view/PropertySelectView.ts +++ b/packages/core/test/specs/style_manager/view/PropertySelectView.ts @@ -16,7 +16,7 @@ describe('PropertySelectView', () => { const propValue = 'test1value'; const defValue = 'test2value'; let options: any = [ - { id: 'test1value', style: 'test:style' }, + { id: 'test1value', style: 'color: red' }, { id: 'test2', value: 'test2value' }, ]; @@ -69,7 +69,8 @@ describe('PropertySelectView', () => { expect((children[1] as any).value).toEqual(options[1].id); expect(children[0].textContent).toEqual(options[0].id); expect(children[1].textContent).toEqual(options[1].id); - expect(children[0].getAttribute('style')).toEqual(options[0].style); + // Applied through the CSSOM, never written as a `style` attribute + expect((children[0] as HTMLElement).style.color).toEqual('red'); expect(children[1].getAttribute('style')).toEqual(null); }); diff --git a/packages/core/test/specs/utils/noInlineStyleAttributes.ts b/packages/core/test/specs/utils/noInlineStyleAttributes.ts new file mode 100644 index 000000000..1bad203b7 --- /dev/null +++ b/packages/core/test/specs/utils/noInlineStyleAttributes.ts @@ -0,0 +1,62 @@ +import fs from 'fs'; +import path from 'path'; + +/** + * The editor renders most of its chrome by assigning HTML strings, so a + * `style="..."` literal in a template ends up parsed as an inline style + * attribute, which a strict `style-src-attr` CSP blocks. The same goes for + * `setAttribute('style', ...)`. + * + * CSSOM writes (`el.style.prop = value`, `setStyleText`) are not covered by CSP + * and are the supported way to apply runtime values. + */ +const SRC_DIR = path.join(__dirname, '../../../src'); + +// Files allowed to keep an inline style, with the reason why +const ALLOWED: Record = { + 'dom_components/model/ComponentImage.ts': + 'SVG placeholder serialized to a base64 data URL and used as `img` src, so it is a separate document governed by `img-src`', + 'dom_components/view/ComponentView.ts': + 'writes the style attribute of a user component, which is the content the editor exists to author (and is off by default via `avoidInlineStyle`)', +}; + +const PATTERNS = [ + { name: 'style attribute in markup', re: /(^|[^-\w])style\s*=\s*["'`]/ }, + { name: "setAttribute('style')", re: /setAttribute\(\s*['"`]style['"`]/ }, +]; + +const stripComments = (code: string) => code.replace(/\/\*[\s\S]*?\*\//g, '').replace(/(^|[^:])\/\/.*$/gm, '$1'); + +const walk = (dir: string): string[] => + fs.readdirSync(dir, { withFileTypes: true }).reduce((res, entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return res.concat(walk(full)); + return entry.name.endsWith('.ts') ? res.concat(full) : res; + }, []); + +describe('No inline style attributes in editor markup', () => { + test('src is free of `style=` and `setAttribute("style")`, except the documented cases', () => { + const found: string[] = []; + + walk(SRC_DIR).forEach((file) => { + const relative = path.relative(SRC_DIR, file).split(path.sep).join('/'); + if (ALLOWED[relative]) return; + + stripComments(fs.readFileSync(file, 'utf8')) + .split('\n') + .forEach((line, i) => { + PATTERNS.forEach(({ name, re }) => { + re.test(line) && found.push(`${relative}:${i + 1} (${name}) ${line.trim()}`); + }); + }); + }); + + expect(found).toEqual([]); + }); + + test('the allowlist only names files that exist', () => { + Object.keys(ALLOWED).forEach((relative) => { + expect(fs.existsSync(path.join(SRC_DIR, relative))).toBe(true); + }); + }); +}); diff --git a/packages/core/test/specs/utils/setStyleText.ts b/packages/core/test/specs/utils/setStyleText.ts new file mode 100644 index 000000000..50a8910d9 --- /dev/null +++ b/packages/core/test/specs/utils/setStyleText.ts @@ -0,0 +1,63 @@ +import { setStyleText } from '../../../src/utils/dom'; + +describe('setStyleText', () => { + let el: HTMLElement; + + beforeEach(() => { + el = document.createElement('div'); + }); + + test('applies a single declaration', () => { + setStyleText(el, 'color: red'); + expect(el.style.color).toBe('red'); + }); + + test('applies multiple declarations', () => { + setStyleText(el, 'color: red; padding-left: 10px'); + expect(el.style.color).toBe('red'); + expect(el.style.paddingLeft).toBe('10px'); + }); + + test('replaces any style previously set', () => { + setStyleText(el, 'color: red; width: 10px'); + setStyleText(el, 'color: blue'); + expect(el.style.color).toBe('blue'); + expect(el.style.width).toBe(''); + }); + + test('keeps `;` nested in functions', () => { + const url = 'data:image/gif;base64,R0lGODlh'; + setStyleText(el, `background-image: url(${url}); color: red`); + // jsdom re-serializes the url with quotes, what matters is that the + // `;` inside it did not split the declaration + expect(el.style.backgroundImage).toContain(url); + expect(el.style.color).toBe('red'); + }); + + test('keeps `;` nested in strings', () => { + setStyleText(el, `content: "a;b"; color: red`); + expect(el.style.color).toBe('red'); + }); + + test('supports !important', () => { + setStyleText(el, 'color: red !important'); + expect(el.style.getPropertyPriority('color')).toBe('important'); + expect(el.style.color).toBe('red'); + }); + + test('supports custom properties', () => { + setStyleText(el, '--my-var: 10px'); + expect(el.style.getPropertyValue('--my-var')).toBe('10px'); + }); + + test('tolerates empty, partial and trailing declarations', () => { + setStyleText(el, ';; color: red ;; padding ;'); + expect(el.style.color).toBe('red'); + }); + + test('clears the style with an empty input', () => { + setStyleText(el, 'color: red'); + setStyleText(el); + expect(el.getAttribute('style')).toBe(null); + }); +});