Skip to content

Commit efe9f1d

Browse files
kingpanther13claude
andcommitted
fix(themes): refuse engine-theme actions on an unidentifiable account
expected_current does not establish account identity. It compares values, so when ha-mcp's own user and the sidecar's engine user happen to hold the same theme the comparison passes and get_engine_theme reads, and set_engine_theme overwrites, the WRONG profile -- leaving the engine account untouched. My earlier answer that the compare-and-set covered this was wrong. An explicitly configured engine URL yields no addon_credential, so the only fallback is ha-mcp's own credential, which under the dedicated-engine- account setup this feature recommends is a different Home Assistant user. Both engine-theme actions now refuse in that case with an error naming the sidecar situation and pointing at the engine account's own Profile page, rather than acting on a profile that may not be the engine's. The restore examples in the tool docstring and beta.md omitted expected_current, teaching the unguarded form to any agent following them after a warning. Both now pass the warning's expected_current alongside the value. Raised by Codex. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Nm7tyA1nfxNCWFXaR3AxV
1 parent f3b8c0b commit efe9f1d

3 files changed

Lines changed: 80 additions & 3 deletions

File tree

docs/beta.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ The screenshot and dashboard-get tools **detect** this and report it, but
190190
never write: they read the engine account's saved theme before and after the
191191
render and, when it changed, emit a warning naming the previous value. Undoing
192192
it is a separate, explicitly write-annotated call —
193-
`ha_manage_theme(action="set_engine_theme", value=...)` — so these tools stay
193+
`ha_manage_theme(action="set_engine_theme", value=..., expected_current=...)`
194+
— passing both values from the warning, so a theme changed in the meantime is
195+
refused rather than overwritten — so these tools stay
194196
honestly `readOnlyHint: True` (#1991). `ha_manage_theme(action=
195197
"get_engine_theme")` inspects the same value. Note this is the engine
196198
account's *per-user* profile, a different layer from the backend default that

src/ha_mcp/tools/tools_themes.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,33 @@ async def _engine_credential(self) -> Any:
5050
from ..dashboard_screenshot.theme_guard import ThemeGuard
5151

5252
engine_target = await resolve_engine()
53+
if engine_target.addon_credential is None:
54+
# An explicitly configured engine URL yields no addon_credential,
55+
# so the only fallback is ha-mcp's OWN credential -- which under
56+
# the dedicated-engine-account setup this feature recommends is a
57+
# different Home Assistant user. Acting on it would read and
58+
# overwrite the wrong profile while leaving the engine account
59+
# untouched. expected_current cannot save us: it compares values,
60+
# not identity, and two users sharing a theme compare equal.
61+
raise_tool_error(
62+
create_error_response(
63+
ErrorCode.SERVICE_CALL_FAILED,
64+
"The screenshot engine's Home Assistant account cannot be "
65+
"identified, so its theme cannot be read or written.",
66+
suggestions=[
67+
(
68+
"This applies to an explicitly configured engine "
69+
+ "URL (a Docker/standalone sidecar), where the "
70+
+ "engine's own token is not discoverable."
71+
),
72+
(
73+
"Restore the theme from that account's own "
74+
+ "session: Profile > General in the Home "
75+
+ "Assistant UI."
76+
),
77+
],
78+
)
79+
)
5380
guard = ThemeGuard.for_capture(engine_target.addon_credential, self._client)
5481
if guard.credential is None:
5582
raise_tool_error(
@@ -271,8 +298,11 @@ async def ha_manage_theme(
271298
action="set", theme_name="default")
272299
- Inspect the engine account's theme: ha_manage_theme(
273300
action="get_engine_theme")
274-
- Undo a screenshot's theme change: ha_manage_theme(
275-
action="set_engine_theme", value={"theme": "", "dark": False})
301+
- Undo a screenshot's theme change (pass BOTH values from the
302+
warning, so a theme changed since then is not overwritten):
303+
ha_manage_theme(action="set_engine_theme",
304+
value={"theme": "", "dark": False},
305+
expected_current={"theme": "default", "dark": True})
276306
"""
277307
try:
278308
if action == "list":

tests/src/unit/test_dashboard_screenshot_theme_guard.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,3 +630,48 @@ async def test_without_force_the_mismatch_is_refused(self) -> None:
630630
force=False,
631631
)
632632
assert _FakeWsClient.user_data[THEME_USER_DATA_KEY] == _DARK_THEME
633+
634+
635+
class TestEngineAccountIdentity:
636+
"""Engine-theme actions refuse when the engine account is unidentifiable.
637+
638+
An explicit engine URL yields no addon_credential, so the only fallback is
639+
ha-mcp's own credential -- a different user under the dedicated-account
640+
setup this feature recommends. expected_current cannot substitute: it
641+
compares values, not identity.
642+
"""
643+
644+
async def test_refuses_when_no_addon_credential(self, monkeypatch: Any) -> None:
645+
from ha_mcp.tools.tools_themes import ThemesTools
646+
647+
async def fake_resolve() -> EngineTarget:
648+
return EngineTarget(url="http://sidecar:10000", addon_credential=None)
649+
650+
monkeypatch.setattr(
651+
"ha_mcp.dashboard_screenshot.provision.resolve_engine", fake_resolve
652+
)
653+
tools = ThemesTools(_client())
654+
655+
with pytest.raises(ToolError) as exc_info:
656+
await tools.ha_manage_theme(action="get_engine_theme")
657+
658+
assert "cannot be identified" in str(exc_info.value)
659+
assert _FakeWsClient.instances == []
660+
661+
async def test_addon_credential_is_accepted(self, monkeypatch: Any) -> None:
662+
from ha_mcp.tools.tools_themes import ThemesTools
663+
664+
async def fake_resolve() -> EngineTarget:
665+
return EngineTarget(
666+
url="http://homeassistant:8123",
667+
addon_credential=_PUPPET_CREDENTIAL,
668+
)
669+
670+
monkeypatch.setattr(
671+
"ha_mcp.dashboard_screenshot.provision.resolve_engine", fake_resolve
672+
)
673+
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_DARK_THEME)
674+
tools = ThemesTools(_client())
675+
676+
result = await tools.ha_manage_theme(action="get_engine_theme")
677+
assert result["data"]["theme"] == _DARK_THEME

0 commit comments

Comments
 (0)