From 9ff8720da5904b67d14642a6dc4fc6859d0de619 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 9 Jul 2026 04:03:22 +0800 Subject: [PATCH] fix(tool-subagent): a partial toolFilter must not materialize an empty allow-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ds-review-bot finding: forcing only the OUTER toolFilter key absent left the nested arrays materializing — a deny-only config gained allow: [], which means deny-EVERYTHING. The nested arrays now default to undefined too; an explicit allow: [] (grant-only children) still survives. Pinned by a capture-provider regression test. --- packages/subagent/tool-subagent/src/index.ts | 9 +++++-- .../tool-subagent/tests/tool-subagent.spec.ts | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index 7cf6f0ddbb..35c23c7fed 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -102,9 +102,14 @@ export const Config: z = z.object({ // deny-everything, silently. Force the omitted key to stay absent (the same // shape discipline as SystemPrompt's toolOrder); the cast is needed because // .default() expects the object type. + // The NESTED arrays get the same treatment as the object itself: a partial + // filter ({deny: […]}) must not materialize allow: [] beside it — an empty + // allow-list means deny-EVERYTHING, so the materialized default would turn + // a deny-one config into deny-all. An EXPLICIT allow: [] (grant-only + // children) survives, since only the omitted key defaults to undefined. toolFilter: z.object({ - allow: z.array(z.string()), - deny: z.array(z.string()), + allow: z.array(z.string()).default(undefined as unknown as string[]), + deny: z.array(z.string()).default(undefined as unknown as string[]), }).default(undefined as unknown as { allow: string[]; deny: string[] }), maxDepth: z.number(), }) diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index 9510df53ab..5fd2ace838 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -484,4 +484,30 @@ describe('dsh-tool-subagent', () => { expect(seen?.toolFilter).toMatchObject({ deny: ['subagent'] }) expect(seen?.maxDepth).toBe(2) }) + + it('a partial toolFilter (deny only) does not materialize an empty allow-list (deny-all trap)', async () => { + let seen: { toolFilter?: { allow?: string[]; deny?: string[] } } | undefined + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + ctx.subagents.registerProvider({ + name: 'capture3', + capabilities: { outputSchema: false, depthLimit: false, toolFilter: true, persona: false }, + inheritsParentContext: false, + start: (request) => { + seen = request + return { + id: AgentId('capture3-child'), + result: Promise.resolve({ output: [{ type: 'text', text: 'ok' }], stopReason: 'completed' as const }), + cancel() {}, + dispose: async () => {}, + } + }, + }) + await ctx.plugin(tool, { provider: 'capture3', toolFilter: { deny: ['subagent'] } }) + await callSubagent(ctx, { description: 'd', prompt: 'p' }) + expect(seen?.toolFilter).toEqual({ deny: ['subagent'] }) + expect(seen?.toolFilter).not.toHaveProperty('allow') + }) })