Fixes all 109 golangci-lint findings (106 contextcheck, 1 gocritic,
2 gosec) that accumulated after D2 wired dbgen (whose queries take ctx)
under ctx-less db.DB wrappers while CI lint was quota-dead. No nolint
comments added; every finding fixed by genuinely threading context.
- db: all 138 hand-written db.DB methods take ctx first; the dbCtx()
Background shim is deleted; raw Query/QueryRow/Exec/Begin use their
Context variants; the four redundant ctx-less passthroughs removed.
db.Auditor/WriteAudit gain ctx.
- Seams: permissions.Checker (DB iface, HasChannelPerm,
RequireChannelAccess) and the service.Store interface mirror the new
signatures (ws.EventStore and plugin.PluginStore already did).
- Callers: api/admin handlers use r.Context(); ws per-message paths use
the connection ctx via DispatchV2; hub loops and startup wiring use
context.Background(); service methods thread ctx where they have one
and Background where no ctx exists. Public service surface reached by
ctx-holding chains (PermissionService.HasChannelPerm/GetRoleForUser/
RequireChannelAccess, message/dm/block/invite/profile methods) is now
ctx-first.
- Detached (context.WithoutCancel) where cancellation would break an
invariant, found by a 3-lens adversarial review of the diff:
* voice-leave background retries (a dead webhook/connection ctx killed
retry 2 before it ran, leaving ghost capacity-holding voice rows)
* rollbackVoiceJoin's compensating delete (its trigger IS the cancel)
* post-2FA-change DeleteOtherSessions and logout DeleteSession (the
security tail of a committed change must not die with the request)
* all api/ws audit writes (a banned user could suppress their own
login_blocked_banned row by aborting the request mid-bcrypt)
* admin backup VACUUM INTO (an interrupt left a truncated .db that
the backup list presented as restorable)
* post-commit message/edit refetches (a committed message must still
fan out when the sender disconnects)
* hub settings-cache refresh (one dead connection could pin stale
values for the 30s TTL)
- gocritic rangeValCopy fixed (index iteration); gosec G306 excluded in
config with justification (generated source must stay world-readable)
instead of flipping genprotocol output to 0o600.
Verified: gofmt/vet, all four build-tag variants, full suite, deadlock
pass, full -race pass, golangci-lint 0 issues uncapped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deletes Server/store (SQLiteStore, MemStore, the composed Store
interface) and collapses to a single sqlc-backed db package, executing
the prior audit's P4 "single data layer" direction (finding #6).
SQLiteStore was a pure pass-through to *db.DB, so consumers now depend
on narrow interfaces that *db.DB satisfies directly:
- service.Store (service/datastore.go, renamed from store/store.go)
- ws.EventStore (ws/eventstore.go)
- plugin.PluginStore (plugin/pluginstore.go)
The event- and plugin-KV methods that lived in the store's SQLite
implementation move into the db package (db/event_queries.go,
db/plugin_queries.go), keeping their raw-SQL form.
Tests: the MemStore-based unit tests now run against a real in-memory
SQLite db opened per-test with migrations applied, via package-local
seed helpers. Fault-injection tests embed a real *db.DB and override the
single method under test, preserving error-path coverage. Full server
suite and sqlc-verify are green.
Docs: audit finding #6 and A-2026-07-06 marked resolved; decisions D3
updated; architecture server.md / data-model.md diagrams and prose
updated to the api -> service -> db layering.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UA17KPvqGBX3XbXYnMf1rA
PostgresStore was 86% stubs behind a build tag nothing enables, pgdbgen
carried hand-added build tags that fought sqlc-verify, and the runtime never
threaded store.Store through the handler boundary. Single-engine reality
shrinks the W1-3 attachment-ownership fix and ends the pgdbgen churn.
Removed: store/postgres.go, db/pgdbgen/, db/queries/postgres/,
migrations/postgres/, the sqlc postgres block, pgx from go.mod, the
startup-refusal branch, and the dead Postgres config surface.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clean sweep of every actionable item from the two Copilot review passes
on head 59ae4d8. Grouped by severity:
─── Crash / security (must-fix) ─────────────────────────────────────
1. main.go:140 — telemetryShutdown nil panic.
telemetry.Init can return (nil, err) on the -tags otel skeleton
path; the deferred closure would then call a nil function. Normalise
to a no-op shutdown when Init errors so the defer is always safe.
2. api/upload_handler.go — permSvc nil deref.
MountUploadRoutes + handleServeFile dereference permSvc on every
authenticated file request. Add a fail-fast panic at mount time so
the misconfiguration surfaces at wiring, not on the first 500.
Update upload_handler_test.go to pass a real PermissionService built
on the test DB (the existing tests were missing the argument entirely,
which meant the package wouldn't compile — this fixes the real bug
Copilot flagged).
3. ws/event_persister.go — NewEventPersister nil EventStore panic.
run() dereferences p.store on every flush. Panic at constructor
time instead so the crash happens once at startup rather than
minutes later in a background goroutine.
4. plugin/host_ui.go — serve-time symlink check.
rejectSymlinksUnder only runs at install time, so a symlink created
post-install (accidental or malicious) would be followed by
http.ServeFile and leak host files. Add an os.Lstat + ModeSymlink
check + IsRegular check to AssetHandler on every request. Cheap
relative to the file read and closes the TOCTOU window.
─── Correctness / observability (should-fix) ───────────────────────
5. ws/deps.go:77 — requirePerm hides misconfig as FORBIDDEN.
Previously, nil database, nil perms, or a GetRoleForUser error all
returned ErrCodeForbidden with the same message, making operator
failures indistinguishable from legitimate permission denials.
Split the branches: misconfig + DB error now return ErrCodeInternal
with a server-side slog.Error so operators see the real problem;
FORBIDDEN is reserved for the actual permission-bit check.
6. telemetry/metrics.go — ServiceCallDurationMs renamed to Sec.
Field name said "Ms" but the instrument name was
`service_call_duration_seconds` with unit "s". Renamed the field
and updated all 8 service-layer callers so the struct field and
metric semantics match.
7. ws/event_persister.go — flushEvy typo → flushEvery.
Renamed the field and the one call site in run().
─── Comments out of sync with code ──────────────────────────────────
8. plugin/loader.go — Stat vs Lstat comment.
The comment claimed "Stat (not Lstat)" but the code correctly uses
os.Lstat to detect symlinks. Updated the comment to match the code;
the code was already right.
9. telemetry/telemetry_otel.go — compile claim wrong.
Comment said the file would fail to compile without the upstream
OTel modules, but the skeleton deliberately avoids importing them
and Init returns a runtime error instead. Updated the comment to
reflect actual CI behaviour (the -tags otel build step passes
today but doesn't exercise real telemetry).
─── Nit / polish ────────────────────────────────────────────────────
10. ws/event_pruner.go — startup delay magic constant.
Hard-coded time.Minute made the "run shortly after startup"
behaviour untestable (a test with a 100ms interval would still
wait a full minute). Cap the startup delay by the interval:
min(interval, time.Minute). Documented via a new `maxStartupDelay`
constant.
11. ws/event_pruner_test.go — new file.
Unit coverage for runPrune cutoff correctness, error swallowing,
StartEventPruner nil-store short-circuit, ctx cancellation, and
the interval-bounded startup delay from fix#10. Uses a fakeEventStore
stub that records every prune call and signals the first one so
tests don't sleep.
─── Verification ────────────────────────────────────────────────────
gofmt -l clean. No network access in sandbox so `go vet` and `go test`
could not run; the changes are local and surgical and every touched
file compiles in isolation against the existing signatures.
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
Phase B + C review pass: critical security and correctness fixes.
Security
- S1: plugin admin endpoints now require admin.RequireAdminAuth in addition
to AdminIPRestrict. Previously a LAN attacker on the allowed CIDR could
list/enable/disable/uninstall plugins without a session.
- S2: rewrite plugin HTTPDo allowlist with proper net/url parsing. Empty
entries are ignored, suffix matches require a dot boundary, and a custom
Dialer rejects loopback / RFC1918 / link-local addresses to close the
DNS-rebinding TOCTOU window. Redirects re-validated.
- S3 + #9: manifest Name pinned to ^[a-z0-9][a-z0-9_-]{0,63}$, Entrypoint
and UI tab assets validated against absolute / "..", NUL byte, backslash
and non-canonical paths. Asset handler hardened with filepath.Rel check
for symlink and prefix-without-separator escapes.
- S5: pluginBridge postMessage handler ignores the pluginId in the message
body and uses an e.source -> contentWindow lookup instead, defeating
spoofed messages from same-origin scripts.
- S8: HTTPDo body capped at 5 MiB via io.LimitReader, redirects bounded
to 5 hops.
Correctness
- Critical seq alignment: PersistEvent now takes the hub-assigned seq as a
required parameter so the events table row seq always matches the wrapped
payload seq. Hub seeds its in-memory atomic counter from MAX(events.seq)
on startup. Drops in the persister queue no longer mis-align row vs
payload seq.
- #1: live plugin.Registry constructed in main.go BEFORE NewRouter and
threaded through; admin handler is no longer wired with nil.
- #3: EventPersister.Stop is now safe to call without a prior Start by
tracking a started flag — previously deadlocked waiting on done.
Wiring
- NewRouter signature gains *plugin.Registry; two test callers updated.
- admin.RequireAdminAuth exported as a thin wrapper over the existing
package-private adminAuthMiddleware.
- sqlc query templates updated for the new PersistEvent + GetMaxEventSeq
contracts (sqlite + postgres).
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
Server:
- Fix YAML injection in LiveKit config generation (quote values)
- Revert token TTL to 4h (no server-side JWT revocation)
- Derive LiveKit publish permissions from user role (prevent SFU bypass)
- Add CAS guard for webhook/voice_leave race condition
- Add voice_leave broadcast to rollbackVoiceJoin (prevent ghost state)
- Limit webhook body to 64KB (prevent memory abuse)
- Add rate limit to voice_token_refresh handler (1/60s)
- Add LiveKit health check endpoint (GET /api/v1/livekit/health, 503 on degraded)
- Add voice_token_refresh WS handler for client-initiated token refresh
- Consolidate voice quality constants (single source of truth)
- Fix video limit TOCTOU race (count from DB instead of LiveKit API)
- Raise default voice_max_video from 10 to 25 (Discord parity)
- Add CountActiveCameras DB query
- Non-blocking broadcast send, circuit breaker, exponential backoff
- Close send channel before context cancel in serve.go
- Guard voice mute/deafen for active channel
- Delete orphaned message on attachment link failure
- Redact query string from proxy logs (prevent token leak)
- Use instance-level HTTP client for health checks (no redirect following)
- Set cmd.WaitDelay to prevent goroutine leak on Windows
- Log buildJSON marshal errors
Client:
- Refactor livekitSession.ts from singleton module to LiveKitSession class
- Share single AudioContext for all analysers (was 1 per participant)
- Extract createRoom() helper (DRY)
- Add token refresh timer (3.5h interval, re-arms on failure)
- Skip setSpeakers if unchanged (sort in-place, no allocations)
- Distinguish user-initiated leave from connection error in retry
- Add YouTube videoId validation (prevent iframe src injection)
- Add try/finally to disableCamera
- Wrap store subscription callbacks in try/catch
- Track and cancel initial scroll RAF on cleanup
- Add 5s timeout + encodeURIComponent to YouTube oEmbed fetch
- Clean raw mic stream on RNNoise suppressor failure
- Full voice cleanup on logout via cleanupAll()
Tests:
- Add 7 new server tests (webhook parsing, voice guards, quality fallback)
- Fix 2 pre-existing test failures (mute/deafen invalid payload)
Admin panel redesign:
- Rebuild frontend from mockup with Discord-style dark theme
- Stat cards, section cards, role badges, modal system, toast notifications
- All 7 sections: Dashboard, Users, Channels, Audit Log, Settings, Backups, Updates
- Modals replace confirm()/prompt() for all destructive actions
Live server logs (new):
- RingBuffer + MultiHandler tees slog to stdout AND in-memory buffer
- SSE endpoint at /admin/api/logs/stream streams logs in real-time
- Log viewer with level filters (DEBUG/INFO/WARN/ERROR), search,
auto-scroll, pause/resume, copy all, clear
- Color-coded lines by level, source categorization from file paths
Audit log improvements:
- Search filter (actor, action, target, detail)
- Action type dropdown filter
- Copy All and Export CSV buttons
- Instant client-side re-filtering
Console output:
- Switch from JSON to human-readable text format (slog.TextHandler)
- Move startup banner before init logs so it appears first
- CRIT-2: Prevent HTTP header injection in Content-Disposition via mime.FormatMediaType
- HIGH-1: Call hub.GracefulStop() on server shutdown to clean up WebSocket/voice state
- HIGH-2: Remove double send on serveErr channel that caused goroutine leak
- HIGH-3: Handle GetUserByID error after registration to prevent nil panic
- HIGH-4: Return nil,nil on sql.ErrNoRows in GetAttachmentByID (matching codebase convention)
- HIGH-5: Add voiceDone channel to bound RTP goroutine lifetime on voice teardown
- Fix handleVoiceICE to return VOICE_ERROR when no PC (consistent with other voice handlers)
- Update tests to match corrected behavior
Fix 70+ errcheck violations by adding explicit error discards (`_ =`)
for unchecked return values across test helpers and deferred Close()
calls. Remove unused `senderID` field from broadcastMsg and unused
`defaultCleanupMaxWindow` const. Apply De Morgan's law, remove empty
branch, and simplify redundant type declaration per staticcheck.
- Remove premature UpdateUserStatus("online") from REST login handler;
the WebSocket serve.go already sets "online" on actual WS connect
- Add ResetAllUserStatuses() called at server startup to clear stale
statuses from previous runs or crashes (alongside ClearAllVoiceStates)
- Scaffold Go module (github.com/owncord/server) with all package dirs
- config: koanf-based YAML loader with env var overrides, default generation
- db: pure-Go SQLite (modernc, no CGO), WAL mode, FK enforcement, full
15-table schema from SCHEMA.md including FTS5 and idempotent migrations
- auth/tls: ECDSA P-256 self-signed cert generation, LoadOrGenerate for
all 4 TLS modes (self_signed, acme, manual, off)
- api: chi router with request ID middleware, /health and /api/v1/info
- main: graceful shutdown (30s timeout), structured slog JSON logging
- Stubs for ws, storage, admin packages ready for Phase 2+
Test coverage: api 100%, auth 85.7%, db 82.4%, config 80.6%
Binary: chatserver.exe 12MB, GOOS=windows GOARCH=amd64
