fix(engine): disallow innerHTML outside lwc:inner-html (#4578)

Co-authored-by: Will Harney <wharney@salesforce.com>
Co-authored-by: Eugene Kashida <ekashida@gmail.com>

eslint.config.mjs

@@ -55,6 +55,7 @@ export default tseslint.config(
                 {
                     argsIgnorePattern: '^_',
                     caughtErrorsIgnorePattern: '^_',
+                    destructuredArrayIgnorePattern: '^_',
                 },
             ],
 

packages/@lwc/engine-core/src/framework/api.ts

@@ -59,6 +59,7 @@ import {
     VText,
 } from './vnodes';
 import { getComponentRegisteredName } from './component';
+import { createSanitizedHtmlContent, SanitizedHtmlContent } from './sanitized-html-content';
 
 const SymbolIterator: typeof Symbol.iterator = Symbol.iterator;
 
@@ -727,8 +728,9 @@ export function setSanitizeHtmlContentHook(newHookImpl: SanitizeHtmlContentHook)
 }
 
 // [s]anitize [h]tml [c]ontent
-function shc(content: unknown): string {
-    return sanitizeHtmlContentHook(content);
+function shc(content: unknown): SanitizedHtmlContent {
+    const sanitizedString = sanitizeHtmlContentHook(content);
+    return createSanitizedHtmlContent(sanitizedString);
 }
 
 /**

packages/@lwc/engine-core/src/framework/hydration.ts

@@ -61,6 +61,7 @@ import { hydrateStaticParts, traverseAndSetElements } from './modules/static-par
 import { getScopeTokenClass, getStylesheetTokenHost } from './stylesheet';
 import { renderComponent } from './component';
 import { applyRefs } from './modules/refs';
+import { isSanitizedHtmlContentEqual } from './sanitized-html-content';
 
 // These values are the ones from Node.nodeType (https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType)
 const enum EnvNodeTypes {
@@ -240,7 +241,9 @@ function hydrateComment(node: Node, vnode: VComment, renderer: RendererAPI): Nod
     }
 
     const { setProperty } = renderer;
-    setProperty(node, NODE_VALUE_PROP, vnode.text ?? null);
+    // We only set the `nodeValue` property here (on a comment), so we don't need
+    // to sanitize the content as HTML using `safelySetProperty`
+    setProperty(node as Element, NODE_VALUE_PROP, vnode.text ?? null);
     vnode.elm = node;
 
     return node;
@@ -310,7 +313,7 @@ function hydrateElement(elm: Node, vnode: VElement, renderer: RendererAPI): Node
         } = vnode;
         const { getProperty } = renderer;
         if (!isUndefined(props) && !isUndefined(props.innerHTML)) {
-            if (getProperty(elm, 'innerHTML') === props.innerHTML) {
+            if (isSanitizedHtmlContentEqual(getProperty(elm, 'innerHTML'), props.innerHTML)) {
                 // Do a shallow clone since VNodeData may be shared across VNodes due to hoist optimization
                 vnode.data = {
                     ...vnode.data,

packages/@lwc/engine-core/src/framework/modules/attrs.ts

@@ -16,6 +16,7 @@ import { RendererAPI } from '../renderer';
 
 import { EmptyObject } from '../utils';
 import { VBaseElement, VStatic, VStaticPartElement } from '../vnodes';
+import { safelySetProperty } from '../sanitized-html-content';
 
 const ColonCharCode = 58;
 
@@ -51,7 +52,7 @@ export function patchAttributes(
             // Use kebabCaseToCamelCase directly because we don't want to set props like `ariaLabel` or `tabIndex`
             // on a custom element versus just using the more reliable attribute format.
             if (external && (propName = kebabCaseToCamelCase(key)) in elm!) {
-                setProperty(elm, propName, cur);
+                safelySetProperty(setProperty, elm!, propName, cur);
             } else if (StringCharCodeAt.call(key, 3) === ColonCharCode) {
                 // Assume xml namespace
                 setAttribute(elm, key, cur as string, XML_NAMESPACE);

packages/@lwc/engine-core/src/framework/modules/props.ts

@@ -9,6 +9,7 @@ import { logWarn } from '../../shared/logger';
 import { RendererAPI } from '../renderer';
 import { EmptyObject } from '../utils';
 import { VBaseElement } from '../vnodes';
+import { safelySetProperty } from '../sanitized-html-content';
 
 function isLiveBindingProp(sel: string, key: string): boolean {
     // For properties with live bindings, we read values from the DOM element
@@ -66,7 +67,7 @@ export function patchProps(
                     );
                 }
             }
-            setProperty(elm!, key, cur);
+            safelySetProperty(setProperty, elm!, key, cur);
         }
     }
 }

packages/@lwc/engine-core/src/framework/sanitized-html-content.ts

@@ -0,0 +1,76 @@
+import { create as ObjectCreate, isNull, isObject, isUndefined } from '@lwc/shared';
+import { logWarn } from '../shared/logger';
+import type { RendererAPI } from './renderer';
+
+const sanitizedHtmlContentSymbol = Symbol('lwc-get-sanitized-html-content');
+
+export type SanitizedHtmlContent = {
+    [sanitizedHtmlContentSymbol]: unknown;
+};
+
+function isSanitizedHtmlContent(object: any): object is SanitizedHtmlContent {
+    return isObject(object) && !isNull(object) && sanitizedHtmlContentSymbol in object;
+}
+
+function unwrapIfNecessary(object: any) {
+    return isSanitizedHtmlContent(object) ? object[sanitizedHtmlContentSymbol] : object;
+}
+
+/**
+ * Wrap a pre-sanitized string designated for `.innerHTML` via `lwc:inner-html`
+ * as an object with a Symbol that only we have access to.
+ * @param sanitizedString
+ * @returns SanitizedHtmlContent
+ */
+export function createSanitizedHtmlContent(sanitizedString: unknown): SanitizedHtmlContent {
+    return ObjectCreate(null, {
+        [sanitizedHtmlContentSymbol]: {
+            value: sanitizedString,
+            configurable: false,
+            writable: false,
+        },
+    });
+}
+
+/**
+ * Safely call setProperty on an Element while handling any SanitizedHtmlContent objects correctly
+ *
+ * @param setProperty - renderer.setProperty
+ * @param elm - Element
+ * @param key - key to set
+ * @param value -  value to set
+ */
+export function safelySetProperty(
+    setProperty: RendererAPI['setProperty'],
+    elm: Element,
+    key: string,
+    value: any
+) {
+    // See W-16614337
+    // we support setting innerHTML to `undefined` because it's inherently safe
+    if ((key === 'innerHTML' || key === 'outerHTML') && !isUndefined(value)) {
+        if (isSanitizedHtmlContent(value)) {
+            // it's a SanitizedHtmlContent object
+            setProperty(elm, key, value[sanitizedHtmlContentSymbol]);
+        } else {
+            // not a SanitizedHtmlContent object
+            if (process.env.NODE_ENV !== 'production') {
+                logWarn(
+                    `Cannot set property "${key}". Instead, use lwc:inner-html or lwc:dom-manual.`
+                );
+            }
+        }
+    } else {
+        setProperty(elm, key, value);
+    }
+}
+
+/**
+ * Given two objects (likely either a string or a SanitizedHtmlContent object), return true if their
+ * string values are equivalent.
+ * @param first
+ * @param second
+ */
+export function isSanitizedHtmlContentEqual(first: any, second: any) {
+    return unwrapIfNecessary(first) === unwrapIfNecessary(second);
+}

packages/@lwc/engine-dom/src/renderer/index.ts

@@ -71,7 +71,11 @@ function getProperty(node: Node, key: string): any {
     return (node as any)[key];
 }
 
-function setProperty(node: Node, key: string, value: any): void {
+function setProperty<K extends string>(
+    node: Node & Record<K, unknown>,
+    key: K,
+    value: unknown
+): void {
     (node as any)[key] = value;
 }
 

packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts

@@ -52,7 +52,11 @@ async function compileFixture({ input, dirname }: { input: string; dirname: stri
         ],
         onwarn({ message, code }) {
             // TODO [#3331]: The existing lwc:dynamic fixture test will generate warnings that can be safely suppressed.
-            if (!message.includes('LWC1187') && code !== 'CIRCULAR_DEPENDENCY') {
+            const shouldIgnoreWarning =
+                message.includes('LWC1187') ||
+                message.includes('-h-t-m-l') ||
+                code === 'CIRCULAR_DEPENDENCY';
+            if (!shouldIgnoreWarning) {
                 throw new Error(message);
             }
         },

packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/expected.html

@@ -0,0 +1,88 @@
+<x-inner>
+  <template shadowrootmode="open">
+    <div data-expect-no-warning data-id="div-inner-static" inner-h-t-m-l="replaced">
+      original
+    </div>
+    <div data-expect-no-warning data-id="div-inner-computed" inner-h-t-m-l="injected">
+      original
+    </div>
+    <div data-id="div-inner-spread">
+      original
+    </div>
+    <x-component data-id="cmp-inner-static">
+      <template shadowrootmode="open">
+        <slot>
+        </slot>
+      </template>
+      original
+    </x-component>
+    <x-component data-id="cmp-inner-computed">
+      <template shadowrootmode="open">
+        <slot>
+        </slot>
+      </template>
+      original
+    </x-component>
+    <x-component data-id="cmp-inner-spread">
+      <template shadowrootmode="open">
+        <slot>
+        </slot>
+      </template>
+      original
+    </x-component>
+    <omg-whatever data-id="external-inner-static" inner-h-t-m-l="replaced">
+      original
+    </omg-whatever>
+    <omg-whatever data-id="external-inner-computed" inner-h-t-m-l="injected">
+      original
+    </omg-whatever>
+    <omg-whatever data-id="external-inner-spread">
+      original
+    </omg-whatever>
+    <div data-expect-no-warning data-id="div-outer-static" outer-h-t-m-l="replaced">
+      original
+    </div>
+    <div data-expect-no-warning data-id="div-outer-computed" outer-h-t-m-l="injected">
+      original
+    </div>
+    <div data-id="div-outer-spread">
+      original
+    </div>
+    <x-component data-id="cmp-outer-static">
+      <template shadowrootmode="open">
+        <slot>
+        </slot>
+      </template>
+      original
+    </x-component>
+    <x-component data-id="cmp-outer-computed">
+      <template shadowrootmode="open">
+        <slot>
+        </slot>
+      </template>
+      original
+    </x-component>
+    <x-component data-id="cmp-outer-spread">
+      <template shadowrootmode="open">
+        <slot>
+        </slot>
+      </template>
+      original
+    </x-component>
+    <omg-whatever data-id="external-outer-static">
+      original
+    </omg-whatever>
+    <omg-whatever data-id="external-outer-computed">
+      original
+    </omg-whatever>
+    <omg-whatever data-id="external-outer-spread">
+      original
+    </omg-whatever>
+    <span>
+      lwc:inner-html
+    </span>
+    <p>
+      injected
+    </p>
+  </template>
+</x-inner>

packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/index.js

@@ -0,0 +1,3 @@
+export const tagName = 'x-inner';
+export { default } from 'x/inner';
+export * from 'x/inner';

packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.html

@@ -0,0 +1,3 @@
+<template>
+    <slot></slot>
+</template>

packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.js

@@ -0,0 +1,3 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {}

packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.html

@@ -0,0 +1,28 @@
+<template>
+  <div data-id="div-inner-static" data-expect-no-warning inner-h-t-m-l="replaced">original</div>
+  <div data-id="div-inner-computed" data-expect-no-warning inner-h-t-m-l={computed}>original</div>
+  <div data-id="div-inner-spread" lwc:spread={spread}>original</div>
+
+  <x-component data-id="cmp-inner-static" inner-h-t-m-l="replaced">original</x-component>
+  <x-component data-id="cmp-inner-computed" inner-h-t-m-l={computed}>original</x-component>
+  <x-component data-id="cmp-inner-spread" lwc:spread={spread}>original</x-component>
+
+  <omg-whatever lwc:external data-id="external-inner-static" inner-h-t-m-l="replaced">original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-inner-computed" inner-h-t-m-l={computed}>original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-inner-spread" lwc:spread={spread}>original</omg-whatever>
+
+  <div data-id="div-outer-static" data-expect-no-warning outer-h-t-m-l="replaced">original</div>
+  <div data-id="div-outer-computed" data-expect-no-warning outer-h-t-m-l={computed}>original</div>
+  <div data-id="div-outer-spread" lwc:spread={spread}>original</div>
+
+  <x-component data-id="cmp-outer-static" outer-h-t-m-l="replaced">original</x-component>
+  <x-component data-id="cmp-outer-computed" outer-h-t-m-l={computed}>original</x-component>
+  <x-component data-id="cmp-outer-spread" lwc:spread={spread}>original</x-component>
+
+  <omg-whatever lwc:external data-id="external-outer-static" outer-h-t-m-l="replaced">original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-outer-computed" outer-h-t-m-l={computed}>original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-outer-spread" lwc:spread={spread}>original</omg-whatever>
+
+  <span lwc:inner-html="lwc:inner-html"></span>
+  <p lwc:inner-html={computed}></p>
+</template>

packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.js

@@ -0,0 +1,6 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {
+    computed = 'injected';
+    spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' };
+}

packages/@lwc/integration-karma/test-hydration/inner-outer-html/index.spec.js

@@ -0,0 +1,16 @@
+export default {
+    props: {},
+    advancedTest(target, { consoleSpy }) {
+        const ids = Object.entries(TestUtils.extractDataIds(target)).filter(
+            ([id]) => !id.endsWith('.shadowRoot')
+        );
+        for (const [id, node] of ids) {
+            expect(node.childNodes.length).toBe(1);
+            expect(node.firstChild.nodeType).toBe(Node.TEXT_NODE);
+            const expected = id.startsWith('lwc-inner-html-') ? 'injected' : 'original';
+            expect(node.firstChild.nodeValue).toBe(expected);
+        }
+        expect(consoleSpy.calls.warn).toHaveSize(0);
+        expect(consoleSpy.calls.error).toHaveSize(0);
+    },
+};

packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.html

@@ -0,0 +1,3 @@
+<template>
+    <slot></slot>
+</template>

packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.js

@@ -0,0 +1,3 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {}

packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.html

@@ -0,0 +1,28 @@
+<template>
+  <div data-id="div-inner-static" data-expect-no-warning inner-h-t-m-l="replaced">original</div>
+  <div data-id="div-inner-computed" data-expect-no-warning inner-h-t-m-l={computed}>original</div>
+  <div data-id="div-inner-spread" lwc:spread={spread}>original</div>
+
+  <x-component data-id="cmp-inner-static" inner-h-t-m-l="replaced">original</x-component>
+  <x-component data-id="cmp-inner-computed" inner-h-t-m-l={computed}>original</x-component>
+  <x-component data-id="cmp-inner-spread" lwc:spread={spread}>original</x-component>
+
+  <omg-whatever lwc:external data-id="external-inner-static" inner-h-t-m-l="replaced">original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-inner-computed" inner-h-t-m-l={computed}>original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-inner-spread" lwc:spread={spread}>original</omg-whatever>
+
+  <div data-id="div-outer-static" data-expect-no-warning outer-h-t-m-l="replaced">original</div>
+  <div data-id="div-outer-computed" data-expect-no-warning outer-h-t-m-l={computed}>original</div>
+  <div data-id="div-outer-spread" lwc:spread={spread}>original</div>
+
+  <x-component data-id="cmp-outer-static" outer-h-t-m-l="replaced">original</x-component>
+  <x-component data-id="cmp-outer-computed" outer-h-t-m-l={computed}>original</x-component>
+  <x-component data-id="cmp-outer-spread" lwc:spread={spread}>original</x-component>
+
+  <omg-whatever lwc:external data-id="external-outer-static" outer-h-t-m-l="replaced">original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-outer-computed" outer-h-t-m-l={computed}>original</omg-whatever>
+  <omg-whatever lwc:external data-id="external-outer-spread" lwc:spread={spread}>original</omg-whatever>
+
+  <span data-id="lwc-inner-html-static" lwc:inner-html="injected"></span>
+  <p data-id="lwc-inner-html-computed" lwc:inner-html={computed}></p>
+</template>

packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.js

@@ -0,0 +1,6 @@
+import { LightningElement } from 'lwc';
+
+export default class extends LightningElement {
+    computed = 'injected';
+    spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' };
+}