diff --git a/CLAUDE.md b/CLAUDE.md index 61ba49f7a..abeeaafcc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,7 +66,7 @@ For ad-hoc queries from skills or scripts, use the in-tree wrapper rather than t | `src/session-manager.ts` | Resolves sessions; opens `inbound.db` / `outbound.db`; manages heartbeat path | | `src/container-runner.ts` | Spawns per-agent-group Docker containers with session DB + outbox mounts, OneCLI `ensureAgent` | | `src/container-runtime.ts` | Docker CLI wrapper (runtime binary, host-gateway args, mount args), orphan cleanup | -| `src/guard/` | Privileged-action decision seam: `guard()` → allow \| hold \| deny. Domain-free leaf (decision function + registration-derived action catalog); domain baselines register from module-edge `guard.ts` adapters (cli, agent-to-agent, self-mod, permissions). All four handler registries wrap at registration; approved replays carry the approval row as a grant and re-check the structural baseline. Policy-as-data (tighten-only rule sources) is deferred to guarded-actions phase 3. Conformance test: `src/guard/conformance.test.ts` | +| `src/guard/` | Privileged-action decision seam: `guard(action, input)` → allow \| hold \| deny. Domain-free leaf; domain baselines are `defineGuardedAction` values exported from module-edge `guard.ts` adapters (cli, agent-to-agent, self-mod, permissions) — consults hold the value, so a missing/typo'd wiring is a compile error, never a fail-open. All four handler registries require a guard spec or an explicit `unguarded()` declaration at the registration site. Approved replays carry the approval row as a grant and re-check the structural baseline. Boot check = grant continuations only (`src/guard-conformance.ts`); Policy-as-data is deferred to guarded-actions phase 3. Conformance test: `src/guard/conformance.test.ts` | | `src/modules/permissions/access.ts` | `canAccessAgentGroup` — owner / global admin / scoped admin / member resolution against `user_roles` + `agent_group_members` | | `src/modules/approvals/primitive.ts` | `pickApprover`, `pickApprovalDelivery`, `requestApproval`, approval-handler registry | | `src/command-gate.ts` | Router-side admin command gate — queries `user_roles` directly (no env var, no container-side check) | diff --git a/src/cli/delivery-action.ts b/src/cli/delivery-action.ts index 5c693be6f..5a759cb21 100644 --- a/src/cli/delivery-action.ts +++ b/src/cli/delivery-action.ts @@ -8,52 +8,57 @@ import type Database from 'better-sqlite3'; import { registerDeliveryAction } from '../delivery.js'; +import { unguarded } from '../guard/index.js'; import { insertMessage } from '../db/session-db.js'; import { log } from '../log.js'; import { dispatch } from './dispatch.js'; import type { RequestFrame } from './frame.js'; import type { Session } from '../types.js'; -registerDeliveryAction('cli_request', async (content, session, inDb) => { - const requestId = content.requestId as string; - const command = content.command as string; - const args = (content.args as Record) ?? {}; +registerDeliveryAction( + 'cli_request', + async (content, session, inDb) => { + const requestId = content.requestId as string; + const command = content.command as string; + const args = (content.args as Record) ?? {}; - if (!requestId || !command) { - log.warn('cli_request missing requestId or command', { sessionId: session.id }); - return; - } + if (!requestId || !command) { + log.warn('cli_request missing requestId or command', { sessionId: session.id }); + return; + } - const req: RequestFrame = { id: requestId, command, args }; - const ctx = { - caller: 'agent' as const, - sessionId: session.id, - agentGroupId: session.agent_group_id, - messagingGroupId: session.messaging_group_id ?? '', - }; + const req: RequestFrame = { id: requestId, command, args }; + const ctx = { + caller: 'agent' as const, + sessionId: session.id, + agentGroupId: session.agent_group_id, + messagingGroupId: session.messaging_group_id ?? '', + }; - log.info('CLI request from agent', { requestId, command, sessionId: session.id }); + log.info('CLI request from agent', { requestId, command, sessionId: session.id }); - const response = await dispatch(req, ctx); + const response = await dispatch(req, ctx); - // Write response to inbound.db so the container can read it. - // trigger=0: don't wake the agent — this is an inline response to a tool call. - insertMessage(inDb, { - id: `cli-resp-${requestId}`, - kind: 'system', - timestamp: new Date().toISOString(), - platformId: null, - channelType: null, - threadId: null, - content: JSON.stringify({ - type: 'cli_response', - requestId, - frame: response, - }), - processAfter: null, - recurrence: null, - trigger: 0, - }); + // Write response to inbound.db so the container can read it. + // trigger=0: don't wake the agent — this is an inline response to a tool call. + insertMessage(inDb, { + id: `cli-resp-${requestId}`, + kind: 'system', + timestamp: new Date().toISOString(), + platformId: null, + channelType: null, + threadId: null, + content: JSON.stringify({ + type: 'cli_response', + requestId, + frame: response, + }), + processAfter: null, + recurrence: null, + trigger: 0, + }); - log.info('CLI response written', { requestId, ok: response.ok, sessionId: session.id }); -}); + log.info('CLI response written', { requestId, ok: response.ok, sessionId: session.id }); + }, + unguarded('transport envelope — every inner command is guarded at dispatch'), +); diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 99ca7b18d..bb44144d1 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -20,9 +20,8 @@ import { registerApprovalHandler, requestApproval } from '../modules/approvals/i import type { PendingApproval } from '../types.js'; import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './frame.js'; import { getResource } from './crud.js'; -import { commandGuardAction } from './guard.js'; import { listVerbs, renderVerbHelp } from './help-render.js'; -import { listCommands, lookup } from './registry.js'; +import { commandGuard, listCommands, lookup } from './registry.js'; type DispatchOptions = { /** Verified approval row when a command is replayed after approval. */ @@ -101,8 +100,7 @@ export async function dispatch( } } - const decision = guard({ - action: commandGuardAction(cmd), + const decision = guard(commandGuard(cmd.name), { actor: actorFor(ctx), payload: req.args, grant: opts.grant ?? null, diff --git a/src/cli/registry.ts b/src/cli/registry.ts index f80e4e02f..82419fb6f 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -8,7 +8,7 @@ * registers the help commands, so the registry is populated before the host's * CLI server accepts connections. */ -import { registerGuardedAction } from '../guard/index.js'; +import { defineGuardedAction, type GuardedAction } from '../guard/index.js'; import { commandGuardSpec } from './guard.js'; import type { CallerContext } from './frame.js'; @@ -61,6 +61,7 @@ export type CommandDef = { }; const registry = new Map(); +const commandGuards = new Map(); export function register(def: CommandDef): void { if (registry.has(def.name)) { @@ -68,9 +69,18 @@ export function register(def: CommandDef): void { } registry.set(def.name, def as CommandDef); // Declaration is registration: every command gets a guard-catalog entry - // derived from its own definition — dispatch consults the guard, and the - // conformance test walks the registry against the catalog. - registerGuardedAction(commandGuardSpec(def as CommandDef)); + // derived from its own definition, in the same call that registers it — a + // command cannot exist without a guard, and dispatch consults it by value. + commandGuards.set(def.name, defineGuardedAction(commandGuardSpec(def as CommandDef))); +} + +/** The guard defined for a registered command — total for anything register() accepted. */ +export function commandGuard(name: string): GuardedAction { + const g = commandGuards.get(name); + if (!g) { + throw new Error(`CLI command "${name}" has no guard — was it registered through register()?`); + } + return g; } export function lookup(name: string): CommandDef | undefined { diff --git a/src/delivery-actions.test.ts b/src/delivery-actions.test.ts index f9054978b..fc38d769a 100644 --- a/src/delivery-actions.test.ts +++ b/src/delivery-actions.test.ts @@ -4,7 +4,9 @@ * `registerDeliveryAction` is the hook modules use to handle system-kind * outbound messages; `getDeliveryAction` is the read side that makes those * registrations behavior-testable. Goes red if either half of the registry - * is removed or the two stop sharing the same map. + * is removed or the two stop sharing the same map. Every registration now + * carries a guard spec or an explicit unguarded() declaration — + * omission is a type error. */ import { describe, it, expect, vi } from 'vitest'; @@ -16,11 +18,14 @@ vi.mock('./container-runner.js', () => ({ })); import { registerDeliveryAction, getDeliveryAction, type DeliveryActionHandler } from './delivery.js'; +import { defineGuardedAction, HOLD, unguarded } from './guard/index.js'; + +const testUnguarded = unguarded('test — registry mechanics only'); describe('delivery action registry', () => { it('getDeliveryAction returns the handler registerDeliveryAction registered', () => { const handler: DeliveryActionHandler = async () => {}; - registerDeliveryAction('test_registry_action', handler); + registerDeliveryAction('test_registry_action', handler, testUnguarded); expect(getDeliveryAction('test_registry_action')).toBe(handler); }); @@ -31,26 +36,32 @@ describe('delivery action registry', () => { it('re-registering an action overwrites the previous handler', () => { const first: DeliveryActionHandler = async () => {}; const second: DeliveryActionHandler = async () => {}; - registerDeliveryAction('test_overwrite_action', first); - registerDeliveryAction('test_overwrite_action', second); + registerDeliveryAction('test_overwrite_action', first, testUnguarded); + registerDeliveryAction('test_overwrite_action', second, testUnguarded); expect(getDeliveryAction('test_overwrite_action')).toBe(second); }); it('refuses to replace a guard-wrapped action with an unguarded handler', () => { + const guardAction = defineGuardedAction({ + action: 'test.guarded-overwrite', + baseline: () => HOLD('t'), + }); registerDeliveryAction('test_guarded_overwrite', async () => {}, { - guardAction: 'test.guarded-overwrite', + guardAction, requestHold: async () => {}, }); - // Disarming the guard by re-registering without a spec must throw — - // otherwise the catalog (and the conformance walk) would still report - // the action guarded while the live path runs unguarded. - expect(() => registerDeliveryAction('test_guarded_overwrite', async () => {})).toThrow(/disarm the guard/); + // Disarming the guard by re-registering unguarded must throw — otherwise + // the action's catalog entry would still exist while the live path runs + // unguarded. + expect(() => registerDeliveryAction('test_guarded_overwrite', async () => {}, testUnguarded)).toThrow( + /disarm the guard/, + ); // Re-registering WITH a spec stays allowed (a legitimate replacement // keeps the action guarded). registerDeliveryAction('test_guarded_overwrite', async () => {}, { - guardAction: 'test.guarded-overwrite', + guardAction, requestHold: async () => {}, }); expect(getDeliveryAction('test_guarded_overwrite')).toBeDefined(); diff --git a/src/delivery.ts b/src/delivery.ts index 949450b85..668028754 100644 --- a/src/delivery.ts +++ b/src/delivery.ts @@ -20,7 +20,7 @@ import { markDeliveryFailed, migrateDeliveredTable, } from './db/session-db.js'; -import { guard } from './guard/index.js'; +import { guard, type GuardedAction, type Unguarded } from './guard/index.js'; import { log } from './log.js'; import { normalizeOptions } from './channels/ask-question.js'; import { clearOutbox, openInboundDb, openOutboundDb, readOutboxFiles } from './session-manager.js'; @@ -397,15 +397,16 @@ async function deliverMessage( * `registerDeliveryAction`. Unknown actions log "Unknown system action". * * Privileged delivery actions (create_agent, install_packages, - * add_mcp_server) register with a guard spec: the registry wraps the handler - * so the guard's decision — allow / hold / deny — stands between the - * container's outbound row and the handler body, and the wrapped path is the - * only path (the raw handler is never stored). On approve, the continuation - * re-enters the same wrapped entry carrying the approval row as its grant - * (`reenterGuardedDeliveryAction`), so the structural baseline is re-checked - * live. Plain actions (scheduling self-actions, the cli_request bridge — its - * inner commands are guarded at dispatch) register without a spec and are - * not catalog actions. + * add_mcp_server) register with a guard spec: every path to the handler body + * — dispatch, approved replay, test lookup — goes through the guard consult + * (allow / hold / deny), so there is no unguarded route to it. On approve, + * the continuation re-enters the same entry carrying the approval row as its + * grant (`reenterGuardedDeliveryAction`), so the structural baseline is + * re-checked live. Plain actions (scheduling self-actions, the cli_request + * bridge — its inner commands are guarded at dispatch) register with an + * explicit `unguarded()` declaration instead of a spec — omission is + * not representable, so the decision to run unguarded is visible, and + * justified, at the registration site. */ export type DeliveryActionHandler = ( content: Record, @@ -417,8 +418,8 @@ export type DeliveryActionHandler = ( export type GuardedDeliveryHandler = (content: Record, session: Session) => Promise; export interface DeliveryGuardSpec { - /** Dotted guard-catalog action consulted before the handler runs. */ - guardAction: string; + /** Guard action consulted before the handler runs — the defined value, not a name. */ + guardAction: GuardedAction; /** * Domain validation that runs before the guard — malformed requests are * answered (notify) without ever creating a hold. Return false to stop. @@ -430,54 +431,53 @@ export interface DeliveryGuardSpec { onDeny?: (content: Record, session: Session, reason: string) => void; } -const actionHandlers = new Map(); -const guardedActions = new Map(); +type DeliveryEntry = + | { guard: Unguarded; handler: DeliveryActionHandler } + | { guard: DeliveryGuardSpec; handler: GuardedDeliveryHandler }; -export function registerDeliveryAction(action: string, handler: DeliveryActionHandler): void; +const deliveryActions = new Map(); + +function isUnguardedEntry(entry: DeliveryEntry): entry is Extract { + return 'reason' in entry.guard; +} + +export function registerDeliveryAction(action: string, handler: DeliveryActionHandler, unguardedDecl: Unguarded): void; export function registerDeliveryAction(action: string, handler: GuardedDeliveryHandler, spec: DeliveryGuardSpec): void; export function registerDeliveryAction( action: string, handler: DeliveryActionHandler | GuardedDeliveryHandler, - spec?: DeliveryGuardSpec, + guardDecl: DeliveryGuardSpec | Unguarded, ): void { - if (actionHandlers.has(action)) { + const existing = deliveryActions.get(action); + if (existing) { // Replacing a guard-wrapped action with an unguarded handler would - // disarm the guard while the catalog (and the conformance walk) still - // report it guarded — refuse. A skill that wants to extend a guarded - // action must compose at the module's exported functions instead, or - // re-register with a guard spec of its own. - if (!spec && guardedActions.has(action)) { + // disarm the guard while its catalog entry still exists — refuse. A + // skill that wants to extend a guarded action must compose at the + // module's exported functions instead, or re-register with a guard spec + // of its own. + if ('reason' in guardDecl && !isUnguardedEntry(existing)) { throw new Error( `delivery action "${action}" is guard-wrapped; re-registering it without a guard spec would disarm the guard`, ); } log.warn('Delivery action handler overwritten', { action }); } - if (!spec) { - actionHandlers.set(action, handler as DeliveryActionHandler); - return; - } - guardedActions.set(action, { spec, handler: handler as GuardedDeliveryHandler }); - actionHandlers.set(action, (content, session) => runGuardedDeliveryAction(action, content, session, null)); + // The overloads pair each handler shape with its declaration; the merged + // implementation signature erases that pairing, hence the one cast. + deliveryActions.set(action, { guard: guardDecl, handler } as DeliveryEntry); } -async function runGuardedDeliveryAction( +async function runGuarded( action: string, + entry: Extract, content: Record, session: Session, grant: PendingApproval | null, ): Promise { - const entry = guardedActions.get(action); - if (!entry) { - log.warn('Unknown guarded delivery action', { action }); - return; - } - const { spec, handler } = entry; - + const spec = entry.guard; if (spec.precheck && !(await spec.precheck(content, session))) return; - const decision = guard({ - action: spec.guardAction, + const decision = guard(spec.guardAction, { actor: { kind: 'agent', agentGroupId: session.agent_group_id, sessionId: session.id }, payload: content, grant, @@ -492,32 +492,37 @@ async function runGuardedDeliveryAction( await spec.requestHold(content, session); return; } - await handler(content, session); + await entry.handler(content, session); } /** * Approve continuation for a guard-wrapped delivery action: re-enter the - * wrapped entry with the approval row as the grant. The guard treats the - * grant as hold-satisfied but re-runs the structural baseline, so - * approve-then-revoke does not execute. Domains register this as their - * approval handler in the same line that registers the action. + * entry with the approval row as the grant. The guard treats the grant as + * hold-satisfied but re-runs the structural baseline, so approve-then-revoke + * does not execute. Domains register this as their approval handler in the + * same line that registers the action. */ export function reenterGuardedDeliveryAction(action: string) { return async (ctx: { session: Session; payload: Record; approval: PendingApproval }) => { - await runGuardedDeliveryAction(action, ctx.payload, ctx.session, ctx.approval); + const entry = deliveryActions.get(action); + if (!entry || isUnguardedEntry(entry)) { + log.warn('Approved replay for an action that is not guard-wrapped — dropping', { action }); + return; + } + await runGuarded(action, entry, ctx.payload, ctx.session, ctx.approval); }; } -/** Look up a registered delivery-action handler. Lets module registrations be behavior-tested. */ +/** + * The invocable for a registered action — the raw handler for unguarded + * entries, the guard-consulting path for guarded ones. Dispatch and tests + * both come through here; there is no route around the guard. + */ export function getDeliveryAction(action: string): DeliveryActionHandler | undefined { - return actionHandlers.get(action); -} - -/** Registered delivery actions with their guard mapping, for the conformance test. */ -export function listDeliveryActions(): { action: string; guardAction: string | null }[] { - return [...actionHandlers.keys()] - .sort() - .map((action) => ({ action, guardAction: guardedActions.get(action)?.spec.guardAction ?? null })); + const entry = deliveryActions.get(action); + if (!entry) return undefined; + if (isUnguardedEntry(entry)) return entry.handler; + return (content, session) => runGuarded(action, entry, content, session, null); } /** @@ -533,7 +538,7 @@ async function handleSystemAction( const action = content.action as string; log.info('System action from agent', { sessionId: session.id, action }); - const registered = actionHandlers.get(action); + const registered = getDeliveryAction(action); if (registered) { await registered(content, session, inDb); return; diff --git a/src/guard-conformance.ts b/src/guard-conformance.ts index 8cbf562be..4f7a3fbcd 100644 --- a/src/guard-conformance.ts +++ b/src/guard-conformance.ts @@ -1,85 +1,49 @@ /** - * Guard conformance — the non-bypass invariant, shared by CI and boot. + * Boot-time guard sanity — the one cross-registry invariant left to check + * after all import-time registrations have run. * - * CI only protects code that goes through the repo's CI, but NanoClaw's - * extension model is skill-installed code: /add-* skills copy modules into - * the user's tree and register handlers on machines where the test suite - * never runs. Running the same walk at boot turns the conformance test's - * guarantee into a runtime invariant at exactly that trust boundary: every - * registration is an import-time side effect, so by the time main() runs - * the registries are complete and an unmapped privileged entry is - * detectable before the host accepts a single message. + * The old registry walk is gone: everything it detected is now + * unconstructible at the API level. + * - A consult site cannot name a missing catalog entry — guard() takes the + * GuardedAction VALUE returned by defineGuardedAction, so a dropped + * module-edge import or a typo'd action is a compile error (and a forged + * value is denied at runtime), not a silent allow. + * - A handler cannot register unguarded by omission — every registry + * (delivery actions, response handlers, interceptors, CLI commands) + * requires a guard spec or an explicit unguarded() declaration + * at the registration site. * - * Fail-closed: the host refuses to start (the upgrade-tripwire posture). - * A conformance failure is code mis-composition — fixable with the host - * down — and it surfaces at skill-install time, when the installing agent - * is watching, instead of running unguarded until someone runs pnpm test. - * - * Limit: the walk verifies DECLARED mappings ("every delivery action is - * guarded or explicitly exempt") — it cannot infer which actions are - * privileged. That declaration stays on the author; the exemption list - * below is the loud, reviewable escape hatch. + * What remains is completeness ACROSS registries: a guarded action that + * holds via `approvalAction` needs a registered approval handler, or an + * approved card resolves into nothing — the hold has no continuation. That + * pairing only exists once every module has loaded (catalog entries and + * approval handlers register from different modules), so it stays a boot + * check with the fail-closed posture: the host refuses to start, surfacing + * the mis-composition at skill-install time instead of at the first + * approved card. */ -import { commandGuardAction } from './cli/guard.js'; -import { listCommands } from './cli/registry.js'; -import { listDeliveryActions } from './delivery.js'; -import { getGuardedAction } from './guard/index.js'; +import { listGuardedActions } from './guard/index.js'; import { log } from './log.js'; +import { getApprovalHandler } from './modules/approvals/primitive.js'; -/** - * Delivery actions that deliberately carry no guard mapping (the declared - * exemption class, per the guarded-actions design decision 1): - * - scheduling self-actions — an agent mutating only its own task rows; - * not a privileged action class (yet). - * - cli_request — the transport bridge into dispatch(); every inner - * command is guarded at dispatch, so the envelope carries no privilege. - */ -export const EXEMPT_DELIVERY_ACTIONS = new Set([ - 'schedule_task', - 'cancel_task', - 'pause_task', - 'resume_task', - 'update_task', - 'cli_request', -]); - -/** Walk the live registries against the guard catalog. Empty = conformant. */ -export function guardConformanceViolations(): string[] { - const violations: string[] = []; - - for (const cmd of listCommands()) { - const entry = getGuardedAction(commandGuardAction(cmd)); - if (!entry) { - violations.push(`command "${cmd.name}" has no guard-catalog entry`); - continue; - } - if (cmd.access === 'approval' && entry.approvalAction !== 'cli_command') { - violations.push(`mutating command "${cmd.name}" maps to a catalog entry that cannot hold`); - } - } - - for (const { action, guardAction } of listDeliveryActions()) { - if (guardAction === null) { - if (!EXEMPT_DELIVERY_ACTIONS.has(action)) { - violations.push(`delivery action "${action}" is neither guard-mapped nor on the declared exemption list`); - } - continue; - } - if (!getGuardedAction(guardAction)) { - violations.push(`delivery action "${action}" maps to unregistered guard action "${guardAction}"`); - } - } - - return violations; +/** Holding actions with no approve continuation. Empty = conformant. */ +export function grantContinuationGaps(): string[] { + return listGuardedActions() + .filter((spec) => spec.approvalAction && !getApprovalHandler(spec.approvalAction)) + .map( + (spec) => + `guarded action "${spec.action}" holds via approval action "${spec.approvalAction}" ` + + 'but no approval handler is registered — an approved hold would have no continuation', + ); } /** - * Boot check: refuse to start when any privileged registration is unmapped. + * Boot check: refuse to start when a holding action has no continuation. * Call after all import-time registrations (any point in main()). */ export function enforceGuardConformance(): void { - const violations = guardConformanceViolations(); - if (violations.length === 0) return; + const gaps = grantContinuationGaps(); + if (gaps.length === 0) return; console.error( [ @@ -87,20 +51,20 @@ export function enforceGuardConformance(): void { '='.repeat(64), 'NanoClaw stopped: guard conformance failure', '='.repeat(64), - 'A privileged registration is not mapped to the guard catalog —', - 'it would run with no allow/hold/deny decision. This usually means', - 'a skill (or local change) registered a command or delivery action', - 'without a guard spec.', + 'A guarded action can hold for approval, but no approval handler is', + 'registered for its approval action — an admin could click Approve', + 'and nothing would execute. This usually means a module (or skill)', + 'defined a holding baseline without registering its continuation.', '', - ...violations.map((v) => ` - ${v}`), + ...gaps.map((g) => ` - ${g}`), '', - 'Fix the registration (pass a guard spec / derive a catalog entry),', - 'or — only for genuinely unprivileged self-actions — add it to', - 'EXEMPT_DELIVERY_ACTIONS in src/guard-conformance.ts.', + 'Register the approval handler (registerApprovalHandler) in the same', + 'module that defines the guarded action, or drop approvalAction from', + 'the definition if the action can never hold.', '='.repeat(64), '', ].join('\n'), ); - log.error('Guard conformance failure — refusing to start', { violations }); + log.error('Guard conformance failure — refusing to start', { gaps }); process.exit(1); } diff --git a/src/guard/conformance.test.ts b/src/guard/conformance.test.ts index 7f476321a..a1621841c 100644 --- a/src/guard/conformance.test.ts +++ b/src/guard/conformance.test.ts @@ -1,20 +1,15 @@ /** - * Guard conformance — the non-bypass invariant, checked structurally. + * Guard conformance — the boot invariant, checked with the real registries. * - * Walks the real command registry and the real delivery-action registry - * (loaded via their production barrels) against the guard catalog. The walk - * itself lives in src/guard-conformance.ts and runs twice: here in CI, and - * at every boot (enforceGuardConformance in index.ts refuses to start on a - * violation) — CI can't see skill-installed registrations, the boot check - * can. A new privileged command or delivery action cannot quietly ship - * ungated: registration derives the catalog entry, and this walk makes - * drift loud. - * - * The declared exemption classes live with the walk - * (EXEMPT_DELIVERY_ACTIONS): scheduling self-actions and the cli_request - * transport bridge (its inner commands are guarded at dispatch). Reads - * (list/get/help) are catalog-mapped via registration too, but their - * baselines allow; the mutating set is what MUST be mapped. + * The old registry walk is gone: an unmapped consult or an undeclared + * unguarded registration is now unconstructible — guard() takes the defined + * GuardedAction value (a dropped module-edge import or typo'd name is a + * compile error), and every registry requires a guard spec or an explicit + * unguarded() declaration. What's left to verify structurally is the + * cross-registry pairing the compiler can't see: every holding action has a + * registered approve continuation. The check runs here in CI and at every + * boot (enforceGuardConformance refuses to start) — CI can't see + * skill-installed registrations, the boot check can. */ import { describe, expect, it } from 'vitest'; @@ -22,68 +17,68 @@ import { describe, expect, it } from 'vitest'; import '../cli/commands/index.js'; import '../modules/index.js'; import '../cli/delivery-action.js'; +import '../cli/dispatch.js'; // registers the cli_command approval handler -import { listCommands } from '../cli/registry.js'; -import { commandGuardAction } from '../cli/guard.js'; -import { listDeliveryActions, registerDeliveryAction } from '../delivery.js'; -import { EXEMPT_DELIVERY_ACTIONS, guardConformanceViolations } from '../guard-conformance.js'; -import { getGuardedAction } from './guard-actions.js'; +import { commandGuard, listCommands } from '../cli/registry.js'; +import { grantContinuationGaps } from '../guard-conformance.js'; +import { getApprovalHandler } from '../modules/approvals/primitive.js'; +import { defineGuardedAction, listGuardedActions } from './guard-actions.js'; +import { HOLD } from './types.js'; describe('guard conformance', () => { - it('the full walk (shared with the boot check) reports zero violations', () => { - expect(guardConformanceViolations()).toEqual([]); + it('the grant-continuation check (shared with the boot check) reports zero gaps', () => { + expect(grantContinuationGaps()).toEqual([]); }); - it('every mutating ncl command maps to a guard catalog entry that can hold', () => { + it('every holding action pairs with a registered approval handler', () => { + const holding = listGuardedActions().filter((spec) => spec.approvalAction); + expect(holding.length).toBeGreaterThan(0); + + const dangling = holding.filter((spec) => !getApprovalHandler(spec.approvalAction as string)); + expect(dangling.map((s) => s.action)).toEqual([]); + }); + + it('every mutating ncl command derives a guard that holds via cli_command', () => { const mutating = listCommands().filter((cmd) => cmd.access === 'approval'); expect(mutating.length).toBeGreaterThan(0); - const unmapped = mutating.filter((cmd) => { - const entry = getGuardedAction(commandGuardAction(cmd)); - return !entry || entry.approvalAction !== 'cli_command'; - }); - expect(unmapped.map((c) => c.name)).toEqual([]); + const wrong = mutating.filter((cmd) => commandGuard(cmd.name).approvalAction !== 'cli_command'); + expect(wrong.map((c) => c.name)).toEqual([]); }); - it('every registered command (reads included) has a catalog entry — denied reads still surface as denials', () => { - const unmapped = listCommands().filter((cmd) => !getGuardedAction(commandGuardAction(cmd))); - expect(unmapped.map((c) => c.name)).toEqual([]); + it('the domain catalog entries are defined once the module barrels load', () => { + const actions = new Set(listGuardedActions().map((s) => s.action)); + for (const expected of [ + 'agents.create', + 'a2a.send', + 'self_mod.install_packages', + 'self_mod.add_mcp_server', + 'senders.admit', + 'channels.register', + ]) { + expect(actions.has(expected), `catalog is missing "${expected}"`).toBe(true); + } }); - it('every delivery action is guard-mapped or on the declared exemption list', () => { - const actions = listDeliveryActions(); - expect(actions.length).toBeGreaterThan(0); - - const unmapped = actions.filter( - ({ action, guardAction }) => guardAction === null && !EXEMPT_DELIVERY_ACTIONS.has(action), + it('defining the same action twice throws — names are the catalog key', () => { + defineGuardedAction({ action: 'test.dup-define', baseline: () => HOLD('x') }); + expect(() => defineGuardedAction({ action: 'test.dup-define', baseline: () => HOLD('x') })).toThrow( + /already defined/, ); - expect(unmapped.map((a) => a.action)).toEqual([]); - - const danglingCatalog = actions.filter(({ guardAction }) => guardAction !== null && !getGuardedAction(guardAction)); - expect(danglingCatalog.map((a) => a.action)).toEqual([]); }); - it('the privileged delivery actions are the guarded ones', () => { - const guarded = Object.fromEntries( - listDeliveryActions() - .filter((a) => a.guardAction !== null) - .map((a) => [a.action, a.guardAction]), - ); - expect(guarded).toEqual({ - create_agent: 'agents.create', - install_packages: 'self_mod.install_packages', - add_mcp_server: 'self_mod.add_mcp_server', + // KEEP LAST: defines a holding action with no continuation into the shared + // per-worker catalog, so every gap check after this point sees it. + it('the check names a holding action with no approve continuation (what boot refuses on)', () => { + defineGuardedAction({ + action: 'test.dangling-hold', + approvalAction: 'test_dangling_hold_approved', + baseline: () => HOLD('always'), }); - }); - // KEEP LAST: registers a rogue action into the shared per-worker registry, - // so every walk after this point sees the violation. - it('the walk names an unguarded, non-exempt delivery action (what the boot check refuses on)', () => { - registerDeliveryAction('test_rogue_privileged_action', async () => {}); - - const violations = guardConformanceViolations(); - expect(violations).toHaveLength(1); - expect(violations[0]).toContain('test_rogue_privileged_action'); - expect(violations[0]).toContain('neither guard-mapped nor on the declared exemption list'); + const gaps = grantContinuationGaps(); + expect(gaps).toHaveLength(1); + expect(gaps[0]).toContain('test.dangling-hold'); + expect(gaps[0]).toContain('no approval handler'); }); }); diff --git a/src/guard/guard-actions.ts b/src/guard/guard-actions.ts index 32ce0da09..85f4e6cc3 100644 --- a/src/guard/guard-actions.ts +++ b/src/guard/guard-actions.ts @@ -1,19 +1,23 @@ /** * The action catalog — the enforcement boundary. * - * An action either is in the catalog (and passes a decision) or is not (and - * needs none — reads, scheduling self-actions). Declaration is registration: - * entries are derived at the registries' registration sites (command - * registry, delivery actions, response handlers, interceptors, module - * edges), never maintained in a second file. The conformance test walks the - * registries against this catalog so an unmapped privileged action fails CI. + * An action either is defined here (and every consult passes its decision) + * or cannot be consulted at all: guard() takes the GuardedAction VALUE + * returned by defineGuardedAction, so the wiring between a consult site and + * its baseline is a symbol reference the compiler checks. A dropped + * module-edge import or a typo'd action name is a build error, not a + * runtime fail-open — there is no lookup that can miss. + * + * Definitions are still recorded by name so boot can enumerate them: the + * grant-continuation check (src/guard-conformance.ts) pairs every holding + * action with its registered approval handler, and duplicate names are + * refused at definition time (grants match on the name). */ -import { log } from '../log.js'; import type { GuardDecision, GuardInput } from './types.js'; import type { PendingApproval } from '../types.js'; export interface GuardedActionSpec { - /** Dotted action name — the catalog key. */ + /** Dotted action name, e.g. 'roles.grant', 'agents.create', 'a2a.send'. */ action: string; /** * Today's structural checks for this action, verbatim — the only source of @@ -35,19 +39,35 @@ export interface GuardedActionSpec { grantMatches?: (grant: PendingApproval, input: GuardInput) => boolean; } -const guardedActions = new Map(); +declare const guardedActionBrand: unique symbol; +/** + * A defined guarded action — only defineGuardedAction can mint one. The + * brand makes the type nominal: a hand-rolled { action, baseline } object + * does not typecheck at a consult site, and fails the runtime check too. + */ +export type GuardedAction = Readonly & { readonly [guardedActionBrand]: true }; -export function registerGuardedAction(spec: GuardedActionSpec): void { - if (guardedActions.has(spec.action)) { - log.warn('Guarded action re-registered (overwriting)', { action: spec.action }); +const defined = new Map(); +const minted = new WeakSet(); + +export function defineGuardedAction(spec: GuardedActionSpec): GuardedAction { + if (defined.has(spec.action)) { + throw new Error(`guarded action "${spec.action}" is already defined — action names are the catalog key`); } - guardedActions.set(spec.action, spec); + const def = Object.freeze({ ...spec }) as GuardedAction; + minted.add(def); + defined.set(spec.action, def); + return def; } -export function getGuardedAction(action: string): GuardedActionSpec | undefined { - return guardedActions.get(action); +/** + * Runtime backstop for callers outside the type system (plain JS, casts): + * only values minted by defineGuardedAction pass — guard() denies the rest. + */ +export function isGuardedAction(value: unknown): value is GuardedAction { + return typeof value === 'object' && value !== null && minted.has(value); } -export function listGuardedActions(): GuardedActionSpec[] { - return [...guardedActions.values()].sort((a, b) => a.action.localeCompare(b.action)); +export function listGuardedActions(): GuardedAction[] { + return [...defined.values()].sort((a, b) => a.action.localeCompare(b.action)); } diff --git a/src/guard/guard.test.ts b/src/guard/guard.test.ts index 88cf203a8..db2a76e79 100644 --- a/src/guard/guard.test.ts +++ b/src/guard/guard.test.ts @@ -1,16 +1,16 @@ /** * Guard decision-function unit tests: the baseline is the decision (allow / - * hold / deny returned as-is, non-catalog actions allow), grant semantics - * (satisfies holds, never denies; invalid → refuse), and the fail-closed - * posture on a throwing baseline. + * hold / deny returned as-is), grant semantics (satisfies holds, never + * denies; invalid → refuse), the runtime backstop against forged action + * values, and the fail-closed posture on a throwing baseline. * - * Uses synthetic catalog actions registered per test — the registry is - * per-worker module state with no reset, so action names are unique. + * Uses synthetic actions defined per test — the catalog is per-worker module + * state with no reset, so action names are unique. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { guard } from './guard.js'; -import { registerGuardedAction } from './guard-actions.js'; +import { defineGuardedAction, type GuardedAction } from './guard-actions.js'; import { ALLOW, DENY, HOLD, type GuardInput } from './types.js'; const mockGetPendingApproval = vi.fn(); @@ -23,8 +23,8 @@ vi.mock('../log.js', () => ({ const AGENT = { kind: 'agent', agentGroupId: 'ag-1', sessionId: 'sess-1' } as const; -function input(action: string, extra: Partial = {}): GuardInput { - return { action, actor: AGENT, payload: {}, ...extra }; +function input(extra: Partial = {}): GuardInput { + return { actor: AGENT, payload: {}, ...extra }; } beforeEach(() => { @@ -36,18 +36,14 @@ afterEach(() => { }); describe('the baseline is the decision', () => { - it('non-catalog action → allow', () => { - expect(guard(input('test.unregistered-read')).effect).toBe('allow'); - }); - it('baseline allow → allow', () => { - registerGuardedAction({ action: 't.allow1', baseline: () => ALLOW('ok') }); - expect(guard(input('t.allow1')).effect).toBe('allow'); + const action = defineGuardedAction({ action: 't.allow1', baseline: () => ALLOW('ok') }); + expect(guard(action, input()).effect).toBe('allow'); }); it('baseline hold → hold, default approver chain', () => { - registerGuardedAction({ action: 't.hold1', baseline: () => HOLD('needs approval') }); - const d = guard(input('t.hold1')); + const action = defineGuardedAction({ action: 't.hold1', baseline: () => HOLD('needs approval') }); + const d = guard(action, input()); expect(d.effect).toBe('hold'); if (d.effect === 'hold') { expect(d.reason).toBe('needs approval'); @@ -56,18 +52,25 @@ describe('the baseline is the decision', () => { }); it('baseline hold → hold, carrying a named approver', () => { - registerGuardedAction({ action: 't.hold2', baseline: () => HOLD('policy row', 'telegram:dana') }); - const d = guard(input('t.hold2')); + const action = defineGuardedAction({ action: 't.hold2', baseline: () => HOLD('policy row', 'telegram:dana') }); + const d = guard(action, input()); expect(d.effect).toBe('hold'); if (d.effect === 'hold') expect(d.approverUserId).toBe('telegram:dana'); }); it('baseline deny → deny, carrying the reason', () => { - registerGuardedAction({ action: 't.deny1', baseline: () => DENY('structurally unauthorized') }); - const d = guard(input('t.deny1')); + const action = defineGuardedAction({ action: 't.deny1', baseline: () => DENY('structurally unauthorized') }); + const d = guard(action, input()); expect(d.effect).toBe('deny'); if (d.effect === 'deny') expect(d.reason).toBe('structurally unauthorized'); }); + + it('a forged action value (not from defineGuardedAction) is denied', () => { + const forged = { action: 't.forged', baseline: () => ALLOW('never vetted') } as unknown as GuardedAction; + const d = guard(forged, input()); + expect(d.effect).toBe('deny'); + if (d.effect === 'deny') expect(d.reason).toContain('undefined action'); + }); }); describe('grants', () => { @@ -75,49 +78,53 @@ describe('grants', () => { ({ approval_id: 'appr-1', action, payload: '{}' }) as unknown as NonNullable; it('a valid live grant satisfies a hold', () => { - registerGuardedAction({ + const action = defineGuardedAction({ action: 't.g1', approvalAction: 'g1_approved', baseline: () => HOLD('b'), }); const grant = grantRow('g1_approved'); mockGetPendingApproval.mockReturnValue(grant); - expect(guard(input('t.g1', { grant })).effect).toBe('allow'); + expect(guard(action, input({ grant })).effect).toBe('allow'); }); it('a grant never satisfies a deny — the baseline is re-checked live', () => { - registerGuardedAction({ action: 't.g2', approvalAction: 'g2_approved', baseline: () => DENY('revoked since') }); + const action = defineGuardedAction({ + action: 't.g2', + approvalAction: 'g2_approved', + baseline: () => DENY('revoked since'), + }); const grant = grantRow('g2_approved'); mockGetPendingApproval.mockReturnValue(grant); - const d = guard(input('t.g2', { grant })); + const d = guard(action, input({ grant })); expect(d.effect).toBe('deny'); if (d.effect === 'deny') expect(d.reason).toBe('revoked since'); }); it('a dead grant (row deleted) refuses instead of re-holding', () => { - registerGuardedAction({ + const action = defineGuardedAction({ action: 't.g3', approvalAction: 'g3_approved', baseline: () => HOLD('b'), }); mockGetPendingApproval.mockReturnValue(undefined); - const d = guard(input('t.g3', { grant: grantRow('g3_approved') })); + const d = guard(action, input({ grant: grantRow('g3_approved') })); expect(d.effect).toBe('deny'); }); it("a grant for a different action doesn't transfer", () => { - registerGuardedAction({ + const action = defineGuardedAction({ action: 't.g4', approvalAction: 'g4_approved', baseline: () => HOLD('b'), }); const grant = grantRow('other_action'); mockGetPendingApproval.mockReturnValue(grant); - expect(guard(input('t.g4', { grant })).effect).toBe('deny'); + expect(guard(action, input({ grant })).effect).toBe('deny'); }); it('a domain grantMatches binding can refuse a payload mismatch', () => { - registerGuardedAction({ + const action = defineGuardedAction({ action: 't.g5', approvalAction: 'g5_approved', grantMatches: () => false, @@ -125,25 +132,29 @@ describe('grants', () => { }); const grant = grantRow('g5_approved'); mockGetPendingApproval.mockReturnValue(grant); - expect(guard(input('t.g5', { grant })).effect).toBe('deny'); + expect(guard(action, input({ grant })).effect).toBe('deny'); }); it('a grant on an already-allowed action is a no-op', () => { - registerGuardedAction({ action: 't.g6', approvalAction: 'g6_approved', baseline: () => ALLOW('ok') }); + const action = defineGuardedAction({ + action: 't.g6', + approvalAction: 'g6_approved', + baseline: () => ALLOW('ok'), + }); const grant = grantRow('g6_approved'); mockGetPendingApproval.mockReturnValue(grant); - expect(guard(input('t.g6', { grant })).effect).toBe('allow'); + expect(guard(action, input({ grant })).effect).toBe('allow'); }); }); describe('fail-closed posture', () => { it('a throwing baseline denies', () => { - registerGuardedAction({ + const action = defineGuardedAction({ action: 't.f1', baseline: () => { throw new Error('boom'); }, }); - expect(guard(input('t.f1')).effect).toBe('deny'); + expect(guard(action, input()).effect).toBe('deny'); }); }); diff --git a/src/guard/guard.ts b/src/guard/guard.ts index 55bed0223..56008a212 100644 --- a/src/guard/guard.ts +++ b/src/guard/guard.ts @@ -1,36 +1,47 @@ /** * guard() — the one decision function every privileged action consults. * - * The decision is the catalog entry's structural baseline — today's code - * checks, registered per action at the module edges. Non-catalog actions - * (reads, scheduling self-actions) allow. Policy-as-data (tighten-only rule - * sources composing with the baseline) is deliberately deferred to phase 3 - * of the guarded-actions design, where the generalized rules table arrives - * with its first operator-visible consumer; until then the one policy table + * The decision is the action's structural baseline — today's code checks, + * defined per action at the module edges. The consult site holds the + * GuardedAction value itself (defineGuardedAction), so there is no name + * lookup and no fail-open path for an unknown action: an unwired consult is + * a compile error, and a value that didn't come from defineGuardedAction is + * denied at runtime. Policy-as-data (tighten-only rule sources composing + * with the baseline) is deliberately deferred to phase 3 of the + * guarded-actions design, where the generalized rules table arrives with its + * first operator-visible consumer; until then the one policy table * (agent_message_policies) is consulted inside the a2a.send baseline. * * Grants: an approved replay carries the verified approval row. A valid - * grant (live pending row whose action matches the catalog entry's approval - * action, plus any domain binding) satisfies a hold — the human already - * decided — but NEVER a deny: the baseline is re-checked live, so - * approve-then-revoke no longer executes. A grant that is present but - * invalid fails closed to deny (no second card). + * grant (live pending row whose action matches the entry's approval action, + * plus any domain binding) satisfies a hold — the human already decided — + * but NEVER a deny: the baseline is re-checked live, so approve-then-revoke + * no longer executes. A grant that is present but invalid fails closed to + * deny (no second card). * * The guard itself fails closed: a throwing baseline denies. */ import { getPendingApproval } from '../db/sessions.js'; import { log } from '../log.js'; -import { getGuardedAction } from './guard-actions.js'; +import { isGuardedAction, type GuardedAction } from './guard-actions.js'; import { ALLOW, DENY, type GuardDecision, type GuardInput } from './types.js'; -export function guard(input: GuardInput): GuardDecision { - const entry = getGuardedAction(input.action); +export function guard(action: GuardedAction, input: GuardInput): GuardDecision { + if (!isGuardedAction(action)) { + // JS-level backstop — the branded type already forbids this. A + // hand-rolled object must not carry a baseline never vetted at + // definition time. + log.error('Guard consulted with an undefined action — failing closed', { + action: (action as { action?: unknown } | null)?.action, + }); + return DENY('guard consulted with an undefined action (failing closed)'); + } let decision: GuardDecision; try { - decision = entry ? entry.baseline(input) : ALLOW('non-catalog action'); + decision = action.baseline(input); } catch (err) { - log.error('Guard evaluation threw — failing closed', { action: input.action, err }); + log.error('Guard evaluation threw — failing closed', { action: action.action, err }); return DENY('guard failure (failing closed)'); } @@ -42,24 +53,20 @@ export function guard(input: GuardInput): GuardDecision { // An invalid grant on a replay is a refusal, not a fresh hold — approved // replays must execute exactly once. - if (entry && grantSatisfies(input, entry.approvalAction, entry.grantMatches)) { + if (grantSatisfies(action, input)) { return ALLOW(`hold satisfied by approval ${input.grant.approval_id}`); } return DENY('replay carried an invalid or mismatched grant'); } -function grantSatisfies( - input: GuardInput, - approvalAction: string | undefined, - grantMatches: ((grant: NonNullable, input: GuardInput) => boolean) | undefined, -): boolean { +function grantSatisfies(action: GuardedAction, input: GuardInput): boolean { const grant = input.grant; - if (!grant || !approvalAction) return false; - if (grant.action !== approvalAction) return false; + if (!grant || !action.approvalAction) return false; + if (grant.action !== action.approvalAction) return false; // The row must still be live — resolution deletes it, so a grant can only // execute once and a fabricated row object doesn't pass. const live = getPendingApproval(grant.approval_id); - if (!live || live.action !== approvalAction) return false; - if (grantMatches && !grantMatches(grant, input)) return false; + if (!live || live.action !== action.approvalAction) return false; + if (action.grantMatches && !action.grantMatches(grant, input)) return false; return true; } diff --git a/src/guard/index.ts b/src/guard/index.ts index 89781dfb3..4d58ccaf9 100644 --- a/src/guard/index.ts +++ b/src/guard/index.ts @@ -2,15 +2,27 @@ * Guard — the privileged-action decision seam (guarded-actions phase 2). * * See the guarded-actions decisions doc on the team hub. One decision - * function (guard.ts) and a registration-derived action catalog - * (guard-actions.ts). - * Domain-free leaf: domain baselines register from the domain modules' edges. + * function (guard.ts) and a definition-derived action catalog + * (guard-actions.ts). Consults carry the GuardedAction value returned by + * defineGuardedAction — never a name to look up — so mis-wiring is a build + * error, not a runtime fail-open. + * Domain-free leaf: domain baselines are defined at the domain modules' edges. */ export { guard } from './guard.js'; export { - registerGuardedAction, - getGuardedAction, + defineGuardedAction, + isGuardedAction, listGuardedActions, + type GuardedAction, type GuardedActionSpec, } from './guard-actions.js'; -export { ALLOW, DENY, HOLD, type GuardActor, type GuardDecision, type GuardInput } from './types.js'; +export { + ALLOW, + DENY, + HOLD, + unguarded, + type GuardActor, + type GuardDecision, + type GuardInput, + type Unguarded, +} from './types.js'; diff --git a/src/guard/types.ts b/src/guard/types.ts index 03233a42f..960ed353a 100644 --- a/src/guard/types.ts +++ b/src/guard/types.ts @@ -4,8 +4,9 @@ * The guard is a domain-free leaf: this module may import the DB read layer, * config, log, and shared types — never src/cli/* or src/modules/*. Domain * knowledge (what an action's structural baseline checks) arrives via - * registration: catalog entries (guard-actions.ts) are registered by the domain - * modules at their module edges. + * definition: domain modules call defineGuardedAction (guard-actions.ts) at + * their module edges and pass the returned value to every consult and + * registration site — the wiring is a symbol reference the compiler checks. */ import type { PendingApproval } from '../types.js'; @@ -17,8 +18,6 @@ export type GuardActor = | { kind: 'system' }; export interface GuardInput { - /** Dotted catalog action name, e.g. 'roles.grant', 'agents.create', 'a2a.send'. */ - action: string; actor: GuardActor; /** Domain resource reference, e.g. { from, to } for a2a.send. */ resource?: Record; @@ -32,6 +31,20 @@ export interface GuardInput { grant?: PendingApproval | null; } +declare const unguardedBrand: unique symbol; +/** + * A registration that deliberately carries no guard. Omission is not + * representable — every registry requires either a guard spec or this + * marker, so the decision to run unguarded is visible, and justified, in + * the diff that registers the handler. The reason travels with the + * registration; `grep "unguarded("` is the complete inventory. + */ +export type Unguarded = { readonly reason: string; readonly [unguardedBrand]: true }; + +export function unguarded(reason: string): Unguarded { + return Object.freeze({ reason }) as Unguarded; +} + export type GuardDecision = | { effect: 'allow'; reason: string } | { effect: 'hold'; reason: string; approverUserId?: string } diff --git a/src/modules/agent-to-agent/agent-route.ts b/src/modules/agent-to-agent/agent-route.ts index feb52ad59..398baa832 100644 --- a/src/modules/agent-to-agent/agent-route.ts +++ b/src/modules/agent-to-agent/agent-route.ts @@ -32,7 +32,7 @@ import { log } from '../../log.js'; import { openInboundDb, resolveSession, sessionDir, writeSessionMessage } from '../../session-manager.js'; import type { PendingApproval, Session } from '../../types.js'; import { requestApproval } from '../approvals/index.js'; -import { A2A_MESSAGE_GATE_ACTION } from './guard.js'; +import { A2A_MESSAGE_GATE_ACTION, a2aSend } from './guard.js'; export { isSafeAttachmentName }; export { A2A_MESSAGE_GATE_ACTION } from './guard.js'; @@ -247,8 +247,7 @@ export async function routeAgentMessage( // allow, agent_message_policies hold. An approved replay carries the // grant — the hold is satisfied but the structure is re-checked live, so // revoking a destination between hold and approve blocks delivery. - const decision = guard({ - action: 'a2a.send', + const decision = guard(a2aSend, { actor: { kind: 'agent', agentGroupId: sourceAgentGroupId, sessionId: session.id }, resource: { from: sourceAgentGroupId, to: targetAgentGroupId }, payload: { id: msg.id, platform_id: targetAgentGroupId, content: msg.content, in_reply_to: msg.in_reply_to }, diff --git a/src/modules/agent-to-agent/guard.ts b/src/modules/agent-to-agent/guard.ts index 740405804..ce2cf9ab7 100644 --- a/src/modules/agent-to-agent/guard.ts +++ b/src/modules/agent-to-agent/guard.ts @@ -19,7 +19,7 @@ */ import { getAgentGroup } from '../../db/agent-groups.js'; import { getContainerConfig } from '../../db/container-configs.js'; -import { ALLOW, DENY, HOLD, registerGuardedAction } from '../../guard/index.js'; +import { ALLOW, DENY, HOLD, defineGuardedAction } from '../../guard/index.js'; import { hasDestination } from './db/agent-destinations.js'; import { getMessagePolicy } from './db/agent-message-policies.js'; @@ -30,7 +30,7 @@ import { getMessagePolicy } from './db/agent-message-policies.js'; */ export const A2A_MESSAGE_GATE_ACTION = 'a2a_message_gate'; -registerGuardedAction({ +export const agentsCreate = defineGuardedAction({ action: 'agents.create', approvalAction: 'create_agent', // Bind a create_agent grant to the name that was approved. @@ -56,7 +56,7 @@ registerGuardedAction({ }, }); -registerGuardedAction({ +export const a2aSend = defineGuardedAction({ action: 'a2a.send', approvalAction: A2A_MESSAGE_GATE_ACTION, // Bind an a2a grant to the exact held message target. diff --git a/src/modules/agent-to-agent/index.ts b/src/modules/agent-to-agent/index.ts index fa9047ecb..2114acaca 100644 --- a/src/modules/agent-to-agent/index.ts +++ b/src/modules/agent-to-agent/index.ts @@ -21,15 +21,15 @@ * system action logs "Unknown system action", `channel_type='agent'` messages * throw because the module isn't installed. */ -import './guard.js'; import { reenterGuardedDeliveryAction, registerDeliveryAction } from '../../delivery.js'; import { notifyAgent, registerApprovalHandler } from '../approvals/index.js'; import { A2A_MESSAGE_GATE_ACTION } from './agent-route.js'; import { createAgent, requestCreateAgentHold, validateCreateAgent } from './create-agent.js'; +import { agentsCreate } from './guard.js'; import { applyA2aMessageGate } from './message-gate.js'; registerDeliveryAction('create_agent', createAgent, { - guardAction: 'agents.create', + guardAction: agentsCreate, precheck: validateCreateAgent, requestHold: requestCreateAgentHold, onDeny: (_content, session, reason) => notifyAgent(session, `create_agent denied: ${reason}`), diff --git a/src/modules/approvals/index.ts b/src/modules/approvals/index.ts index 8dcb274f5..924145b91 100644 --- a/src/modules/approvals/index.ts +++ b/src/modules/approvals/index.ts @@ -23,6 +23,7 @@ * + approval handlers via this module's public API. */ import { onDeliveryAdapterReady } from '../../delivery.js'; +import { unguarded } from '../../guard/index.js'; import { registerResponseHandler, onShutdown } from '../../response-registry.js'; import { handleApprovalsResponse } from './response-handler.js'; import { startOneCLIApprovalHandler, stopOneCLIApprovalHandler } from './onecli-approvals.js'; @@ -34,7 +35,10 @@ export type { ApprovalHandler, ApprovalHandlerContext, RequestApprovalOptions } // loads reason-capture.js, registering its message-interceptor on import. export { sweepAwaitingReasonRejects } from './reason-capture.js'; -registerResponseHandler(handleApprovalsResponse); +registerResponseHandler( + handleApprovalsResponse, + unguarded('self-authorizing — resolves each pending_approvals row against its own approver rules before dispatching'), +); onDeliveryAdapterReady((adapter) => { startOneCLIApprovalHandler(adapter); diff --git a/src/modules/approvals/reason-capture.ts b/src/modules/approvals/reason-capture.ts index 79724a277..03dcf6074 100644 --- a/src/modules/approvals/reason-capture.ts +++ b/src/modules/approvals/reason-capture.ts @@ -20,6 +20,7 @@ */ import type { InboundEvent } from '../../channels/adapter.js'; import { getDeliveryAdapter } from '../../delivery.js'; +import { unguarded } from '../../guard/index.js'; import { deletePendingApproval, getExpiredAwaitingReasonApprovals, @@ -151,7 +152,10 @@ export async function captureReasonReply(event: InboundEvent): Promise return true; } -registerMessageInterceptor(captureReasonReply); +registerMessageInterceptor( + captureReasonReply, + unguarded('self-authorizing — only captures a DM from the admin whose reject click armed it (keyed by userId + DM)'), +); /** * Host-sweep finalizer: any reject-with-reason hold whose window elapsed (admin diff --git a/src/modules/interactive/index.ts b/src/modules/interactive/index.ts index 324adbe7e..3aada6fa1 100644 --- a/src/modules/interactive/index.ts +++ b/src/modules/interactive/index.ts @@ -13,6 +13,7 @@ import { getDb, hasTable } from '../../db/connection.js'; import { deletePendingQuestion, getPendingQuestion, getSession } from '../../db/sessions.js'; import { wakeContainer } from '../../container-runner.js'; +import { unguarded } from '../../guard/index.js'; import { registerResponseHandler, type ResponsePayload } from '../../response-registry.js'; import { log } from '../../log.js'; import { writeSessionMessage } from '../../session-manager.js'; @@ -56,4 +57,7 @@ async function handleInteractiveResponse(payload: ResponsePayload): Promise { const policy = input.payload.policy; @@ -36,7 +36,7 @@ registerGuardedAction({ }, }); -registerGuardedAction({ +export const channelsRegister = defineGuardedAction({ action: 'channels.register', baseline: (input) => { if (input.actor.kind !== 'human') return DENY('channel registration resolves via human clicks/replies'); diff --git a/src/modules/permissions/index.ts b/src/modules/permissions/index.ts index 4ead62019..abc64f6d8 100644 --- a/src/modules/permissions/index.ts +++ b/src/modules/permissions/index.ts @@ -32,8 +32,8 @@ import { registerResponseHandler, type ResponsePayload } from '../../response-re import { getDeliveryAdapter } from '../../delivery.js'; import { log } from '../../log.js'; import type { MessagingGroup, MessagingGroupAgent } from '../../types.js'; -import './guard.js'; -import { guard } from '../../guard/index.js'; +import { guard, unguarded } from '../../guard/index.js'; +import { channelsRegister, sendersAdmit } from './guard.js'; import { canAccessAgentGroup } from './access.js'; import { buildAgentSelectionOptions, @@ -135,8 +135,7 @@ function handleUnknownSender( // — unknown_sender_policy verbatim: strict → deny, request_approval → hold, // public → allow (short-circuited before the gate). Drop-recording and the // hold creation stay here. - const decision = guard({ - action: 'senders.admit', + const decision = guard(sendersAdmit, { actor: userId ? { kind: 'human', userId } : { kind: 'system' }, payload: { messagingGroupId: mg.id, @@ -294,7 +293,12 @@ async function handleSenderApprovalResponse(payload: ResponsePayload): Promise getPendingChannelApproval(payload.questionId) !== undefined, }); @@ -640,7 +644,7 @@ const captureAgentNameReply = async (event: InboundEvent): Promise => { }; registerMessageInterceptor(captureAgentNameReply, { - action: 'channels.register', + action: channelsRegister, claims: (event) => { const userId = extractAndUpsertUser(event); if (!userId) return null; diff --git a/src/modules/scheduling/index.ts b/src/modules/scheduling/index.ts index 14ef7d550..d1f2ccf0b 100644 --- a/src/modules/scheduling/index.ts +++ b/src/modules/scheduling/index.ts @@ -19,6 +19,7 @@ * module piggybacks on the core schema. */ import { registerDeliveryAction } from '../../delivery.js'; +import { unguarded } from '../../guard/index.js'; import { handleCancelTask, handlePauseTask, @@ -27,8 +28,10 @@ import { handleUpdateTask, } from './actions.js'; -registerDeliveryAction('schedule_task', handleScheduleTask); -registerDeliveryAction('cancel_task', handleCancelTask); -registerDeliveryAction('pause_task', handlePauseTask); -registerDeliveryAction('resume_task', handleResumeTask); -registerDeliveryAction('update_task', handleUpdateTask); +const selfAction = unguarded('agent self-action — mutates only its own task rows; not a privileged class (yet)'); + +registerDeliveryAction('schedule_task', handleScheduleTask, selfAction); +registerDeliveryAction('cancel_task', handleCancelTask, selfAction); +registerDeliveryAction('pause_task', handlePauseTask, selfAction); +registerDeliveryAction('resume_task', handleResumeTask, selfAction); +registerDeliveryAction('update_task', handleUpdateTask, selfAction); diff --git a/src/modules/self-mod/guard.ts b/src/modules/self-mod/guard.ts index 85faa3492..0b80bc7a7 100644 --- a/src/modules/self-mod/guard.ts +++ b/src/modules/self-mod/guard.ts @@ -8,7 +8,7 @@ * add-package` etc. — are separate catalog actions derived from the command * registry.) */ -import { DENY, HOLD, registerGuardedAction, type GuardInput } from '../../guard/index.js'; +import { DENY, HOLD, defineGuardedAction, type GuardInput } from '../../guard/index.js'; function selfModBaseline(label: string) { return (input: GuardInput) => { @@ -19,13 +19,13 @@ function selfModBaseline(label: string) { }; } -registerGuardedAction({ +export const selfModInstallPackages = defineGuardedAction({ action: 'self_mod.install_packages', approvalAction: 'install_packages', baseline: selfModBaseline('install_packages'), }); -registerGuardedAction({ +export const selfModAddMcpServer = defineGuardedAction({ action: 'self_mod.add_mcp_server', approvalAction: 'add_mcp_server', baseline: selfModBaseline('add_mcp_server'), diff --git a/src/modules/self-mod/index.ts b/src/modules/self-mod/index.ts index f5dd53251..8cdbc30f7 100644 --- a/src/modules/self-mod/index.ts +++ b/src/modules/self-mod/index.ts @@ -24,10 +24,10 @@ * system messages with these actions, but delivery logs "Unknown system * action" and drops them. Admin never sees a card; nothing changes. */ -import './guard.js'; import { reenterGuardedDeliveryAction, registerDeliveryAction } from '../../delivery.js'; import { notifyAgent, registerApprovalHandler } from '../approvals/index.js'; import { applyAddMcpServer, applyInstallPackages } from './apply.js'; +import { selfModAddMcpServer, selfModInstallPackages } from './guard.js'; import { requestAddMcpServerHold, requestInstallPackagesHold, @@ -36,13 +36,13 @@ import { } from './request.js'; registerDeliveryAction('install_packages', applyInstallPackages, { - guardAction: 'self_mod.install_packages', + guardAction: selfModInstallPackages, precheck: validateInstallPackages, requestHold: requestInstallPackagesHold, onDeny: (_content, session, reason) => notifyAgent(session, `install_packages denied: ${reason}`), }); registerDeliveryAction('add_mcp_server', applyAddMcpServer, { - guardAction: 'self_mod.add_mcp_server', + guardAction: selfModAddMcpServer, precheck: validateAddMcpServer, requestHold: requestAddMcpServerHold, onDeny: (_content, session, reason) => notifyAgent(session, `add_mcp_server denied: ${reason}`), diff --git a/src/response-registry.ts b/src/response-registry.ts index 57c42e511..bb7cd8c21 100644 --- a/src/response-registry.ts +++ b/src/response-registry.ts @@ -16,9 +16,11 @@ * the click and the handler, and the wrapped path is the only path. `claims` * is the handler's own claim test (does this questionId belong to me?) so an * unauthorized click is claimed-and-dropped without stealing other handlers' - * responses. + * responses. The guard argument is not optional — a handler that runs + * unguarded must declare so with `unguarded()`, at the registration + * site, in the diff that adds it. */ -import { guard, type GuardActor } from './guard/index.js'; +import { guard, type GuardActor, type GuardedAction, type Unguarded } from './guard/index.js'; import { log } from './log.js'; export interface ResponsePayload { @@ -33,8 +35,8 @@ export interface ResponsePayload { export type ResponseHandler = (payload: ResponsePayload) => Promise; export interface ResponseGuardSpec { - /** Dotted guard-catalog action consulted before the handler runs. */ - action: string; + /** Guard action consulted before the handler runs — the defined value, not a name. */ + action: GuardedAction; /** Would this handler claim the response? (Its own row lookup.) */ claims: (payload: ResponsePayload) => boolean; } @@ -47,22 +49,23 @@ function responseActor(payload: ResponsePayload): GuardActor { return { kind: 'human', userId }; } -export function registerResponseHandler(handler: ResponseHandler, guardSpec?: ResponseGuardSpec): void { - if (!guardSpec) { +export function registerResponseHandler(handler: ResponseHandler, guardDecl: ResponseGuardSpec | Unguarded): void { + if ('reason' in guardDecl) { + // Explicitly declared unguarded — the carried reason is the reviewable record. responseHandlers.push(handler); return; } + const spec = guardDecl; responseHandlers.push(async (payload) => { - if (!guardSpec.claims(payload)) return false; - const decision = guard({ - action: guardSpec.action, + if (!spec.claims(payload)) return false; + const decision = guard(spec.action, { actor: responseActor(payload), payload: { questionId: payload.questionId, value: payload.value }, }); if (decision.effect !== 'allow') { // Claim the response so it's not unclaimed-logged, but do nothing. log.warn('Response click rejected by guard', { - action: guardSpec.action, + action: spec.action.action, questionId: payload.questionId, userId: payload.userId, reason: decision.reason, diff --git a/src/router.ts b/src/router.ts index d89db0272..dbca8c218 100644 --- a/src/router.ts +++ b/src/router.ts @@ -20,7 +20,7 @@ import { getChannelAdapter } from './channels/channel-registry.js'; import { gateCommand } from './command-gate.js'; import { getAgentGroup } from './db/agent-groups.js'; -import { guard, type GuardActor } from './guard/index.js'; +import { guard, type GuardActor, type GuardedAction, type Unguarded } from './guard/index.js'; import { recordDroppedMessage } from './db/dropped-messages.js'; import { createMessagingGroup, @@ -124,13 +124,15 @@ export function setSenderScopeGate(fn: SenderScopeGateFn): void { * registers with a guard spec: the registry wraps it so the guard's decision * stands between the free-text reply and the handler. `claims` returns the * guard consult for events the interceptor would act on (null = not mine, - * pass through); a deny consumes the message without acting. + * pass through); a deny consumes the message without acting. The guard + * argument is not optional — an interceptor that runs unguarded must declare + * so with `unguarded()` at the registration site. */ export type MessageInterceptorFn = (event: InboundEvent) => Promise; export interface InterceptorGuardSpec { - /** Dotted guard-catalog action consulted before the interceptor acts. */ - action: string; + /** Guard action consulted before the interceptor acts — the defined value, not a name. */ + action: GuardedAction; /** The guard consult for events this interceptor would act on; null = not mine. */ claims: (event: InboundEvent) => { actor: GuardActor; payload: Record } | null; /** Domain cleanup when the guard denies (e.g. disarm the capture). */ @@ -139,18 +141,23 @@ export interface InterceptorGuardSpec { const messageInterceptors: MessageInterceptorFn[] = []; -export function registerMessageInterceptor(fn: MessageInterceptorFn, guardSpec?: InterceptorGuardSpec): void { - if (!guardSpec) { +export function registerMessageInterceptor( + fn: MessageInterceptorFn, + guardDecl: InterceptorGuardSpec | Unguarded, +): void { + if ('reason' in guardDecl) { + // Explicitly declared unguarded — the carried reason is the reviewable record. messageInterceptors.push(fn); return; } + const guardSpec = guardDecl; messageInterceptors.push(async (event) => { const consult = guardSpec.claims(event); if (!consult) return fn(event); - const decision = guard({ action: guardSpec.action, actor: consult.actor, payload: consult.payload }); + const decision = guard(guardSpec.action, { actor: consult.actor, payload: consult.payload }); if (decision.effect !== 'allow') { log.warn('Interceptor capture rejected by guard — consuming without acting', { - action: guardSpec.action, + action: guardSpec.action.action, reason: decision.reason, }); guardSpec.onDeny?.(event);