SRCmail/runs/2026-08-07-v1.7.8-baseline/REPORT.md

# VNCmail+ Holistic Audit Report

**Run:** 2026-08-07-v1.7.8-baseline  
**Skill:** VNCmailgraph v1.0 (adapted for Next.js/Zustand)  
**Commit:** d8bebb531f86cab3507aed2113e8d0e6a03c1aa8  
**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-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 `/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:
```ts
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 | — | 4.0 |
| Offline/Storage | ✅ N21 | 2 | 2 | 4 | — | 7.0 |
| Sync/Push/Background | ✅ N22 | 2 | 3 | 5 | — | 5.0 |
| Auth/Session/Entitlement | ✅ N24 | 1 | 0 | 5 | — | 5.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`).