diff --git a/.changeset/editor-bug-hunt-batch.md b/.changeset/editor-bug-hunt-batch.md new file mode 100644 index 00000000..c28163a0 --- /dev/null +++ b/.changeset/editor-bug-hunt-batch.md @@ -0,0 +1,17 @@ +--- +"@templatical/core": patch +"@templatical/editor": patch +--- + +Batch of bug fixes hardening editor correctness and security: + +- **Link dialog rejects dangerous URL schemes.** `javascript:`, `data:`, `vbscript:`, `file:` (plus case-bypasses like `JaVaScRiPt:` and whitespace-padded variants) are now dropped at link-insert time. Safe schemes (`http`, `https`, `mailto`, `tel`, `ftp`, `ftps`, `sms`, `xmpp`, `cid`) and `#` anchors still pass through. +- **`v-html` content sanitized before render.** `ParagraphBlock` and `TitleBlock` now scrub `

after

', + styles: {}, + } as any, + viewport: "desktop", + }, + global: { provide: baseProvide() }, + }); + + const html = wrapper.html(); + expect(html).not.toContain(" { + const wrapper = mount(ParagraphBlock, { + props: { + block: { + id: "p-1", + type: "paragraph", + content: '

click

', + styles: {}, + } as any, + viewport: "desktop", + }, + global: { provide: baseProvide() }, + }); + + const html = wrapper.html(); + expect(html).not.toContain("javascript:"); + expect(html).toContain("click"); + }); + + it("ParagraphBlock preserves safe HTML (bold, italic, https links)", () => { + const wrapper = mount(ParagraphBlock, { + props: { + block: { + id: "p-1", + type: "paragraph", + content: + '

bold italic link

', + styles: {}, + } as any, + viewport: "desktop", + }, + global: { provide: baseProvide() }, + }); + + const html = wrapper.html(); + expect(html).toContain("bold"); + expect(html).toContain("italic"); + expect(html).toContain('href="https://example.com"'); + }); + + it("TitleBlock strips inline event handlers from block.content", () => { + const wrapper = mount(TitleBlock, { + props: { + block: { + id: "t-1", + type: "title", + content: '

Hello world

', + level: 1, + styles: {}, + } as any, + viewport: "desktop", + }, + global: { provide: baseProvide() }, + }); + + const html = wrapper.html(); + expect(html).not.toContain("onmouseover"); + expect(html).not.toContain("__pwn"); + expect(html).toContain("Hello"); + expect(html).toContain("world"); + }); + + it("TitleBlock removes javascript: anchor hrefs", () => { + const wrapper = mount(TitleBlock, { + props: { + block: { + id: "t-1", + type: "title", + content: '

x

', + level: 2, + styles: {}, + } as any, + viewport: "desktop", + }, + global: { provide: baseProvide() }, + }); + + const html = wrapper.html(); + expect(html).not.toContain("javascript:"); + }); +}); diff --git a/packages/editor/tests/mediaPickerAliveGuard.test.ts b/packages/editor/tests/mediaPickerAliveGuard.test.ts new file mode 100644 index 00000000..1295a114 --- /dev/null +++ b/packages/editor/tests/mediaPickerAliveGuard.test.ts @@ -0,0 +1,204 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from "vitest"; +import { nextTick } from "vue"; +import { mount } from "@vue/test-utils"; +import { SYNTAX_PRESETS } from "@templatical/types"; +import enTranslations from "../src/i18n/locales/en"; +import { + MERGE_TAGS_KEY, + MERGE_TAG_SYNTAX_KEY, + ON_REQUEST_MEDIA_KEY, + TRANSLATIONS_KEY, +} from "../src/keys"; +import ImageBlock from "../src/components/blocks/ImageBlock.vue"; +import ImageToolbar from "../src/components/toolbar/ImageToolbar.vue"; +import VideoToolbar from "../src/components/toolbar/VideoToolbar.vue"; +import ImageField from "../src/components/toolbar/fields/ImageField.vue"; + +interface MediaResolver { + resolveWithObservable: () => { urlReadAfterResolve: () => boolean }; +} + +/** + * Builds an `onRequestMedia` stub that returns a controllable Promise. + * Resolving it provides a media object whose `url` getter sets a flag — + * this lets the test observe whether the caller's post-`await` branch + * runs (Vue silently no-ops `emit` on an unmounted instance, so checking + * `wrapper.emitted()` would mask the bug). + */ +function setupOnRequestMedia(): { + fn: ReturnType; + next: () => MediaResolver; +} { + const queue: Array<(value: { url: string; alt?: string } | null) => void> = []; + const fn = vi.fn( + () => + new Promise<{ url: string; alt?: string } | null>((resolve) => { + queue.push(resolve); + }), + ); + return { + fn, + next: () => { + const resolve = queue.shift(); + if (!resolve) throw new Error("no pending media request"); + return { + resolveWithObservable: () => { + let urlRead = false; + const result = { + get url() { + urlRead = true; + return "after-unmount.png"; + }, + alt: "Late", + }; + resolve(result); + return { urlReadAfterResolve: () => urlRead }; + }, + }; + }, + }; +} + +function baseProvide(onRequestMedia: unknown) { + return { + [ON_REQUEST_MEDIA_KEY as symbol]: onRequestMedia, + [TRANSLATIONS_KEY as symbol]: enTranslations, + [MERGE_TAG_SYNTAX_KEY as symbol]: SYNTAX_PRESETS.liquid, + [MERGE_TAGS_KEY as symbol]: [], + }; +} + +describe("media picker alive guard", () => { + it("ImageBlock does not emit update after the component unmounts", async () => { + const media = setupOnRequestMedia(); + const wrapper = mount(ImageBlock, { + props: { + block: { + id: "img-1", + type: "image", + src: "", + alt: "", + width: "full", + align: "center", + linkUrl: "", + placeholderUrl: "", + styles: {}, + } as any, + viewport: "desktop", + }, + global: { + provide: baseProvide(media.fn), + }, + }); + + const button = wrapper.find("button"); + expect(button.exists()).toBe(true); + await button.trigger("click"); + expect(media.fn).toHaveBeenCalled(); + + wrapper.unmount(); + const observable = media.next().resolveWithObservable(); + await nextTick(); + + expect(observable.urlReadAfterResolve()).toBe(false); + }); + + it("ImageToolbar does not emit update after the component unmounts", async () => { + const media = setupOnRequestMedia(); + const wrapper = mount(ImageToolbar, { + props: { + block: { + id: "img-1", + type: "image", + src: "https://example.com/initial.png", + alt: "", + width: "full", + align: "center", + linkUrl: "", + placeholderUrl: "", + styles: {}, + } as any, + }, + global: { + provide: baseProvide(media.fn), + }, + }); + + const browseButton = wrapper + .findAll("button") + .find((b) => b.text().includes(enTranslations.image.browseMedia)); + expect(browseButton).toBeTruthy(); + await browseButton!.trigger("click"); + expect(media.fn).toHaveBeenCalled(); + + wrapper.unmount(); + const observable = media.next().resolveWithObservable(); + await nextTick(); + + expect(observable.urlReadAfterResolve()).toBe(false); + }); + + it("VideoToolbar does not emit update after the component unmounts", async () => { + const media = setupOnRequestMedia(); + const wrapper = mount(VideoToolbar, { + props: { + block: { + id: "vid-1", + type: "video", + url: "https://example.com/video.mp4", + thumbnailUrl: "", + width: "full", + align: "center", + styles: {}, + } as any, + }, + global: { + provide: baseProvide(media.fn), + }, + }); + + const browseButton = wrapper + .findAll("button") + .find((b) => b.text().includes(enTranslations.image.browseMedia)); + expect(browseButton).toBeTruthy(); + await browseButton!.trigger("click"); + expect(media.fn).toHaveBeenCalled(); + + wrapper.unmount(); + const observable = media.next().resolveWithObservable(); + await nextTick(); + + expect(observable.urlReadAfterResolve()).toBe(false); + }); + + it("ImageField does not emit update:modelValue after the component unmounts", async () => { + const media = setupOnRequestMedia(); + const wrapper = mount(ImageField, { + props: { + field: { + key: "hero", + type: "image", + label: "Hero image", + } as any, + modelValue: "", + }, + global: { + provide: baseProvide(media.fn), + }, + }); + + const browseButton = wrapper + .findAll("button") + .find((b) => b.text().includes(enTranslations.image.browseMedia)); + expect(browseButton).toBeTruthy(); + await browseButton!.trigger("click"); + expect(media.fn).toHaveBeenCalled(); + + wrapper.unmount(); + const observable = media.next().resolveWithObservable(); + await nextTick(); + + expect(observable.urlReadAfterResolve()).toBe(false); + }); +}); diff --git a/packages/editor/tests/mergeTagSuggestion.test.ts b/packages/editor/tests/mergeTagSuggestion.test.ts index 109922d5..b5111340 100644 --- a/packages/editor/tests/mergeTagSuggestion.test.ts +++ b/packages/editor/tests/mergeTagSuggestion.test.ts @@ -230,6 +230,29 @@ describe('MergeTagSuggestion mount target', () => { expect(src).toContain('.dom'); expect(src).not.toContain('editor.options.element'); }); + + // Regression: repositionAfterPaint queues a requestAnimationFrame whose + // callback runs after onExit has cleared `container` and `app`. The null + // checks inside position() prevent a crash, but the closure still pins + // the unmounted Vue app and torn-down DOM nodes for one frame. onExit + // must cancel the pending rAF. + it('source stores rAF handle and cancels it in onExit', async () => { + const fs = await import('node:fs'); + const src = fs.readFileSync( + 'src/extensions/MergeTagSuggestion.ts', + 'utf8', + ); + expect(src).toContain('cancelAnimationFrame'); + // The cancellation must live inside the onExit handler — not just + // anywhere in the file. Slice from onExit to the next `},` at the + // same indentation level. + const onExitStart = src.indexOf('onExit:'); + expect(onExitStart).toBeGreaterThan(-1); + const onExitEnd = src.indexOf('},', onExitStart); + expect(onExitEnd).toBeGreaterThan(onExitStart); + const onExitBody = src.slice(onExitStart, onExitEnd); + expect(onExitBody).toContain('cancelAnimationFrame'); + }); }); describe('MergeTagSuggestion extension ordering in editors', () => { diff --git a/packages/editor/tests/sanitizeRichTextHtml.test.ts b/packages/editor/tests/sanitizeRichTextHtml.test.ts new file mode 100644 index 00000000..2a5725a8 --- /dev/null +++ b/packages/editor/tests/sanitizeRichTextHtml.test.ts @@ -0,0 +1,126 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from "vitest"; +import { sanitizeRichTextHtml } from "../src/utils/sanitizeRichTextHtml"; + +describe("sanitizeRichTextHtml", () => { + it("removes inline event handler attributes from arbitrary tags", () => { + const result = sanitizeRichTextHtml( + '', + ); + expect(result).not.toContain("onerror"); + expect(result).toContain('src="x.png"'); + }); + + it("strips

World

", + ); + expect(result).not.toContain(" and

ok

", + ); + expect(result).not.toContain("ok

"); + }); + + it("removes href with javascript: scheme", () => { + const result = sanitizeRichTextHtml( + 'click', + ); + expect(result).not.toContain("javascript:"); + expect(result).toContain("click"); + }); + + it("removes href with vbscript: scheme", () => { + const result = sanitizeRichTextHtml( + 'click', + ); + expect(result).not.toContain("vbscript:"); + }); + + it("keeps safe http/https/mailto/tel hrefs intact", () => { + expect(sanitizeRichTextHtml('x')).toContain( + 'href="https://example.com"', + ); + expect(sanitizeRichTextHtml('x')).toContain( + 'href="mailto:x@y.com"', + ); + expect(sanitizeRichTextHtml('x')).toContain( + 'href="tel:+1234"', + ); + }); + + it("keeps anchor (#) hrefs intact", () => { + expect(sanitizeRichTextHtml('x')).toContain( + 'href="#section"', + ); + }); + + it("removes src with javascript: scheme", () => { + const result = sanitizeRichTextHtml(''); + expect(result).not.toContain("javascript:"); + }); + + it("removes non-image data: src", () => { + const result = sanitizeRichTextHtml( + '', + ); + expect(result).not.toContain("data:text/html"); + }); + + it("keeps data:image/* src intact", () => { + const result = sanitizeRichTextHtml( + '', + ); + expect(result).toContain("data:image/png;base64,iVBORw0KGgo="); + }); + + it("rejects case-bypassed JAVASCRIPT: scheme", () => { + const result = sanitizeRichTextHtml('x'); + expect(result).not.toMatch(/JaVaScRiPt/i); + }); + + it("removes srcdoc attribute (defensive iframe-strip backup)", () => { + //