Wire the sqlc-generated dbgen package into db.DB so it stops being dead
code (audit A-2026-07-05) and becomes the real, CI-verified query layer.
db.DB now holds a *dbgen.Queries (initialized in Open via dbgen.New).
Query method bodies delegate to it; sqlc owns the SQL text and parameter
binding (make sqlc-verify), while db keeps its stable public API and
domain model types so no caller in api/admin/ws/service changes. The
migration is incremental — a method either delegates to d.q.* or still
runs raw SQL — so both layers are correct during the transition.
Converted domains (now load-bearing through sqlc):
- blocks: BlockUser, UnblockUser, IsBlocked, IsEitherBlocked,
ListBlockedUsers (added the query to blocks.sql + regenerated).
Empty ListBlockedUsers now returns []int64{} instead of nil, matching
the MemStore backend — a latent inconsistency fixed, not a regression.
- lockouts: UpsertLockout, LoadActiveLockouts, CleanupExpiredLockouts,
DeleteLockout (RFC3339 time formatting/parsing kept in the wrappers).
- roles: GetRoleByID, ListRoles, GetRoleForUser via a shared roleFromGen
mapper (int64 position/is_default -> int/bool). GetUserWithRole stays
raw for now.
Remaining domains stay on raw SQL and are tracked in
docs/plans/sqlc-adoption.md; store/ event+plugin SQL is intentionally
excluded (that layer is removed in D3). Decisions doc + audit closure
updated (A-2026-07-05 -> in progress).
Verified: go build ./...; go test -race ./db ./service ./auth ./ws (api
green non-race, race run matches CI's -timeout 20m); make sqlc-verify and
protocol-verify pass with the regenerated output committed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UA17KPvqGBX3XbXYnMf1rA
23 KiB
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) |
CLOSED 2026-07-19 — HTTP TOFU proxy implemented (http_proxy.rs + httpProxy.ts); REST path now cert-pinned, acceptInvalidCerts removed |
| 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) | CLOSED 2026-07-19 — full refresh of api.md/protocol.md/schema.md landed (all §2 fix-spec items); keep-current-per-PR rule now applies |
| 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 |
IN PROGRESS 2026-07-19 — dbgen now wired into db.DB and load-bearing (blocks/lockouts/roles delegate); no longer dead. Remaining domains tracked in plans/sqlc-adoption.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 |
CLOSED 2026-07-19 — beachhead, adapters, build plugin, and Solid deps removed; client-architecture.md retired in favor of architecture/client.md |
| A-2026-07-13 | LOW | Dead schema: sounds table survives soundboard removal (correction 2026-07-19: audit_log_v6 is only a transient rename inside migration 003, not a coexisting table) |
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
- Carried-over items from audit-2026-04-07
- Spec-conformance matrix
- Server architecture findings
- Client architecture findings
- Process & CI findings
- 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) |
|---|---|---|---|
| 1–5 | 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 open — Server/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).
Update 2026-07-19: the spec refresh landed — every fix-spec row below is resolved in the specs; evidence is retained for the record. Item A remains open pending the D1 implementation.
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, 001–015; 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. | FIXED 2026-07-19 — TOFU HTTP proxy (src-tauri/src/http_proxy.rs + src/lib/httpProxy.ts) tunnels REST through a cert-pinned loopback; allowSelfSigned/acceptInvalidCerts and the dangerous-settings feature removed. |
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.yml — client-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 009–015; 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.