mirror of
https://github.com/jmagar/unraid-mcp.git
synced 2026-03-23 12:39:24 -07:00
fix(safety): add stop_array to DESTRUCTIVE_ACTIONS, add error propagation test
stop_array can cause data loss for running containers/VMs that depend on array shares — requires confirm=True like other destructive mutations. - Add stop_array to DESTRUCTIVE_ACTIONS and desc_map in array.py - Update safety audit KNOWN_DESTRUCTIVE[array] to include stop_array - Add stop_array negative/positive tests (test_array.py, safety tests) - Add test_snapshot_wraps_bare_exception to test_live.py (bare Exception from subscribe_once is wrapped by tool_error_handler into ToolError) 748 tests passing
This commit is contained in:
@@ -47,7 +47,7 @@ KNOWN_DESTRUCTIVE: dict[str, dict[str, set[str] | str]] = {
|
||||
"module": "unraid_mcp.tools.array",
|
||||
"register_fn": "register_array_tool",
|
||||
"tool_name": "unraid_array",
|
||||
"actions": {"remove_disk", "clear_disk_stats"},
|
||||
"actions": {"remove_disk", "clear_disk_stats", "stop_array"},
|
||||
"runtime_set": ARRAY_DESTRUCTIVE,
|
||||
},
|
||||
"vm": {
|
||||
@@ -198,6 +198,7 @@ _DESTRUCTIVE_TEST_CASES: list[tuple[str, str, dict]] = [
|
||||
# Array
|
||||
("array", "remove_disk", {"disk_id": "abc123:local"}),
|
||||
("array", "clear_disk_stats", {"disk_id": "abc123:local"}),
|
||||
("array", "stop_array", {}),
|
||||
# VM
|
||||
("vm", "force_stop", {"vm_id": "test-vm-uuid"}),
|
||||
("vm", "reset", {"vm_id": "test-vm-uuid"}),
|
||||
@@ -468,6 +469,12 @@ class TestConfirmAllowsExecution:
|
||||
result = await tool_fn(action="clear_disk_stats", disk_id="abc:local", confirm=True)
|
||||
assert result["success"] is True
|
||||
|
||||
async def test_array_stop_array_with_confirm(self, _mock_array_graphql: AsyncMock) -> None:
|
||||
_mock_array_graphql.return_value = {"array": {"setState": {"state": "STOPPED"}}}
|
||||
tool_fn = make_tool_fn("unraid_mcp.tools.array", "register_array_tool", "unraid_array")
|
||||
result = await tool_fn(action="stop_array", confirm=True)
|
||||
assert result["success"] is True
|
||||
|
||||
async def test_plugins_remove_with_confirm(self, _mock_plugins_graphql: AsyncMock) -> None:
|
||||
_mock_plugins_graphql.return_value = {"removePlugin": True}
|
||||
tool_fn = make_tool_fn(
|
||||
|
||||
@@ -172,9 +172,15 @@ async def test_start_array(_mock_graphql):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_array(_mock_graphql):
|
||||
async def test_stop_array_requires_confirm(_mock_graphql):
|
||||
with pytest.raises(ToolError, match="not confirmed"):
|
||||
await _make_tool()(action="stop_array", confirm=False)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_array_with_confirm(_mock_graphql):
|
||||
_mock_graphql.return_value = {"array": {"setState": {"state": "STOPPED"}}}
|
||||
result = await _make_tool()(action="stop_array")
|
||||
result = await _make_tool()(action="stop_array", confirm=True)
|
||||
assert result["success"] is True
|
||||
|
||||
|
||||
|
||||
@@ -112,3 +112,14 @@ async def test_log_tail_rejects_invalid_path(mcp, _mock_subscribe_collect):
|
||||
tool_fn = _make_live_tool(mcp)
|
||||
with pytest.raises(ToolError, match="must start with"):
|
||||
await tool_fn(action="log_tail", path="/etc/shadow")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_snapshot_wraps_bare_exception(mcp, _mock_subscribe_once):
|
||||
"""Bare exceptions from subscribe_once are wrapped in ToolError by tool_error_handler."""
|
||||
from unraid_mcp.core.exceptions import ToolError
|
||||
|
||||
_mock_subscribe_once.side_effect = RuntimeError("WebSocket connection refused")
|
||||
tool_fn = _make_live_tool(mcp)
|
||||
with pytest.raises(ToolError):
|
||||
await tool_fn(action="cpu")
|
||||
|
||||
@@ -95,7 +95,7 @@ MUTATIONS: dict[str, str] = {
|
||||
""",
|
||||
}
|
||||
|
||||
DESTRUCTIVE_ACTIONS = {"remove_disk", "clear_disk_stats"}
|
||||
DESTRUCTIVE_ACTIONS = {"remove_disk", "clear_disk_stats", "stop_array"}
|
||||
ALL_ACTIONS = set(QUERIES) | set(MUTATIONS)
|
||||
|
||||
ARRAY_ACTIONS = Literal[
|
||||
@@ -163,6 +163,7 @@ def register_array_tool(mcp: FastMCP) -> None:
|
||||
desc_map = {
|
||||
"remove_disk": f"Remove disk **{disk_id}** from the array. The array must be stopped first.",
|
||||
"clear_disk_stats": f"Clear all I/O statistics for disk **{disk_id}**. This cannot be undone.",
|
||||
"stop_array": "Stop the Unraid array. Running containers and VMs may lose access to array shares.",
|
||||
}
|
||||
confirmed = await elicit_destructive_confirmation(ctx, action, desc_map[action])
|
||||
if not confirmed:
|
||||
|
||||
Reference in New Issue
Block a user