File 0001-feat-security-add-write-enable-flag-to-control-state.patch of Package mcp-server-uyuni

From c0300db9db241f1fa9ccdfb9f21bdb8f32425d00 Mon Sep 17 00:00:00 2001
From: Jordi Massaguer Pla <jmassaguerpla@suse.com>
Date: Sun, 27 Jul 2025 21:58:56 +0200
Subject: [PATCH 1/7] feat(security): add write-enable flag to control
 state-changing tools

To enhance security and prevent accidental or unauthorized modifications, the server now operates in a read-only mode by default. All tools that perform write operations (via POST requests) are disabled unless explicitly enabled.

This is implemented through:
- A new environment variable, `UYUNI_MCP_WRITE_TOOLS_ENABLED`, which must be set to 'true' to activate write capabilities.
- A `@write_tool` decorator that conditionally registers tools based on this flag, making them invisible to the LLM when disabled.
- A secondary safety check in the `_call_uyuni_api` helper to block any direct POST requests if the flag is not enabled.
- Updated documentation (README.md, SECURITY.md, CONTRIBUTING.md, TEST_CASES.md) to reflect the new security model and testing requirements.

This provides a robust, layered approach to securing the server against unintended state changes.

Signed-off-by: Jordi Massaguer Pla <jmassaguerpla@suse.com>
---
 CONTRIBUTING.md                | 12 +++++++++++
 README.md                      |  5 +++++
 SECURITY.md                    | 17 +++++++++++----
 TEST_CASES.md                  |  2 ++
 src/mcp_server_uyuni/server.py | 39 ++++++++++++++++++++++++++++------
 5 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 2859551..18acb3b 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -67,6 +67,18 @@ chmod +x .git/hooks/prepare-commit-msg
 ```
 This hook will prepend a basic template to your commit message editor.
 
+## Secure Write Operations with a Feature Flag
+
+To enhance security, the server operates in a read-only mode by default. Any tool that performs a write operation (e.g., using a POST request to the Uyuni API) should be disabled unless explicitly enabled by the server operator.
+
+This is enforced using two layers of protection:
+
+Conditional Tool Registration with @write_tool: A custom decorator, @write_tool(), is provided. This decorator only registers the tool with the MCP server if the UYUNI_MCP_WRITE_TOOLS_ENABLED environment variable is set to 'true'. If the variable is false or not set, the tool is never registered and is completely invisible to the LLM. This is the primary mechanism for controlling access to write operations. Usage: Simply replace @mcp.tool() with @write_tool() for any function that modifies state.
+
+Example: @write_tool() async def remove_system(system_identifier: Union[str, int], confirm: bool = False) -> str: """ Removes a system. This is a write operation. """ # ... implementation ...
+
+API Helper Safety Check: As a second layer of defense, the _call_uyuni_api helper function contains a check that will block any request with the POST method if UYUNI_MCP_WRITE_TOOLS_ENABLED is not true. This prevents accidental exposure of a write action if a developer forgets to use the @write_tool decorator.
+
 ## Learnings and Patterns for MCP Tool Development
 
 When developing tools for an MCP server, the primary consumer is an LLM. This requires a slightly different approach than traditional API design where a human is reading documentation. The following are some best practices and patterns we've learned that help the LLM understand and use the tools more effectively.
diff --git a/README.md b/README.md
index e5d2b70..0c05fad 100644
--- a/README.md
+++ b/README.md
@@ -39,12 +39,17 @@ UYUNI_USER=admin
 UYUNI_PASS=admin
 # Optional: Set to 'false' to disable SSL certificate verification. Defaults to 'true'.
 # UYUNI_SSL_VERIFY=false
+# Optional: Set to 'true' to enable tools that perform write actions (e.g., POST requests). Defaults to 'false'.
+# UYUNI_MCP_WRITE_TOOLS_ENABLED=false
 # Optional: Set the transport protocol. Can be 'stdio' (default) or 'http'.
 # UYUNI_MCP_TRANSPORT=stdio
 
 > [!WARNING]
 > **Security Note on HTTP Transport:** When `UYUNI_MCP_TRANSPORT` is set to `http`, the server runs without authentication. This means any client with network access can execute commands. Only use this mode in a trusted, isolated network environment. For more details, see the Security Policy.
 
+> [!WARNING]
+> **Security Note on Write Tools:** Enabling `UYUNI_MCP_WRITE_TOOLS_ENABLED` allows the execution of state-changing and potentially destructive actions (e.g., removing systems, applying updates). When combined with `UYUNI_MCP_TRANSPORT=http`, this risk is amplified, as any client with network access can perform these actions. Only enable write tools in a trusted environment.
+
 # Optional: Set the path for the server log file. Defaults to logging to the console.
 # UYUNI_MCP_LOG_FILE_PATH=/var/log/mcp-server-uyuni.log
 UYUNI_SSH_PRIV_KEY="-----BEGIN OPENSSH PRIVATE KEY-----\n..."
diff --git a/SECURITY.md b/SECURITY.md
index be7d20a..0ffe2e2 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -20,20 +20,29 @@ UYUNI_PASS=<your_uyuni_password>
 *   **Critical:** This `config` file **must not be shared or committed to version control**. It should be treated as highly confidential.
 *   **Usage:** The MCP server imports this file as an environment file to obtain the necessary credentials for interacting with the Uyuni server.
 
-## MCP Server Authentication
+## MCP Server Authentication and Authorization
 
 *   **No Authentication:** Currently, the MCP server itself does not implement any form of authentication or authorization.
 *   **Access Implication:** Anyone who has access to the environment where the MCP server can be run, can execute any of the tools and actions provided by the MCP server.
-*
+
+### Write Tool Enablement (Default: Disabled)
+
+By default, the server runs in a **read-only** mode for safety. All tools that can change the state of the Uyuni server or the systems it manages (e.g., `remove_system`, `schedule_apply_pending_updates_to_system`) are disabled and not visible to the language model.
+
+To enable these "write tools", you must explicitly set the `UYUNI_MCP_WRITE_TOOLS_ENABLED` environment variable to `true`.
+
+> [!WARNING]
+> Enabling write tools is a significant security decision. It grants any client that can connect to the MCP server the ability to perform destructive actions. This risk is amplified when using the `http` transport.
+
 ### Transport Layer Implications
 
 The server can be run with two different transport layers, configured via the `UYUNI_MCP_TRANSPORT` environment variable:
 
 *   **`stdio` (default):** In this mode, the server communicates over standard input/output. Access is limited to processes that can execute the server binary directly on the host machine. This is the most secure mode of operation.
-*   **`http`:** In this mode, the server runs as an HTTP service. Because there is no authentication layer, **any client with network access to the server's host and port can execute any tool**. This includes destructive actions like removing systems or applying updates.
+*   **`http`:** In this mode, the server runs as an HTTP service. Because there is no authentication layer, **any client with network access to the server's host and port can execute any tool**.
 
 > [!WARNING]
-> Running the server with the `http` transport in an untrusted network environment poses a significant security risk. It is recommended to use this mode only in isolated, trusted networks or to implement network-level controls (e.g., firewall rules) to restrict access to authorized clients only.
+> Running the server with `UYUNI_MCP_TRANSPORT=http` and `UYUNI_MCP_WRITE_TOOLS_ENABLED=true` in an untrusted network environment poses a significant security risk. This combination allows any client with network access to perform destructive actions without authentication. It is strongly recommended to use this configuration only in isolated, trusted networks or to implement network-level controls (e.g., firewall rules) to restrict access to authorized clients only.
 
 ## Tool Execution and Confirmation
 
diff --git a/TEST_CASES.md b/TEST_CASES.md
index 66a8e1b..b68e6d1 100644
--- a/TEST_CASES.md
+++ b/TEST_CASES.md
@@ -9,6 +9,8 @@ This document tracks the manual test cases executed for different versions/tags
 
 This document tracks the manual test cases executed for different versions/tags of the `mcp-server-uyuni` project.
 
+To run any tests that perform write actions, the UYUNI_MCP_WRITE_TOOLS_ENABLED environment variable must be set to true.
+
 ## Test Case Table
 
 | Test Case ID | Tool / Feature Tested                      | Question / Prompt                                           | Expected Result                                                                                                                                                           | Status (v0.1.0) | Status (v0.2.0) | Status (v0.2.1) | Status (v0.3.0) | Status (v0.4.0) | Notes / Bug ID |
diff --git a/src/mcp_server_uyuni/server.py b/src/mcp_server_uyuni/server.py
index 5a2cb30..8ca3ff8 100644
--- a/src/mcp_server_uyuni/server.py
+++ b/src/mcp_server_uyuni/server.py
@@ -47,6 +47,7 @@ UYUNI_USER = os.environ.get('UYUNI_USER')
 UYUNI_PASS = os.environ.get('UYUNI_PASS')
 # UYUNI_MCP_SSL_VERIFY is optional and defaults to True. Set to 'false', '0', or 'no' to disable.
 UYUNI_MCP_SSL_VERIFY = os.environ.get('UYUNI_MCP_SSL_VERIFY', 'true').lower() not in ('false', '0', 'no')
+UYUNI_MCP_WRITE_TOOLS_ENABLED = os.environ.get('UYUNI_MCP_WRITE_TOOLS_ENABLED', 'false').lower() in ('true', '1', 'yes')
 UYUNI_MCP_TRANSPORT = os.environ.get('UYUNI_MCP_TRANSPORT', 'stdio')
 UYUNI_MCP_LOG_FILE_PATH = os.environ.get('UYUNI_MCP_LOG_FILE_PATH') # Defaults to None if not set
 
@@ -55,6 +56,24 @@ logger = get_logger(log_file=UYUNI_MCP_LOG_FILE_PATH, transport=UYUNI_MCP_TRANSP
 # Sentinel object to indicate an expected timeout for long-running actions
 TIMEOUT_HAPPENED = object()
 
+def write_tool(*decorator_args, **decorator_kwargs):
+    """
+    A decorator that registers a function as an MCP tool only if write
+    tools are enabled via the UYUNI_MCP_WRITE_TOOLS_ENABLED environment variable.
+    """
+    # 2. This is the actual decorator that gets applied to the tool function.
+    def decorator(func):
+        if UYUNI_MCP_WRITE_TOOLS_ENABLED:
+            # 3a. If enabled, it applies the @mcp.tool() decorator, registering the function.
+            return mcp.tool(*decorator_args, **decorator_kwargs)(func)
+        
+        # 3b. If disabled, it does nothing and just returns the original,
+        #     un-decorated function. It is never registered.
+        return func
+    
+    # 1. The factory returns the decorator.
+    return decorator
+
 async def _call_uyuni_api(
     client: httpx.AsyncClient,
     method: str,
@@ -72,6 +91,14 @@ async def _call_uyuni_api(
     Handles login, request execution, error handling, and basic response parsing.
     """
 
+    # Safety check: Do not allow POST requests if write tools are disabled.
+    # This acts as a secondary guard after the @write_tool decorator.
+    if method.upper() == 'POST' and not UYUNI_MCP_WRITE_TOOLS_ENABLED:
+        error_msg = (f"Attempted to call a write API ({api_path}) while write tools are disabled. "
+                     "Please set UYUNI_MCP_WRITE_TOOLS_ENABLED to 'true' to enable them.")
+        logger.error(error_msg)
+        return error_msg
+
     if perform_login:
         login_data = {"login": UYUNI_USER, "password": UYUNI_PASS}
         try:
@@ -530,7 +557,7 @@ async def check_all_systems_for_updates(ctx: Context) -> List[Dict[str, Any]]:
     print(f"Finished checking systems. Found {len(systems_with_updates)} systems with updates.")
     return systems_with_updates
 
-@mcp.tool()
+@write_tool()
 async def schedule_apply_pending_updates_to_system(system_identifier: Union[str, int], ctx: Context, confirm: bool = False) -> str:
 
     """
@@ -604,7 +631,7 @@ async def schedule_apply_pending_updates_to_system(system_identifier: Union[str,
                  print(f"Failed to schedule errata for system {system_identifier} or unexpected API response format. Result: {api_result}")
             return ""
 
-@mcp.tool()
+@write_tool()
 async def schedule_apply_specific_update(system_identifier: Union[str, int], errata_id: int, ctx: Context, confirm: bool = False) -> str:
 
     """
@@ -659,7 +686,7 @@ async def schedule_apply_specific_update(system_identifier: Union[str, int], err
                 print(f"Failed to schedule specific update (errata ID: {errata_id}) for system {system_identifier} or unexpected API result format. Result: {api_result}")
             return ""
 
-@mcp.tool()
+@write_tool()
 async def add_system(
     host: str,
     ctx: Context,
@@ -780,7 +807,7 @@ async def add_system(
         return f"System {host} was NOT successfully scheduled to be added. Check server logs."
 
 
-@mcp.tool()
+@write_tool()
 async def remove_system(system_identifier: Union[str, int], ctx: Context, cleanup: bool = True, confirm: bool = False) -> str:
     """
     Removes/deletes a system from being managed by Uyuni.
@@ -987,7 +1014,7 @@ async def get_systems_needing_reboot(ctx: Context) -> List[Dict[str, Any]]:
 
     return systems_needing_reboot_list
 
-@mcp.tool()
+@write_tool()
 async def schedule_system_reboot(system_identifier: Union[str, int], ctx:Context, confirm: bool = False) -> str:
 
     """
@@ -1089,7 +1116,7 @@ async def list_all_scheduled_actions(ctx: Context) -> List[Dict[str, Any]]:
             print(f"Warning: Expected a list for all scheduled actions, but received: {type(api_result)}")
     return processed_actions_list
 
-@mcp.tool()
+@write_tool()
 async def cancel_action(action_id: int, ctx: Context, confirm: bool = False) -> str:
     """
     Cancels a specified action on the Uyuni server.
-- 
2.43.0

openSUSE Build Service is sponsored by