From f0b3dd1dba5cc1b585c858d91a040d019e02e923 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 1 Nov 2026 10:37:56 -0800 Subject: [PATCH] Sitesmith capture: CSS judged via CSSOM (no regex), single URL gate (no % / dot segments / backslash), all resources inlined through a bounded no-credentials fetch, CSP img-src data: only; bypass-class proof tests (unit + Firefox/Chromium) Co-Authored-By: Claude Sonnet 5.5 --- craft/scripts/e2e-capture-security-entry.ts | 4 +- craft/scripts/e2e-capture-security.mjs | 15 +- craft/src/panels/sitesmith/selfReview.ts | 61 ++++ craft/src/utils/compose-review-html.ts | 28 ++ craft/src/utils/sanitize-capture-html.test.ts | 97 +++++- craft/src/utils/sanitize-capture-html.ts | 294 ++++++++++++++---- craft/src/utils/self-review-capture.ts | 117 +++++-- 7 files changed, 523 insertions(+), 93 deletions(-) create mode 100644 craft/src/panels/sitesmith/selfReview.ts create mode 100644 craft/src/utils/compose-review-html.ts diff --git a/craft/scripts/e2e-capture-security-entry.ts b/craft/scripts/e2e-capture-security-entry.ts index ba83388..5ad3533 100644 --- a/craft/scripts/e2e-capture-security-entry.ts +++ b/craft/scripts/e2e-capture-security-entry.ts @@ -1,13 +1,13 @@ // Bundled by scripts/e2e-capture-security.mjs and run in real Firefox + Chromium. import { captureHtml } from '../src/utils/self-review-capture'; -import { captureCsp } from '../src/utils/sanitize-capture-html'; +import { CAPTURE_CSP } from '../src/utils/sanitize-capture-html'; (window as any).captureHtml = captureHtml; /** Defence-in-depth probe: the capture frame WITHOUT the sanitiser (sandbox + CSP only). */ (window as any).rawFrame = (html: string) => new Promise((resolve) => { const f = document.createElement('iframe'); f.setAttribute('sandbox', 'allow-same-origin'); - const CSP = captureCsp(location.origin); + const CSP = CAPTURE_CSP; f.setAttribute('csp', CSP); f.style.cssText = 'position:fixed;left:-9999px;width:800px;height:600px'; f.addEventListener('load', () => setTimeout(resolve, 800), { once: true }); diff --git a/craft/scripts/e2e-capture-security.mjs b/craft/scripts/e2e-capture-security.mjs index 11fa801..1f54196 100644 --- a/craft/scripts/e2e-capture-security.mjs +++ b/craft/scripts/e2e-capture-security.mjs @@ -36,6 +36,17 @@ const EVIL = `
x
s
+ +
css bypass probes
+

Visible paragraph so the capture has real content ${'lorem ipsum '.repeat(40)}

`; const BENIGN = ` @@ -73,7 +84,7 @@ async function run(name, launcher, exe) { const pwned = await page.evaluate(() => window.__pwned); check(name, 'hostile page: parent sentinel window.__pwned untouched', pwned === undefined, `(${pwned})`); check(name, 'hostile page: no postMessage to the editor', messages === 0, `(${messages})`); - check(name, 'hostile page: no request to the panel origin /beacon', beacons.length === 0, JSON.stringify(beacons)); + check(name, 'hostile page: ZERO requests to the panel origin /beacon (every CSS/URL/encoding bypass class)', beacons.length === 0, JSON.stringify(beacons)); check(name, 'hostile page: no request to evil.example', evilReq.length === 0, JSON.stringify(evilReq)); check(name, 'hostile page: capture result is safe (ok, or refused) and frame removed', (res.ok || typeof res.reason === 'string') && (await page.evaluate(() => document.querySelectorAll('iframe').length)) === 0, JSON.stringify(res)); check(name, 'hostile page: sanitised content still captured (readable page)', res.ok === true && res.size > 3000, JSON.stringify(res)); @@ -86,7 +97,7 @@ async function run(name, launcher, exe) { check(name, 'unsanitised probe: sandbox+CSP alone stop script execution (__pwned)', pwned2 === undefined, `(${pwned2})`); check(name, 'unsanitised probe: no postMessage', messages === 0); // Same-origin passive GETs (img/css url) are allowed by CSP 'self'; that is why the sanitiser also restricts them. Script-initiated ones must never occur. - check(name, 'unsanitised probe: no script-initiated requests (js/img-js beacons)', !beacons.some((b) => /\?(js|img-js|refresh|import|iframe|object|embed|video)$/.test(b)), JSON.stringify(beacons)); + check(name, 'unsanitised probe: CSP alone (img-src data:) lets no request reach the panel origin', beacons.length === 0, JSON.stringify(beacons)); check(name, 'unsanitised probe: no evil.example requests', evilReq.length === 0, JSON.stringify(evilReq)); await page.evaluate(() => document.querySelectorAll('iframe').forEach((f) => f.remove())); diff --git a/craft/src/panels/sitesmith/selfReview.ts b/craft/src/panels/sitesmith/selfReview.ts new file mode 100644 index 0000000..9680566 --- /dev/null +++ b/craft/src/panels/sitesmith/selfReview.ts @@ -0,0 +1,61 @@ +import type { SendResult } from '../../types/sitesmith'; +import type { CaptureResult } from '../../utils/self-review-capture'; + +/** Instruction for the one follow-up job. The render rides in the mockup slot, so say plainly what it is. */ +export const REVIEW_MESSAGE = + 'The attached image is a screenshot of the page as it is built right now. It is NOT a mockup to copy. ' + + 'Review it like a careful designer: look for text that is hard to read, cramped or uneven spacing, misaligned or ' + + 'broken layout, images that are cropped badly, missing icons, and sections that look generic. ' + + 'Fix only real problems with a small patch; if the page already looks right, answer with a short "ask"-free patch with no ops and say it looks good. ' + + 'Do not redesign the page and do not change the wording.'; + +export interface SelfReviewDeps { + /** Published-style HTML of the current page (untrusted; the capture sanitises it). */ + composeHtml: () => string; + /** `assets/` -> same-origin URL, for images in the capture. */ + assetUrls: Record; + allowUrls: string[]; + capture: (html: string, o: { allowUrls: string[]; timeoutMs: number }) => Promise; + upload: (file: File) => Promise<{ ok: true; id: string } | { ok: false; message: string }>; + /** Start the follow-up job and wait for it. */ + review: (message: string, imageId: string) => Promise; + /** Apply the review result (contrast repair included). */ + applyResult: (res: SendResult) => Promise<{ ok: boolean; message?: string; summary?: string }>; + removeImage: (id: string) => Promise; + resolveAssetUrls: (html: string, urls: Record) => string; +} + +export type SelfReviewOutcome = + | { status: 'applied'; note: string } + | { status: 'looks-good'; note: string } + | { status: 'skipped'; note: string }; + +const skip = (why: string): SelfReviewOutcome => ({ status: 'skipped', note: `Self-review skipped: ${why}. Your page was built as normal.` }); + +/** One review round, never more. Never throws; any failure is a non-blocking note. */ +export async function runSelfReview(d: SelfReviewDeps): Promise { + let imageId: string | null = null; + try { + let html: string; + try { html = d.resolveAssetUrls(d.composeHtml(), d.assetUrls); } catch { return skip('the page could not be rendered'); } + const cap = await d.capture(html, { allowUrls: d.allowUrls, timeoutMs: 20000 }); + if (!cap.ok) return skip(cap.reason); + const file = new File([cap.blob], 'self-review-render.jpg', { type: 'image/jpeg' }); + const up = await d.upload(file); + if (!up.ok) return skip(up.message || 'the screenshot could not be uploaded'); + imageId = up.id; + const res = await d.review(REVIEW_MESSAGE, imageId); + if (!res.ok) return skip(res.message || 'the review request failed'); + if (res.response.type === 'ask') return { status: 'looks-good', note: 'Self-review: Sitesmith had no changes to make.' }; + if (res.response.type === 'patch' && res.response.ops.length === 0) return { status: 'looks-good', note: 'Self-review: Sitesmith looked at the finished page and found nothing to fix.' }; + const applied = await d.applyResult(res); + if (!applied.ok) return skip(applied.message || 'the fixes could not be applied'); + const extra = applied.summary ? ` ${applied.summary}.` : ''; + return { status: 'applied', note: `Self-review: Sitesmith looked at the finished page and made fixes.${extra} Use Undo to go back.` }; + } catch (e) { + return skip(e instanceof Error && e.message ? e.message.slice(0, 100) : 'something went wrong'); + } finally { + // The render is private scratch, not a keepsake: it would otherwise eat the per-site image quota. + if (imageId) { try { await d.removeImage(imageId); } catch { /* best effort */ } } + } +} diff --git a/craft/src/utils/compose-review-html.ts b/craft/src/utils/compose-review-html.ts new file mode 100644 index 0000000..da5833f --- /dev/null +++ b/craft/src/utils/compose-review-html.ts @@ -0,0 +1,28 @@ +import type { SiteDesign } from '../state/SiteDesignContext'; +import { exportBodyHtml, exportComposedPreviewHtml } from './html-export'; + +export interface ComposeInput { + /** `query.serialize()` of the page on the canvas. */ + serialized: string; + headerState: string | null; + footerState: string | null; + title: string; + design: SiteDesign; +} + +/** + * Published-style document for the page (header + body + footer, token CSS, fonts), built exactly like + * the editor's Preview but WITHOUT the site's custom head code (customer scripts must never reach the + * capture). The result is still untrusted and goes through sanitizeCaptureHtml before it is rendered. + */ +export function composeReviewHtml(i: ComposeInput): string { + const header = i.headerState ? exportBodyHtml(i.headerState, 'header').html : ''; + const body = exportBodyHtml(i.serialized).html; + const footer = i.footerState ? exportBodyHtml(i.footerState, 'footer').html : ''; + return exportComposedPreviewHtml(i.serialized, header + body + footer, { + title: i.title || 'Page', + includeFonts: true, + design: i.design, + favicon: '', + }); +} diff --git a/craft/src/utils/sanitize-capture-html.test.ts b/craft/src/utils/sanitize-capture-html.test.ts index faee743..13b3d42 100644 --- a/craft/src/utils/sanitize-capture-html.test.ts +++ b/craft/src/utils/sanitize-capture-html.test.ts @@ -105,12 +105,95 @@ describe('sanitizeCaptureHtml', () => { }); }); -describe('captureCsp', () => { - test('spells the origin out instead of self (Firefox does not match self in srcdoc frames)', async () => { - const { captureCsp } = await import('./sanitize-capture-html'); - expect(captureCsp('https://panel.example')).toContain('img-src https://panel.example data:'); - expect(captureCsp('https://panel.example')).not.toContain("'self'"); - expect(captureCsp('javascript:alert(1)')).toContain("'self'"); - expect(captureCsp()).toBe(CAPTURE_CSP); + +describe('CSS is judged from the parsed model, not by regex (bypass classes)', () => { + const css = (c: string, allow: string[] = []) => { + const r = sanitizeCaptureHtml(`

x

`, { allowUrls: allow }) as any; + expect(r.ok).toBe(true); + return dom(r.html).querySelector('style:not([data-capture])')?.textContent ?? ''; + }; + const inline = (v: string) => { + const r = sanitizeCaptureHtml(`
x
`) as any; + return dom(r.html).getElementById('d')?.getAttribute('style') ?? null; + }; + const noFetch = (out: string) => { + expect(out).not.toMatch(/beacon|evil|\/api\//); + expect(out).not.toMatch(/image-set|@import|expression|-moz-binding|behavior/i); + }; + + test.each([ + ['escape-encoded url(', 'p{background:u\\72l(/beacon?e1)}'], + ['escape inside the target', 'p{background:url("\\2f beacon?e2")}'], + ['image-set with string', 'p{background-image:image-set("/beacon?e3" 1x)}'], + ['-webkit-image-set', 'p{background-image:-webkit-image-set(url(/beacon?e4) 1x)}'], + ['image() / cross-fade()', 'p{background-image:cross-fade(url(/beacon?e5),url(/beacon?e5b),50%)}'], + ['quoted ) in the url', 'p{background:url("x)/beacon?e6")}'], + ['comment splitting the token', 'p{background:ur/**/l(/beacon?e7)}'], + ['comment inside url()', 'p{background:url(/*x*/ /beacon?e8)}'], + ['custom-property indirection', ':root{--u:url(/beacon?e9)} p{background:var(--u)}'], + ['@import', '@import url(/beacon?e10); p{color:red}'], + ['escaped @import', '@im\\70ort url(/beacon?e11); p{color:red}'], + ['@namespace', '@namespace url(/beacon?e12); p{color:red}'], + ['expression()', 'p{width:expression(alert(1))}'], + ['-moz-binding / behavior', 'p{-moz-binding:url(/beacon?e13);behavior:url(/beacon?e14)}'], + ['@font-face src to a same-origin API endpoint', '@font-face{font-family:x;src:url(/api/site-builder.php?action=delete_asset)} p{font-family:x}'], + ['@font-face src, other host', '@font-face{font-family:x;src:url(https://evil.example/f.woff2)}'], + ['percent-encoded traversal', 'p{background:url(assets/%2e%2e/api/x)}'], + ['double-encoded traversal', 'p{background:url(assets/%252e%252e/api/x)}'], + ['mixed-case encoded', 'p{background:url(assets/%2E%2E%2Fapi/x)}'], + ['dot segments', 'p{background:url(assets/../api/x)}'], + ['protocol-relative', 'p{background:url(//evil.example/x)}'], + ['javascript: url', 'p{background:url(javascript:alert(1))}'], + ['blob: / data:text', 'p{background:url(blob:http://x/y)} q{background:url("data:text/html,")}'], + ['keyframes with url', '@keyframes k{from{background:url(/beacon?e15)}to{color:red}}'], + ['@media wrapping', '@media (min-width:1px){p{background:url(/beacon?e16)}}'], + ])('%s is dropped', (_n, c) => { + noFetch(css(c)); + }); + + test('benign CSS survives, re-serialised from the parsed model', () => { + const out = css('@media (max-width:600px){.a{color:red}} .b{background:linear-gradient(135deg,#0f172a,#334155);padding:8px} @keyframes k{from{opacity:0}to{opacity:1}} .c{background:url(assets/p.png)}'); + expect(out).toContain('@media'); + expect(out).toContain('linear-gradient'); + expect(out).toContain('@keyframes k'); + expect(out).toMatch(/url\(["']?assets\/p\.png["']?\)/); + }); + + test('exact allow-listed URLs pass in CSS and attributes; near misses do not', () => { + const a = '/api/site-builder.php?action=asset&f=a.png'; + expect(css(`p{background:url(${a})}`, [a])).toContain(a); + expect(css(`p{background:url(${a}&x=1)}`, [a])).not.toContain('action=asset'); + }); + + test('inline style: injection and fetchers are dropped, plain declarations kept', () => { + expect(inline('color:red;padding:4px')).toContain('color:red'); + expect(inline('background:url(/beacon?i1)') ?? '').not.toMatch(/beacon|image-set|url\(/i); + expect(inline('color:red}b{background:url(/beacon?i2)') ?? '').not.toMatch(/beacon/); + expect(inline('background:image-set("/beacon?i3" 1x)') ?? '').not.toMatch(/beacon|image-set|url\(/i); + expect(inline('background:u\\72l(/beacon?i4)') ?? '').not.toMatch(/beacon|image-set|url\(/i); + expect(inline('background-image:url(assets/p.png);color:red')).toContain('color:red'); + }); + + test.each([ + ['assets/%2e%2e/api/x'], ['assets/%2E%2E/x'], ['assets/%252e%252e/x'], ['assets/..%2fx'], ['assets/a%2Fb'], + ['assets\\..\\x'], ['assets/../x'], ['assets/./x'], ['//evil.example/x'], ['/beacon?x'], ['x.png'], ['blob:http://x/y'], + ['javascript:alert(1)'], ['data:text/html;base64,PHNjcmlwdD4='], ['data:image/svg+xml;base64,PHN2Zz48L3N2Zz4='], ['assets/a\u0000b'], + ])('img src %j is replaced by the placeholder', (src) => { + const r = sanitizeCaptureHtml(``) as any; + expect(dom(r.html).getElementById('i')!.getAttribute('src')).toBe(BLOCKED_IMAGE); + }); + + test('plain assets/ images are kept', () => { + const r = sanitizeCaptureHtml('') as any; + expect(dom(r.html).getElementById('i')!.getAttribute('src')).toBe('assets/photo-1.webp'); + expect(dom(r.html).getElementById('j')!.getAttribute('src')).toBe('assets/sub/dir/p.png'); + }); + + test('SVG presentation attributes may only reference in-document ids', () => { + const r = sanitizeCaptureHtml('') as any; + const rect = dom(r.html).getElementById('a')!; + expect(rect.getAttribute('fill')).toBe('url(#g)'); + expect(rect.hasAttribute('filter')).toBe(false); + expect(rect.hasAttribute('mask')).toBe(false); }); }); diff --git a/craft/src/utils/sanitize-capture-html.ts b/craft/src/utils/sanitize-capture-html.ts index ef59349..9f8e275 100644 --- a/craft/src/utils/sanitize-capture-html.ts +++ b/craft/src/utils/sanitize-capture-html.ts @@ -2,101 +2,254 @@ * Sanitiser for the self-review capture. The page HTML contains model output and user text, so it is * treated as hostile: it is parsed with DOMParser (an inert document: nothing runs, nothing loads), * stripped to a small allow-list, re-serialised, and only then handed to a script-less iframe whose - * own CSP (meta) forbids scripts and network access except fonts/stylesheets from two named hosts. + * own CSP (meta) forbids scripts and every network load except fonts/stylesheets from named hosts. * - * Guarantees (each covered by sanitize-capture-html.test.ts and the real-browser proof - * scripts/e2e-capture-security.mjs): no script, frame, object, embed, base, refresh, import, event - * handler, javascript:/vbscript:/data:text URL, @import, or non-same-origin url() survives; output is - * a fixed point of the sanitiser (mutation-XSS guard). + * CSS is NEVER filtered with regular expressions. Every stylesheet and style attribute is parsed by the + * browser's own CSSOM (inert), each rule and declaration is judged from the PARSED, normalised value + * (so escapes, comments, quoting and image-set() tricks are already resolved), and the output is + * re-serialised from that model. Anything not understood is dropped, not repaired. + * + * URLs (attributes and CSS alike) go through one strict gate, `checkUrl`: data: images, `assets/` + * (no percent signs, no dot segments), the caller's exact allow-list, and font files from named hosts. + * Nothing else is ever fetched by the editor on the page's behalf. + * + * Guarantees (sanitize-capture-html.test.ts + the real-browser proof scripts/e2e-capture-security.mjs): + * no script, frame, object, embed, base, refresh, import, event handler, javascript:/vbscript:/data:text + * URL, @import, escape-encoded or image-set() url, or non-allow-listed url() survives; the output is a + * fixed point of the sanitiser (mutation-XSS guard). */ export const CAPTURE_CSP = [ "default-src 'none'", "script-src 'none'", - "img-src 'self' data:", + 'img-src data:', "style-src 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com", - "font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com", + 'font-src data: https://fonts.gstatic.com https://cdnjs.cloudflare.com', "base-uri 'none'", "form-action 'none'", "frame-src 'none'", "object-src 'none'", ].join('; '); -/** Firefox does not match 'self' inside a srcdoc iframe, so callers pass the real origin to spell it out. */ -export function captureCsp(origin?: string): string { - return origin && /^https?:\/\/[A-Za-z0-9.:\[\]-]+$/.test(origin) ? CAPTURE_CSP.replace(/'self'/g, origin) : CAPTURE_CSP; -} - -const REMOVE_TAGS = new Set(['script', - 'iframe', 'frame', 'frameset', 'object', 'embed', 'applet', 'base', 'noscript', 'template', 'math', +const REMOVE_TAGS = new Set([ + 'script', 'iframe', 'frame', 'frameset', 'object', 'embed', 'applet', 'base', 'noscript', 'template', 'math', 'video', 'audio', 'source', 'track', 'portal', 'foreignobject', 'animate', 'set', 'animatetransform', 'animatemotion', - 'meta', 'title-script', + 'meta', ]); const URL_ATTRS = new Set(['href', 'src', 'action', 'formaction', 'xlink:href', 'srcset', 'poster', 'data', 'background', 'ping', 'cite', 'longdesc', 'manifest', 'codebase', 'usemap', 'lowsrc', 'dynsrc']); const DROP_ATTRS = new Set(['srcdoc', 'http-equiv', 'is', 'formmethod', 'target', 'ping']); -const FONT_SHEET_HOSTS = /^https:\/\/(fonts\.googleapis\.com\/|cdnjs\.cloudflare\.com\/ajax\/libs\/font-awesome\/)/i; -const FONT_FILE_HOSTS = /^https:\/\/(fonts\.gstatic\.com\/|cdnjs\.cloudflare\.com\/ajax\/libs\/font-awesome\/)/i; +export const FONT_SHEET_HOSTS = /^https:\/\/(fonts\.googleapis\.com\/|cdnjs\.cloudflare\.com\/ajax\/libs\/font-awesome\/)/i; +export const FONT_FILE_HOSTS = /^https:\/\/(fonts\.gstatic\.com\/|cdnjs\.cloudflare\.com\/ajax\/libs\/font-awesome\/)/i; -/** 1x1 grey PNG: stands in for an image the capture may not fetch (external host), keeping layout. */ +/** 1x1 grey PNG: stands in for an image the capture may not fetch, keeping layout. */ export const BLOCKED_IMAGE = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mO8d+/efwAIgQMeVxsvbAAAAABJRU5ErkJggg=='; -const squash = (v: string) => v.replace(/[\s\u0000-\u001f\u007f-\u009f]/g, '').toLowerCase(); - export interface SanitizeOptions { - /** Exact same-origin URLs (root-relative path + query) that may be fetched, e.g. the site's asset URLs. */ + /** Exact URLs (root-relative path + query, as the editor knows them) that may be fetched, e.g. the site's asset URLs. */ allowUrls?: string[]; - /** Page origin, written into the frame CSP in place of 'self'. */ - origin?: string; } -interface Policy { allow: Set } +export interface Policy { allow: Set } + +export function makePolicy(allowUrls: string[] = []): Policy { + return { allow: new Set(allowUrls.filter((u) => typeof u === 'string' && /^\/[^\s"'<>\\]*$/.test(u) && !u.startsWith('//'))) }; +} + +const CTRL = /[\u0000-\u001f\u007f-\u009f\\]/; +const DATA_IMAGE = /^data:image\/(png|jpeg|gif|webp);base64,[A-Za-z0-9+/=]+$/; +const DATA_FONT = /^data:(font\/(woff2?|ttf|otf)|application\/(font-woff2?|x-font-(woff|ttf|otf)|vnd\.ms-fontobject|octet-stream));base64,[A-Za-z0-9+/=]+$/; +const ASSET_PATH = /^assets\/[A-Za-z0-9._~+-]+(?:\/[A-Za-z0-9._~+-]+)*$/; /** - * A reference the capture frame may load: a data: image, a published-style `assets/` path, or an - * exact URL the caller allow-listed. Every other relative or same-origin URL is refused: a GET to an - * arbitrary panel path (with the user's cookies) is not something page markup gets to trigger. + * The single URL gate. Returns the canonical form to emit, or null when the URL may not be used. + * `font` widens it to font data URLs and the named font hosts. */ -function isLocalRef(raw: string, pol: Policy): boolean { +export function checkUrl(raw: string, pol: Policy, font = false): string | null { + if (typeof raw !== 'string') return null; const t = raw.trim(); - const v = squash(raw); - if (!v) return true; - if (v.startsWith('data:image/png') || v.startsWith('data:image/jpeg') || v.startsWith('data:image/gif') || v.startsWith('data:image/webp')) return true; - if (/^assets\/[A-Za-z0-9._%~+\/-]{1,200}$/.test(t) && !t.includes("..")) return true; - return pol.allow.has(t); + if (!t || t.length > 4000) return null; + if (pol.allow.has(t)) return t; // exact, caller-supplied (server-provided asset URLs) + if (CTRL.test(t) || t.startsWith('//') || t.includes('%')) return null; + if (DATA_IMAGE.test(t)) return t; + if (font && DATA_FONT.test(t)) return t; + if (ASSET_PATH.test(t) && !t.split('/').some((s) => /^\.+$/.test(s))) return t; + if (font) { + let u: URL; + try { u = new URL(t); } catch { return null; } + if (u.protocol === 'https:' && !u.username && !u.password && FONT_FILE_HOSTS.test(u.href) && !u.href.includes('%')) return u.href; + } + return null; } -function safeLink(raw: string): boolean { - const v = squash(raw); - if (/^(javascript|vbscript|data|blob|file):/.test(v)) return false; - if (/^[a-z][a-z0-9+.-]*:/.test(v)) return /^(https?|mailto|tel):/.test(v); - return true; +// ---------------------------------------------------------------- CSS (CSSOM, never regex) + +const RULE = { STYLE: 1, MEDIA: 4, FONT_FACE: 5, KEYFRAMES: 7, KEYFRAME: 8, SUPPORTS: 12 } as const; +const BAD_PROP = /^(behavior|-moz-binding|-ms-behavior)$/i; +// Functions that can fetch: anything but plain url() is dropped outright. +const FETCH_FN = /(?:^|[^a-z0-9_-])(image-set|-webkit-image-set|src|cross-fade|-webkit-cross-fade|element|-moz-element|paint|image)\s*\(/i; +const FETCH_ANY = /(?:^|[^a-z0-9_-])(url|image-set|-webkit-image-set|src|cross-fade|-webkit-cross-fade|element|-moz-element|paint|image)\s*\(/i; +const SAFE_KEY = /^[A-Za-z0-9%., -]+$/; +const SAFE_IDENT = /^[A-Za-z0-9_-]+$/; + +function baseDir(): string { + try { return new URL('./', typeof document !== 'undefined' ? document.baseURI : 'http://localhost/').href; } catch { return ''; } } -/** Remove @import and any url() that is not a same-origin / data-image / allowed font reference. */ -export function cleanCss(css: string, pol: Policy = { allow: new Set() }): string { - let out = css.replace(/@import\b[^;{}]*;?/gi, ''); - out = out.replace(/expression\s*\(|-moz-binding|behavior\s*:/gi, 'x-removed('); - out = out.replace(/url\(\s*(["']?)([^)"']*)\1\s*\)/gi, (m, _q, u: string) => { - const t = u.trim(); - if (isLocalRef(t, pol) || FONT_FILE_HOSTS.test(t)) return m; - return 'none'; - }); +/** Browsers serialise url() targets resolved against the document: map back to what was written. */ +function unresolve(u: string): string[] { + const out = [u]; + const dir = baseDir(); + if (dir && u.startsWith(dir)) out.push(u.slice(dir.length)); + try { + const abs = new URL(u); + if (typeof document !== 'undefined' && abs.origin === new URL(document.baseURI).origin) out.push(abs.pathname + abs.search); + } catch { /* relative: already in `out` */ } return out; } -export type SanitizeResult = { ok: true; html: string; csp?: string } | { ok: false; reason: string }; +/** Judge one parsed declaration value: every fetching function must be plain url() with an allowed target. */ +function valueOk(value: string, pol: Policy, font: boolean): boolean { + if (/[\\]/.test(value)) return false; + if (/expression\s*\(/i.test(value) || /javascript:/i.test(value)) return false; + if (!FETCH_ANY.test(value)) return true; + if (FETCH_FN.test(value)) return false; + let rest = value; + const re = /url\(\s*(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)'|([^"'()\s\\]+))\s*\)/gi; + let m: RegExpExecArray | null; + let n = 0; + while ((m = re.exec(value))) { + n++; + const target = m[1] ?? m[2] ?? m[3] ?? ''; + if (!unresolve(target).some((c) => checkUrl(c, pol, font))) return false; + } + rest = value.replace(re, ''); + if (/url\s*\(/i.test(rest)) return false; // an url( we could not parse as a plain quoted/bare token + return n > 0; +} -function walk(root: Element, doc: Document, pol: Policy): void { +/** Canonical form of every url() in an accepted value, so the emitted CSS carries the checked targets only. */ +function canonicalize(value: string, pol: Policy, font: boolean): string { + return value.replace(/url\(\s*(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)'|([^"'()\s\\]+))\s*\)/gi, (_m, a, b, c) => { + const target = a ?? b ?? c ?? ''; + for (const cand of unresolve(target)) { + const ok = checkUrl(cand, pol, font); + if (ok) return `url("${ok.replace(/"/g, '')}")`; + } + return 'none'; + }); +} + +function declarations(style: CSSStyleDeclaration, pol: Policy, font = false): string { + const out: string[] = []; + for (let i = 0; i < style.length; i++) { + const prop = style[i]; + if (!prop || /[^A-Za-z0-9_-]/.test(prop) || BAD_PROP.test(prop)) continue; + const value = style.getPropertyValue(prop); + if (!value || !valueOk(value, pol, font || prop === 'src')) continue; + const pri = style.getPropertyPriority(prop) ? ' !important' : ''; + out.push(`${prop}:${canonicalize(value, pol, font || prop === 'src')}${pri}`); + } + return out.join(';'); +} + +function parseSheet(css: string): CSSRuleList | null { + try { + if (typeof CSSStyleSheet !== 'undefined') { + const s = new CSSStyleSheet(); + s.replaceSync(css); + return s.cssRules; + } + } catch { /* fall through to the inert document */ } + try { + const d = document.implementation.createHTMLDocument(''); + const st = d.createElement('style'); + st.textContent = css; + d.head.appendChild(st); + return st.sheet ? st.sheet.cssRules : null; + } catch { return null; } +} + +function rulesToCss(rules: CSSRuleList, pol: Policy): string { + const out: string[] = []; + for (const r of Array.from(rules)) { + switch (r.type) { + case RULE.STYLE: { + const sr = r as CSSStyleRule; + if (/\\/.test(sr.selectorText) || /[<>]/.test(sr.selectorText.replace(/[>+~]/g, ''))) break; + const d = declarations(sr.style, pol); + if (d) out.push(`${sr.selectorText}{${d}}`); + break; + } + case RULE.FONT_FACE: { + const d = declarations((r as CSSFontFaceRule).style, pol, true); + if (/(^|;)src:/.test(d)) out.push(`@font-face{${d}}`); + break; + } + case RULE.MEDIA: { + const mr = r as CSSMediaRule; + const cond = mr.media.mediaText; + if (/[\\{}<]/.test(cond)) break; + const inner = rulesToCss(mr.cssRules, pol); + if (inner) out.push(`@media ${cond}{${inner}}`); + break; + } + case RULE.SUPPORTS: { + const sr = r as CSSRule & { conditionText?: string; cssRules: CSSRuleList }; + const cond = sr.conditionText ?? ''; + if (!cond || /[\\{}<]/.test(cond)) break; + const inner = rulesToCss(sr.cssRules, pol); + if (inner) out.push(`@supports ${cond}{${inner}}`); + break; + } + case RULE.KEYFRAMES: { + const kr = r as CSSKeyframesRule; + if (!SAFE_IDENT.test(kr.name)) break; + const frames: string[] = []; + for (const f of Array.from(kr.cssRules) as CSSKeyframeRule[]) { + if (f.type !== RULE.KEYFRAME || !SAFE_KEY.test(f.keyText)) continue; + const d = declarations(f.style, pol); + if (d) frames.push(`${f.keyText}{${d}}`); + } + if (frames.length) out.push(`@keyframes ${kr.name}{${frames.join('')}}`); + break; + } + default: + // @import, @namespace, @page, @property, @layer, @container, unknown at-rules: dropped. + break; + } + } + return out.join('\n'); +} + +/** Sanitise a stylesheet's text through the CSSOM. Empty string when nothing safe is left. */ +export function cleanCss(css: string, pol: Policy = { allow: new Set() }): string { + const rules = parseSheet(css); + return rules ? rulesToCss(rules, pol) : ''; +} + +/** Sanitise a style="" attribute value; '' when it is not a plain declaration list. */ +export function cleanInlineStyle(value: string, pol: Policy = { allow: new Set() }): string { + const rules = parseSheet(`a{${value}}`); + if (!rules || rules.length !== 1 || rules[0].type !== RULE.STYLE) return ''; + const sr = rules[0] as CSSStyleRule; + if (sr.selectorText.trim() !== 'a') return ''; + return declarations(sr.style, pol); +} + +// ---------------------------------------------------------------- HTML + +export type SanitizeResult = { ok: true; html: string; csp: string } | { ok: false; reason: string }; + +function walk(root: Element, pol: Policy): void { const all = Array.from(root.querySelectorAll('*')); for (const el of all) { - if (!el.isConnected && !root.contains(el)) continue; const tag = el.localName.toLowerCase(); if (REMOVE_TAGS.has(tag)) { el.remove(); continue; } if (tag === 'link') { const rel = (el.getAttribute('rel') || '').toLowerCase(); const href = (el.getAttribute('href') || '').trim(); - if (!(rel === 'stylesheet' && FONT_SHEET_HOSTS.test(href))) { el.remove(); continue; } + if (!(rel === 'stylesheet' && FONT_SHEET_HOSTS.test(href) && !CTRL.test(href) && !href.includes('%2'))) { el.remove(); continue; } for (const a of Array.from(el.attributes)) if (!['rel', 'href', 'crossorigin', 'referrerpolicy'].includes(a.name)) el.removeAttribute(a.name); continue; } @@ -113,18 +266,35 @@ function walk(root: Element, doc: Document, pol: Policy): void { for (const a of Array.from(el.attributes)) { const name = a.name.toLowerCase(); if (name.startsWith('on') || DROP_ATTRS.has(name)) { el.removeAttribute(a.name); continue; } - if (name === 'style') { el.setAttribute(a.name, cleanCss(a.value, pol)); continue; } + if (name === 'style') { + const s = cleanInlineStyle(a.value, pol); + if (s) el.setAttribute(a.name, s); else el.removeAttribute(a.name); + continue; + } if (URL_ATTRS.has(name)) { if (name === 'srcset') { el.removeAttribute(a.name); continue; } // one src is enough for a capture if (name === 'href' && (tag === 'a' || tag === 'area')) { if (!safeLink(a.value)) el.removeAttribute(a.name); continue; } if (name === 'href' && el.namespaceURI === 'http://www.w3.org/2000/svg') { if (!a.value.trim().startsWith('#')) el.removeAttribute(a.name); continue; } - if (!isLocalRef(a.value, pol)) { - if (name === 'src' && tag === 'img') { el.setAttribute('src', BLOCKED_IMAGE); el.removeAttribute('srcset'); } else el.removeAttribute(a.name); - } + const ok = checkUrl(a.value, pol); + if (ok) { if (ok !== a.value) el.setAttribute(a.name, ok); } + else if (name === 'src' && tag === 'img') { el.setAttribute('src', BLOCKED_IMAGE); el.removeAttribute('srcset'); } + else el.removeAttribute(a.name); + continue; + } + // Presentation attributes (fill, filter, mask...) may carry url(): only in-document #refs. + if (/url\s*\(/i.test(a.value) || /[\\]/.test(a.value) && /url/i.test(a.value)) { + const only = Array.from(a.value.matchAll(/url\(\s*["']?([^"')]*)["']?\s*\)/gi)).every((m) => m[1].trim().startsWith('#')); + if (!only || /\\/.test(a.value)) el.removeAttribute(a.name); } } } - void doc; +} + +function safeLink(raw: string): boolean { + const v = raw.replace(/[\s\u0000-\u001f\u007f-\u009f]/g, '').toLowerCase(); + if (/^(javascript|vbscript|data|blob|file):/.test(v)) return false; + if (/^[a-z][a-z0-9+.-]*:/.test(v)) return /^(https?|mailto|tel):/.test(v); + return true; } function core(html: string, pol: Policy): string | null { @@ -132,12 +302,12 @@ function core(html: string, pol: Policy): string | null { try { doc = new DOMParser().parseFromString(html, 'text/html'); } catch { return null; } const root = doc.documentElement; if (!root) return null; - walk(root, doc, pol); + walk(root, pol); return root.outerHTML; } /** Our own head additions, applied last to already-clean markup (trusted constants). */ -function addCaptureHead(clean: string, csp0: string): string | null { +function addCaptureHead(clean: string): string | null { let doc: Document; try { doc = new DOMParser().parseFromString(clean, 'text/html'); } catch { return null; } const root = doc.documentElement; @@ -145,7 +315,7 @@ function addCaptureHead(clean: string, csp0: string): string | null { if (!head) { head = doc.createElement('head'); root.insertBefore(head, root.firstChild); } const csp = doc.createElement('meta'); csp.setAttribute('http-equiv', 'Content-Security-Policy'); - csp.setAttribute('content', csp0); + csp.setAttribute('content', CAPTURE_CSP); head.insertBefore(csp, head.firstChild); const un = doc.createElement('style'); un.setAttribute('data-capture', ''); @@ -157,13 +327,13 @@ function addCaptureHead(clean: string, csp0: string): string | null { /** Strip a model/user-derived page to something safe to render in the script-less capture frame. */ export function sanitizeCaptureHtml(html: string, opts: SanitizeOptions = {}): SanitizeResult { - const pol: Policy = { allow: new Set((opts.allowUrls ?? []).filter((u) => /^\/[^\s"'<>]*$/.test(u))) }; + const pol = makePolicy(opts.allowUrls); if (typeof html !== 'string' || !html.trim()) return { ok: false, reason: 'no page to capture' }; const first = core(html, pol); if (first === null) return { ok: false, reason: 'page could not be parsed' }; // Mutation-XSS guard: what we output must re-parse to itself under the same sanitiser. const second = core(first, pol); if (second === null || second !== first) return { ok: false, reason: 'page markup was not stable under sanitising' }; - const out = addCaptureHead(first, captureCsp(opts.origin)); - return out === null ? { ok: false, reason: 'page could not be parsed' } : { ok: true, html: out, csp: captureCsp(opts.origin) }; + const out = addCaptureHead(first); + return out === null ? { ok: false, reason: 'page could not be parsed' } : { ok: true, html: out, csp: CAPTURE_CSP }; } diff --git a/craft/src/utils/self-review-capture.ts b/craft/src/utils/self-review-capture.ts index d322df6..6398d33 100644 --- a/craft/src/utils/self-review-capture.ts +++ b/craft/src/utils/self-review-capture.ts @@ -13,7 +13,7 @@ * skip the review with a non-blocking note. */ import { toJpeg } from 'html-to-image'; -import { sanitizeCaptureHtml } from './sanitize-capture-html'; +import { sanitizeCaptureHtml, checkUrl, makePolicy, BLOCKED_IMAGE, FONT_FILE_HOSTS, type Policy } from './sanitize-capture-html'; export const FONT_AWESOME_CSS = 'https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.3.1/css/all.min.css'; @@ -76,6 +76,8 @@ export function resolveAssetUrls(html: string, urls: Record): st const MAX_FONT_FACES = 24; const MAX_FONT_BYTES = 2.5 * 1024 * 1024; +const MAX_RESOURCE_BYTES = 4 * 1024 * 1024; +const FETCH_TIMEOUT_MS = 8000; const blobToDataUrl = (b: Blob) => new Promise((resolve, reject) => { const r = new FileReader(); @@ -84,6 +86,29 @@ const blobToDataUrl = (b: Blob) => new Promise((resolve, reject) => { r.readAsDataURL(b); }); +/** + * The ONLY network path the capture uses on a page's behalf. A URL must be one the sanitiser would have + * allowed (exact allow-list entry, assets/ path, or a font file on a named host); no credentials unless it + * is an exact allow-listed (server-provided, same-origin) URL; no cache; no redirects; size and time caps. + */ +export async function fetchBounded(url: string, pol: Policy, maxBytes = MAX_RESOURCE_BYTES): Promise { + const isFont = FONT_FILE_HOSTS.test(url); + if (!isFont && !checkUrl(url, pol)) return null; + const ctl = new AbortController(); + const timer = setTimeout(() => ctl.abort(), FETCH_TIMEOUT_MS); + try { + const res = await fetch(url, { + credentials: !isFont && pol.allow.has(url) ? 'same-origin' : 'omit', + cache: 'no-store', redirect: 'error', signal: ctl.signal, + }); + if (!res.ok) return null; + const len = Number(res.headers.get('content-length') || 0); + if (len > maxBytes) return null; + const blob = await res.blob(); + return blob.size > maxBytes ? null : blob; + } catch { return null; } finally { clearTimeout(timer); } +} + /** True when a @font-face unicode-range could cover basic Latin (or has no range at all). */ function coversLatin(range: string): boolean { if (!range.trim()) return true; @@ -97,12 +122,12 @@ function coversLatin(range: string): boolean { } /** - * Our own web-font embedding. html-to-image's built-in version silently produced - * fallback fonts for Google Fonts, Font Awesome and same-origin @font-face in the - * measurement (see the capture notes), so we inline every readable @font-face as a - * data URL ourselves and hand the result in as `fontEmbedCSS`. + * Our own web-font embedding (html-to-image's built-in version silently produced fallback fonts for + * Google Fonts and Font Awesome in the measurement). Reads the frame's stylesheets (only sanitiser-allowed + * s and our already-sanitised