fix: close two security analyzer bypass paths in sandbox executor (#14690)

## Summary

Two bypass vectors in the sandbox code security analyzer allowed
malicious code to pass the safety check undetected and reach the Docker
executor.

### 1. JavaScript: template-literal bypass of `require()` block

The `SecureJavaScriptAnalyzer` regex patterns used `['"]` to match
module names, covering only single and double quotes. An attacker could
use ES6 template literals to bypass all three `require` checks:

`javascript
const cp = require(`child_process`);
async function main() {
  return cp.execSync('cat /etc/passwd').toString();
}
`

The same bypass applied to `fs` and `worker_threads`.

**Fix:** Updated all three `require` patterns from `['"]` to `['"\]` to
also match backtick template literals.

### 2. Python: `builtins` not blocked + attribute-call blind spot in
`visit_Call`

`visit_Call` only checked `ast.Name` nodes, so attribute-style calls
like `module.func()` were invisible to the analyzer. Additionally,
`builtins` was absent from `DANGEROUS_IMPORTS`. Combined, this allowed:

`python
import builtins
def main():
    builtins.exec('import os; os.system("id")')
`

Neither the import nor the exec call triggered any flag.

**Fix:** Added `builtins` to `DANGEROUS_IMPORTS` and added an
`ast.Attribute` branch to `visit_Call` so that `module.dangerous_func()`
style calls are caught alongside bare `dangerous_func()` calls.

## Tests

Added four regression tests covering each new bypass vector:
- `test_javascript_child_process_template_literal_is_rejected`
- `test_javascript_fs_template_literal_is_rejected`
- `test_python_builtins_import_is_rejected`
- `test_python_attribute_eval_call_is_rejected`

---------

Co-authored-by: bounty-hunter <bounty@hunter.local>

agent/sandbox/executor_manager/services/security.py

@@ -26,7 +26,7 @@ class SecurePythonAnalyzer(ast.NodeVisitor):
     An AST-based analyzer for detecting unsafe Python code patterns.
     """
 
-    DANGEROUS_IMPORTS = {"os", "subprocess", "sys", "shutil", "socket", "ctypes", "pickle", "threading", "multiprocessing", "asyncio", "http.client", "ftplib", "telnetlib"}
+    DANGEROUS_IMPORTS = {"os", "subprocess", "sys", "shutil", "socket", "ctypes", "pickle", "threading", "multiprocessing", "asyncio", "http.client", "ftplib", "telnetlib", "builtins"}
 
     DANGEROUS_CALLS = {
         "eval",
@@ -77,6 +77,16 @@ class SecurePythonAnalyzer(ast.NodeVisitor):
         """Check for dangerous function calls."""
         if isinstance(node.func, ast.Name) and node.func.id in self.DANGEROUS_CALLS:
             self.unsafe_items.append((f"Call: {node.func.id}", node.lineno))
+        elif isinstance(node.func, ast.Attribute) and node.func.attr in self.DANGEROUS_CALLS:
+            # Surface the attribute-style match in the analyzer log so that
+            # incident response can grep for it just like the other unsafe-item
+            # findings; the bare append is invisible to operators.
+            logger.warning(
+                "[SafeCheck] Attribute-style dangerous call detected: %s (line %s)",
+                node.func.attr,
+                node.lineno,
+            )
+            self.unsafe_items.append((f"Call: {node.func.attr}", node.lineno))
         self.generic_visit(node)
 
     def visit_Attribute(self, node: ast.Attribute):
@@ -154,9 +164,9 @@ class SecurePythonAnalyzer(ast.NodeVisitor):
 
 class SecureJavaScriptAnalyzer:
     DANGEROUS_PATTERNS = [
-        (re.compile(r"""require\s*\(\s*['"]child_process['"]\s*\)"""), "Require: child_process"),
-        (re.compile(r"""require\s*\(\s*['"]fs['"]\s*\)"""), "Require: fs"),
-        (re.compile(r"""require\s*\(\s*['"]worker_threads['"]\s*\)"""), "Require: worker_threads"),
+        (re.compile(r"""require\s*\(\s*['"`]child_process['"`]\s*\)"""), "Require: child_process"),
+        (re.compile(r"""require\s*\(\s*['"`]fs['"`]\s*\)"""), "Require: fs"),
+        (re.compile(r"""require\s*\(\s*['"`]worker_threads['"`]\s*\)"""), "Require: worker_threads"),
         (re.compile(r"""\beval\s*\("""), "Call: eval"),
         (re.compile(r"""\bFunction\s*\("""), "Call: Function"),
         (re.compile(r"""\bprocess\s*\.\s*binding\s*\("""), "Call: process.binding"),

agent/sandbox/tests/test_security.py

@@ -45,6 +45,60 @@ def test_javascript_eval_is_rejected():
     assert any("eval" in issue.lower() for issue, _ in issues)
 
 
+def test_javascript_child_process_template_literal_is_rejected():
+    """Template literal backticks bypass single/double-quote regex patterns."""
+    is_safe, issues = analyze_code_security(
+        "const cp = require(`child_process`); async function main() { return 'ok'; }",
+        SupportLanguage.NODEJS,
+    )
+
+    assert is_safe is False
+    assert any("child_process" in issue for issue, _ in issues)
+
+
+def test_javascript_fs_template_literal_is_rejected():
+    is_safe, issues = analyze_code_security(
+        "const fs = require(`fs`); async function main() { return fs.readFileSync('/etc/passwd', 'utf8'); }",
+        SupportLanguage.NODEJS,
+    )
+
+    assert is_safe is False
+    assert any("fs" in issue for issue, _ in issues)
+
+
+def test_python_builtins_import_is_rejected():
+    """builtins module gives access to eval/exec and must be blocked."""
+    is_safe, issues = analyze_code_security(
+        "import builtins\ndef main():\n    builtins.eval('1+1')",
+        SupportLanguage.PYTHON,
+    )
+
+    assert is_safe is False
+    # Pin the specific reason: rejection must come from the new ``builtins``
+    # entry in ``DANGEROUS_IMPORTS``, not from some unrelated parse error.
+    assert any("builtins" in issue for issue, _ in issues), (
+        f"expected an issue mentioning 'builtins', got {issues!r}"
+    )
+
+
+def test_python_attribute_eval_call_is_rejected():
+    """Attribute-style dangerous calls (builtins.eval) must be caught."""
+    is_safe, issues = analyze_code_security(
+        "import builtins\ndef main():\n    builtins.exec('import os')",
+        SupportLanguage.PYTHON,
+    )
+
+    assert is_safe is False
+    # Pin the specific reason: rejection must come from the new
+    # ``ast.Attribute`` branch in ``visit_Call`` flagging the ``exec`` call,
+    # not from the ``import builtins`` line above. We assert ``exec`` is in at
+    # least one finding so the test fails if visit_Call's attribute branch is
+    # ever reverted.
+    assert any("exec" in issue for issue, _ in issues), (
+        f"expected an issue mentioning 'exec', got {issues!r}"
+    )
+
+
 def test_javascript_safe_code_still_passes():
     is_safe, issues = analyze_code_security(
         "async function main(args) { return { answer: args.value ?? null }; }",