fix: survive read-only filesystems at startup

Dockerfile

@@ -31,8 +31,13 @@ LABEL org.opencontainers.image.title="Home Assistant MCP Server" \
       org.opencontainers.image.licenses="MIT" \
       io.modelcontextprotocol.server.name="io.github.homeassistant-ai/ha-mcp"
 
-# Create non-root user for security
-RUN groupadd -r mcpuser && useradd -r -g mcpuser -m mcpuser
+# Create non-root user. /home/mcpuser is mode 0755 (not the default 0700) so
+# that callers running with `--user UID:GID` overrides — common in hardened
+# Docker setups, see issue #1125 — can stat HOME-relative paths. Write
+# access stays restricted to mcpuser via ownership.
+RUN groupadd -r mcpuser \
+    && useradd -r -g mcpuser -m mcpuser \
+    && chmod 0755 /home/mcpuser
 
 WORKDIR /app
 
@@ -43,6 +48,12 @@ COPY --chown=mcpuser:mcpuser fastmcp.json fastmcp-http.json ./
 
 USER mcpuser
 
+# Set HOME explicitly. Docker doesn't auto-derive HOME from /etc/passwd when
+# a USER directive is set (moby/moby#2968), leaving HOME=/ at runtime. That
+# made Path.home() resolve to "/" and ha-mcp tried to mkdir "/.ha-mcp" on
+# every start — fatal under `read_only: true` (issue #1125).
+ENV HOME=/home/mcpuser
+
 # Activate virtual environment via PATH
 ENV PATH="/app/.venv/bin:$PATH"
 

src/ha_mcp/settings_ui.py

@@ -19,8 +19,10 @@
 from starlette.requests import Request
 from starlette.responses import HTMLResponse, JSONResponse
 
+from ._version import is_running_in_addon
 from .errors import ErrorCode, create_error_response
 from .transforms import DEFAULT_PINNED_TOOLS
+from .utils.data_paths import get_data_dir
 
 if TYPE_CHECKING:
     from fastmcp import FastMCP
@@ -99,36 +101,38 @@
 }
 
 
-def _is_addon() -> bool:
-    """Return True when running inside the Home Assistant add-on container.
+def _get_config_path() -> Path:
+    """Return the path to the tool config JSON file.
 
-    Mirrors the existing convention in this module (and ``__main__.py``)
-    of treating ``SUPERVISOR_TOKEN`` as the add-on detector. Using the env
-    var is more reliable than checking for ``/data`` because some Docker
-    setups (and macOS dev environments) have a ``/data`` directory that
-    isn't the add-on data dir.
+    Delegates directory resolution to :func:`utils.data_paths.get_data_dir`,
+    which handles ``HA_MCP_CONFIG_DIR`` override, add-on ``/data``,
+    home-dir, and tmpdir fallback (memoized).
     """
-    return bool(os.environ.get("SUPERVISOR_TOKEN"))
-
-
-def _get_config_path() -> Path:
-    """Return the path to the tool config JSON file."""
-    if _is_addon():
-        return Path("/data") / "tool_config.json"
-    home_dir = Path.home() / ".ha-mcp"
-    home_dir.mkdir(parents=True, exist_ok=True)
-    return home_dir / "tool_config.json"
+    return get_data_dir() / "tool_config.json"
 
 
 def load_tool_config(settings: Settings | None = None) -> dict[str, Any]:
     """Load persisted tool config, seeding from env vars if no file exists."""
     path = _get_config_path()
-    if path.exists():
+    # ``Path.exists()`` only swallows ``ENOENT/ENOTDIR/EBADF/ELOOP``; an
+    # ``EACCES`` (e.g. ``HA_MCP_CONFIG_DIR`` pointing at a dir that exists
+    # but isn't readable by the runtime UID) propagates. Read directly and
+    # treat ``FileNotFoundError`` as "no config yet"; log other ``OSError``s.
+    try:
+        raw = path.read_text()
+    except FileNotFoundError:
+        raw = None
+    except OSError:
+        logger.warning("Cannot read tool config at %s", path, exc_info=True)
+        raw = None
+
+    if raw is not None:
         try:
-            result: dict[str, Any] = json.loads(path.read_text())
+            result: dict[str, Any] = json.loads(raw)
+        except json.JSONDecodeError:
+            logger.warning("Tool config at %s is not valid JSON; ignoring.", path)
+        else:
             return result
-        except (OSError, json.JSONDecodeError):
-            logger.warning("Failed to read tool config from %s", path)
 
     if settings is None:
         return {}
@@ -156,14 +160,23 @@ def load_tool_config(settings: Settings | None = None) -> dict[str, Any]:
     return {}
 
 
-def save_tool_config(config: dict[str, Any]) -> None:
-    """Persist tool config to disk."""
+def save_tool_config(config: dict[str, Any]) -> bool:
+    """Persist tool config to disk.
+
+    Returns True on success, False on failure (read-only filesystem,
+    permission denied, etc.). Caller is responsible for surfacing the
+    failure to the user — the HTTP route at ``_save_tools`` returns 500
+    so the UI's ``saveConfig`` shows "Save failed!" instead of the
+    misleading "Saved — restart required".
+    """
     path = _get_config_path()
     try:
         path.write_text(json.dumps(config, indent=2))
-        logger.info("Saved tool config to %s", path)
     except OSError:
         logger.exception("Failed to save tool config to %s", path)
+        return False
+    logger.info("Saved tool config to %s", path)
+    return True
 
 
 async def _get_tool_metadata(server: HomeAssistantSmartMCPServer) -> list[dict[str, Any]]:
@@ -760,7 +773,18 @@ async def _save_tools(request: Request) -> JSONResponse:
 
         config = load_tool_config()
         config["tools"] = states
-        save_tool_config(config)
+        if not save_tool_config(config):
+            return JSONResponse(
+                create_error_response(
+                    ErrorCode.INTERNAL_ERROR,
+                    "Failed to persist tool config to disk",
+                    suggestions=[
+                        "Set HA_MCP_CONFIG_DIR to a writable path (read-only filesystem?)",
+                        "Check the server logs for the underlying OSError",
+                    ],
+                ),
+                status_code=500,
+            )
 
         disabled_count = sum(1 for s in states.values() if s == "disabled")
         pinned_count = sum(1 for s in states.values() if s == "pinned")
@@ -823,11 +847,11 @@ async def _restart_addon(_: Request) -> JSONResponse:
 
     async def _settings_info(_: Request) -> JSONResponse:
         return JSONResponse({
-            "is_addon": _is_addon(),
+            "is_addon": is_running_in_addon(),
         })
 
     secret_prefix = secret_path.rstrip("/") if secret_path else ""
-    is_addon = _is_addon()
+    is_addon = is_running_in_addon()
 
     if not is_addon and not secret_prefix:
         logger.warning(

src/ha_mcp/utils/data_paths.py

@@ -0,0 +1,106 @@
+"""Resolve a writable directory for ha-mcp persistent data.
+
+Single source of truth for "where does ha-mcp write its files?" — used
+by both ``settings_ui`` (tool config) and ``usage_logger`` (rolling
+JSONL).
+"""
+
+from __future__ import annotations
+
+import functools
+import logging
+import os
+import tempfile
+from pathlib import Path
+
+from .._version import is_running_in_addon
+
+logger = logging.getLogger(__name__)
+
+
+@functools.lru_cache(maxsize=1)
+def get_data_dir() -> Path:
+    """Return a writable directory for ha-mcp persistent data (memoized).
+
+    Resolution order:
+
+    1. ``HA_MCP_CONFIG_DIR`` env var — explicit override, e.g. for hardened
+       Docker setups bind-mounting a writable volume into a
+       ``read_only: true`` container.
+    2. ``/data`` — Home Assistant add-on (``SUPERVISOR_TOKEN`` set; writable
+       supervisor data dir).
+    3. ``~/.ha-mcp`` — standard install. Skipped when ``HA_MCP_CONFIG_DIR``
+       was set but failed: an explicit override means "use this exact
+       location", and silently writing to ``$HOME`` instead would surprise
+       users who chose the override deliberately.
+    4. ``<tempdir>/ha-mcp`` — last-resort fallback when the previously
+       chosen step fails (read-only filesystem; ``HOME`` unset so
+       ``Path.home()`` resolves to ``/``; or ``HA_MCP_CONFIG_DIR`` set but
+       its mkdir raises). Loses persistence across restarts but lets the
+       server start; users wanting persistence should set
+       ``HA_MCP_CONFIG_DIR`` to a writable path.
+
+    Memoized so the fallback warning emits once at startup rather than on
+    every save/load HTTP request. Tests reset via
+    ``get_data_dir.cache_clear()``.
+    """
+    return _resolve_data_dir()
+
+
+def _resolve_data_dir() -> Path:
+    """Resolve the data directory (uncached); see ``get_data_dir`` for priority."""
+    config_dir_env = os.environ.get("HA_MCP_CONFIG_DIR")
+    preferred: Path | None = None
+    if config_dir_env:
+        custom_dir = Path(config_dir_env)
+        try:
+            custom_dir.mkdir(parents=True, exist_ok=True)
+        except OSError as e:
+            logger.warning(
+                "HA_MCP_CONFIG_DIR=%s could not be prepared (%s: %s); "
+                "falling back to a tmpdir.",
+                custom_dir,
+                type(e).__name__,
+                e,
+            )
+            preferred = custom_dir
+        else:
+            return custom_dir
+
+    if is_running_in_addon():
+        return Path("/data")
+
+    if preferred is None:
+        home_dir = Path.home() / ".ha-mcp"
+        try:
+            home_dir.mkdir(parents=True, exist_ok=True)
+        except OSError:
+            preferred = home_dir
+        else:
+            return home_dir
+
+    fallback = Path(tempfile.gettempdir()) / "ha-mcp"
+    try:
+        fallback.mkdir(parents=True, exist_ok=True)
+    except OSError as e:
+        # Even the tmpdir is unwritable. Return the path anyway: callers
+        # that wrap writes in try/except OSError can degrade gracefully
+        # (no persistence, but the server still starts).
+        logger.warning(
+            "Cannot write ha-mcp data to %s or fallback %s (%s: %s); "
+            "persistence is disabled. "
+            "Set HA_MCP_CONFIG_DIR to a writable path for persistence.",
+            preferred,
+            fallback,
+            type(e).__name__,
+            e,
+        )
+    else:
+        logger.warning(
+            "Cannot write ha-mcp data to %s (read-only filesystem or HOME unset). "
+            "Falling back to %s — data will NOT persist across restarts. "
+            "Set HA_MCP_CONFIG_DIR to a writable path for persistence.",
+            preferred,
+            fallback,
+        )
+    return fallback

src/ha_mcp/utils/usage_logger.py

@@ -13,6 +13,10 @@
 from queue import Queue
 from typing import Any
 
+from .data_paths import get_data_dir
+
+logger = logging.getLogger(__name__)
+
 # Default ring buffer size - keeps last N entries in memory
 DEFAULT_RING_BUFFER_SIZE = 200
 
@@ -46,13 +50,15 @@ def emit(self, record: logging.LogRecord) -> None:
             return
 
         with self._lock:
-            self._logs.append({
-                "timestamp": datetime.now(UTC).isoformat(),
-                "level": record.levelname,
-                "logger": record.name,
-                "message": record.getMessage(),
-                "elapsed_seconds": round(elapsed, 2),
-            })
+            self._logs.append(
+                {
+                    "timestamp": datetime.now(UTC).isoformat(),
+                    "level": record.levelname,
+                    "logger": record.name,
+                    "message": record.getMessage(),
+                    "elapsed_seconds": round(elapsed, 2),
+                }
+            )
 
     def get_logs(self) -> list[dict[str, Any]]:
         """Get collected startup logs."""
@@ -115,15 +121,26 @@ def __init__(
         if log_file_path:
             self.log_file_path = Path(log_file_path)
         else:
-            # Use user's home directory by default to avoid read-only filesystem errors
-            # when running via uvx/npx which might have read-only CWD
-            self.log_file_path = Path.home() / ".ha-mcp" / "logs" / "mcp_usage.jsonl"
+            # Defer to the shared resolver so logs follow the same precedence
+            # as the settings-UI tool config (HA_MCP_CONFIG_DIR > /data >
+            # ~/.ha-mcp > tempdir). Avoids polluting the filesystem root when
+            # HOME is unset and avoids surprising users who bind-mount a
+            # writable volume via HA_MCP_CONFIG_DIR but find logs missing.
+            self.log_file_path = get_data_dir() / "logs" / "mcp_usage.jsonl"
 
         try:
             self.log_file_path.parent.mkdir(parents=True, exist_ok=True)
-        except OSError:
-            # Directory creation failed (e.g., read-only filesystem)
-            # Disable logging silently to avoid disrupting the MCP server
+        except OSError as e:
+            # Directory creation failed (e.g., read-only filesystem). Surface
+            # the reason instead of silently dropping every log — operators
+            # otherwise see an empty mcp_usage.jsonl with no clue why.
+            logger.warning(
+                "Usage logging disabled — could not create %s (%s: %s). "
+                "Set HA_MCP_CONFIG_DIR to a writable path to enable persistence.",
+                self.log_file_path.parent,
+                type(e).__name__,
+                e,
+            )
             self._enabled = False
 
         # In-memory ring buffer for fast access to recent logs