Files
SRCmail/runs/2026-08-07-v1.7.8-baseline/REPORT.md
T
Bernd Rodler a622e3755b fix: Phase 1 critical+high fixes (17/18 items)
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.
2026-08-07 12:17:41 +02:00

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-storage from 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_VERSION silently 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, but useSettingsStore.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 inside updateSetting(). 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 online listener registered; no offline listener. When browser goes offline, WS keeps retrying, SSE keeps reconnecting, polling keeps firing — all silently failing, draining battery.
  • Recommendation: Add offline handler that calls closePushNotifications(). Gate with navigator.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-62isHashed() 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,149CROSS_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-92updateTask, 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 /changes cursors
  • 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:

  1. Cursor-provenance regression (branded types don't survive JSON round-trips)
  2. Concurrent-writer SQLITE_BUSY on push-triggered index+replica writes
  3. 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() (not isCordova/isElectron)
  • Feature flags → lib/admin/types.ts:FeatureGates (not zimbra-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 DAG
  • runs/2026-08-07-v1.7.8-baseline/graph.md — Execution graph + node assignments
  • runs/2026-08-07-v1.7.8-baseline/REPORT.md — This report
  • runs/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).