mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-28 05:43:57 +02:00
fix: auto-approve URL permission requests
- map GitHub Copilot SDK PermissionRequestUrl to web_fetch so runtime tool auto-approval matches URL fetch permissions - include hook tool names in approval tool-name extraction - include requested URLs in approval detail when manual review is still required - add sidecar tests covering URL and hook permission handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -403,16 +403,29 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner
|
|||||||
string? normalizedToolName = string.IsNullOrWhiteSpace(toolName)
|
string? normalizedToolName = string.IsNullOrWhiteSpace(toolName)
|
||||||
? null
|
? null
|
||||||
: toolName.Trim();
|
: toolName.Trim();
|
||||||
|
string? requestedUrl = request is PermissionRequestUrl urlRequest && !string.IsNullOrWhiteSpace(urlRequest.Url)
|
||||||
|
? urlRequest.Url.Trim()
|
||||||
|
: null;
|
||||||
string title = normalizedToolName is null
|
string title = normalizedToolName is null
|
||||||
? $"Approve {permissionKind}"
|
? $"Approve {permissionKind}"
|
||||||
: $"Approve {normalizedToolName}";
|
: $"Approve {normalizedToolName}";
|
||||||
string detail = normalizedToolName is null
|
string detail = normalizedToolName is null
|
||||||
? sessionId is null
|
? $"{agentName} requested {permissionKind} permission"
|
||||||
? $"{agentName} requested {permissionKind} permission."
|
: $"{agentName} requested {permissionKind} permission for tool \"{normalizedToolName}\"";
|
||||||
: $"{agentName} requested {permissionKind} permission for Copilot session {sessionId}."
|
|
||||||
: sessionId is null
|
if (requestedUrl is not null)
|
||||||
? $"{agentName} requested {permissionKind} permission for tool \"{normalizedToolName}\"."
|
{
|
||||||
: $"{agentName} requested {permissionKind} permission for tool \"{normalizedToolName}\" in Copilot session {sessionId}.";
|
detail = $"{detail} to access \"{requestedUrl}\"";
|
||||||
|
}
|
||||||
|
|
||||||
|
if (sessionId is not null)
|
||||||
|
{
|
||||||
|
detail = normalizedToolName is null
|
||||||
|
? $"{detail} for Copilot session {sessionId}"
|
||||||
|
: $"{detail} in Copilot session {sessionId}";
|
||||||
|
}
|
||||||
|
|
||||||
|
detail = $"{detail}.";
|
||||||
|
|
||||||
return new ApprovalRequestedEventDto
|
return new ApprovalRequestedEventDto
|
||||||
{
|
{
|
||||||
@@ -482,6 +495,8 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner
|
|||||||
{
|
{
|
||||||
PermissionRequestMcp mcp when !string.IsNullOrWhiteSpace(mcp.ToolName) => mcp.ToolName.Trim(),
|
PermissionRequestMcp mcp when !string.IsNullOrWhiteSpace(mcp.ToolName) => mcp.ToolName.Trim(),
|
||||||
PermissionRequestCustomTool customTool when !string.IsNullOrWhiteSpace(customTool.ToolName) => customTool.ToolName.Trim(),
|
PermissionRequestCustomTool customTool when !string.IsNullOrWhiteSpace(customTool.ToolName) => customTool.ToolName.Trim(),
|
||||||
|
PermissionRequestHook hook when !string.IsNullOrWhiteSpace(hook.ToolName) => hook.ToolName.Trim(),
|
||||||
|
PermissionRequestUrl => "web_fetch",
|
||||||
_ => null,
|
_ => null,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -247,17 +247,18 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
AgentIds = ["agent-1"],
|
AgentIds = ["agent-1"],
|
||||||
},
|
},
|
||||||
],
|
],
|
||||||
AutoApprovedToolNames = ["lsp_ts_hover"],
|
AutoApprovedToolNames = ["lsp_ts_hover", "web_fetch"],
|
||||||
};
|
};
|
||||||
|
|
||||||
Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "lsp_ts_hover"));
|
Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "lsp_ts_hover"));
|
||||||
|
Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "web_fetch"));
|
||||||
Assert.True(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "lsp_ts_definition"));
|
Assert.True(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "lsp_ts_definition"));
|
||||||
Assert.True(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", null));
|
Assert.True(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", null));
|
||||||
Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-2", "lsp_ts_definition"));
|
Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-2", "lsp_ts_definition"));
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void TryGetApprovalToolName_ReadsMcpAndCustomToolRequests()
|
public void TryGetApprovalToolName_ReadsMcpCustomHookAndUrlRequests()
|
||||||
{
|
{
|
||||||
Assert.True(
|
Assert.True(
|
||||||
CopilotWorkflowRunner.TryGetApprovalToolName(
|
CopilotWorkflowRunner.TryGetApprovalToolName(
|
||||||
@@ -283,6 +284,30 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
out string? customToolName));
|
out string? customToolName));
|
||||||
Assert.Equal("lsp_ts_hover", customToolName);
|
Assert.Equal("lsp_ts_hover", customToolName);
|
||||||
|
|
||||||
|
Assert.True(
|
||||||
|
CopilotWorkflowRunner.TryGetApprovalToolName(
|
||||||
|
new PermissionRequestHook
|
||||||
|
{
|
||||||
|
Kind = "hook",
|
||||||
|
ToolName = "web_fetch",
|
||||||
|
ToolArgs = """{"url":"https://example.com"}""",
|
||||||
|
HookMessage = "Review required before fetch",
|
||||||
|
},
|
||||||
|
out string? hookToolName));
|
||||||
|
Assert.Equal("web_fetch", hookToolName);
|
||||||
|
|
||||||
|
Assert.True(
|
||||||
|
CopilotWorkflowRunner.TryGetApprovalToolName(
|
||||||
|
new PermissionRequestUrl
|
||||||
|
{
|
||||||
|
Kind = "url",
|
||||||
|
ToolCallId = "tool-call-1",
|
||||||
|
Intention = "Fetch the requested page",
|
||||||
|
Url = "https://example.com/docs",
|
||||||
|
},
|
||||||
|
out string? urlToolName));
|
||||||
|
Assert.Equal("web_fetch", urlToolName);
|
||||||
|
|
||||||
Assert.False(
|
Assert.False(
|
||||||
CopilotWorkflowRunner.TryGetApprovalToolName(
|
CopilotWorkflowRunner.TryGetApprovalToolName(
|
||||||
new PermissionRequestShell
|
new PermissionRequestShell
|
||||||
@@ -328,6 +353,37 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
Assert.Contains("tool \"lsp_ts_hover\"", approvalEvent.Detail);
|
Assert.Contains("tool \"lsp_ts_hover\"", approvalEvent.Detail);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void BuildPermissionApprovalEvent_IncludesRequestedUrlForUrlPermissions()
|
||||||
|
{
|
||||||
|
ApprovalRequestedEventDto approvalEvent = CopilotWorkflowRunner.BuildPermissionApprovalEvent(
|
||||||
|
new RunTurnCommandDto
|
||||||
|
{
|
||||||
|
RequestId = "turn-1",
|
||||||
|
SessionId = "session-1",
|
||||||
|
},
|
||||||
|
CreateAgent("agent-1", "Analyst"),
|
||||||
|
new PermissionRequestUrl
|
||||||
|
{
|
||||||
|
Kind = "url",
|
||||||
|
ToolCallId = "tool-call-1",
|
||||||
|
Intention = "Fetch the requested page",
|
||||||
|
Url = "https://example.com/docs",
|
||||||
|
},
|
||||||
|
new PermissionInvocation
|
||||||
|
{
|
||||||
|
SessionId = "copilot-session-1",
|
||||||
|
},
|
||||||
|
"approval-1",
|
||||||
|
"web_fetch");
|
||||||
|
|
||||||
|
Assert.Equal("web_fetch", approvalEvent.ToolName);
|
||||||
|
Assert.Equal("Approve web_fetch", approvalEvent.Title);
|
||||||
|
Assert.Contains("url permission", approvalEvent.Detail);
|
||||||
|
Assert.Contains("tool \"web_fetch\"", approvalEvent.Detail);
|
||||||
|
Assert.Contains("https://example.com/docs", approvalEvent.Detail);
|
||||||
|
}
|
||||||
|
|
||||||
private static PatternAgentDefinitionDto CreateAgent(string id, string name)
|
private static PatternAgentDefinitionDto CreateAgent(string id, string name)
|
||||||
{
|
{
|
||||||
return new PatternAgentDefinitionDto
|
return new PatternAgentDefinitionDto
|
||||||
|
|||||||
Reference in New Issue
Block a user