mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-26 21:03:58 +02:00
fix: resolve hook permissions to proper categories for approval
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>
This commit is contained in:
@@ -21,6 +21,24 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
private const string HookPermissionKind = "hook";
|
private const string HookPermissionKind = "hook";
|
||||||
private const string ToolCallingActivityType = "tool-calling";
|
private const string ToolCallingActivityType = "tool-calling";
|
||||||
|
|
||||||
|
private static readonly Dictionary<string, string> 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<string, PendingApprovalRequest> _pendingApprovals = new(StringComparer.Ordinal);
|
private readonly ConcurrentDictionary<string, PendingApprovalRequest> _pendingApprovals = new(StringComparer.Ordinal);
|
||||||
private readonly ConcurrentDictionary<string, ConcurrentDictionary<string, byte>> _requestApprovedTools = new(StringComparer.Ordinal);
|
private readonly ConcurrentDictionary<string, ConcurrentDictionary<string, byte>> _requestApprovedTools = new(StringComparer.Ordinal);
|
||||||
|
|
||||||
@@ -140,6 +158,16 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
string permissionKind = string.IsNullOrWhiteSpace(request.Kind)
|
string permissionKind = string.IsNullOrWhiteSpace(request.Kind)
|
||||||
? "tool access"
|
? "tool access"
|
||||||
: request.Kind.Trim();
|
: 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 agentName = string.IsNullOrWhiteSpace(agent.Name) ? agent.Id : agent.Name;
|
||||||
string? sessionId = NormalizeOptionalString(invocation.SessionId);
|
string? sessionId = NormalizeOptionalString(invocation.SessionId);
|
||||||
string? normalizedToolName = NormalizeOptionalString(toolName);
|
string? normalizedToolName = NormalizeOptionalString(toolName);
|
||||||
@@ -476,10 +504,22 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
PermissionRequestWrite => WritePermissionKind,
|
PermissionRequestWrite => WritePermissionKind,
|
||||||
PermissionRequestRead => ReadPermissionKind,
|
PermissionRequestRead => ReadPermissionKind,
|
||||||
PermissionRequestMemory => StoreMemoryToolName,
|
PermissionRequestMemory => StoreMemoryToolName,
|
||||||
|
PermissionRequestHook hook => ResolveHookToolCategory(hook.ToolName),
|
||||||
_ => null,
|
_ => 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(
|
private static bool MatchesAutoApprovedTool(
|
||||||
IReadOnlyList<string> autoApprovedToolNames,
|
IReadOnlyList<string> autoApprovedToolNames,
|
||||||
string? toolName,
|
string? toolName,
|
||||||
|
|||||||
@@ -248,11 +248,13 @@ internal static class CopilotSessionHooks
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
string? autoApprovedToolName = CopilotApprovalCoordinator.ResolveHookToolCategory(toolName) ?? toolName;
|
||||||
|
|
||||||
bool requiresApproval = CopilotApprovalCoordinator.RequiresToolCallApproval(
|
bool requiresApproval = CopilotApprovalCoordinator.RequiresToolCallApproval(
|
||||||
command.Pattern.ApprovalPolicy,
|
command.Pattern.ApprovalPolicy,
|
||||||
agentDefinition.Id,
|
agentDefinition.Id,
|
||||||
toolName,
|
toolName,
|
||||||
toolName);
|
autoApprovedToolName);
|
||||||
|
|
||||||
return new PreToolUseHookOutput
|
return new PreToolUseHookOutput
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -151,6 +151,26 @@ public sealed class CopilotSessionHooksTests
|
|||||||
Assert.Equal("ask", decision?.PermissionDecision);
|
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]
|
[Fact]
|
||||||
public async Task Create_RunsConfiguredNonPreToolHooks()
|
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)
|
private static HookCommandDefinition CreateHookCommand(string name)
|
||||||
=> new()
|
=> new()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -1325,6 +1325,155 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
Assert.Equal("https://example.com", args["url"]);
|
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]
|
[Fact]
|
||||||
public async Task RequestApprovalAsync_RaisesApprovalAndCompletesAfterResolution()
|
public async Task RequestApprovalAsync_RaisesApprovalAndCompletesAfterResolution()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user