fix(canvas): enforce read-only viewer mode at editor layer #27

Merged

sameh-farouk merged 1 commit from fix/canvas-viewer-readonly-enforcement into development

2026-04-27 17:57:24 +00:00

sameh-farouk commented

2026-04-27 16:36:22 +00:00

Member

Summary

Closes #26.

Fixes the data-loss illusion for canvas viewers. The local Tiptap editor previously accepted toolbar-driven and keyboard-driven mutations even with setEditable(false), so format changes appeared then vanished on refresh. Server-side gates were already correct — this PR is purely client-side.

What changed

filterTransaction ProseMirror plugin registered on viewers' editors. Rejects any doc-changing transaction not tagged with ySyncPluginKey (the meta @tiptap/y-tiptap sets on remote Yjs sync transactions). This is the structural choke point — catches programmatic dispatch (editor.chain().toggleBold().run()), keyboard shortcuts (Ctrl+B), and any future command path.
editable: !isViewer at editor construction (vs late setEditable(false)) — eliminates a flash-of-editable window and pairs cleanly with the plugin.
Skip the bubble-menu and table-toolbar setup IIFEs for viewers — no selectionUpdate handlers attached, no positioning math, no DOM mutations. The menus simply never appear.
computeIsViewer(user, canvas) helper + module-scope isViewer flag, hoisted so the filterTransaction plugin (registered after editor creation) closes over the resolved value. Computed before initEditor() so the editor enters the right mode from the first render.
CSS rules under body[data-canvas-mode="viewer"] hide the static top toolbar, share button, and the popovers/pickers as belt-and-suspenders against any errant selection event.
"View only" badge in the topbar — explicit chip with bi-eye icon, replacing the previous subtle "View only" sync-status text. The status text is now free to show real connection state for viewers.
Vendor bundle: re-export ySyncPluginKey (from @tiptap/y-tiptap) and Plugin (from @tiptap/pm/state). One-time ./scripts/vendor-bundle/build.sh regen included in the diff.

Why filterTransaction (research note)

The naive fix — editor.setEditable(false) + hide buttons — doesn't work because Tiptap's editable flag is a UI hint that gates contenteditable input but not programmatic transaction dispatch. editor.chain().toggleBold().run() calls view.dispatch(tr) which doesn't check editable. The bubble-menu onclick handlers go through this path, and so do keyboard shortcuts via the keymap plugin.

filterTransaction is the canonical ProseMirror hook for blocking transactions before they apply to state. Plugins registered on the editor have their filterTransaction invoked for every transaction; returning false drops it. Allowing transactions tagged with ySyncPluginKey preserves live updates from editors so viewers still see real-time collaboration.

This is the same pattern Outline (the open-source Notion alternative) uses for read-only collaborative documents.

Dev-mode regression check

Scenario	Before	After
Proxy: owner / editor collaborator	full edit	full edit (no change)
Proxy: viewer collaborator	inconsistent — top toolbar hidden, bubble menu mutates locally, edits vanish on refresh (data-loss illusion)	truly read-only, "View only" badge
Dev: picker → owner / editor collaborator	full edit	full edit (no change)
Dev: picker → viewer collaborator	data-loss illusion (and edits actually persisted in dev mode due to `is_editor=is_dev` default in WS handler + RPC dev-mode bypass at canvas.rs:69)	truly read-only
Dev: picker → non-collaborator	same as viewer-collab above	truly read-only
Legacy dev: anonymous, `currentUser.id===0`	full edit	full edit (preserved by `!user

The two "behavior changes" for dev-mode picker-as-non-editor scenarios are bug-fixes, not regressions: the existing client-side intent at the previous isViewer line said "Viewer if explicitly set as viewer, OR if no collaborator record" but was being defeated by the bubble menu's programmatic dispatch combined with the WS handler defaulting is_editor=is_dev and the RPC dev-mode bypass.

Test plan

As viewer_test (canvas role viewer on canvas owned by sameh): hard-refresh → see content, no toolbar, no bubble menu on selection, "View only" badge visible.
As viewer_test: try every formatting affordance you can find (color, bold via console / keyboard shortcuts) → no visible changes apply, OR appear momentarily then revert (depending on how filterTransaction synchronizes with the view).
As viewer_test: refresh → DB canvas_state and canvases.updated_at unchanged.
As editor on a different canvas: full editing surface intact, all toolbars + bubble menu work as before.
Live collaboration: while logged in as viewer in one tab, edit as creator in another tab → viewer sees live updates (Yjs sync transactions pass through filterTransaction).
Bundle exports ySyncPluginKey and Plugin (verified in served bytes).
node --check on canvas-app.js passes.
Server-side gates (WS handler + canvas.save_state RPC) verified earlier in #10 — still authoritative; this PR doesn't change them.

Out of scope

Console-level evasion: window.editor.chain().toggleBold().run() from DevTools is blocked by the same filterTransaction plugin. Even if it weren't, the server still drops the resulting Yjs Update; refresh reverts. Honest UI for legitimate users; not a security boundary.
The loadChannels auto-create-#general fallback (chat-app.js:962) surfaced in the same investigation is a separate chat-sidebar concern; should get its own issue + PR.

🤖 Generated with Claude Code

## Summary Closes #26. Fixes the data-loss illusion for canvas viewers. The local Tiptap editor previously accepted toolbar-driven and keyboard-driven mutations even with `setEditable(false)`, so format changes appeared then vanished on refresh. Server-side gates were already correct — this PR is purely client-side. ## What changed - **`filterTransaction` ProseMirror plugin** registered on viewers' editors. Rejects any doc-changing transaction not tagged with `ySyncPluginKey` (the meta `@tiptap/y-tiptap` sets on remote Yjs sync transactions). This is the structural choke point — catches programmatic dispatch (`editor.chain().toggleBold().run()`), keyboard shortcuts (Ctrl+B), and any future command path. - **`editable: !isViewer`** at editor construction (vs late `setEditable(false)`) — eliminates a flash-of-editable window and pairs cleanly with the plugin. - **Skip the bubble-menu and table-toolbar setup IIFEs** for viewers — no selectionUpdate handlers attached, no positioning math, no DOM mutations. The menus simply never appear. - **`computeIsViewer(user, canvas)` helper** + module-scope `isViewer` flag, hoisted so the filterTransaction plugin (registered after editor creation) closes over the resolved value. Computed before `initEditor()` so the editor enters the right mode from the first render. - **CSS rules** under `body[data-canvas-mode="viewer"]` hide the static top toolbar, share button, and the popovers/pickers as belt-and-suspenders against any errant selection event. - **"View only" badge** in the topbar — explicit chip with `bi-eye` icon, replacing the previous subtle "View only" sync-status text. The status text is now free to show real connection state for viewers. - **Vendor bundle**: re-export `ySyncPluginKey` (from `@tiptap/y-tiptap`) and `Plugin` (from `@tiptap/pm/state`). One-time `./scripts/vendor-bundle/build.sh` regen included in the diff. ## Why filterTransaction (research note) The naive fix — `editor.setEditable(false)` + hide buttons — doesn't work because Tiptap's `editable` flag is a UI hint that gates contenteditable input but **not** programmatic transaction dispatch. `editor.chain().toggleBold().run()` calls `view.dispatch(tr)` which doesn't check `editable`. The bubble-menu onclick handlers go through this path, and so do keyboard shortcuts via the keymap plugin. `filterTransaction` is the canonical ProseMirror hook for blocking transactions before they apply to state. Plugins registered on the editor have their `filterTransaction` invoked for every transaction; returning false drops it. Allowing transactions tagged with `ySyncPluginKey` preserves live updates from editors so viewers still see real-time collaboration. This is the same pattern Outline (the open-source Notion alternative) uses for read-only collaborative documents. ## Dev-mode regression check | Scenario | Before | After | |---|---|---| | Proxy: owner / editor collaborator | full edit | full edit (no change) | | Proxy: viewer collaborator | inconsistent — top toolbar hidden, bubble menu mutates locally, edits vanish on refresh (data-loss illusion) | truly read-only, "View only" badge | | Dev: picker → owner / editor collaborator | full edit | full edit (no change) | | Dev: picker → viewer collaborator | data-loss illusion (and edits actually persisted in dev mode due to `is_editor=is_dev` default in WS handler + RPC dev-mode bypass at canvas.rs:69) | truly read-only | | Dev: picker → non-collaborator | same as viewer-collab above | truly read-only | | Legacy dev: anonymous, `currentUser.id===0` | full edit | full edit (preserved by `!user || !user.id` early-return in `computeIsViewer`) | The two "behavior changes" for dev-mode picker-as-non-editor scenarios are bug-fixes, not regressions: the existing client-side intent at the previous `isViewer` line said "Viewer if explicitly set as viewer, OR if no collaborator record" but was being defeated by the bubble menu's programmatic dispatch combined with the WS handler defaulting `is_editor=is_dev` and the RPC dev-mode bypass. ## Test plan - [x] As `viewer_test` (canvas role `viewer` on canvas owned by sameh): hard-refresh → see content, no toolbar, no bubble menu on selection, "View only" badge visible. - [x] As `viewer_test`: try every formatting affordance you can find (color, bold via console / keyboard shortcuts) → no visible changes apply, OR appear momentarily then revert (depending on how filterTransaction synchronizes with the view). - [x] As `viewer_test`: refresh → DB `canvas_state` and `canvases.updated_at` unchanged. - [x] As editor on a different canvas: full editing surface intact, all toolbars + bubble menu work as before. - [x] Live collaboration: while logged in as viewer in one tab, edit as creator in another tab → viewer sees live updates (Yjs sync transactions pass through filterTransaction). - [x] Bundle exports `ySyncPluginKey` and `Plugin` (verified in served bytes). - [x] `node --check` on canvas-app.js passes. - [x] Server-side gates (WS handler + `canvas.save_state` RPC) verified earlier in #10 — still authoritative; this PR doesn't change them. ## Out of scope - Console-level evasion: `window.editor.chain().toggleBold().run()` from DevTools is blocked by the same filterTransaction plugin. Even if it weren't, the server still drops the resulting Yjs Update; refresh reverts. Honest UI for legitimate users; not a security boundary. - The `loadChannels` auto-create-`#general` fallback (chat-app.js:962) surfaced in the same investigation is a separate chat-sidebar concern; should get its own issue + PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

sameh-farouk added 1 commit

2026-04-27 16:36:22 +00:00

fix(canvas): enforce read-only viewer mode at editor layer 335c8e798b

Viewers (canvas role 'viewer' or non-collaborator workspace members) saw
formatting changes apply locally, then vanish on refresh — the data-loss
illusion. The server already rejected viewer writes correctly (WS handler
drops Update frames; canvas.save_state RPC rejects with PermissionDenied)
but the client-side editor accepted local mutations because Tiptap's
`setEditable(false)` only blocks contenteditable input, not programmatic
dispatch from toolbar buttons or keyboard shortcuts.

Fix uses filterTransaction as the structural choke point: a ProseMirror
plugin registered on viewers' editors rejects any doc-changing transaction
that isn't tagged by @tiptap/y-tiptap's ySyncPluginKey (which marks remote
Yjs sync transactions). Local user edits — programmatic, keyboard-driven,
or console-evasion — never apply to the local Doc. Live updates from
editors still flow through.

Layered with: editable=false at editor construction (catches typing),
skipping bubble-menu and table-toolbar setup IIFEs for viewers (no
hover surfaces appear), CSS rules under body[data-canvas-mode="viewer"]
hide the static top toolbar / share button / popovers, and a "View only"
badge in the topbar makes the read-only state unambiguous instead of
relying on the previous subtle sync-status text replacement.

Bundle: ySyncPluginKey re-exported from @tiptap/y-tiptap and Plugin from
@tiptap/pm/state, both needed for the filterTransaction wiring.

Dev-mode regression check: legacy "no picker, anonymous id=0" path stays
editable (computeIsViewer early-returns false on falsy user.id). Picker
flows that pick a non-collaborator now correctly enter read-only mode —
this was the existing client-side intent at the previous isViewer line
1849, which was being defeated by the bubble menu's programmatic dispatch
combined with the WS handler's `is_editor = is_dev` default and the
RPC-layer dev-mode bypass at canvas.rs:69.

Empirically verified: viewer can see live updates from the editor,
filterTransaction blocks all local doc changes (UI clicks + keyboard +
console), DB unchanged after click-spam. Server-side gates remain
authoritative.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>