diff --git a/docs/compatibility.md b/docs/compatibility.md index bc8f54cd3..268c077a3 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -124,20 +124,13 @@ The `--share` option is not available via SDK. Workarounds: ### Permission Control +The SDK uses a **deny-by-default** permission model. All permission requests (file writes, shell commands, URL fetches, etc.) are denied unless your app provides an `onPermissionRequest` handler. + Instead of `--allow-all-paths` or `--yolo`, use the permission handler: ```typescript const session = await client.createSession({ - onPermissionRequest: async (request) => { - // Auto-approve everything (equivalent to --yolo) - return { approved: true }; - - // Or implement custom logic - if (request.kind === "shell") { - return { approved: request.command.startsWith("git") }; - } - return { approved: true }; - }, + onPermissionRequest: approveAll, }); ``` diff --git a/dotnet/samples/Chat.cs b/dotnet/samples/Chat.cs index abaefc7b6..f4f12cfa2 100644 --- a/dotnet/samples/Chat.cs +++ b/dotnet/samples/Chat.cs @@ -1,7 +1,10 @@ using GitHub.Copilot.SDK; await using var client = new CopilotClient(); -await using var session = await client.CreateSessionAsync(); +await using var session = await client.CreateSessionAsync(new SessionConfig +{ + OnPermissionRequest = PermissionHandler.ApproveAll +}); using var _ = session.On(evt => { diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index f000be805..0b77af866 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -377,7 +377,7 @@ public async Task CreateSessionAsync(SessionConfig? config = nul config?.AvailableTools, config?.ExcludedTools, config?.Provider, - config?.OnPermissionRequest != null ? true : null, + (bool?)true, config?.OnUserInputRequest != null ? true : null, hasHooks ? true : null, config?.WorkingDirectory, @@ -461,7 +461,7 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes config?.AvailableTools, config?.ExcludedTools, config?.Provider, - config?.OnPermissionRequest != null ? true : null, + (bool?)true, config?.OnUserInputRequest != null ? true : null, hasHooks ? true : null, config?.WorkingDirectory, diff --git a/dotnet/src/PermissionHandlers.cs b/dotnet/src/PermissionHandlers.cs new file mode 100644 index 000000000..22e5bdb17 --- /dev/null +++ b/dotnet/src/PermissionHandlers.cs @@ -0,0 +1,13 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +namespace GitHub.Copilot.SDK; + +/// Provides pre-built implementations. +public static class PermissionHandler +{ + /// A that approves all permission requests. + public static PermissionRequestHandler ApproveAll { get; } = + (_, _) => Task.FromResult(new PermissionRequestResult { Kind = "approved" }); +} diff --git a/dotnet/src/Session.cs b/dotnet/src/Session.cs index 34f4d02d5..4feeb9f95 100644 --- a/dotnet/src/Session.cs +++ b/dotnet/src/Session.cs @@ -47,7 +47,7 @@ public partial class CopilotSession : IAsyncDisposable private readonly HashSet _eventHandlers = new(); private readonly Dictionary _toolHandlers = new(); private readonly JsonRpc _rpc; - private PermissionHandler? _permissionHandler; + private PermissionRequestHandler? _permissionHandler; private readonly SemaphoreSlim _permissionHandlerLock = new(1, 1); private UserInputHandler? _userInputHandler; private readonly SemaphoreSlim _userInputHandlerLock = new(1, 1); @@ -292,7 +292,7 @@ internal void RegisterTools(ICollection tools) /// When the assistant needs permission to perform certain actions (e.g., file operations), /// this handler is called to approve or deny the request. /// - internal void RegisterPermissionHandler(PermissionHandler handler) + internal void RegisterPermissionHandler(PermissionRequestHandler handler) { _permissionHandlerLock.Wait(); try @@ -313,7 +313,7 @@ internal void RegisterPermissionHandler(PermissionHandler handler) internal async Task HandlePermissionRequestAsync(JsonElement permissionRequestData) { await _permissionHandlerLock.WaitAsync(); - PermissionHandler? handler; + PermissionRequestHandler? handler; try { handler = _permissionHandler; diff --git a/dotnet/src/Types.cs b/dotnet/src/Types.cs index 50e39ff7c..277e88b86 100644 --- a/dotnet/src/Types.cs +++ b/dotnet/src/Types.cs @@ -166,7 +166,7 @@ public class PermissionInvocation public string SessionId { get; set; } = string.Empty; } -public delegate Task PermissionHandler(PermissionRequest request, PermissionInvocation invocation); +public delegate Task PermissionRequestHandler(PermissionRequest request, PermissionInvocation invocation); // ============================================================================ // User Input Handler Types @@ -793,7 +793,7 @@ protected SessionConfig(SessionConfig? other) /// Handler for permission requests from the server. /// When provided, the server will call this handler to request permission for operations. /// - public PermissionHandler? OnPermissionRequest { get; set; } + public PermissionRequestHandler? OnPermissionRequest { get; set; } /// /// Handler for user input requests from the agent. @@ -932,7 +932,7 @@ protected ResumeSessionConfig(ResumeSessionConfig? other) /// Handler for permission requests from the server. /// When provided, the server will call this handler to request permission for operations. /// - public PermissionHandler? OnPermissionRequest { get; set; } + public PermissionRequestHandler? OnPermissionRequest { get; set; } /// /// Handler for user input requests from the agent. diff --git a/dotnet/test/HooksTests.cs b/dotnet/test/HooksTests.cs index 34f6ecabf..44a6e66c2 100644 --- a/dotnet/test/HooksTests.cs +++ b/dotnet/test/HooksTests.cs @@ -17,6 +17,7 @@ public async Task Should_Invoke_PreToolUse_Hook_When_Model_Runs_A_Tool() CopilotSession? session = null; session = await Client.CreateSessionAsync(new SessionConfig { + OnPermissionRequest = PermissionHandler.ApproveAll, Hooks = new SessionHooks { OnPreToolUse = (input, invocation) => @@ -52,6 +53,7 @@ public async Task Should_Invoke_PostToolUse_Hook_After_Model_Runs_A_Tool() CopilotSession? session = null; session = await Client.CreateSessionAsync(new SessionConfig { + OnPermissionRequest = PermissionHandler.ApproveAll, Hooks = new SessionHooks { OnPostToolUse = (input, invocation) => @@ -89,6 +91,7 @@ public async Task Should_Invoke_Both_PreToolUse_And_PostToolUse_Hooks_For_Single var session = await Client.CreateSessionAsync(new SessionConfig { + OnPermissionRequest = PermissionHandler.ApproveAll, Hooks = new SessionHooks { OnPreToolUse = (input, invocation) => @@ -130,6 +133,7 @@ public async Task Should_Deny_Tool_Execution_When_PreToolUse_Returns_Deny() var session = await Client.CreateSessionAsync(new SessionConfig { + OnPermissionRequest = PermissionHandler.ApproveAll, Hooks = new SessionHooks { OnPreToolUse = (input, invocation) => diff --git a/dotnet/test/McpAndAgentsTests.cs b/dotnet/test/McpAndAgentsTests.cs index 2e50e77bc..644a70bf3 100644 --- a/dotnet/test/McpAndAgentsTests.cs +++ b/dotnet/test/McpAndAgentsTests.cs @@ -279,7 +279,8 @@ public async Task Should_Pass_Literal_Env_Values_To_Mcp_Server_Subprocess() var session = await Client.CreateSessionAsync(new SessionConfig { - McpServers = mcpServers + McpServers = mcpServers, + OnPermissionRequest = PermissionHandler.ApproveAll, }); Assert.Matches(@"^[a-f0-9-]+$", session.SessionId); diff --git a/dotnet/test/PermissionTests.cs b/dotnet/test/PermissionTests.cs index 237eb1f68..b1295be91 100644 --- a/dotnet/test/PermissionTests.cs +++ b/dotnet/test/PermissionTests.cs @@ -70,6 +70,30 @@ await session.SendAsync(new MessageOptions Assert.Equal("protected content", content); } + [Fact] + public async Task Should_Deny_Tool_Operations_By_Default_When_No_Handler_Is_Provided() + { + var session = await Client.CreateSessionAsync(new SessionConfig()); + var permissionDenied = false; + + session.On(evt => + { + if (evt is ToolExecutionCompleteEvent toolEvt && + !toolEvt.Data.Success && + toolEvt.Data.Error?.Message.Contains("Permission denied") == true) + { + permissionDenied = true; + } + }); + + await session.SendAndWaitAsync(new MessageOptions + { + Prompt = "Run 'node --version'" + }); + + Assert.True(permissionDenied, "Expected a tool.execution_complete event with Permission denied result"); + } + [Fact] public async Task Should_Work_Without_Permission_Handler__Default_Behavior_() { @@ -161,6 +185,37 @@ await session.SendAsync(new MessageOptions Assert.Matches("fail|cannot|unable|permission", message?.Data.Content?.ToLowerInvariant() ?? string.Empty); } + [Fact] + public async Task Should_Deny_Tool_Operations_By_Default_When_No_Handler_Is_Provided_After_Resume() + { + var session1 = await Client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll + }); + var sessionId = session1.SessionId; + await session1.SendAndWaitAsync(new MessageOptions { Prompt = "What is 1+1?" }); + + var session2 = await Client.ResumeSessionAsync(sessionId); + var permissionDenied = false; + + session2.On(evt => + { + if (evt is ToolExecutionCompleteEvent toolEvt && + !toolEvt.Data.Success && + toolEvt.Data.Error?.Message.Contains("Permission denied") == true) + { + permissionDenied = true; + } + }); + + await session2.SendAndWaitAsync(new MessageOptions + { + Prompt = "Run 'node --version'" + }); + + Assert.True(permissionDenied, "Expected a tool.execution_complete event with Permission denied result"); + } + [Fact] public async Task Should_Receive_ToolCallId_In_Permission_Requests() { diff --git a/dotnet/test/SessionTests.cs b/dotnet/test/SessionTests.cs index 920ee67d6..c9a152ce9 100644 --- a/dotnet/test/SessionTests.cs +++ b/dotnet/test/SessionTests.cs @@ -333,7 +333,10 @@ public async Task Should_Receive_Session_Events() [Fact] public async Task Send_Returns_Immediately_While_Events_Stream_In_Background() { - var session = await Client.CreateSessionAsync(); + var session = await Client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + }); var events = new List(); session.On(evt => events.Add(evt.Type)); diff --git a/dotnet/test/ToolsTests.cs b/dotnet/test/ToolsTests.cs index 3d7741c99..ad1ab7a21 100644 --- a/dotnet/test/ToolsTests.cs +++ b/dotnet/test/ToolsTests.cs @@ -21,7 +21,10 @@ await File.WriteAllTextAsync( Path.Combine(Ctx.WorkDir, "README.md"), "# ELIZA, the only chatbot you'll ever need"); - var session = await Client.CreateSessionAsync(); + var session = await Client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + }); await session.SendAsync(new MessageOptions { diff --git a/go/client.go b/go/client.go index 77a9eeeda..e415ab777 100644 --- a/go/client.go +++ b/go/client.go @@ -473,9 +473,6 @@ func (c *Client) CreateSession(ctx context.Context, config *SessionConfig) (*Ses if config.Streaming { req.Streaming = Bool(true) } - if config.OnPermissionRequest != nil { - req.RequestPermission = Bool(true) - } if config.OnUserInputRequest != nil { req.RequestUserInput = Bool(true) } @@ -488,6 +485,7 @@ func (c *Client) CreateSession(ctx context.Context, config *SessionConfig) (*Ses req.Hooks = Bool(true) } } + req.RequestPermission = Bool(true) result, err := c.client.Request("session.create", req) if err != nil { @@ -562,9 +560,6 @@ func (c *Client) ResumeSessionWithOptions(ctx context.Context, sessionID string, if config.Streaming { req.Streaming = Bool(true) } - if config.OnPermissionRequest != nil { - req.RequestPermission = Bool(true) - } if config.OnUserInputRequest != nil { req.RequestUserInput = Bool(true) } @@ -588,6 +583,7 @@ func (c *Client) ResumeSessionWithOptions(ctx context.Context, sessionID string, req.DisabledSkills = config.DisabledSkills req.InfiniteSessions = config.InfiniteSessions } + req.RequestPermission = Bool(true) result, err := c.client.Request("session.resume", req) if err != nil { diff --git a/go/internal/e2e/hooks_test.go b/go/internal/e2e/hooks_test.go index 9f1a9ec05..70aa6ec71 100644 --- a/go/internal/e2e/hooks_test.go +++ b/go/internal/e2e/hooks_test.go @@ -22,6 +22,7 @@ func TestHooks(t *testing.T) { var mu sync.Mutex session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, Hooks: &copilot.SessionHooks{ OnPreToolUse: func(input copilot.PreToolUseHookInput, invocation copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) { mu.Lock() @@ -80,6 +81,7 @@ func TestHooks(t *testing.T) { var mu sync.Mutex session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, Hooks: &copilot.SessionHooks{ OnPostToolUse: func(input copilot.PostToolUseHookInput, invocation copilot.HookInvocation) (*copilot.PostToolUseHookOutput, error) { mu.Lock() @@ -145,6 +147,7 @@ func TestHooks(t *testing.T) { var mu sync.Mutex session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, Hooks: &copilot.SessionHooks{ OnPreToolUse: func(input copilot.PreToolUseHookInput, invocation copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) { mu.Lock() @@ -214,6 +217,7 @@ func TestHooks(t *testing.T) { var mu sync.Mutex session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, Hooks: &copilot.SessionHooks{ OnPreToolUse: func(input copilot.PreToolUseHookInput, invocation copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) { mu.Lock() diff --git a/go/internal/e2e/mcp_and_agents_test.go b/go/internal/e2e/mcp_and_agents_test.go index 7ba851482..f8325b9f4 100644 --- a/go/internal/e2e/mcp_and_agents_test.go +++ b/go/internal/e2e/mcp_and_agents_test.go @@ -126,7 +126,8 @@ func TestMCPServers(t *testing.T) { } session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ - MCPServers: mcpServers, + MCPServers: mcpServers, + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, }) if err != nil { t.Fatalf("Failed to create session: %v", err) diff --git a/go/internal/e2e/permissions_test.go b/go/internal/e2e/permissions_test.go index a891548c7..1584f0244 100644 --- a/go/internal/e2e/permissions_test.go +++ b/go/internal/e2e/permissions_test.go @@ -157,6 +157,87 @@ func TestPermissions(t *testing.T) { } }) + t.Run("should deny tool operations by default when no handler is provided", func(t *testing.T) { + ctx.ConfigureForTest(t) + + session, err := client.CreateSession(t.Context(), nil) + if err != nil { + t.Fatalf("Failed to create session: %v", err) + } + + var mu sync.Mutex + permissionDenied := false + + session.On(func(event copilot.SessionEvent) { + if event.Type == copilot.ToolExecutionComplete && + event.Data.Success != nil && !*event.Data.Success && + event.Data.Error != nil && event.Data.Error.ErrorClass != nil && + strings.Contains(event.Data.Error.ErrorClass.Message, "Permission denied") { + mu.Lock() + permissionDenied = true + mu.Unlock() + } + }) + + if _, err = session.SendAndWait(t.Context(), copilot.MessageOptions{ + Prompt: "Run 'node --version'", + }); err != nil { + t.Fatalf("Failed to send message: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if !permissionDenied { + t.Error("Expected a tool.execution_complete event with Permission denied result") + } + }) + + t.Run("should deny tool operations by default when no handler is provided after resume", func(t *testing.T) { + ctx.ConfigureForTest(t) + + session1, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, + }) + if err != nil { + t.Fatalf("Failed to create session: %v", err) + } + sessionID := session1.SessionID + if _, err = session1.SendAndWait(t.Context(), copilot.MessageOptions{Prompt: "What is 1+1?"}); err != nil { + t.Fatalf("Failed to send message: %v", err) + } + + session2, err := client.ResumeSession(t.Context(), sessionID) + if err != nil { + t.Fatalf("Failed to resume session: %v", err) + } + + var mu sync.Mutex + permissionDenied := false + + session2.On(func(event copilot.SessionEvent) { + if event.Type == copilot.ToolExecutionComplete && + event.Data.Success != nil && !*event.Data.Success && + event.Data.Error != nil && event.Data.Error.ErrorClass != nil && + strings.Contains(event.Data.Error.ErrorClass.Message, "Permission denied") { + mu.Lock() + permissionDenied = true + mu.Unlock() + } + }) + + if _, err = session2.SendAndWait(t.Context(), copilot.MessageOptions{ + Prompt: "Run 'node --version'", + }); err != nil { + t.Fatalf("Failed to send message: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if !permissionDenied { + t.Error("Expected a tool.execution_complete event with Permission denied result") + } + }) + t.Run("without permission handler", func(t *testing.T) { ctx.ConfigureForTest(t) diff --git a/go/internal/e2e/session_test.go b/go/internal/e2e/session_test.go index 6a98da60a..87341838a 100644 --- a/go/internal/e2e/session_test.go +++ b/go/internal/e2e/session_test.go @@ -463,7 +463,9 @@ func TestSession(t *testing.T) { t.Run("should abort a session", func(t *testing.T) { ctx.ConfigureForTest(t) - session, err := client.CreateSession(t.Context(), nil) + session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, + }) if err != nil { t.Fatalf("Failed to create session: %v", err) } diff --git a/go/internal/e2e/tools_test.go b/go/internal/e2e/tools_test.go index 5af9079ce..d54bdcb14 100644 --- a/go/internal/e2e/tools_test.go +++ b/go/internal/e2e/tools_test.go @@ -25,7 +25,9 @@ func TestTools(t *testing.T) { t.Fatalf("Failed to write test file: %v", err) } - session, err := client.CreateSession(t.Context(), nil) + session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, + }) if err != nil { t.Fatalf("Failed to create session: %v", err) } diff --git a/go/permissions.go b/go/permissions.go new file mode 100644 index 000000000..91ff776cf --- /dev/null +++ b/go/permissions.go @@ -0,0 +1,11 @@ +package copilot + +// PermissionHandler provides pre-built OnPermissionRequest implementations. +var PermissionHandler = struct { + // ApproveAll approves all permission requests. + ApproveAll PermissionHandlerFunc +}{ + ApproveAll: func(_ PermissionRequest, _ PermissionInvocation) (PermissionRequestResult, error) { + return PermissionRequestResult{Kind: "approved"}, nil + }, +} diff --git a/go/samples/chat.go b/go/samples/chat.go index 0e6e0d9a2..4fc11ffda 100644 --- a/go/samples/chat.go +++ b/go/samples/chat.go @@ -23,7 +23,10 @@ func main() { } defer client.Stop() - session, err := client.CreateSession(ctx, nil) + session, err := client.CreateSession(ctx, &copilot.SessionConfig{ + CLIPath: cliPath, + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, + }) if err != nil { panic(err) } diff --git a/go/session.go b/go/session.go index ce1a3eff0..12d1b1afa 100644 --- a/go/session.go +++ b/go/session.go @@ -58,7 +58,7 @@ type Session struct { handlerMutex sync.RWMutex toolHandlers map[string]ToolHandler toolHandlersM sync.RWMutex - permissionHandler PermissionHandler + permissionHandler PermissionHandlerFunc permissionMux sync.RWMutex userInputHandler UserInputHandler userInputMux sync.RWMutex @@ -290,14 +290,14 @@ func (s *Session) getToolHandler(name string) (ToolHandler, bool) { // operations), this handler is called to approve or deny the request. // // This method is internal and typically called when creating a session. -func (s *Session) registerPermissionHandler(handler PermissionHandler) { +func (s *Session) registerPermissionHandler(handler PermissionHandlerFunc) { s.permissionMux.Lock() defer s.permissionMux.Unlock() s.permissionHandler = handler } // getPermissionHandler returns the currently registered permission handler, or nil. -func (s *Session) getPermissionHandler() PermissionHandler { +func (s *Session) getPermissionHandler() PermissionHandlerFunc { s.permissionMux.RLock() defer s.permissionMux.RUnlock() return s.permissionHandler diff --git a/go/types.go b/go/types.go index b9b649b68..b0f6b7e22 100644 --- a/go/types.go +++ b/go/types.go @@ -112,9 +112,9 @@ type PermissionRequestResult struct { Rules []any `json:"rules,omitempty"` } -// PermissionHandler executes a permission request +// PermissionHandlerFunc executes a permission request // The handler should return a PermissionRequestResult. Returning an error denies the permission. -type PermissionHandler func(request PermissionRequest, invocation PermissionInvocation) (PermissionRequestResult, error) +type PermissionHandlerFunc func(request PermissionRequest, invocation PermissionInvocation) (PermissionRequestResult, error) // PermissionInvocation provides context about a permission request type PermissionInvocation struct { @@ -349,8 +349,10 @@ type SessionConfig struct { // ExcludedTools is a list of tool names to disable. All other tools remain available. // Ignored if AvailableTools is specified. ExcludedTools []string - // OnPermissionRequest is a handler for permission requests from the server - OnPermissionRequest PermissionHandler + // OnPermissionRequest is a handler for permission requests from the server. + // If nil, all permission requests are denied by default. + // Provide a handler to approve operations (file writes, shell commands, URL fetches, etc.). + OnPermissionRequest PermissionHandlerFunc // OnUserInputRequest is a handler for user input requests from the agent (enables ask_user tool) OnUserInputRequest UserInputHandler // Hooks configures hook handlers for session lifecycle events @@ -426,8 +428,10 @@ type ResumeSessionConfig struct { // ReasoningEffort level for models that support it. // Valid values: "low", "medium", "high", "xhigh" ReasoningEffort string - // OnPermissionRequest is a handler for permission requests from the server - OnPermissionRequest PermissionHandler + // OnPermissionRequest is a handler for permission requests from the server. + // If nil, all permission requests are denied by default. + // Provide a handler to approve operations (file writes, shell commands, URL fetches, etc.). + OnPermissionRequest PermissionHandlerFunc // OnUserInputRequest is a handler for user input requests from the agent (enables ask_user tool) OnUserInputRequest UserInputHandler // Hooks configures hook handlers for session lifecycle events diff --git a/nodejs/samples/chat.ts b/nodejs/samples/chat.ts index f0381bb8b..e2e05fdc3 100644 --- a/nodejs/samples/chat.ts +++ b/nodejs/samples/chat.ts @@ -1,9 +1,11 @@ import * as readline from "node:readline"; -import { CopilotClient, type SessionEvent } from "@github/copilot-sdk"; +import { CopilotClient, approveAll, type SessionEvent } from "@github/copilot-sdk"; async function main() { const client = new CopilotClient(); - const session = await client.createSession(); + const session = await client.createSession({ + onPermissionRequest: approveAll, + }); session.on((event: SessionEvent) => { let output: string | null = null; diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 5d7413140..876ca719e 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -523,7 +523,7 @@ export class CopilotClient { availableTools: config.availableTools, excludedTools: config.excludedTools, provider: config.provider, - requestPermission: !!config.onPermissionRequest, + requestPermission: true, requestUserInput: !!config.onUserInputRequest, hooks: !!(config.hooks && Object.values(config.hooks).some(Boolean)), workingDirectory: config.workingDirectory, @@ -605,7 +605,7 @@ export class CopilotClient { parameters: toJsonSchema(tool.parameters), })), provider: config.provider, - requestPermission: !!config.onPermissionRequest, + requestPermission: true, requestUserInput: !!config.onUserInputRequest, hooks: !!(config.hooks && Object.values(config.hooks).some(Boolean)), workingDirectory: config.workingDirectory, diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index 5e73a1bb2..f2655f2fc 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -10,7 +10,7 @@ export { CopilotClient } from "./client.js"; export { CopilotSession, type AssistantMessageEvent } from "./session.js"; -export { defineTool } from "./types.js"; +export { defineTool, approveAll } from "./types.js"; export type { ConnectionState, CopilotClientOptions, diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index c28068043..516d65558 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -230,6 +230,8 @@ export type PermissionHandler = ( invocation: { sessionId: string } ) => Promise | PermissionRequestResult; +export const approveAll: PermissionHandler = () => ({ kind: "approved" }); + // ============================================================================ // User Input Request Types // ============================================================================ diff --git a/nodejs/test/e2e/hooks.test.ts b/nodejs/test/e2e/hooks.test.ts index 0a91f466f..18cc9fea0 100644 --- a/nodejs/test/e2e/hooks.test.ts +++ b/nodejs/test/e2e/hooks.test.ts @@ -11,6 +11,7 @@ import type { PostToolUseHookInput, PostToolUseHookOutput, } from "../../src/index.js"; +import { approveAll } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext.js"; describe("Session hooks", async () => { @@ -20,6 +21,7 @@ describe("Session hooks", async () => { const preToolUseInputs: PreToolUseHookInput[] = []; const session = await client.createSession({ + onPermissionRequest: approveAll, hooks: { onPreToolUse: async (input, invocation) => { preToolUseInputs.push(input); @@ -50,6 +52,7 @@ describe("Session hooks", async () => { const postToolUseInputs: PostToolUseHookInput[] = []; const session = await client.createSession({ + onPermissionRequest: approveAll, hooks: { onPostToolUse: async (input, invocation) => { postToolUseInputs.push(input); @@ -81,6 +84,7 @@ describe("Session hooks", async () => { const postToolUseInputs: PostToolUseHookInput[] = []; const session = await client.createSession({ + onPermissionRequest: approveAll, hooks: { onPreToolUse: async (input) => { preToolUseInputs.push(input); diff --git a/nodejs/test/e2e/mcp_and_agents.test.ts b/nodejs/test/e2e/mcp_and_agents.test.ts index 2cd3f37d5..7b7aabf06 100644 --- a/nodejs/test/e2e/mcp_and_agents.test.ts +++ b/nodejs/test/e2e/mcp_and_agents.test.ts @@ -6,6 +6,7 @@ import { dirname, resolve } from "path"; import { fileURLToPath } from "url"; import { describe, expect, it } from "vitest"; import type { CustomAgentConfig, MCPLocalServerConfig, MCPServerConfig } from "../../src/index.js"; +import { approveAll } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext.js"; const __filename = fileURLToPath(import.meta.url); @@ -108,6 +109,7 @@ describe("MCP Servers and Custom Agents", async () => { const session = await client.createSession({ mcpServers, + onPermissionRequest: approveAll, }); expect(session.sessionId).toBeDefined(); diff --git a/nodejs/test/e2e/permissions.test.ts b/nodejs/test/e2e/permissions.test.ts index 91bad2b03..b68446ee9 100644 --- a/nodejs/test/e2e/permissions.test.ts +++ b/nodejs/test/e2e/permissions.test.ts @@ -6,6 +6,7 @@ import { readFile, writeFile } from "fs/promises"; import { join } from "path"; import { describe, expect, it } from "vitest"; import type { PermissionRequest, PermissionRequestResult } from "../../src/index.js"; +import { approveAll } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext.js"; describe("Permission callbacks", async () => { @@ -63,6 +64,51 @@ describe("Permission callbacks", async () => { await session.destroy(); }); + it("should deny tool operations by default when no handler is provided", async () => { + let permissionDenied = false; + + const session = await client.createSession(); + session.on((event) => { + if ( + event.type === "tool.execution_complete" && + !event.data.success && + event.data.error?.message.includes("Permission denied") + ) { + permissionDenied = true; + } + }); + + await session.sendAndWait({ prompt: "Run 'node --version'" }); + + expect(permissionDenied).toBe(true); + + await session.destroy(); + }); + + it("should deny tool operations by default when no handler is provided after resume", async () => { + const session1 = await client.createSession({ onPermissionRequest: approveAll }); + const sessionId = session1.sessionId; + await session1.sendAndWait({ prompt: "What is 1+1?" }); + + const session2 = await client.resumeSession(sessionId); + let permissionDenied = false; + session2.on((event) => { + if ( + event.type === "tool.execution_complete" && + !event.data.success && + event.data.error?.message.includes("Permission denied") + ) { + permissionDenied = true; + } + }); + + await session2.sendAndWait({ prompt: "Run 'node --version'" }); + + expect(permissionDenied).toBe(true); + + await session2.destroy(); + }); + it("should work without permission handler (default behavior)", async () => { // Create session without onPermissionRequest handler const session = await client.createSession(); diff --git a/nodejs/test/e2e/session.test.ts b/nodejs/test/e2e/session.test.ts index de1e9e6d9..09c293a53 100644 --- a/nodejs/test/e2e/session.test.ts +++ b/nodejs/test/e2e/session.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, onTestFinished } from "vitest"; import { ParsedHttpExchange } from "../../../test/harness/replayingCapiProxy.js"; -import { CopilotClient } from "../../src/index.js"; +import { CopilotClient, approveAll } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext.js"; import { getFinalAssistantMessage, getNextEventOfType } from "./harness/sdkTestHelper.js"; @@ -366,7 +366,9 @@ describe("Send Blocking Behavior", async () => { const { copilotClient: client } = await createSdkTestContext(); it("send returns immediately while events stream in background", async () => { - const session = await client.createSession(); + const session = await client.createSession({ + onPermissionRequest: approveAll, + }); const events: string[] = []; session.on((event) => { diff --git a/nodejs/test/e2e/tools.test.ts b/nodejs/test/e2e/tools.test.ts index 85960b839..3db24dff7 100644 --- a/nodejs/test/e2e/tools.test.ts +++ b/nodejs/test/e2e/tools.test.ts @@ -6,7 +6,7 @@ import { writeFile } from "fs/promises"; import { join } from "path"; import { assert, describe, expect, it } from "vitest"; import { z } from "zod"; -import { defineTool } from "../../src/index.js"; +import { defineTool, approveAll } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext"; describe("Custom tools", async () => { @@ -15,7 +15,9 @@ describe("Custom tools", async () => { it("invokes built-in tools", async () => { await writeFile(join(workDir, "README.md"), "# ELIZA, the only chatbot you'll ever need"); - const session = await client.createSession(); + const session = await client.createSession({ + onPermissionRequest: approveAll, + }); const assistantMessage = await session.sendAndWait({ prompt: "What's the first line of README.md in this directory?", }); diff --git a/python/copilot/client.py b/python/copilot/client.py index 99154f43e..c4e69c13d 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -485,10 +485,9 @@ async def create_session(self, config: Optional[SessionConfig] = None) -> Copilo if excluded_tools: payload["excludedTools"] = excluded_tools - # Enable permission request callback if handler provided + # Always enable permission request callback (deny by default if no handler provided) on_permission_request = cfg.get("on_permission_request") - if on_permission_request: - payload["requestPermission"] = True + payload["requestPermission"] = True # Enable user input request callback if handler provided on_user_input_request = cfg.get("on_user_input_request") @@ -662,10 +661,9 @@ async def resume_session( if streaming is not None: payload["streaming"] = streaming - # Enable permission request callback if handler provided + # Always enable permission request callback (deny by default if no handler provided) on_permission_request = cfg.get("on_permission_request") - if on_permission_request: - payload["requestPermission"] = True + payload["requestPermission"] = True # Enable user input request callback if handler provided on_user_input_request = cfg.get("on_user_input_request") diff --git a/python/copilot/session.py b/python/copilot/session.py index d7bd1a3f4..7332f6c5f 100644 --- a/python/copilot/session.py +++ b/python/copilot/session.py @@ -14,13 +14,13 @@ from .generated.session_events import SessionEvent, SessionEventType, session_event_from_dict from .types import ( MessageOptions, - PermissionHandler, SessionHooks, Tool, ToolHandler, UserInputHandler, UserInputRequest, UserInputResponse, + _PermissionHandlerFn, ) from .types import ( SessionEvent as SessionEventTypeAlias, @@ -74,7 +74,7 @@ def __init__(self, session_id: str, client: Any, workspace_path: Optional[str] = self._event_handlers_lock = threading.Lock() self._tool_handlers: dict[str, ToolHandler] = {} self._tool_handlers_lock = threading.Lock() - self._permission_handler: Optional[PermissionHandler] = None + self._permission_handler: Optional[_PermissionHandlerFn] = None self._permission_handler_lock = threading.Lock() self._user_input_handler: Optional[UserInputHandler] = None self._user_input_handler_lock = threading.Lock() @@ -291,7 +291,7 @@ def _get_tool_handler(self, name: str) -> Optional[ToolHandler]: with self._tool_handlers_lock: return self._tool_handlers.get(name) - def _register_permission_handler(self, handler: Optional[PermissionHandler]) -> None: + def _register_permission_handler(self, handler: Optional[_PermissionHandlerFn]) -> None: """ Register a handler for permission requests. diff --git a/python/copilot/types.py b/python/copilot/types.py index 0f127d445..5fe7ee380 100644 --- a/python/copilot/types.py +++ b/python/copilot/types.py @@ -186,12 +186,18 @@ class PermissionRequestResult(TypedDict, total=False): rules: list[Any] -PermissionHandler = Callable[ +_PermissionHandlerFn = Callable[ [PermissionRequest, dict[str, str]], Union[PermissionRequestResult, Awaitable[PermissionRequestResult]], ] +class PermissionHandler: + @staticmethod + def approve_all(request: Any, invocation: Any) -> dict: + return {"kind": "approved"} + + # ============================================================================ # User Input Request Types # ============================================================================ @@ -473,7 +479,7 @@ class SessionConfig(TypedDict, total=False): # List of tool names to disable (ignored if available_tools is set) excluded_tools: list[str] # Handler for permission requests from the server - on_permission_request: PermissionHandler + on_permission_request: _PermissionHandlerFn # Handler for user input requests from the agent (enables ask_user tool) on_user_input_request: UserInputHandler # Hook handlers for intercepting session lifecycle events @@ -540,8 +546,8 @@ class ResumeSessionConfig(TypedDict, total=False): provider: ProviderConfig # Reasoning effort level for models that support it. reasoning_effort: ReasoningEffort - on_permission_request: PermissionHandler - # Handler for user input requests from the agent (enables ask_user tool) + on_permission_request: _PermissionHandlerFn + # Handler for user input requestsfrom the agent (enables ask_user tool) on_user_input_request: UserInputHandler # Hook handlers for intercepting session lifecycle events hooks: SessionHooks diff --git a/python/e2e/test_hooks.py b/python/e2e/test_hooks.py index b64628e0a..8278fb33c 100644 --- a/python/e2e/test_hooks.py +++ b/python/e2e/test_hooks.py @@ -4,6 +4,8 @@ import pytest +from copilot import PermissionHandler + from .testharness import E2ETestContext from .testharness.helper import write_file @@ -21,7 +23,12 @@ async def on_pre_tool_use(input_data, invocation): # Allow the tool to run return {"permissionDecision": "allow"} - session = await ctx.client.create_session({"hooks": {"on_pre_tool_use": on_pre_tool_use}}) + session = await ctx.client.create_session( + { + "hooks": {"on_pre_tool_use": on_pre_tool_use}, + "on_permission_request": PermissionHandler.approve_all, + } + ) # Create a file for the model to read write_file(ctx.work_dir, "hello.txt", "Hello from the test!") @@ -49,7 +56,12 @@ async def on_post_tool_use(input_data, invocation): assert invocation["session_id"] == session.session_id return None - session = await ctx.client.create_session({"hooks": {"on_post_tool_use": on_post_tool_use}}) + session = await ctx.client.create_session( + { + "hooks": {"on_post_tool_use": on_post_tool_use}, + "on_permission_request": PermissionHandler.approve_all, + } + ) # Create a file for the model to read write_file(ctx.work_dir, "world.txt", "World from the test!") @@ -87,7 +99,8 @@ async def on_post_tool_use(input_data, invocation): "hooks": { "on_pre_tool_use": on_pre_tool_use, "on_post_tool_use": on_post_tool_use, - } + }, + "on_permission_request": PermissionHandler.approve_all, } ) @@ -118,7 +131,12 @@ async def on_pre_tool_use(input_data, invocation): # Deny all tool calls return {"permissionDecision": "deny"} - session = await ctx.client.create_session({"hooks": {"on_pre_tool_use": on_pre_tool_use}}) + session = await ctx.client.create_session( + { + "hooks": {"on_pre_tool_use": on_pre_tool_use}, + "on_permission_request": PermissionHandler.approve_all, + } + ) # Create a file original_content = "Original content that should not be modified" diff --git a/python/e2e/test_mcp_and_agents.py b/python/e2e/test_mcp_and_agents.py index 3dd7f4aab..7ca4b8c2b 100644 --- a/python/e2e/test_mcp_and_agents.py +++ b/python/e2e/test_mcp_and_agents.py @@ -6,7 +6,7 @@ import pytest -from copilot import CustomAgentConfig, MCPServerConfig +from copilot import CustomAgentConfig, MCPServerConfig, PermissionHandler from .testharness import E2ETestContext, get_final_assistant_message @@ -87,7 +87,12 @@ async def test_should_pass_literal_env_values_to_mcp_server_subprocess( } } - session = await ctx.client.create_session({"mcp_servers": mcp_servers}) + session = await ctx.client.create_session( + { + "mcp_servers": mcp_servers, + "on_permission_request": PermissionHandler.approve_all, + } + ) assert session.session_id is not None diff --git a/python/e2e/test_permissions.py b/python/e2e/test_permissions.py index 7635219d4..80b69ebba 100644 --- a/python/e2e/test_permissions.py +++ b/python/e2e/test_permissions.py @@ -6,7 +6,7 @@ import pytest -from copilot import PermissionRequest, PermissionRequestResult +from copilot import PermissionHandler, PermissionRequest, PermissionRequestResult from .testharness import E2ETestContext from .testharness.helper import read_file, write_file @@ -68,6 +68,72 @@ def on_permission_request( await session.destroy() + async def test_should_deny_tool_operations_by_default_when_no_handler_is_provided( + self, ctx: E2ETestContext + ): + session = await ctx.client.create_session() + + denied_events = [] + done_event = asyncio.Event() + + def on_event(event): + if event.type.value == "tool.execution_complete" and event.data.success is False: + error = event.data.error + msg = ( + error + if isinstance(error, str) + else (getattr(error, "message", None) if error is not None else None) + ) + if msg and "Permission denied" in msg: + denied_events.append(event) + elif event.type.value == "session.idle": + done_event.set() + + session.on(on_event) + + await session.send({"prompt": "Run 'node --version'"}) + await asyncio.wait_for(done_event.wait(), timeout=60) + + assert len(denied_events) > 0 + + await session.destroy() + + async def test_should_deny_tool_operations_by_default_when_no_handler_is_provided_after_resume( + self, ctx: E2ETestContext + ): + session1 = await ctx.client.create_session( + {"on_permission_request": PermissionHandler.approve_all} + ) + session_id = session1.session_id + await session1.send_and_wait({"prompt": "What is 1+1?"}) + + session2 = await ctx.client.resume_session(session_id) + + denied_events = [] + done_event = asyncio.Event() + + def on_event(event): + if event.type.value == "tool.execution_complete" and event.data.success is False: + error = event.data.error + msg = ( + error + if isinstance(error, str) + else (getattr(error, "message", None) if error is not None else None) + ) + if msg and "Permission denied" in msg: + denied_events.append(event) + elif event.type.value == "session.idle": + done_event.set() + + session2.on(on_event) + + await session2.send({"prompt": "Run 'node --version'"}) + await asyncio.wait_for(done_event.wait(), timeout=60) + + assert len(denied_events) > 0 + + await session2.destroy() + async def test_should_work_without_permission_handler__default_behavior_( self, ctx: E2ETestContext ): diff --git a/python/e2e/test_tools.py b/python/e2e/test_tools.py index 2e024887c..10e61cf15 100644 --- a/python/e2e/test_tools.py +++ b/python/e2e/test_tools.py @@ -5,7 +5,7 @@ import pytest from pydantic import BaseModel, Field -from copilot import ToolInvocation, define_tool +from copilot import PermissionHandler, ToolInvocation, define_tool from .testharness import E2ETestContext, get_final_assistant_message @@ -18,7 +18,9 @@ async def test_invokes_built_in_tools(self, ctx: E2ETestContext): with open(readme_path, "w") as f: f.write("# ELIZA, the only chatbot you'll ever need") - session = await ctx.client.create_session() + session = await ctx.client.create_session( + {"on_permission_request": PermissionHandler.approve_all} + ) await session.send({"prompt": "What's the first line of README.md in this directory?"}) assistant_message = await get_final_assistant_message(session) diff --git a/python/samples/chat.py b/python/samples/chat.py index cfdd2eee0..eb781e4e2 100644 --- a/python/samples/chat.py +++ b/python/samples/chat.py @@ -1,6 +1,6 @@ import asyncio -from copilot import CopilotClient +from copilot import CopilotClient, PermissionHandler BLUE = "\033[34m" RESET = "\033[0m" @@ -9,7 +9,11 @@ async def main(): client = CopilotClient() await client.start() - session = await client.create_session() + session = await client.create_session( + { + "on_permission_request": PermissionHandler.approve_all, + } + ) def on_event(event): output = None diff --git a/test/snapshots/permissions/should_deny_tool_operations_by_default_when_no_handler_is_provided.yaml b/test/snapshots/permissions/should_deny_tool_operations_by_default_when_no_handler_is_provided.yaml new file mode 100644 index 000000000..4413bb20a --- /dev/null +++ b/test/snapshots/permissions/should_deny_tool_operations_by_default_when_no_handler_is_provided.yaml @@ -0,0 +1,49 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: Run 'node --version' + - role: assistant + tool_calls: + - id: toolcall_0 + type: function + function: + name: report_intent + arguments: '{"intent":"Checking Node.js version"}' + - role: assistant + tool_calls: + - id: toolcall_1 + type: function + function: + name: ${shell} + arguments: '{"command":"node --version","description":"Check Node.js version"}' + - messages: + - role: system + content: ${system} + - role: user + content: Run 'node --version' + - role: assistant + tool_calls: + - id: toolcall_0 + type: function + function: + name: report_intent + arguments: '{"intent":"Checking Node.js version"}' + - id: toolcall_1 + type: function + function: + name: ${shell} + arguments: '{"command":"node --version","description":"Check Node.js version"}' + - role: tool + tool_call_id: toolcall_0 + content: Intent logged + - role: tool + tool_call_id: toolcall_1 + content: Permission denied and could not request permission from user + - role: assistant + content: I received a permission denied error. It appears I don't have permission to execute the `node --version` + command in this environment. This might be due to security restrictions or the command not being available in + the current context. diff --git a/test/snapshots/permissions/should_deny_tool_operations_by_default_when_no_handler_is_provided_after_resume.yaml b/test/snapshots/permissions/should_deny_tool_operations_by_default_when_no_handler_is_provided_after_resume.yaml new file mode 100644 index 000000000..788a1a783 --- /dev/null +++ b/test/snapshots/permissions/should_deny_tool_operations_by_default_when_no_handler_is_provided_after_resume.yaml @@ -0,0 +1,55 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: What is 1+1? + - role: assistant + content: 1+1 = 2 + - role: user + content: Run 'node --version' + - role: assistant + tool_calls: + - id: toolcall_0 + type: function + function: + name: report_intent + arguments: '{"intent":"Checking Node.js version"}' + - role: assistant + tool_calls: + - id: toolcall_1 + type: function + function: + name: ${shell} + arguments: '{"command":"node --version","description":"Check Node.js version"}' + - messages: + - role: system + content: ${system} + - role: user + content: What is 1+1? + - role: assistant + content: 1+1 = 2 + - role: user + content: Run 'node --version' + - role: assistant + tool_calls: + - id: toolcall_0 + type: function + function: + name: report_intent + arguments: '{"intent":"Checking Node.js version"}' + - id: toolcall_1 + type: function + function: + name: ${shell} + arguments: '{"command":"node --version","description":"Check Node.js version"}' + - role: tool + tool_call_id: toolcall_0 + content: Intent logged + - role: tool + tool_call_id: toolcall_1 + content: Permission denied and could not request permission from user + - role: assistant + content: Permission was denied to run the command. I don't have access to execute shell commands in this environment.