fix(diagnostics): D2 — redact through homeassistant.helpers.redact (t37)
`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.
This commit is contained in:
1 parent
37b2accfda
commit
bd72ae3170
3 files changed
+98
-10
No files matched your search
@@ -16,6 +16,7 @@ from dataclasses import asdict, is_dataclass
|
|||||||
|
|
||||||
from homeassistant.config_entries import ConfigEntry
|
from homeassistant.config_entries import ConfigEntry
|
||||||
from homeassistant.core import HomeAssistant
|
from homeassistant.core import HomeAssistant
|
||||||
|
from homeassistant.helpers.redact import async_redact_data
|
||||||
|
|
||||||
from .connection import ShieldConnectionManager
|
from .connection import ShieldConnectionManager
|
||||||
|
|
||||||
@@ -87,15 +88,13 @@ async def async_get_config_entry_diagnostics(
|
|||||||
"is_on": manager.is_on,
|
"is_on": manager.is_on,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
# G14 (t30): the documented helper is the primary redaction path with
|
# G14 (t30) + t37 D2: the documented helper is the primary redaction path
|
||||||
# an explicit field list; the marker scan above runs defensively after.
|
# with an explicit field list (the hosts included), and the marker scan
|
||||||
try:
|
# above runs defensively after it. It is imported at MODULE scope from
|
||||||
from homeassistant.util.diagnostic import async_redact_data
|
# homeassistant.helpers.redact — the real module; the old
|
||||||
except ImportError:
|
# `homeassistant.util.diagnostic` never existed, and the swallowed
|
||||||
async_redact_data = None
|
# ImportError turned the whole pass into a silent no-op.
|
||||||
if async_redact_data is not None:
|
return _redact(async_redact_data(payload, _TO_REDACT))
|
||||||
payload = async_redact_data(payload, _TO_REDACT)
|
|
||||||
return _redact(payload)
|
|
||||||
|
|
||||||
|
|
||||||
__all__ = ["async_get_config_entry_diagnostics"]
|
__all__ = ["async_get_config_entry_diagnostics"]
|
||||||
@@ -10,6 +10,7 @@ import asyncio
|
|||||||
import enum
|
import enum
|
||||||
import sys
|
import sys
|
||||||
import types
|
import types
|
||||||
|
from collections.abc import Mapping
|
||||||
|
|
||||||
import voluptuous_mirror
|
import voluptuous_mirror
|
||||||
|
|
||||||
@@ -29,6 +30,7 @@ ENTITIES_STUB_NAMES = (
|
|||||||
"homeassistant.helpers.device_registry",
|
"homeassistant.helpers.device_registry",
|
||||||
"homeassistant.helpers.selector",
|
"homeassistant.helpers.selector",
|
||||||
"homeassistant.helpers.event",
|
"homeassistant.helpers.event",
|
||||||
|
"homeassistant.helpers.redact",
|
||||||
"homeassistant.helpers.entity",
|
"homeassistant.helpers.entity",
|
||||||
"homeassistant.helpers.entity_platform",
|
"homeassistant.helpers.entity_platform",
|
||||||
"nsh.helpers",
|
"nsh.helpers",
|
||||||
@@ -410,6 +412,42 @@ def install() -> None:
|
|||||||
entity_platform.AddEntitiesCallback = object
|
entity_platform.AddEntitiesCallback = object
|
||||||
helpers.entity_platform = entity_platform
|
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.event"] = event
|
||||||
sys.modules["homeassistant.helpers.entity"] = entity
|
sys.modules["homeassistant.helpers.entity"] = entity
|
||||||
sys.modules["homeassistant.helpers.entity_platform"] = entity_platform
|
sys.modules["homeassistant.helpers.entity_platform"] = entity_platform
|
||||||
|
|||||||
@@ -994,10 +994,16 @@ class TestEntryDiagnostics:
|
|||||||
from types import SimpleNamespace as DiagEntry
|
from types import SimpleNamespace as DiagEntry
|
||||||
|
|
||||||
entry = DiagEntry(
|
entry = DiagEntry(
|
||||||
data={},
|
data={
|
||||||
|
"name": "Shield",
|
||||||
|
"androidtv_entity": "media_player.shield",
|
||||||
|
"nested": {"adb_host": "10.0.0.5"},
|
||||||
|
},
|
||||||
options={
|
options={
|
||||||
"adb_command_enabled": True,
|
"adb_command_enabled": True,
|
||||||
"protocol_pairing_code": "4821",
|
"protocol_pairing_code": "4821",
|
||||||
|
"adb_host": "192.168.1.50",
|
||||||
|
"protocol_host": "192.168.1.50",
|
||||||
},
|
},
|
||||||
title="Shield",
|
title="Shield",
|
||||||
)
|
)
|
||||||
@@ -1010,6 +1016,14 @@ class TestEntryDiagnostics:
|
|||||||
payload = await async_get_config_entry_diagnostics(None, entry)
|
payload = await async_get_config_entry_diagnostics(None, entry)
|
||||||
|
|
||||||
assert payload["entry"]["options"]["protocol_pairing_code"] == "**REDACTED**"
|
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 payload["entry"]["options"]["adb_command_enabled"] is True
|
||||||
assert "awake" in payload["store"]
|
assert "awake" in payload["store"]
|
||||||
assert payload["transport"]["adb"]["connected"] is not None
|
assert payload["transport"]["adb"]["connected"] is not None
|
||||||
@@ -1062,6 +1076,43 @@ class TestFaithfulHaStubs:
|
|||||||
domain="nvidia_shield", # G18 keyword of BrowseMediaSource only
|
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:
|
class TestBrowseMediaDirectory:
|
||||||
"""P4-2c: the app directory root and its launch path."""
|
"""P4-2c: the app directory root and its launch path."""
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user