From ae2c09cbde2a3c0d5fd3d759e88d686fd6f165e3 Mon Sep 17 00:00:00 2001 From: glifocat Date: Thu, 23 Apr 2026 10:33:54 +0200 Subject: [PATCH 1/8] docs: add fork-specific notes in FORK.md --- FORK.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 FORK.md diff --git a/FORK.md b/FORK.md new file mode 100644 index 0000000..f92e260 --- /dev/null +++ b/FORK.md @@ -0,0 +1,15 @@ +# nanoclaw-glifocat + +Personal [NanoClaw](https://github.com/qwibitai/nanoclaw) fork - customizations, upstream contributions and experimentation. + +## About + +I'm a core maintainer of NanoClaw and also maintain the official documentation at [docs.nanoclaw.dev](https://docs.nanoclaw.dev). + +This fork is where I develop fixes and features before contributing them upstream, test experimental integrations, and run my personal agent setup. + +## Links + +- **Upstream repo:** [qwibitai/nanoclaw](https://github.com/qwibitai/nanoclaw) +- **Official docs:** [docs.nanoclaw.dev](https://docs.nanoclaw.dev) +- **Docs repo:** [glifocat/nanoclaw-docs](https://github.com/glifocat/nanoclaw-docs) From 47c85d0985a157552edd08e1c20f91f2135fac66 Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:30:15 +0200 Subject: [PATCH 2/8] fix(cli-scope): add scopeField to ResourceDef for fail-closed group scope --- src/cli/crud.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cli/crud.ts b/src/cli/crud.ts index 9c7ed99..f2bb82b 100644 --- a/src/cli/crud.ts +++ b/src/cli/crud.ts @@ -52,6 +52,12 @@ export interface ResourceDef { description: string; /** Primary key column name. */ idColumn: string; + /** + * Column that carries the agent group ID for group-scope enforcement. + * Required on every resource in the CLI whitelist (groups, sessions, + * destinations, members). When absent, post-handler filtering fails closed. + */ + scopeField?: string; columns: ColumnDef[]; /** Which standard CRUD operations are enabled. */ operations: { From 8a8ec84ef1cb560d46d10257f7a7f9e597b729df Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:30:25 +0200 Subject: [PATCH 3/8] fix(cli-scope): fail-closed scopeField enforcement and sessions-get oracle guard --- src/cli/dispatch.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 68db969..13d4761 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -11,6 +11,7 @@ import { getAgentGroup } from '../db/agent-groups.js'; import { getSession } from '../db/sessions.js'; import { registerApprovalHandler, requestApproval } from '../modules/approvals/index.js'; import type { CallerContext, ErrorCode, RequestFrame, ResponseFrame } from './frame.js'; +import { getResource } from './crud.js'; import { lookup } from './registry.js'; export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise { @@ -87,6 +88,16 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise).agent_group_id !== ctx.agentGroupId) { + return err(req.id, 'handler-error', `session not found: ${req.args.id}`); + } + } } } @@ -131,7 +142,12 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise @@ -139,7 +155,7 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise)[groupField] === ctx.agentGroupId, ); - } else if (data && typeof data === 'object' && groupField in (data as Record)) { + } else if (data && typeof data === 'object') { if ((data as Record)[groupField] !== ctx.agentGroupId) { return err(req.id, 'forbidden', 'Resource belongs to a different agent group.'); } From 610a6925190abbcf7f3fa5440e6b3611e81d549a Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:30:30 +0200 Subject: [PATCH 4/8] fix(cli-scope): add scopeField to groups, sessions, destinations, members --- src/cli/resources/groups.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/resources/groups.ts b/src/cli/resources/groups.ts index 83344e0..031ccb5 100644 --- a/src/cli/resources/groups.ts +++ b/src/cli/resources/groups.ts @@ -38,6 +38,7 @@ registerResource({ description: 'Agent group — a logical agent identity. Each group has its own workspace folder (CLAUDE.md, skills, container config), conversation history, and container image. Multiple messaging groups can be wired to one agent group.', idColumn: 'id', + scopeField: 'id', columns: [ { name: 'id', type: 'string', description: 'UUID.', generated: true }, { From d8aa46c0a759896db4069de82e29d0d24f4d48bf Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:30:40 +0200 Subject: [PATCH 5/8] fix(cli-scope): add scopeField to groups, sessions, destinations, members --- src/cli/resources/sessions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/resources/sessions.ts b/src/cli/resources/sessions.ts index f60fccc..33e2407 100644 --- a/src/cli/resources/sessions.ts +++ b/src/cli/resources/sessions.ts @@ -7,6 +7,7 @@ registerResource({ description: 'Session — the runtime unit. Maps one (agent_group, messaging_group, thread) combination to a container with its own inbound.db and outbound.db. Created automatically by the router when a message arrives.', idColumn: 'id', + scopeField: 'agent_group_id', columns: [ { name: 'id', type: 'string', description: 'UUID.', generated: true }, { name: 'agent_group_id', type: 'string', description: 'Agent group this session runs.' }, From bf34857d115b1ed8e8027799b3d164b4eded7acf Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:30:41 +0200 Subject: [PATCH 6/8] fix(cli-scope): add scopeField to groups, sessions, destinations, members --- src/cli/resources/destinations.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/resources/destinations.ts b/src/cli/resources/destinations.ts index 4a56029..0ac95f3 100644 --- a/src/cli/resources/destinations.ts +++ b/src/cli/resources/destinations.ts @@ -8,6 +8,7 @@ registerResource({ description: 'Agent destination — per-agent routing entry and ACL. Each row authorizes an agent to send messages to a target (channel or another agent) and assigns a local name the agent uses to address it. Names are scoped to the source agent — two agents can have different local names for the same target. Created automatically when wiring channels or when agents create child agents.', idColumn: 'agent_group_id', + scopeField: 'agent_group_id', columns: [ { name: 'agent_group_id', From b323b55efee07a7758c892beb0254f5068d59d7a Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:30:41 +0200 Subject: [PATCH 7/8] fix(cli-scope): add scopeField to groups, sessions, destinations, members --- src/cli/resources/members.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/resources/members.ts b/src/cli/resources/members.ts index ac529be..2a93430 100644 --- a/src/cli/resources/members.ts +++ b/src/cli/resources/members.ts @@ -8,6 +8,7 @@ registerResource({ description: 'Agent group member — grants an unprivileged user permission to interact with an agent group. Users with admin or owner roles on the group are implicitly members and do not need a separate membership row. Membership is checked by the router when sender_scope is "known".', idColumn: 'user_id', + scopeField: 'agent_group_id', columns: [ { name: 'user_id', From c6d5cd7d022a30dcb084693863ea7e5aca0156ea Mon Sep 17 00:00:00 2001 From: glifocat Date: Sun, 10 May 2026 20:47:51 +0200 Subject: [PATCH 8/8] fixup(cli-scope): build error, false-positive on custom ops, tests, drop FORK.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on this branch: - Fix TS2352 build error in dispatch.ts: `getSession()` returns `Session`, which has no index signature, so `(s as Record)` is rejected by tsc. `Session.agent_group_id` exists — read it directly. - Fix a regression introduced by dropping the `groupField in data` guard: the post-handler scope check now runs for *every* command under a whitelisted resource, including custom ops, which return ad-hoc shapes. `ncl groups config get` (access:open, reachable by a group-scoped agent) returns a config object with no `id` field → `data['id'] !== ctx.agentGroupId` → `forbidden`, even on the agent's own config. Fix: tag the auto-generated list/get handlers with `generic: 'list' | 'get'` on `CommandDef` (set in `registerResource`) and run the post-handler check only when `cmd.generic` is set. Generic handlers return raw DB rows that carry `scopeField`; custom ops are already pinned to the caller's group by the pre-handler `--id` auto-fill or the approval gate. Fail-closed-when-`scopeField`-missing is preserved (now scoped to generic list/get). - Tests: `dispatch.test.ts` mocks `getResource` (the real resources aren't registered in this unit), tags the two post-handler test commands as `generic`, and adds coverage for: custom op returning a non-row object not being rejected; `sessions-get` pre-handler returning "session not found" for foreign and non-existent UUIDs (no existence oracle) and allowing the caller's own session; generic list/get failing closed when a resource declares no `scopeField`. Full suite: 323 passing. - Remove FORK.md from the PR diff — it's the fork's personal README, carried in because the branch was cut from the fork's `main` rather than upstream. Co-Authored-By: Claude Opus 4.7 (1M context) --- FORK.md | 15 ------ src/cli/crud.ts | 2 + src/cli/dispatch.test.ts | 109 +++++++++++++++++++++++++++++++++++++++ src/cli/dispatch.ts | 23 ++++++--- src/cli/registry.ts | 7 +++ 5 files changed, 134 insertions(+), 22 deletions(-) delete mode 100644 FORK.md diff --git a/FORK.md b/FORK.md deleted file mode 100644 index f92e260..0000000 --- a/FORK.md +++ /dev/null @@ -1,15 +0,0 @@ -# nanoclaw-glifocat - -Personal [NanoClaw](https://github.com/qwibitai/nanoclaw) fork - customizations, upstream contributions and experimentation. - -## About - -I'm a core maintainer of NanoClaw and also maintain the official documentation at [docs.nanoclaw.dev](https://docs.nanoclaw.dev). - -This fork is where I develop fixes and features before contributing them upstream, test experimental integrations, and run my personal agent setup. - -## Links - -- **Upstream repo:** [qwibitai/nanoclaw](https://github.com/qwibitai/nanoclaw) -- **Official docs:** [docs.nanoclaw.dev](https://docs.nanoclaw.dev) -- **Docs repo:** [glifocat/nanoclaw-docs](https://github.com/glifocat/nanoclaw-docs) diff --git a/src/cli/crud.ts b/src/cli/crud.ts index f2bb82b..4889bf0 100644 --- a/src/cli/crud.ts +++ b/src/cli/crud.ts @@ -232,6 +232,7 @@ export function registerResource(def: ResourceDef): void { description: `List all ${def.plural}.`, access: def.operations.list, resource: def.plural, + generic: 'list', parseArgs: (raw) => normalizeArgs(raw), handler: genericList(def), }); @@ -243,6 +244,7 @@ export function registerResource(def: ResourceDef): void { description: `Get a ${def.name} by ID.`, access: def.operations.get, resource: def.plural, + generic: 'get', parseArgs: (raw) => normalizeArgs(raw), handler: genericGet(def), }); diff --git a/src/cli/dispatch.test.ts b/src/cli/dispatch.test.ts index b63d712..20ff187 100644 --- a/src/cli/dispatch.test.ts +++ b/src/cli/dispatch.test.ts @@ -21,6 +21,13 @@ vi.mock('../db/sessions.js', () => ({ getSession: (...args: unknown[]) => mockGetSession(...args), })); +// dispatch's post-handler looks up the resource's `scopeField` via getResource. +// The real resources aren't registered in this unit test, so mock it. +const mockGetResource = vi.fn(); +vi.mock('./crud.js', () => ({ + getResource: (...args: unknown[]) => mockGetResource(...args), +})); + vi.mock('../modules/approvals/index.js', () => ({ registerApprovalHandler: vi.fn(), requestApproval: vi.fn(), @@ -97,6 +104,7 @@ register({ description: 'returns mock group rows', resource: 'groups', access: 'open', + generic: 'list', parseArgs: (raw) => raw, handler: async () => [ { id: 'g1', name: 'my-group' }, @@ -109,6 +117,7 @@ register({ description: 'returns a mock session row', resource: 'sessions', access: 'open', + generic: 'get', parseArgs: (raw) => raw, handler: async (args) => ({ id: args.id, @@ -116,11 +125,43 @@ register({ }), }); +// A custom op under the `groups` resource that returns a config-shaped object +// (no `id` key). The post-handler must not touch this — only `generic` handlers. +register({ + name: 'groups-config-get', + description: 'custom op returning a config object (no id)', + resource: 'groups', + access: 'open', + parseArgs: (raw) => raw, + handler: async () => ({ agent_group_id: 'g1', model: 'opus' }), +}); + +// The real `sessions-get` name — triggers the pre-handler ownership check. +register({ + name: 'sessions-get', + description: 'generic sessions get', + resource: 'sessions', + access: 'open', + generic: 'get', + parseArgs: (raw) => raw, + handler: async (args) => ({ id: (args as Record).id, agent_group_id: 'g1' }), +}); + import { dispatch } from './dispatch.js'; import type { CallerContext } from './frame.js'; beforeEach(() => { vi.clearAllMocks(); + // Default: the four CLI-whitelisted resources with their real scopeFields. + const scopeFields: Record = { + groups: 'id', + sessions: 'agent_group_id', + destinations: 'agent_group_id', + members: 'agent_group_id', + }; + mockGetResource.mockImplementation((plural: string) => + scopeFields[plural] ? { scopeField: scopeFields[plural] } : undefined, + ); }); // --- Helpers --- @@ -402,4 +443,72 @@ describe('CLI scope enforcement', () => { expect(data).toHaveLength(2); // both groups returned } }); + + // --- Custom ops bypass post-handler row filtering (regression: #2392 review) --- + + it('group: a custom op returning a non-row object is not falsely rejected', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + + // groups-config-get is access:open and reachable by a group-scoped agent; + // it returns { agent_group_id, model } with no `id` field. Before this fix + // the post-handler compared data['id'] (undefined) and returned forbidden. + const resp = await dispatch({ id: '1', command: 'groups-config-get', args: {} }, agentCtx()); + + expect(resp.ok).toBe(true); + if (resp.ok) { + expect((resp.data as { model: string }).model).toBe('opus'); + } + }); + + // --- sessions-get pre-handler ownership check (no existence oracle) --- + + it('group: sessions-get returns "session not found" for a foreign session UUID', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetSession.mockReturnValue({ id: 's-x', agent_group_id: 'other-group' }); + + const resp = await dispatch({ id: '1', command: 'sessions-get', args: { id: 's-x' } }, agentCtx()); + + expect(resp.ok).toBe(false); + if (!resp.ok) { + expect(resp.error.code).toBe('handler-error'); + expect(resp.error.message).toContain('session not found'); + } + }); + + it('group: sessions-get returns "session not found" for a non-existent UUID', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetSession.mockReturnValue(undefined); + + const resp = await dispatch({ id: '1', command: 'sessions-get', args: { id: 's-nope' } }, agentCtx()); + + expect(resp.ok).toBe(false); + if (!resp.ok) { + expect(resp.error.code).toBe('handler-error'); + expect(resp.error.message).toContain('session not found'); + } + }); + + it('group: sessions-get allows the caller’s own session', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetSession.mockReturnValue({ id: 's-mine', agent_group_id: 'g1' }); + + const resp = await dispatch({ id: '1', command: 'sessions-get', args: { id: 's-mine' } }, agentCtx()); + + expect(resp.ok).toBe(true); + }); + + // --- Fail-closed regression guard for a missing scopeField --- + + it('group: generic list/get fails closed when the resource declares no scopeField', async () => { + mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' }); + mockGetResource.mockReturnValue(undefined); // a whitelisted resource that forgot scopeField + + const resp = await dispatch({ id: '1', command: 'groups-list-data', args: {} }, agentCtx()); + + expect(resp.ok).toBe(false); + if (!resp.ok) { + expect(resp.error.code).toBe('forbidden'); + expect(resp.error.message).toContain('not available in group scope'); + } + }); }); diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 13d4761..7878a9b 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -94,7 +94,7 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise).agent_group_id !== ctx.agentGroupId) { + if (!s || s.agent_group_id !== ctx.agentGroupId) { return err(req.id, 'handler-error', `session not found: ${req.args.id}`); } } @@ -135,17 +135,26 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise = { access: Access; /** Resource this command belongs to (for help grouping). */ resource?: string; + /** + * Set on the auto-generated `list` / `get` handlers (see `registerResource`). + * These return raw DB rows that carry the resource's `scopeField`, so the + * dispatcher applies post-handler group-scope filtering to their output. + * Custom operations return ad-hoc shapes and leave this undefined. + */ + generic?: 'list' | 'get'; /** Validates `frame.args` and produces the typed handler input. Throws on invalid. */ parseArgs: (raw: Record) => TArgs; handler: (args: TArgs, ctx: CallerContext) => Promise;