Fix image-block placeholder (src="" override) + published Gallery/SVG regression (safeImageUrl) #5

Merged
jknapp merged 3 commits from placeholder-and-gallery-fix into main 2026-07-13 02:54:24 +00:00
8 changed files with 156 additions and 26 deletions
Showing only changes of commit 3f3c6fb851 - Show all commits
+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 { useSiteDesign } from '../../state/SiteDesignContext'; import { useSiteDesign } from '../../state/SiteDesignContext';
import { escapeHtml, escapeAttr, safeUrl } from '../../utils/escape'; import { escapeHtml, escapeAttr, safeUrl, safeImageUrl } from '../../utils/escape';
/* ---------- Types ---------- */ /* ---------- Types ---------- */
@@ -104,7 +104,7 @@ Logo.craft = {
let innerHtml: string; let innerHtml: string;
if (props.type === 'image' && props.imageSrc) { if (props.type === 'image' && props.imageSrc) {
const imgStyle = cssPropsToString({ width: props.imageWidth || '120px', height: 'auto', display: 'block' }); const imgStyle = cssPropsToString({ width: props.imageWidth || '120px', height: 'auto', display: 'block' });
innerHtml = `<img src="${escapeAttr(safeUrl(props.imageSrc))}" alt="${escapeAttr(props.text || 'Logo')}"${imgStyle ? ` style="${imgStyle}"` : ''} />`; innerHtml = `<img src="${escapeAttr(safeImageUrl(props.imageSrc))}" alt="${escapeAttr(props.text || 'Logo')}"${imgStyle ? ` style="${imgStyle}"` : ''} />`;
} else { } else {
const spanStyle = cssPropsToString({ const spanStyle = cssPropsToString({
fontWeight: props.fontWeight || '700', fontWeight: props.fontWeight || '700',
+2 -2
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, scopeId } from '../../utils/escape'; import { escapeHtml, escapeAttr, safeUrl, safeImageUrl, cssValue, scopeId } from '../../utils/escape';
/* ---------- Types ---------- */ /* ---------- Types ---------- */
@@ -252,7 +252,7 @@ Navbar.craft = {
let logoHtml: string; let logoHtml: string;
if (props.logoType === 'image' && props.logoImage) { if (props.logoType === 'image' && props.logoImage) {
const imgStyle = cssPropsToString({ width: props.logoWidth || '120px', height: 'auto', display: 'block' }); const imgStyle = cssPropsToString({ width: props.logoWidth || '120px', height: 'auto', display: 'block' });
logoHtml = `<a href="${escapeAttr(safeUrl(logoUrl))}" style="text-decoration:none;display:flex;align-items:center;flex-shrink:0"><img src="${escapeAttr(safeUrl(props.logoImage))}" alt="${escapeAttr(props.logoText || 'Logo')}"${imgStyle ? ` style="${imgStyle}"` : ''} /></a>`; logoHtml = `<a href="${escapeAttr(safeUrl(logoUrl))}" style="text-decoration:none;display:flex;align-items:center;flex-shrink:0"><img src="${escapeAttr(safeImageUrl(props.logoImage))}" alt="${escapeAttr(props.logoText || 'Logo')}"${imgStyle ? ` style="${imgStyle}"` : ''} /></a>`;
} else { } else {
const logoStyle = cssPropsToString({ const logoStyle = cssPropsToString({
fontWeight: '700', fontWeight: '700',
+2 -2
View File
@@ -1,7 +1,7 @@
import React, { CSSProperties, useCallback, useRef } from 'react'; import React, { CSSProperties, useCallback, useRef } 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 { escapeAttr, safeImageUrl } from '../../utils/escape';
export const PLACEHOLDER_SRC = "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='400' height='300'%3E%3Cdefs%3E%3ClinearGradient id='bg' x1='0' y1='0' x2='0' y2='1'%3E%3Cstop offset='0%25' stop-color='%23f1f5f9'/%3E%3Cstop offset='100%25' stop-color='%23e2e8f0'/%3E%3C/linearGradient%3E%3C/defs%3E%3Crect fill='url(%23bg)' width='400' height='300' rx='12'/%3E%3Crect x='2' y='2' width='396' height='296' rx='10' fill='none' stroke='%23cbd5e1' stroke-width='2' stroke-dasharray='8 4'/%3E%3Cg transform='translate(200,110)'%3E%3Crect x='-28' y='-28' width='56' height='56' rx='12' fill='%23cbd5e1' opacity='0.5'/%3E%3Cpath d='M-12 8 L-4 -2 L2 4 L8 -6 L16 8Z' fill='%2394a3b8'/%3E%3Ccircle cx='-6' cy='-10' r='5' fill='%2394a3b8'/%3E%3C/g%3E%3Ctext x='200' y='160' text-anchor='middle' fill='%2364748b' font-family='Inter,sans-serif' font-size='15' font-weight='500'%3EDrop image here%3C/text%3E%3Ctext x='200' y='182' text-anchor='middle' fill='%2394a3b8' font-family='Inter,sans-serif' font-size='12'%3Eor click to upload%3C/text%3E%3C/svg%3E"; export const PLACEHOLDER_SRC = "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='400' height='300'%3E%3Cdefs%3E%3ClinearGradient id='bg' x1='0' y1='0' x2='0' y2='1'%3E%3Cstop offset='0%25' stop-color='%23f1f5f9'/%3E%3Cstop offset='100%25' stop-color='%23e2e8f0'/%3E%3C/linearGradient%3E%3C/defs%3E%3Crect fill='url(%23bg)' width='400' height='300' rx='12'/%3E%3Crect x='2' y='2' width='396' height='296' rx='10' fill='none' stroke='%23cbd5e1' stroke-width='2' stroke-dasharray='8 4'/%3E%3Cg transform='translate(200,110)'%3E%3Crect x='-28' y='-28' width='56' height='56' rx='12' fill='%23cbd5e1' opacity='0.5'/%3E%3Cpath d='M-12 8 L-4 -2 L2 4 L8 -6 L16 8Z' fill='%2394a3b8'/%3E%3Ccircle cx='-6' cy='-10' r='5' fill='%2394a3b8'/%3E%3C/g%3E%3Ctext x='200' y='160' text-anchor='middle' fill='%2364748b' font-family='Inter,sans-serif' font-size='15' font-weight='500'%3EDrop image here%3C/text%3E%3Ctext x='200' y='182' text-anchor='middle' fill='%2394a3b8' font-family='Inter,sans-serif' font-size='12'%3Eor click to upload%3C/text%3E%3C/svg%3E";
@@ -95,5 +95,5 @@ ImageBlock.craft = {
} }
const s = cssPropsToString({ display: 'block', maxWidth: '100%', ...props.style }); const s = cssPropsToString({ display: 'block', maxWidth: '100%', ...props.style });
const alt = props.alt ? ` alt="${escapeAttr(props.alt)}"` : ' alt=""'; const alt = props.alt ? ` alt="${escapeAttr(props.alt)}"` : ' alt=""';
return { html: `<img src="${escapeAttr(safeUrl(src))}"${alt}${s ? ` style="${s}"` : ''} />` }; return { html: `<img src="${escapeAttr(safeImageUrl(src))}"${alt}${s ? ` style="${s}"` : ''} />` };
}; };
@@ -94,6 +94,32 @@ describe('Gallery.toHtml deterministic + unique scope ids (thread node id, no Ma
}); });
}); });
describe('Gallery.toHtml default SVG placeholder images survive export (Bug 2 regression)', () => {
test('a default data:image/svg+xml image emits a non-empty img src, not src=""', () => {
const { html } = toHtml({}, ''); // no images prop -> component default SVG placeholders
expect(html).not.toContain('src=""');
expect(html).toMatch(/src="data:image\/svg\+xml[^"]*"/);
});
test('an explicit data:image/svg+xml gallery image src is preserved (not stripped to empty)', () => {
const svg = 'data:image/svg+xml,%3Csvg%2F%3E';
const { html } = toHtml({ images: [{ src: svg, alt: 'a' }] }, '');
expect(html).toContain(`src="${svg}"`);
});
test('lightbox data-lb-src also preserves data:image/svg+xml (still an image context)', () => {
const svg = 'data:image/svg+xml,%3Csvg%2F%3E';
const { html } = toHtml({ images: [{ src: svg, alt: 'a' }], lightbox: true }, '');
expect(html).toContain(`data-lb-src="${svg}"`);
});
test('a javascript: gallery image src still yields an empty src (safeImageUrl still blocks it)', () => {
const { html } = toHtml({ images: [{ src: 'javascript:alert(1)', alt: 'a' }] }, '');
expect(html).toContain('src=""');
expect(html).not.toContain('javascript:');
});
});
describe('Gallery.toHtml lightbox focus management (M-2)', () => { describe('Gallery.toHtml lightbox focus management (M-2)', () => {
const props = { images: [{ src: '/a.jpg', alt: 'a' }], lightbox: true }; const props = { images: [{ src: '/a.jpg', alt: 'a' }], lightbox: true };
+3 -3
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, safeUrl, scopeId, cssValue } from '../../utils/escape'; import { escapeHtml, escapeAttr, safeImageUrl, scopeId, cssValue } from '../../utils/escape';
interface GalleryImage { interface GalleryImage {
src: string; src: string;
@@ -154,10 +154,10 @@ Gallery.craft = {
// inline onclick with an interpolated src -- a single delegated click // inline onclick with an interpolated src -- a single delegated click
// listener below reads it, so a src containing a quote can't break out // listener below reads it, so a src containing a quote can't break out
// of a per-item event-handler string. // of a per-item event-handler string.
const lbAttr = lightbox ? ` data-lb-src="${escapeAttr(safeUrl(img.src || ''))}" role="button" tabindex="0"` : ''; const lbAttr = lightbox ? ` data-lb-src="${escapeAttr(safeImageUrl(img.src || ''))}" role="button" tabindex="0"` : '';
const itemStyle = lightbox ? 'cursor:pointer;position:relative;overflow:hidden;border-radius:8px' : 'position:relative;overflow:hidden;border-radius:8px'; const itemStyle = lightbox ? 'cursor:pointer;position:relative;overflow:hidden;border-radius:8px' : 'position:relative;overflow:hidden;border-radius:8px';
return `<div${lbAttr} style="${itemStyle}"> return `<div${lbAttr} style="${itemStyle}">
<img src="${escapeAttr(safeUrl(img.src || ''))}" alt="${escapeAttr(img.alt)}" style="width:100%;height:200px;object-fit:cover;display:block;border-radius:8px;background-color:#f1f5f9" /> <img src="${escapeAttr(safeImageUrl(img.src || ''))}" alt="${escapeAttr(img.alt)}" style="width:100%;height:200px;object-fit:cover;display:block;border-radius:8px;background-color:#f1f5f9" />
${caption} ${caption}
</div>`; </div>`;
}).join('\n '); }).join('\n ');
+59 -1
View File
@@ -1,5 +1,5 @@
import { describe, test, expect } from 'vitest'; import { describe, test, expect } from 'vitest';
import { escapeHtml, escapeAttr, safeUrl, stableHash, scopeId, sanitizeFormMethod, sanitizeInputType } from './escape'; import { escapeHtml, escapeAttr, safeUrl, safeImageUrl, stableHash, scopeId, sanitizeFormMethod, sanitizeInputType } from './escape';
describe('escapeHtml', () => { describe('escapeHtml', () => {
test('escapes &, <, >, "', () => { test('escapes &, <, >, "', () => {
@@ -112,6 +112,64 @@ describe('safeUrl', () => {
}); });
}); });
describe('safeImageUrl (Bug 2: image-context sink -- data:image/svg+xml is safe as an <img>/CSS url() target)', () => {
test('allows data:image/svg+xml (a raw, non-base64 SVG data URI, as Gallery/ImageBlock placeholders use)', () => {
const s = 'data:image/svg+xml,<svg/>';
expect(safeImageUrl(s)).toBe(s);
});
test('allows data:image/svg+xml;base64 variant', () => {
const s = 'data:image/svg+xml;base64,PHN2Zy8+';
expect(safeImageUrl(s)).toBe(s);
});
test('allows other data:image/* types unchanged', () => {
expect(safeImageUrl('data:image/png;base64,x')).toBe('data:image/png;base64,x');
expect(safeImageUrl('data:image/jpeg;base64,x')).toBe('data:image/jpeg;base64,x');
expect(safeImageUrl('data:image/webp;base64,x')).toBe('data:image/webp;base64,x');
expect(safeImageUrl('data:image/gif;base64,x')).toBe('data:image/gif;base64,x');
});
test('still blocks javascript: scheme', () => {
expect(safeImageUrl('javascript:alert(1)')).toBe('');
});
test('still blocks vbscript: scheme', () => {
expect(safeImageUrl('vbscript:msgbox(1)')).toBe('');
});
test('still blocks data:text/html', () => {
expect(safeImageUrl('data:text/html,x')).toBe('');
});
test('still blocks non-image data: types generally (e.g. data:application/...)', () => {
expect(safeImageUrl('data:application/javascript,alert(1)')).toBe('');
});
test('blocks obfuscated (whitespace/case) javascript: scheme, same as safeUrl', () => {
expect(safeImageUrl(' JavaScript:alert(1)')).toBe('');
expect(safeImageUrl('java\tscript:alert(1)')).toBe('');
expect(safeImageUrl('&#106;avascript:alert(1)')).toBe('');
});
test('allows ordinary http(s)/relative urls unchanged, same as safeUrl', () => {
expect(safeImageUrl('https://example.com/photo.jpg')).toBe('https://example.com/photo.jpg');
expect(safeImageUrl('/assets/photo.jpg')).toBe('/assets/photo.jpg');
expect(safeImageUrl('')).toBe('');
});
test('coerces non-string to empty string', () => {
expect(safeImageUrl(null as any)).toBe('');
expect(safeImageUrl(undefined as any)).toBe('');
});
});
describe('safeUrl still blocks data:image/svg+xml (href/iframe/navigation context unchanged by safeImageUrl addition)', () => {
test('safeUrl(data:image/svg+xml,...) is still blocked', () => {
expect(safeUrl('data:image/svg+xml,<svg onload=alert(1)>')).toBe('');
});
});
describe('stableHash', () => { describe('stableHash', () => {
test('same input always produces the same output', () => { test('same input always produces the same output', () => {
expect(stableHash('hello')).toBe(stableHash('hello')); expect(stableHash('hello')).toBe(stableHash('hello'));
+56 -14
View File
@@ -64,20 +64,7 @@ export function safeUrl(s: string): string {
const trimmed = s.trim(); const trimmed = s.trim();
if (trimmed === '') return ''; if (trimmed === '') return '';
// Build a normalized copy for scheme detection only: decode numeric HTML const collapsed = normalizeForSchemeCheck(trimmed);
// entities (catches e.g. &#106; -> 'j'), strip whitespace/control chars,
// and lowercase.
const normalized = decodeNumericEntities(trimmed)
.replace(/[\x00-\x20]+/g, '')
.toLowerCase();
// Strip any colons before matching the scheme prefix. This defeats
// obfuscation that splices extra colons into the scheme name itself
// (e.g. decoding &#58; mid-word produces "java:script:alert(1)", which
// would otherwise dodge a literal "^javascript:" check) while still
// reliably catching the real "javascript:"/"vbscript:"/"data:text/html"
// prefixes once their own colon is removed.
const collapsed = normalized.replace(/:/g, '');
if (DANGEROUS_SCHEME_PREFIXES.some((prefix) => collapsed.startsWith(prefix))) { if (DANGEROUS_SCHEME_PREFIXES.some((prefix) => collapsed.startsWith(prefix))) {
return ''; return '';
@@ -86,6 +73,61 @@ export function safeUrl(s: string): string {
return trimmed; return trimmed;
} }
/**
* Image-context variant of `safeUrl` for IMAGE sinks only (`<img src>`, CSS
* `url(...)` backgrounds). `data:image/svg+xml` is safe here -- loaded as an
* image, an SVG is rasterized and never executes an inline <script>/onload=
* the way it would as a navigation/iframe document -- so, unlike `safeUrl`,
* this ALLOWS every `data:image/*` subtype (svg+xml, png, jpeg, gif, webp,
* ..., with or without `;base64`).
*
* Still blocks `javascript:` / `vbscript:` (same obfuscation-resistant
* normalization as `safeUrl`), and -- because this is an image sink, not a
* general-purpose URL sink -- treats `data:` as an ALLOWLIST rather than a
* blocklist: any `data:` URI whose type is NOT `image/*` (`data:text/html`,
* `data:application/javascript`, `data:text/javascript`, etc.) is blocked
* too, since none of those are legitimate image sources.
*
* Do NOT use this for href/iframe/form-action/navigation sinks -- keep
* those on `safeUrl`, which still blocks `data:image/svg+xml`.
*/
export function safeImageUrl(s: unknown): string {
if (typeof s !== 'string') return '';
const trimmed = s.trim();
if (trimmed === '') return '';
const collapsed = normalizeForSchemeCheck(trimmed);
if (collapsed.startsWith('javascript') || collapsed.startsWith('vbscript')) {
return '';
}
if (collapsed.startsWith('data') && !collapsed.startsWith('dataimage')) {
// A data: URI whose MIME type isn't image/* -- e.g. data:text/html,
// data:application/javascript, data:text/javascript. No legitimate
// image source needs these; block unconditionally.
return '';
}
return trimmed;
}
// Normalizes a trimmed URL string for scheme-prefix matching only: decodes
// numeric HTML entities (catches e.g. &#106; -> 'j'), strips whitespace /
// control chars, lowercases, then strips every colon. Stripping colons
// defeats obfuscation that splices extra colons into the scheme name itself
// (e.g. decoding &#58; mid-word produces "java:script:alert(1)", which would
// otherwise dodge a literal "^javascript:" check) while still reliably
// catching the real "javascript:"/"vbscript:"/"data:..." prefixes once their
// own colon is removed. Used for prefix `.startsWith()` checks only -- the
// original (non-collapsed) string is always what gets returned/emitted.
function normalizeForSchemeCheck(trimmed: string): string {
return decodeNumericEntities(trimmed)
.replace(/[\x00-\x20]+/g, '')
.toLowerCase()
.replace(/:/g, '');
}
/** /**
* Neutralizes CSS-context breakout for a single design-token value (color, * Neutralizes CSS-context breakout for a single design-token value (color,
* length, gradient, etc.) so it is safe to interpolate RAW into either CSS * length, gradient, etc.) so it is safe to interpolate RAW into either CSS
+6 -2
View File
@@ -1,5 +1,5 @@
import { CSSProperties } from 'react'; import { CSSProperties } from 'react';
import { escapeAttr, safeUrl } from './escape'; import { escapeAttr, safeImageUrl } from './escape';
const camelToKebab = (str: string): string => const camelToKebab = (str: string): string =>
str.replace(/[A-Z]/g, (m) => '-' + m.toLowerCase()); str.replace(/[A-Z]/g, (m) => '-' + m.toLowerCase());
@@ -41,8 +41,12 @@ function sanitizeCssValue(raw: string): string {
// Neutralize the url(...) reference: validate/strip the scheme and // Neutralize the url(...) reference: validate/strip the scheme and
// re-wrap in single quotes with the contents escaped for attribute // re-wrap in single quotes with the contents escaped for attribute
// safety. This is already fully safe, `;` and all. // safety. This is already fully safe, `;` and all.
// Image-context sink (background/mask/border-image url()): use
// safeImageUrl, not safeUrl -- a data:image/svg+xml background is safe
// (rasterized, never executed as a document) and must survive here, the
// same way it must survive on an <img src>.
const inner = m[2]; const inner = m[2];
out += `url('${escapeAttr(safeUrl(inner.trim()))}')`; out += `url('${escapeAttr(safeImageUrl(inner.trim()))}')`;
lastIndex = URL_RE.lastIndex; lastIndex = URL_RE.lastIndex;
} }
out += sanitizeBreakoutChars(raw.slice(lastIndex)); out += sanitizeBreakoutChars(raw.slice(lastIndex));