diff --git a/craft/package.json b/craft/package.json index 1e910b9..56282f4 100644 --- a/craft/package.json +++ b/craft/package.json @@ -17,6 +17,7 @@ "test:browser:colorpick": "node scripts/e2e-color-pick.mjs", "test:browser:colors": "node scripts/e2e-color-system.mjs", "test:browser:sitesmith": "node scripts/e2e-sitesmith.mjs", + "test:browser:sitesmith-cleaned": "node scripts/e2e-sitesmith-cleaned.mjs", "test:unit:watch": "vitest" }, "dependencies": { diff --git a/craft/scripts/e2e-sitesmith-cleaned-entry.ts b/craft/scripts/e2e-sitesmith-cleaned-entry.ts new file mode 100644 index 0000000..aab9431 --- /dev/null +++ b/craft/scripts/e2e-sitesmith-cleaned-entry.ts @@ -0,0 +1,31 @@ +// Bundled (esbuild) by scripts/e2e-sitesmith-cleaned.mjs: turns one ordinary Sitesmith response into a +// published-style page, with or without the apply-time cleaner, through the real exporter. +import { cleanAiResponse } from '../src/sitesmith/clean-ai-response'; +import { flattenTreeForCraft } from '../src/utils/craft-tree'; +import { exportToHtml } from '../src/utils/html-export'; +import { DEFAULT_SITE_DESIGN } from '../src/state/SiteDesignContext'; +import type { SitesmithResponse } from '../src/types/sitesmith'; + +const n = (type: string, props: Record, nodes?: any[]) => ({ type: { resolvedName: type }, props, ...(nodes ? { nodes } : {}) }); + +export const RESPONSE: SitesmithResponse = { + type: 'replace', scope: 'page', message: 'Built a page.', + pages: [{ name: 'Home', tree: n('Container', { node_id: 'ROOT', tag: 'div' }, [ + n('Navbar', { node_id: 'nav', logoType: 'text', logoText: 'Brand', logoUrl: '/', links: [{ text: 'Home', href: '/' }, { text: 'Old link', href: 'javascript:void(0)' }] }), + n('Heading', { node_id: 'h', text: 'Fresh bread every morning', level: 'h1' }), + n('ImageBlock', { node_id: 'i1', src: 'assets/hero.png', alt: 'Our bakery' }), + n('ImageBlock', { node_id: 'i2', src: 'https://example.com/photo.png', alt: 'Off-list host' }), + n('ImageBlock', { node_id: 'i3', src: 'https://placehold.co/600x400?text=Team', alt: 'Placeholder' }), + n('Container', { node_id: 'c', tag: 'div', style: { backgroundImage: 'url(https://example.com/bg.png)', padding: '24px' } }, [ + n('ButtonLink', { node_id: 'b', text: 'Book a table', href: 'https://example.com/book' }), + ]), + n('VideoBlock', { node_id: 'v', videoUrl: 'https://example.com/clip.mp4' }), + ]) }], +}; + +export function render(clean: boolean): string { + const resp = clean ? cleanAiResponse(RESPONSE) : RESPONSE; + if (resp.type !== 'replace') throw new Error('unexpected'); + const flat = flattenTreeForCraft(resp.pages[0].tree); + return exportToHtml(JSON.stringify(flat.nodes), { title: 'Home', includeFonts: true, design: DEFAULT_SITE_DESIGN }).html; +} diff --git a/craft/scripts/e2e-sitesmith-cleaned.mjs b/craft/scripts/e2e-sitesmith-cleaned.mjs new file mode 100644 index 0000000..f9865ac --- /dev/null +++ b/craft/scripts/e2e-sitesmith-cleaned.mjs @@ -0,0 +1,82 @@ +#!/usr/bin/env node +/** + * Real-browser check (Firefox AND Chromium, headless) for the Sitesmith apply-time cleaner (G8): a page + * published from a CLEANED model response loads nothing outside the allow-listed hosts and carries no + * inline event handlers or javascript: URLs. The same response UNCLEANED is loaded as a control, to + * show the check can see an off-list request. Nothing leaves the machine: every request is answered + * or aborted by page.route (off-site requests are only recorded). + * npm run test:browser:sitesmith-cleaned Env: CHROMIUM_PATH / FIREFOX_PATH + */ +import { chromium, firefox } from '@playwright/test'; +import { build as esbuild } from 'esbuild'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const out = await esbuild({ + entryPoints: [path.join(root, 'scripts/e2e-sitesmith-cleaned-entry.ts')], + bundle: true, write: false, format: 'esm', platform: 'node', jsx: 'automatic', logLevel: 'error', + define: { 'process.env.NODE_ENV': '"production"', __EDITOR_BUILD__: '"test"' }, + banner: { js: "import { createRequire } from 'node:module'; const require = createRequire(import.meta.url);" }, +}); +const tmp = path.join(os.tmpdir(), `wsb-ss-cleaned-${process.pid}.mjs`); +fs.writeFileSync(tmp, out.outputFiles[0].text); +const ent = await import(tmp); +fs.rmSync(tmp, { force: true }); + +const SITE = 'http://site.test'; +// The published page's own head resources (fonts, Font Awesome) + the cleaner's image hosts. +const ALLOWED_HOSTS = new Set(['site.test', 'fonts.googleapis.com', 'fonts.gstatic.com', 'cdnjs.cloudflare.com', 'images.pexels.com', 'placehold.co']); +const PNG = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', 'base64'); + +let pass = 0; let fail = 0; +const ok = (c, m, d = '') => { if (c) { pass++; console.log('PASS ', m); } else { fail++; console.log('FAIL ', m, d); } }; + +async function load(browser, html) { + const ctx = await browser.newContext(); + const page = await ctx.newPage(); + const hosts = new Set(); + await page.route('**/*', async (route) => { + const u = new URL(route.request().url()); + hosts.add(u.hostname); + if (u.origin === SITE) { + if (u.pathname === '/' || u.pathname === '/index.html') return route.fulfill({ status: 200, contentType: 'text/html', body: html }); + if (u.pathname.startsWith('/assets/')) return route.fulfill({ status: 200, contentType: 'image/png', body: PNG }); + return route.fulfill({ status: 404, body: '' }); + } + return route.abort(); // never contact a real host + }); + await page.goto(SITE + '/', { waitUntil: 'load' }); + await page.waitForTimeout(300); + const dom = await page.evaluate(() => { + const handlers = []; + const jsUrls = []; + for (const el of document.querySelectorAll('*')) { + for (const a of el.attributes) { + if (/^on/i.test(a.name)) handlers.push(`${el.tagName}.${a.name}`); + if (['href', 'src', 'action', 'formaction', 'poster', 'data'].includes(a.name) && /^\s*(javascript|vbscript|data):/i.test(a.value)) jsUrls.push(`${el.tagName}.${a.name}`); + } + } + return { handlers, jsUrls, text: document.body.innerText }; + }); + await ctx.close(); + return { hosts: [...hosts].sort(), dom }; +} + +for (const [name, launcher, exe] of [['chromium', chromium, process.env.CHROMIUM_PATH], ['firefox', firefox, process.env.FIREFOX_PATH]]) { + const browser = await launcher.launch({ executablePath: exe || undefined, ...(name === 'chromium' ? { args: ['--no-sandbox'] } : {}) }); + const control = await load(browser, ent.render(false)); + ok(control.hosts.includes('example.com'), `[${name}] control (uncleaned): the check sees the off-list request to example.com`, JSON.stringify(control.hosts)); + const cleaned = await load(browser, ent.render(true)); + const offList = cleaned.hosts.filter((h) => !ALLOWED_HOSTS.has(h)); + ok(offList.length === 0, `[${name}] cleaned: no request outside the allow-listed hosts`, JSON.stringify(cleaned.hosts)); + ok(cleaned.hosts.includes('site.test'), `[${name}] cleaned: the site's own asset was requested`, JSON.stringify(cleaned.hosts)); + ok(cleaned.dom.handlers.length === 0, `[${name}] cleaned: no inline event handlers`, JSON.stringify(cleaned.dom.handlers)); + ok(cleaned.dom.jsUrls.length === 0, `[${name}] cleaned: no javascript:/data: URLs in URL attributes`, JSON.stringify(cleaned.dom.jsUrls)); + ok(/Fresh bread every morning/.test(cleaned.dom.text) && /Book a table/.test(cleaned.dom.text), `[${name}] cleaned: the ordinary content is all there`); + await browser.close(); +} +console.log(fail ? `\nFAILED (${fail})` : `\nALL PASS (${pass})`); +process.exit(fail ? 1 : 0); diff --git a/craft/scripts/e2e-sitesmith-mock-server.mjs b/craft/scripts/e2e-sitesmith-mock-server.mjs index 248cb1a..2dd4d37 100644 --- a/craft/scripts/e2e-sitesmith-mock-server.mjs +++ b/craft/scripts/e2e-sitesmith-mock-server.mjs @@ -288,6 +288,7 @@ export async function createMockServer(o) { s.refCalls.push({ action, body }); refs = refs.filter((x) => x.id !== body.id); return json(res, 200, { ok: true }); + case 'review_render_upload': case 'ref_image_upload': { const parts = parseMultipart(raw, ct); const fields = Object.fromEntries(parts.filter((p) => p.filename === null).map((p) => [p.name, p.data.toString('utf8')])); @@ -298,7 +299,7 @@ export async function createMockServer(o) { const info = sniff(file.data); if (!info) return json(res, 415, { ok: false, error: 'unsupported_type', message: 'That file is not a picture Sitesmith can use. Use a JPEG, PNG, WebP or GIF.' }); const id = crypto.randomBytes(16).toString('hex'); - const meta = { id, kind: fields.kind === 'screenshot' ? 'screenshot' : 'mockup', name: file.filename || 'image', width: info.width, height: info.height, size: file.data.length, created_at: new Date().toISOString() }; + const meta = { id, kind: action === 'review_render_upload' ? 'review_render' : fields.kind === 'screenshot' ? 'screenshot' : 'mockup', name: file.filename || 'image', width: info.width, height: info.height, size: file.data.length, created_at: new Date().toISOString() }; images.set(id, { meta, bytes: file.data, mime: info.mime }); s.uploads[s.uploads.length - 1].id = id; s.uploads[s.uploads.length - 1].mime = info.mime; diff --git a/craft/scripts/e2e-sitesmith.mjs b/craft/scripts/e2e-sitesmith.mjs index 0a4ce80..926bfca 100644 --- a/craft/scripts/e2e-sitesmith.mjs +++ b/craft/scripts/e2e-sitesmith.mjs @@ -320,8 +320,8 @@ scenario('8', 'self-review round trip (one round)', async ({ page, name, ok }) = ok('patch from the second job applied', true); ok('two start jobs', srv.starts.length === 2, String(srv.starts.length)); const up = srv.uploads[0]; - ok('render uploaded as a JPEG mockup named self-review-render.jpg, > 2000 bytes', - !!up && up.mime === 'image/jpeg' && up.kind === 'mockup' && up.name === 'self-review-render.jpg' && up.size > 2000, JSON.stringify(up)); + ok('render uploaded through review_render_upload (kind review_render) as a JPEG named self-review-render.jpg, > 2000 bytes', + !!up && up.mime === 'image/jpeg' && up.kind === 'review_render' && up.name === 'self-review-render.jpg' && up.size > 2000, JSON.stringify(up)); const s2 = srv.starts[1] ?? {}; ok('second job message starts "Review the attached screenshot"', typeof s2.message === 'string' && s2.message.startsWith('Review the attached screenshot'), String(s2.message).slice(0, 60)); ok('second job is intent "review" carrying the upload id in review_image_ids (not mockup_ids)', s2.intent === 'review' && Array.isArray(s2.review_image_ids) && s2.review_image_ids.includes(up?.id) && !s2.mockup_ids, JSON.stringify({ intent: s2.intent, review_image_ids: s2.review_image_ids, mockup_ids: s2.mockup_ids })); diff --git a/craft/src/hooks/useSitesmith.ts b/craft/src/hooks/useSitesmith.ts index 2a86450..0c8986c 100644 --- a/craft/src/hooks/useSitesmith.ts +++ b/craft/src/hooks/useSitesmith.ts @@ -35,7 +35,7 @@ export interface SitesmithReference { export interface SitesmithRefImage { id: string; - kind: 'mockup' | 'screenshot'; + kind: 'mockup' | 'screenshot' | 'review_render'; name: string; width: number | null; height: number | null; @@ -490,6 +490,13 @@ export function useSitesmith(siteId: number) { form.append('file', file); return call<{ image: SitesmithRefImage }>('ref_image_upload', { post: true, form }); }, [call, siteId]); + /** The page render for ONE self-review job (backend kind 'review_render': never listed, deleted when the job ends). */ + const uploadReviewRender = useCallback((file: File | Blob) => { + const form = new FormData(); + form.append('site_id', String(siteId)); + form.append('file', file); + return call<{ image: SitesmithRefImage }>('review_render_upload', { post: true, form }); + }, [call, siteId]); const deleteRefImage = useCallback((id: string) => call<{}>('ref_image_delete', { post: true, json: { site_id: siteId, id } }), [call, siteId]); const refImageUrl = useCallback((id: string): string => @@ -501,6 +508,6 @@ export function useSitesmith(siteId: number) { summary, messages, loading, error, send, refreshEntitlement, clearHistory, activeJob, resumeJob, cancel, capabilities, capabilitiesFailed, listRefs, addReference, updateReference, deleteReference, - uploadRefImage, deleteRefImage, refImageUrl, listAssets, + uploadRefImage, uploadReviewRender, deleteRefImage, refImageUrl, listAssets, }; } diff --git a/craft/src/panels/sitesmith/SitesmithModal.flows.test.tsx b/craft/src/panels/sitesmith/SitesmithModal.flows.test.tsx index 25af50b..3979b85 100644 --- a/craft/src/panels/sitesmith/SitesmithModal.flows.test.tsx +++ b/craft/src/panels/sitesmith/SitesmithModal.flows.test.tsx @@ -52,7 +52,7 @@ function installFetch() { if (url.includes('action=history')) return json(net.history); if (url.includes('action=refs_list')) return json({ ok: true, references: net.refs, images: [] }); if (url.includes('action=assets')) return json({ ok: true, assets: net.assets }); - if (url.includes('action=ref_image_upload')) { net.uploads++; return net.uploadFail ? json({ ok: false, error: 'x', message: 'Upload failed' }, 500) : json({ ok: true, image: { id: 'f'.repeat(32), kind: 'mockup', name: 'self-review-render.jpg', width: 1280, height: 900, size: 5000 } }); } + if (url.includes('action=review_render_upload')) { net.uploads++; return net.uploadFail ? json({ ok: false, error: 'x', message: 'Upload failed' }, 500) : json({ ok: true, image: { id: 'f'.repeat(32), kind: 'mockup', name: 'self-review-render.jpg', width: 1280, height: 900, size: 5000 } }); } if (url.includes('action=ref_image_delete')) { net.deleted.push(JSON.parse(init.body).id); return json({ ok: true }); } return json({ ok: true, messages: [] }); })); diff --git a/craft/src/panels/sitesmith/SitesmithModal.tsx b/craft/src/panels/sitesmith/SitesmithModal.tsx index dccbfa8..7e2d731 100644 --- a/craft/src/panels/sitesmith/SitesmithModal.tsx +++ b/craft/src/panels/sitesmith/SitesmithModal.tsx @@ -198,7 +198,7 @@ export const SitesmithModal: React.FC = ({ onClose, target }) => { resolveAssetUrls, capture: (html, o) => captureHtml(html, o), upload: async (file) => { - const r = await ss.uploadRefImage(file, 'mockup'); + const r = await ss.uploadReviewRender(file); return r.ok ? { ok: true, id: r.image.id } : { ok: false, message: r.message }; }, review: (msg, imageId) => send(msg, summarizeCanvas(query.getSerializedNodes()), undefined, { diff --git a/craft/src/utils/canvas-summary.lines.test.ts b/craft/src/utils/canvas-summary.lines.test.ts new file mode 100644 index 0000000..798f7e8 --- /dev/null +++ b/craft/src/utils/canvas-summary.lines.test.ts @@ -0,0 +1,16 @@ +import { describe, test, expect } from 'vitest'; +import { summarizeCanvas } from './canvas-summary'; + +describe('summarizeCanvas: one line per node (self-review summaries are validated line by line)', () => { + test('newlines in text and quotes in names never split or break a line', () => { + const state: any = { + ROOT: { type: { resolvedName: 'Container' }, props: { tag: 'div' }, nodes: ['a'] }, + a: { type: { resolvedName: 'TextBlock' }, props: { aiName: 'Say "hi"', node_id: 'ai-t1', text: 'Line one\nLine two\r\nthree' }, nodes: [] }, + }; + const lines = summarizeCanvas(state).split('\n'); + expect(lines).toHaveLength(2); + expect(lines[1]).toBe(` - TextBlock id=ai-t1 name="Say 'hi'" {text=Line one Line two three}`); + const lineRe = /^(?: {2})*- [A-Za-z][A-Za-z0-9]{0,40} id=([A-Za-z0-9_-]{1,64}) name="[^"\n]{0,120}" \{.{0,400}\}$/u; + for (const l of lines) expect(l).toMatch(lineRe); + }); +}); diff --git a/craft/src/utils/canvas-summary.ts b/craft/src/utils/canvas-summary.ts index 1a3487e..9985ff7 100644 --- a/craft/src/utils/canvas-summary.ts +++ b/craft/src/utils/canvas-summary.ts @@ -10,14 +10,17 @@ export function summarizeCanvas(state: SerializedNodes, maxChars = 6000): string const indent = ' '.repeat(depth); const type = typeof node.type === 'object' ? (node.type as any).resolvedName : String(node.type); const props = node.props || {}; - const aiName = (props as any).aiName ?? ''; + // One line per node, always: the server validates review summaries line by line + // (api/sitesmith.php sitesmith_review_summary_node_ids), so no newline or quote may leak in. + const flat = (t: string) => t.replace(/[\u0000-\u001f\u007f\u2028\u2029]+/g, ' '); + const aiName = flat(String((props as any).aiName ?? '')).replace(/"/g, "'").slice(0, 100); const nodeId = (props as any).node_id ?? id; const interesting: string[] = []; for (const [k, v] of Object.entries(props)) { // `html` (#46) is markup for the plain `text` mirror -- summarise text only. if (k === 'aiName' || k === 'node_id' || k === 'style' || k === 'html') continue; if (v == null) continue; - const repr = typeof v === 'string' ? v : JSON.stringify(v); + const repr = flat(typeof v === 'string' ? v : JSON.stringify(v)); const truncated = repr.length > 60 ? repr.slice(0, 57) + '…' : repr; interesting.push(`${k}=${truncated}`); if (interesting.length >= 3) break;