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

[konevenkatesh]

Problem

Placing any plugin node (e.g. trees:tree from the first-party Nature plugin) in a saved scene makes every subsequent autosave fail with 400.

Two validation gaps in apiGraphSchema (apps/editor/lib/graph-schema.ts):

1. Every node is validated against the static AnyNode union, which doesn't include namespaced plugin kinds — the plugin node itself is rejected (Invalid enum value. Expected 'wall' | …, received 'trees:tree').
2. LevelNode.children only accepts builtin typed-id patterns, so a level containing a tree_… child id fails too.

Repro: run apps/editor, create a saved scene, place a tree from the Nature panel, watch PUT /api/scenes/[id] return 400 (the editor shows a persistent save error and no further changes persist).

Fix

• Nodes whose type matches a namespaced plugin kind (ns:kind) validate against the BaseNode envelope plus a deep scan that rejects dangerous URL schemes (javascript:, vbscript:, file:, ftp:, non-image data:) anywhere in the node. This preserves the Phase 3 SSRF / script-URL posture without importing plugin/renderer code into the API route (plugin schemas live in packages that pull in UI code).
• Builtin containers are validated against AnyNode with plugin child ids filtered from a copy — the stored graph is unchanged (a regression test pins this).

Verification

• 8 new regression tests in apps/editor/lib/graph-schema.test.ts (5 of 8 fail without the fix): plugin node accepted, level-with-plugin-child accepted, parsed graph keeps plugin child ids, bad envelope rejected, dangerous URL schemes rejected (with data:image/… still allowed), invalid builtin nodes and non-namespaced unknown types still rejected.
• End-to-end on the running app: placed a Nature oak in a saved scene — PUT /api/scenes/[id] returns 200 and the stored graph contains the trees:tree node. Replaying the identical payload against the pre-fix schema returns 400.
• bun test apps/editor/lib/ 16/16 pass; biome check clean; no new type errors.

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches untrusted graph validation at POST/PUT boundaries with new foreign-node logic; changes are security-sensitive but aim to preserve the existing AssetUrl/SSRF posture rather than relax it.

Overview
Fixes 400 autosave failures when saved scenes include plugin nodes (e.g. trees:tree) by extending apiGraphSchema so types outside AnyNode are treated as foreign nodes: BaseNode envelope plus a bounded deep walk that applies the core AssetUrl allowlist to URL-shaped strings (with C0-control stripping and prose/drive paths excluded).

Builtin containers still go through AnyNode, but plugin child ids are removed only from a validation copy so levels and similar nodes can list plugin children without changing what gets stored. AssetUrl is re-exported from @pascal-app/core/schema for the editor validator, and graph-schema.test.ts adds regression coverage for acceptance, SSRF-style URLs, nesting limits, and unchanged builtin rejection behavior.

^{Reviewed by Cursor Bugbot for commit 4ed4ea1. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Aymericr]

Thanks for this — the bug is real and worth fixing, and your tests are honest (I confirmed 5 of the 8 fail against main). I've reworked the implementation on your branch and want to be explicit about why, because two of the reasons are things I only found by running your code.

What I kept: the diagnosis, the container-children insight (validating a copy with foreign child ids stripped, so the stored graph is untouched), and most of your test cases.

What changed, and why.

1. The scheme denylist reopened what the strict schema was added to close. DANGEROUS_STRING enumerated the hostile schemes, so anything not listed passed. Running your branch, a plugin node accepted all of these:

http://169.254.169.254/latest/meta-data   ws:// and wss://
http://evil.example/beacon.png            gopher://
//evil.example/x                          about:blank
https://evil.example/x.png                (defeats PASCAL_ALLOWED_ASSET_ORIGINS entirely)

It also missed obfuscation, because the regex is start-anchored on the raw string while browsers ignore C0 controls inside a scheme: java\tscript:alert(1), \0javascript:alert(1), and uppercase DATA:IMAGE/svg+xml were all accepted.

Core already has the inverse of this — AssetUrl (packages/core/src/schema/asset-url.ts) enumerates the safe schemes. An allowlist doesn't have to be exhaustive to be sound, which is exactly the property you want at an untrusted boundary. The rewrite strips C0 controls, decides whether a string is URL-shaped, and hands it to AssetUrl. All of the above are now rejected, and the PASCAL_ALLOWED_ASSET_ORIGINS narrowing applies to plugin nodes too.

2. PLUGIN_KIND invented a constraint the registry doesn't have. Requiring type to match ^ns:kind$ would make the save API the de-facto spec for plugin naming: it rejects acme:my_couch, Acme:Couch, acme:furniture:couch, and the worked example in wiki/architecture/plugin-authoring.md, which registers kind: 'couch' with no namespace at all. Membership is now decided by "not in AnyNode" — the same test packages/core/src/validation/validate-build-json.ts uses via KNOWN_TYPES. That's the existing precedent for "a kind I can't resolve," and it doesn't constrain naming.

The regex had a second cost: because file/ftp/data/javascript/vbscript were denied anywhere in the node, a plugin namespaced ftp: was permanently unsaveable, and a node named "FTP: north bed" failed to save. Only URL-shaped values are checked now, so plugins can store prose in name/metadata exactly as builtin nodes do — which matters, since main already accepts javascript: inside a builtin wall's metadata. The posture is per-field AssetUrl on URL fields, not "no scary substring anywhere," and the fix now matches that.

3. findDangerousString recursed unbounded. A ~240 KB body with deep array nesting throws RangeError past safeParse, so the route answers 500 where the contract is a 400 with issues (main returns success: false on the same payload). The walk is now depth- and visit-bounded and reports a validation issue instead.

4. Rebased onto main. Your branch predated #501, so it dropped installedPlugins from the object schema — merging as-is would have silently stripped it on every save. Also worth noting: gap 2 in your description (LevelNode.children rejecting tree_… ids) was fixed on main by 3126b0b/08d1bbf0, so only gap 1 remained. The children-copy logic is still needed, but for BuildingNode/RoofNode, which do name their child kinds.

One implementation note you may find surprising: the node's own type is exempt from the URL scan, because a namespaced kind like trees:tree is scheme-shaped by construction and would otherwise flag itself.

AssetUrl is now exported from @pascal-app/core/schema so the API route can reach it. 11 tests, all passing; bun run check (1584 files) and check-types (9/9) clean.

Credit stays with you on the commit. Merging once CI is green.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 4ed4ea1. Configure here.}

[cursor]

Foreign ids use wrong key

Medium Severity

The foreignIds set is populated with the node's record key, but children arrays reference the node's id field. If these differ, foreign child nodes aren't correctly filtered before AnyNode validation, which can cause builtin containers to fail validation.

 

^{Reviewed by Cursor Bugbot for commit 4ed4ea1. Configure here.}