Files
OwnCord/docs/audit-2026-07-19.md
T
Claude 2e7a80171b feat(server,client): protocol codegen + audit quick-wins batch
Protocol codegen (decision D4, audit A-2026-07-08):
- Add docs/protocol-schema.json as the real single source of truth for
  WS message-type constants, making the long-standing 'generated from'
  comment in both constant files true.
- Add Server/scripts/genprotocol, a generator emitting both
  Server/ws/message_types.go and Client .../lib/protocolTypes.ts
  (constants byte-for-byte value-identical to before; only headers,
  ordering alignment, and provenance comments changed).
- Add make protocol-generate / protocol-verify and wire protocol-verify
  into CI next to sqlc-verify.

Quick wins (decision D8):
- admin: log LogAudit write failures in the backup handlers instead of
  discarding them (prior audit #10).
- api: fix self-contradictory upload Cache-Control to 'private,
  no-cache' per remediation plan W3-4; drop the now-unused
  fileCacheMaxAgeSeconds constant; update test.
- ws: route the hub settings cache through db.GetSetting instead of
  inline SQL.
- ws: fix a latent data race — main.go wires SetEventPersister and
  SetEventStore after NewRouter has already started the hub Run loop,
  which reads those fields on the broadcast/replay paths. They (and
  pluginSink, which one test sets post-Run) are now atomic pointers;
  the remaining pre-Run-only setters reject late calls with an error
  log instead of racing silently.

Update the audit closure table and decisions doc statuses accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UA17KPvqGBX3XbXYnMf1rA
2026-07-19 13:32:58 +00:00

22 KiB
Raw Blame History

OwnCord — Architectural Audit & Spec-Conformance Review

Date: 2026-07-19 Branch: claude/blueprints-architectural-audit-k927qb (audited tree: ddc49f0 = main) Scope: whole-system architecture mapping + code-by-spec conformance. Read-only — no code or spec changes ship with this audit; the companion blueprint set lives in docs/architecture/. Relationship to prior audit: successor to audit-2026-04-07.md, which remains the closure tracker for its own findings. Carried-over items are re-verified in §1, not restated.


Finding closure status (maintained; update statuses in place)

Standing rule: every HIGH below gets either a closing commit link or an explicit accepted-risk note before the beta gate. MEDIUMs are folded into the backlog (§6) and closed opportunistically.

ID Sev Finding Status
A-2026-07-01 HIGH announcement channel type: documented in 3 specs and offered by the admin API, but hard-rejected by DB triggers DECIDED 2026-07-19 — implement end-to-end (D1) (see plans/audit-2026-07-19-decisions.md)
A-2026-07-02 HIGH Client HTTP path accepts any TLS certificate (allowSelfSigned hardcoded; no TOFU pinning, unlike WS/LiveKit paths) DECIDED 2026-07-19 — next security work: TOFU HTTP proxy in Rust (D5) (see plans/audit-2026-07-19-decisions.md)
A-2026-07-03 HIGH Reference specs (api.md / protocol.md / schema.md) frozen at 2026-04-02; systemic drift incl. whole undocumented subsystems (voice E2EE, plugins) DECIDED 2026-07-19 — one refresh PR, after protocol codegen (D7) (see plans/audit-2026-07-19-decisions.md)
A-2026-07-04 HIGH Client unit test suite "KNOWN RED" and non-blocking in CI; E2E never gated OPEN — supersedes prior #11's scope
A-2026-07-05 MEDIUM Dead sqlc layer: Server/db/dbgen/ (~3.5k LOC) generated + CI-verified but imported by nothing DECIDED 2026-07-19 — adopt sqlc as the real query layer (D2) (see plans/audit-2026-07-19-decisions.md)
A-2026-07-06 MEDIUM Three coexisting DB-access styles (raw *db.DB in api/admin/ws, store.Store under service, dead dbgen) DECIDED 2026-07-19 — single data layer: sqlc-backed db pkg, remove store/ (D2+D3) (see plans/audit-2026-07-19-decisions.md)
A-2026-07-07 MEDIUM Channel-visibility logic duplicated across ~4 sites with "must mirror" comments OPEN
A-2026-07-08 MEDIUM Protocol constants on both sides claim generation from docs/protocol-schema.json, which does not exist in the repo CLOSED 2026-07-19 — codegen implemented: docs/protocol-schema.json + Server/scripts/genprotocol + make protocol-verify CI gate
A-2026-07-09 MEDIUM Dual V1+V2 WS dispatch (strangler-fig) still live; two parsers/registries to keep in sync OPEN
A-2026-07-10 MEDIUM api.NewRouter god-constructor: builds services, hub, LiveKit, updater, admin, plugins; spawns goroutines; mounts everything OPEN
A-2026-07-11 MEDIUM ws.Hub mega-object with post-construction Set* wiring ("must be called before Run") OPEN
A-2026-07-12 MEDIUM Abandoned SolidJS beachhead still in-tree; docs/client-architecture.md describes the abandoned architecture DECIDED 2026-07-19 — delete beachhead + retire the stale doc (D6) (see plans/audit-2026-07-19-decisions.md)
A-2026-07-13 LOW Dead schema: sounds table survives soundboard removal; audit_log + audit_log_v6 coexist OPEN
A-2026-07-14 LOW Scattered client constants (#5865F2 ×18, localhost:8443 ×3); 64 timer call sites with manual lifecycle OPEN
A-2026-07-15 LOW docs/plans/security-hardening-remediation.md partly stale (references deleted store/postgres.go) OPEN

Table of Contents

  1. Carried-over items from audit-2026-04-07
  2. Spec-conformance matrix
  3. Server architecture findings
  4. Client architecture findings
  5. Process & CI findings
  6. Prioritized improvement backlog

1. Carried-over items from audit-2026-04-07

Re-verified in today's tree. Statuses below reflect the code, not the prior audit's table; details stay in audit-2026-04-07.md.

Prior # Sev Finding (one-line) Re-verification (2026-07-19)
15 CRITICAL Plugin governance (timeout, storage isolation, ACL, event rate limit, HTTP exfiltration) Unchanged since prior closure table; plugins still default-disabled (plugins.enabled: false), which is the standing mitigation
6 HIGH Server/store/ untested Still zero _test.go files in Server/store/. The "SUPERSEDED — remove in P4" plan has not executed; the package remains the designated abstraction seam with no direct tests
7 HIGH Client unit coverage Suite is large (157 test files) but currently KNOWN RED and non-blocking — see A-2026-07-04
9 MEDIUM auth_handler bypasses service layer Confirmed openServer/api/router.go:101 passes database *db.DB to MountAuthRoutes while sibling mounts receive svc
10 MEDIUM Audit-trail write failures silently ignored Fixed 2026-07-19 at the two flagged backup-handler sites (errors now logged). Wider scope discovered: the _ = LogAudit pattern exists at 23 call sites across admin/api/ws/service — appears to be a deliberate best-effort convention; policy decision tracked in the decisions doc (D8 note)
11 MEDIUM E2E not in CI Confirmed open — no Playwright job exists in .github/workflows/ci.yml
W3-4 (remediation plan) LOW Contradictory upload cache header Fixed 2026-07-19 — now private, no-cache per the remediation plan's prescription

2. Spec-conformance matrix

The three reference specs were last meaningfully edited 2026-04-02 (git log on each file); the code is at v1.1.0-alpha.2 with substantial July 2026 changes. Resolution column: fix-spec (doc catches up to code), fix-code (code is wrong), decide (product decision needed first).

2.1 docs/api.md

ID Spec says Code does Evidence Sev Resolution
B GET /api/v1/info returns {name, version} version removed (anti-fingerprinting, "C-2") Server/api/router.go infoResponse{Name} only MEDIUM fix-spec
C GET /health returns version version removed for the same reason Server/api/router.go healthResponse{Status, Uptime, OnlineUsers} MEDIUM fix-spec
F — (absent) Profile surface exists: PATCH /api/v1/users/me, PUT /users/me/password, GET/DELETE /users/me/sessions Server/api/profile_handler.go (MountProfileRoutes) MEDIUM fix-spec
G — (absent) Plugin admin REST surface: /api/v1/admin/plugins (list/enable/disable/uninstall/install-zip) Server/api/router.go:250 MEDIUM fix-spec

2.2 docs/protocol.md

ID Spec says Code does Evidence Sev Resolution
D1 — (voice E2EE absent) Full E2EE signaling: client voice_e2ee_announce/voice_e2ee_offer, server broadcast/relay of both Server/ws/message_types.go, Server/ws/voice_e2ee.go; flow in architecture/voice-e2ee.md HIGH fix-spec
D2 — (absent) voice_speakers server event Server/ws/message_types.go (MsgTypeVoiceSpeakers) LOW fix-spec
D3 — (absent) user_update broadcast on profile change Server/ws/message_types.go; wired via MountProfileRoutes broadcaster LOW fix-spec
D4 member_leave appears only in the seq table Fully implemented broadcast Server/ws/message_types.go (MsgTypeMemberLeave) LOW fix-spec
E auth_ok payload = {user, server_name, motd} Also carries replay_source (none|buffer|db) Server/ws/serve.go auth path; CHANGELOG Phase B LOW fix-spec

2.3 docs/schema.md

ID Spec says Code does Evidence Sev Resolution
A Channel types: text, voice, announcement, dm (also in api.md and protocol.md; admin API offers announcement) DB triggers reject any type outside text|voice|dm Server/migrations/013_channel_type_constraint.sql vs Server/admin/handlers_channels.go HIGH decide — either implement announcement channels end-to-end or remove the type from specs + admin API
H Migration history stops at "008", with wrong numbering (two 003_* entries; DM tables labeled 008) 15 migrations exist, 001015; real order diverges from 004 onward Server/migrations/ directory listing MEDIUM fix-spec
I — (absent) 9 tables undocumented: login_attempts, settings, emoji, sounds, rate_lockouts, user_blocks, events, plugins, plugin_kv Server/migrations/001,011,012,014,015 — see architecture/data-model.md MEDIUM fix-spec
J attachments DDL without uploader_id Column added for upload-ownership checks Server/migrations/010_attachment_uploader.sql MEDIUM fix-spec
K — (sqlc unmentioned) sqlc.yaml + Server/db/dbgen/ exist (currently dead — A-2026-07-05) sqlc.yaml, Server/db/dbgen/ LOW fix-spec (document whichever way A-2026-07-05 resolves)

Systemic conclusion: drift is not isolated typos — entire subsystems (E2EE signaling, plugins, six migrations) postdate the specs. Item A is the only case where code and spec actively contradict at runtime: an admin can select a channel type the database will refuse to insert. Recommended handling: one coherent spec-refresh PR (backlog item 4) rather than piecemeal edits, plus a decision on A first.


3. Server architecture findings

Verdict: the intended layering (api → service → store → db) is sound and the service layer respects it; test discipline is excellent (test LOC ≈ 1.6× source; race + deadlock + mutation tooling). The findings are about the seams that grew around that design.

ID Sev Area Evidence Finding Recommendation Effort
A-2026-07-05 MEDIUM Data layer Server/db/dbgen/ (~3.5k LOC), sqlc.yaml, CI sqlc-verify job sqlc output is generated, version-pinned, CI-verified — and imported by nothing. Hand-written raw SQL in Server/db/*_queries.go is what runs. Decide the Phase-A question: adopt dbgen inside db.DB method bodies, or delete dbgen/ + queries/ + the CI job. Either ends the illusion of a second data layer. S
A-2026-07-06 MEDIUM Layering All Mount*Routes signatures take database *db.DB alongside svc (Server/api/*_handler.go); Server/admin handlers take *db.DB ~359 direct database.* calls above the store seam; three access styles coexist. The abstraction exists but cannot be relied on (e.g. for a future backend swap or for test doubles). Consolidate incrementally: new handlers service-only; migrate one mount per PR, starting with auth (prior #9). L
A-2026-07-07 MEDIUM Correctness risk Server/ws/serve.go (buildReady, computeAllowedChannels), Server/ws/hub.go (RefreshChannelVisibility), REST handleListChannels Channel-visibility filtering implemented ~4× with comments instructing they "must mirror" each other. The recent private-channel fixes (e.g. 2bfe6d6) show this is actively churning — a drift between copies is an information-disclosure bug waiting to happen. Extract a single VisibleChannels(userID) (natural home: service.PermissionService or the existing permissions.Checker), consume it from all four sites, and add a test asserting REST and WS agree. M
A-2026-07-08 MEDIUM Protocol integrity Server/ws/message_types.go header comment; Client/…/src/lib/protocolTypes.ts header + "Extensions (not in protocol-schema.json…)" comments Both sides claim docs/protocol-schema.json is the generated single source of truth. The file does not exist; the two constant sets are maintained by hand and have already grown divergent "extension" entries. Either commit a real protocol-schema.json + generator (best: also emits protocol.md tables), or delete the claim and add a cross-language equality test over the two constant sets. M
A-2026-07-09 MEDIUM Real-time Server/ws/handlers.go (handleMessage V2-then-V1 fallback), dual registration in NewHub (Server/ws/hub.go) Strangler-fig V1+V2 dispatch is live: two parsers (lenient/strict), two registries, per-type duplication. Finish the migration: port remaining V1 types to V2, then delete the V1 path. Track remaining types in an issue so the count visibly shrinks. M/L
A-2026-07-10 MEDIUM Composition Server/api/router.go:34 (NewRouter, ~278 lines) God-constructor builds rate limiter, TOTP key, storage, services, hub, LiveKit client+process, updater, admin + plugin handlers; spawns goroutines; returns a cleanup closure covering only one of them. Hard to test wiring in isolation; lifecycle ownership is implicit. Split construction (a Deps/App struct built in main.go) from route mounting (NewRouter(deps)); return a composite io.Closer. M
A-2026-07-11 MEDIUM Real-time Server/ws/hub.go (SetLiveKit, SetEventPersister, SetPluginRegistry, …) Hub is a mega-object wired post-construction via setters that "must be called before Run" — temporal coupling; a missed setter is a nil-deref at runtime, not a compile error. Move required collaborators into NewHub params (or an options struct validated before Run). Full Hub decomposition is a separate, larger effort (backlog 12). S (constructor) / L (decomposition)
MEDIUM Layering Server/ws/hub.go:182 (refreshSettingsLocked) Hub runs inline SELECT value FROM settings WHERE key='server_name' instead of using SettingsStore — the only raw SQL in the real-time layer. Fixed 2026-07-19 — now uses db.GetSetting; full consolidation folds into A-2026-07-06. S
LOW Scaling posture Server/auth/ratelimit.go (documented), in-memory pub/sub + ring buffer, process-local TOTP replay Single-instance coupling is structural and documented — this is a deliberate design, not a bug. Recorded here so the constraint stays visible (architecture/system-overview.md D8). No action now; revisit only if multi-instance ever becomes a goal.
A-2026-07-13 LOW Schema hygiene sounds table (Server/migrations/001), audit_log + audit_log_v6 (003) Dead/duplicated schema: soundboard was removed but its table remains; two audit-log tables coexist after the 003 rebuild. Add a cleanup migration (drop sounds, finish the audit_log consolidation) next time a migration ships anyway. S

4. Client architecture findings

Verdict: the client's fundamentals are strong — immutable store discipline, TOFU pinning in Rust, keychain credentials with IPC redaction, generation counters against stale listeners, and an unusually deep test/tooling stack (Vitest, dual Playwright suites, Stryker, oxlint + type-checked ESLint, Knip). Findings target structure and one security gap.

ID Sev Area Evidence Finding Recommendation Effort
A-2026-07-02 HIGH Security src/main.ts (allowSelfSigned: true at API-client construction); src-tauri http plugin built with dangerous-settings Every REST call accepts any certificate. The WS and LiveKit paths pin TOFU fingerprints in Rust; the HTTP path — which carries the auth token on every request — does not. An active MITM can capture tokens without triggering the cert-mismatch modal. Implement the acknowledged fix: a TOFU HTTP proxy in Rust (mirror ws_proxy.rs), or route REST through a pinned Rust command. Until then this is the client's weakest transport link. M
A-2026-07-12 MEDIUM Coherence src/components/solid/ (154 LOC), src/lib/solidMount.ts, src/lib/solidAdapter.ts, vite-plugin-solid config; CHANGELOG "Solid.js migration (abandoned)" The abandoned migration's beachhead, adapters, build plugin, and test deps remain, and docs/client-architecture.md (2026-03-30) still describes a SolidJS client. Two mental models for contributors, one of them false. Delete the beachhead + adapters + build plugin; replace client-architecture.md content with a pointer to architecture/client.md or a rewrite. S
MEDIUM Maintainability src/lib/livekitSession.ts (1,719 LOC); AccountTab.ts (845), SidebarArea.ts (812), LoginForm.ts (699) livekitSession.ts owns the connection state machine, E2EE, track management, reconnection, and diagnostics in one class. It is the highest-risk file to modify in the client. Extract E2EE (already has e2eeCrypto.ts as a seam) and track management into collaborators; keep the state machine as the core. Settle for splitting the settings tabs opportunistically. M
MEDIUM State src/stores/voice.store.ts imports members/auth stores; auth.store.clearAuth() reaches into leaveVoice() + notification cleanup; business logic in main.ts subscribers Cross-store singleton coupling: teardown ordering lives implicitly in import graphs and bootstrap subscribers. Introduce a thin session-lifecycle module (login/logout orchestration) that calls stores, so stores stop calling each other. M
A-2026-07-14 LOW Hygiene #5865F2 literal ×18 across 11 files (main.ts, ServerPanel.ts, DmSidebar.ts, MemberPickerModal.ts, SidebarArea.ts, …); localhost:8443 ×3; 64 setTimeout/setInterval sites; 12 innerHTML uses Scattered magic values and manual timer lifecycles; src/lib/constants.ts holds a single constant. Centralize into constants/tokens; adopt a tiny managedTimer(destroyScope) helper so destroy() paths can't leak intervals. S
LOW Error handling catch {} swallows in preferences.ts, ws.ts unsubscribe cleanup, disconnectProxy; widespread void-prefixed fire-and-forget Mostly deliberate (per lint config), but a handful of swallows hide real failures (e.g. preference persistence silently failing). Log-at-debug in the swallow sites; keep the pattern otherwise. S

5. Process & CI findings

ID Sev Evidence Finding Recommendation Effort
A-2026-07-04 HIGH .github/workflows/ci.ymlclient-tests job annotated "KNOWN RED pending reboot-plan P2 triage"; no Playwright job The Go side is gated hard (race, deadlock tag, govulncheck, golangci-lint, sqlc-verify, 4-tag build matrix) but the client's 157-file test suite is red and non-blocking, and E2E never runs in CI. For an AI-first workflow where "quality [is] validated primarily through automated checks" (README), the client half of that promise is currently unenforced. Triage the red suite to green, flip client-tests to blocking, then add at least the web Playwright suite as a nightly non-blocking job (prior #11) before promoting it to a gate. M
A-2026-07-03 HIGH git log on docs/api.md, protocol.md, schema.md (all 2026-04-02) vs code churn through 2026-07-19 No process keeps the reference specs current — the July burst (events table, plugins, E2EE, private channels) shipped without touching them. After the one-time refresh (backlog 4), add a PR-checklist line (mirroring the blueprint maintenance rule in architecture/README.md): protocol/API/schema changes update the matching spec in the same PR. S
A-2026-07-15 LOW docs/plans/security-hardening-remediation.md W1-3/W3-5 reference Server/store/postgres.go (deleted) The remediation plan predates the Postgres removal; two waves partially target dead code. Annotate the affected items rather than rewriting the plan. S
LOW ci.yml tauri-build job: if: github.event_name == 'pull_request' && github.base_ref == 'main' Full client build (incl. Clippy -D warnings, cargo audit) never runs on push to main — a merge that breaks the native build is caught only at the next PR. Add a push-to-main trigger for tauri-build (or a nightly). S

6. Prioritized improvement backlog

Ranked by severity × effort; quick wins float within tier. S/M/L ≈ hours / days / week+.

# Item Finding Sev Effort
1 Decide + resolve the announcement channel-type contradiction (implement or strip from specs/admin) A-2026-07-01 HIGH S (decision)
2 Delete (or finally adopt) the dead db/dbgen sqlc layer; adjust the sqlc-verify CI job to match A-2026-07-05 MEDIUM S
3 Extract the single channel-visibility function replacing the 4 "must mirror" copies; add REST/WS agreement test A-2026-07-07 MEDIUM M
4 One-PR refresh of api.md / protocol.md / schema.md against §2, incl. E2EE + plugins + migrations 009015; then enforce spec-updates-with-code via PR checklist A-2026-07-03 HIGH M
5 Execute the store-layer decision from prior #6: remove Server/store/ (P4 plan) or test it directly prior #6 HIGH M
6 Client HTTP TOFU pinning (Rust proxy mirroring ws_proxy.rs) A-2026-07-02 HIGH M
7 Stop discarding LogAudit errors in admin/handlers_backup.go; fix the contradictory upload Cache-Control prior #10, W3-4 MEDIUM S
8 Remove the SolidJS beachhead + adapters; retire or rewrite docs/client-architecture.md A-2026-07-12 MEDIUM S
9 Resolve the protocol-schema.json ghost: real codegen or an equality test between Go and TS constant sets A-2026-07-08 MEDIUM M
10 Green + blocking client unit suite; nightly Playwright A-2026-07-04 HIGH M
11 Finish the V2 dispatch migration; delete V1 A-2026-07-09 MEDIUM M/L
12 Consolidate DB access behind the service layer (start with auth routes, prior #9); then Hub constructor cleanup and decomposition A-2026-07-06, -10, -11 MEDIUM L

Blueprints referenced throughout live in docs/architecture/; update a diagram and its doc in the same PR as any structural change to its source-of-truth files. Line-number evidence in this report is a snapshot of commit ddc49f0.