Batch of bug fixes hardening editor correctness and security

.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 `<script>`/`<style>`/`<iframe>`/`on*` event handlers and unsafe `href` / `src` schemes from `block.content` before binding it to `v-html`. Closes the XSS path where a malicious or compromised template JSON could execute code on canvas load. TipTap-authored content (the common case) is unaffected.
+- **Block duplication regenerates nested IDs.** Cloning a `table`, `social`, or `menu` block previously reused identical `rows[].id` / `cells[].id` / `icons[].id` / `items[].id` from the source, violating the unique-id invariant.
+- **Removing a section clears descendant selection.** Previously, deleting an ancestor with a child selected left `selectedBlockId` dangling on the now-orphan id. The full subtree is walked on remove and selection is cleared if any descendant id matches.
+- **`addBlock` / `moveBlock` validate `columnIndex` against the section layout.** Passing `columnIndex: 5` on a `"2"`-layout section no longer creates phantom columns persisted into JSON; out-of-range indices are rejected and `moveBlock` leaves the source intact.
+- **Media-picker callers guard against post-unmount writes.** `ImageBlock`, `ImageToolbar`, `VideoToolbar`, and the custom-block `ImageField` now check an alive flag after `await onRequestMedia()`. Closing the editor mid-pick no longer triggers zombie `emit("update")` / pulse-ref writes on a torn-down component.
+- **Keyboard shortcuts scoped to the active editor when two are mounted.** Each `useEditorCore` instance previously installed its own `document` keydown listener, so a single `Cmd+Z` fired both editors' undo handlers. The new `activeEditorTracker` routes shortcuts to the editor the user most recently interacted with (single-editor pages keep the original always-active behavior).
+- **`MergeTagSuggestion` cancels its pending `requestAnimationFrame` on exit.** The reposition-after-paint frame previously ran after the popup tore down, pinning the Vue app and DOM nodes for one frame.
+- **`useMergeTagField.insertMergeTag` no longer emits after the host component unmounts.** A scope-dispose flag now gates the post-`await requestMergeTag()` writes (emit + `isEditing` + `nextTick`).
+- **`useFonts.loadCustomFonts` no longer flips `isLoaded` after dispose.** The post-`Promise.allSettled` write is gated by the same scope-dispose flag.

packages/core/src/block-actions.ts

@@ -1,6 +1,20 @@
 import type { Block, BlockDefaults, BlockType } from "@templatical/types";
 import { createBlock, generateId } from "@templatical/types";
 
+function regenerateNestedIds(block: Block): void {
+  if (block.type === "table") {
+    block.rows = block.rows.map((row) => ({
+      ...row,
+      id: generateId(),
+      cells: row.cells.map((cell) => ({ ...cell, id: generateId() })),
+    }));
+  } else if (block.type === "social") {
+    block.icons = block.icons.map((icon) => ({ ...icon, id: generateId() }));
+  } else if (block.type === "menu") {
+    block.items = block.items.map((item) => ({ ...item, id: generateId() }));
+  }
+}
+
 export interface UseBlockActionsOptions {
   addBlock: (
     block: Block,
@@ -64,12 +78,14 @@ export function useBlockActions(
   ): Block {
     const cloned = JSON.parse(JSON.stringify(block)) as Block;
     cloned.id = generateId();
+    regenerateNestedIds(cloned);
 
     if (cloned.type === "section") {
       cloned.children = cloned.children.map((column) =>
         column.map((child) => {
           const clonedChild = JSON.parse(JSON.stringify(child)) as Block;
           clonedChild.id = generateId();
+          regenerateNestedIds(clonedChild);
           return clonedChild;
         }),
       );

packages/core/src/editor.ts

@@ -1,12 +1,19 @@
 import type {
   Block,
+  ColumnLayout,
   TemplateContent,
   TemplateDefaults,
   TemplateSettings,
   UiTheme,
   ViewportSize,
 } from "@templatical/types";
 import { createDefaultTemplateContent } from "@templatical/types";
+
+function getColumnCount(layout: ColumnLayout): number {
+  if (layout === "1") return 1;
+  if (layout === "3") return 3;
+  return 2;
+}
 import {
   computed,
   reactive,
@@ -108,6 +115,17 @@ export function useEditor(options: UseEditorOptions): UseEditorReturn {
     return null;
   }
 
+  function collectBlockIds(block: Block, ids: Set<string>): void {
+    ids.add(block.id);
+    if (block.type === "section") {
+      for (const column of block.children) {
+        for (const child of column) {
+          collectBlockIds(child, ids);
+        }
+      }
+    }
+  }
+
   function findBlockParent(
     blocks: Block[],
     id: string,
@@ -223,6 +241,9 @@ export function useEditor(options: UseEditorOptions): UseEditorReturn {
       }
       const section = findBlockById(state.content.blocks, targetSectionId);
       if (section && section.type === "section") {
+        if (columnIndex < 0 || columnIndex >= getColumnCount(section.columns)) {
+          return;
+        }
         section.children[columnIndex] = section.children[columnIndex] || [];
         const targetArray = section.children[columnIndex];
         if (index !== undefined && index < targetArray.length) {
@@ -249,9 +270,13 @@ export function useEditor(options: UseEditorOptions): UseEditorReturn {
     if (parent) {
       const index = parent.blocks.findIndex((b) => b.id === blockId);
       if (index !== -1) {
-        parent.blocks.splice(index, 1);
-        if (state.selectedBlockId === blockId) {
-          state.selectedBlockId = null;
+        const [removed] = parent.blocks.splice(index, 1);
+        if (state.selectedBlockId) {
+          const removedIds = new Set<string>();
+          collectBlockIds(removed, removedIds);
+          if (removedIds.has(state.selectedBlockId)) {
+            state.selectedBlockId = null;
+          }
         }
         state.isDirty = true;
       }
@@ -283,6 +308,9 @@ export function useEditor(options: UseEditorOptions): UseEditorReturn {
     if (targetSectionId) {
       const section = findBlockById(state.content.blocks, targetSectionId);
       if (!section || section.type !== "section") return;
+      if (columnIndex < 0 || columnIndex >= getColumnCount(section.columns)) {
+        return;
+      }
       section.children[columnIndex] = section.children[columnIndex] || [];
       targetArray = section.children[columnIndex];
     } else {

packages/core/tests/block-actions.test.ts

@@ -1,10 +1,16 @@
 import { describe, expect, it, vi } from 'vitest';
 import {
     createDefaultTemplateContent,
+    createMenuBlock,
     createParagraphBlock,
     createSectionBlock,
+    createSocialIconsBlock,
+    createTableBlock,
     createTitleBlock,
     type BlockDefaults,
+    type MenuBlock,
+    type SocialIconsBlock,
+    type TableBlock,
 } from '@templatical/types';
 import { useBlockActions, useEditor } from '../src';
 
@@ -159,6 +165,75 @@ describe('useBlockActions', () => {
         actions.duplicateBlock(original);
         expect(original.id).toBe(originalId);
     });
+
+    it('duplicates table block with unique row and cell IDs', () => {
+        const opts = createMockOptions();
+        const actions = useBlockActions(opts);
+        const original = createTableBlock();
+        const sourceRowIds = original.rows.map((r) => r.id);
+        const sourceCellIds = original.rows.flatMap((r) => r.cells.map((c) => c.id));
+
+        const cloned = actions.duplicateBlock(original) as TableBlock;
+        expect(cloned.type).toBe('table');
+
+        const clonedRowIds = cloned.rows.map((r) => r.id);
+        const clonedCellIds = cloned.rows.flatMap((r) => r.cells.map((c) => c.id));
+
+        for (const id of clonedRowIds) {
+            expect(sourceRowIds).not.toContain(id);
+        }
+        for (const id of clonedCellIds) {
+            expect(sourceCellIds).not.toContain(id);
+        }
+        // Cloned IDs unique among themselves
+        expect(new Set(clonedRowIds).size).toBe(clonedRowIds.length);
+        expect(new Set(clonedCellIds).size).toBe(clonedCellIds.length);
+        // Cell content preserved
+        expect(cloned.rows[0].cells[0].content).toBe(original.rows[0].cells[0].content);
+    });
+
+    it('duplicates social icons block with unique icon IDs', () => {
+        const opts = createMockOptions();
+        const actions = useBlockActions(opts);
+        const original = createSocialIconsBlock({
+            icons: [
+                { id: 'icon-1', platform: 'twitter', url: 'https://twitter.com/x' },
+                { id: 'icon-2', platform: 'facebook', url: 'https://facebook.com/x' },
+            ],
+        });
+
+        const cloned = actions.duplicateBlock(original) as SocialIconsBlock;
+        expect(cloned.type).toBe('social');
+        expect(cloned.icons).toHaveLength(2);
+
+        expect(cloned.icons[0].id).not.toBe('icon-1');
+        expect(cloned.icons[1].id).not.toBe('icon-2');
+        expect(cloned.icons[0].id).not.toBe(cloned.icons[1].id);
+        // Non-id fields preserved
+        expect(cloned.icons[0].platform).toBe('twitter');
+        expect(cloned.icons[1].url).toBe('https://facebook.com/x');
+    });
+
+    it('duplicates menu block with unique item IDs', () => {
+        const opts = createMockOptions();
+        const actions = useBlockActions(opts);
+        const original = createMenuBlock({
+            items: [
+                { id: 'item-1', label: 'Home', url: 'https://example.com' },
+                { id: 'item-2', label: 'About', url: 'https://example.com/about' },
+            ],
+        });
+
+        const cloned = actions.duplicateBlock(original) as MenuBlock;
+        expect(cloned.type).toBe('menu');
+        expect(cloned.items).toHaveLength(2);
+
+        expect(cloned.items[0].id).not.toBe('item-1');
+        expect(cloned.items[1].id).not.toBe('item-2');
+        expect(cloned.items[0].id).not.toBe(cloned.items[1].id);
+        expect(cloned.items[0].label).toBe('Home');
+        expect(cloned.items[1].label).toBe('About');
+    });
 });
 
 describe('useBlockActions duplicate inserts after source', () => {

packages/core/tests/editor.test.ts

@@ -156,6 +156,45 @@ describe('useEditor', () => {
         expect(editor.state.selectedBlockId).toBeNull();
     });
 
+    it('clears selection when removing a section whose child is selected', () => {
+        const content = createDefaultTemplateContent();
+        const child = createParagraphBlock({ content: '<p>Child</p>' });
+        const section = createSectionBlock({ children: [[child]] });
+        content.blocks = [section];
+        const editor = useEditor({ content });
+        editor.selectBlock(child.id);
+        expect(editor.state.selectedBlockId).toBe(child.id);
+
+        editor.removeBlock(section.id);
+        expect(editor.state.content.blocks).toHaveLength(0);
+        expect(editor.state.selectedBlockId).toBeNull();
+    });
+
+    it('clears selection when removing a deeply nested ancestor', () => {
+        const content = createDefaultTemplateContent();
+        const leaf = createParagraphBlock({ content: '<p>Leaf</p>' });
+        const innerSection = createSectionBlock({ children: [[leaf]] });
+        const outerSection = createSectionBlock({ children: [[innerSection]] });
+        content.blocks = [outerSection];
+        const editor = useEditor({ content });
+        editor.selectBlock(leaf.id);
+
+        editor.removeBlock(outerSection.id);
+        expect(editor.state.selectedBlockId).toBeNull();
+    });
+
+    it('does not clear selection when removing an unrelated block', () => {
+        const content = createDefaultTemplateContent();
+        const a = createParagraphBlock({ content: '<p>A</p>' });
+        const b = createParagraphBlock({ content: '<p>B</p>' });
+        content.blocks = [a, b];
+        const editor = useEditor({ content });
+        editor.selectBlock(a.id);
+
+        editor.removeBlock(b.id);
+        expect(editor.state.selectedBlockId).toBe(a.id);
+    });
+
     it('moves block to new position', () => {
         const content = createDefaultTemplateContent();
         const block1 = createParagraphBlock({ content: '<p>First</p>' });
@@ -581,9 +620,10 @@ describe('moveBlock into section column', () => {
         const content = createDefaultTemplateContent();
         const rootBlock = createParagraphBlock({ content: '<p>Root</p>' });
         const section = createSectionBlock({
+            columns: '2',
             children: [[]],
         });
-        // Clear column to simulate undefined
+        // Clear column to simulate undefined within the declared layout
         (section as any).children[1] = undefined;
         content.blocks = [rootBlock, section];
         const editor = useEditor({ content });
@@ -599,6 +639,98 @@ describe('moveBlock into section column', () => {
     });
 });
 
+describe('column index validation', () => {
+    it('addBlock rejects out-of-range columnIndex for "1" layout section', () => {
+        const content = createDefaultTemplateContent();
+        const section = createSectionBlock({ columns: '1', children: [[]] });
+        content.blocks = [section];
+        const editor = useEditor({ content });
+
+        const added = createParagraphBlock({ content: '<p>X</p>' });
+        editor.addBlock(added, section.id, 5);
+
+        const sec = editor.state.content.blocks[0];
+        expect(sec.type).toBe('section');
+        if (sec.type === 'section') {
+            expect(sec.children.length).toBe(1);
+        }
+    });
+
+    it('addBlock rejects out-of-range columnIndex for "2" layout section', () => {
+        const content = createDefaultTemplateContent();
+        const section = createSectionBlock({ columns: '2', children: [[], []] });
+        content.blocks = [section];
+        const editor = useEditor({ content });
+
+        const added = createParagraphBlock({ content: '<p>X</p>' });
+        editor.addBlock(added, section.id, 2);
+
+        const sec = editor.state.content.blocks[0];
+        if (sec.type === 'section') {
+            expect(sec.children.length).toBe(2);
+            expect(sec.children[0]).toHaveLength(0);
+            expect(sec.children[1]).toHaveLength(0);
+        }
+    });
+
+    it('addBlock accepts max valid columnIndex for "3" layout section', () => {
+        const content = createDefaultTemplateContent();
+        const section = createSectionBlock({
+            columns: '3',
+            children: [[], [], []],
+        });
+        content.blocks = [section];
+        const editor = useEditor({ content });
+
+        const added = createParagraphBlock({ content: '<p>X</p>' });
+        editor.addBlock(added, section.id, 2);
+
+        const sec = editor.state.content.blocks[0];
+        if (sec.type === 'section') {
+            expect(sec.children[2]).toHaveLength(1);
+            expect(sec.children[2][0].id).toBe(added.id);
+        }
+    });
+
+    it('addBlock accepts columnIndex 1 for "2-1" layout section', () => {
+        const content = createDefaultTemplateContent();
+        const section = createSectionBlock({
+            columns: '2-1',
+            children: [[], []],
+        });
+        content.blocks = [section];
+        const editor = useEditor({ content });
+
+        const added = createParagraphBlock({ content: '<p>X</p>' });
+        editor.addBlock(added, section.id, 1);
+
+        const sec = editor.state.content.blocks[0];
+        if (sec.type === 'section') {
+            expect(sec.children.length).toBe(2);
+            expect(sec.children[1]).toHaveLength(1);
+        }
+    });
+
+    it('moveBlock rejects out-of-range columnIndex and leaves source intact', () => {
+        const content = createDefaultTemplateContent();
+        const rootBlock = createParagraphBlock({ content: '<p>Root</p>' });
+        const section = createSectionBlock({ columns: '2', children: [[], []] });
+        content.blocks = [rootBlock, section];
+        const editor = useEditor({ content });
+
+        editor.moveBlock(rootBlock.id, 0, section.id, 7);
+
+        expect(editor.state.content.blocks).toHaveLength(2);
+        expect(editor.state.content.blocks[0].id).toBe(rootBlock.id);
+        const sec = editor.state.content.blocks[1];
+        if (sec.type === 'section') {
+            expect(sec.children.length).toBe(2);
+            expect(sec.children[0]).toHaveLength(0);
+            expect(sec.children[1]).toHaveLength(0);
+        }
+    });
+});
+
 describe('findBlockLocation', () => {
     it('returns location for a top-level block with no section parent', () => {
         const content = createDefaultTemplateContent();