From eca8e9636d97c6d8ece8324431ed73450fe1364b Mon Sep 17 00:00:00 2001 From: Justin Chu Date: Sun, 14 Jun 2026 08:31:08 -0700 Subject: [PATCH] agents: disable Lead run/sub-agent permissions The Lead agent must not run programs or spawn sub-agents. Enforce this in the runtime (not just the prompt): - CopilotSdkAdapter: add excludedNativeToolsForRole(). The Lead session now excludes write/edit tools plus all shell/exec tools and all sub-agent (task) launchers. Read tools (view/read_file/grep/glob) remain available so the Lead can read files for context. Applied to spawn + resumeSession. - AcpAdapter.createTerminal: keep the read-only command allowlist for the lead role and reject execution with a message to delegate to the Director. Tests cover the new exclusion helper (Lead excludes run + sub-agent tools, keeps read tools; Director keeps sub-agents; non-management roles get none). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/server/src/agents/AcpAdapter.ts | 8 ++- .../server/src/agents/CopilotSdkAdapter.ts | 57 +++++++++++++------ .../tests/agents/copilot-sdk-adapter.test.ts | 52 ++++++++++++++++- .../agents/lead-command-allowlist.test.ts | 7 ++- 4 files changed, 100 insertions(+), 24 deletions(-) diff --git a/packages/server/src/agents/AcpAdapter.ts b/packages/server/src/agents/AcpAdapter.ts index 1a577966..27c8d8c6 100644 --- a/packages/server/src/agents/AcpAdapter.ts +++ b/packages/server/src/agents/AcpAdapter.ts @@ -293,7 +293,9 @@ export class AcpAdapter extends AgentAdapter { // --- Terminal capabilities (Client provides these to the Agent) --- async createTerminal(params: CreateTerminalRequest): Promise { - // Lead should not run arbitrary commands — only read operations + // Lead has no permission to run programs and no sub-agents, but it MAY + // read files for context. Allow only read-only commands; deny everything + // else (execution, writes, sub-agents). Delegate the rest to the Director. if (session.role === 'lead') { const cmd = params.command; const cmdLower = cmd.toLowerCase(); @@ -306,9 +308,11 @@ export class AcpAdapter extends AgentAdapter { const firstToken = cmdLower.trim().split(/\s+/)[0]; // Handle absolute paths like /usr/bin/cat → cat const cmdBasename = firstToken.split('/').pop() ?? firstToken; + // Read-only commands only. Lead can read files for context, but cannot + // run programs, write files, or spawn sub-agents. const allowedLeadCmds = ['cat', 'ls', 'find', 'grep', 'head', 'tail', 'wc', 'echo', 'flightdeck']; if (!allowedLeadCmds.includes(cmdBasename)) { - throw new Error(`Role 'lead' cannot run '${params.command}'. Lead agents can only run read-only commands. Delegate implementation to worker agents.`); + throw new Error(`Role 'lead' cannot run '${params.command}'. Lead has no permission to run programs or spawn sub-agents — only read-only file commands are allowed. Delegate execution to the Director via flightdeck_send.`); } } const termId = `term-${randomUUID().slice(0, 8)}`; diff --git a/packages/server/src/agents/CopilotSdkAdapter.ts b/packages/server/src/agents/CopilotSdkAdapter.ts index 2c6069c6..451ca1fa 100644 --- a/packages/server/src/agents/CopilotSdkAdapter.ts +++ b/packages/server/src/agents/CopilotSdkAdapter.ts @@ -60,6 +60,41 @@ export interface CopilotAgentSession { sawReasoningDelta?: boolean; } +/** + * Native Copilot CLI tools that must be excluded for a given role. + * + * - Management roles (lead, director, scout) never get write/execute tools — + * they coordinate, they don't modify the workspace. + * - The Lead is additionally barred from running programs and from spawning + * sub-agents: it has no execution permission and no sub-agent capability. + * Anything requiring execution must be delegated to the Director. + */ +export function excludedNativeToolsForRole(role: string): string[] | undefined { + const isManagement = ['lead', 'director', 'scout'].includes(role); + if (!isManagement) return undefined; + + const tools = [ + // Write/execute — legacy tool names (Copilot CLI <1.0.40) + 'bash', 'str_replace_editor', 'write_file', 'apply_patch', + 'insert_edit_into_file', 'create_file', 'delete_file', + // Write/execute — new tool names (Copilot CLI >=1.0.40) + 'write_bash', 'create', 'edit', + ]; + + if (role === 'lead') { + // Lead has no run permission and no sub-agents. Exclude every shell/exec + // and sub-agent launcher tool name across CLI versions. + tools.push( + // Run / shell tools + 'shell', 'exec', 'run_in_terminal', 'read_bash', 'stop_bash', + // Sub-agent / task delegation launchers + 'task', 'subagent', 'run_subagent', 'launch_agent', 'spawn_agent', + ); + } + + return Array.from(new Set(tools)); +} + export class CopilotSdkAdapter extends AgentAdapter { readonly runtime: AgentRuntime = 'acp'; private client: CopilotClient | null = null; @@ -1075,16 +1110,9 @@ export class CopilotSdkAdapter extends AgentAdapter { // Format: fd-{agentId} — deterministic, maps 1:1 to the agent. const sessionId = `fd-${aid}`; - // For management roles (lead, director, scout), exclude write/execute tools - // They can still read files for context, but cannot modify anything - const isManagement = ['lead', 'director', 'scout'].includes(opts.role); - const excludedNativeTools = isManagement ? [ - // Legacy tool names (Copilot CLI <1.0.40) - 'bash', 'str_replace_editor', 'write_file', 'apply_patch', - 'insert_edit_into_file', 'create_file', 'delete_file', - // New tool names (Copilot CLI >=1.0.40) - 'write_bash', 'create', 'edit', - ] : undefined; + // For management roles (lead, director, scout), exclude write/execute tools. + // The Lead is further restricted: no run permission and no sub-agents. + const excludedNativeTools = excludedNativeToolsForRole(opts.role); const sessionConfig: SessionConfig = { sessionId, @@ -1306,14 +1334,7 @@ export class CopilotSdkAdapter extends AgentAdapter { const aid = (opts.agentId ?? makeAgentId(opts.role, Date.now().toString())) as AgentId; const tools = this.buildTools(aid, opts.role as AgentRole, opts.projectName); - const isManagement = ['lead', 'director', 'scout'].includes(opts.role); - const excludedNativeTools = isManagement ? [ - // Legacy tool names (Copilot CLI <1.0.40) - 'bash', 'str_replace_editor', 'write_file', 'apply_patch', - 'insert_edit_into_file', 'create_file', 'delete_file', - // New tool names (Copilot CLI >=1.0.40) - 'write_bash', 'create', 'edit', - ] : undefined; + const excludedNativeTools = excludedNativeToolsForRole(opts.role); const session = await client.resumeSession(opts.previousSessionId, { model: opts.model ?? this.defaultModel, diff --git a/packages/server/tests/agents/copilot-sdk-adapter.test.ts b/packages/server/tests/agents/copilot-sdk-adapter.test.ts index af631c30..e2b212f3 100644 --- a/packages/server/tests/agents/copilot-sdk-adapter.test.ts +++ b/packages/server/tests/agents/copilot-sdk-adapter.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { CopilotSdkAdapter } from '../../src/agents/CopilotSdkAdapter.js'; +import { CopilotSdkAdapter, excludedNativeToolsForRole } from '../../src/agents/CopilotSdkAdapter.js'; const mockFetch = vi.fn(); @@ -29,6 +29,56 @@ describe('CopilotSdkAdapter', () => { return getAllTools(role).map((t: any) => t.name); } + // ─── Excluded Native Tools (run permission + sub-agents) ──── + + describe('excluded native tools by role', () => { + it('lead has no run permission (shell/exec tools excluded)', () => { + const excluded = excludedNativeToolsForRole('lead')!; + for (const t of ['bash', 'write_bash', 'shell', 'exec', 'run_in_terminal']) { + expect(excluded).toContain(t); + } + }); + + it('lead has no sub-agents (task/subagent launchers excluded)', () => { + const excluded = excludedNativeToolsForRole('lead')!; + for (const t of ['task', 'subagent', 'run_subagent', 'launch_agent', 'spawn_agent']) { + expect(excluded).toContain(t); + } + }); + + it('lead still excludes write/edit tools', () => { + const excluded = excludedNativeToolsForRole('lead')!; + expect(excluded).toContain('write_file'); + expect(excluded).toContain('edit'); + }); + + it('lead keeps read tools (can read files for context)', () => { + const excluded = excludedNativeToolsForRole('lead')!; + for (const t of ['view', 'read_file', 'grep', 'glob']) { + expect(excluded).not.toContain(t); + } + }); + + it('director excludes write/execute but is not stripped of sub-agents', () => { + const excluded = excludedNativeToolsForRole('director')!; + expect(excluded).toContain('bash'); + expect(excluded).toContain('write_bash'); + expect(excluded).not.toContain('task'); + expect(excluded).not.toContain('subagent'); + expect(excluded).not.toContain('shell'); + }); + + it('non-management roles get no exclusions', () => { + expect(excludedNativeToolsForRole('worker')).toBeUndefined(); + expect(excludedNativeToolsForRole('reviewer')).toBeUndefined(); + }); + + it('returns a de-duplicated list', () => { + const excluded = excludedNativeToolsForRole('lead')!; + expect(excluded.length).toBe(new Set(excluded).size); + }); + }); + // ─── Role-Based Tool Gating ───────────────────────────────── describe('role-based tool gating', () => { diff --git a/packages/server/tests/agents/lead-command-allowlist.test.ts b/packages/server/tests/agents/lead-command-allowlist.test.ts index 5ad2dd07..7e3adce8 100644 --- a/packages/server/tests/agents/lead-command-allowlist.test.ts +++ b/packages/server/tests/agents/lead-command-allowlist.test.ts @@ -2,7 +2,8 @@ import { describe, it, expect } from 'vitest'; /** * Tests for lead role command allowlist validation. - * Extracted logic mirrors AcpAdapter.ts createTerminal guard. + * The Lead has no run/sub-agent permission, but MAY read files for context. + * Mirrors AcpAdapter.ts createTerminal guard for role 'lead'. */ const allowedLeadCmds = ['cat', 'ls', 'find', 'grep', 'head', 'tail', 'wc', 'echo', 'flightdeck']; @@ -22,7 +23,7 @@ function validateLeadCommand(command: string): { allowed: boolean; reason?: stri } describe('Lead role command allowlist', () => { - describe('allowed commands', () => { + describe('read-only commands are allowed (Lead can read files)', () => { it.each([ 'cat file.txt', 'ls -la', @@ -60,7 +61,7 @@ describe('Lead role command allowlist', () => { }); }); - describe('disallowed commands', () => { + describe('non-read-only / execution commands - rejected', () => { it.each([ 'rm -rf /', 'chmod 777 file',