From 588e838dc4a3b0eb7cc44857675521d179cfa1d9 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Sat, 26 Jul 2025 04:36:46 -0400 Subject: [PATCH] Fix CLI runtime invalid path error handling (#9814) Co-authored-by: openhands --- openhands/runtime/base.py | 17 ++++++++++++++--- openhands/runtime/impl/cli/cli_runtime.py | 5 ++--- tests/unit/test_cli_workspace.py | 5 ++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/openhands/runtime/base.py b/openhands/runtime/base.py index 1423092579..9296a5bee1 100644 --- a/openhands/runtime/base.py +++ b/openhands/runtime/base.py @@ -17,7 +17,9 @@ import httpx from openhands.core.config import OpenHandsConfig, SandboxConfig from openhands.core.config.mcp_config import MCPConfig, MCPStdioServerConfig -from openhands.core.exceptions import AgentRuntimeDisconnectedError +from openhands.core.exceptions import ( + AgentRuntimeDisconnectedError, +) from openhands.core.logger import openhands_logger as logger from openhands.events import EventSource, EventStream, EventStreamSubscriber from openhands.events.action import ( @@ -340,10 +342,19 @@ class Runtime(FileEditRuntimeMixin): observation: Observation = await self.call_tool_mcp(event) else: observation = await call_sync_from_async(self.run_action, event) + except PermissionError as e: + # Handle PermissionError specially - convert to ErrorObservation + # so the agent can receive feedback and continue execution + observation = ErrorObservation(content=str(e)) + except (httpx.NetworkError, AgentRuntimeDisconnectedError) as e: + runtime_status = RuntimeStatus.ERROR_RUNTIME_DISCONNECTED + error_message = f'{type(e).__name__}: {str(e)}' + self.log('error', f'Unexpected error while running action: {error_message}') + self.log('error', f'Problematic action: {str(event)}') + self.set_runtime_status(runtime_status, error_message) + return except Exception as e: runtime_status = RuntimeStatus.ERROR - if isinstance(e, (httpx.NetworkError, AgentRuntimeDisconnectedError)): - runtime_status = RuntimeStatus.ERROR_RUNTIME_DISCONNECTED error_message = f'{type(e).__name__}: {str(e)}' self.log('error', f'Unexpected error while running action: {error_message}') self.log('error', f'Problematic action: {str(event)}') diff --git a/openhands/runtime/impl/cli/cli_runtime.py b/openhands/runtime/impl/cli/cli_runtime.py index 44ac62ac05..105b066e18 100644 --- a/openhands/runtime/impl/cli/cli_runtime.py +++ b/openhands/runtime/impl/cli/cli_runtime.py @@ -25,7 +25,6 @@ from pydantic import SecretStr from openhands.core.config import OpenHandsConfig from openhands.core.config.mcp_config import MCPConfig, MCPStdioServerConfig -from openhands.core.exceptions import LLMMalformedActionError from openhands.core.logger import openhands_logger as logger from openhands.events import EventStream from openhands.events.action import ( @@ -508,7 +507,7 @@ class CLIRuntime(Runtime): ) elif filename.startswith('/'): if not filename.startswith(self._workspace_path): - raise LLMMalformedActionError( + raise PermissionError( f'Invalid path: {filename}. You can only work with files in {self._workspace_path}.' ) actual_filename = filename @@ -520,7 +519,7 @@ class CLIRuntime(Runtime): # Check if the resolved path is still within the workspace if not resolved_path.startswith(self._workspace_path): - raise LLMMalformedActionError( + raise PermissionError( f'Invalid path traversal: {filename}. Path resolves outside the workspace. Resolved: {resolved_path}, Workspace: {self._workspace_path}' ) diff --git a/tests/unit/test_cli_workspace.py b/tests/unit/test_cli_workspace.py index 2dbb30973b..43822fd580 100644 --- a/tests/unit/test_cli_workspace.py +++ b/tests/unit/test_cli_workspace.py @@ -6,7 +6,6 @@ import tempfile import pytest from openhands.core.config import OpenHandsConfig -from openhands.core.exceptions import LLMMalformedActionError from openhands.events import EventStream from openhands.runtime.impl.cli.cli_runtime import CLIRuntime from openhands.storage import get_file_store @@ -49,7 +48,7 @@ def test_sanitize_filename_relative_path(cli_runtime): def test_sanitize_filename_outside_workspace(cli_runtime): """Test _sanitize_filename with a path outside the workspace.""" test_path = '/tmp/test.txt' # Path outside workspace - with pytest.raises(LLMMalformedActionError) as exc_info: + with pytest.raises(PermissionError) as exc_info: cli_runtime._sanitize_filename(test_path) assert 'Invalid path:' in str(exc_info.value) assert 'You can only work with files in' in str(exc_info.value) @@ -58,7 +57,7 @@ def test_sanitize_filename_outside_workspace(cli_runtime): def test_sanitize_filename_path_traversal(cli_runtime): """Test _sanitize_filename with path traversal attempt.""" test_path = os.path.join(cli_runtime._workspace_path, '..', 'test.txt') - with pytest.raises(LLMMalformedActionError) as exc_info: + with pytest.raises(PermissionError) as exc_info: cli_runtime._sanitize_filename(test_path) assert 'Invalid path traversal:' in str(exc_info.value) assert 'Path resolves outside the workspace' in str(exc_info.value)