# Audit 2026-07-19 — Maintainer Decisions

> **Status (verified 2026-08-04): Shipped (decision record).** All 13
> decisions carry an Implemented status below; spot-verified against the code
> (D2 `db/dbgen` wiring, D9 `VisibleChannelIDs`, D13 `HasServerPerm`).

**Date decided:** 2026-07-19 (D1–D8); 2026-07-20 (D9–D12); 2026-07-21 (D13)
**Decided by:** J3vb
**Status:** decisions recorded; greenlit items (D4, D7, D8) implemented 2026-07-19 — see per-row Status. **2026-07-20:** backlog items 3 and 11 (D9, D10) implemented — channel-visibility unified through `permissions.Checker`; V2 dispatch migration finished and V1 deleted. **2026-07-20 (P3):** the five plugin CRITICALs carried over from audit-2026-04-07 dispositioned (D11) — four closed, one accepted as residual risk, which keeps plugins default-disabled at the beta gate. **2026-07-23:** D13 implemented — server-scoped permission rule unified in `permissions.HasServerPerm`; override-fetch errors fail closed instead of dropping (and caching the loss of) every channel `deny`; the fifth D9 site routed through the `Checker`.
**Source:** decision points raised by [docs/audit-2026-07-19.md](../audit-2026-07-19.md)

This document records the maintainer's answers to the open decision points from
the 2026-07-19 architectural audit, so follow-up PRs can implement against a
settled direction instead of re-litigating it. Update the **Status** column
here (and the audit's closure table) as items land.

## Decisions

| # | Decision point | Audit ID | Decision | Status |
|---|----------------|----------|----------|--------|
| D1 | `announcement` channel type (documented + offered by admin API, rejected by DB triggers) | A-2026-07-01 | **Implement end-to-end**: migration to allow the type, posting-permission semantics, admin support, client rendering, spec updates. Not a doc-strip — this becomes a real feature. | **Implemented 2026-07-19**: migration 016 allows `announcement`; posting requires MANAGE_MESSAGES (readable like text); admin already offered it; client renders a megaphone icon + unread counts; specs updated. |
| D2 | Data-layer direction (raw SQL vs dead sqlc `db/dbgen` vs `store.Store`) | A-2026-07-05 / A-2026-07-06 | **Adopt sqlc for real**: wire `db.DB` method bodies to the generated `dbgen` queries so sqlc becomes the actual, type-checked query layer. The `sqlc-verify` CI job stays and starts earning its keep. | **Largely done 2026-07-19**: `dbgen.Queries` wired into `db.DB`; 97 methods across all domains delegate to sqlc (no longer dead code). ~43 raw calls remain by design (variable IN, FTS, multi-statement tx, PRAGMA/VACUUM) — tracked in [sqlc-adoption.md](sqlc-adoption.md). |
| D3 | Fate of `Server/store/` (untested abstraction seam) | prior audit #6 | **Remove `store/`** via interface segregation: delete `SQLiteStore` + `MemStore` + the `store` package; port the event/plugin methods into `db`; each consumer depends on a small interface `*db.DB` satisfies. | **Implemented 2026-07-19**: the `store/` package is deleted. Event and plugin methods moved into `db` (`event_queries.go`, `plugin_queries.go`); consumers depend on narrow interfaces `*db.DB` satisfies (`service.Store`, `ws.EventStore`, `plugin.PluginStore`). All service/ws/plugin/api tests now run against a real in-memory SQLite `db` with seed helpers; fault-injection tests embed a real `*db.DB` and override the one method under test. Full server suite + `sqlc-verify` green. |
| D4 | Protocol constants sync (`message_types.go` / `protocolTypes.ts` claim a nonexistent `docs/protocol-schema.json`) | A-2026-07-08 | **Create real codegen**: commit an actual `protocol-schema.json` plus a generator that emits the Go and TS constant files (and, ideally, protocol.md's message table), making the "single source of truth" comment true. | **Implemented 2026-07-19**: `docs/protocol-schema.json` + `Server/scripts/genprotocol` + `make protocol-generate`/`protocol-verify` + CI gate. protocol.md table generation deferred to D7. |
| D5 | Client HTTP TLS gap (`allowSelfSigned: true`, no TOFU pinning on the REST path) | A-2026-07-02 | **Next security work**: build the TOFU HTTP proxy in Rust (mirroring `ws_proxy.rs`) as the next security task — highest-priority security item. | **Implemented 2026-07-19** — `src-tauri/src/http_proxy.rs` (per-host loopback TCP→TLS tunnels, shared TOFU cert store + `cert-tofu` events) + `src/lib/httpProxy.ts`; REST/health/attachments routed through it; `acceptInvalidCerts`/`allowSelfSigned` and the `dangerous-settings` feature removed. See [http-tofu-proxy.md](http-tofu-proxy.md). |
| D6 | Abandoned SolidJS beachhead + stale `docs/client-architecture.md` | A-2026-07-12 | **Delete it all**: remove `src/components/solid/`, `solidMount`/`solidAdapter`, `vite-plugin-solid`, and Solid test deps; retire `client-architecture.md` in favor of [docs/architecture/client.md](../architecture/client.md). | **Implemented 2026-07-19** — solid/ dir, solidMount/solidAdapter, setup-solid tests, vite-plugin-solid, jsx tsconfig settings, and solid-js/@solidjs deps all removed; client-architecture.md is now a pointer. |
| D7 | Spec refresh strategy for api.md / protocol.md / schema.md | A-2026-07-03 | **One refresh PR first**, using the audit's §2 conformance matrix as the checklist; afterwards specs are kept current per-PR (see the maintenance rule in [docs/architecture/README.md](../architecture/README.md)). Announcement channels (D1) later update the *fresh* specs. | **Implemented 2026-07-19** — all three specs refreshed against the code (incl. E2EE protocol section, migrations 001–015, profile/blocks/plugin-admin endpoints); reference tables now point at `protocol-schema.json`. |
| D8 | What to implement first | backlog §6 | **Greenlit now: Protocol codegen (D4) + the quick-wins batch** — `LogAudit` error handling (`admin/handlers_backup.go`), contradictory upload `Cache-Control` (`upload_handler.go`), hub inline settings SQL through the data layer (`ws/hub.go`), Hub constructor cleanup (required collaborators into `NewHub`). | **Implemented 2026-07-19** (all four quick wins + D4). Hub cleanup shipped as: race fix — `eventPersister`/`eventStore`/`pluginSink` are now atomic (they were plain fields written by `main.go` after `NewRouter` had already started `Run`); remaining pre-Run setters now reject late calls with an error log instead of racing silently. Note discovered during the work: the discarded-`LogAudit` pattern is repo-wide (23 call sites) — the two tracker-flagged backup handlers are fixed; whether best-effort audit writes stay the convention elsewhere needs a policy decision. |
| D9 | Channel-visibility unification (rule duplicated across ~4 "must mirror" sites) | A-2026-07-07 / backlog 3 | **Greenlit 2026-07-20 — implement**: funnel all four sites through the existing `permissions.Checker` predicate + one filter helper; add a REST/WS agreement test. See [channel-visibility-unification.md](channel-visibility-unification.md). | **Implemented 2026-07-20** — `permissions.Checker.VisibleChannelIDs` + `ChannelRef`; `ListVisibleChannels`, `buildReady`, `computeAllowedChannels` delegate; `RefreshChannelVisibility` uses `HasChannelPerm`. REST/WS agreement test asserts all three sites yield the identical non-DM set. |
| D10 | Finish the V2 dispatch migration; delete V1 | A-2026-07-09 / backlog 11 | **Greenlit 2026-07-20 — implement**: port the 3 remaining V1 types (`chat_command`, `voice_join`, `voice_leave`) to V2, then delete the V1 registry + fallback path. Server-internal only, no wire change. See [v2-dispatch-migration.md](v2-dispatch-migration.md). | **Implemented 2026-07-20** — the 3 types ported to typed V2 handlers (voice join/leave hand off to the hub routines via new `Result.JoinVoice`/`LeaveVoice` appliers); V1 registry + `handleMessage` fallback deleted; a constructor↔handler parity guard test locks it shut. No wire change. |
| D11 | Disposition of the five plugin CRITICALs from audit-2026-04-07 (§1 carried-over row) | prior #1–#5 | **Close what the code already closes; fix the one cheap real gap; accept the one that hardening cannot fix.** Verified each against `Server/plugin/` rather than the tracker: #1 (no `invokeCommand` timeout) closed by PR #1182 — per-call CPU budget with a 100 ms floor plus `WithCloseOnContextDone` and lazy re-instantiation so an overrun does not brick the plugin. #2 (storage key isolation) closed as structural — the namespace is the caller's `Instance.ID` and `plugin_kv PRIMARY KEY (plugin_id, key)`; no parameter exists by which a plugin could name another's namespace, so the finding's premise was wrong. #3 (per-command ACL) was a **real gap** and is fixed here: the manifest gains a `commands` block and `RegisterCommand` refuses undeclared names, so `list_commands` can no longer widen a plugin's command surface behind the admin's back. #4 (event rate limit) closed because no guest code executes on the event path — precisely: `EventSink.Dispatch` has exactly one caller outside the plugin package's tests (`Server/ws/hub.go:1034`, on every broadcast when plugins are enabled, on the hub goroutine under `seqMu`), but its loop body invokes no guest code and no production code calls `EventSink.Subscribe`, so the subscriber set is always empty. Rather than build a limiter for guest calls that do not happen, the requirement is recorded as a SECURITY GATE comment on `Dispatch` and `Subscribe` — the exact places someone would wire delivery — including the warning that the hot call site already exists and sits under the hub's `seqMu`. #5 (HTTP exfiltration to an allowlisted host) **stays open as accepted residual risk** — an allowlisted host is by definition permitted, so closing it needs egress content policy and per-plugin allowlists (a runtime redesign, ~1–2 weeks), explicitly out of scope for P3. | **Implemented 2026-07-20** — closure tables in [audit-2026-04-07.md](../audit-2026-04-07.md) and the §1 row of [audit-2026-07-19.md](../audit-2026-07-19.md) updated; manifest `commands` ACL + key-size cap + five pinning tests landed (`Server/plugin/audit_closure_test.go`). Because #5 remains open, the standing rule fires as written: **plugins ship default-disabled at the beta gate** — re-verified in `config.DefaultConfig()` (`Plugins.Enabled: false`, empty `HTTPAllowlist`). |
| D12 | Who supplies the GIF (Klipy) API key | P3 item 1 / A-2026-07-02 family | **Decided 2026-07-20 — per-operator key, feature default-off.** The key previously shipped inside the client bundle via `VITE_KLIPY_API_KEY`. That is not a sharing arrangement but a disclosure: Vite inlines the value verbatim, so anyone who downloaded the client could extract the maintainer's key and use it for any purpose, with the maintainer carrying the quota, abuse and terms-of-service exposure. The key is now server-side only (`gif.api_key` / `OWNCORD_GIF_API_KEY`). **Alternatives considered and rejected:** a project-hosted proxy holding the maintainer's key (preserves zero-config GIFs and keeps revoke/rate-limit control, but introduces a hard central dependency into a self-hosted product, puts every server's search queries through maintainer infrastructure, and leaves the maintainer paying the quota), and a hybrid falling back to that proxy when unconfigured (same objections, opt-out only). **Consequence accepted:** each operator requests their own key at partner.klipy.com; fresh installs have GIFs off and the client shows "GIFs are not enabled on this server". Discoverability is handled in the README feature list and a quick-start section rather than by defaulting the feature on. | **Implemented 2026-07-20** — server proxy + default-off contract in #1198; `VITE_KLIPY_API_KEY` deleted from source and from all three release build jobs. Old key rotation is a maintainer action, sequenced after the new path is verified working. |
| D13 | Server-wide permission rule hand-rolled outside `permissions`; channel `deny` dropped — and cached — on override-fetch error | A-2026-07-16 | **Greenlit 2026-07-21 — implement**: add `permissions.HasServerPerm` (admin bypass OR all-of bit test, four lines, no DB) and collapse `RequirePermission` + `ModerationService.requireBanPermission` onto it; fail closed at both override-fetch sites; delegate `PermissionService.HasChannelPerm` and `GetAccessibleChannelIDs` to the `Checker`. Explicitly **not** making `RequirePermission` channel-aware — neither of its routes has a channel id, and a per-channel allow must never open a server-wide gate. Auth-route direct-db sweep (backlog row 12 / A-2026-07-06) deferred to a future D14; row 12 unchanged by this work. See [permission-middleware-consolidation.md](permission-middleware-consolidation.md). | **Implemented 2026-07-23** — `HasServerPerm` owns the rule (multi-bit masks now all-of); both fetch sites fail closed with `slog.Error` (nothing cached on error, so the next request retries); fifth D9 site (`GetAccessibleChannelIDs`) routes through `VisibleChannelIDs`; `AuthMiddleware` gains the dangling-role nil guard (401). Locked by failing-first deny tests plus 403 locks on both `RequirePermission` routes. |

## Suggested sequencing

1. **Now (greenlit):**
   - Quick-wins batch (small, independent, low-risk).
   - Protocol codegen (D4) — also unblocks the protocol.md portion of D7.
2. **Next:**
   - Spec refresh PR (D7) — after D4 so protocol.md tables can be generated.
   - Client HTTP TOFU proxy (D5) — next security work.
   - Solid cleanup (D6) — small, can slot in anywhere.
3. **Then (larger, sequenced together):**
   - sqlc adoption (D2), followed by `store/` removal (D3) once queries are
     type-checked — doing D3 after D2 avoids rewiring services twice.
   - Announcement channels (D1) — after the spec refresh so it patches fresh
     specs; needs its own small design note (permission semantics: who can
     post vs read).

## Explicitly not decided here

- ~~Channel-visibility unification (audit backlog 3) and finishing the V2
  dispatch migration (backlog 11) were not greenlit yet.~~ **Greenlit 2026-07-20
  (D9, D10)** — design notes written; see the rows above.
- Client unit suite triage to green/blocking (A-2026-07-04) remains tracked in
  the audit; no scheduling decision was taken.
