Files
oikos/plans/done/2026-07-08-oikos-gaps-and-improvements.md
dtoro 104593de0c plans: review and archive all plans to done/
Architecture has changed drastically (hexagonal refactor, web client
extraction). Every active plan has been reviewed, annotated with
'Completed' or 'Won't do' status, and moved to plans/done/.

Completed (7): gaps-and-improvements, liveness-drift, gated-execution,
  nomos-code-review, codebase-cleanup, mascot-physics, backend-eval
Won't do (7): prometheus-lxc, control-room-webui, activity-gaps,
  activity-timeline, frontend-os-apps, haos-capability-gaps, arr-audit
2026-08-16 11:18:47 +02:00

255 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 2026-07-08 — Oikos gaps, broken things, and improvements
**Reviewed 2026-08-16 — Status: Completed** — many items resolved during 2026-07-1112; remaining gaps superseded by hexagonal refactor (v0.34.0).
**Status:** In Progress — audited 2026-07-11, re-audited 2026-07-12 for
drift from the `cmd/hermes``cmd/nomos` rename and later fixes. Done: A1
(approval FK bug), A3 (Hermes→Nomos help text), **Section C** (toy NLU /
silent-wrong-answer fallback — nomos now calls real `listTools()` and
routes unmatched queries to `/chat` instead of guessing, per
`cmd/nomos/main.go:444-465`), **D.5** (SOUL.md/actuator architecture
mismatch — `nomos/SOUL.md:21-22,40` now accurately documents SSH via the
`run` tool), D1 (`upsert_knowledge`), D4-partial (general `run` tool).
Still open: A2 (notifier flooding/dedup), A4 (`resolveHost` dead code), A5
(`queryRows` stringly-typed columns), A6 (stale `get_state_snapshot`
description), B1-B5 (enrollment auth, fake Infisical creds, `/query`
mesh-only auth unenforced, insecure host key checking, optional
`caller_pubkey`), D2/D3 (no `get_approval_status`/`list_pending_approvals`/
signal ack-resolve-mute tools), E-partial (Caddyfile placeholders still
present; tool count now 33, documented in AGENTS.md as of 2026-07-12).
2026-07-12 re-audit also refreshed every `cmd/hermes``cmd/nomos` and
`internal/mcp/server.go` line-number citation below (the file grew from 28
to 33 registered tools since 2026-07-11) — content/status of each finding
unchanged, only citations moved.
## Goal
Full-project review of Oikos from two vantage points — a user interacting
through Hermes, and an agent working through the MCP tool surface — with every
finding verified against source (file:line), plus a prioritized fix order.
This plan is the map; each numbered fix is small enough to land independently.
---
## A. Confirmed bugs (verified in source)
### A1. Approvals are never created — FK violation, errors swallowed (CRITICAL)
`createApproval` at `internal/mcp/server.go:970` inserts a fresh
`uuid.NewV7()` as `approvals.entity_id`, but
`migrations/003_operations.up.sql:41` declares
`entity_id UUID PRIMARY KEY REFERENCES entities(id)`. The INSERT always
violates the FK, and both `pool.Exec` errors are discarded.
Net effect: `request_execution` for `systemctl enable/disable` or
`apt_upgrade` marks the execution `pending_approval`
(`internal/mcp/server.go:311`, `:366`) and tells the agent it's queued, but no
approval row exists → the notifier never sends a Matrix alert → the execution
is orphaned forever. From the Hermes user's perspective, config mutations
silently dead-end while appearing accepted.
Contrast: `request_execution` (server.go:286) correctly creates a companion
`entities` row for the execution first — approvals just never got the same
treatment.
**Fix:**
- Preferred: migrate `approvals` to its own `id UUID PRIMARY KEY` (not FK'd to
`entities`), keeping `subject_entity_id` as the entity link. Update notifier
and `DecideApproval` queries accordingly.
- Alternative (no migration): create a companion `entities` row like
executions do.
- Either way: check and log every `Exec` error in `createApproval`, and verify
the `UPDATE executions SET approval_id = $2 WHERE entity_id = $1` column
semantics (`executions` is also keyed by `entity_id`).
### A2. Matrix message-flooding vectors (`internal/notifier/notifier.go`)
- Initial alert is guarded by `alert_sent_at` (notifier.go:103), but the guard
is written *after* the Matrix send (notifier.go:111). If the UPDATE fails
after a successful send, the ticker re-sends every cycle.
- No dedup of approvals by `(subject_entity_id, action, payload)`. Once A1 is
fixed, every retried `request_execution` mints a new approval → one Matrix
message each.
- `pollReactions` (notifier.go:118) issues one Matrix relations GET per
pending approval per 30s poll, uncapped — ignored approvals accumulate for
their 1h lifetime and multiply API calls. It also re-dispatches decisions
for approvals stuck `pending` (no "already acted" guard if the
DecideApproval call errors without flipping status).
**Fix:** mark-then-send (or transactional outbox) for `alert_sent_at`; upsert/
dedup open approvals on `(subject_entity_id, action, payload)`; cap + backoff
on reaction polling; guard against re-dispatching a decision already in
flight.
### A3. Hermes "help" is broken + dead code
`cmd/hermes/main.go:165` handles "what can you do"/"help" by calling
`client.callTool("tools/list", nil)` — a `tools/call` for a tool literally
named `tools/list`, which doesn't exist. The correct `listTools()` helper
(main.go:318) is dead code, never called. **Fix:** wire `listTools()` in.
### A4. `resolveHost` never returns a per-entity SSH user
`internal/mcp/server.go:1222` (was :943 — line moved) — the named return
`sshUser` is always `""`; the per-entity user branch is dead and everything
relies on `sshExec`'s global default fallback. **Fix:** read the SSH user
from entity attributes or delete the dead return to make the behavior
honest.
### A5. `queryRows` stringifies every column
`internal/mcp/server.go:1090` (was :861 — line moved) renders all values via
`fmt.Sprintf("%v", ...)`, so numbers, bools, timestamps, and JSON all reach
agents as strings. **Fix:** type-preserving serialization (pass through
pgx-native values into `json.Marshal`) — improves every read tool at once.
### A6. `get_state_snapshot` description is stale
`internal/mcp/server.go:863` (was :689 — line moved) still advertises "disk,
drift count" — columns removed in commit 3ea43ad. **Fix:** update the
description.
---
## B. Security gaps
### B1. Enrollment is unauthenticated, with a false comment
`internal/httpapi/server.go:111` (was :97) says "unauthenticated (IP-gated
in handler)" but `EnrollClient` (`internal/httpapi/impl.go:1166`, was
:1099) performs no IP check at all — the only gate is the target entity
being in state `planned`/`provisioning`. Caddy's `@enroll` matcher bypasses
Authentik. Anyone reaching `oikos.hubris.network` who knows (or guesses) a
planned slug receives that node's **age private key** in the HTTP response
body. Still open — line numbers only, substance unchanged.
**Fix:** enforce a real gate (mesh-CIDR check, one-time enrollment token
minted when the entity is created, or both), and stop returning the age
private key in the response — have the client fetch it from the secret store.
### B2. Fake Infisical credentials returned to enrollees
`internal/httpapi/impl.go:1260-1261` (was :1191-1192) returns
`"inf_client_"+uuid` / `"inf_secret_"+uuid` — random strings wired to
nothing. Enrolled clients hold credentials that authenticate against
nothing. Still open — line numbers only, substance unchanged.
**Fix:** implement `CreateMachineIdentity` in `internal/secrets/infisical.go`,
or return no credentials and document the manual step.
### B3. Nomos's `/query` has no auth
`nomos/config.yaml:9` (was `hermes/config.yaml:9`) sets `mesh_only: true`
but `cmd/nomos/main.go` (was `cmd/hermes/main.go`) never reads or enforces
it — it serves any caller on :8092, who can invoke `request_execution`.
Still open, now also tracked as C1 in
[2026-07-11-nomos-agent-code-review.md](2026-07-11-nomos-agent-code-review.md),
deferred by the operator. **Fix:** enforce mesh-CIDR (or bearer token) in
the handler; fail closed.
### B4. SSH host keys not verified
`ssh.InsecureIgnoreHostKey()` at `internal/mcp/server.go:1155` (was :920).
Still open — line number only, substance unchanged.
**Fix:** known_hosts pinning (keys are already inventory-managed per node).
### B5. `list_my_secrets` enumerates all node pubkeys
Without `caller_pubkey`, `internal/mcp/server.go:879-883` (was :709-720)
returns every entity that has an `age_pubkey`; nothing ties the caller to
what it may list. Still open — line numbers only, substance unchanged.
**Fix:** require `caller_pubkey` and scope results to the caller's
entitlements.
---
## C. User perspective (interacting via Hermes) — RESOLVED
**Resolved as of the Hermes→Nomos rewrite (verified 2026-07-12).** This
entire section described `cmd/hermes`, which no longer exists — Hermes was
renamed and rebuilt as `cmd/nomos`, a real LLM-backed agent loop, which is
exactly the recommendation below. `cmd/nomos/main.go:444-465` now calls the
real `listTools()` for "help"/"what can you do", and routes unmatched
queries to "natural language queries belong to `/chat`..." instead of
silently falling back to `get_health_summary`. Kept below for history —
original text unchanged.
- `routeQuery` NLU is hardcoded `strings.Contains`; `extractEntity`
(`cmd/hermes/main.go:173`) recognizes only 5 services (`authentik, caddy,
vaultwarden, gitea, immich`) plus `mac-mini`/`hubris`. Any other entity →
"no entity found", and *any* unmatched query silently falls back to
`get_health_summary` — wrong answers that look like answers.
- No conversation/session context; no follow-up capability.
- Config mutations appear accepted but silently dead-end (A1).
**Recommendation:** either make Hermes a real LLM-backed agent loop (Claude
API driving the 28 MCP tools) or explicitly scope it as a structured-tool
gateway: remove the toy NLU, make the fallback say "I don't understand this
query; here are the tools" (via the fixed `listTools()`), and document that
natural language belongs to the calling agent, not the gateway.
---
## D. Agent perspective (MCP tooling gaps)
28 tools are registered in `internal/mcp/server.go` (README says 15, AGENTS.md
says 21 — both stale). Missing capabilities:
1. **No knowledge write.** AGENTS.md tells agents to register knowledge via
`POST /api/v1/knowledge/{slug}`, but there is no MCP tool — MCP-only agents
cannot write back what they learn. Add `upsert_knowledge`.
2. **No entity/signal mutation.** Create/patch entity, state transitions, and
signal ack/resolve/mute all exist in REST (`internal/httpapi/impl.go`,
`phase3.go`) but not in MCP. Add at least signal ack/resolve/mute and a
policy-gated entity attribute patch.
3. **No approval visibility.** After `request_execution` returns
`pending_approval`, an agent has no way to check or reference the approval.
Add `get_approval_status` / `list_pending_approvals`.
4. Execution actions limited to `restart | systemctl | pct_exec |
apt_upgrade` — no deploy/rollback/config-edit path. Partially
superseded: the general `run` MCP tool (D4-partial, done) covers
arbitrary commands now; `request_execution`'s fixed enum is still there
for the specific actions it names (see
[2026-07-10-general-gated-execution.md](2026-07-10-general-gated-execution.md)).
5. **RESOLVED (verified 2026-07-12).** Architecture/doc mismatch:
`hermes/SOUL.md` claimed "no SSH access; all mutations flow through the
actuator", but the MCP server ran `restart`/`pct_exec` synchronously over
SSH from inside the api process. `nomos/SOUL.md:21-22,40` now accurately
documents SSH access via the policy-gated `run` tool — matches the
architecture the general-gated-execution plan built. No longer a
mismatch.
---
## E. Doc drift / housekeeping
- **RESOLVED (verified 2026-07-12):** Tool counts. README 15 / AGENTS.md 21
/ actual 28 was already stale by 2026-07-11 (registered tools grew to
33) — AGENTS.md now documents all 33 with the full catalog (2026-07-12).
- **Still open:** `compose/caddy/Caddyfile.oikos` retains literal
`<mac-mini-mesh-ip>` placeholders (this repo's copy is a reference only —
see [2026-07-12-wails-desktop-app.md](2026-07-12-wails-desktop-app.md)'s
"Plan review" — the real config lives in `dtoro/caddy-conf`).
- **RESOLVED:** `.agents/HERMES.md` renamed to `.agents/NOMOS.md`; the
duplicate-line bug itself is still present at `.agents/NOMOS.md:11` —
only the file citation was stale, the underlying nit is still open.
- **RESOLVED (verified 2026-07-12):** `plans/index.md` drift — the broken
link, TRMNL/Grimmory Active/Done mismatch, and missing `.hermes/plans/`
entries described here are no longer present in the current
`plans/index.md`; already fixed sometime after this plan was written.
- **RESOLVED (verified 2026-07-12):** `plans/2026-07-05-oikos-prometheus-lxc.md`
already self-corrected both the deleted-file references and the LXC 131
collision in its own 2026-07-08 changelog — this bullet describes a
pre-fix state.
---
## F. Prioritized fix order
1. **A1** approval FK + error handling — unblocks the entire
approval → Matrix → execution path.
2. **A2** notifier flooding guards — this branch's namesake.
3. **B1/B2** enrollment security + **B3** Hermes auth.
4. **D** MCP tool additions — approval status first, then knowledge write,
then signal ops.
5. **C** Hermes routing honesty + **A3-A6**, **B4/B5**, **E** drift cleanup.