|
|
|
@@ -0,0 +1,399 @@
|
|
|
|
|
# OwnCord — Comprehensive Project Audit
|
|
|
|
|
**Date:** 2026-04-07
|
|
|
|
|
**Branch:** claude/plan-phases-b-c-bGpoS
|
|
|
|
|
**Audited by:** 6 parallel Claude agents across 8 dimensions
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Table of Contents
|
|
|
|
|
1. [Architecture](#1-architecture)
|
|
|
|
|
2. [Code Quality](#2-code-quality)
|
|
|
|
|
3. [Security](#3-security)
|
|
|
|
|
4. [Dependencies & Supply Chain](#4-dependencies--supply-chain)
|
|
|
|
|
5. [Test Coverage & Quality](#5-test-coverage--quality)
|
|
|
|
|
6. [CI/CD & DevEx](#6-cicd--devex)
|
|
|
|
|
7. [Observability](#7-observability)
|
|
|
|
|
8. [Plugin System Governance](#8-plugin-system-governance)
|
|
|
|
|
9. [Prioritized Top-10 Action List](#9-prioritized-top-10-action-list)
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 1. Architecture
|
|
|
|
|
|
|
|
|
|
### Layer Map
|
|
|
|
|
|
|
|
|
|
| Layer | Location | Responsibility |
|
|
|
|
|
|-------|----------|---------------|
|
|
|
|
|
| **Client UI** | `Client/tauri-client/src/components/` | SolidJS UI components, settings, voice |
|
|
|
|
|
| **Client Pages** | `Client/tauri-client/src/pages/` | MainPage, ConnectPage entry points |
|
|
|
|
|
| **Client Stores** | `Client/tauri-client/src/stores/` | Reactive state (auth, channels, messages, voice, dm, ui, roles) |
|
|
|
|
|
| **Client Lib** | `Client/tauri-client/src/lib/` | API client, WebSocket client, dispatcher, LiveKit session, theme |
|
|
|
|
|
| **Tauri Rust** | `Client/tauri-client/src-tauri/` | WS proxy, credential manager, PTT, update notifications |
|
|
|
|
|
| **Server API** | `Server/api/` | REST handlers (auth, channels, messages, DMs, invites, uploads) |
|
|
|
|
|
| **Server WS** | `Server/ws/` | Real-time hub, event persistence, command dispatch, LiveKit integration |
|
|
|
|
|
| **Server Service** | `Server/service/` | Domain business logic (Message, Channel, Permission, User, DM, Invite, Block, Voice) |
|
|
|
|
|
| **Server Store** | `Server/store/` | Data access layer (SQLiteStore, PostgresStore stub, MemStore) |
|
|
|
|
|
| **Server DB** | `Server/db/` | SQL schema, migrations, sqlc-generated queries |
|
|
|
|
|
| **Server Plugin** | `Server/plugin/` | WASM runtime (Wazero), loader, manifest, registry, capability-scoped host APIs |
|
|
|
|
|
| **Server Admin** | `Server/admin/` | Admin UI, backup management, log streaming |
|
|
|
|
|
| **Server Auth** | `Server/auth/` | Authentication, session management, rate limiting, TOTP |
|
|
|
|
|
|
|
|
|
|
### Communication Patterns
|
|
|
|
|
|
|
|
|
|
| Channel | Usage |
|
|
|
|
|
|---------|-------|
|
|
|
|
|
| **REST API** | Auth, channel CRUD, file uploads, initial data fetch — uses Tauri HTTP plugin for self-signed cert support |
|
|
|
|
|
| **WebSocket** | Real-time events (messages, presence, typing, voice) — single connection per client, envelope format `{type, id, payload}` |
|
|
|
|
|
| **Tauri IPC** | `ws_connect/send/disconnect`, credential manager, PTT, update checks — desktop-only features |
|
|
|
|
|
| **LiveKit** | HTTP voice token acquisition, reverse-proxy WebSocket signaling at `/livekit/*`, webhook for participant events |
|
|
|
|
|
|
|
|
|
|
### Dependency Direction
|
|
|
|
|
|
|
|
|
|
Layering is **healthy and mostly respected**:
|
|
|
|
|
- ✅ `api/ → service/ → store/` holds
|
|
|
|
|
- ✅ `ws/` does not import `api/` (no circular deps)
|
|
|
|
|
- ✅ Plugin system imports service layer cleanly
|
|
|
|
|
- ✅ Auth middleware isolated; not interspersed with handlers
|
|
|
|
|
|
|
|
|
|
### Anti-patterns
|
|
|
|
|
|
|
|
|
|
| SEVERITY | File | Finding |
|
|
|
|
|
|----------|------|---------|
|
|
|
|
|
| MEDIUM | `Server/api/auth_handler.go` | Auth handler queries DB directly, bypassing service layer — inconsistent with `channel_handler.go` pattern |
|
|
|
|
|
| MEDIUM | `Client/tauri-client/src/lib/dispatcher.ts` | 84 registered listeners creates implicit coupling; hard to trace data flow |
|
|
|
|
|
| MEDIUM | `Client/tauri-client/src/lib/livekitSession.ts` (1710 lines) | Monolith mixing LiveKit SDK, audio pipeline, diagnostics — should decompose |
|
|
|
|
|
| MEDIUM | `Client/tauri-client/src/pages/MainPage.ts` (554 lines) | Mixes routing, layout, store subscriptions, cleanup — violates single responsibility |
|
|
|
|
|
| MEDIUM | `Server/service/message.go` (715 lines) | Combines send/search/delete/edit/fetch — split into focused services |
|
|
|
|
|
| LOW | `Client/tauri-client/src/main.ts` (538 lines) | Entry point handles init, error handlers, theme, health checks, connection orchestration |
|
|
|
|
|
| LOW | `Server/api/router.go` (394 lines) | Mixes middleware setup, route registration, and business logic wiring |
|
|
|
|
|
| LOW | `Server/admin/` | Admin package operates directly on DB; not using service layer — may diverge from REST/WS semantics |
|
|
|
|
|
| LOW | PostgreSQL backend | Fully scaffolded but query methods stubbed; migration path undocumented |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 2. Code Quality
|
|
|
|
|
|
|
|
|
|
### Go — Error Handling
|
|
|
|
|
|
|
|
|
|
| SEVERITY | File:Line | Finding |
|
|
|
|
|
|----------|-----------|---------|
|
|
|
|
|
| MEDIUM | `Server/admin/logstream.go:398-426` | SSE handler ignores write errors with `_, _ = fmt.Fprintf()` — client disconnections not detectable |
|
|
|
|
|
| MEDIUM | `Server/admin/handlers_backup.go:55,139` | `database.LogAudit()` errors silently discarded with `_ =` — audit trail may silently fail |
|
|
|
|
|
| MEDIUM | `Server/admin/handlers_users.go` (5 instances) | `LogAudit()` errors discarded — affects compliance/security audit trail |
|
|
|
|
|
| LOW | `Server/updater/updater.go` (13 instances) | Excessive `_ =` error suppression; no logging fallback |
|
|
|
|
|
| LOW | `Server/db/db.go` (9 instances) | Unchecked errors in setup/teardown paths |
|
|
|
|
|
|
|
|
|
|
### Go — Interface Design
|
|
|
|
|
|
|
|
|
|
- ✅ **Strong**: Store package has clear interface hierarchy (`Store → MessageStore, ChannelStore, UserStore, SessionStore, RoleStore`) with proper concrete implementations
|
|
|
|
|
- ⚠️ **Concern**: Service layer handlers receive concrete `*db.DB` or `*Hub` rather than small interfaces — reduces testability
|
|
|
|
|
|
|
|
|
|
### Go — Large Files (>800 lines)
|
|
|
|
|
|
|
|
|
|
| File | Lines |
|
|
|
|
|
|------|-------|
|
|
|
|
|
| `Server/ws/hub.go` | 919 — mixes client management, event persistence, plugin integration, voice key-holding |
|
|
|
|
|
| `Server/store/memstore.go` | 766 |
|
|
|
|
|
| `Server/service/message.go` | 715 |
|
|
|
|
|
| `Server/store/postgres.go` | 833 — largely stubbed scaffolding |
|
|
|
|
|
|
|
|
|
|
### Go — Security-Relevant TODOs
|
|
|
|
|
|
|
|
|
|
| File | TODO |
|
|
|
|
|
|------|------|
|
|
|
|
|
| `Server/ws/command.go` | Validate attachment URL scheme (require HTTPS) — security gap |
|
|
|
|
|
| `Server/ws/registry.go` | Stack trace may contain sensitive function arguments |
|
|
|
|
|
| `Server/ws/voice_leave.go` | Propagate context through `livekit.RemoveParticipant()` call |
|
|
|
|
|
| `Server/ws/voice_e2ee.go` | Re-check key-holder status inside `sendToUserIfInVoiceChannel` |
|
|
|
|
|
| `Server/store/sqlite.go` | Incomplete transaction-scoped store wrapper |
|
|
|
|
|
|
|
|
|
|
### TypeScript — Type Safety
|
|
|
|
|
|
|
|
|
|
| SEVERITY | File:Line | Finding |
|
|
|
|
|
|----------|-----------|---------|
|
|
|
|
|
| MEDIUM | Multiple components | ~43 event handlers with untyped `(e)` parameters — should be typed as `MouseEvent`, `DragEvent`, etc. |
|
|
|
|
|
| LOW | `src/lib/audioPipeline.ts:75` | Single `as any` cast (justified by eslint-disable comment; acceptable) |
|
|
|
|
|
|
|
|
|
|
### TypeScript — Large Components
|
|
|
|
|
|
|
|
|
|
| File | Lines | Action |
|
|
|
|
|
|------|-------|--------|
|
|
|
|
|
| `src/components/settings/AccountTab.ts` | 845 | **Extract**: password-section.ts, totp-section.ts, status-selector.ts |
|
|
|
|
|
| `src/components/EmojiPicker.ts` | 665 | Extract emoji data file |
|
|
|
|
|
| `src/components/MessageList.ts` | 662 | Extract height-calc module |
|
|
|
|
|
| `src/components/MessageInput.ts` | 599 | Reasonable given feature density |
|
|
|
|
|
|
|
|
|
|
### TypeScript — Error Handling Gaps
|
|
|
|
|
|
|
|
|
|
| SEVERITY | File | Finding |
|
|
|
|
|
|----------|------|---------|
|
|
|
|
|
| MEDIUM | `src/components/InviteManager.ts:96,151` | `.then()` chains without `.catch()` — silent rejection |
|
|
|
|
|
| MEDIUM | `src/components/message-input/file-upload.ts:57` | `.then()` without catch — corrupt file failures silent |
|
|
|
|
|
| MEDIUM | `src/components/message-list/attachments.ts:341` | `void fetchImageAsDataUrl().then()` — image load failures not surfaced |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 3. Security
|
|
|
|
|
|
|
|
|
|
### Overall Posture: **GOOD** (no critical issues in core app security)
|
|
|
|
|
|
|
|
|
|
### Secrets & Configuration
|
|
|
|
|
- ✅ `.env.example` uses placeholder values; file gitignored
|
|
|
|
|
- ✅ No hardcoded secrets found in source, Tauri config, or configs
|
|
|
|
|
- ✅ LiveKit credentials require 32+ character minimum
|
|
|
|
|
|
|
|
|
|
### Input Validation
|
|
|
|
|
- ✅ Username validation with `auth.ValidateUsername()` + bluemonday HTML sanitization
|
|
|
|
|
- ✅ File uploads use `filepath.Base()` to prevent path traversal
|
|
|
|
|
- ✅ Plugin uploads: magic-byte ZIP validation, 16 MiB hard cap, decompression bomb protection, symlink rejection
|
|
|
|
|
- MEDIUM | `Server/api/auth_handler.go:151` | Username validation doesn't explicitly check for Unicode control characters, RTL overrides, or zero-width chars — potential homograph attack vector
|
|
|
|
|
|
|
|
|
|
### SQL Injection
|
|
|
|
|
- ✅ All standard queries use parameterized statements
|
|
|
|
|
- ✅ `SearchMessagesInChannels` uses `fmt.Sprintf` only for `?` placeholder structure (not user data) — **safe by design**
|
|
|
|
|
- ✅ FTS query sanitization: whitelist-only (letters, digits, spaces, hyphens), max 200 runes
|
|
|
|
|
|
|
|
|
|
### Authentication & Authorization
|
|
|
|
|
- ✅ `AuthMiddleware`: Bearer token extraction → hash → session lookup → expiry → ban check → context injection
|
|
|
|
|
- ✅ `RequirePermission`: Admin bit (0x40000000) bypasses; 403 on insufficient permissions
|
|
|
|
|
- ✅ WebSocket in-band auth within 10-second deadline; re-validated every 10 messages
|
|
|
|
|
- ✅ Fail-closed: nil role = zero access
|
|
|
|
|
- ✅ Permission-filtered reconnect replay (prevents data leakage after permission changes)
|
|
|
|
|
|
|
|
|
|
### Rate Limiting
|
|
|
|
|
- ✅ Per-endpoint configurable rate limits with 429 + Retry-After
|
|
|
|
|
- ✅ Login, register, TOTP verification all rate-limited
|
|
|
|
|
- ✅ Sensitive endpoints (account deletion, TOTP management) rate-limited
|
|
|
|
|
- ✅ IP extraction validates trusted CIDR proxies — prevents rate-limit bypass via spoofed headers
|
|
|
|
|
|
|
|
|
|
### Tauri Security
|
|
|
|
|
- ✅ CSP enforced; `withGlobalTauri: false` (Tauri API not exposed on `window`)
|
|
|
|
|
- ✅ Capabilities-based permission model (Tauri v2)
|
|
|
|
|
- ✅ No devtools in production build
|
|
|
|
|
|
|
|
|
|
### WebSocket Security
|
|
|
|
|
- ✅ Auth deadline prevents resource exhaustion from slow clients
|
|
|
|
|
- ✅ Priority-based send channels (high/normal/low) prevent low-priority spam blocking critical messages
|
|
|
|
|
- ✅ Read limit enforced (`wsReadLimitBytes = config.MaxMessageBytes`)
|
|
|
|
|
|
|
|
|
|
### Observations (not blocking)
|
|
|
|
|
|
|
|
|
|
| SEVERITY | Area | Finding |
|
|
|
|
|
|----------|------|---------|
|
|
|
|
|
| MEDIUM | Username validation | Add explicit Unicode control character / zero-width char rejection |
|
|
|
|
|
| MEDIUM | TOTP secrets | Confirm AES-256 encryption of stored secrets with per-record salt |
|
|
|
|
|
| MEDIUM | LiveKit webhook | Confirm webhook signature validation cannot be bypassed with malformed JWT |
|
|
|
|
|
| LOW | Session tokens | Confirm cryptographically secure RNG and per-session salt for token hashing |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 4. Dependencies & Supply Chain
|
|
|
|
|
|
|
|
|
|
### Overall Posture: **MODERATE RISK** (Go excellent, npm floating)
|
|
|
|
|
|
|
|
|
|
### Go Modules — 30 direct deps, ALL exact-pinned ✅
|
|
|
|
|
Notable: `golang.org/x/crypto v0.49.0`, `github.com/corazawaf/coraza/v3 v3.6.0`, `github.com/tetratelabs/wazero v1.11.0`, `modernc.org/sqlite v1.48.0`
|
|
|
|
|
|
|
|
|
|
### npm — 13 production deps, ALL floating (^) ⚠️
|
|
|
|
|
|
|
|
|
|
| Risk | Package | Version |
|
|
|
|
|
|------|---------|---------|
|
|
|
|
|
| HIGH | `@tauri-apps/plugin-updater` | `^2.10.0` — update mechanism, should pin |
|
|
|
|
|
| HIGH | `@tauri-apps/plugin-global-shortcut` | `^2` — any 2.x allowed |
|
|
|
|
|
| HIGH | `@tauri-apps/plugin-notification` | `^2` — any 2.x allowed |
|
|
|
|
|
| HIGH | `@tauri-apps/plugin-store` | `^2` — any 2.x allowed |
|
|
|
|
|
| MEDIUM | `livekit-client` | `^2.18.0` |
|
|
|
|
|
| MEDIUM | `solid-js` | `^1.9.3` |
|
|
|
|
|
| LOW | `zod` | `^4.3.6` |
|
|
|
|
|
|
|
|
|
|
**Mitigating factor:** `package-lock.json` (8,832 lines) is committed and locks transitive deps.
|
|
|
|
|
|
|
|
|
|
### Lockfile Status
|
|
|
|
|
|
|
|
|
|
| Ecosystem | Status |
|
|
|
|
|
|-----------|--------|
|
|
|
|
|
| Go | ✅ `go.sum` committed, 455 entries |
|
|
|
|
|
| npm (client) | ✅ `package-lock.json` committed, 8,832 lines |
|
|
|
|
|
| npm (root) | ✅ `package-lock.json` committed, 492 lines |
|
|
|
|
|
|
|
|
|
|
### License Compliance
|
|
|
|
|
- OwnCord: **AGPLv3**
|
|
|
|
|
- All direct dependencies: Apache 2.0 or MIT — **no copyleft incompatibilities**
|
|
|
|
|
|
|
|
|
|
### Known Vulnerabilities
|
|
|
|
|
- No CRITICAL CVEs in direct dependencies as of Feb 2025
|
|
|
|
|
- `golang.org/x/crypto v0.49.0` — verify no disclosed CVEs since mid-2024 release
|
|
|
|
|
- Run `npm audit` and `govulncheck` periodically
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 5. Test Coverage & Quality
|
|
|
|
|
|
|
|
|
|
### Go — Package Coverage
|
|
|
|
|
|
|
|
|
|
| Package | Test Files | Status |
|
|
|
|
|
|---------|-----------|--------|
|
|
|
|
|
| `api/` | 23 | ✅ Heavy behavioral coverage |
|
|
|
|
|
| `ws/` | 32 | ✅ Connect/disconnect, commands, auth, deadlock |
|
|
|
|
|
| `auth/` | 8 | ✅ Core flows, TOTP, helpers |
|
|
|
|
|
| `db/` | 18 | ✅ Operations and migrations |
|
|
|
|
|
| `admin/` | 11 | ✅ Good coverage |
|
|
|
|
|
| `plugin/` | 6 | ✅ Loading, sandbox, manifest |
|
|
|
|
|
| `permissions/` | 3 | ✅ |
|
|
|
|
|
| `service/` | 2 | ⚠️ Only message + permission |
|
|
|
|
|
| `**store/**` | **0** | ❌ **CRITICAL GAP — data persistence untested** |
|
|
|
|
|
| `syncutil/` | 0 | ⚠️ Concurrency utils untested |
|
|
|
|
|
| `scripts/` | 0 | Low priority (CLI tool) |
|
|
|
|
|
|
|
|
|
|
### Go — Test Quality: GOOD
|
|
|
|
|
- Behavioral tests using in-memory SQLite (`:memory:`) with full schema
|
|
|
|
|
- Table-driven patterns used throughout
|
|
|
|
|
- `goleak.VerifyTestMain()` in `auth/` — goroutine leak detection
|
|
|
|
|
- Deadlock detection tests via `go test -tags deadlock`
|
|
|
|
|
|
|
|
|
|
### Go — Critical Coverage Gaps
|
|
|
|
|
|
|
|
|
|
| SEVERITY | Gap |
|
|
|
|
|
|----------|-----|
|
|
|
|
|
| HIGH | `Server/store/` — data persistence layer has zero test coverage |
|
|
|
|
|
| HIGH | No PostgreSQL integration tests — all use in-memory SQLite; schema drift not caught |
|
|
|
|
|
| MEDIUM | `Server/syncutil/` — concurrency utilities untested |
|
|
|
|
|
| MEDIUM | Migration safety not validated across versions |
|
|
|
|
|
|
|
|
|
|
### TypeScript — Test Files
|
|
|
|
|
- **1 component unit test**: `src/components/solid/Badge.test.tsx`
|
|
|
|
|
- **44 E2E specs**: Playwright (20 browser-mode, 22 native-mode + helpers)
|
|
|
|
|
- **1 jsdom smoke test**: `tests/browser/smoke.test.ts`
|
|
|
|
|
|
|
|
|
|
### TypeScript — E2E Coverage: EXCELLENT
|
|
|
|
|
Auth flow, channels, messages, DMs, health/reconnect, UI overlays, voice controls, theme persistence — all covered in 44 Playwright specs.
|
|
|
|
|
|
|
|
|
|
### TypeScript — Unit Coverage: MINIMAL (<10%)
|
|
|
|
|
|
|
|
|
|
| SEVERITY | Gap |
|
|
|
|
|
|----------|-----|
|
|
|
|
|
| HIGH | No tests for `src/lib/` utilities (api.ts, ws.ts, dispatcher.ts, livekitSession.ts) |
|
|
|
|
|
| HIGH | No tests for `src/stores/` (messages, channels, voice, auth) |
|
|
|
|
|
| MEDIUM | 7 files explicitly excluded from coverage thresholds: `main.ts`, `updater.ts`, `credentials.ts`, etc. |
|
|
|
|
|
| MEDIUM | 70% coverage threshold configured in vitest but unit tests too sparse to enforce |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 6. CI/CD & DevEx
|
|
|
|
|
|
|
|
|
|
### Pipeline Gates
|
|
|
|
|
|
|
|
|
|
| Job | OS | Gates |
|
|
|
|
|
|-----|----|-------|
|
|
|
|
|
| `server-build-test` | Windows + Ubuntu | ✅ Build (4 tag variants), Tests (race + deadlock), golangci-lint, govulncheck |
|
|
|
|
|
| `client-check` | Windows | ✅ TypeScript check, ESLint, Oxlint, Prettier, Vitest, npm audit, Knip |
|
|
|
|
|
| `server-docker-build` | Ubuntu | ✅ Docker build verification |
|
|
|
|
|
| `tauri-build` | Windows + Ubuntu + Ubuntu-ARM | ✅ Rust lint, Rust audit, full Tauri build — PR to main only |
|
|
|
|
|
|
|
|
|
|
### Linting Enforcement
|
|
|
|
|
- **Go**: golangci-lint v2.11.3 — hard fail; rules: gocritic, gosec, errcheck, bodyclose, contextcheck, staticcheck
|
|
|
|
|
- **TypeScript**: ESLint with `no-floating-promises: error`, `no-unused-vars: error`; Prettier format check; Oxlint
|
|
|
|
|
- **Enforcement**: Hard fail for all linting/formatting in CI ✅
|
|
|
|
|
|
|
|
|
|
### Gaps
|
|
|
|
|
|
|
|
|
|
| SEVERITY | Finding |
|
|
|
|
|
|----------|---------|
|
|
|
|
|
| MEDIUM | E2E tests not in main CI — only run on PRs to main, not on merge. Code could merge without E2E gate |
|
|
|
|
|
| MEDIUM | No branch protection rules visible in repo config |
|
|
|
|
|
| LOW | No `.nvmrc` / `.node-version` file — Node 20 only enforced in CI, not locally |
|
|
|
|
|
| LOW | Rust toolchain pinned to `stable` (not specific version) — builds can drift |
|
|
|
|
|
| LOW | `Knip` runs with `|| true` (advisory only, not blocking) |
|
|
|
|
|
|
|
|
|
|
### Build Reproducibility
|
|
|
|
|
- ✅ Go: `go 1.25.0` pinned in `go.mod`; `go.sum` committed
|
|
|
|
|
- ✅ Node: Version 20 pinned in CI; `npm ci` used (clean install)
|
|
|
|
|
- ✅ 4 Go build tag variants tested: default, otel, wazero, otel+wazero
|
|
|
|
|
- ✅ Lockfiles current (go.sum: Apr 6, package-lock: Apr 7)
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 7. Observability
|
|
|
|
|
|
|
|
|
|
### Logging: STRONG ✅
|
|
|
|
|
- **Library**: `log/slog` (stdlib, Go 1.21) with multi-handler — stdout (INFO+) + ring buffer (DEBUG+)
|
|
|
|
|
- **Structured**: All logs use key-value pairs (`slog.Info("msg", "key", val)`)
|
|
|
|
|
- **Context present**: actor_id, user_id, channel_id, operation names logged consistently
|
|
|
|
|
- **Admin log viewer**: 2000-entry ring buffer in `Server/admin/logstream.go`
|
|
|
|
|
- **Minor**: `Server/scripts/seed.go` uses `fmt.Printf` (non-production CLI, acceptable)
|
|
|
|
|
|
|
|
|
|
### Metrics & Tracing: PRESENT (build-tag gated)
|
|
|
|
|
- OpenTelemetry SDK with no-op default; full telemetry via `-tags otel`
|
|
|
|
|
- Prometheus metrics export at `/metrics`
|
|
|
|
|
- Request ID propagation via `X-Request-Id` header ✅
|
|
|
|
|
- Health endpoints: `GET /health`, `GET /api/v1/health`, `GET /api/v1/livekit/health` ✅
|
|
|
|
|
|
|
|
|
|
### Error Surfacing: GOOD ✅
|
|
|
|
|
- Errors logged server-side with `slog.Error` before HTTP response
|
|
|
|
|
- Consistent HTTP status codes (400 validation, 503 feature unavailable, 500 internal)
|
|
|
|
|
- `writeErr()` / `writeJSON()` helper in admin ensure consistent error response shape
|
|
|
|
|
|
|
|
|
|
### Client-Side: LIMITED ⚠️
|
|
|
|
|
| SEVERITY | Finding |
|
|
|
|
|
|----------|---------|
|
|
|
|
|
| MEDIUM | No global error boundary component — unhandled promise rejections may fail silently |
|
|
|
|
|
| LOW | No crash/error reporting integration (Sentry, etc.) |
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 8. Plugin System Governance
|
|
|
|
|
|
|
|
|
|
### Plugin Architecture
|
|
|
|
|
- WASM isolation via Wazero (behind `-tags wazero` build flag)
|
|
|
|
|
- Manifest-declared capabilities: `commands`, `events`, `storage`, `http`, `ui`
|
|
|
|
|
- Memory capped per runtime (default 64 MiB); CPU budget configurable
|
|
|
|
|
|
|
|
|
|
### CRITICAL Issues
|
|
|
|
|
|
|
|
|
|
| SEVERITY | File:Line | Finding |
|
|
|
|
|
|----------|-----------|---------|
|
|
|
|
|
| **CRITICAL** | `Server/plugin/sandbox_wazero.go:162,211` | `invokeCommand` has **no timeout** — a looping plugin hangs the goroutine indefinitely |
|
|
|
|
|
| **CRITICAL** | `Server/plugin/host_storage.go` | Storage capability has **no key isolation** — plugin can read/write ANY key in the plugin store, not just its own |
|
|
|
|
|
| **CRITICAL** | `Server/plugin/host_http.go:162-240` | HTTP capability allows plugins to **exfiltrate data** by POSTing captured payload to any allowlisted host |
|
|
|
|
|
| **CRITICAL** | `Server/plugin/registry.go:129-133` | All commands auto-registered if manifest declares `commands` capability — **no per-command ACL** |
|
|
|
|
|
| **CRITICAL** | `Server/plugin/host_events.go` | **No rate limit** on event delivery to plugins — malicious plugin could slow server by processing events slowly |
|
|
|
|
|
| MEDIUM | `Server/plugin/host_http.go:64-96` | DNS rebinding TOCTOU between `rejectPrivateAddrs` check and TCP dial — not fully atomic |
|
|
|
|
|
| LOW | `Server/plugin/registry.go` | Default (non-Wazero) stub build doesn't emit WARN if plugins are configured but stub is running |
|
|
|
|
|
| LOW | `Server/plugin/loader.go` | Plugin discovery walks filesystem on every server start — no manifest caching |
|
|
|
|
|
|
|
|
|
|
### Strengths
|
|
|
|
|
- ✅ Per-plugin WASM module isolation (memory, execution)
|
|
|
|
|
- ✅ Private IP SSRF protection via `rejectPrivateAddrs()`
|
|
|
|
|
- ✅ Symlink rejection in ZIP extraction (zip-slip protection)
|
|
|
|
|
- ✅ Decompression bomb protection on plugin upload
|
|
|
|
|
- ✅ Graceful lifecycle (disable/uninstall/close) with module cleanup
|
|
|
|
|
- ✅ Plugin registry tracks all loaded instances with mutex-protected map
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 9. Prioritized Top-10 Action List
|
|
|
|
|
|
|
|
|
|
| Priority | SEVERITY | Area | Action | File |
|
|
|
|
|
|----------|----------|------|--------|------|
|
|
|
|
|
| 1 | CRITICAL | Plugin | Add `context.WithTimeout(ctx, cfg.CPUBudgetMs)` around `invokeCommand` to prevent infinite hangs | `Server/plugin/sandbox_wazero.go:162` |
|
|
|
|
|
| 2 | CRITICAL | Plugin | Namespace storage keys by plugin ID: `fmt.Sprintf("%d/%s", pluginID, key)` in `PluginGet/Set/Delete` | `Server/plugin/host_storage.go` |
|
|
|
|
|
| 3 | CRITICAL | Plugin | Add per-command ACL in registry — check `plugin.Manifest.AllowedCommands` or require explicit command declaration | `Server/plugin/registry.go:129` |
|
|
|
|
|
| 4 | CRITICAL | Plugin | Rate-limit event delivery to plugins (token bucket per plugin) to prevent event flooding DoS | `Server/plugin/host_events.go` |
|
|
|
|
|
| 5 | HIGH | Tests | Add `store/` package tests — data persistence is completely untested; cover SQLiteStore CRUD and query methods | `Server/store/` |
|
|
|
|
|
| 6 | HIGH | Tests | Add TypeScript unit tests for `src/lib/` (api.ts, ws.ts, dispatcher.ts) and `src/stores/` — currently <10% unit coverage | `Client/tauri-client/src/lib/` |
|
|
|
|
|
| 7 | HIGH | Deps | Pin critical npm packages to exact versions: `@tauri-apps/plugin-updater`, `vite`, `@tauri-apps/plugin-store` | `Client/tauri-client/package.json` |
|
|
|
|
|
| 8 | MEDIUM | Architecture | Refactor `Server/api/auth_handler.go` to route through `AuthService` — currently bypasses service layer | `Server/api/auth_handler.go` |
|
|
|
|
|
| 9 | MEDIUM | Code Quality | Silence audit trail loss — wrap `database.LogAudit()` calls with `slog.Error` on failure instead of `_ =` | `Server/admin/handlers_*.go` |
|
|
|
|
|
| 10 | MEDIUM | CI/CD | Add E2E gate to `ci.yml` on push to main (not only on PR) + add `.nvmrc` for local Node version enforcement | `.github/workflows/ci.yml` |
|
|
|
|
|
|
|
|
|
|
### Bonus (quick wins)
|
|
|
|
|
|
|
|
|
|
- Add `.catch()` handlers to `.then()` chains in `InviteManager.ts`, `file-upload.ts`, `attachments.ts`
|
|
|
|
|
- Add `context.WithTimeout` guard in `Server/ws/voice_leave.go` (`livekit.RemoveParticipant` has `//nolint:contextcheck`)
|
|
|
|
|
- Add HTTPS-only URL validation in `Server/ws/command.go` (existing TODO)
|
|
|
|
|
- Add frontend error boundary component wrapping `MainPage`
|
|
|
|
|
- Add Unicode/invisible-character validation to username registration
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
*Report generated from 6 parallel audit agents. All findings are read-only analysis; no code was modified.*
|