mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
Fix CLI runtime invalid path error handling (#9814)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -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)}')
|
||||
|
||||
@@ -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}'
|
||||
)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user