Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/praisonai-agents/praisonaiagents/context/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,9 @@ def _get_output_path(self, agent_name: str = "") -> Path:
# Verify the resolved path stays within the expected parent dir
resolved = result.resolve()
parent_resolved = self.path.parent.resolve()
if not str(resolved).startswith(str(parent_resolved) + '/') and resolved.parent != parent_resolved:
try:
resolved.relative_to(parent_resolved)
except ValueError:
# Fallback to a safe hash-based name
import hashlib
hash_name = hashlib.sha256(agent_name.encode()).hexdigest()[:16]
Expand Down
24 changes: 16 additions & 8 deletions src/praisonai-ts/src/workflows/yaml-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ export interface ParsedWorkflow {
errors: string[];
}

// Whitelist of allowed step keys to prevent injection
const ALLOWED_STEP_KEYS = new Set([
'type', 'agent', 'tool', 'input', 'output', 'condition',
'onError', 'maxRetries', 'timeout', 'loopCondition', 'maxIterations',
]);

/**
* Parse YAML string into workflow definition
*/
Expand Down Expand Up @@ -85,10 +91,6 @@ export function parseYAMLWorkflow(yamlContent: string): YAMLWorkflowDefinition {
};
} else if (currentStep) {
// Step properties — whitelist allowed keys to prevent injection
const ALLOWED_STEP_KEYS = new Set([
'type', 'agent', 'tool', 'input', 'output', 'condition',
'onError', 'maxRetries', 'timeout', 'loopCondition', 'maxIterations',
]);
if (!ALLOWED_STEP_KEYS.has(key)) {
// Ignore unknown keys — do not allow arbitrary property injection
continue;
Expand Down Expand Up @@ -282,27 +284,33 @@ export async function loadWorkflowFromFile(

// SECURITY: Prevent path traversal
const normalizedPath = path.normalize(filePath);
if (normalizedPath.includes('..')) {
throw new Error('Path traversal detected: ".." is not allowed in file paths');
// Check for '..' as path segments (not just substring)
const pathSegments = normalizedPath.split(path.sep);
if (pathSegments.includes('..')) {
throw new Error('Path traversal detected: ".." path segments are not allowed');
}

let effectivePath: string;
// If basePath is specified, ensure resolvedPath stays within it
if (options.basePath) {
const resolvedBase = path.resolve(options.basePath);
const resolvedFile = path.resolve(options.basePath, normalizedPath);
if (!resolvedFile.startsWith(resolvedBase + path.sep) && resolvedFile !== resolvedBase) {
throw new Error(`File path must be within base directory: ${options.basePath}`);
}
effectivePath = resolvedFile;
} else {
effectivePath = path.resolve(normalizedPath);
}

// SECURITY: Enforce file size limit (default 1 MB)
const maxSize = options.maxFileSizeBytes ?? 1_048_576;
const stat = await fs.stat(filePath);
const stat = await fs.stat(effectivePath);
if (stat.size > maxSize) {
throw new Error(`File too large: ${stat.size} bytes exceeds limit of ${maxSize} bytes`);
}

const content = await fs.readFile(filePath, 'utf-8');
const content = await fs.readFile(effectivePath, 'utf-8');
const definition = parseYAMLWorkflow(content);
return createWorkflowFromYAML(definition, agents, tools);
}
Expand Down
28 changes: 17 additions & 11 deletions src/praisonai/praisonai/cli/features/agent_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@
import asyncio
import json
import logging
from typing import Callable, Dict, List, TYPE_CHECKING
from typing import Callable, Dict, List, Optional, TYPE_CHECKING

if TYPE_CHECKING:
from .interactive_runtime import InteractiveRuntime
from .code_intelligence import CodeIntelligenceRouter
from .action_orchestrator import ActionOrchestrator

import os
import re
from pathlib import Path

logger = logging.getLogger(__name__)


def _sanitize_filepath(filepath: str, workspace: str = None) -> str:
def _sanitize_filepath(filepath: str, workspace: Optional[str] = None) -> str:
"""Validate and sanitize a filepath against injection attacks.

Raises ValueError if the path is unsafe.
Expand All @@ -45,9 +45,11 @@ def _sanitize_filepath(filepath: str, workspace: str = None) -> str:

# If we have a workspace, ensure the resolved path stays within it
if workspace:
resolved = os.path.realpath(os.path.join(workspace, normalized))
ws_resolved = os.path.realpath(workspace)
if not resolved.startswith(ws_resolved + os.sep) and resolved != ws_resolved:
resolved = Path(os.path.realpath(os.path.join(workspace, normalized)))
ws_resolved = Path(os.path.realpath(workspace))
try:
resolved.relative_to(ws_resolved)
except ValueError:
raise ValueError(
f"Path {filepath!r} resolves outside workspace {workspace!r}"
)
Expand All @@ -70,9 +72,9 @@ def _sanitize_command(command: str) -> str:
DANGEROUS_PATTERNS = [
'$(', '`', # Command substitution
'&&', '||', # Command chaining
';', # Command separator
'>>', '> ', # Output redirection
'| ', # Pipe
'>>', '>', # Output redirection
'|', ';', '&', # Pipe and separators
'\n', '\r' # Line breaks
]
for pattern in DANGEROUS_PATTERNS:
if pattern in command:
Expand Down Expand Up @@ -513,7 +515,9 @@ def read_file(filepath: str) -> str:
# SECURITY: Ensure resolved path stays within workspace
resolved = path.resolve()
ws_resolved = Path(runtime.config.workspace).resolve()
if not str(resolved).startswith(str(ws_resolved) + os.sep) and resolved != ws_resolved:
try:
resolved.relative_to(ws_resolved)
except ValueError:
return json.dumps({"error": f"Path escapes workspace: {filepath}"})

if not resolved.exists():
Expand Down Expand Up @@ -556,7 +560,9 @@ def list_files(directory: str = ".", pattern: str = "*") -> str:
# SECURITY: Ensure resolved path stays within workspace
resolved = path.resolve()
ws_resolved = Path(runtime.config.workspace).resolve()
if not str(resolved).startswith(str(ws_resolved) + os.sep) and resolved != ws_resolved:
try:
resolved.relative_to(ws_resolved)
except ValueError:
return json.dumps({"error": f"Directory escapes workspace: {directory}"})
Comment on lines 560 to 566
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list_files only verifies that the directory resolves within the workspace, but then iterates path.glob() and calls f.stat() on each entry. If a directory contains symlinks to locations outside the workspace, f.stat() will follow the symlink and can leak metadata (and potentially enable later confusion about what is “in” the workspace). Consider resolving each f (or skipping symlinks) and enforcing f.resolve().relative_to(ws_resolved) before including it, and/or using lstat semantics for size collection.

Copilot uses AI. Check for mistakes.

if not resolved.exists():
Expand Down
Loading