From e930b868e42b865e1e922e3d685a44863d3d04f1 Mon Sep 17 00:00:00 2001 From: Jacob Magar Date: Sat, 14 Mar 2026 13:57:04 -0400 Subject: [PATCH] feat(creds): write to ~/.unraid-mcp/.env with 700/600 permissions, seed from .env.example - _write_env now creates CREDENTIALS_DIR (mode 700) and writes credentials to CREDENTIALS_ENV_PATH (mode 600) instead of PROJECT_ROOT/.env - On first run (no .env yet), seeds file content from .env.example to preserve comments and structure - elicit_and_configure catches NotImplementedError from ctx.elicit() so clients that don't support elicitation return False gracefully instead of propagating the exception - Updated test_elicit_and_configure_writes_env_file to patch CREDENTIALS_DIR and CREDENTIALS_ENV_PATH instead of PROJECT_ROOT - Added 5 new tests covering dir/file permissions, .env.example seeding, in-place credential update, and NotImplementedError guard --- tests/test_setup.py | 140 ++++++++++++++++++++++++++++++++++++++- unraid_mcp/core/setup.py | 94 +++++++++++++++++--------- 2 files changed, 200 insertions(+), 34 deletions(-) diff --git a/tests/test_setup.py b/tests/test_setup.py index 9adbf7c..31221fd 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -112,16 +112,20 @@ async def test_elicit_and_configure_writes_env_file(tmp_path): mock_result.data.api_key = "abc123secret" mock_ctx.elicit = AsyncMock(return_value=mock_result) + creds_dir = tmp_path / "creds" + creds_file = creds_dir / ".env" + with ( + patch("unraid_mcp.core.setup.CREDENTIALS_DIR", creds_dir), + patch("unraid_mcp.core.setup.CREDENTIALS_ENV_PATH", creds_file), patch("unraid_mcp.core.setup.PROJECT_ROOT", tmp_path), patch("unraid_mcp.core.setup.apply_runtime_config") as mock_apply, ): result = await elicit_and_configure(mock_ctx) assert result is True - env_file = tmp_path / ".env" - assert env_file.exists() - content = env_file.read_text() + assert creds_file.exists() + content = creds_file.read_text() assert "UNRAID_API_URL=https://myunraid.example.com/graphql" in content assert "UNRAID_API_KEY=abc123secret" in content mock_apply.assert_called_once_with("https://myunraid.example.com/graphql", "abc123secret") @@ -256,3 +260,133 @@ def test_credentials_env_path_is_dot_env_inside_credentials_dir(): import unraid_mcp.config.settings as s assert s.CREDENTIALS_ENV_PATH == s.CREDENTIALS_DIR / ".env" + + +import stat # noqa: E402 + + +def test_write_env_creates_credentials_dir_with_700_permissions(tmp_path): + """_write_env creates CREDENTIALS_DIR with mode 700 (owner-only).""" + from unraid_mcp.core.setup import _write_env + + creds_dir = tmp_path / "creds" + creds_file = creds_dir / ".env" + + with ( + patch("unraid_mcp.core.setup.CREDENTIALS_DIR", creds_dir), + patch("unraid_mcp.core.setup.CREDENTIALS_ENV_PATH", creds_file), + ): + _write_env("https://example.com", "mykey") + + assert creds_dir.exists() + dir_mode = stat.S_IMODE(creds_dir.stat().st_mode) + assert dir_mode == 0o700, f"Expected 0o700, got {oct(dir_mode)}" + + +def test_write_env_sets_file_permissions_600(tmp_path): + """_write_env sets .env file permissions to 600 (owner read/write only).""" + from unraid_mcp.core.setup import _write_env + + creds_dir = tmp_path / "creds" + creds_file = creds_dir / ".env" + + with ( + patch("unraid_mcp.core.setup.CREDENTIALS_DIR", creds_dir), + patch("unraid_mcp.core.setup.CREDENTIALS_ENV_PATH", creds_file), + ): + _write_env("https://example.com", "mykey") + + file_mode = stat.S_IMODE(creds_file.stat().st_mode) + assert file_mode == 0o600, f"Expected 0o600, got {oct(file_mode)}" + + +def test_write_env_seeds_from_env_example_on_first_run(tmp_path): + """_write_env copies .env.example structure and replaces credentials in-place.""" + from unraid_mcp.core.setup import _write_env + + creds_dir = tmp_path / "creds" + creds_file = creds_dir / ".env" + # Create a fake .env.example + example = tmp_path / ".env.example" + example.write_text( + "# Example config\nFOO=bar\nUNRAID_API_URL=placeholder\nUNRAID_API_KEY=placeholder\n" + ) + + with ( + patch("unraid_mcp.core.setup.CREDENTIALS_DIR", creds_dir), + patch("unraid_mcp.core.setup.CREDENTIALS_ENV_PATH", creds_file), + patch("unraid_mcp.core.setup.PROJECT_ROOT", tmp_path), + ): + _write_env("https://real.url", "realkey") + + content = creds_file.read_text() + assert "UNRAID_API_URL=https://real.url" in content + assert "UNRAID_API_KEY=realkey" in content + assert "# Example config" in content # comment preserved + assert "FOO=bar" in content # other vars preserved + assert "placeholder" not in content # old credential values replaced + # Credentials should be at their original position (after comments), not prepended before them + lines = content.splitlines() + url_idx = next(i for i, line in enumerate(lines) if line.startswith("UNRAID_API_URL=")) + comment_idx = next(i for i, line in enumerate(lines) if line.startswith("# Example config")) + assert comment_idx < url_idx # Comment comes before credentials + + +def test_write_env_first_run_no_example_file(tmp_path): + """_write_env works on first run when .env.example does not exist.""" + from unraid_mcp.core.setup import _write_env + + creds_dir = tmp_path / "creds" + creds_file = creds_dir / ".env" + # tmp_path has no .env.example + + with ( + patch("unraid_mcp.core.setup.CREDENTIALS_DIR", creds_dir), + patch("unraid_mcp.core.setup.CREDENTIALS_ENV_PATH", creds_file), + patch("unraid_mcp.core.setup.PROJECT_ROOT", tmp_path), + ): + _write_env("https://myserver.com", "mykey123") + + assert creds_file.exists() + content = creds_file.read_text() + assert "UNRAID_API_URL=https://myserver.com" in content + assert "UNRAID_API_KEY=mykey123" in content + + +def test_write_env_updates_existing_credentials_in_place(tmp_path): + """_write_env updates credentials without destroying other vars.""" + from unraid_mcp.core.setup import _write_env + + creds_dir = tmp_path / "creds" + creds_dir.mkdir(mode=0o700) + creds_file = creds_dir / ".env" + creds_file.write_text( + "UNRAID_API_URL=https://old.url\nUNRAID_API_KEY=oldkey\nUNRAID_VERIFY_SSL=false\n" + ) + + with ( + patch("unraid_mcp.core.setup.CREDENTIALS_DIR", creds_dir), + patch("unraid_mcp.core.setup.CREDENTIALS_ENV_PATH", creds_file), + patch("unraid_mcp.core.setup.PROJECT_ROOT", tmp_path), + ): + _write_env("https://new.url", "newkey") + + content = creds_file.read_text() + assert "UNRAID_API_URL=https://new.url" in content + assert "UNRAID_API_KEY=newkey" in content + assert "UNRAID_VERIFY_SSL=false" in content # preserved + assert "old" not in content + + +@pytest.mark.asyncio +async def test_elicit_and_configure_returns_false_when_client_not_supported(): + """elicit_and_configure returns False when client raises NotImplementedError.""" + from unittest.mock import AsyncMock, MagicMock + + from unraid_mcp.core.setup import elicit_and_configure + + mock_ctx = MagicMock() + mock_ctx.elicit = AsyncMock(side_effect=NotImplementedError("elicitation not supported")) + + result = await elicit_and_configure(mock_ctx) + assert result is False diff --git a/unraid_mcp/core/setup.py b/unraid_mcp/core/setup.py index e452b50..ed935b1 100644 --- a/unraid_mcp/core/setup.py +++ b/unraid_mcp/core/setup.py @@ -2,7 +2,7 @@ When UNRAID_API_URL or UNRAID_API_KEY are absent, tools call `elicit_and_configure(ctx)` to collect them from the user and persist -them to .env in the server root directory. +them to ~/.unraid-mcp/.env with restricted permissions. """ from __future__ import annotations @@ -12,7 +12,12 @@ from dataclasses import dataclass from fastmcp import Context from ..config.logging import logger -from ..config.settings import PROJECT_ROOT, apply_runtime_config +from ..config.settings import ( + CREDENTIALS_DIR, + CREDENTIALS_ENV_PATH, + PROJECT_ROOT, + apply_runtime_config, +) @dataclass @@ -24,7 +29,7 @@ class _UnraidCredentials: async def elicit_and_configure(ctx: Context | None) -> bool: """Prompt the user for Unraid credentials via MCP elicitation. - Writes accepted credentials to .env in PROJECT_ROOT and applies them + Writes accepted credentials to CREDENTIALS_ENV_PATH and applies them to the running process via apply_runtime_config(). Args: @@ -32,7 +37,8 @@ async def elicit_and_configure(ctx: Context | None) -> bool: (no context available to prompt the user). Returns: - True if credentials were accepted and applied, False if declined/cancelled. + True if credentials were accepted and applied, False if declined/cancelled + or if the MCP client does not support elicitation. """ if ctx is None: logger.warning( @@ -41,15 +47,23 @@ async def elicit_and_configure(ctx: Context | None) -> bool: ) return False - result = await ctx.elicit( - message=( - "Unraid MCP needs your Unraid server credentials to connect.\n\n" - "• **API URL**: Your Unraid GraphQL endpoint " - "(e.g. `https://10-1-0-2.xxx.myunraid.net:31337`)\n" - "• **API Key**: Found in Unraid → Settings → Management Access → API Keys" - ), - response_type=_UnraidCredentials, - ) + try: + result = await ctx.elicit( + message=( + "Unraid MCP needs your Unraid server credentials to connect.\n\n" + "• **API URL**: Your Unraid GraphQL endpoint " + "(e.g. `https://10-1-0-2.xxx.myunraid.net:31337`)\n" + "• **API Key**: Found in Unraid → Settings → Management Access → API Keys" + ), + response_type=_UnraidCredentials, + ) + except NotImplementedError: + logger.warning( + "MCP client does not support elicitation. " + "Use unraid_health action=setup or create %s manually.", + CREDENTIALS_ENV_PATH, + ) + return False if result.action != "accept": logger.warning("Credential elicitation %s — server remains unconfigured.", result.action) @@ -61,29 +75,47 @@ async def elicit_and_configure(ctx: Context | None) -> bool: _write_env(api_url, api_key) apply_runtime_config(api_url, api_key) - logger.info("Credentials configured via elicitation and persisted to .env.") + logger.info("Credentials configured via elicitation and persisted to %s.", CREDENTIALS_ENV_PATH) return True def _write_env(api_url: str, api_key: str) -> None: - """Write or update .env in PROJECT_ROOT with credential values. + """Write or update credentials in CREDENTIALS_ENV_PATH. - Preserves any existing lines that are not UNRAID_API_URL or UNRAID_API_KEY. + Creates CREDENTIALS_DIR (mode 700) if needed. On first run, seeds from + .env.example to preserve comments and structure. Sets file mode to 600. """ - env_path = PROJECT_ROOT / ".env" - existing_lines: list[str] = [] + # Ensure directory exists with restricted permissions (chmod after to bypass umask) + CREDENTIALS_DIR.mkdir(parents=True, exist_ok=True) + CREDENTIALS_DIR.chmod(0o700) - if env_path.exists(): - for line in env_path.read_text().splitlines(): - stripped = line.strip() - if stripped.startswith("UNRAID_API_URL=") or stripped.startswith("UNRAID_API_KEY="): - continue # Will be replaced below - existing_lines.append(line) + if CREDENTIALS_ENV_PATH.exists(): + template_lines = CREDENTIALS_ENV_PATH.read_text().splitlines() + else: + example_path = PROJECT_ROOT / ".env.example" + template_lines = example_path.read_text().splitlines() if example_path.exists() else [] - new_lines = [ - f"UNRAID_API_URL={api_url}", - f"UNRAID_API_KEY={api_key}", - *existing_lines, - ] - env_path.write_text("\n".join(new_lines) + "\n") - logger.debug("Wrote credentials to %s", env_path) + # Replace credentials in-place; append at end if not found in template + url_written = False + key_written = False + new_lines: list[str] = [] + for line in template_lines: + stripped = line.strip() + if stripped.startswith("UNRAID_API_URL="): + new_lines.append(f"UNRAID_API_URL={api_url}") + url_written = True + elif stripped.startswith("UNRAID_API_KEY="): + new_lines.append(f"UNRAID_API_KEY={api_key}") + key_written = True + else: + new_lines.append(line) + + # If not found in template (empty or missing keys), append at end + if not url_written: + new_lines.append(f"UNRAID_API_URL={api_url}") + if not key_written: + new_lines.append(f"UNRAID_API_KEY={api_key}") + + CREDENTIALS_ENV_PATH.write_text("\n".join(new_lines) + "\n") + CREDENTIALS_ENV_PATH.chmod(0o600) + logger.info("Credentials written to %s (mode 600)", CREDENTIALS_ENV_PATH)