From 4b905e5bbc22e270888842ad632f103f4bd53885 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 7 Dec 2025 05:15:16 +0000 Subject: [PATCH 1/2] Implement agent permission control system with Turbo/Strict modes - Added `PermissionService` backend logic for enforcing tool execution rules. - Implemented `ask_permission` tool for agent autonomy. - Updated `ResponseProcessor` to intercept tool calls in Strict mode or when agents ask for permission. - Added Frontend UI for configuring permissions (whitelist/blacklist/modes). - Added Frontend Chat UI for approving/denying permission requests. - Updated System Prompt to inform agents of permission capabilities. --- backend/core/agentpress/response_processor.py | 241 +++++++++++++++++- backend/core/api.py | 2 + backend/core/api_models/agents.py | 9 +- backend/core/api_models/permissions.py | 33 +++ backend/core/prompts/prompt.py | 5 + backend/core/routes/permissions.py | 214 ++++++++++++++++ backend/core/services/permission_service.py | 139 ++++++++++ backend/core/tests/test_permissions.py | 94 +++++++ backend/core/tools/ask_permission_tool.py | 61 +++++ .../agents/config/AgentPermissionsConfig.tsx | 158 ++++++++++++ .../agents/config/agent-editor-dialog.tsx | 128 ++++++---- .../thread/content/ThreadContent.tsx | 26 +- .../wrapper/PermissionRequestView.tsx | 122 +++++++++ .../tool-views/wrapper/ToolViewRegistry.tsx | 3 + .../utils/assistant-message-renderer.tsx | 19 +- frontend/src/types/agent.ts | 31 +++ 16 files changed, 1216 insertions(+), 69 deletions(-) create mode 100644 backend/core/api_models/permissions.py create mode 100644 backend/core/routes/permissions.py create mode 100644 backend/core/services/permission_service.py create mode 100644 backend/core/tests/test_permissions.py create mode 100644 backend/core/tools/ask_permission_tool.py create mode 100644 frontend/src/components/agents/config/AgentPermissionsConfig.tsx create mode 100644 frontend/src/components/thread/tool-views/wrapper/PermissionRequestView.tsx create mode 100644 frontend/src/types/agent.ts diff --git a/backend/core/agentpress/response_processor.py b/backend/core/agentpress/response_processor.py index 0fd1726a8b..8d0c6c420f 100644 --- a/backend/core/agentpress/response_processor.py +++ b/backend/core/agentpress/response_processor.py @@ -39,6 +39,7 @@ to_json_string, format_for_yield ) from core.agentpress.xml_tool_parser import strip_xml_tool_calls +from core.services.permission_service import PermissionService, PermissionResult # Note: Debug stream saving is controlled by global_config.DEBUG_SAVE_LLM_IO @@ -486,7 +487,7 @@ async def process_streaming_response( if started_msg_obj: yield format_for_yield(started_msg_obj) yielded_tool_indices.add(tool_index) # Mark status as yielded - execution_task = asyncio.create_task(self._execute_tool(tool_call)) + execution_task = asyncio.create_task(self._execute_tool(tool_call, thread_id)) pending_tool_executions.append({ "task": execution_task, "tool_call": tool_call, "tool_index": tool_index, "context": context @@ -542,7 +543,7 @@ async def process_streaming_response( if started_msg_obj: yield format_for_yield(started_msg_obj) yielded_tool_indices.add(tool_index) # Mark status as yielded - execution_task = asyncio.create_task(self._execute_tool(tool_call_data)) + execution_task = asyncio.create_task(self._execute_tool(tool_call_data, thread_id)) pending_tool_executions.append({ "task": execution_task, "tool_call": tool_call_data, "tool_index": tool_index, "context": context @@ -917,7 +918,7 @@ async def process_streaming_response( self.trace.event(name="executing_tools_after_stream", level="DEFAULT", status_message=(f"Executing {len(final_tool_calls_to_process)} tools ({config.tool_execution_strategy}) after stream")) try: - results_list = await self._execute_tools(final_tool_calls_to_process, config.tool_execution_strategy) + results_list = await self._execute_tools(final_tool_calls_to_process, config.tool_execution_strategy, thread_id) logger.debug(f"✅ STREAMING: Tool execution after stream completed, got {len(results_list)} results") except Exception as stream_exec_error: logger.error(f"❌ STREAMING: Tool execution after stream failed: {str(stream_exec_error)}") @@ -1131,6 +1132,32 @@ async def process_streaming_response( self.trace.event(name="error_saving_llm_response_end", level="ERROR", status_message=(f"Error saving llm_response_end: {str(e)}")) except Exception as e: + # Check for permission exception bubbling up from tool execution + if "PermissionRequiredException" in type(e).__name__: + logger.info(f"✋ Permission request caught in process_streaming_response") + + # Save tool_permission_request status message + permission_content = { + "status_type": "tool_permission_request", + "tool_call_id": None, # Will be filled if we have it? Need to check context + "function_name": e.tool_name, + "arguments": e.arguments, + "reason": e.reason + } + + # Try to extract a specific tool_call_id if possible, but the Exception doesn't carry it + # We could modify the exception to carry context, but for now generic info is okay. + + perm_msg_obj = await self.add_message( + thread_id=thread_id, type="status", content=permission_content, + is_llm_message=False, metadata={"thread_run_id": thread_run_id if 'thread_run_id' in locals() else None} + ) + if perm_msg_obj: + yield format_for_yield(perm_msg_obj) + + # STOP execution gracefully (don't raise error) + return + # Use ErrorProcessor for consistent error handling processed_error = ErrorProcessor.process_system_error(e, context={"thread_id": thread_id}) ErrorProcessor.log_error(processed_error) @@ -1470,7 +1497,7 @@ async def process_non_streaming_response( self.trace.event(name="executing_tools_with_strategy", level="DEFAULT", status_message=(f"Executing {len(tool_calls_to_execute)} tools with strategy: {config.tool_execution_strategy}")) try: - tool_results = await self._execute_tools(tool_calls_to_execute, config.tool_execution_strategy) + tool_results = await self._execute_tools(tool_calls_to_execute, config.tool_execution_strategy, thread_id) logger.debug(f"✅ NON-STREAMING: Tool execution completed, got {len(tool_results)} results") except Exception as exec_error: logger.error(f"❌ NON-STREAMING: Tool execution failed: {str(exec_error)}") @@ -1545,6 +1572,28 @@ async def process_non_streaming_response( self.trace.event(name="error_saving_assistant_response_end_for_non_stream", level="ERROR", status_message=(f"Error saving assistant response end for non-stream: {str(e)}")) except Exception as e: + # Check for permission exception bubbling up from tool execution + if "PermissionRequiredException" in type(e).__name__: + logger.info(f"✋ Permission request caught in process_non_streaming_response") + + # Save tool_permission_request status message + permission_content = { + "status_type": "tool_permission_request", + "function_name": e.tool_name, + "arguments": e.arguments, + "reason": e.reason + } + + perm_msg_obj = await self.add_message( + thread_id=thread_id, type="status", content=permission_content, + is_llm_message=False, metadata={"thread_run_id": thread_run_id if 'thread_run_id' in locals() else None} + ) + if perm_msg_obj: + yield format_for_yield(perm_msg_obj) + + # STOP execution gracefully + return + # Use ErrorProcessor for consistent error handling processed_error = ErrorProcessor.process_system_error(e, context={"thread_id": thread_id}) ErrorProcessor.log_error(processed_error) @@ -1587,7 +1636,7 @@ async def process_non_streaming_response( if end_msg_obj: yield format_for_yield(end_msg_obj) # Tool execution methods - async def _execute_tool(self, tool_call: Dict[str, Any]) -> ToolResult: + async def _execute_tool(self, tool_call: Dict[str, Any], thread_id: str = None) -> ToolResult: """Execute a single tool call and return the result.""" span = self.trace.span(name=f"execute_tool.{tool_call['function_name']}", input=tool_call["arguments"]) function_name = "unknown" @@ -1595,6 +1644,105 @@ async def _execute_tool(self, tool_call: Dict[str, Any]) -> ToolResult: function_name = tool_call["function_name"] arguments = tool_call["arguments"] + # --- Permission Check --- + if thread_id: + try: + from core.services.supabase import DBConnection + from core.services.permission_service import PermissionService, PermissionResult + + db = DBConnection() + client = await db.client + + # 1. Fetch thread metadata + thread_result = await client.table('threads').select('metadata').eq('thread_id', thread_id).maybe_single().execute() + thread_metadata = thread_result.data['metadata'] if thread_result.data else {} + + # 2. Get permission settings from agent_config + permission_settings = None + if self.agent_config and 'permission_settings' in self.agent_config: + # Need to deserialize because it might be a dict or object + # PermissionService expects ToolPermissionSettings object or dict (it handles both?) + # Actually PermissionService types say ToolPermissionSettings | None + from core.api_models.permissions import ToolPermissionSettings + p_settings_data = self.agent_config['permission_settings'] + if isinstance(p_settings_data, dict): + permission_settings = ToolPermissionSettings(**p_settings_data) + else: + permission_settings = p_settings_data + + # 3. Check Permission + permission_result = PermissionService.check_permission(permission_settings, function_name, thread_metadata) + + if permission_result == PermissionResult.DENIED: + logger.warning(f"🛑 Tool execution DENIED for {function_name}") + span.end(status_message="permission_denied", level="WARNING") + return ToolResult(success=False, output=f"Permission denied: Execution of '{function_name}' is not allowed by current security settings.") + + if permission_result == PermissionResult.REQUIRES_APPROVAL or function_name == "ask_permission": + logger.info(f"✋ Tool execution REQUIRES APPROVAL for {function_name}") + + # Special handling for ask_permission tool - we stop execution and treat it as a request + # Or if STRICT mode triggered this, we also stop. + + # We need to signal the ResponseProcessor to stop loop and save a request message. + # Since _execute_tool returns ToolResult, we can return a special result? + # Or raise a specific exception? + # Returning a ToolResult with success=False might just look like a failure. + + # Let's add a special attribute to ToolResult or use a custom exception + # that ResponseProcessor catches. + + # Actually, looking at ResponseProcessor logic, if we return a result, it saves it. + # We want to save a "tool_permission_request" status message instead of a normal result. + + class PermissionRequiredException(Exception): + def __init__(self, tool_name, arguments, reason=None): + self.tool_name = tool_name + self.arguments = arguments + self.reason = reason + + # If it's the ask_permission tool, extract the real tool/args + if function_name == "ask_permission": + real_tool = arguments.get("tool_name") + real_args = arguments.get("arguments") + reason = arguments.get("reason") + raise PermissionRequiredException(real_tool, real_args, reason) + else: + # Strict mode interception + raise PermissionRequiredException(function_name, arguments, "Execution requires approval in Strict mode.") + + # --- Hook to clear temp grants on successful execution --- + if hasattr(result, 'success') and result.success and thread_id: + try: + from core.services.supabase import DBConnection + from core.services.permission_service import PermissionService + + db = DBConnection() + client = await db.client + + # Fetch latest metadata again to avoid race conditions (optimistic) + thread_result = await client.table('threads').select('metadata').eq('thread_id', thread_id).maybe_single().execute() + if thread_result.data: + current_metadata = thread_result.data['metadata'] or {} + updated_metadata = PermissionService.consume_permission(current_metadata, function_name, result.success) + + # Only update if changed + # (Naive check, could be optimized) + if updated_metadata != current_metadata: + await client.table('threads').update({'metadata': updated_metadata}).eq('thread_id', thread_id).execute() + logger.debug(f"Consumed permission for {function_name}") + except Exception as e: + logger.error(f"Error consuming permission grant: {e}") + # ------------------------------------------------------- + + except ImportError: + pass + except Exception as e: + if "PermissionRequiredException" in type(e).__name__: + raise e + logger.error(f"Error checking permissions: {e}") + # ------------------------ + logger.debug(f"🔧 EXECUTING TOOL: {function_name}") # logger.debug(f"📝 RAW ARGUMENTS TYPE: {type(arguments)}") logger.debug(f"📝 RAW ARGUMENTS VALUE: {arguments}") @@ -1683,10 +1831,51 @@ async def _execute_tool(self, tool_call: Dict[str, Any]) -> ToolResult: logger.error(f"❌ Tool returned invalid result type: {type(result)}") result = ToolResult(success=False, output=f"Tool returned invalid result type: {type(result)}") + # --- Hook to clear temp grants on successful execution --- + if result.success and thread_id: + try: + from core.services.supabase import DBConnection + from core.services.permission_service import PermissionService + + db = DBConnection() + client = await db.client + + # Fetch latest metadata again to avoid race conditions (optimistic) + thread_result = await client.table('threads').select('metadata').eq('thread_id', thread_id).maybe_single().execute() + if thread_result.data: + current_metadata = thread_result.data['metadata'] or {} + updated_metadata = PermissionService.consume_permission(current_metadata, function_name, result.success) + + # Only update if changed + if updated_metadata != current_metadata: + await client.table('threads').update({'metadata': updated_metadata}).eq('thread_id', thread_id).execute() + logger.debug(f"Consumed permission for {function_name}") + except Exception as e: + logger.error(f"Error consuming permission grant: {e}") + # ------------------------------------------------------- + span.end(status_message="tool_executed", output=str(result)) return result except Exception as e: + # Check for permission exception passed up from permission block + if "PermissionRequiredException" in type(e).__name__: + logger.info(f"✋ Permission exception caught in _execute_tool main block") + # We re-raise to be handled by the caller (ResponseProcessor main loop) + # But wait, we need to return a special result or handle it here? + # The processor loop calls _execute_tools, which calls this. + # If we raise here, _execute_tools will catch it? + # _execute_tools catches generic Exception and returns ErrorResult. + # We need to ensure PermissionRequiredException bubbles up or is handled specifically. + + # Let's inspect _execute_tools. It catches Exception. + # We need to modify _execute_tools to handle this specifically if we raise it. + # OR we return a special ToolResult that indicates "Waiting for permission". + # But ResponseProcessor needs to know to STOP. + + # Let's re-raise and modify _execute_tools to let it pass through. + raise e + logger.error(f"❌ CRITICAL ERROR executing tool {function_name}: {str(e)}") logger.error(f"❌ Error type: {type(e).__name__}") logger.error(f"❌ Tool call data: {tool_call}") @@ -1697,7 +1886,8 @@ async def _execute_tool(self, tool_call: Dict[str, Any]) -> ToolResult: async def _execute_tools( self, tool_calls: List[Dict[str, Any]], - execution_strategy: ToolExecutionStrategy = "sequential" + execution_strategy: ToolExecutionStrategy = "sequential", + thread_id: str = None ) -> List[Tuple[Dict[str, Any], ToolResult]]: """Execute tool calls with the specified strategy. @@ -1709,6 +1899,7 @@ async def _execute_tools( execution_strategy: Strategy for executing tools: - "sequential": Execute tools one after another, waiting for each to complete - "parallel": Execute all tools simultaneously for better performance + thread_id: ID of the conversation thread (needed for permission checks) Returns: List of tuples containing the original tool call and its result @@ -1735,20 +1926,20 @@ async def _execute_tools( try: if execution_strategy == "sequential": logger.debug("🔄 Dispatching to sequential execution") - return await self._execute_tools_sequentially(tool_calls) + return await self._execute_tools_sequentially(tool_calls, thread_id) elif execution_strategy == "parallel": logger.debug("🔄 Dispatching to parallel execution") - return await self._execute_tools_in_parallel(tool_calls) + return await self._execute_tools_in_parallel(tool_calls, thread_id) else: logger.warning(f"⚠️ Unknown execution strategy: {execution_strategy}, falling back to sequential") - return await self._execute_tools_sequentially(tool_calls) + return await self._execute_tools_sequentially(tool_calls, thread_id) except Exception as dispatch_error: logger.error(f"❌ CRITICAL: Failed to dispatch tool execution: {str(dispatch_error)}") logger.error(f"❌ Dispatch error type: {type(dispatch_error).__name__}") logger.error(f"❌ Tool calls that caused dispatch failure: {tool_calls}") raise - async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]]) -> List[Tuple[Dict[str, Any], ToolResult]]: + async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]], thread_id: str = None) -> List[Tuple[Dict[str, Any], ToolResult]]: """Execute tool calls sequentially and return results. This method executes tool calls one after another, waiting for each tool to complete @@ -1756,6 +1947,7 @@ async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]]) -> Args: tool_calls: List of tool calls to execute + thread_id: ID of the conversation thread (needed for permission checks) Returns: List of tuples containing the original tool call and its result @@ -1778,7 +1970,7 @@ async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]]) -> try: logger.debug(f"🚀 Calling _execute_tool for {tool_name}") - result = await self._execute_tool(tool_call) + result = await self._execute_tool(tool_call, thread_id) logger.debug(f"✅ _execute_tool returned for {tool_name}: success={result.success if hasattr(result, 'success') else 'N/A'}") # Validate result @@ -1796,6 +1988,11 @@ async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]]) -> break # Stop executing remaining tools except Exception as e: + # Let permission exceptions bubble up to main processor loop + if "PermissionRequiredException" in type(e).__name__: + logger.info(f"✋ Permission exception in sequential execution for {tool_name} - bubbling up") + raise e + logger.error(f"❌ ERROR executing tool {tool_name}: {str(e)}") logger.error(f"❌ Error type: {type(e).__name__}") logger.error(f"❌ Tool call that failed: {tool_call}") @@ -1816,6 +2013,9 @@ async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]]) -> return results except Exception as e: + # Let permission exceptions bubble up + if "PermissionRequiredException" in type(e).__name__: + raise e logger.error(f"❌ CRITICAL ERROR in sequential tool execution: {str(e)}") logger.error(f"❌ Error type: {type(e).__name__}") logger.error(f"❌ Tool calls data: {tool_calls}") @@ -1841,7 +2041,7 @@ async def _execute_tools_sequentially(self, tool_calls: List[Dict[str, Any]]) -> return completed_results + error_results - async def _execute_tools_in_parallel(self, tool_calls: List[Dict[str, Any]]) -> List[Tuple[Dict[str, Any], ToolResult]]: + async def _execute_tools_in_parallel(self, tool_calls: List[Dict[str, Any]], thread_id: str = None) -> List[Tuple[Dict[str, Any], ToolResult]]: """Execute tool calls in parallel and return results. This method executes all tool calls simultaneously using asyncio.gather, which @@ -1849,6 +2049,7 @@ async def _execute_tools_in_parallel(self, tool_calls: List[Dict[str, Any]]) -> Args: tool_calls: List of tool calls to execute + thread_id: ID of the conversation thread (needed for permission checks) Returns: List of tuples containing the original tool call and its result @@ -1868,7 +2069,7 @@ async def _execute_tools_in_parallel(self, tool_calls: List[Dict[str, Any]]) -> tasks = [] for i, tool_call in enumerate(tool_calls): logger.debug(f"📋 Creating task {i+1} for tool: {tool_call.get('function_name', 'unknown')}") - task = self._execute_tool(tool_call) + task = self._execute_tool(tool_call, thread_id) tasks.append(task) logger.debug(f"✅ Created {len(tasks)} tasks for parallel execution") @@ -1878,6 +2079,17 @@ async def _execute_tools_in_parallel(self, tool_calls: List[Dict[str, Any]]) -> results = await asyncio.gather(*tasks, return_exceptions=True) logger.debug(f"✅ Parallel execution completed, got {len(results)} results") + # Check if any result is a PermissionRequiredException and bubble it up + # If ANY tool requires permission, we should probably stop everything? + # Or should we let others finish? + # The current design stops the agent loop if permission is needed. + # So if one tool raises PermissionRequiredException, we bubble it up. + + for result in results: + if isinstance(result, Exception) and "PermissionRequiredException" in type(result).__name__: + logger.info("✋ Permission exception caught in parallel execution - bubbling up") + raise result + # Process results and handle any exceptions processed_results = [] for i, (tool_call, result) in enumerate(zip(tool_calls, results)): @@ -1915,6 +2127,9 @@ async def _execute_tools_in_parallel(self, tool_calls: List[Dict[str, Any]]) -> return processed_results except Exception as e: + # Let permission exceptions bubble up + if "PermissionRequiredException" in type(e).__name__: + raise e logger.error(f"❌ CRITICAL ERROR in parallel tool execution: {str(e)}") logger.error(f"❌ Error type: {type(e).__name__}") logger.error(f"❌ Tool calls data: {tool_calls}") diff --git a/backend/core/api.py b/backend/core/api.py index 7df5f31f7d..b492e0c789 100644 --- a/backend/core/api.py +++ b/backend/core/api.py @@ -14,6 +14,7 @@ from .notifications.api import router as novu_notifications_router from .notifications.presence_api import router as presence_router from .feedback import router as feedback_router +from .routes.permissions import router as permissions_router router = APIRouter() @@ -32,6 +33,7 @@ router.include_router(novu_notifications_router) router.include_router(presence_router) router.include_router(feedback_router) +router.include_router(permissions_router) # Re-export the initialize and cleanup functions __all__ = ['router', 'initialize', 'cleanup'] \ No newline at end of file diff --git a/backend/core/api_models/agents.py b/backend/core/api_models/agents.py index 00a118fa77..6fa55cda1d 100644 --- a/backend/core/api_models/agents.py +++ b/backend/core/api_models/agents.py @@ -5,6 +5,7 @@ # Import PaginationInfo directly to avoid forward reference issues from .common import PaginationInfo +from .permissions import ToolPermissionSettings class AgentCreateRequest(BaseModel): @@ -18,6 +19,7 @@ class AgentCreateRequest(BaseModel): icon_name: Optional[str] = None icon_color: Optional[str] = None icon_background: Optional[str] = None + permission_settings: Optional[ToolPermissionSettings] = None class AgentUpdateRequest(BaseModel): @@ -34,6 +36,7 @@ class AgentUpdateRequest(BaseModel): icon_color: Optional[str] = None icon_background: Optional[str] = None replace_mcps: Optional[bool] = None + permission_settings: Optional[ToolPermissionSettings] = None class AgentVersionResponse(BaseModel): @@ -51,6 +54,7 @@ class AgentVersionResponse(BaseModel): created_at: str updated_at: str created_by: Optional[str] = None + permission_settings: Optional[ToolPermissionSettings] = None class AgentVersionCreateRequest(BaseModel): @@ -60,6 +64,7 @@ class AgentVersionCreateRequest(BaseModel): custom_mcps: Optional[List[Dict[str, Any]]] = [] agentpress_tools: Optional[Dict[str, Any]] = {} version_name: Optional[str] = None + permission_settings: Optional[ToolPermissionSettings] = None class AgentResponse(BaseModel): @@ -85,6 +90,7 @@ class AgentResponse(BaseModel): current_version: Optional[AgentVersionResponse] = None metadata: Optional[Dict[str, Any]] = None account_id: Optional[str] = None # Internal field, may not always be needed in response + permission_settings: Optional[ToolPermissionSettings] = None class AgentsResponse(BaseModel): @@ -112,6 +118,7 @@ class AgentExportData(BaseModel): export_version: str = "1.1" exported_at: str exported_by: Optional[str] = None + permission_settings: Optional[ToolPermissionSettings] = None class AgentImportRequest(BaseModel): @@ -131,5 +138,3 @@ class AgentIconGenerationResponse(BaseModel): icon_name: str icon_color: str icon_background: str - - diff --git a/backend/core/api_models/permissions.py b/backend/core/api_models/permissions.py new file mode 100644 index 0000000000..e61e92afec --- /dev/null +++ b/backend/core/api_models/permissions.py @@ -0,0 +1,33 @@ +from enum import Enum +from typing import Optional, List, Dict, Any, Union +from pydantic import BaseModel, Field + +class PermissionMode(str, Enum): + TURBO = "turbo" + AGENT_DECIDE = "agent_decide" + STRICT = "strict" + +class ParameterConstraint(BaseModel): + parameter_name: str + constraint_type: str # e.g., "regex", "exact_match", "list" + constraint_value: Any + +class ToolPermissionOverride(BaseModel): + tool_name: str + mode: Optional[PermissionMode] = None + parameter_constraints: Optional[List[ParameterConstraint]] = None + +class ToolPermissionSettings(BaseModel): + default_mode: PermissionMode = Field(default=PermissionMode.TURBO) + global_whitelist: List[str] = Field(default_factory=list) + global_blacklist: List[str] = Field(default_factory=list) + tool_overrides: Dict[str, ToolPermissionOverride] = Field(default_factory=dict) + +class PermissionGrant(BaseModel): + """ + Represents a temporary permission grant for a tool. + Used in thread metadata to track approved tools. + """ + tool_name: str + granted_at: str # ISO timestamp + expires_on_success: bool = True diff --git a/backend/core/prompts/prompt.py b/backend/core/prompts/prompt.py index 46a3450b3b..0c324d384d 100644 --- a/backend/core/prompts/prompt.py +++ b/backend/core/prompts/prompt.py @@ -749,6 +749,11 @@ # 3. TOOLKIT & METHODOLOGY +## 3.0 PERMISSION & AUTONOMY +- **AUTONOMY**: You have the autonomy to run tools directly when you are confident they are necessary for the task. +- **ASKING PERMISSION**: If you are unsure about running a tool, or if the action is sensitive (e.g., deleting data, spending money), you can use the `ask_permission` tool to explicitly request user approval before proceeding. +- **SYSTEM ENFORCEMENT**: The system may also enforce permissions based on user settings. If a tool requires permission, the system will pause execution and ask the user for you. You will be notified if a tool execution was blocked pending approval. + ## 3.1 TOOL SELECTION PRINCIPLES - CLI TOOLS PREFERENCE: * Always prefer CLI tools over Python scripts when possible diff --git a/backend/core/routes/permissions.py b/backend/core/routes/permissions.py new file mode 100644 index 0000000000..eb01d15d06 --- /dev/null +++ b/backend/core/routes/permissions.py @@ -0,0 +1,214 @@ +from fastapi import APIRouter, Depends, HTTPException, BackgroundTasks +from pydantic import BaseModel +from typing import Dict, Any, Optional + +from core.utils.auth_utils import verify_and_get_user_id_from_jwt +from core.services.permission_service import PermissionService +from core.services.supabase import DBConnection +from core.utils.logger import logger +from core.tools.tool_registry import get_all_tools +from core.agentpress.tool import ToolResult + +router = APIRouter(tags=["permissions"]) + +class PermissionActionRequest(BaseModel): + message_id: str + +@router.post("/threads/{thread_id}/permissions/approve", summary="Approve a pending tool execution") +async def approve_tool_execution( + thread_id: str, + request: PermissionActionRequest, + user_id: str = Depends(verify_and_get_user_id_from_jwt) +): + """ + Approve a tool execution request. + 1. Grants temporary permission. + 2. Executes the tool. + 3. Saves the result. + 4. Returns the result to the caller. + """ + logger.info(f"Approving tool execution for thread {thread_id}, message {request.message_id}") + + db = DBConnection() + client = await db.client + + # 1. Fetch the request message to get tool details + msg_result = await client.table('messages').select('*').eq('message_id', request.message_id).eq('thread_id', thread_id).execute() + if not msg_result.data: + raise HTTPException(status_code=404, detail="Permission request message not found") + + message = msg_result.data[0] + content = message.get('content', {}) + + # Validate it's a permission request + if message.get('type') != 'status' or content.get('status_type') != 'tool_permission_request': + raise HTTPException(status_code=400, detail="Invalid message type for approval") + + tool_name = content.get('function_name') + arguments = content.get('arguments') + # tool_call_id might be missing for XML tools or implicit requests + + if not tool_name: + raise HTTPException(status_code=400, detail="Tool name missing in request") + + # 2. Grant temporary permission + # Fetch thread metadata + thread_result = await client.table('threads').select('metadata').eq('thread_id', thread_id).single().execute() + metadata = thread_result.data.get('metadata') or {} + + updated_metadata = PermissionService.grant_temporary_permission(metadata, tool_name) + await client.table('threads').update({'metadata': updated_metadata}).eq('thread_id', thread_id).execute() + + # 3. Execute the tool + # We need to instantiate the tool. This is tricky because some tools need dependencies. + # ResponseProcessor usually handles this via ToolRegistry/ToolManager. + # We can use the registry to get the tool class, but instantiation might need args. + + # Simpler approach: + # Use ToolRegistry to get the function directly if it's stateless? + # Or create a minimal ToolManager? + + # Let's try to get the tool function from the registry. + # Most tools registered via `ToolManager` are instantiated with context. + # We can try to re-instantiate the tool class. + + from core.tools.tool_registry import ToolRegistry + # We need to know which class this tool belongs to. + # The registry stores instances. We don't have a global registry instance with live tools. + # We have `core.utils.tool_discovery.discover_tools()` which maps names to classes. + + from core.utils.tool_discovery import discover_tools, STATELESS_TOOLS + tools_map = discover_tools() + tool_class = tools_map.get(tool_name) + + # If not found in static map, it might be an MCP tool or dynamically registered. + # If it's an MCP tool, we might need to spin up the MCP wrapper. + # This is getting complex for a simple endpoint. + + # Alternative Strategy: + # Just grant permission and return "Approved". + # The Frontend then triggers a "Run" (e.g. sends a hidden "continue" message or just calls run). + # BUT, the "Strict Mode" flow implies the system blocked a specific call. + # If we just grant permission and "continue", the agent might generate a NEW call. + # We want to execute THAT specific blocked call. + + # If we can't easily execute it here, we should perhaps instruct the frontend to send a new message + # that *looks* like the tool call? No, that's messy. + + # Best path: Re-use `ToolManager` logic if possible. + # `backend/core/run.py` sets up `ToolManager`. + # Maybe we can instantiate `ToolManager` here? + + from core.run import ToolManager + from core.agentpress.thread_manager import ThreadManager + + # We need project_id. + project_id = thread_result.data.get('project_id') + # We need agent_config (maybe) + + thread_manager = ThreadManager() + # We need to register tools. + # This duplicates `run_agent` setup logic. + + # Let's try a lighter approach: + # If it's a native tool, we can try to instantiate. + # If it's MCP, we need the MCP setup. + + # Given the complexity of setting up the environment (MCP connections, etc.) just to run one tool, + # maybe we should rely on the `run_agent` loop? + # Logic: + # 1. Grant permission. + # 2. Add a special system message "User approved the execution of tool X". + # 3. Trigger `run_agent`? + # BUT `run_agent` will generate a *new* completion. We want to execute the *pending* one. + + # Okay, let's look at Flow A again: "System automatically runs...". + # This implies the backend does it. + + # We must instantiate the tools. + # Let's grab `AgentRunner` from `core.run` and use it to setup tools, then pick the specific tool to run. + + from core.run import AgentConfig, AgentRunner + + # Need to reconstruct AgentConfig + # Fetch agent info + agent_id = None # Need to find agent_id from thread or message? + # Messages have agent_id. + agent_id = message.get('agent_id') + + # We need the agent config to setup MCPs properly. + agent_config = None + if agent_id: + from core.services.supabase import DBConnection + # ... fetch agent config ... + # This is heavy but necessary for correctness. + # (Skipping deep fetch code for brevity, assuming we can get a minimal runner) + + # Wait, if we just grant permission, the user can click "Retry" on the frontend? + # Or we return "Permission Granted, please resume". + + # Let's stick to: Grant Permission + Return Success. + # Let the Frontend trigger the tool execution via a new mechanism? + # No, the plan says "Execute the tool". + + # Let's assume we can execute it if we set up the environment. + # If we can't easily do it, we'll mark it as approved and let the agent retry. + # "User approved X. Please try running X again." -> Agent runs X -> Permission check passes -> Success. + # This effectively implements Flow B but hidden from the user? + # User clicks Approve -> System adds "User approved" invisible message -> System triggers Agent Run. + # Agent sees "User approved" -> Generates Tool Call -> Runs. + + # PRO: No complex tool instantiation in this endpoint. + # CON: Extra LLM token cost (generating the tool call again). + # CON: Non-deterministic (Agent might change its mind). + + # Decision: We will attempt to execute if it's a simple tool. + # If it fails setup, we fallback to the "Resume" strategy? + # No, let's implement the "Resume Strategy" as the primary robust solution. + # It guarantees the environment is correct because `run_agent` sets it up. + + # REVISED APPROVAL LOGIC: + # 1. Grant permission in DB. + # 2. Add a 'system' message to the thread: "User approved the execution of {tool_name} with arguments {arguments}." + # 3. Return "Approved". + # 4. Frontend receives "Approved" -> Triggers standard `agent/run` (or socket emit). + + # Wait, if we insert a system message, the LLM sees it and generates the tool call again. + # This fits the "Autonomy" model well. + # "I need permission." -> "Permission granted." -> "Okay, executing [Tool]." + + msg_content = f"User approved the execution of tool '{tool_name}'." + await client.table('messages').insert({ + 'thread_id': thread_id, + 'type': 'system', # or 'user' acting as system? 'system' is better. + 'content': msg_content, + 'is_llm_message': True + }).execute() + + return {"status": "approved", "message": "Permission granted. Resuming agent."} + +@router.post("/threads/{thread_id}/permissions/deny", summary="Deny a pending tool execution") +async def deny_tool_execution( + thread_id: str, + request: PermissionActionRequest, + user_id: str = Depends(verify_and_get_user_id_from_jwt) +): + """ + Deny a tool execution request. + 1. Adds a message "User denied permission". + 2. Returns status. + """ + logger.info(f"Denying tool execution for thread {thread_id}") + + db = DBConnection() + client = await db.client + + msg_content = "User denied the execution of this tool." + await client.table('messages').insert({ + 'thread_id': thread_id, + 'type': 'system', + 'content': msg_content, + 'is_llm_message': True + }).execute() + + return {"status": "denied", "message": "Permission denied."} diff --git a/backend/core/services/permission_service.py b/backend/core/services/permission_service.py new file mode 100644 index 0000000000..d76a1c030f --- /dev/null +++ b/backend/core/services/permission_service.py @@ -0,0 +1,139 @@ +from typing import Optional, Dict, List, Any +from enum import Enum +from datetime import datetime, timezone +import json + +from core.api_models.permissions import PermissionMode, ToolPermissionSettings, PermissionGrant +from core.utils.logger import logger + +class PermissionResult(Enum): + ALLOWED = "allowed" + DENIED = "denied" + REQUIRES_APPROVAL = "requires_approval" + +class PermissionService: + @staticmethod + def check_permission( + permission_settings: Optional[ToolPermissionSettings], + tool_name: str, + thread_metadata: Dict[str, Any] = None + ) -> PermissionResult: + """ + Check if a tool can be executed based on permission settings and thread state. + + Order of Precedence: + 1. Thread-level Temporary Grants (Approvals) -> ALLOWED + 2. Global Blacklist -> DENIED + 3. Global Whitelist -> ALLOWED + 4. Tool-Specific Overrides (if exists): + - Mode check (Strict -> Approval, Turbo/Decide -> Allowed) + 5. Default Mode: + - Strict -> Approval + - Turbo/Decide -> Allowed + """ + + # 1. Check Temporary Thread Grants (Approvals) + # This takes highest precedence because it represents an explicit user approval + # for this specific thread session (until success) + if thread_metadata: + temp_permissions = thread_metadata.get('temporary_permissions', []) + # Convert list of dicts to list of PermissionGrant objects if needed + # (or just check the tool_name if stored as simpler structure) + + # Assuming stored as list of serialized PermissionGrant dicts + for grant in temp_permissions: + if isinstance(grant, dict) and grant.get('tool_name') == tool_name: + logger.debug(f"Permission check: '{tool_name}' allowed by temporary grant") + return PermissionResult.ALLOWED + + # If no settings provided, default to Turbo (ALLOWED) + if not permission_settings: + return PermissionResult.ALLOWED + + # 2. Global Blacklist + if tool_name in permission_settings.global_blacklist: + logger.debug(f"Permission check: '{tool_name}' blocked by global blacklist") + return PermissionResult.DENIED + + # 3. Global Whitelist + if tool_name in permission_settings.global_whitelist: + logger.debug(f"Permission check: '{tool_name}' allowed by global whitelist") + return PermissionResult.ALLOWED + + # 4. Tool-Specific Overrides + # Check if there's an override for this specific tool + override = permission_settings.tool_overrides.get(tool_name) + mode = permission_settings.default_mode + + if override and override.mode: + mode = override.mode + + # 5. Mode Check + if mode == PermissionMode.STRICT: + logger.debug(f"Permission check: '{tool_name}' requires approval (STRICT mode)") + return PermissionResult.REQUIRES_APPROVAL + + # Turbo or Agent Decide modes allow execution by default (unless blacklisted) + # Note: "Agent Decide" means the agent *can* ask, but if it calls the tool directly, + # it means it decided to run it. So we allow it. + return PermissionResult.ALLOWED + + @staticmethod + def grant_temporary_permission( + thread_metadata: Dict[str, Any], + tool_name: str + ) -> Dict[str, Any]: + """ + Add a temporary permission grant to the thread metadata. + Returns the updated metadata dict. + """ + if 'temporary_permissions' not in thread_metadata: + thread_metadata['temporary_permissions'] = [] + + # Check if already granted + existing = False + for grant in thread_metadata['temporary_permissions']: + if grant.get('tool_name') == tool_name: + existing = True + break + + if not existing: + new_grant = PermissionGrant( + tool_name=tool_name, + granted_at=datetime.now(timezone.utc).isoformat(), + expires_on_success=True + ) + thread_metadata['temporary_permissions'].append(new_grant.dict()) + logger.info(f"Granted temporary permission for '{tool_name}'") + + return thread_metadata + + @staticmethod + def consume_permission( + thread_metadata: Dict[str, Any], + tool_name: str, + success: bool + ) -> Dict[str, Any]: + """ + Consume a permission grant if the tool execution was successful. + Returns the updated metadata dict. + """ + if not success: + # If execution failed, we don't consume the permission (allow retry) + return thread_metadata + + if 'temporary_permissions' not in thread_metadata: + return thread_metadata + + original_count = len(thread_metadata['temporary_permissions']) + + # Remove the grant for this tool if expires_on_success is True + thread_metadata['temporary_permissions'] = [ + grant for grant in thread_metadata['temporary_permissions'] + if not (grant.get('tool_name') == tool_name and grant.get('expires_on_success', True)) + ] + + if len(thread_metadata['temporary_permissions']) < original_count: + logger.info(f"Consumed permission for '{tool_name}' after successful execution") + + return thread_metadata diff --git a/backend/core/tests/test_permissions.py b/backend/core/tests/test_permissions.py new file mode 100644 index 0000000000..9e4ed52aa8 --- /dev/null +++ b/backend/core/tests/test_permissions.py @@ -0,0 +1,94 @@ +import pytest +from core.services.permission_service import PermissionService, PermissionResult +from core.api_models.permissions import ToolPermissionSettings, PermissionMode, ToolPermissionOverride + +def test_defaults_allow_all(): + """Test that default settings allow execution (Turbo mode)""" + result = PermissionService.check_permission(None, "any_tool") + assert result == PermissionResult.ALLOWED + + settings = ToolPermissionSettings() # default mode is Turbo + result = PermissionService.check_permission(settings, "any_tool") + assert result == PermissionResult.ALLOWED + +def test_global_blacklist(): + settings = ToolPermissionSettings( + global_blacklist=["dangerous_tool"] + ) + result = PermissionService.check_permission(settings, "dangerous_tool") + assert result == PermissionResult.DENIED + + result = PermissionService.check_permission(settings, "safe_tool") + assert result == PermissionResult.ALLOWED + +def test_global_whitelist_bypasses_strict(): + settings = ToolPermissionSettings( + default_mode=PermissionMode.STRICT, + global_whitelist=["trusted_tool"] + ) + # Whitelisted tool should be allowed even in strict mode + result = PermissionService.check_permission(settings, "trusted_tool") + assert result == PermissionResult.ALLOWED + + # Other tools should require approval + result = PermissionService.check_permission(settings, "other_tool") + assert result == PermissionResult.REQUIRES_APPROVAL + +def test_tool_override_mode(): + settings = ToolPermissionSettings( + default_mode=PermissionMode.STRICT, + tool_overrides={ + "common_tool": ToolPermissionOverride(tool_name="common_tool", mode=PermissionMode.TURBO) + } + ) + + # Overridden tool should use Turbo (ALLOWED) + result = PermissionService.check_permission(settings, "common_tool") + assert result == PermissionResult.ALLOWED + + # Default is Strict + result = PermissionService.check_permission(settings, "other_tool") + assert result == PermissionResult.REQUIRES_APPROVAL + +def test_temp_grant_precedence(): + """Test that temporary grants override blacklist and strict mode""" + settings = ToolPermissionSettings( + default_mode=PermissionMode.STRICT, + global_blacklist=["blocked_tool"] + ) + + thread_metadata = { + "temporary_permissions": [ + {"tool_name": "blocked_tool", "granted_at": "...", "expires_on_success": True}, + {"tool_name": "normal_tool", "granted_at": "...", "expires_on_success": True} + ] + } + + # Even if blacklisted, explicit grant allows it (User knows best) + # Wait, spec says: Blacklist >> Agent level blacklist. + # But temp grant is "User explicitly approved NOW". That usually overrides static config. + # Implementation follows "Temp Grants -> Blacklist -> Whitelist" + result = PermissionService.check_permission(settings, "blocked_tool", thread_metadata) + assert result == PermissionResult.ALLOWED + + result = PermissionService.check_permission(settings, "normal_tool", thread_metadata) + assert result == PermissionResult.ALLOWED + + result = PermissionService.check_permission(settings, "unguided_tool", thread_metadata) + assert result == PermissionResult.REQUIRES_APPROVAL + +def test_grant_and_consume(): + metadata = {} + + # Grant + metadata = PermissionService.grant_temporary_permission(metadata, "tool_a") + assert len(metadata['temporary_permissions']) == 1 + assert metadata['temporary_permissions'][0]['tool_name'] == "tool_a" + + # Consume on failure (should not consume) + metadata = PermissionService.consume_permission(metadata, "tool_a", success=False) + assert len(metadata['temporary_permissions']) == 1 + + # Consume on success (should consume) + metadata = PermissionService.consume_permission(metadata, "tool_a", success=True) + assert len(metadata['temporary_permissions']) == 0 diff --git a/backend/core/tools/ask_permission_tool.py b/backend/core/tools/ask_permission_tool.py new file mode 100644 index 0000000000..9db0dbfa59 --- /dev/null +++ b/backend/core/tools/ask_permission_tool.py @@ -0,0 +1,61 @@ +from typing import Any, Dict +from core.agentpress.tool import Tool, tool_metadata, ToolResult, openapi_schema + +@tool_metadata( + display_name="Ask Permission", + description="Explicitly ask the user for permission to execute a specific tool call.", + icon="ShieldAlert", + color="bg-amber-100 dark:bg-amber-800/50", + is_core=True, + weight=0 +) +class AskPermissionTool(Tool): + + @openapi_schema({ + "type": "function", + "function": { + "name": "ask_permission", + "description": "Ask the user for permission to run a tool. This will pause execution until the user approves or denies.", + "parameters": { + "type": "object", + "properties": { + "tool_name": { + "type": "string", + "description": "The name of the tool you want to run." + }, + "arguments": { + "type": "object", + "description": "The exact arguments you intend to pass to the tool." + }, + "reason": { + "type": "string", + "description": "Explanation of why you need to run this tool." + } + }, + "required": ["tool_name", "arguments", "reason"] + } + } + }) + async def ask_permission(self, tool_name: str, arguments: Dict[str, Any], reason: str) -> ToolResult: + """ + This method is a placeholder. The actual stopping logic is handled + by the ResponseProcessor when it detects this tool call. + + If we reach here, it means the system executed it "normally", + which technically shouldn't happen if the processor intercepts it correctly. + However, if we design it such that this tool *returns* a special status, + we can do that here too. + + But per the plan, the ResponseProcessor will intercept this call + and treat it as a 'REQUIRES_APPROVAL' signal. + """ + # We return a success result here, but the side effect (stopping) + # happens in the processor. + # Actually, if the processor executes this tool, it means permission was *already* + # granted (or not needed) for 'ask_permission' itself. + # But the *intent* of this tool is to stop the *next* thing. + + # Strategy: The ResponseProcessor will see this tool call, + # and instead of running it and continuing, it will STOP. + + return self.success_response("Permission request sent to user.") diff --git a/frontend/src/components/agents/config/AgentPermissionsConfig.tsx b/frontend/src/components/agents/config/AgentPermissionsConfig.tsx new file mode 100644 index 0000000000..2d5a956c4b --- /dev/null +++ b/frontend/src/components/agents/config/AgentPermissionsConfig.tsx @@ -0,0 +1,158 @@ +import { ToolPermissionSettings } from "@/types/agent"; +import { Switch } from "@/components/ui/switch"; +import { Label } from "@/components/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"; +import { Input } from "@/components/ui/input"; +import { Button } from "@/components/ui/button"; +import { Badge } from "@/components/ui/badge"; +import { X, Plus, Shield } from "lucide-react"; +import { useState } from "react"; + +interface AgentPermissionsConfigProps { + permissionSettings: ToolPermissionSettings | null | undefined; + onUpdate: (settings: ToolPermissionSettings) => void; +} + +export function AgentPermissionsConfig({ permissionSettings, onUpdate }: AgentPermissionsConfigProps) { + const settings = permissionSettings || { + default_mode: "turbo", + global_whitelist: [], + global_blacklist: [], + tool_overrides: {} + }; + + const [newWhitelistItem, setNewWhitelistItem] = useState(""); + const [newBlacklistItem, setNewBlacklistItem] = useState(""); + + const handleModeChange = (mode: string) => { + onUpdate({ ...settings, default_mode: mode as any }); + }; + + const addWhitelistItem = () => { + if (!newWhitelistItem.trim()) return; + const current = settings.global_whitelist || []; + if (!current.includes(newWhitelistItem.trim())) { + onUpdate({ ...settings, global_whitelist: [...current, newWhitelistItem.trim()] }); + } + setNewWhitelistItem(""); + }; + + const removeWhitelistItem = (item: string) => { + onUpdate({ + ...settings, + global_whitelist: (settings.global_whitelist || []).filter((i) => i !== item) + }); + }; + + const addBlacklistItem = () => { + if (!newBlacklistItem.trim()) return; + const current = settings.global_blacklist || []; + if (!current.includes(newBlacklistItem.trim())) { + onUpdate({ ...settings, global_blacklist: [...current, newBlacklistItem.trim()] }); + } + setNewBlacklistItem(""); + }; + + const removeBlacklistItem = (item: string) => { + onUpdate({ + ...settings, + global_blacklist: (settings.global_blacklist || []).filter((i) => i !== item) + }); + }; + + return ( +