Skip to content

FM-MAP1: maps-as-item, Path B viewer + scene-Notes deprecation + visibility filter (security)#27

Merged
keyxmakerx merged 1 commit into
mainfrom
claude/maps-as-item-NbIhE
May 8, 2026
Merged

FM-MAP1: maps-as-item, Path B viewer + scene-Notes deprecation + visibility filter (security)#27
keyxmakerx merged 1 commit into
mainfrom
claude/maps-as-item-NbIhE

Conversation

@keyxmakerx

Copy link
Copy Markdown
Owner

Security headline — visibility leak fix

The previous Notes-on-Scenes path leaked DM-only data to players. Two specific defects, both gone in this PR:

  • _chronicleMarkerToFoundry (map-sync.mjs:526-556 on the prior revision) ignored visibility and visibility_rules entirely. Every Chronicle marker became a Foundry Note on a Scene, which Foundry then replicated to every player regardless of audience. Any campaign that synced markers had a leak in production.
  • _foundryNoteToChronicle (map-sync.mjs:580-591) hardcoded visibility: 'everyone' on outgoing markers — Foundry GMs could not create dm_only markers via Foundry at all.

Both code paths are removed. The replacement filters at GM-side materialization with render-time defense-in-depth on every client.

Visibility model (Phase A)

Data Player flag GM memory Render filter
Markers visibility=dm_only ❌ never GM-only display
Markers visibility=everyone ✅ with visibility_rules per-user allowed_users/denied_users
Drawings is_hidden=true ❌ never GM-only display
Drawings is_visible=false ❌ not stored not rendered
Tokens, layers none (no per-user vis in schema)
Fog of war ❌ never DOM element absent for non-GM

Acknowledged residual leak (FM-MAP3 follow-up)

Markers with visibility=everyone AND visibility_rules.allowed_users non-empty are flag-stored with the rules data attached. The render filter prevents display for non-allowed users, but a DOM inspection would surface name/description. Closing this fully needs per-user materialization, which tracks with Chronicle's eventual per-user auth model.

Filed as FM-MAP3 and queued behind FM-MAP2.

Player-facing surface walk (non-GM audit)

Every surface confirmed clean by mental walkthrough as a non-GM test user:

  • JournalEntry flags inspection (DOM): only visibility=everyone markers and is_visible && !is_hidden drawings reach flags. Fog never written.
  • MapViewerSheet on player: _getMapSync() returns uninitialized MapSync (init never runs for non-GM); falls through to flag-only render path. Render filter applies visibility_rules per user. Fog SVG element absent from DOM, not just CSS-hidden.
  • Right-click on Chronicle marker: handler returns early for non-GM; cannot open marker config.
  • Chronicle-tools toolbar ("Place Chronicle marker", "Open in Chronicle"): gated {{#if isGM}} in template; players never see the controls.
  • Sync dashboard: GM-only window; not a leak vector.

Architecture (Path B, locked per dispatch)

Chronicle maps materialize as JournalEntries (one entry per map, image-type page) inside an auto-created "Chronicle Maps" folder. The MapViewerSheet renders markers, drawings, tokens, fog, and layers as SVG/DOM overlays inside the existing pan/zoom container. Foundry's Scene system stays for combat/lighting/walls — Chronicle maps no longer touch it.

Materialization is idempotent by mapId page flag (not by name — users may rename folders/entries). Re-running on every world load is safe.

What got removed

  • map-sync.mjs Scene Notes wiring: createNote/updateNote/deleteNote hook chain, _onMarkerCreated → scene.createEmbeddedDocuments('Note'), scene.getFlag('mapId') reads, "Link to Chronicle Map" scene-nav menu.
  • sync-dashboard.mjs: link-scene/unlink-scene/link-map controls and handlers. Map tab now shows "Open in Foundry" (opens materialized JournalEntry) and "Open in Chronicle" (deep-link), with marker/drawing/token counts.

What got added

  • map-sync.mjs API/sync layer: in-memory cache of all sub-resources per map, visibility-filtered flag writes, materialization, archive-on-delete, WS handlers for map.*, marker.*, drawing.*, token.*, layer.*, fog.*, and a 5s polling fallback marked // TODO(FM-MAP1-WS) for sub-resource types Chronicle does not yet emit. Markers are NOT polled — Chronicle's MapEventPublisher already emits marker.* events (verified per Chronicle audit).
  • map-viewer.mjs: SVG drawings overlay (rectangle/ellipse/line/polygon/freehand), token sprites with HP bars, fog overlay (GM-only, absent from DOM otherwise), Chronicle marker rendering with solid-fill visual distinction and audience tooltip, render-time visibility filter, and a GM-only "Place Chronicle marker" toolbar button feeding a new ChronicleMarkerConfigDialog (with visibility=dm_only/everyone selector — closes the second visibility defect).
  • Local pins coexist with Chronicle markers: dotted outline vs solid fill, per-pin tooltips ("Personal annotation" vs "Chronicle marker, visible to X"), separate right-click menus, no menu collision. Local pin behavior is otherwise unchanged.
  • archive-on-delete: when Chronicle reports map.deleted, the JournalEntry is preserved if local pins exist on the page; the mapId/chronicleMapMeta flags are stripped, the entry name gets "(archived)" suffix, and the user is notified. Local annotations are user data and are never auto-deleted.
  • WS allowlist extended (api-client.mjs:18-25) for new sub-resource types.

Out of scope (deferred)

  • Editor UI for drawings/tokens/fog/layers — Phase B / FM-MAP2.
  • expected_updated_at concurrency on map sub-resource writes — Chronicle C-MAP1 ships server-side first; MapSync writes will adopt it in a follow-up.
  • Per-Foundry-user API keys.
  • Wizard import "link-map" step still sets a stale Scene flag; harmless and will be cleaned up in a wizard refresh PR.

Test plan

  • Install on a Foundry world with an existing Chronicle campaign that has maps, markers, drawings, tokens, fog, and layers.
  • On first connect as GM: confirm "Chronicle Maps" folder is auto-created and all maps materialize as JournalEntries with image-type pages.
  • Re-run by reloading the world: confirm idempotency (no duplicate entries, names not clobbered if the user renamed an entry, mapId flag preserved).
  • Open a map's JournalEntry as GM: confirm markers, drawings, tokens, fog (GM-only), and layers all render.
  • As a non-GM player: open the same JournalEntry; confirm DM-only markers absent, hidden drawings absent, fog overlay absent. DOM-inspect to verify chronicleMarkers flag contains no visibility=dm_only entries and chronicleDrawings contains no is_hidden=true entries.
  • Create a marker via the GM "Place Chronicle marker" toolbar button with visibility=dm_only; confirm WS event lands, GM sees it, player does not.
  • Place a local pin on a Chronicle map page; confirm dotted-outline visual, "Personal annotation" tooltip, no collision with Chronicle marker right-click.
  • Simulate map.deleted for a map with local pins → confirm JournalEntry archived (name suffix, flags stripped, pins preserved). For a map without local pins → confirm JournalEntry deleted.
  • Watch console for // TODO(FM-MAP1-WS) polling activity (5s interval) while a Chronicle-linked viewer is open; confirm polling stops on close.
  • Confirm marker WS path works end-to-end without polling fallback (Chronicle MapEventPublisher already emits).

Generated by Claude Code

…bility filter (security)

Headline: closes the marker visibility leak that existed in the previous
Notes-on-Scenes path. _chronicleMarkerToFoundry ignored visibility/visibility_rules,
and _foundryNoteToChronicle hardcoded visibility:'everyone'. Both paths are gone.

Architecture (Path B)

Chronicle maps now materialize as JournalEntries (one entry per map, image-type
page) inside an auto-created "Chronicle Maps" folder. The MapViewerSheet renders
markers, drawings, tokens, fog, and layers as SVG/DOM overlays inside the
existing pan/zoom container. The Foundry Scene system is no longer used for
Chronicle maps — Scenes remain available for Foundry's combat/lighting/walls
purposes, untouched.

Materialization is idempotent by mapId page flag (not by name — users may
rename folders/entries). Re-running on every world load is safe.

Visibility model (Phase A)

Filtering happens at GM-side materialization, with render-time defense-in-depth
on every client:

- Markers visibility=dm_only → never written to flags. GM memory only.
- Markers visibility=everyone → flag-stored. visibility_rules embedded; the
  client filters at render time per the user's mapped Chronicle id.
- Drawings is_hidden=true → GM memory only.
- Drawings is_visible=false → not stored.
- Tokens, layers → flag-stored (no per-user vis in schema).
- Fog of war → GM memory only. Server filtering plus client-side absence
  (not "hidden via CSS" — the SVG element is not in the DOM for non-GM).

Acknowledged Phase A leak: markers carrying visibility_rules.allowed_users
have name/description in player flag DOM (since the marker's visibility=everyone
qualifies it for flag storage). The render filter prevents display, but a
DOM inspection would surface the data. Closing this requires per-user
materialization (out of scope; tracks with Chronicle's eventual per-user auth).

What got removed

- map-sync.mjs Scene Notes wiring: the entire createNote/updateNote/deleteNote
  hook chain, _onMarkerCreated → scene.createEmbeddedDocuments('Note'),
  scene.getFlag mapId reads, and the "Link to Chronicle Map" scene-nav menu.
- sync-dashboard.mjs: link-scene/unlink-scene/link-map controls and handlers.
  The map tab now shows "Open in Foundry" (opens the materialized JournalEntry)
  and "Open in Chronicle" (deep-link), with marker/drawing/token counts.

What got added

- map-sync.mjs API/sync layer: in-memory cache of all sub-resources per map,
  visibility-filtered flag writes, materialization, archive-on-delete,
  WS handlers for map.*, marker.*, drawing.*, token.*, layer.*, fog.*,
  and a 5s polling fallback marked TODO(FM-MAP1-WS) for sub-resource types
  Chronicle does not yet emit (drawings/tokens/layers/fog). Markers are
  not polled — Chronicle's MapEventPublisher already emits marker.* events.
- map-viewer.mjs: SVG drawings overlay (rectangle/ellipse/line/polygon/freehand),
  token sprites with HP bars, fog overlay (GM-only, absent from DOM otherwise),
  Chronicle marker rendering with solid-fill visual distinction and audience
  tooltip, render-time visibility filter, and a GM-only "Place Chronicle marker"
  toolbar button feeding a new ChronicleMarkerConfigDialog (with visibility=
  dm_only/everyone selector).
- Local pins coexist with Chronicle markers: dotted outline vs solid fill,
  per-pin tooltips ("Personal annotation" vs "Chronicle marker, visible to X"),
  separate right-click menus, no menu collision. Local pin behavior is
  otherwise unchanged.
- archive-on-delete: when Chronicle reports map.deleted, the JournalEntry is
  preserved if local pins exist on the page; the mapId/chronicleMapMeta flags
  are stripped, the entry name gets "(archived)" suffix, and the user is
  notified. Local annotations are user data and are never auto-deleted.
- WS allowlist extended (api-client.mjs:18-25) for the new sub-resource types.

Visibility-leak walk (every player-facing surface confirmed clean)

- JournalEntry flag inspection: only visibility=everyone markers and is_visible
  drawings reach flags. is_hidden drawings, dm_only markers, and fog never do.
- MapViewerSheet on player: _getMapSync returns uninitialized MapSync (init
  never runs for non-GM); falls through to flag-only render path. Render
  filter applies visibility_rules per user. Fog SVG element absent from DOM.
- Right-click marker: handler returns early for non-GM; cannot open editor.
- Chronicle-tools toolbar (place marker / open in Chronicle): gated {{#if isGM}}.

Out of scope (deferred)

- Editor UI for drawings/tokens/fog/layers (Phase B / FM-MAP2).
- expected_updated_at concurrency on map sub-resource writes (Chronicle C-MAP1
  ships server-side first; MapSync writes will adopt it in a follow-up).
- Per-Foundry-user API keys.
- Wizard import "link-map" step still sets a stale scene flag; harmless and
  will be cleaned up in a wizard refresh PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants