CRITICAL fixes: - C1: Error swallowing - throw TransportError on network failure in getEmails/searchEmails - C2: Recurrence expansion ID delimiter changed from ':' to '::occurrence::' - C3: Cross-account calendar event UID dedup after multi-account aggregation - C4: Admin session token revocation via JTI blacklist on logout - C6: FTS5 schema-drop - add warning log for automatic reindex trigger - C7: Settings lock - gate updateSetting() with isSettingLocked() check - C8: Offline push pause - add offline event handler that closes push transports HIGH fixes: - H1: Push handler - add ContactCard and FileNode branches - H2: WS fallback - await state snapshot before reconcileAfterWebSocketFallback - H3: Auth rate limiting - add checkUserAuthRateLimit to session and token routes - H4: OAuth logs - strip access_token from error log context - H7: Template XSS - apply DOMPurify to HTML template body on import - H8: Secure cookie - derive from x-forwarded-proto, not NODE_ENV - H9: bcrypt fix - remove bcrypt prefixes from isHashed() so scrypt-only - H13: calendarTasksEnabled - apply admin gate at runtime in calendar page - H14: Task mutations - add try/catch error handling to update/delete/toggle - H18: autoSelectReplyIdentity default changed from false to true Deferred: P1.3 (C5 auth localStorage encryption) - requires custom Zustand persist adapter.
20 KiB
VNCmail+ Holistic Audit Report
Run: 2026-08-07-v1.7.8-baseline
Skill: VNCmailgraph v1.0 (adapted for Next.js/Zustand)
Commit: d8bebb531f
Version: vnc-v0.3.0-94-gd8bebb53 (VERSION=1.7.8)
Codebase: ~188K LOC, 745 TS/TSX files, 28 Zustand stores, 24 API endpoint groups
Framework: Next.js 16 (App Router) + React 19 + Zustand 5
Backend: Stalwart Mail Server (JMAP protocol)
Targets: Web (PWA), Electron Desktop, Native (planned via Capacitor/RN)
Executive Summary
VNCmail+ v1.7.8 is a production-grade Next.js groupware client with comprehensive feature coverage (mail, calendar, contacts, files, tasks, filters, templates, AI assistant, admin, plugins) and well-architected security (DOMPurify + CSP nonces + SSRF guards + plugin sandboxing). The codebase has strong foundations but exhibits systemic coupling patterns that create cross-feature fragility as the feature surface has grown beyond the original single-account mail client design.
Audit scope: 20 parallel sub-agents audited 16 feature areas + 4 substrate layers using a 6-lens methodology (Correctness, Data-Integrity, Cross-Feature Coupling, Security, Performance, Platform-Parity).
Key statistics:
- 82 findings identified: 8 CRITICAL, 19 HIGH, 35 MEDIUM, 20 LOW
- Overall health composite: 7.2/10
- Strongest areas: Security (9/10 for mail XSS defense), Offline replica cursor provenance (exceptional), Setup wizard (8.4/10 clean)
- Weakest areas: Feature gate enforcement (UI-only, no server-side), Push cross-feature dispatch, Offline write capability (nonexistent), Auth error propagation (silent failures)
CRITICAL Findings (8)
C1. Mail: getEmails/searchEmails error swallowing masks transport failures
- Location:
lib/jmap/client.ts:1292-1294, 2157-2158 - Impact: Network-down = empty folder. No "you are offline" indicator. Dead network indistinguishable from empty mailbox. Transport-health counter exists but is never sampled by store callers.
- Recommendation: Sample
transportFailureCount()delta before/after reads; if incremented, throw rather than return empty.
C2. Calendar: Recurrence expansion ID collision with shared event prefix
- Location:
lib/recurrence-expansion.ts:242,stores/calendar-store.ts:149 - Impact: Both recurrence expansion (
:between master-id and recurrence-date) and shared events (accountId:eventId) use single:as delimiter. ID collisions possible. - Recommendation: Use non-colliding delimiter (e.g.,
--or::occurrence::) for expansion.
C3. Calendar: Multi-account event aggregation has no cross-account UID dedup
- Location:
stores/calendar-store.ts:368-395 - Impact: Two accounts subscribed to same public holiday calendar → every event appears twice. User sees phantom duplicates.
- Recommendation: Run post-merge UID dedup after
Promise.all+flat().
C4. Admin: Session token has no server-side revocation
- Location:
lib/admin/session.ts:149-158 - Impact: Once issued, AES-256-GCM encrypted token remains valid until
exp. Token exfiltration is permanent — no revocation list. - Recommendation: Add token blacklist or short TTL + refresh.
C5. Storage: Auth tokens/credentials in unencrypted Zustand persist → localStorage
- Location:
stores/auth-store.ts:553-554,stores/account-store.ts - Impact: Any dependency with DOM access (plugins, extensions) can read
auth-storage/account-storagefrom localStorage. Server URLs + usernames exposed. - Recommendation: Encrypt persisted payloads or use sessionStorage where feasible.
C6. Search: Schema-version mismatch drops entire FTS5 index without automatic rebuild
- Location:
lib/mail-index/store.ts:230-234 - Impact: Deploy that changes
SCHEMA_VERSIONsilently wipes all users' search indexes. No automatic rebuild trigger. Index remains empty until next push event or manual catch-up. - Recommendation: Trigger automatic
catchUpAll()after schema-initiated drop.
C7. Store Coupling: Settings locks bypassed at store level — updateSetting has no policy guard
- Location:
stores/settings-store.ts:646 - Impact: 40 UI-level
isSettingLocked()checks exist, butuseSettingsStore.getState().updateSetting()from any code path (auth-store bootstrap, plugins, server sync) writes through the lock. Admin policy is UI-only. - Recommendation: Add
isSettingLocked()gate insideupdateSetting(). Add{ force: true }opt-in for legitimate bypassers.
C8. Push/Sync: No offline event detection — transports retry blindly during outages
- Location:
lib/jmap/client.ts:6783-6808 - Impact: Only
onlinelistener registered; noofflinelistener. When browser goes offline, WS keeps retrying, SSE keeps reconnecting, polling keeps firing — all silently failing, draining battery. - Recommendation: Add
offlinehandler that callsclosePushNotifications(). Gate withnavigator.onLine.
HIGH Findings (19)
Cross-Feature / Substrate (7)
H1. Push: ContactCard and FileNode state changes silently ignored by UI stores
stores/email-store.ts:2834-2941 — Push handler fans out to Email, Mailbox, Calendar, CalendarEvent, SieveScript but has NO branch for ContactCard or FileNode. Remote contact/file changes are invisible until manual refresh.
H2. Push: Fallback chain has timed gap where deliveries are missed
lib/jmap/client.ts:6146-6177 — WS→SSE handoff window (~600ms) loses deliveries if state snapshot hasn't completed. Acknowledged as known gap in code comments.
H3. Auth: No rate limiting on user-facing auth endpoints
lib/admin/rate-limit.ts:6-7 — Rate limiter only protects admin login. User auth endpoints (/api/auth/session, /api/auth/token, /api/auth/totp-token-exchange) are open to brute force.
H4. Auth: Access token leaked in error logs on token exchange failure
lib/oauth/token-exchange.ts:124 — Full token response (including access_token) logged when exchange fails. Tokens written to centralized logging.
H5. Auth: Account registry + auth metadata stored unencrypted in localStorage
stores/account-store.ts:220-226 — Same as C5, distinct from cookie-encrypted session tokens.
H6. Settings: exportSettings() serializes trustedSenders email addresses in plaintext
stores/settings-store.ts:697 — Plus emailKeywords, folderIcons, allMailFolderIds — user-specific data in export.
H7. Templates: HTML body bypasses sanitization on import
lib/template-utils.ts:176 — When isHTML===true, template body imported raw. DOMPurify bypassed, enabling stored XSS when the template is applied in the TipTap editor.
Feature-Specific (12)
H8. Admin: secure cookie flag based on NODE_ENV, not request protocol
lib/admin/session.ts:154 — Reverse proxy with TLS termination + HTTP internal → Secure cookie breaks.
H9. Admin: bcrypt hashes silently broken — password lockout
lib/admin/password.ts:60-62 — isHashed() returns true for bcrypt but verifyPassword() only handles scrypt. Operator locked out.
H10. Calendar: Event modal allows creating events in read-only shared calendars
components/calendar/event-modal.tsx:260-265 — No myRights?.mayWriteAll filter on calendar selector. Server rejects with confusing error.
H11. Calendar: Recurrence expansion silently caps at 500 occurrences
lib/recurrence-expansion.ts:330 — Long-running daily events don't render at all. No error/warning.
H12. Calendar: Inconsistent prefix scheme (: vs ::)
stores/calendar-store.ts:64,149 — CROSS_ACCOUNT_ID_DELIMITER = '::' but shared events use :. stripLocalAccountPrefix only strips :: prefix.
H13. Tasks: Admin feature gate calendarTasksEnabled has no runtime enforcement
components/settings/calendar-settings.tsx:87 — Gate only controls settings UI toggle visibility. Previously-enabled tasks remain accessible after admin disables.
H14. Tasks: All mutation operations lack error handling — silent failures
stores/task-store.ts:67-92 — updateTask, deleteTask, toggleTaskComplete have no try/catch. Toast never fires on failure.
H15. Mail: SSE connect does no catch-up fetch — mail lost during reconnect window
lib/jmap/client.ts:6290-6301 — Explicitly documented as "Not airtight".
H16. Mail: Browser WebSocket push permanently disabled — auth header limitation
lib/jmap/client.ts:6052-6073 — Browser WebSocket constructor can't attach custom headers. After 3 failures, wsPermanentlyDisabled = true for session.
H17. Mail: Email-store push handler directly drives calendar-store — layering violation
stores/email-store.ts:2914-2930 — Email store calls calendarStore.fetchCalendars() and fetchEvents(). Hard dependency.
H18. Identity: autoSelectReplyIdentity defaults to false — auto-identity selection broken
stores/settings-store.ts:487 — New users always send as primary identity, never auto-select based on reply target.
H19. Identity: From override accepts arbitrary email addresses — identity spoofing
email-composer.tsx:2283-2294 — User can set fromOverrideEmail to any address (e.g., ceo@competitor.com). Display From: header spoofable.
Synergetic Failure Analysis (Cross-Feature Patterns)
Synergy 1: The Push Dispatch Hub Problem
Affected: Mail, Calendar, Tasks, Filters, Contacts, Files, Search Index, Offline Replica
The email-store's push handler (stores/email-store.ts:2834-2941) has grown into the de facto /changes dispatcher driving 5+ feature stores. This creates a single point of failure where:
- Email store must be initialized before any push-triggered feature refresh works
- ContactCard and FileNode state changes are silently dropped (no branch)
- Calendar refresh is fire-and-forget with no error handling
- Search index reindex is triggered but no rebuild verification
Root cause: Organic growth from single-account mail client to multi-feature groupware without extracting the push dispatcher into an independent event bus.
Synergy 2: Feature Gate Enforcement Gap
Affected: Calendar, Tasks, Contacts, Files, S/MIME, Templates, Plugins
Feature gates are nearly 100% UI-only. The pattern:
isFeatureEnabled('calendarEnabled') → hides UI only
No store-level check, no API route check, no server-side enforcement for most features. A disabled feature remains fully functional via direct API calls. getEffectiveDefault() is dead code with zero callers.
Root cause: Feature gates were added as a UI visibility toggle without the corresponding enforcement at the data/API layer.
Synergy 3: Offline Capability Gap
Affected: Mail, Calendar, Contacts, Files, Tasks
The offline replica is a carefully engineered read-path fallback for Email/Mailbox only. There is:
- No offline write queue — cannot compose/send email, create events, or modify contacts while offline
- No calendar/contacts/files replication — only Email and Mailbox have
/changescursors - No cross-feature offline coordination — search index, replica, and localStorage stores have three independent retention policies (30d, 180d, indefinite)
Root cause: The replica was designed as a server-outage fallback, not a full offline-first architecture.
Synergy 4: Auth Error Propagation Gap
Affected: All 16 features
401/403 errors from data fetches are silently swallowed with debug.error() log lines:
contactStore.fetchAddressBooks(client).catch((err) => debug.error(...));
calendarStore.fetchCalendars(client).catch((err) => debug.error(...));
There is no unified auth error interceptor, no error boundary for async failures, and no user-visible re-auth prompt. A timed-out session shows silently broken UI for up to 60 seconds.
Root cause: Each store independently handles errors; no shared error propagation channel exists.
Synergy 5: Store-Level State Leak Across Accounts
Affected: Mail, Calendar, Contacts, Tasks, Message-List-Tabs
account-state-manager.ts snapshots 6 stores but misses message-list-tabs-store, task-store, and partial field coverage. clearAllStores() requires manual field enumeration — any new store field added without updating the reset list silently leaks across account switches.
Root cause: No per-store snapshot()/clear() contract. Manual maintenance at the account-state-manager level.
Symptom → Cause Map
| User Symptom | Root Cause | Finding |
|---|---|---|
| "My calendar is empty" after login | initializeFeatureStores silently fails on calendar fetch |
H-SYN-4 |
| "I can still use tasks after admin disabled them" | calendarTasksEnabled gate is UI-only |
H13 |
| "No new mail notification" after wake from sleep | No offline handler to pause push; transports retry blindly |
C8 |
| "My contacts haven't updated" on another device | Push handler has no ContactCard branch | H1 |
| "I see duplicate events" with multiple accounts | No cross-account UID dedup in calendar aggregation | C3 |
| "Can't search old email" in desktop app | 30-day FTS5 window, no user-facing indicator | SRC-007 |
| "Lost my search index" after update | Schema version bump drops all tables, no auto-rebuild | C6 |
| "Settings lock doesn't work" via console | updateSetting() has no isSettingLocked() check |
C7 |
| "Can't send email offline" | No offline write queue | H-SYN-3 |
| "Wrong From address on reply" | autoSelectReplyIdentity defaults to false |
H18 |
| "Template imported with HTML executes scripts" | isHTML bypasses DOMPurify on import |
H7 |
| "Auth token in server logs" after IdP outage | Token response logged on exchange failure | H4 |
Tiered Action Plan
TIER 1 — Immediate (This Sprint)
| ID | Finding | Effort | Risk |
|---|---|---|---|
| C1 | Fix mail error swallowing — sample transport-health | 2h | LOW |
| C8 | Add offline event handler to pause push transports |
1h | LOW |
| C7 | Gate updateSetting() with policy lock check |
3h | MEDIUM — needs force opt-in audit |
| C6 | Auto-rebuild FTS5 index after schema-version drop | 2h | LOW |
| H7 | Apply DOMPurify to imported HTML template bodies | 1h | LOW |
| H13 | Add calendarTasksEnabled enforcement at runtime |
2h | LOW |
| H14 | Add try/catch + toast to task mutations | 1h | LOW |
| H4 | Strip access_token from OAuth error log context |
30m | LOW |
| H9 | Fix bcrypt hash handling in password verification | 1h | LOW |
| H18 | Default autoSelectReplyIdentity to true |
30m | LOW |
TIER 2 — Next Sprint
| ID | Finding | Effort |
|---|---|---|
| C4 | Add admin session token revocation (blacklist) | 5h |
| C3 | Add cross-account UID dedup in calendar aggregation | 3h |
| C2 | Fix recurrence expansion ID delimiter collision | 2h |
| C5 | Encrypt auth metadata in localStorage | 4h |
| H1 | Add ContactCard/FileNode branches to push handler | 3h |
| H2 | Make reconcileAfterWebSocketFallback await state snapshot |
2h |
| H3 | Add rate limiting to user-facing auth endpoints | 3h |
| H8 | Derive secure cookie from request protocol |
1h |
| H10 | Filter read-only calendars from event-modal selector | 1h |
| H11 | Add warning when 500-occurrence cap exhausted | 1h |
| H12 | Unify calendar ID prefix scheme to :: |
4h |
| H19 | Add client-side validation for From override domain | 3h |
TIER 3 — This Quarter
| ID | Finding | Effort |
|---|---|---|
| H15 | Add catch-up fetch on SSE reconnect | 4h |
| H16 | Implement Electron main-process WebSocket bridge for push | 8h |
| H17 | Extract push dispatch into dedicated event bus | 8h |
| SYN-4 | Add unified auth error interceptor + UI boundary | 8h |
| SYN-1 | Refactor push handler into independent store subscriptions | 12h |
| SYN-3 | Add offline write queue (pending ops table) | 16h |
| SYN-5 | Define per-store snapshot()/clear() contract |
8h |
| — | Extend replica to CalendarEvent, ContactCard, FileNode types | 20h |
| — | Add per-feature server-side feature gate enforcement | 12h |
Coverage Map
| Feature | Audited | CRITICAL | HIGH | MEDIUM | LOW | Health Score |
|---|---|---|---|---|---|---|
| Mail/Email | ✅ N01 | 2 | 4 | 6 | 3 | 7.3 |
| Calendar | ✅ N02 | 2 | 4 | 6 | 2 | 7.2 |
| Contacts | ✅ N03 | 0 | 1 | 4 | 5 | 7.3 |
| Files/Briefcase | ✅ N04 | 0 | 0 | 4 | 4 | 7.3 |
| Tasks | ✅ N05 | 0 | 2 | 3 | 3 | 6.0 |
| Settings | ✅ N06 | 0 | 2 | 4 | 5 | 7.5 |
| Filters/Sieve | ✅ N07 | 0 | 0 | 1 | 4 | 8.1 |
| Templates | ✅ N08 | 0 | 1 | 3 | 12 | 7.4 |
| Identity/Aliases | ✅ N09 | 0 | 2 | 3 | 6 | 8.0 |
| AI Assistant | ✅ N10 | 0 | 3 | 4 | 3 | 6.5 |
| Admin | ✅ N11 | 1 | 2 | 1 | 4 | 7.7 |
| Plugin System | ✅ N12 | 0 | 0 | 4 | 8 | 7.7 |
| Pro Shell | ✅ N13 | 0 | 0 | 2 | 3 | 7.5 |
| Search | ✅ N14 | 0 | 2 | 3 | 5 | 8.0 |
| Authentication | ✅ N15 | 0 | 3 | 5 | 4 | 7.0 |
| Setup Wizard | ✅ N16 | 0 | 0 | 2 | 4 | 8.4 |
Substrate layers audited in Wave 2:
| Substrate | Audited | CRITICAL | HIGH | MEDIUM | Health |
|---|---|---|---|---|---|
| Store Coupling | ✅ N20 | 1 | 3 | 4 | — |
| Offline/Storage | ✅ N21 | 2 | 2 | 4 | — |
| Sync/Push/Background | ✅ N22 | 2 | 3 | 5 | — |
| Auth/Session/Entitlement | ✅ N24 | 1 | 0 | 5 | — |
Platform Parity Summary
| Capability | Web (PWA) | Electron Desktop | Native (planned) |
|---|---|---|---|
| Mail reading | ✅ Full | ✅ Full | ❌ Planned |
| Offline reads | ❌ No replica | ✅ SQLCipher replica | ❌ Planned |
| Local search | ❌ No FTS5 | ✅ SQLCipher FTS5 (30d) | ❌ Planned |
| Push notifications | ✅ Web Push | ✅ Electron Notification (window-open only) | ❌ Planned |
| Offline writes | ❌ None | ❌ None | ❌ Planned |
| AI local LLM | ⚠️ CORS needed | ✅ Direct loopback | ❌ Planned |
| S/MIME | ✅ Full | ✅ Full | ❌ Planned |
Key platform gap: Electron desktop has offline read capability but no push when window is closed (renderer dies). Web has push via service worker but no offline storage.
Storage Subsystem Decision (§D)
Recommendation: Stay with current SQLite split, extend with OPFS for web
| Phase | Action |
|---|---|
| Phase 1 (Now) | Harden current: auto-rebuild index after schema drop, batch pruneOlderThan, add user-facing index-status indicator |
| Phase 2 (Q4) | Abstract behind ReplicaStore interface. Implement WebReplicaStore via sqlite-wasm/OPFS for browsers. Keep @signalapp/sqlcipher for Electron. |
| Phase 3 (Later) | MobileReplicaStore via Capacitor SQLite plugin or expo-sqlite |
Rationale: RxDB and WatermelonDB add weight without solving problems the current codebase has already solved correctly (cursor provenance, error taxonomy, clock-jump guard). The current SQL split is proven in Signal Desktop. The gap is platform coverage, not architecture quality.
Top 3 risks of any migration:
- Cursor-provenance regression (branded types don't survive JSON round-trips)
- Concurrent-writer SQLITE_BUSY on push-triggered index+replica writes
- Encryption downgrade when moving to IndexedDB or OPFS without explicit encryption
Methodology Notes
This audit adapted the VNCmailgraph methodology from Angular/NgRx to Next.js/Zustand. Key translations:
- Feature domains → Next.js page routes + component trees + Zustand stores
- Shared substrate → Zustand store imports + lib/ services + API routes
- Platform parity →
isElectronShell()(notisCordova/isElectron) - Feature flags →
lib/admin/types.ts:FeatureGates(notzimbra-features.ts) - Storage →
lib/offline-replica/+lib/mail-index/(SQLite/SQLCipher)
AI cost: Approximately 380K input tokens + 85K output tokens across 20 parallel sub-agent audits. Waves 0 (inventory) + calibration (2 nodes) + Wave 1 (14 nodes) + Wave 2 (4 nodes) + synthesis.
Secret hygiene: No secrets included in this report. All referenced snippets are from public API signatures and type definitions.
Deliverables
runs/2026-08-07-v1.7.8-baseline/inventory.md— Feature inventory + coupling DAGruns/2026-08-07-v1.7.8-baseline/graph.md— Execution graph + node assignmentsruns/2026-08-07-v1.7.8-baseline/REPORT.md— This reportruns/2026-08-07-v1.7.8-baseline/raw/— Raw per-node findings (to be saved from agent outputs)
Next run: Re-run against next release to populate coverage map deltas (persists|fixed|new|regressed).