test: pin the behavior 100% coverage was not pinning

tests/properties/test_identifier_codec.py

@@ -0,0 +1,113 @@
+"""
+Property-based tests for the slot identifier codecs.
+
+The sibling tag codec in ``providers/_util.py`` has had round-trip properties
+since the property suite was created; these codecs did not, and the asymmetry
+cost a real bug. ``parse_slot_device_identifier`` gated on ``str.isdigit()``,
+which rejects the negative slot the builder will happily encode, so such a
+device parsed to ``None`` and became invisible to both the orphan sweep and
+the removal hook (issue #1399). A round-trip property finds it in well under a
+second; the example-based tests that shipped alongside the builder did not,
+because nobody thinks to write down slot ``-1`` by hand.
+"""
+
+from __future__ import annotations
+
+from hypothesis import given, strategies as st
+
+from custom_components.lock_code_manager.domain.config import (
+    build_slot_device_identifier,
+    build_slot_unique_id,
+    parse_slot_device_identifier,
+)
+
+# Home Assistant generates entry ids as fixed-length lowercase alphanumerics.
+# Fixed length matters to the codec: it is what makes one entry id incapable
+# of being a prefix of another.
+ENTRY_IDS = st.text(
+    alphabet="0123456789abcdefghijklmnopqrstuvwxyz", min_size=26, max_size=26
+)
+
+# Deliberately unbounded, including negative and zero. The slots YAML schema
+# puts no lower bound on the key, so the codec must survive whatever a user
+# can configure -- constraining this strategy to "sensible" slots would
+# reproduce exactly the blind spot that let the bug through.
+SLOT_NUMS = st.integers()
+
+KEYS = st.text(min_size=1, max_size=20).filter(lambda s: "|" not in s)
+
+
+@given(entry_id=ENTRY_IDS, slot=SLOT_NUMS)
+def test_device_identifier_round_trips(entry_id: str, slot: int) -> None:
+    """Anything the builder encodes, the parser recovers unchanged."""
+    identifier = build_slot_device_identifier(entry_id, slot)
+    assert parse_slot_device_identifier(entry_id, identifier) == slot
+
+
+@given(entry_id=ENTRY_IDS)
+def test_entry_device_is_not_a_slot_device(entry_id: str) -> None:
+    """
+    The entry's own device carries a bare entry id and must not parse.
+
+    The removal hook distinguishes the two by exactly this: a ``None`` here
+    means "the hub device", which must outlive every slot hanging off it.
+    """
+    assert parse_slot_device_identifier(entry_id, entry_id) is None
+
+
+@given(entry_id=ENTRY_IDS, other_id=ENTRY_IDS, slot=SLOT_NUMS)
+def test_other_entrys_device_never_parses(
+    entry_id: str, other_id: str, slot: int
+) -> None:
+    """A device belonging to a different entry is never claimed as ours."""
+    if entry_id == other_id:
+        return
+    foreign = build_slot_device_identifier(other_id, slot)
+    assert parse_slot_device_identifier(entry_id, foreign) is None
+
+
+@given(entry_id=ENTRY_IDS, identifier=st.text(max_size=60))
+def test_parse_is_total(entry_id: str, identifier: str) -> None:
+    """
+    The parser never raises, whatever it is handed.
+
+    It runs over every device already in the registry, including ones written
+    by older versions and by other integrations, so a crash here would take
+    out setup for the whole entry.
+    """
+    result = parse_slot_device_identifier(entry_id, identifier)
+    assert result is None or isinstance(result, int)
+
+
+@given(entry_id=ENTRY_IDS, slot=SLOT_NUMS)
+def test_parse_accepts_only_the_builders_own_spelling(entry_id: str, slot: int) -> None:
+    """
+    Int-parseable spellings the builder never emits are rejected.
+
+    ``int()`` happily accepts ``+1``, ``1_0``, ``01`` and surrounding
+    whitespace. Admitting those would map several distinct identifiers onto
+    one slot, so two registry devices could both claim to be slot 1.
+    """
+    canonical = build_slot_device_identifier(entry_id, slot)
+    for alias in (f"+{slot}", f"0{slot}", f" {slot}", f"{slot} "):
+        candidate = f"{entry_id}|{alias}"
+        if candidate == canonical:
+            continue
+        assert parse_slot_device_identifier(entry_id, candidate) is None
+
+
+@given(entry_id=ENTRY_IDS, slot=SLOT_NUMS, key=KEYS)
+def test_slot_unique_id_is_injective(entry_id: str, slot: int, key: str) -> None:
+    """
+    Distinct (slot, key) pairs never collide into one unique id.
+
+    A collision would make two entities fight over one registry row. The
+    per-lock variant must also stay distinct from the standard one, which is
+    what the trailing lock entity id buys.
+    """
+    standard = build_slot_unique_id(entry_id, slot, key)
+    assert standard == f"{entry_id}|{slot}|{key}"
+
+    per_lock = build_slot_unique_id(entry_id, slot, key, "lock.front_door")
+    assert per_lock == f"{standard}|lock.front_door"
+    assert per_lock != standard

tests/properties/test_pin_masking.py

@@ -0,0 +1,128 @@
+"""
+Property-based tests for the PIN masking and de-obfuscation round trip.
+
+Masking is what lets users paste debug logs into a public issue, and
+de-obfuscation is what lets the maintainer read them back. Both directions
+matter: a mask that leaks is a disclosure, and a round trip that loses the
+value makes the whole scheme pointless. Example-based tests pin a handful of
+PINs; these pin the invariants over the whole input space.
+"""
+
+from __future__ import annotations
+
+import re
+
+from hypothesis import given, strategies as st
+
+from custom_components.lock_code_manager.domain.util import (
+    deobfuscate_pins,
+    mask_pin,
+)
+
+# Real PINs are digit strings; the lock firmware cannot store anything else.
+PINS = st.text(alphabet="0123456789", min_size=1, max_size=12)
+SLOTS = st.integers(min_value=1, max_value=9999)
+INSTANCE_IDS = st.text(alphabet="0123456789abcdef", min_size=8, max_size=32)
+
+TOKEN_RE = re.compile(r"^pin#[0-9a-f]{8}$")
+
+
+@given(pin=PINS, slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_mask_emits_the_documented_token_shape(
+    pin: str, slot: int, instance_id: str
+) -> None:
+    """Every masked PIN is the fixed ``pin#`` + 8 lowercase hex form."""
+    assert TOKEN_RE.match(mask_pin(pin, slot, instance_id))
+
+
+@given(pin=PINS, slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_mask_does_not_leak_pin_length(pin: str, slot: int, instance_id: str) -> None:
+    """
+    The token is a constant width whatever the PIN's length.
+
+    A variable-width token would disclose the PIN's length, which for a
+    4-to-8-digit keypad code is a meaningful chunk of the search space.
+    """
+    assert len(mask_pin(pin, slot, instance_id)) == len("pin#") + 8
+
+
+@given(pin=PINS, slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_mask_is_deterministic(pin: str, slot: int, instance_id: str) -> None:
+    """
+    The same PIN on the same slot always masks identically.
+
+    This is what makes a log readable: one PIN reads as one token throughout,
+    so the reader can follow it across lines without ever seeing its value.
+    """
+    assert mask_pin(pin, slot, instance_id) == mask_pin(pin, slot, instance_id)
+
+
+@given(pin=PINS, slot=SLOTS, other_slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_same_pin_on_different_slots_masks_differently(
+    pin: str, slot: int, other_slot: int, instance_id: str
+) -> None:
+    """
+    The slot is part of the salt, so one PIN in two slots reads as two tokens.
+
+    Without this, a log would reveal that two slots share a PIN.
+    """
+    if slot == other_slot:
+        return
+    assert mask_pin(pin, slot, instance_id) != mask_pin(pin, other_slot, instance_id)
+
+
+@given(slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_empty_pin_is_not_masked_into_a_token(slot: int, instance_id: str) -> None:
+    """A missing PIN reads as ``<empty>``, never as a token implying a value."""
+    for empty in (None, ""):
+        assert mask_pin(empty, slot, instance_id) == "<empty>"
+
+
+@given(pin=PINS, slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_round_trip_recovers_the_pin(pin: str, slot: int, instance_id: str) -> None:
+    """A masked PIN embedded in log text is recovered exactly by the table."""
+    token = mask_pin(pin, slot, instance_id)
+    text = f"Setting usercode on lock.front slot {slot} (pin={token}, source=sync)"
+
+    deobfuscated, summary = deobfuscate_pins(text, {token: pin})
+
+    assert deobfuscated == text.replace(token, pin)
+    assert summary["total"] == 1
+    assert summary["matched"] == 1
+    assert summary["unmatched_tokens"] == []
+
+
+@given(pin=PINS, slot=SLOTS, instance_id=INSTANCE_IDS)
+def test_unknown_token_is_left_verbatim(pin: str, slot: int, instance_id: str) -> None:
+    """
+    A token with no table entry survives unchanged and is reported.
+
+    The output stays paste-compatible with the original log, which is what
+    makes it safe to run against a log whose PINs have since been rotated.
+    """
+    token = mask_pin(pin, slot, instance_id)
+    text = f"slot {slot} pin={token}"
+
+    deobfuscated, summary = deobfuscate_pins(text, {})
+
+    assert deobfuscated == text
+    assert summary["total"] == 1
+    assert summary["matched"] == 0
+    assert summary["unmatched_tokens"] == [token]
+
+
+@given(text=st.text(max_size=200))
+def test_deobfuscate_is_total_and_summary_is_coherent(text: str) -> None:
+    """
+    Arbitrary text never raises, and the summary always describes the output.
+
+    This runs over user-supplied log paste, so it has to survive anything.
+    """
+    deobfuscated, summary = deobfuscate_pins(text, {})
+
+    # With an empty table nothing can be substituted.
+    assert deobfuscated == text
+    assert summary["matched"] == 0
+    assert summary["matched"] <= summary["total"]
+    assert summary["unmatched_tokens"] == sorted(set(summary["unmatched_tokens"]))
+    assert len(summary["unmatched_tokens"]) <= summary["total"]

tests/test_config.py

@@ -15,6 +15,7 @@
     EntryConfig,
     EntryConfigDiff,
     build_slot_device_identifier,
+    build_slot_unique_id,
     parse_slot_device_identifier,
 )
 from custom_components.lock_code_manager.domain.queries import get_entry_config
@@ -564,3 +565,98 @@ def test_parse_slot_device_identifier_rejects_builder_aliases(suffix: str) -> No
     two distinct identifiers onto one slot.
     """
     assert parse_slot_device_identifier("abc123", f"abc123|{suffix}") is None
+
+
+# --- has_changes: one field at a time (mutation-testing gap) ---
+
+
+@pytest.mark.parametrize(
+    ("label", "old", "new"),
+    [
+        (
+            "slots_added",
+            {CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot()}},
+            {CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot(), 2: _slot()}},
+        ),
+        (
+            "slots_removed",
+            {CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot(), 2: _slot()}},
+            {CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot()}},
+        ),
+        (
+            "locks_added",
+            {CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot()}},
+            {CONF_LOCKS: ["lock.a", "lock.b"], CONF_SLOTS: {1: _slot()}},
+        ),
+        (
+            "locks_removed",
+            {CONF_LOCKS: ["lock.a", "lock.b"], CONF_SLOTS: {1: _slot()}},
+            {CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot()}},
+        ),
+    ],
+)
+def test_has_changes_is_true_for_each_field_alone(
+    label: str, old: dict, new: dict
+) -> None:
+    """
+    Any ONE of the four diff fields alone is enough to report a change.
+
+    The existing tests only ever move slots and locks together, or neither,
+    so each individual disjunct in ``has_changes`` went unpinned -- mutating
+    any single ``or`` to ``and`` survived. ``has_changes`` gates the Lovelace
+    dashboard re-render, so a collapsed disjunct means a user who only adds a
+    slot (or only removes a lock) silently gets a stale dashboard.
+    """
+    diff = EntryConfigDiff(old=_cfg(old), new=_cfg(new))
+
+    assert diff.has_changes is True, f"{label} alone should count as a change"
+    # Exactly the named field is populated; the other three stay empty.
+    populated = {
+        name
+        for name in ("slots_added", "slots_removed", "locks_added", "locks_removed")
+        if getattr(diff, name)
+    }
+    assert populated == {label}
+
+
+def test_has_changes_is_false_when_only_slot_contents_change() -> None:
+    """
+    Editing a slot's PIN is not a structural change.
+
+    ``has_changes`` asks specifically about added/removed slots and locks;
+    a PIN edit must not trigger a dashboard re-render.
+    """
+    old = _cfg({CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot(pin="1111")}})
+    new = _cfg({CONF_LOCKS: ["lock.a"], CONF_SLOTS: {1: _slot(pin="2222")}})
+
+    assert EntryConfigDiff(old=old, new=new).has_changes is False
+
+
+# --- build_slot_unique_id format (never referenced by any test) ---
+
+
+def test_build_slot_unique_id_standard_format() -> None:
+    """
+    The standard unique id is entry|slot|key, pipe-delimited.
+
+    This string is the entity registry key. Changing the separator silently
+    orphans every existing entity, so the exact format is the contract --
+    yet no test referenced this function at all.
+    """
+    assert build_slot_unique_id("abc123", 4, "pin") == "abc123|4|pin"
+
+
+def test_build_slot_unique_id_per_lock_format() -> None:
+    """The per-lock variant appends the lock entity id as a fourth segment."""
+    assert (
+        build_slot_unique_id("abc123", 4, "in_sync", "lock.front_door")
+        == "abc123|4|in_sync|lock.front_door"
+    )
+
+
+def test_build_slot_unique_id_variants_never_collide() -> None:
+    """A per-lock id is always distinct from the standard id it extends."""
+    standard = build_slot_unique_id("abc123", 4, "code")
+    per_lock = build_slot_unique_id("abc123", 4, "code", "lock.front_door")
+    assert standard != per_lock
+    assert per_lock.startswith(f"{standard}|")