plans: add oikos gaps review and control-room webui plans
Documents a full-project review (confirmed bugs, security gaps, user- and agent-perspective gaps) and a realtime control-room web UI plan, per prior codebase exploration on this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
202
plans/2026-07-08-oikos-gaps-and-improvements.md
Normal file
202
plans/2026-07-08-oikos-gaps-and-improvements.md
Normal file
@@ -0,0 +1,202 @@
|
||||
# 2026-07-08 — Oikos gaps, broken things, and improvements
|
||||
|
||||
**Status:** Planned
|
||||
|
||||
## 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:943` — 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:861` 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:689` 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:97` says "unauthenticated (IP-gated in handler)"
|
||||
but `EnrollClient` (`internal/httpapi/impl.go: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.
|
||||
|
||||
**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:1191-1192` returns `"inf_client_"+uuid` /
|
||||
`"inf_secret_"+uuid` — random strings wired to nothing. Enrolled clients hold
|
||||
credentials that authenticate against nothing.
|
||||
**Fix:** implement `CreateMachineIdentity` in `internal/secrets/infisical.go`,
|
||||
or return no credentials and document the manual step.
|
||||
|
||||
### B3. Hermes `/query` has no auth
|
||||
|
||||
`hermes/config.yaml:9` sets `mesh_only: true` but `cmd/hermes/main.go` never
|
||||
reads or enforces it — it serves any caller on :8092, who can invoke
|
||||
`request_execution`. **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:920`.
|
||||
**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:709-720` returns every entity
|
||||
that has an `age_pubkey`; nothing ties the caller to what it may list.
|
||||
**Fix:** require `caller_pubkey` and scope results to the caller's
|
||||
entitlements.
|
||||
|
||||
---
|
||||
|
||||
## C. User perspective (interacting via Hermes)
|
||||
|
||||
- `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.
|
||||
5. Architecture/doc mismatch: `hermes/SOUL.md` claims "no SSH access; all
|
||||
mutations flow through the actuator", but the MCP server runs
|
||||
`restart`/`pct_exec` synchronously over SSH from inside the api process
|
||||
(`sshExec`, server.go:902). Align docs or move execution to the actuator.
|
||||
|
||||
---
|
||||
|
||||
## E. Doc drift / housekeeping
|
||||
|
||||
- Tool counts: README 15 / AGENTS.md 21 / actual 28 — regenerate from
|
||||
`internal/mcp/server.go` (consider a doc-gen make target).
|
||||
- `compose/caddy/Caddyfile.oikos` retains literal `<mac-mini-mesh-ip>`
|
||||
placeholders in all three vhosts.
|
||||
- `.agents/HERMES.md` lists "`inventory.yaml`, `inventory.yaml`" (duplicate).
|
||||
- `plans/index.md` drift: fix-MCP-tools row sat in Active with a broken link
|
||||
after the file moved to `done/` (fixed alongside this plan); TRMNL listed
|
||||
active though in `done/`; Grimmory header says `in-progress` though in
|
||||
`done/`; `.hermes/plans/` (7 executed plans) missing from disk.
|
||||
- `plans/2026-07-05-oikos-prometheus-lxc.md` (~0% done) references deleted
|
||||
`oikos/scheduler.py` and `bin/homelab`; LXC 131 collision unresolved.
|
||||
|
||||
---
|
||||
|
||||
## 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.
|
||||
Reference in New Issue
Block a user