From bd72ae31708568166b07241f2ec43a83fb5d0df2 Mon Sep 17 00:00:00 2001 From: James Mackay Date: Sat, 10 Oct 2026 21:20:04 +0100 Subject: [PATCH] =?UTF-8?q?fix(diagnostics):=20D2=20=E2=80=94=20redact=20t?= =?UTF-8?q?hrough=20homeassistant.helpers.redact=20(t37)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `homeassistant.util.diagnostic` does not exist — verified against a real install: `import homeassistant.util.diagnostic` raises ModuleNotFoundError, and the helper has always lived in `homeassistant.helpers.redact`. The import sat inside a `try/except ImportError` that set the helper to None, so the documented G14 pass — the one carrying `_TO_REDACT`, hosts included — silently did nothing; only the marker scan (pairing-/key-shaped names) masked anything, which is why the existing test's pairing-code assertion stayed green. The helper is now imported at MODULE scope from homeassistant.helpers.redact and applied unconditionally after the marker-name pass. A missing module is a loud import error at integration load instead of a silent downgrade. * the diagnostics test now asserts `adb_host` and `protocol_host` really are masked in the emitted payload (options and a nested data value); * TestFaithfulHaStubs asserts the module USES the helper from that path (`nsh.diagnostics.async_redact_data is homeassistant.helpers.redact.async_redact_data`), that no production module imports the non-existent module (AST), and that the try/except ImportError fallback is gone; * `entity_stublib` gains a faithful `homeassistant.helpers.redact` stub (same masking, same nested mapping/list recursion, same None/empty-string skip) so the harness meets the import the way a real install does. Verified: 287 passed, 2 skipped (D2 stage); ruff clean. Full gate re-run on the completed branch: 289 passed / 2 skipped and 290 passed / 1 skipped. --- .../nvidia_shield/diagnostics.py | 17 +++--- tests/entity_stublib.py | 38 +++++++++++++ tests/test_command_routing.py | 53 ++++++++++++++++++- 3 files changed, 98 insertions(+), 10 deletions(-) diff --git a/custom_components/nvidia_shield/diagnostics.py b/custom_components/nvidia_shield/diagnostics.py index 1a3f7ff..dad6078 100644 --- a/custom_components/nvidia_shield/diagnostics.py +++ b/custom_components/nvidia_shield/diagnostics.py @@ -16,6 +16,7 @@ from dataclasses import asdict, is_dataclass from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant +from homeassistant.helpers.redact import async_redact_data from .connection import ShieldConnectionManager @@ -87,15 +88,13 @@ async def async_get_config_entry_diagnostics( "is_on": manager.is_on, }, } - # G14 (t30): the documented helper is the primary redaction path with - # an explicit field list; the marker scan above runs defensively after. - try: - from homeassistant.util.diagnostic import async_redact_data - except ImportError: - async_redact_data = None - if async_redact_data is not None: - payload = async_redact_data(payload, _TO_REDACT) - return _redact(payload) + # G14 (t30) + t37 D2: the documented helper is the primary redaction path + # with an explicit field list (the hosts included), and the marker scan + # above runs defensively after it. It is imported at MODULE scope from + # homeassistant.helpers.redact — the real module; the old + # `homeassistant.util.diagnostic` never existed, and the swallowed + # ImportError turned the whole pass into a silent no-op. + return _redact(async_redact_data(payload, _TO_REDACT)) __all__ = ["async_get_config_entry_diagnostics"] diff --git a/tests/entity_stublib.py b/tests/entity_stublib.py index ff48b73..cda738a 100644 --- a/tests/entity_stublib.py +++ b/tests/entity_stublib.py @@ -10,6 +10,7 @@ import asyncio import enum import sys import types +from collections.abc import Mapping import voluptuous_mirror @@ -29,6 +30,7 @@ ENTITIES_STUB_NAMES = ( "homeassistant.helpers.device_registry", "homeassistant.helpers.selector", "homeassistant.helpers.event", + "homeassistant.helpers.redact", "homeassistant.helpers.entity", "homeassistant.helpers.entity_platform", "nsh.helpers", @@ -410,6 +412,42 @@ def install() -> None: entity_platform.AddEntitiesCallback = object helpers.entity_platform = entity_platform + # t37 D2: homeassistant.helpers.redact.async_redact_data — the REAL helper + # (diagnostics.py imports it at module scope; `homeassistant.util.diagnostic` + # never existed). Modelled faithfully: the same masking of keys found in + # ``to_redact``, the same nested mapping/list recursion, the same skip of + # None/empty-string values. + redact = types.ModuleType("homeassistant.helpers.redact") + redact.REDACTED = "**REDACTED**" + + def _stub_async_redact_data(data, to_redact): + if not isinstance(data, (Mapping, list)): + return data + if isinstance(data, list): + return [_stub_async_redact_data(val, to_redact) for val in data] + redacted = {**data} + for key, value in redacted.items(): + if value is None: + continue + if isinstance(value, str) and not value: + continue + if key in to_redact: + if isinstance(to_redact, Mapping): + redacted[key] = to_redact[key](value) + else: + redacted[key] = redact.REDACTED + elif isinstance(value, Mapping): + redacted[key] = _stub_async_redact_data(value, to_redact) + elif isinstance(value, list): + redacted[key] = [ + _stub_async_redact_data(item, to_redact) for item in value + ] + return redacted + + redact.async_redact_data = _stub_async_redact_data + helpers.redact = redact + sys.modules["homeassistant.helpers.redact"] = redact + sys.modules["homeassistant.helpers.event"] = event sys.modules["homeassistant.helpers.entity"] = entity sys.modules["homeassistant.helpers.entity_platform"] = entity_platform diff --git a/tests/test_command_routing.py b/tests/test_command_routing.py index bb511c5..6929ad6 100644 --- a/tests/test_command_routing.py +++ b/tests/test_command_routing.py @@ -994,10 +994,16 @@ class TestEntryDiagnostics: from types import SimpleNamespace as DiagEntry entry = DiagEntry( - data={}, + data={ + "name": "Shield", + "androidtv_entity": "media_player.shield", + "nested": {"adb_host": "10.0.0.5"}, + }, options={ "adb_command_enabled": True, "protocol_pairing_code": "4821", + "adb_host": "192.168.1.50", + "protocol_host": "192.168.1.50", }, title="Shield", ) @@ -1010,6 +1016,14 @@ class TestEntryDiagnostics: payload = await async_get_config_entry_diagnostics(None, entry) assert payload["entry"]["options"]["protocol_pairing_code"] == "**REDACTED**" + # t37 D2: the G14 field list (hosts included) really applies now — the + # old import pointed at homeassistant.util.diagnostic, which does not + # exist, so this pass silently did nothing and only the marker scan + # (pairing-/key-shaped names) masked anything. + assert payload["entry"]["options"]["adb_host"] == "**REDACTED**" + assert payload["entry"]["options"]["protocol_host"] == "**REDACTED**" + assert payload["entry"]["data"]["nested"]["adb_host"] == "**REDACTED**" + assert payload["entry"]["data"]["androidtv_entity"] == "media_player.shield" assert payload["entry"]["options"]["adb_command_enabled"] is True assert "awake" in payload["store"] assert payload["transport"]["adb"]["connected"] is not None @@ -1062,6 +1076,43 @@ class TestFaithfulHaStubs: domain="nvidia_shield", # G18 keyword of BrowseMediaSource only ) + def test_redaction_uses_the_real_helper_module(self): + import homeassistant.helpers.redact as redact + import nsh.diagnostics as diagnostics + + assert redact.REDACTED == "**REDACTED**" + # the module actually USES the helper from the real module path + assert diagnostics.async_redact_data is redact.async_redact_data + assert {"adb_host", "protocol_host"} <= diagnostics._TO_REDACT + assert diagnostics._REDACTED == redact.REDACTED + + def test_no_production_module_imports_the_nonexistent_diagnostic_module(self): + """t37 D2: `homeassistant.util.diagnostic` never existed.""" + offenders = [ + f"{path.name}:{node.lineno}" + for path in sorted(_SHIELD_DIR.glob("*.py")) + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))) + if isinstance(node, ast.ImportFrom) + and node.module == "homeassistant.util.diagnostic" + ] + assert offenders == [] + + def test_diagnostics_has_no_silent_import_fallback(self): + """The redaction import is at module scope — loud, not swallowed.""" + tree = ast.parse( + (_SHIELD_DIR / "diagnostics.py").read_text(encoding="utf-8") + ) + caught = [ + handler + for node in ast.walk(tree) + if isinstance(node, ast.Try) + for handler in node.handlers + if isinstance(handler.type, ast.Name) + and handler.type.id == "ImportError" + ] + assert caught == [] + + class TestBrowseMediaDirectory: """P4-2c: the app directory root and its launch path."""