Skip to content

Commit 7cc4ecc

Browse files
gavrielcclaude
andcommitted
refactor(modules): extract permissions as optional module
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) <noreply@anthropic.com>
1 parent e75af5e commit 7cc4ecc

16 files changed

Lines changed: 279 additions & 304 deletions

File tree

container/agent-runner/src/poll-loop.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ export async function runPollLoop(config: PollLoopConfig): Promise<void> {
8080

8181
// Handle commands: categorize chat messages
8282
const adminUserIds = config.adminUserIds ?? new Set<string>();
83+
// Permissionless mode: when the permissions module isn't installed on
84+
// the host, NANOCLAW_ADMIN_USER_IDS arrives empty. Treat every sender
85+
// with an identifiable senderId as admin so admin commands still work.
86+
const permissionless = adminUserIds.size === 0;
8387
const normalMessages = [];
8488
const commandIds: string[] = [];
8589

@@ -99,7 +103,8 @@ export async function runPollLoop(config: PollLoopConfig): Promise<void> {
99103
}
100104

101105
if (cmdInfo.category === 'admin') {
102-
if (!cmdInfo.senderId || !adminUserIds.has(cmdInfo.senderId)) {
106+
const authorized = permissionless ? !!cmdInfo.senderId : !!cmdInfo.senderId && adminUserIds.has(cmdInfo.senderId);
107+
if (!authorized) {
103108
log(`Admin command denied: ${cmdInfo.command} from ${cmdInfo.senderId} (msg: ${msg.id})`);
104109
writeMessageOut({
105110
id: generateId(),

docs/module-contract.md

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,46 @@ This doc is the authoritative reference for how core and modules connect. Everyt
44

55
## Principles
66

7-
- Core runs standalone. The `src/modules/index.ts` barrel can be empty and NanoClaw still routes messages in and delivers responses out.
8-
- Modules are independent. No module imports from another module. Cross-module coordination goes through a core dispatcher.
7+
- 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.
8+
- 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.).
99
- 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.
10-
- Removing a module = delete files + remove barrel imports + revert any `MODULE-HOOK` content. Migration files stay (data is preserved).
10+
- 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.
1111

1212
## Module taxonomy
1313

14-
Three categories:
14+
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**.
1515

16-
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").
17-
2. **Registry-based modules** — live on the `modules` branch, installed via `/add-<name>` skills. Plug into core through one of the four registries below.
18-
3. **Channel adapters** — live on the `channels` branch, installed via `/add-<channel>` skills. Not covered by this contract; they use the pre-existing `ChannelAdapter` interface and `registerChannelAdapter()`.
16+
### 1. Default modules
1917

20-
Current default modules:
18+
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.
2119

22-
- `src/modules/typing/` — typing indicator refresh
23-
- `src/modules/mount-security/` — container mount allowlist validation
20+
Current: `typing`, `mount-security`.
21+
22+
### 2. Optional modules
23+
24+
Live on the `modules` branch. Installed via `/add-<name>` 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.
25+
26+
Current: `interactive`, `approvals`, `scheduling`, `permissions`. Pending: `agent-to-agent`.
27+
28+
### 3. Channel adapters
29+
30+
Live on the `channels` branch, installed via `/add-<channel>` skills. Not covered by this contract; they use the pre-existing `ChannelAdapter` interface and `registerChannelAdapter()`.
31+
32+
## Dependency rule
33+
34+
```
35+
core ← default modules ← optional modules
36+
```
37+
38+
- **Core** may import from core and from default modules.
39+
- **Default modules** may import from core and from other default modules. They must not import from optional modules.
40+
- **Optional modules** may import from core and from default modules. They must not import from each other.
41+
42+
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.
43+
44+
### Known transitional violations
45+
46+
- `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.
2447

2548
## The four registries
2649

src/access.test.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,15 @@
11
import { beforeEach, afterEach, describe, expect, it } from 'vitest';
22

3-
import { canAccessAgentGroup, pickApprovalDelivery, pickApprover } from './access.js';
3+
import { pickApprovalDelivery, pickApprover } from './access.js';
44
import type { ChannelAdapter, OutboundMessage } from './channels/adapter.js';
55
import { initChannelAdapters, registerChannelAdapter, teardownChannelAdapters } from './channels/channel-registry.js';
6-
import {
7-
addMember,
8-
closeDb,
9-
createAgentGroup,
10-
createMessagingGroup,
11-
createUser,
12-
getUserDm,
13-
grantRole,
14-
hasAnyOwner,
15-
initTestDb,
16-
isMember,
17-
isOwner,
18-
runMigrations,
19-
} from './db/index.js';
20-
import { ensureUserDm } from './user-dm.js';
6+
import { closeDb, createAgentGroup, createMessagingGroup, initTestDb, runMigrations } from './db/index.js';
7+
import { canAccessAgentGroup } from './modules/permissions/access.js';
8+
import { addMember, isMember } from './modules/permissions/db/agent-group-members.js';
9+
import { createUser } from './modules/permissions/db/users.js';
10+
import { grantRole, hasAnyOwner, isOwner } from './modules/permissions/db/user-roles.js';
11+
import { getUserDm } from './modules/permissions/db/user-dms.js';
12+
import { ensureUserDm } from './modules/permissions/user-dm.js';
2113

2214
function now(): string {
2315
return new Date().toISOString();

src/access.ts

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,30 @@
11
/**
2-
* Access control + approval routing.
2+
* Approval routing helpers (temporary home).
33
*
4-
* Privilege is user-level, not group-level. A user holds zero or more roles
5-
* (owner | admin) via `user_roles`, and is optionally "known" in specific
6-
* agent groups via `agent_group_members`. Admins are implicitly members of
7-
* the groups they administer.
4+
* These functions pick an approver for a sensitive action and resolve the
5+
* DM messaging_group they should be delivered to. They're called only from
6+
* the approvals module.
87
*
9-
* Sensitive actions trigger an approval flow, routed to the admin of the
10-
* originating agent group; if none, the owner. Approval delivery lands in
11-
* the approver's DM on (ideally) the same channel kind as the originating
12-
* request. DM resolution (including cold DMs) is handled by ensureUserDm.
8+
* PR #5 moved the access-decision half of this file (canAccessAgentGroup +
9+
* AccessDecision) into src/modules/permissions/. The approver-picking half
10+
* stays here as a temporary shim — PR #7 relocates it into a new default
11+
* approvals-primitive module alongside the approvals re-tier.
12+
*
13+
* Tier note: this file lives in core but imports from the permissions
14+
* optional module. That's a deliberate temporary violation; see the module
15+
* contract + REFACTOR_PLAN open question #3.
1316
*/
14-
import { getAgentGroup } from './db/agent-groups.js';
15-
import { isMember } from './db/agent-group-members.js';
1617
import {
1718
getAdminsOfAgentGroup,
1819
getGlobalAdmins,
1920
getOwners,
20-
hasAdminPrivilege,
21-
isAdminOfAgentGroup,
22-
isGlobalAdmin,
23-
isOwner,
24-
} from './db/user-roles.js';
25-
import { getUser } from './db/users.js';
26-
import { ensureUserDm } from './user-dm.js';
21+
} from './modules/permissions/db/user-roles.js';
22+
import { ensureUserDm } from './modules/permissions/user-dm.js';
2723
import type { MessagingGroup } from './types.js';
2824

29-
export type AccessDecision =
30-
| { allowed: true; reason: 'owner' | 'global_admin' | 'admin_of_group' | 'member' }
31-
| { allowed: false; reason: 'unknown_user' | 'not_member' };
32-
33-
/** Can this user interact with this agent group? */
34-
export function canAccessAgentGroup(userId: string, agentGroupId: string): AccessDecision {
35-
if (!getUser(userId)) return { allowed: false, reason: 'unknown_user' };
36-
if (isOwner(userId)) return { allowed: true, reason: 'owner' };
37-
if (isGlobalAdmin(userId)) return { allowed: true, reason: 'global_admin' };
38-
if (isAdminOfAgentGroup(userId, agentGroupId)) return { allowed: true, reason: 'admin_of_group' };
39-
if (isMember(userId, agentGroupId)) return { allowed: true, reason: 'member' };
40-
return { allowed: false, reason: 'not_member' };
41-
}
42-
43-
/** Can this user perform privileged (admin) operations on this agent group? */
44-
export function canAdminAgentGroup(userId: string, agentGroupId: string): boolean {
45-
return hasAdminPrivilege(userId, agentGroupId);
46-
}
47-
4825
/**
4926
* Ordered list of user IDs eligible to approve an action for the given agent
5027
* group. Preference: admins @ that group → global admins → owners.
51-
*
52-
* The approver-picking policy is to try local admins first (they have direct
53-
* context for the group), then fall back to global scope.
5428
*/
5529
export function pickApprover(agentGroupId: string | null): string[] {
5630
const approvers: string[] = [];
@@ -100,15 +74,6 @@ export async function pickApprovalDelivery(
10074
return null;
10175
}
10276

103-
/**
104-
* Resolve the agent group id for a session's originating request. Used by
105-
* approval routing so we know which scope to pick admins from.
106-
*/
107-
export function agentGroupIdForSession(sessionAgentGroupId: string | null): string | null {
108-
if (!sessionAgentGroupId) return null;
109-
return getAgentGroup(sessionAgentGroupId)?.id ?? null;
110-
}
111-
11277
function channelTypeOf(userId: string): string {
11378
const idx = userId.indexOf(':');
11479
return idx < 0 ? '' : userId.slice(0, idx);

src/container-runner.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { readContainerConfig, writeContainerConfig } from './container-config.js
1414
import { CONTAINER_RUNTIME_BIN, hostGatewayArgs, readonlyMountArgs, stopContainer } from './container-runtime.js';
1515
import { getAgentGroup } from './db/agent-groups.js';
1616
import { getDb, hasTable } from './db/connection.js';
17-
import { getAdminsOfAgentGroup, getGlobalAdmins, getOwners } from './db/user-roles.js';
1817
import { initGroupFilesystem } from './group-init.js';
1918
import { stopTypingRefresh } from './modules/typing/index.js';
2019
import { log } from './log.js';
@@ -288,14 +287,26 @@ async function buildContainerArgs(
288287
// Computed at wake time: owners + global admins + admins scoped to this
289288
// agent group. Role changes take effect on next container spawn.
290289
//
291-
// Guarded: if the permissions module isn't installed, `user_roles`
292-
// doesn't exist and the set stays empty — the formatter treats an
293-
// empty admin set as permissionless (every sender is admin).
290+
// SQL inlined to keep core independent of the permissions module — we
291+
// guard on the `user_roles` table directly. If the permissions module
292+
// isn't installed, the table doesn't exist and the set stays empty; the
293+
// formatter treats an empty admin set as permissionless mode (every
294+
// sender is admin).
294295
const adminUserIds = new Set<string>();
295296
if (hasTable(getDb(), 'user_roles')) {
296-
for (const r of getOwners()) adminUserIds.add(r.user_id);
297-
for (const r of getGlobalAdmins()) adminUserIds.add(r.user_id);
298-
for (const r of getAdminsOfAgentGroup(agentGroup.id)) adminUserIds.add(r.user_id);
297+
const db = getDb();
298+
const owners = db
299+
.prepare("SELECT user_id FROM user_roles WHERE role = 'owner' AND agent_group_id IS NULL")
300+
.all() as Array<{ user_id: string }>;
301+
const globalAdmins = db
302+
.prepare("SELECT user_id FROM user_roles WHERE role = 'admin' AND agent_group_id IS NULL")
303+
.all() as Array<{ user_id: string }>;
304+
const scopedAdmins = db
305+
.prepare("SELECT user_id FROM user_roles WHERE role = 'admin' AND agent_group_id = ?")
306+
.all(agentGroup.id) as Array<{ user_id: string }>;
307+
for (const r of owners) adminUserIds.add(r.user_id);
308+
for (const r of globalAdmins) adminUserIds.add(r.user_id);
309+
for (const r of scopedAdmins) adminUserIds.add(r.user_id);
299310
}
300311
if (adminUserIds.size > 0) {
301312
args.push('-e', `NANOCLAW_ADMIN_USER_IDS=${Array.from(adminUserIds).join(',')}`);

src/db/index.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,6 @@ export {
88
updateAgentGroup,
99
deleteAgentGroup,
1010
} from './agent-groups.js';
11-
export { createUser, upsertUser, getUser, getAllUsers, updateDisplayName, deleteUser } from './users.js';
12-
export {
13-
grantRole,
14-
revokeRole,
15-
getUserRoles,
16-
isOwner,
17-
isGlobalAdmin,
18-
isAdminOfAgentGroup,
19-
hasAdminPrivilege,
20-
getOwners,
21-
hasAnyOwner,
22-
getGlobalAdmins,
23-
getAdminsOfAgentGroup,
24-
} from './user-roles.js';
25-
export { addMember, removeMember, getMembers, isMember, hasMembershipRow } from './agent-group-members.js';
26-
export { upsertUserDm, getUserDm, getUserDmsForUser, deleteUserDm } from './user-dms.js';
2711
export {
2812
createMessagingGroup,
2913
getMessagingGroup,

src/modules/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,5 @@
1616
import './interactive/index.js';
1717
import './approvals/index.js';
1818
import './scheduling/index.js';
19+
import './permissions/index.js';
1920

src/modules/permissions/access.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Access control (permissions module half of src/access.ts).
3+
*
4+
* Privilege is user-level, not group-level. A user holds zero or more roles
5+
* (owner | admin) via `user_roles`, and is optionally "known" in specific
6+
* agent groups via `agent_group_members`. Admins are implicitly members of
7+
* the groups they administer.
8+
*
9+
* The approver-picking functions (pickApprover, pickApprovalDelivery) stay
10+
* in src/access.ts for now — they move into the approvals module in the
11+
* planned PR #7 re-tier.
12+
*/
13+
import { isMember } from './db/agent-group-members.js';
14+
import { isAdminOfAgentGroup, isGlobalAdmin, isOwner } from './db/user-roles.js';
15+
import { getUser } from './db/users.js';
16+
17+
export type AccessDecision =
18+
| { allowed: true; reason: 'owner' | 'global_admin' | 'admin_of_group' | 'member' }
19+
| { allowed: false; reason: 'unknown_user' | 'not_member' };
20+
21+
/** Can this user interact with this agent group? */
22+
export function canAccessAgentGroup(userId: string, agentGroupId: string): AccessDecision {
23+
if (!getUser(userId)) return { allowed: false, reason: 'unknown_user' };
24+
if (isOwner(userId)) return { allowed: true, reason: 'owner' };
25+
if (isGlobalAdmin(userId)) return { allowed: true, reason: 'global_admin' };
26+
if (isAdminOfAgentGroup(userId, agentGroupId)) return { allowed: true, reason: 'admin_of_group' };
27+
if (isMember(userId, agentGroupId)) return { allowed: true, reason: 'member' };
28+
return { allowed: false, reason: 'not_member' };
29+
}

src/db/agent-group-members.ts renamed to src/modules/permissions/db/agent-group-members.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import type { AgentGroupMember } from '../types.js';
2-
import { getDb } from './connection.js';
1+
import type { AgentGroupMember } from '../../../types.js';
2+
import { getDb } from '../../../db/connection.js';
33
import { isAdminOfAgentGroup, isGlobalAdmin, isOwner } from './user-roles.js';
44

55
export function addMember(row: AgentGroupMember): void {

src/db/dropped-messages.ts renamed to src/modules/permissions/db/dropped-messages.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getDb } from './connection.js';
1+
import { getDb } from '../../../db/connection.js';
22

33
export interface UnregisteredSender {
44
channel_type: string;

0 commit comments

Comments
 (0)