diff --git a/container/agent-runner/src/poll-loop.ts b/container/agent-runner/src/poll-loop.ts index 288c060..e5ad1a5 100644 --- a/container/agent-runner/src/poll-loop.ts +++ b/container/agent-runner/src/poll-loop.ts @@ -80,10 +80,6 @@ export async function runPollLoop(config: PollLoopConfig): Promise { // Handle commands: categorize chat messages const adminUserIds = config.adminUserIds ?? new Set(); - // Permissionless mode: when the permissions module isn't installed on - // the host, NANOCLAW_ADMIN_USER_IDS arrives empty. Treat every sender - // with an identifiable senderId as admin so admin commands still work. - const permissionless = adminUserIds.size === 0; const normalMessages = []; const commandIds: string[] = []; @@ -103,8 +99,7 @@ export async function runPollLoop(config: PollLoopConfig): Promise { } if (cmdInfo.category === 'admin') { - const authorized = permissionless ? !!cmdInfo.senderId : !!cmdInfo.senderId && adminUserIds.has(cmdInfo.senderId); - if (!authorized) { + if (!cmdInfo.senderId || !adminUserIds.has(cmdInfo.senderId)) { log(`Admin command denied: ${cmdInfo.command} from ${cmdInfo.senderId} (msg: ${msg.id})`); writeMessageOut({ id: generateId(), diff --git a/docs/module-contract.md b/docs/module-contract.md index cc423e8..e226a38 100644 --- a/docs/module-contract.md +++ b/docs/module-contract.md @@ -68,30 +68,42 @@ export function registerDeliveryAction(action: string, handler: ActionHandler): **Current consumers:** scheduling (5 actions — `schedule_task`, `cancel_task`, `pause_task`, `resume_task`, `update_task`), approvals (3 actions — `install_packages`, `request_rebuild`, `add_mcp_server`), agent-to-agent (`create_agent`, and the agent-routing branch keyed as a pseudo-action `agent_route`). -### 2. Router inbound gate +### 2. Router sender resolver + access gate + +Two separate setters, called at different points in `routeInbound`. Preserves the pre-refactor ordering: sender-upsert side effects fire even when the message is ultimately dropped by wiring or trigger rules. ```typescript // src/router.ts -type InboundGateResult = - | { allowed: true; userId: string | null } - | { allowed: false; userId: string | null; reason: string }; +type SenderResolverFn = (event: InboundEvent) => string | null; -type InboundGateFn = ( +export function setSenderResolver(fn: SenderResolverFn): void; + +type AccessGateResult = + | { allowed: true } + | { allowed: false; reason: string }; + +type AccessGateFn = ( event: InboundEvent, + userId: string | null, mg: MessagingGroup, agentGroupId: string, -) => InboundGateResult; +) => AccessGateResult; -export function setInboundGate(fn: InboundGateFn): void; +export function setAccessGate(fn: AccessGateFn): void; ``` -**Purpose:** single-setter gate that owns both sender resolution (user upsert) and access decision. Takes the raw event because the permissions module needs the sender fields inside `event.message.content`. +**Call order in `routeInbound`:** +1. Resolve messaging group. +2. **Sender resolver** (if set). Permissions upserts the users row here so the record exists even if agent resolution drops the message. +3. Resolve wired agents; `no_agent_wired` → record + drop. (Core writes the dropped_messages row.) +4. Pick agent by trigger rules; `no_trigger_match` → record + drop. +5. **Access gate** (if set). On refusal it writes its own `dropped_messages` row keyed by policy reason. -**Default when unset:** `{ allowed: true, userId: null }`. Every message routes through, no users table is needed, downstream must tolerate `userId=null`. +**Defaults when unset:** resolver returns null; gate defaults to `{ allowed: true }`. Every message routes through, no users table is needed, downstream tolerates `userId=null`. -**Current consumer:** permissions module. +**Current consumer:** permissions module (registers both). -**Not a registry, a setter.** There is one decision per inbound message and one module that owns it. Calling `setInboundGate` twice overwrites; core does not iterate. +**Not registries, setters.** There is one sender and one access decision per inbound message and one module that owns both. Calling `setSenderResolver` / `setAccessGate` twice overwrites; core does not iterate. ### 3. Response dispatcher diff --git a/src/modules/permissions/db/dropped-messages.ts b/src/db/dropped-messages.ts similarity index 96% rename from src/modules/permissions/db/dropped-messages.ts rename to src/db/dropped-messages.ts index 316b804..ce6c923 100644 --- a/src/modules/permissions/db/dropped-messages.ts +++ b/src/db/dropped-messages.ts @@ -1,4 +1,4 @@ -import { getDb } from '../../../db/connection.js'; +import { getDb } from './connection.js'; export interface UnregisteredSender { channel_type: string; diff --git a/src/modules/permissions/index.ts b/src/modules/permissions/index.ts index e79424d..e7cc282 100644 --- a/src/modules/permissions/index.ts +++ b/src/modules/permissions/index.ts @@ -1,25 +1,25 @@ /** * Permissions module — sender resolution + access gate. * - * Registers a single inbound-gate via setInboundGate(). The gate owns: - * 1. Sender resolution: parse the channel adapter's payload, derive a - * namespaced user id, and upsert the `users` row on first sight so - * role/access lookups land on a real record thereafter. - * 2. Access decision: owners → global admins → scoped admins → members. - * 3. Unknown-sender policy: strict drops; request_approval is a TODO - * (pending the `add_group_member` action kind). - * 4. Audit trail: drops get logged into `dropped_messages`. + * Registers two hooks into the core router: + * 1. setSenderResolver — runs before agent resolution. Parses the payload, + * derives a namespaced user id, and upserts the `users` row on first + * sight. Returns null when the payload doesn't carry enough to identify + * a sender. + * 2. setAccessGate — runs after agent resolution. Enforces the + * unknown_sender_policy (strict/request_approval/public) and the + * owner/global-admin/scoped-admin/member access hierarchy. Records its + * own `dropped_messages` row on refusal (structural drops are recorded + * by core). * - * Without this module: core's router defaults to allow-all (PR #2), every - * message routes through, and no users table is needed. Drops are not - * recorded anywhere. Admin commands inside the container fall back to - * permissionless mode (see formatter.ts). + * Without this module: sender resolution is a no-op (userId=null); the + * access gate is not registered and core defaults to allow-all. */ -import { setInboundGate, type InboundEvent, type InboundGateResult } from '../../router.js'; +import { recordDroppedMessage } from '../../db/dropped-messages.js'; +import { setAccessGate, setSenderResolver, type AccessGateResult, type InboundEvent } from '../../router.js'; import { log } from '../../log.js'; import type { MessagingGroup } from '../../types.js'; import { canAccessAgentGroup } from './access.js'; -import { recordDroppedMessage } from './db/dropped-messages.js'; import { getUser, upsertUser } from './db/users.js'; function extractAndUpsertUser(event: InboundEvent): string | null { @@ -111,24 +111,24 @@ function handleUnknownSender( // 'public' should have been handled before the gate; fall through silently. } -setInboundGate((event, mg, agentGroupId): InboundGateResult => { - const userId = extractAndUpsertUser(event); +setSenderResolver(extractAndUpsertUser); +setAccessGate((event, userId, mg, agentGroupId): AccessGateResult => { // Public channels skip the access check entirely. if (mg.unknown_sender_policy === 'public') { - return { allowed: true, userId }; + return { allowed: true }; } if (!userId) { handleUnknownSender(mg, null, agentGroupId, 'unknown_user', event); - return { allowed: false, userId: null, reason: 'unknown_user' }; + return { allowed: false, reason: 'unknown_user' }; } const decision = canAccessAgentGroup(userId, agentGroupId); if (decision.allowed) { - return { allowed: true, userId }; + return { allowed: true }; } handleUnknownSender(mg, userId, agentGroupId, decision.reason, event); - return { allowed: false, userId, reason: decision.reason }; + return { allowed: false, reason: decision.reason }; }); diff --git a/src/router.ts b/src/router.ts index 0da6822..8971f7f 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,16 +1,24 @@ /** * Inbound message routing. * - * Channel adapter event → resolve messaging group → pick agent → inbound - * gate (if set) → resolve/create session → write messages_in → wake - * container. + * Channel adapter event → resolve messaging group → sender resolver → + * resolve/pick agent → access gate → resolve/create session → write + * messages_in → wake container. * - * Access model lives in the permissions module via `setInboundGate`. Without - * the module, the gate is unset and every message routes through - * (downstream code tolerates `userId=null`). Drops by policy are only - * recorded when the permissions module is loaded; core just logs. + * Two module hooks (registered by the permissions module): + * - `setSenderResolver` runs BEFORE agent resolution so user rows get + * upserted even if the message ends up dropped by agent wiring. + * Without the module, userId is null and downstream code tolerates it. + * - `setAccessGate` runs AFTER agent resolution so policy decisions can + * branch on the target agent group. Without the module, access is + * allow-all. + * + * `dropped_messages` is core audit infra. Core writes rows for structural + * drops (no agent wired, no trigger match); the access gate writes rows + * for policy refusals. */ import { getChannelAdapter } from './channels/channel-registry.js'; +import { recordDroppedMessage } from './db/dropped-messages.js'; import { getMessagingGroupByPlatform, createMessagingGroup, getMessagingGroupAgents } from './db/messaging-groups.js'; import { startTypingRefresh } from './modules/typing/index.js'; import { log } from './log.js'; @@ -36,29 +44,57 @@ export interface InboundEvent { } /** - * Inbound gate hook. + * Sender-resolver hook. Runs before agent resolution. * - * The permissions module registers a gate that owns sender resolution + - * access decision + unknown-sender policy + drop-audit recording. Without - * a gate, core defaults to allow-all with `userId=null`. - * - * Takes the raw event so the gate can read sender fields from - * `event.message.content`. Returns either allowed=true with a `userId` - * (null if unresolved) or allowed=false with a reason; core drops on refusal. + * The permissions module registers this to extract the sender's namespaced + * user id and upsert the users row. Returns null when the payload doesn't + * carry enough info to identify a sender. Without the hook, every message + * arrives at the gate with userId=null. */ -export type InboundGateResult = - | { allowed: true; userId: string | null } - | { allowed: false; userId: string | null; reason: string }; +export type SenderResolverFn = (event: InboundEvent) => string | null; -export type InboundGateFn = (event: InboundEvent, mg: MessagingGroup, agentGroupId: string) => InboundGateResult; +let senderResolver: SenderResolverFn | null = null; -let inboundGate: InboundGateFn | null = null; - -export function setInboundGate(fn: InboundGateFn): void { - if (inboundGate) { - log.warn('Inbound gate overwritten'); +export function setSenderResolver(fn: SenderResolverFn): void { + if (senderResolver) { + log.warn('Sender resolver overwritten'); + } + senderResolver = fn; +} + +/** + * Access-gate hook. Runs after agent resolution. + * + * The permissions module registers this; without it, core defaults to + * allow-all. The gate receives the raw event so it can extract the sender + * name for audit-trail purposes, and it is responsible for recording its + * own `dropped_messages` row on refusal (structural drops are already + * recorded by core before the gate runs). + */ +export type AccessGateResult = { allowed: true } | { allowed: false; reason: string }; + +export type AccessGateFn = ( + event: InboundEvent, + userId: string | null, + mg: MessagingGroup, + agentGroupId: string, +) => AccessGateResult; + +let accessGate: AccessGateFn | null = null; + +export function setAccessGate(fn: AccessGateFn): void { + if (accessGate) { + log.warn('Access gate overwritten'); + } + accessGate = fn; +} + +function safeParseContent(raw: string): { text?: string; sender?: string; senderId?: string } { + try { + return JSON.parse(raw); + } catch { + return { text: raw }; } - inboundGate = fn; } /** @@ -95,7 +131,13 @@ export async function routeInbound(event: InboundEvent): Promise { }); } - // 2. Resolve agent groups wired to this messaging group. + // 2. Sender resolution (permissions module upserts the users row as a + // side effect so later role/access lookups find a real record). + // Without the module, userId is null — downstream tolerates it. + const userId: string | null = senderResolver ? senderResolver(event) : null; + + // 3. Resolve agent groups wired to this messaging group. Structural + // drops record to dropped_messages for audit. const agents = getMessagingGroupAgents(mg.id); if (agents.length === 0) { log.warn('MESSAGE DROPPED — no agent groups wired to this channel. Run setup register step to configure.', { @@ -103,6 +145,16 @@ export async function routeInbound(event: InboundEvent): Promise { channelType: event.channelType, platformId: event.platformId, }); + const parsed = safeParseContent(event.message.content); + recordDroppedMessage({ + channel_type: event.channelType, + platform_id: event.platformId, + user_id: userId, + sender_name: parsed.sender ?? null, + reason: 'no_agent_wired', + messaging_group_id: mg.id, + agent_group_id: null, + }); return; } @@ -112,17 +164,25 @@ export async function routeInbound(event: InboundEvent): Promise { messagingGroupId: mg.id, channelType: event.channelType, }); + const parsed = safeParseContent(event.message.content); + recordDroppedMessage({ + channel_type: event.channelType, + platform_id: event.platformId, + user_id: userId, + sender_name: parsed.sender ?? null, + reason: 'no_trigger_match', + messaging_group_id: mg.id, + agent_group_id: null, + }); return; } - // 3. Inbound gate (if the permissions module is loaded). Otherwise - // allow-all with userId=null — downstream code tolerates null. - let userId: string | null = null; - if (inboundGate) { - const result = inboundGate(event, mg, match.agent_group_id); - userId = result.userId; + // 4. Access gate (if the permissions module is loaded). Otherwise + // allow-all. + if (accessGate) { + const result = accessGate(event, userId, mg, match.agent_group_id); if (!result.allowed) { - log.info('MESSAGE DROPPED — inbound gate refused', { + log.info('MESSAGE DROPPED — access gate refused', { messagingGroupId: mg.id, agentGroupId: match.agent_group_id, userId, @@ -132,7 +192,7 @@ export async function routeInbound(event: InboundEvent): Promise { } } - // 4. Resolve or create session. + // 5. Resolve or create session. // // Adapter thread policy overrides the wiring's session_mode: if the adapter // is threaded, each thread gets its own session regardless of what the @@ -148,7 +208,7 @@ export async function routeInbound(event: InboundEvent): Promise { } const { session, created } = resolveSession(match.agent_group_id, mg.id, event.threadId, effectiveSessionMode); - // 5. Write message to session DB + // 6. Write message to session DB writeSessionMessage(session.agent_group_id, session.id, { id: event.message.id || generateId(), kind: event.message.kind, @@ -167,10 +227,10 @@ export async function routeInbound(event: InboundEvent): Promise { created, }); - // 6. Show typing indicator while the agent processes. + // 7. Show typing indicator while the agent processes. startTypingRefresh(session.id, session.agent_group_id, event.channelType, event.platformId, event.threadId); - // 7. Wake container + // 8. Wake container const freshSession = getSession(session.id); if (freshSession) { await wakeContainer(freshSession);