fix(editor): accept plugin node kinds in the scene save API

apps/editor/lib/graph-schema.test.ts

@@ -0,0 +1,167 @@
+import { expect, test } from 'bun:test'
+import { apiGraphSchema } from './graph-schema'
+
+function buildGraph(nodes: Record<string, unknown>, rootNodeIds: string[] = []) {
+  return { nodes, rootNodeIds }
+}
+
+const LEVEL_ID = 'level_a1b2c3d4e5f6g7h8'
+const TREE_ID = 'tree_a1b2c3d4e5f6g7h8'
+
+const level = (children: string[] = []) => ({
+  object: 'node',
+  id: LEVEL_ID,
+  type: 'level',
+  parentId: null,
+  children,
+  level: 0,
+})
+
+const pluginTree = (overrides: Record<string, unknown> = {}) => ({
+  object: 'node',
+  id: TREE_ID,
+  type: 'trees:tree',
+  parentId: LEVEL_ID,
+  position: [1, 0, 2],
+  rotation: 0,
+  ...overrides,
+})
+
+test('accepts a graph containing a plugin node kind', () => {
+  const graph = buildGraph({ [TREE_ID]: pluginTree() }, [LEVEL_ID])
+
+  expect(apiGraphSchema.safeParse(graph).success).toBe(true)
+})
+
+test('accepts a builtin container whose children include a plugin node id', () => {
+  const graph = buildGraph({ [LEVEL_ID]: level([TREE_ID]), [TREE_ID]: pluginTree() }, [LEVEL_ID])
+
+  expect(apiGraphSchema.safeParse(graph).success).toBe(true)
+})
+
+test('keeps plugin child ids in the parsed graph', () => {
+  const graph = buildGraph({ [LEVEL_ID]: level([TREE_ID]), [TREE_ID]: pluginTree() }, [LEVEL_ID])
+
+  const res = apiGraphSchema.safeParse(graph)
+
+  expect(res.success).toBe(true)
+  expect((res.data?.nodes[LEVEL_ID] as { children: string[] }).children).toEqual([TREE_ID])
+})
+
+test('preserves installedPlugins alongside a plugin node', () => {
+  const res = apiGraphSchema.safeParse({
+    ...buildGraph({ [TREE_ID]: pluginTree() }, [LEVEL_ID]),
+    installedPlugins: ['pascal:trees'],
+  })
+
+  expect(res.success).toBe(true)
+  expect(res.data?.installedPlugins).toEqual(['pascal:trees'])
+})
+
+test('rejects a plugin node that fails the base envelope', () => {
+  const graph = buildGraph({ tree_bad: pluginTree({ id: 42 }) })
+
+  expect(apiGraphSchema.safeParse(graph).success).toBe(false)
+})
+
+// The `AssetUrl` allowlist is the whole Phase 3 posture: every scheme outside
+// it is rejected, so this list does not have to be exhaustive to be sound. A
+// denylist would — which is why one isn't used. `169.254.169.254` is the cloud
+// instance-metadata endpoint, the canonical SSRF target.
+test('rejects URL-shaped plugin fields outside the AssetUrl allowlist', () => {
+  for (const url of [
+    'javascript:alert(1)',
+    ' file:///etc/passwd',
+    'data:text/html,<script>1</script>',
+    'http://169.254.169.254/latest/meta-data',
+    'http://evil.example/beacon.png',
+    'ws://evil.example/socket',
+    'gopher://evil.example/x',
+    'about:blank',
+    // C0 controls inside the scheme: a browser ignores them and navigates, so
+    // a prefix match on the raw string is not enough.
+    'java\tscript:alert(1)',
+    '\u0000javascript:alert(1)',
+    // Scheme matching must be case-insensitive.
+    'DATA:TEXT/HTML,<script>1</script>',
+  ]) {
+    const graph = buildGraph({ [TREE_ID]: pluginTree({ config: { textures: [{ src: url }] } }) })
+
+    const res = apiGraphSchema.safeParse(graph)
+
+    expect(res.success, `expected ${JSON.stringify(url)} to be rejected`).toBe(false)
+    expect(res.error?.issues[0]?.message).toBe('URL is not in the allowed scheme list')
+  }
+})
+
+test('accepts the asset URL forms core allows', () => {
+  for (const url of [
+    'data:image/png;base64,iVBORw0KGgo=',
+    'https://cdn.example/tree.webp',
+    'asset://tree-bark',
+    'blob:https://editor.pascal.app/9f1c',
+    '/textures/bark.webp',
+    'http://localhost:3000/textures/bark.webp',
+  ]) {
+    const graph = buildGraph({ [TREE_ID]: pluginTree({ thumbnail: url }) })
+
+    expect(apiGraphSchema.safeParse(graph).success, `expected ${url} to be accepted`).toBe(true)
+  }
+})
+
+// Prose that happens to start with a word and a colon is not a URL. A plugin
+// may put arbitrary text in `name` / `metadata`, exactly as builtin nodes do —
+// the allowlist applies to URL-shaped values, not to every string.
+test('does not treat prose or drive paths as URLs', () => {
+  for (const text of [
+    'FTP: north bed',
+    'note: see plan 3',
+    'Data: unavailable',
+    'C:\\Users\\me\\plan.png',
+    'Oak tree',
+  ]) {
+    const graph = buildGraph({ [TREE_ID]: pluginTree({ name: text, metadata: { note: text } }) })
+
+    expect(apiGraphSchema.safeParse(graph).success, `expected ${text} to be accepted`).toBe(true)
+  }
+})
+
+// A recursive walk over untrusted JSON must not throw past `safeParse` — the
+// route would answer 500 where the contract is a 400 with issues.
+test('reports deeply nested plugin nodes as a validation issue, not a crash', () => {
+  let nested: unknown = 'leaf'
+  for (let i = 0; i < 100_000; i++) nested = [nested]
+  const graph = buildGraph({ [TREE_ID]: pluginTree({ nested }) })
+
+  const res = apiGraphSchema.safeParse(graph)
+
+  expect(res.success).toBe(false)
+  expect(res.error?.issues[0]?.message).toBe('Node is too deeply nested to validate')
+})
+
+test('still rejects invalid builtin nodes', () => {
+  const graph = buildGraph({
+    wall_bad: { object: 'node', id: 'wall_a1b2c3d4e5f6g7h8', type: 'wall' },
+  })
+
+  expect(apiGraphSchema.safeParse(graph).success).toBe(false)
+})
+
+// Unnamespaced kinds are legitimate: `wiki/architecture/plugin-authoring.md`
+// requires plugin *ids* to look like `vendor:pack`, never kinds, and its worked
+// example registers `kind: 'couch'`. Membership is decided by "not in AnyNode",
+// so such a node is validated as foreign rather than rejected outright.
+test('treats an unnamespaced unknown type as a foreign node', () => {
+  const couch = {
+    object: 'node',
+    id: 'couch_a1b2c3d4e5f6g7h8',
+    type: 'couch',
+    parentId: LEVEL_ID,
+  }
+
+  expect(apiGraphSchema.safeParse(buildGraph({ [couch.id]: couch })).success).toBe(true)
+  expect(
+    apiGraphSchema.safeParse(buildGraph({ [couch.id]: { ...couch, src: 'javascript:alert(1)' } }))
+      .success,
+  ).toBe(false)
+})

apps/editor/lib/graph-schema.ts

@@ -1,4 +1,4 @@
-import { AnyNode } from '@pascal-app/core/schema'
+import { AnyNode, AssetUrl, BaseNode } from '@pascal-app/core/schema'
 import { z } from 'zod'
 
 /**
@@ -11,7 +11,90 @@ import { z } from 'zod'
  * route can silently accept malicious URLs via the `graph` payload.
  *
  * Phase 8 P4 found the POST bypass; Phase 10 A2 found the PUT bypass.
+ *
+ * Nodes minted by plugins (`trees:tree`, plus anything a third-party pack
+ * registers) are outside `AnyNode`, and their schemas live in packages that
+ * pull in renderer/UI code a route handler must not import — the node registry
+ * is empty in this process regardless, since nothing registers kinds
+ * server-side. Such a node is validated the way `validate-build-json` treats a
+ * kind it cannot resolve: as a foreign node, held to the `BaseNode` envelope
+ * plus the same `AssetUrl` allowlist applied to every URL-shaped string it
+ * carries. That keeps the Phase 3 posture on a branch that must accept unknown
+ * fields without resorting to a denylist, which would have to enumerate every
+ * hostile scheme where `AssetUrl` already enumerates the safe ones.
+ */
+
+const KNOWN_TYPES = new Set<string>(
+  AnyNode.options.map((o) => o.shape.type.parse(undefined) as string),
+)
+
+/** The envelope every persisted node satisfies, builtin or foreign. */
+const ForeignNodeEnvelope = BaseNode.extend({
+  type: z.string().min(1),
+  children: z.array(z.string()).optional(),
+}).loose()
+
+// A walk over attacker-shaped JSON needs both bounds: depth, so a nest of
+// arrays can't overflow the stack (an exception thrown past `safeParse` is a
+// 500, not the 400 the caller is owed), and a visit count, so a wide-but-flat
+// body can't burn the request budget.
+const MAX_SCAN_DEPTH = 48
+const MAX_SCAN_VALUES = 50_000
+
+/**
+ * Whether a string is trying to be a URL, and so has to satisfy `AssetUrl`.
+ *
+ * C0 controls are stripped first: browsers ignore them inside a scheme, so
+ * `java\tscript:alert(1)` navigates while reading as free text to a naive
+ * prefix match. The scheme must be followed immediately by a non-space — that
+ * is what separates `ftp://host/x` from prose like `FTP: north bed`, which a
+ * plugin is entitled to store in a node name. The two-character minimum on the
+ * scheme leaves Windows drive paths (`C:\…`) as free text; every scheme worth
+ * rejecting is longer than one letter.
  */
+// biome-ignore lint/suspicious/noControlCharactersInRegex: stripping C0 controls is the point — browsers ignore them mid-scheme.
+const CONTROL_CHARS = /[\u0000-\u001f\u007f]/g
+const SCHEME = /^[a-z][a-z0-9+.-]+:[^\s]/i
+function isUrlShaped(value: string): boolean {
+  const bare = value.replace(CONTROL_CHARS, '').trimStart()
+  return SCHEME.test(bare) || bare.startsWith('//')
+}
+
+type ScanResult = { path: (string | number)[]; reason: 'url' | 'too-complex' }
+
+/**
+ * First URL-shaped string `AssetUrl` rejects, or a breach of the walk bounds.
+ *
+ * The node's own `type` is exempt: a namespaced kind (`trees:tree`) is
+ * scheme-shaped by construction, and the envelope has already validated it.
+ */
+function findRejectedUrl(root: unknown): ScanResult | null {
+  let budget = MAX_SCAN_VALUES
+  const walk = (value: unknown, path: (string | number)[], depth: number): ScanResult | null => {
+    if (depth > MAX_SCAN_DEPTH || budget-- <= 0) return { path, reason: 'too-complex' }
+    if (typeof value === 'string') {
+      if (isUrlShaped(value) && !AssetUrl.safeParse(value).success) return { path, reason: 'url' }
+      return null
+    }
+    if (Array.isArray(value)) {
+      for (let i = 0; i < value.length; i++) {
+        const hit = walk(value[i], [...path, i], depth + 1)
+        if (hit) return hit
+      }
+      return null
+    }
+    if (value && typeof value === 'object') {
+      for (const [key, child] of Object.entries(value)) {
+        if (depth === 0 && key === 'type') continue
+        const hit = walk(child, [...path, key], depth + 1)
+        if (hit) return hit
+      }
+    }
+    return null
+  }
+  return walk(root, [], 0)
+}
+
 export const apiGraphSchema = z
   .object({
     nodes: z.record(z.string(), z.unknown()),
@@ -20,16 +103,57 @@ export const apiGraphSchema = z
     installedPlugins: z.array(z.string().min(1)).optional(),
   })
   .superRefine((value, ctx) => {
+    const addIssues = (nodeId: string, error: z.ZodError) => {
+      for (const issue of error.issues) {
+        ctx.addIssue({
+          code: 'custom',
+          path: ['nodes', nodeId, ...issue.path],
+          message: issue.message,
+        })
+      }
+    }
+
+    // Ids of foreign nodes in this graph. Builtin container schemas name the
+    // child kinds they accept (`BuildingNode.children`, `RoofNode.children`),
+    // so a container holding a plugin child fails against `AnyNode` even
+    // though the relationship is legitimate. Those ids are dropped from a
+    // *copy* handed to `AnyNode`; the stored graph keeps them, and each
+    // foreign node is still validated on its own.
+    const foreignIds = new Set<string>()
+    for (const [nodeId, node] of Object.entries(value.nodes)) {
+      const type = (node as { type?: unknown } | null)?.type
+      if (typeof type === 'string' && !KNOWN_TYPES.has(type)) foreignIds.add(nodeId)
+    }
+
     for (const [nodeId, node] of Object.entries(value.nodes)) {
-      const res = AnyNode.safeParse(node)
-      if (!res.success) {
-        for (const issue of res.error.issues) {
+      const type = (node as { type?: unknown } | null)?.type
+
+      if (typeof type === 'string' && !KNOWN_TYPES.has(type)) {
+        const res = ForeignNodeEnvelope.safeParse(node)
+        if (!res.success) {
+          addIssues(nodeId, res.error)
+          continue
+        }
+        const rejected = findRejectedUrl(node)
+        if (rejected) {
           ctx.addIssue({
             code: 'custom',
-            path: ['nodes', nodeId, ...issue.path],
-            message: issue.message,
+            path: ['nodes', nodeId, ...rejected.path],
+            message:
+              rejected.reason === 'url'
+                ? 'URL is not in the allowed scheme list'
+                : 'Node is too deeply nested to validate',
           })
         }
+        continue
       }
+
+      const children = (node as { children?: unknown } | null)?.children
+      const candidate =
+        foreignIds.size > 0 && Array.isArray(children)
+          ? { ...(node as object), children: children.filter((c) => !foreignIds.has(c as string)) }
+          : node
+      const res = AnyNode.safeParse(candidate)
+      if (!res.success) addIssues(nodeId, res.error)
     }
   })

packages/core/src/schema/index.ts

@@ -6,6 +6,8 @@ export {
   type SolarPanelPresetDims,
   SolarPanelPresetKey,
 } from '../solar-panel-presets'
+// Asset URL allowlist
+export { ALLOWED_ORIGINS_ENV, ALLOWED_SCHEMES, AssetUrl } from './asset-url'
 export { BaseNode, generateId, Material, nodeType, objectId } from './base'
 // Camera
 export { CameraSchema } from './camera'