Skip to content

Commit 8e308d4

Browse files
fix: managed agents sandbox security (fixes #1426) (#1442)
* fix: managed agents sandbox security (fixes #1426) Add ManagedSandboxRequired exception for package installation safety Modify LocalManagedAgent to use compute providers for secure execution Implement compute-based tool execution routing for shell commands Add host_packages_ok safety opt-out flag for developer workflows Remove unused sandbox_type config field Add comprehensive tests for security functionality Security improvements: - Packages install in sandbox when compute provider attached - Raise exception when packages specified without compute/opt-out - Route execute_command, read_file, write_file, list_files through compute - Maintain backward compatibility with explicit opt-out flag Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com> * test: force asyncio.run fallback path in compute-bridge tests --------- Co-authored-by: praisonai-triage-agent[bot] <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
1 parent 6df0437 commit 8e308d4

File tree

3 files changed

+426
-7
lines changed

3 files changed

+426
-7
lines changed

src/praisonai-agents/tests/managed/test_managed_factory.py

Lines changed: 213 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ def test_defaults(self):
5353
cfg = LocalManagedConfig()
5454
assert cfg.name == "Agent"
5555
assert cfg.model == "gpt-4o"
56-
assert cfg.sandbox_type == "subprocess"
5756
assert cfg.max_turns == 25
5857
assert "execute_command" in cfg.tools
58+
assert cfg.host_packages_ok is False
5959

6060
def test_custom_config(self):
6161
from praisonai.integrations.managed_local import LocalManagedConfig
@@ -353,6 +353,218 @@ def test_packages_in_config(self):
353353
cfg = LocalManagedConfig(packages={"pip": ["pandas", "numpy"]})
354354
assert cfg.packages == {"pip": ["pandas", "numpy"]}
355355

356+
def test_host_packages_ok_default_false(self):
357+
from praisonai.integrations.managed_local import LocalManagedConfig
358+
cfg = LocalManagedConfig()
359+
assert cfg.host_packages_ok is False
360+
361+
def test_host_packages_ok_explicit_true(self):
362+
from praisonai.integrations.managed_local import LocalManagedConfig
363+
cfg = LocalManagedConfig(host_packages_ok=True)
364+
assert cfg.host_packages_ok is True
365+
366+
367+
class TestManagedSandboxSafety:
368+
"""Test security features for managed agents package installation."""
369+
370+
def test_install_packages_without_compute_raises(self):
371+
"""Test that package installation without compute provider raises ManagedSandboxRequired."""
372+
import pytest
373+
from praisonai.integrations.managed_local import LocalManagedAgent, LocalManagedConfig
374+
from praisonai.integrations.managed_agents import ManagedSandboxRequired
375+
376+
cfg = LocalManagedConfig(packages={"pip": ["requests"]})
377+
agent = LocalManagedAgent(config=cfg)
378+
379+
with pytest.raises(ManagedSandboxRequired) as exc_info:
380+
agent._install_packages()
381+
382+
assert "Package installation requested" in str(exc_info.value)
383+
assert "security risk" in str(exc_info.value)
384+
assert "compute='docker'" in str(exc_info.value)
385+
assert "host_packages_ok=True" in str(exc_info.value)
386+
387+
def test_install_packages_with_host_packages_ok_succeeds(self):
388+
"""Test that package installation with host_packages_ok=True succeeds."""
389+
from unittest.mock import patch
390+
from praisonai.integrations.managed_local import LocalManagedAgent, LocalManagedConfig
391+
392+
cfg = LocalManagedConfig(packages={"pip": ["requests"]}, host_packages_ok=True)
393+
agent = LocalManagedAgent(config=cfg)
394+
395+
with patch('praisonai.integrations.managed_local.subprocess.run') as mock_run:
396+
mock_run.return_value = None
397+
agent._install_packages() # Should not raise
398+
mock_run.assert_called_once()
399+
400+
def test_install_packages_with_compute_uses_sandbox(self):
401+
"""Test that package installation with compute provider uses sandbox."""
402+
from unittest.mock import AsyncMock, patch
403+
from praisonai.integrations.managed_local import LocalManagedAgent, LocalManagedConfig
404+
405+
cfg = LocalManagedConfig(packages={"pip": ["requests"]})
406+
agent = LocalManagedAgent(config=cfg, compute="local")
407+
408+
# Mock the compute execution
409+
with patch.object(agent, 'provision_compute') as mock_provision, \
410+
patch.object(agent._compute, 'execute') as mock_execute, \
411+
patch('asyncio.run') as mock_asyncio_run, \
412+
patch('asyncio.get_event_loop') as mock_get_loop:
413+
414+
mock_provision.return_value = None
415+
mock_execute.return_value = {"exit_code": 0, "stdout": "installed"}
416+
agent._compute_instance_id = "test_instance"
417+
mock_asyncio_run.return_value = {"exit_code": 0, "stdout": "installed"}
418+
419+
agent._install_packages()
420+
421+
# Verify subprocess.run was NOT called (no host installation)
422+
with patch('praisonai.integrations.managed_local.subprocess.run') as mock_run:
423+
agent._install_packages()
424+
mock_run.assert_not_called()
425+
426+
def test_no_packages_no_error(self):
427+
"""Test that agents without packages work normally."""
428+
from praisonai.integrations.managed_local import LocalManagedAgent
429+
agent = LocalManagedAgent()
430+
agent._install_packages() # Should not raise
431+
432+
def test_empty_packages_no_error(self):
433+
"""Test that empty packages dict works normally."""
434+
from praisonai.integrations.managed_local import LocalManagedConfig, LocalManagedAgent
435+
cfg = LocalManagedConfig(packages={"pip": []})
436+
agent = LocalManagedAgent(config=cfg)
437+
agent._install_packages() # Should not raise
438+
439+
440+
class TestComputeToolBridge:
441+
"""Test compute-based tool execution routing."""
442+
443+
def test_bridged_tools_created_when_compute_attached(self):
444+
"""Test that shell-based tools are bridged when compute is attached."""
445+
from praisonai.integrations.managed_local import LocalManagedAgent
446+
agent = LocalManagedAgent(compute="local")
447+
tools = agent._resolve_tools()
448+
449+
# Should have tools but they should be wrapped/bridged versions
450+
tool_names = [getattr(t, '__name__', str(t)) for t in tools if callable(t)]
451+
assert "execute_command" in tool_names
452+
453+
def test_non_bridged_tools_use_original_when_no_compute(self):
454+
"""Test that tools use original implementation when no compute."""
455+
from praisonai.integrations.managed_local import LocalManagedAgent
456+
agent = LocalManagedAgent()
457+
tools = agent._resolve_tools()
458+
459+
# Should have original tools
460+
tool_names = [getattr(t, '__name__', str(t)) for t in tools if callable(t)]
461+
assert "execute_command" in tool_names
462+
463+
def test_compute_bridge_tool_execute_command(self):
464+
"""Test that execute_command is properly bridged to compute."""
465+
from unittest.mock import AsyncMock, patch
466+
from praisonai.integrations.managed_local import LocalManagedAgent
467+
468+
agent = LocalManagedAgent(compute="local")
469+
agent._compute_instance_id = "test_instance"
470+
471+
# Create a bridge tool for execute_command
472+
original_func = lambda command: "original result"
473+
bridge_tool = agent._create_compute_bridge_tool("execute_command", original_func)
474+
475+
with patch.object(agent._compute, 'execute'), \
476+
patch('asyncio.get_event_loop', side_effect=RuntimeError('no loop')), \
477+
patch('asyncio.run') as mock_asyncio_run:
478+
479+
mock_asyncio_run.return_value = {"exit_code": 0, "stdout": "compute result"}
480+
481+
result = bridge_tool("echo hello")
482+
assert result == "compute result"
483+
484+
# Verify it attempted to run in compute, not locally
485+
mock_asyncio_run.assert_called()
486+
487+
def test_compute_bridge_tool_read_file(self):
488+
"""Test that read_file is properly bridged to compute."""
489+
from unittest.mock import patch
490+
from praisonai.integrations.managed_local import LocalManagedAgent
491+
492+
agent = LocalManagedAgent(compute="local")
493+
agent._compute_instance_id = "test_instance"
494+
495+
original_func = lambda filepath: "original content"
496+
bridge_tool = agent._create_compute_bridge_tool("read_file", original_func)
497+
498+
with patch.object(agent, '_bridge_file_tool') as mock_bridge:
499+
mock_bridge.return_value = "file content from compute"
500+
501+
result = bridge_tool("/path/to/file")
502+
assert result == "file content from compute"
503+
mock_bridge.assert_called_once_with("read_file", "/path/to/file")
504+
505+
def test_compute_bridge_tool_write_file(self):
506+
"""Test that write_file is properly bridged to compute."""
507+
from unittest.mock import patch
508+
from praisonai.integrations.managed_local import LocalManagedAgent
509+
510+
agent = LocalManagedAgent(compute="local")
511+
agent._compute_instance_id = "test_instance"
512+
513+
original_func = lambda filepath, content: "written locally"
514+
bridge_tool = agent._create_compute_bridge_tool("write_file", original_func)
515+
516+
with patch.object(agent, '_bridge_file_tool') as mock_bridge:
517+
mock_bridge.return_value = "written to compute"
518+
519+
result = bridge_tool("/path/to/file", "content")
520+
assert result == "written to compute"
521+
mock_bridge.assert_called_once_with("write_file", "/path/to/file", "content")
522+
523+
def test_bridge_file_tool_read(self):
524+
"""Test _bridge_file_tool for read operations."""
525+
from unittest.mock import patch
526+
from praisonai.integrations.managed_local import LocalManagedAgent
527+
528+
agent = LocalManagedAgent(compute="local")
529+
agent._compute_instance_id = "test_instance"
530+
531+
with patch('asyncio.get_event_loop', side_effect=RuntimeError('no loop')), \
532+
patch('asyncio.run') as mock_asyncio_run:
533+
mock_asyncio_run.return_value = {"exit_code": 0, "stdout": "file contents"}
534+
535+
result = agent._bridge_file_tool("read_file", "/test/file")
536+
assert result == "file contents"
537+
538+
def test_bridge_file_tool_write(self):
539+
"""Test _bridge_file_tool for write operations."""
540+
from unittest.mock import patch
541+
from praisonai.integrations.managed_local import LocalManagedAgent
542+
543+
agent = LocalManagedAgent(compute="local")
544+
agent._compute_instance_id = "test_instance"
545+
546+
with patch('asyncio.get_event_loop', side_effect=RuntimeError('no loop')), \
547+
patch('asyncio.run') as mock_asyncio_run:
548+
mock_asyncio_run.return_value = {"exit_code": 0, "stdout": ""}
549+
550+
result = agent._bridge_file_tool("write_file", "/test/file", "content")
551+
assert result == ""
552+
553+
def test_bridge_file_tool_list(self):
554+
"""Test _bridge_file_tool for list operations."""
555+
from unittest.mock import patch
556+
from praisonai.integrations.managed_local import LocalManagedAgent
557+
558+
agent = LocalManagedAgent(compute="local")
559+
agent._compute_instance_id = "test_instance"
560+
561+
with patch('asyncio.get_event_loop', side_effect=RuntimeError('no loop')), \
562+
patch('asyncio.run') as mock_asyncio_run:
563+
mock_asyncio_run.return_value = {"exit_code": 0, "stdout": "file1\nfile2\n"}
564+
565+
result = agent._bridge_file_tool("list_files", "/test/dir")
566+
assert result == "file1\nfile2\n"
567+
356568

357569
class TestUpdateAgentKeepsSession:
358570
def test_update_preserves_session(self):

src/praisonai/praisonai/integrations/managed_agents.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,24 @@
3636
logger = logging.getLogger(__name__)
3737

3838

39+
# ---------------------------------------------------------------------------
40+
# Shared exceptions
41+
# ---------------------------------------------------------------------------
42+
43+
class ManagedSandboxRequired(RuntimeError):
44+
"""Raised when package installation is attempted without proper sandboxing.
45+
46+
This exception is raised when `LocalManagedAgent` is configured with packages
47+
but no compute provider is specified, creating a security risk where packages
48+
would be installed on the host system.
49+
50+
To fix this error, either:
51+
1. Specify a compute provider: `LocalManagedAgent(compute="docker", ...)`
52+
2. Explicitly allow host packages: `LocalManagedConfig(host_packages_ok=True)`
53+
"""
54+
pass
55+
56+
3957
# ---------------------------------------------------------------------------
4058
# ManagedConfig — Anthropic-specific configuration dataclass
4159
# Lives in the Wrapper (not Core SDK) because its fields map directly to

0 commit comments

Comments
 (0)