From b56f9c5c5814cb407d44c2a181e06e6e166ebe74 Mon Sep 17 00:00:00 2001 From: Moshe Krupper Date: Thu, 9 Jul 2026 02:07:34 +0300 Subject: [PATCH] =?UTF-8?q?feat:=20/add-audit=20skill=20=E2=80=94=20opt-in?= =?UTF-8?q?=20ncl-surface=20audit=20log=20on=20the=20guard=20seam?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the local audit log as an install skill, scoped to the ncl command surface: one withAudit(dispatch) composition at the definition site covers both transports and the grant-carrying approved replay; holds are recorded as pending events correlated by approval id (recovered from the hold row), so --correlation returns the whole gated chain. src/audit/ installs as a domain-free leaf — schema v1, recursive key-pattern redactor (JSON string args parsed so inner secret keys mask), NDJSON day-files under data/audit/, fail-open + loud emit, post-write exporter hooks, boot writability refusal, self-contained retention prune (boot + daily, unref'd timer). `ncl audit list` is host + global-scope only (off the group-scope allowlist, fails closed) and errors clearly on a disabled box. Core reach-ins: the dispatch composition and the resource-barrel import, both guarded by a shipped AST/behavior wiring test that also pins guard conformance clean — the CI invariant that every holding action keeps its registered approve continuation. Approval-lifecycle events, OneCLI credential holds, and channel/sender events are later increments that attach as pure adds. Rebased onto the guard-seam refactors: boot-time conformance was dropped upstream (now a CI-only conformance test), so the wiring test asserts that invariant directly instead of importing the removed grantContinuationGaps(). Co-Authored-By: Claude Fable 5 Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/skills/add-audit/REMOVE.md | 52 +++ .claude/skills/add-audit/SKILL.md | 211 +++++++++++++ .../add-audit/add/src/audit-wiring.test.ts | 143 +++++++++ .../skills/add-audit/add/src/audit/config.ts | 30 ++ .../skills/add-audit/add/src/audit/emit.ts | 43 +++ .../add-audit/add/src/audit/hooks.test.ts | 203 ++++++++++++ .../skills/add-audit/add/src/audit/hooks.ts | 92 ++++++ .../skills/add-audit/add/src/audit/index.ts | 18 ++ .../skills/add-audit/add/src/audit/init.ts | 54 ++++ .../add-audit/add/src/audit/reader.test.ts | 153 +++++++++ .../skills/add-audit/add/src/audit/reader.ts | 160 ++++++++++ .../add-audit/add/src/audit/redact.test.ts | 69 ++++ .../skills/add-audit/add/src/audit/redact.ts | 32 ++ .../add-audit/add/src/audit/store.test.ts | 187 +++++++++++ .../skills/add-audit/add/src/audit/store.ts | 81 +++++ .../skills/add-audit/add/src/audit/types.ts | 60 ++++ .../skills/add-audit/add/src/audit/vocab.ts | 39 +++ .../add/src/cli/dispatch.audit.test.ts | 297 ++++++++++++++++++ .../add-audit/add/src/cli/dispatch.audit.ts | 238 ++++++++++++++ .../add-audit/add/src/cli/resources/audit.ts | 56 ++++ CHANGELOG.md | 1 + 21 files changed, 2219 insertions(+) create mode 100644 .claude/skills/add-audit/REMOVE.md create mode 100644 .claude/skills/add-audit/SKILL.md create mode 100644 .claude/skills/add-audit/add/src/audit-wiring.test.ts create mode 100644 .claude/skills/add-audit/add/src/audit/config.ts create mode 100644 .claude/skills/add-audit/add/src/audit/emit.ts create mode 100644 .claude/skills/add-audit/add/src/audit/hooks.test.ts create mode 100644 .claude/skills/add-audit/add/src/audit/hooks.ts create mode 100644 .claude/skills/add-audit/add/src/audit/index.ts create mode 100644 .claude/skills/add-audit/add/src/audit/init.ts create mode 100644 .claude/skills/add-audit/add/src/audit/reader.test.ts create mode 100644 .claude/skills/add-audit/add/src/audit/reader.ts create mode 100644 .claude/skills/add-audit/add/src/audit/redact.test.ts create mode 100644 .claude/skills/add-audit/add/src/audit/redact.ts create mode 100644 .claude/skills/add-audit/add/src/audit/store.test.ts create mode 100644 .claude/skills/add-audit/add/src/audit/store.ts create mode 100644 .claude/skills/add-audit/add/src/audit/types.ts create mode 100644 .claude/skills/add-audit/add/src/audit/vocab.ts create mode 100644 .claude/skills/add-audit/add/src/cli/dispatch.audit.test.ts create mode 100644 .claude/skills/add-audit/add/src/cli/dispatch.audit.ts create mode 100644 .claude/skills/add-audit/add/src/cli/resources/audit.ts diff --git a/.claude/skills/add-audit/REMOVE.md b/.claude/skills/add-audit/REMOVE.md new file mode 100644 index 000000000..2fa5608ae --- /dev/null +++ b/.claude/skills/add-audit/REMOVE.md @@ -0,0 +1,52 @@ +# Remove /add-audit + +Reverses every change the skill made. Safe to re-run even if some pieces are +already gone. Run from the NanoClaw project root. + +## 1. Delete the copied files + +```bash +rm -rf src/audit +rm -f src/cli/dispatch.audit.ts src/cli/dispatch.audit.test.ts +rm -f src/cli/resources/audit.ts +rm -f src/audit-wiring.test.ts +``` + +## 2. Revert the dispatch composition + +In `src/cli/dispatch.ts`, delete (not comment out) the three edits the skill +made: + +1. The import line: `import { withAudit } from './dispatch.audit.js';` +2. The composition block (comment + `export const dispatch = withAudit(dispatchInner);`) +3. Rename the dispatcher back — change `async function dispatchInner(` to + `export async function dispatch(` + +## 3. Unregister the resource + +Delete the `import './audit.js';` line from `src/cli/resources/index.ts`. + +## 4. Remove the settings + +Delete the `AUDIT_ENABLED` and `AUDIT_RETENTION_DAYS` lines from `.env`. + +## 5. Rebuild and restart + +```bash +pnpm run build +source setup/lib/install-slug.sh +launchctl kickstart -k gui/$(id -u)/$(launchd_label) # macOS +# systemctl --user restart $(systemd_unit) # Linux +``` + +## Day-files + +`data/audit/*.ndjson` are the operator's records and are deliberately left in +place. To purge them too: + +```bash +rm -rf data/audit +``` + +No dependency was added, nothing under `container/` was touched, and no DB +schema changed — there is nothing else to undo. diff --git a/.claude/skills/add-audit/SKILL.md b/.claude/skills/add-audit/SKILL.md new file mode 100644 index 000000000..965d55499 --- /dev/null +++ b/.claude/skills/add-audit/SKILL.md @@ -0,0 +1,211 @@ +--- +name: add-audit +description: Add an opt-in local audit log for the ncl command surface — every dispatch outcome on both transports (host socket and container), including scope denials, approval holds, and approved replays, written as SIEM-shaped append-only NDJSON day-files under data/audit/. Read back with `ncl audit list`; plug exporters in via registerAuditHook. Off until AUDIT_ENABLED=true. +--- + +# /add-audit — Local Audit Log (ncl surface) + +Records one canonical audit event for every command that reaches the `ncl` +dispatcher — human over the host socket or agent over the container transport +— including denials. Both transports converge on the exported `dispatch`, so +one composition covers the whole surface: there is no second door. + +``` +ncl (host socket) ──┐ + ├─→ dispatch = withAudit(dispatchInner) ─→ one NDJSON line +container cli_request ┘ │ data/audit/.ndjson +approved replay (grant) ───────┘ +``` + +What one event carries: `actor` (`host:` or the agent group), +`origin` (transport, session, channel), dotted `action` from the guard +catalog (e.g. `groups.config.add-mcp-server`), touched/attempted `resources`, +`outcome` (`success · failure · denied · pending`), `correlation_id` (the +approval id on gated chains — the pending event and the approved replay share +it, so `--correlation ` returns the whole chain), and redacted `details`. + +Scope of this increment: the ncl dispatch surface only. Approval-lifecycle +events (request/decision as their own events), OneCLI credential holds, and +channel/sender events are later increments — they attach to `src/audit/` as +pure adds (new `*.audit.ts` adapters and observer subscriptions). + +## Steps + +### 0. Pre-flight (idempotency) + +The apply is safe to re-run; every step below is guarded. Skip to +**Enable** if all of these already hold: + +- `src/audit/` exists +- `src/cli/resources/index.ts` contains `import './audit.js';` +- `src/cli/dispatch.ts` contains `export const dispatch = withAudit(dispatchInner);` + +Before editing, verify the reach-in targets still exist: `src/cli/dispatch.ts` +must contain `export async function dispatch(` (or the already-applied +composition), and `src/cli/resources/index.ts` must be the resource barrel (a +list of `import './.js';` lines). If either has moved, stop and +adapt rather than guessing. + +### 1. Copy the payload + +From the NanoClaw project root: + +```bash +cp -R "${CLAUDE_SKILL_DIR}/add/src/." src/ +``` + +What lands (mirrors of the destination paths): + +- `src/audit/` — the domain-free leaf: event schema (`types.ts`), env config + (`config.ts`), redactor, NDJSON day-file store, emit seam, post-write hooks, + reader, boot/maintenance wiring, vocabulary — plus its tests. +- `src/cli/dispatch.audit.ts` — the CLI adapter: `withAudit` middleware and + the actor/origin/resource mapping (+ `dispatch.audit.test.ts`). +- `src/cli/resources/audit.ts` — the read-only `ncl audit` resource. +- `src/audit-wiring.test.ts` — goes red if either core edit below is deleted + or drifts. + +### 2. Register the resource + +Append to `src/cli/resources/index.ts` (skip if the line is already present): + +```typescript +import './audit.js'; +``` + +### 3. Compose the dispatch middleware + +This is the skill's one functional reach-in, in `src/cli/dispatch.ts`. Three +small edits: + +1. Add the import (next to the other `./` imports): + + ```typescript + import { withAudit } from './dispatch.audit.js'; + ``` + +2. Rename the dispatcher declaration — change + + ```typescript + export async function dispatch( + ``` + + to + + ```typescript + async function dispatchInner( + ``` + +3. Directly after that function's closing brace (before the + `registerApprovalHandler('cli_command', …)` block), add: + + ```typescript + // Audit middleware (installed by /add-audit): the exported dispatch is the + // wrapped function, so both transports and the approved replay below all + // pass the one composition. + export const dispatch = withAudit(dispatchInner); + ``` + +The composition must live at the definition site (not at the import sites): +the approved-replay handler in the same file calls `dispatch(...)` too, and +only the wrapped export covers it. `src/audit-wiring.test.ts` asserts exactly +this shape via the TypeScript AST. + +Loading `dispatch.audit.ts` also boots the audit log: on an enabled box it +asserts `data/audit/` is writable (refusing to start beats a silent audit +gap), runs the boot retention prune, and arms an unref'd maintenance timer. +Nothing else in core changes — no `index.ts` or `host-sweep.ts` edits. + +### 4. Enable + +Add the two settings to `.env` (idempotent — overwrite or append): + +```bash +grep -q '^AUDIT_ENABLED=' .env && sed -i.bak 's/^AUDIT_ENABLED=.*/AUDIT_ENABLED=true/' .env && rm -f .env.bak || echo 'AUDIT_ENABLED=true' >> .env +grep -q '^AUDIT_RETENTION_DAYS=' .env || echo 'AUDIT_RETENTION_DAYS=90' >> .env +``` + +`AUDIT_ENABLED` is the master switch — off (or absent) means `emitAuditEvent` +is a no-op and `data/audit/` is never created. `AUDIT_RETENTION_DAYS`: day +files strictly older than the horizon are hard-deleted (unlinked) at boot and +once per UTC day; `0` = keep forever; unset = 90. + +### 5. Build and test + +Run `build` before the tests — it's what catches a missed copy or a drifted +import path across the whole composed tree: + +```bash +pnpm run build +pnpm exec vitest run src/audit src/cli/dispatch.audit.test.ts src/audit-wiring.test.ts +pnpm test +``` + +### 6. Restart the service + +```bash +source setup/lib/install-slug.sh +launchctl kickstart -k gui/$(id -u)/$(launchd_label) # macOS +# systemctl --user restart $(systemd_unit) # Linux +``` + +### 7. Verify (runtime smoke) + +```bash +ncl groups list # any command — this one is now an audit event +cat data/audit/$(date -u +%F).ndjson | tail -1 +ncl audit list --limit 5 +``` + +The last line of the day-file is the `groups.list` event you just caused. + +## Reading it back + +``` +ncl audit list [--actor ] [--action ] [--resource ] + [--outcome success|failure|denied|pending|approved|rejected] + [--since 7d|24h|30m|ISO] [--until …] [--correlation ] + [--limit N] [--format ndjson] +``` + +Newest first, default limit 100. `--action groups.config` matches the whole +dotted subtree. `--format ndjson` streams the stored lines verbatim — pipe to +a file for SIEM import. On a disabled box the command errors with +`audit log is disabled — set AUDIT_ENABLED=true` rather than returning an +empty list that would read as "no actions happened". + +The resource is deliberately **not** on the group-scope allowlist: audit +spans agent groups, so group-scoped agents are refused before the handler +(fails closed). Host callers and `cli_scope: global` agents only. + +## Redaction, failure posture, exporters + +- Every event's `details` passes a recursive key-pattern redactor + (`/(token|secret|key|password|credential|auth|bearer)/i` → `[REDACTED]`, + ~2 KB per-value cap) at the single emit seam. +- Fail-open + loud: a failed append is `log.error`'d and the audited action + proceeds. At boot, an enabled box refuses to start if `data/audit/` isn't + writable. +- In-process exporters register via `registerAuditHook` (from + `src/audit/index.js`) — post-write hooks that fire only after the local + append succeeds, so an external system can never be ahead of the source of + truth. Ship one as its own `/add-*` skill; declare `/add-audit` as its + dependency. + +## Troubleshooting + +- **Host won't start, banner names the audit directory** — `AUDIT_ENABLED=true` + but `data/audit/` isn't writable. Fix permissions or disable audit. +- **`audit log is disabled — set AUDIT_ENABLED=true`** — the read-back guard; + enable in `.env` and restart. +- **An agent gets "CLI access is scoped to this agent group" for `ncl audit`** + — by design; grant the group `cli_scope: global` only if it should read + cross-group history. +- **`pnpm test` on an enabled box adds a few events to today's day-file** — + expected: the base dispatch tests drive real dispatches. Harmless noise; + the audit test suites themselves write only to temp dirs or capture buffers. + +## Removal + +See [REMOVE.md](REMOVE.md) — reverses every change; existing day-files are +left in place (they're the operator's records). diff --git a/.claude/skills/add-audit/add/src/audit-wiring.test.ts b/.claude/skills/add-audit/add/src/audit-wiring.test.ts new file mode 100644 index 000000000..398d51aac --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit-wiring.test.ts @@ -0,0 +1,143 @@ +/** + * Wiring tests for the /add-audit skill's two core edits — they go red if + * either edit is deleted or drifts: + * + * 1. src/cli/dispatch.ts — the exported dispatch must be the composed + * `withAudit(dispatchInner)` (AST check: only the definition-site + * composition covers both transports AND the in-module approved replay). + * 2. src/cli/resources/index.ts — the audit resource must register through + * the real barrel (behavior check), stay OFF the group-scope allowlist, + * and leave guard conformance clean. + * + * Plus the emit invariant the module's discipline rests on: emitAuditEvent + * appears only in src/audit/ and *.audit.ts adapter files. + */ +import fs from 'fs'; +import path from 'path'; +import ts from 'typescript'; +import { describe, expect, it } from 'vitest'; + +// Same production barrels the guard conformance test loads (mirrors +// src/guard/conformance.test.ts). +import './cli/commands/index.js'; +import './modules/index.js'; +import './cli/delivery-action.js'; +import './cli/dispatch.js'; + +import { commandGuard, GROUP_SCOPE_RESOURCES, lookup } from './cli/registry.js'; +import { getApprovalHandler } from './modules/approvals/primitive.js'; +import { listGuardedActions } from './guard/index.js'; + +describe('audit resource registration (barrel wiring)', () => { + it('registers audit-list through the real resource barrel', () => { + const cmd = lookup('audit-list'); + expect(cmd).toBeDefined(); + expect(cmd?.action).toBe('audit.list'); + expect(cmd?.access).toBe('open'); + }); + + it('derives a guard-catalog entry for the audit command', () => { + const guard = commandGuard('audit-list'); + expect(guard.action).toBe('audit.list'); + }); + + it('is NOT on the group-scope allowlist — group-scoped agents fail closed', () => { + expect(GROUP_SCOPE_RESOURCES.has('audit')).toBe(false); + }); + + it('leaves guard conformance clean with the audit resource registered', () => { + // The invariant guard conformance checks: every holding action pairs with a + // registered approve continuation. audit.list is `open` (never holds), so + // registering the resource can't introduce a gap — this pins that. + const dangling = listGuardedActions() + .filter((spec) => spec.grantActionName) + .filter((spec) => !getApprovalHandler(spec.grantActionName as string)); + expect(dangling.map((s) => s.action)).toEqual([]); + }); +}); + +describe('dispatch composition (AST wiring)', () => { + const source = fs.readFileSync(path.resolve('src/cli/dispatch.ts'), 'utf8'); + const sf = ts.createSourceFile('dispatch.ts', source, ts.ScriptTarget.Latest, true); + + const hasExportModifier = (node: ts.HasModifiers): boolean => + (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword); + + let importsWithAudit = false; + let innerDeclaredUnexported = false; + let exportsWrappedDispatch = false; + let exportsUnwrappedDispatchFn = false; + + sf.forEachChild((node) => { + if ( + ts.isImportDeclaration(node) && + ts.isStringLiteral(node.moduleSpecifier) && + node.moduleSpecifier.text === './dispatch.audit.js' + ) { + const named = node.importClause?.namedBindings; + if (named && ts.isNamedImports(named) && named.elements.some((e) => e.name.text === 'withAudit')) { + importsWithAudit = true; + } + } + if (ts.isFunctionDeclaration(node) && node.name?.text === 'dispatchInner') { + innerDeclaredUnexported = !hasExportModifier(node); + } + if (ts.isFunctionDeclaration(node) && node.name?.text === 'dispatch' && hasExportModifier(node)) { + exportsUnwrappedDispatchFn = true; + } + if (ts.isVariableStatement(node) && hasExportModifier(node)) { + for (const decl of node.declarationList.declarations) { + if ( + ts.isIdentifier(decl.name) && + decl.name.text === 'dispatch' && + decl.initializer && + ts.isCallExpression(decl.initializer) && + ts.isIdentifier(decl.initializer.expression) && + decl.initializer.expression.text === 'withAudit' && + decl.initializer.arguments.length === 1 && + ts.isIdentifier(decl.initializer.arguments[0]) && + decl.initializer.arguments[0].text === 'dispatchInner' + ) { + exportsWrappedDispatch = true; + } + } + } + }); + + it('imports withAudit from the audit adapter', () => { + expect(importsWithAudit).toBe(true); + }); + + it('keeps the inner dispatcher unexported (the guarded path is the only path)', () => { + expect(innerDeclaredUnexported).toBe(true); + }); + + it('exports dispatch as withAudit(dispatchInner)', () => { + expect(exportsWrappedDispatch).toBe(true); + expect(exportsUnwrappedDispatchFn).toBe(false); + }); +}); + +describe('emit invariant', () => { + it('emitAuditEvent appears only in src/audit/ and *.audit.ts files', () => { + const srcRoot = path.resolve('src'); + const offenders: string[] = []; + const walk = (dir: string): void => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(full); + continue; + } + if (!entry.name.endsWith('.ts')) continue; + const rel = path.relative(srcRoot, full).split(path.sep).join('/'); + if (rel === 'audit-wiring.test.ts') continue; // this file names the symbol + if (rel.startsWith('audit/')) continue; + if (/\.audit(\.test)?\.ts$/.test(rel)) continue; + if (fs.readFileSync(full, 'utf8').includes('emitAuditEvent')) offenders.push(rel); + } + }; + walk(srcRoot); + expect(offenders).toEqual([]); + }); +}); diff --git a/.claude/skills/add-audit/add/src/audit/config.ts b/.claude/skills/add-audit/add/src/audit/config.ts new file mode 100644 index 000000000..73a8194a7 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/config.ts @@ -0,0 +1,30 @@ +/** + * Audit env config (installed by /add-audit) — the two audit vars, read the + * same way src/config.ts reads every host setting (readEnvFile from .env, + * with process.env taking precedence). Kept audit-owned so installing the + * skill never edits core config.ts: the feature's whole footprint in core is + * the dispatch composition in dispatch.ts and the resource-barrel import. + */ +import { readEnvFile } from '../env.js'; + +const envConfig = readEnvFile(['AUDIT_ENABLED', 'AUDIT_RETENTION_DAYS']); + +/** + * Master switch. Off by default — nothing is persisted (and data/audit/ is + * never created) until an operator sets AUDIT_ENABLED=true. + */ +export const AUDIT_ENABLED = (process.env.AUDIT_ENABLED || envConfig.AUDIT_ENABLED) === 'true'; + +/** + * Day-file retention horizon in days; consulted only when audit is enabled. + * Unset or unparseable → 90. An explicit 0 (or negative) = keep forever. + */ +function parseRetentionDays(raw: string | undefined): number { + if (raw === undefined || raw.trim() === '') return 90; + const n = Number.parseInt(raw, 10); + return Number.isNaN(n) ? 90 : n; +} + +export const AUDIT_RETENTION_DAYS = parseRetentionDays( + process.env.AUDIT_RETENTION_DAYS || envConfig.AUDIT_RETENTION_DAYS, +); diff --git a/.claude/skills/add-audit/add/src/audit/emit.ts b/.claude/skills/add-audit/add/src/audit/emit.ts new file mode 100644 index 000000000..51e9460cc --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/emit.ts @@ -0,0 +1,43 @@ +/** + * The single emit seam. Adapters import emitAuditEvent from here directly + * (it is deliberately not re-exported by the barrel): only src/audit/ and + * `*.audit.ts` adapter files may call it — grep holds the invariant. + * + * The opt-in check lives here, so the whole feature switches at one point. + * Fail-open + loud: a failed append (or a throwing input thunk) is + * log.error'd and the audited action proceeds. + */ +import { randomUUID } from 'crypto'; + +import { log } from '../log.js'; +import { AUDIT_ENABLED } from './config.js'; +import { notifyAuditHooks } from './hooks.js'; +import { redactDetails } from './redact.js'; +import { appendAuditLine } from './store.js'; +import type { AuditEvent, AuditEventInput } from './types.js'; + +export function emitAuditEvent(input: AuditEventInput | (() => AuditEventInput)): void { + if (!AUDIT_ENABLED) return; // The one opt-in check — the whole feature switches here. + try { + if (typeof input === 'function') input = input(); + const event: AuditEvent = { + event_id: randomUUID(), + time: new Date().toISOString(), + schema_version: 1, + actor: { ...input.actor, email: null, user_id: null, group_ids: null }, + origin: input.origin, + action: input.action, + resources: input.resources, + outcome: input.outcome, + correlation_id: input.correlationId ?? null, + details: redactDetails(input.details ?? {}), + }; + const line = JSON.stringify(event); + appendAuditLine(line); + notifyAuditHooks(event, line); + // eslint-disable-next-line no-catch-all/no-catch-all -- fail-open is the posture: an audit failure must never take down the audited action + } catch (err) { + const action = typeof input === 'function' ? undefined : input.action; + log.error('Audit append failed — action proceeding (fail-open)', { action, err }); + } +} diff --git a/.claude/skills/add-audit/add/src/audit/hooks.test.ts b/.claude/skills/add-audit/add/src/audit/hooks.test.ts new file mode 100644 index 000000000..f8ee21610 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/hooks.test.ts @@ -0,0 +1,203 @@ +/** + * Post-write hook contract: hooks observe the LOG (fire only after a + * successful append, exported ⊆ written), failures are isolated everywhere, + * and the lifecycle (init/maintain/shutdown) behaves. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const state = vi.hoisted(() => ({ enabled: true, appendThrows: false, appended: [] as string[] })); + +vi.mock('../config.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + DATA_DIR: '/tmp/nanoclaw-test-hooks-unused', + }; +}); + +vi.mock('./config.js', () => ({ + get AUDIT_ENABLED() { + return state.enabled; + }, + AUDIT_RETENTION_DAYS: 90, +})); + +vi.mock('./store.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + appendAuditLine: (line: string) => { + if (state.appendThrows) throw new Error('disk full'); + state.appended.push(line); + }, + }; +}); + +vi.mock('../log.js', () => ({ + log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() }, +})); + +let hooks: typeof import('./hooks.js'); +let emit: typeof import('./emit.js'); +let log: (typeof import('../log.js'))['log']; + +beforeEach(async () => { + state.enabled = true; + state.appendThrows = false; + state.appended.length = 0; + vi.resetModules(); // fresh hook registry per test + hooks = await import('./hooks.js'); + emit = await import('./emit.js'); + log = (await import('../log.js')).log; + vi.clearAllMocks(); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +const EVENT_INPUT = { + actor: { type: 'human' as const, id: 'host:test' }, + origin: { transport: 'socket' as const }, + action: 'groups.list', + resources: [{ type: 'agent_group' }], + outcome: 'success' as const, + details: { limit: 5 }, +}; + +describe('post-write notification', () => { + it('calls a registered hook with the parsed event and the exact stored line', () => { + const seen: Array<{ event: import('./types.js').AuditEvent; line: string }> = []; + hooks.registerAuditHook({ name: 'demo', onEvent: (event, line) => seen.push({ event, line }) }); + + emit.emitAuditEvent(EVENT_INPUT); + + expect(state.appended).toHaveLength(1); + expect(seen).toHaveLength(1); + expect(seen[0].line).toBe(state.appended[0]); + expect(JSON.parse(seen[0].line)).toEqual(seen[0].event); + expect(seen[0].event.action).toBe('groups.list'); + }); + + it('does NOT call hooks when the local append fails — exported ⊆ written', () => { + const onEvent = vi.fn(); + hooks.registerAuditHook({ name: 'demo', onEvent }); + state.appendThrows = true; + + expect(() => emit.emitAuditEvent(EVENT_INPUT)).not.toThrow(); // action still proceeds + expect(onEvent).not.toHaveBeenCalled(); + expect(log.error).toHaveBeenCalledWith(expect.stringContaining('Audit append failed'), expect.anything()); + }); + + it('does NOT call hooks when audit is disabled', () => { + const onEvent = vi.fn(); + hooks.registerAuditHook({ name: 'demo', onEvent }); + state.enabled = false; + + emit.emitAuditEvent(EVENT_INPUT); + + expect(state.appended).toHaveLength(0); + expect(onEvent).not.toHaveBeenCalled(); + }); + + it('isolates a throwing hook: the write survives, later hooks still run, the action proceeds', () => { + const second = vi.fn(); + hooks.registerAuditHook({ + name: 'broken', + onEvent: () => { + throw new Error('exporter exploded'); + }, + }); + hooks.registerAuditHook({ name: 'healthy', onEvent: second }); + + expect(() => emit.emitAuditEvent(EVENT_INPUT)).not.toThrow(); + + expect(state.appended).toHaveLength(1); // the log has the event regardless + expect(second).toHaveBeenCalledTimes(1); + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('Audit hook threw'), + expect.objectContaining({ hook: 'broken', action: 'groups.list' }), + ); + }); +}); + +describe('lifecycle', () => { + it('initAuditHooks surfaces a failing init as a fatal error naming the hook', () => { + hooks.registerAuditHook({ name: 'ok', onEvent: () => {}, init: vi.fn() }); + hooks.registerAuditHook({ + name: 'bad-boot', + onEvent: () => {}, + init: () => { + throw new Error('no route to collector'); + }, + }); + + expect(() => hooks.initAuditHooks()).toThrow(/audit hook "bad-boot" failed to initialize.*no route/); + }); + + it('maintainAuditHooks calls every maintain and isolates throws', () => { + const m1 = vi.fn(() => { + throw new Error('flush failed'); + }); + const m2 = vi.fn(); + hooks.registerAuditHook({ name: 'a', onEvent: () => {}, maintain: m1 }); + hooks.registerAuditHook({ name: 'b', onEvent: () => {}, maintain: m2 }); + hooks.registerAuditHook({ name: 'c', onEvent: () => {} }); // no maintain — fine + + expect(() => hooks.maintainAuditHooks()).not.toThrow(); + expect(m1).toHaveBeenCalledTimes(1); + expect(m2).toHaveBeenCalledTimes(1); + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('maintenance failed'), + expect.objectContaining({ hook: 'a' }), + ); + }); + + it('shutdownAuditHooks awaits async shutdowns and isolates throws', async () => { + const order: string[] = []; + hooks.registerAuditHook({ + name: 'a', + onEvent: () => {}, + shutdown: async () => { + await Promise.resolve(); + order.push('a'); + }, + }); + hooks.registerAuditHook({ + name: 'b', + onEvent: () => {}, + shutdown: () => { + throw new Error('handle already closed'); + }, + }); + hooks.registerAuditHook({ + name: 'c', + onEvent: () => {}, + shutdown: () => { + order.push('c'); + }, + }); + + await hooks.shutdownAuditHooks(); + + expect(order).toEqual(['a', 'c']); + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('shutdown failed'), + expect.objectContaining({ hook: 'b' }), + ); + }); + + it('maintainAudit skips hook maintenance when audit is disabled', async () => { + const init = await import('./init.js'); + const maintain = vi.fn(); + hooks.registerAuditHook({ name: 'a', onEvent: () => {}, maintain }); + + state.enabled = false; + init.maintainAudit(); + expect(maintain).not.toHaveBeenCalled(); + + state.enabled = true; + init.maintainAudit(); + expect(maintain).toHaveBeenCalledTimes(1); + }); +}); diff --git a/.claude/skills/add-audit/add/src/audit/hooks.ts b/.claude/skills/add-audit/add/src/audit/hooks.ts new file mode 100644 index 000000000..01d0abdca --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/hooks.ts @@ -0,0 +1,92 @@ +/** + * Post-write audit hooks — the in-process extension seam. + * + * A hook observes the audit LOG, not the event stream: `onEvent` fires only + * after an event has been durably appended to the local day-file, so anything + * a hook exports is guaranteed to exist in the source of truth + * (exported ⊆ written). If the local append fails, hooks are not called — + * and a hook that misses events (crash, restart) catches up by reading the + * day-files, which is the at-least-once story. + * + * Registration follows the tree's observer idiom (registerApprovalResolvedHandler, + * registerResponseHandler, …): an in-tree or skill-installed module calls + * `registerAuditHook(...)` at import time — no core edits, and credentials or + * transport for an external system live in that module, never here. + */ +import { log } from '../log.js'; +import type { AuditEvent } from './types.js'; + +export interface AuditHook { + /** Short identifier used in logs and lifecycle errors. */ + name: string; + /** + * Called after a successful local append. `line` is the exact stored bytes + * (one NDJSON line, no trailing newline); `event` is the parsed record. + * MUST be fast and non-blocking — this runs on the audited action's call + * path. A real exporter buffers here and does its IO from `maintain`/its own + * timers. Throwing is tolerated: isolated and logged, never propagated. + */ + onEvent(event: AuditEvent, line: string): void; + /** Boot hook, called only when audit is enabled. Throw = host refuses to start. */ + init?(): void; + /** Periodic maintenance — called from the audit maintenance timer (enabled boxes only). */ + maintain?(): void; + /** Graceful-shutdown hook (flush buffers, close handles). */ + shutdown?(): void | Promise; +} + +const hooks: AuditHook[] = []; + +export function registerAuditHook(hook: AuditHook): void { + hooks.push(hook); +} + +/** Fan out one written event to every hook, isolating failures per hook. */ +export function notifyAuditHooks(event: AuditEvent, line: string): void { + for (const hook of hooks) { + try { + hook.onEvent(event, line); + // eslint-disable-next-line no-catch-all/no-catch-all -- isolation is the contract: one bad hook must not affect the log, other hooks, or the audited action + } catch (err) { + log.error('Audit hook threw — event is safely in the log', { hook: hook.name, action: event.action, err }); + } + } +} + +/** Boot lifecycle. A hook that can't start is a silent-export-gap risk — fatal. */ +export function initAuditHooks(): void { + for (const hook of hooks) { + try { + hook.init?.(); + } catch (err) { + throw new Error( + `audit hook "${hook.name}" failed to initialize: ${err instanceof Error ? err.message : String(err)}`, + { cause: err }, + ); + } + } +} + +/** Periodic lifecycle — maintenance, isolated per hook. */ +export function maintainAuditHooks(): void { + for (const hook of hooks) { + try { + hook.maintain?.(); + // eslint-disable-next-line no-catch-all/no-catch-all -- one hook's maintenance failure must not stop the others (or the timer) + } catch (err) { + log.error('Audit hook maintenance failed', { hook: hook.name, err }); + } + } +} + +/** Shutdown lifecycle — awaited by the host's graceful shutdown. */ +export async function shutdownAuditHooks(): Promise { + for (const hook of hooks) { + try { + await hook.shutdown?.(); + // eslint-disable-next-line no-catch-all/no-catch-all -- shutdown must drain every hook even when one throws + } catch (err) { + log.error('Audit hook shutdown failed', { hook: hook.name, err }); + } + } +} diff --git a/.claude/skills/add-audit/add/src/audit/index.ts b/.claude/skills/add-audit/add/src/audit/index.ts new file mode 100644 index 000000000..8bc655821 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/index.ts @@ -0,0 +1,18 @@ +/** + * Opt-in local audit log — installed by /add-audit. + * + * This directory is a domain-free leaf: the event schema, the emit seam, the + * store, the reader, post-write hooks, and shared vocabulary. What gets + * audited — and how each domain describes itself — lives in the domain-owned + * `*.audit.ts` adapter files next to the code they observe. Business logic + * contains zero audit calls. + * + * emitAuditEvent is deliberately NOT re-exported here: adapters import it + * from './emit.js' directly, and only src/audit/ + `*.audit.ts` may call it. + */ +export * from './types.js'; +export { redactDetails } from './redact.js'; +export { AUDIT_DIR } from './store.js'; +export { AUDIT_ENABLED, AUDIT_RETENTION_DAYS } from './config.js'; +export { initAuditLog, maintainAudit } from './init.js'; +export { type AuditHook, registerAuditHook } from './hooks.js'; diff --git a/.claude/skills/add-audit/add/src/audit/init.ts b/.claude/skills/add-audit/add/src/audit/init.ts new file mode 100644 index 000000000..d6ccb2b9f --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/init.ts @@ -0,0 +1,54 @@ +/** + * Boot-time audit wiring, self-contained (installed by /add-audit). + * + * initAuditLog() runs at module load of the CLI audit adapter + * (cli/dispatch.audit.ts) — i.e. during the host's barrel phase, before the + * CLI server or any delivery poll accepts work — because dispatch.ts, which + * both transports import, composes withAudit at module scope. When enabled: + * assert data/audit/ is writable (refusing to start beats running with a + * silent audit gap), run the boot prune, start the registered post-write + * hooks, and arm the maintenance timer. + * + * The timer owns the daily cadence in-module (checked hourly; the prune + * itself fires at most once per UTC day) so installing the skill touches + * dispatch.ts and the resource barrel only — no host-sweep edit. It is + * unref'd: it never keeps the process alive. + */ +import { log } from '../log.js'; +import { onShutdown } from '../response-registry.js'; +import { AUDIT_ENABLED, AUDIT_RETENTION_DAYS } from './config.js'; +import { initAuditHooks, maintainAuditHooks, shutdownAuditHooks } from './hooks.js'; +import { assertAuditWritable, AUDIT_DIR, markPrunedToday, pruneAuditLog, pruneAuditLogIfDue } from './store.js'; + +const MAINTENANCE_INTERVAL_MS = 60 * 60 * 1000; + +let initialized = false; + +export function initAuditLog(): void { + if (!AUDIT_ENABLED || initialized) return; + initialized = true; + try { + assertAuditWritable(); + } catch (err) { + throw new Error( + `AUDIT_ENABLED=true but the audit directory is not writable: ${AUDIT_DIR} (${err instanceof Error ? err.message : String(err)})`, + { cause: err }, + ); + } + pruneAuditLog(); + markPrunedToday(); + initAuditHooks(); // throw → boot fails, same posture as the writability assert + onShutdown(() => shutdownAuditHooks()); + setInterval(maintainAudit, MAINTENANCE_INTERVAL_MS).unref(); + log.info('Audit log enabled', { dir: AUDIT_DIR, retentionDays: AUDIT_RETENTION_DAYS }); +} + +/** + * One maintenance tick: retention prune (throttled to once per UTC day + * internally) plus every hook's periodic maintenance. No-op when audit is + * disabled. Exported so a future scheduler can drive it too. + */ +export function maintainAudit(): void { + pruneAuditLogIfDue(); + if (AUDIT_ENABLED) maintainAuditHooks(); +} diff --git a/.claude/skills/add-audit/add/src/audit/reader.test.ts b/.claude/skills/add-audit/add/src/audit/reader.test.ts new file mode 100644 index 000000000..012785ffb --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/reader.test.ts @@ -0,0 +1,153 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const state = vi.hoisted(() => ({ dataDir: '', enabled: true })); + +vi.mock('../config.js', () => ({ + get DATA_DIR() { + return state.dataDir; + }, +})); + +vi.mock('./config.js', () => ({ + get AUDIT_ENABLED() { + return state.enabled; + }, + AUDIT_RETENTION_DAYS: 90, +})); + +vi.mock('../log.js', () => ({ + log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() }, +})); + +let reader: typeof import('./reader.js'); +let store: typeof import('./store.js'); + +beforeEach(async () => { + state.dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'audit-reader-')); + state.enabled = true; + vi.resetModules(); + store = await import('./store.js'); + reader = await import('./reader.js'); +}); + +afterEach(() => { + fs.rmSync(state.dataDir, { recursive: true, force: true }); +}); + +function dayString(daysAgo: number): string { + return store.utcDay(new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000)); +} + +function isoAt(daysAgo: number, tag: number): string { + const base = new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000); + return `${base.toISOString().slice(0, 10)}T10:00:0${tag}.000Z`; +} + +let seq = 0; +function seedEvent(daysAgo: number, overrides: Record = {}): Record { + seq += 1; + const event = { + event_id: `e-${seq}`, + time: isoAt(daysAgo, seq % 10), + schema_version: 1, + actor: { type: 'human', id: 'host:moshe', email: null, user_id: null, group_ids: null }, + origin: { transport: 'socket' }, + action: 'groups.list', + resources: [{ type: 'agent_group', id: 'ag-1' }], + outcome: 'success', + correlation_id: null, + details: {}, + ...overrides, + }; + const dir = path.join(state.dataDir, 'audit'); + fs.mkdirSync(dir, { recursive: true }); + fs.appendFileSync(path.join(dir, `${dayString(daysAgo)}.ndjson`), JSON.stringify(event) + '\n'); + return event; +} + +describe('listAuditEvents', () => { + it('throws the disabled error rather than returning an empty list', () => { + state.enabled = false; + expect(() => reader.listAuditEvents({})).toThrow('audit log is disabled — set AUDIT_ENABLED=true'); + }); + + it('returns flat rows newest-first across day-files, honoring --limit', () => { + seedEvent(2, { event_id: 'old' }); + seedEvent(1, { event_id: 'mid-a' }); + seedEvent(1, { event_id: 'mid-b' }); + seedEvent(0, { event_id: 'new' }); + + const rows = reader.listAuditEvents({}) as Array>; + expect(rows.map((r) => r.event_id)).toEqual(['new', 'mid-b', 'mid-a', 'old']); + expect(rows[0]).toMatchObject({ actor: 'host:moshe', action: 'groups.list', outcome: 'success' }); + expect(rows[0].resources).toBe('agent_group:ag-1'); + + const limited = reader.listAuditEvents({ limit: 2 }) as Array>; + expect(limited.map((r) => r.event_id)).toEqual(['new', 'mid-b']); + }); + + it('filters by actor, outcome, correlation, and resource (id or type)', () => { + seedEvent(0, { event_id: 'a', actor: { type: 'agent', id: 'ag-1' }, outcome: 'denied' }); + seedEvent(0, { event_id: 'b', correlation_id: 'appr-9', resources: [{ type: 'approval', id: 'appr-9' }] }); + seedEvent(0, { event_id: 'c', resources: [{ type: 'user', id: 'slack:U1' }] }); + + expect((reader.listAuditEvents({ actor: 'ag-1' }) as unknown[]).length).toBe(1); + expect((reader.listAuditEvents({ outcome: 'denied' }) as unknown[]).length).toBe(1); + expect((reader.listAuditEvents({ correlation: 'appr-9' }) as unknown[]).length).toBe(1); + expect((reader.listAuditEvents({ resource: 'slack:U1' }) as unknown[]).length).toBe(1); + expect((reader.listAuditEvents({ resource: 'approval' }) as unknown[]).length).toBe(1); + }); + + it('matches actions exactly or by dotted prefix', () => { + seedEvent(0, { event_id: 'cfg', action: 'groups.config.add-mcp-server' }); + seedEvent(0, { event_id: 'list', action: 'groups.list' }); + seedEvent(0, { event_id: 'other', action: 'sessions.list' }); + + expect((reader.listAuditEvents({ action: 'groups' }) as unknown[]).length).toBe(2); + expect((reader.listAuditEvents({ action: 'groups.config' }) as unknown[]).length).toBe(1); + expect((reader.listAuditEvents({ action: 'groups.list' }) as unknown[]).length).toBe(1); + // Prefix means dotted segments, not substrings. + expect((reader.listAuditEvents({ action: 'group' }) as unknown[]).length).toBe(0); + }); + + it('applies --since/--until with relative and ISO forms', () => { + seedEvent(5, { event_id: 'old' }); + seedEvent(0, { event_id: 'recent' }); + + const relative = reader.listAuditEvents({ since: '2d' }) as Array>; + expect(relative.map((r) => r.event_id)).toEqual(['recent']); + + const iso = reader.listAuditEvents({ until: dayString(2) }) as Array>; + expect(iso.map((r) => r.event_id)).toEqual(['old']); + + expect(() => reader.listAuditEvents({ since: 'yesterdayish' })).toThrow('invalid --since'); + }); + + it('--format ndjson returns the stored lines verbatim', () => { + const seeded = seedEvent(0, { event_id: 'x1' }); + const out = reader.listAuditEvents({ format: 'ndjson' }); + expect(typeof out).toBe('string'); + expect(JSON.parse(out as string)).toEqual(seeded); + expect(() => reader.listAuditEvents({ format: 'csv' })).toThrow('invalid --format'); + }); + + it('skips malformed stored lines and still returns the rest', () => { + seedEvent(0, { event_id: 'good' }); + fs.appendFileSync(path.join(state.dataDir, 'audit', `${dayString(0)}.ndjson`), 'not-json\n'); + + const rows = reader.listAuditEvents({}) as Array>; + expect(rows.map((r) => r.event_id)).toEqual(['good']); + }); + + it('rejects an unknown --outcome value', () => { + expect(() => reader.listAuditEvents({ outcome: 'meh' })).toThrow('invalid --outcome'); + }); + + it('returns empty when nothing has been recorded yet', () => { + expect(reader.listAuditEvents({})).toEqual([]); + }); +}); diff --git a/.claude/skills/add-audit/add/src/audit/reader.ts b/.claude/skills/add-audit/add/src/audit/reader.ts new file mode 100644 index 000000000..a96c6453f --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/reader.ts @@ -0,0 +1,160 @@ +/** + * Read-back for `ncl audit list` — a newest-first stream-scan over the + * day-files. No index: fine at v1 volume, and adding one later doesn't change + * the store. NDJSON export returns the stored lines verbatim. + */ +import fs from 'fs'; +import path from 'path'; + +import { log } from '../log.js'; +import { AUDIT_ENABLED } from './config.js'; +import { AUDIT_DIR, utcDay } from './store.js'; +import type { AuditEvent, AuditOutcome } from './types.js'; + +const OUTCOMES: ReadonlySet = new Set(['success', 'failure', 'denied', 'pending', 'approved', 'rejected']); +const DAY_FILE_RE = /^(\d{4}-\d{2}-\d{2})\.ndjson$/; +const DEFAULT_LIMIT = 100; + +export interface AuditQuery { + actor?: string; + /** Exact action or dotted prefix (`groups` matches `groups.config.update`). */ + action?: string; + /** Matches any resources[] entry by id or by type. */ + resource?: string; + outcome?: AuditOutcome; + sinceMs?: number; + untilMs?: number; + correlation?: string; + limit: number; +} + +/** `7d` / `24h` / `30m` relative to now, or an ISO date/datetime (UTC). */ +export function parseTimeFlag(value: string, flag: string): number { + const rel = /^(\d+)([dhm])$/.exec(value); + if (rel) { + const n = Number(rel[1]); + const unitMs = rel[2] === 'd' ? 86_400_000 : rel[2] === 'h' ? 3_600_000 : 60_000; + return Date.now() - n * unitMs; + } + const abs = Date.parse(value); + if (!Number.isNaN(abs)) return abs; + throw new Error(`invalid ${flag} value "${value}" — use e.g. 7d, 24h, 30m, or an ISO date`); +} + +/** Newest first across files and within each file, up to q.limit. */ +export function queryAuditEvents(q: AuditQuery): { events: AuditEvent[]; lines: string[] } { + const events: AuditEvent[] = []; + const lines: string[] = []; + let malformed = 0; + + for (const { day, file } of dayFilesNewestFirst()) { + if (events.length >= q.limit) break; + // Whole-day skip: a file can't match a window its day lies outside. + if (q.sinceMs !== undefined && day < utcDay(new Date(q.sinceMs))) continue; + if (q.untilMs !== undefined && day > utcDay(new Date(q.untilMs))) continue; + + let content: string; + try { + content = fs.readFileSync(file, 'utf8'); + // eslint-disable-next-line no-catch-all/no-catch-all -- a torn/pruned day-file must not fail the whole query + } catch (err) { + log.warn('Audit reader failed to read day-file', { file, err }); + continue; + } + const fileLines = content.split('\n').filter((l) => l.trim() !== ''); + // Lines within a file are chronological — walk backwards for newest-first. + for (let i = fileLines.length - 1; i >= 0 && events.length < q.limit; i--) { + let event: AuditEvent; + try { + event = JSON.parse(fileLines[i]) as AuditEvent; + // eslint-disable-next-line no-catch-all/no-catch-all -- malformed stored lines are skipped (counted + warned below) + } catch { + malformed++; + continue; + } + if (!matches(event, q)) continue; + events.push(event); + lines.push(fileLines[i]); + } + } + + if (malformed > 0) { + log.warn('Audit reader skipped malformed lines', { malformed }); + } + return { events, lines }; +} + +function dayFilesNewestFirst(): Array<{ day: string; file: string }> { + let entries: string[]; + try { + entries = fs.readdirSync(AUDIT_DIR); + // eslint-disable-next-line no-catch-all/no-catch-all -- no audit dir yet means no events, not an error + } catch { + return []; + } + return entries + .map((e) => DAY_FILE_RE.exec(e)) + .filter((m): m is RegExpExecArray => m !== null) + .map((m) => ({ day: m[1], file: path.join(AUDIT_DIR, m[0]) })) + .sort((a, b) => (a.day < b.day ? 1 : -1)); +} + +function matches(event: AuditEvent, q: AuditQuery): boolean { + if (q.actor !== undefined && event.actor?.id !== q.actor) return false; + if (q.action !== undefined && event.action !== q.action && !event.action?.startsWith(q.action + '.')) return false; + if (q.outcome !== undefined && event.outcome !== q.outcome) return false; + if (q.correlation !== undefined && event.correlation_id !== q.correlation) return false; + if (q.resource !== undefined) { + const hit = (event.resources ?? []).some((r) => r.id === q.resource || r.type === q.resource); + if (!hit) return false; + } + const t = Date.parse(event.time ?? ''); + if (q.sinceMs !== undefined && !(t >= q.sinceMs)) return false; + if (q.untilMs !== undefined && !(t <= q.untilMs)) return false; + return true; +} + +/** + * `ncl audit list` handler. Disabled → an explicit error: an empty list would + * read as "no actions happened", which is a different truth than "not + * recording". `--format ndjson` returns the stored lines verbatim (the human + * formatter passes strings through); default returns flat rows for the table. + */ +export function listAuditEvents(args: Record): string | Array> { + if (!AUDIT_ENABLED) { + throw new Error('audit log is disabled — set AUDIT_ENABLED=true'); + } + + const format = args.format !== undefined ? String(args.format) : ''; + if (format && format !== 'ndjson') { + throw new Error(`invalid --format "${format}" — only "ndjson" is supported`); + } + const outcome = args.outcome !== undefined ? String(args.outcome) : undefined; + if (outcome !== undefined && !OUTCOMES.has(outcome)) { + throw new Error(`invalid --outcome "${outcome}" — one of: ${[...OUTCOMES].join(', ')}`); + } + + const q: AuditQuery = { + actor: args.actor !== undefined ? String(args.actor) : undefined, + action: args.action !== undefined ? String(args.action) : undefined, + resource: args.resource !== undefined ? String(args.resource) : undefined, + outcome: outcome as AuditOutcome | undefined, + correlation: args.correlation !== undefined ? String(args.correlation) : undefined, + sinceMs: args.since !== undefined ? parseTimeFlag(String(args.since), '--since') : undefined, + untilMs: args.until !== undefined ? parseTimeFlag(String(args.until), '--until') : undefined, + limit: args.limit !== undefined ? Math.max(1, Number(args.limit) || DEFAULT_LIMIT) : DEFAULT_LIMIT, + }; + + const { events, lines } = queryAuditEvents(q); + if (format === 'ndjson') return lines.join('\n'); + + return events.map((e) => ({ + time: e.time, + actor: e.actor?.id ?? '', + action: e.action, + resources: (e.resources ?? []).map((r) => (r.id ? `${r.type}:${r.id}` : r.type)).join(' '), + outcome: e.outcome, + correlation: e.correlation_id ?? '', + event_id: e.event_id, + })); +} diff --git a/.claude/skills/add-audit/add/src/audit/redact.test.ts b/.claude/skills/add-audit/add/src/audit/redact.test.ts new file mode 100644 index 000000000..68ad88132 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/redact.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest'; + +import { redactDetails } from './redact.js'; + +describe('redactDetails', () => { + it('masks sensitive keys at any depth, case-insensitively', () => { + const out = redactDetails({ + name: 'notion', + env: { NOTION_TOKEN: 'secret-value', SAFE_VALUE: 'ok' }, + nested: { Authorization: 'Bearer abc', list: [{ 'api-key': 'k' }, { plain: 'p' }] }, + password: 'hunter2', + }); + expect(out).toEqual({ + name: 'notion', + env: { NOTION_TOKEN: '[REDACTED]', SAFE_VALUE: 'ok' }, + nested: { Authorization: '[REDACTED]', list: [{ 'api-key': '[REDACTED]' }, { plain: 'p' }] }, + password: '[REDACTED]', + }); + }); + + it('matches the documented key pattern (token|secret|key|password|credential|auth|bearer)', () => { + const out = redactDetails({ + access_token: 'x', + clientSecret: 'x', + ssh_key: 'x', + credentials: 'x', + oauth_flow: 'x', + bearerValue: 'x', + username: 'moshe', + }); + expect(Object.entries(out).filter(([, v]) => v === '[REDACTED]')).toHaveLength(6); + expect(out.username).toBe('moshe'); + }); + + it('never recurses into a masked key — the whole value is replaced', () => { + const out = redactDetails({ auth: { inner: 'visible?' } }); + expect(out.auth).toBe('[REDACTED]'); + }); + + it('truncates strings over 2 KB post-redaction', () => { + const long = 'a'.repeat(5000); + const out = redactDetails({ blob: long, short: 'b' }); + expect(out.blob).toBe('a'.repeat(2048) + '…[truncated]'); + expect(out.short).toBe('b'); + }); + + it('passes non-string scalars through untouched', () => { + const out = redactDetails({ n: 42, b: true, z: null, u: undefined }); + expect(out).toEqual({ n: 42, b: true, z: null, u: undefined }); + }); + + it('caps depth (cycle guard) without throwing', () => { + const cyclic: Record = { a: 1 }; + cyclic.self = cyclic; + const out = redactDetails(cyclic); + let cursor: unknown = out; + for (let i = 0; i < 20 && typeof cursor === 'object' && cursor !== null; i++) { + cursor = (cursor as Record).self; + } + expect(cursor).toBe('[MAX_DEPTH]'); + }); + + it('does not mutate the input', () => { + const input = { token: 'x', nested: { list: ['a'.repeat(3000)] } }; + const snapshot = JSON.parse(JSON.stringify(input)); + redactDetails(input); + expect(input).toEqual(snapshot); + }); +}); diff --git a/.claude/skills/add-audit/add/src/audit/redact.ts b/.claude/skills/add-audit/add/src/audit/redact.ts new file mode 100644 index 000000000..2db4f047b --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/redact.ts @@ -0,0 +1,32 @@ +/** + * The one redactor, applied to every event's `details` at the emit seam — + * guarding current and future surfaces by default. Key-pattern mask plus a + * per-value size cap; message bodies are never passed in (emit sites record + * shape only — the audit log is a governance record, not a chat archive). + */ + +const SENSITIVE_KEY = /(token|secret|key|password|credential|auth|bearer)/i; +const MAX_VALUE_CHARS = 2048; +const MAX_DEPTH = 8; + +export function redactDetails(details: Record): Record { + return redactObject(details, 0); +} + +function redactObject(obj: Record, depth: number): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(obj)) { + out[k] = SENSITIVE_KEY.test(k) ? '[REDACTED]' : redactValue(v, depth + 1); + } + return out; +} + +function redactValue(value: unknown, depth: number): unknown { + if (typeof value === 'string') { + return value.length > MAX_VALUE_CHARS ? `${value.slice(0, MAX_VALUE_CHARS)}…[truncated]` : value; + } + if (value === null || typeof value !== 'object') return value; + if (depth > MAX_DEPTH) return '[MAX_DEPTH]'; + if (Array.isArray(value)) return value.map((v) => redactValue(v, depth + 1)); + return redactObject(value as Record, depth); +} diff --git a/.claude/skills/add-audit/add/src/audit/store.test.ts b/.claude/skills/add-audit/add/src/audit/store.test.ts new file mode 100644 index 000000000..1d31958c1 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/store.test.ts @@ -0,0 +1,187 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Core config (DATA_DIR) and the audit config (toggles) are mocked so +// store/emit resolve a per-test temp DATA_DIR and audit switches. Getters +// keep the values live across vi.resetModules(). +const state = vi.hoisted(() => ({ dataDir: '', enabled: true, retention: 90 })); + +vi.mock('../config.js', () => ({ + get DATA_DIR() { + return state.dataDir; + }, +})); + +vi.mock('./config.js', () => ({ + get AUDIT_ENABLED() { + return state.enabled; + }, + get AUDIT_RETENTION_DAYS() { + return state.retention; + }, +})); + +vi.mock('../log.js', () => ({ + log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() }, +})); + +let store: typeof import('./store.js'); +let emit: typeof import('./emit.js'); +let log: (typeof import('../log.js'))['log']; + +beforeEach(async () => { + state.dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'audit-store-')); + state.enabled = true; + state.retention = 90; + vi.resetModules(); + store = await import('./store.js'); + emit = await import('./emit.js'); + log = (await import('../log.js')).log; + vi.clearAllMocks(); +}); + +afterEach(() => { + fs.chmodSync(path.join(state.dataDir), 0o700); + const auditDir = path.join(state.dataDir, 'audit'); + if (fs.existsSync(auditDir)) fs.chmodSync(auditDir, 0o700); + fs.rmSync(state.dataDir, { recursive: true, force: true }); +}); + +function auditDir(): string { + return path.join(state.dataDir, 'audit'); +} + +function writeDayFile(day: string, lines = 1): void { + fs.mkdirSync(auditDir(), { recursive: true }); + fs.writeFileSync(path.join(auditDir(), `${day}.ndjson`), '{"x":1}\n'.repeat(lines)); +} + +function dayString(daysAgo: number): string { + return store.utcDay(new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000)); +} + +const EVENT_INPUT = { + actor: { type: 'human' as const, id: 'host:test' }, + origin: { transport: 'socket' as const }, + action: 'groups.list', + resources: [{ type: 'agent_group' }], + outcome: 'success' as const, +}; + +describe('appendAuditLine', () => { + it("appends one line to today's UTC day-file, creating the directory lazily", () => { + expect(fs.existsSync(auditDir())).toBe(false); + store.appendAuditLine('{"a":1}'); + store.appendAuditLine('{"b":2}'); + const file = path.join(auditDir(), `${store.utcDay()}.ndjson`); + expect(fs.readFileSync(file, 'utf8')).toBe('{"a":1}\n{"b":2}\n'); + }); +}); + +describe('emitAuditEvent', () => { + it('writes a schema_version-1 record with envelope fields and null enrichment', () => { + emit.emitAuditEvent({ ...EVENT_INPUT, details: { limit: 100 } }); + const file = path.join(auditDir(), `${store.utcDay()}.ndjson`); + const record = JSON.parse(fs.readFileSync(file, 'utf8').trim()); + expect(record).toMatchObject({ + schema_version: 1, + actor: { type: 'human', id: 'host:test', email: null, user_id: null, group_ids: null }, + origin: { transport: 'socket' }, + action: 'groups.list', + outcome: 'success', + correlation_id: null, + details: { limit: 100 }, + }); + expect(record.event_id).toMatch(/^[0-9a-f-]{36}$/); + expect(new Date(record.time).toISOString()).toBe(record.time); + }); + + it('redacts details at the emit seam', () => { + emit.emitAuditEvent({ ...EVENT_INPUT, details: { env: { API_TOKEN: 'x' } } }); + const file = path.join(auditDir(), `${store.utcDay()}.ndjson`); + expect(fs.readFileSync(file, 'utf8')).toContain('"API_TOKEN":"[REDACTED]"'); + }); + + it('is a no-op when audit is disabled — the directory is never created', () => { + state.enabled = false; + emit.emitAuditEvent(EVENT_INPUT); + expect(fs.existsSync(auditDir())).toBe(false); + }); + + it('fails open and loud when the append fails', () => { + fs.mkdirSync(auditDir(), { recursive: true }); + fs.chmodSync(auditDir(), 0o500); + expect(() => emit.emitAuditEvent(EVENT_INPUT)).not.toThrow(); + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('Audit append failed'), + expect.objectContaining({ action: 'groups.list' }), + ); + }); +}); + +describe('assertAuditWritable', () => { + it('creates the directory and probes it with a zero-byte append', () => { + store.assertAuditWritable(); + expect(fs.existsSync(path.join(auditDir(), `${store.utcDay()}.ndjson`))).toBe(true); + }); + + it('throws when the directory is not writable', () => { + fs.mkdirSync(auditDir(), { recursive: true }); + fs.chmodSync(auditDir(), 0o500); + expect(() => store.assertAuditWritable()).toThrow(); + }); +}); + +describe('pruneAuditLog', () => { + it('unlinks only day-files strictly older than the horizon', () => { + writeDayFile(dayString(100)); + writeDayFile(dayString(91)); + writeDayFile(dayString(90)); + writeDayFile(dayString(1)); + fs.writeFileSync(path.join(auditDir(), 'not-a-day-file.txt'), 'keep'); + fs.writeFileSync(path.join(auditDir(), '2026-01-01.ndjson.bak'), 'keep'); + store.pruneAuditLog(90); + const left = fs.readdirSync(auditDir()).sort(); + expect(left).toEqual( + [`${dayString(90)}.ndjson`, `${dayString(1)}.ndjson`, '2026-01-01.ndjson.bak', 'not-a-day-file.txt'].sort(), + ); + }); + + it('keeps forever when retention is 0', () => { + writeDayFile(dayString(400)); + store.pruneAuditLog(0); + expect(fs.existsSync(path.join(auditDir(), `${dayString(400)}.ndjson`))).toBe(true); + }); + + it('is a no-op when the audit directory does not exist', () => { + expect(() => store.pruneAuditLog(90)).not.toThrow(); + }); +}); + +describe('pruneAuditLogIfDue', () => { + it('prunes at most once per UTC day', () => { + writeDayFile(dayString(100)); + store.pruneAuditLogIfDue(); + expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(false); + writeDayFile(dayString(100)); + store.pruneAuditLogIfDue(); + expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(true); + }); + + it('does nothing when audit is disabled', () => { + state.enabled = false; + writeDayFile(dayString(100)); + store.pruneAuditLogIfDue(); + expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(true); + }); + + it('does nothing after markPrunedToday until the day rolls over', () => { + store.markPrunedToday(); + writeDayFile(dayString(100)); + store.pruneAuditLogIfDue(); + expect(fs.existsSync(path.join(auditDir(), `${dayString(100)}.ndjson`))).toBe(true); + }); +}); diff --git a/.claude/skills/add-audit/add/src/audit/store.ts b/.claude/skills/add-audit/add/src/audit/store.ts new file mode 100644 index 000000000..fab1b3b56 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/store.ts @@ -0,0 +1,81 @@ +/** + * Day-file store — daily NDJSON files under data/audit/, host process is the + * single writer. Append-only is structural: nothing here can update a line, + * and retention is unlinking whole files (a literal hard delete). Both + * transports converge host-side, so single-writer holds by construction. + * + * This module throws on fs failure; the fail-open posture (log.error, action + * proceeds) lives in emit.ts, and the boot-time strictness (refuse to start) + * lives in init.ts. + */ +import fs from 'fs'; +import path from 'path'; + +import { DATA_DIR } from '../config.js'; +import { log } from '../log.js'; +import { AUDIT_ENABLED, AUDIT_RETENTION_DAYS } from './config.js'; + +export const AUDIT_DIR = path.join(DATA_DIR, 'audit'); +const DAY_FILE_RE = /^(\d{4}-\d{2}-\d{2})\.ndjson$/; +const DAY_MS = 24 * 60 * 60 * 1000; + +export function utcDay(d: Date = new Date()): string { + return d.toISOString().slice(0, 10); +} + +export function dayFilePath(day: string): string { + return path.join(AUDIT_DIR, `${day}.ndjson`); +} + +/** The single append point. Throws on fs failure — emitAuditEvent catches. */ +export function appendAuditLine(line: string): void { + fs.mkdirSync(AUDIT_DIR, { recursive: true }); + fs.appendFileSync(dayFilePath(utcDay()), line + '\n'); +} + +/** Boot-time writability assert — a zero-byte append is a true write probe. */ +export function assertAuditWritable(): void { + fs.mkdirSync(AUDIT_DIR, { recursive: true }); + fs.appendFileSync(dayFilePath(utcDay()), ''); +} + +/** Unlink day-files strictly older than (today UTC − retentionDays). 0 or negative = keep forever. */ +export function pruneAuditLog(retentionDays: number = AUDIT_RETENTION_DAYS): void { + if (retentionDays <= 0) return; + let entries: string[]; + try { + entries = fs.readdirSync(AUDIT_DIR); + // eslint-disable-next-line no-catch-all/no-catch-all -- no audit dir yet means nothing to prune, not an error + } catch { + return; + } + const horizon = utcDay(new Date(Date.now() - retentionDays * DAY_MS)); + let pruned = 0; + for (const entry of entries) { + const m = DAY_FILE_RE.exec(entry); + if (!m || m[1] >= horizon) continue; + try { + fs.unlinkSync(path.join(AUDIT_DIR, entry)); + pruned++; + // eslint-disable-next-line no-catch-all/no-catch-all -- one stuck file must not stop the prune of the others + } catch (err) { + log.error('Audit prune failed to unlink day-file', { file: entry, err }); + } + } + if (pruned > 0) log.info('Audit retention pruned day-files', { pruned, retentionDays }); +} + +let lastPruneDay: string | null = null; + +/** Retention prune, throttled to once per UTC day — safe to call every tick. */ +export function pruneAuditLogIfDue(): void { + if (!AUDIT_ENABLED || AUDIT_RETENTION_DAYS <= 0) return; + const today = utcDay(); + if (lastPruneDay === today) return; + lastPruneDay = today; + pruneAuditLog(); +} + +export function markPrunedToday(): void { + lastPruneDay = utcDay(); +} diff --git a/.claude/skills/add-audit/add/src/audit/types.ts b/.claude/skills/add-audit/add/src/audit/types.ts new file mode 100644 index 000000000..fb345cbfc --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/types.ts @@ -0,0 +1,60 @@ +/** + * Canonical audit event — one flat, SIEM-shaped record per action. + * + * Fields are chosen so they project losslessly onto OCSF and Elastic ECS + * (the two schemas SIEMs converge on); the store keeps the neutral shape and + * a future forwarder pays the translation once, at the edge. `schema_version` + * covers evolution — additive changes only within a version. + */ + +export type AuditActorType = 'human' | 'agent' | 'system'; + +export interface AuditActor { + type: AuditActorType; + /** `host:` | `:` | agent group id | `host` (system). */ + id: string; +} + +export interface AuditOrigin { + transport: 'socket' | 'container' | 'channel'; + session_id?: string; + messaging_group_id?: string; + channel?: string; +} + +export interface AuditResource { + type: string; + /** Omitted when only the attempted type is known (e.g. a denied list). */ + id?: string; +} + +export type AuditOutcome = 'success' | 'failure' | 'denied' | 'pending' | 'approved' | 'rejected'; + +/** What emit sites provide. Envelope fields are stamped by emitAuditEvent. */ +export interface AuditEventInput { + actor: AuditActor; + origin: AuditOrigin; + /** Dotted namespaced verb, e.g. `groups.config.add-mcp-server`. */ + action: string; + resources: AuditResource[]; + outcome: AuditOutcome; + /** The approval id on gated chains; null/omitted otherwise. */ + correlationId?: string | null; + details?: Record; +} + +export interface AuditEvent { + event_id: string; + time: string; + schema_version: 1; + actor: AuditActor & { email: null; user_id: null; group_ids: null }; + origin: AuditOrigin; + action: string; + resources: AuditResource[]; + outcome: AuditOutcome; + correlation_id: string | null; + details: Record; +} + +/** Actor for timer/sweep-driven outcomes (expiries, startup sweeps). */ +export const SYSTEM_ACTOR: AuditActor = { type: 'system', id: 'host' }; diff --git a/.claude/skills/add-audit/add/src/audit/vocab.ts b/.claude/skills/add-audit/add/src/audit/vocab.ts new file mode 100644 index 000000000..fd79fd166 --- /dev/null +++ b/.claude/skills/add-audit/add/src/audit/vocab.ts @@ -0,0 +1,39 @@ +/** + * Domain-free event vocabulary — actor and origin constructors shared by the + * domain-owned `*.audit.ts` adapters (the CLI adapter today; approval and + * channel adapters attach here in later increments). Pure derivation; nothing + * here writes the log. + * + * Leaf rule: this module (like the rest of src/audit/) may depend on node, + * config/log, shared types, and the db read layer — never on src/cli/* or + * src/modules/*. Domain-specific mapping (CLI resources, approval payloads) + * lives in the adapter file of the domain that owns it. + */ +import os from 'os'; + +import { getMessagingGroup } from '../db/messaging-groups.js'; +import type { AuditOrigin } from './types.js'; + +/** + * Host callers stamp `host:` daemon-side: the ncl socket is + * 0600 and owned by the install user, so the identity is accurate by + * construction without peer credentials. + */ +export function hostUser(): string { + try { + return os.userInfo().username; + // eslint-disable-next-line no-catch-all/no-catch-all -- os.userInfo throws on exotic hosts; a fallback actor id beats no audit event + } catch { + return process.env.USER || 'unknown'; + } +} + +export function containerOrigin(sessionId: string, messagingGroupId: string | null): AuditOrigin { + const origin: AuditOrigin = { transport: 'container', session_id: sessionId }; + if (messagingGroupId) { + origin.messaging_group_id = messagingGroupId; + const channel = getMessagingGroup(messagingGroupId)?.channel_type; + if (channel) origin.channel = channel; + } + return origin; +} diff --git a/.claude/skills/add-audit/add/src/cli/dispatch.audit.test.ts b/.claude/skills/add-audit/add/src/cli/dispatch.audit.test.ts new file mode 100644 index 000000000..a6e32e202 --- /dev/null +++ b/.claude/skills/add-audit/add/src/cli/dispatch.audit.test.ts @@ -0,0 +1,297 @@ +/** + * Audit middleware behavior of the exported dispatch — what gets recorded, + * for whom, and how gated chains correlate. Drives the real wrapped dispatch + * (real registry, real guard); audit is force-enabled and the store's append + * is captured. DB reads and approval delivery are mocked. + */ +import os from 'os'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { PendingApproval } from '../types.js'; + +const appended = vi.hoisted(() => ({ lines: [] as string[] })); +const pendingRows = vi.hoisted(() => ({ rows: [] as unknown[] })); + +vi.mock('../audit/config.js', () => ({ + AUDIT_ENABLED: true, + AUDIT_RETENTION_DAYS: 90, +})); + +// Neutralize the adapter's module-scope boot (writability assert, prune, +// maintenance timer) — the middleware is the unit under test here. +vi.mock('../audit/init.js', () => ({ + initAuditLog: vi.fn(), + maintainAudit: vi.fn(), +})); + +vi.mock('../audit/store.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + appendAuditLine: (line: string) => { + appended.lines.push(line); + }, + }; +}); + +vi.mock('../log.js', () => ({ + log: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() }, +})); + +const mockGetContainerConfig = vi.fn(); +vi.mock('../db/container-configs.js', () => ({ + getContainerConfig: (...args: unknown[]) => mockGetContainerConfig(...args), +})); + +vi.mock('../db/agent-groups.js', () => ({ + getAgentGroup: vi.fn(() => ({ id: 'g1', name: 'Group One' })), +})); + +const mockGetPendingApproval = vi.fn(); +vi.mock('../db/sessions.js', () => ({ + getSession: vi.fn(() => ({ id: 's1', agent_group_id: 'g1', messaging_group_id: 'mg1' })), + getPendingApproval: (...args: unknown[]) => mockGetPendingApproval(...args), + getPendingApprovalsByAction: () => pendingRows.rows, +})); + +vi.mock('../db/messaging-groups.js', () => ({ + getMessagingGroup: vi.fn(() => ({ channel_type: 'slack' })), +})); + +const mockGetResource = vi.fn(); +vi.mock('./crud.js', () => ({ + getResource: (...args: unknown[]) => mockGetResource(...args), +})); + +vi.mock('../modules/approvals/index.js', () => ({ + registerApprovalHandler: vi.fn(), + requestApproval: vi.fn(async () => undefined), +})); + +import { register } from './registry.js'; + +register({ + name: 'groups-test', + description: 'echo command on the groups resource', + action: 'groups.test', + resource: 'groups', + access: 'open', + parseArgs: (raw) => raw, + handler: async (args) => ({ echo: args }), +}); + +register({ + name: 'groups-get', + description: 'echo command for dash-joined id resolution', + action: 'groups.get', + resource: 'groups', + access: 'open', + parseArgs: (raw) => raw, + handler: async (args) => ({ echo: args }), +}); + +register({ + name: 'wirings-list', + description: 'not on the group-scope allowlist', + action: 'wirings.list', + resource: 'wirings', + access: 'open', + parseArgs: (raw) => raw, + handler: async () => [], +}); + +register({ + name: 'groups-fail', + description: 'handler that throws', + action: 'groups.fail', + resource: 'groups', + access: 'open', + parseArgs: (raw) => raw, + handler: async () => { + throw new Error('boom'); + }, +}); + +register({ + name: 'groups-gated', + description: 'approval-gated command', + action: 'groups.gated', + resource: 'groups', + access: 'approval', + parseArgs: (raw) => raw, + handler: async () => 'ran', +}); + +import { dispatch } from './dispatch.js'; +import type { CallerContext } from './frame.js'; + +const AGENT_CTX: CallerContext = { caller: 'agent', sessionId: 's1', agentGroupId: 'g1', messagingGroupId: 'mg1' }; + +function grantRow(frameId: string, command: string): PendingApproval { + return { + approval_id: 'appr-123-abc', + session_id: 's1', + request_id: 'appr-123-abc', + action: 'cli_command', + payload: JSON.stringify({ frame: { id: frameId, command, args: {} }, callerContext: AGENT_CTX }), + created_at: new Date().toISOString(), + agent_group_id: 'g1', + channel_type: null, + platform_id: null, + platform_message_id: null, + expires_at: null, + status: 'pending', + title: 'CLI: groups-gated', + options_json: '[]', + approver_user_id: null, + }; +} + +function events(): Array> { + return appended.lines.map((l) => JSON.parse(l)); +} + +beforeEach(() => { + vi.clearAllMocks(); + appended.lines.length = 0; + pendingRows.rows = []; + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetResource.mockImplementation((plural: string) => (plural === 'groups' ? { scopeField: 'id' } : undefined)); +}); + +describe('withAudit(dispatch)', () => { + it('records a success event for a host caller with socket origin and host actor', async () => { + const resp = await dispatch({ id: '1', command: 'groups-test', args: { foo: 'bar' } }, { caller: 'host' }); + + expect(resp.ok).toBe(true); + const [event] = events(); + expect(event).toMatchObject({ + schema_version: 1, + actor: { type: 'human', id: `host:${os.userInfo().username}`, email: null }, + origin: { transport: 'socket' }, + action: 'groups.test', + outcome: 'success', + correlation_id: null, + details: { foo: 'bar' }, + }); + }); + + it('records effective args after group auto-fill, with container origin and channel', async () => { + await dispatch({ id: '1', command: 'groups-test', args: {} }, AGENT_CTX); + + const [event] = events(); + expect(event.actor).toMatchObject({ type: 'agent', id: 'g1' }); + expect(event.origin).toEqual({ + transport: 'container', + session_id: 's1', + messaging_group_id: 'mg1', + channel: 'slack', + }); + expect(event.details).toMatchObject({ id: 'g1', agent_group_id: 'g1', group: 'g1' }); + expect(event.resources).toContainEqual({ type: 'agent_group', id: 'g1' }); + }); + + it('records a denied event for a scope denial, naming the attempted resource type', async () => { + const resp = await dispatch({ id: '1', command: 'wirings-list', args: {} }, AGENT_CTX); + + expect(resp.ok).toBe(false); + const [event] = events(); + expect(event).toMatchObject({ + action: 'wirings.list', + outcome: 'denied', + resources: [{ type: 'wirings' }], + details: { error: 'forbidden' }, + }); + expect(event.details.reason).toContain('scoped'); + }); + + it('records a failure event when the handler throws', async () => { + await dispatch({ id: '1', command: 'groups-fail', args: {} }, { caller: 'host' }); + + const [event] = events(); + expect(event).toMatchObject({ action: 'groups.fail', outcome: 'failure', details: { error: 'handler-error' } }); + expect(event.details.reason).toContain('boom'); + }); + + it('records a hold as a pending event correlated to the approval row it created', async () => { + pendingRows.rows = [grantRow('1', 'groups-gated')]; + + const resp = await dispatch({ id: '1', command: 'groups-gated', args: {} }, AGENT_CTX); + + expect(resp.ok).toBe(false); + if (!resp.ok) expect(resp.error.code).toBe('approval-pending'); + const [event] = events(); + expect(event).toMatchObject({ + action: 'groups.gated', + outcome: 'pending', + correlation_id: 'appr-123-abc', + }); + expect(event.resources).toContainEqual({ type: 'approval', id: 'appr-123-abc' }); + expect(event.details.error).toBeUndefined(); + }); + + it('records an uncorrelated pending event when no approval row was created (no approver)', async () => { + pendingRows.rows = []; + + await dispatch({ id: '1', command: 'groups-gated', args: {} }, AGENT_CTX); + + const [event] = events(); + expect(event).toMatchObject({ outcome: 'pending', correlation_id: null }); + }); + + it('records an approved replay as success with the grant approval id as correlation_id', async () => { + const grant = grantRow('9', 'groups-gated'); + mockGetPendingApproval.mockReturnValue(grant); + + const resp = await dispatch({ id: '9', command: 'groups-gated', args: {} }, AGENT_CTX, { grant }); + + expect(resp.ok).toBe(true); + const [event] = events(); + expect(event).toMatchObject({ + action: 'groups.gated', + outcome: 'success', + correlation_id: 'appr-123-abc', + }); + expect(event.resources).toContainEqual({ type: 'approval', id: 'appr-123-abc' }); + }); + + it('records unknown commands as cli.unknown-command with the raw name in details', async () => { + await dispatch({ id: '1', command: 'nope-nothing', args: {} }, { caller: 'host' }); + + const [event] = events(); + expect(event).toMatchObject({ + action: 'cli.unknown-command', + outcome: 'failure', + resources: [], + details: { command: 'nope-nothing', error: 'unknown-command' }, + }); + }); + + it('records the resolved command and id for dash-joined positional ids', async () => { + const uuid = '550e8400-e29b-41d4-a716-446655440000'; + await dispatch({ id: '1', command: `groups-get-${uuid}`, args: {} }, { caller: 'host' }); + + const [event] = events(); + expect(event).toMatchObject({ action: 'groups.get', outcome: 'success' }); + expect(event.resources).toContainEqual({ type: 'agent_group', id: uuid }); + expect(event.details.id).toBe(uuid); + }); + + it('normalizes hyphenated arg keys in details', async () => { + await dispatch({ id: '1', command: 'groups-test', args: { 'dry-run': 'true' } }, { caller: 'host' }); + + const [event] = events(); + expect(event.details).toMatchObject({ dry_run: 'true' }); + }); + + it('parses JSON string args so the redactor reaches their inner keys', async () => { + await dispatch( + { id: '1', command: 'groups-test', args: { env: '{"NOTION_TOKEN":"tok-123","SAFE":"ok"}', note: '{not json' } }, + { caller: 'host' }, + ); + + const [event] = events(); + expect(event.details.env).toEqual({ NOTION_TOKEN: '[REDACTED]', SAFE: 'ok' }); + expect(event.details.note).toBe('{not json'); + }); +}); diff --git a/.claude/skills/add-audit/add/src/cli/dispatch.audit.ts b/.claude/skills/add-audit/add/src/cli/dispatch.audit.ts new file mode 100644 index 000000000..e2eca6726 --- /dev/null +++ b/.claude/skills/add-audit/add/src/cli/dispatch.audit.ts @@ -0,0 +1,238 @@ +/** + * CLI audit adapter (installed by /add-audit) — owns how the dispatcher + * describes itself to the audit log: the dispatch middleware plus the + * CLI-specific actor/origin/resource mapping. Composed in dispatch.ts as + * `export const dispatch = withAudit(dispatchInner)`; business logic there + * contains zero audit calls. + * + * Loading this module also boots the audit log (writability assert, boot + * prune, hook lifecycle, maintenance timer): dispatch.ts is imported by both + * transports during the host's barrel phase, so initAuditLog() runs before + * any command is accepted — and an enabled box with an unwritable + * data/audit/ refuses to start. + */ +import { emitAuditEvent } from '../audit/emit.js'; +import { initAuditLog } from '../audit/init.js'; +import { type AuditActor, type AuditOrigin, type AuditOutcome, type AuditResource } from '../audit/types.js'; +import { containerOrigin, hostUser } from '../audit/vocab.js'; +import { getContainerConfig } from '../db/container-configs.js'; +import { getPendingApprovalsByAction } from '../db/sessions.js'; +import type { PendingApproval } from '../types.js'; +import { getResource } from './crud.js'; +import type { CallerContext, RequestFrame, ResponseFrame } from './frame.js'; +import { type CommandDef, lookup } from './registry.js'; + +initAuditLog(); + +// ── CLI mapping ── + +/** + * Host callers stamp `host:` daemon-side (the ncl socket is + * 0600 and owned by the install user); container callers are their agent group. + */ +export function actorForCaller(ctx: CallerContext): AuditActor { + return ctx.caller === 'host' ? { type: 'human', id: `host:${hostUser()}` } : { type: 'agent', id: ctx.agentGroupId }; +} + +export function originForCaller(ctx: CallerContext): AuditOrigin { + if (ctx.caller === 'host') return { transport: 'socket' }; + return containerOrigin(ctx.sessionId, ctx.messagingGroupId || null); +} + +/** + * Frame-level args use `--hyphen-keys`; recorded details use the same + * underscore form the parsed handlers see. Mirrors crud's normalizeArgs + * (kept local so audit doesn't depend on a module tests commonly mock). + */ +export function normalizeArgKeys(raw: Record): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(raw)) { + out[k.replace(/-/g, '_')] = v; + } + return out; +} + +/** CLI resource plural → audit resource type, where the singular isn't it. */ +const RESOURCE_TYPE_OVERRIDES: Record = { + groups: 'agent_group', + 'messaging-groups': 'messaging_group', + 'dropped-messages': 'dropped_message', + 'user-dms': 'user_dm', +}; + +/** + * Derive touched/attempted resources from a command's effective args. Generic + * by design: `id` → the command's own resource, group/user args → their + * types, and a bare `{type}` entry when nothing else is known (a denied + * `users list` still names what was attempted). + */ +export function resourcesForCli(cmd: CommandDef, args: Record): AuditResource[] { + if (!cmd.resource) return []; + const type = RESOURCE_TYPE_OVERRIDES[cmd.resource] ?? getResource(cmd.resource)?.name ?? cmd.resource; + + const out: AuditResource[] = []; + const push = (t: string, id: unknown): void => { + if (typeof id !== 'string' || !id) return; + if (!out.some((r) => r.type === t && r.id === id)) out.push({ type: t, id }); + }; + push(type, args.id); + push('agent_group', args.agent_group_id ?? args.group); + push('user', args.user); + if (out.length === 0) out.push({ type }); + return out; +} + +// ── Dispatch mechanics, mirrored for the record ── +// Dispatch resolves the command and auto-fills args on a local copy of the +// frame that never leaves it, so the middleware mirrors the two documented +// mechanics below. The copies are mechanical, and drift only ever degrades a +// record's detail (a fallback action name, a missing auto-filled arg) — never +// dispatch behavior, and never an outcome. + +/** + * Mirror of dispatch's command resolution: exact lookup, then the longest + * registered dash-prefix with the remainder recorded as --id. + */ +function resolveForRecord(req: RequestFrame): { cmd?: CommandDef; args: Record } { + const direct = lookup(req.command); + if (direct) return { cmd: direct, args: req.args }; + let shortened = req.command; + let idx: number; + while ((idx = shortened.lastIndexOf('-')) > 0) { + shortened = shortened.slice(0, idx); + const fallback = lookup(shortened); + if (fallback) { + const tail = req.command.slice(shortened.length + 1); + return { cmd: fallback, args: { ...req.args, id: req.args.id ?? tail } }; + } + } + return { args: req.args }; +} + +/** + * Mirror of dispatch's group-scope arg auto-fill, so the record shows the + * effective args the handler saw (e.g. which agent group a bare + * `sessions list` actually listed). + */ +function effectiveArgs( + cmd: CommandDef | undefined, + args: Record, + ctx: CallerContext, +): Record { + if (ctx.caller !== 'agent') return args; + if ((getContainerConfig(ctx.agentGroupId)?.cli_scope ?? 'group') !== 'group') return args; + const fill: Record = { + agent_group_id: args.agent_group_id ?? ctx.agentGroupId, + group: args.group ?? ctx.agentGroupId, + }; + if (cmd?.resource === 'groups' || cmd?.resource === 'destinations') { + fill.id = args.id ?? ctx.agentGroupId; + } + return { ...args, ...fill }; +} + +/** + * CLI args arrive as strings, so a JSON-object/array value (e.g. + * `--env '{"NOTION_TOKEN":"…"}'`) would reach the redactor as one opaque + * string under an innocent key and its inner secret keys would survive. + * Recording the parsed form lets the redactor walk inside — the audit log's + * "secrets never land" property depends on it for exactly this flow. + */ +function parseJsonishValues(args: Record): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(args)) { + if (typeof v === 'string' && (v.startsWith('{') || v.startsWith('['))) { + try { + out[k] = JSON.parse(v); + continue; + // eslint-disable-next-line no-catch-all/no-catch-all -- not JSON after all: record the string as-is + } catch { + /* fall through */ + } + } + out[k] = v; + } + return out; +} + +/** + * The approval row a hold just created for this frame — it gives the pending + * event the same correlation_id the approved replay will carry as its guard + * grant. requestApproval keeps the minted id internal, so the row is + * recovered by the frame id it stored in its payload; no row (e.g. no + * configured approver) → the hold is still recorded, uncorrelated. + */ +function holdApprovalIdFor(frameId: string): string | null { + const rows = getPendingApprovalsByAction('cli_command'); + for (let i = rows.length - 1; i >= 0; i--) { + try { + const payload = JSON.parse(rows[i].payload) as { frame?: { id?: string } }; + if (payload.frame?.id === frameId) return rows[i].approval_id; + // eslint-disable-next-line no-catch-all/no-catch-all -- a row with an unparseable payload is simply not this frame's hold + } catch { + continue; + } + } + return null; +} + +// ── The dispatch middleware ── + +type DispatchInner = ( + req: RequestFrame, + ctx: CallerContext, + opts?: { grant?: PendingApproval }, +) => Promise; + +/** + * Dispatch middleware — the exported `dispatch` is the wrapped function, so + * the socket server, the container delivery-action, and the in-module + * approved replay are all covered by the one composition. + * + * Outcome derives from the response frame: ok → success, forbidden → denied + * (captures pre-handler scope denials), approval-pending → pending (the + * record of a hold), anything else → failure. Correlation is the approval id: + * an approved replay carries the approval row as its guard grant + * (opts.grant), and a fresh hold is correlated by recovering the row it just + * created — so `--correlation ` returns the whole gated chain. + */ +export function withAudit(inner: DispatchInner): DispatchInner { + return async (req, ctx, opts = {}) => { + const res = await inner(req, ctx, opts); + emitAuditEvent(() => { + const resolved = resolveForRecord(req); + const cmd = resolved.cmd; + const pending = !res.ok && res.error.code === 'approval-pending'; + const outcome: AuditOutcome = res.ok + ? 'success' + : res.error.code === 'forbidden' + ? 'denied' + : pending + ? 'pending' + : 'failure'; + // A denial records the attempt as asked (raw args): nothing ran, so the + // auto-fill never conceptually happened — and a cross-group attempt + // must show the foreign id the caller passed, not a filled-in own-group. + const args = normalizeArgKeys(outcome === 'denied' ? resolved.args : effectiveArgs(cmd, resolved.args, ctx)); + const correlationId = opts.grant?.approval_id ?? (pending ? holdApprovalIdFor(req.id) : null); + const details: Record = parseJsonishValues(args); + if (!res.ok && !pending) { + details.error = res.error.code; + details.reason = res.error.message; + } + if (!cmd) details.command = req.command; + const resources = cmd ? resourcesForCli(cmd, args) : []; + if (correlationId) resources.push({ type: 'approval', id: correlationId }); + return { + actor: actorForCaller(ctx), + origin: originForCaller(ctx), + action: cmd ? (cmd.action ?? `cli.${cmd.name}`) : 'cli.unknown-command', + resources, + outcome, + correlationId, + details, + }; + }); + return res; + }; +} diff --git a/.claude/skills/add-audit/add/src/cli/resources/audit.ts b/.claude/skills/add-audit/add/src/cli/resources/audit.ts new file mode 100644 index 000000000..da9e2298f --- /dev/null +++ b/.claude/skills/add-audit/add/src/cli/resources/audit.ts @@ -0,0 +1,56 @@ +import { listAuditEvents } from '../../audit/reader.js'; +import { registerResource } from '../crud.js'; + +/** + * Read-only audit resource (installed by /add-audit). The "table" is the + * NDJSON day-file store — no generic CRUD verbs are declared, so it is never + * queried as SQL; `list` is a custom operation backed by the audit reader. + * + * Deliberately NOT on the group-scope allowlist (GROUP_SCOPE_RESOURCES): + * audit spans agent groups, so group-scoped agents are refused before the + * handler — the resource is host + `cli_scope: global` only, by omission + * (fails closed). + */ +registerResource({ + name: 'audit_event', + plural: 'audit', + table: '(data/audit/*.ndjson)', + description: 'Local audit log — one event per ncl command. Newest first. Requires AUDIT_ENABLED=true.', + idColumn: 'event_id', + columns: [ + { name: 'actor', type: 'string', description: 'Filter: exact actor id (host:, :, agent group id)' }, + { name: 'action', type: 'string', description: 'Filter: exact action or dotted prefix (e.g. groups.config)' }, + { name: 'resource', type: 'string', description: 'Filter: matches any event resource by id or type' }, + { + name: 'outcome', + type: 'string', + description: 'Filter: event outcome', + enum: ['success', 'failure', 'denied', 'pending', 'approved', 'rejected'], + }, + { name: 'since', type: 'string', description: 'Window start: 7d / 24h / 30m relative, or ISO date' }, + { name: 'until', type: 'string', description: 'Window end: same formats as --since' }, + { name: 'correlation', type: 'string', description: 'Filter: approval id tying a gated chain together' }, + { name: 'limit', type: 'number', description: 'Max events (default 100, newest first)' }, + { name: 'format', type: 'string', description: 'Output format', enum: ['ndjson'] }, + ], + operations: {}, + customOperations: { + list: { + access: 'open', + description: 'Query audit events, newest first. --format ndjson streams the stored lines for SIEM export.', + examples: [ + 'ncl audit list --outcome denied --since 7d', + 'ncl audit list --correlation appr-1751970000000-x1y2z3', + 'ncl audit list --action groups.config --format ndjson', + ], + handler: async (args) => listAuditEvents(args), + formatHuman: (data) => { + // NDJSON export prints the stored lines verbatim; for the table view + // this throws so dispatch falls back to the row objects, which every + // client already renders. + if (typeof data === 'string') return data; + throw new Error('render rows client-side'); + }, + }, + }, +}); diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d8f2ab78..a38984f2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to NanoClaw will be documented in this file. ## [Unreleased] +- **New skill: `/add-audit` — opt-in local audit log for the `ncl` surface.** Installs an append-only, SIEM-shaped audit log: every command through the dispatcher (both transports — host socket and container — including scope denials, approval holds, and approved replays) becomes one canonical NDJSON event under `data/audit/.ndjson`. Gated chains share a `correlation_id` (the approval id: the hold's `pending` event and the replay's terminal event carry the same one); `details` pass a recursive secret-key redactor (~2 KB value cap). `AUDIT_RETENTION_DAYS` (default 90; `0` = keep forever) hard-deletes day-files past the horizon at boot and once per UTC day. Read back with `ncl audit list [--actor --action --resource --outcome --since --until --correlation --limit] [--format ndjson]` — host + global-scope callers only (the resource stays off the group-scope allowlist, so group-scoped agents fail closed). In-process exporters plug in via `registerAuditHook` — post-write hooks that fire only after the local append succeeds. Off by default: nothing is persisted (and `data/audit/` is never created) until `AUDIT_ENABLED=true`, an enabled box refuses to boot if the directory isn't writable, and a disabled box answers `ncl audit list` with a clear error instead of an empty list. The skill's whole core footprint is the dispatch composition (`export const dispatch = withAudit(dispatchInner)`) plus the resource-barrel import, both guarded by a shipped wiring test. Approval-lifecycle events (request/decision as their own events), OneCLI credential holds, and channel/sender events are later increments on the same schema. - **The guard seam.** Every privileged action crossing the container or channel boundary now passes ONE decision function — `guard()` in the new `src/guard/` leaf — before it executes: `allow`, `hold` (through the flow's existing approval mechanics), or `deny`. The keyed registries (ncl commands, delivery actions) wrap their handlers at registration, so the guarded path is the only path by construction: every ncl command derives its guard from its own definition inside `register()`, and a delivery action registers with a guard spec or an explicit `unguarded()` declaration — omission doesn't compile, and `grep "unguarded("` is the complete justified inventory. The broadcast hooks (response handlers, message interceptors) stay plain registrations; the one privileged click path — channel registration — consults the guard inline in its handler, like the a2a route and the unknown-sender gate. Consults carry the branded `GuardedAction` value returned by `defineGuardedAction`, so a typo'd or dropped wiring is a compile error and a forged value is denied at runtime — never a fail-open. The decision is today's checks verbatim (cli_scope enforcement, create_agent's scope branch, a2a destination ACL + `agent_message_policies` hold, self-mod unconditional hold, unknown_sender_policy, channel-registration click auth, host-as-trusted-caller in code). Policy-as-data (tighten-only rule sources composing with the decision) is deliberately deferred — a generalized rules table can arrive later, with its first operator-visible consumer. Approved replays now carry the verified approval row as a **grant** — the `approved: true` boolean is deleted — and the guard re-runs the structural checks on every replay. Two outcome changes are inherent to that replay semantics and ship deliberately: **(a)** approving a held a2a message after its destination was revoked no longer delivers it (the checks re-run live); **(b)** a forged, already-consumed, or wrong-action/wrong-payload grant refuses the replay instead of executing (previously any in-process caller passing `approved: true` executed). Zero rules ship by default — the seeded posture is today's behavior. (The D1/D2/D4 click-auth fixes from the guarded-actions defect inventory are NOT in this change — they belong to the approval-contract PR.) - **Delivery-registry hardening.** Re-registering a guard-wrapped delivery action *without* a guard spec now throws at registration — previously that silently replaced the guarded handler while the catalog still reported the action as guarded. (The cross-registry pairing — every holding action has a registered approve continuation — is enforced by the conformance test; at runtime a missing continuation already resolves loudly, telling the requester no handler is installed.) - [BREAKING] **`whatsapp-formatting` and `slack-formatting` container skills moved from trunk to the `channels` branch.** They now install with their channel — `/add-whatsapp` / `/add-slack` and the setup installers copy them in — so installs without those channels stop carrying channel-specific formatting instructions in every agent's context. **Migration — only if this install has the channel wired** (check `src/channels/whatsapp.ts` / `src/channels/slack.ts`): updating removes both skills from the working tree — WhatsApp agents lose the formatting fragment from their composed CLAUDE.md on next spawn, Slack agents lose the mrkdwn skill from `~/.claude/skills`. Re-run the matching skill, `/add-whatsapp` or `/add-slack` (idempotent), to restore. Installs without the channel need nothing — do NOT run the add-skill just in case; it installs the full channel adapter.