Skip to content

Commit a41fcaf

Browse files
fix: resolve wrapper layer security gaps (fixes #1962)
- Add consistent is_protected() checks to search_replace.py and append_to_file() - Make execute_command() enforce DANGEROUS_COMMANDS with allow_dangerous parameter - Remove duplicate AgentOps init from _select_autogen_version() - Replace eager agentops import with lazy framework availability check - Add threading lock and persistent file handle to AuditLogHook for thread safety - Fix enable_injection_defense() to return both hook IDs for proper cleanup Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
1 parent 1ad58ca commit a41fcaf

7 files changed

Lines changed: 75 additions & 20 deletions

File tree

src/praisonai/praisonai/agents_generator.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -495,15 +495,6 @@ def _select_autogen_version(self, framework, config):
495495

496496
framework = "autogen_v4" if use_v4 else "autogen"
497497

498-
# Initialize AgentOps if configured
499-
agentops_api_key = os.getenv("AGENTOPS_API_KEY")
500-
if agentops_api_key:
501-
try:
502-
import agentops
503-
agentops.init(agentops_api_key, default_tags=[framework])
504-
except ImportError:
505-
pass
506-
507498
return framework
508499

509500
def _validate_cli_backend_compatibility(self, config, framework):

src/praisonai/praisonai/code/tools/execute_command.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ def execute_command(
7171
capture_output: bool = True,
7272
shell: bool = False,
7373
env: Optional[Dict[str, str]] = None,
74+
allow_dangerous: bool = False,
7475
) -> Dict[str, Any]:
7576
"""
7677
Execute a shell command and return the result.
@@ -85,6 +86,7 @@ def execute_command(
8586
capture_output: Whether to capture stdout/stderr
8687
shell: DEPRECATED - shell=True is no longer supported for security reasons
8788
env: Additional environment variables
89+
allow_dangerous: Allow execution of known dangerous commands
8890
8991
Returns:
9092
Dictionary with:
@@ -120,6 +122,28 @@ def execute_command(
120122
'stderr': '',
121123
}
122124

125+
# Check if command is dangerous using the existing safety classifier
126+
if not allow_dangerous:
127+
try:
128+
parts = shlex.split(command)
129+
if parts:
130+
base_cmd = os.path.basename(parts[0]).lower()
131+
if base_cmd in DANGEROUS_COMMANDS:
132+
return {
133+
'success': False,
134+
'error': (
135+
f"Command '{base_cmd}' is in DANGEROUS_COMMANDS; "
136+
f"pass allow_dangerous=True to override."
137+
),
138+
'command': command,
139+
'exit_code': -1,
140+
'stdout': '',
141+
'stderr': '',
142+
}
143+
except ValueError:
144+
# If we can't parse the command, let it proceed and let later validation catch it
145+
pass
146+
123147
# Resolve working directory
124148
if cwd:
125149
if workspace and not os.path.isabs(cwd):

src/praisonai/praisonai/code/tools/search_replace.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
file_exists,
1414
is_path_within_directory,
1515
)
16+
from ...security.protected import is_protected, get_protection_reason
1617

1718

1819
def search_replace(
@@ -61,6 +62,17 @@ def search_replace(
6162
else:
6263
abs_path = os.path.abspath(path)
6364

65+
# Protected path check — never allow modification of system files
66+
if is_protected(abs_path):
67+
reason = get_protection_reason(abs_path) or "Protected system file"
68+
return {
69+
'success': False,
70+
'error': f"Path '{path}' is protected: {reason}",
71+
'path': path,
72+
'operations_applied': 0,
73+
'total_replacements': 0,
74+
}
75+
6476
# Security check
6577
if workspace:
6678
if not is_path_within_directory(abs_path, workspace):

src/praisonai/praisonai/code/tools/write_file.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ def append_to_file(
183183
else:
184184
abs_path = os.path.abspath(path)
185185

186+
# Protected path check — never allow modification of system files
187+
if is_protected(abs_path):
188+
reason = get_protection_reason(abs_path) or "Protected system file"
189+
return {
190+
'success': False,
191+
'error': f"Path '{path}' is protected: {reason}",
192+
'path': path,
193+
}
194+
186195
# Security check
187196
if workspace:
188197
if not is_path_within_directory(abs_path, workspace):

src/praisonai/praisonai/observability/hooks.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,7 @@ def _end_agentops(status: str) -> None:
7575
logger.warning("agentops.end_session failed: %s", e)
7676

7777

78-
# Constants for checking availability
79-
try:
80-
import agentops
81-
AGENTOPS_AVAILABLE = True
82-
except ImportError:
83-
AGENTOPS_AVAILABLE = False
78+
def is_agentops_available() -> bool:
79+
"""Lazy check — does not import agentops at module load time."""
80+
from .._framework_availability import is_available
81+
return is_available("agentops")

src/praisonai/praisonai/security/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ def enable_injection_defense(
100100
trusted_sources: Source names that bypass blocking (in addition to defaults).
101101
102102
Returns:
103-
Hook ID string (can be used to remove the hook later).
103+
Dictionary with hook IDs: {"before_tool": str, "before_agent": str}
104104
105105
Example:
106106
>>> from praisonai.security import enable_injection_defense
@@ -119,9 +119,9 @@ def enable_injection_defense(
119119

120120
# Register both before_tool and before_agent hooks
121121
tool_hook_id = add_hook("before_tool", defense.create_hook())
122-
add_hook("before_agent", defense.create_agent_hook())
122+
agent_hook_id = add_hook("before_agent", defense.create_agent_hook())
123123

124-
return tool_hook_id
124+
return {"before_tool": tool_hook_id, "before_agent": agent_hook_id}
125125

126126

127127
def enable_audit_log(

src/praisonai/praisonai/security/audit.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import json
1010
import logging
1111
import os
12+
import threading
1213
from datetime import datetime, timezone
1314
from pathlib import Path
1415
from typing import Any, Callable, Optional
@@ -51,6 +52,9 @@ def __init__(
5152
self._include_output = include_output
5253
self._max_output_chars = max_output_chars
5354
self._ensure_dir()
55+
self._lock = threading.Lock()
56+
# Single long-lived handle; reopened lazily if it gets rotated out.
57+
self._fh = None
5458

5559
def _ensure_dir(self) -> None:
5660
"""Create parent directory if it doesn't exist."""
@@ -59,12 +63,29 @@ def _ensure_dir(self) -> None:
5963

6064
def _write(self, entry: dict) -> None:
6165
"""Append a JSON line to the audit log."""
66+
line = json.dumps(entry, default=str) + "\n"
6267
try:
63-
with open(self._log_path, "a", encoding="utf-8") as f:
64-
f.write(json.dumps(entry, default=str) + "\n")
68+
with self._lock:
69+
# Lazy initialize file handle
70+
if self._fh is None:
71+
self._fh = open(self._log_path, "a", encoding="utf-8")
72+
self._fh.write(line)
73+
self._fh.flush()
74+
os.fsync(self._fh.fileno()) # optional, for crash-durability
6575
except OSError as e:
6676
logger.error("[praisonai.security.audit] Failed to write audit log: %s", e)
6777

78+
def close(self) -> None:
79+
"""Close the audit log file handle."""
80+
with self._lock:
81+
if self._fh is not None:
82+
try:
83+
self._fh.close()
84+
except OSError:
85+
pass
86+
finally:
87+
self._fh = None
88+
6889
def create_after_tool_hook(self) -> Callable:
6990
"""
7091
Create an AFTER_TOOL hook function.

0 commit comments

Comments
 (0)