Skip to content

Commit 782d125

Browse files
committed
fix(visibility): preserve report diagnostics on stdio
1 parent 3352b2c commit 782d125

14 files changed

Lines changed: 157 additions & 50 deletions

File tree

.github/workflows/haos-e2e-stdio-tests.yml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,28 @@ jobs:
162162
run: ls -lh /tmp/haos-test-image.qcow2
163163

164164
- name: Run full E2E suite through stdio against HAOS
165+
# Keep self-restarting addon tests away from concurrent workers. The
166+
# stdio lane normally skips them because they are inaddon-only, but
167+
# retaining the two-phase contract makes that isolation fail-safe if
168+
# their transport coverage is widened later.
165169
run: |
166170
cd tests
167-
uv run pytest src/e2e/ -n2 --dist loadscope -v --tb=short --maxfail=0 ${{ github.event.inputs.pytest_args }}
171+
# PYTEST_ARGS is intentionally word-split into individual pytest args.
172+
# shellcheck disable=SC2086
173+
uv run pytest src/e2e/ \
174+
-n2 --dist loadscope -v --tb=short --maxfail=0 \
175+
-m "not addon_disruptive" $PYTEST_ARGS
176+
# shellcheck disable=SC2086
177+
uv run pytest src/e2e/ \
178+
-n0 -v --tb=short --maxfail=0 \
179+
-m addon_disruptive $PYTEST_ARGS \
180+
|| { rc=$?; [ "$rc" -eq 5 ] && echo "no disruptive tests in selection"; [ "$rc" -eq 5 ]; }
168181
env:
169182
HAMCP_ENV_FILE: "tests/.env.test"
170183
HAOS_TEST_IMAGE_PATH: /tmp/haos-test-image.qcow2
171184
HAOS_TEST_MODE: stdio
172185
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
186+
PYTEST_ARGS: ${{ github.event.inputs.pytest_args }}
173187

174188
- name: Extract HA diagnostics from booted qcow2
175189
if: always()

src/ha_mcp/settings_ui/locales/en.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@
9898
"visibility.enforce.title": "Enforce mode",
9999
"visibility.enforce.help": "Extends the filter from decluttering search and overview to every tool read. A direct read of a hidden entity returns “not found”, and a content read (dashboard, template, automation, trace, log, or file) that would surface a hidden entity is refused. Best-effort concealment against incidental exposure, not a hardened security boundary.",
100100
"visibility.enforce.only_enforced": "Make hidden entities unreadable across all tools (not just search)",
101+
"visibility.enforce.report_issue_help": "The issue-report tool stays available by default so it can collect diagnostics when visibility data cannot be loaded. Enable this only if issue reports and logs must also be checked for hidden entity IDs.",
102+
"visibility.enforce.restrict_report_issue": "Apply enforce mode to ha_report_issue",
101103
"visibility.categories.title": "Hide by category",
102104
"visibility.categories.help": "Home Assistant tags auto-generated entities as diagnostic or config.",
103105
"visibility.categories.legend": "Entity categories to hide",

src/ha_mcp/settings_ui/settings.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,8 @@ <h2 class="a11y-section-title" data-i18n="accessibility.colors.title">Custom col
487487
<h2 class="a11y-section-title" data-i18n="visibility.enforce.title">Enforce mode</h2>
488488
<p class="a11y-section-help" data-i18n="visibility.enforce.help">Extends the filter from decluttering search and overview to every tool read. A direct read of a hidden entity returns &ldquo;not found&rdquo;, and a content read (dashboard, template, automation, trace, log, or file) that would surface a hidden entity is refused. Best-effort concealment against incidental exposure, not a hardened security boundary.</p>
489489
<label class="a11y-option"><input type="checkbox" id="visibility-enforce"> <span data-i18n="visibility.enforce.only_enforced">Make hidden entities unreadable across all tools (not just search)</span></label>
490+
<p class="a11y-section-help" data-i18n="visibility.enforce.report_issue_help">The issue-report tool stays available by default so it can collect diagnostics when visibility data cannot be loaded. Enable this only if issue reports and logs must also be checked for hidden entity IDs.</p>
491+
<label class="a11y-option"><input type="checkbox" id="visibility-restrict-report-issue"> <span data-i18n="visibility.enforce.restrict_report_issue">Apply enforce mode to ha_report_issue</span></label>
490492
</section>
491493

492494
<section class="a11y-section">

src/ha_mcp/settings_ui/settings.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4488,6 +4488,7 @@ async function visibilityLoadConfig() {
44884488
const cats = c.exclude_categories || [];
44894489
document.getElementById('visibility-enabled').checked = !!c.enabled;
44904490
document.getElementById('visibility-enforce').checked = !!c.enforce;
4491+
document.getElementById('visibility-restrict-report-issue').checked = !!c.restrict_report_issue;
44914492
document.getElementById('visibility-cat-diagnostic').checked = cats.includes('diagnostic');
44924493
document.getElementById('visibility-cat-config').checked = cats.includes('config');
44934494
document.getElementById('visibility-exclude-hidden').checked = !!c.exclude_hidden;
@@ -4509,6 +4510,7 @@ async function visibilitySaveConfig() {
45094510
version: visibilityVersion,
45104511
enabled: document.getElementById('visibility-enabled').checked,
45114512
enforce: document.getElementById('visibility-enforce').checked,
4513+
restrict_report_issue: document.getElementById('visibility-restrict-report-issue').checked,
45124514
exclude_categories: cats,
45134515
exclude_hidden: document.getElementById('visibility-exclude-hidden').checked,
45144516
deny_entity_ids: _visibilityParseList(document.getElementById('visibility-deny').value, '\n'),

src/ha_mcp/stdio_settings_sidecar.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,6 +1217,19 @@ async def dispatch(
12171217
handlers["policy_get_value_source"],
12181218
methods=["GET"],
12191219
),
1220+
# Entity visibility filter endpoints. The sidecar owns a hand-maintained
1221+
# route table (it cannot reuse FastMCP custom-route registration), so
1222+
# keep this pair in lockstep with settings_ui.register_settings_routes.
1223+
Route(
1224+
f"{secret_prefix}/api/visibility/config",
1225+
handlers["visibility_get_config"],
1226+
methods=["GET"],
1227+
),
1228+
Route(
1229+
f"{secret_prefix}/api/visibility/config",
1230+
handlers["visibility_put_config"],
1231+
methods=["PUT"],
1232+
),
12201233
]
12211234

12221235
# /shutdown — POST endpoint that drops the disable sentinel and

src/ha_mcp/visibility/enforcement.py

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
- **Unscannable surfaces are refused wholesale** while enforce is active — sandbox
2020
code execution and screenshot/pixel output can read arbitrary state that no text
2121
scan can attribute to an entity.
22+
- **Issue reports stay available by default.** ``ha_report_issue`` is the recovery
23+
path when visibility inputs fail, so operators must explicitly opt it into the
24+
same inbound/outbound scans with ``restrict_report_issue``.
2225
2326
This is a strong read barrier against *incidental* exposure, not a cryptographic
2427
guarantee: a Jinja template that derives a hidden entity's state without ever
@@ -70,6 +73,7 @@
7073
_DASHBOARD_TOOL = "ha_config_get_dashboard"
7174
_DASHBOARD_SET_TOOL = "ha_config_set_dashboard"
7275
_CUSTOM_TOOL = "ha_manage_custom_tool"
76+
_REPORT_ISSUE_TOOL = "ha_report_issue"
7377

7478
_SANDBOX_REASON = (
7579
"'{name}' executes sandbox code that can read arbitrary Home Assistant state, "
@@ -249,8 +253,8 @@ def _coerce_arguments(arguments: Any) -> dict[str, Any] | None:
249253
return None
250254

251255

252-
def _unwrap_proxy_call(args: dict[str, Any]) -> dict[str, Any] | None:
253-
"""Extract the innermost proxy-envelope ``arguments`` dict, or None if unusable.
256+
def _unwrap_proxy_call(args: dict[str, Any]) -> tuple[str, dict[str, Any]] | None:
257+
"""Extract the innermost proxy target and arguments, or None if unusable.
254258
255259
Mirrors ``ReadOnlyMiddleware._unwrap_proxy_call``: the categorized call proxies
256260
accept ``arguments`` as a JSON string, so an inner exact entity_id match would
@@ -269,7 +273,15 @@ def _unwrap_proxy_call(args: dict[str, Any]) -> dict[str, Any] | None:
269273
arguments = _coerce_arguments(arguments.get("arguments"))
270274
if not isinstance(name, str) or arguments is None:
271275
return None
272-
return arguments
276+
return name, arguments
277+
278+
279+
def _effective_tool_name(name: str, args: dict[str, Any]) -> str:
280+
"""Return the real tool name when this call is a categorized proxy."""
281+
if name not in PROXY_META_TOOLS:
282+
return name
283+
inner = _unwrap_proxy_call(args)
284+
return inner[0] if inner is not None else name
273285

274286

275287
def _config_cache_key(config: VisibilityConfig) -> str:
@@ -324,6 +336,14 @@ async def _active_config(self, tool_name: str) -> VisibilityConfig | None:
324336
"using last-known-good config" if config else "failing closed",
325337
exc_info=True,
326338
)
339+
# ``ha_report_issue`` is intentionally the diagnostic escape hatch. A
340+
# missing/corrupt first config load therefore follows its safe default
341+
# (unrestricted), while a last-known-good explicit opt-in continues to
342+
# enforce. Successful loads consult the live toggle on every call.
343+
if tool_name == _REPORT_ISSUE_TOOL and (
344+
config is None or not config.restrict_report_issue
345+
):
346+
return None
327347
if config is None:
328348
_raise_enforced(tool_name, _CONFIG_LOAD_FAILED_REASON)
329349
if not (
@@ -354,12 +374,11 @@ class VisibilityInboundEnforcement(_VisibilityEnforcementBase):
354374
async def on_call_tool(
355375
self, context: MiddlewareContext, call_next: CallNext
356376
) -> Any:
357-
config = await self._active_config(context.message.name)
358-
if config is None:
359-
return await call_next(context)
360-
361377
name = context.message.name
362378
args = context.message.arguments or {}
379+
config = await self._active_config(_effective_tool_name(name, args))
380+
if config is None:
381+
return await call_next(context)
363382

364383
reason = _unscannable_reason(name, args)
365384
if reason is not None:
@@ -387,7 +406,7 @@ def _scan_inbound(
387406
if name in PROXY_META_TOOLS:
388407
inner = _unwrap_proxy_call(args)
389408
if inner is not None:
390-
exact, embedded = _scan_value(inner, hidden, regex)
409+
exact, embedded = _scan_value(inner[1], hidden, regex)
391410
raw_exact, raw_embedded = _scan_value(args, hidden, regex)
392411
exact = exact or raw_exact
393412
embedded = embedded or raw_embedded
@@ -409,10 +428,11 @@ class VisibilityOutboundEnforcement(_VisibilityEnforcementBase):
409428
async def on_call_tool(
410429
self, context: MiddlewareContext, call_next: CallNext
411430
) -> Any:
412-
config = await self._active_config(context.message.name)
431+
name = context.message.name
432+
args = context.message.arguments or {}
433+
config = await self._active_config(_effective_tool_name(name, args))
413434
if config is None:
414435
return await call_next(context)
415-
name = context.message.name
416436
try:
417437
_hidden, regex = await self._hidden_and_regex(config)
418438
except VisibilityDataUnavailable:

src/ha_mcp/visibility/model.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ class VisibilityConfig(BaseModel):
4242
# the same hidden set is applied, not which entities are hidden, so it is
4343
# deliberately absent from ``to_wire`` and ``config_has_active_hide_dimensions``.
4444
enforce: bool = False
45+
# ``ha_report_issue`` is the recovery path when visibility enforcement or
46+
# its HA registry inputs fail. Keep it outside the barrier by default so it
47+
# can return diagnostics; operators who treat report/log output as sensitive
48+
# can opt it back into the normal inbound + outbound scans.
49+
restrict_report_issue: bool = False
4550

4651
def to_wire(self) -> dict[str, Any]:
4752
"""Serialize the hide dimensions for the component ``search`` fast path.

tests/src/e2e/basic/test_backend_dispatch_smoke.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@
8585
# gain 10; container is unchanged because the tests run there.
8686
# Entries below are CI-observed item counts, bumped only for intentional
8787
# marker-gated additions rather than runtime skips.
88-
"container": 74, # +1 HAOS stdio visibility e2e (haos_only)
88+
"container": 74, # +1 HAOS stdio visibility e2e (haos_stdio_only)
8989
"haos": 49, # +1 HAOS stdio visibility e2e (haos_stdio_only)
9090
# HAOS stdio is the external HAOS set plus ``external_only`` tests, whose
91-
# test-process monkeypatches cannot reach the subprocess server. Start with
92-
# a conservative static ceiling; replace it with the CI-observed count once
93-
# the first lane run reports its collection summary.
94-
"haos_stdio": 112,
91+
# test-process monkeypatches cannot reach the subprocess server. The first
92+
# full lane run on 2026-08-18 observed 103 collection-time marker skips
93+
# (plus 9 runtime skips); keep the same five-item buffer as established lanes.
94+
"haos_stdio": 108,
9595
"haos_inaddon": 77, # +1 HAOS stdio visibility e2e (haos_stdio_only)
9696
# Embedded backend (#1527, E2E_BACKEND=embedded). Skips exactly the container
9797
# lane's marker-skips PLUS two embedded-specific additions:
@@ -106,7 +106,7 @@
106106
# 1) + not_on_embedded 2 = 101. Parametrize inflates that to the CI-observed
107107
# count the entry below is pinned to — 133 on this PR's run, 132 before the
108108
# self-restart e2e. Read the count off a run rather than deriving it.
109-
"embedded": 134, # +1 HAOS stdio visibility e2e (haos_only)
109+
"embedded": 134, # +1 HAOS stdio visibility e2e (haos_stdio_only)
110110
# HAOS embedded backend (#1527, HAOS_TEST_MODE=embedded). A HAOS lane, so it
111111
# skips the SAME set as the external HAOS lane (container_only + inaddon_only)
112112
# PLUS two haos_embedded-specific additions:

tests/src/e2e/conftest.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,9 @@ def pytest_collection_modifyitems(config, items):
241241
- ``container_only``: only runs on the testcontainer backend.
242242
- ``external_only``: any tier where the server-under-test lives IN the
243243
pytest process — plain testcontainer AND HAOS external (``mcp_client``
244-
is an in-process FastMCP server talking HTTP to HAOS). Skipped on
245-
inaddon, container-embedded and HAOS-embedded. The name is historical
246-
and does NOT mean "HAOS external only"; see the skip expression below.
244+
is an in-process FastMCP server talking HTTP to HAOS). Skipped on stdio,
245+
inaddon, container-embedded, and HAOS-embedded. The name is historical and
246+
does NOT mean "HAOS external only"; see the skip expression below.
247247
- ``inaddon_only``: HAOS inaddon mode only (``mcp_client`` is HTTP
248248
to the addon's MCP endpoint, ``is_running_in_addon()=True`` paths
249249
exercised). Skipped on external mode and on testcontainer.
@@ -264,10 +264,10 @@ def pytest_collection_modifyitems(config, items):
264264
# The HAOS embedded lane (#1527) IS a HAOS backend (``haos`` True — qcow2
265265
# staged), so ``haos_only`` runs and ``container_only`` skips exactly like the
266266
# other HAOS lanes. Its server-under-test is the in-process MCP server
267-
# inside the HAOS core container, driven over its ingress webhook — same
268-
# out-of-process constraint as inaddon / container-embedded, so ``external_only``
269-
# skips here too; and the haos_only embedded smoke module is redundant with the
270-
# lane's own session backend, so it skips via ``not_on_haos_embedded``.
267+
# inside the HAOS core container, driven over its ingress webhook — the same
268+
# out-of-process constraint as stdio/inaddon/container-embedded, so
269+
# ``external_only`` skips here too; the haos_only embedded smoke module is
270+
# redundant with the lane's session backend and skips via ``not_on_haos_embedded``.
271271
haos_embedded = haos and is_haos_embedded_mode()
272272
skip_haos = pytest.mark.skip(
273273
reason="HAOS backend not selected (set HAOS_TEST_IMAGE_PATH)"
@@ -282,7 +282,7 @@ def pytest_collection_modifyitems(config, items):
282282
reason="HAOS stdio mode required (set HAOS_TEST_MODE=stdio)"
283283
)
284284
skip_external_only = pytest.mark.skip(
285-
reason="out-of-process server (inaddon/embedded); test needs an "
285+
reason="out-of-process server (stdio/inaddon/embedded); test needs an "
286286
"in-process server it can reconfigure via env/monkeypatch or reach an "
287287
"in-process mock"
288288
)
@@ -2823,9 +2823,7 @@ async def mcp_client(
28232823
client = _stdio_client(container_info, haos_stdio_config_dir)
28242824
try:
28252825
async with client:
2826-
logger.debug(
2827-
"🔗 FastMCP client connected (stdio subprocess transport)"
2828-
)
2826+
logger.debug("🔗 FastMCP client connected (stdio subprocess transport)")
28292827
yield client
28302828
finally:
28312829
_retire_stdio_sidecar(haos_stdio_config_dir)
@@ -2897,6 +2895,7 @@ def _stdio_client(container_info: dict[str, Any], config_dir: Path) -> Client:
28972895
command="ha-mcp",
28982896
args=[],
28992897
env=_stdio_env(container_info, config_dir),
2898+
keep_alive=False,
29002899
)
29012900
return Client(transport)
29022901

tests/src/e2e/haos_only/test_stdio_entity_visibility.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,9 @@ async def test_stdio_sidecar_config_drives_real_visibility_middleware(
6464
# Seed the subprocess usage log with the entity id. report_issue's
6565
# recent_logs field makes the later outbound-policy assertion
6666
# deterministic without creating or mutating any HA entity.
67-
await mcp.call_tool_success(
68-
"ha_get_state", {"entity_id": _HIDDEN_ENTITY}
69-
)
67+
await mcp.call_tool_success("ha_get_state", {"entity_id": _HIDDEN_ENTITY})
7068

71-
current = await asyncio.to_thread(
72-
_visibility_request, settings_url, "GET"
73-
)
69+
current = await asyncio.to_thread(_visibility_request, settings_url, "GET")
7470
saved = await asyncio.to_thread(
7571
_visibility_request,
7672
settings_url,
@@ -119,10 +115,9 @@ async def test_stdio_sidecar_config_drives_real_visibility_middleware(
119115
"ha_report_issue",
120116
{"tool_call_count": 2, "fields": ["recent_logs"]},
121117
)
122-
assert (
123-
restricted_report["error"]["code"]
124-
== "ENTITY_VISIBILITY_ENFORCED"
125-
), restricted_report
118+
assert restricted_report["error"]["code"] == "ENTITY_VISIBILITY_ENFORCED", (
119+
restricted_report
120+
)
126121
finally:
127122
# Session-scoped stdio clients are shared by many tests on this worker;
128123
# always restore no-op visibility even when an API assertion fails.

0 commit comments

Comments
 (0)