Skip to content

feat(canvas): canvases are plain .excalidraw files in the vault - #946

Merged
h4yfans merged 5 commits into
mainfrom
canvas-files-in-vault
Aug 4, 2026
Merged

feat(canvas): canvases are plain .excalidraw files in the vault#946
h4yfans merged 5 commits into
mainfrom
canvas-files-in-vault

Conversation

@h4yfans

@h4yfans h4yfans commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Why

A canvas was the only user content with no file on disk. The scene lived in canvases.snapshot_ciphertext, encrypted with the vault key — which tied the ink to one machine's master key. Two ordinary user journeys destroyed it:

  1. Free → paid. A local-only user draws canvases, then turns on sync. performFirstDeviceSetup mints a brand-new master key from a new recovery phrase and rebinds the vault; bindLocalVaultToMasterKey purges sync state and agent chat but never touched canvases. Every pre-upgrade canvas became undecryptable — and unpushable, so no server copy existed to recover from. Silent, permanent.
  2. Copy the vault. USB, git, Dropbox. The key lives in the OS keychain, not in the folder, so the canvas could not be opened on the other machine at all — while notes, journals and attachments travelled fine.

The encryption bought nothing: everything else in the same folder is already plaintext, so the local threat model was unchanged while the fragility was entirely real.

What changed

The file is the source of truth; the table is an index.

<vault>/canvases/Istanbul Weekend.excalidraw   # the scene, plaintext, valid Excalidraw
<vault>/canvases/library.excalidrawlib         # shapes panel, Excalidraw's own format
  • Self-describing files. A memry key ({ id, createdAt, updatedAt }) rides inside the document. Excalidraw ignores unknown top-level keys, so the file still opens in excalidraw.com and a single copied file keeps its identity.
  • Canonical text. Fixed key order, unknown keys (including the memryAssets image sidecar) preserved and sorted. Two devices emit identical bytes for identical ink — which is what the sync conflict-copy comparison relies on.
  • Vault-open reconcile (canvas/reconcile.ts): migrates legacy encrypted snapshots once, and adopts documents that arrived with the folder. A file renamed outside the app re-points its row instead of duplicating. Rows whose file is missing are reported, never tombstoned — a half-copied vault must not delete canvases.
  • Unreadable is explicit. A legacy snapshot this device has no key for keeps its ciphertext (recoverable if the old key returns) and is served as unreadable; the editor refuses to mount rather than autosave an empty scene over recoverable ink.
  • No key material on the canvas path at all. getCanvasContext() no longer resolves a vault key; the keychain is consulted only by the one-way migration.
  • Sync unchanged on the wire. Push reads the file, apply writes it. Transport encryption still wraps a per-item key under the vault key — the server never sees plaintext.

Backward compatibility

  • Migration 0045 is additive: file_path added; snapshot_ciphertext stays NOT NULL and is blanked ('') only as a row successfully migrates. No DB reset, nothing dropped.
  • Older app versions keep reading their own ciphertext; newer ones read the file.
  • canvas_library_items rows are migrated into the library file and tombstoned; the ciphertext column is left intact.

Verification

  • pnpm --filter @memry/desktop test:main — 4796 passed, 1 skipped, 0 failed
  • pnpm typecheck — 16/16 tasks green · pnpm lint — 0 errors
  • pnpm check:architecture, pnpm check:contracts, pnpm ipc:check, i18n:check — pass
  • pnpm docs:impact --base origin/main --strict + pnpm docs:build — pass
  • New coverage: scene-file.test.ts (26), reconcile.test.ts (12 — including the free→paid key change and the copied-vault adoption), rewritten store.test.ts (15, one test opens a canvas from a copied vault folder), canvas-handler.test.ts (24), canvas-handlers.test.ts (23, asserts the keychain is never touched).

Not covered by automated tests: a manual walkthrough in the running app (open an existing vault with legacy canvases, verify migration + rendering).


Also on this branch: the notes:updated sync payload fix

Carried here rather than into its own PR. The three sync-path emitters shipped { id, source: 'sync' } with no changes, which NoteUpdatedEvent requires. Renderer subscribers read changes.content unguarded, so every note applied by a pull threw inside the preload listener loop and that subscriber silently dropped the event — link caches never invalidated after a pull.

Both emit paths are (channel: string, data: unknown), so typecheck could not see the violation. The three call sites now go through a typed emitNoteUpdated helper, and onNoteUpdated normalizes a missing changes once so an older main process stays tolerated. source gains 'sync', which was always emitted at runtime but never admitted by the union.

The write-back emit carries content now, but scheduleWriteback also fires for local typing on a 500ms debounce that beats the 1000ms save — so note.tsx skips source: 'sync' rather than remounting the editor mid-keystroke over bytes the IPC CRDT provider already applied.

🤖 Generated with Claude Code

h4yfans added 2 commits August 4, 2026 22:58
A canvas was the only user content with no file on disk: the scene lived
in canvases.snapshot_ciphertext, encrypted with the vault key. That tied
the ink to one machine's master key, so it died in two ordinary cases:

- a local-only user turns on sync — first-device setup mints a NEW master
  key from a new recovery phrase, rebinds the vault, and every canvas
  drawn before the upgrade becomes undecryptable (and unpushable, so no
  server copy exists either);
- the vault folder is copied to another machine (USB, git, Dropbox) —
  the key lives in the OS keychain, not in the folder, so the canvas
  cannot be opened there at all.

Encrypting it bought nothing: notes, journals and attachments in the same
folder are already plaintext, so the threat model was unchanged while the
fragility was entirely real.

Now the file is the source of truth and the table is an index:

- `<vault>/canvases/<Title>.excalidraw`, valid Excalidraw JSON with a
  `memry` sidecar (id + timestamps) so a single copied file is
  self-describing; canonical key order so two devices emit identical
  bytes for identical ink (the conflict-copy check compares text).
- `<vault>/canvases/library.excalidrawlib` replaces the encrypted
  canvas_library_items rows (not a sync type — the file is the store).
- Vault open migrates legacy snapshots once and adopts files that
  arrived with the folder. A snapshot we cannot decrypt KEEPS its
  ciphertext and surfaces as `unreadable` instead of mounting an editor
  that would autosave over recoverable ink.
- Sync is unchanged on the wire: push reads the file, apply writes it,
  transport encryption in sync/encrypt.ts still wraps a per-item key
  under the vault key, so the server never sees plaintext.

Additive migration (0045): `file_path` added, `snapshot_ciphertext` kept
NOT NULL and blanked ('') as rows migrate. No DB reset, no data dropped.
Copilot AI lite review requested due to automatic review settings August 4, 2026 20:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added documentation Improvements or additions to documentation enhancement New feature or request test labels Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit ca4daa8.

Comment thread apps/desktop/src/main/canvas/scene-file.ts Fixed
Comment thread apps/desktop/src/main/canvas/scene-file.ts Fixed
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

The three sync-path emitters shipped `{ id, source: 'sync' }` with no
`changes`, which `NoteUpdatedEvent` requires. Renderer subscribers read
`changes.content` unguarded, so every note applied by a pull threw inside
the preload listener loop and that subscriber silently dropped the event —
link caches never invalidated after a pull.

Both emit paths are `(channel: string, data: unknown)`, so typecheck could
not see the violation. Route the three call sites through a typed
`emitNoteUpdated` helper instead, and normalize a missing `changes` once in
`onNoteUpdated` so an older main process stays tolerated.

The write-back emit carries `content` now, but `scheduleWriteback` also
fires for local typing on a 500ms debounce that beats the 1000ms save — so
note.tsx skips `source: 'sync'` rather than remounting the editor
mid-keystroke over bytes the IPC CRDT provider already applied.
Copilot AI review requested due to automatic review settings August 4, 2026 21:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ux and Windows

Windows was the real bug, not a hypothetical: vault-relative paths were built
with path.join, so a canvas created on Windows stored `canvases\Plan.excalidraw`
in file_path. Copy that vault to a Mac — the whole point of this PR — and the
backslash is one bogus filename, not a directory. Paths are now always stored
forward-slashed (the same `normalizeRelativePath` convention notes use) and
re-joined natively at read time.

The rest of the platform gap, all of it on the write path:

- Transient EBUSY/EPERM/EACCES retry (50/150/450ms) around write, rename and
  delete. On Windows a cloud-sync client or antivirus scanner holds vault files
  for a moment; the async `withTransientFsRetry` in vault/file-ops.ts already
  does this, but the canvas path is synchronous (the sync apply writes inside a
  better-sqlite3 transaction), so this is its sync twin.
- Windows reserved device names (CON, NUL, COM1, LPT9…) are rejected by Win32
  whatever the extension, and they are ordinary canvas titles. Suffixed, not
  replaced, so the user still recognizes the file.
- Trailing dots and spaces are silently trimmed by Win32, so `Plan.` and `Plan `
  both resolved to `Plan` and quietly collided. Stripped up front.
- Filename collisions compare case-insensitively: macOS and Windows default to
  case-insensitive filesystems, so two canvases that coexist on Linux must not
  merge into one when the folder is copied to a Mac. A canvas's own file is
  excluded, or a case-only title edit ("Plan" → "plan") lands on "plan 2".
- The `.excalidraw` extension is matched case-insensitively when listing and
  when deriving a title from a filename.
- A directory sitting where a document should be (ENOTDIR/EISDIR after a bad
  copy) reads as "no document" instead of taking the canvas surface down.

Also closes the CodeQL "insecure temporary file" alert on the atomic write: the
temp file now uses a random name opened `wx` with owner-only permissions and is
cleaned up on failure, matching vault/file-ops.atomicWrite. A predictable temp
name in a user-writable directory is a symlink-swap target.

Test expectations that hard-coded path.join are now platform-independent, so the
suite means the same thing on the Windows runner.
Copilot AI review requested due to automatic review settings August 4, 2026 21:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

… them

macOS stores filenames decomposed: a canvas titled "Yağmur" is written NFC and
comes back from readdir as NFD — different bytes for the same name. Reconcile
compared raw strings, so every vault open would see a "new" document and rewrite
the row's path.

Comparisons now go through `canvasPathKey` (NFC + lowercase); the STORED path
still keeps the bytes as they exist on disk, because Linux filesystems are
normalization-sensitive and an NFC-normalized path would fail to open a file
that arrived NFD from a Mac.
Copilot AI review requested due to automatic review settings August 4, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@h4yfans
h4yfans marked this pull request as ready for review August 4, 2026 21:19
@h4yfans
h4yfans merged commit 3d7676e into main Aug 4, 2026
16 checks passed
@h4yfans
h4yfans deleted the canvas-files-in-vault branch August 4, 2026 21:20
const tmp = path.join(path.dirname(absolutePath), `.${randomBytes(6).toString('hex')}.tmp`)
try {
writeFileSync(tmp, content, { encoding: 'utf-8', mode: 0o600, flag: 'wx' })
const fd = openSync(tmp, 'r+')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants