fix: guard Node.contains() against non-Node event targets (#101) (#103)

Co-authored-by: Ona <no-reply@ona.com>

Author: zacharias-ona

.agents/conventions.md

@@ -504,6 +504,29 @@ For the full automation roster and workflow details, see `.ona/skills/developmen
 - No `as` casts unless unavoidable (add a comment explaining why)
 - Prefer interfaces for object shapes, types for unions/intersections
 
+### DOM event target safety
+
+`MouseEvent.target`, `MouseEvent.relatedTarget`, and `FocusEvent.relatedTarget` are
+typed as `EventTarget | null`. They can be non-Node objects (e.g. when the mouse
+leaves to a cross-origin iframe or the browser window). Never cast them with
+`as Node` or `as HTMLElement` — use `instanceof` guards instead.
+
+```typescript
+// ✅ Correct — instanceof guard before DOM API call
+const target = event.relatedTarget;
+if (target instanceof Node && container.contains(target)) { ... }
+
+// ✅ Correct — instanceof guard before property access
+const target = event.target;
+if (!(target instanceof HTMLElement)) return;
+target.closest(".menu");
+
+// ❌ Wrong — unsafe cast, throws TypeError if target is not a Node
+container.contains(event.target as Node)
+```
+
+A static analysis test (`node-contains-safety.test.ts`) enforces this convention.
+
 ## shadcn/ui (base-nova style)
 
 This project uses shadcn/ui v4 with the `base-nova` style, which uses `@base-ui/react`

src/components/editor/draggable-block-plugin.tsx

@@ -164,7 +164,8 @@ export function DraggableBlockPlugin({
   // Track the hovered block element for showing the drag handle
   const handleMouseMove = useCallback(
     (event: MouseEvent) => {
-      const target = event.target as HTMLElement;
+      const target = event.target;
+      if (!(target instanceof HTMLElement)) return;
       if (isOnMenu(target) || isDraggingRef.current) return;
 
       const blockElem = getBlockElement(anchorElem, editor, event);
@@ -182,8 +183,9 @@ export function DraggableBlockPlugin({
   const handleMouseLeave = useCallback((event: MouseEvent) => {
     if (menuRef.current && !isDraggingRef.current) {
       // Don't hide if the mouse moved to the drag handle itself
-      const relatedTarget = event.relatedTarget as HTMLElement | null;
-      if (relatedTarget && isOnMenu(relatedTarget)) return;
+      const relatedTarget = event.relatedTarget;
+      if (relatedTarget instanceof HTMLElement && isOnMenu(relatedTarget))
+        return;
 
       menuRef.current.style.opacity = "0";
       targetBlockElemRef.current = null;
@@ -203,8 +205,10 @@ export function DraggableBlockPlugin({
     (event: React.MouseEvent) => {
       if (isDraggingRef.current) return;
       // Don't hide if the mouse moved back into the anchor element
-      const relatedTarget = event.relatedTarget as HTMLElement | null;
-      if (relatedTarget && anchorElem.contains(relatedTarget)) return;
+      // relatedTarget can be a non-Node EventTarget (e.g. cross-origin iframe)
+      const relatedTarget = event.relatedTarget;
+      if (relatedTarget instanceof Node && anchorElem.contains(relatedTarget))
+        return;
 
       if (menuRef.current) {
         menuRef.current.style.opacity = "0";

src/components/editor/node-contains-safety.test.ts

@@ -0,0 +1,70 @@
+import { describe, it, expect } from "vitest";
+import { readFileSync, readdirSync, statSync } from "fs";
+import { resolve, join } from "path";
+
+/**
+ * Regression tests for issue #101: TypeError in Node.contains() from event handlers.
+ *
+ * MouseEvent.relatedTarget and Event.target are typed as EventTarget | null,
+ * which can be a non-Node object (e.g. when mouse leaves to a cross-origin
+ * iframe). Casting these to Node/HTMLElement and passing them to Node.contains()
+ * or Element.closest() throws a TypeError.
+ *
+ * These tests scan source files for unsafe `as Node` or `as HTMLElement` casts
+ * on event target properties, ensuring instanceof guards are used instead.
+ */
+
+/** Recursively collect .ts/.tsx files under a directory */
+function collectSourceFiles(dir: string): string[] {
+  const files: string[] = [];
+  for (const entry of readdirSync(dir)) {
+    const full = join(dir, entry);
+    if (statSync(full).isDirectory()) {
+      files.push(...collectSourceFiles(full));
+    } else if (/\.tsx?$/.test(entry) && !entry.includes(".test.")) {
+      files.push(full);
+    }
+  }
+  return files;
+}
+
+/**
+ * Matches patterns like:
+ *   event.target as Node
+ *   event.relatedTarget as HTMLElement
+ *   e.target as HTMLElement | null
+ *   event.relatedTarget as HTMLElement | null
+ *
+ * These are unsafe because EventTarget is not necessarily a Node.
+ */
+const UNSAFE_EVENT_TARGET_CAST =
+  /\.\s*(?:target|relatedTarget)\s+as\s+(?:Node|HTMLElement)(?:\s*\|\s*null)?/g;
+
+describe("DOM API type safety — no unsafe event target casts", () => {
+  const srcRoot = resolve(__dirname, "../..");
+  const sourceFiles = collectSourceFiles(join(srcRoot, "components"));
+
+  it("no files cast event.target or event.relatedTarget as Node/HTMLElement", () => {
+    const violations: string[] = [];
+
+    for (const filePath of sourceFiles) {
+      const content = readFileSync(filePath, "utf-8");
+      const lines = content.split("\n");
+
+      for (let i = 0; i < lines.length; i++) {
+        const matches = lines[i].match(UNSAFE_EVENT_TARGET_CAST);
+        if (matches) {
+          const relative = filePath.replace(srcRoot + "/", "");
+          violations.push(`${relative}:${i + 1}: ${matches[0].trim()}`);
+        }
+      }
+    }
+
+    expect(
+      violations,
+      "Use `instanceof Node` or `instanceof HTMLElement` guards instead of `as` casts on event targets. " +
+        "See issue #101.\n\nViolations:\n" +
+        violations.join("\n")
+    ).toHaveLength(0);
+  });
+});

src/components/sidebar/page-search.tsx

@@ -110,9 +110,12 @@ export function PageSearch() {
   // Close on click outside
   useEffect(() => {
     function handleClickOutside(e: MouseEvent) {
+      // e.target can be a non-Node EventTarget (e.g. cross-origin iframe)
+      const target = e.target;
       if (
         containerRef.current &&
-        !containerRef.current.contains(e.target as Node)
+        (!(target instanceof Node) ||
+          !containerRef.current.contains(target))
       ) {
         setOpen(false);
       }