mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-06-04 10:14:47 +08:00
fix(security): block cli_scope escalation and cross-group data leaks
Group-scoped agents could previously: - See all agent groups via `groups list` (generic list skips --id filter) - Look up any session by UUID via `sessions get` - Request cli_scope change to global via config update approval Fixed by: - Post-handler filtering: list results filtered, get results verified against caller's agent_group_id - Pre-handler --id check scoped to resources where id IS the group ID (groups, destinations) so session UUIDs aren't falsely rejected - cli_scope/cli-scope args blocked outright for group-scoped agents, before the approval gate Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -91,6 +91,31 @@ register({
|
||||
handler: async (args) => ({ echo: args }),
|
||||
});
|
||||
|
||||
// Commands that return data shaped like real resources (for post-handler filtering tests)
|
||||
register({
|
||||
name: 'groups-list-data',
|
||||
description: 'returns mock group rows',
|
||||
resource: 'groups',
|
||||
access: 'open',
|
||||
parseArgs: (raw) => raw,
|
||||
handler: async () => [
|
||||
{ id: 'g1', name: 'my-group' },
|
||||
{ id: 'g2', name: 'other-group' },
|
||||
],
|
||||
});
|
||||
|
||||
register({
|
||||
name: 'sessions-get-data',
|
||||
description: 'returns a mock session row',
|
||||
resource: 'sessions',
|
||||
access: 'open',
|
||||
parseArgs: (raw) => raw,
|
||||
handler: async (args) => ({
|
||||
id: args.id,
|
||||
agent_group_id: (args as Record<string, unknown>).belongs_to ?? 'g1',
|
||||
}),
|
||||
});
|
||||
|
||||
import { dispatch } from './dispatch.js';
|
||||
import type { CallerContext } from './frame.js';
|
||||
|
||||
@@ -157,6 +182,35 @@ describe('CLI scope enforcement', () => {
|
||||
expect(resp.ok).toBe(true);
|
||||
});
|
||||
|
||||
it('group: blocks cli_scope escalation', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
|
||||
const resp = await dispatch(
|
||||
{ id: '1', command: 'groups-test', args: { cli_scope: 'global' } },
|
||||
agentCtx(),
|
||||
);
|
||||
|
||||
expect(resp.ok).toBe(false);
|
||||
if (!resp.ok) {
|
||||
expect(resp.error.code).toBe('forbidden');
|
||||
expect(resp.error.message).toContain('cli_scope');
|
||||
}
|
||||
});
|
||||
|
||||
it('group: blocks cli-scope escalation (hyphenated)', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
|
||||
const resp = await dispatch(
|
||||
{ id: '1', command: 'groups-test', args: { 'cli-scope': 'global' } },
|
||||
agentCtx(),
|
||||
);
|
||||
|
||||
expect(resp.ok).toBe(false);
|
||||
if (!resp.ok) {
|
||||
expect(resp.error.code).toBe('forbidden');
|
||||
}
|
||||
});
|
||||
|
||||
it('group: blocks non-group resources', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
|
||||
@@ -301,4 +355,57 @@ describe('CLI scope enforcement', () => {
|
||||
expect(resp.ok).toBe(true);
|
||||
expect(mockGetContainerConfig).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// --- Post-handler filtering ---
|
||||
|
||||
it('group: groups list filters out other groups', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
|
||||
const resp = await dispatch({ id: '1', command: 'groups-list-data', args: {} }, agentCtx());
|
||||
|
||||
expect(resp.ok).toBe(true);
|
||||
if (resp.ok) {
|
||||
const data = resp.data as Array<{ id: string }>;
|
||||
expect(data).toHaveLength(1);
|
||||
expect(data[0].id).toBe('g1');
|
||||
}
|
||||
});
|
||||
|
||||
it('group: sessions get rejects cross-group session', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
|
||||
const resp = await dispatch(
|
||||
{ id: '1', command: 'sessions-get-data', args: { id: 's-123', belongs_to: 'other-group' } },
|
||||
agentCtx(),
|
||||
);
|
||||
|
||||
expect(resp.ok).toBe(false);
|
||||
if (!resp.ok) {
|
||||
expect(resp.error.code).toBe('forbidden');
|
||||
expect(resp.error.message).toContain('different agent group');
|
||||
}
|
||||
});
|
||||
|
||||
it('group: sessions get allows own-group session', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'group' });
|
||||
|
||||
const resp = await dispatch(
|
||||
{ id: '1', command: 'sessions-get-data', args: { id: 's-123', belongs_to: 'g1' } },
|
||||
agentCtx(),
|
||||
);
|
||||
|
||||
expect(resp.ok).toBe(true);
|
||||
});
|
||||
|
||||
it('global: no post-handler filtering', async () => {
|
||||
mockGetContainerConfig.mockReturnValue({ cli_scope: 'global' });
|
||||
|
||||
const resp = await dispatch({ id: '1', command: 'groups-list-data', args: {} }, agentCtx());
|
||||
|
||||
expect(resp.ok).toBe(true);
|
||||
if (resp.ok) {
|
||||
const data = resp.data as Array<{ id: string }>;
|
||||
expect(data).toHaveLength(2); // both groups returned
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
+32
-2
@@ -55,12 +55,21 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise<R
|
||||
|
||||
// Enforce group scope on all agent-group-related args.
|
||||
// Different resources use different arg names for the agent group ID.
|
||||
const groupArgs = ['id', 'agent_group_id', 'group'] as const;
|
||||
// Only check --id for resources where it IS the agent group ID.
|
||||
const groupArgs = ['agent_group_id', 'group'] as const;
|
||||
for (const key of groupArgs) {
|
||||
if (req.args[key] && req.args[key] !== ctx.agentGroupId) {
|
||||
return err(req.id, 'forbidden', 'CLI access is scoped to this agent group.');
|
||||
}
|
||||
}
|
||||
if ((cmd.resource === 'groups' || cmd.resource === 'destinations') && req.args.id && req.args.id !== ctx.agentGroupId) {
|
||||
return err(req.id, 'forbidden', 'CLI access is scoped to this agent group.');
|
||||
}
|
||||
|
||||
// Block cli_scope changes from group-scoped agents (privilege escalation)
|
||||
if (req.args.cli_scope !== undefined || req.args['cli-scope'] !== undefined) {
|
||||
return err(req.id, 'forbidden', 'Cannot change cli_scope from a group-scoped agent.');
|
||||
}
|
||||
|
||||
// Auto-fill agent-group-related args so the agent doesn't need
|
||||
// to pass its own group ID explicitly.
|
||||
@@ -109,7 +118,28 @@ export async function dispatch(req: RequestFrame, ctx: CallerContext): Promise<R
|
||||
}
|
||||
|
||||
try {
|
||||
const data = await cmd.handler(parsed, ctx);
|
||||
let data = await cmd.handler(parsed, ctx);
|
||||
|
||||
// Post-handler group scope enforcement: filter/verify results belong
|
||||
// to the caller's agent group. Catches leaks that pre-handler auto-fill
|
||||
// can't prevent (e.g. `groups list` where the id arg is skipped by the
|
||||
// generic list handler, or `sessions get` by UUID).
|
||||
if (ctx.caller === 'agent' && cmd.resource) {
|
||||
const configRow = getContainerConfig(ctx.agentGroupId);
|
||||
if ((configRow?.cli_scope ?? 'group') === 'group') {
|
||||
const groupField = cmd.resource === 'groups' ? 'id' : 'agent_group_id';
|
||||
if (Array.isArray(data)) {
|
||||
data = data.filter(
|
||||
(row) => typeof row === 'object' && row !== null && (row as Record<string, unknown>)[groupField] === ctx.agentGroupId,
|
||||
);
|
||||
} else if (data && typeof data === 'object' && groupField in (data as Record<string, unknown>)) {
|
||||
if ((data as Record<string, unknown>)[groupField] !== ctx.agentGroupId) {
|
||||
return err(req.id, 'forbidden', 'Resource belongs to a different agent group.');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { id: req.id, ok: true, data };
|
||||
} catch (e) {
|
||||
return err(req.id, 'handler-error', errMsg(e));
|
||||
|
||||
Reference in New Issue
Block a user