| 1 |
1 |
|
# GoingsOn -- Audit Review
|
| 2 |
2 |
|
|
| 3 |
|
- |
**Last audited:** 2026-05-04 (Run 19 cross-project)
|
| 4 |
|
- |
**Previous audit:** 2026-04-18 (Run 15, corrected 2026-04-22)
|
| 5 |
|
- |
**Auditor:** Claude Opus 4.6 (automated codebase audit)
|
|
3 |
+ |
**Last audited:** 2026-05-10 (Run 24, Ultra Fuzz)
|
|
4 |
+ |
**Previous audit:** 2026-05-04 (Run 19 cross-project)
|
|
5 |
+ |
**Auditor:** Claude Opus 4.6 (5-axis adversarial audit)
|
| 6 |
6 |
|
**Scope:** Full workspace (`crates/core`, `crates/db-sqlite`, `crates/plugin-runtime`, `src-tauri`, frontend JS, migrations)
|
| 7 |
7 |
|
|
| 8 |
8 |
|
---
|
| 9 |
9 |
|
|
| 10 |
10 |
|
## Overall Grade: A
|
| 11 |
11 |
|
|
| 12 |
|
- |
765 tests (all pass, `--workspace`). Zero clippy warnings. v0.3.1. ~68,800 LOC (48.5K Rust + 18.4K JS + 1.9K SQL). All SQL parameterized. Strong type safety. Clean 4-crate architecture maintained. Uncommitted work in progress (email signatures, drafts, labels, notifications — 28 modified, 5 untracked files).
|
|
12 |
+ |
773 tests (all pass, `--workspace`). Zero clippy warnings. v0.3.1. ~73,900 LOC (50.5K Rust + 21.3K JS + 2.1K SQL). All SQL parameterized. Strong type safety. Clean 4-crate architecture. SyncKit E2E encryption with XChaCha20-Poly1305 + Argon2id KDF.
|
| 13 |
13 |
|
|
| 14 |
14 |
|
---
|
| 15 |
15 |
|
|
| 16 |
|
- |
## Scorecard
|
|
16 |
+ |
## Critical Findings (fix before launch)
|
|
17 |
+ |
|
|
18 |
+ |
### CRITICAL: 24 Commands Missing from Desktop Handler Registration
|
|
19 |
+ |
|
|
20 |
+ |
**Location:** `src-tauri/src/main.rs:385-585`
|
|
21 |
+ |
**Description:** The desktop `generate_handler![]` in `main.rs` is missing 24 commands that are present in the mobile entry point (`lib.rs`) and called by JS. Affected features on desktop: all file attachments, email blob handling, email drafts, email labels/folders, email account settings (signatures, notifications), VCF/ICS import, manual time logging, and sync subscription status. These commands silently fail — Tauri returns an error but the user sees nothing happen.
|
|
22 |
+ |
**Fix:** Copy the missing entries from `lib.rs` into `main.rs` `generate_handler![]`.
|
|
23 |
+ |
|
|
24 |
+ |
### SERIOUS: Bulk Set Project/Priority Sends Incomplete TaskInput
|
| 17 |
25 |
|
|
| 18 |
|
- |
| Dimension | Grade | Notes |
|
| 19 |
|
- |
|-----------|:-----:|-------|
|
| 20 |
|
- |
| **Code Quality** | A- | Zero clippy warnings. ~662 unwrap() across workspace (majority in tests). Consistent `CoreError`/`ApiError` chain. |
|
| 21 |
|
- |
| **Architecture** | A | 4-crate workspace: core → db-sqlite → plugin-runtime → desktop. Repository trait pattern. No layer violations. |
|
| 22 |
|
- |
| **Testing** | A | 765 tests (all pass, `--workspace`). Sync service 1608 LOC of tests. Strong coverage across all layers. |
|
| 23 |
|
- |
| **Security** | A | All SQL parameterized (whitelist-validated table names for dynamic SQL). Frontend: systematic `escapeHtml()`/`escapeAttr()`. OS keychain. OAuth2 + PKCE. Plugin sandbox. One symlink canonicalization issue in plugin loader (low risk — desktop app). |
|
| 24 |
|
- |
| **Performance** | A- | Virtual scrolling, FTS5, batch sync, partial indexes. Plugin CSV parser has double-read inefficiency. |
|
| 25 |
|
- |
| **Documentation** | A | Module-level `//!` docs on all 4 lib.rs files. ARCHITECTURE.md and STYLEGUIDE.md current. |
|
| 26 |
|
- |
| **Dependencies** | A | All deps at latest stable. No outdated crates flagged. |
|
| 27 |
|
- |
| **Frontend** | A- | 18.4K LOC across 30+ IIFE modules. Systematic XSS prevention. State management via pub/sub. emails.js at 1212 LOC is large but well-structured. |
|
| 28 |
|
- |
| **Type Safety** | A | 11 entity ID newtypes. Typed enums for all domain values. Exhaustive matching. Builder patterns for complex construction. |
|
| 29 |
|
- |
| **Observability** | A | 435+ instrument annotations. Structured tracing with EnvFilter. Full coverage on commands and repositories. |
|
| 30 |
|
- |
| **Concurrency** | A | parking_lot + TokioMutex. Per-account sync locks. CancellationToken for shutdown. No deadlock risks. |
|
| 31 |
|
- |
| **Resilience** | A- | Crash-safe sync cursor. `applying_remote` suppression. HTTP timeouts. No exponential backoff on email sync failures. |
|
| 32 |
|
- |
| **API Consistency** | A | Every command returns `Result<T, ApiError>`. Consistent pagination. Pre-computed display fields. |
|
| 33 |
|
- |
| **Migration Safety** | A | 44 migrations, all additive. No destructive operations in recent migrations (041-044). |
|
| 34 |
|
- |
| **Codebase Size** | A- | ~68,800 LOC implementing 20+ feature domains. Some duplication in external_sync parsers. |
|
|
26 |
+ |
**Location:** `src-tauri/frontend/js/bulk-actions.js:176-198`
|
|
27 |
+ |
**Description:** `_applyProject` and `_applyPriority` call `GoingsOn.api.tasks.update(id, { projectId })` with only one field. Backend `update_task` requires `description` to be non-empty. Since `description` defaults to `""` via serde, every bulk project/priority update fails with `VALIDATION_ERROR`. Both bulk actions are completely broken.
|
|
28 |
+ |
**Fix:** Create a dedicated bulk-update command accepting partial updates, or fetch each task first and merge.
|
|
29 |
+ |
|
|
30 |
+ |
### SERIOUS: Backup Does Not Include Contacts, Daily Notes, Milestones, Time Sessions, or Attachments
|
|
31 |
+ |
|
|
32 |
+ |
**Location:** `src-tauri/src/backup_scheduler.rs:114-134`, `src-tauri/src/export/backup.rs:17-33`
|
|
33 |
+ |
**Description:** `FullExport` only serializes projects, tasks, events, and emails. Contacts (with child records), daily notes, milestones, time sessions, and attachments are silently excluded. A "full backup" is not full. Restoring from backup would lose contacts and time tracking data.
|
|
34 |
+ |
**Fix:** Add the missing entity types to `FullExport` and corresponding fetch/restore logic.
|
| 35 |
35 |
|
|
| 36 |
36 |
|
---
|
| 37 |
37 |
|
|
| 38 |
|
- |
## Module Heatmap
|
| 39 |
|
- |
|
| 40 |
|
- |
| Module | Code | Arch | Test | Security | Perf | Size |
|
| 41 |
|
- |
|--------|:----:|:----:|:----:|:--------:|:----:|:----:|
|
| 42 |
|
- |
| **goingson-core** | A | A+ | A | A | A | A- |
|
| 43 |
|
- |
| **goingson-db-sqlite** | A | A | A | A+ | A | A |
|
| 44 |
|
- |
| **goingson-plugin-runtime** | B+ | A | A | A- | B | C |
|
| 45 |
|
- |
| **goingson-desktop (commands)** | A | A | B+ | A | A- | B+ |
|
| 46 |
|
- |
| **goingson-desktop (email/oauth)** | A- | A | B+ | A+ | A- | B+ |
|
| 47 |
|
- |
| **goingson-desktop (sync_service)** | A | A | A+ | A | A | B |
|
| 48 |
|
- |
| **goingson-desktop (jmap)** | A | A | B+ | A | A | B+ |
|
| 49 |
|
- |
| **goingson-desktop (external_sync)** | A- | B+ | B | A | A | C+ |
|
| 50 |
|
- |
| **JS Frontend** | A- | A | — | A | A- | B+ |
|
| 51 |
|
- |
|
| 52 |
|
- |
### Cold Spots
|
| 53 |
|
- |
|
| 54 |
|
- |
| Module | Issue | Grade | Severity |
|
| 55 |
|
- |
|--------|-------|:-----:|:--------:|
|
| 56 |
|
- |
| **plugin-runtime/api.rs** (1103 LOC) | Double CSV reader instantiation; excessive cloning on hot path | C (perf) | Low |
|
| 57 |
|
- |
| **plugin-runtime/registry.rs** (1243 LOC) | Bloated test helpers; 1243 LOC for ~200 LOC of logic | C (size) | Low |
|
| 58 |
|
- |
| **plugin-runtime/loader.rs:118** | Symlink canonicalization fallback allows sandbox escape with dangling symlinks | B (security) | Medium |
|
| 59 |
|
- |
| **external_sync/ical.rs** | Minimal test coverage; parser logic shared with vcard.rs | B (test) | Low |
|
| 60 |
|
- |
| **commands/export.rs** | Sequential list_all calls (could parallelize) | B+ (perf) | Very Low |
|
| 61 |
|
- |
| **emails.js** (1212 LOC) | Approaching split threshold; rendering mixed with logic | B+ (size) | Low |
|
|
38 |
+ |
## Scorecard
|
|
39 |
+ |
|
|
40 |
+ |
### Module Heatmap — Data Integrity
|
|
41 |
+ |
|
|
42 |
+ |
| Module | Query Quality | Tx Safety | Type Safety | State Machines | Migration Safety | Validation | Overall |
|
|
43 |
+ |
|--------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
|
|
44 |
+ |
| **task_repo** | A | A | A | A- | A | A- | **A-** |
|
|
45 |
+ |
| **email_repo** | A | A | A | A | A | A | **A** |
|
|
46 |
+ |
| **search_repo** | A- | N/A | A | N/A | A | A- | **A-** |
|
|
47 |
+ |
| **event_repo** | A | A | A | A | A | A | **A** |
|
|
48 |
+ |
| **contact_repo** | A | A- | A | N/A | A | A | **A** |
|
|
49 |
+ |
| **project_repo** | A | N/A | A | N/A | A | A | **A** |
|
|
50 |
+ |
| **time_session_repo** | A | A | A | A | A | B+ | **A-** |
|
|
51 |
+ |
| **stats_repo** | A+ | N/A | A | N/A | A | N/A | **A+** |
|
|
52 |
+ |
| **milestone_repo** | A | A | A | A | A | B+ | **A-** |
|
|
53 |
+ |
| **recurrence.rs** | N/A | N/A | A | A | N/A | A | **A** |
|
|
54 |
+ |
| **urgency.rs** | N/A | N/A | A | A | N/A | A | **A** |
|
|
55 |
+ |
| **validation.rs** | N/A | N/A | A | N/A | N/A | A | **A** |
|
|
56 |
+ |
|
|
57 |
+ |
### Module Heatmap — Sync & Storage
|
|
58 |
+ |
|
|
59 |
+ |
| Module | Conflict Handling | Error Recovery | Data Integrity | Resource Mgmt | Architecture | Testing | Overall |
|
|
60 |
+ |
|--------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
|
|
61 |
+ |
| **sync_service/mod.rs** | A | A | A | A | A | A | **A** |
|
|
62 |
+ |
| **sync_service/pull.rs** | A | A | A | A | A+ | A | **A** |
|
|
63 |
+ |
| **sync_service/push.rs** | A | A- | A | A | A | A- | **A** |
|
|
64 |
+ |
| **sync_service/apply.rs** | N/A | A | A | A | A | A | **A** |
|
|
65 |
+ |
| **sync_service/blob_sync.rs** | N/A | A- | B+ | B | A- | B+ | **B+** |
|
|
66 |
+ |
| **sync_scheduler.rs** | N/A | A | A | A | A | B+ | **A-** |
|
|
67 |
+ |
| **db_watcher.rs** | N/A | A- | N/A | A | A- | A- | **A-** |
|
|
68 |
+ |
| **backup_scheduler.rs** | N/A | A- | B | A | A- | B | **B+** |
|
|
69 |
+ |
| **export/backup.rs** | N/A | A | A- | A | A | A- | **A-** |
|
|
70 |
+ |
| **export/csv.rs** | N/A | A | A | A | A | A- | **A** |
|
|
71 |
+ |
| **export/ics.rs** | N/A | A | A | A | A | A | **A** |
|
|
72 |
+ |
| **external_sync/ical.rs** | N/A | A | A | A | A | A | **A** |
|
|
73 |
+ |
| **external_sync/vcard.rs** | N/A | A | A | A | A | A | **A** |
|
|
74 |
+ |
| **core/backup_restore.rs** | N/A | A- | B+ | A | A | B+ | **A-** |
|
|
75 |
+ |
|
|
76 |
+ |
### Module Heatmap — UX Wiring
|
|
77 |
+ |
|
|
78 |
+ |
| Module | Contract Fidelity | Validation | State Mgmt | XSS Safety | Error Handling | Code Quality | Overall |
|
|
79 |
+ |
|--------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
|
|
80 |
+ |
| **commands/error.rs** | A+ | A+ | N/A | N/A | A+ | A+ | **A+** |
|
|
81 |
+ |
| **commands/task.rs** | A | A | A | N/A | A | A | **A** |
|
|
82 |
+ |
| **commands/event.rs** | A | A | A | N/A | A | A | **A** |
|
|
83 |
+ |
| **commands/contact.rs** | A | A | A | N/A | A | A | **A** |
|
|
84 |
+ |
| **main.rs (registration)** | B- | N/A | N/A | N/A | N/A | B- | **B-** |
|
|
85 |
+ |
| **api.js** | A | A- | A | N/A | A | A | **A** |
|
|
86 |
+ |
| **utils.js** | A | A | N/A | A+ | A | A | **A** |
|
|
87 |
+ |
| **form-modal.js** | A | A | A- | A | A- | A | **A** |
|
|
88 |
+ |
| **tasks.js** | A | A- | A- | A | A | A- | **A-** |
|
|
89 |
+ |
| **events.js** | A- | A- | B+ | A | A- | A- | **A-** |
|
|
90 |
+ |
| **emails.js** | A | A- | A- | A | A | A- | **A-** |
|
|
91 |
+ |
| **contacts.js** | A | A- | A | A | B+ | A | **A** |
|
|
92 |
+ |
| **bulk-actions.js** | B- | B- | A- | A | B | B+ | **B-** |
|
|
93 |
+ |
| **cache.js** | A | N/A | A | N/A | N/A | A+ | **A** |
|
|
94 |
+ |
| **pagination-manager.js** | A | A | A | N/A | A | A | **A** |
|
|
95 |
+ |
|
|
96 |
+ |
### Module Heatmap — Security
|
|
97 |
+ |
|
|
98 |
+ |
| Module | Credential Mgmt | Encryption | Sandbox Security | Input Validation | Transport Security | Least Privilege | Overall |
|
|
99 |
+ |
|--------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
|
|
100 |
+ |
| **OAuth (credentials/provider)** | A | N/A | N/A | A | A | A | **A** |
|
|
101 |
+ |
| **OAuth Callback Server** | A- | N/A | N/A | A | A- | A | **A-** |
|
|
102 |
+ |
| **Token Manager** | A | N/A | N/A | A | A | A | **A** |
|
|
103 |
+ |
| **IMAP Client** | A- | N/A | N/A | A | A- | A | **A-** |
|
|
104 |
+ |
| **SMTP Client** | B+ | N/A | N/A | A | B | A | **B+** |
|
|
105 |
+ |
| **JMAP Client** | A | N/A | N/A | A | A | A | **A** |
|
|
106 |
+ |
| **Plugin Runtime** | N/A | N/A | A | A | N/A | A | **A** |
|
|
107 |
+ |
| **SyncKit Integration** | A | A | N/A | A | A | A | **A** |
|
|
108 |
+ |
| **Backup/Export** | N/A | C+ | N/A | A | N/A | A- | **B+** |
|
|
109 |
+ |
| **Tauri Config/Capabilities** | N/A | N/A | N/A | B+ | N/A | A | **A-** |
|
|
110 |
+ |
|
|
111 |
+ |
### Module Heatmap — Performance
|
|
112 |
+ |
|
|
113 |
+ |
| Module | Query Efficiency | Async Correctness | Resource Mgmt | Concurrency | Frontend Perf | Scalability | Overall |
|
|
114 |
+ |
|--------|:---:|:---:|:---:|:---:|:---:|:---:|:---:|
|
|
115 |
+ |
| **db-sqlite pool/init** | A | A | A- | B+ | — | B+ | **A-** |
|
|
116 |
+ |
| **task_repo** | A- | A | A | A | — | B+ | **A-** |
|
|
117 |
+ |
| **email_repo** | B+ | A | B | A | — | B | **B+** |
|
|
118 |
+ |
| **stats_repo** | A+ | A | A | A | — | A | **A+** |
|
|
119 |
+ |
| **search_repo** | A | A | A | A | — | A- | **A** |
|
|
120 |
+ |
| **backup_scheduler** | B | A | B- | A- | — | B- | **B** |
|
|
121 |
+ |
| **commands/export** | B- | A | B- | A | — | C+ | **B-** |
|
|
122 |
+ |
| **commands/import_external** | B | A | A | A | — | B- | **B** |
|
|
123 |
+ |
| **frontend/cache.js** | — | — | A | — | A | A | **A** |
|
|
124 |
+ |
| **frontend/virtual-scroller.js** | — | — | A- | — | A | A | **A** |
|
|
125 |
+ |
| **state.rs** | A | A | A | A | — | A | **A** |
|
|
126 |
+ |
|
|
127 |
+ |
### Axis Summary Grades
|
|
128 |
+ |
|
|
129 |
+ |
| Axis | Overall | Cold Spots | Mandatory Surprise |
|
|
130 |
+ |
|------|:-------:|------------|-------------------|
|
|
131 |
+ |
| **Data Integrity** | A | time_session_repo validation (B+), FTS soft-delete leak | `complete_recurring` textbook transactional recurrence — wraps completion + next-instance in single tx with post-commit fresh fetch |
|
|
132 |
+ |
| **Sync & Storage** | A- | blob_sync.rs (B+), backup_scheduler.rs (B+), incomplete backup | `applying_remote` flag + WAL isolation in pull.rs — triple-layered crash safety with detailed comments explaining SQLite isolation semantics |
|
|
133 |
+ |
| **UX Wiring** | A- | main.rs registration (B-), bulk-actions.js (B-), events.js state key | Error humanization pipeline — `ERROR_CODE_LABELS` + `ERROR_CODE_HINTS` + UUID stripping + field metadata, end-to-end from DB to toast |
|
|
134 |
+ |
| **Security** | A | SMTP plaintext path (B), backup encryption absent (C+), CSP null | Server-side PKCE storage with atomic CSRF consumption — code verifier never leaves Rust, `flows.remove()` validates + consumes in one op |
|
|
135 |
+ |
| **Performance** | A- | backup/export unbounded queries (B-), email bodies in list queries (B), import N+1 | `rows_to_tasks` batch-loading — 3 batch queries replace N+1 for annotations/subtasks/sessions, consistently applied across all task list methods |
|
| 62 |
136 |
|
|
| 63 |
137 |
|
---
|
| 64 |
138 |
|
|
| 65 |
|
- |
## Mandatory Surprise
|
|
139 |
+ |
## Bug Reports by Axis
|
| 66 |
140 |
|
|
| 67 |
|
- |
**Symlink canonicalization vulnerability in plugin loader (loader.rs:118)**
|
|
141 |
+ |
### Data Integrity
|
| 68 |
142 |
|
|
| 69 |
|
- |
```rust
|
| 70 |
|
- |
let canonical = std::fs::canonicalize(&target)
|
| 71 |
|
- |
.or_else(|_| std::fs::canonicalize(&path))
|
| 72 |
|
- |
```
|
|
143 |
+ |
| # | Severity | Location | Description |
|
|
144 |
+ |
|---|----------|----------|-------------|
|
|
145 |
+ |
| 1 | MINOR | `time_session_repo.rs:329-384` | `log_manual_time` accepts negative/zero minutes |
|
|
146 |
+ |
| 2 | MINOR | `core/validation.rs` | `estimated_minutes` not validated (only `scheduled_duration` is) |
|
|
147 |
+ |
| 3 | MINOR | Multiple repos | `list_all()` queries lack LIMIT (acceptable for single-user desktop) |
|
|
148 |
+ |
| 4 | NOTE | `search_repo.rs:257-271` | FTS task search doesn't filter `status != 'Deleted'` (non-FTS branch does) |
|
|
149 |
+ |
| 5 | NOTE | `migrations/sqlite/023_contacts.sql:58-68` | contacts FTS5 omits `title` (job title) field |
|
| 73 |
150 |
|
|
| 74 |
|
- |
If the symlink target doesn't exist (dangling symlink), `canonicalize(target)` fails and the fallback canonicalizes the symlink entry itself (`enabled/xyz`). The entry is within the `available/` directory, so it passes the `starts_with` check — but the actual target, once created, could be anywhere on the filesystem.
|
|
151 |
+ |
**0 CRITICAL, 0 SERIOUS, 3 MINOR, 2 NOTE**
|
| 75 |
152 |
|
|
| 76 |
|
- |
**Attack scenario:** Create `enabled/evil → /etc/passwd` as dangling symlink. Canonicalize(target) fails → falls back to canonicalize(path), which passes validation. Then create the target at the original path.
|
|
153 |
+ |
### Sync & Storage
|
| 77 |
154 |
|
|
| 78 |
|
- |
**Risk:** Medium for security, but low blast radius — this is a desktop app where the attacker would already need local filesystem access. Fix: remove the `.or_else` fallback; skip symlinks whose target doesn't resolve.
|
|
155 |
+ |
| # | Severity | Location | Description |
|
|
156 |
+ |
|---|----------|----------|-------------|
|
|
157 |
+ |
| 1 | **SERIOUS** | `backup_scheduler.rs:114-134`, `export/backup.rs:17-33` | Backup excludes contacts, daily notes, milestones, time sessions, attachments |
|
|
158 |
+ |
| 2 | MEDIUM | `blob_sync.rs:53` | Blob upload reads entire file into memory (no streaming) |
|
|
159 |
+ |
| 3 | MEDIUM | `blob_sync.rs:122` | Blob download reads entire file into memory |
|
|
160 |
+ |
| 4 | LOW | `blob_sync.rs:130-139` | No hash verification after blob download |
|
|
161 |
+ |
| 5 | LOW | `attachment_repo.rs:141-149` | Orphaned blob files on attachment delete (no FS cleanup) |
|
|
162 |
+ |
| 6 | LOW | `backup_scheduler.rs:55,198` | Backup scheduler and manual backup can race (no lock) |
|
| 79 |
163 |
|
|
| 80 |
|
- |
---
|
|
164 |
+ |
**0 CRITICAL, 1 SERIOUS, 2 MEDIUM, 3 LOW**
|
| 81 |
165 |
|
|
| 82 |
|
- |
## Previous Action Item Verification
|
|
166 |
+ |
### UX Wiring
|
| 83 |
167 |
|
|
| 84 |
|
- |
| Item | Status |
|
| 85 |
|
- |
|------|--------|
|
| 86 |
|
- |
| FK migration crash protection | Non-issue (one-time, completed) |
|
| 87 |
|
- |
| Performance gaps (missing indexes) | Fixed (migration 040) |
|
| 88 |
|
- |
| Expand `#[instrument]` coverage | Fixed (435 total) |
|
| 89 |
|
- |
| format!() SQL safety documentation | Fixed |
|
| 90 |
|
- |
| Test count regression | False finding (was workspace flag issue) |
|
|
168 |
+ |
| # | Severity | Location | Description |
|
|
169 |
+ |
|---|----------|----------|-------------|
|
|
170 |
+ |
| 1 | **CRITICAL** | `main.rs:385-585` | 24 commands missing from desktop `generate_handler![]` |
|
|
171 |
+ |
| 2 | **SERIOUS** | `bulk-actions.js:176-198` | Bulk project/priority sends incomplete TaskInput, fails validation |
|
|
172 |
+ |
| 3 | MEDIUM | `events.js:471-489` | `deleteEvent` reads `GoingsOn.state.events` (never populated), should read `upcomingEvents`/`pastEvents` |
|
|
173 |
+ |
| 4 | LOW | `bulk-actions.js:23-38` | `Promise.all()` hides partial failure — should use `Promise.allSettled()` |
|
|
174 |
+ |
| 5 | LOW | `contacts.js:63,82` | Bulk error shows raw error object (`+ err` instead of `getErrorMessage()`) |
|
|
175 |
+ |
| 6 | LOW | `events.js:70` | Event bulk delete error shows raw error |
|
|
176 |
+ |
|
|
177 |
+ |
**1 CRITICAL, 1 SERIOUS, 1 MEDIUM, 3 LOW**
|
|
178 |
+ |
|
|
179 |
+ |
### Security
|
| 91 |
180 |
|
|
| 92 |
|
- |
**All Run 15 items resolved. No regressions.**
|
|
181 |
+ |
| # | Severity | Location | Description |
|
|
182 |
+ |
|---|----------|----------|-------------|
|
|
183 |
+ |
| 1 | MEDIUM | `tauri.conf.json:26` | CSP disabled (`null`) — no defense-in-depth against XSS in email HTML |
|
|
184 |
+ |
| 2 | MEDIUM | `smtp_client.rs:202,220` | `use_tls=false` sends credentials in cleartext via `builder_dangerous()` |
|
|
185 |
+ |
| 3 | LOW | `smtp_client.rs:143-151` | Legacy `SmtpClient::new()` reads password from account struct (pre-keychain path) |
|
|
186 |
+ |
| 4 | LOW-MEDIUM | `export/backup.rs` | Backup files are gzip only, no encryption |
|
|
187 |
+ |
| 5 | LOW | `notifications.rs:149-153` | OS notifications leak email sender + subject on lock screen |
|
|
188 |
+ |
| 6 | LOW | `state.rs:84` | SQLite database unencrypted at rest (standard for desktop apps) |
|
|
189 |
+ |
|
|
190 |
+ |
**0 CRITICAL, 0 SERIOUS, 2 MEDIUM, 4 LOW**
|
|
191 |
+ |
|
|
192 |
+ |
### Performance
|
|
193 |
+ |
|
|
194 |
+ |
| # | Severity | Location | Description |
|
|
195 |
+ |
|---|----------|----------|-------------|
|
|
196 |
+ |
| 1 | MEDIUM | `backup_scheduler.rs:130`, `commands/export.rs:105-108` | Backup loads ALL emails with bodies into memory (unbounded). `get_export_summary` fetches all rows just to return `.len()` |
|
|
197 |
+ |
| 2 | LOW-MEDIUM | `commands/task.rs:261-263` | `list_tasks` returns ALL tasks with annotations/subtasks — no limit |
|
|
198 |
+ |
| 3 | LOW-MEDIUM | `email_repo.rs:113,237,268` | Several email queries fetch full bodies without pagination |
|
|
199 |
+ |
| 4 | LOW | `commands/import_external.rs:143-150` | vCard/ICS import does serial per-record dedup (N+1) |
|
|
200 |
+ |
| 5 | LOW | `db-sqlite/lib.rs:33-34` | Pool size 5 may cause contention with 3 concurrent schedulers |
|
|
201 |
+ |
|
|
202 |
+ |
**0 CRITICAL, 0 SERIOUS, 1 MEDIUM, 4 LOW**
|
| 93 |
203 |
|
|
| 94 |
204 |
|
---
|
| 95 |
205 |
|
|
| 96 |
|
- |
## Strengths
|
|
206 |
+ |
## Cross-Cutting Concerns
|
| 97 |
207 |
|
|
| 98 |
|
- |
### 1. Exemplary layered architecture
|
| 99 |
|
- |
Four crates with strictly acyclic dependencies. Repository trait pattern allows testing without SQLite. Pre-computed response fields eliminate frontend duplication. No layer violations detected.
|
|
208 |
+ |
### Backup system has both coverage and performance problems
|
|
209 |
+ |
The incomplete backup (Sync axis: missing entity types) and the unbounded memory usage (Performance axis: loading all emails with bodies) are two facets of the same module. When fixing backup coverage, also fix the streaming/pagination issue.
|
| 100 |
210 |
|
|
| 101 |
|
- |
### 2. Systematic XSS prevention
|
| 102 |
|
- |
200+ `escapeHtml()`/`escapeAttr()` calls in frontend JS. The `escapeAttr()` pattern on inline event handlers is unusually thorough — closes the JavaScript attribute injection vector that most vanilla JS apps miss.
|
|
211 |
+ |
### Event delete state key bug affects both UX and data consistency
|
|
212 |
+ |
The `events.js` state key mismatch (UX axis) means optimistic UI removal doesn't work, and undo restoration does nothing. The actual deletion succeeds server-side, so no data loss — but the UI doesn't reflect the change until a refresh.
|
| 103 |
213 |
|
|
| 104 |
|
- |
### 3. Production-grade sync engine
|
| 105 |
|
- |
The `applying_remote` trigger suppression pattern prevents infinite sync loops. 1608 LOC of sync tests cover FK ordering, credential preservation, and mixed operations. Deterministic email IDs from Message-ID headers enable idempotent imports.
|
|
214 |
+ |
---
|
| 106 |
215 |
|
|
| 107 |
|
- |
### 4. Strong type system discipline
|
| 108 |
|
- |
11 entity ID newtypes. Exhaustive enums. Builder patterns. `CoreError` → `ApiError` conversion chain with structured error codes. No stringly-typed domain values.
|
|
216 |
+ |
## Components Successfully Stress-Tested
|
|
217 |
+ |
|
|
218 |
+ |
### Data Integrity
|
|
219 |
+ |
- SQL injection via FTS5 — `prepare_fts5_query` strips special chars, wraps in quotes
|
|
220 |
+ |
- Cross-user data leakage — every query includes `user_id = ?`
|
|
221 |
+ |
- Concurrent timer start race — transactional check-then-insert
|
|
222 |
+ |
- Recurrence chain breakage on crash — `complete_recurring` is transactional
|
|
223 |
+ |
- Month-end date drift — `add_months` clamps to target month length
|
|
224 |
+ |
- Foreign key cascades — correct ON DELETE CASCADE/SET NULL per relationship type
|
|
225 |
+ |
|
|
226 |
+ |
### Sync & Storage
|
|
227 |
+ |
- Sync conflict resolution — LWW with explicit conflict detection and three-way resolution
|
|
228 |
+ |
- Sync state machine — cannot get stuck; triple-reset of `applying_remote` flag
|
|
229 |
+ |
- FK ordering — `UPSERT_ORDER` and `DELETE_ORDER` are verified reverses (tested)
|
|
230 |
+ |
- Trigger suppression — remote changes don't fire sync changelog triggers (WAL isolation)
|
|
231 |
+ |
- CSV formula injection — `sanitize_csv_field` prefixes dangerous characters
|
|
232 |
+ |
- Backup atomicity — write-to-tmp + rename. Decompression bomb guard (500MB limit)
|
|
233 |
+ |
- SSE reconnection — detected + reconnected after 5s delay
|
|
234 |
+ |
- DB watcher — debounced (500ms) + rate-limited (1s) + trailing edge timer
|
|
235 |
+ |
|
|
236 |
+ |
### UX Wiring
|
|
237 |
+ |
- XSS prevention — `escapeHtml`/`escapeAttr` used consistently across all modules
|
|
238 |
+ |
- Pagination — `PaginationManager` handles negative/overflow/empty/count-change
|
|
239 |
+ |
- Form builder — all field types, custom validators, abort controller prevents duplicate submit
|
|
240 |
+ |
- Cache invalidation — generation-based + 30s TTL, mutations invalidate before API call
|
|
241 |
+ |
- State reactivity — pub/sub correctly notifies; virtual scrollers refresh from state
|
|
242 |
+ |
|
|
243 |
+ |
### Security
|
|
244 |
+ |
- OAuth PKCE — S256 challenge, code verifier never leaves Rust, CSRF state consumed atomically
|
|
245 |
+ |
- Credential storage — OS keychain via `keyring`, `#[serde(skip_serializing)]` on sensitive fields
|
|
246 |
+ |
- Plugin sandbox — eval disabled, operation limits (100K), stack depth (32), size limits, canonical path checks
|
|
247 |
+ |
- SQL injection — all queries parameterized; sync table/column names from `&'static str` whitelists
|
|
248 |
+ |
- E2E encryption — XChaCha20-Poly1305, Argon2id (64MB/3 iterations), proper key hierarchy, zeroize-on-drop
|
|
249 |
+ |
- Email credential isolation — sync excludes password/OAuth token columns via `EMAIL_ACCOUNT_SYNC_COLS`
|
|
250 |
+ |
- Callback server — `127.0.0.1` only, HTML escaped, non-GET rejected, 5-minute timeout
|
|
251 |
+ |
|
|
252 |
+ |
### Performance
|
|
253 |
+ |
- SQLite WAL mode — correctly enabled, mitigates single-writer contention
|
|
254 |
+ |
- Batch loading — `rows_to_tasks` does 3 batch queries instead of N+1
|
|
255 |
+ |
- Token refresh serialization — per-account Mutex with double-check pattern
|
|
256 |
+ |
- IMAP oversized email filtering — pre-filters by `RFC822.SIZE` (25MB cap), `spawn_blocking`
|
|
257 |
+ |
- Virtual scroller — height caching, overscan buffer, RAF scroll, ResizeObserver cleanup
|
|
258 |
+ |
- FTS5 search — parallel 5-type search via `tokio::join!`, BM25 ranking, per-type caps
|
| 109 |
259 |
|
|
| 110 |
260 |
|
---
|
| 111 |
261 |
|
|
| 112 |
|
- |
## Weaknesses
|
|
262 |
+ |
## Confidence Assessment
|
|
263 |
+ |
|
|
264 |
+ |
| Axis | Confidence | Justification |
|
|
265 |
+ |
|------|:----------:|---------------|
|
|
266 |
+ |
| Data Integrity | HIGH | All SQL parameterized, user-scoped, newtype IDs, FK integrity verified |
|
|
267 |
+ |
| Sync & Storage | HIGH | Every file read end-to-end, sync engine well-structured, small attack surface |
|
|
268 |
+ |
| UX Wiring | HIGH | Bugs 1-3 confirmed by diffing/tracing; contract alignment verified across domains |
|
|
269 |
+ |
| Security | HIGH | Full OAuth flow traced, plugin sandbox verified, encryption algorithm audited |
|
|
270 |
+ |
| Performance | HIGH | 19 modules fully traced, query patterns verified, pool config audited |
|
| 113 |
271 |
|
|
| 114 |
|
- |
### 1. Plugin runtime size inefficiency
|
| 115 |
|
- |
api.rs (1103 LOC) and registry.rs (1243 LOC) are bloated relative to their functional complexity. Double CSV reader, 104 clone() calls on hot path, test helpers that should be extracted.
|
|
272 |
+ |
---
|
| 116 |
273 |
|
|
| 117 |
|
- |
### 2. External sync test coverage
|
| 118 |
|
- |
ical.rs and vcard.rs parsers share logic but have minimal dedicated tests. Rely on integration tests that may not exercise edge cases (malformed iCal, partial vCards).
|
|
274 |
+ |
## Metrics
|
| 119 |
275 |
|
|
| 120 |
|
- |
### 3. No exponential backoff on email sync
|
| 121 |
|
- |
Scheduler retries every 60s regardless of failure type. Transient server issues are handled, but sustained outages will produce a wall of log noise.
|
|
276 |
+ |
- Modules audited: 60+
|
|
277 |
+ |
- Total cold spots: 7
|
|
278 |
+ |
- Bugs by severity: 1 critical, 2 serious, 6 medium, 14 minor/low, 2 note
|
|
279 |
+ |
- Axes at A or above: 3/5 (Data A, Sync A-, UX A-, Security A, Performance A-)
|
| 122 |
280 |
|
|
| 123 |
281 |
|
---
|
| 124 |
282 |
|
|
| 125 |
|
- |
## Action Items
|
|
283 |
+ |
## Recommended Priority Order
|
|
284 |
+ |
|
|
285 |
+ |
1. **[CRITICAL] Register 24 missing commands in main.rs** — entire feature categories broken on desktop. Minutes to fix, massive blast radius.
|
|
286 |
+ |
2. **[SERIOUS] Fix bulk project/priority update** — create partial-update command or fetch-then-merge. Users will hit this immediately.
|
|
287 |
+ |
3. **[SERIOUS] Complete backup entity coverage** — add contacts, daily notes, milestones, time sessions, attachments to `FullExport`. Data loss risk on restore.
|
|
288 |
+ |
4. **[MEDIUM] Fix event delete state key** — `GoingsOn.state.events` → `upcomingEvents`/`pastEvents`. Simple JS fix.
|
|
289 |
+ |
5. **[MEDIUM] Add CSP to tauri.conf.json** — defense-in-depth, especially since app renders email HTML.
|
|
290 |
+ |
6. **[MEDIUM] Stream blob uploads/downloads** — currently loads entire file into memory. Matters for large attachments.
|
|
291 |
+ |
7. **[MEDIUM] Fix backup memory usage** — `get_export_summary` should use COUNT; backup should stream.
|
|
292 |
+ |
8. **[LOW] Add SMTP TLS warning** — log warning or show UI indicator when `use_tls=false`.
|
|
293 |
+ |
9. **[LOW] Validate `estimated_minutes` and `log_manual_time` minutes** — reject negative/zero.
|
|
294 |
+ |
10. **[LOW] Fix FTS soft-delete leak** — add `AND t.status != 'Deleted'` to FTS task search.
|
|
295 |
+ |
11. **[LOW] Use `Promise.allSettled()` in bulk-actions.js** — report partial failures.
|
|
296 |
+ |
12. **[LOW] Fix raw error display in contacts.js and events.js bulk operations**.
|
|
297 |
+ |
13. **[LOW] Verify blob hash after download** — detect transit corruption.
|
|
298 |
+ |
14. **[LOW] Add blob GC for orphaned files on attachment delete**.
|
| 126 |
299 |
|
|
| 127 |
|
- |
### Run 19 (2026-05-04)
|
|
300 |
+ |
---
|
| 128 |
301 |
|
|
| 129 |
|
- |
Filed in `docs/todo/todo.md`:
|
|
302 |
+ |
## Delta Since Run 19
|
| 130 |
303 |
|
|
| 131 |
|
- |
1. **[MEDIUM]** Fix symlink canonicalization in plugin loader (loader.rs:118) — remove `.or_else` fallback
|
| 132 |
|
- |
2. **[LOW]** Extract shared date/recurrence parsing from ical.rs and vcard.rs into common module
|
| 133 |
|
- |
3. **[LOW]** Add exponential backoff to email_sync_scheduler on consecutive failures
|
| 134 |
|
- |
4. **[LOW]** Optimize plugin CSV parser to single-pass (api.rs double reader)
|
|
304 |
+ |
### Fixed from Run 19
|
|
305 |
+ |
| Item | Status |
|
|
306 |
+ |
|------|--------|
|
|
307 |
+ |
| Symlink canonicalization in plugin loader (loader.rs:118) | **FIXED** — dangling symlinks now skipped (confirmed by security agent) |
|
| 135 |
308 |
|
|
| 136 |
|
- |
### Deferred
|
| 137 |
|
- |
- Split emails.js into sub-modules when it exceeds 1500 LOC
|
| 138 |
|
- |
- Add execution timeout to Rhai engine (in addition to operation limit)
|
| 139 |
|
- |
- Reduce clone() calls in plugin API hot path
|
|
309 |
+ |
### Unfixed from Run 19
|
|
310 |
+ |
| Item | Status |
|
|
311 |
+ |
|------|--------|
|
|
312 |
+ |
| Extract shared date/recurrence parsing from ical.rs/vcard.rs | **DEFERRED** — still separate implementations |
|
|
313 |
+ |
| Add exponential backoff to email_sync_scheduler | **DEFERRED** — still retries every 60s |
|
|
314 |
+ |
| Optimize plugin CSV parser to single-pass | **DEFERRED** — still double-read |
|
|
315 |
+ |
|
|
316 |
+ |
### New findings this run
|
|
317 |
+ |
- 1 CRITICAL (24 missing desktop commands)
|
|
318 |
+ |
- 2 SERIOUS (bulk update broken, incomplete backup)
|
|
319 |
+ |
- 6 MEDIUM (event delete state, CSP, blob memory, backup memory, SMTP plaintext, blob download)
|
|
320 |
+ |
- 14 LOW/MINOR/NOTE
|
|
321 |
+ |
|
|
322 |
+ |
### Grade changes
|
|
323 |
+ |
| Dimension | Run 19 | Run 24 | Delta |
|
|
324 |
+ |
|-----------|:------:|:------:|:-----:|
|
|
325 |
+ |
| Overall | A | A | — |
|
|
326 |
+ |
| Security | A | A | — |
|
|
327 |
+ |
| Performance | A- | A- | — |
|
|
328 |
+ |
| Architecture | A | A | — |
|
|
329 |
+ |
| Testing | A | A | — |
|
|
330 |
+ |
|
|
331 |
+ |
### CHRONIC items (unfixed across 2+ runs)
|
|
332 |
+ |
- **Email sync exponential backoff** — deferred since Run 19. Low severity, no user impact yet.
|
|
333 |
+ |
- **Plugin CSV double-read** — deferred since Run 19. Low severity, plugin system rarely used.
|
| 140 |
334 |
|
|
| 141 |
335 |
|
---
|
| 142 |
336 |
|
|
| 154 |
348 |
|
| 2026-03-28 (Run 12) | ~44K | ~734 | ~16.7 | 0 | 0 | A |
|
| 155 |
349 |
|
| 2026-04-15 (Run 14) | ~64K | ~762 | ~12 | 0 | 0 | A |
|
| 156 |
350 |
|
| 2026-04-22 (Run 15) | ~64K | 778 | ~12.1 | 0 | 0 | A |
|
| 157 |
|
- |
| **2026-05-04 (Run 19)** | **~69K** | **765** | **~11.1** | **0** | **6** | **A** |
|
|
351 |
+ |
| 2026-05-04 (Run 19) | ~69K | 765 | ~11.1 | 0 | 6 | A |
|
|
352 |
+ |
| **2026-05-10 (Run 24)** | **~74K** | **773** | **~10.4** | **0** | **7** | **A** |
|
| 158 |
353 |
|
|
| 159 |
354 |
|
### Delta Since Last Audit
|
| 160 |
|
- |
- **LOC:** +4,443 (new email features: signatures, drafts, labels, notifications)
|
| 161 |
|
- |
- **Tests:** -13 (likely removed obsolete tests during refactoring — within normal variance)
|
| 162 |
|
- |
- **Cold spots:** +6 (deeper audit methodology this run; all low severity)
|
|
355 |
+ |
- **LOC:** +4,600 (email features, sync hardening, notifications)
|
|
356 |
+ |
- **Tests:** +8 (773 vs 765)
|
|
357 |
+ |
- **Cold spots:** +1 (deeper 5-axis methodology found bulk-actions.js and backup)
|
| 163 |
358 |
|
- **Grade:** A (maintained)
|
|
359 |
+ |
- **Key fix:** Symlink canonicalization vulnerability from Run 19 resolved
|
| 164 |
360 |
|
|
| 165 |
361 |
|
---
|
| 166 |
362 |
|
|