fix(collab): validate synchronized node data (#517)

* fix(collab): validate synchronized node data

- Exclude renderer-only text picture caches from Yjs payloads

- Normalize source metadata and geometry at the remote boundary

- Cover malformed payloads and typed geometry round trips

* fix(collab): reject malformed geometry fills

* fix(collab): validate nested fill metadata

* test(collab): preserve valid nested fill metadata

CHANGELOG.md

@@ -41,6 +41,7 @@
 
 ### Fixed
 
+- Harden collaboration node synchronization against malformed remote source metadata and geometry while excluding derived text-renderer caches.
 - Transfer native `.fig` exports over binary Tauri IPC instead of JSON byte arrays, preventing large desktop saves from being truncated or exhausting WebView memory. (#484)
 - Keep unsaved source-less documents recoverable after their editor tab is closed, matching Figma's retained offline-change behavior.
 - Decode zstd-compressed FIG containers, reject invalid compressed payloads, and preserve exact fixture byte ranges. (#397)

src/app/collab/node-codec.ts

@@ -0,0 +1,193 @@
+import type {
+  Fill,
+  FillType,
+  GeometryPath,
+  SceneNode,
+  SourceMetadata
+} from '@open-pencil/scene-graph'
+import { copyFills } from '@open-pencil/scene-graph/copy'
+import { createDefaultSourceMetadata } from '@open-pencil/scene-graph/node-defaults'
+import type { Matrix, Vector } from '@open-pencil/scene-graph/primitives'
+
+const DERIVED_NODE_FIELDS = new Set<keyof SceneNode>(['textPicture'])
+const FILL_TYPES = new Set<FillType>([
+  'SOLID',
+  'GRADIENT_LINEAR',
+  'GRADIENT_RADIAL',
+  'GRADIENT_ANGULAR',
+  'GRADIENT_DIAMOND',
+  'IMAGE',
+  'VIDEO',
+  'PATTERN',
+  'NOISE',
+  'CUSTOM'
+])
+
+type YjsNodeLike = {
+  entries(): IterableIterator<[string, unknown]>
+}
+
+export function encodeNodeForYjs(node: SceneNode): Record<string, unknown> {
+  const encoded: Record<string, unknown> = {}
+  for (const [key, value] of Object.entries(node)) {
+    if (DERIVED_NODE_FIELDS.has(key as keyof SceneNode)) continue
+    encoded[key] = structuredClone(value)
+  }
+  return encoded
+}
+
+export function syncEncodedNodeToYMap(
+  node: SceneNode,
+  ynode: { delete(key: string): void; set(key: string, value: unknown): void }
+): void {
+  for (const key of DERIVED_NODE_FIELDS) ynode.delete(key)
+  for (const [key, value] of Object.entries(encodeNodeForYjs(node))) ynode.set(key, value)
+}
+
+export function decodeNodeFromYjs(ynode: YjsNodeLike): Partial<SceneNode> {
+  const props: Record<string, unknown> = {}
+  for (const [key, value] of ynode.entries()) {
+    if (DERIVED_NODE_FIELDS.has(key as keyof SceneNode)) continue
+    props[key] = structuredClone(value)
+  }
+
+  props.source = normalizeSourceMetadata(props.source)
+  if ('fillGeometry' in props) props.fillGeometry = normalizeGeometryPaths(props.fillGeometry)
+  if ('strokeGeometry' in props) props.strokeGeometry = normalizeGeometryPaths(props.strokeGeometry)
+  props.textPicture = null
+  return props as Partial<SceneNode>
+}
+
+export function normalizeSourceMetadata(source: unknown): SourceMetadata {
+  const defaults = createDefaultSourceMetadata()
+  if (!isRecord(source)) return defaults
+
+  const fig = isRecord(source.fig) ? source.fig : {}
+  return {
+    format: source.format === 'fig' ? 'fig' : null,
+    id: stringOrNull(source.id),
+    orderKey: stringOrNull(source.orderKey),
+    editedFields: stringArray(source.editedFields),
+    fig: {
+      rawSize: normalizeVector(fig.rawSize),
+      rawTransform: normalizeMatrix(fig.rawTransform),
+      rawNodeFields: isRecord(fig.rawNodeFields) ? structuredClone(fig.rawNodeFields) : {},
+      layout: isRecord(fig.layout)
+        ? (structuredClone(fig.layout) as SourceMetadata['fig']['layout'])
+        : null,
+      symbolOverrides: arrayOrEmpty(fig.symbolOverrides),
+      componentPropAssignments: arrayOrEmpty(fig.componentPropAssignments),
+      derivedSymbolData: arrayOrEmpty(fig.derivedSymbolData),
+      derivedSymbolDataLayoutVersion: numberOrNull(fig.derivedSymbolDataLayoutVersion),
+      uniformScaleFactor: numberOrNull(fig.uniformScaleFactor)
+    }
+  }
+}
+
+function normalizeGeometryPaths(value: unknown): GeometryPath[] {
+  if (!Array.isArray(value)) return []
+  const paths: GeometryPath[] = []
+  for (const item of value) {
+    if (!isRecord(item) || !(item.commandsBlob instanceof Uint8Array)) continue
+    const path: GeometryPath = {
+      windingRule: item.windingRule === 'EVENODD' ? 'EVENODD' : 'NONZERO',
+      commandsBlob: new Uint8Array(item.commandsBlob)
+    }
+    const fills = normalizeFills(item.fills)
+    if (fills.length > 0) path.fills = copyFills(fills)
+    if (typeof item.fillStyleId === 'string') path.fillStyleId = item.fillStyleId
+    paths.push(path)
+  }
+  return paths
+}
+
+function normalizeVector(value: unknown): Vector | null {
+  if (!isRecord(value) || !isFiniteNumber(value.x) || !isFiniteNumber(value.y)) return null
+  return { x: value.x, y: value.y }
+}
+
+function normalizeMatrix(value: unknown): Matrix | null {
+  if (!isRecord(value)) return null
+  const entries = [value.m00, value.m01, value.m02, value.m10, value.m11, value.m12]
+  if (!entries.every((item) => isFiniteNumber(item))) return null
+  return {
+    m00: value.m00 as number,
+    m01: value.m01 as number,
+    m02: value.m02 as number,
+    m10: value.m10 as number,
+    m11: value.m11 as number,
+    m12: value.m12 as number
+  }
+}
+
+function normalizeFills(value: unknown): Fill[] {
+  return Array.isArray(value) ? value.filter(isFill) : []
+}
+
+function isFill(value: unknown): value is Fill {
+  return (
+    isRecord(value) &&
+    typeof value.type === 'string' &&
+    FILL_TYPES.has(value.type as FillType) &&
+    isColor(value.color) &&
+    isFiniteNumber(value.opacity) &&
+    typeof value.visible === 'boolean' &&
+    isOptionalGradientStops(value.gradientStops) &&
+    isOptionalMatrix(value.gradientTransform) &&
+    isOptionalMatrix(value.imageTransform) &&
+    isOptionalVector(value.patternSpacing) &&
+    isOptionalVector(value.noiseSize)
+  )
+}
+
+function isOptionalGradientStops(value: unknown): boolean {
+  return (
+    value === undefined ||
+    (Array.isArray(value) &&
+      value.every((stop) => isRecord(stop) && isFiniteNumber(stop.position) && isColor(stop.color)))
+  )
+}
+
+function isOptionalMatrix(value: unknown): boolean {
+  return value === undefined || normalizeMatrix(value) !== null
+}
+
+function isOptionalVector(value: unknown): boolean {
+  return value === undefined || normalizeVector(value) !== null
+}
+
+function isColor(value: unknown): boolean {
+  return (
+    isRecord(value) &&
+    isFiniteNumber(value.r) &&
+    isFiniteNumber(value.g) &&
+    isFiniteNumber(value.b) &&
+    isFiniteNumber(value.a)
+  )
+}
+
+function isFiniteNumber(value: unknown): value is number {
+  return typeof value === 'number' && Number.isFinite(value)
+}
+
+function arrayOrEmpty(value: unknown): unknown[] {
+  return Array.isArray(value) ? structuredClone(value) : []
+}
+
+function stringArray(value: unknown): string[] {
+  return Array.isArray(value)
+    ? value.filter((item): item is string => typeof item === 'string')
+    : []
+}
+
+function stringOrNull(value: unknown): string | null {
+  return typeof value === 'string' ? value : null
+}
+
+function numberOrNull(value: unknown): number | null {
+  return typeof value === 'number' && Number.isFinite(value) ? value : null
+}
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null && !Array.isArray(value)
+}

src/app/collab/yjs-sync.ts

@@ -1,7 +1,6 @@
 import * as Y from 'yjs'
 
-import type { SceneNode } from '@open-pencil/scene-graph'
-
+import { decodeNodeFromYjs, syncEncodedNodeToYMap } from '@/app/collab/node-codec'
 import type { EditorStore } from '@/app/editor/active-store'
 
 type YNodes = Y.Map<Y.Map<unknown>>
@@ -37,21 +36,6 @@ function logCollabSyncError(context: string, error: unknown) {
   console.error(`[Collab] ${context}:`, error)
 }
 
-// Clone across the graph/Yjs boundary to avoid shared mutable nested data.
-export function syncNodePropsToYMap(node: SceneNode, ynode: Y.Map<unknown>) {
-  for (const [key, value] of Object.entries(node)) {
-    ynode.set(key, structuredClone(value))
-  }
-}
-
-export function yNodeToProps(ynode: Y.Map<unknown>): Record<string, unknown> {
-  const props: Record<string, unknown> = {}
-  for (const [key, value] of ynode.entries()) {
-    props[key] = structuredClone(value)
-  }
-  return props
-}
-
 export function bindCollabGraphEvents({
   store,
   getYdoc,
@@ -156,7 +140,7 @@ export function createYjsGraphSync({
           ynode = new Y.Map()
           ynodes.set(nodeId, ynode)
         }
-        syncNodePropsToYMap(node, ynode)
+        syncEncodedNodeToYMap(node, ynode)
 
         if (localYimages) {
           for (const fill of node.fills) {
@@ -189,7 +173,7 @@ export function createYjsGraphSync({
             ynode = new Y.Map()
             ynodes.set(node.id, ynode)
           }
-          syncNodePropsToYMap(node, ynode)
+          syncEncodedNodeToYMap(node, ynode)
         }
       })
       if (localYimages) {
@@ -244,20 +228,20 @@ export function createYjsGraphSync({
   function applyYnodeToGraph(nodeId: string, ynode: Y.Map<unknown>) {
     const store = getStore()
     const existing = store.graph.getNode(nodeId)
-    const props = yNodeToProps(ynode)
+    const props = decodeNodeFromYjs(ynode)
     const parentId = typeof props.parentId === 'string' ? props.parentId : null
 
     if (existing) {
-      store.graph.updateNode(nodeId, props as Partial<SceneNode>)
+      store.graph.updateNode(nodeId, props)
       if (parentId === null) store.graph.rootId = nodeId
       ensureCurrentPageExists(store)
       return
     }
 
-    const type = props.type as SceneNode['type'] | undefined
+    const type = props.type
     if (!type) return
     // Parent childIds may arrive before or after the child node.
-    store.graph.createNodeWithId(nodeId, type, parentId, props as Partial<SceneNode>)
+    store.graph.createNodeWithId(nodeId, type, parentId, props)
     if (parentId === null) store.graph.rootId = nodeId
     ensureCurrentPageExists(store)
   }

tests/engine/collab/yjs-sync.test.ts

@@ -5,13 +5,10 @@ import * as Y from 'yjs'
 import type { Fill, GeometryPath, SceneNode } from '@open-pencil/scene-graph'
 import { SceneGraph } from '@open-pencil/scene-graph'
 import { nodeVisualBounds } from '@open-pencil/scene-graph/geometry'
+import { createDefaultSourceMetadata } from '@open-pencil/scene-graph/node-defaults'
 
-import {
-  createYjsGraphSync,
-  registerYjsObservers,
-  syncNodePropsToYMap,
-  yNodeToProps
-} from '@/app/collab/yjs-sync'
+import { decodeNodeFromYjs, syncEncodedNodeToYMap } from '@/app/collab/node-codec'
+import { createYjsGraphSync, registerYjsObservers } from '@/app/collab/yjs-sync'
 import { createEditorStore } from '@/app/editor/session'
 
 import { expectDefined, getNodeOrThrow } from '#tests/helpers/assert'
@@ -19,7 +16,7 @@ import { connectYDocs } from '#tests/helpers/yjs'
 
 // Test copy of the private apply path.
 function applyYnodeToGraph(peer: SceneGraph, nodeId: string, ynode: Y.Map<unknown>) {
-  const props = yNodeToProps(ynode)
+  const props = decodeNodeFromYjs(ynode)
   if (peer.getNode(nodeId)) {
     peer.updateNode(nodeId, props as Partial<SceneNode>)
     return
@@ -38,7 +35,7 @@ function seedHostIntoYjs(host: SceneGraph): Y.Map<Y.Map<unknown>> {
     for (const node of host.getAllNodes()) {
       const ynode = new Y.Map<unknown>()
       ynodes.set(node.id, ynode)
-      syncNodePropsToYMap(node, ynode)
+      syncEncodedNodeToYMap(node, ynode)
     }
   })
   return ynodes
@@ -149,6 +146,97 @@ describe('collab yjs-sync', () => {
     expect(page.childIds).toContain('remote-id')
   })
 
+  test('excludes derived text pictures from collaboration payloads', () => {
+    const graph = new SceneGraph()
+    const page = firstPage(graph)
+    const text = graph.createNode('TEXT', page.id, {
+      text: 'Shared text',
+      textPicture: new Uint8Array([4, 5, 6])
+    })
+    const doc = new Y.Doc()
+    const ynode = new Y.Map<unknown>()
+    doc.getMap<Y.Map<unknown>>('nodes').set(text.id, ynode)
+
+    syncEncodedNodeToYMap(text, ynode)
+
+    expect(ynode.has('textPicture')).toBe(false)
+    ynode.set('textPicture', new Uint8Array([9]))
+    expect(decodeNodeFromYjs(ynode).textPicture).toBeNull()
+  })
+
+  test('normalizes malformed source metadata and geometry at the remote boundary', () => {
+    const doc = new Y.Doc()
+    const ynode = new Y.Map<unknown>()
+    doc.getMap<Y.Map<unknown>>('nodes').set('remote', ynode)
+    ynode.set('source', { format: 'fig', fig: { rawNodeFields: 'invalid' } })
+    ynode.set('fillGeometry', [
+      {
+        windingRule: 'EVENODD',
+        commandsBlob: new Uint8Array([0]),
+        fills: [
+          null,
+          'invalid',
+          {
+            type: 'GRADIENT_LINEAR',
+            color: { r: 0, g: 0, b: 0, a: 1 },
+            opacity: 1,
+            visible: true,
+            gradientStops: 'invalid'
+          },
+          {
+            type: 'NOISE',
+            color: { r: 0, g: 0, b: 0, a: 1 },
+            opacity: 1,
+            visible: true,
+            noiseSize: 'invalid'
+          },
+          {
+            type: 'GRADIENT_LINEAR',
+            color: { r: 0, g: 0, b: 0, a: 1 },
+            opacity: 0.8,
+            visible: true,
+            gradientStops: [
+              { color: { r: 1, g: 0, b: 0, a: 1 }, position: 0 },
+              { color: { r: 0, g: 0, b: 1, a: 1 }, position: 1 }
+            ],
+            gradientTransform: { m00: 1, m01: 0, m02: 0, m10: 0, m11: 1, m12: 0 }
+          },
+          {
+            type: 'SOLID',
+            color: { r: 1, g: 0, b: 0, a: 1 },
+            opacity: 1,
+            visible: true
+          }
+        ]
+      },
+      { windingRule: 'NONZERO', commandsBlob: 'invalid' }
+    ])
+    ynode.set('strokeGeometry', 'invalid')
+
+    const props = decodeNodeFromYjs(ynode)
+    const source = props.source as SceneNode['source']
+    const fillGeometry = props.fillGeometry as GeometryPath[]
+
+    expect(source).toEqual({
+      ...createDefaultSourceMetadata(),
+      format: 'fig'
+    })
+    expect(fillGeometry).toHaveLength(1)
+    expect(fillGeometry[0]?.commandsBlob).toBeInstanceOf(Uint8Array)
+    expect(fillGeometry[0]?.fills).toHaveLength(2)
+    expect(fillGeometry[0]?.fills?.[0]).toMatchObject({
+      type: 'GRADIENT_LINEAR',
+      opacity: 0.8,
+      gradientStops: [
+        { color: { r: 1, g: 0, b: 0, a: 1 }, position: 0 },
+        { color: { r: 0, g: 0, b: 1, a: 1 }, position: 1 }
+      ],
+      gradientTransform: { m00: 1, m01: 0, m02: 0, m10: 0, m11: 1, m12: 0 }
+    })
+    expect(fillGeometry[0]?.fills?.[1]?.type).toBe('SOLID')
+    expect(props.strokeGeometry).toEqual([])
+  })
+
   test('binary geometry fields round-trip as Uint8Array, not strings', () => {
     const host = new SceneGraph()
     const page = firstPage(host)
@@ -163,9 +251,9 @@ describe('collab yjs-sync', () => {
     const doc = new Y.Doc()
     const ynode = new Y.Map<unknown>()
     doc.getMap<Y.Map<unknown>>('nodes').set(ellipse.id, ynode)
-    syncNodePropsToYMap(ellipse, ynode)
+    syncEncodedNodeToYMap(ellipse, ynode)
     blob[0] = 99
-    const props = yNodeToProps(ynode)
+    const props = decodeNodeFromYjs(ynode)
 
     expect(typeof ynode.get('fillGeometry')).not.toBe('string')
     const decoded = props.fillGeometry as GeometryPath[]
@@ -207,11 +295,11 @@ describe('collab yjs-sync', () => {
     doc.transact(() => {
       const pageYnode = new Y.Map<unknown>()
       ynodes.set(hostPage.id, pageYnode)
-      syncNodePropsToYMap({ ...hostPage, childIds: [] } as SceneNode, pageYnode)
+      syncEncodedNodeToYMap({ ...hostPage, childIds: [] } as SceneNode, pageYnode)
 
       const rectYnode = new Y.Map<unknown>()
       ynodes.set(rect.id, rectYnode)
-      syncNodePropsToYMap(rect, rectYnode)
+      syncEncodedNodeToYMap(rect, rectYnode)
     })
 
     const peer = new SceneGraph()