From 806e21cb1333358c833d450f875a4b0cf43511f3 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 2 Jun 2026 00:17:53 +1000 Subject: [PATCH] docs: mark resolved items (auth hardening, crash-proofing, context allow-list, Yerin tools) Co-Authored-By: Claude Opus 4.8 --- docs/code-review-2026-06-01.md | 6 ++++ docs/security-sweep-2026-06-01.md | 21 +++++++++----- docs/yerin-security-agent.md | 47 +++++++++++++++---------------- 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/docs/code-review-2026-06-01.md b/docs/code-review-2026-06-01.md index 63755a5..6ddfaa4 100644 --- a/docs/code-review-2026-06-01.md +++ b/docs/code-review-2026-06-01.md @@ -6,6 +6,12 @@ audit pattern, clean tool registry, disciplined safe-DOM and SSRF handling. Item below are improvements, ordered by value. Severity ≠ urgency — most are "before scale / before more agents", not "broken now". +> **Resolved 2026-06-02:** #1 (pool error handler), #2 (upsert-arm guard), #3 +> (`verifyToken` O(1) selector+verifier), and #7 (`context` column allow-list) +> are **done** — see the security sweep doc + their tests. Remaining open: #4 +> (FTS index alignment — needs a prod `EXPLAIN`), #5 (dedupe bearer parsing), #6 +> (doc/code symbol drift). + ## Correctness / robustness 1. **`pool.js` has no error handler or statement timeout.** `lib/db/pool.js` diff --git a/docs/security-sweep-2026-06-01.md b/docs/security-sweep-2026-06-01.md index 9f180aa..67c4442 100644 --- a/docs/security-sweep-2026-06-01.md +++ b/docs/security-sweep-2026-06-01.md @@ -21,7 +21,15 @@ length mismatch). Both auth paths now use it. Tests: `tests/auth/safe_compare.te --- -## 🔧 HIGH — `verifyToken` does an O(n) bcrypt scan over every token +## ✅ FIXED (2026-06-02) — `verifyToken` O(n) bcrypt scan +Resolved via the selector+verifier split (migration `010_token_selector.sql`). +New tokens are `vk_.`: the non-secret `selector` is indexed +and locates exactly one row (O(1)); only the `verifier` is bcrypt-hashed. Legacy +NULL-selector tokens still verify via a fallback scan over the shrinking legacy +set. Dropped the useless `idx_agent_tokens_hash`. Tests: +`tests/repos/token_selector.test.js`. Original finding below. + +## 🔧 ~~HIGH~~ — `verifyToken` does an O(n) bcrypt scan over every token `lib/db/repos/agents.js::verifyToken` loads **all** non-revoked agent tokens and runs `bcrypt.compare` against each (cost factor 12 ≈ 250 ms/compare). @@ -62,12 +70,11 @@ server's own needs) rather than the whole environment. --- -## ℹī¸ LOW — `context` tool returns `SELECT *` of the active entity -`lib/ai/agent/tools/context.js` returns every column of the active row to the -agent. For `resources` that includes `monitoring`/`metadata` JSON, which may hold -connection hints or `vault_path` pointers. Not a secret-value leak today (the -resolver keeps values out of the row), but project a column allow-list before -Yerin (or any future agent) queries resource rows broadly. +## ✅ FIXED (2026-06-02) — `context` tool `SELECT *` of the active entity +`lib/ai/agent/tools/context.js` now projects a `SAFE_COLUMNS` allow-list for +`resources` (scalar fields only — never the `monitoring`/`metadata` JSON blobs). +Also added `resource` to the tool's `TABLE` map (it was previously unhandled). +Test in `tests/ai/agent/tools/context.test.js`. --- diff --git a/docs/yerin-security-agent.md b/docs/yerin-security-agent.md index bad59cd..42370a4 100644 --- a/docs/yerin-security-agent.md +++ b/docs/yerin-security-agent.md @@ -20,7 +20,7 @@ audit layer's write-time redaction. `agent_inventory` deliberately never does `SELECT *`. -## Built this pass (TDD, on `main`, not deployed) +## Built (TDD, on `main`, not deployed) — 5 tools `lib/ai/agent/tools/security/` + `securityRegistry`, tests in `tests/ai/security_tools.test.js`: @@ -28,19 +28,13 @@ | Tool | What it answers | |------|-----------------| | `audit_log` | "Who did what, newest first?" Filter by `actor_kind` / `actor_id`. Reads the redacted audit trail. Capped at 200. | -| `agent_inventory` | "Which agents exist and what is each allowed to do?" Returns id/slug/name/kind/model + capabilities + scopes. Never returns token material. | +| `agent_inventory` | "Which agents exist and what is each allowed to do?" id/slug/name/kind/model + capabilities + scopes. No token material. | +| `pending_review` | The queue of agent-proposed (suggest-tier) changes awaiting approval — where injected/misbehaving intent surfaces. | +| `resource_exposure` | Attack-surface inventory: every resource's host/url/status across spaces. No `monitoring`/`metadata` blobs, no credentials. | +| `token_audit` | Agent tokens with label/last_used/revoked_at (never the hash) — spot stale/unused credentials. | ## Roadmap — tools to add next (designed, not yet built) -Each is read-only and maps to an existing repo, so each is a small TDD task: - -- **`pending_review`** — list `pending_changes` awaiting approval (the queue of - agent-proposed mutations). Wraps `pendingChanges.listPending`. Security-relevant - because it's exactly where a prompt-injected agent's intent surfaces. -- **`resource_exposure`** — inventory of resources with `url`/`host`/`status` - (attack surface / what's reachable). Wraps `resources.listBySpace` across spaces. -- **`token_audit`** — agent tokens with `label`, `last_used`, `revoked_at` - (NOT the hash) to spot stale/unused credentials. Needs a small repo read. - **`recent_captures`** — newly ingested refs/source_docs (untrusted external content entering the system), so Yerin can flag suspicious inbound material. @@ -50,21 +44,24 @@ Stretch (needs new plumbing, your call): - **Active probes** (e.g. "is this CT's admin port exposed?") would require network egress — must go through `safeFetch` and be owner-gated. Defer. -## Wiring Yerin up (the remaining integration steps — left for you) +## Wiring Yerin up -These touch live agent seeding / MCP config, so they're documented rather than -done unsupervised: - -1. **Seed the agent.** A migration (010) inserting a `yerin` agent row with - `capabilities = {"read": true}`, `kind: 'claude'` (or `'ollama'` if you want - her cheap/local), and a persona prompt. Mirror migration 007's companion seed. -2. **Expose `securityRegistry` over MCP.** `lib/mcp/companion-stdio.js` currently - hardcodes `companionRegistry`. Parameterise it (env `VOID_TOOL_REGISTRY=security` - selects the registry) so a Yerin session spawns with her tools. ~10 lines. -3. **A Yerin entry point.** Either a route (`POST /api/security/ask`) reusing the - `claude_cli` driver with Yerin's persona + `securityRegistry`, or a scheduled - cron that runs a standing "anything suspicious in the last 24h?" pass and files - the result as a Sacred Valley card (ties into Plan 6). +1. ✅ **Seed the agent.** Migration `011_yerin.sql` inserts a `yerin` agent — + `capabilities {"read":true,"suggest":false,"write":false}`, `kind:'claude'`, + `model:NULL` (server default; flip to a local Ollama model anytime by setting + `agents.model`). Read-only by design. +2. ✅ **Expose `securityRegistry` over MCP.** `lib/mcp/companion-stdio.js` now + selects the registry from `VOID_TOOL_REGISTRY` (`security` → Yerin's tools; + default → companion). Test: `tests/mcp/registry_select.test.js`. +3. âŦœ **A Yerin entry point — LEFT FOR YOU** (it's a new API/UX surface; deserves + your attention, not an unsupervised guess). Two shapes: + - A route (`POST /api/security/ask`) reusing the `claude_cli` driver with + Yerin's persona + an MCP config that sets `VOID_TOOL_REGISTRY=security`. + Mirror `lib/api/routes/companion.js` (SSE, conversation persistence). + - Or a scheduled cron: a standing "anything suspicious in the last 24h?" pass + that files its result as a Sacred Valley card (ties into Plan 6). + - Either way she'll need a **persona prompt** (the Cradle Yerin voice + + security-analyst framing) — worth writing together. ## Adjacent agent roles (so the roster stays coherent)