fix(file-editor): address audit findings without broadening scope

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/bullet-list.test.ts

@@ -50,14 +50,14 @@ describe('typed bullet list joining', () => {
     expect(ed.view.dom.querySelectorAll(':scope > ul')).toHaveLength(1)
   })
 
-  it('joins both neighbors when turning their separating paragraph into a bullet', () => {
+  it('joins the preceding list without merging two existing roots', () => {
     const ed = mount('<ul><li><p>one</p></li></ul><p></p><ul><li><p>two</p></li></ul>')
     ed.commands.setTextSelection(10)
     typeBullet(ed)
     ed.commands.insertContent('new')
 
-    expect(ed.getJSON().content?.filter((node) => node.type === 'bulletList')).toHaveLength(1)
-    expect(ed.getMarkdown().trim()).toBe('- one\n- new\n- two')
+    expect(ed.getJSON().content?.filter((node) => node.type === 'bulletList')).toHaveLength(2)
+    expect(ed.getMarkdown().trim()).toBe('- one\n- new\n\n- two')
   })
 
   it('does not join across an intentional blank paragraph', () => {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx

@@ -184,6 +184,27 @@ afterEach(async () => {
 })
 
 describe('loaded rich editor lifecycle', () => {
+  it('explains a picker selection whose insertion anchor was invalidated', async () => {
+    await render('before TARGET after')
+    const editor = getEditor()
+    await act(async () => {
+      editor.commands.setTextSelection({ from: 8, to: 14 })
+      editor.storage.slashCommand.insertImage?.(8)
+      editor.commands.insertContentAt({ from: 7, to: 15 }, 'changed')
+    })
+    const input = container.querySelector<HTMLInputElement>('input[type="file"]')!
+    Object.defineProperty(input, 'files', {
+      value: [new File(['image'], 'image.png', { type: 'image/png' })],
+    })
+    await act(async () => input.dispatchEvent(new Event('change', { bubbles: true })))
+    expect(uploadFile).not.toHaveBeenCalled()
+    expect(toast.info).toHaveBeenLastCalledWith(
+      'The insertion location changed. Choose a new location and select the image again.'
+    )
+    expect(editor.getText()).toContain('changed')
+    expect(input.value).toBe('')
+  })
+
   it.each(['cancel', 'invalidate'] as const)(
     'explains a completed upload without inserting after its anchor is %s',
     async (action) => {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-upload.test.ts

@@ -1,7 +1,9 @@
 /** @vitest-environment jsdom */
 import { Editor } from '@tiptap/core'
+import Collaboration from '@tiptap/extension-collaboration'
 import { undoDepth } from '@tiptap/pm/history'
 import { afterEach, describe, expect, it } from 'vitest'
+import * as Y from 'yjs'
 import { createMarkdownContentExtensions } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/extensions'
 import {
   beginImageUploads,
@@ -24,6 +26,40 @@ function mount(content = '<p>abcd</p>') {
 }
 
 describe('image upload anchors', () => {
+  it.each([false, true])(
+    'safely cancels an anchor replaced by a peer update (unrelated paragraph=%s)',
+    (unrelated) => {
+      const localDoc = new Y.Doc()
+      const remoteDoc = new Y.Doc()
+      const extensions = (doc: Y.Doc) => [
+        ...createMarkdownContentExtensions({}, { disableHistory: true }),
+        Collaboration.configure({ document: doc }),
+        ImageUploadPlaceholders,
+      ]
+      editor = new Editor({ extensions: extensions(localDoc) })
+      editor.commands.setContent('<p>before TARGET after</p><p>other</p>')
+      Y.applyUpdate(remoteDoc, Y.encodeStateAsUpdate(localDoc))
+      const peer = new Editor({ extensions: extensions(remoteDoc) })
+      try {
+        const [id] = beginImageUploads(editor, { from: 8, to: 14 }, ['image.png'])
+        const position = unrelated ? peer.state.doc.content.size - 1 : 10
+        peer.commands.insertContentAt(position, 'PEER ')
+        Y.applyUpdate(localDoc, Y.encodeStateAsUpdate(remoteDoc))
+        const updated = editor.getJSON()
+        expect(updated).toEqual(peer.getJSON())
+        expect(editor.getText()).toContain('PEER ')
+        expect(findImageUploadRange(editor, id)).toBeNull()
+        expect(finishImageUpload(editor, id, '/image.png', 'image')).toBe(false)
+        expect(editor.getJSON()).toEqual(updated)
+      } finally {
+        peer.destroy()
+        editor.destroy()
+        localDoc.destroy()
+        remoteDoc.destroy()
+      }
+    }
+  )
+
   it('replaces selected text only on success and never serializes the placeholder', () => {
     const editor = mount('<p>before REPLACE after</p>')
     const [id] = beginImageUploads(editor, { from: 8, to: 15 }, ['image.png'])
@@ -122,6 +158,8 @@ describe('image upload anchors', () => {
     expect(finishImageUpload(editor, id, '/image.png', 'image')).toBe(true)
     expect(editor.state.selection.from).toBe(2)
     expect(editor.state.doc.firstChild?.textContent).toBe('PREFIX ab')
+    expect(editor.state.doc.child(1).type.name).toBe('image')
+    expect(editor.state.doc.lastChild?.textContent).toBe('cd')
   })
 
   it('drops uploads whose surrounding content was deleted', () => {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/list-collaboration.test.ts

@@ -16,9 +16,9 @@ interface Peer {
 const cleanups: Array<() => void> = []
 afterEach(() => cleanups.splice(0).forEach((cleanup) => cleanup()))
 
-function pair(): { a: Peer; b: Peer } {
+function pair(content = '## Todos\n\n- one\n- two\n- three', html = false): { a: Peer; b: Peer } {
   document.elementFromPoint ??= () => null
-  const seed = markdownToYDoc('## Todos\n\n- one\n- two\n- three')
+  const seed = markdownToYDoc(html ? '' : content)
   const make = (): Peer => {
     const doc = new Y.Doc()
     Y.applyUpdate(doc, Y.encodeStateAsUpdate(seed))
@@ -41,6 +41,11 @@ function pair(): { a: Peer; b: Peer } {
     return { doc, editor, updates }
   }
   const a = make()
+  if (html) {
+    a.editor.commands.setContent(content)
+    Y.applyUpdate(seed, Y.encodeStateAsUpdate(a.doc))
+    a.updates.length = 0
+  }
   const b = make()
   seed.destroy()
   return { a, b }
@@ -75,6 +80,57 @@ function reconnect(a: Peer, b: Peer, reversed: boolean): void {
 }
 
 describe('list editing with delayed peer updates', () => {
+  describe.each(['bullet', 'ordered', 'task'] as const)('%s list boundaries', (kind) => {
+    it.each(
+      [1, 2].flatMap((beforeCount) =>
+        [false, true].flatMap((nested) =>
+          [false, true].map((reversed) => ({ beforeCount, nested, reversed }))
+        )
+      )
+    )(
+      'retains both existing roots ($beforeCount items, nested=$nested, reversed=$reversed)',
+      ({ beforeCount, nested, reversed }) => {
+        const words = [...['one', 'two'].slice(0, beforeCount), 'three', 'four']
+        const list = (items: string[], start: number) => {
+          const tag = kind === 'ordered' ? 'ol' : 'ul'
+          const attrs =
+            kind === 'task'
+              ? ' data-type="taskList"'
+              : kind === 'ordered'
+                ? ` start="${start}"`
+                : ''
+          const itemAttrs = kind === 'task' ? ' data-type="taskItem" data-checked="false"' : ''
+          return `<${tag}${attrs}>${items.map((word) => `<li${itemAttrs}><p>${word}</p></li>`).join('')}</${tag}>`
+        }
+        const marker = kind === 'ordered' ? `${beforeCount + 1}.` : kind === 'task' ? '[ ]' : '-'
+        const content = `${list(words.slice(0, beforeCount), 1)}<p>${marker}</p>${list(words.slice(beforeCount), beforeCount + 2)}`
+        const { a, b } = pair(nested ? `<ul><li><p>parent</p>${content}</li></ul>` : content, true)
+        for (const word of words) {
+          b.editor.commands.insertContentAt(findText(b.editor, word), 'PEER ')
+        }
+        a.editor.commands.setTextSelection(findText(a.editor, marker) + marker.length)
+        const { from, to } = a.editor.state.selection
+        expect(
+          a.editor.view.someProp('handleTextInput', (handler) =>
+            handler(a.editor.view, from, to, ' ', () => a.editor.state.tr)
+          )
+        ).toBe(true)
+        a.editor.commands.insertContent('new item')
+        const container = nested ? a.editor.state.doc.firstChild!.firstChild! : a.editor.state.doc
+        const listType =
+          kind === 'task' ? 'taskList' : kind === 'ordered' ? 'orderedList' : 'bulletList'
+        const lists = Array.from({ length: container.childCount }, (_, index) =>
+          container.child(index)
+        ).filter((node) => node.type.name === listType)
+        expect(lists.map((node) => node.childCount)).toEqual([beforeCount + 1, 2])
+        expect(a.editor.state.selection.$from.parent.textContent).toBe('new item')
+        reconnect(a, b, reversed)
+        for (const word of words) expect(a.editor.state.doc.textContent).toContain(`PEER ${word}`)
+        expect(a.editor.state.doc.textContent).toContain('new item')
+      }
+    )
+  })
+
   it.each([false, true])(
     'joining above a list retains every peer insertion (reordered=%s)',
     (reversed) => {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/list-input-rules.test.ts

@@ -83,16 +83,16 @@ describe('typed numbered list joining', () => {
     expect(ed.getMarkdown().trim()).toBe(`${start}. new\n${start + 1}. **one**\n${start + 2}. two`)
   })
 
-  it('joins both neighbors only when both numbering boundaries continue', () => {
+  it('joins a preceding continuation without merging two existing roots', () => {
     const ed = mount(
       '<ol start="4"><li><p>one</p></li></ol><p></p><ol start="6"><li><p>three</p></li></ol>'
     )
     selectEmptyParagraph(ed)
     expect(typeMarker(ed, '5.')).toBe(true)
     ed.commands.insertContent('two')
 
-    expect(ed.getJSON().content?.filter((node) => node.type === 'orderedList')).toHaveLength(1)
-    expect(ed.getMarkdown().trim()).toBe('4. one\n5. two\n6. three')
+    expect(ed.getJSON().content?.filter((node) => node.type === 'orderedList')).toHaveLength(2)
+    expect(ed.getMarkdown().trim()).toBe('4. one\n5. two\n\n6) three')
   })
 
   it.each([1, 3, 7])('preserves a following explicit restart at %s', (nextStart) => {
@@ -149,20 +149,17 @@ describe('typed task list joining', () => {
     }
   )
 
-  it('joins checklists on both sides of the new item', () => {
+  it('joins the preceding checklist without merging two existing roots', () => {
     const ed = mount(`${TASKS}<p></p>${TASKS}`)
     selectEmptyParagraph(ed)
     expect(typeMarker(ed, '[ ]')).toBe(true)
     ed.commands.insertContent('new')
 
     const lists = ed.getJSON().content?.filter((node) => node.type === 'taskList')
-    expect(lists).toHaveLength(1)
-    expect(lists?.[0].content?.map((node) => node.attrs?.checked)).toEqual([
-      true,
-      false,
-      false,
-      true,
-      false,
+    expect(lists).toHaveLength(2)
+    expect(lists?.map((list) => list.content?.map((node) => node.attrs?.checked))).toEqual([
+      [true, false, false],
+      [true, false],
     ])
     expect(ed.state.selection.$from.parent.textContent).toBe('new')
   })
@@ -182,19 +179,22 @@ describe('list input-rule boundaries and undo', () => {
   it.each([
     ['5.', '<ol start="4"><li><p>one</p></li></ol>', '<ol start="6"><li><p>three</p></li></ol>'],
     ['[x]', TASKS, TASKS],
-  ])('undoes both adjacent joins together for %s', (marker, before, after) => {
-    const ed = mount(`${before}<p></p>${after}`)
-    const originalBefore = ed.state.doc.child(0).toJSON()
-    const originalAfter = ed.state.doc.child(2).toJSON()
-    selectEmptyParagraph(ed)
-    expect(typeMarker(ed, marker)).toBe(true)
-    expect(ed.commands.undoInputRule()).toBe(true)
+  ])(
+    'undoes the preceding join while retaining the following list for %s',
+    (marker, before, after) => {
+      const ed = mount(`${before}<p></p>${after}`)
+      const originalBefore = ed.state.doc.child(0).toJSON()
+      const originalAfter = ed.state.doc.child(2).toJSON()
+      selectEmptyParagraph(ed)
+      expect(typeMarker(ed, marker)).toBe(true)
+      expect(ed.commands.undoInputRule()).toBe(true)
 
-    expect(ed.state.doc.child(0).toJSON()).toEqual(originalBefore)
-    expect(ed.state.doc.child(1).textContent).toBe(`${marker} `)
-    expect(ed.state.doc.child(2).toJSON()).toEqual(originalAfter)
-    expect(ed.state.selection.$from.parent.textContent).toBe(`${marker} `)
-  })
+      expect(ed.state.doc.child(0).toJSON()).toEqual(originalBefore)
+      expect(ed.state.doc.child(1).textContent).toBe(`${marker} `)
+      expect(ed.state.doc.child(2).toJSON()).toEqual(originalAfter)
+      expect(ed.state.selection.$from.parent.textContent).toBe(`${marker} `)
+    }
+  )
 
   it.each([
     ['1.', '<ol start="2"><li><p>one</p></li></ol>', 'orderedList'],

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/list-input-rules.ts

@@ -47,6 +47,9 @@ export function joinListInputRules(
             }
           }
 
+          /** A backward join now includes an existing root; merging another can lose delayed Yjs edits. */
+          if (list.node.childCount !== 1) return
+
           const after = list.pos + list.node.nodeSize
           const next = tr.doc.nodeAt(after)
           if (next && compatible(list.node, next) && canJoin(tr.doc, after)) tr.join(after)

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-storage.test.ts

@@ -187,8 +187,16 @@ describe('GFM table capabilities', () => {
     expect(editor.commands.toggleHeaderRow()).toBe(false)
     expect(editor.commands.toggleHeaderColumn()).toBe(false)
     expect(editor.commands.toggleHeaderCell()).toBe(false)
-    expect(editor.commands.mergeCells()).toBe(false)
     expect(editor.commands.setCellAttribute('colwidth', [240])).toBe(false)
+    const table = editor.state.doc.firstChild!
+    const firstCell = 2
+    const secondCell = firstCell + table.firstChild!.firstChild!.nodeSize
+    editor.view.dispatch(
+      editor.state.tr.setSelection(CellSelection.create(editor.state.doc, firstCell, secondCell))
+    )
+    expect(editor.state.selection).toBeInstanceOf(CellSelection)
+    expect(editor.can().mergeCells()).toBe(false)
+    expect(editor.commands.mergeCells()).toBe(false)
     expectPreserved(editor)
   })
 

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx

@@ -1379,7 +1379,14 @@ export function LoadedRichMarkdownEditor({
             if (editor && anchor) removeImageUpload(editor, anchor)
             pendingImageAnchorRef.current = null
             input.value = ''
-            if (images.length > 0 && range !== null) void insertImagesRef.current(images, range)
+            if (images.length === 0) return
+            if (range === null) {
+              toast.info(
+                'The insertion location changed. Choose a new location and select the image again.'
+              )
+              return
+            }
+            void insertImagesRef.current(images, range)
           }}
         />
         {showPlaceholder && placeholderContent && (

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-field.test.tsx

@@ -0,0 +1,84 @@
+/** @vitest-environment jsdom */
+import { act } from 'react'
+import { toast } from '@sim/emcn'
+import { PASTE_LIMITS, PASTE_RENDER_THRESHOLDS } from '@sim/utils/paste'
+import type { Editor } from '@tiptap/core'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { RichMarkdownField } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-field'
+
+vi.mock(
+  '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention',
+  () => ({
+    useEditorMentions: vi.fn(),
+  })
+)
+vi.mock(
+  '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/bubble-menu',
+  () => ({
+    EditorBubbleMenu: () => null,
+  })
+)
+vi.mock(
+  '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/menus/link-hover-card',
+  () => ({
+    LinkHoverCard: () => null,
+  })
+)
+
+let root: Root
+let container: HTMLDivElement
+beforeEach(() => {
+  Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true })
+  vi.spyOn(toast, 'warning').mockReturnValue('paste-warning')
+  container = document.createElement('div')
+  document.body.append(container)
+  root = createRoot(container)
+})
+afterEach(async () => {
+  await act(async () => root.unmount())
+  container.remove()
+  vi.restoreAllMocks()
+})
+
+describe('shared field paste admission with real extensions', () => {
+  it.each([false, true])('counts preserved frontmatter (near limit=%s)', async (nearLimit) => {
+    const frontmatter = `---\ndescription: ${'f'.repeat(nearLimit ? 900 : 10)}\n---\n\n`
+    const body = 'x'.repeat(PASTE_RENDER_THRESHOLDS.ENHANCED_TEXT_CHARACTERS - 1000)
+    const onChange = vi.fn()
+    await act(async () =>
+      root.render(<RichMarkdownField value={frontmatter + body} onChange={onChange} />)
+    )
+    const element = container.querySelector<HTMLElement & { editor: Editor }>('.tiptap')
+    expect(element).not.toBeNull()
+    const editor = element!.editor
+    await act(async () => editor.commands.setTextSelection(editor.state.doc.content.size - 1))
+    const before = editor.getJSON()
+    const event = new Event('paste', { bubbles: true, cancelable: true })
+    Object.defineProperty(event, 'clipboardData', {
+      value: {
+        files: [],
+        items: [],
+        types: ['text/plain'],
+        getData: (type: string) => (type === 'text/plain' ? 'y'.repeat(100) : ''),
+      },
+    })
+    await act(async () => element!.dispatchEvent(event))
+    if (nearLimit) {
+      expect(editor.getJSON()).toEqual(before)
+      expect(onChange).not.toHaveBeenCalled()
+      expect(toast.warning).toHaveBeenCalledOnce()
+    } else {
+      expect(onChange).toHaveBeenCalledOnce()
+      expect(onChange.mock.lastCall?.[0]).toContain(frontmatter.trim())
+      expect(new TextEncoder().encode(onChange.mock.lastCall?.[0]).byteLength).toBeLessThanOrEqual(
+        PASTE_LIMITS.RICH_MARKDOWN_BYTES
+      )
+      expect(onChange.mock.lastCall?.[0].length).toBeLessThanOrEqual(
+        PASTE_RENDER_THRESHOLDS.ENHANCED_TEXT_CHARACTERS
+      )
+      expect(editor.getText()).toBe(body + 'y'.repeat(100))
+      expect(toast.warning).not.toHaveBeenCalled()
+    }
+  })
+})