Skip to content

Commit cb59ef6

Browse files
julienldclaude
andauthored
fix(internal): reject non-Name/Attribute call targets in python_sandbox (#772)
* fix(internal): reject non-Name/Attribute call targets in python_sandbox Adds else branch to ast.Call validation so subscript calls (config['fn']()) and chained calls (config.get('fn')()) are rejected instead of silently passing. The sandbox documented that only named functions and whitelisted methods are allowed -- this makes that guarantee accurate. In production config is always JSON-deserialized (no callables possible), so there is no user-facing impact. Credits: @restriction (responsible disclosure) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(internal): clarify python_sandbox is not a security boundary Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(internal): add regression tests for subscript and chained call blocking Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 65c19bb commit cb59ef6

3 files changed

Lines changed: 24 additions & 3 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ This project is licensed under the MIT License - see the [LICENSE](LICENSE) file
245245
- **[@saphid](https://github.com/saphid)** — Config entry options flow tools (initial design, #590).
246246
- **[@adraguidev](https://github.com/adraguidev)** — Fix menu-based config entry flows for group helpers (#647).
247247
- **[@transportrefer](https://github.com/transportrefer)** — Integration options inspection (`ha_get_integration` schema support, #689).
248+
- **[@restriction](https://github.com/restriction)** — Responsible disclosure: python_transform sandbox missing call target validation.
248249

249250
---
250251

src/ha_mcp/utils/python_sandbox.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
"""
2-
Python expression sandbox using AST validation.
2+
Python expression validation for dashboard transformations.
33
4-
Provides safe execution of Python expressions for dashboard transformations.
5-
Blocks imports, file I/O, dangerous builtins, and common sandbox escapes.
4+
Restricts expressions to a known-safe subset: dict/list operations,
5+
basic control flow, and whitelisted methods. Not a security boundary —
6+
callers are already authenticated MCP users with full HA access.
67
"""
78

89
import ast
@@ -189,6 +190,11 @@ def validate_expression(expr: str) -> tuple[bool, str]:
189190
f"Forbidden method: {method_name} (allowed: {', '.join(sorted(SAFE_METHODS))})",
190191
)
191192

193+
# Reject subscript calls, chained calls, and all other non-standard targets
194+
# e.g., config['fn']() or config.get('fn')() would bypass Name/Attribute checks
195+
else:
196+
return False, f"Forbidden call target type: {type(node.func).__name__}"
197+
192198
# Block function definitions (could be used for obfuscation)
193199
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
194200
return False, "Forbidden: function/class definitions not allowed"

tests/src/unit/test_python_sandbox.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,20 @@ def test_block_forbidden_method(self):
129129
assert valid is False
130130
assert "method" in error.lower()
131131

132+
def test_block_subscript_call(self):
133+
"""Test that calls on subscript results are blocked."""
134+
expr = "config['fn']()"
135+
valid, error = validate_expression(expr)
136+
assert valid is False
137+
assert "Subscript" in error
138+
139+
def test_block_chained_call(self):
140+
"""Test that calls on method results are blocked."""
141+
expr = "config.get('fn')()"
142+
valid, error = validate_expression(expr)
143+
assert valid is False
144+
assert "Call" in error
145+
132146

133147
class TestSafeExecute:
134148
"""Test safe execution of expressions."""

0 commit comments

Comments
 (0)