diff --git a/tests/http_layer/test_request_construction.py b/tests/http_layer/test_request_construction.py index 9c095cd..c0f5196 100644 --- a/tests/http_layer/test_request_construction.py +++ b/tests/http_layer/test_request_construction.py @@ -576,7 +576,7 @@ class TestVMToolRequests: @respx.mock async def test_force_stop_requires_confirm(self) -> None: 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") @respx.mock @@ -593,7 +593,7 @@ class TestVMToolRequests: @respx.mock async def test_reset_requires_confirm(self) -> None: 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") @respx.mock @@ -880,7 +880,7 @@ class TestNotificationsToolRequests: @respx.mock async def test_delete_requires_confirm(self) -> None: 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") @respx.mock @@ -990,7 +990,7 @@ class TestRCloneToolRequests: @respx.mock async def test_delete_remote_requires_confirm(self) -> None: 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") @respx.mock diff --git a/tests/test_notifications.py b/tests/test_notifications.py index 4472c4a..c39e989 100644 --- a/tests/test_notifications.py +++ b/tests/test_notifications.py @@ -40,12 +40,12 @@ def _make_tool(): class TestNotificationsValidation: async def test_delete_requires_confirm(self, _mock_graphql: AsyncMock) -> None: 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") async def test_delete_archived_requires_confirm(self, _mock_graphql: AsyncMock) -> None: tool_fn = _make_tool() - with pytest.raises(ToolError, match="destructive"): + with pytest.raises(ToolError, match="not confirmed"): await tool_fn(action="delete_archived") async def test_create_requires_fields(self, _mock_graphql: AsyncMock) -> None: diff --git a/tests/test_rclone.py b/tests/test_rclone.py index c5a7103..e413c8a 100644 --- a/tests/test_rclone.py +++ b/tests/test_rclone.py @@ -22,7 +22,7 @@ def _make_tool(): class TestRcloneValidation: async def test_delete_requires_confirm(self) -> None: 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") async def test_create_requires_fields(self) -> None: diff --git a/tests/test_storage.py b/tests/test_storage.py index 224878b..86c6e64 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -290,7 +290,7 @@ class TestStorageNetworkErrors: class TestStorageFlashBackup: async def test_flash_backup_requires_confirm(self, _mock_graphql: AsyncMock) -> None: tool_fn = _make_tool() - with pytest.raises(ToolError, match="destructive"): + with pytest.raises(ToolError, match="not confirmed"): await tool_fn( action="flash_backup", remote_name="r", source_path="/boot", destination_path="r:b" ) diff --git a/tests/test_vm.py b/tests/test_vm.py index 7680a5d..24cd4b7 100644 --- a/tests/test_vm.py +++ b/tests/test_vm.py @@ -31,7 +31,7 @@ class TestVmValidation: async def test_destructive_actions_require_confirm(self, _mock_graphql: AsyncMock) -> None: tool_fn = _make_tool() 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") async def test_destructive_vm_id_check_before_confirm(self, _mock_graphql: AsyncMock) -> None: diff --git a/unraid_mcp/tools/notifications.py b/unraid_mcp/tools/notifications.py index 7ad7fb6..52e4006 100644 --- a/unraid_mcp/tools/notifications.py +++ b/unraid_mcp/tools/notifications.py @@ -6,11 +6,12 @@ creating, archiving, and deleting system notifications. from typing import Any, Literal, get_args -from fastmcp import FastMCP +from fastmcp import Context, FastMCP from ..config.logging import logger from ..core.client import make_graphql_request from ..core.exceptions import ToolError, tool_error_handler +from ..core.guards import gate_destructive_action QUERIES: dict[str, str] = { @@ -143,6 +144,7 @@ def register_notifications_tool(mcp: FastMCP) -> None: @mcp.tool() async def unraid_notifications( action: NOTIFICATION_ACTIONS, + ctx: Context | None = None, confirm: bool = False, notification_id: 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: raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}") - if action in DESTRUCTIVE_ACTIONS and not confirm: - raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.") + await gate_destructive_action( + 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). # Invalid values waste a rate-limited request and may leak schema details in errors. diff --git a/unraid_mcp/tools/rclone.py b/unraid_mcp/tools/rclone.py index 7b4b846..4a8e706 100644 --- a/unraid_mcp/tools/rclone.py +++ b/unraid_mcp/tools/rclone.py @@ -7,11 +7,12 @@ cloud storage remotes (S3, Google Drive, Dropbox, FTP, etc.). import re from typing import Any, Literal, get_args -from fastmcp import FastMCP +from fastmcp import Context, FastMCP from ..config.logging import logger from ..core.client import make_graphql_request from ..core.exceptions import ToolError, tool_error_handler +from ..core.guards import gate_destructive_action QUERIES: dict[str, str] = { @@ -110,6 +111,7 @@ def register_rclone_tool(mcp: FastMCP) -> None: @mcp.tool() async def unraid_rclone( action: RCLONE_ACTIONS, + ctx: Context | None = None, confirm: bool = False, name: 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: raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}") - if action in DESTRUCTIVE_ACTIONS and not confirm: - raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.") + await gate_destructive_action( + ctx, + action, + DESTRUCTIVE_ACTIONS, + confirm, + f"Delete rclone remote **{name}**. This cannot be undone.", + ) with tool_error_handler("rclone", action, logger): logger.info(f"Executing unraid_rclone action={action}") diff --git a/unraid_mcp/tools/settings.py b/unraid_mcp/tools/settings.py index f3291a8..6e0e1dd 100644 --- a/unraid_mcp/tools/settings.py +++ b/unraid_mcp/tools/settings.py @@ -6,11 +6,12 @@ configuration and UPS monitoring. from typing import Any, Literal, get_args -from fastmcp import FastMCP +from fastmcp import Context, FastMCP from ..config.logging import logger from ..core.client import make_graphql_request from ..core.exceptions import ToolError, tool_error_handler +from ..core.guards import gate_destructive_action MUTATIONS: dict[str, str] = { @@ -51,6 +52,7 @@ def register_settings_tool(mcp: FastMCP) -> None: @mcp.tool() async def unraid_settings( action: SETTINGS_ACTIONS, + ctx: Context | None = None, confirm: bool = False, settings_input: 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: raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}") - if action in DESTRUCTIVE_ACTIONS and not confirm: - raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.") + await gate_destructive_action( + ctx, + action, + DESTRUCTIVE_ACTIONS, + confirm, + "Configure UPS monitoring. This will overwrite the current UPS daemon settings.", + ) with tool_error_handler("settings", action, logger): logger.info(f"Executing unraid_settings action={action}") diff --git a/unraid_mcp/tools/storage.py b/unraid_mcp/tools/storage.py index 1d910b9..62c1077 100644 --- a/unraid_mcp/tools/storage.py +++ b/unraid_mcp/tools/storage.py @@ -7,11 +7,12 @@ log files, and log content retrieval. import os from typing import Any, Literal, get_args -from fastmcp import FastMCP +from fastmcp import Context, FastMCP from ..config.logging import logger from ..core.client import DISK_TIMEOUT, make_graphql_request from ..core.exceptions import ToolError, tool_error_handler +from ..core.guards import gate_destructive_action from ..core.utils import format_bytes @@ -88,6 +89,7 @@ def register_storage_tool(mcp: FastMCP) -> None: @mcp.tool() async def unraid_storage( action: STORAGE_ACTIONS, + ctx: Context | None = None, disk_id: str | None = None, log_path: str | None = None, tail_lines: int = 100, @@ -110,8 +112,14 @@ def register_storage_tool(mcp: FastMCP) -> None: if action not in ALL_ACTIONS: raise ToolError(f"Invalid action '{action}'. Must be one of: {sorted(ALL_ACTIONS)}") - if action in DESTRUCTIVE_ACTIONS and not confirm: - raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.") + await gate_destructive_action( + 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: raise ToolError("disk_id is required for 'disk_details' action") diff --git a/unraid_mcp/tools/virtualization.py b/unraid_mcp/tools/virtualization.py index f54baf7..05cebac 100644 --- a/unraid_mcp/tools/virtualization.py +++ b/unraid_mcp/tools/virtualization.py @@ -6,11 +6,12 @@ including start, stop, pause, resume, force stop, reboot, and reset. from typing import Any, Literal, get_args -from fastmcp import FastMCP +from fastmcp import Context, FastMCP from ..config.logging import logger from ..core.client import make_graphql_request from ..core.exceptions import ToolError, tool_error_handler +from ..core.guards import gate_destructive_action QUERIES: dict[str, str] = { @@ -88,6 +89,7 @@ def register_vm_tool(mcp: FastMCP) -> None: @mcp.tool() async def unraid_vm( action: VM_ACTIONS, + ctx: Context | None = None, vm_id: str | None = None, confirm: bool = False, ) -> dict[str, Any]: @@ -110,8 +112,16 @@ def register_vm_tool(mcp: FastMCP) -> None: if action != "list" and not vm_id: raise ToolError(f"vm_id is required for '{action}' action") - if action in DESTRUCTIVE_ACTIONS and not confirm: - raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.") + await gate_destructive_action( + 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): logger.info(f"Executing unraid_vm action={action}")