Files
Maven/docs/evals/2026-08-10-repo-audit.md
T
claude c0f4074a5d Give the audit's open findings a home and a trigger (V-674)
Nineteen of the twenty findings were open, and they lived in an untracked
audit.md at the repo root that no next session would have read. The one that
is closed, the unauthenticated mavgpud proxy, went out as V-673.

The report is now a frozen measurement under docs/evals/, dated and never
edited again — including when a finding it names gets fixed. The live state
moved to docs/caveats/, one entry per limit, each carrying its Vikunja id and
the condition that makes it worth fixing. A caveat with no revisit trigger is
a complaint, so every entry has one. Closing a limit deletes its entry rather
than editing the measurement that found it.

Two directory indexes come with it. docs/CLAUDE.md states the tier rule the
repo already followed by convention: living docs corrected in place, evals
frozen by date, caveats deleted when fixed. docs/caveats/CLAUDE.md indexes the
nineteen by claim and severity, because an index of filenames adds nothing a
directory listing does not.

Tasks V-675 through V-693 carry the plans. The doc line and the tracker now
join in both directions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ESv8hqNPseYt1CnotZpqDz
2026-08-11 10:41:54 +04:00

22 KiB
Raw Blame History

Repository deep-audit report

Date: 2026-08-10 Revised: 2026-08-11, a verification pass over every cited line. Six claims were wrong as first written and are corrected in place. Two findings were added.

Frozen 2026-08-11, V-674. A dated measurement is never edited after the day, and that holds for a finding which later gets fixed. The live state of each one sits in docs/caveats/ with its task id. The fix sits in the subsystem's living doc under docs/. Read this file for the evidence, not for what is still true.

Read-only audit complete. I found 20 issue-specific candidates not represented by a matching Vikunja task: 3 High, 15 Medium, 2 Low. No code or Vikunja tasks were changed.

I compared all 366 tasks in Maven project 2. Existing items such as V-317, V-589, V-597, V-603, V-605, V-608, V-643 and V-647 were excluded except where a new finding is clearly separate.

1. Unhandled edge cases and silent failures

Empty STT responses suppress the local fallback

A — Location: internal/stt/http.go:85 accepts {} as a successful transcript and returns empty text at line 89. internal/stt/pair.go:143 only falls back when err != nil.

B — Severity: Medium. A malformed 200 OK from the workstation silently drops a voice turn instead of invoking mavsttd.

C — Proposed fix: Require nonblank transcript text and a valid confidence range; treat missing required fields as an error so Pair falls back. Bound the response decoder and add {}, {"text":""}, oversized-body and invalid-confidence tests.

Open-Meteo can report invented zero-degree weather

A — Location: internal/weather/openmeteo.go:47 uses value fields, while line 95 accepts {} and line 100 interprets it as WMO 0 and 0°C.

B — Severity: Medium. A valid JSON error/degraded response becomes plausible but false weather. This is separate from V-589, which covered HTTP status handling.

C — Proposed fix: Make current_weather and its required members pointer/nullable fields, validate presence and ranges, and cap both forecast and geocoder bodies.

Dialogue persistence errors disappear completely

A — Location: Corrupt rows are silently skipped at internal/dialogue/session.go:127; delete failures are discarded at line 191; marshal and save failures are swallowed at lines 201 and 209.

B — Severity: Medium. Restart continuity can silently vanish, while failed deletes can resurrect stale conversation state.

C — Proposed fix: Return or report persistence errors with session ID and operation; quarantine/delete corrupt rows; use bounded contexts instead of context.Background(). Keep current-turn availability, but expose the lost restart guarantee through logs/metrics.

2. Concurrency and race conditions

Reminder state transition is a read-then-write lost update

A — Location: internal/store/reminders.go:172 reads pending, then line 182 updates without checking the old state.

B — Severity: Medium now; High once V-622 adds cancellation surfaces. Concurrent fire/cancel operations can both succeed and the last writer wins. This invariant is not described in V-622.

C — Proposed fix: Use one conditional statement: UPDATE ... WHERE id=? AND status='pending'; inspect RowsAffected, then distinguish not-found from invalid transition. Add simultaneous fired/cancelled tests under -race.

TCP listener shutdown can leave Accept blocked forever

A — Location: internal/netaddr/netaddr.go:180 waits only on conns and errc. During close, line 201 may select the already-closed done branch without publishing the listener error; Close at line 225 does not wake Accept.

B — Severity: Medium. During close both errc and done are ready in acceptLoop's select and Go picks uniformly, so roughly one close in two strands a waiting Accept forever on a TCP seam.

C — Proposed fix: Add case <-l.done: return nil, net.ErrClosed to Accept, and make error/channel closure ownership explicit. Test an in-flight Accept concurrently with Close.

3. Data fetching: waterfalls and duplicated scans

Fact enrichment performs a serial 20-call network waterfall

A — Location: cmd/mavend/factenrichment.go:160 resolves each fact sequentially; line 223 makes the Nexus call. Each request can consume ten seconds at cmd/mavend/ecosystem.go:63.

B — Severity: Medium. A slow-but-reachable Nexus can hold one tick for roughly 20 × 10s, preventing the worker from observing its intended interval. V-647 only covered the duplicate queue scan.

C — Proposed fix: Prefer a Nexus batch-resolution endpoint. Otherwise use bounded concurrency, such as four workers, while preserving per-fact backoff and the 20-attempt ceiling.

A recall miss scans two whole tables

A — Location: The query source table runs embed, memory and notes in order at cmd/mavend/actions_query.go:161 through line 164. MemoryStore.Search scans every row of memory_vectors at internal/store/memory.go:82; a miss then calls the legacy notes query at cmd/mavend/actions_query.go:718, which decodes every note vector at internal/store/notes.go:63 and sorts the whole table at line 69.

B — Severity: Medium at scale. This is not an N+1. It is two full scans per missed recall. The memory scan is already cheap per losing row on purpose, costing one dot product read off the stored bytes with no []float32 materialized, so the duplicated decode cost is the notes path alone. Both recall widths are tiny (memoryRecallWidth 3, noteRecallWidth 5), so the work is in the scan, not the result set. V-581 is a generic sweep of this file, but does not identify this issue.

C — Proposed fix: Establish the invariant that every recallable note exists in memory_vectors, then remove the fallback. internal/store/backfill.go already rewrites note rows into the unified index, so the backfill exists; what is missing is proof that the two exclusion filters agree, since QueryNotes filters on notHisWordsSQL while Search filters on memory.NonRecallPrefix. Until they do, query only notes missing from the unified index and rank with the existing bounded top-K heap.

4. Dependency health and version pinning

Reachable published vulnerabilities in the pinned toolchain and x/text

A — Location: go.mod:3 and Makefile:7 pin Go 1.25.5; go.mod:25 pins x/text 0.14.0. Reachable traces include normalization at internal/router/onnxembedder.go:364, HTML rendering at cmd/mavweb/shell.go:154, email header decoding at internal/email/message.go:244, and reverse proxying at cmd/mavgpud/main.go:212.

B — Severity: High. govulncheck found 20 reachable advisories: one in x/text and 19 in the Go standard library, including template XSS, parser complexity/DoS and TLS issues. The official database says x/text before 0.39.0 can loop on invalid UTF-8; Go 1.25.12 contains the accumulated security corrections. See GO-2026-5970, GO-2026-4980, and the Go release history.

C — Proposed fix: Upgrade the vendored toolchain to at least 1.25.12, preferably current 1.26.5 after compatibility testing; upgrade x/text to at least 0.39.0/current 0.40.0; tidy and re-vendor. Add govulncheck ./... to the repository gate.

No dependency was three major versions behind. The remaining direct updates were minor/patch releases. .opencode's npm audit reported zero vulnerabilities and no peer conflicts.

5. Security exposure

WebAuthn enrollment is open and step-up state is process-global

A — Location: Registration endpoints have no existing-credential or bootstrap authorization at cmd/mavweb/webauthn.go:92 and line 103. RegisterBegin also answers GET, while RegisterFinish requires POST. A single server-wide session is created at cmd/mavweb/main.go:170, backed by one assertedAt timestamp at internal/webauthn/session.go:20. The voice WebSocket accepts every origin at cmd/mavweb/voiceproxy.go:48.

B — Severity: High, and scoped to enrollment and session binding. Assertion itself is sound. internal/webauthn/webauthn.go:221 requests userVerification: "required", and line 304 rejects a sign count that did not increase. What is broken is that any client past the reverse proxy can enroll its own key, and that after any successful assertion every client inherits the same five-minute step-up window. The wildcard WebSocket origin makes cross-site use easier. V-317 covers which routes are gated, and V-605 covers challenge-map growth, not enrollment or session binding.

C — Proposed fix: Permit first enrollment only through a local/one-time bootstrap ceremony; require an already-authenticated credential for subsequent enrollment. Bind step-up to a signed, HttpOnly, SameSite browser session and exact RP origin. Restrict WebSocket origins and add CSRF/origin validation to mutating routes.

mavgpud exposes an unauthenticated GPU/model proxy on the LAN

Closed 2026-08-11, V-673. The reasoning now lives in docs/offload.md, beside the rest of the workstation seam, and docs/deployment.md carries the operational line in the daemon table. This block is a pointer, not a second home: read those, not this.

  • Boundary and limits: cmd/mavgpud/auth.go, wired in cmd/mavgpud/main.go.
  • Client half: internal/llm/client.go, internal/llm/remote.go, internal/config/workstation.go, cmd/mavend/voicewire.go.
  • Deploy: token_file in deploy/mavgpud.json, MAVEN_GPU_TOKEN in deploy/telegram.env.example.
  • Commits: 95e7427, 1c13d22, 5596cdd, 9bb3425.

Deploy step, not yet done: write the token to /home/kami/.config/mavgpud.token on workpc and put the same value in MAVEN_GPU_TOKEN on homesrv, before restarting either side. mavgpud refuses to start without it, and a homesrv missing it falls back to the resident model.

Passkey credential persistence is not crash-atomic

A — Location: cmd/mavweb/credentials.go:36 serializes the full credential map and overwrites the live file directly with os.WriteFile at line 41.

B — Severity: Medium. A crash, disk-full event or interrupted write can corrupt every enrolled credential and prevent mavweb from starting.

C — Proposed fix: Write a 0600 temporary file in the same directory, fsync, rename atomically, then sync the directory. Preserve the last known-good file and test simulated write failures.

No new tracked hardcoded keys, raw user-concatenated SQL, eval, or shell execution of untrusted strings were found. The historical DB-key exposure is already covered by V-12.

6. Circular dependencies and layering

Domain packages depend directly on storage/wire DTOs

A — Location: Dialogue imports store and exposes store.DialogueSessionRow in its port at internal/dialogue/session.go:9 and line 75. The pure morning planner accepts store.Fact at internal/morning/plan.go:72. Auth policy imports IPC method and caller types at internal/auth/policy.go:8 and internal/auth/scope.go:33.

B — Severity: Low. There is no current Go import cycle, but domain changes are coupled to database and IPC schema changes.

C — Proposed fix: Make domain packages own their DTOs and ports—dialogue persistence records, morning evidence, auth operation/caller identity—and adapt them in store/IPC/cmd wiring.

No circular Go imports were found; compilation and go vet both succeeded.

7. Dead code and zombie endpoints

Eleven production symbols are unreachable

A — Location: deadcode found:

  • cmd/mavwaked/vad.go:244PCMToF32
  • cmd/mavwaked/vad.go:265AudioDuration
  • internal/crawl/watch.go:86Watcher.Watches
  • internal/phraser/confirm.go:137IsC
  • internal/phraser/plural.go:13CountWord
  • internal/phraser/eval/checks.go:80HisGender
  • internal/update/update.go:363WithClock
  • internal/voice/errors.go:72jsonMarshal
  • internal/voice/errors.go:73jsonUnmarshal
  • internal/webauthn/cbor.go:98cborValue.At
  • internal/worker/server.go:64Server.SetSynthesizer

Three of the eleven do not want deleting, and the 2026-08-11 pass checked each:

  • internal/phraser/eval/checks.go:80 HisGender is deliberately exposed and deliberately uncalled. The comment at line 72 ties the trio to V-399, and cmd/mavend/personaguard.go:94 states why this one is not run on a phrased message. Deleting it removes a documented seam.
  • cmd/mavwaked/vad.go:265 AudioDuration duplicates what internal/audio already computes. Call that instead of deleting the body.
  • internal/phraser/plural.go:13 CountWord is a one-line alias for say.CountWord, and every real caller already uses say directly. Safe to delete outright.

B — Severity: Low. They increase API and test surface, and some comments claim callers that no longer exist.

C — Proposed fix: Delete the genuinely obsolete symbols. Where one is an intended extension seam, add the actual caller and a contract test, or record why it stays. Add deadcode ./... with an explicit allowlist to the audit gate, since an unannotated list invites deleting the three above.

The repository history begins on 2026-07-03, so a six-month rotten-feature-flag check is not yet applicable. One zombie HTTP route does exist: /ws is wired at cmd/mavweb/main.go:235 and no shipped client reaches it, since the browser posts to /api/ptt. Section 8 carries the detail.

8. Performance hot paths and memory/resource leaks

Digest dedupe happens after paying the LLM cost

A — Location: cmd/mavend/tick_digest.go:147 calls PhraseNudge before EnqueueDigestEntry reports the dedupe at line 153. The else if deduped { continue } is the last statement in the loop body, so it changes nothing.

B — Severity: Medium. Every tick that continues suppressing the same rule can invoke the model again, contrary to the cache claim in the preceding comment.

C — Proposed fix: Check for a live pending entry by stable rule/candidate fingerprint before phrasing, or persist/cache the phrased result with a TTL. Add a test asserting one phraser call across repeated suppressed ticks.

PTT reads an unbounded body and neither server sets header or idle limits

A — Location: handlePTT performs an unlimited io.ReadAll at cmd/mavweb/voiceproxy.go:125. Mavweb and mavgpud construct servers without header or idle limits at cmd/mavweb/main.go:242 and cmd/mavgpud/main.go:148. Separately, handleWS never calls conn.SetReadLimit, so the dependency default of 32,768 bytes applies at vendor/github.com/coder/websocket/read.go:92, about one second of 16kHz mono PCM.

B — Severity: Medium for the HTTP side. A client can force unbounded body allocation or hold a connection open indefinitely. Low for the WebSocket read limit, because /ws has no caller: the browser client posts PCM to /api/ptt at cmd/mavweb/static/app.js:114, and /ws is wired at cmd/mavweb/main.go:235 but reached only from handlers_test.go. The 64MiB maxFrame at cmd/mavweb/voiceproxy.go:27 is not an unapplied declaration. It caps the mavend voice wire at line 194 and line 212, which is the "either direction" its comment names.

C — Proposed fix: Use http.MaxBytesReader for PTT and return 413 on overflow. Configure ReadHeaderTimeout, IdleTimeout and header limits on both servers. Decide /ws separately: either give it an audio-duration read limit and a client, or delete it. See section 7.

9. Error propagation and user feedback

Mavweb has no consistent, traceable error contract

A — Location: Some handlers expose raw internal errors, such as cmd/mavweb/tools.go:57, cmd/mavweb/routines.go:58 and cmd/mavweb/webauthn.go:122. Others return generic errors without a request/incident identifier, such as cmd/mavweb/facts.go:90. No HTTP request-ID middleware was found.

B — Severity: Medium. Raw errors can disclose implementation details, while generic errors cannot be correlated with the correct log entry.

C — Proposed fix: Add a central writeProblem/error-page helper with a stable error code and generated request ID; log the full wrapped error server-side and return only a sanitized message plus the ID. Carry the ID into IPC/ecosystem correlation where possible.

10. Configuration drift and environment assumptions

Committed absolute paths make builds and deployment host-specific

A — Location: go.mod:31 replaces Hexis with /home/kami/apps/hexis. start-maven.sh:11 hardcodes the Maven checkout and line 40 hardcodes the data directory. deploy/mavgpud.json:12 and line 35 contain workstation-specific model/Python paths.

B — Severity: Medium. Vendoring masks the go.mod problem for ordinary builds, but -mod=mod, tidy and fresh non-Kami checkouts fail. Deployment files cannot be reused safely on another host.

C — Proposed fix: Pin a real Hexis module revision; keep local replacement in an uncommitted go.work. Derive script root from the script location and make data paths configurable. Split mavgpud into a committed template plus host-local override.

Environment examples do not cover deployed variables

A — Location: Active config references MAVEN_STT_TOKEN at deploy/mavend.json:101, Compose references MAVEN_AMBIENT_TOKEN at docker-compose.yml:109, and the GPU service expects CW2_TOKEN at deploy/mavgpud.service:15. deploy/telegram.env.example:5 documents only Telegram and ntfy. The loader deliberately converts missing variables to empty settings at internal/config/config.go:333.

B — Severity: Medium. A fresh deployment can lose remote STT or ambient authentication and run on fallback behavior despite apparently valid config. It is not silent: internal/config/config.go:340 logs which variables were unset and states that whatever they configure is off. What is missing is a startup failure and an example file naming them.

C — Proposed fix: Maintain one canonical secret manifest/example covering every referenced variable, or service-specific examples with validation. Fail startup when an enabled integration lacks its required secret; permit empty variables only for explicitly disabled blocks.

No production/staging debug-mode or mock-gateway drift was found.

11. Declared invariants with no guard

CLAUDE.md names several rules as load-bearing. Two of them are enforced by nothing, which the first pass missed because it audited generic categories only.

heads_path may equal model_path and nothing objects

A — Location: cmd/mavend/voicewire.go:168 reads cfg.Voice.Embedder.HeadsPath and loads it without comparing it to the model path. The rule is stated at internal/config/voice.go:41, which says the heads graph is a fine-tuned COPY, and again in CLAUDE.md.

B — Severity: Medium. Pointing both keys at the same file degrades recall, because the routing heads then score with the same graph the resident e5-small uses. There is no error and no log line, so the failure looks like ordinary recall drift.

C — Proposed fix: Reject the config at load when heads_path equals model_path after path cleaning. A daemon that cannot route well should refuse to start rather than answer worse.

baselineGrammars mirrors buildRouter by hand

A — Location: internal/router/eval/eval_test.go:263 restates the stage 0 rule set in the daemon's order, and its own comment says so. claims_test.go:30, heads_test.go:76 and eval_test.go:221 all score against it. Nothing compares the two lists.

B — Severity: Medium. A grammar added to buildRouter and not to the fixture means every routing measurement scores a set nobody runs, which is the failure mode CLAUDE.md warns about by name.

C — Proposed fix: Export the grammar set from one place and have both buildRouter and the fixture consume it, or add a test that diffs the two by grammar name and fails on drift.

tokenizerRev and preRouteLadder were checked and need nothing. internal/router/onnxembedder.go:91 bakes the rev into the embedder key, so a bump changes the key and triggers re-embedding. cmd/mavend/voice.go:278 passes preRouteLadder to decision.Expect, so a missing rung is observable.

Cross-cutting subsystem candidates

The recurring findings suggest five reusable patterns:

  • A bounded, required-field-validating JSON client for STT, weather, ecosystem and model calls.
  • Authenticated remote-service middleware providing token checks, body limits, concurrency limits and correlation IDs.
  • Atomic state-transition helpers using conditional SQL and RowsAffected.
  • A uniform HTTP problem/error envelope.
  • A repository health gate combining staticcheck, govulncheck, deadcode and dependency audits.

Validation

  • make fmt-check and make vet passed. make audit passed too, but it is a git-grep inventory over loc, todo, stubs, docs, tests and gaps (scripts/audit.sh), not a static-analysis gate. Do not read it as one.
  • staticcheck, govulncheck, deadcode, tracked-secret and history scans and npm audit were run out of tree. None of the three Go analyzers is installed on this box or wired into any make target, which is the argument for section 4's proposed gate.
  • The advisory version numbers in section 4 could not be re-checked offline on 2026-08-11. deps/go/go/VERSION reads go1.25.5, built 2025-11-26, so the eight-month gap behind current supports the upgrade claim.
  • The first whole-tree race run failed once in cmd/mavend while analyzers were compiling concurrently; a fresh isolated go test -race -count=1 ./cmd/mavend passed in 100.6 seconds, so the transient result was not counted as a defect.
  • The pre-existing deploy/mavwaked.service modification and untracked deploy/asoundrc remained untouched.