From 925cb0d7d61088b351da8c212199793591738e2e Mon Sep 17 00:00:00 2001 From: root Date: Wed, 3 Jun 2026 07:54:57 +1000 Subject: [PATCH] =?UTF-8?q?chore:=202.0.0-alpha.9=20=E2=80=94=20security?= =?UTF-8?q?=20&=20correctness=20hardening=20(Void=203.0=20quick=20wins)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Q3: prod void DB role NOSUPERUSER (vector marked trusted; deploy/README documents it) - Q4: buildChildEnv allow-list for the claude subprocess (no OWNER_TOKEN/DATABASE_URL/secrets leak) - Q5: pending-change approve claims-before-applying + reopens on failure (no re-approvable dup) - Q6: /capture/upload validates space_id (UUID+existence); pg pool statement_timeout 30s - Q9: disabled failing syncoid-donatello timer on Z Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 8 +++++++ deploy/README.md | 13 +++++++++++ lib/ai/claude_cli.js | 36 ++++++++++++++++++++++++++----- lib/api/routes/capture.js | 8 +++++-- lib/api/routes/pending_changes.js | 11 +++++++++- lib/db/pool.js | 5 ++++- lib/db/repos/pending_changes.js | 9 ++++++++ package.json | 2 +- server.js | 2 +- tests/ai/claude_cli_env.test.js | 36 +++++++++++++++++++++++++++++++ tests/api/capture.test.js | 14 ++++++++++++ tests/api/pending_changes.test.js | 17 +++++++++++++++ 12 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 tests/ai/claude_cli_env.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index e159069..908c25e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to Void 2.0 are documented here. Format: [Keep a Changelog](https://keepachangelog.com). +## 2.0.0-alpha.9 — Hardening pass (Void 3.0 quick wins) +- Security: prod `void` DB role revoked SUPERUSER (CT 310; `vector` marked trusted so the test harness still creates it as non-superuser). An app-process compromise no longer escalates to full-cluster compromise. +- Security: the `claude` companion subprocess now gets an explicit env allow-list (`buildChildEnv`) instead of the full `process.env` — `OWNER_TOKEN`/`DATABASE_URL`/Karakeep/ANTHROPIC secrets no longer reach the CLI. MCP tools are unaffected (they get DB env via the explicit `--mcp-config`). +- Correctness: pending-change **approve** now claims the change (atomic `WHERE status='pending'`) *before* applying, and reopens it on apply failure — eliminates the re-approvable duplicate after a crash. +- Hardening: `/api/capture/upload` validates `space_id` (UUID + existence); pg pool gets a 30s `statement_timeout`. +- Ops: disabled the failing `syncoid-donatello` timer on Z (pools out pending parts). +- Deferred: per-space tag uniqueness needs a `space_id` column on `tags` → folded into the polymorphic-`space_id` project. + ## 2.0.0-alpha.8 — Sacred Valley (Plan 6) - Two-band #/sacred-valley dashboard: draggable data cards (clock, weather, host-perf, speedtest, jobs, inbox, search) with server-persisted layout (custom CSS-grid reorder, no resize). - Little Blue Health band: config service registry, 60s pg-boss health checks, grouped status tiles, locally-cached service icons (no CDN leak). diff --git a/deploy/README.md b/deploy/README.md index 7dc4dd3..25c1259 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -1,5 +1,18 @@ # Deploy notes — Void 2.0 +## DB role posture (CT 310 — `void2-db`, alpha-9+) +- The `void` DB role is **NOSUPERUSER** (least privilege). It owns the `void` + `void_test` + databases and the `public` schema, so it can run all migrations and the test-harness + `resetDb` without superuser. +- The `vector` (pgvector) extension was marked **trusted** so the non-superuser `void` role + can `CREATE EXTENSION vector` (needed by `tests/helpers/db.js` on each reset): + ``` + echo 'trusted = true' >> /usr/share/postgresql/16/extension/vector.control + ``` + **⚠ Re-apply this after any pgvector package upgrade** (the package may overwrite the + control file). `pgcrypto` ships trusted already. +- Revert (emergency): as `postgres` on CT 310, `ALTER ROLE void SUPERUSER;`. + ## App deploy (CT 311 — `void2-app`) One-time setup on the target host: diff --git a/lib/ai/claude_cli.js b/lib/ai/claude_cli.js index 166c502..1429efb 100644 --- a/lib/ai/claude_cli.js +++ b/lib/ai/claude_cli.js @@ -47,6 +47,35 @@ import { spawn } from 'child_process'; import { createInterface } from 'readline'; +// Benign, non-secret vars the `claude` CLI legitimately needs in its child env. +// HOME is added separately (subscription creds live at ~/.claude). Everything +// else — OWNER_TOKEN, DATABASE_URL, KARAKEEP_*, ANTHROPIC_* — is intentionally +// excluded. The companion MCP server gets DATABASE_URL/OLLAMA_URL via its own +// explicit --mcp-config env (see companion.js), so it does NOT rely on these. +const ENV_ALLOW = [ + 'PATH', 'LANG', 'LANGUAGE', 'LC_ALL', 'LC_CTYPE', 'USER', 'LOGNAME', 'SHELL', + 'TERM', 'TMPDIR', 'XDG_RUNTIME_DIR', 'XDG_CONFIG_HOME', 'XDG_CACHE_HOME', 'XDG_DATA_HOME' +]; + +/** + * Build an allow-listed child environment for the `claude` subprocess. Only + * benign system vars + `CLAUDE_*` config pass through; app secrets never do. + * Excluding ANTHROPIC_* also forces subscription/OAuth auth. + * @param {object} parentEnv Source env (usually process.env) + * @param {string} [home] Override for HOME (service-user creds dir) + */ +export function buildChildEnv(parentEnv = process.env, home) { + const out = {}; + for (const k of ENV_ALLOW) { + if (parentEnv[k] !== undefined) out[k] = parentEnv[k]; + } + for (const k of Object.keys(parentEnv)) { + if (k.startsWith('CLAUDE_') && !k.startsWith('ANTHROPIC')) out[k] = parentEnv[k]; + } + out.HOME = home || parentEnv.HOME; + return out; +} + /** * Run a single non-interactive Claude CLI turn. * @@ -116,11 +145,8 @@ export async function runClaudeTurn(opts) { // positional prompt after them is swallowed ("Input must be provided..."). The // CLI reads the prompt from stdin in --print mode. - // Child env: clone, strip API key env vars so CLI uses subscription/OAuth auth - const childEnv = { ...process.env }; - delete childEnv.ANTHROPIC_API_KEY; - delete childEnv.ANTHROPIC_AUTH_TOKEN; - if (home) childEnv.HOME = home; + // Child env: allow-list only (no app secrets reach the CLI). See buildChildEnv. + const childEnv = buildChildEnv(process.env, home); // Accumulated state let text = ''; diff --git a/lib/api/routes/capture.js b/lib/api/routes/capture.js index fa1abc5..57cffa7 100644 --- a/lib/api/routes/capture.js +++ b/lib/api/routes/capture.js @@ -67,8 +67,12 @@ router.post('/upload', return res.status(400).json({ error: { code: 'validation_failed', message: 'file required' } }); } const space_id = req.body.space_id; - if (!space_id) { - return res.status(400).json({ error: { code: 'validation_failed', message: 'space_id required' } }); + if (!z.string().uuid().safeParse(space_id).success) { + return res.status(400).json({ error: { code: 'validation_failed', message: 'space_id must be a UUID' } }); + } + const { rowCount } = await pool.query('SELECT 1 FROM spaces WHERE id=$1', [space_id]); + if (!rowCount) { + return res.status(404).json({ error: { code: 'not_found', message: 'space not found' } }); } let meta = {}; if (req.body.meta) { diff --git a/lib/api/routes/pending_changes.js b/lib/api/routes/pending_changes.js index 652e435..fc9f3df 100644 --- a/lib/api/routes/pending_changes.js +++ b/lib/api/routes/pending_changes.js @@ -74,8 +74,17 @@ router.post('/:id/approve', if (row.status !== 'pending') { throw new ConflictError(`pending change is already ${row.status}`); } - const entity_id = await applyPendingChange(row, req.actor); + // Claim atomically BEFORE applying: resolve() guards `WHERE status='pending'`, + // so a crash/retry can't double-apply (the create path would duplicate). const resolved = await pendingRepo.resolve(req.params.id, 'approved', req.actor?.id ?? 'owner'); + if (!resolved) throw new ConflictError('pending change is already resolved'); + let entity_id; + try { + entity_id = await applyPendingChange(row, req.actor); + } catch (e) { + await pendingRepo.reopen(req.params.id); // un-claim so the owner can retry + throw e; + } await audit.recordAudit( req.actor, 'approve', row.entity_type, entity_id, null, { pending_id: row.id, original_agent_id: row.agent_id, action: row.action } diff --git a/lib/db/pool.js b/lib/db/pool.js index 8c65bb9..46ef1a6 100644 --- a/lib/db/pool.js +++ b/lib/db/pool.js @@ -5,7 +5,10 @@ import { log } from '../log.js'; export const pool = new pg.Pool({ connectionString: process.env.DATABASE_URL, max: 10, - idleTimeoutMillis: 30_000 + idleTimeoutMillis: 30_000, + // Server-side cap so a pathological query can't pin a connection indefinitely. + // Generous enough for migrations + hybrid search on this homelab-scale DB. + statement_timeout: 30_000 }); // An idle pooled client can emit 'error' (DB restart, replication failover on diff --git a/lib/db/repos/pending_changes.js b/lib/db/repos/pending_changes.js index 71e58e2..a79805f 100644 --- a/lib/db/repos/pending_changes.js +++ b/lib/db/repos/pending_changes.js @@ -30,3 +30,12 @@ export async function resolve(id, status, resolved_by) { ); return r; } + +// Compensating action: return a claimed change to 'pending' if applying it +// failed, so the owner can retry (prevents a claimed-but-unapplied dead change). +export async function reopen(id) { + await pool.query( + `UPDATE pending_changes SET status='pending', resolved_at=NULL, resolved_by=NULL WHERE id=$1`, + [id] + ); +} diff --git a/package.json b/package.json index de9c13e..d5a192b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "void-server", - "version": "2.0.0-alpha.8", + "version": "2.0.0-alpha.9", "type": "module", "private": true, "scripts": { diff --git a/server.js b/server.js index 476b51a..f14331b 100644 --- a/server.js +++ b/server.js @@ -9,7 +9,7 @@ import { router as ingestRouter } from './lib/api/routes/ingest.js'; import { router as iconsRouter } from './lib/api/routes/icons.js'; import { startCron } from './lib/cron/index.js'; -const VERSION = '2.0.0-alpha.8'; +const VERSION = '2.0.0-alpha.9'; export function createApp() { const app = express(); diff --git a/tests/ai/claude_cli_env.test.js b/tests/ai/claude_cli_env.test.js new file mode 100644 index 0000000..3cef9f7 --- /dev/null +++ b/tests/ai/claude_cli_env.test.js @@ -0,0 +1,36 @@ +import { describe, it, expect } from 'vitest'; +import { buildChildEnv } from '../../lib/ai/claude_cli.js'; + +describe('buildChildEnv (Q4 — claude child env allow-list)', () => { + const parent = { + PATH: '/usr/bin', HOME: '/root', LANG: 'C.UTF-8', TERM: 'xterm', + CLAUDE_EXE: '/usr/bin/claude', CLAUDE_CONFIG_DIR: '/cfg', + OWNER_TOKEN: 'secret-owner', DATABASE_URL: 'postgres://void:pw@db/void', + KARAKEEP_API_TOKEN: 'kk', ANTHROPIC_API_KEY: 'sk-ant', ANTHROPIC_AUTH_TOKEN: 'tok', + VOID_TOOL_REGISTRY: 'security' + }; + + it('passes through benign system + CLAUDE_* vars', () => { + const env = buildChildEnv(parent); + expect(env.PATH).toBe('/usr/bin'); + expect(env.LANG).toBe('C.UTF-8'); + expect(env.TERM).toBe('xterm'); + expect(env.CLAUDE_EXE).toBe('/usr/bin/claude'); + expect(env.CLAUDE_CONFIG_DIR).toBe('/cfg'); + }); + + it('NEVER leaks app secrets or API keys into the child', () => { + const env = buildChildEnv(parent); + expect(env.OWNER_TOKEN).toBeUndefined(); + expect(env.DATABASE_URL).toBeUndefined(); + expect(env.KARAKEEP_API_TOKEN).toBeUndefined(); + expect(env.ANTHROPIC_API_KEY).toBeUndefined(); + expect(env.ANTHROPIC_AUTH_TOKEN).toBeUndefined(); + expect(env.VOID_TOOL_REGISTRY).toBeUndefined(); + }); + + it('uses the home override for HOME when given', () => { + expect(buildChildEnv(parent, '/var/lib/void').HOME).toBe('/var/lib/void'); + expect(buildChildEnv(parent).HOME).toBe('/root'); + }); +}); diff --git a/tests/api/capture.test.js b/tests/api/capture.test.js index 89b131d..15c2459 100644 --- a/tests/api/capture.test.js +++ b/tests/api/capture.test.js @@ -58,6 +58,20 @@ describe('capture api', () => { expect(rows[0].kind).toBe('file'); }); + it('POST /api/capture/upload rejects a non-UUID space_id (Q6)', async () => { + const res = await request(app).post('/api/capture/upload').set(ownerHeaders) + .field('space_id', 'not-a-uuid') + .attach('file', Buffer.from('hi'), { filename: 'a.txt', contentType: 'text/plain' }); + expect(res.status).toBe(400); + }); + + it('POST /api/capture/upload rejects a non-existent space (Q6)', async () => { + const res = await request(app).post('/api/capture/upload').set(ownerHeaders) + .field('space_id', '00000000-0000-0000-0000-000000000000') + .attach('file', Buffer.from('hi'), { filename: 'a.txt', contentType: 'text/plain' }); + expect(res.status).toBe(404); + }); + it('POST /api/capture rejects missing url', async () => { const res = await request(app).post('/api/capture').set(ownerHeaders) .send({ space_id: sp.id }); diff --git a/tests/api/pending_changes.test.js b/tests/api/pending_changes.test.js index 50e337c..dbeba2d 100644 --- a/tests/api/pending_changes.test.js +++ b/tests/api/pending_changes.test.js @@ -5,6 +5,7 @@ import * as spacesRepo from '../../lib/db/repos/spaces.js'; import * as projectsRepo from '../../lib/db/repos/projects.js'; import * as pagesRepo from '../../lib/db/repos/pages.js'; import * as agentsRepo from '../../lib/db/repos/agents.js'; +import * as pendingChanges from '../../lib/db/repos/pending_changes.js'; let app, ownerHeaders, space, project, page; const owner = { kind: 'user', id: null }; @@ -144,6 +145,22 @@ describe('pending_changes routes', () => { expect(second.status).toBe(409); }); + it('apply failure reopens the change to pending (Q5 — no claimed-but-unapplied)', async () => { + const { agent } = await mintSuggestAgent(`sug-fail-${Date.now()}`); + // A page-create whose apply will FK-fail: space_id points at no space. + const change = await pendingChanges.create({ + agent_id: agent.id, entity_type: 'page', action: 'create', + payload: { space_id: '00000000-0000-0000-0000-000000000000', slug: 'x', title: 'X' }, + reason: 'test' + }); + const ap = await request(app) + .post(`/api/pending-changes/${change.id}/approve`).set(ownerHeaders); + expect(ap.status).toBe(500); // apply threw (FK violation) + const after = await pendingChanges.getById(change.id); + expect(after.status).toBe('pending'); // claimed-then-reopened, retryable + expect(after.resolved_at).toBeNull(); + }); + it('reject-then-approve → 409', async () => { const { headers } = await mintSuggestAgent(`sug-ra-${Date.now()}`); const sug = await request(app).post(`/api/spaces/${space.id}/pages`).set(headers)