Skip to content

Commit e7d6c35

Browse files
kingpanther13claude
andcommitted
fix(themes): never treat an unproven credential as the engine's account
Two Codex P1s, both about credential-to-engine identity. The second undermines the refusal added in efe9f1d, so that fix was incomplete. An explicit engine URL is never provably paired with a credential. On HA OS, _addon_credential_best_effort() hands back the DISCOVERED Puppet app's credential without checking it identifies the engine that URL points at, so a sidecar URL plus a running app produced a credential for the wrong account -- and my earlier check, which only tested for a MISSING addon_credential, passed it straight through. The engine-theme actions now refuse whenever an engine URL is set explicitly, regardless of what discovery turned up. That pairing was documented as a safe no-op, and it was, while the guard only wrote back its own snapshot; it stopped being one when these actions began reading and writing a profile on request. The capture guard has the same uncertainty but must not simply go dark: its client-credential fallback is what protects the common single-user Docker setup, where ha-mcp and the engine really are the same user. It stays active and now tracks whether its credential is provably the engine's. When it is not, the report says so and tells the agent to verify before restoring, rather than implying the engine's account was the one observed. Also fixes an import in the new resolver guard that reached beyond the package (...config from src/ha_mcp/tools/), which turned the refusal into an INTERNAL_ERROR. The identity tests caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Nm7tyA1nfxNCWFXaR3AxV
1 parent 9e2350b commit e7d6c35

3 files changed

Lines changed: 68 additions & 5 deletions

File tree

src/ha_mcp/dashboard_screenshot/theme_guard.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ class ThemeGuard:
191191
_snapshot: Any = None
192192
_snapshot_taken: bool = False
193193
changed_from: Any = None
194+
# False when the credential is ha-mcp's own rather than the engine's own
195+
# token: it MAY be the same Home Assistant user (the common single-user
196+
# setup) but that cannot be established, so the report says so.
197+
credential_is_engine: bool = True
194198

195199
@classmethod
196200
def for_capture(
@@ -200,12 +204,17 @@ def for_capture(
200204
) -> ThemeGuard:
201205
"""Resolve the engine user's credential for one capture batch."""
202206
credential = addon_credential or _client_credential(client)
207+
# Only the add-on's own token provably belongs to the engine. The
208+
# client fallback MAY be the same user (the common single-user setup)
209+
# but that cannot be established, so the report says so rather than
210+
# implying the engine account was the one observed.
211+
provably_engine = addon_credential is not None
203212
if credential is None:
204213
logger.debug(
205214
"Dashboard theme guard inactive: no engine credential is "
206215
"discoverable in this deployment"
207216
)
208-
return cls(credential=credential)
217+
return cls(credential=credential, credential_is_engine=provably_engine)
209218

210219
@asynccontextmanager
211220
async def _session(self) -> AsyncIterator[HomeAssistantWebSocketClient]:
@@ -353,6 +362,14 @@ async def detect_change(self) -> None:
353362
"meantime. To stop this happening at all, give the screenshot "
354363
"engine its own Home Assistant user and long-lived token, so "
355364
"its writes land on an account nobody looks at."
365+
+ (
366+
""
367+
if self.credential_is_engine
368+
else " NOTE: this was observed with ha-mcp's own Home "
369+
"Assistant credential rather than the engine's own token, so "
370+
"it is the engine's account only if both run as the same "
371+
"user. Verify before restoring."
372+
)
356373
)
357374

358375

src/ha_mcp/tools/tools_themes.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,22 @@ async def _engine_credential(self) -> Any:
4646
engine-account setup is NOT the account ha-mcp itself authenticates
4747
as -- so these actions cannot simply reuse ha-mcp's own client.
4848
"""
49+
from ..config import get_global_settings
4950
from ..dashboard_screenshot.provision import resolve_engine
5051
from ..dashboard_screenshot.theme_guard import ThemeGuard
5152

53+
explicit_url = (
54+
get_global_settings().dashboard_screenshot_engine_url or ""
55+
).strip()
5256
engine_target = await resolve_engine()
53-
if engine_target.addon_credential is None:
57+
# An explicit URL is never provably paired with a credential. Even on
58+
# HA OS, _addon_credential_best_effort() hands back the DISCOVERED
59+
# Puppet app's credential without checking it identifies the engine
60+
# that URL points at -- so a sidecar URL plus a running app yields a
61+
# credential for the wrong account. That was a safe no-op while the
62+
# guard only wrote back its own snapshot; it is not safe now that
63+
# these actions read and write a profile on request.
64+
if explicit_url or engine_target.addon_credential is None:
5465
# An explicitly configured engine URL yields no addon_credential,
5566
# so the only fallback is ha-mcp's OWN credential -- which under
5667
# the dedicated-engine-account setup this feature recommends is a
@@ -65,9 +76,10 @@ async def _engine_credential(self) -> Any:
6576
"identified, so its theme cannot be read or written.",
6677
suggestions=[
6778
(
68-
"This applies to an explicitly configured engine "
69-
+ "URL (a Docker/standalone sidecar), where the "
70-
+ "engine's own token is not discoverable."
79+
"This applies whenever an engine URL is set "
80+
+ "explicitly: the engine's own token is not "
81+
+ "discoverable, and a credential discovered via "
82+
+ "the Supervisor is not provably that engine's."
7183
),
7284
(
7385
"Restore the theme from that account's own "

tests/src/unit/test_dashboard_screenshot_theme_guard.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,3 +755,37 @@ async def test_remote_cleartext_session_warns_and_sends_nothing(self) -> None:
755755

756756
assert _FakeWsClient.instances == []
757757
assert guard.warnings
758+
759+
760+
class TestCredentialProvenance:
761+
"""A client-fallback credential is not provably the engine's account."""
762+
763+
def test_addon_credential_is_provably_the_engine(self) -> None:
764+
guard = ThemeGuard.for_capture(_PUPPET_CREDENTIAL, _client())
765+
assert guard.credential_is_engine is True
766+
767+
def test_client_fallback_is_not_provably_the_engine(self) -> None:
768+
guard = ThemeGuard.for_capture(None, _client())
769+
assert guard.credential is not None
770+
assert guard.credential_is_engine is False
771+
772+
async def test_uncertain_provenance_is_stated_in_the_report(self) -> None:
773+
"""The agent must not restore a profile we cannot vouch for."""
774+
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_DARK_THEME)
775+
guard = ThemeGuard.for_capture(None, _client())
776+
await guard.take_snapshot()
777+
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_CLOBBERED_THEME)
778+
await guard.detect_change()
779+
780+
assert guard.warnings
781+
assert "only if both run as the same user" in guard.warnings[0]
782+
783+
async def test_engine_credential_report_carries_no_caveat(self) -> None:
784+
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_DARK_THEME)
785+
guard = ThemeGuard.for_capture(_PUPPET_CREDENTIAL, None)
786+
await guard.take_snapshot()
787+
_FakeWsClient.user_data[THEME_USER_DATA_KEY] = dict(_CLOBBERED_THEME)
788+
await guard.detect_change()
789+
790+
assert guard.warnings
791+
assert "same user" not in guard.warnings[0]

0 commit comments

Comments
 (0)