-
Notifications
You must be signed in to change notification settings - Fork 0
π‘οΈ Sentinel: [CRITICAL] Fix environment leakage and sandbox bypass in unit test gate #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/bootstrap-scaffold
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |||||||||||||
| import subprocess | ||||||||||||||
| import sys | ||||||||||||||
| import tempfile | ||||||||||||||
| import textwrap | ||||||||||||||
| from typing import Any, Dict, List, Tuple | ||||||||||||||
|
|
||||||||||||||
| # ============================================================================= | ||||||||||||||
|
|
@@ -67,17 +68,20 @@ | |||||||||||||
| # TUNABLE: Add more dangerous patterns to block | ||||||||||||||
| DANGEROUS_PATTERNS = [ | ||||||||||||||
| # Dangerous imports (including comma-separated and aliased) | ||||||||||||||
| r"\bimport\s+[^#\n]*\b(os|subprocess|sys|shutil|socket|requests|urllib|pathlib|pickle|pty|code|bdb|pdb|multiprocessing|threading|tempfile|ftplib|smtplib|telnetlib|http|xmlrpc)\b", | ||||||||||||||
| r"\bfrom\s+(os|subprocess|sys|shutil|socket|requests|urllib|pathlib|pickle|pty|code|bdb|pdb|multiprocessing|threading|tempfile|ftplib|smtplib|telnetlib|http|xmlrpc)\b", | ||||||||||||||
| # Dangerous built-ins | ||||||||||||||
| r"\bimport\s+[^#\n]*\b(os|subprocess|sys|shutil|socket|requests|urllib|pathlib|pickle|pty|code|bdb|pdb|multiprocessing|threading|tempfile|ftplib|smtplib|telnetlib|http|xmlrpc|importlib|pkgutil|pydoc)\b", | ||||||||||||||
| r"\bfrom\s+(os|subprocess|sys|shutil|socket|requests|urllib|pathlib|pickle|pty|code|bdb|pdb|multiprocessing|threading|tempfile|ftplib|smtplib|telnetlib|http|xmlrpc|importlib|pkgutil|pydoc)\b", | ||||||||||||||
| # Dangerous built-ins and internal attributes | ||||||||||||||
| r"\beval\s*\(", | ||||||||||||||
| r"\bexec\s*\(", | ||||||||||||||
| r"\b__import__\s*\(", | ||||||||||||||
| r"\bgetattr\s*\(", | ||||||||||||||
| r"\bsetattr\s*\(", | ||||||||||||||
| r"\bbreakpoint\s*\(", | ||||||||||||||
| r"\b__builtins__\b", | ||||||||||||||
| r"\b__globals__\b", | ||||||||||||||
| r"\b__subclasses__\b", | ||||||||||||||
| # Dangerous module functions | ||||||||||||||
| r"\bos\.(system|popen|spawn|remove|unlink|rmdir|mkdir|chmod|chown|kill|exec|fork|pipe)\b", | ||||||||||||||
| r"\bos\.(system|popen|spawn|remove|unlink|rmdir|mkdir|chmod|chown|kill|exec|fork|pipe|environ)\b", | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regex for dangerous
Suggested change
|
||||||||||||||
| r"\bsubprocess\.(run|call|check_call|check_output|Popen)\b", | ||||||||||||||
| r"\bshutil\.(rmtree|move|copy|copy2|copyfile|copymode|copystat|chown)\b", | ||||||||||||||
| r"\bpickle\.(load|loads)\b", | ||||||||||||||
|
|
@@ -214,6 +218,10 @@ def test_python_code(code: str, temp_dir: str, execution_timeout: int = 5) -> Tu | |||||||||||||
| test_file = os.path.join(temp_dir, "test_code.py") | ||||||||||||||
|
|
||||||||||||||
| # Wrap code to capture output safely | ||||||||||||||
| # SECURITY: Indent the user code correctly before inserting into template | ||||||||||||||
| # to avoid SyntaxError and ensure proper execution. | ||||||||||||||
| indented_code = textwrap.indent(code, " ") | ||||||||||||||
|
|
||||||||||||||
| wrapped_code = f""" | ||||||||||||||
| import sys | ||||||||||||||
| import io | ||||||||||||||
|
|
@@ -229,7 +237,7 @@ def test_python_code(code: str, temp_dir: str, execution_timeout: int = 5) -> Tu | |||||||||||||
| sys.stderr = stderr_capture | ||||||||||||||
|
|
||||||||||||||
| # Execute the user's code | ||||||||||||||
| {code} | ||||||||||||||
| {indented_code} | ||||||||||||||
|
|
||||||||||||||
| sys.stdout = original_stdout | ||||||||||||||
| sys.stderr = original_stderr | ||||||||||||||
|
|
@@ -257,13 +265,19 @@ def test_python_code(code: str, temp_dir: str, execution_timeout: int = 5) -> Tu | |||||||||||||
|
|
||||||||||||||
| # Try to execute with timeout | ||||||||||||||
| try: | ||||||||||||||
| # SECURITY: Filter environment to prevent secret leakage from parent process. | ||||||||||||||
| # Only allow essential variables for execution. | ||||||||||||||
| allowed_env_keys = {"PATH", "PYTHONPATH", "LANG", "PYTHONIOENCODING"} | ||||||||||||||
| allowed_env = {k: os.environ[k] for k in allowed_env_keys if k in os.environ} | ||||||||||||||
| allowed_env["PYTHONPATH"] = temp_dir | ||||||||||||||
|
Comment on lines
+270
to
+272
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Including
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| result = subprocess.run( | ||||||||||||||
| [sys.executable, test_file], | ||||||||||||||
| capture_output=True, | ||||||||||||||
| text=True, | ||||||||||||||
| timeout=execution_timeout, | ||||||||||||||
| cwd=temp_dir, | ||||||||||||||
| env={**os.environ, "PYTHONPATH": temp_dir}, | ||||||||||||||
| env=allowed_env, | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| stdout = result.stdout | ||||||||||||||
|
|
@@ -367,7 +381,8 @@ def load_jsonl(path: str) -> List[Dict[str, Any]]: | |||||||||||||
|
|
||||||||||||||
| def save_jsonl(samples: List[Dict[str, Any]], path: str) -> None: | ||||||||||||||
| """Save samples to JSONL file.""" | ||||||||||||||
| os.makedirs(os.path.dirname(path), exist_ok=True) | ||||||||||||||
| if os.path.dirname(path): | ||||||||||||||
| os.makedirs(os.path.dirname(path), exist_ok=True) | ||||||||||||||
|
|
||||||||||||||
| with open(path, "w") as f: | ||||||||||||||
| for sample in samples: | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list of dangerous imports should include
builtins. While you have blocked access to the__builtins__attribute, an attacker could still useimport builtinsto access sensitive functions likeevalorexecif they can bypass the other regex patterns (e.g., viabuiltins.__dict__).