Files
unraid-mcp/.plan.md
Jacob Magar 184b8aca1c fix: address 18 CRITICAL+HIGH PR review comments
**Critical Fixes (7 issues):**
- Fix GraphQL schema field names in users tool (role→roles, remove email)
- Fix GraphQL mutation signatures (addUserInput, deleteUser input)
- Fix dict(None) TypeError guards in users tool (use `or {}` pattern)
- Fix FastAPI version constraint (0.116.1→0.115.0)
- Fix WebSocket SSL context handling (support CA bundles, bool, and None)
- Fix critical disk threshold treated as warning (split counters)

**High Priority Fixes (11 issues):**
- Fix Docker update/remove action response field mapping
- Fix path traversal vulnerability in log validation (normalize paths)
- Fix deleteApiKeys validation (check response before success)
- Fix rclone create_remote validation (check response)
- Fix keys input_data type annotation (dict[str, Any])
- Fix VM domain/domains fallback restoration

**Changes by file:**
- unraid_mcp/tools/docker.py: Response field mapping
- unraid_mcp/tools/info.py: Split critical/warning counters
- unraid_mcp/tools/storage.py: Path normalization for traversal protection
- unraid_mcp/tools/users.py: GraphQL schema + null handling
- unraid_mcp/tools/keys.py: Validation + type annotations
- unraid_mcp/tools/rclone.py: Response validation
- unraid_mcp/tools/virtualization.py: Domain fallback
- unraid_mcp/subscriptions/manager.py: SSL context creation
- pyproject.toml: FastAPI version fix
- tests/*: New tests for all fixes

**Review threads resolved:**
PRRT_kwDOO6Hdxs5uu70L, PRRT_kwDOO6Hdxs5uu70O, PRRT_kwDOO6Hdxs5uu70V,
PRRT_kwDOO6Hdxs5uu70e, PRRT_kwDOO6Hdxs5uu70i, PRRT_kwDOO6Hdxs5uu7zn,
PRRT_kwDOO6Hdxs5uu7z_, PRRT_kwDOO6Hdxs5uu7sI, PRRT_kwDOO6Hdxs5uu7sJ,
PRRT_kwDOO6Hdxs5uu7sK, PRRT_kwDOO6Hdxs5uu7Tk, PRRT_kwDOO6Hdxs5uu7Tn,
PRRT_kwDOO6Hdxs5uu7Tr, PRRT_kwDOO6Hdxs5uu7Ts, PRRT_kwDOO6Hdxs5uu7Tu,
PRRT_kwDOO6Hdxs5uu7Tv, PRRT_kwDOO6Hdxs5uu7Tw, PRRT_kwDOO6Hdxs5uu7Tx

All tests passing.

Co-authored-by: docker-fixer <agent@pr-fixes>
Co-authored-by: info-fixer <agent@pr-fixes>
Co-authored-by: storage-fixer <agent@pr-fixes>
Co-authored-by: users-fixer <agent@pr-fixes>
Co-authored-by: config-fixer <agent@pr-fixes>
Co-authored-by: websocket-fixer <agent@pr-fixes>
Co-authored-by: keys-rclone-fixer <agent@pr-fixes>
Co-authored-by: vm-fixer <agent@pr-fixes>
2026-02-15 16:42:58 -05:00

18 KiB

Implementation Plan: mcporter Integration Tests + Destructive Action Gating

Date: 2026-02-15 Status: Awaiting Approval Estimated Effort: 8-12 hours

Overview

Implement comprehensive integration testing using mcporter CLI to validate all 86 tool actions (after removing 4 destructive array operations) against live Unraid servers, plus add environment variable gates for remaining destructive actions to prevent accidental operations.

Requirements

  1. Remove destructive array operations - start, stop, shutdown, reboot should not be exposed via MCP
  2. Add per-tool environment variable gates - UNRAID_ALLOW_*_DESTRUCTIVE flags for remaining destructive actions
  3. Build mcporter test suite - Real end-to-end testing of all 86 actions against live servers (tootie/shart)
  4. Document all actions - Comprehensive action catalog with test specifications

Architecture Changes

1. Settings Infrastructure (Pydantic-based)

File: unraid_mcp/config/settings.py

  • Migrate from simple os.getenv() to Pydantic BaseSettings
  • Add 7 destructive action gate flags (all default to False for safety):
    • allow_docker_destructive (docker remove)
    • allow_vm_destructive (vm force_stop, reset)
    • allow_notifications_destructive (delete, delete_archived)
    • allow_rclone_destructive (delete_remote)
    • allow_users_destructive (user delete)
    • allow_keys_destructive (key delete)
    • allow_array_destructive (REMOVED - no longer needed after task 1)
  • Add get_config_summary() method showing gate status
  • Maintain backwards compatibility via module-level exports

Dependencies: Add pydantic-settings to pyproject.toml

2. Tool Implementation Pattern

Pattern for all tools with destructive actions:

from ..config.settings import settings

# In tool function:
if action in DESTRUCTIVE_ACTIONS:
    # Check 1: Environment variable gate (first line of defense)
    if not settings.allow_{tool}_destructive:
        raise ToolError(
            f"Destructive {tool} action '{action}' is disabled. "
            f"Set UNRAID_ALLOW_{TOOL}_DESTRUCTIVE=true to enable. "
            f"This is a safety gate to prevent accidental operations."
        )

    # Check 2: Runtime confirmation (second line of defense)
    if not confirm:
        raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")

Tools requiring updates:

  • unraid_mcp/tools/docker.py (1 action: remove)
  • unraid_mcp/tools/virtualization.py (2 actions: force_stop, reset)
  • unraid_mcp/tools/notifications.py (2 actions: delete, delete_archived)
  • unraid_mcp/tools/rclone.py (1 action: delete_remote)
  • unraid_mcp/tools/users.py (1 action: delete)
  • unraid_mcp/tools/keys.py (1 action: delete)

3. mcporter Integration Test Suite

New Directory Structure:

tests/integration/
├── helpers/
│   ├── mcporter.sh          # mcporter wrapper (call_tool, call_destructive, get_field)
│   ├── validation.sh        # Response validation (assert_fields, assert_equals, assert_success)
│   └── reporting.sh         # Test reporting (init_report, record_test, generate_summary)
├── tools/
│   ├── test_health.sh       # 3 actions
│   ├── test_info.sh         # 19 actions
│   ├── test_storage.sh      # 6 actions
│   ├── test_docker.sh       # 15 actions
│   ├── test_vm.sh           # 9 actions
│   ├── test_notifications.sh # 9 actions
│   ├── test_rclone.sh       # 4 actions
│   ├── test_users.sh        # 8 actions
│   ├── test_keys.sh         # 5 actions
│   └── test_array.sh        # 8 actions (after removal)
├── run-all.sh               # Master test runner (parallel/sequential)
├── run-tool.sh              # Single tool runner
└── README.md                # Integration test documentation

mcporter Configuration: config/mcporter.json

{
  "mcpServers": {
    "unraid-tootie": {
      "command": "uv",
      "args": ["run", "unraid-mcp-server"],
      "env": {
        "UNRAID_API_URL": "https://myunraid.net:31337/graphql",
        "UNRAID_API_KEY": "${UNRAID_TOOTIE_API_KEY}",
        "UNRAID_VERIFY_SSL": "false",
        "UNRAID_MCP_TRANSPORT": "stdio"
      },
      "cwd": "/home/jmagar/workspace/unraid-mcp"
    },
    "unraid-shart": {
      "command": "uv",
      "args": ["run", "unraid-mcp-server"],
      "env": {
        "UNRAID_API_URL": "http://100.118.209.1/graphql",
        "UNRAID_API_KEY": "${UNRAID_SHART_API_KEY}",
        "UNRAID_VERIFY_SSL": "false",
        "UNRAID_MCP_TRANSPORT": "stdio"
      },
      "cwd": "/home/jmagar/workspace/unraid-mcp"
    }
  }
}

Implementation Tasks

Task 1: Remove Destructive Array Operations

Files:

  • unraid_mcp/tools/array.py
  • tests/test_array.py

Changes:

  1. Remove from MUTATIONS dict:
    • start (lines 24-28)
    • stop (lines 29-33)
    • shutdown (lines 69-73)
    • reboot (lines 74-78)
  2. Remove from DESTRUCTIVE_ACTIONS set (line 81) - set becomes empty {}
  3. Remove from ARRAY_ACTIONS Literal type (lines 85-86)
  4. Update docstring removing these 4 actions (lines 105-106, 115-116)
  5. Remove tests for these actions in tests/test_array.py

Acceptance:

  • Array tool has 8 actions (down from 12)
  • DESTRUCTIVE_ACTIONS is empty set
  • Tests pass for remaining actions
  • Removed mutations are not callable

Task 2: Add Pydantic Settings with Destructive Gates

Files:

  • unraid_mcp/config/settings.py
  • pyproject.toml
  • .env.example

Changes:

  1. Add dependency: pydantic-settings>=2.12 in pyproject.toml dependencies

  2. Update settings.py:

    • Import BaseSettings from pydantic_settings
    • Create UnraidSettings class with all config fields
    • Add 6 destructive gate fields (all default to False):
      • allow_docker_destructive: bool = Field(default=False, ...)
      • allow_vm_destructive: bool = Field(default=False, ...)
      • allow_notifications_destructive: bool = Field(default=False, ...)
      • allow_rclone_destructive: bool = Field(default=False, ...)
      • allow_users_destructive: bool = Field(default=False, ...)
      • allow_keys_destructive: bool = Field(default=False, ...)
    • Add get_config_summary() method including gate status
    • Instantiate global settings = UnraidSettings()
    • Keep backwards compatibility exports
  3. Update .env.example: Add section documenting all destructive gates

Acceptance:

  • settings instance loads successfully
  • All gate fields default to False
  • get_config_summary() shows gate status
  • Backwards compatibility maintained (existing code still works)

Task 3: Update Tools with Environment Variable Gates

Files to update:

  • unraid_mcp/tools/docker.py
  • unraid_mcp/tools/virtualization.py
  • unraid_mcp/tools/notifications.py
  • unraid_mcp/tools/rclone.py
  • unraid_mcp/tools/users.py
  • unraid_mcp/tools/keys.py

Pattern for each tool:

  1. Add import: from ..config.settings import settings
  2. Add gate check before confirm check in destructive action handler:
    if action in DESTRUCTIVE_ACTIONS:
        if not settings.allow_{tool}_destructive:
            raise ToolError(
                f"Destructive {tool} action '{action}' is disabled. "
                f"Set UNRAID_ALLOW_{TOOL}_DESTRUCTIVE=true to enable."
            )
        if not confirm:
            raise ToolError(f"Action '{action}' is destructive. Set confirm=True to proceed.")
    
  3. Update tool docstring documenting security requirements

Acceptance (per tool):

  • Destructive action fails with clear error when env var not set
  • Destructive action still requires confirm=True when env var is set
  • Both checks must pass for execution
  • Error messages guide user to correct env var

Task 4: Update Test Suite with Settings Mocking

Files:

  • tests/conftest.py
  • tests/test_docker.py
  • tests/test_vm.py
  • tests/test_notifications.py
  • tests/test_rclone.py
  • tests/test_users.py
  • tests/test_keys.py

Changes:

  1. Add fixtures to conftest.py:

    @pytest.fixture
    def mock_settings():
        # All gates disabled
    
    @pytest.fixture
    def mock_settings_all_enabled(mock_settings):
        # All gates enabled
    
  2. Update each test file:

    • Add mock_settings parameter to fixtures
    • Wrap tool calls with with patch("unraid_mcp.tools.{tool}.settings", mock_settings):
    • Add 3 destructive action tests:
      • Test gate check (env var not set, confirm=True → fails)
      • Test confirm check (env var set, confirm=False → fails)
      • Test success (env var set, confirm=True → succeeds)

Acceptance:

  • All 150 existing tests pass
  • New gate tests cover all destructive actions
  • Tests verify correct error messages
  • Tests use mocked settings (don't rely on actual env vars)

Task 5: Create mcporter Configuration

Files:

  • config/mcporter.json (new)
  • tests/integration/README.md (new)

Changes:

  1. Create config/mcporter.json with tootie and shart server configs
  2. Document how to use mcporter with the server in README
  3. Include instructions for loading credentials from ~/workspace/homelab/.env

Acceptance:

  • mcporter list unraid-tootie shows all tools
  • mcporter call unraid-tootie.unraid_health action=test_connection succeeds
  • Configuration works for both servers

Task 6: Build mcporter Helper Libraries

Files to create:

  • tests/integration/helpers/mcporter.sh
  • tests/integration/helpers/validation.sh
  • tests/integration/helpers/reporting.sh

Functions to implement:

mcporter.sh:

  • call_tool <tool> <action> [params...] - Call tool via mcporter, return JSON
  • call_destructive <tool> <action> <env_var> [params...] - Safe destructive call
  • get_field <json> <jq_path> - Extract field from JSON
  • is_success <json> - Check if response indicates success
  • get_error <json> - Extract error message

validation.sh:

  • assert_fields <json> <field>... - Verify required fields exist
  • assert_equals <json> <field> <expected> - Field value equality
  • assert_matches <json> <field> <pattern> - Field matches regex
  • assert_success <json> - Response indicates success
  • assert_failure <json> [pattern] - Response indicates failure (negative test)

reporting.sh:

  • init_report <tool> - Initialize JSON report file
  • record_test <report> <action> <status> [error] - Record test result
  • generate_summary - Generate console summary from all reports

Acceptance:

  • Helper functions work correctly
  • Error handling is robust
  • Functions are reusable across all tool tests

Task 7: Implement Tool Test Scripts

Files to create:

  • tests/integration/tools/test_health.sh (3 actions)
  • tests/integration/tools/test_info.sh (19 actions)
  • tests/integration/tools/test_storage.sh (6 actions)
  • tests/integration/tools/test_docker.sh (15 actions)
  • tests/integration/tools/test_vm.sh (9 actions)
  • tests/integration/tools/test_notifications.sh (9 actions)
  • tests/integration/tools/test_rclone.sh (4 actions)
  • tests/integration/tools/test_users.sh (8 actions)
  • tests/integration/tools/test_keys.sh (5 actions)
  • tests/integration/tools/test_array.sh (8 actions)

Per-script implementation:

  1. Source helper libraries
  2. Initialize report
  3. Implement test functions for each action:
    • Basic functionality test
    • Response structure validation
    • Parameter validation
    • Destructive action gate tests (if applicable)
  4. Run all tests and record results
  5. Return exit code based on failures

Priority order (implement in this sequence):

  1. test_health.sh - Simplest (3 actions, no destructive)
  2. test_info.sh - Large but straightforward (19 query actions)
  3. test_storage.sh - Moderate (6 query actions)
  4. test_docker.sh - Complex (15 actions, 1 destructive)
  5. test_vm.sh - Complex (9 actions, 2 destructive)
  6. test_notifications.sh - Moderate (9 actions, 2 destructive)
  7. test_rclone.sh - Simple (4 actions, 1 destructive)
  8. test_users.sh - Moderate (8 actions, 1 destructive)
  9. test_keys.sh - Simple (5 actions, 1 destructive)
  10. test_array.sh - Moderate (8 actions, no destructive after removal)

Acceptance:

  • Each script tests all actions for its tool
  • Tests validate response structure
  • Destructive action gates are tested
  • Scripts generate JSON reports
  • Exit code indicates success/failure

Task 8: Build Test Runners

Files to create:

  • tests/integration/run-all.sh
  • tests/integration/run-tool.sh

run-all.sh features:

  • Load credentials from ~/workspace/homelab/.env
  • Support sequential and parallel execution modes
  • Run all 10 tool test scripts
  • Generate summary report
  • Return exit code based on any failures

run-tool.sh features:

  • Accept tool name as argument
  • Load credentials
  • Execute single tool test script
  • Pass through exit code

Acceptance:

  • run-all.sh executes all tool tests
  • Parallel mode works correctly (no race conditions)
  • Summary report shows pass/fail/skip counts
  • run-tool.sh health runs only health tests
  • Exit codes are correct

Task 9: Document Action Catalog

File to create:

  • docs/testing/action-catalog.md

Content:

  • Table of all 86 actions across 10 tools
  • For each action:
    • Tool name
    • Action name
    • Type (query/mutation/compound)
    • Required parameters
    • Optional parameters
    • Destructive? (yes/no + env var if yes)
    • Expected response structure
    • Example mcporter call
    • Validation criteria

Acceptance:

  • All 86 actions documented
  • Specifications are detailed and accurate
  • Examples are runnable
  • Becomes source of truth for test implementation

Task 10: Integration Documentation

Files to create/update:

  • tests/integration/README.md
  • docs/testing/integration-tests.md
  • docs/testing/test-environments.md
  • README.md (add integration test section)

Content:

  • How to run integration tests
  • How to configure mcporter
  • Server setup (tootie/shart)
  • Environment variable gates
  • Destructive action testing
  • CI/CD integration
  • Troubleshooting

Acceptance:

  • Clear setup instructions
  • Examples for common use cases
  • Integration with existing pytest docs
  • CI/CD pipeline documented

Testing Strategy

Unit Tests (pytest - existing)

  • 150 tests across 10 tool modules
  • Mock GraphQL responses
  • Fast, isolated, offline
  • Cover edge cases and error paths

Integration Tests (mcporter - new)

  • 86 tests (one per action)
  • Real Unraid server calls
  • Slow, dependent, online
  • Validate actual API behavior

Test Matrix

Tool Actions pytest Tests mcporter Tests Destructive
health 3 10 3 0
info 19 98 19 0
storage 6 11 6 0
docker 15 28 15 1
vm 9 25 9 2
notifications 9 7 9 2
rclone 4 (pending) 4 1
users 8 (pending) 8 1
keys 5 (pending) 5 1
array 8 26 8 0
TOTAL 86 ~150 86 8

Validation Checklist

Code Changes

  • Array tool has 8 actions (removed start/stop/shutdown/reboot)
  • Settings class with 6 destructive gate flags
  • All 6 tools updated with environment variable gates
  • All 6 tool tests updated with gate test cases
  • All existing 150 pytest tests pass
  • pydantic-settings added to dependencies
  • .env.example updated with gate documentation

Integration Tests

  • mcporter configuration works for both servers
  • All 3 helper libraries implemented
  • All 10 tool test scripts implemented
  • Test runners (run-all, run-tool) work correctly
  • All 86 actions have test coverage
  • Destructive action gates are tested
  • Reports generate correctly

Documentation

  • Action catalog documents all 86 actions
  • Integration test README is clear
  • Environment setup documented
  • CI/CD integration documented
  • Project README updated

Success Criteria

  1. Safety: Destructive actions require both env var AND confirm=True
  2. Coverage: All 86 actions have integration tests
  3. Quality: Clear error messages guide users to correct env vars
  4. Automation: Test suite runs via single command
  5. Documentation: Complete action catalog and testing guide

Risks & Mitigations

Risk: Breaking existing deployments

Impact: HIGH - Users suddenly can't execute destructive actions Mitigation:

  • Clear error messages with exact env var to set
  • Document migration in release notes
  • Default to disabled (safe) but guide users to enable

Risk: Integration tests are flaky

Impact: MEDIUM - CI/CD unreliable Mitigation:

  • Test against stable servers (tootie/shart)
  • Implement retry logic for network errors
  • Skip destructive tests if env vars not set (not failures)

Risk: mcporter configuration complexity

Impact: LOW - Difficult for contributors to run tests Mitigation:

  • Clear setup documentation
  • Example .env template
  • Helper script to validate setup

Dependencies

  • pydantic-settings>=2.12 (Python package)
  • mcporter (npm package - user must install)
  • jq (system package for JSON parsing in bash)
  • Access to tootie/shart servers (for integration tests)
  • Credentials in ~/workspace/homelab/.env

Timeline Estimate

Task Estimated Time
1. Remove array ops 30 min
2. Add settings infrastructure 1 hour
3. Update tools with gates 2 hours
4. Update test suite 2 hours
5. mcporter config 30 min
6. Helper libraries 1.5 hours
7. Tool test scripts 4 hours
8. Test runners 1 hour
9. Action catalog 2 hours
10. Documentation 1.5 hours
Total ~12 hours

Notes

  • Integration tests complement (not replace) existing pytest suite
  • Tests validate actual Unraid API behavior, not just our code
  • Environment variable gates provide defense-in-depth security
  • mcporter enables real-world validation impossible with mocked tests
  • Action catalog becomes living documentation for all tools

Plan Status: Awaiting user approval Next Step: Review plan, make adjustments, then execute via task list