diff --git a/apps/app/src/lib/foreign-dom-mutation-guard.test.tsx b/apps/app/src/lib/foreign-dom-mutation-guard.test.tsx index 245618be00..85330efb67 100644 --- a/apps/app/src/lib/foreign-dom-mutation-guard.test.tsx +++ b/apps/app/src/lib/foreign-dom-mutation-guard.test.tsx @@ -5,6 +5,9 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { foreignDomMutationCount, installForeignDomMutationGuard, + pluginHostNodeMoveRefusalCount, + runWithPluginDomIsolation, + runWithPluginDomIsolationAsync, uninstallForeignDomMutationGuardForTest, } from "./foreign-dom-mutation-guard"; @@ -70,7 +73,6 @@ describe("foreign DOM mutation guard", () => { expect(foreignDomMutationCount()).toBe(1); }); - it("suppresses the removeChild that a foreign reparent turns into a throw", () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); const reactParent = document.createElement("div"); @@ -122,4 +124,245 @@ describe("foreign DOM mutation guard", () => { expect(foreignDomMutationCount()).toBe(0); expect(warn).not.toHaveBeenCalled(); }); + + it("suppresses replaceChild when the node it would replace has moved away", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const parent = document.createElement("div"); + const oldChild = document.createElement("span"); + parent.appendChild(oldChild); + installForeignDomMutationGuard(); + document.createElement("font").appendChild(oldChild); + + const replacement = document.createElement("b"); + expect(() => parent.replaceChild(replacement, oldChild)).not.toThrow(); + expect(replacement.parentNode).toBe(parent); + expect(foreignDomMutationCount()).toBe(1); + }); + + /** + * The File Reveal plugin wraps a host control in a span and appends a + * sibling button. The crash happens when the wrapped node is itself the + * host child React removes or reorders — a sidebar row button, not a + * nested link inside a still-mounted list item. + */ + function wrapLikeFileReveal(control: HTMLElement): HTMLElement { + const parent = control.parentNode; + if (parent === null) throw new Error("control has no parent"); + const group = document.createElement("span"); + const button = document.createElement("button"); + button.type = "button"; + parent.insertBefore(group, control); + group.append(control, button); + return group; + } + + function removeListItemAfterFileRevealWrap(): Error[] { + const errors: Error[] = []; + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container, { + onUncaughtError: (error) => { + errors.push(error instanceof Error ? error : new Error(String(error))); + }, + }); + + function List({ items }: { items: string[] }) { + return ( +
+ {items.map((item) => ( + + ))} +
+ ); + } + + const run = (work: () => void): void => { + try { + act(work); + } catch (error) { + errors.push(error instanceof Error ? error : new Error(String(error))); + } + }; + + run(() => + root.render(), + ); + const middle = container.querySelector("[data-testid='src/b.ts']"); + expect(middle).toBeInstanceOf(HTMLElement); + wrapLikeFileReveal(middle as HTMLElement); + + run(() => root.render()); + run(() => root.unmount()); + container.remove(); + return errors; + } + + it("keeps a File Reveal wrap from crashing when a list item is removed", () => { + const unguarded = removeListItemAfterFileRevealWrap(); + expect(unguarded.length).toBeGreaterThan(0); + expect(unguarded[0]?.message).toMatch(/not a child of this node/); + + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + expect(removeListItemAfterFileRevealWrap()).toEqual([]); + expect(foreignDomMutationCount()).toBeGreaterThan(0); + }); + + it("stops a plugin content script from stealing a React-owned node", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + + src/app.ts + , + ); + }); + const link = container.querySelector("[data-testid='file-link']"); + expect(link).toBeInstanceOf(HTMLAnchorElement); + const reactParent = link!.parentNode; + expect(reactParent).not.toBeNull(); + + runWithPluginDomIsolation(() => { + wrapLikeFileReveal(link as HTMLElement); + }, "file-reveal"); + + expect(link!.parentNode).toBe(reactParent); + expect(pluginHostNodeMoveRefusalCount()).toBe(1); + expect(container.querySelector("button")).not.toBeNull(); + + act(() => root.unmount()); + container.remove(); + }); + + it("keeps a MutationObserver created by a plugin from stealing nodes later", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render(
); + }); + const hostList = container.querySelector("[data-testid='host-list']"); + expect(hostList).toBeInstanceOf(HTMLElement); + + runWithPluginDomIsolation(() => { + const observer = new MutationObserver((records) => { + for (const record of records) { + for (const node of record.addedNodes) { + if (node instanceof HTMLAnchorElement) wrapLikeFileReveal(node); + } + } + }); + observer.observe(hostList!, { childList: true }); + }, "file-reveal"); + + act(() => { + root.render( +
+ + src/later.ts + +
, + ); + }); + await act(async () => { + await Promise.resolve(); + }); + + const lateLink = container.querySelector("[data-testid='late-link']"); + expect(lateLink).toBeInstanceOf(HTMLAnchorElement); + expect(lateLink!.parentNode).toBe(hostList); + expect(pluginHostNodeMoveRefusalCount()).toBeGreaterThan(0); + + act(() => root.unmount()); + container.remove(); + }); + + it("stops replaceChildren from adopting a React-owned node", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + + src/app.ts + , + ); + }); + const link = container.querySelector("[data-testid='replace-link']"); + expect(link).toBeInstanceOf(HTMLAnchorElement); + const reactParent = link!.parentNode; + + runWithPluginDomIsolation(() => { + const group = document.createElement("span"); + reactParent!.insertBefore(group, link); + group.replaceChildren(link!); + }, "file-reveal"); + + expect(link!.parentNode).toBe(reactParent); + expect(pluginHostNodeMoveRefusalCount()).toBeGreaterThan(0); + + act(() => root.unmount()); + container.remove(); + }); + + it("keeps isolation across await and event listeners", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + + src/app.ts + , + ); + }); + const link = container.querySelector("[data-testid='await-link']"); + expect(link).toBeInstanceOf(HTMLAnchorElement); + const reactParent = link!.parentNode; + const trigger = document.createElement("button"); + container.append(trigger); + + await runWithPluginDomIsolationAsync(async () => { + await Promise.resolve(); + wrapLikeFileReveal(link as HTMLElement); + }, "file-reveal"); + expect(link!.parentNode).toBe(reactParent); + + runWithPluginDomIsolation(() => { + trigger.addEventListener("click", () => { + wrapLikeFileReveal(link as HTMLElement); + }); + }, "file-reveal"); + trigger.click(); + expect(link!.parentNode).toBe(reactParent); + expect(pluginHostNodeMoveRefusalCount()).toBeGreaterThan(0); + + act(() => root.unmount()); + container.remove(); + }); + + it("preserves MutationObserver subclass identity", () => { + installForeignDomMutationGuard(); + class ExtraObserver extends MutationObserver {} + const observer = new ExtraObserver(() => undefined); + expect(observer).toBeInstanceOf(ExtraObserver); + expect(observer).toBeInstanceOf(MutationObserver); + observer.disconnect(); + }); }); diff --git a/apps/app/src/lib/foreign-dom-mutation-guard.ts b/apps/app/src/lib/foreign-dom-mutation-guard.ts index 56d66928db..732ce6d332 100644 --- a/apps/app/src/lib/foreign-dom-mutation-guard.ts +++ b/apps/app/src/lib/foreign-dom-mutation-guard.ts @@ -3,57 +3,80 @@ * * React owns every node it renders and remembers which parent each one belongs * to. On unmount it calls `parent.removeChild(node)` directly. When another - * agent in the page — a browser extension's content script, the browser's own - * page translator, a bookmarklet — moves or removes one of those nodes first, - * React's call throws: + * agent in the page — a browser extension's content script, a plugin content + * script, the browser's own page translator, a bookmarklet — moves or removes + * one of those nodes first, React's call throws: * * NotFoundError: Failed to execute 'removeChild' on 'Node': The node to be * removed is not a child of this node. * * React catches that throw in the commit phase and escalates it to the nearest - * error boundary. bb had none above the router, so React tore the whole root - * down and the user got a blank page. `AppErrorBoundary` now catches it and - * shows a recovery screen, but a recovery screen still costs the user their - * place for a mutation that only ever affected one subtree. This guard keeps - * the fault from reaching that boundary at all. + * error boundary. List updates are worse: `insertBefore` during placement is + * not wrapped, so a stolen reference node blanks the window even when + * `AppErrorBoundary` would have caught a deletion. This guard keeps both + * faults from reaching that boundary. * - * This guard makes both calls non-fatal. `removeChild` returns the node instead - * of throwing when it already has a different parent (the node is gone from - * where React expected it, which is exactly the outcome React wanted). - * `insertBefore` appends instead of throwing when the reference node has moved - * away, so the content still reaches the DOM rather than vanishing. + * `removeChild` returns the node instead of throwing when it already has a + * different parent. `insertBefore` appends instead of throwing when the + * reference node has moved away. `replaceChild` does the same when the node + * it would replace is gone. Wrappers only change the case that would throw. + * + * Plugin content scripts are a second, tighter layer. While one of those + * scripts runs, the guard also refuses to move a React-owned node to a new + * parent. Continuations the script schedules — MutationObserver callbacks, + * timers, microtasks, `await`, and event listeners — keep the same rule. + * The plugin can still insert its own sibling controls, but it cannot steal + * a host button or link out of React's tree. * * This is the workaround React's own maintainers publish for translated pages - * (facebook/react#11538). It is deliberately narrow: both wrappers only change - * behavior in the case that would otherwise throw, so every well-formed call - * reaches the native method unchanged. + * (facebook/react#11538), plus a host-side fence for trusted plugin scripts + * that still share the document. * - * Suppressed calls are counted and the first few are logged, because a burst of - * them from a build where no extension is involved would point at a real bug in - * our own rendering instead. + * Suppressed calls are counted and the first few are logged, because a burst + * of them from a build where no extension or plugin is involved would point + * at a real bug in our own rendering instead. */ /** Log at most this many suppressions, then only keep counting. */ const MAX_LOGGED_SUPPRESSIONS = 3; +const REACT_FIBER_PREFIXES = [ + "__reactFiber$", + "__reactInternalInstance$", +] as const; + type RemoveChild = (child: T) => T; type InsertBefore = (node: T, child: Node | null) => T; +type ReplaceChild = (node: Node, child: T) => T; +type AppendChild = (node: T) => T; +type AppendLike = (...nodes: Array) => void; interface InstalledGuard { - removeChild: RemoveChild; - insertBefore: InsertBefore; + restore: () => void; } let installed: InstalledGuard | null = null; let suppressedCount = 0; let loggedCount = 0; +let refusedMoveCount = 0; +let loggedRefusalCount = 0; +let isolationDepth = 0; +let isolationLabel: string | null = null; /** How many throwing DOM calls the guard has absorbed this session. */ export function foreignDomMutationCount(): number { return suppressedCount; } -function describe(node: Node): string { +/** + * How many times a plugin content script was stopped from moving a React-owned + * node out of the tree. + */ +export function pluginHostNodeMoveRefusalCount(): number { + return refusedMoveCount; +} + +function describeNode(node: Node): string { if (node instanceof Element) { const id = node.id ? `#${node.id}` : ""; const testId = node.getAttribute("data-testid"); @@ -63,8 +86,27 @@ function describe(node: Node): string { return `#node(${node.nodeType})`; } +function isNotFoundError(error: unknown): boolean { + return error instanceof DOMException && error.name === "NotFoundError"; +} + +function isHierarchyRequestError(error: unknown): boolean { + return ( + error instanceof DOMException && error.name === "HierarchyRequestError" + ); +} + +function isReactHostNode(node: Node): boolean { + for (const key of Object.getOwnPropertyNames(node)) { + for (const prefix of REACT_FIBER_PREFIXES) { + if (key.startsWith(prefix)) return true; + } + } + return false; +} + function recordSuppression( - operation: "removeChild" | "insertBefore", + operation: "removeChild" | "insertBefore" | "replaceChild", node: Node, expectedParent: Node, ): void { @@ -72,28 +114,181 @@ function recordSuppression( if (loggedCount >= MAX_LOGGED_SUPPRESSIONS) return; loggedCount += 1; console.warn( - `[bb] ${operation}: ${describe(node)} is no longer a child of ${describe( + `[bb] ${operation}: ${describeNode(node)} is no longer a child of ${describeNode( expectedParent, - )}. Something outside React moved or removed it (a browser extension or ` + - `page translation is the usual cause); the call was suppressed instead ` + - `of crashing the app.`, + )}. Something outside React moved or removed it (a browser extension, ` + + `plugin content script, or page translation is the usual cause); the ` + + `call was suppressed instead of crashing the app.`, { node, expectedParent, actualParent: node.parentNode }, ); } +function recordRefusedMove(node: Node, attemptedParent: Node): void { + refusedMoveCount += 1; + if (loggedRefusalCount >= MAX_LOGGED_SUPPRESSIONS) return; + loggedRefusalCount += 1; + const owner = + isolationLabel === null + ? "a plugin content script" + : `plugin "${isolationLabel}"`; + console.warn( + `[bb] ${owner} tried to move ${describeNode(node)} out of React's tree. The ` + + `move was blocked so the app does not crash when that node is later ` + + `removed or reordered.`, + { node, attemptedParent, actualParent: node.parentNode }, + ); +} + +/** + * True when a plugin script is trying to adopt a React-owned node into a + * different parent. Same-parent reorders stay allowed. A detached React node + * may still attach to another React host (a plugin-triggered commit). It may + * not attach to a foreign parent after `remove` / `replaceWith`. + */ +function refusePluginReparent(node: Node, newParent: Node): boolean { + if (isolationDepth === 0) return false; + if (node.parentNode === newParent) return false; + if (!isReactHostNode(node)) return false; + if (node.parentNode === null && isReactHostNode(newParent)) return false; + recordRefusedMove(node, newParent); + return true; +} + +function wrapCallback( + callback: (...args: Args) => Result, + label: string | null, +): (...args: Args) => Result { + return (...args: Args) => + runWithPluginDomIsolation(() => callback(...args), label ?? undefined); +} + +function enterIsolation(label?: string): string | null { + const previousLabel = isolationLabel; + isolationDepth += 1; + if (label !== undefined) isolationLabel = label; + return previousLabel; +} + +function leaveIsolation(previousLabel: string | null): void { + isolationDepth -= 1; + isolationLabel = previousLabel; +} + +/** + * Run `fn` as plugin content-script DOM. React-owned nodes cannot be moved to + * a new parent for the duration of the synchronous call. Work the script + * schedules from here — observers, timers, and listeners — keeps the same + * rule. + */ +export function runWithPluginDomIsolation(fn: () => T, label?: string): T { + const previousLabel = enterIsolation(label); + try { + return fn(); + } finally { + leaveIsolation(previousLabel); + } +} + +function whenAborted(signal: AbortSignal): Promise { + return new Promise((resolve) => { + if (signal.aborted) { + resolve(); + return; + } + signal.addEventListener("abort", () => resolve(), { once: true }); + }); +} + +/** + * Same rule as {@link runWithPluginDomIsolation}, held across the returned + * promise so an `async` mount or disposer cannot steal a node after `await`. + * Abort `signal` to drop the fence if the host gives up on a stuck mount. + * The original work still runs; it is just no longer isolated. + */ +export async function runWithPluginDomIsolationAsync( + fn: () => T | Promise, + label?: string, + signal?: AbortSignal, +): Promise { + const previousLabel = enterIsolation(label); + const work = Promise.resolve().then(fn); + try { + if (signal === undefined) return await work; + const winner = await Promise.race([ + work.then((value) => ({ ok: true as const, value })), + whenAborted(signal).then(() => ({ ok: false as const })), + ]); + if (winner.ok) return winner.value; + } finally { + leaveIsolation(previousLabel); + } + return await work; +} + +function filterAppendNodes( + parent: Node, + nodes: Array, +): Array { + if (isolationDepth === 0) return nodes; + const kept: Array = []; + for (const node of nodes) { + if (typeof node !== "string" && refusePluginReparent(node, parent)) { + continue; + } + kept.push(node); + } + return kept; +} + /** - * Wrap `Node.prototype.removeChild` / `insertBefore`. Safe to call more than - * once; only the first call installs. + * Wrap the DOM methods React (and plugins) use to move nodes. Safe to call + * more than once; only the first call installs. */ export function installForeignDomMutationGuard(): void { if (installed !== null || typeof Node !== "function") return; const nativeRemoveChild = Node.prototype.removeChild; const nativeInsertBefore = Node.prototype.insertBefore; + const nativeReplaceChild = Node.prototype.replaceChild; + const nativeAppendChild = Node.prototype.appendChild; + const nativeElementAppend = Element.prototype.append; + const nativeElementPrepend = Element.prototype.prepend; + const nativeElementBefore = Element.prototype.before; + const nativeElementAfter = Element.prototype.after; + const nativeElementReplaceWith = Element.prototype.replaceWith; + const nativeDocumentAppend = Document.prototype.append; + const nativeDocumentPrepend = Document.prototype.prepend; + const nativeFragmentAppend = DocumentFragment.prototype.append; + const nativeFragmentPrepend = DocumentFragment.prototype.prepend; + const nativeElementReplaceChildren = Element.prototype.replaceChildren; + const nativeDocumentReplaceChildren = Document.prototype.replaceChildren; + const nativeFragmentReplaceChildren = + DocumentFragment.prototype.replaceChildren; + const nativeInsertAdjacentElement = Element.prototype.insertAdjacentElement; + const nativeRangeInsertNode = + typeof Range === "function" ? Range.prototype.insertNode : null; + const nativeAddEventListener = EventTarget.prototype.addEventListener; + const nativeRemoveEventListener = EventTarget.prototype.removeEventListener; + const originalSetTimeout = window.setTimeout; + const nativeSetTimeout = originalSetTimeout.bind(window); + const originalSetInterval = window.setInterval; + const nativeSetInterval = originalSetInterval.bind(window); + const originalQueueMicrotask = + typeof queueMicrotask === "function" ? queueMicrotask : null; + const nativeQueueMicrotask = + originalQueueMicrotask === null + ? null + : originalQueueMicrotask.bind(window); + const NativeMutationObserver = + typeof MutationObserver === "function" ? MutationObserver : null; + const listenerWraps = new WeakMap< + EventListenerOrEventListenerObject, + EventListenerOrEventListenerObject + >(); - // Both wrappers return their own argument rather than the native return - // value: the DOM spec defines each of these as returning the node it was - // handed, so this keeps the generic result type without an `as` cast. + // Wrappers return their own argument rather than the native return value: + // the DOM spec defines each of these as returning the node it was handed, + // so this keeps the generic result type without an `as` cast. const guardedRemoveChild: RemoveChild = function removeChild( this: Node, child: T, @@ -102,35 +297,318 @@ export function installForeignDomMutationGuard(): void { recordSuppression("removeChild", child, this); return child; } - nativeRemoveChild.call(this, child); + try { + nativeRemoveChild.call(this, child); + } catch (error) { + if (isNotFoundError(error)) { + recordSuppression("removeChild", child, this); + return child; + } + throw error; + } return child; }; const guardedInsertBefore: InsertBefore = function insertBefore< T extends Node, >(this: Node, node: T, child: Node | null): T { + if (refusePluginReparent(node, this)) return node; if (child !== null && child.parentNode !== this) { recordSuppression("insertBefore", child, this); - // Append rather than drop the node: the ordering is already wrong - // because of the foreign mutation, but the content stays reachable. - nativeInsertBefore.call(this, node, null); + try { + nativeInsertBefore.call(this, node, null); + } catch (error) { + if (!isNotFoundError(error) && !isHierarchyRequestError(error)) { + throw error; + } + } return node; } - nativeInsertBefore.call(this, node, child); + try { + nativeInsertBefore.call(this, node, child); + } catch (error) { + if (isNotFoundError(error)) { + recordSuppression("insertBefore", child ?? node, this); + return node; + } + throw error; + } return node; }; + const guardedReplaceChild: ReplaceChild = function replaceChild< + T extends Node, + >(this: Node, node: Node, child: T): T { + if (refusePluginReparent(node, this)) return child; + if (child.parentNode !== this) { + recordSuppression("replaceChild", child, this); + if (node.parentNode !== this && !refusePluginReparent(node, this)) { + try { + nativeInsertBefore.call(this, node, null); + } catch (error) { + if (!isNotFoundError(error) && !isHierarchyRequestError(error)) { + throw error; + } + } + } + return child; + } + try { + nativeReplaceChild.call(this, node, child); + } catch (error) { + if (isNotFoundError(error)) { + recordSuppression("replaceChild", child, this); + return child; + } + throw error; + } + return child; + }; + + const guardedAppendChild: AppendChild = function appendChild( + this: Node, + node: T, + ): T { + if (refusePluginReparent(node, this)) return node; + nativeAppendChild.call(this, node); + return node; + }; + + const guardedParentAppend = (native: AppendLike): AppendLike => + function append(this: ParentNode, ...nodes: Array): void { + const kept = filterAppendNodes(this, nodes); + if (kept.length === 0) return; + native.apply(this, kept); + }; + + const guardedAdjacent = ( + native: AppendLike, + resolveParent: (self: Element) => Node | null, + ): AppendLike => + function adjacent(this: Element, ...nodes: Array): void { + const parent = resolveParent(this) ?? this; + const kept = filterAppendNodes(parent, nodes); + if (kept.length === 0) return; + native.apply(this, kept); + }; + Node.prototype.removeChild = guardedRemoveChild; Node.prototype.insertBefore = guardedInsertBefore; - installed = { removeChild: nativeRemoveChild, insertBefore: nativeInsertBefore }; + Node.prototype.replaceChild = guardedReplaceChild; + Node.prototype.appendChild = guardedAppendChild; + Element.prototype.append = guardedParentAppend(nativeElementAppend); + Element.prototype.prepend = guardedParentAppend(nativeElementPrepend); + Element.prototype.before = guardedAdjacent( + nativeElementBefore, + (self) => self.parentNode, + ); + Element.prototype.after = guardedAdjacent( + nativeElementAfter, + (self) => self.parentNode, + ); + Element.prototype.replaceWith = guardedAdjacent( + nativeElementReplaceWith, + (self) => self.parentNode, + ); + Document.prototype.append = guardedParentAppend(nativeDocumentAppend); + Document.prototype.prepend = guardedParentAppend(nativeDocumentPrepend); + DocumentFragment.prototype.append = guardedParentAppend(nativeFragmentAppend); + DocumentFragment.prototype.prepend = guardedParentAppend( + nativeFragmentPrepend, + ); + + const guardedReplaceChildren = (native: AppendLike): AppendLike => + function replaceChildren( + this: ParentNode, + ...nodes: Array + ): void { + native.apply(this, filterAppendNodes(this, nodes)); + }; + Element.prototype.replaceChildren = guardedReplaceChildren( + nativeElementReplaceChildren, + ); + Document.prototype.replaceChildren = guardedReplaceChildren( + nativeDocumentReplaceChildren, + ); + DocumentFragment.prototype.replaceChildren = guardedReplaceChildren( + nativeFragmentReplaceChildren, + ); + + Element.prototype.insertAdjacentElement = function insertAdjacentElement( + position: InsertPosition, + element: Element, + ): Element | null { + const parent = + position === "beforebegin" || position === "afterend" + ? this.parentNode + : this; + if (parent !== null && refusePluginReparent(element, parent)) return null; + return nativeInsertAdjacentElement.call(this, position, element); + }; + + if (nativeRangeInsertNode !== null) { + Range.prototype.insertNode = function insertNode(node: Node): void { + const container = this.commonAncestorContainer; + const parent = + container.nodeType === Node.TEXT_NODE + ? container.parentNode + : container; + if (parent !== null && refusePluginReparent(node, parent)) return; + nativeRangeInsertNode.call(this, node); + }; + } + + function wrapListener( + listener: EventListenerOrEventListenerObject, + label: string | null, + ): EventListenerOrEventListenerObject { + const existing = listenerWraps.get(listener); + if (existing !== undefined) return existing; + const wrapped: EventListenerOrEventListenerObject = + typeof listener === "function" + ? wrapCallback(listener, label) + : { + handleEvent: (event: Event) => + runWithPluginDomIsolation( + () => listener.handleEvent(event), + label ?? undefined, + ), + }; + listenerWraps.set(listener, wrapped); + return wrapped; + } + + EventTarget.prototype.addEventListener = function addEventListener( + type: string, + listener: EventListenerOrEventListenerObject | null, + options?: boolean | AddEventListenerOptions, + ): void { + const scheduled = + isolationDepth > 0 && listener !== null + ? wrapListener(listener, isolationLabel) + : listener; + nativeAddEventListener.call(this, type, scheduled, options); + }; + EventTarget.prototype.removeEventListener = function removeEventListener( + type: string, + listener: EventListenerOrEventListenerObject | null, + options?: boolean | EventListenerOptions, + ): void { + const wrapped = listener === null ? undefined : listenerWraps.get(listener); + nativeRemoveEventListener.call(this, type, listener, options); + if (wrapped !== undefined && wrapped !== listener) { + nativeRemoveEventListener.call(this, type, wrapped, options); + } + }; + + function wrapTimerHandler( + handler: TimerHandler, + label: string | null, + ): TimerHandler { + if (typeof handler !== "function") return handler; + return function isolatedTimer(this: unknown, ...cbArgs: unknown[]) { + return runWithPluginDomIsolation( + () => Reflect.apply(handler, this, cbArgs), + label ?? undefined, + ); + }; + } + + window.setTimeout = (( + handler: TimerHandler, + timeout?: number, + ...args: unknown[] + ) => { + const scheduled = + isolationDepth > 0 ? wrapTimerHandler(handler, isolationLabel) : handler; + return nativeSetTimeout(scheduled, timeout, ...args); + }) as typeof setTimeout; + + window.setInterval = (( + handler: TimerHandler, + timeout?: number, + ...args: unknown[] + ) => { + const scheduled = + isolationDepth > 0 ? wrapTimerHandler(handler, isolationLabel) : handler; + return nativeSetInterval(scheduled, timeout, ...args); + }) as typeof setInterval; + + if (nativeQueueMicrotask !== null) { + window.queueMicrotask = (callback: VoidFunction) => { + nativeQueueMicrotask( + isolationDepth > 0 ? wrapCallback(callback, isolationLabel) : callback, + ); + }; + } + + if (NativeMutationObserver !== null) { + window.MutationObserver = class IsolatedMutationObserver extends ( + NativeMutationObserver + ) { + constructor(callback: MutationCallback) { + const label = isolationLabel; + super( + isolationDepth > 0 + ? (records, observer) => { + runWithPluginDomIsolation( + () => callback(records, observer), + label ?? undefined, + ); + } + : callback, + ); + } + }; + } + + installed = { + restore: () => { + Node.prototype.removeChild = nativeRemoveChild; + Node.prototype.insertBefore = nativeInsertBefore; + Node.prototype.replaceChild = nativeReplaceChild; + Node.prototype.appendChild = nativeAppendChild; + Element.prototype.append = nativeElementAppend; + Element.prototype.prepend = nativeElementPrepend; + Element.prototype.before = nativeElementBefore; + Element.prototype.after = nativeElementAfter; + Element.prototype.replaceWith = nativeElementReplaceWith; + Document.prototype.append = nativeDocumentAppend; + Document.prototype.prepend = nativeDocumentPrepend; + DocumentFragment.prototype.append = nativeFragmentAppend; + DocumentFragment.prototype.prepend = nativeFragmentPrepend; + Element.prototype.replaceChildren = nativeElementReplaceChildren; + Document.prototype.replaceChildren = nativeDocumentReplaceChildren; + DocumentFragment.prototype.replaceChildren = + nativeFragmentReplaceChildren; + Element.prototype.insertAdjacentElement = nativeInsertAdjacentElement; + if (nativeRangeInsertNode !== null) { + Range.prototype.insertNode = nativeRangeInsertNode; + } + EventTarget.prototype.addEventListener = nativeAddEventListener; + EventTarget.prototype.removeEventListener = nativeRemoveEventListener; + window.setTimeout = originalSetTimeout; + window.setInterval = originalSetInterval; + if (originalQueueMicrotask !== null) { + window.queueMicrotask = originalQueueMicrotask; + } + if (NativeMutationObserver !== null) { + window.MutationObserver = NativeMutationObserver; + } + }, + }; } /** Restore the native methods and the counters. Test-only. */ export function uninstallForeignDomMutationGuardForTest(): void { - if (installed === null) return; - Node.prototype.removeChild = installed.removeChild; - Node.prototype.insertBefore = installed.insertBefore; - installed = null; + if (installed !== null) { + installed.restore(); + installed = null; + } suppressedCount = 0; loggedCount = 0; + refusedMoveCount = 0; + loggedRefusalCount = 0; + isolationDepth = 0; + isolationLabel = null; } diff --git a/apps/app/src/lib/plugin-frontend-reload.test.ts b/apps/app/src/lib/plugin-frontend-reload.test.ts index e05d51d039..00b446c633 100644 --- a/apps/app/src/lib/plugin-frontend-reload.test.ts +++ b/apps/app/src/lib/plugin-frontend-reload.test.ts @@ -1,7 +1,14 @@ // @vitest-environment jsdom import type { PluginComposerThreadRowStatus } from "@bb/plugin-sdk"; +import { createElement } from "react"; +import { createRoot } from "react-dom/client"; +import { act } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + installForeignDomMutationGuard, + uninstallForeignDomMutationGuardForTest, +} from "./foreign-dom-mutation-guard"; import { definePluginApp } from "./plugin-app-definition"; import { applyPluginCss, @@ -63,6 +70,7 @@ function contentScriptModule( afterEach(() => { resetPluginThreadRowStatusesForTest(); + uninstallForeignDomMutationGuardForTest(); }); function makeDeps(initial: PluginFrontendCandidate[] = []) { @@ -688,6 +696,110 @@ describe("reconcilePluginFrontends", () => { await reconcilePluginFrontends(state, deps); expect(deps.applyCss).toHaveBeenLastCalledWith("hello", null); }); + + it("does not let a content script steal a React-owned host node", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + createElement( + "a", + { href: "?path=src/app.ts", "data-testid": "file-link" }, + "src/app.ts", + ), + ); + }); + const link = container.querySelector("[data-testid='file-link']"); + expect(link).toBeInstanceOf(HTMLAnchorElement); + const reactParent = link!.parentNode; + + const state = createPluginFrontendReconcileState(); + const deps = makeDeps([candidate("file-reveal", "v1")]); + deps.importModule.mockResolvedValue( + contentScriptModule((app) => { + app.contentScripts.register({ + id: "file-reveal-buttons", + mount() { + const control = document.querySelector("[data-testid='file-link']"); + if ( + !(control instanceof HTMLElement) || + control.parentNode === null + ) { + return; + } + const group = document.createElement("span"); + const button = document.createElement("button"); + control.parentNode.insertBefore(group, control); + group.append(control, button); + }, + }); + }), + ); + + await reconcilePluginFrontends(state, deps); + expect(link!.parentNode).toBe(reactParent); + expect(container.querySelector("button")).not.toBeNull(); + + act(() => root.unmount()); + container.remove(); + }); + + it("does not let an async content-script mount steal a React-owned node", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + installForeignDomMutationGuard(); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + createElement( + "a", + { href: "?path=src/app.ts", "data-testid": "async-file-link" }, + "src/app.ts", + ), + ); + }); + const link = container.querySelector("[data-testid='async-file-link']"); + expect(link).toBeInstanceOf(HTMLAnchorElement); + const reactParent = link!.parentNode; + + const state = createPluginFrontendReconcileState(); + const deps = makeDeps([candidate("file-reveal", "v1")]); + deps.importModule.mockResolvedValue( + contentScriptModule((app) => { + app.contentScripts.register({ + id: "file-reveal-buttons", + async mount() { + await Promise.resolve(); + const control = document.querySelector( + "[data-testid='async-file-link']", + ); + if ( + !(control instanceof HTMLElement) || + control.parentNode === null + ) { + return; + } + const group = document.createElement("span"); + const button = document.createElement("button"); + control.parentNode.insertBefore(group, control); + group.append(control, button); + }, + }); + }), + ); + + await reconcilePluginFrontends(state, deps); + expect(link!.parentNode).toBe(reactParent); + + act(() => root.unmount()); + container.remove(); + }); }); describe("applyPluginCss", () => { diff --git a/apps/app/src/lib/plugin-frontend.ts b/apps/app/src/lib/plugin-frontend.ts index 6715b7da9d..a6dd57011b 100644 --- a/apps/app/src/lib/plugin-frontend.ts +++ b/apps/app/src/lib/plugin-frontend.ts @@ -31,6 +31,7 @@ import type { } from "@bb/plugin-sdk"; import { normalizePluginThreadRowStatus } from "@bb/plugin-sdk/internal/composer-customization-validation"; import { resetCrashedPluginSlots } from "@/components/plugin/PluginSlotMount"; +import { runWithPluginDomIsolationAsync } from "./foreign-dom-mutation-guard"; import { collectPluginAppRegistrations, isPluginAppDefinition, @@ -442,7 +443,7 @@ async function callDisposer( deps: PluginFrontendReconcileDeps, ): Promise { try { - await disposer(); + await runWithPluginDomIsolationAsync(() => disposer(), pluginId); return null; } catch (error) { const message = errorMessage(error); @@ -506,38 +507,46 @@ async function mountWithTimeout( let timeoutId: ReturnType | undefined; let timedOut = false; const mountPromise = Promise.resolve().then(() => - registration.mount({ - pluginId, - generation, - signal: controller.signal, - experimental_setThreadRowStatus: (threadId: unknown, status: unknown) => { - if (controller.signal.aborted) return; - if (typeof threadId !== "string") { - deps.warn( - `bb plugin "${pluginId}": contentScript.experimental_setThreadRowStatus: "threadId" must be a non-empty string`, - ); - return; - } - const normalizedThreadId = threadId.trim(); - if (normalizedThreadId.length === 0) { - deps.warn( - `bb plugin "${pluginId}": contentScript.experimental_setThreadRowStatus: "threadId" must be a non-empty string`, - ); - return; - } - const normalizedStatus = normalizePluginThreadRowStatus( - status, - (reason) => deps.warn(`bb plugin "${pluginId}": ${reason}`), - ); - if (normalizedStatus === undefined) return; - setPluginThreadRowStatus( - normalizedThreadId, + runWithPluginDomIsolationAsync( + () => + registration.mount({ pluginId, - normalizedStatus, - statusOwner, - ); - }, - }), + generation, + signal: controller.signal, + experimental_setThreadRowStatus: ( + threadId: unknown, + status: unknown, + ) => { + if (controller.signal.aborted) return; + if (typeof threadId !== "string") { + deps.warn( + `bb plugin "${pluginId}": contentScript.experimental_setThreadRowStatus: "threadId" must be a non-empty string`, + ); + return; + } + const normalizedThreadId = threadId.trim(); + if (normalizedThreadId.length === 0) { + deps.warn( + `bb plugin "${pluginId}": contentScript.experimental_setThreadRowStatus: "threadId" must be a non-empty string`, + ); + return; + } + const normalizedStatus = normalizePluginThreadRowStatus( + status, + (reason) => deps.warn(`bb plugin "${pluginId}": ${reason}`), + ); + if (normalizedStatus === undefined) return; + setPluginThreadRowStatus( + normalizedThreadId, + pluginId, + normalizedStatus, + statusOwner, + ); + }, + }), + pluginId, + controller.signal, + ), ); const timeoutMs = deps.mountTimeoutMs ?? DEFAULT_CONTENT_SCRIPT_MOUNT_TIMEOUT_MS;