mirror of
https://github.com/jmagar/unraid-mcp.git
synced 2026-03-23 12:39:24 -07:00
feat(guards): wire elicitation into notifications/vm/rclone/settings/storage
Replace hard ToolError guard with gate_destructive_action() in 5 tools so destructive actions prompt for interactive confirmation via MCP elicitation when ctx is available, and still accept confirm=True as a bypass. Update all test match strings from "destructive" to "not confirmed" accordingly.
This commit is contained in:
@@ -576,7 +576,7 @@ class TestVMToolRequests:
|
|||||||
@respx.mock
|
@respx.mock
|
||||||
async def test_force_stop_requires_confirm(self) -> None:
|
async def test_force_stop_requires_confirm(self) -> None:
|
||||||
tool = self._get_tool()
|
tool = self._get_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool(action="force_stop", vm_id="vm-789")
|
await tool(action="force_stop", vm_id="vm-789")
|
||||||
|
|
||||||
@respx.mock
|
@respx.mock
|
||||||
@@ -593,7 +593,7 @@ class TestVMToolRequests:
|
|||||||
@respx.mock
|
@respx.mock
|
||||||
async def test_reset_requires_confirm(self) -> None:
|
async def test_reset_requires_confirm(self) -> None:
|
||||||
tool = self._get_tool()
|
tool = self._get_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool(action="reset", vm_id="vm-abc")
|
await tool(action="reset", vm_id="vm-abc")
|
||||||
|
|
||||||
@respx.mock
|
@respx.mock
|
||||||
@@ -880,7 +880,7 @@ class TestNotificationsToolRequests:
|
|||||||
@respx.mock
|
@respx.mock
|
||||||
async def test_delete_requires_confirm(self) -> None:
|
async def test_delete_requires_confirm(self) -> None:
|
||||||
tool = self._get_tool()
|
tool = self._get_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool(action="delete", notification_id="n1", notification_type="UNREAD")
|
await tool(action="delete", notification_id="n1", notification_type="UNREAD")
|
||||||
|
|
||||||
@respx.mock
|
@respx.mock
|
||||||
@@ -990,7 +990,7 @@ class TestRCloneToolRequests:
|
|||||||
@respx.mock
|
@respx.mock
|
||||||
async def test_delete_remote_requires_confirm(self) -> None:
|
async def test_delete_remote_requires_confirm(self) -> None:
|
||||||
tool = self._get_tool()
|
tool = self._get_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool(action="delete_remote", name="old-remote")
|
await tool(action="delete_remote", name="old-remote")
|
||||||
|
|
||||||
@respx.mock
|
@respx.mock
|
||||||
|
|||||||
@@ -40,12 +40,12 @@ def _make_tool():
|
|||||||
class TestNotificationsValidation:
|
class TestNotificationsValidation:
|
||||||
async def test_delete_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
|
async def test_delete_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
|
||||||
tool_fn = _make_tool()
|
tool_fn = _make_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool_fn(action="delete", notification_id="n:1", notification_type="UNREAD")
|
await tool_fn(action="delete", notification_id="n:1", notification_type="UNREAD")
|
||||||
|
|
||||||
async def test_delete_archived_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
|
async def test_delete_archived_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
|
||||||
tool_fn = _make_tool()
|
tool_fn = _make_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool_fn(action="delete_archived")
|
await tool_fn(action="delete_archived")
|
||||||
|
|
||||||
async def test_create_requires_fields(self, _mock_graphql: AsyncMock) -> None:
|
async def test_create_requires_fields(self, _mock_graphql: AsyncMock) -> None:
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ def _make_tool():
|
|||||||
class TestRcloneValidation:
|
class TestRcloneValidation:
|
||||||
async def test_delete_requires_confirm(self) -> None:
|
async def test_delete_requires_confirm(self) -> None:
|
||||||
tool_fn = _make_tool()
|
tool_fn = _make_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool_fn(action="delete_remote", name="gdrive")
|
await tool_fn(action="delete_remote", name="gdrive")
|
||||||
|
|
||||||
async def test_create_requires_fields(self) -> None:
|
async def test_create_requires_fields(self) -> None:
|
||||||
|
|||||||
@@ -290,7 +290,7 @@ class TestStorageNetworkErrors:
|
|||||||
class TestStorageFlashBackup:
|
class TestStorageFlashBackup:
|
||||||
async def test_flash_backup_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
|
async def test_flash_backup_requires_confirm(self, _mock_graphql: AsyncMock) -> None:
|
||||||
tool_fn = _make_tool()
|
tool_fn = _make_tool()
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool_fn(
|
await tool_fn(
|
||||||
action="flash_backup", remote_name="r", source_path="/boot", destination_path="r:b"
|
action="flash_backup", remote_name="r", source_path="/boot", destination_path="r:b"
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ class TestVmValidation:
|
|||||||
async def test_destructive_actions_require_confirm(self, _mock_graphql: AsyncMock) -> None:
|
async def test_destructive_actions_require_confirm(self, _mock_graphql: AsyncMock) -> None:
|
||||||
tool_fn = _make_tool()
|
tool_fn = _make_tool()
|
||||||
for action in ("force_stop", "reset"):
|
for action in ("force_stop", "reset"):
|
||||||
with pytest.raises(ToolError, match="destructive"):
|
with pytest.raises(ToolError, match="not confirmed"):
|
||||||
await tool_fn(action=action, vm_id="uuid-1")
|
await tool_fn(action=action, vm_id="uuid-1")
|
||||||
|
|
||||||
async def test_destructive_vm_id_check_before_confirm(self, _mock_graphql: AsyncMock) -> None:
|
async def test_destructive_vm_id_check_before_confirm(self, _mock_graphql: AsyncMock) -> None:
|
||||||
|
|||||||
@@ -6,11 +6,12 @@ creating, archiving, and deleting system notifications.
|
|||||||
|
|
||||||
from typing import Any, Literal, get_args
|
from typing import Any, Literal, get_args
|
||||||
|
|
||||||
from fastmcp import FastMCP
|
from fastmcp import Context, FastMCP
|
||||||
|
|
||||||
from ..config.logging import logger
|
from ..config.logging import logger
|
||||||
from ..core.client import make_graphql_request
|
from ..core.client import make_graphql_request
|
||||||
from ..core.exceptions import ToolError, tool_error_handler
|
from ..core.exceptions import ToolError, tool_error_handler
|
||||||
|
from ..core.guards import gate_destructive_action
|
||||||
|
|
||||||
|
|
||||||
QUERIES: dict[str, str] = {
|
QUERIES: dict[str, str] = {
|
||||||
@@ -143,6 +144,7 @@ def register_notifications_tool(mcp: FastMCP) -> None:
|
|||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def unraid_notifications(
|
async def unraid_notifications(
|
||||||
action: NOTIFICATION_ACTIONS,
|
action: NOTIFICATION_ACTIONS,
|
||||||
|
ctx: Context | None = None,
|
||||||
confirm: bool = False,
|
confirm: bool = False,
|
||||||
notification_id: str | None = None,
|
notification_id: str | None = None,
|
||||||
notification_ids: list[str] | None = None,
|
notification_ids: list[str] | None = None,
|
||||||
@@ -174,8 +176,16 @@ def register_notifications_tool(mcp: FastMCP) -> None:
|
|||||||
if action not in ALL_ACTIONS:
|
if action not in ALL_ACTIONS:
|
||||||
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
||||||
|
|
||||||
if action in DESTRUCTIVE_ACTIONS and not confirm:
|
await gate_destructive_action(
|
||||||
raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")
|
ctx,
|
||||||
|
action,
|
||||||
|
DESTRUCTIVE_ACTIONS,
|
||||||
|
confirm,
|
||||||
|
{
|
||||||
|
"delete": f"Delete notification **{notification_id}** permanently. This cannot be undone.",
|
||||||
|
"delete_archived": "Delete ALL archived notifications permanently. This cannot be undone.",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
# Validate enum parameters before dispatching to GraphQL (SEC-M04).
|
# Validate enum parameters before dispatching to GraphQL (SEC-M04).
|
||||||
# Invalid values waste a rate-limited request and may leak schema details in errors.
|
# Invalid values waste a rate-limited request and may leak schema details in errors.
|
||||||
|
|||||||
@@ -7,11 +7,12 @@ cloud storage remotes (S3, Google Drive, Dropbox, FTP, etc.).
|
|||||||
import re
|
import re
|
||||||
from typing import Any, Literal, get_args
|
from typing import Any, Literal, get_args
|
||||||
|
|
||||||
from fastmcp import FastMCP
|
from fastmcp import Context, FastMCP
|
||||||
|
|
||||||
from ..config.logging import logger
|
from ..config.logging import logger
|
||||||
from ..core.client import make_graphql_request
|
from ..core.client import make_graphql_request
|
||||||
from ..core.exceptions import ToolError, tool_error_handler
|
from ..core.exceptions import ToolError, tool_error_handler
|
||||||
|
from ..core.guards import gate_destructive_action
|
||||||
|
|
||||||
|
|
||||||
QUERIES: dict[str, str] = {
|
QUERIES: dict[str, str] = {
|
||||||
@@ -110,6 +111,7 @@ def register_rclone_tool(mcp: FastMCP) -> None:
|
|||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def unraid_rclone(
|
async def unraid_rclone(
|
||||||
action: RCLONE_ACTIONS,
|
action: RCLONE_ACTIONS,
|
||||||
|
ctx: Context | None = None,
|
||||||
confirm: bool = False,
|
confirm: bool = False,
|
||||||
name: str | None = None,
|
name: str | None = None,
|
||||||
provider_type: str | None = None,
|
provider_type: str | None = None,
|
||||||
@@ -126,8 +128,13 @@ def register_rclone_tool(mcp: FastMCP) -> None:
|
|||||||
if action not in ALL_ACTIONS:
|
if action not in ALL_ACTIONS:
|
||||||
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
||||||
|
|
||||||
if action in DESTRUCTIVE_ACTIONS and not confirm:
|
await gate_destructive_action(
|
||||||
raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")
|
ctx,
|
||||||
|
action,
|
||||||
|
DESTRUCTIVE_ACTIONS,
|
||||||
|
confirm,
|
||||||
|
f"Delete rclone remote **{name}**. This cannot be undone.",
|
||||||
|
)
|
||||||
|
|
||||||
with tool_error_handler("rclone", action, logger):
|
with tool_error_handler("rclone", action, logger):
|
||||||
logger.info(f"Executing unraid_rclone action={action}")
|
logger.info(f"Executing unraid_rclone action={action}")
|
||||||
|
|||||||
@@ -6,11 +6,12 @@ configuration and UPS monitoring.
|
|||||||
|
|
||||||
from typing import Any, Literal, get_args
|
from typing import Any, Literal, get_args
|
||||||
|
|
||||||
from fastmcp import FastMCP
|
from fastmcp import Context, FastMCP
|
||||||
|
|
||||||
from ..config.logging import logger
|
from ..config.logging import logger
|
||||||
from ..core.client import make_graphql_request
|
from ..core.client import make_graphql_request
|
||||||
from ..core.exceptions import ToolError, tool_error_handler
|
from ..core.exceptions import ToolError, tool_error_handler
|
||||||
|
from ..core.guards import gate_destructive_action
|
||||||
|
|
||||||
|
|
||||||
MUTATIONS: dict[str, str] = {
|
MUTATIONS: dict[str, str] = {
|
||||||
@@ -51,6 +52,7 @@ def register_settings_tool(mcp: FastMCP) -> None:
|
|||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def unraid_settings(
|
async def unraid_settings(
|
||||||
action: SETTINGS_ACTIONS,
|
action: SETTINGS_ACTIONS,
|
||||||
|
ctx: Context | None = None,
|
||||||
confirm: bool = False,
|
confirm: bool = False,
|
||||||
settings_input: dict[str, Any] | None = None,
|
settings_input: dict[str, Any] | None = None,
|
||||||
ups_config: dict[str, Any] | None = None,
|
ups_config: dict[str, Any] | None = None,
|
||||||
@@ -64,8 +66,13 @@ def register_settings_tool(mcp: FastMCP) -> None:
|
|||||||
if action not in ALL_ACTIONS:
|
if action not in ALL_ACTIONS:
|
||||||
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
||||||
|
|
||||||
if action in DESTRUCTIVE_ACTIONS and not confirm:
|
await gate_destructive_action(
|
||||||
raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")
|
ctx,
|
||||||
|
action,
|
||||||
|
DESTRUCTIVE_ACTIONS,
|
||||||
|
confirm,
|
||||||
|
"Configure UPS monitoring. This will overwrite the current UPS daemon settings.",
|
||||||
|
)
|
||||||
|
|
||||||
with tool_error_handler("settings", action, logger):
|
with tool_error_handler("settings", action, logger):
|
||||||
logger.info(f"Executing unraid_settings action={action}")
|
logger.info(f"Executing unraid_settings action={action}")
|
||||||
|
|||||||
@@ -7,11 +7,12 @@ log files, and log content retrieval.
|
|||||||
import os
|
import os
|
||||||
from typing import Any, Literal, get_args
|
from typing import Any, Literal, get_args
|
||||||
|
|
||||||
from fastmcp import FastMCP
|
from fastmcp import Context, FastMCP
|
||||||
|
|
||||||
from ..config.logging import logger
|
from ..config.logging import logger
|
||||||
from ..core.client import DISK_TIMEOUT, make_graphql_request
|
from ..core.client import DISK_TIMEOUT, make_graphql_request
|
||||||
from ..core.exceptions import ToolError, tool_error_handler
|
from ..core.exceptions import ToolError, tool_error_handler
|
||||||
|
from ..core.guards import gate_destructive_action
|
||||||
from ..core.utils import format_bytes
|
from ..core.utils import format_bytes
|
||||||
|
|
||||||
|
|
||||||
@@ -88,6 +89,7 @@ def register_storage_tool(mcp: FastMCP) -> None:
|
|||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def unraid_storage(
|
async def unraid_storage(
|
||||||
action: STORAGE_ACTIONS,
|
action: STORAGE_ACTIONS,
|
||||||
|
ctx: Context | None = None,
|
||||||
disk_id: str | None = None,
|
disk_id: str | None = None,
|
||||||
log_path: str | None = None,
|
log_path: str | None = None,
|
||||||
tail_lines: int = 100,
|
tail_lines: int = 100,
|
||||||
@@ -110,8 +112,14 @@ def register_storage_tool(mcp: FastMCP) -> None:
|
|||||||
if action not in ALL_ACTIONS:
|
if action not in ALL_ACTIONS:
|
||||||
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}")
|
||||||
|
|
||||||
if action in DESTRUCTIVE_ACTIONS and not confirm:
|
await gate_destructive_action(
|
||||||
raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")
|
ctx,
|
||||||
|
action,
|
||||||
|
DESTRUCTIVE_ACTIONS,
|
||||||
|
confirm,
|
||||||
|
f"Back up flash drive to **{remote_name}:{destination_path}**. "
|
||||||
|
"Existing backups at this destination will be overwritten.",
|
||||||
|
)
|
||||||
|
|
||||||
if action == "disk_details" and not disk_id:
|
if action == "disk_details" and not disk_id:
|
||||||
raise ToolError("disk_id is required for 'disk_details' action")
|
raise ToolError("disk_id is required for 'disk_details' action")
|
||||||
|
|||||||
@@ -6,11 +6,12 @@ including start, stop, pause, resume, force stop, reboot, and reset.
|
|||||||
|
|
||||||
from typing import Any, Literal, get_args
|
from typing import Any, Literal, get_args
|
||||||
|
|
||||||
from fastmcp import FastMCP
|
from fastmcp import Context, FastMCP
|
||||||
|
|
||||||
from ..config.logging import logger
|
from ..config.logging import logger
|
||||||
from ..core.client import make_graphql_request
|
from ..core.client import make_graphql_request
|
||||||
from ..core.exceptions import ToolError, tool_error_handler
|
from ..core.exceptions import ToolError, tool_error_handler
|
||||||
|
from ..core.guards import gate_destructive_action
|
||||||
|
|
||||||
|
|
||||||
QUERIES: dict[str, str] = {
|
QUERIES: dict[str, str] = {
|
||||||
@@ -88,6 +89,7 @@ def register_vm_tool(mcp: FastMCP) -> None:
|
|||||||
@mcp.tool()
|
@mcp.tool()
|
||||||
async def unraid_vm(
|
async def unraid_vm(
|
||||||
action: VM_ACTIONS,
|
action: VM_ACTIONS,
|
||||||
|
ctx: Context | None = None,
|
||||||
vm_id: str | None = None,
|
vm_id: str | None = None,
|
||||||
confirm: bool = False,
|
confirm: bool = False,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
@@ -110,8 +112,16 @@ def register_vm_tool(mcp: FastMCP) -> None:
|
|||||||
if action != "list" and not vm_id:
|
if action != "list" and not vm_id:
|
||||||
raise ToolError(f"vm_id is required for '{action}' action")
|
raise ToolError(f"vm_id is required for '{action}' action")
|
||||||
|
|
||||||
if action in DESTRUCTIVE_ACTIONS and not confirm:
|
await gate_destructive_action(
|
||||||
raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")
|
ctx,
|
||||||
|
action,
|
||||||
|
DESTRUCTIVE_ACTIONS,
|
||||||
|
confirm,
|
||||||
|
{
|
||||||
|
"force_stop": f"Force stop VM **{vm_id}**. Unsaved data may be lost.",
|
||||||
|
"reset": f"Reset VM **{vm_id}**. This is a hard reset — unsaved data may be lost.",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
with tool_error_handler("vm", action, logger):
|
with tool_error_handler("vm", action, logger):
|
||||||
logger.info(f"Executing unraid_vm action={action}")
|
logger.info(f"Executing unraid_vm action={action}")
|
||||||
|
|||||||
Reference in New Issue
Block a user