Skip to content

Commit 5024a43

Browse files
amccats3409claude
andcommitted
fix: tolerate read-only filesystem when locating tool config (#1125)
`:latest` crashed at startup under hardened Docker setups (`read_only: true`, `user: 1000:1000`) because `_get_config_path()` did an unconditional `mkdir(parents=True, exist_ok=True)` on `Path.home() / ".ha-mcp"`. Two contributing factors: 1. The Dockerfile didn't set `ENV HOME`, so under `USER mcpuser` Docker left `HOME=/`. `Path.home()` resolved to `/`, ha-mcp tried to mkdir `/.ha-mcp`, and `read_only: true` made that fatal (issue #1125). 2. Even on writable filesystems this silently polluted the container's filesystem root with a `/.ha-mcp/` directory. Coordinated changes: - `settings_ui.py::_resolve_config_path`: honor a new `HA_MCP_CONFIG_DIR` env var (explicit override for hardened Docker setups bind-mounting a writable volume), and wrap the home-dir mkdir in `try/except OSError` so we fall back to a tmpdir path instead of crashing. HA_MCP_CONFIG_DIR failure also chains into the tmpdir fallback (mirrors the home-dir branch's behavior). Result is memoized at module level via `_CONFIG_PATH_CACHE` so the warning emits once at startup, not on every save/load HTTP request. - `settings_ui.py::load_tool_config`: replace the `path.exists()` / `read_text()` two-step with a single `try: read_text() except OSError:`. `Path.exists()` only swallows ENOENT/ENOTDIR/EBADF/ELOOP — EACCES propagates, which would re-introduce the same class of crash if `HA_MCP_CONFIG_DIR` pointed at a dir whose parent isn't traversable by the runtime UID. - `Dockerfile`: set `ENV HOME=/home/mcpuser` so `Path.home()` resolves correctly under the default user (default-user setups now persist settings to `~/.ha-mcp` instead of `/.ha-mcp`). - `Dockerfile`: `chmod 0755 /home/mcpuser`. The base image's `/etc/login.defs` sets `HOME_MODE=0700`; that's fine when the container runs as mcpuser, but un-traversable for users that override `--user UID:GID` (the issue reporter does so). Anything that stats a path under HOME (set above) then raises PermissionError. Making the dir mode 0755 keeps it traversable for any uid; write access stays restricted to mcpuser. Tests: 5 unit tests in `TestConfigPath`: - HA_MCP_CONFIG_DIR overrides SUPERVISOR_TOKEN - Falls back to tmpdir when home is unwritable (#1125 regression) - HA_MCP_CONFIG_DIR mkdir failure chains to tmpdir fallback - load_tool_config doesn't crash when path's parent is unreadable - Fallback warning emits once via module cache (no log spam) Verified locally with three Docker scenarios: bondskin's hardened compose (`--read-only --user 1000:1000 --tmpfs /tmp`), default user, and `HA_MCP_CONFIG_DIR=/data/ha-mcp` bind-mount. Server starts cleanly in all three; original stack trace from #1125 no longer reproduces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 29397dc commit 5024a43

3 files changed

Lines changed: 287 additions & 12 deletions

File tree

Dockerfile

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,14 @@ LABEL org.opencontainers.image.title="Home Assistant MCP Server" \
3131
org.opencontainers.image.licenses="MIT" \
3232
io.modelcontextprotocol.server.name="io.github.homeassistant-ai/ha-mcp"
3333

34-
# Create non-root user for security
35-
RUN groupadd -r mcpuser && useradd -r -g mcpuser -m mcpuser
34+
# Create non-root user. The base image's HOME_MODE is 0700 — fine when running
35+
# as mcpuser, but it makes /home/mcpuser un-traversable when users override
36+
# `--user UID:GID` (the issue #1125 reporter does so). Any code that stats a
37+
# path under HOME (set below) then raises PermissionError. chmod 0755 makes
38+
# the dir traversable for any uid; write access stays restricted to mcpuser.
39+
RUN groupadd -r mcpuser \
40+
&& useradd -r -g mcpuser -m mcpuser \
41+
&& chmod 0755 /home/mcpuser
3642

3743
WORKDIR /app
3844

@@ -43,6 +49,12 @@ COPY --chown=mcpuser:mcpuser fastmcp.json fastmcp-http.json ./
4349

4450
USER mcpuser
4551

52+
# Set HOME explicitly. Docker doesn't auto-derive HOME from /etc/passwd when
53+
# a USER directive is set (moby/moby#2968), leaving HOME=/ at runtime. That
54+
# made Path.home() resolve to "/" and ha-mcp tried to mkdir "/.ha-mcp" on
55+
# every start — fatal under `read_only: true` (issue #1125).
56+
ENV HOME=/home/mcpuser
57+
4658
# Activate virtual environment via PATH
4759
ENV PATH="/app/.venv/bin:$PATH"
4860

src/ha_mcp/settings_ui.py

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import json
1313
import logging
1414
import os
15+
import tempfile
1516
from pathlib import Path
1617
from typing import TYPE_CHECKING, Any
1718

@@ -111,24 +112,114 @@ def _is_addon() -> bool:
111112
return bool(os.environ.get("SUPERVISOR_TOKEN"))
112113

113114

114-
def _get_config_path() -> Path:
115-
"""Return the path to the tool config JSON file."""
115+
# Cached resolved config path. Populated on first ``_get_config_path()`` call;
116+
# any warnings about unwritable dirs / fallback usage emit once at startup
117+
# rather than on every save/load HTTP request.
118+
_CONFIG_PATH_CACHE: Path | None = None
119+
120+
121+
def _resolve_config_path() -> Path:
122+
"""Resolve the tool config JSON path, falling back if writes are blocked.
123+
124+
Priority:
125+
126+
1. ``HA_MCP_CONFIG_DIR`` env var — explicit override, e.g. for hardened
127+
Docker setups that bind-mount a writable volume into a
128+
``read_only: true`` container.
129+
2. ``/data/tool_config.json`` — Home Assistant add-on (writable
130+
supervisor data dir).
131+
3. ``~/.ha-mcp/tool_config.json`` — standard.
132+
133+
If neither (1) nor (3) is writable (read-only filesystem, or ``HOME``
134+
unset so ``Path.home()`` resolves to ``/``), falls back to
135+
``<tempdir>/ha-mcp/tool_config.json``. The fallback loses persistence
136+
across restarts but lets the server start; users wanting persistence
137+
should set ``HA_MCP_CONFIG_DIR``.
138+
"""
139+
config_dir_env = os.environ.get("HA_MCP_CONFIG_DIR")
140+
if config_dir_env:
141+
custom_dir = Path(config_dir_env)
142+
try:
143+
custom_dir.mkdir(parents=True, exist_ok=True)
144+
return custom_dir / "tool_config.json"
145+
except OSError as e:
146+
logger.warning(
147+
"HA_MCP_CONFIG_DIR=%s could not be prepared (%s: %s); "
148+
"falling back to a tmpdir.",
149+
custom_dir,
150+
type(e).__name__,
151+
e,
152+
)
153+
preferred = custom_dir
154+
else:
155+
preferred = None
156+
116157
if _is_addon():
117158
return Path("/data") / "tool_config.json"
118-
home_dir = Path.home() / ".ha-mcp"
119-
home_dir.mkdir(parents=True, exist_ok=True)
120-
return home_dir / "tool_config.json"
159+
160+
if preferred is None:
161+
home_dir = Path.home() / ".ha-mcp"
162+
try:
163+
home_dir.mkdir(parents=True, exist_ok=True)
164+
return home_dir / "tool_config.json"
165+
except OSError:
166+
preferred = home_dir
167+
168+
fallback = Path(tempfile.gettempdir()) / "ha-mcp"
169+
try:
170+
fallback.mkdir(parents=True, exist_ok=True)
171+
logger.warning(
172+
"Cannot write tool config to %s (read-only filesystem or HOME unset). "
173+
"Falling back to %s — settings will NOT persist across restarts. "
174+
"Set HA_MCP_CONFIG_DIR to a writable path for persistence.",
175+
preferred,
176+
fallback,
177+
)
178+
except OSError as e:
179+
# Even the tmpdir is unwritable. Return the path anyway so the
180+
# caller's own try/except (save_tool_config wraps writes in
181+
# try/except OSError) can handle it as a degraded-but-running mode.
182+
logger.warning(
183+
"Cannot write tool config to %s or fallback %s (%s: %s); "
184+
"settings persistence is disabled. "
185+
"Set HA_MCP_CONFIG_DIR to a writable path for persistence.",
186+
preferred,
187+
fallback,
188+
type(e).__name__,
189+
e,
190+
)
191+
return fallback / "tool_config.json"
192+
193+
194+
def _get_config_path() -> Path:
195+
"""Return the cached tool config JSON path (resolves on first call)."""
196+
global _CONFIG_PATH_CACHE
197+
if _CONFIG_PATH_CACHE is None:
198+
_CONFIG_PATH_CACHE = _resolve_config_path()
199+
return _CONFIG_PATH_CACHE
121200

122201

123202
def load_tool_config(settings: Settings | None = None) -> dict[str, Any]:
124203
"""Load persisted tool config, seeding from env vars if no file exists."""
125204
path = _get_config_path()
126-
if path.exists():
205+
# ``Path.exists()`` only swallows ``ENOENT/ENOTDIR/EBADF/ELOOP``; an
206+
# ``EACCES`` (e.g. ``HA_MCP_CONFIG_DIR`` pointing at a dir that exists
207+
# but isn't readable by the runtime UID) propagates. Read directly and
208+
# treat ``FileNotFoundError`` as "no config yet"; log other ``OSError``s.
209+
try:
210+
raw = path.read_text()
211+
except FileNotFoundError:
212+
raw = None
213+
except OSError:
214+
logger.warning("Cannot read tool config at %s", path)
215+
raw = None
216+
217+
if raw is not None:
127218
try:
128-
result: dict[str, Any] = json.loads(path.read_text())
219+
result: dict[str, Any] = json.loads(raw)
129220
return result
130-
except (OSError, json.JSONDecodeError):
131-
logger.warning("Failed to read tool config from %s", path)
221+
except json.JSONDecodeError:
222+
logger.warning("Tool config at %s is not valid JSON; ignoring.", path)
132223

133224
if settings is None:
134225
return {}

tests/src/unit/test_settings_ui.py

Lines changed: 173 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,20 +137,192 @@ def test_empty_config_no_disable(self):
137137
mcp.disable.assert_not_called()
138138

139139

140+
@pytest.fixture(autouse=True)
141+
def _reset_config_path_cache():
142+
"""Clear the module-level resolved-path cache between tests."""
143+
import ha_mcp.settings_ui as su
144+
145+
su._CONFIG_PATH_CACHE = None
146+
yield
147+
su._CONFIG_PATH_CACHE = None
148+
149+
140150
class TestConfigPath:
141-
"""Test _get_config_path uses SUPERVISOR_TOKEN, not /data heuristic."""
151+
"""Tests for _get_config_path priority order and fallbacks.
152+
153+
Priority: HA_MCP_CONFIG_DIR > SUPERVISOR_TOKEN/data > home > tmpdir.
154+
"""
142155

143156
def test_addon_path_when_supervisor_token_set(self, monkeypatch):
144157
monkeypatch.setenv("SUPERVISOR_TOKEN", "fake")
158+
monkeypatch.delenv("HA_MCP_CONFIG_DIR", raising=False)
145159
assert _get_config_path() == Path("/data/tool_config.json")
146160

147161
def test_home_path_when_no_supervisor_token(self, monkeypatch, tmp_path):
148162
monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
163+
monkeypatch.delenv("HA_MCP_CONFIG_DIR", raising=False)
149164
monkeypatch.setattr(Path, "home", lambda: tmp_path)
150165
result = _get_config_path()
151166
assert result == tmp_path / ".ha-mcp" / "tool_config.json"
152167
assert (tmp_path / ".ha-mcp").is_dir()
153168

169+
def test_ha_mcp_config_dir_overrides_supervisor_token(self, monkeypatch, tmp_path):
170+
"""HA_MCP_CONFIG_DIR takes precedence even when SUPERVISOR_TOKEN is set.
171+
172+
Lets add-on users override the default ``/data`` location, and lets
173+
hardened-Docker users bind-mount a writable volume without depending
174+
on ``$HOME``.
175+
"""
176+
custom_dir = tmp_path / "custom"
177+
monkeypatch.setenv("HA_MCP_CONFIG_DIR", str(custom_dir))
178+
monkeypatch.setenv("SUPERVISOR_TOKEN", "fake") # would normally route to /data
179+
result = _get_config_path()
180+
assert result == custom_dir / "tool_config.json"
181+
assert custom_dir.is_dir()
182+
183+
def test_falls_back_to_tmpdir_when_home_unwritable(self, monkeypatch, tmp_path):
184+
"""Fall back to a tmpdir path when ``~/.ha-mcp`` can't be created.
185+
186+
Covers the issue #1125 scenario: ``read_only: true`` Docker, or
187+
``HOME=/`` so ``mkdir(/.ha-mcp)`` fails. The function must return
188+
a usable path instead of crashing.
189+
"""
190+
monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
191+
monkeypatch.delenv("HA_MCP_CONFIG_DIR", raising=False)
192+
readonly_home = tmp_path / "readonly-home"
193+
readonly_home.mkdir()
194+
monkeypatch.setattr(Path, "home", lambda: readonly_home)
195+
original_mkdir = Path.mkdir
196+
197+
def fake_mkdir(self: Path, *args, **kwargs):
198+
if self == readonly_home / ".ha-mcp":
199+
raise OSError(30, "Read-only file system")
200+
return original_mkdir(self, *args, **kwargs)
201+
202+
monkeypatch.setattr(Path, "mkdir", fake_mkdir)
203+
fallback_root = tmp_path / "fallback-tmp"
204+
fallback_root.mkdir()
205+
monkeypatch.setattr(
206+
"ha_mcp.settings_ui.tempfile.gettempdir", lambda: str(fallback_root)
207+
)
208+
209+
result = _get_config_path()
210+
211+
assert result == fallback_root / "ha-mcp" / "tool_config.json"
212+
assert (fallback_root / "ha-mcp").is_dir()
213+
214+
def test_ha_mcp_config_dir_unwritable_chains_to_tmpdir(
215+
self, monkeypatch, tmp_path
216+
):
217+
"""HA_MCP_CONFIG_DIR mkdir failure chains to the tmpdir fallback.
218+
219+
Avoids returning a known-broken path and emitting an OSError on
220+
every save. Mirrors the home-dir branch's fallback.
221+
"""
222+
monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
223+
readonly_parent = tmp_path / "readonly-parent"
224+
readonly_parent.mkdir()
225+
broken_target = readonly_parent / "cannot-create"
226+
original_mkdir = Path.mkdir
227+
228+
def fake_mkdir(self: Path, *args, **kwargs):
229+
if self == broken_target:
230+
raise OSError(30, "Read-only file system")
231+
return original_mkdir(self, *args, **kwargs)
232+
233+
monkeypatch.setattr(Path, "mkdir", fake_mkdir)
234+
monkeypatch.setenv("HA_MCP_CONFIG_DIR", str(broken_target))
235+
monkeypatch.setattr(Path, "home", lambda: tmp_path / "unused-home")
236+
fallback_root = tmp_path / "fallback-tmp"
237+
fallback_root.mkdir()
238+
monkeypatch.setattr(
239+
"ha_mcp.settings_ui.tempfile.gettempdir", lambda: str(fallback_root)
240+
)
241+
242+
result = _get_config_path()
243+
244+
assert result == fallback_root / "ha-mcp" / "tool_config.json"
245+
assert (fallback_root / "ha-mcp").is_dir()
246+
assert not broken_target.exists()
247+
248+
def test_load_tool_config_does_not_crash_on_unreadable_config_dir(
249+
self, monkeypatch, tmp_path
250+
):
251+
"""Regression for #1125 + the same-class follow-up bug.
252+
253+
When the resolved path's parent isn't traversable by the runtime
254+
UID (e.g. ``HA_MCP_CONFIG_DIR`` pointing at an existing 0700 dir
255+
owned by another user), ``Path.exists()`` would raise
256+
``PermissionError`` because ``EACCES`` is not in
257+
``pathlib._IGNORED_ERRNOS``. ``load_tool_config()`` must treat it
258+
as "no config yet" instead of crashing.
259+
"""
260+
monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
261+
monkeypatch.delenv("HA_MCP_CONFIG_DIR", raising=False)
262+
unreadable_dir = tmp_path / "unreadable"
263+
unreadable_dir.mkdir()
264+
monkeypatch.setattr(Path, "home", lambda: tmp_path / "unused-home")
265+
# Stub _get_config_path directly to point at the unreadable dir,
266+
# avoiding having to make the real os.chmod call (which is racy
267+
# and platform-specific).
268+
cfg_path = unreadable_dir / "tool_config.json"
269+
monkeypatch.setattr(
270+
"ha_mcp.settings_ui._get_config_path", lambda: cfg_path
271+
)
272+
273+
original_read = Path.read_text
274+
275+
def fake_read_text(self: Path, *args, **kwargs):
276+
if self == cfg_path:
277+
raise PermissionError(13, "Permission denied")
278+
return original_read(self, *args, **kwargs)
279+
280+
monkeypatch.setattr(Path, "read_text", fake_read_text)
281+
282+
# Must not raise.
283+
assert load_tool_config() == {}
284+
285+
def test_warning_emitted_only_once_via_module_cache(
286+
self, monkeypatch, tmp_path, caplog
287+
):
288+
"""The fallback warning must emit once, not on every call.
289+
290+
``_get_config_path`` is invoked from every save/load HTTP request;
291+
without the module-level cache the same warning would spam logs
292+
on every UI toggle.
293+
"""
294+
monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
295+
monkeypatch.delenv("HA_MCP_CONFIG_DIR", raising=False)
296+
readonly_home = tmp_path / "readonly-home"
297+
readonly_home.mkdir()
298+
monkeypatch.setattr(Path, "home", lambda: readonly_home)
299+
original_mkdir = Path.mkdir
300+
301+
def fake_mkdir(self: Path, *args, **kwargs):
302+
if self == readonly_home / ".ha-mcp":
303+
raise OSError(30, "Read-only file system")
304+
return original_mkdir(self, *args, **kwargs)
305+
306+
monkeypatch.setattr(Path, "mkdir", fake_mkdir)
307+
fallback_root = tmp_path / "fallback-tmp"
308+
fallback_root.mkdir()
309+
monkeypatch.setattr(
310+
"ha_mcp.settings_ui.tempfile.gettempdir", lambda: str(fallback_root)
311+
)
312+
313+
import logging
314+
315+
with caplog.at_level(logging.WARNING, logger="ha_mcp.settings_ui"):
316+
for _ in range(5):
317+
_get_config_path()
318+
319+
fallback_warnings = [
320+
r for r in caplog.records if "Falling back" in r.getMessage()
321+
]
322+
assert len(fallback_warnings) == 1, (
323+
f"expected single fallback warning, got {len(fallback_warnings)}"
324+
)
325+
154326

155327
class TestFeatureGatedTools:
156328
"""Test the FEATURE_GATED_TOOLS dict aligns with the beta tag system."""

0 commit comments

Comments
 (0)