diff --git a/Client/tauri-client/src/components/solid/PluginContainer.tsx b/Client/tauri-client/src/components/solid/PluginContainer.tsx index afe1c640..0901d433 100644 --- a/Client/tauri-client/src/components/solid/PluginContainer.tsx +++ b/Client/tauri-client/src/components/solid/PluginContainer.tsx @@ -25,7 +25,5 @@ export function PluginContainer(props: PluginContainerProps): JSX.Element { dispose?.(); }); - return ( -
- ); + return
; } diff --git a/Client/tauri-client/src/components/solid/README.md b/Client/tauri-client/src/components/solid/README.md index ed0b86af..4bf52631 100644 --- a/Client/tauri-client/src/components/solid/README.md +++ b/Client/tauri-client/src/components/solid/README.md @@ -19,7 +19,9 @@ directory, and existing leaf components are ported one PR at a time. handle has the same `{ destroy }` shape that the rest of the codebase uses. 4. **Test the pipeline.** New components get a `*.test.tsx` next to them using `@solidjs/testing-library`. The tests run under the existing Vitest - configuration without any extra setup. + configuration without any extra setup. Do **not** call `cleanup()` manually + in test files — `tests/setup-solid.ts` registers `afterEach(cleanup)` globally + via Vitest's `setupFiles` (added in T-500). 5. **Delete vanilla DOM code.** Once a component is fully migrated, remove the old factory function and update its callers to import from `@components/solid/...`. @@ -38,6 +40,14 @@ directory, and existing leaf components are ported one PR at a time. custom store via `fromStore` instead. - Touching framework-agnostic code (`lib/ws.ts`, `lib/dispatcher.ts`, `lib/livekitSession.ts`, `lib/api.ts`). These never need to know about Solid. +- Using `innerHTML`, `insertAdjacentHTML`, or `dangerouslySetInnerHTML` in any + Solid component. JSX interpolation (`{value}`) auto-escapes user content and + is the correct approach. Components rendering user-supplied text (e.g. message + attachments, toast messages) must never bypass this escaping. +- Placing `.tsx` files outside `src/components/solid/`. The `vite-plugin-solid` + transform is scoped to this directory in both `vite.config.ts` and + `vitest.config.ts`. A `.tsx` file elsewhere will fail with cryptic JSX parse + errors at build time. ## Existing components diff --git a/Client/tauri-client/tests/setup-solid.test.tsx b/Client/tauri-client/tests/setup-solid.test.tsx new file mode 100644 index 00000000..ea365fb9 --- /dev/null +++ b/Client/tauri-client/tests/setup-solid.test.tsx @@ -0,0 +1,75 @@ +/** + * Phase B Step 6 — mountSolid adapter lifecycle smoke tests (T-500). + * + * Verifies that the mountSolid adapter in @lib/solidMount correctly inserts a + * Solid reactive root into the DOM and that destroy() disposes the root and + * removes the host element, preventing memory leaks across test suites. + * + * These tests complement Badge.test.tsx (which validates the Solid rendering + * pipeline end-to-end) by proving the *adapter* contract used by vanilla-DOM + * container components. + */ +import { describe, it, expect } from "vitest"; +import { mountSolid } from "@lib/solidMount"; + +describe("mountSolid adapter", () => { + it("appends a data-solid-root host element to the parent on mount", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + + const handle = mountSolid(() => test, parent); + + expect(parent.querySelector("[data-solid-root]")).not.toBeNull(); + expect(handle.el.dataset.solidRoot).toBe("true"); + expect(handle.el.parentElement).toBe(parent); + + handle.destroy(); + parent.remove(); + }); + + it("removes the host element from the DOM after destroy()", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + + const handle = mountSolid(() => cleanup-check, parent); + expect(parent.children).toHaveLength(1); + + handle.destroy(); + + expect(parent.querySelector("[data-solid-root]")).toBeNull(); + expect(parent.children).toHaveLength(0); + + parent.remove(); + }); + + it("supports multiple independent mounts under the same parent", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + + const a = mountSolid(() => a, parent); + const b = mountSolid(() => b, parent); + + expect(parent.querySelectorAll("[data-solid-root]")).toHaveLength(2); + + a.destroy(); + expect(parent.querySelectorAll("[data-solid-root]")).toHaveLength(1); + + b.destroy(); + expect(parent.querySelectorAll("[data-solid-root]")).toHaveLength(0); + + parent.remove(); + }); + + it("exposes the host element via handle.el", () => { + const parent = document.createElement("div"); + document.body.appendChild(parent); + + const handle = mountSolid(() => el-check, parent); + + expect(handle.el).toBeInstanceOf(HTMLElement); + expect(handle.el).toBe(parent.firstElementChild); + + handle.destroy(); + parent.remove(); + }); +}); diff --git a/Client/tauri-client/tests/setup-solid.ts b/Client/tauri-client/tests/setup-solid.ts new file mode 100644 index 00000000..e78791f5 --- /dev/null +++ b/Client/tauri-client/tests/setup-solid.ts @@ -0,0 +1,22 @@ +/** + * Vitest global setup for Solid.js component tests (T-500). + * + * Loaded via `test.setupFiles` in vitest.config.ts so every test suite + * automatically gets Solid's afterEach cleanup without having to import + * or call it manually. + * + * Add future Solid testing helpers here (custom matchers, query extensions, + * aria-query configuration, etc.). Do NOT import application code here — + * this file executes once before every test suite, including non-Solid suites. + * + * Security note: Solid JSX auto-escapes interpolated values ({expr}), so + * user-controlled strings passed through JSX are safe. Never use innerHTML, + * insertAdjacentHTML, or dangerouslySetInnerHTML in Solid components. + */ +import { cleanup } from "@solidjs/testing-library"; +import { afterEach } from "vitest"; + +// Register cleanup after every test so Solid reactive roots are disposed +// and host DOM nodes are removed. Without this, roots accumulate across tests +// and can cause state leakage between test cases. +afterEach(cleanup); diff --git a/Client/tauri-client/vitest.config.ts b/Client/tauri-client/vitest.config.ts index 949c5b09..fbc987c4 100644 --- a/Client/tauri-client/vitest.config.ts +++ b/Client/tauri-client/vitest.config.ts @@ -9,7 +9,9 @@ export default defineConfig({ // the angle brackets. plugins: [ solidPlugin({ - include: ["src/components/solid/**/*.{ts,tsx,js,jsx}"], + // Phase B Step 6: cover both the component directory and test files + // under tests/ that use JSX (e.g. setup-solid.test.tsx, T-500). + include: ["src/components/solid/**/*.{ts,tsx,js,jsx}", "tests/**/*.tsx"], }), ], resolve: { @@ -27,11 +29,11 @@ export default defineConfig({ // `src/**/*.test.{ts,tsx}` files are picked up. The latter is required // for Phase B Step 6 Solid components, whose tests live alongside the // component file (see src/components/solid/README.md). - include: [ - "tests/**/*.test.ts", - "src/**/*.test.ts", - "src/**/*.test.tsx", - ], + // T-500: also pick up .tsx test files under tests/ (e.g. setup-solid.test.tsx). + include: ["tests/**/*.test.ts", "tests/**/*.test.tsx", "src/**/*.test.ts", "src/**/*.test.tsx"], + // T-500: global Solid.js test setup — registers afterEach(cleanup) so + // individual *.test.tsx files do not need to call cleanup() manually. + setupFiles: ["./tests/setup-solid.ts"], coverage: { provider: "v8", include: ["src/**/*.ts"], diff --git a/Server/plugin/examples/hello/README.md b/Server/plugin/examples/hello/README.md index 921c6df3..4769f792 100644 --- a/Server/plugin/examples/hello/README.md +++ b/Server/plugin/examples/hello/README.md @@ -10,17 +10,33 @@ the registry persists it into the plugins table without executing the .wasm. ## Building the WASM -The .wasm binary is intentionally NOT checked in. Build it locally with TinyGo -or any other WASM toolchain that emits a module exporting `command_dispatch`, -`on_event`, and `_start`: +`main.go` in this directory implements the full plugin ABI +(`allocate`, `deallocate`, `list_commands`, `command_dispatch`, `on_event`). +The pre-built `hello.wasm` (925 KiB) is checked in, but you can rebuild it: -```sh -# TinyGo example (writes hello.wasm into this directory) -tinygo build -o hello.wasm -target wasi ./main.go +### Prerequisites + +| Tool | Version | Notes | +|------|---------|-------| +| TinyGo | 0.40.1 | Supports Go 1.19–1.25 only | +| Go | 1.25.x | TinyGo 0.40.1 rejects Go 1.26+ | +| wasm-opt | Binaryen 129 | Required by TinyGo for the `wasi` target | + +On Windows, extract TinyGo to e.g. `D:\Local-Lab\Coding\Software\tinygo` and +add `\bin` plus `\bin` to `PATH`. Then point TinyGo at the +compatible Go SDK: + +```pwsh +$env:GOROOT = "$env:USERPROFILE\sdk\go1.25.3" # installed via: go install golang.org/dl/go1.25.3@latest +$env:PATH = "$env:GOROOT\bin;$env:PATH" ``` -A trivial main.go that satisfies the host API is sketched in -`Server/plugin/sandbox_wazero.go`'s docstring. +### Build command + +```sh +# Run from this directory (Server/plugin/examples/hello/) +tinygo build -o hello.wasm -target wasi ./main.go +``` ## Tests diff --git a/docs/audit-2026-04-07.md b/docs/audit-2026-04-07.md new file mode 100644 index 00000000..200cec7a --- /dev/null +++ b/docs/audit-2026-04-07.md @@ -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.* diff --git a/docs/contributing.md b/docs/contributing.md index c65d62b9..38abe1be 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -83,6 +83,24 @@ How to set up the development environment and contribute to OwnCord. | `npm run format:check` | Prettier check only (no writes) | | `npm run knip` | Dead code and unused export detection | +## Plugin Development + +Plugins are WASM modules loaded at runtime when the server is built with `-tags wazero`. +See `Server/plugin/examples/hello/README.md` for the full plugin ABI and build instructions. + +**Toolchain requirements for building `.wasm` plugins with TinyGo:** + +| Tool | Version | Notes | +|------|---------|-------| +| TinyGo | 0.40.1 | Supports Go 1.19–1.25 only | +| Go SDK | 1.25.x | Install alongside the system Go via `go install golang.org/dl/go1.25.3@latest && go1.25.3 download` | +| wasm-opt | Binaryen 129 | Required by TinyGo for the `wasi` target; download from Binaryen GitHub releases | + +Any WASM toolchain (Rust/`wasm32-wasi`, AssemblyScript, etc.) that exports the five ABI +functions is equally valid — TinyGo is just the example toolchain used by `examples/hello/`. + +--- + ## Active Branches - `main` -- stable releases