feat: fix cross-device sync with E2EE key hierarchy and docs cleanup - #177
Conversation
…and auto-trial Sync was broken on all platforms due to 4 cascading issues: 1. Missing DB migrations for tag_sync_log and notebook_sync_log tables (server 500s) 2. Per-device encryption keys preventing cross-device decryption 3. Magic link emails opening browser instead of desktop app 4. No subscription created on signup (sync gated behind Pro) Changes: - Add migrations 0005 (sync tables), 0006 (shared_notes columns), 0007 (user_keys) - Implement E2EE key hierarchy: passphrase → PBKDF2 → Master Key → AES-KW wrapped CEK - Add GET/POST /sync/keys endpoints for key exchange - Refactor EncryptionService with key derivation, wrap/unwrap, recovery key, legacy migration - Add requestSingleInstanceLock + second-instance handler for Windows/Linux deep links - Send readied:// deep link URLs for desktop magic link emails - Auto-create 14-day trial subscription on first auth verify - Clean up 29 stale doc artifacts, archive 26 historical plans, update SECURITY.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR implements end-to-end encryption key management across desktop and API layers. Changes include database migrations for user encryption keys and sync logs, new E2EE key API endpoints, desktop encryption service refactoring for key lifecycle operations, IPC handlers for encryption flows, magic-link client differentiation for deep links, subscription auto-provisioning, and legacy documentation cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Desktop as Desktop App
participant Main as Main Process
participant EncSvc as EncryptionService
participant ApiClient as ApiClient
participant API as Backend API
participant DB as Database
Desktop->>Main: setupKeys(passphrase)
Main->>EncSvc: setupKeys(passphrase)
EncSvc->>EncSvc: Generate CEK & derive MK via PBKDF2
EncSvc->>EncSvc: Wrap CEK & recovery key
EncSvc->>EncSvc: Cache CEK locally
EncSvc-->>Main: KeySetupResult{salt, wrappedCek, recoveryKey}
Main->>ApiClient: setEncryptionKeys(salt, wrappedCek, kdfParams)
ApiClient->>API: POST /sync/keys
API->>DB: upsert user_keys
API-->>ApiClient: {success: true}
ApiClient-->>Main: success
Main-->>Desktop: {success, recoveryKey}
sequenceDiagram
participant NewDevice as New Device
participant Main as Main Process
participant EncSvc as EncryptionService
participant ApiClient as ApiClient
participant API as Backend API
NewDevice->>Main: unlockWithPassphrase(passphrase)
Main->>ApiClient: getEncryptionKeys()
ApiClient->>API: GET /sync/keys
API-->>ApiClient: {salt, wrappedCek, kdfParams}
ApiClient-->>Main: keyBundle
Main->>EncSvc: unlockWithPassphrase(passphrase, salt, wrappedCek, kdfParams)
EncSvc->>EncSvc: Derive MK from passphrase + salt via PBKDF2
EncSvc->>EncSvc: Unwrap CEK using derived MK
EncSvc->>EncSvc: Cache CEK locally
EncSvc-->>Main: success/wrongPassphrase
Main-->>NewDevice: {success, wrongPassphrase?}
sequenceDiagram
participant User as User
participant DeepLink as Deep Link Handler
participant App1 as Instance 1 (Running)
participant App2 as Instance 2 (Launched)
participant MainWindow as Main Window
User->>DeepLink: Open readied://auth/verify?token=...
DeepLink->>App2: Launch with readied:// URL
App2->>App2: requestSingleInstanceLock()
App2-->>App2: Lock fails (App1 has it)
App2->>App1: second-instance event + URL
App1->>App1: Parse token from URL
App1->>MainWindow: auth:verify-token IPC
MainWindow->>MainWindow: Process auth
App1->>App1: Focus & restore window
App2->>App2: Exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09501f15c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| salt, | ||
| wrappedCek, | ||
| wrappedCekRecovery: wrappedCekRecovery ?? null, | ||
| kdfParams: JSON.stringify(kdfParams), |
There was a problem hiding this comment.
sync.post('/keys') always writes wrappedCekRecovery: wrappedCekRecovery ?? null during upsert, so updates that omit this field erase any previously stored recovery-wrapped CEK. The new desktop encryption:changePassphrase flow sends only salt, wrappedCek, and kdfParams, which means a normal passphrase change silently clears recovery data and makes unlockWithRecoveryKey unusable afterward.
Useful? React with 👍 / 👎.
| app.on('second-instance', (_event, commandLine) => { | ||
| const log = getLogger(); | ||
| // On Windows, the deep link URL is the last argument | ||
| const deepLinkUrl = commandLine.find(arg => arg.startsWith('readied://')); |
There was a problem hiding this comment.
The Windows/Linux deep-link token is only extracted inside the second-instance handler. If the app is closed and the user clicks a magic link, Electron starts a first instance with readied://... in startup args, but this code path never reads initial process.argv, so the auth token is dropped and verification does not proceed unless an instance was already running.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| return false; // No key available — passphrase setup required | ||
| } catch (error) { |
There was a problem hiding this comment.
initialize() now returns false when neither cache nor legacy key exists, instead of creating a key. Startup still continues after awaiting initialization, so fresh installs can run with this.key === null; subsequent sync encrypt/decrypt operations throw Encryption key not initialized, which breaks first-device sync until some separate setup flow populates a key.
Useful? React with 👍 / 👎.
## Summary Fixes 3 issues flagged by Codex bot review on PR #177. ## Changes ### 1. Preserve recovery key on passphrase change The `POST /sync/keys` upsert was overwriting `wrappedCekRecovery` with `null` when not provided (e.g., during passphrase change). Now only overwrites if explicitly sent. ### 2. Parse deep link on cold start (Windows/Linux) When the app isn't running and user clicks a `readied://` magic link, the OS launches a new instance with the URL in `process.argv`. Previously only `second-instance` parsed this. Now startup args are checked too. ### 3. Guard sync with encryption readiness `syncNow()` now checks `encryptionService.isReady()` before attempting encrypt/decrypt. Fresh installs without passphrase setup get a clear error instead of crashing. ## Test plan - [x] `pnpm typecheck` passes (17/17) - [x] `pnpm test` passes (42/42) - [ ] Manual: change passphrase → verify recovery key still works - [ ] Manual: close app → click magic link on Windows → app launches and authenticates - [ ] Manual: fresh install without passphrase → sync attempt shows clear error 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Bug Fixes * Deep link authentication tokens passed as command-line arguments are now properly recognized and processed on application startup * Sync operations are prevented from proceeding when the encryption service is not initialized, ensuring system stability * Recovery-wrapped keys are now properly preserved during sync operations instead of being inadvertently cleared <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
What was broken
Sync failed on all platforms due to 4 cascading issues:
tag_sync_logandnotebook_sync_logtables never created in productionKey changes
Sync & E2EE
user_keystable for key hierarchyEncryptionServicerefactored: PBKDF2 key derivation, AES-KW wrap/unwrap, recovery key, legacy key migrationGET/POST /sync/keysendpoints for cross-device key exchangerequestSingleInstanceLock+second-instancehandler for Windows/Linux deep linksreadied://deep link URLs for desktop magic link emailsPOST /auth/verifyDocs cleanup
SEMANA_2_COMPLETE.md,BACKEND_INTEGRATION_COMPLETE.md,TESTING_SYNC.mddocs/archived/PLUGIN_SYSTEM.md→docs/,TODO_MONITORING.md→OBSERVABILITY.mdSECURITY.mdsupported versions (0.2.x → 0.9.x)Test plan
pnpm typecheck— 17/17 tasks passpnpm test— 42/42 tests passGET /sync/statusreturnsenabled: true🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation