diff --git a/docs/audit-2026-08-19.md b/docs/audit-2026-08-19.md new file mode 100644 index 00000000..cdbce42b --- /dev/null +++ b/docs/audit-2026-08-19.md @@ -0,0 +1,441 @@ +# OwnCord — Repo Health Audit + +**Date:** 2026-08-19 +**Branch:** `claude/repo-health-audit-s0xnyo` (audited tree: `eacba10` = `main`) +**Scope:** full-repo health check — prior-finding closure verification, dynamic +checks (build/test/boot), doc/code drift across the three reference specs, and +a static sweep focused on the WebSocket hub, reconnect/sync handling, and the +REST/WS boundary. Read-only; no code changes ship with this audit. +**Relationship to prior audits:** successor to the 2026-08-04 pair +([security](audit-2026-08-04.md), [docs & coverage](audit-2026-08-04-docs-and-coverage.md)), +which remain the closure trackers for their own findings. Every item those +audits left open is re-verified in §2. +**Method:** ten parallel analysis passes (3 prior-finding verification, 3 +doc-drift, 4 static-sweep), every BROKEN/FRAGILE candidate then re-traced by an +independent adversarial verifier instructed to refute it (7 confirmed, 1 +downgraded, 0 refuted). Every dynamic-check number in §3 comes from an actual +run in this session. Rank legend: **BROKEN** = fails or states falsehoods now +· **FRAGILE** = works today, likely to break · **DEBT** = cleanup, no user +impact. + +--- + +## 1. Executive summary + +The code is in the best shape any of the six audits has found it: every suite +passes, the server boots clean, the bug-hunt ledger is fully closed (188 +fixed / 2 declined / 1 duplicate / **0 open**), and the heavily-audited +subsystems — hub locking, replay tiers, permission funnels, plugin sandbox — +all verified sound again (§7). + +What has decayed is the **documentation**, again. Fifteen days after the +2026-08-04 spec refresh, the three reference docs carry ten newly wrong +statements (§4): `schema.md` misses two migrations and documents the exact FK +behavior migration 030 was written to remove; `protocol.md` claims DM chat +events are unsequenced when they are stamped, ring-buffered and replayable, +and documents five rate limits as "None" that are all enforced; +`api.md` tells non-admins they can call an admin-only endpoint and documents +three error codes the server never emits. The keep-docs-current-per-PR rule +(A-2026-07-03) is not holding on its own. + +On the code side the sweep found **five FRAGILE items and no BROKEN ones** — +two silent-failure gaps in auth persistence, a role-change/handshake race that +can leave a socket on stale subscriptions, dead-on-arrival client replay-dedup +machinery, and one block-enforcement asymmetry (blocking a user does not evict +them from a live 1:1 DM voice call) that continues the exact defect family the +2026-08-04 security review closed. One client unit test is a load-sensitive +timeout flake. Everything else is debt. + +--- + +## 2. Prior-finding closure verification + +Every item left open by a prior audit, re-verified against `eacba10`. + +### 2.1 Carried findings + +| Item | Sev | Verdict 2026-08-19 | Evidence | +|---|---|---|---| +| A-2026-07-10 router god-constructor | MED | **PARTIALLY FIXED** | `NewRouter` is down to 171 lines (`Server/api/router.go:39-209`) with 7 extracted helpers (`routerTOTPKey` :213 … `routerMetricsRoutes` :412), but remains the composition root: builds limiter/services/hub, spawns `go hub.Run()` (:161), and the returned cleanup (:204-206) still covers only `limiterStopCh` | +| A-2026-07-11 Hub `Set*` post-construction wiring | MED | **STILL OPEN, hazard mitigated** | 6 setters remain (`ws/hub_events.go:40,48,60,70`, `ws/hub_livekit.go:10,48`); three are now atomic stores safe post-`Run`, the other three are guarded by `rejectIfRunning` (`ws/hub.go:721-728`) — a missed setter is no longer a race, but the temporal coupling stands | +| A-2026-07-14 scattered client constants | LOW | **STILL OPEN, grew** | `#5865F2` ×19 across 12 files (was 18/11); `localhost:8443` ×3; `setTimeout/setInterval` at 114 call-site lines across 43 non-test files (was 64); `src/lib/constants.ts` still holds only 2 constants | +| 2026-04-07 #9 auth handler bypasses service layer | MED | **STILL OPEN** | `Server/api/router.go:117` passes `database *db.DB` to `MountAuthRoutes` while siblings get `svc`; handlers call `database.*` directly (`auth_handler.go:166,201,209,483,572,584,673,757`) | +| 2026-04-07 #5 plugin HTTP exfiltration | accepted | **ACCEPTED RISK INTACT** | All four mitigations re-verified: defaults off + empty allowlist (`config/config.go:351-357`), empty allowlist denies all (`plugin/host_http.go:140-158`), WASI-only runtime with no host imports (`plugin/sandbox_wazero.go:100-113`), inert `EventSink.Dispatch` + no production `Subscribe` (`plugin/host_events.go:122-131`) | +| A-2026-07-06 residual: direct DB above service layer | MED | **IMPROVED ~50%** | ~170 direct call sites in non-test `api`/`admin`/`ws` (was ~359), 30 files; top offenders `ws/voice_join.go` (14), `admin/handlers_channel_perms.go` (13), `ws/hub_broadcast.go` (11) | +| `channelCanSend` copy (permission-middleware plan disclosure) | MED | **STILL OPEN** | `Server/ws/serve_ready.go:142-160` still hand-mirrors `MessageService.checkSendPermission` (`service/message_perms.go:78-102`); guarded by the mirror test `ws/can_send_test.go` but synced by hand | +| audit-2026-08-04 §5: backup restore hardcodes DB path; 500-path serves a closed DB | obs | **FIXED** | `admin.SetDatabasePath` wired from `main.go:337` (pinned by `handlers_backup_test.go:553-577`); every restore failure branch now rolls back the safety copy and/or restarts (`handlers_backup.go:276-329`) — no branch leaves a closed DB serving | +| T-2026-07-25-11 residual: `HandleLiveKitHealthForTest` duplicate | LOW | **STILL OPEN (test-only)** | All 7 legacy callers unmigrated (`api/middleware_test.go:1026,1049,1075`, `api/coverage_push_test.go:327,350,373,642`); production handler is single-sourced | +| T-2026-07-25 §4: `logctx.WithGroup` nesting decision | LOW | **STILL OPEN by design** | Behavior unchanged (`logctx.go:52-54`), pinned by test; no production code opens a logger-level group; doc comment defers explicitly (note: stray word "ponytail" at `logctx.go:49`) | +| T-2026-07-25 backlog #8: gofmt drift in `storage/storage.go` | LOW | **FIXED — but recurred elsewhere** | `gofmt -l Server/` is clean except one **new** drifted file, `admin/handlers_users_broadcast_test.go` (§6); no gofmt gate exists in CI or `.golangci.yml`, which is why it drifted in | +| T-2026-07-25-17/-18/-19 (`main.go`/seed untested; `MainPage.ts`/`main.ts` excluded; no Go/Rust coverage floor) | LOW | **UNCHANGED, documented** | vitest excludes still carry written justifications (+1 justified entry: `noise-suppression.ts`); the rest are recorded accepted positions | +| T-2026-07-25-20 `ws` flake under `-coverpkg` | LOW | **CAN'T VERIFY** | Not reproduced this session (plain `go test` used); stays on watch | +| DC-14 reserved protocol entries (`voice_speakers`, `member_leave`) | P3 | **STILL OPEN, deliberate** | Constants + schema entries exist, zero non-test emit sites; consistently documented "Reserved" in `protocol.md:1506,1508`; a protocol rev for the owner to schedule | +| `admin-e2e` soak graduation | P2 | **STILL PENDING** | `ci.yml:311` `continue-on-error: true`, graduation criterion recorded 2026-08-15 (~30 consecutive green main runs, `ci.yml:302-307`) | +| 2026-07-19 §5: `tauri-build` never runs on push to `main` | LOW | **STILL TRUE** | `ci.yml:440-443` gates on `pull_request` + `base_ref == main`; only other desktop build is the `v*`-tag release workflow | +| `livekitSession.ts` monolith | MED | **STILL OPEN, grew** | 1809 LOC (was 1719); satellites `roomEventHandlers.ts`/`livekitDiagnostics.ts` were added without shrinking the core. `AccountTab.ts` 1185, `SidebarArea.ts` 848, `LoginForm.ts` 784 | +| TODO inventory | — | **3 remain** | `ws/voice_e2ee.go:167` (key-holder TOCTOU, documented accepted), `ws/deps.go:271` (generics), `SidebarArea.ts:659` (H16 DOM thrash). The `update_handlers.go` container TODO is **resolved** (503 `CONTAINER_DEPLOYMENT` via `updater.RunningInContainer`, `update_handlers.go:49-53`) | + +### 2.2 CI gates and pins (all verified holding) + +`.nvmrc` = 20 matches all six `setup-node` pins; knip, `client-e2e`, +`client-e2e-parity` genuinely blocking (no `continue-on-error`, no `|| true`); +`-tags wazero ./plugin/...` and `-tags otel ./telemetry/...` run in CI +(`ci.yml:87-89`); `rust-tests` runs on every event; +`tests/e2e/E2E-ISSUES.md` matches the live Playwright configs (293 tests vs +the doc's dated 291 — normal growth, counts are labeled as a run snapshot). + +### 2.3 Plan statuses (docs/plans/) + +| Plan | Verdict | Notes | +|---|---|---| +| security-hardening-remediation | **complete, verified** | W2-4/W3-3 closed with the named locking tests present | +| channel-visibility-unification, http-tofu-proxy, permission-middleware-consolidation, v2-dispatch-migration, audit-2026-07-19-decisions | **shipped, nothing reopened** | only cosmetic line-number drift in citations | +| security-scan-2026-07-22-remediation | **shipped; header stale in the good direction** | F3 follow-up 3 (`getIdentityPin` fail-open) is marked open but was fixed (`identity.ts:158-175`, tri-state lookup citing DC-08) | +| sqlc-adoption | **accurate** | residual raw queries (variable `IN`, FTS, tx) still deliberately raw as listed | +| tauri-capability-narrowing | **DNS-rebinding follow-up still open** | `embeds.ts:8` still fetches user-posted URLs from the renderer via `@tauri-apps/plugin-http`; `capabilities/default.json` still grants `https://*` (:33,:36); no Rust og-meta command exists | +| infrastructure-roadmap | **shipped except 2 named leftovers** | TOTP/partial-auth persister seam (no persist code in `auth/totp.go`) and published capacity numbers (the load-baseline workflow exists to produce them) | +| discord-parity | **phases 1-6 shipped; named leftovers open** | role hoist/mentionable + `@RoleName` mentions, categories as real entities; its still-dead-code list is one item stale (sounds table was dropped by migration 029) | +| bug-detection-improvements | **header stale — partially implemented** | header says "not implemented" but Tier 1a (`make fuzz`, `Server/Makefile:43-48`) and Tier 2 (5 ESLint rules, delivered 2026-08-08 per its own body) exist; Tiers 3-4 open | +| slash-commands | **design-only, as stated** | no `plugin_commands` migration, no autocomplete surface | + +--- + +## 3. Dynamic checks (this session, at `eacba10`) + +Environment: Linux container, 4 CPU / 15 GB; go 1.24.7 host + +`GOTOOLCHAIN=auto`; Node 22.22.2 (CI pins 20), npm 10.9.7. + +| Check | Command | Result | +|---|---|---| +| Go vet | `cd Server && go vet ./...` | **PASS** | +| Go tests | `go test -count=1 ./...` | **PASS** — 15 test packages ok (`ws` 54s, `db` 14.5s) | +| Client typecheck | `npm run typecheck` | **PASS** | +| Client lint | `npm run lint` | **PASS** — exit 0 with 25 non-blocking `no-underscore-dangle` warnings, all in `livekitSession.ts:1656-1759` (delegating-method section) | +| Client unit suite | `npm test` | **5044/5045** — one timeout failure, shown to be a load flake (§5, F-5); passes 3/3 in isolation | +| Server build | `go build .` | **PASS** | +| Server boot | run binary in empty dir | **CLEAN** — first-run config + certs generated, expected WARNs only; `/health` 200 `{"status":"ok","uptime":1,"online_users":0}`; `/admin` 200 serving the SPA (145,663 bytes); LiveKit auto-download + managed start; clean shutdown on SIGTERM | +| Client build | `npm run build` | **PASS** (chunk-size warnings only) | +| Protocol codegen gate | `go run ./scripts/genprotocol` diffed against committed output | **CLEAN** — byte-identical Go + TS | +| sqlc gate | `make sqlc-verify` | **SKIPPED** — sqlc binary absent in this container; CI covers it | +| knip | `npx knip` | **exit 0** — zero unused files/exports/deps (4 cosmetic config hints) | +| Go module hygiene | `go mod tidy -diff` | **CLEAN** — empty diff | + +--- + +## 4. New findings — BROKEN (all documentation) + +No code-level BROKEN findings. All ten are reference-doc statements that are +false against the running code; a reader following them writes wrong clients +or wrong ops procedures. All were independently re-verified. + +### schema.md + +| ID | Finding | Evidence | Fix | +|---|---|---|---| +| B-01 | Migration table ends at 029; `attachments.message_id` documented as `ON DELETE CASCADE`, the exact behavior migration 030 removed (`ON DELETE SET NULL`, to stop stranding uploaded files); 031 undocumented | `schema.md:44-74`, `:420` vs `migrations/030_attachments_unlink_on_message_delete.sql:25` (+ header lines 1-16), `031_sessions_expiry_index.sql` | Add 030/031 rows; flip :420 to `SET NULL` with the unlink semantics | +| B-02 | Indexes table lists 3 dropped indexes (`idx_sessions_token`, `idx_invites_code`, `idx_channel_overrides_channel_role`) and omits 7 live ones (`idx_channel_overrides_role`, `idx_attachments_message`, `idx_messages_pinned`, `idx_api_tokens_user`, `idx_users_avatar`, `idx_channels_dm_group`, `idx_sessions_expires_at`) | `schema.md:719,723,728` vs `migrations/019_perf_indexes.sql`, `020_drop_redundant_indexes.sql:6-7`, `018:27`, `027:28`, `028:29`, `031:13`; the doc's own migration rows (:64-65) contradict its index table | Rewrite the table from cumulative migration state | +| B-03 | "The connection pool is pinned to a single connection" — false for file-backed (production) DBs, which run a 1-writer + up-to-64-reader pool split | `schema.md:27` vs `db/db.go:223` (`writer.SetMaxOpenConns(1)`), `:239-244` (reader pool `max(4, NumCPU)`, `database.max_readers`) | Describe the writer/reader split and the knob | + +### protocol.md + +| ID | Finding | Evidence | Fix | +|---|---|---|---| +| B-04 | DM `chat_message`/`chat_edited`/`chat_deleted`/`reaction_update` documented as unsequenced and excluded from the ring buffer; they are seq-stamped, ring-buffered AND persisted (tier-1 replayable) | `protocol.md:98,263-264,1489-1494` vs `ws/emit.go:23-26` → `hub_broadcast.go:923-935` (`nextSeq` + `replayBuf.Push` + `persistEvent`); emit sites `handlers_chat.go:102,123,146`, `handlers_reaction.go:47` | Fix the seq tables + reconnection section | +| B-05 | `plugin_broadcast` documented "Has seq? No"; it rides the sequenced, replayable channel-broadcast path | `protocol.md:1524` vs `ws/event.go:347-357`, `hub_broadcast.go:939-964` | Flip the row to Yes | +| B-06 | `RATE_LIMITED` WS errors claimed to "include retry_after in seconds"; no WS error payload ever carries it (only REST 429 headers do) | `protocol.md:1399` vs `ws/messages.go:332-358`, `ws/event.go:7-12`; repo-wide grep negative | Delete the clause (or implement the field) | +| B-07 | Five c2s types documented rate limit "None" are all limited: `channel_focus` 5/s, `mark_read` 5/s, `call_decline` 1/3s, `chat_command` 5/s, `ping` 2/s | `protocol.md:1461,1462,1478,1479,1480,1535` vs `ws/handlers_presence.go:19-22,107,135`, `handlers_call.go:15-18,75-77`, `handlers_command.go:29-36,49-51` (comment cites OC-0091 — limiter added, doc never updated), `handlers_ping.go:14-16` | Document all five incl. silent-drop vs `RATE_LIMITED` behavior | +| B-08 | E2EE prose says "Both E2EE message types are rate limited at 5 per second", contradicting the doc's own table (offers 64/s) and the code; the inner 5/s-per-target offer cap is undocumented | `protocol.md:1112` vs `:1433,1437-1439`; `ws/voice_e2ee.go:16-24,213-223` | Fix the prose; document the per-target inner cap | + +### api.md + +| ID | Finding | Evidence | Fix | +|---|---|---|---| +| B-09 | `GET /api/v1/diagnostics/connectivity` documented "any authenticated user"; code requires ADMINISTRATOR (H-8) — a non-admin following the doc gets 403. Also: limiter is per-IP not "per user"; `livekit_url` is sanitized to host:port, not the documented full URL | `api.md:2472,2473,2485` vs `api/router.go:153-158`, `middleware.go:228-229`, `diagnostics_handler.go:54-58` | Fix auth line, limiter scope, example | +| B-10 | Error-code table documents `SERVER_ERROR` / `INTERNAL` / `TOO_LARGE` — none ever emitted by REST (real 500 code is `INTERNAL_ERROR`, ×60+ sites; oversize upload is 400 `BAD_REQUEST`); `STORAGE_ERROR` 507 missing entirely. A client switching on documented codes never matches | `api.md:57-58,119,181,229,310` vs greps (zero `SERVER_ERROR`/`TOO_LARGE` emissions), `upload_handler.go:115-146`, `storage/storage.go:172`, `ws/errors.go:6` | Replace with `INTERNAL_ERROR`, correct the 413 row, add `STORAGE_ERROR` | + +--- + +## 5. New findings — FRAGILE + +**F-1 — Blocking a user does not evict them from a live 1:1 DM voice call.** +`BlockUser` commits with zero side effects (`service/block.go:24-54`; +`api/dm_handler.go:83-88` takes no evictor). The only block re-checks for a +user already in voice are client-volunteered (`ws/voice_join.go:136-141` join +gate; `:579-590` token refresh). The minute-sweep — whose own invariant +comment says "Revocation must evict a live session, not merely block the next +join" (`ws/hub_sweep.go:167`) — re-checks only role CONNECT_VOICE +(`hub_sweep.go:185,328-354`), which has no DM/block branch; LiveKit validates +tokens at join only, so a client that stops calling `voice_token_refresh` +keeps its SFU session indefinitely. Contrast: `CloseDM` evicts via +`dmVoiceEvictor` (`api/dm_handler.go:236-242`). This is the same +guard-asymmetry family as A-2026-08-03 (block check on rings), one sink over. +*Adversarially verified: CONFIRMED.* +**Fix:** have `handleBlockUser` evict via the existing `dmVoiceEvictor` for +the pair's shared 1:1 DM channel (mirroring `handleCloseDM`), or add a +DM-block re-check to the sweep. + +**F-2 — Role reassignment racing a WS handshake leaves the socket on +old-role subscriptions.** The handshake resolves permissions from the +auth-time `c.user` snapshot (`ws/serve.go:124-132`; `computeAllowedChannels` +via `GetRoleByID(user.RoleID)` at `:670`, used by both `:306` and `:736`), and +nothing revalidates handshake-time subscriptions (the OC-0024 recheck covers +only `applySetChannelID`; the sweep re-checks CONNECT_VOICE only). The +demotion's live revocation, `revokeUnreadableChannels` +(`hub_broadcast.go:797-879`), early-returns if the user is not yet in +`h.clients` (:818-822) and its `Unsubscribe` no-ops on a replaced client via +the pubsub identity guard (`pubsub.go:133-134`) — so a role change landing in +the handshake window is never applied to that socket, which then receives +events for channels the new role cannot see until the next reconnect. +`RefreshChannelVisibility` handles both hazards explicitly (`hub_broadcast.go:443-444`); +the role path predates that pattern. *Adversarially verified: CONFIRMED.* +**Fix:** re-read the user row inside `reconnectPrecheck`/`handleFreshConnect`, +and make `revokeUnreadableChannels` re-resolve the live client before +`sendMsg`/`Unsubscribe`, mirroring `RefreshChannelVisibility`. + +**F-3 — Auth lockout persistence failures are silent.** All three lockout +writes discard errors with no log: `_ = r.store.UpsertLockout(...)` +(`auth/ratelimit.go:172`), `_ = ...DeleteLockout` (:234), +`_ = ...CleanupExpiredLockouts` (:283) — while the load path got exactly this +fix as ledger finding OC-0061 (the file's only `slog` call, :88). The doc +comment at :162-164 says "The persist write must land once the lockout is +decided", yet a failed write leaves zero trace and the lockout evaporates on +restart. *Adversarially verified: CONFIRMED (not an OC-0061 duplicate — this +is the symmetric write-path gap).* +**Fix:** mirror OC-0061 — `slog.Warn` on all three error paths. + +**F-4 — Session-cap eviction failure is invisible (observability gap; +downgraded from a stronger claim).** `CreateSession` discards the H-6 +eviction error (`db/auth_queries.go:244` `_ = d.q.EvictOldestSessions(...)`) +with no log anywhere in the chain. The verifier established the cap is not +durably disabled — the DELETE trims to the newest 24 so the next successful +login self-heals, and a persistent DB failure also fails the subsequent +`InsertSession` — so the residue is: a transient failure lets one session +exceed the cap, and no failure of a security control is ever logged. +*Adversarially verified: DOWNGRADED to this statement.* +**Fix:** `slog.Warn` on eviction error only. + +**F-5 — One client unit test is a load-sensitive timeout flake.** +`tests/unit/message-list.test.ts` › "does not report success when +renderWindow's own >30-in-2s breaker drops the rebuild" performs 30 +synchronous full `renderWindow` rebuilds of a 100-row jsdom list inside the +default 5s vitest timeout. In this session's full run (concurrent with `go +test` on a 4-CPU box) it timed out — 5044/5045 — then passed 3/3 in isolation +(~4.2s per whole-file run). Same class as the open T-2026-07-25-20 watch item. +**Fix:** give that test an explicit larger timeout (or shrink the fixture) so +CI-under-load cannot produce a spurious red. + +**F-6 — Client replay-dedup machinery can never engage (dead safety net + +misleading tests).** The server always writes `auth_ok` *before* the replay +burst (`ws/serve.go:531,537-544`), but the client creates `replayDedup` in the +socket-`open` handler and clears it when `auth_ok` is processed +(`ws.ts:463-465`, `:358`) — so the dedup set and `isReplaying()` (`ws.ts:788`) +are inactive for every real replayed frame; the dispatcher's gates at +`dispatcher.ts:580,592` never fire, and its own comment at `:612-618` admits +the timing. The pinning tests inject replay frames in an order a +spec-compliant server never produces (`ws-reconnect.test.ts:409-497`). +Meanwhile the server *can* legitimately duplicate a voice frame on resume (the +voice supplement is re-read outside `seqMu`, `serve.go:263-271,553-556`), which +is what the machinery was presumably for. Today's no-op behavior is actually +correct for unread counts, making this inert-but-misleading rather than +harmful. *Adversarially verified: CONFIRMED.* +**Fix:** either delete the inert machinery (and fix comments + tests) or move +the clear to end-of-burst — decide, don't leave it half-true. + +--- + +## 6. New findings — DEBT + +Grouped; every item carries its evidence at the cited lines. + +**Docs/comments** +- D-01 `protocol.md` error-code table missing `BAD_PAYLOAD` and + `NOT_KEY_HOLDER` (emitted at `ws/voice_e2ee.go:119-199`). +- D-02 `protocol.md` ready/member_join field lists incomplete: `voice_states` + omits `username`/`speaking`/`camera`/`screenshare`, `roles` omits + `position`/`is_default`, `member_join` omits its top-level `status` field + (`serve_ready.go:365`, `db/models.go:99-106,234-245`, `ws/messages.go:56-67`). +- D-03 `api.md`: body-cap exemption list wrong (3 exempt prefixes, not 1 — + `api/constants.go:177-184`); `identity_public_key` accepted by + `PATCH /users/me` but undocumented (`profile_handler.go:29-37,100-114`); + plugin endpoints return plain-text errors contradicting the "all errors use + the JSON envelope" claim, and the `X-Plugin-Runtime` header is undocumented + (`plugins_handler.go:51-136`); `/health`'s 503 degraded state + `reason` + field undocumented (`router.go:446-453,509-544`); metrics/LiveKit endpoints + have their own CIDR keys with admin fallback, not "admin CIDRs" + (`config/config.go:219-233`); `updates/apply` can 409 + `RESTART_PENDING`/`UPDATE_IN_PROGRESS`, absent from its table + (`admin/restart.go:104-112`). +- D-04 Stale comments: `buildReady`'s claims "no slow_mode … voice_* extras" + contradicted by `readyChannelPayloads` directly above + (`serve_ready.go:312-315` vs `:216-224`); `tsconfig.e2e.json:2-3` + + `ci.yml:155-156` say "47 spec files + the three playwright configs" (52 and + four); stray word "ponytail" in `logctx.go:49`. +- D-05 Three plan headers understate shipped work (bug-detection-improvements, + security-scan-2022-remediation F3-3, discord-parity's sounds-table row) — + all drift in the direction that misdirects planning toward finished work. +- D-06 **Branch-flow docs contradict practice**: README:230-232 and + `contributing.md:152-153` say branch from `dev`, PR to `dev`, "dev is merged + to main for releases"; root `CLAUDE.md:63` says branch from `main`, PR to + `main`. Practice matches CLAUDE.md — the 5 most recent merged PRs + (#1390-#1394) are `base:main`, and the sync direction is inverted (commit + `40cba79` merges `main` *into* `dev`; `dev` is 4 graph-refresh commits + ahead, 0 behind). + +**Dead code / hygiene (deadcode + knip corroborated)** +- D-07 `Hub.hasChannelPerm` is unreachable in production; its doc comment + claims the sweep calls it, but the sweep uses `hasChannelPermChecked` + (`ws/handlers.go:286-292` vs `hub_sweep.go:185,320-354`); the F5 guarantee + test pins the dead copy (`voice_perm_stale_test.go`). +- D-08 `UserService.SetCustomStatus`/`ClearCustomStatus`: zero production + callers; "Called on logout" comment false (logout writes the DB directly, + `auth_handler.go:584`); duplicates the length validation + `HandlePresenceUpdate` owns (`service/user.go:201-215` vs `channel.go:183`). +- D-09 Production-unused hub APIs: `PubSub.PublishHigh` (`pubsub.go:181`), + `Hub.BroadcastToAllLow` (`hub_broadcast.go:907`), + `EventRingBuffer.EventsSince` (`ringbuffer.go:43`), and the + `Hub.Register/Unregister` clientEvents queue (`hub.go:446-453`, whose + `Run` branch is test-only and would fail closed for a future production + caller); `TopicsForClient`'s "debugging and tests" doc is stale — it has a + production caller (`hub_broadcast.go:850`). +- D-10 gofmt drift: `admin/handlers_users_broadcast_test.go` (one misaligned + field); no gofmt/gofumpt gate exists in CI or `.golangci.yml`. +- D-11 25 non-blocking `no-underscore-dangle` lint warnings accumulated in + `livekitSession.ts:1656-1759`. + +**Boundary asymmetries (all privileged-actor or minor; the next members of +the audit-2026-08-04 defect family — worth sweeping in one pass)** +- D-12 `typing_start` requires only READ_MESSAGES on guild channels while the + send it advertises requires SEND_MESSAGES (announcement: MANAGE_MESSAGES) — + a send-muted member can still broadcast typing (`service/channel.go:133` vs + `message_perms.go:93-99`; the DM branch *is* fully gated). +- D-13 `getPermChannel` answers a DM id with 400 "DM channels do not support + permission overrides" while its sibling `getAdminChannel` deliberately + answers 404 to avoid confirming which ids are DMs (A-2026-08-02) — same + perimeter, one oracle left (`admin/handlers_channel_perms.go:40-44` vs + `handlers_channels.go:44-61`). +- D-14 Admin channel create/patch has no length cap or sanitation on + name/topic/category (only the 1 MiB body cap); the group-DM writer of the + same column caps at 100 runes and its comment claims a channel-name parity + that does not exist (`admin/handlers_channels.go:104-116,171-183` vs + `service/dm.go:173-175,234-237`). +- D-15 Invite create/revoke are the only privileged mutation family with no + audit row and no log line — an operator cannot see who minted an invite + (`service/invite.go:29-77`; every sibling family audits: roles, bans, + emoji, settings, channels, purge, auth). +- D-16 `channel_focus`'s read-state write is called "the load-bearing half" + in its own comment, then discarded unlogged + (`service/channel.go:275-286`); a persistent failure means unread badges + never clear with zero trace. + +--- + +## 7. Verified clean (read end-to-end and found sound) + +Recorded so future audits don't re-spend the effort. Hub dispatch loop (panic +breaker, stop idempotence, register/replacement ordering); every `seqMu` +critical section (no lock held across real I/O; shed frames never burn a seq); +lock ordering across `seqMu`/`h.mu`/`ps.mu`/`rb.mu`/`c.mu`/`keyHolderMu` with +no reverse edge; pubsub index hygiene and identity guards; three-tier client +queues + writePump anti-starvation; topic rate limiter and pruner/persister +lifecycle (no goroutine leaks found); ring-buffer tier boundaries (both edges +fall conservative to full ready) and the closed lost-update window +(register-inside-`seqMu`); cold-tier contiguity probes including the +correct off-by-one on the prune gap; replay authorization incl. the OC-0206 +watermark re-check under `seqMu`; seq reseeding across restarts; client +backoff/generation discipline (OC-0219) and optimistic-send reconciliation +across reconnects; REST/WS message-family parity (send/edit/delete/reaction/ +pin/purge all funnel through the same service gates); REST-mutation fan-out +ordering (post-commit, `context.WithoutCancel`, watermark bumps); admin +users/bans/roles/override families (hierarchy, audit rows, invalidation +ordering); plugin sandbox mitigations (§2.1); protocol type inventory +(27 c2s / 39 s2c identical across schema, Go, TS, and doc tables) and +generated-code gates (protocol codegen byte-identical; `go mod tidy` clean; +knip zero findings; no TS zombie modules; Rust `unwrap`s confined to test +modules); E2EE section of protocol.md including the `eacba10` epoch change — +fully documented and accurate. + +--- + +## 8. Verdict + +**MUST fix (before the next feature work lands on top):** +1. The ten BROKEN reference-doc statements (§4) — one spec-refresh PR, same + shape as 2026-08-04's C1. Docs are the record everything else keys off, + and two of the ten are security-relevant reader traps (B-09 auth claim, + B-07 undocumented limits). +2. F-1 block/voice eviction — it is the live continuation of the + guard-asymmetry family the security review closed; small, testable fix. +3. F-3 + F-4 + D-16 silent-failure logging — three `slog.Warn` lines + restoring visibility to security/UX controls. + +**CAN wait (scheduled, not urgent):** +F-2 (handshake race — real but narrow window; needs a careful hub change + +deadlock-tag run), F-5 (test timeout bump), F-6 (delete-or-fix decision), +all §6 DEBT including the D-12..D-15 asymmetry sweep and the D-06 +contributing-docs correction, and every §2 carried architecture item +(service-layer consolidation, hub constructor, client constants, monoliths). + +**Already done (no action):** everything in §2 marked FIXED/intact — backup +restore, CI gates, `.nvmrc`, plugin mitigations, DC-08, W2-4/W3-3, protocol +codegen and ledger hygiene — plus §7's verified-clean list. The bug-hunt +ledger has zero open findings. + +--- + +## 9. Roadmap + +### 9.1 Fix order (every BROKEN + FRAGILE, dependencies first, smallest verifiable step first) + +1. **F-5** test-timeout bump (one line in one test) — protects the CI signal + every later step relies on. +2. **§4 B-01..B-10** spec-refresh PR for schema.md/protocol.md/api.md, + folding in D-01..D-05 while in the files — pure docs, zero code risk, ends + the "docs are the record" debt before code changes add more drift. +3. **F-3 + F-4 + D-16** logging PR (three `slog.Warn` sites, mirrors the + shipped OC-0061 pattern) — trivial, independently testable. +4. **F-1** block → DM-voice eviction (reuses the existing `dmVoiceEvictor` + seam `CloseDM` already exercises; pin with a test like + A-2026-08-03's) — smallest security-adjacent code fix. +5. **F-2** handshake/role-change race — hub-internal, needs the `ci-check` + deadlock pass; do after 4 so the voice-eviction test infrastructure is + fresh. +6. **F-6** replay-dedup decision (delete vs repair) — last because it needs + an owner call; either branch also fixes the misleading tests/comments. + +### 9.2 Alpha exit — what stands between here and a stable beta +*(each item backed by a finding above or an existing plan)* + +- **Graduate the two non-blocking CI legs**: `admin-e2e` (soak criterion + already written in `ci.yml:302-307`) and a `tauri-build` push-to-main or + nightly trigger (2026-07-19 §5, still true) — after which every surface is + gated. +- **Add a gofmt gate** (D-10 proved the gap; one CI line). +- **Close the DNS-rebinding follow-up** (tauri-capability-narrowing): move + link-preview fetches behind a Rust command and drop the `https://*` + capability — the last open item from the F3 security track. +- **Finish the two infrastructure-roadmap leftovers**: TOTP/partial-auth + persister seam, and publish the capacity numbers the load-baseline workflow + exists to produce. +- **Start the service-layer consolidation** at `MountAuthRoutes` + (2026-04-07 #9, the designated first step of backlog 12), then the hub + constructor cleanup (A-2026-07-11 S-effort half). +- **Unify `channelCanSend`** onto the permission checker — the last + hand-mirrored visibility/permission copy. +- **Protocol rev decision** (owner): emit-or-drop `voice_speakers` / + `member_leave` (DC-14), and wire the plugin wire types into the client or + document desktop non-support (D-02's sibling). +- **Docs process hardening**: the 15-day re-drift (§4) says the per-PR rule + alone doesn't hold; add the docs-check line to CI or the PR template + checklist with teeth (a grep-able "docs reviewed" gate), plus the D-06 + contributing-flow correction so new contributors aim at the right branch. + +### 9.3 Ideas — opinion, not findings + +1. Slash-commands Phase A (plan exists, design-only today) — first + user-visible plugin payoff. +2. Role hoist/mentionable + `@RoleName` mentions (discord-parity leftover). +3. Categories as real entities (discord-parity leftover). +4. Linux ARM64 server binary + prebuilt Docker image (README platform-matrix + gap). +5. macOS PTT support (currently a stub, noted in discord-parity's dead-code + list).