diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts index d782141e81c..fff5f035a81 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts @@ -14,8 +14,10 @@ import { extractImgSrcs, findHostedImageAttrs, hasHostedImageHtml, + htmlReferencesSrc, isInlineRouteSrc, shouldSkipFileUpload, + toSameOriginPath, } from './image-paste' // jsdom lacks `elementFromPoint`; the Placeholder extension's viewport tracking calls it on mount. @@ -288,3 +290,97 @@ describe('findHostedImageAttrs', () => { expect(originalAlt).toBe('photo') }) }) + +describe('origin-aware src normalization (browser-native drag/copy enrichment uses ABSOLUTE urls)', () => { + const ORIGIN = 'https://www.staging.sim.ai' + + it('toSameOriginPath: relative passes through; same-origin absolute → path+query; cross-origin → null', () => { + expect(toSameOriginPath('/api/files/view/wf_a', ORIGIN)).toBe('/api/files/view/wf_a') + expect(toSameOriginPath(`${ORIGIN}/api/workspaces/ws-1/files/inline?fileId=wf_a`, ORIGIN)).toBe( + '/api/workspaces/ws-1/files/inline?fileId=wf_a' + ) + expect(toSameOriginPath('https://evil.example.com/api/files/view/wf_a', ORIGIN)).toBeNull() + }) + + it('isInlineRouteSrc accepts the same-origin ABSOLUTE inline route (the real dragged-img src on staging)', () => { + expect(isInlineRouteSrc(`${ORIGIN}/api/workspaces/ws-1/files/inline?fileId=wf_a`, ORIGIN)).toBe( + true + ) + expect( + isInlineRouteSrc( + 'https://evil.example.com/api/workspaces/ws-1/files/inline?fileId=wf_a', + ORIGIN + ) + ).toBe(false) + }) + + it('hasHostedImageHtml matches absolute same-origin srcs and rejects cross-origin ones', () => { + const isHosted = (src: string) => extractEmbeddedFileRef(src) !== null + expect(hasHostedImageHtml(``, isHosted, ORIGIN)).toBe( + true + ) + expect( + hasHostedImageHtml( + ``, + isHosted, + ORIGIN + ) + ).toBe(true) + expect( + hasHostedImageHtml( + '', + isHosted, + ORIGIN + ) + ).toBe(false) + }) + + it('findHostedImageAttrs matches when the clipboard html carries the absolute rendered url', () => { + const ws = createWorkspaceFileContentSource('ws-1') + const editor = new Editor({ + extensions: createMarkdownEditorExtensions({ placeholder: '' }), + content: { + type: 'doc', + content: [{ type: 'image', attrs: { src: '/api/files/view/wf_abc', alt: 'photo' } }], + }, + }) + const absolute = `${ORIGIN}/api/workspaces/ws-1/files/inline?fileId=wf_abc` + const match = findHostedImageAttrs(editor.state.doc, [absolute], ws.resolveImageSrc, ORIGIN) + expect(match?.src).toBe('/api/files/view/wf_abc') + }) +}) + +describe('htmlReferencesSrc (the "this drop is MY dragged image" check)', () => { + const ORIGIN = 'https://www.staging.sim.ai' + const RESOLVED = '/api/workspaces/ws-1/files/inline?fileId=wf_a' + + it('true when the html img src is the absolute form of the resolved src', () => { + expect(htmlReferencesSrc(``, RESOLVED, ORIGIN)).toBe(true) + }) + + it('true for the relative form too (ProseMirror-serialized html)', () => { + expect(htmlReferencesSrc(``, RESOLVED, ORIGIN)).toBe(true) + }) + + // Identity must work for cross-origin srcs too: a doc's image may legitimately point at a CDN or + // badge URL, and dragging THAT node to reorder it must still match — under the previous + // same-origin-path comparison it fell into the duplicate-upload branch instead. + it('matches an external (cross-origin) image against its own exact absolute url', () => { + const EXTERNAL = 'https://cdn.example.com/badge.svg' + expect(htmlReferencesSrc(``, EXTERNAL, ORIGIN)).toBe(true) + expect( + htmlReferencesSrc('', EXTERNAL, ORIGIN) + ).toBe(false) + }) + + it('false for a different image, empty html, missing resolved src, or cross-origin', () => { + expect(htmlReferencesSrc(``, RESOLVED, ORIGIN)).toBe( + false + ) + expect(htmlReferencesSrc('', RESOLVED, ORIGIN)).toBe(false) + expect(htmlReferencesSrc(``, undefined, ORIGIN)).toBe(false) + expect( + htmlReferencesSrc(``, RESOLVED, ORIGIN) + ).toBe(false) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts index d3bc170f317..c3edc938661 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts @@ -13,27 +13,63 @@ export function extractImageFiles(transfer: DataTransfer | null): File[] { .filter((file): file is File => file !== null) } -// `src` may be double-quoted, single-quoted, or (validly) unquoted per the HTML spec — the browser's -// own clipboard serialization always quotes it, but other producers of `text/html` are not obligated -// to. +/** + * Matches `` `src` attribute values: double-quoted, single-quoted, or (validly) unquoted per + * the HTML spec — the browser's own clipboard serialization always quotes it, but other producers + * of `text/html` are not obligated to. + */ const IMG_SRC_RE = /]*\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'>]+))/gi + +/** Query params under which the inline route addresses a workspace file. */ const INLINE_ROUTE_QUERY_KEYS = new Set(['key', 'fileId']) +/** + * The page's own origin — clipboard/dataTransfer srcs on any other origin are never ours. + * Deliberately `window.location.origin`, NOT `getBaseUrl()`: the browser serializes a dragged/copied + * ``'s URL against the origin the page is ACTUALLY being viewed on, which can legitimately + * differ from the configured `NEXT_PUBLIC_APP_URL` (localhost dev against a shared env, preview + * deploys, apex-vs-www) — comparing against the configured URL would silently fail on any such + * origin, which is the exact bug class this normalization exists to fix. + */ +function runtimeOrigin(): string { + return typeof window === 'undefined' ? '' : window.location.origin +} + +/** + * Normalizes a clipboard/dataTransfer img `src` to an origin-relative `pathname?query`, or `null` + * when it belongs to a different origin. The browser's NATIVE drag/copy enrichment (dragging or + * "Copy Image" on a rendered ``) serializes the ABSOLUTE resolved URL — + * `https://host/api/workspaces/…/inline?…` — while everything the app compares against (persisted + * refs, `resolveImageSrc` output) is origin-relative, so both must be brought into the same space + * before comparing. A cross-origin src must never be treated as ours. + */ +export function toSameOriginPath(src: string, origin = runtimeOrigin()): string | null { + try { + const base = origin || 'http://placeholder' + const parsed = new URL(src, base) + if (parsed.origin !== base) return null + return parsed.pathname + parsed.search + } catch { + return null + } +} + /** * True for the *display-layer* inline route `resolveImageSrc` (see `use-file-content-source.tsx`) * rewrites an embed to — workspace-scoped `/api/workspaces/{workspaceId}/files/inline?key=…`/`?fileId=…` * or public-share-scoped `/api/files/public/{token}/inline?key=…`/`?fileId=…`. This is the shape * actually rendered into ``, and so what a same-page copy's `text/html` clipboard payload - * actually contains — NOT the raw stored reference `extractEmbeddedFileRef` (in - * `@/lib/uploads/utils/embedded-image-ref`) recognizes, which only matches the persisted `src` before - * that rewrite. Checked separately from (rather than folded into) `extractEmbeddedFileRef` since that - * helper is shared with server-side authorization/export code operating on persisted content, where - * this display-only shape should never legitimately appear. + * actually contains (absolute — see {@link toSameOriginPath}) — NOT the raw stored reference + * `extractEmbeddedFileRef` (in `@/lib/uploads/utils/embedded-image-ref`) recognizes, which only + * matches the persisted `src` before that rewrite. Checked separately from (rather than folded into) + * `extractEmbeddedFileRef` since that helper is shared with server-side authorization/export code + * operating on persisted content, where this display-only shape should never legitimately appear. */ -export function isInlineRouteSrc(src: string): boolean { +export function isInlineRouteSrc(src: string, origin = runtimeOrigin()): boolean { + const path = toSameOriginPath(src, origin) + if (path === null) return false try { - const parsed = new URL(src, 'http://placeholder') - if (parsed.origin !== 'http://placeholder') return false + const parsed = new URL(path, 'http://placeholder') if (!parsed.pathname.endsWith('/inline')) return false for (const key of parsed.searchParams.keys()) { if (INLINE_ROUTE_QUERY_KEYS.has(key)) return true @@ -62,9 +98,53 @@ export function extractImgSrcs(html: string): string[] { * select it) makes the browser put BOTH `text/html` (the real serialized node, with its real hosted * `src`) AND a synthesized image `File` onto the clipboard — the same "drag a web image out" behavior * that {@link extractImageFiles} alone can't tell apart from a genuinely new external image paste. + * Srcs are normalized origin-relative first ({@link toSameOriginPath}): ProseMirror's own clipboard + * serialization writes the persisted relative src, but the BROWSER's native enrichment writes the + * absolute resolved URL. */ -export function hasHostedImageHtml(html: string, isHostedRef: (src: string) => boolean): boolean { - return extractImgSrcs(html).some((src) => isHostedRef(src) || isInlineRouteSrc(src)) +export function hasHostedImageHtml( + html: string, + isHostedRef: (src: string) => boolean, + origin = runtimeOrigin() +): boolean { + return extractImgSrcs(html).some((src) => { + const path = toSameOriginPath(src, origin) + return path !== null && (isHostedRef(path) || isInlineRouteSrc(path, origin)) + }) +} + +/** Resolves `src` to a full absolute URL against `origin`, or `null` when unparseable. */ +function toAbsoluteUrl(src: string, origin: string): string | null { + try { + return new URL(src, origin || 'http://placeholder').href + } catch { + return null + } +} + +/** + * True when `html` contains an `` whose src resolves to the same ABSOLUTE URL as + * `resolvedSrc` (a `resolveImageSrc` output for a node already in this document). This is the + * "that drop is MY dragged image" check for internal drag-reorder: TipTap's node-view dragstart + * bypasses ProseMirror's serialization entirely (no PM `text/html`, no `view.dragging`) but + * NodeSelects the dragged image, and the browser's native enrichment carries the absolute rendered + * URL of exactly that node. + * + * Deliberately compares full absolute URLs rather than same-origin paths ({@link toSameOriginPath}): + * identity is the question here, not hosted-by-us membership — a doc's image may legitimately have a + * cross-origin `src` (a README badge, a CDN image), and dragging THAT node to reorder it must match + * too, or it falls into the duplicate-upload path. Relative and absolute spellings of the same URL + * still compare equal because both sides resolve against the page origin. + */ +export function htmlReferencesSrc( + html: string, + resolvedSrc: string | undefined, + origin = runtimeOrigin() +): boolean { + if (!html || !resolvedSrc) return false + const target = toAbsoluteUrl(resolvedSrc, origin) + if (target === null) return false + return extractImgSrcs(html).some((src) => toAbsoluteUrl(src, origin) === target) } /** @@ -108,20 +188,26 @@ interface DescendantsDoc { * the clipboard/dataTransfer `html`, whose `src` is `resolveImageSrc`'s rewritten *display* URL, not the * real persisted one. Inserting a node built from that display URL would bake it into the document, * which public share/export/referenced-by-doc tracking don't recognize (they only match the persisted - * shape) — this lookup avoids ever constructing such a node in the first place. + * shape) — this lookup avoids ever constructing such a node in the first place. Both sides of the + * comparison are normalized origin-relative ({@link toSameOriginPath}): the clipboard html may carry + * the browser's absolute URLs. */ export function findHostedImageAttrs( doc: DescendantsDoc, targetSrcs: string[], - resolveImageSrc: (src: string | undefined) => string | undefined + resolveImageSrc: (src: string | undefined) => string | undefined, + origin = runtimeOrigin() ): Record | null { - const targets = new Set(targetSrcs) + const targets = new Set( + targetSrcs.map((src) => toSameOriginPath(src, origin)).filter((p): p is string => p !== null) + ) let found: Record | null = null doc.descendants((node) => { if (found) return false if (node.type.name === 'image') { const resolved = resolveImageSrc(node.attrs.src as string | undefined) - if (resolved && targets.has(resolved)) { + const resolvedPath = resolved ? toSameOriginPath(resolved, origin) : null + if (resolvedPath && targets.has(resolvedPath)) { found = { ...node.attrs } return false } diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index 5057c835af9..9107dbd795d 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -3,6 +3,9 @@ import { memo, useEffect, useRef, useState } from 'react' import { cn, toast } from '@sim/emcn' import type { JSONContent } from '@tiptap/core' +import { Fragment, Slice } from '@tiptap/pm/model' +import { NodeSelection } from '@tiptap/pm/state' +import { dropPoint } from '@tiptap/pm/transform' import type { Editor } from '@tiptap/react' import { EditorContent, useEditor } from '@tiptap/react' import { useRouter } from 'next/navigation' @@ -19,6 +22,7 @@ import { extractImageFiles, extractImgSrcs, findHostedImageAttrs, + htmlReferencesSrc, shouldSkipFileUpload, } from './image-paste' import { @@ -216,6 +220,8 @@ export function LoadedRichMarkdownEditor({ const uploadFile = useUploadWorkspaceFile() const editorInstanceRef = useRef(null) const source = useFileContentSource() + const resolveImageSrcRef = useRef(source.resolveImageSrc) + resolveImageSrcRef.current = source.resolveImageSrc /** * The `/Image` slash command opens this hidden picker; `pendingImagePosRef` holds the caret position @@ -367,22 +373,58 @@ export function LoadedRichMarkdownEditor({ * the browser doesn't navigate away from the editor; internal text drags carry no files and fall * through to the default behavior. * - * Dragging an existing image node to reorder it is also an internal drag, but the browser's - * native drag-and-drop synthesizes an image `File` into `event.dataTransfer` for a dragged `` - * (the same mechanism that lets a user drag a web image out to their desktop) — indistinguishable - * from a real external drop by `dataTransfer` files/items alone. When the accompanying `text/html` - * shows it's a same-page drag of an already-hosted image, bail out and let ProseMirror's own - * default move logic run — it relocates the actual existing node object (`dragging.node`), never - * re-parsing html, so (unlike the paste case above) there's no display-vs-persisted src risk here. - * Checked from `html`, not `view.dragging`, so a stale `dragging` flag (ProseMirror clears it up - * to ~50ms late via `dragend` when a prior internal drag was dropped outside this view) can never - * suppress the plain-file swallow guard below for an unrelated drop that happens to land in that - * window — this only reacts to what THIS specific drop's `html` actually contains. + * Drag-REORDER of an image node is the deceptive case. TipTap's node-view dragstart bypasses + * ProseMirror's own drag serialization entirely — no PM `text/html`, no `view.dragging` — but it + * DOES NodeSelect the dragged image; what the drop carries instead is the browser's native + * enrichment for a dragged ``: an image `File` plus `text/html` whose src is the ABSOLUTE + * rendered URL of that exact node. So when the drop's html points at the currently-selected image + * node ({@link htmlReferencesSrc}), this drop IS that node being moved, and the move must be + * performed here: uploading would duplicate it (the original never moves), and falling through to + * ProseMirror is no better — with `view.dragging` unset its default drop PARSES the html into a + * copy (persisting the display-layer src, which share/export tracking don't recognize) and never + * deletes the original. The gate accepts at most one file (not exactly one): some drag transports + * (e.g. CDP-driven input) carry the html alone, and a genuinely external drop can never reference + * the currently-selected node's own resolved src. + * + * The move itself is the same shape as ProseMirror's own: compute the drop point on the + * pre-delete doc, delete the source, map the insert position through that delete. A null + * `dropPoint` (no valid insertion point) is a handled no-op — the node stays put, still + * selected — never a raw-position fallback, which `tr.insert` could throw on (PM's own null + * fallback is only safe because it uses the forgiving `replaceRangeWith`). + * + * PM-serialized drags (a text selection spanning an image, dragged from a textblock) still reach + * the `shouldSkipFileUpload` bail below: PM set `view.dragging` for those itself, so its default + * move logic is correct there. */ handleDrop: (view, event) => { if (!view.editable) return false const images = extractImageFiles(event.dataTransfer) const html = event.dataTransfer?.getData('text/html') ?? '' + const { selection } = view.state + if ( + images.length <= 1 && + selection instanceof NodeSelection && + selection.node.type.name === 'image' && + htmlReferencesSrc(html, resolveImageSrcRef.current(selection.node.attrs.src)) + ) { + event.preventDefault() + const coords = view.posAtCoords({ left: event.clientX, top: event.clientY }) + if (!coords) return true + const node = selection.node + const tr = view.state.tr + const insertPos = dropPoint( + view.state.doc, + coords.pos, + new Slice(Fragment.from(node), 0, 0) + ) + if (insertPos === null) return true + tr.delete(selection.from, selection.to) + const mapped = tr.mapping.map(insertPos) + tr.insert(mapped, node) + tr.setSelection(NodeSelection.create(tr.doc, mapped)) + view.dispatch(tr.scrollIntoView()) + return true + } if (shouldSkipFileUpload(images, html, (src) => extractEmbeddedFileRef(src) !== null)) { return false }