diff --git a/craft/src/components/basic/Heading.toHtml.test.ts b/craft/src/components/basic/Heading.toHtml.test.ts
new file mode 100644
index 0000000..f13d115
--- /dev/null
+++ b/craft/src/components/basic/Heading.toHtml.test.ts
@@ -0,0 +1,40 @@
+import { describe, test, expect } from 'vitest';
+import { Heading } from './Heading';
+
+const toHtml = (Heading as any).toHtml;
+
+describe('Heading.toHtml level allowlist (adversarial re-review, same class as C1)', () => {
+ test('a malicious level value clamps to h2 -- no injected
, no broken-out tag', () => {
+ const { html } = toHtml({ text: 'x', level: 'h2>
')).toBe(true);
+ });
+
+ test('a numeric out-of-range level (99) clamps to h2', () => {
+ const { html } = toHtml({ text: 'x', level: 99 as any }, '');
+ expect(html.startsWith('
')).toBe(true);
+ });
+
+ test('a non-heading string level clamps to h2', () => {
+ const { html } = toHtml({ text: 'x', level: 'script' as any }, '');
+ expect(html.startsWith(' {
+ const { html } = toHtml({ text: 'x', level: 'h4' }, '');
+ expect(html).toContain('');
+ });
+
+ test('all valid levels h1-h6 still work', () => {
+ for (const level of ['h1', 'h2', 'h3', 'h4', 'h5', 'h6']) {
+ const { html } = toHtml({ text: 'x', level }, '');
+ expect(html.startsWith(`<${level}`)).toBe(true);
+ expect(html.endsWith(`${level}>`)).toBe(true);
+ }
+ });
+});
diff --git a/craft/src/components/basic/Heading.tsx b/craft/src/components/basic/Heading.tsx
index 158f911..da3e654 100644
--- a/craft/src/components/basic/Heading.tsx
+++ b/craft/src/components/basic/Heading.tsx
@@ -4,6 +4,16 @@ import { cssPropsToString } from '../../utils/style-helpers';
type HeadingLevel = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6';
+// `level` is settable via the AI `update_props` path and from deserialized
+// saved state -- neither type-checked at runtime -- and is interpolated
+// directly into the tag position (`React.createElement(level, ...)` /
+// `<${tag}` in `toHtml`). A malicious value like `h2>
+ (ALLOWED_HEADING_LEVELS as readonly unknown[]).includes(level) ? (level as HeadingLevel) : 'h2';
+
interface HeadingProps {
text?: string;
level?: HeadingLevel;
@@ -30,6 +40,7 @@ export const Heading: UserComponent = ({
selected: node.events.selected,
}));
+ const safeLevel = sanitizeHeadingLevel(level);
const elRef = useRef(null);
const editedTextRef = useRef(null);
@@ -59,7 +70,7 @@ export const Heading: UserComponent = ({
}
}, [text, selected]);
- return React.createElement(level, {
+ return React.createElement(safeLevel, {
ref: (ref: HTMLElement | null): void => {
elRef.current = ref;
if (ref) connect(drag(ref));
@@ -98,7 +109,7 @@ Heading.craft = {
};
(Heading as any).toHtml = (props: HeadingProps, _childrenHtml: string) => {
- const tag = props.level || 'h2';
+ const tag = sanitizeHeadingLevel(props.level);
const safeText = (props.text || '').replace(//g, '>');
const styleStr = cssPropsToString(props.style);
return { html: `<${tag}${styleStr ? ` style="${styleStr}"` : ''}>${safeText}${tag}>` };
diff --git a/craft/src/components/layout/Container.toHtml.test.ts b/craft/src/components/layout/Container.toHtml.test.ts
index d26be50..a08a0cb 100644
--- a/craft/src/components/layout/Container.toHtml.test.ts
+++ b/craft/src/components/layout/Container.toHtml.test.ts
@@ -33,3 +33,33 @@ describe('Container.toHtml cssId/cssClass', () => {
expect(html).toContain('id="anchor-id"');
});
});
+
+describe('Container.toHtml tag allowlist (adversarial re-review, same class as C1)', () => {
+ test('a malicious tag value falls back to div -- no injected
, no broken-out attrs', () => {
+ const { html } = toHtml({ tag: 'div>
')).toBe(true);
+ });
+
+ test('a tag value outside the known-safe set falls back to div', () => {
+ const { html } = toHtml({ tag: 'script' }, 'child');
+ expect(html.startsWith(' {
+ const { html } = toHtml({ tag: 'section' }, 'child');
+ expect(html).toContain('
');
+ });
+
+ test('all other allowlisted tags still work', () => {
+ for (const tag of ['div', 'article', 'header', 'footer', 'main']) {
+ const { html } = toHtml({ tag }, 'child');
+ expect(html.startsWith(`<${tag}`)).toBe(true);
+ expect(html.endsWith(`${tag}>`)).toBe(true);
+ }
+ });
+});
diff --git a/craft/src/components/layout/Container.tsx b/craft/src/components/layout/Container.tsx
index 904c8e7..892ed2a 100644
--- a/craft/src/components/layout/Container.tsx
+++ b/craft/src/components/layout/Container.tsx
@@ -3,6 +3,19 @@ import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers';
import { escapeAttr } from '../../utils/escape';
+// The only tag names Container actually supports (matches the TS union
+// below and the `tag` default in `.craft.props`). `tag` is settable via the
+// AI `update_props` path and from deserialized saved state -- neither is
+// type-checked at runtime -- so a malicious value like
+// `div>
+ (ALLOWED_CONTAINER_TAGS as readonly unknown[]).includes(tag) ? (tag as ContainerTag) : 'div';
+
interface ContainerProps {
style?: CSSProperties;
tag?: 'div' | 'section' | 'article' | 'header' | 'footer' | 'main';
@@ -42,6 +55,7 @@ export const Container: UserComponent = ({
}) => {
const { connectors: { connect, drag } } = useNode();
+ const safeTag = sanitizeContainerTag(tag);
const needsBoxedWrapper = contentWidth === 'boxed';
const flexStyles = flexAlignFromTextAlign(style.textAlign);
@@ -58,7 +72,7 @@ export const Container: UserComponent = ({
const idValue = cssId || anchorId || undefined;
const el = React.createElement(
- tag,
+ safeTag,
{
ref: (ref: HTMLElement | null): void => { if (ref) connect(drag(ref)); },
style: outerStyle,
@@ -97,7 +111,7 @@ Container.craft = {
/* ---------- HTML export ---------- */
(Container as any).toHtml = (props: ContainerProps, childrenHtml: string) => {
- const tag = props.tag || 'div';
+ const tag = sanitizeContainerTag(props.tag);
const isBoxed = props.contentWidth === 'boxed';
const flexStyles = flexAlignFromTextAlign(props.style?.textAlign);
diff --git a/craft/src/utils/html-export.test.ts b/craft/src/utils/html-export.test.ts
index 25046d7..ba92cde 100644
--- a/craft/src/utils/html-export.test.ts
+++ b/craft/src/utils/html-export.test.ts
@@ -58,3 +58,42 @@ describe('buildDataAttrs escapes animation/animationDelay (C2)', () => {
expect(html).toContain('data-animation-delay="0.5s"');
});
});
+
+/**
+ * Adversarial re-review of C1: the `typeName === 'Container' || typeName ===
+ * 'div'` fallback in renderNode (hit when a node's `resolvedName` isn't in
+ * the component resolver, e.g. legacy/tampered saved state) interpolated
+ * `props.tag` raw into `<${tag}` with no validation -- the same breakout
+ * class as the fixed Container.toHtml bug, reachable via deserialized saved
+ * state.
+ */
+describe('renderNode div-fallback allowlists props.tag', () => {
+ const makeUnresolvedDivState = (tag: unknown) =>
+ JSON.stringify({
+ ROOT: {
+ type: { resolvedName: 'div' }, // not in componentResolver -> hits the fallback branch
+ isCanvas: true,
+ props: { tag, style: {} },
+ displayName: 'div',
+ custom: {},
+ hidden: false,
+ nodes: [],
+ linkedNodes: {},
+ },
+ });
+
+ test('a malicious tag value falls back to div -- no injected
', () => {
+ const state = makeUnresolvedDivState('div>
{
+ const state = makeUnresolvedDivState('section');
+ const { html } = exportBodyHtml(state);
+ expect(html).toContain('');
+ });
+});
diff --git a/craft/src/utils/html-export.ts b/craft/src/utils/html-export.ts
index 7da9a9e..fcf06dd 100644
--- a/craft/src/utils/html-export.ts
+++ b/craft/src/utils/html-export.ts
@@ -1,6 +1,7 @@
import { componentResolver } from '../components/resolver';
import { cssPropsToString } from './style-helpers';
import { escapeHtml, escapeAttr } from './escape';
+import { sanitizeContainerTag } from '../components/layout/Container';
export interface ExportOptions {
title?: string;
@@ -94,7 +95,11 @@ function renderNode(nodes: Record, nodeId: string): { html: string
// Fallback: wrap children in a div with inline styles
if (typeName === 'Container' || typeName === 'div') {
const styleStr = cssPropsToString(props.style);
- const tag = props.tag || 'div';
+ // `props.tag` reaches this fallback the same way it reaches
+ // `Container.toHtml` -- via AI `update_props` or deserialized saved
+ // state, neither type-checked at runtime -- so it must be allowlisted
+ // before hitting the `<${tag}` template position below.
+ const tag = sanitizeContainerTag(props.tag);
return {
html: `<${tag}${dataAttrs}${styleStr ? ` style="${styleStr}"` : ''}>${allChildrenHtml}${tag}>`,
};
diff --git a/craft/src/utils/style-helpers.test.ts b/craft/src/utils/style-helpers.test.ts
index 3c2df45..43fcbec 100644
--- a/craft/src/utils/style-helpers.test.ts
+++ b/craft/src/utils/style-helpers.test.ts
@@ -55,6 +55,39 @@ describe('cssPropsToString sanitizes emitted values (A5)', () => {
});
});
+describe('cssPropsToString sanitizes emitted VALUES that are not strings (adversarial re-review of C1)', () => {
+ test('an array value containing an attribute-breakout payload is neutralized', () => {
+ const out = cssPropsToString({
+ color: ['red', '">
'],
+ } as any);
+ // No raw `"` (would close style="...") and no raw `<`/`>` (defense in
+ // depth) may survive -- they must come back as HTML entities, leaving
+ // the payload as inert text rather than a live tag.
+ expect(out).not.toMatch(/[<>"]/);
+ expect(out).toContain('<img');
+ expect(out).toContain('"');
+ });
+
+ test('an object value containing an attribute-breakout payload is neutralized', () => {
+ const out = cssPropsToString({
+ color: { toString: () => '">
' },
+ } as any);
+ expect(out).not.toMatch(/[<>"]/);
+ expect(out).toContain('<img');
+ expect(out).toContain('"');
+ });
+
+ test('a number value is emitted raw/unchanged', () => {
+ const out = cssPropsToString({ zIndex: 5 } as any);
+ expect(out).toBe('z-index:5');
+ });
+
+ test('a normal string value still works after the fix', () => {
+ const out = cssPropsToString({ color: 'red' } as any);
+ expect(out).toBe('color:red');
+ });
+});
+
describe('cssPropsToString sanitizes emitted KEYS (C1 -- style-object key breakout)', () => {
test('a key containing a double-quote is dropped entirely, not emitted', () => {
const out = cssPropsToString({
diff --git a/craft/src/utils/style-helpers.ts b/craft/src/utils/style-helpers.ts
index ea9f38f..3288b55 100644
--- a/craft/src/utils/style-helpers.ts
+++ b/craft/src/utils/style-helpers.ts
@@ -9,10 +9,16 @@ const URL_RE = /url\(\s*(['"]?)([\s\S]*?)\1\s*\)/gi;
// Outside of a url(...) reference, a `;` is never legitimate (declarations
// are separated by it) -- stray semicolons are how a breakout injects a
// second property -- and a raw `"` would close the `style="..."` attribute
-// early. Inside url('...') the content has already been made safe via
-// escapeAttr(safeUrl(...)), including any `;` required by data-URI syntax
-// (`data:;base64,`), so this must never be applied there.
-const sanitizeBreakoutChars = (s: string): string => s.replace(/;/g, '').replace(/"/g, '"');
+// early. `<`/`>` are escaped too as defense-in-depth: they're inert inside a
+// properly quote-terminated `style="..."` attribute, but a value can reach
+// this function from a non-string source (array/object coerced via
+// `String(v)`, see `cssPropsToString` below) so we don't want to rely solely
+// on the outer quote holding. Inside url('...') the content has already been
+// made safe via escapeAttr(safeUrl(...)), including any `;` required by
+// data-URI syntax (`data:;base64,`), so this must never be
+// applied there.
+const sanitizeBreakoutChars = (s: string): string =>
+ s.replace(/;/g, '').replace(/"/g, '"').replace(//g, '>');
/**
* Sanitizes a single CSS declaration value so it can never terminate the
@@ -60,7 +66,14 @@ export function cssPropsToString(style: CSSProperties | undefined): string {
.filter(([, v]) => v !== undefined && v !== null && v !== '')
.map(([k, v]) => [camelToKebab(k), v] as const)
.filter(([k]) => VALID_CSS_KEY_RE.test(k))
- .map(([k, v]) => `${k}:${typeof v === 'string' ? sanitizeCssValue(v) : v}`)
+ // Only a genuine `number` is safe to interpolate raw (numbers can never
+ // contain a `"`/`<`/`>`/`;` breakout character). Every other type --
+ // string, array, object, etc. -- must be coerced to a string and run
+ // through `sanitizeCssValue`. Without this, a non-string value (e.g. an
+ // array like `['red', '">
']`) skips
+ // sanitization entirely and is template-coerced (`${v}`) raw into the
+ // `style="..."` attribute, breaking out via the un-escaped `"`.
+ .map(([k, v]) => `${k}:${typeof v === 'number' ? v : sanitizeCssValue(String(v))}`)
.join(';');
}