Skip to content

Commit 95cbb28

Browse files
committed
fix(policy): mypy bool cast + broaden e2e coverage (#966)
mypy: bool(_ci(val) == _ci(pv)) — _ci returns Any (passes non-strings through unchanged), so eq/neq comparisons need an explicit bool wrap. Tests: previous e2e only covered the happy block→approve→re-call path. Add four more cases against the live testcontainer: - wildcard `args.*` gates when any arg matches the value - wildcard `args.*` passes through when no arg matches - case-insensitive matching (rule 'lock' gates caller 'LOCK') - deny → middleware raises USER_DENIED, tool never runs - remember_minutes>0: second call within the window skips the queue
1 parent 44a0bad commit 95cbb28

2 files changed

Lines changed: 135 additions & 2 deletions

File tree

src/ha_mcp/policy/evaluator.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ def _op_matches(val: Any, op: str, pv: Any) -> bool:
6363
"""
6464
match op:
6565
case "eq":
66-
return _ci(val) == _ci(pv)
66+
return bool(_ci(val) == _ci(pv))
6767
case "neq":
68-
return _ci(val) != _ci(pv)
68+
return bool(_ci(val) != _ci(pv))
6969
case "in":
7070
return _ci(val) in [_ci(x) for x in (pv or [])]
7171
case "not_in":

tests/src/e2e/policy/test_approval_flow.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,3 +182,136 @@ async def test_blocked_call_then_approve_then_recall(policy_enabled_mcp):
182182
"entity_id": "light.bed_light",
183183
}
184184
await _expect_blocked(client, other_args)
185+
186+
187+
async def _install_rule(handlers, rule: dict[str, Any]) -> None:
188+
current_resp = await handlers["policy_get_config"](_make_request())
189+
current = json.loads(current_resp.body)
190+
body = {
191+
"wait_seconds": 5,
192+
"approval_ttl_minutes": 5,
193+
"rules": [rule],
194+
"version": current["version"],
195+
}
196+
put_resp = await handlers["policy_put_config"](_make_request(body))
197+
assert put_resp.status_code == 200, put_resp.body
198+
199+
200+
@pytest.mark.asyncio
201+
async def test_wildcard_path_gates_when_any_arg_matches(policy_enabled_mcp):
202+
"""`args.*` fans out: blocks when ANY arg equals the gated value."""
203+
client, server, handlers = policy_enabled_mcp
204+
await _install_rule(
205+
handlers,
206+
{
207+
"tool_name": "ha_call_service",
208+
"when": [{"path": "args.*", "op": "eq", "value": "light"}],
209+
"remember_minutes": 0,
210+
},
211+
)
212+
# domain="light" → matches via wildcard
213+
await _expect_blocked(
214+
client,
215+
{"domain": "light", "service": "turn_on", "entity_id": "light.bed_light"},
216+
)
217+
assert server.approval_queue.list_pending()
218+
219+
220+
@pytest.mark.asyncio
221+
async def test_wildcard_path_passes_when_no_arg_matches(policy_enabled_mcp):
222+
"""`args.*` does NOT gate when no arg satisfies the condition."""
223+
client, server, handlers = policy_enabled_mcp
224+
await _install_rule(
225+
handlers,
226+
{
227+
"tool_name": "ha_call_service",
228+
"when": [{"path": "args.*", "op": "eq", "value": "lock"}],
229+
"remember_minutes": 0,
230+
},
231+
)
232+
# No arg equals "lock" → call must pass through to the real tool.
233+
result = await client.call_tool(
234+
"ha_call_service",
235+
{"domain": "light", "service": "turn_on", "entity_id": "light.bed_light"},
236+
)
237+
assert not result.is_error, result
238+
assert server.approval_queue.list_pending() == []
239+
240+
241+
@pytest.mark.asyncio
242+
async def test_case_insensitive_match_gates_regardless_of_caller_casing(
243+
policy_enabled_mcp,
244+
):
245+
"""Rule value 'lock' should gate calls with 'LOCK', 'Lock', etc."""
246+
client, server, handlers = policy_enabled_mcp
247+
await _install_rule(
248+
handlers,
249+
{
250+
"tool_name": "ha_call_service",
251+
"when": [{"path": "args.domain", "op": "eq", "value": "lock"}],
252+
"remember_minutes": 0,
253+
},
254+
)
255+
# Caller capitalises — CI matching must still gate.
256+
await _expect_blocked(
257+
client, {"domain": "LOCK", "service": "unlock", "entity_id": "lock.front"}
258+
)
259+
pending = server.approval_queue.list_pending()
260+
assert len(pending) == 1
261+
262+
263+
@pytest.mark.asyncio
264+
async def test_deny_raises_user_denied(policy_enabled_mcp):
265+
"""POST /deny → middleware raises USER_DENIED, never calls the tool."""
266+
client, server, handlers = policy_enabled_mcp
267+
await _install_rule(
268+
handlers,
269+
{
270+
"tool_name": "ha_call_service",
271+
"when": [{"path": "args.domain", "op": "eq", "value": "light"}],
272+
"remember_minutes": 0,
273+
},
274+
)
275+
args = {"domain": "light", "service": "turn_on", "entity_id": "light.bed_light"}
276+
await _expect_blocked(client, args)
277+
token = server.approval_queue.list_pending()[0].token
278+
deny_resp = await handlers["policy_post_deny"](_make_request({"token": token}))
279+
assert deny_resp.status_code == 200, deny_resp.body
280+
# Re-call with same args: middleware sees the denied entry → USER_DENIED.
281+
try:
282+
result = await client.call_tool("ha_call_service", args)
283+
except ToolError as exc:
284+
body = tool_error_to_result(exc)
285+
else:
286+
body = parse_mcp_result(result)
287+
assert body.get("error", {}).get("code") == "USER_DENIED", body
288+
289+
290+
@pytest.mark.asyncio
291+
async def test_remember_minutes_skips_approval_within_window(policy_enabled_mcp):
292+
"""remember_minutes>0: a second call within the window bypasses gating."""
293+
client, server, handlers = policy_enabled_mcp
294+
await _install_rule(
295+
handlers,
296+
{
297+
"tool_name": "ha_call_service",
298+
"when": [{"path": "args.domain", "op": "eq", "value": "light"}],
299+
"remember_minutes": 5,
300+
},
301+
)
302+
args = {"domain": "light", "service": "turn_on", "entity_id": "light.bed_light"}
303+
await _expect_blocked(client, args)
304+
token = server.approval_queue.list_pending()[0].token
305+
approve_resp = await handlers["policy_post_approve"](
306+
_make_request({"token": token})
307+
)
308+
assert approve_resp.status_code == 200
309+
# First post-approval call consumes the pending entry AND seeds the
310+
# remember-cache. Second call within the 5-minute window should skip
311+
# the queue entirely.
312+
result_a = await client.call_tool("ha_call_service", args)
313+
assert not result_a.is_error
314+
result_b = await client.call_tool("ha_call_service", args)
315+
assert not result_b.is_error
316+
# Pending must be empty — neither call left an entry behind.
317+
assert server.approval_queue.list_pending() == []

0 commit comments

Comments
 (0)