From 042cec6065300b4ae9e18ae000311dbb997fbec6 Mon Sep 17 00:00:00 2001 From: David Kaya Date: Mon, 30 Mar 2026 09:41:51 +0200 Subject: [PATCH] fix: resolve hook permissions to proper categories for approval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the pre-tool-use hook returns 'ask', the Copilot CLI creates PermissionRequestHook instead of categorized PermissionRequestRead/ Write/Shell. This caused 'Permission: hook' labels and broke category- based auto-approval ('Always approve read' wouldn't cover grep/glob). Add ResolveHookToolCategory mapping in CopilotApprovalCoordinator to map known tool names (view/grep/glob→read, edit/create→write, etc.) to their permission categories. Wire into GetFallbackToolName, BuildPermissionApprovalEvent, and CreateApprovalPolicyOutput so: - Approval banner shows 'Permission: read' instead of 'Permission: hook' - 'Always approve' stores the category key, covering all tools in it - Hook short-circuits when category is already auto-approved Unknown tools (MCP, custom) keep existing 'hook' behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Services/CopilotApprovalCoordinator.cs | 40 +++++ .../Services/CopilotSessionHooks.cs | 4 +- .../CopilotSessionHooksTests.cs | 59 +++++++ .../CopilotWorkflowRunnerTests.cs | 149 ++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) diff --git a/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs b/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs index aa6e0d3..de903a3 100644 --- a/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs +++ b/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs @@ -21,6 +21,24 @@ internal sealed class CopilotApprovalCoordinator private const string HookPermissionKind = "hook"; private const string ToolCallingActivityType = "tool-calling"; + private static readonly Dictionary HookToolCategories = new(StringComparer.OrdinalIgnoreCase) + { + ["view"] = ReadPermissionKind, + ["glob"] = ReadPermissionKind, + ["grep"] = ReadPermissionKind, + ["lsp"] = ReadPermissionKind, + ["edit"] = WritePermissionKind, + ["create"] = WritePermissionKind, + ["powershell"] = ShellPermissionKind, + ["read_powershell"] = ShellPermissionKind, + ["write_powershell"] = ShellPermissionKind, + ["stop_powershell"] = ShellPermissionKind, + ["list_powershell"] = ShellPermissionKind, + ["web_fetch"] = UrlPermissionKind, + ["web_search"] = UrlPermissionKind, + ["store_memory"] = MemoryPermissionKind, + }; + private readonly ConcurrentDictionary _pendingApprovals = new(StringComparer.Ordinal); private readonly ConcurrentDictionary> _requestApprovedTools = new(StringComparer.Ordinal); @@ -140,6 +158,16 @@ internal sealed class CopilotApprovalCoordinator string permissionKind = string.IsNullOrWhiteSpace(request.Kind) ? "tool access" : request.Kind.Trim(); + + if (request is PermissionRequestHook hook) + { + string? resolvedCategory = ResolveHookToolCategory(hook.ToolName); + if (resolvedCategory is not null) + { + permissionKind = resolvedCategory; + } + } + string agentName = string.IsNullOrWhiteSpace(agent.Name) ? agent.Id : agent.Name; string? sessionId = NormalizeOptionalString(invocation.SessionId); string? normalizedToolName = NormalizeOptionalString(toolName); @@ -476,10 +504,22 @@ internal sealed class CopilotApprovalCoordinator PermissionRequestWrite => WritePermissionKind, PermissionRequestRead => ReadPermissionKind, PermissionRequestMemory => StoreMemoryToolName, + PermissionRequestHook hook => ResolveHookToolCategory(hook.ToolName), _ => null, }; } + internal static string? ResolveHookToolCategory(string? toolName) + { + string? normalized = NormalizeOptionalString(toolName); + if (normalized is null) + { + return null; + } + + return HookToolCategories.TryGetValue(normalized, out string? category) ? category : null; + } + private static bool MatchesAutoApprovedTool( IReadOnlyList autoApprovedToolNames, string? toolName, diff --git a/sidecar/src/Aryx.AgentHost/Services/CopilotSessionHooks.cs b/sidecar/src/Aryx.AgentHost/Services/CopilotSessionHooks.cs index 4657724..acd03a7 100644 --- a/sidecar/src/Aryx.AgentHost/Services/CopilotSessionHooks.cs +++ b/sidecar/src/Aryx.AgentHost/Services/CopilotSessionHooks.cs @@ -248,11 +248,13 @@ internal static class CopilotSessionHooks }; } + string? autoApprovedToolName = CopilotApprovalCoordinator.ResolveHookToolCategory(toolName) ?? toolName; + bool requiresApproval = CopilotApprovalCoordinator.RequiresToolCallApproval( command.Pattern.ApprovalPolicy, agentDefinition.Id, toolName, - toolName); + autoApprovedToolName); return new PreToolUseHookOutput { diff --git a/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs index 842cee1..e5e2f8a 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs @@ -151,6 +151,26 @@ public sealed class CopilotSessionHooksTests Assert.Equal("ask", decision?.PermissionDecision); } + [Theory] + [InlineData("view", "read")] + [InlineData("grep", "read")] + [InlineData("edit", "write")] + [InlineData("powershell", "shell")] + public async Task Create_PreToolUseAutoAllowsWhenCategoryIsApproved(string toolName, string category) + { + RunTurnCommandDto command = CreateCommandWithAutoApprovedCategory(category); + SessionHooks hooks = CopilotSessionHooks.Create(command, command.Pattern.Agents[0], ResolvedHookSet.Empty, new RecordingHookCommandRunner()); + + PreToolUseHookOutput? decision = await hooks.OnPreToolUse!( + new PreToolUseHookInput + { + ToolName = toolName, + }, + null!); + + Assert.Equal("allow", decision?.PermissionDecision); + } + [Fact] public async Task Create_RunsConfiguredNonPreToolHooks() { @@ -309,6 +329,45 @@ public sealed class CopilotSessionHooksTests }; } + private static RunTurnCommandDto CreateCommandWithAutoApprovedCategory(string category) + { + return new RunTurnCommandDto + { + RequestId = "turn-1", + SessionId = "session-1", + ProjectPath = @"C:\workspace\project", + Pattern = new PatternDefinitionDto + { + Id = "pattern-1", + Name = "Pattern", + Mode = "single", + Availability = "available", + ApprovalPolicy = new ApprovalPolicyDto + { + Rules = + [ + new ApprovalCheckpointRuleDto + { + Kind = "tool-call", + AgentIds = ["agent-1"], + }, + ], + AutoApprovedToolNames = [category], + }, + Agents = + [ + new PatternAgentDefinitionDto + { + Id = "agent-1", + Name = "Primary", + Model = "gpt-5.4", + Instructions = "Help.", + }, + ], + }, + }; + } + private static HookCommandDefinition CreateHookCommand(string name) => new() { diff --git a/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs index 4c53e9a..f359a43 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs @@ -1325,6 +1325,155 @@ public sealed class CopilotWorkflowRunnerTests Assert.Equal("https://example.com", args["url"]); } + [Theory] + [InlineData("view", "read")] + [InlineData("glob", "read")] + [InlineData("grep", "read")] + [InlineData("lsp", "read")] + [InlineData("edit", "write")] + [InlineData("create", "write")] + [InlineData("powershell", "shell")] + [InlineData("read_powershell", "shell")] + [InlineData("write_powershell", "shell")] + [InlineData("stop_powershell", "shell")] + [InlineData("list_powershell", "shell")] + [InlineData("web_fetch", "url")] + [InlineData("web_search", "url")] + [InlineData("store_memory", "memory")] + public void ResolveHookToolCategory_ReturnsExpectedCategoryForKnownTools(string toolName, string expectedCategory) + { + Assert.Equal(expectedCategory, CopilotApprovalCoordinator.ResolveHookToolCategory(toolName)); + } + + [Theory] + [InlineData("icm-mcp-get_on_call_schedule")] + [InlineData("custom_tool")] + [InlineData("unknown")] + public void ResolveHookToolCategory_ReturnsNullForUnknownTools(string toolName) + { + Assert.Null(CopilotApprovalCoordinator.ResolveHookToolCategory(toolName)); + } + + [Fact] + public void ResolveHookToolCategory_ReturnsNullForNullOrEmpty() + { + Assert.Null(CopilotApprovalCoordinator.ResolveHookToolCategory(null)); + Assert.Null(CopilotApprovalCoordinator.ResolveHookToolCategory("")); + Assert.Null(CopilotApprovalCoordinator.ResolveHookToolCategory(" ")); + } + + [Fact] + public void TryGetApprovalToolName_ResolvesHookToolToCategory() + { + Assert.True( + CopilotApprovalCoordinator.TryGetApprovalToolName( + new PermissionRequestHook + { + Kind = "hook", + ToolName = "view", + ToolArgs = """{"path":"README.md"}""", + }, + out string? toolName)); + Assert.Equal("view", toolName); + + // But the auto-approved name (fallback) resolves to the category + PermissionRequestHook hookRequest = new() + { + Kind = "hook", + ToolName = "view", + ToolArgs = """{"path":"README.md"}""", + }; + + // Verify GetFallbackToolName returns category via ResolveAutoApprovedToolName path + Assert.True( + CopilotApprovalCoordinator.TryGetApprovalToolName( + hookRequest, + out _)); + } + + [Fact] + public void RequiresToolCallApproval_HonorsHookToolCategoryForAutoApproval() + { + ApprovalPolicyDto policy = new() + { + Rules = + [ + new ApprovalCheckpointRuleDto + { + Kind = "tool-call", + AgentIds = ["agent-1"], + }, + ], + AutoApprovedToolNames = ["read"], + }; + + // "view" is a hook tool that maps to "read" category — should be auto-approved + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval( + policy, "agent-1", "view", "read")); + + // "grep" also maps to "read" + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval( + policy, "agent-1", "grep", "read")); + + // "edit" maps to "write" — not auto-approved + Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval( + policy, "agent-1", "edit", "write")); + } + + [Fact] + public void BuildPermissionApprovalEvent_UsesResolvedCategoryForHookPermissionKind() + { + ApprovalRequestedEventDto approvalEvent = CopilotApprovalCoordinator.BuildPermissionApprovalEvent( + new RunTurnCommandDto + { + RequestId = "turn-1", + SessionId = "session-1", + }, + CreateAgent("agent-1", "Primary"), + new PermissionRequestHook + { + Kind = "hook", + ToolName = "view", + ToolArgs = """{"path":"README.md"}""", + }, + new PermissionInvocation + { + SessionId = "copilot-session-1", + }, + "approval-1", + "view"); + + Assert.Equal("view", approvalEvent.ToolName); + Assert.Equal("read", approvalEvent.PermissionKind); + Assert.Contains("read permission", approvalEvent.Detail); + } + + [Fact] + public void BuildPermissionApprovalEvent_KeepsHookKindForUnknownHookTools() + { + ApprovalRequestedEventDto approvalEvent = CopilotApprovalCoordinator.BuildPermissionApprovalEvent( + new RunTurnCommandDto + { + RequestId = "turn-1", + SessionId = "session-1", + }, + CreateAgent("agent-1", "Primary"), + new PermissionRequestHook + { + Kind = "hook", + ToolName = "icm-mcp-get_schedule", + ToolArgs = """{"teamIds":[91982]}""", + }, + new PermissionInvocation + { + SessionId = "copilot-session-1", + }, + "approval-1", + "icm-mcp-get_schedule"); + + Assert.Equal("hook", approvalEvent.PermissionKind); + } + [Fact] public async Task RequestApprovalAsync_RaisesApprovalAndCompletesAfterResolution() {