Skip to content

Commit f73ea70

Browse files
fix: allow reading blueprints/ by default (#1965) (#1988)
* fix: allow reading blueprints/ by default (#1965) Blueprint files under blueprints/ were rejected by ha_read_file and ha_list_files because "blueprints" was absent from the component's built-in ALLOWED_READ_DIRS, even though blueprints hold only community YAML with no secrets. Add it as a read-only default (present in ALLOWED_READ_DIRS, deliberately NOT in ALLOWED_WRITE_DIRS) so both tools can read/list blueprint sources without a custom allowlist entry. ha_get_blueprint stays the preferred way to read a blueprint body; a short "escape hatch" note now steers callers there and to the other dedicated read tools. Also reconcile the read/list allowlist docs with the actual allowlist: services.yaml read_file was missing dashboards/**, and the list_files docs omitted the configured packages/ folder. Rides the pending 1.2.3 component version (no bump). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address review — scope read-only claim, complete doc parity, add guard Review round on #1988: - const.py: the "read-only" comment overclaimed. handle_write_file also honors user-configured extra_dirs (#1567), so an admin adding "blueprints" as a custom directory would grant write. Scope the claim to the default allowlist, note the extra_dirs write path, and clarify blueprint/save is the WS command ha_import_blueprint calls internally. - services.yaml: write_file/delete_file descriptions omitted dashboards/ even though it is in ALLOWED_WRITE_DIRS — the same doc-parity drift this PR fixed for read_file/list_files. Now consistent across all four surfaces. - Add test_filesystem_doc_parity.py: a self-contained guard asserting every ALLOWED_READ_DIRS/ALLOWED_WRITE_DIRS entry is documented in the tool docstrings and services.yaml path descriptions, so a future allowlist edit that forgets a doc surface fails CI. Parses sources with ast/yaml and imports nothing from HA/fastmcp, so it runs in CI and locally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5b731f3 commit f73ea70

7 files changed

Lines changed: 227 additions & 15 deletions

File tree

custom_components/ha_mcp_tools/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,8 @@ def _is_path_allowed_for_read(
10001000
10011001
Allowed:
10021002
- Files directly in config dir: configuration.yaml, automations.yaml, etc.
1003-
- Files in allowed directories: www/, themes/, custom_templates/
1003+
- Files in allowed directories: www/, themes/, custom_templates/,
1004+
dashboards/, blueprints/ (blueprints is read-only — issue #1965)
10041005
- Files matching patterns: <packages-folder>/*.yaml, custom_components/**/*.py
10051006
- User-configured extra directories (``extra_dirs``), granted read+write
10061007
(issue #1567)

custom_components/ha_mcp_tools/const.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,17 @@
4040
TOOLS_ENTRY_LEGACY_TITLE = "HA MCP Tools"
4141
MIN_EMBEDDED_HOME_ASSISTANT_VERSION = "2026.6.0"
4242

43-
# Allowed directories for file operations (relative to config dir)
44-
ALLOWED_READ_DIRS = ["www", "themes", "custom_templates", "dashboards"]
43+
# Allowed directories for file operations (relative to config dir).
44+
# "blueprints" is read-only BY DEFAULT — in ALLOWED_READ_DIRS but not
45+
# ALLOWED_WRITE_DIRS, so ha_write_file / ha_delete_file reject it (raw blueprint
46+
# reads are safe: community YAML, no secrets — issue #1965). This is the default
47+
# allowlist, not an absolute guarantee: an admin who adds "blueprints" as a
48+
# custom extra directory (issue #1567, see _current_extra_dirs) grants it
49+
# read+write, since extra_dirs are honored on the write path too. Blueprint
50+
# writes should instead go through ha_import_blueprint (which invokes the
51+
# blueprint/save WS command internally). Prefer ha_get_blueprint for the parsed
52+
# body; raw read is the escape hatch for the exact on-disk text.
53+
ALLOWED_READ_DIRS = ["www", "themes", "custom_templates", "dashboards", "blueprints"]
4554
ALLOWED_WRITE_DIRS = ["www", "themes", "custom_templates", "dashboards"]
4655

4756
# NON-OVERRIDABLE deny floor for the user-configurable extra read/write

custom_components/ha_mcp_tools/services.yaml

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ list_files:
5858
fields:
5959
path:
6060
name: Path
61-
description: Relative path from config directory (e.g., "www/", "themes/")
61+
description: >-
62+
Relative path from config directory. Allowed: www/, themes/,
63+
custom_templates/, dashboards/, blueprints/, plus your configured
64+
packages/ folder (e.g., "www/", "blueprints/automation").
6265
required: true
6366
example: "www/"
6467
selector:
@@ -81,7 +84,8 @@ read_file:
8184
Relative path from config directory. Allowed paths include
8285
configuration.yaml, automations.yaml, scripts.yaml, scenes.yaml,
8386
secrets.yaml (values masked), home-assistant.log, www/**, themes/**,
84-
custom_templates/**, packages/*.yaml, custom_components/**/*.py
87+
custom_templates/**, dashboards/**, blueprints/**, packages/*.yaml,
88+
custom_components/**/*.py
8589
required: true
8690
example: "configuration.yaml"
8791
selector:
@@ -119,13 +123,13 @@ read_file:
119123

120124
write_file:
121125
name: Write File
122-
description: Write a file to allowed directories (www/, themes/, custom_templates/).
126+
description: Write a file to allowed directories (www/, themes/, custom_templates/, dashboards/).
123127
fields:
124128
path:
125129
name: Path
126130
description: >-
127-
Relative path from config directory. Must be in www/, themes/, or
128-
custom_templates/.
131+
Relative path from config directory. Must be in www/, themes/,
132+
custom_templates/, or dashboards/.
129133
required: true
130134
example: "www/custom.css"
131135
selector:
@@ -155,13 +159,13 @@ write_file:
155159

156160
delete_file:
157161
name: Delete File
158-
description: Delete a file from allowed directories (www/, themes/, custom_templates/).
162+
description: Delete a file from allowed directories (www/, themes/, custom_templates/, dashboards/).
159163
fields:
160164
path:
161165
name: Path
162166
description: >-
163-
Relative path from config directory. Must be in www/, themes/, or
164-
custom_templates/.
167+
Relative path from config directory. Must be in www/, themes/,
168+
custom_templates/, or dashboards/.
165169
required: true
166170
example: "www/old-file.css"
167171
selector:

src/ha_mcp/tools/tools_filesystem.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,8 @@ async def ha_list_files(
406406
Field(
407407
description=(
408408
"Directory path. Relative to the config dir for the built-in "
409-
"allowlist (www/, themes/, custom_templates/, dashboards/). "
409+
"allowlist (www/, themes/, custom_templates/, dashboards/, "
410+
"blueprints/). "
410411
"Custom directories and HAOS sibling volumes "
411412
"(/share, /media, /ssl, /backup) configured in the ha-mcp "
412413
"settings UI are also allowed (pass the absolute path). "
@@ -427,14 +428,18 @@ async def ha_list_files(
427428
) -> dict[str, Any]:
428429
"""List files in a directory within the Home Assistant config directory.
429430
430-
Lists files in allowed directories (www/, themes/, custom_templates/, dashboards/) with
431-
optional glob pattern filtering. Returns file names, sizes, and modification times.
431+
Lists files in allowed directories (www/, themes/, custom_templates/,
432+
dashboards/, blueprints/) with optional glob pattern filtering. Returns
433+
file names, sizes, and modification times.
432434
433435
**Allowed Directories:**
434436
- `www/` - Web assets (CSS, JS, images for dashboards)
435437
- `themes/` - Theme files
436438
- `custom_templates/` - Jinja2 template files
437439
- `dashboards/` - YAML-mode dashboard files
440+
- `blueprints/` - Automation/script blueprint sources (read-only)
441+
- Your configured `packages/` folder, when `homeassistant: packages:` is
442+
set (the folder name you bound, default `packages/`)
438443
- Plus any custom directories OR HAOS sibling volumes (`/share`,
439444
`/media`, `/ssl`, `/backup`) configured in the ha-mcp settings UI
440445
(pass the absolute path for volumes)
@@ -549,6 +554,11 @@ async def ha_read_file(
549554
) -> dict[str, Any]:
550555
"""Read a file from the Home Assistant config directory.
551556
557+
General-purpose escape hatch — prefer a dedicated tool when one exists:
558+
ha_get_blueprint for a blueprint body, ha_config_get_yaml for a config
559+
key, ha_config_get_automation/script/scene for storage-mode items. Reach
560+
for ha_read_file only for raw on-disk text those tools don't expose.
561+
552562
Reads files from allowed paths within the config directory. Some files
553563
have special handling:
554564
- `secrets.yaml`: Values are masked for security
@@ -559,7 +569,7 @@ async def ha_read_file(
559569
- `secrets.yaml` (values masked)
560570
- `packages/*.yaml`
561571
- `home-assistant.log` (tail only)
562-
- `www/**`, `themes/**`, `custom_templates/**`, `dashboards/**`
572+
- `www/**`, `themes/**`, `custom_templates/**`, `dashboards/**`, `blueprints/**`
563573
- `custom_components/**/*.py` (read-only)
564574
- Plus any custom directories OR HAOS sibling volumes (`/share`,
565575
`/media`, `/ssl`, `/backup`) configured in the ha-mcp settings UI

tests/src/e2e/workflows/filesystem/test_file_operations.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,28 @@ async def test_list_files_in_www_directory(self, mcp_client_with_filesystem):
269269

270270
logger.info(f"Found files: {[f['name'] for f in files]}")
271271

272+
async def test_list_files_in_blueprints_directory(self, mcp_client_with_filesystem):
273+
"""Blueprints dir is listable by default (issue #1965)."""
274+
service_check = await _check_mcp_tools_service_available(
275+
mcp_client_with_filesystem
276+
)
277+
_skip_if_component_not_installed(service_check, "List files in blueprints")
278+
279+
async with MCPAssertions(mcp_client_with_filesystem) as mcp:
280+
result_data = await mcp.call_tool_success(
281+
"ha_list_files",
282+
{"path": "blueprints/automation/homeassistant"},
283+
)
284+
285+
data = result_data
286+
assert data.get("success") is True, f"List blueprints failed: {data}"
287+
names = [f["name"] for f in data.get("files", [])]
288+
assert "motion_light.yaml" in names, (
289+
f"Expected motion_light.yaml under blueprints/: {names}"
290+
)
291+
292+
logger.info(f"Listed blueprints/automation/homeassistant: {names}")
293+
272294
async def test_list_files_with_pattern_filter(self, mcp_client_with_filesystem):
273295
"""Test listing files with glob pattern filter."""
274296
service_check = await _check_mcp_tools_service_available(
@@ -363,6 +385,28 @@ async def test_read_configuration_yaml(self, mcp_client_with_filesystem):
363385
f"Successfully read configuration.yaml ({data.get('size', 0)} bytes)"
364386
)
365387

388+
async def test_read_blueprint_file(self, mcp_client_with_filesystem):
389+
"""Blueprint files are readable by default (issue #1965)."""
390+
service_check = await _check_mcp_tools_service_available(
391+
mcp_client_with_filesystem
392+
)
393+
_skip_if_component_not_installed(service_check, "Read blueprint file")
394+
395+
async with MCPAssertions(mcp_client_with_filesystem) as mcp:
396+
result_data = await mcp.call_tool_success(
397+
"ha_read_file",
398+
{"path": "blueprints/automation/homeassistant/motion_light.yaml"},
399+
)
400+
401+
data = result_data
402+
assert data.get("success") is True, f"Read blueprint failed: {data}"
403+
content = data.get("content", "")
404+
assert "blueprint:" in content, (
405+
f"Content doesn't look like a blueprint: {content[:200]}"
406+
)
407+
408+
logger.info(f"Successfully read blueprint ({data.get('size', 0)} bytes)")
409+
366410
async def test_read_secrets_yaml_masked(self, mcp_client_with_filesystem):
367411
"""Test reading secrets.yaml - values should be masked."""
368412
service_check = await _check_mcp_tools_service_available(

tests/src/unit/test_custom_component_filesystem.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,30 @@ def test_allows_dashboards_directory(self, tmp_path):
185185
is True
186186
)
187187

188+
def test_allows_blueprints_directory_read_only(self, tmp_path):
189+
"""blueprints/ is listable/readable but NOT writable (issue #1965)."""
190+
# Read/list allowed
191+
assert (
192+
_is_path_allowed_for_dir(tmp_path, "blueprints/", ALLOWED_READ_DIRS) is True
193+
)
194+
assert (
195+
_is_path_allowed_for_dir(
196+
tmp_path, "blueprints/automation/author/thing.yaml", ALLOWED_READ_DIRS
197+
)
198+
is True
199+
)
200+
# Writes deliberately denied — blueprints is absent from ALLOWED_WRITE_DIRS
201+
assert (
202+
_is_path_allowed_for_dir(tmp_path, "blueprints/", ALLOWED_WRITE_DIRS)
203+
is False
204+
)
205+
assert (
206+
_is_path_allowed_for_dir(
207+
tmp_path, "blueprints/automation/author/thing.yaml", ALLOWED_WRITE_DIRS
208+
)
209+
is False
210+
)
211+
188212

189213
class TestIsPathAllowedForRead:
190214
"""Test _is_path_allowed_for_read function."""
@@ -226,6 +250,19 @@ def test_allows_packages_yaml(self, tmp_path):
226250
"""Should allow reading packages/*.yaml files."""
227251
assert _is_path_allowed_for_read(tmp_path, "packages/lights.yaml") is True
228252

253+
def test_allows_blueprints_files(self, tmp_path):
254+
"""Should allow reading files under blueprints/ (issue #1965)."""
255+
assert (
256+
_is_path_allowed_for_read(
257+
tmp_path, "blueprints/automation/homeassistant/motion_light.yaml"
258+
)
259+
is True
260+
)
261+
assert (
262+
_is_path_allowed_for_read(tmp_path, "blueprints/script/author/thing.yaml")
263+
is True
264+
)
265+
229266
def test_allows_custom_components_py_files(self, tmp_path):
230267
"""Should allow reading custom_components/**/*.py files."""
231268
assert (
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
"""Doc <-> allowlist parity guard for the filesystem tools (issue #1965).
2+
3+
Every directory in the component's ``ALLOWED_READ_DIRS`` / ``ALLOWED_WRITE_DIRS``
4+
must be documented in both the server tool docstrings
5+
(``ha_read_file`` / ``ha_list_files`` / ``ha_write_file`` / ``ha_delete_file`` in
6+
``src/ha_mcp/tools/tools_filesystem.py``) and the component's ``services.yaml``
7+
``path`` field descriptions. This catches the drift where a directory is added to
8+
the allowlist but a doc surface is forgotten -- exactly what happened for
9+
``dashboards/`` and is being corrected here for ``blueprints/``.
10+
11+
Self-contained on purpose: it parses the sources with ``ast`` / ``yaml`` and
12+
imports nothing from Home Assistant or fastmcp, so it runs in CI *and* locally
13+
without either installed.
14+
"""
15+
16+
import ast
17+
import importlib.util
18+
from pathlib import Path
19+
20+
import pytest
21+
import yaml
22+
23+
_REPO_ROOT = Path(__file__).resolve().parents[3]
24+
_COMPONENT_DIR = _REPO_ROOT / "custom_components" / "ha_mcp_tools"
25+
_TOOLS_FILE = _REPO_ROOT / "src" / "ha_mcp" / "tools" / "tools_filesystem.py"
26+
_SERVICES_FILE = _COMPONENT_DIR / "services.yaml"
27+
28+
29+
def _load_const_standalone():
30+
"""Load ``const.py`` directly, bypassing the HA-dependent package __init__."""
31+
spec = importlib.util.spec_from_file_location(
32+
"_ha_mcp_tools_const_parity", _COMPONENT_DIR / "const.py"
33+
)
34+
module = importlib.util.module_from_spec(spec)
35+
spec.loader.exec_module(module)
36+
return module
37+
38+
39+
_CONST = _load_const_standalone()
40+
ALLOWED_READ_DIRS = _CONST.ALLOWED_READ_DIRS
41+
ALLOWED_WRITE_DIRS = _CONST.ALLOWED_WRITE_DIRS
42+
43+
44+
def _tool_docstrings():
45+
"""Map every top-level/def name in tools_filesystem.py to its docstring."""
46+
tree = ast.parse(_TOOLS_FILE.read_text(encoding="utf-8"))
47+
docs = {}
48+
for node in ast.walk(tree):
49+
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
50+
doc = ast.get_docstring(node)
51+
if doc is not None:
52+
docs[node.name] = doc
53+
return docs
54+
55+
56+
def _service_path_descriptions():
57+
"""Map each service name to its ``path`` field description in services.yaml."""
58+
data = yaml.safe_load(_SERVICES_FILE.read_text(encoding="utf-8"))
59+
return {
60+
name: (svc.get("fields", {}).get("path", {}).get("description", "") or "")
61+
for name, svc in data.items()
62+
}
63+
64+
65+
_DOCS = _tool_docstrings()
66+
_SERVICE_PATH_DESCS = _service_path_descriptions()
67+
68+
_READ_TOOLS = ("ha_read_file", "ha_list_files")
69+
_WRITE_TOOLS = ("ha_write_file", "ha_delete_file")
70+
_READ_SERVICES = ("read_file", "list_files")
71+
_WRITE_SERVICES = ("write_file", "delete_file")
72+
73+
74+
@pytest.mark.parametrize("tool", _READ_TOOLS)
75+
@pytest.mark.parametrize("directory", ALLOWED_READ_DIRS)
76+
def test_read_dirs_documented_in_tool_docstrings(directory, tool):
77+
assert f"{directory}/" in _DOCS[tool], (
78+
f"{directory!r} is in ALLOWED_READ_DIRS but not documented in the "
79+
f"{tool} docstring (tools_filesystem.py)"
80+
)
81+
82+
83+
@pytest.mark.parametrize("tool", _WRITE_TOOLS)
84+
@pytest.mark.parametrize("directory", ALLOWED_WRITE_DIRS)
85+
def test_write_dirs_documented_in_tool_docstrings(directory, tool):
86+
assert f"{directory}/" in _DOCS[tool], (
87+
f"{directory!r} is in ALLOWED_WRITE_DIRS but not documented in the "
88+
f"{tool} docstring (tools_filesystem.py)"
89+
)
90+
91+
92+
@pytest.mark.parametrize("service", _READ_SERVICES)
93+
@pytest.mark.parametrize("directory", ALLOWED_READ_DIRS)
94+
def test_read_dirs_documented_in_services_yaml(directory, service):
95+
assert f"{directory}/" in _SERVICE_PATH_DESCS[service], (
96+
f"{directory!r} is in ALLOWED_READ_DIRS but not documented in the "
97+
f"services.yaml {service} path description"
98+
)
99+
100+
101+
@pytest.mark.parametrize("service", _WRITE_SERVICES)
102+
@pytest.mark.parametrize("directory", ALLOWED_WRITE_DIRS)
103+
def test_write_dirs_documented_in_services_yaml(directory, service):
104+
assert f"{directory}/" in _SERVICE_PATH_DESCS[service], (
105+
f"{directory!r} is in ALLOWED_WRITE_DIRS but not documented in the "
106+
f"services.yaml {service} path description"
107+
)

0 commit comments

Comments
 (0)