From 7cc4ecc3bec35bf023f2ee1db66125ff6f65288d Mon Sep 17 00:00:00 2001 From: gavrielc Date: Sat, 18 Apr 2026 17:42:14 +0300 Subject: [PATCH 1/2] refactor(modules): extract permissions as optional module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves user-roles / users / agent-group-members / user-dms / dropped-messages / user-dm / canAccessAgentGroup into src/modules/permissions/. Module registers a single inbound-gate that owns sender resolution, access decision, unknown-sender policy, and drop-audit recording. Router slimmed from 357 → 179 lines; the inline fallback chain (extractAndUpsertUser / enforceAccess / handleUnknownSender / recordDroppedMessage) is gone — without the permissions module core defaults to allow-all with userId=null. container-runner's admin-ID query is now inline SQL guarded by sqlite_master on user_roles, keeping core free of any import from the permissions module. The container-side formatter falls back to permissionless mode when NANOCLAW_ADMIN_USER_IDS is empty: every sender with an identifiable senderId is treated as admin. Module contract doc formalizes the tier model and the dependency rule (core ← default modules ← optional modules). One transitional violation flagged: src/access.ts (core) imports from the permissions module for its remaining approver-picking helpers; resolves in the planned PR #7 re-tier. Validation: host build clean, 137/137 host tests, 17/17 container tests, typecheck clean, service boots to "NanoClaw running" with permissions module registering its gate and clean SIGTERM shutdown. Co-Authored-By: Claude Opus 4.7 (1M context) --- container/agent-runner/src/poll-loop.ts | 7 +- docs/module-contract.md | 43 +++- src/access.test.ts | 24 +- src/access.ts | 63 ++--- src/container-runner.ts | 25 +- src/db/index.ts | 16 -- src/modules/index.ts | 1 + src/modules/permissions/access.ts | 29 +++ .../permissions}/db/agent-group-members.ts | 4 +- .../permissions}/db/dropped-messages.ts | 2 +- src/{ => modules/permissions}/db/user-dms.ts | 4 +- .../permissions}/db/user-roles.ts | 4 +- src/{ => modules/permissions}/db/users.ts | 4 +- src/modules/permissions/index.ts | 134 +++++++++++ src/{ => modules/permissions}/user-dm.ts | 8 +- src/router.ts | 215 ++---------------- 16 files changed, 279 insertions(+), 304 deletions(-) create mode 100644 src/modules/permissions/access.ts rename src/{ => modules/permissions}/db/agent-group-members.ts (93%) rename src/{ => modules/permissions}/db/dropped-messages.ts (96%) rename src/{ => modules/permissions}/db/user-dms.ts (90%) rename src/{ => modules/permissions}/db/user-roles.ts (96%) rename src/{ => modules/permissions}/db/users.ts (91%) create mode 100644 src/modules/permissions/index.ts rename src/{ => modules/permissions}/user-dm.ts (96%) diff --git a/container/agent-runner/src/poll-loop.ts b/container/agent-runner/src/poll-loop.ts index e5ad1a5..288c060 100644 --- a/container/agent-runner/src/poll-loop.ts +++ b/container/agent-runner/src/poll-loop.ts @@ -80,6 +80,10 @@ 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[] = []; @@ -99,7 +103,8 @@ export async function runPollLoop(config: PollLoopConfig): Promise { } if (cmdInfo.category === 'admin') { - if (!cmdInfo.senderId || !adminUserIds.has(cmdInfo.senderId)) { + const authorized = permissionless ? !!cmdInfo.senderId : !!cmdInfo.senderId && adminUserIds.has(cmdInfo.senderId); + if (!authorized) { 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 c65a6b3..cc423e8 100644 --- a/docs/module-contract.md +++ b/docs/module-contract.md @@ -4,23 +4,46 @@ This doc is the authoritative reference for how core and modules connect. Everyt ## Principles -- Core runs standalone. The `src/modules/index.ts` barrel can be empty and NanoClaw still routes messages in and delivers responses out. -- Modules are independent. No module imports from another module. Cross-module coordination goes through a core dispatcher. +- Core runs standalone (modulo default modules — see tiers below). The optional-module portion of the `src/modules/index.ts` barrel can be empty and NanoClaw still routes messages in and delivers responses out. +- Optional modules are independent. No optional module imports from another optional module. Cross-module coordination goes through a core registry (delivery action, response handler, etc.). - Registries exist only when multiple modules plug into the same decision point. Single-consumer integrations use skill edits (`MODULE-HOOK` markers) or stay inline with `sqlite_master` guards. -- Removing a module = delete files + remove barrel imports + revert any `MODULE-HOOK` content. Migration files stay (data is preserved). +- Removing an optional module = delete files + remove barrel imports + revert any `MODULE-HOOK` content. Migration files stay (data is preserved). Removing a default module is more invasive: it requires editing the core files that import from it. ## Module taxonomy -Three categories: +Three categories. All three live under `src/modules/` (or equivalent adapter dirs) with the same folder layout; the distinction is about **shipping** and **who can depend on them**. -1. **Default modules** — ship on `main`, live in `src/modules/` for signaling, core imports them directly. No hook, no registry. Removing requires editing core imports (deliberately less frictionless than registry modules — the friction signals "not really core, but you probably want it"). -2. **Registry-based modules** — live on the `modules` branch, installed via `/add-` skills. Plug into core through one of the four registries below. -3. **Channel adapters** — live on the `channels` branch, installed via `/add-` skills. Not covered by this contract; they use the pre-existing `ChannelAdapter` interface and `registerChannelAdapter()`. +### 1. Default modules -Current default modules: +Ship with `main` in `src/modules/`. Imported by the default `src/modules/index.ts` barrel from day one. They are not really core — they live under `src/modules/` specifically to signal "not really core, rippable if needed" — but they're always present on a `main` install. Core imports from them directly. No hook, no registry indirection for the exports themselves. -- `src/modules/typing/` — typing indicator refresh -- `src/modules/mount-security/` — container mount allowlist validation +Current: `typing`, `mount-security`. + +### 2. Optional modules + +Live on the `modules` branch. Installed via `/add-` skills that cherry-pick files. Register into core via one of the four registries (or `MODULE-HOOK` skill edits). Core and other optional modules must not statically import an optional module's code. + +Current: `interactive`, `approvals`, `scheduling`, `permissions`. Pending: `agent-to-agent`. + +### 3. Channel adapters + +Live on the `channels` branch, installed via `/add-` skills. Not covered by this contract; they use the pre-existing `ChannelAdapter` interface and `registerChannelAdapter()`. + +## Dependency rule + +``` +core ← default modules ← optional modules +``` + +- **Core** may import from core and from default modules. +- **Default modules** may import from core and from other default modules. They must not import from optional modules. +- **Optional modules** may import from core and from default modules. They must not import from each other. + +Peer-to-peer coupling between optional modules goes through a core registry — see "The four registries" below. This keeps the module dependency graph a DAG and install order irrelevant. + +### Known transitional violations + +- `src/access.ts` (core) imports from `src/modules/permissions/` (optional). Shim left from PR #5; resolved in the planned approvals re-tier (PR #7) which moves approver-picking into a new default `approvals-primitive` module that may then depend on permissions however it likes — at which point `src/access.ts` ceases to exist. ## The four registries diff --git a/src/access.test.ts b/src/access.test.ts index ff59656..92c01ce 100644 --- a/src/access.test.ts +++ b/src/access.test.ts @@ -1,23 +1,15 @@ import { beforeEach, afterEach, describe, expect, it } from 'vitest'; -import { canAccessAgentGroup, pickApprovalDelivery, pickApprover } from './access.js'; +import { pickApprovalDelivery, pickApprover } from './access.js'; import type { ChannelAdapter, OutboundMessage } from './channels/adapter.js'; import { initChannelAdapters, registerChannelAdapter, teardownChannelAdapters } from './channels/channel-registry.js'; -import { - addMember, - closeDb, - createAgentGroup, - createMessagingGroup, - createUser, - getUserDm, - grantRole, - hasAnyOwner, - initTestDb, - isMember, - isOwner, - runMigrations, -} from './db/index.js'; -import { ensureUserDm } from './user-dm.js'; +import { closeDb, createAgentGroup, createMessagingGroup, initTestDb, runMigrations } from './db/index.js'; +import { canAccessAgentGroup } from './modules/permissions/access.js'; +import { addMember, isMember } from './modules/permissions/db/agent-group-members.js'; +import { createUser } from './modules/permissions/db/users.js'; +import { grantRole, hasAnyOwner, isOwner } from './modules/permissions/db/user-roles.js'; +import { getUserDm } from './modules/permissions/db/user-dms.js'; +import { ensureUserDm } from './modules/permissions/user-dm.js'; function now(): string { return new Date().toISOString(); diff --git a/src/access.ts b/src/access.ts index 7a6d4c1..818e5e8 100644 --- a/src/access.ts +++ b/src/access.ts @@ -1,56 +1,30 @@ /** - * Access control + approval routing. + * Approval routing helpers (temporary home). * - * Privilege is user-level, not group-level. A user holds zero or more roles - * (owner | admin) via `user_roles`, and is optionally "known" in specific - * agent groups via `agent_group_members`. Admins are implicitly members of - * the groups they administer. + * These functions pick an approver for a sensitive action and resolve the + * DM messaging_group they should be delivered to. They're called only from + * the approvals module. * - * Sensitive actions trigger an approval flow, routed to the admin of the - * originating agent group; if none, the owner. Approval delivery lands in - * the approver's DM on (ideally) the same channel kind as the originating - * request. DM resolution (including cold DMs) is handled by ensureUserDm. + * PR #5 moved the access-decision half of this file (canAccessAgentGroup + + * AccessDecision) into src/modules/permissions/. The approver-picking half + * stays here as a temporary shim — PR #7 relocates it into a new default + * approvals-primitive module alongside the approvals re-tier. + * + * Tier note: this file lives in core but imports from the permissions + * optional module. That's a deliberate temporary violation; see the module + * contract + REFACTOR_PLAN open question #3. */ -import { getAgentGroup } from './db/agent-groups.js'; -import { isMember } from './db/agent-group-members.js'; import { getAdminsOfAgentGroup, getGlobalAdmins, getOwners, - hasAdminPrivilege, - isAdminOfAgentGroup, - isGlobalAdmin, - isOwner, -} from './db/user-roles.js'; -import { getUser } from './db/users.js'; -import { ensureUserDm } from './user-dm.js'; +} from './modules/permissions/db/user-roles.js'; +import { ensureUserDm } from './modules/permissions/user-dm.js'; import type { MessagingGroup } from './types.js'; -export type AccessDecision = - | { allowed: true; reason: 'owner' | 'global_admin' | 'admin_of_group' | 'member' } - | { allowed: false; reason: 'unknown_user' | 'not_member' }; - -/** Can this user interact with this agent group? */ -export function canAccessAgentGroup(userId: string, agentGroupId: string): AccessDecision { - if (!getUser(userId)) return { allowed: false, reason: 'unknown_user' }; - if (isOwner(userId)) return { allowed: true, reason: 'owner' }; - if (isGlobalAdmin(userId)) return { allowed: true, reason: 'global_admin' }; - if (isAdminOfAgentGroup(userId, agentGroupId)) return { allowed: true, reason: 'admin_of_group' }; - if (isMember(userId, agentGroupId)) return { allowed: true, reason: 'member' }; - return { allowed: false, reason: 'not_member' }; -} - -/** Can this user perform privileged (admin) operations on this agent group? */ -export function canAdminAgentGroup(userId: string, agentGroupId: string): boolean { - return hasAdminPrivilege(userId, agentGroupId); -} - /** * Ordered list of user IDs eligible to approve an action for the given agent * group. Preference: admins @ that group → global admins → owners. - * - * The approver-picking policy is to try local admins first (they have direct - * context for the group), then fall back to global scope. */ export function pickApprover(agentGroupId: string | null): string[] { const approvers: string[] = []; @@ -100,15 +74,6 @@ export async function pickApprovalDelivery( return null; } -/** - * Resolve the agent group id for a session's originating request. Used by - * approval routing so we know which scope to pick admins from. - */ -export function agentGroupIdForSession(sessionAgentGroupId: string | null): string | null { - if (!sessionAgentGroupId) return null; - return getAgentGroup(sessionAgentGroupId)?.id ?? null; -} - function channelTypeOf(userId: string): string { const idx = userId.indexOf(':'); return idx < 0 ? '' : userId.slice(0, idx); diff --git a/src/container-runner.ts b/src/container-runner.ts index 18d7eb9..1fe3e57 100644 --- a/src/container-runner.ts +++ b/src/container-runner.ts @@ -14,7 +14,6 @@ import { readContainerConfig, writeContainerConfig } from './container-config.js import { CONTAINER_RUNTIME_BIN, hostGatewayArgs, readonlyMountArgs, stopContainer } from './container-runtime.js'; import { getAgentGroup } from './db/agent-groups.js'; import { getDb, hasTable } from './db/connection.js'; -import { getAdminsOfAgentGroup, getGlobalAdmins, getOwners } from './db/user-roles.js'; import { initGroupFilesystem } from './group-init.js'; import { stopTypingRefresh } from './modules/typing/index.js'; import { log } from './log.js'; @@ -288,14 +287,26 @@ async function buildContainerArgs( // Computed at wake time: owners + global admins + admins scoped to this // agent group. Role changes take effect on next container spawn. // - // Guarded: if the permissions module isn't installed, `user_roles` - // doesn't exist and the set stays empty — the formatter treats an - // empty admin set as permissionless (every sender is admin). + // SQL inlined to keep core independent of the permissions module — we + // guard on the `user_roles` table directly. If the permissions module + // isn't installed, the table doesn't exist and the set stays empty; the + // formatter treats an empty admin set as permissionless mode (every + // sender is admin). const adminUserIds = new Set(); if (hasTable(getDb(), 'user_roles')) { - for (const r of getOwners()) adminUserIds.add(r.user_id); - for (const r of getGlobalAdmins()) adminUserIds.add(r.user_id); - for (const r of getAdminsOfAgentGroup(agentGroup.id)) adminUserIds.add(r.user_id); + const db = getDb(); + const owners = db + .prepare("SELECT user_id FROM user_roles WHERE role = 'owner' AND agent_group_id IS NULL") + .all() as Array<{ user_id: string }>; + const globalAdmins = db + .prepare("SELECT user_id FROM user_roles WHERE role = 'admin' AND agent_group_id IS NULL") + .all() as Array<{ user_id: string }>; + const scopedAdmins = db + .prepare("SELECT user_id FROM user_roles WHERE role = 'admin' AND agent_group_id = ?") + .all(agentGroup.id) as Array<{ user_id: string }>; + for (const r of owners) adminUserIds.add(r.user_id); + for (const r of globalAdmins) adminUserIds.add(r.user_id); + for (const r of scopedAdmins) adminUserIds.add(r.user_id); } if (adminUserIds.size > 0) { args.push('-e', `NANOCLAW_ADMIN_USER_IDS=${Array.from(adminUserIds).join(',')}`); diff --git a/src/db/index.ts b/src/db/index.ts index eaabe5b..0e4285a 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -8,22 +8,6 @@ export { updateAgentGroup, deleteAgentGroup, } from './agent-groups.js'; -export { createUser, upsertUser, getUser, getAllUsers, updateDisplayName, deleteUser } from './users.js'; -export { - grantRole, - revokeRole, - getUserRoles, - isOwner, - isGlobalAdmin, - isAdminOfAgentGroup, - hasAdminPrivilege, - getOwners, - hasAnyOwner, - getGlobalAdmins, - getAdminsOfAgentGroup, -} from './user-roles.js'; -export { addMember, removeMember, getMembers, isMember, hasMembershipRow } from './agent-group-members.js'; -export { upsertUserDm, getUserDm, getUserDmsForUser, deleteUserDm } from './user-dms.js'; export { createMessagingGroup, getMessagingGroup, diff --git a/src/modules/index.ts b/src/modules/index.ts index ed6b832..3ca1b5f 100644 --- a/src/modules/index.ts +++ b/src/modules/index.ts @@ -16,4 +16,5 @@ import './interactive/index.js'; import './approvals/index.js'; import './scheduling/index.js'; +import './permissions/index.js'; diff --git a/src/modules/permissions/access.ts b/src/modules/permissions/access.ts new file mode 100644 index 0000000..fb7c491 --- /dev/null +++ b/src/modules/permissions/access.ts @@ -0,0 +1,29 @@ +/** + * Access control (permissions module half of src/access.ts). + * + * Privilege is user-level, not group-level. A user holds zero or more roles + * (owner | admin) via `user_roles`, and is optionally "known" in specific + * agent groups via `agent_group_members`. Admins are implicitly members of + * the groups they administer. + * + * The approver-picking functions (pickApprover, pickApprovalDelivery) stay + * in src/access.ts for now — they move into the approvals module in the + * planned PR #7 re-tier. + */ +import { isMember } from './db/agent-group-members.js'; +import { isAdminOfAgentGroup, isGlobalAdmin, isOwner } from './db/user-roles.js'; +import { getUser } from './db/users.js'; + +export type AccessDecision = + | { allowed: true; reason: 'owner' | 'global_admin' | 'admin_of_group' | 'member' } + | { allowed: false; reason: 'unknown_user' | 'not_member' }; + +/** Can this user interact with this agent group? */ +export function canAccessAgentGroup(userId: string, agentGroupId: string): AccessDecision { + if (!getUser(userId)) return { allowed: false, reason: 'unknown_user' }; + if (isOwner(userId)) return { allowed: true, reason: 'owner' }; + if (isGlobalAdmin(userId)) return { allowed: true, reason: 'global_admin' }; + if (isAdminOfAgentGroup(userId, agentGroupId)) return { allowed: true, reason: 'admin_of_group' }; + if (isMember(userId, agentGroupId)) return { allowed: true, reason: 'member' }; + return { allowed: false, reason: 'not_member' }; +} diff --git a/src/db/agent-group-members.ts b/src/modules/permissions/db/agent-group-members.ts similarity index 93% rename from src/db/agent-group-members.ts rename to src/modules/permissions/db/agent-group-members.ts index d2b6c8f..e5e96b4 100644 --- a/src/db/agent-group-members.ts +++ b/src/modules/permissions/db/agent-group-members.ts @@ -1,5 +1,5 @@ -import type { AgentGroupMember } from '../types.js'; -import { getDb } from './connection.js'; +import type { AgentGroupMember } from '../../../types.js'; +import { getDb } from '../../../db/connection.js'; import { isAdminOfAgentGroup, isGlobalAdmin, isOwner } from './user-roles.js'; export function addMember(row: AgentGroupMember): void { diff --git a/src/db/dropped-messages.ts b/src/modules/permissions/db/dropped-messages.ts similarity index 96% rename from src/db/dropped-messages.ts rename to src/modules/permissions/db/dropped-messages.ts index ce6c923..316b804 100644 --- a/src/db/dropped-messages.ts +++ b/src/modules/permissions/db/dropped-messages.ts @@ -1,4 +1,4 @@ -import { getDb } from './connection.js'; +import { getDb } from '../../../db/connection.js'; export interface UnregisteredSender { channel_type: string; diff --git a/src/db/user-dms.ts b/src/modules/permissions/db/user-dms.ts similarity index 90% rename from src/db/user-dms.ts rename to src/modules/permissions/db/user-dms.ts index c788662..6cb3473 100644 --- a/src/db/user-dms.ts +++ b/src/modules/permissions/db/user-dms.ts @@ -1,5 +1,5 @@ -import type { UserDm } from '../types.js'; -import { getDb } from './connection.js'; +import type { UserDm } from '../../../types.js'; +import { getDb } from '../../../db/connection.js'; export function upsertUserDm(row: UserDm): void { getDb() diff --git a/src/db/user-roles.ts b/src/modules/permissions/db/user-roles.ts similarity index 96% rename from src/db/user-roles.ts rename to src/modules/permissions/db/user-roles.ts index 4dad28e..a27d508 100644 --- a/src/db/user-roles.ts +++ b/src/modules/permissions/db/user-roles.ts @@ -1,5 +1,5 @@ -import type { UserRole, UserRoleKind } from '../types.js'; -import { getDb } from './connection.js'; +import type { UserRole, UserRoleKind } from '../../../types.js'; +import { getDb } from '../../../db/connection.js'; /** * Grant a role. Owner rows must have agent_group_id = null (enforced here, diff --git a/src/db/users.ts b/src/modules/permissions/db/users.ts similarity index 91% rename from src/db/users.ts rename to src/modules/permissions/db/users.ts index 5b99fcc..31a14a7 100644 --- a/src/db/users.ts +++ b/src/modules/permissions/db/users.ts @@ -1,5 +1,5 @@ -import type { User } from '../types.js'; -import { getDb } from './connection.js'; +import type { User } from '../../../types.js'; +import { getDb } from '../../../db/connection.js'; export function createUser(user: User): void { getDb() diff --git a/src/modules/permissions/index.ts b/src/modules/permissions/index.ts new file mode 100644 index 0000000..e79424d --- /dev/null +++ b/src/modules/permissions/index.ts @@ -0,0 +1,134 @@ +/** + * 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`. + * + * 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). + */ +import { setInboundGate, type InboundEvent, type InboundGateResult } 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 { + let content: Record; + try { + content = JSON.parse(event.message.content) as Record; + } catch { + return null; + } + + // chat-sdk-bridge serializes author info as a nested `author.userId` and + // does NOT populate top-level `senderId`. Older adapters (v1, native) put + // `senderId` or `sender` directly at the top level. Check all three. + const senderIdField = typeof content.senderId === 'string' ? content.senderId : undefined; + const senderField = typeof content.sender === 'string' ? content.sender : undefined; + const author = + typeof content.author === 'object' && content.author !== null + ? (content.author as Record) + : undefined; + const authorUserId = typeof author?.userId === 'string' ? (author.userId as string) : undefined; + const senderName = + (typeof content.senderName === 'string' ? content.senderName : undefined) ?? + (typeof author?.fullName === 'string' ? (author.fullName as string) : undefined) ?? + (typeof author?.userName === 'string' ? (author.userName as string) : undefined); + + const rawHandle = senderIdField ?? senderField ?? authorUserId; + if (!rawHandle) return null; + + const userId = rawHandle.includes(':') ? rawHandle : `${event.channelType}:${rawHandle}`; + if (!getUser(userId)) { + upsertUser({ + id: userId, + kind: event.channelType, + display_name: senderName ?? null, + created_at: new Date().toISOString(), + }); + } + return userId; +} + +function safeParseContent(raw: string): { text?: string; sender?: string; senderId?: string } { + try { + return JSON.parse(raw); + } catch { + return { text: raw }; + } +} + +function handleUnknownSender( + mg: MessagingGroup, + userId: string | null, + agentGroupId: string, + accessReason: string, + event: InboundEvent, +): void { + const parsed = safeParseContent(event.message.content); + const dropRecord = { + channel_type: event.channelType, + platform_id: event.platformId, + user_id: userId, + sender_name: parsed.sender ?? null, + reason: `unknown_sender_${mg.unknown_sender_policy}`, + messaging_group_id: mg.id, + agent_group_id: agentGroupId, + }; + + if (mg.unknown_sender_policy === 'strict') { + log.info('MESSAGE DROPPED — unknown sender (strict policy)', { + messagingGroupId: mg.id, + agentGroupId, + userId, + accessReason, + }); + recordDroppedMessage(dropRecord); + return; + } + + if (mg.unknown_sender_policy === 'request_approval') { + log.info('MESSAGE DROPPED — unknown sender (approval flow TODO)', { + messagingGroupId: mg.id, + agentGroupId, + userId, + accessReason, + }); + recordDroppedMessage(dropRecord); + return; + } + + // 'public' should have been handled before the gate; fall through silently. +} + +setInboundGate((event, mg, agentGroupId): InboundGateResult => { + const userId = extractAndUpsertUser(event); + + // Public channels skip the access check entirely. + if (mg.unknown_sender_policy === 'public') { + return { allowed: true, userId }; + } + + if (!userId) { + handleUnknownSender(mg, null, agentGroupId, 'unknown_user', event); + return { allowed: false, userId: null, reason: 'unknown_user' }; + } + + const decision = canAccessAgentGroup(userId, agentGroupId); + if (decision.allowed) { + return { allowed: true, userId }; + } + + handleUnknownSender(mg, userId, agentGroupId, decision.reason, event); + return { allowed: false, userId, reason: decision.reason }; +}); diff --git a/src/user-dm.ts b/src/modules/permissions/user-dm.ts similarity index 96% rename from src/user-dm.ts rename to src/modules/permissions/user-dm.ts index d373ae3..ef9566a 100644 --- a/src/user-dm.ts +++ b/src/modules/permissions/user-dm.ts @@ -32,12 +32,12 @@ * channel on repeated calls, so re-resolving after a cache miss is always * safe — worst case we round-trip redundantly. */ -import { getChannelAdapter } from './channels/channel-registry.js'; -import { getMessagingGroup, getMessagingGroupByPlatform, createMessagingGroup } from './db/messaging-groups.js'; +import { getChannelAdapter } from '../../channels/channel-registry.js'; +import { getMessagingGroup, getMessagingGroupByPlatform, createMessagingGroup } from '../../db/messaging-groups.js'; +import { log } from '../../log.js'; +import type { MessagingGroup, User } from '../../types.js'; import { getUser } from './db/users.js'; import { getUserDm, upsertUserDm } from './db/user-dms.js'; -import { log } from './log.js'; -import type { MessagingGroup, User } from './types.js'; /** * Return a messaging_group usable to DM this user, creating it lazily if diff --git a/src/router.ts b/src/router.ts index b1ef684..0da6822 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,32 +1,22 @@ /** - * Inbound message routing for v2. + * Inbound message routing. * - * Channel adapter event → resolve messaging group → access gate → resolve - * agent group → resolve/create session → write messages_in → wake container. + * Channel adapter event → resolve messaging group → pick agent → inbound + * gate (if set) → resolve/create session → write messages_in → wake + * container. * - * Privilege / access model: - * - Owners and global admins: always allowed - * - Scoped admins: allowed in their agent group - * - Known members (agent_group_members row): allowed in that agent group - * - Everyone else: message is dropped per `messaging_groups.unknown_sender_policy` - * (strict / request_approval / public) - * - * Sender normalization: we derive a namespaced user id from the message - * content. This is best-effort — native adapters put `sender` in content, - * chat-sdk-bridge adapters put `senderId`. Adapters should populate both - * wherever possible so the gate can land on a real user row. + * 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. */ -import { canAccessAgentGroup } from './access.js'; import { getChannelAdapter } from './channels/channel-registry.js'; -import { isMember } from './db/agent-group-members.js'; import { getMessagingGroupByPlatform, createMessagingGroup, getMessagingGroupAgents } from './db/messaging-groups.js'; -import { upsertUser, getUser } from './db/users.js'; import { startTypingRefresh } from './modules/typing/index.js'; import { log } from './log.js'; import { resolveSession, writeSessionMessage } from './session-manager.js'; import { wakeContainer } from './container-runner.js'; import { getSession } from './db/sessions.js'; -import { recordDroppedMessage } from './db/dropped-messages.js'; import type { MessagingGroup, MessagingGroupAgent } from './types.js'; function generateId(): string { @@ -46,17 +36,15 @@ export interface InboundEvent { } /** - * Inbound gate registry. + * Inbound gate hook. * - * A module (permissions, today) can register a single gate function that - * owns sender resolution + access decision. Without a registered gate, - * core falls back to the inline `extractAndUpsertUser` + - * `enforceAccess` + `handleUnknownSender` chain. + * 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 the - * message on refusal. + * (null if unresolved) or allowed=false with a reason; core drops on refusal. */ export type InboundGateResult = | { allowed: true; userId: string | null } @@ -79,9 +67,7 @@ export function setInboundGate(fn: InboundGateFn): void { */ export async function routeInbound(event: InboundEvent): Promise { // 0. Apply the adapter's thread policy. Non-threaded adapters (Telegram, - // WhatsApp, iMessage, email) collapse threads to the channel — the - // agent always replies to the main channel regardless of where the - // inbound came from. + // WhatsApp, iMessage, email) collapse threads to the channel. const adapter = getChannelAdapter(event.channelType); if (adapter && !adapter.supportsThreads) { event = { ...event, threadId: null }; @@ -109,8 +95,7 @@ export async function routeInbound(event: InboundEvent): Promise { }); } - // 2. Resolve agent groups wired to this messaging group. (The gate runs - // after this so it can decide based on the target agent group.) + // 2. Resolve agent groups wired to this messaging group. 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.', { @@ -118,43 +103,21 @@ 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: parsed.senderId ?? null, - sender_name: parsed.sender ?? null, - reason: 'no_agent_wired', - messaging_group_id: mg.id, - agent_group_id: null, - }); return; } - // Pick the best matching agent (highest priority, trigger matching in future) const match = pickAgent(agents, event); if (!match) { log.warn('MESSAGE DROPPED — no agent matched trigger rules', { messagingGroupId: mg.id, channelType: event.channelType, }); - const parsed = safeParseContent(event.message.content); - recordDroppedMessage({ - channel_type: event.channelType, - platform_id: event.platformId, - user_id: parsed.senderId ?? null, - sender_name: parsed.sender ?? null, - reason: 'no_trigger_match', - messaging_group_id: mg.id, - agent_group_id: null, - }); return; } - // 3. Inbound gate: sender resolution + access decision. If a module - // registered a gate, it owns the whole thing (it can upsert users, - // check roles, etc.). Otherwise fall back to the inline chain. - let userId: string | null; + // 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; @@ -167,23 +130,13 @@ export async function routeInbound(event: InboundEvent): Promise { }); return; } - } else { - userId = extractAndUpsertUser(event); - if (mg.unknown_sender_policy !== 'public') { - const gate = enforceAccess(userId, match.agent_group_id); - if (!gate.allowed) { - handleUnknownSender(mg, userId, match.agent_group_id, gate.reason, event); - return; - } - } } - // 5. Resolve or create session. + // 4. 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 - // wiring says, because "thread = session" is the first-class model for - // threaded platforms. Agent-shared is preserved because it expresses a + // wiring says. Agent-shared is preserved because it expresses a // cross-channel intent the adapter can't know about. // // Exception: DMs (is_group=0). Sub-threads within a DM are a UX affordance, @@ -195,7 +148,7 @@ export async function routeInbound(event: InboundEvent): Promise { } const { session, created } = resolveSession(match.agent_group_id, mg.id, event.threadId, effectiveSessionMode); - // 6. Write message to session DB + // 5. Write message to session DB writeSessionMessage(session.agent_group_id, session.id, { id: event.message.id || generateId(), kind: event.message.kind, @@ -214,16 +167,10 @@ export async function routeInbound(event: InboundEvent): Promise { created, }); - // 7. Show typing indicator while the agent processes. Refresh on a short - // interval so platforms like Discord (which auto-expire typing after - // ~10s) keep showing it for the full thinking window. Gated on the - // heartbeat file's mtime after an initial grace period, so typing stops - // as soon as the agent goes idle — not when the container eventually - // exits. Container-runner also calls stopTypingRefresh on exit as a - // fast-path cleanup. + // 6. Show typing indicator while the agent processes. startTypingRefresh(session.id, session.agent_group_id, event.channelType, event.platformId, event.threadId); - // 8. Wake container + // 7. Wake container const freshSession = getSession(session.id); if (freshSession) { await wakeContainer(freshSession); @@ -239,119 +186,3 @@ function pickAgent(agents: MessagingGroupAgent[], _event: InboundEvent): Messagi // TODO: apply trigger_rules matching (pattern, mentionOnly, etc.) return agents[0] ?? null; } - -/** - * Best-effort sender extraction. Returns a namespaced user id like - * `telegram:123` or null if nothing usable is present. - * - * Side-effect: upserts the user into the `users` table so access/approval - * lookups can find them on subsequent messages. - * - * The namespace uses the channel_type as `kind` for now — e.g. `whatsapp:...` - * rather than `phone:...`. That's imprecise (a phone number is really the - * identifier, not the channel) but it keeps the first cut simple. A proper - * kind mapping (channel → kind) can happen when we start linking identities - * across channels. - */ -function extractAndUpsertUser(event: InboundEvent): string | null { - let content: Record; - try { - content = JSON.parse(event.message.content) as Record; - } catch { - return null; - } - - // chat-sdk-bridge serializes author info as a nested `author.userId` and - // does NOT populate top-level `senderId`. Older adapters (v1, native) put - // `senderId` or `sender` directly at the top level. Check all three. - const senderIdField = typeof content.senderId === 'string' ? content.senderId : undefined; - const senderField = typeof content.sender === 'string' ? content.sender : undefined; - const author = - typeof content.author === 'object' && content.author !== null - ? (content.author as Record) - : undefined; - const authorUserId = typeof author?.userId === 'string' ? (author.userId as string) : undefined; - const senderName = - (typeof content.senderName === 'string' ? content.senderName : undefined) ?? - (typeof author?.fullName === 'string' ? (author.fullName as string) : undefined) ?? - (typeof author?.userName === 'string' ? (author.userName as string) : undefined); - - const rawHandle = senderIdField ?? senderField ?? authorUserId; - if (!rawHandle) return null; - - // If the raw handle already contains ':' it's pre-namespaced (the older - // adapters put it in that form). Otherwise prepend the channel type. - const userId = rawHandle.includes(':') ? rawHandle : `${event.channelType}:${rawHandle}`; - if (!getUser(userId)) { - upsertUser({ - id: userId, - kind: event.channelType, - display_name: senderName ?? null, - created_at: new Date().toISOString(), - }); - } - return userId; -} - -function enforceAccess(userId: string | null, agentGroupId: string): { allowed: boolean; reason: string } { - if (!userId) return { allowed: false, reason: 'unknown_user' }; - const decision = canAccessAgentGroup(userId, agentGroupId); - if (decision.allowed) return { allowed: true, reason: decision.reason }; - return { allowed: false, reason: decision.reason }; -} - -function handleUnknownSender( - mg: MessagingGroup, - userId: string | null, - agentGroupId: string, - accessReason: string, - event: InboundEvent, -): void { - const parsed = safeParseContent(event.message.content); - const dropRecord = { - channel_type: event.channelType, - platform_id: event.platformId, - user_id: userId, - sender_name: parsed.sender ?? null, - reason: `unknown_sender_${mg.unknown_sender_policy}`, - messaging_group_id: mg.id, - agent_group_id: agentGroupId, - }; - - // In 'strict' mode we just drop. In 'request_approval' mode we log and - // queue an approval to add the sender as a member — the approval flow - // itself is a follow-up (needs an action kind like `add_group_member`). - if (mg.unknown_sender_policy === 'strict') { - log.info('MESSAGE DROPPED — unknown sender (strict policy)', { - messagingGroupId: mg.id, - agentGroupId, - userId, - accessReason, - }); - recordDroppedMessage(dropRecord); - return; - } - - if (mg.unknown_sender_policy === 'request_approval') { - log.info('MESSAGE DROPPED — unknown sender (approval flow TODO)', { - messagingGroupId: mg.id, - agentGroupId, - userId, - accessReason, - }); - recordDroppedMessage(dropRecord); - return; - } - - // Should be unreachable — 'public' was handled before the gate. - // Ensure the membership invariant isn't in an odd state. - void isMember; -} - -function safeParseContent(raw: string): { text?: string; sender?: string; senderId?: string } { - try { - return JSON.parse(raw); - } catch { - return { text: raw }; - } -} From 32bcc2c5ae1897d65bc636b05cfb86eff6139d52 Mon Sep 17 00:00:00 2001 From: gavrielc Date: Sat, 18 Apr 2026 18:00:10 +0300 Subject: [PATCH 2/2] refactor(permissions): preserve pre-PR behavior in three spots PR #5 review flagged three behavior changes that shouldn't have slipped in. This commit reverts each to match the pre-refactor behavior exactly. 1. User upsert ordering. Split the router hook into two setters: setSenderResolver (runs before agent resolution) and setAccessGate (runs after). Restores the pre-PR sequence where the users row is upserted even if the message is dropped by wiring or trigger rules. 2. dropped_messages audit. Moved src/modules/permissions/db/dropped-messages.ts back to src/db/dropped-messages.ts. The table is core audit infra, not permissions-specific. Router re-writes rows for no_agent_wired and no_trigger_match; the access gate writes rows for policy refusals. 3. Permissionless container fallback. Dropped. poll-loop restores the original deny-all check when NANOCLAW_ADMIN_USER_IDS is empty. Module contract doc updated with the two-hook shape. Validation: host build clean, 137/137 host tests, 17/17 container tests, typecheck clean, service boots to "NanoClaw running" with permissions module registering both hooks and clean SIGTERM shutdown. Co-Authored-By: Claude Opus 4.7 (1M context) --- container/agent-runner/src/poll-loop.ts | 7 +- docs/module-contract.md | 34 +++-- .../permissions => }/db/dropped-messages.ts | 2 +- src/modules/permissions/index.ts | 40 +++--- src/router.ts | 134 +++++++++++++----- 5 files changed, 142 insertions(+), 75 deletions(-) rename src/{modules/permissions => }/db/dropped-messages.ts (96%) 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);