docs(audit): record D9 LogAudit best-effort error-handling policy

Add decision D9 (dated 2026-07-20) to the audit decisions doc capturing the
maintainer-approved policy: best-effort audit writes, never silently
discarded, routed through db.WriteAudit. Mark carried-over finding #10 in
docs/audit-2026-07-19.md as RESOLVED with the helper adoption.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-20 10:48:13 +02:00
co-authored by Claude Fable 5
parent b60bc8d04b
commit 4e27f95333
2 changed files with 3 additions and 2 deletions
+1 -1
View File
@@ -53,7 +53,7 @@ audit's table; details stay in [audit-2026-04-07.md](audit-2026-04-07.md).
| 6 | HIGH | `Server/store/` untested | **RESOLVED 2026-07-19 (D3)** — the `store/` package is deleted rather than tested. `SQLiteStore` was a pure pass-through to `*db.DB`; its event/plugin methods moved into `db` (`event_queries.go`, `plugin_queries.go`). Consumers now depend on narrow interfaces `*db.DB` satisfies (`service.Store`, `ws.EventStore`, `plugin.PluginStore`), and the former `MemStore`-based unit tests run against a real in-memory SQLite `db` — so the code paths that were untested through the seam are now exercised directly |
| 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) |
| 10 | MEDIUM | Audit-trail write failures silently ignored | **RESOLVED 2026-07-20 (D9)** — best-effort audit writes are kept as the convention, but no longer silently discarded: every `LogAudit` call site now routes through the shared `db.WriteAudit` helper (`db/audit.go`), which logs a failed write with actor/action/target context without failing the request. All ~26 call sites across admin/api/ws/service converted from `_ = LogAudit(...)`; pinned by `db/audit_test.go` |
| 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 -1
View File
@@ -2,7 +2,7 @@
**Date decided:** 2026-07-19
**Decided by:** J3vb
**Status:** decisions recorded; greenlit items (D4, D7, D8) implemented 2026-07-19 — see per-row Status
**Status:** decisions recorded; greenlit items (D4, D7, D8) implemented 2026-07-19; D9 (repo-wide `LogAudit` policy, flagged by the D8 note) decided + implemented 2026-07-20 — see per-row Status
**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
@@ -22,6 +22,7 @@ here (and the audit's closure table) as items land.
| 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 001015, 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 | Repo-wide `LogAudit` error-handling policy (the open question the D8 note flagged) | backlog §6 / D8 | **Decided 2026-07-20:** audit writes stay **best-effort** — a `LogAudit` failure must never fail or abort the request — but a failed write must never be silently discarded either. Every call site routes through one small shared helper that logs the failure with request context (actor/action/target). Not a blanket "make audit failures fatal": the request path is unchanged, only the silent `_ =` discard is removed. | **Implemented 2026-07-20:** `db.WriteAudit(auditor, actor, action, targetType, targetID, detail)` helper added in `db/audit.go` (structural `Auditor` interface — both `*db.DB` and the service-layer `Store` satisfy it, so api/admin/ws/service all reach it without an import cycle). All ~26 `LogAudit` call sites converted from `_ = LogAudit(...)` (and the two backup handlers' inline `if err` blocks) to `db.WriteAudit`; the `detail` string is deliberately not logged (may be sensitive). Pinned by `db/audit_test.go`: failure is logged and never propagated, success logs nothing, detail never leaks. |
## Suggested sequencing