feat(mcp,editor): Option A+B storage + 10 agent deliverables (Phase 7)
Ships the combined filesystem/Supabase storage adapter + MCP scene lifecycle tools + Next.js API routes + editor /scene/[id] route, so an MCP save is directly openable at /scene/<id> without any injection hack. End-to-end verified: 10/10 e2e steps pass. Storage (A1/A2/A3): - SceneStore interface + error classes + slug helpers - FilesystemSceneStore at $PASCAL_DATA_DIR (defaults XDG/~/.pascal) with atomic writes, .index sidecar, optimistic locking - SupabaseSceneStore with scenes + scene_revisions tables, RLS migration SQL, mock-backed unit tests - createSceneStore(env) auto-selects based on SUPABASE_URL + SUPABASE_SERVICE_ROLE_KEY MCP tools (A4, A8, A9, A10): - save_scene / load_scene / list_scenes / delete_scene / rename_scene - list_templates / create_from_template (3 seed templates: empty-studio, two-bedroom, garden-house) - generate_variants (7 mutation kinds, seeded RNG, save=true|false) - photo_to_scene (vision sampling → scene graph → save) Editor (A5, A6): - /api/scenes + /api/scenes/[id] with RFC 7232 If-Match locking - /scene/[id] and /scenes route pages with save button, SceneLoader - Removed the window.__pascalScene dev injection hack Security + UX edges (A7, A8): - AssetUrl Zod validator: asset:// blob: data:image/ /path https: (http://localhost for dev) + PASCAL_ALLOWED_ASSET_ORIGINS env allowlist. Hardens scan.url, guide.url, item.asset.src, material.texture.url, MaterialMaps.*Map - Auto-frame camera on empty→non-empty scene transition (camera-controls:fit-scene emitter event) Shared utilities: - rehydrateSiteChildren() extracted to packages/mcp/src/lib/ and used by both create-from-template and generate-variants to work around the SiteNode.children-as-objects vs. ids inconsistency (CROSS_CUTTING §2) - Storage + MCP subpath exports added to packages/mcp/package.json (CROSS_CUTTING §4) Tests: 293 pass / 0 fail across 40 files (was 142 pre-Phase-7). Biome: clean. Phase-7 e2e script at packages/mcp/test-reports/phase7-e2e.ts: MCP HTTP + editor Next.js both point at $PASCAL_DATA_DIR = /tmp/pascal-e2e, save_scene from MCP, GET /api/scenes/<id> from editor server, /scenes list page renders all saved scenes, scene page renders SceneLoader, delete_scene works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Adrian Perez
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
42bd05db9c
commit
e8d0b13ff5
@@ -95,3 +95,107 @@ Delete `.github/workflows/mcp-ci.yml`.
|
||||
|
||||
---
|
||||
|
||||
## 4. `packages/mcp/package.json` — added `./storage` subpath export
|
||||
|
||||
### What
|
||||
|
||||
Added a `./storage` entry to the `"exports"` map of `@pascal-app/mcp`, pointing at the built `dist/storage/index.{js,d.ts}`. The existing `"."` entry is unchanged.
|
||||
|
||||
### Why
|
||||
|
||||
The Next.js editor (`apps/editor`) needs access to `createSceneStore()` + the `SceneStore` types/errors in server-only code (API route handlers + `lib/scene-store-server.ts`). The main entry `.` pulls in the full MCP server surface (tools, transports, MCP SDK), which is overkill for a consumer that only needs the storage adapter. The subpath export lets `apps/editor` do `import { createSceneStore, SceneVersionConflictError } from '@pascal-app/mcp/storage'` without dragging the rest of the package.
|
||||
|
||||
### Impact
|
||||
|
||||
Zero on existing consumers. Purely additive. The `.` entry continues to export `SceneBridge`, `createPascalMcpServer`, etc., exactly as before.
|
||||
|
||||
### Reversibility
|
||||
|
||||
Remove the `./storage` entry from `exports` and update `apps/editor` to inline the types / use a different factory. No data or behavior changes — pure module-graph shaping.
|
||||
|
||||
### Related
|
||||
|
||||
- `apps/editor/package.json` adds `@pascal-app/mcp` as a workspace dependency so the subpath resolves.
|
||||
- `apps/editor/lib/scene-store-server.ts` and `apps/editor/app/api/scenes/**` consume this subpath.
|
||||
|
||||
---
|
||||
|
||||
## 5. `packages/core/src/schema/asset-url.ts` — URL scheme allowlist on scene URL fields
|
||||
|
||||
### What
|
||||
|
||||
Introduced a shared `AssetUrl` Zod validator and replaced the bare `z.string()`
|
||||
on every URL-bearing field in core's schemas:
|
||||
|
||||
- `scan.url` (`packages/core/src/schema/nodes/scan.ts`)
|
||||
- `guide.url` (`packages/core/src/schema/nodes/guide.ts`)
|
||||
- `item.asset.src` (`packages/core/src/schema/nodes/item.ts`)
|
||||
- `material.texture.url` (`packages/core/src/schema/material.ts`)
|
||||
- `material.maps.*` (`albedoMap`, `normalMap`, `roughnessMap`, `metalnessMap`,
|
||||
`aoMap`, `displacementMap`, `emissiveMap`, `bumpMap`, `alphaMap`, `lightMap`)
|
||||
|
||||
The validator accepts `asset://…`, `blob:…`, `data:image/…`, `/…` app-relative
|
||||
paths, `https://…`, and `http://localhost|127.0.0.1/…`. Optional origin
|
||||
narrowing via `process.env.PASCAL_ALLOWED_ASSET_ORIGINS` (comma-separated).
|
||||
Rejects `javascript:`, `file:`, `ftp:`, `ws:`, `data:text/html`,
|
||||
`data:application/*`, link-local / private IPs over bare `http`, empty strings,
|
||||
and non-URL garbage.
|
||||
|
||||
### Why
|
||||
|
||||
Phase 3 security audit (`packages/mcp/test-reports/research/R9-production-readiness.md`
|
||||
entry "URL validation in scenes"): an attacker-crafted scene containing
|
||||
`javascript:alert(1)` or `http://169.254.169.254/latest/meta-data/` for a
|
||||
texture URL would beacon or exfiltrate when the editor renders it.
|
||||
`AnyNode.safeParse`, used by the MCP bridge, now rejects those payloads at the
|
||||
schema boundary.
|
||||
|
||||
### Impact
|
||||
|
||||
- **Existing scenes**: localStorage-resident scenes bypass strict validation on
|
||||
load (the store's `setScene` only runs `safeParse` on stair-type via
|
||||
`migrateNodes`), so this is *not* a breakage for returning users. Legacy
|
||||
URLs will keep loading; only explicit MCP-bridge `safeParse` calls reject.
|
||||
- **MCP consumers**: one existing test
|
||||
(`packages/mcp/src/bridge/scene-bridge.test.ts`, previously using
|
||||
`src: 'data:model/gltf-binary;base64,'`) now fails because `data:model/` is
|
||||
not in the allowlist. Replaced with `asset://test/chair.glb` — the only
|
||||
sanctioned scheme for an in-repo ItemNode fixture.
|
||||
- **Other packages**: `@pascal-app/viewer`, `@pascal-app/editor`, and
|
||||
`material-library.ts` all continue to work because every built-in URL is a
|
||||
`/material/…` app-relative path (allowlisted).
|
||||
|
||||
### Known gaps / follow-ups
|
||||
|
||||
1. **`item.asset.thumbnail` stayed untyped** — the field is still bare
|
||||
`z.string()` in `item.ts`. The Phase 3 audit called it out alongside `src`,
|
||||
but the Phase 7 task scope only required `src`. Follow-up: apply `AssetUrl`
|
||||
to `thumbnail` as well. Verify the `place-item` tool's default
|
||||
`thumbnail: ''` (currently empty string) gets a proper fallback first.
|
||||
2. **`dist/` pollution** — `packages/core/tsconfig.json` `include`s `src` and
|
||||
doesn't exclude `**/*.test.ts`, so the new `asset-url.test.ts` is emitted
|
||||
to `dist/schema/`. Harmless (nothing imports it), but should be excluded
|
||||
for a clean publish. Mirror the `exclude: ["**/*.test.ts"]` pattern used
|
||||
in `packages/mcp/tsconfig.json`. Out of scope for A7 because tsconfig is
|
||||
not in the ownership list.
|
||||
3. **`bun:test` typing** — the test file uses `@ts-expect-error` on its
|
||||
`bun:test` import because `@pascal-app/core` does not depend on
|
||||
`@types/bun`. Adding it as a dev dep (or, preferred, excluding tests from
|
||||
the core tsc build per gap 2) would remove the directive.
|
||||
4. **`data:image/svg+xml` loophole** — passes the validator because it starts
|
||||
with `data:image/`, but SVG can carry inline scripts. If the editor ever
|
||||
renders SVG via unsanitised HTML-injection APIs or `<foreignObject>`, this
|
||||
becomes an injection vector. Consider a stricter variant
|
||||
(`data:image/(png|jpe?g|webp|gif)`) for texture slots where SVG isn't needed.
|
||||
5. **Same-origin HTTP scenes** — `http://localhost` is allowed for dev, but a
|
||||
scene persisted in dev and shared in prod will still validate. Consider
|
||||
gating on `NODE_ENV` once we have a stable env-flag story.
|
||||
|
||||
### Reversibility
|
||||
|
||||
Delete `packages/core/src/schema/asset-url.ts` and revert the five imports in
|
||||
`scan.ts`, `guide.ts`, `item.ts`, and `material.ts` to `z.string()`. The
|
||||
scene-bridge test update is self-contained.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user