Skip to content

Commit d091c16

Browse files
fix: tighten managed local package install safety and restore session compat
Agent-Logs-Url: https://github.com/MervinPraison/PraisonAI/sessions/be07f0d7-3e03-4987-808d-5f87bfcae00b Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>
1 parent 0e86e3b commit d091c16

File tree

2 files changed

+71
-25
lines changed

2 files changed

+71
-25
lines changed

src/praisonai/praisonai/integrations/managed_local.py

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -476,42 +476,42 @@ def _install_packages(self) -> None:
476476
if not pip_pkgs:
477477
return
478478

479+
compute_install_succeeded = False
480+
479481
# If compute provider is available, use it
480482
if self._compute is not None:
481483
# Use sandbox_type from config
482484
sandbox_type = self._cfg.get("sandbox_type", "subprocess")
483485
logger.info("[local_managed] installing packages via compute provider (%s): %s", sandbox_type, pip_pkgs)
484486
try:
485-
import asyncio
486-
# Run async operation in sync context
487-
loop = asyncio.get_event_loop()
488-
if loop.is_running():
489-
# We're in async context, need to run in executor
487+
try:
488+
asyncio.get_running_loop()
489+
# We're in async context, run in a separate thread loop.
490490
import threading
491-
result = [None]
492491
exception = [None]
493-
492+
494493
def run_install():
495494
try:
496-
new_loop = asyncio.new_event_loop()
497-
asyncio.set_event_loop(new_loop)
498-
result[0] = new_loop.run_until_complete(self._install_via_compute(pip_pkgs))
495+
asyncio.run(self._install_via_compute(pip_pkgs))
499496
except Exception as e:
500497
exception[0] = e
501-
finally:
502-
new_loop.close()
503-
498+
504499
thread = threading.Thread(target=run_install)
505500
thread.start()
506501
thread.join()
507-
502+
508503
if exception[0]:
509504
raise exception[0]
510-
else:
511-
loop.run_until_complete(self._install_via_compute(pip_pkgs))
505+
except RuntimeError:
506+
# No running loop in this thread.
507+
asyncio.run(self._install_via_compute(pip_pkgs))
508+
compute_install_succeeded = True
512509
except Exception as e:
513510
logger.warning("[local_managed] compute package install failed: %s", e)
514511
# Fall through to host installation if allowed
512+
513+
if compute_install_succeeded:
514+
return
515515

516516
# Host installation - only if explicitly allowed
517517
if not self._cfg.get("host_packages_ok", False):
@@ -544,7 +544,8 @@ async def _install_via_compute(self, pip_pkgs: List[str]) -> None:
544544
await self.provision_compute(config=config)
545545
else:
546546
# Install on existing instance
547-
cmd = f"pip install -q {' '.join(pip_pkgs)}"
547+
import shlex
548+
cmd = "pip install -q " + " ".join(shlex.quote(pkg) for pkg in pip_pkgs)
548549
await self.execute_in_compute(cmd, timeout=120)
549550

550551
def _ensure_agent(self) -> Any:
@@ -773,29 +774,32 @@ def interrupt(self) -> None:
773774
# ------------------------------------------------------------------
774775
def retrieve_session(self) -> Dict[str, Any]:
775776
"""Retrieve current session metadata using unified SessionInfo schema."""
777+
if not self._session_id:
778+
return {}
779+
776780
self._sync_usage()
777781

778782
# Use unified SessionInfo schema for consistency with Anthropic backend
779783
try:
780784
from praisonaiagents.managed import SessionInfo
781785
session_info = SessionInfo(
782-
id=self._session_id or "",
783-
status="idle" if self._session_id else "none",
786+
id=self._session_id,
787+
status="idle",
784788
usage={
785789
"input_tokens": self.total_input_tokens,
786790
"output_tokens": self.total_output_tokens,
787-
} if self._session_id else None
791+
}
788792
)
789793
return session_info.to_dict()
790794
except ImportError:
791795
# Fallback to old format if SessionInfo not available
792796
return {
793-
"id": self._session_id or "",
794-
"status": "idle" if self._session_id else "none",
797+
"id": self._session_id,
798+
"status": "idle",
795799
"usage": {
796800
"input_tokens": self.total_input_tokens,
797801
"output_tokens": self.total_output_tokens,
798-
} if self._session_id else None,
802+
},
799803
}
800804

801805
def list_sessions(self, **kwargs) -> List[Dict[str, Any]]:

src/praisonai/tests/unit/integrations/test_managed_agents.py

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"""
88

99
import pytest
10-
from unittest.mock import Mock, patch, MagicMock
10+
from unittest.mock import Mock, patch, MagicMock, AsyncMock
1111

1212

1313
def test_managed_config_dataclass():
@@ -298,6 +298,48 @@ def test_backward_compatible_aliases():
298298
assert ManagedBackendConfig == ManagedConfig
299299

300300

301+
def test_local_retrieve_session_no_session_returns_empty_dict():
302+
"""Local backend should preserve empty-session behavior."""
303+
from praisonai.integrations.managed_local import LocalManagedAgent
304+
305+
managed = LocalManagedAgent()
306+
assert managed.retrieve_session() == {}
307+
308+
309+
@patch("praisonai.integrations.managed_local.subprocess.run")
310+
def test_local_install_packages_prefers_compute_and_skips_host(mock_subprocess_run):
311+
"""Successful compute install must not fall through to host install."""
312+
from praisonai.integrations.managed_local import LocalManagedAgent, LocalManagedConfig
313+
314+
managed = LocalManagedAgent(
315+
config=LocalManagedConfig(packages={"pip": ["requests"]})
316+
)
317+
managed._compute = object()
318+
managed._install_via_compute = AsyncMock(return_value=None)
319+
320+
managed._install_packages()
321+
322+
managed._install_via_compute.assert_awaited_once_with(["requests"])
323+
mock_subprocess_run.assert_not_called()
324+
325+
326+
@pytest.mark.asyncio
327+
async def test_local_install_via_compute_quotes_package_names():
328+
"""Package names passed through shell command should be shell-escaped."""
329+
from praisonai.integrations.managed_local import LocalManagedAgent
330+
331+
managed = LocalManagedAgent()
332+
managed._compute_instance_id = "inst_123"
333+
managed.execute_in_compute = AsyncMock(return_value={"stdout": "", "stderr": "", "exit_code": 0})
334+
335+
await managed._install_via_compute(["requests", "bad;echo pwned"])
336+
337+
managed.execute_in_compute.assert_awaited_once()
338+
command = managed.execute_in_compute.await_args.args[0]
339+
assert "pip install -q " in command
340+
assert "'bad;echo pwned'" in command
341+
342+
301343
@patch('praisonai.integrations.managed_agents.logger')
302344
def test_logging_integration(mock_logger):
303345
"""Test that managed agents include proper logging."""
@@ -308,4 +350,4 @@ def test_logging_integration(mock_logger):
308350
managed.reset_all()
309351

310352
# Verify logging is available (don't assert specific calls since they may not happen in unit tests)
311-
assert mock_logger is not None
353+
assert mock_logger is not None

0 commit comments

Comments
 (0)