From 7cc4ecc3bec35bf023f2ee1db66125ff6f65288d Mon Sep 17 00:00:00 2001 From: gavrielc Date: Sat, 18 Apr 2026 17:42:14 +0300 Subject: [PATCH] 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 e5ad1a5cd..288c060a2 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 c65a6b394..cc423e8f8 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 ff59656f2..92c01ce77 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 7a6d4c1e4..818e5e8f1 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 18d7eb9ac..1fe3e578a 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 eaabe5bb7..0e4285a02 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 ed6b83266..3ca1b5ff6 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 000000000..fb7c4919f --- /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 d2b6c8f3c..e5e96b472 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 ce6c9237e..316b804c5 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 c78866203..6cb347358 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 4dad28ea8..a27d50895 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 5b99fcc00..31a14a742 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 000000000..e79424d16 --- /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 d373ae32b..ef9566ad8 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 b1ef6844d..0da682260 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 }; - } -}