-
Notifications
You must be signed in to change notification settings - Fork 8.2k
ref: add sandbox to exec call in validate endpoint #10696
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: main
Are you sure you want to change the base?
Changes from 4 commits
87bf735
912836b
3019e1f
0328ae8
f5cf8ff
b574590
95aa641
d614f01
52a36a1
4b42d7c
8744c5a
84bca94
3a6412f
50ff06f
156a018
74941b5
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 |
|---|---|---|
|
|
@@ -77,3 +77,221 @@ | |
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": code}, headers={"Authorization": "Bearer fake"}) | ||
| assert response.status_code == status.HTTP_401_UNAUTHORIZED | ||
|
|
||
|
|
||
| # Security tests to verify dangerous operations are blocked by default | ||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_blocks_dangerous_imports_by_default(client: AsyncClient, logged_in_headers): | ||
| """Test that dangerous imports are blocked by default.""" | ||
| # Code with dangerous imports should be blocked | ||
| dangerous_code = """ | ||
| import os | ||
| import subprocess | ||
| def test(): | ||
| return os.getcwd() | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": dangerous_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should have errors - dangerous imports are blocked | ||
| assert len(result["imports"]["errors"]) > 0 or len(result["function"]["errors"]) > 0 | ||
| # Should mention that module is blocked | ||
| all_errors = result["imports"]["errors"] + result["function"]["errors"] | ||
| assert any("blocked" in str(err).lower() or "os" in str(err).lower() or "subprocess" in str(err).lower() for err in all_errors) | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_blocks_dangerous_builtins_by_default(client: AsyncClient, logged_in_headers): | ||
| """Test that dangerous builtins are blocked by default.""" | ||
| # Code using dangerous builtins in default args should be blocked | ||
| # (function-definition-time execution catches this) | ||
| dangerous_code = """ | ||
| def test(x=open('/etc/passwd', 'r').read()): | ||
| return x | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": dangerous_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should have errors - dangerous builtins are blocked | ||
| assert len(result["function"]["errors"]) > 0 | ||
| # Should mention that builtin is blocked | ||
| assert any("blocked" in str(err).lower() or "open" in str(err).lower() for err in result["function"]["errors"]) | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_cannot_access_server_globals(client: AsyncClient, logged_in_headers): | ||
| """Test that code cannot access server's global namespace.""" | ||
| # Try to access something that would be in server globals | ||
| code_trying_to_escape = """ | ||
| def test(): | ||
| # Try to access parent frame or server globals | ||
| import sys | ||
| # This should execute but in isolation - can't access server's sys module state | ||
| return sys.version | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": code_trying_to_escape}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Code executes but in isolation - sys is imported fresh, not server's sys | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_allows_safe_code(client: AsyncClient, logged_in_headers): | ||
| """Test that legitimate safe code still works.""" | ||
| safe_code = """ | ||
| from typing import List, Optional | ||
| def process(items: List[str]) -> Optional[str]: | ||
| return items[0] if items else None | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": safe_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should have no errors for safe code | ||
| assert len(result["imports"]["errors"]) == 0 | ||
| assert len(result["function"]["errors"]) == 0 | ||
|
|
||
|
|
||
| # Isolation verification tests | ||
| # | ||
| # IMPORTANT: These tests verify that the validate endpoint uses the sandbox. | ||
| # The actual isolation behavior is tested in src/lfx/tests/unit/custom/test_sandbox_isolation.py | ||
| # These tests just verify that dangerous code executes without crashing (proves sandbox is used) | ||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_allows_safe_imports(client: AsyncClient, logged_in_headers): | ||
| """Test that safe imports are allowed.""" | ||
| # Code with safe imports should work | ||
| safe_code = """ | ||
| from typing import List, Optional | ||
| import json | ||
| import math | ||
| def test(items: List[str]) -> Optional[str]: | ||
| return json.dumps({"count": math.sqrt(len(items))}) | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": safe_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should execute without errors - safe imports are allowed | ||
| assert len(result["imports"]["errors"]) == 0 | ||
| assert len(result["function"]["errors"]) == 0 | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_allows_third_party_libraries(client: AsyncClient, logged_in_headers): | ||
| """Test that third-party libraries (not in a whitelist) can be imported. | ||
| Users should be able to import legitimate third-party libraries like AI libraries, | ||
| data processing libraries, etc. We only block dangerous system-level modules. | ||
| """ | ||
| # Try importing a common third-party library that wouldn't be in a whitelist | ||
| # Using 'requests' as an example - it's a legitimate library but not dangerous | ||
| # Note: This test will fail if 'requests' isn't installed, but that's okay | ||
| # The important thing is that if it IS installed, it should be allowed | ||
| third_party_code = """ | ||
| try: | ||
| import requests | ||
| HAS_REQUESTS = True | ||
| except ImportError: | ||
| HAS_REQUESTS = False | ||
| def test(): | ||
| return HAS_REQUESTS | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": third_party_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should execute without errors - third-party libraries are allowed | ||
| # (unless they're in BLOCKED_MODULES like 'os', 'subprocess', etc.) | ||
| assert len(result["imports"]["errors"]) == 0 | ||
| assert len(result["function"]["errors"]) == 0 | ||
|
|
||
|
Comment on lines
162
to
190
Contributor
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. 🧩 Analysis chainThird‑party library test conflicts with sandbox BLOCKED_MODULES (
# Using 'requests' as an example - it's a legitimate library but not dangerous
...
assert len(result["imports"]["errors"]) == 0
assert len(result["function"]["errors"]) == 0However, the sandbox’s You should align test expectations with the sandbox policy in one of these ways:
Given the top‑level docstring in 🏁 Script executed: #!/bin/bash
# Double‑check the current BLOCKED_MODULES definition and where this test runs.
rg -n "BLOCKED_MODULES" -S src/lfx/src/lfx/custom/sandbox.py
rg -n "test_validate_code_allows_third_party_libraries" -SLength of output: 424 I need to inspect the actual contents of the BLOCKED_MODULES set to verify the review comment's claim. Let me gather that information: [/run_scripts] 🏁 Script executed: #!/bin/bash
# View the BLOCKED_MODULES set definition (lines around 42)
sed -n '42,80p' src/lfx/src/lfx/custom/sandbox.pyLength of output: 700 Now I need to verify what happens during validation when a blocked module is encountered, and confirm the test would fail as claimed: [/run_scripts] 🏁 Script executed: #!/bin/bash
# Check the validation logic around line 156 that checks BLOCKED_MODULES
sed -n '150,175p' src/lfx/src/lfx/custom/sandbox.pyLength of output: 1285 I need to see how the validate endpoint handles SecurityViolation exceptions and what error structure gets returned: [/run_scripts] 🏁 Script executed: #!/bin/bash
# Find the validate endpoint code for the /api/v1/validate/code route
rg -n "validate/code" -A 20 -B 5 src/backend/app/api/v1/Length of output: 63 Let me search more broadly for the validate endpoint implementation: [/run_scripts] 🏁 Script executed: #!/bin/bash
# Find the validate endpoint code by searching for the route handler
rg -n "def.*validate.*code" -A 15 src/backend/Length of output: 297840 Blocked module ("requests") in sandbox makes this test fail — update test or sandbox policy. BLOCKED_MODULES in src/lfx/src/lfx/custom/sandbox.py explicitly contains "requests" and "httpx" (around line 42) and the import hook raises SecurityViolation for blocked modules (check around line 156). The test src/backend/tests/unit/api/v1/test_validate.py::test_validate_code_allows_third_party_libraries (lines 181–209) expects no import/function errors but will fail under the current sandbox policy.
🧰 Tools🪛 GitHub Check: Ruff Style Check (3.13)[failure] 184-184: Ruff (W293) |
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_allows_langflow_modules(client: AsyncClient, logged_in_headers): | ||
| """Test that langflow.* modules are allowed.""" | ||
| # Code importing langflow.* modules should work | ||
| langflow_code = """ | ||
| import langflow | ||
| from langflow.schema import Data | ||
| def test(): | ||
| return Data(data={"test": "value"}) | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": langflow_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should execute without errors - langflow.* modules are allowed | ||
| assert len(result["imports"]["errors"]) == 0 | ||
| assert len(result["function"]["errors"]) == 0 | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_allows_lfx_modules(client: AsyncClient, logged_in_headers): | ||
| """Test that lfx.* modules are allowed.""" | ||
| # Code importing lfx.* modules should work | ||
| lfx_code = """ | ||
| import lfx | ||
| from lfx.custom import Component | ||
| def test(): | ||
| return Component() | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": lfx_code}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should execute without errors - lfx.* modules are allowed | ||
| assert len(result["imports"]["errors"]) == 0 | ||
| assert len(result["function"]["errors"]) == 0 | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_cannot_access_real_builtins(client: AsyncClient, logged_in_headers): | ||
| """Test that code cannot escape via real __builtins__. | ||
| If isolation were broken, code could access the real __builtins__ to escape. | ||
| We test this by trying to access __builtins__ directly (which should be isolated). | ||
| """ | ||
| # Try to access __builtins__ directly - if isolation is broken, this would work | ||
| code_trying_builtins_escape = """ | ||
| def test(): | ||
| # Try to access __builtins__ directly from globals | ||
| # If isolation is broken, this would give us the real builtins | ||
| try: | ||
| real_builtins = globals().get('__builtins__') | ||
| # If we got real builtins, we could escape - but we should get isolated version | ||
| return type(real_builtins).__name__ | ||
| except Exception: | ||
| return 'isolated' | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": code_trying_builtins_escape}, headers=logged_in_headers) | ||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Code should execute successfully - the key is that __builtins__ is isolated | ||
| # We can't easily verify it's isolated via API, but execution without errors | ||
| # combined with other isolation tests proves it works | ||
| assert len(result["function"]["errors"]) == 0 # Should execute without errors | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("active_user") | ||
| async def test_validate_code_uses_isolated_sandbox(client: AsyncClient, logged_in_headers): | ||
| """Test that validate endpoint uses isolated sandbox. | ||
| This test verifies that code executes in sandbox (doesn't crash). | ||
| Actual isolation behavior is tested in sandbox unit tests. | ||
| """ | ||
| # Code that sets variables - should execute without errors | ||
| # (Isolation is verified in sandbox unit tests) | ||
| code_with_variables = """ | ||
| MY_VAR = "test" | ||
| def test(): | ||
| return MY_VAR | ||
| """ | ||
| response = await client.post("api/v1/validate/code", json={"code": code_with_variables}, headers=logged_in_headers) | ||
|
||
| result = response.json() | ||
| assert response.status_code == status.HTTP_200_OK | ||
| # Should execute without errors - proves sandbox is being used | ||
| assert len(result["function"]["errors"]) == 0 | ||
|
|
||
|
|
||
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.
Fix Ruff E501 / F841 in dangerous‑imports and server‑globals tests.
Ruff is (correctly) flagging:
resultis assigned but never used.You can resolve all three without changing test semantics:
This keeps the intent while satisfying Ruff line‑length and unused‑variable rules. As per pipeline failures.
🧰 Tools
🪛 GitHub Actions: Ruff Style Check
[error] 101-101: Ruff: E501 Line too long (131 > 120). Line exceeds maximum line length.
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 134-134: Ruff (F841)
src/backend/tests/unit/api/v1/test_validate.py:134:5: F841 Local variable
resultis assigned to but never used[failure] 133-133: Ruff (E501)
src/backend/tests/unit/api/v1/test_validate.py:133:121: E501 Line too long (121 > 120)
[failure] 101-101: Ruff (E501)
src/backend/tests/unit/api/v1/test_validate.py:101:121: E501 Line too long (131 > 120)