Compare commits

...
Author SHA1 Message Date
shadowdaoandClaude Opus 4.8 bf4a9f48eb security: block data:image/svg+xml + sandbox HtmlBlock iframes
M-5: safeUrl() blocked javascript:/vbscript:/data:text/html but
allowed data:image/svg+xml, which can execute inline <script>/onload=
when loaded as a document/navigation target despite its "image" MIME
type (defense in depth -- not currently reachable to execution via
this sink, but closing it). Added `data:image/svg+xml` to the existing
DANGEROUS_SCHEME_PREFIXES check, so it's caught after the same
entity-decode/whitespace-strip/lowercase normalization used for the
other blocked schemes (obfuscated variants included). Other
data:image/* types (png/jpeg/gif/webp, ...) remain allowed unchanged.

M-6: HtmlBlock's purifyHtml() allowed <iframe src> through with no
`sandbox` attribute -- a clickjacking/phishing vector even with
DOMPurify already stripping script/on*=. Added a DOMPurify
afterSanitizeAttributes hook, scoped tightly to each purifyHtml() call
(added right before sanitize(), removed in a finally right after) so
it can't leak onto other DOMPurify uses or accumulate duplicates
across repeated calls, that force-sets a restrictive sandbox
(allow-scripts allow-same-origin allow-popups allow-forms -- no
allow-top-navigation) and referrerpolicy=no-referrer on every iframe
that survives sanitization.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 18:31:25 -07:00
shadowdaoandClaude Opus 4.8 86455413d0 fix: unique addPage ids + collision-free scopeId hashing
M-3: PageContext.addPage minted ids from bare `page_${Date.now()}` --
two adds inside the same millisecond collided on id, so a subsequent
rename/delete/save silently acted on both pages at once. Added a
module-scoped monotonic counter combined with the timestamp
(nextPageId(), exported for direct unit testing) and used it
everywhere an addPage-style id is minted (addPage, replaceAllPages).

M-4: scopeId() lowercased + stripped non-alphanumeric characters from
the node id into a slug, so two node ids differing only by
case/punctuation (e.g. "AbC" vs "abc", or "a-b" vs "ab") collapsed
onto the same scope -- defeating the whole point of scoping ids per
node (M-1/Menu/Tabs/ColumnLayout/Gallery/etc. all rely on it). Now
hashes the raw node id via the existing djb2 stableHash() instead of
slugifying it: still deterministic (same id -> same scope) and a valid
CSS ident, but collision-resistant across case/punctuation. This
changes the exact scope strings Menu/Tabs/ColumnLayout/Gallery/etc.
emit -- expected and fine, since none of their tests pinned an exact
scope value (all already asserted structure/uniqueness).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 18:31:12 -07:00
shadowdaoandClaude Opus 4.8 0cbc58f8d1 a11y/security: scope Navbar ids + hover styles, Gallery lightbox focus trap
M-1: Navbar.toHtml emitted a fixed id="navbar-links" and unscoped
.navbar-link/.navbar-cta :hover selectors -- two Navbars on one page
collided on the duplicate id and cross-applied each other's hover
colors (later <style> block wins in the cascade). Scope both on the
Craft node id via scopeId(), matching the Menu/Tabs pattern: the links
container gets a unique id, aria-controls/the hamburger toggle script
reference it, and the hover rules are prefixed with a per-instance
class on the <nav> root.

M-2: Gallery lightbox had no focus management -- opening it left focus
wherever it was (behind the now-visible overlay) and closing it never
restored it. The inline script now stashes document.activeElement on
open, moves focus to a new accessible close button, traps Tab on the
close button while the dialog is open, and restores the saved focus on
close (Escape, backdrop click, or the close button).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 18:30:40 -07:00
shadowdaoandClaude Opus 4.8 92841e3f35 feat(builder): send + restore site head code in save/load
Extend the save payload with head_code + design so the backend can
inject SiteDesign.headCode into published pages, and restore design
tokens on load() so the editor reflects the last-saved state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 18:22:38 -07:00
jknappandClaude Opus 4.8 e892ee0e53 Merge PR #3: site builder security & data-loss hardening + asset picker + audit backlog
51 impl commits + 10 adversarial-review fix commits. All 339→502 tests green; final adversarial XSS pass: PASS.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 01:13:27 +00:00
shadowdaoandClaude Opus 4.8 591a51dcc2 fix(builder): escape/allowlist all attribute-value sinks incl. numeric/enum props (XSS)
An adversarial pass found 5 Critical XSS sinks where props declared number/enum
in TypeScript were interpolated raw into exported HTML attribute values,
trusting the type — but nothing enforces it at runtime (AI update_props only
validates node_id; deserialized saved state is untyped JSON). Fixed all 5
(NumberCounter data-target, StarRating aria-label, FormContainer method,
ContactForm/InputField input type) plus 6 sibling sinks found by an exhaustive
audit of every attribute-value interpolation across src/components: a
JS-source injection into ContentSlider's inline setInterval script, a
prototype-pollution-adjacent allowlist gap in Section's divider-shape lookup,
TextareaField rows, Testimonials rating aria-label, HeroSimple textAlign, and
MapEmbed zoom. Adds shared sanitizeFormMethod/sanitizeInputType allowlist
helpers to utils/escape.ts alongside the existing escapeAttr/safeUrl/cssValue
primitives. Every fix is TDD'd: a malicious-value test reproduces the raw
injection against the pre-fix code, then passes after the fix.

502 tests green (npx vitest run), tsc + vite build green (npm run build).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 18:03:44 -07:00
shadowdaoandClaude Opus 4.8 7ba91d9829 fix(builder): sanitize non-string style values + allowlist element tags (XSS)
Adversarial re-review found the C1 fix incomplete plus an adjacent
same-class XSS, both reachable via the AI update_props path and
deserialized saved state:

- cssPropsToString only ran sanitizeCssValue on typeof-string values, so a
  non-string style value (array/object) with a valid key skipped
  sanitization entirely and was template-coerced raw into style="...",
  e.g. { color: ['red', '"><img src=x onerror=alert(1)>'] }. Now every
  non-number value is coerced with String() and sanitized; numbers stay
  raw. sanitizeBreakoutChars also now escapes < and > (previously only ;
  and ") as defense-in-depth, since values can reach it from non-string
  sources.

- props.tag (Container) and props.level (Heading) were interpolated raw
  into the tag position of exported HTML (`<${tag}`, `<${level}`) with no
  runtime validation, letting a malicious value break out of the tag
  entirely. Both are now allowlisted/clamped against their known-safe sets
  (div/section/article/header/footer/main; h1-h6), falling back to
  div/h2. Applied in Container's live render + toHtml, Heading's live
  render + toHtml, and the typeName==='div' fallback branch in
  html-export.ts's renderNode (hit for unresolved/legacy node types).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 17:44:59 -07:00
shadowdaoandClaude Opus 4.8 e12fb89ada Fix M-4: punctuation-only page name yields empty slug ('.html')
slugify('!!!') stripped down to '' since punctuation-only names have no
a-z0-9 characters left; buildSavePayload then wrote filename = '' +
'.html'. slugify now falls back to 'page' when the computed slug is
empty, so uniqueSlug's existing dedupe (page, page-2, ...) applies same
as any other collision. Landing page slug stays forced to 'index'.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 17:33:33 -07:00
shadowdaoandClaude Opus 4.8 25507cb57a Fix INT: ContentSlider Slides editor wrote wrong prop key (image vs imageSrc)
MediaStylePanel's Slides array editor guarded on item.image !== undefined
and wrote `image` via AssetPicker's onChange, but ContentSlider (render
+ toHtml) reads slide.imageSrc. Default slides (imageSrc:'', no `image`
key) never showed an image picker at all, and any `image` value written
was a silent no-op on render/export.

Editor now guards/reads/writes `imageSrc` throughout, and the
"add slide" emptyItem matches defaultSlides' exact shape
(type/imageSrc/heading/text/bgColor).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 17:32:47 -07:00
shadowdaoandClaude Opus 4.8 9d3bbc2c46 Fix I-2: dangling parent on shell/ColumnLayout-rooted AI-replace trees
treeToCraftState's ROOT re-key branch reparented nodes['ROOT'].nodes
children to 'ROOT' but not nodes['ROOT'].linkedNodes children. For a
ColumnLayout- or Section/BackgroundSection/FormContainer-rooted AI
`replace`, flattenTreeForCraft puts content in linkedNodes (col-N /
section-inner etc.) whose parent was left pointing at the OLD root id
-- which is then deleted, leaving a dangling parent reference that
breaks select/move/delete of those nodes in the Craft.js editor.

Now the same loop that reparents nodes[] children also reparents
linkedNodes children.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 17:31:01 -07:00
shadowdaoandClaude Opus 4.8 25e674badd Fix I-1: landing-page edits lost when activePageId goes dangling
buildSavePayload matched the active page by page.id === activePageId,
but activePageId defaults to the hardcoded 'home' and load() never
updated it. Deleting the original Home page (its replacement gets id
page_<ts>), reloading, editing, and saving would then match no page --
the live edit only reached the legacy top-level craft_state, while the
authoritative pages[]/pages_craft_state[0] for index.html fell back to
stale state.

Belt-and-suspenders fix:
- buildSavePayload now falls back to pages[0] as the effective active
  page when a real page is active but activePageId matches nothing.
- useWhpApi's load() now points activePageId at the freshly restored
  first page via a new PageContext setActivePageIdDirect setter
  (bookkeeping only -- no re-serialize/deserialize), so activePageId
  stays valid after every load.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 17:29:32 -07:00
shadowdaoandClaude Opus 4.8 e5f30a4a56 Fix C1/C2: XSS via unsanitized style keys and animation attrs
C1: cssPropsToString emitted the camelToKebab'd style-object KEY
unsanitized while only sanitizing the VALUE. A malicious style key
containing a quote (reachable via AI update_props or deserialized
saved state) could close the style="..." attribute and inject a live
element. Now validates each key against a CSS property/custom-prop
allowlist and drops anything that doesn't match.

C2: buildDataAttrs (html-export.ts) interpolated props.animation and
props.animationDelay directly into data-animation="..."/
data-animation-delay="..." with no escaping, for every exported node.
Now routes both through escapeAttr.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 17:27:22 -07:00
64 changed files with 2059 additions and 86 deletions
@@ -0,0 +1,54 @@
import { describe, test, expect } from 'vitest';
import { ButtonLink } from './ButtonLink';
const toHtml = (ButtonLink as any).toHtml;
describe('ButtonLink.toHtml href sanitization (attacker-controlled `href` prop)', () => {
test('a javascript: URL is neutralized', () => {
const { html } = toHtml({ href: 'javascript:alert(1)', text: 'Click' }, '');
expect(html).not.toContain('javascript:alert');
});
test('a quote-breakout href does not escape the href attribute', () => {
const malicious = '"><script>alert(1)</script>';
const { html } = toHtml({ href: malicious, text: 'Click' }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
test('a normal href still renders correctly', () => {
const { html } = toHtml({ href: 'https://example.com', text: 'Click' }, '');
expect(html).toContain('href="https://example.com"');
});
});
describe('ButtonLink.toHtml target (boolean-gated, not raw interpolation)', () => {
test('an attribute-breakout value for target does not reach the output raw', () => {
const malicious = '_blank" onmouseover="alert(1)' as any;
const { html } = toHtml({ href: '#', text: 'x', target: malicious }, '');
expect(html).not.toContain('onmouseover');
});
test('target="_blank" still adds rel=noopener noreferrer', () => {
const { html } = toHtml({ href: '#', text: 'x', target: '_blank' }, '');
expect(html).toContain('target="_blank"');
expect(html).toContain('rel="noopener noreferrer"');
});
});
describe('ButtonLink.toHtml text escaping (attacker-controlled `text` prop)', () => {
test('a tag-breakout attempt in text is neutralized (no injected element)', () => {
const { html } = toHtml({ href: '#', text: '</a><img src=x onerror=alert(1)>' }, '');
expect(html).not.toContain('<img');
expect(html).toContain('&lt;img');
});
test('ampersand is escaped for well-formed text content (consistency with escapeHtml)', () => {
const { html } = toHtml({ href: '#', text: 'Tom & Jerry' }, '');
expect(html).toContain('Tom &amp; Jerry');
});
test('a normal text value still renders unchanged', () => {
const { html } = toHtml({ href: '#', text: 'Click Me' }, '');
expect(html).toContain('>Click Me</a>');
});
});
+2 -2
View File
@@ -1,7 +1,7 @@
import React, { CSSProperties } from 'react'; import React, { CSSProperties } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeAttr, safeUrl } from '../../utils/escape'; import { escapeHtml, escapeAttr, safeUrl } from '../../utils/escape';
interface ButtonLinkProps { interface ButtonLinkProps {
text?: string; text?: string;
@@ -78,7 +78,7 @@ ButtonLink.craft = {
textDecoration: 'none', textDecoration: 'none',
...props.style, ...props.style,
}); });
const escapedText = (props.text || '').replace(/</g, '&lt;').replace(/>/g, '&gt;'); const escapedText = escapeHtml(props.text || '');
const targetAttr = props.target === '_blank' ? ' target="_blank" rel="noopener noreferrer"' : ''; const targetAttr = props.target === '_blank' ? ' target="_blank" rel="noopener noreferrer"' : '';
return { return {
html: `<a href="${escapeAttr(safeUrl(props.href || '#'))}"${targetAttr}${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</a>`, html: `<a href="${escapeAttr(safeUrl(props.href || '#'))}"${targetAttr}${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</a>`,
@@ -0,0 +1,34 @@
import { describe, test, expect } from 'vitest';
import { Divider } from './Divider';
const toHtml = (Divider as any).toHtml;
describe('Divider.toHtml normal rendering', () => {
test('renders thickness/color into the border-top style', () => {
const { html } = toHtml({ thickness: '2px', color: '#ff0000' }, '');
expect(html).toContain('border-top:2px solid #ff0000');
});
});
describe('Divider.toHtml XSS hardening (thickness/color into style=)', () => {
test('a thickness value with an attribute-breakout string cannot escape style=""', () => {
const malicious = '1px" onmouseover="alert(1)';
const { html } = toHtml({ thickness: malicious as any, color: '#000' }, '');
// The quote must not survive unescaped -- otherwise it closes style=""
// early and "onmouseover" becomes a live, attacker-controlled attribute.
expect(html).not.toMatch(/"\s+onmouseover="/);
expect(html).not.toMatch(/style="[^"]*"[^>]*onmouseover/);
});
test('a color value with a </style><script> breakout is neutralized', () => {
const malicious = '#000</style><script>alert(1)</script>';
const { html } = toHtml({ thickness: '1px', color: malicious as any }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
test('a non-string thickness (object) does not raw-splice into style=""', () => {
const malicious = { toString: () => '1px" onmouseover="alert(1)' };
const { html } = toHtml({ thickness: malicious as any, color: '#000' }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
});
@@ -0,0 +1,22 @@
import { describe, test, expect } from 'vitest';
import { Footer } from './Footer';
const toHtml = (Footer as any).toHtml;
describe('Footer.toHtml text escaping (attacker-controlled `text` prop)', () => {
test('a tag-breakout attempt in text is neutralized (no injected element)', () => {
const { html } = toHtml({ text: '</footer><img src=x onerror=alert(1)>' }, '');
expect(html).not.toContain('<img');
expect(html).toContain('&lt;img');
});
test('ampersand is escaped for well-formed text content (consistency with escapeHtml)', () => {
const { html } = toHtml({ text: 'Terms & Conditions' }, '');
expect(html).toContain('Terms &amp; Conditions');
});
test('a normal copyright text value still renders unchanged', () => {
const { html } = toHtml({ text: '© 2026 MySite. All rights reserved.' }, '');
expect(html).toContain('© 2026 MySite. All rights reserved.');
});
});
+2 -1
View File
@@ -1,6 +1,7 @@
import React, { CSSProperties, useCallback, useRef, useEffect } from 'react'; import React, { CSSProperties, useCallback, useRef, useEffect } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeHtml } from '../../utils/escape';
interface FooterProps { interface FooterProps {
text?: string; text?: string;
@@ -107,6 +108,6 @@ Footer.craft = {
textAlign: 'center', textAlign: 'center',
...props.style, ...props.style,
}); });
const escapedText = (props.text || '').replace(/</g, '&lt;').replace(/>/g, '&gt;'); const escapedText = escapeHtml(props.text || '');
return { html: `<footer${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</footer>` }; return { html: `<footer${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</footer>` };
}; };
@@ -0,0 +1,58 @@
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 <img>, no broken-out tag', () => {
const { html } = toHtml({ text: 'x', level: 'h2><img src=x onerror=alert(1)' }, '');
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror');
expect(html.startsWith('<h2')).toBe(true);
expect(html.endsWith('</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('<h2')).toBe(true);
expect(html.endsWith('</h2>')).toBe(true);
});
test('a non-heading string level clamps to h2', () => {
const { html } = toHtml({ text: 'x', level: 'script' as any }, '');
expect(html.startsWith('<h2')).toBe(true);
expect(html).not.toContain('<script');
});
test('a normal valid level (h4) still emits <h4', () => {
const { html } = toHtml({ text: 'x', level: 'h4' }, '');
expect(html).toContain('<h4');
expect(html).toContain('</h4>');
});
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);
}
});
});
describe('Heading.toHtml text escaping (attacker-controlled `text` prop)', () => {
test('a tag-breakout attempt in text is neutralized (no injected element)', () => {
const { html } = toHtml({ text: '</h2><img src=x onerror=alert(1)>', level: 'h2' }, '');
expect(html).not.toContain('<img');
expect(html).toContain('&lt;img');
});
test('ampersand is escaped for well-formed text content (consistency with escapeHtml)', () => {
const { html } = toHtml({ text: 'Fish & Chips', level: 'h2' }, '');
expect(html).toContain('Fish &amp; Chips');
});
test('a normal text value still renders unchanged', () => {
const { html } = toHtml({ text: 'Hello world', level: 'h2' }, '');
expect(html).toBe('<h2>Hello world</h2>');
});
});
+15 -3
View File
@@ -1,9 +1,20 @@
import React, { CSSProperties, useCallback, useRef, useEffect } from 'react'; import React, { CSSProperties, useCallback, useRef, useEffect } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeHtml } from '../../utils/escape';
type HeadingLevel = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'; 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><img src=x
// onerror=alert(1)` (or a non-h1-6 string) must never reach that position
// unchecked. Anything not in this allowlist clamps to `'h2'`.
const ALLOWED_HEADING_LEVELS = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'] as const;
const sanitizeHeadingLevel = (level: unknown): HeadingLevel =>
(ALLOWED_HEADING_LEVELS as readonly unknown[]).includes(level) ? (level as HeadingLevel) : 'h2';
interface HeadingProps { interface HeadingProps {
text?: string; text?: string;
level?: HeadingLevel; level?: HeadingLevel;
@@ -30,6 +41,7 @@ export const Heading: UserComponent<HeadingProps> = ({
selected: node.events.selected, selected: node.events.selected,
})); }));
const safeLevel = sanitizeHeadingLevel(level);
const elRef = useRef<HTMLElement | null>(null); const elRef = useRef<HTMLElement | null>(null);
const editedTextRef = useRef<string | null>(null); const editedTextRef = useRef<string | null>(null);
@@ -59,7 +71,7 @@ export const Heading: UserComponent<HeadingProps> = ({
} }
}, [text, selected]); }, [text, selected]);
return React.createElement(level, { return React.createElement(safeLevel, {
ref: (ref: HTMLElement | null): void => { ref: (ref: HTMLElement | null): void => {
elRef.current = ref; elRef.current = ref;
if (ref) connect(drag(ref)); if (ref) connect(drag(ref));
@@ -98,8 +110,8 @@ Heading.craft = {
}; };
(Heading as any).toHtml = (props: HeadingProps, _childrenHtml: string) => { (Heading as any).toHtml = (props: HeadingProps, _childrenHtml: string) => {
const tag = props.level || 'h2'; const tag = sanitizeHeadingLevel(props.level);
const safeText = (props.text || '').replace(/</g, '&lt;').replace(/>/g, '&gt;'); const safeText = escapeHtml(props.text || '');
const styleStr = cssPropsToString(props.style); const styleStr = cssPropsToString(props.style);
return { html: `<${tag}${styleStr ? ` style="${styleStr}"` : ''}>${safeText}</${tag}>` }; return { html: `<${tag}${styleStr ? ` style="${styleStr}"` : ''}>${safeText}</${tag}>` };
}; };
@@ -21,3 +21,46 @@ describe('purifyHtml', () => {
expect(purifyHtml('<form><input name="x"></form>')).not.toContain('<form'); expect(purifyHtml('<form><input name="x"></form>')).not.toContain('<form');
}); });
}); });
describe('purifyHtml iframe sandboxing (M-6)', () => {
test('forces a restrictive sandbox attribute onto every iframe', () => {
const out = purifyHtml('<iframe src="https://example.com/"></iframe>');
expect(out).toMatch(/<iframe[^>]*\bsandbox="[^"]+"/);
});
test('sandbox value omits allow-top-navigation (no top-level nav escape)', () => {
const out = purifyHtml('<iframe src="https://example.com/"></iframe>');
const sandbox = out.match(/sandbox="([^"]*)"/)![1];
expect(sandbox).not.toMatch(/allow-top-navigation/);
});
test('legitimate embeds (YouTube) still work and get sandboxed too', () => {
const out = purifyHtml('<iframe src="https://www.youtube.com/embed/abc" allowfullscreen></iframe>');
expect(out).toContain('youtube.com/embed/abc');
expect(out).toMatch(/<iframe[^>]*\bsandbox="[^"]+"/);
});
test('adds referrerpolicy=no-referrer to iframes', () => {
const out = purifyHtml('<iframe src="https://example.com/"></iframe>');
expect(out).toContain('referrerpolicy="no-referrer"');
});
test('script/on* attributes are still stripped alongside the sandboxed iframe', () => {
const out = purifyHtml('<iframe src="https://example.com/" onload="alert(1)"></iframe><script>alert(2)</script>');
expect(out).not.toContain('onload');
expect(out).not.toContain('<script');
});
test('repeated calls do not leak/accumulate the hook (no duplicate sandbox attr, no cross-call state)', () => {
purifyHtml('<iframe src="https://a.example/"></iframe>');
purifyHtml('<iframe src="https://b.example/"></iframe>');
const out = purifyHtml('<iframe src="https://c.example/"></iframe>');
const sandboxMatches = out.match(/sandbox="/g) || [];
expect(sandboxMatches.length).toBe(1);
});
test('a non-iframe element sanitized alongside an iframe is not touched by the hook', () => {
const out = purifyHtml('<p>hi</p><iframe src="https://example.com/"></iframe>');
expect(out).toContain('<p>hi</p>');
});
});
@@ -10,4 +10,16 @@ describe('HtmlBlock.toHtml sanitizes raw code (A4.1)', () => {
expect(html).not.toContain('onclick'); expect(html).not.toContain('onclick');
expect(html).toContain('<p>hi</p>'); expect(html).toContain('<p>hi</p>');
}); });
test('does not wrap output in an unsanitized element carrying the style prop raw', () => {
// toHtml only ever returns the sanitized `code` blob -- there is no
// wrapper <div style="..."> in the exported HTML, so a malicious
// `style` prop (e.g. an attacker-controlled object with a breakout
// toString()) has nothing to splice into.
const malicious = { toString: () => 'color:red" onmouseover="alert(1)' } as any;
const { html } = toHtml({ code: '<p>hi</p>', style: malicious }, '');
expect(html).not.toMatch(/onmouseover/);
expect(html).not.toMatch(/<div/);
expect(html).toBe('<p>hi</p>');
});
}); });
+28
View File
@@ -24,14 +24,42 @@ const PURIFY_CONFIG = {
'href','src','alt','title','target','rel', 'href','src','alt','title','target','rel',
'width','height','class', 'width','height','class',
'allowfullscreen','allow','frameborder', 'allowfullscreen','allow','frameborder',
'sandbox','referrerpolicy',
], ],
ALLOWED_URI_REGEXP: /^(?:(?:https?|mailto|tel|data:image\/[a-z]+;base64,):|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i, ALLOWED_URI_REGEXP: /^(?:(?:https?|mailto|tel|data:image\/[a-z]+;base64,):|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i,
FORBID_TAGS: ['script','style','object','embed','link','meta','form','input','button','select','textarea'], FORBID_TAGS: ['script','style','object','embed','link','meta','form','input','button','select','textarea'],
FORBID_ATTR: [/^on/i], FORBID_ATTR: [/^on/i],
}; };
// M-6: `<iframe>` is allowed (maps/video embeds are a legitimate use case)
// but an iframe with a `src` and NO `sandbox` attribute is a clickjacking/
// phishing vector (DOMPurify already strips <script>/on*=, but an
// unsandboxed iframe still gets full script execution, same-origin-ish
// access via document.domain tricks, top-level navigation, etc., inside
// itself). This hook force-sets a restrictive sandbox on every iframe that
// survives sanitization, keeping `allow-scripts`/`allow-same-origin`/
// `allow-popups`/`allow-forms` (needed for interactive maps/video/oauth
// popups) but deliberately omitting `allow-top-navigation` so an embedded
// page can never redirect/hijack the parent tab.
const IFRAME_SANDBOX_HOOK = (node: Element): void => {
if (node.nodeName === 'IFRAME') {
node.setAttribute('sandbox', 'allow-scripts allow-same-origin allow-popups allow-forms');
node.setAttribute('referrerpolicy', 'no-referrer');
}
};
export function purifyHtml(input: string): string { export function purifyHtml(input: string): string {
// Hook is added immediately before sanitize() and removed immediately
// after, scoped tightly to this single call -- so it can never leak onto
// (or accumulate duplicate copies across) any other DOMPurify.sanitize()
// call elsewhere in the app, and repeated purifyHtml() calls never stack
// multiple copies of the same hook.
DOMPurify.addHook('afterSanitizeAttributes', IFRAME_SANDBOX_HOOK);
try {
return DOMPurify.sanitize(input || '', PURIFY_CONFIG as any) as unknown as string; return DOMPurify.sanitize(input || '', PURIFY_CONFIG as any) as unknown as string;
} finally {
DOMPurify.removeHook('afterSanitizeAttributes', IFRAME_SANDBOX_HOOK as any);
}
} }
export const HtmlBlock: UserComponent<HtmlBlockProps> = ({ code = '', style = {} }) => { export const HtmlBlock: UserComponent<HtmlBlockProps> = ({ code = '', style = {} }) => {
@@ -0,0 +1,46 @@
import { describe, test, expect } from 'vitest';
import { Icon } from './Icon';
const toHtml = (Icon as any).toHtml;
describe('Icon.toHtml normal rendering', () => {
test('renders icon class, size/color style, and link href', () => {
const { html } = toHtml({ icon: 'fa-star', size: '32px', color: '#3b82f6', link: 'https://example.com' }, '');
expect(html).toContain('class="fa fa-star"');
expect(html).toContain('font-size:32px');
expect(html).toContain('color:#3b82f6');
expect(html).toContain('href="https://example.com"');
});
});
describe('Icon.toHtml XSS hardening', () => {
test('an icon name with an attribute-breakout string is escaped, not raw-concatenated', () => {
const malicious = 'star"><script>alert(1)</script>';
const { html } = toHtml({ icon: malicious as any }, '');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toMatch(/class="fa star"><script>/);
});
test('a size value with an attribute-breakout string cannot escape style=""', () => {
const malicious = '24px" onerror="alert(1)';
const { html } = toHtml({ size: malicious as any }, '');
expect(html).not.toMatch(/"\s+onerror="/);
});
test('a bgSize/bgColor breakout via background wrapper is neutralized', () => {
const malicious = '56px" onmouseover="alert(1)';
const { html } = toHtml({ bgShape: 'circle', bgColor: '#fff', bgSize: malicious as any }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
test('a javascript: link is neutralized to an empty href', () => {
const { html } = toHtml({ link: 'javascript:alert(1)' }, '');
expect(html).not.toContain('javascript:alert(1)');
});
test('a link value with an attribute-breakout string cannot escape href=""', () => {
const malicious = 'https://example.com" onclick="alert(1)';
const { html } = toHtml({ link: malicious as any }, '');
expect(html).not.toMatch(/"\s+onclick="/);
});
});
@@ -0,0 +1,62 @@
import { describe, test, expect } from 'vitest';
import { Logo } from './Logo';
/*
* Regression coverage for Logo.toHtml -- audited during the toHtml
* attribute-XSS sweep (see task-cssxss-brief.md) and found already fully
* sanitized (href/src via escapeAttr(safeUrl()), alt/text via escapeAttr /
* escapeHtml, imageWidth/fontSize/etc. routed through cssPropsToString which
* sanitizes every value regardless of declared type). No fix was required;
* these tests lock that behavior in against regressions.
*/
const toHtml = (Logo as any).toHtml;
describe('Logo.toHtml href sanitization (attacker-controlled `href` prop)', () => {
test('a javascript: URL is neutralized', () => {
const { html } = toHtml({ href: 'javascript:alert(1)' }, '');
expect(html).not.toContain('javascript:alert');
});
test('a quote-breakout href does not escape the anchor attribute', () => {
const malicious = '"><script>alert(1)</script>';
const { html } = toHtml({ href: malicious }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
});
describe('Logo.toHtml image src/alt sanitization (type="image")', () => {
test('a javascript: imageSrc is neutralized', () => {
const { html } = toHtml({ type: 'image', imageSrc: 'javascript:alert(1)', text: 'Logo' }, '');
expect(html).not.toContain('javascript:alert');
});
test('a quote-breakout alt (from `text`) does not escape the img attribute', () => {
const malicious = '"><script>alert(1)</script>';
const { html } = toHtml({ type: 'image', imageSrc: 'https://example.com/logo.png', text: malicious }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
test('a non-numeric imageWidth (attribute-breakout attempt) does not escape the style attribute', () => {
const malicious = '1"><script>alert(1)</script>';
const { html } = toHtml({ type: 'image', imageSrc: 'https://example.com/logo.png', imageWidth: malicious }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
});
describe('Logo.toHtml text-logo styling sanitization', () => {
test('a quote-breakout color does not escape the span style attribute', () => {
const malicious = 'red" onmouseover="alert(1)';
const { html } = toHtml({ type: 'text', text: 'MySite', color: malicious }, '');
// The raw `"` must never survive un-escaped inside the style attribute
// value -- if it did, `onmouseover` would land as a REAL new HTML
// attribute (breakout) rather than being inert CSS-value garbage inside
// a properly-escaped style="...".
expect(html).not.toMatch(/style="[^"]*"[^>]*onmouseover/);
});
test('a normal logo renders as expected', () => {
const { html } = toHtml({ type: 'text', text: 'MySite', href: '/' }, '');
expect(html).toContain('href="/"');
expect(html).toContain('MySite');
});
});
@@ -5,28 +5,84 @@ const toHtml = (Navbar as any).toHtml;
describe('Navbar.toHtml hamburger accessibility (F2.3)', () => { describe('Navbar.toHtml hamburger accessibility (F2.3)', () => {
test('mobile toggle button has an accessible name, aria-expanded, and aria-controls', () => { test('mobile toggle button has an accessible name, aria-expanded, and aria-controls', () => {
const { html } = toHtml({ showMobileMenu: true }, ''); const { html } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
expect(html).toMatch(/class="navbar-hamburger"[^>]*aria-label="Toggle navigation menu"/); expect(html).toMatch(/class="navbar-hamburger"[^>]*aria-label="Toggle navigation menu"/);
expect(html).toMatch(/aria-expanded="false"/); expect(html).toMatch(/aria-expanded="false"/);
expect(html).toMatch(/aria-controls="navbar-links"/); expect(html).toMatch(/aria-controls="[^"]+"/);
}); });
test('aria-controls target id exists on the links container', () => { test('aria-controls target id exists on the links container', () => {
const { html } = toHtml({ showMobileMenu: true }, ''); const { html } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
expect(html).toContain('id="navbar-links"'); const controls = html.match(/aria-controls="([^"]+)"/)![1];
expect(html).toContain(`id="${controls}"`);
}); });
test('toggle script flips aria-expanded on click', () => { test('toggle script flips aria-expanded on click', () => {
const { html } = toHtml({ showMobileMenu: true }, ''); const { html } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
expect(html).toMatch(/setAttribute\(['"]aria-expanded['"]/); expect(html).toMatch(/setAttribute\(['"]aria-expanded['"]/);
}); });
test('no mobile menu: no hamburger button emitted', () => { test('no mobile menu: no hamburger button emitted', () => {
const { html } = toHtml({ showMobileMenu: false }, ''); const { html } = toHtml({ showMobileMenu: false }, '', 'node-nav1');
expect(html).not.toContain('navbar-hamburger'); expect(html).not.toContain('navbar-hamburger');
}); });
}); });
describe('Navbar.toHtml node-scoped ids/hover styles (M-1: two navbars must not collide)', () => {
test('no bare unscoped id="navbar-links" is emitted', () => {
const { html } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
expect(html).not.toContain('id="navbar-links"');
});
test('two different node ids produce different links-container ids', () => {
const { html: html1 } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
const { html: html2 } = toHtml({ showMobileMenu: true }, '', 'node-nav2');
const id1 = html1.match(/id="([^"]+)"/)![1];
const id2 = html2.match(/id="([^"]+)"/)![1];
expect(id1).not.toBe(id2);
});
test('aria-controls always equals the actual links-container id', () => {
const { html } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
const controls = html.match(/aria-controls="([^"]+)"/)![1];
const linksId = html.match(/id="([^"]+)"/)![1];
expect(controls).toBe(linksId);
});
test('hover style selectors are scoped per-instance, not bare .navbar-link/.navbar-cta', () => {
const { html } = toHtml({ hoverColor: '#ff0000' }, '', 'node-nav1');
// A selector rule that STARTS the line with .navbar-link:hover (i.e. not
// preceded by a per-instance ancestor class) would be the old, unscoped,
// globally-colliding form.
expect(html).not.toMatch(/^\s*\.navbar-link:hover/m);
expect(html).not.toMatch(/^\s*\.navbar-cta:hover/m);
// still present, just scoped under a per-instance ancestor class
expect(html).toMatch(/\.navbar-link:hover/);
expect(html).toMatch(/\.[\w-]+ \.navbar-link:hover/);
});
test('two navbars with different hoverColor do not leak style onto each other (scoped selectors differ)', () => {
const { html: html1 } = toHtml({ hoverColor: '#ff0000' }, '', 'node-nav1');
const { html: html2 } = toHtml({ hoverColor: '#00ff00' }, '', 'node-nav2');
const scope1 = html1.match(/<style>\s*\.([\w-]+)\s/)![1];
const scope2 = html2.match(/<style>\s*\.([\w-]+)\s/)![1];
expect(scope1).not.toBe(scope2);
expect(html1).toContain(`.${scope1} .navbar-link:hover`);
expect(html2).toContain(`.${scope2} .navbar-link:hover`);
});
test('a normal single navbar still renders its hover style (visual output preserved)', () => {
const { html } = toHtml({ hoverColor: '#ff0000' }, '', 'node-nav1');
expect(html).toMatch(/:hover\s*\{\s*color:\s*#ff0000/);
});
test('same node id -> identical output across calls (deterministic)', () => {
const { html: html1 } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
const { html: html2 } = toHtml({ showMobileMenu: true }, '', 'node-nav1');
expect(html1).toBe(html2);
});
});
describe('Navbar.toHtml XSS hardening (hoverColor/backgroundColor/ctaColor into <style>)', () => { describe('Navbar.toHtml XSS hardening (hoverColor/backgroundColor/ctaColor into <style>)', () => {
test('a hoverColor value containing </style><script> is neutralized in the hover <style> block', () => { test('a hoverColor value containing </style><script> is neutralized in the hover <style> block', () => {
const malicious = '#fff}</style><script>alert(1)</script><style>{'; const malicious = '#fff}</style><script>alert(1)</script><style>{';
+26 -11
View File
@@ -2,7 +2,7 @@ import React, { CSSProperties, useState } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { useSiteDesign } from '../../state/SiteDesignContext'; import { useSiteDesign } from '../../state/SiteDesignContext';
import { escapeHtml, escapeAttr, safeUrl, cssValue } from '../../utils/escape'; import { escapeHtml, escapeAttr, safeUrl, cssValue, scopeId } from '../../utils/escape';
/* ---------- Types ---------- */ /* ---------- Types ---------- */
@@ -210,7 +210,7 @@ Navbar.craft = {
/* ---------- HTML export ---------- */ /* ---------- HTML export ---------- */
(Navbar as any).toHtml = (props: NavbarProps, _childrenHtml: string) => { (Navbar as any).toHtml = (props: NavbarProps, _childrenHtml: string, nodeId?: string) => {
// Sanitized once here -- these are raw string-interpolation sinks below // Sanitized once here -- these are raw string-interpolation sinks below
// (hoverCol/bgColor go into a <style> block, the worst case: </style> // (hoverCol/bgColor go into a <style> block, the worst case: </style>
// breakout -> arbitrary <script>), see task-cssxss-brief.md. // breakout -> arbitrary <script>), see task-cssxss-brief.md.
@@ -224,6 +224,19 @@ Navbar.craft = {
const sticky = props.isSticky; const sticky = props.isSticky;
const mobile = props.showMobileMenu; const mobile = props.showMobileMenu;
const logoUrl = props.logoUrl || '/'; const logoUrl = props.logoUrl || '/';
const links0 = props.links || defaultLinks;
// M-1: deterministic AND unique per-instance scope, keyed on the Craft
// node id. Two Navbars on the same page previously emitted an identical
// fixed id="navbar-links" (invalid duplicate-id HTML, ambiguous
// aria-controls target) and unscoped `.navbar-link:hover`/`.navbar-cta:hover`
// rules in each instance's own <style> block -- since both blocks target
// the SAME global selector, the later one in the DOM silently overrides
// the earlier one's hover color/behavior for BOTH navbars. Scoping the
// links-container id and adding a per-instance class on the <nav> root
// (used to prefix the hover selectors) eliminates both collisions.
const scope = scopeId(nodeId, JSON.stringify(links0) + alignment + pad, 'nav');
const linksId = `${scope}_links`;
const navStyle = cssPropsToString({ const navStyle = cssPropsToString({
display: 'flex', display: 'flex',
@@ -272,21 +285,23 @@ Navbar.craft = {
// via aria-expanded, kept in sync with the .navbar-open class by the // via aria-expanded, kept in sync with the .navbar-open class by the
// inline onclick handler. // inline onclick handler.
const hamburgerHtml = mobile const hamburgerHtml = mobile
? `\n <button class="navbar-hamburger" aria-label="Toggle navigation menu" aria-expanded="false" aria-controls="navbar-links" onclick="var m=this.parentElement.querySelector('.navbar-links');var open=m.classList.toggle('navbar-open');this.setAttribute('aria-expanded', open ? 'true' : 'false');" style="display:none;background:none;border:none;cursor:pointer;padding:4px;flex-direction:column;gap:4px"> ? `\n <button class="navbar-hamburger" aria-label="Toggle navigation menu" aria-expanded="false" aria-controls="${escapeAttr(linksId)}" onclick="var m=document.getElementById('${linksId}');var open=m.classList.toggle('navbar-open');this.setAttribute('aria-expanded', open ? 'true' : 'false');" style="display:none;background:none;border:none;cursor:pointer;padding:4px;flex-direction:column;gap:4px">
<span style="display:block;width:24px;height:2px;background-color:${escapeAttr(textCol)}"></span> <span style="display:block;width:24px;height:2px;background-color:${escapeAttr(textCol)}"></span>
<span style="display:block;width:24px;height:2px;background-color:${escapeAttr(textCol)}"></span> <span style="display:block;width:24px;height:2px;background-color:${escapeAttr(textCol)}"></span>
<span style="display:block;width:24px;height:2px;background-color:${escapeAttr(textCol)}"></span> <span style="display:block;width:24px;height:2px;background-color:${escapeAttr(textCol)}"></span>
</button>` </button>`
: ''; : '';
// Hover CSS // Hover CSS -- scoped under `.${scope}` (a class on the <nav> root, added
// below) so it can only ever match THIS instance's links/CTA, never bleed
// into or get overridden by another Navbar instance's rules.
const hoverCss = `<style> const hoverCss = `<style>
.navbar-link:hover { color: ${hoverCol} !important; } .${scope} .navbar-link:hover { color: ${hoverCol} !important; }
.navbar-cta:hover { filter: brightness(1.1); }${mobile ? ` .${scope} .navbar-cta:hover { filter: brightness(1.1); }${mobile ? `
@media (max-width: 768px) { @media (max-width: 768px) {
.navbar-hamburger { display: flex !important; } .${scope} .navbar-hamburger { display: flex !important; }
.navbar-links { display: none !important; position: absolute; top: 100%; left: 0; right: 0; flex-direction: column !important; background-color: ${bgColor}; padding: 12px 24px; gap: 12px !important; box-shadow: 0 4px 12px rgba(0,0,0,0.1); } .${scope} .navbar-links { display: none !important; position: absolute; top: 100%; left: 0; right: 0; flex-direction: column !important; background-color: ${bgColor}; padding: 12px 24px; gap: 12px !important; box-shadow: 0 4px 12px rgba(0,0,0,0.1); }
.navbar-links.navbar-open { display: flex !important; } .${scope} .navbar-links.navbar-open { display: flex !important; }
}` : ''} }` : ''}
</style>`; </style>`;
@@ -309,9 +324,9 @@ Navbar.craft = {
return { return {
html: `${hoverCss} html: `${hoverCss}
<nav${navStyle ? ` style="${navStyle}${mobile ? ';position:relative' : ''}"` : ''}> <nav class="${scope}"${navStyle ? ` style="${navStyle}${mobile ? ';position:relative' : ''}"` : ''}>
${logoHtml}${hamburgerHtml} ${logoHtml}${hamburgerHtml}
<div class="navbar-links" id="navbar-links" style="display:flex;align-items:center;gap:24px"> <div class="navbar-links" id="${linksId}" style="display:flex;align-items:center;gap:24px">
${linksHtmlWithClass} ${linksHtmlWithClass}
</div> </div>
</nav>`, </nav>`,
@@ -11,3 +11,22 @@ describe('SearchBar.toHtml decorative icons (F2.5)', () => {
icons.forEach((tag: string) => expect(tag).toContain('aria-hidden="true"')); icons.forEach((tag: string) => expect(tag).toContain('aria-hidden="true"'));
}); });
}); });
describe('SearchBar.toHtml XSS hardening (placeholder/buttonText/showButton)', () => {
test('a placeholder value with an attribute-breakout string cannot escape placeholder=""', () => {
const malicious = 'Search..." onmouseover="alert(1)';
const { html } = toHtml({ placeholder: malicious }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
test('a buttonText value with a script tag is escaped as text content, not raw HTML', () => {
const malicious = '<script>alert(1)</script>';
const { html } = toHtml({ buttonText: malicious, showButton: true }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
test('a non-boolean showButton (string "false") still yields fixed, safe border-radius values', () => {
const { html } = toHtml({ showButton: 'false' as any }, '');
expect(html).toMatch(/border-radius:(8px 0 0 8px|8px)/);
});
});
@@ -14,3 +14,40 @@ describe('SocialLinks.toHtml accessibility (F2.5)', () => {
expect(html).toMatch(/<i class="fa fa-twitter"[^>]*aria-hidden="true"/); expect(html).toMatch(/<i class="fa fa-twitter"[^>]*aria-hidden="true"/);
}); });
}); });
describe('SocialLinks.toHtml XSS hardening (iconSize/iconColor/iconBgColor/gap into style=)', () => {
test('an iconSize value with an attribute-breakout string cannot escape style=""', () => {
const malicious = '20px" onmouseover="alert(1)';
const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], iconSize: malicious as any }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
test('an iconColor value with an attribute-breakout string cannot escape style=""', () => {
const malicious = '#fff" onmouseover="alert(1)';
const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], iconColor: malicious as any }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
test('an iconBgColor value with an attribute-breakout string cannot escape style=""', () => {
const malicious = '#374151" onmouseover="alert(1)';
const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], iconShape: 'circle', iconBgColor: malicious as any }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
test('a gap value with an attribute-breakout string cannot escape the wrapper style=""', () => {
const malicious = '10px" onmouseover="alert(1)';
const { html } = toHtml({ links: [{ platform: 'facebook', url: '#' }], gap: malicious as any }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
});
test('a malicious platform key does not produce a raw class-attribute breakout', () => {
const malicious = 'x"><script>alert(1)</script>';
const { html } = toHtml({ links: [{ platform: malicious, url: '#' }] }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
test('a link url with a javascript: scheme is neutralized', () => {
const { html } = toHtml({ links: [{ platform: 'facebook', url: 'javascript:alert(1)' }] }, '');
expect(html).not.toContain('javascript:alert(1)');
});
});
@@ -0,0 +1,26 @@
import { describe, test, expect } from 'vitest';
import { Spacer } from './Spacer';
const toHtml = (Spacer as any).toHtml;
describe('Spacer.toHtml normal rendering', () => {
test('renders height into the style attribute', () => {
const { html } = toHtml({ height: '80px' }, '');
expect(html).toContain('height:80px');
});
});
describe('Spacer.toHtml XSS hardening (height into style=)', () => {
test('a height value with an attribute-breakout string cannot escape style=""', () => {
const malicious = '40px" onmouseover="alert(1)';
const { html } = toHtml({ height: malicious as any }, '');
expect(html).not.toMatch(/"\s+onmouseover="/);
expect(html).not.toMatch(/style="[^"]*"[^>]*onmouseover/);
});
test('a height value with a </style><script> breakout is neutralized', () => {
const malicious = '40px</style><script>alert(1)</script>';
const { html } = toHtml({ height: malicious as any }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
});
@@ -40,3 +40,38 @@ describe('StarRating.toHtml XSS hardening (filledColor/emptyColor/size into styl
expect(html).toContain('color:#ff9900'); expect(html).toContain('color:#ff9900');
}); });
}); });
describe('StarRating.toHtml XSS hardening (rating/maxStars into aria-label, F2.2 CONFIRMED sink)', () => {
test('a maxStars value with an attribute-breakout string is neutralized in aria-label', () => {
const malicious = '5" onmouseover="alert(1)';
const { html } = toHtml({ rating: 3, maxStars: malicious as any }, '');
expect(html).not.toMatch(/onmouseover/);
expect(html).not.toMatch(/aria-label="Rating: 3 out of 5" onmouseover/);
});
test('a rating value with an attribute-breakout string is neutralized in aria-label', () => {
const malicious = '4.5" onmouseover="alert(1)';
const { html } = toHtml({ rating: malicious as any, maxStars: 5 }, '');
expect(html).not.toMatch(/onmouseover/);
});
test('a non-numeric maxStars does not blow up the star loop (no NaN glyph count, no huge output)', () => {
const malicious = '5" onmouseover="alert(1)';
const { html } = toHtml({ rating: 3, maxStars: malicious as any }, '');
const glyphs = html.match(/<i class="fa fa-star"/g) || [];
// Falls back to a sane default star count rather than looping 0 or NaN times.
expect(glyphs.length).toBeGreaterThan(0);
expect(glyphs.length).toBeLessThanOrEqual(50);
});
test('an absurdly large maxStars is clamped to a sane maximum instead of looping unboundedly', () => {
const { html } = toHtml({ rating: 3, maxStars: 1e9 as any }, '');
const glyphs = html.match(/<i class="fa fa-star"/g) || [];
expect(glyphs.length).toBeLessThanOrEqual(50);
});
test('normal numeric rating/maxStars still render the expected aria-label', () => {
const { html } = toHtml({ rating: 4.5, maxStars: 5 }, '');
expect(html).toMatch(/<span role="img" aria-label="Rating: 4\.5 out of 5"/);
});
});
+20 -4
View File
@@ -1,7 +1,7 @@
import React, { CSSProperties } from 'react'; import React, { CSSProperties } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { cssValue } from '../../utils/escape'; import { cssValue, escapeAttr } from '../../utils/escape';
interface StarRatingProps { interface StarRatingProps {
rating?: number; rating?: number;
@@ -98,8 +98,19 @@ StarRating.craft = {
/* ---------- HTML export ---------- */ /* ---------- HTML export ---------- */
(StarRating as any).toHtml = (props: StarRatingProps, _childrenHtml: string) => { (StarRating as any).toHtml = (props: StarRatingProps, _childrenHtml: string) => {
const rating = props.rating ?? 4.5; // `rating`/`maxStars` are declared `number` in TS but arrive unchecked at
const maxStars = props.maxStars || 5; // runtime (AI update_props only validates node_id; deserialized saved
// state is untyped JSON) -- a string like `5" onmouseover="alert(1)`
// breaks out of the aria-label attribute below, and an uncoerced/unclamped
// maxStars can also blow up the star-glyph loop (NaN, absurd loop count,
// or -- observed -- a RangeError from string concatenation overflow with
// e.g. maxStars=1e9). Coerce to numbers with sane fallbacks/clamps first.
const ratingRaw = Number(props.rating);
const rating = Number.isFinite(ratingRaw) ? ratingRaw : 4.5;
const maxStarsRaw = Number(props.maxStars);
const maxStars = Number.isFinite(maxStarsRaw)
? Math.min(Math.max(Math.trunc(maxStarsRaw), 0), 50)
: 5;
// Sanitized -- raw string-interpolation sinks in the star glyphs below. // Sanitized -- raw string-interpolation sinks in the star glyphs below.
const size = cssValue(props.size) || '24px'; const size = cssValue(props.size) || '24px';
const filledColor = cssValue(props.filledColor) || '#f59e0b'; const filledColor = cssValue(props.filledColor) || '#f59e0b';
@@ -125,7 +136,12 @@ StarRating.craft = {
// The star glyphs convey nothing to assistive tech on their own -- wrap // The star glyphs convey nothing to assistive tech on their own -- wrap
// in role="img" with a textual equivalent, and hide the decorative glyphs // in role="img" with a textual equivalent, and hide the decorative glyphs
// themselves (aria-hidden above) so AT doesn't announce each icon. // themselves (aria-hidden above) so AT doesn't announce each icon.
// Belt-and-suspenders: rating/maxStars are already coerced to numbers
// above, but the assembled label is still run through escapeAttr() in
// case a decimal/negative/Infinity edge case produces odd (though no
// longer dangerous) text.
const ariaLabel = escapeAttr(`Rating: ${rating} out of ${maxStars}`);
return { return {
html: `<span role="img" aria-label="Rating: ${rating} out of ${maxStars}"${wrapperStyle ? ` style="${wrapperStyle}"` : ''}>${starsHtml}</span>`, html: `<span role="img" aria-label="${ariaLabel}"${wrapperStyle ? ` style="${wrapperStyle}"` : ''}>${starsHtml}</span>`,
}; };
}; };
@@ -0,0 +1,22 @@
import { describe, test, expect } from 'vitest';
import { TextBlock } from './TextBlock';
const toHtml = (TextBlock as any).toHtml;
describe('TextBlock.toHtml text escaping (attacker-controlled `text` prop)', () => {
test('a tag-breakout attempt in text is neutralized (no injected element)', () => {
const { html } = toHtml({ text: '</p><img src=x onerror=alert(1)>' }, '');
expect(html).not.toContain('<img');
expect(html).toContain('&lt;img');
});
test('ampersand is escaped for well-formed text content (consistency with escapeHtml)', () => {
const { html } = toHtml({ text: 'Tom & Jerry' }, '');
expect(html).toContain('Tom &amp; Jerry');
});
test('a normal text value still renders unchanged', () => {
const { html } = toHtml({ text: 'Hello world' }, '');
expect(html).toBe('<p>Hello world</p>');
});
});
+2 -1
View File
@@ -1,6 +1,7 @@
import React, { CSSProperties, useCallback, useRef, useEffect } from 'react'; import React, { CSSProperties, useCallback, useRef, useEffect } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeHtml } from '../../utils/escape';
interface TextBlockProps { interface TextBlockProps {
text?: string; text?: string;
@@ -95,6 +96,6 @@ TextBlock.craft = {
(TextBlock as any).toHtml = (props: TextBlockProps, _childrenHtml: string) => { (TextBlock as any).toHtml = (props: TextBlockProps, _childrenHtml: string) => {
const styleStr = cssPropsToString(props.style); const styleStr = cssPropsToString(props.style);
const escapedText = (props.text || '').replace(/</g, '&lt;').replace(/>/g, '&gt;'); const escapedText = escapeHtml(props.text || '');
return { html: `<p${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</p>` }; return { html: `<p${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</p>` };
}; };
@@ -110,3 +110,19 @@ describe('ContactForm.toHtml accessibility (F2.1)', () => {
expect(ids1).toEqual(ids2); expect(ids1).toEqual(ids2);
}); });
}); });
describe('ContactForm.toHtml field type attribute sanitization', () => {
test('malicious field.type cannot break out of the input attribute; falls back to type="text"', () => {
const fields = [{ type: 'text"><img src=x onerror=alert(1)>' as any, label: 'Name', name: 'name', placeholder: 'Your name', required: false }];
const { html } = toHtml({ fields }, '');
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror=');
expect(html).toContain('type="text"');
});
test('legitimate email field type still passes through unchanged', () => {
const fields = [{ type: 'email' as const, label: 'Email', name: 'email', placeholder: 'you@example.com', required: false }];
const { html } = toHtml({ fields }, '');
expect(html).toContain('type="email"');
});
});
+2 -2
View File
@@ -2,7 +2,7 @@ import React, { CSSProperties } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { relayFormWiring } from '../../utils/form-relay-wiring'; import { relayFormWiring } from '../../utils/form-relay-wiring';
import { escapeHtml, escapeAttr, slugId, cssValue } from '../../utils/escape'; import { escapeHtml, escapeAttr, slugId, cssValue, sanitizeInputType } from '../../utils/escape';
interface ContactFormField { interface ContactFormField {
type: 'text' | 'email' | 'tel' | 'textarea' | 'select'; type: 'text' | 'email' | 'tel' | 'textarea' | 'select';
@@ -196,7 +196,7 @@ ContactForm.craft = {
const opts = (field.options || []).map((o) => `<option value="${escapeAttr(o)}">${escapeHtml(o)}</option>`).join(''); const opts = (field.options || []).map((o) => `<option value="${escapeAttr(o)}">${escapeHtml(o)}</option>`).join('');
inputHtml = `<select id="${escapeAttr(fieldId)}" name="${escapeAttr(field.name)}" style="${inputStyleStr};cursor:pointer"${reqAttr}><option value="">${escapeHtml(field.placeholder || 'Select...')}</option>${opts}</select>`; inputHtml = `<select id="${escapeAttr(fieldId)}" name="${escapeAttr(field.name)}" style="${inputStyleStr};cursor:pointer"${reqAttr}><option value="">${escapeHtml(field.placeholder || 'Select...')}</option>${opts}</select>`;
} else { } else {
inputHtml = `<input id="${escapeAttr(fieldId)}" type="${field.type}" name="${escapeAttr(field.name)}" placeholder="${escapeAttr(field.placeholder)}" style="${inputStyleStr}"${reqAttr} />`; inputHtml = `<input id="${escapeAttr(fieldId)}" type="${sanitizeInputType(field.type)}" name="${escapeAttr(field.name)}" placeholder="${escapeAttr(field.placeholder)}" style="${inputStyleStr}"${reqAttr} />`;
} }
return `<div style="display:flex;flex-direction:column;gap:6px">${labelHtml}${inputHtml}</div>`; return `<div style="display:flex;flex-direction:column;gap:6px">${labelHtml}${inputHtml}</div>`;
}).join('\n '); }).join('\n ');
@@ -0,0 +1,25 @@
import { describe, test, expect } from 'vitest';
import { FormButton } from './FormButton';
const toHtml = (FormButton as any).toHtml;
describe('FormButton.toHtml', () => {
test('normal text renders as-is', () => {
const { html } = toHtml({ text: 'Send it' }, '');
expect(html).toContain('>Send it<');
expect(html).toContain('type="submit"');
});
test('type="submit" is a hardcoded literal, not prop-driven', () => {
const { html } = toHtml({ text: 'Submit' }, '');
expect(html).toMatch(/<button type="submit"/);
});
test('text content is escaped for <, >, &, and " (consistent with escapeHtml)', () => {
const { html } = toHtml({ text: '<script>alert(1)</script> & "quoted"' }, '');
expect(html).not.toContain('<script>');
expect(html).toContain('&lt;script&gt;alert(1)&lt;/script&gt;');
expect(html).toContain('&amp;');
expect(html).toContain('&quot;quoted&quot;');
});
});
+2 -1
View File
@@ -1,6 +1,7 @@
import React, { CSSProperties } from 'react'; import React, { CSSProperties } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeHtml } from '../../utils/escape';
interface FormButtonProps { interface FormButtonProps {
text?: string; text?: string;
@@ -74,7 +75,7 @@ FormButton.craft = {
cursor: 'pointer', cursor: 'pointer',
...props.style, ...props.style,
}); });
const escapedText = (props.text || 'Submit').replace(/</g, '&lt;').replace(/>/g, '&gt;'); const escapedText = escapeHtml(props.text || 'Submit');
return { return {
html: `<button type="submit"${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</button>`, html: `<button type="submit"${styleStr ? ` style="${styleStr}"` : ''}>${escapedText}</button>`,
}; };
@@ -39,3 +39,16 @@ describe('FormContainer.toHtml relay wiring', () => {
expect(mid1).not.toBe(mid2); expect(mid1).not.toBe(mid2);
}); });
}); });
describe('FormContainer.toHtml method attribute sanitization', () => {
test('malicious method value cannot break out of the attribute; falls back to POST', () => {
const { html } = toHtml({ action: '/legacy', method: 'POST"><script>alert(1)</script>' }, '');
expect(html).not.toContain('<script');
expect(html).toContain('method="POST"');
});
test('legitimate GET method still passes through unchanged (non-relay path)', () => {
const { html } = toHtml({ action: '/legacy', method: 'GET' }, '');
expect(html).toContain('method="GET"');
});
});
+2 -1
View File
@@ -3,6 +3,7 @@ import { useNode, Element, UserComponent } from '@craftjs/core';
import { Container } from '../layout/Container'; import { Container } from '../layout/Container';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { relayFormWiring } from '../../utils/form-relay-wiring'; import { relayFormWiring } from '../../utils/form-relay-wiring';
import { sanitizeFormMethod } from '../../utils/escape';
interface FormContainerProps { interface FormContainerProps {
action?: string; action?: string;
@@ -74,7 +75,7 @@ FormContainer.craft = {
...props.style, ...props.style,
}); });
const { useRelay, marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.action, nodeId); const { useRelay, marker, actionAttr, honeypot } = relayFormWiring(props.recipientEmail, props.thankYouUrl, props.action, nodeId);
const method = useRelay ? 'POST' : (props.method || 'POST'); // relay requires POST const method = useRelay ? 'POST' : sanitizeFormMethod(props.method); // relay requires POST
const body = honeypot + childrenHtml; // honeypot as first child const body = honeypot + childrenHtml; // honeypot as first child
return { return {
html: `${marker}<form action="${actionAttr}" method="${method}"${styleStr ? ` style="${styleStr}"` : ''}>${body}</form>`, html: `${marker}<form action="${actionAttr}" method="${method}"${styleStr ? ` style="${styleStr}"` : ''}>${body}</form>`,
@@ -58,3 +58,17 @@ describe('InputField.toHtml deterministic + unique ids (thread node id, resolves
expect(id1).toBe(id2); expect(id1).toBe(id2);
}); });
}); });
describe('InputField.toHtml type attribute sanitization', () => {
test('malicious type value cannot break out of the attribute; falls back to type="text"', () => {
const { html } = toHtml({ label: 'Name', name: 'name', type: 'text" autofocus onfocus="alert(1)' as any }, '');
expect(html).not.toContain('onfocus=');
expect(html).not.toContain('autofocus');
expect(html).toContain('type="text"');
});
test('legitimate number type still passes through unchanged', () => {
const { html } = toHtml({ label: 'Age', name: 'age', type: 'number' as const }, '');
expect(html).toContain('type="number"');
});
});
+2 -2
View File
@@ -1,7 +1,7 @@
import React, { CSSProperties } from 'react'; import React, { CSSProperties } from 'react';
import { useNode, UserComponent } from '@craftjs/core'; import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeHtml, escapeAttr, scopeId } from '../../utils/escape'; import { escapeHtml, escapeAttr, scopeId, sanitizeInputType } from '../../utils/escape';
interface InputFieldProps { interface InputFieldProps {
label?: string; label?: string;
@@ -111,7 +111,7 @@ InputField.craft = {
return { return {
html: `<div${wrapStyle ? ` style="${wrapStyle}"` : ''}> html: `<div${wrapStyle ? ` style="${wrapStyle}"` : ''}>
${labelHtml} ${labelHtml}
<input id="${escapeAttr(fieldId)}" type="${props.type || 'text'}" name="${escapeAttr(props.name || 'field')}" placeholder="${escapeAttr(props.placeholder || '')}"${reqAttr}${ariaLabelAttr} style="padding:10px 12px;border:1px solid #d4d4d8;border-radius:6px;font-size:14px;color:#18181b;background-color:#ffffff;width:100%;box-sizing:border-box" /> <input id="${escapeAttr(fieldId)}" type="${sanitizeInputType(props.type)}" name="${escapeAttr(props.name || 'field')}" placeholder="${escapeAttr(props.placeholder || '')}"${reqAttr}${ariaLabelAttr} style="padding:10px 12px;border:1px solid #d4d4d8;border-radius:6px;font-size:14px;color:#18181b;background-color:#ffffff;width:100%;box-sizing:border-box" />
</div>`, </div>`,
}; };
}; };
@@ -0,0 +1,39 @@
import { describe, test, expect } from 'vitest';
import { SubscribeForm } from './SubscribeForm';
const toHtml = (SubscribeForm as any).toHtml;
describe('SubscribeForm.toHtml hardcoded attributes stay hardcoded (no raw prop breakout)', () => {
test('form method is always POST regardless of any injected props', () => {
const { html } = toHtml({ heading: 'Join us', method: 'GET"><script>alert(1)</script>' } as any, '');
expect(html).toContain('<form method="POST"');
expect(html).not.toContain('<script');
});
test('email input type is always "email" regardless of any injected props', () => {
const { html } = toHtml({ type: 'text"><img src=x onerror=alert(1)>' } as any, '');
expect(html).toContain('<input type="email"');
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror=');
});
test('layout enum only ever feeds one of two fixed literal style strings, never raw', () => {
const { html: inlineHtml } = toHtml({ layout: 'inline' }, '');
const { html: stackedHtml } = toHtml({ layout: 'stacked' }, '');
expect(inlineHtml).toContain('flex-direction:row');
expect(stackedHtml).toContain('flex-direction:column');
});
test('malicious layout value cannot inject raw CSS/attribute breakout (falls through the isInline boolean check to the stacked literal)', () => {
const { html } = toHtml({ layout: '"><script>alert(1)</script>' as any }, '');
expect(html).not.toContain('<script');
expect(html).toContain('flex-direction:column');
});
test('normal render still produces expected structure', () => {
const { html } = toHtml({ heading: 'Subscribe', placeholder: 'you@example.com', buttonText: 'Go' }, '');
expect(html).toContain('Subscribe');
expect(html).toContain('placeholder="you@example.com"');
expect(html).toContain('>Go<');
});
});
@@ -50,3 +50,16 @@ describe('TextareaField.toHtml deterministic + unique ids (thread node id, resol
expect(id1).toBe(id2); expect(id1).toBe(id2);
}); });
}); });
describe('TextareaField.toHtml rows attribute sanitization', () => {
test('malicious rows value cannot break out of the attribute; falls back to a numeric rows', () => {
const { html } = toHtml({ label: 'Message', name: 'message', rows: '4"><script>alert(1)</script>' as any }, '');
expect(html).not.toContain('<script');
expect(html).toMatch(/rows="\d+"/);
});
test('legitimate numeric rows still passes through unchanged', () => {
const { html } = toHtml({ label: 'Message', name: 'message', rows: 8 }, '');
expect(html).toContain('rows="8"');
});
});
+6 -1
View File
@@ -97,6 +97,11 @@ TextareaField.craft = {
...props.style, ...props.style,
}); });
const reqAttr = props.required ? ' required' : ''; const reqAttr = props.required ? ' required' : '';
// `rows` is declared as a TS `number` but arrives unchecked (AI update_props
// path only validates node_id; deserialized saved-state JSON is untyped at
// runtime), so a string like `4"><script>...` must be coerced to a real
// number before interpolation, not trusted as already-numeric.
const rows = Number(props.rows) || 4;
// Deterministic AND unique id: scoped on the Craft node id so the // Deterministic AND unique id: scoped on the Craft node id so the
// <label for> always matches the <textarea id> AND two TextareaField // <label for> always matches the <textarea id> AND two TextareaField
// instances that share the same (often default) `name` -- e.g. two // instances that share the same (often default) `name` -- e.g. two
@@ -113,7 +118,7 @@ TextareaField.craft = {
return { return {
html: `<div${wrapStyle ? ` style="${wrapStyle}"` : ''}> html: `<div${wrapStyle ? ` style="${wrapStyle}"` : ''}>
${labelHtml} ${labelHtml}
<textarea id="${escapeAttr(fieldId)}" name="${escapeAttr(props.name || 'message')}" placeholder="${escapeAttr(props.placeholder || '')}" rows="${props.rows || 4}"${reqAttr}${ariaLabelAttr} style="padding:10px 12px;border:1px solid #d4d4d8;border-radius:6px;font-size:14px;color:#18181b;background-color:#ffffff;width:100%;box-sizing:border-box;resize:vertical;font-family:inherit"></textarea> <textarea id="${escapeAttr(fieldId)}" name="${escapeAttr(props.name || 'message')}" placeholder="${escapeAttr(props.placeholder || '')}" rows="${escapeAttr(String(rows))}"${reqAttr}${ariaLabelAttr} style="padding:10px 12px;border:1px solid #d4d4d8;border-radius:6px;font-size:14px;color:#18181b;background-color:#ffffff;width:100%;box-sizing:border-box;resize:vertical;font-family:inherit"></textarea>
</div>`, </div>`,
}; };
}; };
@@ -0,0 +1,58 @@
import { describe, test, expect } from 'vitest';
import { BackgroundSection } from './BackgroundSection';
const toHtml = (BackgroundSection as any).toHtml;
describe('BackgroundSection.toHtml anchorId', () => {
test('escapes a malicious anchorId (attribute breakout attempt)', () => {
const { html } = toHtml({ anchorId: 'x" onmouseover="alert(1)' }, 'child');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a normal anchorId still renders correctly', () => {
const { html } = toHtml({ anchorId: 'my-bg-section' }, 'child');
expect(html).toContain('id="my-bg-section"');
});
});
describe('BackgroundSection.toHtml style-value XSS hardening', () => {
test('a malicious bgImage cannot break out of the outer style attribute via url(...)', () => {
const malicious = 'javascript:alert(1)) foo{background:red}</style><script>alert(1)</script';
const { html } = toHtml({ bgImage: malicious }, 'child');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toContain('javascript:alert(1)');
});
test('a malicious bgColor cannot break out of the outer style attribute', () => {
const malicious = 'red" onmouseover="alert(1)';
const { html } = toHtml({ bgColor: malicious }, 'child');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a malicious overlayColor cannot break out of the overlay style attribute', () => {
const malicious = 'red" onmouseover="alert(1)';
const { html } = toHtml({ overlayColor: malicious }, 'child');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a wrong-typed overlayOpacity (string, not number) cannot break out of the overlay style attribute', () => {
const malicious = '0.4" onmouseover="alert(1)' as any;
const { html } = toHtml({ overlayOpacity: malicious }, 'child');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a malicious innerMaxWidth cannot break out of the inner style attribute', () => {
const malicious = '1200px" onmouseover="alert(1)';
const { html } = toHtml({ innerMaxWidth: malicious }, 'child');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('normal props still render correctly', () => {
const { html } = toHtml({ bgImage: 'https://example.com/bg.jpg', bgColor: '#1e293b', overlayColor: '#000000', overlayOpacity: 0.4, innerMaxWidth: '1200px' }, 'child');
expect(html).toContain("url('https://example.com/bg.jpg')");
expect(html).toContain('background-color:#1e293b');
expect(html).toContain('opacity:0.4');
expect(html).toContain('max-width:1200px');
expect(html).toContain('child');
});
});
@@ -33,3 +33,33 @@ describe('Container.toHtml cssId/cssClass', () => {
expect(html).toContain('id="anchor-id"'); 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 <img>, no broken-out attrs', () => {
const { html } = toHtml({ tag: 'div><img src=x onerror=alert(1)' }, 'child');
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror');
expect(html.startsWith('<div')).toBe(true);
expect(html.endsWith('</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('<div')).toBe(true);
expect(html).not.toContain('<script');
});
test('a valid tag (section) still emits <section', () => {
const { html } = toHtml({ tag: 'section' }, 'child');
expect(html).toContain('<section');
expect(html).toContain('</section>');
});
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);
}
});
});
+16 -2
View File
@@ -3,6 +3,19 @@ import { useNode, UserComponent } from '@craftjs/core';
import { cssPropsToString } from '../../utils/style-helpers'; import { cssPropsToString } from '../../utils/style-helpers';
import { escapeAttr } from '../../utils/escape'; 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><img src=x onerror=alert(1)` must never reach the `<${tag}` template
// position in `toHtml`/the live render. Anything not in this allowlist
// falls back to `'div'`.
const ALLOWED_CONTAINER_TAGS = ['div', 'section', 'article', 'header', 'footer', 'main'] as const;
export type ContainerTag = (typeof ALLOWED_CONTAINER_TAGS)[number];
export const sanitizeContainerTag = (tag: unknown): ContainerTag =>
(ALLOWED_CONTAINER_TAGS as readonly unknown[]).includes(tag) ? (tag as ContainerTag) : 'div';
interface ContainerProps { interface ContainerProps {
style?: CSSProperties; style?: CSSProperties;
tag?: 'div' | 'section' | 'article' | 'header' | 'footer' | 'main'; tag?: 'div' | 'section' | 'article' | 'header' | 'footer' | 'main';
@@ -42,6 +55,7 @@ export const Container: UserComponent<ContainerProps> = ({
}) => { }) => {
const { connectors: { connect, drag } } = useNode(); const { connectors: { connect, drag } } = useNode();
const safeTag = sanitizeContainerTag(tag);
const needsBoxedWrapper = contentWidth === 'boxed'; const needsBoxedWrapper = contentWidth === 'boxed';
const flexStyles = flexAlignFromTextAlign(style.textAlign); const flexStyles = flexAlignFromTextAlign(style.textAlign);
@@ -58,7 +72,7 @@ export const Container: UserComponent<ContainerProps> = ({
const idValue = cssId || anchorId || undefined; const idValue = cssId || anchorId || undefined;
const el = React.createElement( const el = React.createElement(
tag, safeTag,
{ {
ref: (ref: HTMLElement | null): void => { if (ref) connect(drag(ref)); }, ref: (ref: HTMLElement | null): void => { if (ref) connect(drag(ref)); },
style: outerStyle, style: outerStyle,
@@ -97,7 +111,7 @@ Container.craft = {
/* ---------- HTML export ---------- */ /* ---------- HTML export ---------- */
(Container as any).toHtml = (props: ContainerProps, childrenHtml: string) => { (Container as any).toHtml = (props: ContainerProps, childrenHtml: string) => {
const tag = props.tag || 'div'; const tag = sanitizeContainerTag(props.tag);
const isBoxed = props.contentWidth === 'boxed'; const isBoxed = props.contentWidth === 'boxed';
const flexStyles = flexAlignFromTextAlign(props.style?.textAlign); const flexStyles = flexAlignFromTextAlign(props.style?.textAlign);
@@ -0,0 +1,75 @@
import { describe, test, expect } from 'vitest';
import { Section } from './Section';
const toHtml = (Section as any).toHtml;
describe('Section.toHtml anchorId', () => {
test('escapes a malicious anchorId (attribute breakout attempt)', () => {
const { html } = toHtml({ anchorId: 'x" onmouseover="alert(1)' }, 'child');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a normal anchorId still renders correctly', () => {
const { html } = toHtml({ anchorId: 'my-section' }, 'child');
expect(html).toContain('id="my-section"');
});
});
describe('Section.toHtml childrenHtml passthrough', () => {
test('children are preserved', () => {
const { html } = toHtml({}, '<p>hello</p>');
expect(html).toContain('<p>hello</p>');
});
});
describe('Section.toHtml shape divider color/height XSS hardening', () => {
test('a malicious topDividerColor cannot break out of the SVG style attribute', () => {
const malicious = 'red" onmouseover="alert(1)';
const { html } = toHtml({ topDivider: 'wave', topDividerColor: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a malicious topDividerColor cannot inject a </style><script> breakout', () => {
const malicious = 'red</style><script>alert(1)</script>';
const { html } = toHtml({ topDivider: 'wave', topDividerColor: malicious }, '');
expect(html).not.toContain('<script>alert(1)</script>');
});
test('a malicious bottomDividerHeight cannot break out of the wrapper style attribute', () => {
const malicious = '50px" onmouseover="alert(1)';
const { html } = toHtml({ bottomDivider: 'angle', bottomDividerHeight: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a normal divider color/height still renders correctly', () => {
const { html } = toHtml({ topDivider: 'wave', topDividerColor: '#123456', topDividerHeight: '80px' }, '');
expect(html).toContain('fill:#123456');
expect(html).toContain('height:80px');
});
test('divider shape "none" emits no divider markup', () => {
const { html } = toHtml({ topDivider: 'none' }, 'child');
expect(html).not.toContain('<svg');
});
test('an unrecognized divider shape value emits no divider markup and no injected content', () => {
const malicious = 'wave"><script>alert(1)</script>' as any;
const { html } = toHtml({ topDivider: malicious }, 'child');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toContain('<svg');
});
test('a prototype-property-name divider shape (__proto__) does not leak [object Object]/function source into the SVG path', () => {
const { html } = toHtml({ topDivider: '__proto__' as any }, 'child');
expect(html).not.toContain('[object');
expect(html).not.toContain('native code');
expect(html).not.toContain('<svg');
});
test('a prototype-property-name divider shape (toString) does not leak Object.prototype.toString source into the SVG path', () => {
const { html } = toHtml({ topDivider: 'toString' as any }, 'child');
expect(html).not.toContain('[object');
expect(html).not.toContain('native code');
expect(html).not.toContain('<svg');
});
});
+12 -2
View File
@@ -38,7 +38,14 @@ const ShapeDivider: React.FC<{
position: 'top' | 'bottom'; position: 'top' | 'bottom';
}> = ({ shape, color, height, position }) => { }> = ({ shape, color, height, position }) => {
if (!shape || shape === 'none') return null; if (!shape || shape === 'none') return null;
const path = DIVIDER_PATHS[shape]; // `shape` is attacker-controlled (AI update_props / deserialized state) and
// not runtime-type-checked. A plain-object index lookup with a string key
// like '__proto__', 'toString', or 'constructor' returns an INHERITED
// Object.prototype value (not undefined), which would otherwise leak
// "[object Object]" / a function's source text into the SVG `d` attribute
// below. hasOwnProperty restricts the lookup to the real allowlisted keys.
if (!Object.prototype.hasOwnProperty.call(DIVIDER_PATHS, shape)) return null;
const path = DIVIDER_PATHS[shape as Exclude<DividerShape, 'none'>];
if (!path) return null; if (!path) return null;
const isTop = position === 'top'; const isTop = position === 'top';
@@ -162,7 +169,10 @@ function buildDividerHtml(
position: 'top' | 'bottom', position: 'top' | 'bottom',
): string { ): string {
if (!shape || shape === 'none') return ''; if (!shape || shape === 'none') return '';
const path = DIVIDER_PATHS[shape]; // See the matching hasOwnProperty guard in <ShapeDivider> above -- same
// prototype-pollution-shaped lookup, same fix.
if (!Object.prototype.hasOwnProperty.call(DIVIDER_PATHS, shape)) return '';
const path = DIVIDER_PATHS[shape as Exclude<DividerShape, 'none'>];
if (!path) return ''; if (!path) return '';
const isTop = position === 'top'; const isTop = position === 'top';
@@ -0,0 +1,34 @@
import { describe, test, expect } from 'vitest';
import { ImageBlock } from './ImageBlock';
const toHtml = (ImageBlock as any).toHtml;
describe('ImageBlock.toHtml src/alt XSS hardening', () => {
test('a javascript: src never reaches the output', () => {
const { html } = toHtml({ src: 'javascript:alert(1)' }, '');
expect(html).not.toContain('javascript:');
});
test('a malicious src cannot break out of the src attribute', () => {
const malicious = 'https://example.com/x.jpg" onerror="alert(1)';
const { html } = toHtml({ src: malicious }, '');
expect(html).not.toContain('onerror="alert(1)"');
});
test('a malicious alt cannot break out of the alt attribute', () => {
const malicious = 'x" onerror="alert(1)';
const { html } = toHtml({ src: 'https://example.com/x.jpg', alt: malicious }, '');
expect(html).not.toContain('onerror="alert(1)"');
});
test('a placeholder/empty src emits no output', () => {
const { html } = toHtml({ src: '' }, '');
expect(html).toBe('');
});
test('a normal image still renders correctly', () => {
const { html } = toHtml({ src: 'https://example.com/photo.jpg', alt: 'A photo' }, '');
expect(html).toContain('src="https://example.com/photo.jpg"');
expect(html).toContain('alt="A photo"');
});
});
@@ -27,3 +27,41 @@ describe('MapEmbed.toHtml iframe src ampersand encoding (F-export review Minor)'
expect(srcMatch![1]).not.toMatch(/&(?!amp;)/); expect(srcMatch![1]).not.toMatch(/&(?!amp;)/);
}); });
}); });
describe('MapEmbed.toHtml address/zoom/height XSS hardening', () => {
test('a malicious address cannot break out of the src or title attribute', () => {
const malicious = 'X" onerror="alert(1)';
const { html } = toHtml({ address: malicious }, '');
expect(html).not.toContain('onerror="alert(1)"');
});
test('a wrong-typed zoom (string with attribute-breakout chars) cannot break out of the src attribute', () => {
const malicious = '14"><script>alert(1)</script>' as any;
const { html } = toHtml({ address: 'X', zoom: malicious }, '');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toContain('"><script');
});
test('a wrong-typed zoom is coerced to a safe numeric value in the exported URL (defense in depth beyond escaping)', () => {
const malicious = '14"><script>alert(1)</script>' as any;
const { html } = toHtml({ address: 'X', zoom: malicious }, '');
const srcMatch = html.match(/<iframe src="([^"]+)"/);
expect(srcMatch).toBeTruthy();
// Decode the entity-escaped src back to a plain string and confirm the
// `z=` param is a bare, well-formed number -- not the raw attacker string.
const decoded = srcMatch![1].replace(/&amp;/g, '&').replace(/&quot;/g, '"').replace(/&lt;/g, '<').replace(/&gt;/g, '>');
expect(decoded).toMatch(/[&?]z=\d+(&|$)/);
});
test('a malicious height cannot break out of the iframe style attribute', () => {
const malicious = '400px" onmouseover="alert(1)';
const { html } = toHtml({ address: 'X', height: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a normal zoom/height still renders correctly', () => {
const { html } = toHtml({ address: 'X', zoom: 10, height: '300px' }, '');
expect(html).toContain('z=10');
expect(html).toContain('height:300px');
});
});
+9 -1
View File
@@ -12,7 +12,15 @@ interface MapEmbedProps {
function buildMapUrl(address: string, zoom: number): string { function buildMapUrl(address: string, zoom: number): string {
const encoded = encodeURIComponent(address); const encoded = encodeURIComponent(address);
return `https://maps.google.com/maps?q=${encoded}&z=${zoom}&output=embed`; // `zoom` is declared as `number` but is not runtime-type-checked (AI
// update_props / deserialized state can hand us anything). The final src
// string is still run through escapeAttr(safeUrl(...)) at the toHtml call
// site, which already blocks attribute-breakout -- but Number-coercing
// here too keeps the emitted URL a well-formed `z=<digits>` query param
// instead of smuggling arbitrary attacker text into it.
const z = Number(zoom);
const safeZoom = Number.isFinite(z) ? z : 14;
return `https://maps.google.com/maps?q=${encoded}&z=${safeZoom}&output=embed`;
} }
export const MapEmbed: UserComponent<MapEmbedProps> = ({ export const MapEmbed: UserComponent<MapEmbedProps> = ({
@@ -72,6 +72,38 @@ describe('VideoBlock.toHtml iframe accessibility (F2.4)', () => {
}); });
}); });
describe('VideoBlock.toHtml overlay/innerMaxWidth XSS hardening (background mode)', () => {
test('a malicious overlayColor cannot break out of the overlay style attribute', () => {
const malicious = 'red" onmouseover="alert(1)';
const { html } = toHtml({ videoUrl: 'https://vimeo.com/123456789', isBackground: true, overlayColor: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a wrong-typed overlayOpacity (string, not number) cannot break out of the overlay style attribute', () => {
const malicious = '50" onmouseover="alert(1)' as any;
const { html } = toHtml({ videoUrl: 'https://vimeo.com/123456789', isBackground: true, overlayOpacity: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a malicious innerMaxWidth cannot break out of the inner style attribute', () => {
const malicious = '1200px" onmouseover="alert(1)';
const { html } = toHtml({ videoUrl: 'https://vimeo.com/123456789', isBackground: true, innerMaxWidth: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a malicious style.borderRadius cannot break out of the style attribute (normal mode, iframe wrapper)', () => {
const malicious = { borderRadius: '8px" onmouseover="alert(1)' } as any;
const { html } = toHtml({ videoUrl: 'https://vimeo.com/123456789', style: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
test('a malicious style.borderRadius cannot break out of the style attribute (direct file <video>)', () => {
const malicious = { borderRadius: '8px" onmouseover="alert(1)' } as any;
const { html } = toHtml({ videoUrl: 'https://example.com/clip.mp4', style: malicious }, '');
expect(html).not.toContain('onmouseover="alert(1)"');
});
});
describe('VideoBlock.toHtml iframe src ampersand encoding (F-export review Minor)', () => { describe('VideoBlock.toHtml iframe src ampersand encoding (F-export review Minor)', () => {
test('embed params joined with literal & are HTML-entity-encoded in the emitted src attribute', () => { test('embed params joined with literal & are HTML-entity-encoded in the emitted src attribute', () => {
// autoplay+muted+controls=false forces buildEmbedParams to concatenate // autoplay+muted+controls=false forces buildEmbedParams to concatenate
@@ -91,3 +91,42 @@ describe('ContentSlider.toHtml autoplay silences aria-live and is pausable (F-ex
expect(html).not.toMatch(/visibilitychange/); expect(html).not.toMatch(/visibilitychange/);
}); });
}); });
describe('ContentSlider.toHtml renders slide.imageSrc as a background-image (INT)', () => {
test('a slide with imageSrc set exports a background-image referencing it', () => {
const slidesWithImage = [
{ type: 'image' as const, imageSrc: 'https://example.com/photo.jpg', heading: 'One' },
];
const { html } = toHtml({ slides: slidesWithImage }, '');
expect(html).toContain("background-image:url('https://example.com/photo.jpg')");
});
test('a slide with no imageSrc falls back to bgColor (no broken/empty background-image url)', () => {
const slidesNoImage = [
{ type: 'image' as const, imageSrc: '', heading: 'One', bgColor: '#123456' },
];
const { html } = toHtml({ slides: slidesNoImage }, '');
expect(html).not.toContain('background-image:url(');
expect(html).toContain('background-color:#123456');
});
});
describe('ContentSlider.toHtml interval is NOT runtime-type-checked -- must be coerced before it reaches the inline <script> numeric context', () => {
test('a malicious interval string cannot inject arbitrary JS into the autoplay setInterval call', () => {
const malicious = '5000);alert(document.domain);//';
const { html } = toHtml({ slides, autoplay: true, interval: malicious }, '');
expect(html).not.toContain('alert(document.domain)');
// the setInterval call must still be well-formed with a plain numeral delay
expect(html).toMatch(/setInterval\(function\(\)\{show\(current\+1\);\},\d+\);/);
});
test('a non-numeric interval falls back to a safe default delay', () => {
const { html } = toHtml({ slides, autoplay: true, interval: 'not-a-number' }, '');
expect(html).toMatch(/setInterval\(function\(\)\{show\(current\+1\);\},5000\);/);
});
test('a normal numeric interval still renders as the exact configured delay', () => {
const { html } = toHtml({ slides, autoplay: true, interval: 3000 }, '');
expect(html).toMatch(/setInterval\(function\(\)\{show\(current\+1\);\},3000\);/);
});
});
@@ -250,11 +250,24 @@ ContentSlider.craft = {
} = props; } = props;
const items = slides.length > 0 ? slides : defaultSlides; const items = slides.length > 0 ? slides : defaultSlides;
// Number() coercion: `interval` is declared `number` in TS but is NOT
// type-checked at runtime -- it arrives raw via the AI `update_props`
// path or a deserialized saved-state blob and is interpolated directly
// into the inline <script>'s `setInterval(fn, ${interval})` call below as
// a bare JS numeral (no quotes around it). A string like
// `5000);alert(1);//` would previously close the setInterval() call and
// splice arbitrary JS into the page's own <script> tag -- worse than an
// HTML attribute breakout, since it runs unconditionally on page load.
// Number() of anything non-numeric collapses safely to NaN, so we fall
// back to the 5000ms default rather than ever interpolating a
// non-numeral.
const intervalNum = Number(interval);
const safeInterval = Number.isFinite(intervalNum) && intervalNum > 0 ? intervalNum : 5000;
// Deterministic AND unique id, scoped on the Craft node id, for this // Deterministic AND unique id, scoped on the Craft node id, for this
// slider's slide/dot element ids and inline-script globals -- so two // slider's slide/dot element ids and inline-script globals -- so two
// ContentSlider instances (e.g. both left at default slides) don't // ContentSlider instances (e.g. both left at default slides) don't
// collide and end up driving each other's rotation. // collide and end up driving each other's rotation.
const uid = scopeId(nodeId, JSON.stringify(items) + interval, 'cs'); const uid = scopeId(nodeId, JSON.stringify(items) + safeInterval, 'cs');
const sectionStyle = cssPropsToString({ const sectionStyle = cssPropsToString({
position: 'relative', position: 'relative',
@@ -339,7 +352,7 @@ ContentSlider.craft = {
window["${uid}_prev"]=function(){ markManualNav(); show(current-1); }; window["${uid}_prev"]=function(){ markManualNav(); show(current-1); };
${autoplayActive ? ` ${autoplayActive ? `
var timer=null; var timer=null;
function start(){ if(!timer && document.visibilityState!=="hidden"){ timer=setInterval(function(){show(current+1);},${interval}); } } function start(){ if(!timer && document.visibilityState!=="hidden"){ timer=setInterval(function(){show(current+1);},${safeInterval}); } }
function stop(){ if(timer){ clearInterval(timer); timer=null; } } function stop(){ if(timer){ clearInterval(timer); timer=null; } }
var root=document.getElementById(uid); var root=document.getElementById(uid);
if(root){ if(root){
@@ -93,3 +93,33 @@ describe('Gallery.toHtml deterministic + unique scope ids (thread node id, no Ma
expect(html1).toBe(html2); expect(html1).toBe(html2);
}); });
}); });
describe('Gallery.toHtml lightbox focus management (M-2)', () => {
const props = { images: [{ src: '/a.jpg', alt: 'a' }], lightbox: true };
test('overlay includes a focusable close control with an accessible name and tabindex', () => {
const { html } = toHtml(props, '', 'node-gal1');
// A close control: a button (or the dialog container) with an accessible
// name (aria-label) and an explicit tabindex so it's keyboard-focusable.
expect(html).toMatch(/aria-label="[^"]*[Cc]lose[^"]*"[^>]*tabindex="-?\d+"|tabindex="-?\d+"[^>]*aria-label="[^"]*[Cc]lose[^"]*"/);
});
test('script saves document.activeElement on open (for focus restore)', () => {
const { html } = toHtml(props, '', 'node-gal1');
expect(html).toMatch(/document\.activeElement/);
});
test('script moves focus to the close control / dialog on open', () => {
const { html } = toHtml(props, '', 'node-gal1');
expect(html).toMatch(/\.focus\(\)/);
});
test('script restores the previously-saved focus on close', () => {
const { html } = toHtml(props, '', 'node-gal1');
// The close function references a stored "last focused element" variable
// and calls .focus() on it, not just moving focus INTO the dialog.
const closeFnMatch = html.match(/function\s+\w+_close\s*\(\)\s*\{[^}]*\}/);
expect(closeFnMatch).not.toBeNull();
expect(closeFnMatch![0]).toMatch(/\.focus\(\)/);
});
});
+30 -2
View File
@@ -166,16 +166,37 @@ Gallery.craft = {
let gridIdAttr = ''; let gridIdAttr = '';
if (lightbox) { if (lightbox) {
gridIdAttr = ` id="${galleryId}_grid"`; gridIdAttr = ` id="${galleryId}_grid"`;
// M-2: focus management for the lightbox dialog.
// - OPEN: stash `document.activeElement` (the thumbnail that triggered
// the open) in a module-scoped var, then move focus onto the close
// button -- so a screen-reader/keyboard user lands inside the dialog
// instead of focus staying on (or silently falling back to <body>)
// behind the now-visible overlay.
// - Tab trap: while the overlay is open, every Tab keypress is
// intercepted and refocuses the close button (the dialog's only
// focusable control besides Escape/click-to-close), so focus can
// never wander out into the page content hidden behind the overlay.
// - CLOSE (Escape, backdrop click, or the close button): restore focus
// to the element stashed on open.
lightboxHtml = ` lightboxHtml = `
<div id="${galleryId}_overlay" role="dialog" aria-modal="true" aria-label="Image preview" onclick="${galleryId}_close()" style="display:none;position:fixed;top:0;left:0;width:100%;height:100%;background:rgba(0,0,0,0.9);z-index:9999;justify-content:center;align-items:center;cursor:pointer"> <div id="${galleryId}_overlay" role="dialog" aria-modal="true" aria-label="Image preview" onclick="${galleryId}_close()" style="display:none;position:fixed;top:0;left:0;width:100%;height:100%;background:rgba(0,0,0,0.9);z-index:9999;justify-content:center;align-items:center;cursor:pointer">
<button type="button" id="${galleryId}_closebtn" aria-label="Close preview" tabindex="-1" onclick="event.stopPropagation();${galleryId}_close()" style="position:absolute;top:16px;right:16px;width:36px;height:36px;border-radius:50%;border:none;background:rgba(255,255,255,0.15);color:#ffffff;font-size:20px;line-height:1;cursor:pointer;display:flex;align-items:center;justify-content:center">&times;</button>
<img id="${galleryId}_img" src="" alt="" style="max-width:90%;max-height:90%;object-fit:contain;border-radius:8px" /> <img id="${galleryId}_img" src="" alt="" style="max-width:90%;max-height:90%;object-fit:contain;border-radius:8px" />
</div> </div>
<script> <script>
function ${galleryId}_close(){document.getElementById('${galleryId}_overlay').style.display='none';} var ${galleryId}_lastFocus = null;
function ${galleryId}_close(){
document.getElementById('${galleryId}_overlay').style.display='none';
if(${galleryId}_lastFocus && ${galleryId}_lastFocus.focus) ${galleryId}_lastFocus.focus();
${galleryId}_lastFocus = null;
}
function ${galleryId}_open(src){ function ${galleryId}_open(src){
${galleryId}_lastFocus = document.activeElement;
var o = document.getElementById('${galleryId}_overlay'); var o = document.getElementById('${galleryId}_overlay');
document.getElementById('${galleryId}_img').src = src; document.getElementById('${galleryId}_img').src = src;
o.style.display = 'flex'; o.style.display = 'flex';
var c = document.getElementById('${galleryId}_closebtn');
if(c) c.focus();
} }
document.getElementById('${galleryId}_grid').addEventListener('click', function(e){ document.getElementById('${galleryId}_grid').addEventListener('click', function(e){
var t = e.target.closest('[data-lb-src]'); var t = e.target.closest('[data-lb-src]');
@@ -190,7 +211,14 @@ document.getElementById('${galleryId}_grid').addEventListener('keydown', functio
${galleryId}_open(t.getAttribute('data-lb-src')); ${galleryId}_open(t.getAttribute('data-lb-src'));
}); });
document.addEventListener('keydown', function(e){ document.addEventListener('keydown', function(e){
if(e.key==='Escape'){ ${galleryId}_close(); } var o = document.getElementById('${galleryId}_overlay');
if(!o || o.style.display==='none') return;
if(e.key==='Escape'){ ${galleryId}_close(); return; }
if(e.key==='Tab'){
e.preventDefault();
var c = document.getElementById('${galleryId}_closebtn');
if(c) c.focus();
}
}); });
</script>`; </script>`;
} }
@@ -0,0 +1,35 @@
import { describe, test, expect } from 'vitest';
import { HeroSimple } from './HeroSimple';
const toHtml = (HeroSimple as any).toHtml;
describe('HeroSimple.toHtml textAlign enum sink (attacker-controlled prop, not enforced at runtime)', () => {
test('malicious textAlign value cannot break out of the content div style attribute', () => {
const { html } = toHtml({
heading: 'Hi',
subtitle: 'There',
textAlign: 'center;"><script>alert(1)</script>',
}, '');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toContain('center;">');
});
test('unrecognized textAlign value falls back to a safe default rather than being echoed raw', () => {
const { html } = toHtml({ heading: 'Hi', subtitle: 'There', textAlign: 'not-a-real-value' as any }, '');
expect(html).not.toContain('text-align:not-a-real-value');
});
test('valid textAlign values are preserved', () => {
const { html: left } = toHtml({ heading: 'Hi', subtitle: 'There', textAlign: 'left' }, '');
expect(left).toContain('text-align:left');
const { html: right } = toHtml({ heading: 'Hi', subtitle: 'There', textAlign: 'right' }, '');
expect(right).toContain('text-align:right');
});
test('normal default render is sane', () => {
const { html } = toHtml({ heading: 'Welcome', subtitle: 'Sub text' }, '');
expect(html).toContain('Welcome');
expect(html).toContain('Sub text');
expect(html).toContain('text-align:center');
});
});
+7 -1
View File
@@ -229,7 +229,13 @@ HeroSimple.craft = {
videoHtml = `<video src="${escapeAttr(safeUrl(props.bgVideo))}" autoplay muted loop playsinline style="position:absolute;top:0;left:0;width:100%;height:100%;object-fit:cover;z-index:0"></video>`; videoHtml = `<video src="${escapeAttr(safeUrl(props.bgVideo))}" autoplay muted loop playsinline style="position:absolute;top:0;left:0;width:100%;height:100%;object-fit:cover;z-index:0"></video>`;
} }
const textAlign = props.textAlign || 'center'; // Allowlisted -- `textAlign` is declared as a 'left'|'center'|'right' union
// but arrives unchecked via AI update_props / deserialized state; it is
// interpolated raw into the content div's style attribute below, so any
// other value must collapse to a known-safe default rather than being
// echoed into the markup.
const ALLOWED_TEXT_ALIGN = ['left', 'center', 'right'];
const textAlign = ALLOWED_TEXT_ALIGN.includes(props.textAlign as string) ? (props.textAlign as string) : 'center';
const justifyBtn = textAlign === 'center' ? 'center' : textAlign === 'right' ? 'flex-end' : 'flex-start'; const justifyBtn = textAlign === 'center' ? 'center' : textAlign === 'right' ? 'flex-end' : 'flex-start';
const ctas = normalizeCtas(props); const ctas = normalizeCtas(props);
@@ -47,3 +47,29 @@ describe('NumberCounter.toHtml deterministic + unique scope ids (thread node id,
expect(wrapId1).not.toBe(wrapId2); expect(wrapId1).not.toBe(wrapId2);
}); });
}); });
describe('NumberCounter.toHtml counter.number is NOT runtime-type-checked -- must be sanitized before it reaches data-target', () => {
test('a malicious counter.number cannot break out of the data-target attribute to inject a <script> tag', () => {
const malicious = [
{ number: '150"><script>alert(1)</script>', suffix: '+', label: 'Evil' },
];
const { html } = toHtml({ counters: malicious }, '', 'node-nc-evil1');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toContain('"><script>');
});
test('a malicious counter.number cannot break out of the data-target attribute to inject an onmouseover handler', () => {
const malicious = [
{ number: '150" onmouseover="alert(1)', suffix: '+', label: 'Evil' },
];
const { html } = toHtml({ counters: malicious }, '', 'node-nc-evil2');
expect(html).not.toContain('onmouseover=');
expect(html).not.toMatch(/data-target="150" onmouseover/);
});
test('normal numeric counter.number values still render as data-target="150"', () => {
const normal = [{ number: 150, suffix: '+', label: 'Projects' }];
const { html } = toHtml({ counters: normal }, '', 'node-nc-normal');
expect(html).toContain('data-target="150"');
});
});
@@ -140,8 +140,20 @@ NumberCounter.craft = {
}); });
const countersHtml = items.map((counter, i) => { const countersHtml = items.map((counter, i) => {
// Number() coercion + escapeAttr: `counter.number` is declared `number`
// per-item inside an array prop, but is NOT type-checked at runtime --
// it arrives raw via the AI `update_props` path or a deserialized
// saved-state blob and was previously interpolated straight into this
// data-target attribute, letting a string like `150"><script>...`
// break out of the attribute and inject markup. Number() collapses any
// non-numeric value safely to NaN (then 0), and escapeAttr is kept as
// defense-in-depth in case Number()'s string coercion output ever
// contains a stray character (it can't today, but the sink should never
// rely solely on the coercion).
const numberVal = Number(counter.number);
const safeNumber = Number.isFinite(numberVal) ? numberVal : 0;
return `<div style="display:flex;flex-direction:column;align-items:center;gap:8px"> return `<div style="display:flex;flex-direction:column;align-items:center;gap:8px">
<span id="${uid}_n${i}" data-target="${counter.number}" data-suffix="${escapeAttr(counter.suffix)}" style="font-size:${numberSize};font-weight:700;color:${numberColor};line-height:1.1;font-family:Inter,sans-serif">0${escapeHtml(counter.suffix)}</span> <span id="${uid}_n${i}" data-target="${escapeAttr(String(safeNumber))}" data-suffix="${escapeAttr(counter.suffix)}" style="font-size:${numberSize};font-weight:700;color:${numberColor};line-height:1.1;font-family:Inter,sans-serif">0${escapeHtml(counter.suffix)}</span>
<span style="font-size:15px;color:${labelColor};font-family:Inter,sans-serif;font-weight:500">${escapeHtml(counter.label)}</span> <span style="font-size:15px;color:${labelColor};font-family:Inter,sans-serif;font-weight:500">${escapeHtml(counter.label)}</span>
</div>`; </div>`;
}).join('\n '); }).join('\n ');
@@ -46,3 +46,28 @@ describe('Testimonials.toHtml decorative star icons (F2.5)', () => {
stars.forEach((tag: string) => expect(tag).toContain('aria-hidden="true"')); stars.forEach((tag: string) => expect(tag).toContain('aria-hidden="true"'));
}); });
}); });
describe('Testimonials.toHtml rating aria-label sink (attacker-controlled `rating`, typed number but unchecked)', () => {
test('malicious rating value cannot break out of the star row aria-label attribute', () => {
const malicious = [
{ quote: 'Q', name: 'N', title: 'T', rating: '5"><script>alert(1)</script>' as any },
];
const { html } = toHtml({ testimonials: malicious, layout: 'grid' }, '');
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).not.toContain('5"><script>');
});
test('non-numeric rating falls back to a safe numeric value', () => {
const malicious = [
{ quote: 'Q', name: 'N', title: 'T', rating: 'not-a-number' as any },
];
const { html } = toHtml({ testimonials: malicious, layout: 'grid' }, '');
expect(html).toMatch(/aria-label="Rating: 0 out of 5"/);
});
test('normal numeric rating still renders correctly', () => {
const { html } = toHtml({ testimonials, layout: 'grid' }, '');
expect(html).toContain('aria-label="Rating: 5 out of 5"');
expect(html).toContain('aria-label="Rating: 4 out of 5"');
});
});
@@ -41,10 +41,16 @@ function renderStars(count: number, color: string): React.ReactNode {
} }
function starsHtml(count: number, color: string): string { function starsHtml(count: number, color: string): string {
// `count` is `Testimonial.rating`, typed `number` but arrives unchecked via
// AI update_props / deserialized state -- coerce to a real number before
// it's interpolated into the aria-label attribute below (both the loop
// comparison and the escapeAttr(String(...)) call are safe against any
// non-numeric/garbage value).
const safeCount = Number(count) || 0;
const stars = [1, 2, 3, 4, 5].map((i) => const stars = [1, 2, 3, 4, 5].map((i) =>
`<i class="fa ${i <= count ? 'fa-star' : 'fa-star-o'}" style="color:${color};font-size:14px" aria-hidden="true"></i>` `<i class="fa ${i <= safeCount ? 'fa-star' : 'fa-star-o'}" style="color:${color};font-size:14px" aria-hidden="true"></i>`
).join(''); ).join('');
return `<div style="display:flex;gap:2px;justify-content:center;margin-bottom:12px" role="img" aria-label="Rating: ${count} out of 5">${stars}</div>`; return `<div style="display:flex;gap:2px;justify-content:center;margin-bottom:12px" role="img" aria-label="${escapeAttr(`Rating: ${safeCount} out of 5`)}">${stars}</div>`;
} }
export const Testimonials: UserComponent<TestimonialsProps> = ({ export const Testimonials: UserComponent<TestimonialsProps> = ({
+60
View File
@@ -1,6 +1,7 @@
import { describe, test, expect } from 'vitest'; import { describe, test, expect } from 'vitest';
import { buildSavePayload } from './useWhpApi'; import { buildSavePayload } from './useWhpApi';
import { PageData } from '../types'; import { PageData } from '../types';
import { DEFAULT_SITE_DESIGN } from '../state/SiteDesignContext';
const pageA: PageData = { id: 'home', name: 'Home', slug: 'index', craftState: 'STORED_HOME' }; const pageA: PageData = { id: 'home', name: 'Home', slug: 'index', craftState: 'STORED_HOME' };
const pageB: PageData = { id: 'page_2', name: 'About', slug: 'about', craftState: 'STORED_ABOUT' }; const pageB: PageData = { id: 'page_2', name: 'About', slug: 'about', craftState: 'STORED_ABOUT' };
@@ -19,6 +20,8 @@ describe('buildSavePayload', () => {
activePageId: '__header__', activePageId: '__header__',
isEditingHeader: true, isEditingHeader: true,
isEditingFooter: false, isEditingFooter: false,
headCode: DEFAULT_SITE_DESIGN.headCode,
design: DEFAULT_SITE_DESIGN,
}); });
// The fresh live canvas must be reflected in header_craft_state, not the stale stored one. // The fresh live canvas must be reflected in header_craft_state, not the stale stored one.
@@ -52,6 +55,8 @@ describe('buildSavePayload', () => {
activePageId: '__footer__', activePageId: '__footer__',
isEditingHeader: false, isEditingHeader: false,
isEditingFooter: true, isEditingFooter: true,
headCode: DEFAULT_SITE_DESIGN.headCode,
design: DEFAULT_SITE_DESIGN,
}); });
expect(payload.footer_craft_state).toBe('LIVE_FOOTER'); expect(payload.footer_craft_state).toBe('LIVE_FOOTER');
@@ -75,6 +80,8 @@ describe('buildSavePayload', () => {
activePageId: 'home', activePageId: 'home',
isEditingHeader: false, isEditingHeader: false,
isEditingFooter: false, isEditingFooter: false,
headCode: DEFAULT_SITE_DESIGN.headCode,
design: DEFAULT_SITE_DESIGN,
}); });
// Live canvas goes to the active page and top-level slots. // Live canvas goes to the active page and top-level slots.
@@ -87,4 +94,57 @@ describe('buildSavePayload', () => {
expect(payload.header_craft_state).toBe('STALE_HEADER'); expect(payload.header_craft_state).toBe('STALE_HEADER');
expect(payload.footer_craft_state).toBe('STALE_FOOTER'); expect(payload.footer_craft_state).toBe('STALE_FOOTER');
}); });
test('I-1: activePageId matches no page (dangling, e.g. original Home deleted then reload reset it to "home") -- live serialize still lands in pages_craft_state[0] / index.html, not lost', () => {
// pages[0] has id 'page_x' (the replacement landing page after the
// original 'home' was deleted); activePageId is stuck at the stale
// default 'home', which matches no entry in `pages`.
const pageX: PageData = { id: 'page_x', name: 'Home', slug: 'index', craftState: 'STORED_X' };
const p2: PageData = { id: 'p2', name: 'About', slug: 'about', craftState: 'STORED_ABOUT_2' };
const payload = buildSavePayload({
siteId: 1,
siteName: 'Test Site',
liveCraftState: 'LIVE_EDIT',
pages: [pageX, p2],
headerPage,
footerPage,
activePageId: 'home',
isEditingHeader: false,
isEditingFooter: false,
headCode: DEFAULT_SITE_DESIGN.headCode,
design: DEFAULT_SITE_DESIGN,
});
// The live edit must land in pages_craft_state[0] (index.html slot),
// not be silently dropped to only the legacy top-level fields.
expect(payload.pages_craft_state[0].craftState).toBe('LIVE_EDIT');
expect(payload.pages[0].filename).toBe('index.html');
// Top-level fields (legacy) should also reflect the live edit.
expect(payload.craft_state).toBe('LIVE_EDIT');
// The other page is untouched.
expect(payload.pages_craft_state.find((p) => p.id === 'p2')?.craftState).toBe('STORED_ABOUT_2');
});
test('head code + design tokens are included in the save payload', () => {
const design = { ...DEFAULT_SITE_DESIGN, headCode: '<meta name="x">' };
const payload = buildSavePayload({
siteId: 1,
siteName: 'Test Site',
liveCraftState: 'LIVE_PAGE_HOME',
pages: [pageA, pageB],
headerPage,
footerPage,
activePageId: 'home',
isEditingHeader: false,
isEditingFooter: false,
headCode: '<meta name="x">',
design,
});
expect(payload.head_code).toBe('<meta name="x">');
expect(payload.design).toEqual(design);
expect(payload.design.headCode).toBe('<meta name="x">');
});
}); });
+53 -4
View File
@@ -2,6 +2,7 @@ import { useCallback } from 'react';
import { useEditor } from '@craftjs/core'; import { useEditor } from '@craftjs/core';
import { useEditorConfig } from '../state/EditorConfigContext'; import { useEditorConfig } from '../state/EditorConfigContext';
import { usePages } from '../state/PageContext'; import { usePages } from '../state/PageContext';
import { useSiteDesign, SiteDesign } from '../state/SiteDesignContext';
import { exportBodyHtml } from '../utils/html-export'; import { exportBodyHtml } from '../utils/html-export';
import { PageData } from '../types'; import { PageData } from '../types';
@@ -18,6 +19,10 @@ export interface BuildSavePayloadInput {
isEditingHeader: boolean; isEditingHeader: boolean;
/** True when the live canvas is showing the footer zone (activePageId === '__footer__'). */ /** True when the live canvas is showing the footer zone (activePageId === '__footer__'). */
isEditingFooter: boolean; isEditingFooter: boolean;
/** Site-wide custom `<head>` code (also present on `design.headCode`). */
headCode: string;
/** Full site design tokens object -- lets load() restore colors/fonts/headCode. */
design: SiteDesign;
} }
/** /**
@@ -45,10 +50,24 @@ export function buildSavePayload(input: BuildSavePayloadInput) {
activePageId, activePageId,
isEditingHeader, isEditingHeader,
isEditingFooter, isEditingFooter,
headCode,
design,
} = input; } = input;
const isPageActive = !isEditingHeader && !isEditingFooter; const isPageActive = !isEditingHeader && !isEditingFooter;
// I-1 (data-loss): `activePageId` can go dangling -- e.g. the original
// Home page (id 'home') is deleted, its replacement gets a fresh id
// (`page_<ts>`), and a reload re-initializes `activePageId` back to the
// hardcoded default `'home'` (see PageContext's `useState('home')`) before
// `load()` has a chance to point it at the actually-restored page. If a
// real page IS active but matches no entry in `pages`, treat `pages[0]`
// (the landing page) as the active one so the live canvas serialization
// still reaches the index.html page slot / `pages_craft_state[0]` instead
// of only the legacy top-level `craft_state`/`html` fields.
const activePageIndex = isPageActive ? pages.findIndex((p) => p.id === activePageId) : -1;
const effectiveActivePageId = activePageIndex !== -1 ? activePageId : pages[0]?.id;
// Fresh header/footer state: the live canvas wins when that zone is the // Fresh header/footer state: the live canvas wins when that zone is the
// one currently being edited; otherwise fall back to the last-committed // one currently being edited; otherwise fall back to the last-committed
// stored state (updated on zone switch by PageContext's saveCurrentState). // stored state (updated on zone switch by PageContext's saveCurrentState).
@@ -115,7 +134,7 @@ export function buildSavePayload(input: BuildSavePayloadInput) {
const filename = i === 0 ? 'index.html' : page.slug + '.html'; const filename = i === 0 ? 'index.html' : page.slug + '.html';
let pageHtml = ''; let pageHtml = '';
if (isPageActive && page.id === activePageId) { if (isPageActive && page.id === effectiveActivePageId) {
// Active page: use the current canvas HTML (already exported above) // Active page: use the current canvas HTML (already exported above)
pageHtml = currentHtml; pageHtml = currentHtml;
} else if (page.craftState) { } else if (page.craftState) {
@@ -146,7 +165,7 @@ export function buildSavePayload(input: BuildSavePayloadInput) {
// reload the editor's clean-URL routing (.htaccess rewrite of /name → // reload the editor's clean-URL routing (.htaccess rewrite of /name →
// name.html) lines up with the file we just wrote (index.html). // name.html) lines up with the file we just wrote (index.html).
slug: i === 0 ? 'index' : page.slug, slug: i === 0 ? 'index' : page.slug,
craftState: (isPageActive && page.id === activePageId) ? liveCraftState : (page.craftState || null), craftState: (isPageActive && page.id === effectiveActivePageId) ? liveCraftState : (page.craftState || null),
})); }));
return { return {
@@ -161,6 +180,8 @@ export function buildSavePayload(input: BuildSavePayloadInput) {
header_craft_state: headerCraftState, header_craft_state: headerCraftState,
footer_craft_state: footerCraftState, footer_craft_state: footerCraftState,
pages_craft_state: pagesGrapesjs, pages_craft_state: pagesGrapesjs,
head_code: headCode,
design,
}; };
} }
@@ -177,7 +198,9 @@ export function useWhpApi() {
setHeaderCraftState, setHeaderCraftState,
setFooterCraftState, setFooterCraftState,
setPagesCraftState, setPagesCraftState,
setActivePageIdDirect,
} = usePages(); } = usePages();
const { design, updateDesign } = useSiteDesign();
const save = useCallback(async () => { const save = useCallback(async () => {
if (!isWHP || !whpConfig) return null; if (!isWHP || !whpConfig) return null;
@@ -197,6 +220,8 @@ export function useWhpApi() {
activePageId, activePageId,
isEditingHeader, isEditingHeader,
isEditingFooter, isEditingFooter,
headCode: design.headCode,
design,
}); });
const resp = await fetch(`${whpConfig.apiUrl}?action=save`, { const resp = await fetch(`${whpConfig.apiUrl}?action=save`, {
@@ -208,7 +233,7 @@ export function useWhpApi() {
body: JSON.stringify(payload), body: JSON.stringify(payload),
}); });
return resp.json(); return resp.json();
}, [isWHP, whpConfig, query, pages, activePageId, headerPage, footerPage, isEditingHeader, isEditingFooter]); }, [isWHP, whpConfig, query, pages, activePageId, headerPage, footerPage, isEditingHeader, isEditingFooter, design]);
const publish = useCallback(async () => { const publish = useCallback(async () => {
if (!isWHP || !whpConfig) return null; if (!isWHP || !whpConfig) return null;
@@ -242,6 +267,17 @@ export function useWhpApi() {
if (data.success && data.project) { if (data.success && data.project) {
const proj = data.project; const proj = data.project;
// Restore site design tokens (colors/fonts/headCode) so the editor
// reflects what was last saved. Prefer the full `design` object when
// present; fall back to just `head_code` for older project.json files
// saved before this field existed (backward-compatible: defaults for
// everything else).
if (proj.design && typeof proj.design === 'object') {
updateDesign(proj.design);
} else if (typeof proj.head_code === 'string') {
updateDesign({ headCode: proj.head_code });
}
// Restore header craft state // Restore header craft state
if (proj.header_craft_state) { if (proj.header_craft_state) {
setHeaderCraftState(typeof proj.header_craft_state === 'string' setHeaderCraftState(typeof proj.header_craft_state === 'string'
@@ -271,10 +307,23 @@ export function useWhpApi() {
console.warn('Failed to load page state:', e); console.warn('Failed to load page state:', e);
} }
} }
// I-1 (data-loss): point activePageId at the page we just loaded
// into the canvas. Without this, activePageId stays at whatever it
// was initialized to (the hardcoded default 'home'), which goes
// dangling the moment the original Home page has been deleted and
// replaced (its replacement gets a fresh `page_<ts>` id) -- the next
// edit+save would then only reach the legacy top-level fields
// instead of the actual page slot. Only do this when a real page is
// being loaded, i.e. we're not currently mid-edit of the header/
// footer zone (switching zones is handled separately by switchPage).
if (!isEditingHeader && !isEditingFooter) {
setActivePageIdDirect(firstPage.id);
}
} }
} }
return data; return data;
}, [isWHP, whpConfig, actions, setHeaderCraftState, setFooterCraftState, setPagesCraftState]); }, [isWHP, whpConfig, actions, setHeaderCraftState, setFooterCraftState, setPagesCraftState, setActivePageIdDirect, isEditingHeader, isEditingFooter, updateDesign]);
const uploadAsset = useCallback( const uploadAsset = useCallback(
async (file: File) => { async (file: File) => {
@@ -0,0 +1,116 @@
import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
import React from 'react';
import { createRoot, Root } from 'react-dom/client';
import { act } from 'react-dom/test-utils';
/* MediaStylePanel (via useNodeProp/ArrayPropEditor in ./shared) only needs
useEditor from @craftjs/core. Mock it following the DOM-harness pattern
used in Footer.editguard.test.tsx / AssetPicker.test.tsx (no
@testing-library/react in this repo) so setProp calls can be observed
without mounting a real <Editor> tree. */
const setPropSpy = vi.fn((_id: string, updater: (p: any) => void) => {
updater(lastProps);
});
let lastProps: any;
vi.mock('@craftjs/core', () => ({
useEditor: () => ({ actions: { setProp: setPropSpy } }),
}));
vi.mock('../../../utils/assets', () => ({
uploadAsset: vi.fn(),
listAssets: vi.fn(),
}));
import { MediaStylePanel } from './MediaStylePanel';
let container: HTMLDivElement;
let root: Root;
function render(ui: React.ReactElement) {
container = document.createElement('div');
document.body.appendChild(container);
act(() => {
root = createRoot(container);
root.render(ui);
});
}
function unmount() {
act(() => { root.unmount(); });
container.remove();
}
function q<T extends Element = Element>(testId: string): T | null {
return container.querySelector(`[data-testid="${testId}"]`);
}
function setValue(input: HTMLInputElement, value: string) {
const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, 'value')!.set!;
setter.call(input, value);
input.dispatchEvent(new Event('input', { bubbles: true }));
}
beforeEach(() => {
setPropSpy.mockClear();
});
afterEach(() => {
if (container) unmount();
});
/**
* INT: ContentSlider (src/components/sections/ContentSlider.tsx) reads
* `slide.imageSrc` in both render and toHtml, but the Slides array editor
* here guarded on `item.image !== undefined` and wrote `image` via
* AssetPicker's onChange -- a key that ContentSlider never reads. Default
* slides (`imageSrc:''`, no `image` key) therefore showed NO image picker at
* all (the guard was never true), and even a manually-added `image` value
* was a silent no-op on export/render.
*/
describe('MediaStylePanel Slides editor uses imageSrc (INT)', () => {
test('a slide shaped like defaultSlides (imageSrc present, no `image` key) renders the image AssetPicker', () => {
lastProps = { slides: [{ type: 'image', imageSrc: '', heading: 'First Slide', text: '', bgColor: '' }] };
render(<MediaStylePanel selectedId="cs-1" nodeProps={lastProps} />);
// Before the fix, the guard `item.image !== undefined` was false for
// this default-shaped slide, so no AssetPicker rendered at all.
expect(q('asset-picker-compact')).not.toBeNull();
});
test('editing the slide image writes imageSrc (not image) via setProp', () => {
lastProps = { slides: [{ type: 'image', imageSrc: '', heading: 'First Slide', text: '', bgColor: '' }] };
render(<MediaStylePanel selectedId="cs-1" nodeProps={lastProps} />);
const urlInput = q<HTMLInputElement>('asset-picker-url-input')!;
expect(urlInput).not.toBeNull();
setValue(urlInput, 'https://example.com/slide.jpg');
act(() => {
urlInput.dispatchEvent(new FocusEvent('focusout', { bubbles: true }));
});
expect(setPropSpy).toHaveBeenCalled();
const updatedSlide = lastProps.slides[0];
expect(updatedSlide.imageSrc).toBe('https://example.com/slide.jpg');
expect(updatedSlide.image).toBeUndefined();
});
test('adding a new slide (emptyItem) includes imageSrc, matching defaultSlides shape', () => {
lastProps = { slides: [] };
render(<MediaStylePanel selectedId="cs-1" nodeProps={lastProps} />);
// The "+ Add Item" button for the Slides ArrayPropEditor.
const addButtons = Array.from(container.querySelectorAll('button')).filter(
(b) => b.textContent?.includes('Add Item'),
);
expect(addButtons.length).toBeGreaterThan(0);
act(() => {
addButtons[0].dispatchEvent(new MouseEvent('click', { bubbles: true }));
});
expect(setPropSpy).toHaveBeenCalled();
expect(lastProps.slides).toHaveLength(1);
expect(lastProps.slides[0]).toHaveProperty('imageSrc');
expect(lastProps.slides[0].image).toBeUndefined();
});
});
@@ -145,14 +145,14 @@ export const MediaStylePanel: React.FC<StylePanelProps> = ({ selectedId, nodePro
}); });
}} placeholder="Text" rows={2} style={{ ...smallInputStyle, resize: 'vertical' }} /> }} placeholder="Text" rows={2} style={{ ...smallInputStyle, resize: 'vertical' }} />
)} )}
{item.image !== undefined && ( {item.imageSrc !== undefined && (
<AssetPicker <AssetPicker
variant="compact" variant="compact"
value={item.image || ''} value={item.imageSrc || ''}
onChange={(url) => { onChange={(url) => {
actions.setProp(selectedId, (props: any) => { actions.setProp(selectedId, (props: any) => {
const updated = [...(props.slides || [])]; const updated = [...(props.slides || [])];
updated[index] = { ...updated[index], image: url }; updated[index] = { ...updated[index], imageSrc: url };
props.slides = updated; props.slides = updated;
}); });
}} }}
@@ -160,7 +160,7 @@ export const MediaStylePanel: React.FC<StylePanelProps> = ({ selectedId, nodePro
)} )}
</div> </div>
)} )}
emptyItem={{ heading: 'New Slide', text: '', image: '' }} emptyItem={{ type: 'image', imageSrc: '', heading: 'New Slide', text: '', bgColor: '' }}
/> />
</CollapsibleSection> </CollapsibleSection>
)} )}
+54 -6
View File
@@ -2,7 +2,7 @@ import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
import React from 'react'; import React from 'react';
import { createRoot, Root } from 'react-dom/client'; import { createRoot, Root } from 'react-dom/client';
import { act } from 'react-dom/test-utils'; import { act } from 'react-dom/test-utils';
import { PageProvider, usePages, uniqueSlug } from './PageContext'; import { PageProvider, usePages, uniqueSlug, nextPageId } from './PageContext';
/* PageContext only needs `useEditor` from @craftjs/core (for query.serialize / /* PageContext only needs `useEditor` from @craftjs/core (for query.serialize /
actions.deserialize during page switches) — mock just that so PageProvider actions.deserialize during page switches) — mock just that so PageProvider
@@ -15,11 +15,10 @@ vi.mock('@craftjs/core', () => ({
}), }),
})); }));
/* addPage mints ids from `Date.now()`. Two adds inside the same test can land /* addPage mints ids via nextPageId() (timestamp + monotonic counter, M-3),
in the same millisecond and collide on id, which is an existing, unrelated so same-millisecond calls no longer collide on id. Date.now() is still
bug (id collision, not slug collision) — out of scope here but it makes pinned/advanced here for determinism across the slug-dedupe assertions
these tests flaky since a colliding id defeats the "other pages" slug below, independent of wall-clock timing. */
lookup. Force distinct ids so the slug-dedupe assertions below are stable. */
let dateNowSpy: ReturnType<typeof vi.spyOn>; let dateNowSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => { beforeEach(() => {
let counter = 1_700_000_000_000; let counter = 1_700_000_000_000;
@@ -48,6 +47,23 @@ function unmount() {
container.remove(); container.remove();
} }
describe('nextPageId (M-3: no same-millisecond id collision)', () => {
test('two calls yield distinct ids even when Date.now() is pinned to a constant', () => {
const spy = vi.spyOn(Date, 'now').mockReturnValue(1_700_000_000_000);
try {
const id1 = nextPageId();
const id2 = nextPageId();
expect(id1).not.toBe(id2);
} finally {
spy.mockRestore();
}
});
test('ids are prefixed with "page_"', () => {
expect(nextPageId()).toMatch(/^page_/);
});
});
describe('uniqueSlug', () => { describe('uniqueSlug', () => {
test('returns base unchanged when no collision', () => { test('returns base unchanged when no collision', () => {
expect(uniqueSlug('about', ['index', 'contact'])).toBe('about'); expect(uniqueSlug('about', ['index', 'contact'])).toBe('about');
@@ -121,6 +137,38 @@ describe('PageProvider slug dedupe', () => {
unmount(); unmount();
}); });
test('M-4: a punctuation-only page name ("!!!") falls back to slug "page", never empty string', () => {
let ctx: ReturnType<typeof usePages> | null = null;
const Consumer: React.FC = () => {
ctx = usePages();
return null;
};
render(
<PageProvider>
<Consumer />
</PageProvider>,
);
act(() => {
ctx!.addPage('!!!', '');
});
const added = ctx!.pages.find((p) => p.name === '!!!')!;
expect(added.slug).toBe('page');
expect(added.slug).not.toBe('');
// A second punctuation-only-named page dedupes to 'page-2', not ''.
act(() => {
ctx!.addPage('???', '');
});
const second = ctx!.pages.find((p) => p.name === '???')!;
expect(second.slug).toBe('page-2');
expect(second.slug).not.toBe('');
unmount();
});
test('landing page slug always stays "index" even if renamed to collide', () => { test('landing page slug always stays "index" even if renamed to collide', () => {
let ctx: ReturnType<typeof usePages> | null = null; let ctx: ReturnType<typeof usePages> | null = null;
const Consumer: React.FC = () => { const Consumer: React.FC = () => {
@@ -96,12 +96,57 @@ describe('treeToCraftState resolvedName guard', () => {
expect(parsed.ROOT.type.resolvedName).toBe('ColumnLayout'); expect(parsed.ROOT.type.resolvedName).toBe('ColumnLayout');
expect(parsed.ROOT.linkedNodes).toEqual({ 'col-0': 'a', 'col-1': 'b' }); expect(parsed.ROOT.linkedNodes).toEqual({ 'col-0': 'a', 'col-1': 'b' });
expect(parsed.ROOT.props.columns).toBe(2); expect(parsed.ROOT.props.columns).toBe(2);
// NB: the ROOT-aliasing step only reassigns `nodes[]` children's parent,
// not `linkedNodes` children's — a pre-existing quirk (not introduced by
// this refactor, and out of scope for E3) that leaves column children's
// `parent` pointing at the tree's original (pre-alias) root id.
expect(parsed['a'].parent).toBe('cols-1');
expect(parsed['a'].isCanvas).toBe(true); expect(parsed['a'].isCanvas).toBe(true);
// I-2: the ROOT-aliasing step must reparent linkedNodes children too, not
// just nodes[] children. Before the fix, 'a'/'b'.parent stayed pointing
// at the tree's original (pre-alias) root id ('cols-1'), which is then
// `delete`d from the output -- a dangling parent reference that breaks
// select/move/delete of those nodes in the Craft.js editor.
expect(parsed['a'].parent).toBe('ROOT');
expect(parsed['b'].parent).toBe('ROOT');
expect(parsed['cols-1']).toBeUndefined();
});
test('I-2: every child parent in the output tree references an id that exists (no dangling reference to the deleted old-root id) -- ColumnLayout root', () => {
const tree: SerializedTreeNode = {
type: { resolvedName: 'ColumnLayout' },
props: { node_id: 'cols-1' },
nodes: [
{ type: { resolvedName: 'Heading' }, props: { node_id: 'a' }, nodes: [] },
{ type: { resolvedName: 'Heading' }, props: { node_id: 'b' }, nodes: [] },
],
};
const parsed = JSON.parse(treeToCraftState(tree));
for (const id of Object.keys(parsed)) {
const parent = parsed[id].parent;
if (parent == null) continue;
expect(parsed[parent]).toBeDefined();
}
});
test('I-2: a Section (SHELL_INNER) root reparents its section-inner linkedNode child to ROOT', () => {
const tree: SerializedTreeNode = {
type: { resolvedName: 'Section' },
props: { node_id: 'sec-root' },
nodes: [
{ type: { resolvedName: 'Heading' }, props: { node_id: 'h-1', text: 'Hi' }, nodes: [] },
],
};
const parsed = JSON.parse(treeToCraftState(tree));
const innerId = parsed.ROOT.linkedNodes['section-inner'];
expect(innerId).toBeDefined();
// The section-inner node itself is a linkedNodes child of the
// (re-keyed) root -- its parent must point at 'ROOT', not the deleted
// original root id 'sec-root'.
expect(parsed[innerId].parent).toBe('ROOT');
expect(parsed['sec-root']).toBeUndefined();
// And every parent reference in the whole tree resolves to a real node.
for (const id of Object.keys(parsed)) {
const parent = parsed[id].parent;
if (parent == null) continue;
expect(parsed[parent]).toBeDefined();
}
}); });
test('style: [] normalizes to {} (regression)', () => { test('style: [] normalizes to {} (regression)', () => {
+55 -3
View File
@@ -21,6 +21,17 @@ interface PageContextValue {
setHeaderCraftState: (craftState: string) => void; setHeaderCraftState: (craftState: string) => void;
setFooterCraftState: (craftState: string) => void; setFooterCraftState: (craftState: string) => void;
setPagesCraftState: (pagesData: { id: string; name: string; slug: string; craftState: string | null }[]) => void; setPagesCraftState: (pagesData: { id: string; name: string; slug: string; craftState: string | null }[]) => void;
/**
* Bookkeeping-only: point `activePageId` at an already-loaded page without
* re-serializing/deserializing the canvas (the caller -- e.g. useWhpApi's
* `load()` -- has already put the right state on the canvas itself). Used
* to fix I-1: `activePageId` defaults to the hardcoded `'home'` and
* `load()` never updated it, so after the original Home page was deleted
* (its replacement gets a fresh `page_<ts>` id) and the app reloaded,
* `activePageId` pointed at nothing in `pages`, and a subsequent edit+save
* only reached the legacy top-level fields.
*/
setActivePageIdDirect: (pageId: string) => void;
/** AI helpers — replace entire site or page with a new tree */ /** AI helpers — replace entire site or page with a new tree */
replaceAllPages: (pages: { name: string; tree: SerializedTreeNode }[]) => void; replaceAllPages: (pages: { name: string; tree: SerializedTreeNode }[]) => void;
replaceCurrentPage: (page: { name: string; tree: SerializedTreeNode }) => void; replaceCurrentPage: (page: { name: string; tree: SerializedTreeNode }) => void;
@@ -32,6 +43,20 @@ interface PageContextValue {
const HEADER_ID = '__header__'; const HEADER_ID = '__header__';
const FOOTER_ID = '__footer__'; const FOOTER_ID = '__footer__';
// M-3: `page_${Date.now()}` alone collides when two pages are minted inside
// the same millisecond (addPage called twice in quick succession, or two AI
// replaceAllPages entries) -- then rename/delete/save operate on both pages
// at once since they share an id. A module-scoped monotonic counter,
// combined with the timestamp, guarantees uniqueness regardless of how many
// ids are minted within the same millisecond. This is state/id-minting code
// (not toHtml/export), so Date.now() here is fine -- see task-minors-brief.md.
let pageIdCounter = 0;
/** Mints a unique page id: timestamp (base36) + a monotonic per-process counter (base36). */
export function nextPageId(): string {
return 'page_' + Date.now().toString(36) + '_' + (++pageIdCounter).toString(36);
}
const EMPTY_CANVAS = const EMPTY_CANVAS =
'{"ROOT":{"type":{"resolvedName":"Container"},"isCanvas":true,"props":{"style":{"minHeight":"100vh","backgroundColor":"#ffffff"},"tag":"div"},"displayName":"Container","custom":{},"hidden":false,"nodes":[],"linkedNodes":{}}}'; '{"ROOT":{"type":{"resolvedName":"Container"},"isCanvas":true,"props":{"style":{"minHeight":"100vh","backgroundColor":"#ffffff"},"tag":"div"},"displayName":"Container","custom":{},"hidden":false,"nodes":[],"linkedNodes":{}}}';
@@ -75,6 +100,16 @@ export function treeToCraftState(tree: SerializedTreeNode): string {
for (const childId of nodes['ROOT'].nodes) { for (const childId of nodes['ROOT'].nodes) {
if (nodes[childId]) nodes[childId].parent = 'ROOT'; if (nodes[childId]) nodes[childId].parent = 'ROOT';
} }
// I-2: linkedNodes children (e.g. ColumnLayout's col-0/col-1, or a
// Section/BackgroundSection/FormContainer's SHELL_INNER wrapper) need
// the same reparenting as nodes[] children above. Without this, a
// ColumnLayout/SHELL_INNER-rooted AI `replace` leaves those children's
// `parent` pointing at the OLD root id, which is then `delete`d --
// producing a dangling parent reference that breaks select/move/delete
// of those nodes in the Craft.js editor.
for (const linkedId of Object.values(nodes['ROOT'].linkedNodes)) {
if (nodes[linkedId]) nodes[linkedId].parent = 'ROOT';
}
} }
return JSON.stringify(nodes); return JSON.stringify(nodes);
} }
@@ -149,6 +184,7 @@ const PageContext = createContext<PageContextValue>({
setHeaderCraftState: () => {}, setHeaderCraftState: () => {},
setFooterCraftState: () => {}, setFooterCraftState: () => {},
setPagesCraftState: () => {}, setPagesCraftState: () => {},
setActivePageIdDirect: () => {},
replaceAllPages: () => {}, replaceAllPages: () => {},
replaceCurrentPage: () => {}, replaceCurrentPage: () => {},
setHeader: () => {}, setHeader: () => {},
@@ -159,12 +195,17 @@ const PageContext = createContext<PageContextValue>({
export const usePages = () => useContext(PageContext); export const usePages = () => useContext(PageContext);
function slugify(name: string): string { function slugify(name: string): string {
return name const slug = name
.toLowerCase() .toLowerCase()
.trim() .trim()
.replace(/[^a-z0-9\s-]/g, '') .replace(/[^a-z0-9\s-]/g, '')
.replace(/\s+/g, '-') .replace(/\s+/g, '-')
.replace(/-+/g, '-'); .replace(/-+/g, '-');
// M-4: a punctuation-only name (e.g. "!!!") strips down to '' -- without a
// fallback, buildSavePayload would write filename = '' + '.html' for that
// page. uniqueSlug's existing dedupe logic then applies on top of this
// fallback the same way it does for any other base ('page', 'page-2', ...).
return slug || 'page';
} }
/** /**
@@ -311,7 +352,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
const addPage = useCallback( const addPage = useCallback(
(name: string, slug: string) => { (name: string, slug: string) => {
const requestedSlug = slug || slugify(name); const requestedSlug = slug || slugify(name);
const id = `page_${Date.now()}`; const id = nextPageId();
// Save current page first // Save current page first
saveCurrentState(); saveCurrentState();
@@ -385,6 +426,16 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
setFooterPage((prev) => ({ ...prev, craftState })); setFooterPage((prev) => ({ ...prev, craftState }));
}, []); }, []);
/**
* Bookkeeping-only setter for `activePageId` -- see the doc comment on
* `PageContextValue.setActivePageIdDirect`. Does NOT serialize/deserialize
* the canvas; callers that need that should use `switchPage` instead.
*/
const setActivePageIdDirect = useCallback((pageId: string) => {
setActivePageId(pageId);
activePageIdRef.current = pageId;
}, []);
/** Allow external code (e.g., load from API) to restore pages with craft states */ /** Allow external code (e.g., load from API) to restore pages with craft states */
const setPagesCraftState = useCallback((pagesData: { id: string; name: string; slug: string; craftState: string | null }[]) => { const setPagesCraftState = useCallback((pagesData: { id: string; name: string; slug: string; craftState: string | null }[]) => {
setPages(pagesData.map((p, i) => ({ setPages(pagesData.map((p, i) => ({
@@ -415,7 +466,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
const slug = i === 0 ? 'index' : uniqueSlug(slugify(p.name), seenSlugs); const slug = i === 0 ? 'index' : uniqueSlug(slugify(p.name), seenSlugs);
seenSlugs.push(slug); seenSlugs.push(slug);
return { return {
id: i === 0 ? 'home' : `page_${Date.now()}_${i}`, id: i === 0 ? 'home' : nextPageId(),
name: p.name, name: p.name,
slug, slug,
craftState: treeToCraftState(p.tree), craftState: treeToCraftState(p.tree),
@@ -492,6 +543,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
setHeaderCraftState, setHeaderCraftState,
setFooterCraftState, setFooterCraftState,
setPagesCraftState, setPagesCraftState,
setActivePageIdDirect,
replaceAllPages, replaceAllPages,
replaceCurrentPage, replaceCurrentPage,
setHeader, setHeader,
+90 -3
View File
@@ -1,5 +1,5 @@
import { describe, test, expect } from 'vitest'; import { describe, test, expect } from 'vitest';
import { escapeHtml, escapeAttr, safeUrl, stableHash, scopeId } from './escape'; import { escapeHtml, escapeAttr, safeUrl, stableHash, scopeId, sanitizeFormMethod, sanitizeInputType } from './escape';
describe('escapeHtml', () => { describe('escapeHtml', () => {
test('escapes &, <, >, "', () => { test('escapes &, <, >, "', () => {
@@ -71,6 +71,24 @@ describe('safeUrl', () => {
expect(safeUrl(s)).toBe(s); expect(safeUrl(s)).toBe(s);
}); });
test('M-5: blocks data:image/svg+xml (can execute script when navigated to directly)', () => {
expect(safeUrl('data:image/svg+xml,<svg onload=alert(1)>')).toBe('');
});
test('M-5: blocks data:image/svg+xml;base64 variant', () => {
expect(safeUrl('data:image/svg+xml;base64,PHN2ZyBvbmxvYWQ9YWxlcnQoMSk+')).toBe('');
});
test('M-5: blocks obfuscated (whitespace/case) data:image/svg+xml', () => {
expect(safeUrl(' DATA:IMAGE/SVG+XML,<svg onload=alert(1)>')).toBe('');
});
test('M-5: still allows other data:image/* types unchanged', () => {
expect(safeUrl('data:image/jpeg;base64,/9j/4AAQ')).toBe('data:image/jpeg;base64,/9j/4AAQ');
expect(safeUrl('data:image/gif;base64,R0lGOD')).toBe('data:image/gif;base64,R0lGOD');
expect(safeUrl('data:image/webp;base64,UklGR')).toBe('data:image/webp;base64,UklGR');
});
test.each([ test.each([
'https://x.com/a?b=1&c=2', 'https://x.com/a?b=1&c=2',
'http://x', 'http://x',
@@ -120,8 +138,24 @@ describe('scopeId', () => {
expect(id1).not.toBe(id2); expect(id1).not.toBe(id2);
}); });
test('node id is slugified (non-alphanumeric characters stripped, lowercased)', () => { test('output is a valid CSS ident: prefix_hash', () => {
expect(scopeId('Node ID! 123', 'seed', 'sb')).toBe('sb_nodeid123'); expect(scopeId('Node ID! 123', 'seed', 'sb')).toMatch(/^sb_[a-z0-9]+$/);
});
test('M-4: case-differing node ids do not collapse to the same scope (no case-fold collision)', () => {
expect(scopeId('AbC', 'seed', 'sb')).not.toBe(scopeId('abc', 'seed', 'sb'));
});
test('M-4: punctuation-differing node ids do not collapse to the same scope', () => {
expect(scopeId('a-b', 'seed', 'sb')).not.toBe(scopeId('ab', 'seed', 'sb'));
});
test('M-4: same input always yields the identical scope id', () => {
expect(scopeId('some-node-id', 'seed', 'sb')).toBe(scopeId('some-node-id', 'seed', 'sb'));
});
test('M-4: output always matches a valid CSS ident pattern', () => {
expect(scopeId('Weird!! Node--ID__123', 'seed', 'sb')).toMatch(/^[a-z]+_[a-z0-9]+$/i);
}); });
test('no nodeId (legacy 2-arg call sites) falls back to a deterministic hash of the seed, not Math.random', () => { test('no nodeId (legacy 2-arg call sites) falls back to a deterministic hash of the seed, not Math.random', () => {
@@ -140,3 +174,56 @@ describe('scopeId', () => {
expect(scopeId('node-abc', 'seed', 'tabs')).toMatch(/^tabs_/); expect(scopeId('node-abc', 'seed', 'tabs')).toMatch(/^tabs_/);
}); });
}); });
describe('sanitizeFormMethod', () => {
test('allows GET and POST unchanged', () => {
expect(sanitizeFormMethod('GET')).toBe('GET');
expect(sanitizeFormMethod('POST')).toBe('POST');
});
test('is case-insensitive and normalizes to uppercase', () => {
expect(sanitizeFormMethod('get')).toBe('GET');
expect(sanitizeFormMethod('post')).toBe('POST');
expect(sanitizeFormMethod('PoSt')).toBe('POST');
});
test('falls back to POST for anything not in the allowlist', () => {
expect(sanitizeFormMethod('DELETE')).toBe('POST');
expect(sanitizeFormMethod('')).toBe('POST');
expect(sanitizeFormMethod(undefined as any)).toBe('POST');
});
test('neutralizes an attribute-breakout injection instead of passing it through', () => {
const malicious = 'POST"><script>alert(1)</script>';
expect(sanitizeFormMethod(malicious)).toBe('POST');
});
test('coerces non-string input safely', () => {
expect(sanitizeFormMethod(123 as any)).toBe('POST');
expect(sanitizeFormMethod(null as any)).toBe('POST');
});
});
describe('sanitizeInputType', () => {
test('allows known-safe input types unchanged', () => {
['text', 'email', 'tel', 'number', 'password', 'url', 'search', 'date', 'checkbox', 'radio', 'hidden'].forEach((t) => {
expect(sanitizeInputType(t)).toBe(t);
});
});
test('falls back to text for anything not in the allowlist', () => {
expect(sanitizeInputType('bogus')).toBe('text');
expect(sanitizeInputType('')).toBe('text');
expect(sanitizeInputType(undefined as any)).toBe('text');
});
test('neutralizes an attribute-breakout injection instead of passing it through', () => {
const malicious = 'text"><img src=x onerror=alert(1)>';
expect(sanitizeInputType(malicious)).toBe('text');
});
test('coerces non-string input safely', () => {
expect(sanitizeInputType(123 as any)).toBe('text');
expect(sanitizeInputType(null as any)).toBe('text');
});
});
+45 -3
View File
@@ -36,7 +36,14 @@ export function escapeAttr(s: string): string {
// Dangerous scheme prefixes, checked against a normalized copy with all // Dangerous scheme prefixes, checked against a normalized copy with all
// colons removed (see safeUrl) so that colon-splicing obfuscation like // colons removed (see safeUrl) so that colon-splicing obfuscation like
// "java:script:alert(1)" can't slip past a literal "javascript:" check. // "java:script:alert(1)" can't slip past a literal "javascript:" check.
const DANGEROUS_SCHEME_PREFIXES = ['javascript', 'vbscript', 'datatext/html']; // M-5: `data:image/svg+xml` is blocked alongside `data:text/html` -- an SVG
// document can carry an inline <script>/onload= just like an HTML document,
// so it executes script when loaded as a document/navigation target (e.g. an
// <a href> or window.open), even though it's nominally an "image" MIME type.
// Other `data:image/*` types (png/jpeg/gif/webp, ...) stay allowed below --
// only this specific scriptable subtype is blocked, regardless of a
// trailing `;base64` or other parameters.
const DANGEROUS_SCHEME_PREFIXES = ['javascript', 'vbscript', 'datatext/html', 'dataimage/svg+xml'];
/** Decodes &#NN; and &#xNN; numeric HTML entities (used to obfuscate scheme names). */ /** Decodes &#NN; and &#xNN; numeric HTML entities (used to obfuscate scheme names). */
function decodeNumericEntities(s: string): string { function decodeNumericEntities(s: string): string {
@@ -151,6 +158,41 @@ export function stableHash(seed: string): string {
* available. * available.
*/ */
export function scopeId(nodeId: string | undefined, fallbackSeed: string, prefix: string): string { export function scopeId(nodeId: string | undefined, fallbackSeed: string, prefix: string): string {
const slug = (nodeId || '').toString().toLowerCase().replace(/[^a-z0-9]+/g, ''); // M-4: hash the raw node id (via the same djb2 `stableHash` used for the
return `${prefix}_${slug || stableHash(fallbackSeed)}`; // fallback path below) rather than lowercasing + stripping punctuation
// into a slug. A slug collapses distinct ids that differ only by
// case/punctuation (e.g. "AbC" and "abc", or "a-b" and "ab") onto the same
// scope; hashing the untouched string keeps them distinct while staying
// deterministic and producing a valid CSS ident (prefix + '_' + [a-z0-9]+).
const seed = (nodeId || '').toString();
return `${prefix}_${stableHash(seed || fallbackSeed)}`;
}
// Shared enum allowlists for `toHtml` attribute sinks fed by props that are
// declared as a TS union (e.g. `method?: 'GET' | 'POST'`) but are NOT
// type-checked at runtime -- they arrive raw via the AI `update_props` path
// (only `node_id` is validated there) or a deserialized saved-state blob.
// Allowlisting -- rather than escaping -- is correct here because the only
// legitimate values are the fixed set below; anything else is either an
// honest mistake or an attribute-breakout attempt, and both collapse safely
// to the same known-good default.
const ALLOWED_FORM_METHODS = ['GET', 'POST'] as const;
/** Allowlists a `<form method>` value, case-insensitively, falling back to 'POST'. */
export function sanitizeFormMethod(m: unknown): 'GET' | 'POST' {
const upper = (typeof m === 'string' ? m : '').trim().toUpperCase();
return (ALLOWED_FORM_METHODS as readonly string[]).includes(upper) ? (upper as 'GET' | 'POST') : 'POST';
}
const ALLOWED_INPUT_TYPES = [
'text', 'email', 'tel', 'number', 'password', 'url', 'search',
'date', 'time', 'datetime-local', 'month', 'week',
'checkbox', 'radio', 'hidden', 'color', 'range', 'file',
] as const;
/** Allowlists an `<input type>` value, falling back to 'text'. */
export function sanitizeInputType(t: unknown): string {
const value = typeof t === 'string' ? t : '';
return (ALLOWED_INPUT_TYPES as readonly string[]).includes(value) ? value : 'text';
} }
+99
View File
@@ -0,0 +1,99 @@
import { describe, test, expect } from 'vitest';
import { exportBodyHtml } from './html-export';
/**
* C2: buildDataAttrs (internal to html-export.ts) previously interpolated
* `props.animation` / `props.animationDelay` directly into
* `data-animation="${...}"` / `data-animation-delay="${...}"` with no
* escaping. Runs for EVERY node (via renderNode -> buildDataAttrs ->
* injectAttrs) and is reachable via AI `update_props` + deserialized saved
* state. A malicious value like `x"><img src=y onerror=alert(1)>` breaks
* out of the attribute and injects a live element.
*/
describe('buildDataAttrs escapes animation/animationDelay (C2)', () => {
const makeState = (props: Record<string, unknown>) =>
JSON.stringify({
ROOT: {
type: { resolvedName: 'Container' },
isCanvas: true,
props: { tag: 'div', style: {}, ...props },
displayName: 'Container',
custom: {},
hidden: false,
nodes: [],
linkedNodes: {},
},
});
test('malicious animation value is escaped -- no raw breakout, no injected <img>', () => {
const payload = 'x"><img src=y onerror=alert(1)>';
const state = makeState({ animation: payload });
const { html } = exportBodyHtml(state);
expect(html).not.toContain('<img src=y onerror=alert(1)>');
expect(html).not.toMatch(/data-animation="x">/);
// The attribute value must be escaped, so no raw `"` or `<` survives
// inside the data-animation attribute region.
expect(html).toContain('data-animation="x&quot;&gt;&lt;img src=y onerror=alert(1)&gt;"');
});
test('malicious animationDelay value is escaped -- no raw breakout', () => {
const payload = 'x"><script>alert(1)</script>';
const state = makeState({ animation: 'fade-in', animationDelay: payload });
const { html } = exportBodyHtml(state);
expect(html).not.toContain('<script>alert(1)</script>');
expect(html).toContain('data-animation-delay="x&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;"');
});
test('normal animation value still emits data-animation="fade-in" unchanged', () => {
const state = makeState({ animation: 'fade-in' });
const { html } = exportBodyHtml(state);
expect(html).toContain('data-animation="fade-in"');
});
test('normal animationDelay value still emits unchanged', () => {
const state = makeState({ animation: 'fade-in', animationDelay: '0.5s' });
const { html } = exportBodyHtml(state);
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 <img>', () => {
const state = makeUnresolvedDivState('div><img src=x onerror=alert(1)');
const { html } = exportBodyHtml(state);
expect(html).not.toContain('<img');
expect(html).not.toContain('onerror');
expect(html.startsWith('<div')).toBe(true);
});
test('a valid tag (section) still emits <section', () => {
const state = makeUnresolvedDivState('section');
const { html } = exportBodyHtml(state);
expect(html).toContain('<section');
expect(html).toContain('</section>');
});
});
+9 -4
View File
@@ -1,6 +1,7 @@
import { componentResolver } from '../components/resolver'; import { componentResolver } from '../components/resolver';
import { cssPropsToString } from './style-helpers'; import { cssPropsToString } from './style-helpers';
import { escapeHtml } from './escape'; import { escapeHtml, escapeAttr } from './escape';
import { sanitizeContainerTag } from '../components/layout/Container';
export interface ExportOptions { export interface ExportOptions {
title?: string; title?: string;
@@ -24,9 +25,9 @@ function buildDataAttrs(props: Record<string, any>): string {
if (props.hideOnTablet) attrs += ' data-hide-tablet'; if (props.hideOnTablet) attrs += ' data-hide-tablet';
if (props.hideOnMobile) attrs += ' data-hide-mobile'; if (props.hideOnMobile) attrs += ' data-hide-mobile';
if (props.animation && props.animation !== 'none') { if (props.animation && props.animation !== 'none') {
attrs += ` data-animation="${props.animation}"`; attrs += ` data-animation="${escapeAttr(String(props.animation))}"`;
if (props.animationDelay && props.animationDelay !== '0') { if (props.animationDelay && props.animationDelay !== '0') {
attrs += ` data-animation-delay="${props.animationDelay}"`; attrs += ` data-animation-delay="${escapeAttr(String(props.animationDelay))}"`;
} }
} }
return attrs; return attrs;
@@ -94,7 +95,11 @@ function renderNode(nodes: Record<string, any>, nodeId: string): { html: string
// Fallback: wrap children in a div with inline styles // Fallback: wrap children in a div with inline styles
if (typeName === 'Container' || typeName === 'div') { if (typeName === 'Container' || typeName === 'div') {
const styleStr = cssPropsToString(props.style); 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 { return {
html: `<${tag}${dataAttrs}${styleStr ? ` style="${styleStr}"` : ''}>${allChildrenHtml}</${tag}>`, html: `<${tag}${dataAttrs}${styleStr ? ` style="${styleStr}"` : ''}>${allChildrenHtml}</${tag}>`,
}; };
+99
View File
@@ -54,3 +54,102 @@ describe('cssPropsToString sanitizes emitted values (A5)', () => {
expect(withoutEntities).not.toContain(';'); expect(withoutEntities).not.toContain(';');
}); });
}); });
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', '"><img src=x onerror=alert(1)>'],
} 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('&lt;img');
expect(out).toContain('&quot;');
});
test('an object value containing an attribute-breakout payload is neutralized', () => {
const out = cssPropsToString({
color: { toString: () => '"><img src=x onerror=alert(1)>' },
} as any);
expect(out).not.toMatch(/[<>"]/);
expect(out).toContain('&lt;img');
expect(out).toContain('&quot;');
});
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({
color: 'red',
'"><img src=x onerror=alert(1)>': '1',
} as any);
expect(out).not.toContain('"><img');
expect(out).not.toContain('onerror');
expect(out).toBe('color:red');
});
test('a key containing > is dropped', () => {
const out = cssPropsToString({ 'foo>bar': '1' } as any);
expect(out).toBe('');
});
test('a key containing < is dropped', () => {
const out = cssPropsToString({ 'foo<bar': '1' } as any);
expect(out).toBe('');
});
test('a key containing a semicolon is dropped', () => {
const out = cssPropsToString({ 'foo;bar': '1' } as any);
expect(out).toBe('');
});
test('a key containing a space is dropped', () => {
const out = cssPropsToString({ 'foo bar': '1' } as any);
expect(out).toBe('');
});
test('legit simple key "color" is preserved', () => {
const out = cssPropsToString({ color: 'red' } as any);
expect(out).toBe('color:red');
});
test('legit camelCase key backgroundColor -> background-color is preserved', () => {
const out = cssPropsToString({ backgroundColor: 'blue' } as any);
expect(out).toBe('background-color:blue');
});
test('a CSS custom property key (--custom-prop) is preserved', () => {
const out = cssPropsToString({ '--custom-prop': '10px' } as any);
expect(out).toBe('--custom-prop:10px');
});
test('a vendor-prefixed key (-webkit-...) is preserved', () => {
const out = cssPropsToString({ WebkitBoxShadow: '0 0 1px red' } as any);
expect(out).toBe('-webkit-box-shadow:0 0 1px red');
});
test('a digits-only key is dropped', () => {
const out = cssPropsToString({ '123': '1' } as any);
expect(out).toBe('');
});
test('other legit keys in the same object still emit even when a malicious key is dropped', () => {
const out = cssPropsToString({
color: 'red',
'"><script>alert(1)</script>': 'x',
backgroundColor: 'blue',
} as any);
expect(out).toBe('color:red;background-color:blue');
});
});
+31 -5
View File
@@ -9,10 +9,16 @@ const URL_RE = /url\(\s*(['"]?)([\s\S]*?)\1\s*\)/gi;
// Outside of a url(...) reference, a `;` is never legitimate (declarations // Outside of a url(...) reference, a `;` is never legitimate (declarations
// are separated by it) -- stray semicolons are how a breakout injects a // are separated by it) -- stray semicolons are how a breakout injects a
// second property -- and a raw `"` would close the `style="..."` attribute // second property -- and a raw `"` would close the `style="..."` attribute
// early. Inside url('...') the content has already been made safe via // early. `<`/`>` are escaped too as defense-in-depth: they're inert inside a
// escapeAttr(safeUrl(...)), including any `;` required by data-URI syntax // properly quote-terminated `style="..."` attribute, but a value can reach
// (`data:<mime>;base64,<payload>`), so this must never be applied there. // this function from a non-string source (array/object coerced via
const sanitizeBreakoutChars = (s: string): string => s.replace(/;/g, '').replace(/"/g, '&quot;'); // `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:<mime>;base64,<payload>`), so this must never be
// applied there.
const sanitizeBreakoutChars = (s: string): string =>
s.replace(/;/g, '').replace(/"/g, '&quot;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
/** /**
* Sanitizes a single CSS declaration value so it can never terminate the * Sanitizes a single CSS declaration value so it can never terminate the
@@ -43,11 +49,31 @@ function sanitizeCssValue(raw: string): string {
return out; return out;
} }
// A real CSS property name (`color`, `background-color`), vendor-prefixed
// property (`-webkit-box-shadow`), or custom property (`--custom-prop`) --
// nothing else. This is a KEY allowlist, not a value sanitizer: it exists
// solely to stop a malicious style object KEY (e.g.
// `'"><img src=x onerror=alert(1)>'`, reachable via AI `update_props` or
// deserialized saved state, which spread arbitrary keys into `p.style`)
// from being emitted unescaped into `style="${camelToKebab(k)}:${...}"` and
// closing the attribute early. Legitimate keys never contain `"`, `>`, `<`,
// `;`, whitespace, `{`, `}`, or digits-only, so this never rejects real CSS.
const VALID_CSS_KEY_RE = /^-{0,2}[a-z][a-z-]*$/;
export function cssPropsToString(style: CSSProperties | undefined): string { export function cssPropsToString(style: CSSProperties | undefined): string {
if (!style) return ''; if (!style) return '';
return Object.entries(style) return Object.entries(style)
.filter(([, v]) => v !== undefined && v !== null && v !== '') .filter(([, v]) => v !== undefined && v !== null && v !== '')
.map(([k, v]) => `${camelToKebab(k)}:${typeof v === 'string' ? sanitizeCssValue(v) : v}`) .map(([k, v]) => [camelToKebab(k), v] as const)
.filter(([k]) => VALID_CSS_KEY_RE.test(k))
// 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', '"><img src=x onerror=alert(1)>']`) 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(';'); .join(';');
} }