fix: critical security vulnerabilities in tools #740
Conversation
- Fixed command injection in shell_tools.py by forcing shell=False - Implemented sandboxed execution in python_tools.py with restricted builtins - Fixed SQL injection in duckdb_tools.py using parameterized queries - Added path traversal protection in file_tools.py - Added SSRF protection in spider_tools.py with URL validation These changes address the critical security issues identified in issue #737 Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
WalkthroughSecurity enhancements were made across multiple utility modules. SQL identifier validation and parameterized queries were added to DuckDB tools. File operations now validate paths to prevent traversal. Python code execution restricts builtins and scans for dangerous patterns. Shell command execution enforces no shell usage. Web scraping now validates URLs to block SSRF vectors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DuckDBTools
User->>DuckDBTools: load_csv(table_name, ...)
DuckDBTools->>DuckDBTools: _validate_identifier(table_name)
DuckDBTools->>DuckDBTools: Validate/quote identifiers
DuckDBTools->>DuckDBTools: Use parameterized queries
DuckDBTools-->>User: Result
sequenceDiagram
participant User
participant FileTools
User->>FileTools: read_file(filepath)
FileTools->>FileTools: _validate_path(filepath)
FileTools->>FileTools: Normalize and check path
FileTools->>FileTools: Perform file operation
FileTools-->>User: File content or error
sequenceDiagram
participant User
participant PythonTools
User->>PythonTools: execute_code(code)
PythonTools->>PythonTools: Scan code for dangerous patterns
alt Dangerous pattern found
PythonTools-->>User: Security error
else
PythonTools->>PythonTools: Setup safe_builtins
PythonTools->>PythonTools: Execute code in safe context
PythonTools-->>User: Execution result
end
sequenceDiagram
participant User
participant ShellTools
User->>ShellTools: execute_command(command)
ShellTools->>ShellTools: shlex.split(command)
ShellTools->>ShellTools: subprocess.run(..., shell=False)
ShellTools-->>User: Command output
sequenceDiagram
participant User
participant SpiderTools
User->>SpiderTools: scrape_page(url)
SpiderTools->>SpiderTools: _validate_url(url)
alt URL invalid
SpiderTools-->>User: Error (invalid/potentially dangerous URL)
else
SpiderTools->>SpiderTools: Perform HTTP request and scrape
SpiderTools-->>User: Scraped data
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @MervinPraison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on addressing a suite of critical security vulnerabilities across various utility modules. The changes harden the system against common attack vectors such as SQL injection, path traversal, command injection, and SSRF, while also implementing a more secure sandboxed environment for Python code execution. The primary goal is to significantly improve the overall security posture of the application's toolset.
Highlights
- SQL Injection Prevention: Implemented robust SQL injection prevention in
duckdb_tools.pyby introducing a_validate_identifiermethod for sanitizing table and column names, and by utilizing parameterized queries for all data and identifier interactions. - Path Traversal Protection: Added comprehensive path traversal protection in
file_tools.pyby introducing a_validate_pathstatic method. This method normalizes and validates all file and directory paths before any file system operations are performed, preventing access to unauthorized locations. - Sandboxed Python Execution: Enhanced
python_tools.pyto provide a more secure sandboxed execution environment. This was achieved by explicitly defining asafe_builtinsdictionary to restrict dangerous built-in functions and by adding a pre-execution check to scan the input code for known dangerous patterns. - Command Injection Mitigation: Addressed command injection vulnerabilities in
shell_tools.pyby removing theshellparameter from theexecute_commandfunction and explicitly settingshell=Falsein allsubprocess.Popencalls. Commands are now always safely split usingshlex. - SSRF Protection: Introduced Server-Side Request Forgery (SSRF) protection in
spider_tools.pywith the addition of a_validate_urlmethod. This method rigorously checks URLs for safe schemes, valid hostnames, and blocks access to local, private, and suspicious internal network addresses before making any HTTP requests.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Security Check Overly Restrictive, Easily Bypassed
The new security check in execute_code uses simple substring matching for dangerous patterns, making it both overly restrictive and easily bypassable. It causes false positives by rejecting legitimate code containing patterns like 'import ', 'from ', or 'open(' in comments, strings, or variable names (e.g., "important_data", "reopen(file)"). This exact string matching is also easily circumvented by using alternative whitespace (e.g., import\tos) or formatting, rendering the security ineffective against determined attackers.
src/praisonai-agents/praisonaiagents/tools/python_tools.py#L119-L135
PraisonAI/src/praisonai-agents/praisonaiagents/tools/python_tools.py
Lines 119 to 135 in abe4d6c
Bug: Path Validation Flaws Enable Traversal
The _validate_path method, intended for path traversal prevention, has two flaws:
- The
if '..' in filepathcheck is overly broad, incorrectly rejecting legitimate filenames containing '..' (e.g., "file..txt") instead of specifically checking for path traversal sequences like../or..\\. - It only validates the original input path for suspicious patterns. While it converts the path to an absolute path, it critically fails to validate that this resolved absolute path remains within an expected safe directory. This allows an attacker to bypass the
..check by providing an absolute path (e.g.,/etc/passwd), leading to a path traversal vulnerability.
src/praisonai-agents/praisonaiagents/tools/file_tools.py#L24-L48
PraisonAI/src/praisonai-agents/praisonaiagents/tools/file_tools.py
Lines 24 to 48 in abe4d6c
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Code Review
This pull request introduces security fixes addressing command injection, SQL injection, path traversal, SSRF, and unsafe code execution vulnerabilities. The path traversal fix in file_tools.py still allows access to arbitrary files via absolute paths. The SSRF protection in spider_tools.py is vulnerable to bypass via HTTP redirects. The SQL type validation in duckdb_tools.py is a bit too strict and might reject valid types.
| if not re.match(r'^[A-Z][A-Z0-9_]*(\([0-9,]+\))?$', col_type.upper()): | ||
| raise ValueError(f"Invalid column type: {col_type}") |
There was a problem hiding this comment.
The regex for validating column types is too restrictive. It doesn't allow for spaces within the parentheses, which is common for types like DECIMAL(10, 2). This will cause a ValueError for valid schema definitions.
The regex should be updated to accommodate optional whitespace.
if not re.match(r'^[A-Z][A-Z0-9_]*(\([0-9,\s]+\))?$', col_type.upper()):
raise ValueError(f"Invalid column type: {col_type}")| # Normalize the path | ||
| normalized = os.path.normpath(filepath) | ||
| absolute = os.path.abspath(normalized) | ||
|
|
||
| # Check for suspicious patterns | ||
| if '..' in filepath or filepath.startswith('~'): | ||
| raise ValueError(f"Suspicious path pattern detected: {filepath}") | ||
|
|
||
| # Additional check: ensure the resolved path doesn't escape expected boundaries | ||
| # This is a basic check - in production, you'd want to define allowed directories | ||
| return absolute |
There was a problem hiding this comment.
The current path validation logic in _validate_path is an improvement but is not sufficient to prevent all path traversal attacks. An attacker can still access any file on the filesystem by providing an absolute path (e.g., /etc/passwd), as the check only looks for .. and ~ in the path string.
A more robust approach is to define a safe base directory and ensure that the resolved absolute path of any file operation is within that directory. This would prevent the tool from accessing unintended files outside its designated workspace.
agent_workspace = os.path.abspath(os.getcwd())
if os.path.isabs(filepath):
raise ValueError(f"Absolute paths are not permitted: {filepath}")
full_path = os.path.normpath(os.path.join(agent_workspace, filepath))
if not full_path.startswith(agent_workspace):
raise ValueError(f"Path traversal attempt detected: {filepath}")
return full_path| # Validate URL to prevent SSRF | ||
| if not self._validate_url(url): | ||
| return {"error": f"Invalid or potentially dangerous URL: {url}"} |
There was a problem hiding this comment.
The URL validation is a great step towards preventing Server-Side Request Forgery (SSRF). However, it can be bypassed if the initial, valid URL returns an HTTP redirect to a malicious or internal URL. The requests library follows redirects by default, and the validation is not performed on subsequent URLs in a redirect chain.
To fix this, you should disable automatic redirects in your session.get() call (allow_redirects=False) and handle them manually in a loop, validating each new URL before making the next request. You should also limit the number of redirects to prevent infinite loops.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/praisonai-agents/praisonaiagents/tools/spider_tools.py (1)
73-76: Simplify the return statement.The static analysis tool correctly identified that this can be simplified by returning the negated condition directly.
Apply this simplification:
- # Block metadata service endpoints - if hostname in ['169.254.169.254', 'metadata.google.internal']: - return False - - return True + # Block metadata service endpoints + return hostname not in ['169.254.169.254', 'metadata.google.internal']src/praisonai-agents/praisonaiagents/tools/duckdb_tools.py (1)
173-174: Column type validation may be too restrictive.The regex pattern for column types is very strict and might reject valid DuckDB types like
DECIMAL(10,2)orVARCHAR(255).Consider a more flexible pattern:
- if not re.match(r'^[A-Z][A-Z0-9_]*(\([0-9,]+\))?$', col_type.upper()): + if not re.match(r'^[A-Z][A-Z0-9_]*(\([0-9,\s]+\))?(\s+[A-Z]+)*$', col_type.upper()):This allows for spaces in precision/scale and additional type modifiers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/praisonai-agents/praisonaiagents/tools/duckdb_tools.py(3 hunks)src/praisonai-agents/praisonaiagents/tools/file_tools.py(8 hunks)src/praisonai-agents/praisonaiagents/tools/python_tools.py(1 hunks)src/praisonai-agents/praisonaiagents/tools/shell_tools.py(2 hunks)src/praisonai-agents/praisonaiagents/tools/spider_tools.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
src/praisonai-agents/praisonaiagents/tools/spider_tools.py
73-76: Return the negated condition directly
Inline condition
(SIM103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor BugBot
- GitHub Check: Run tests and collect coverage
- GitHub Check: test-core (3.11)
- GitHub Check: quick-test
🔇 Additional comments (9)
src/praisonai-agents/praisonaiagents/tools/shell_tools.py (2)
59-66: LGTM! Excellent security improvement.The platform-specific command splitting using
shlex.splitwith appropriateposixflags is well-implemented. This correctly handles quote parsing differences between Windows and Unix-like systems while preventing command injection.
79-79: Security enhancement implemented correctly.Hardcoding
shell=Falseeffectively prevents command injection vulnerabilities by ensuring commands are never executed through a shell interpreter.src/praisonai-agents/praisonaiagents/tools/file_tools.py (1)
63-65: Path validation integration looks good.The integration of path validation into file operations is correctly implemented and will effectively prevent path traversal attacks when the validation logic is fixed.
src/praisonai-agents/praisonaiagents/tools/spider_tools.py (2)
30-79: Excellent SSRF protection implementation.The URL validation covers comprehensive attack vectors including private IP ranges, localhost variants, metadata services, and internal domains. This significantly enhances security against SSRF attacks.
124-127: URL validation integration is secure.The early validation check in
scrape_pageeffectively prevents SSRF attacks by blocking dangerous URLs before any network requests.src/praisonai-agents/praisonaiagents/tools/python_tools.py (2)
52-107: Sandboxing implementation provides good baseline security.The restricted builtins dictionary effectively limits available functions and disables dangerous operations. The explicit None assignments for dangerous functions prevent their use.
153-157: Security concern: eval() usage in sandboxed environment.The use of
eval()for extracting expression values could potentially be exploited, even with restricted builtins.Consider replacing this with safer alternatives or additional validation:
- result = eval( - compile(ast.Expression(tree.body[-1].value), '<string>', 'eval'), - globals_dict, - locals_dict - ) + # Safer approach: get the value from locals after execution + if hasattr(tree.body[-1].value, 'id'): + var_name = tree.body[-1].value.id + result = locals_dict.get(var_name) + else: + result = Nonesrc/praisonai-agents/praisonaiagents/tools/duckdb_tools.py (2)
34-51: SQL identifier validation implementation is secure.The regex pattern correctly validates identifier format and the quoting approach safely handles reserved words and special characters.
186-191: Parameterized queries effectively prevent SQL injection.The use of parameterized queries for file paths and validated identifiers for table names provides robust protection against SQL injection attacks.
| # Additional check: ensure the resolved path doesn't escape expected boundaries | ||
| # This is a basic check - in production, you'd want to define allowed directories | ||
| return absolute |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance boundary validation for production use.
The current implementation lacks enforcement of allowed directory boundaries, which is critical for preventing unauthorized file access.
Consider implementing a more robust boundary check:
+ # Define allowed base directories (configure as needed)
+ allowed_bases = [os.getcwd(), '/tmp', '/var/tmp'] # Example
+
# Additional check: ensure the resolved path doesn't escape expected boundaries
- # This is a basic check - in production, you'd want to define allowed directories
+ if not any(absolute.startswith(base) for base in allowed_bases):
+ raise ValueError(f"Path outside allowed directories: {filepath}")
+
return absolute📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Additional check: ensure the resolved path doesn't escape expected boundaries | |
| # This is a basic check - in production, you'd want to define allowed directories | |
| return absolute | |
| # Define allowed base directories (configure as needed) | |
| allowed_bases = [os.getcwd(), '/tmp', '/var/tmp'] # Example | |
| # Additional check: ensure the resolved path doesn't escape expected boundaries | |
| if not any(absolute.startswith(base) for base in allowed_bases): | |
| raise ValueError(f"Path outside allowed directories: {filepath}") | |
| return absolute |
🤖 Prompt for AI Agents
In src/praisonai-agents/praisonaiagents/tools/file_tools.py around lines 46 to
48, the current code returns the absolute path without verifying if it resides
within allowed directories. To fix this, implement a check that compares the
resolved absolute path against a predefined list of allowed base directories and
only return the path if it is within these boundaries; otherwise, raise an
exception or handle the error to prevent unauthorized file access.
| # Check for suspicious patterns | ||
| if '..' in filepath or filepath.startswith('~'): | ||
| raise ValueError(f"Suspicious path pattern detected: {filepath}") |
There was a problem hiding this comment.
Fix path validation logic to prevent bypass.
The suspicious pattern check is performed on the original filepath before normalization, which could miss path traversal attempts that become apparent only after normalization.
Apply this fix to check patterns after normalization:
- # Check for suspicious patterns
- if '..' in filepath or filepath.startswith('~'):
- raise ValueError(f"Suspicious path pattern detected: {filepath}")
+ # Check for suspicious patterns in normalized path
+ if '..' in normalized or '~' in normalized:
+ raise ValueError(f"Suspicious path pattern detected: {filepath}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Check for suspicious patterns | |
| if '..' in filepath or filepath.startswith('~'): | |
| raise ValueError(f"Suspicious path pattern detected: {filepath}") | |
| # Check for suspicious patterns in normalized path | |
| if '..' in normalized or '~' in normalized: | |
| raise ValueError(f"Suspicious path pattern detected: {filepath}") |
🤖 Prompt for AI Agents
In src/praisonai-agents/praisonaiagents/tools/file_tools.py around lines 42 to
44, the path validation checks for suspicious patterns on the original filepath
before normalization, which can be bypassed. To fix this, normalize the filepath
first and then perform the checks for '..' or '~' patterns on the normalized
path to ensure path traversal attempts are detected reliably.
| dangerous_patterns = [ | ||
| '__import__', 'import ', 'from ', 'exec', 'eval', | ||
| 'compile', 'open(', 'file(', 'input(', 'raw_input', | ||
| '__subclasses__', '__bases__', '__globals__', '__code__', | ||
| '__class__', 'globals(', 'locals(', 'vars(' | ||
| ] | ||
|
|
||
| code_lower = code.lower() | ||
| for pattern in dangerous_patterns: | ||
| if pattern.lower() in code_lower: | ||
| return { | ||
| 'result': None, | ||
| 'stdout': '', | ||
| 'stderr': f'Security Error: Code contains restricted pattern: {pattern}', | ||
| 'success': False | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve dangerous pattern detection accuracy.
The current pattern detection has some issues that could lead to false positives or bypasses:
- Mixed case sensitivity handling
- Overly broad patterns like 'from ' that could block legitimate code
- Missing some potential bypass patterns
Consider this more precise pattern detection:
- dangerous_patterns = [
- '__import__', 'import ', 'from ', 'exec', 'eval',
- 'compile', 'open(', 'file(', 'input(', 'raw_input',
- '__subclasses__', '__bases__', '__globals__', '__code__',
- '__class__', 'globals(', 'locals(', 'vars('
- ]
-
- code_lower = code.lower()
- for pattern in dangerous_patterns:
- if pattern.lower() in code_lower:
+ # Use more precise regex patterns to reduce false positives
+ dangerous_patterns = [
+ r'\b__import__\b',
+ r'\bimport\s+\w+',
+ r'\bfrom\s+\w+\s+import\b',
+ r'\bexec\s*\(',
+ r'\beval\s*\(',
+ r'\bcompile\s*\(',
+ r'\bopen\s*\(',
+ r'\bglobals\s*\(',
+ r'\blocals\s*\(',
+ r'__\w+__' # Dunder attributes
+ ]
+
+ import re
+ for pattern in dangerous_patterns:
+ if re.search(pattern, code, re.IGNORECASE):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dangerous_patterns = [ | |
| '__import__', 'import ', 'from ', 'exec', 'eval', | |
| 'compile', 'open(', 'file(', 'input(', 'raw_input', | |
| '__subclasses__', '__bases__', '__globals__', '__code__', | |
| '__class__', 'globals(', 'locals(', 'vars(' | |
| ] | |
| code_lower = code.lower() | |
| for pattern in dangerous_patterns: | |
| if pattern.lower() in code_lower: | |
| return { | |
| 'result': None, | |
| 'stdout': '', | |
| 'stderr': f'Security Error: Code contains restricted pattern: {pattern}', | |
| 'success': False | |
| } | |
| # Use more precise regex patterns to reduce false positives | |
| dangerous_patterns = [ | |
| r'\b__import__\b', | |
| r'\bimport\s+\w+', | |
| r'\bfrom\s+\w+\s+import\b', | |
| r'\bexec\s*\(', | |
| r'\beval\s*\(', | |
| r'\bcompile\s*\(', | |
| r'\bopen\s*\(', | |
| r'\bglobals\s*\(', | |
| r'\blocals\s*\(', | |
| r'__\w+__' # Dunder attributes | |
| ] | |
| import re | |
| for pattern in dangerous_patterns: | |
| if re.search(pattern, code, re.IGNORECASE): | |
| return { | |
| 'result': None, | |
| 'stdout': '', | |
| 'stderr': f'Security Error: Code contains restricted pattern: {pattern}', | |
| 'success': False | |
| } |
🤖 Prompt for AI Agents
In src/praisonai-agents/praisonaiagents/tools/python_tools.py around lines 120
to 135, improve the dangerous pattern detection by making it case-sensitive and
more precise to avoid false positives and bypasses. Replace broad patterns like
'from ' with more specific ones, and add additional known bypass patterns. Use
exact substring matching or regex with word boundaries instead of simple
lowercase containment checks to increase accuracy.
| exists = conn.execute(""" | ||
| SELECT name FROM sqlite_master | ||
| WHERE type='table' AND name='{table_name}' | ||
| """).fetchone() is not None | ||
| WHERE type='table' AND name=? | ||
| """, [table_name]).fetchone() is not None |
There was a problem hiding this comment.
Fix table existence check for DuckDB.
The query uses sqlite_master which is SQLite-specific, but this is a DuckDB tool. DuckDB uses different system tables.
Apply this fix for DuckDB compatibility:
- exists = conn.execute("""
- SELECT name FROM sqlite_master
- WHERE type='table' AND name=?
- """, [table_name]).fetchone() is not None
+ exists = conn.execute("""
+ SELECT table_name FROM information_schema.tables
+ WHERE table_name=?
+ """, [table_name]).fetchone() is not None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exists = conn.execute(""" | |
| SELECT name FROM sqlite_master | |
| WHERE type='table' AND name='{table_name}' | |
| """).fetchone() is not None | |
| WHERE type='table' AND name=? | |
| """, [table_name]).fetchone() is not None | |
| exists = conn.execute(""" | |
| SELECT table_name FROM information_schema.tables | |
| WHERE table_name=? | |
| """, [table_name]).fetchone() is not None |
🤖 Prompt for AI Agents
In src/praisonai-agents/praisonaiagents/tools/duckdb_tools.py around lines 149
to 152, the table existence check uses a SQLite-specific query on sqlite_master,
which is incompatible with DuckDB. Replace this query with a DuckDB-compatible
one by querying the information_schema.tables or the DuckDB system catalog to
check if the table exists. Adjust the SQL to use DuckDB's metadata tables and
ensure the parameterized query checks for the table name correctly.
User description
These changes address the critical security issues identified in issue #737
PR Type
Bug fix
Description
Fixed critical security vulnerabilities across multiple tool modules
Implemented SQL injection prevention with parameterized queries
Added path traversal protection with input validation
Enforced sandboxed Python execution with restricted builtins
Prevented SSRF attacks with URL validation
Disabled shell execution to prevent command injection
Changes diagram
Changes walkthrough 📝
duckdb_tools.py
SQL injection prevention with parameterized queriessrc/praisonai-agents/praisonaiagents/tools/duckdb_tools.py
_validate_identifier()method to prevent SQL injectionfile_tools.py
Path traversal protection for file operationssrc/praisonai-agents/praisonaiagents/tools/file_tools.py
_validate_path()method to prevent path traversal attacksmove, delete)
python_tools.py
Sandboxed Python execution with restricted builtinssrc/praisonai-agents/praisonaiagents/tools/python_tools.py
safe_builtinsdictionary with limited functions__import__,eval,exec,openshell_tools.py
Command injection prevention by disabling shellsrc/praisonai-agents/praisonaiagents/tools/shell_tools.py
shellparameter fromexecute_command()methodshell=Falsein subprocess execution for securityshlex.split()to properly parse commandsspider_tools.py
SSRF protection with comprehensive URL validationsrc/praisonai-agents/praisonaiagents/tools/spider_tools.py
_validate_url()method to prevent SSRF attacksSummary by CodeRabbit