From bff2f4ff5e8c0702a489b2cd9280c63876836ce7 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 18:13:48 -0400 Subject: [PATCH] Review fix: freeze the engine's overrides snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 (Codex gpt-5.6-terra, MEDIUM): the overrides mapping was shallow-copied, leaving caller-owned lists mutable through _overrides — a later append would change is_present results. Each validated list is copied into an immutable tuple before wrapping; tests prove post-construction appends change nothing and the snapshot refuses mutation. Suite: 996 passed. --- custom_components/nestquest/presence.py | 11 +++++++++-- tests/test_presence.py | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/custom_components/nestquest/presence.py b/custom_components/nestquest/presence.py index 9dabf0d..ff233b4 100644 --- a/custom_components/nestquest/presence.py +++ b/custom_components/nestquest/presence.py @@ -384,10 +384,17 @@ class PresenceEngine: ) # Read-only snapshot: reassignment is refused and no mutator # exists, so a built engine answers consistently for its - # lifetime. + # lifetime. The overrides lists are copied to immutable + # tuples — a shallow mapping copy alone would leave caller- + # owned lists reachable through _overrides, and mutating one + # later would change is_present results. object.__setattr__(self, "_schedules", MappingProxyType(dict(schedules))) object.__setattr__( - self, "_overrides", MappingProxyType(dict(overrides)) + self, + "_overrides", + MappingProxyType( + {child_id: tuple(entries) for child_id, entries in overrides.items()} + ), ) def is_present(self, child_id: object, date: object) -> bool: diff --git a/tests/test_presence.py b/tests/test_presence.py index 284eadf..c47ada6 100644 --- a/tests/test_presence.py +++ b/tests/test_presence.py @@ -574,3 +574,19 @@ def test_engine_overlapping_snapshot_uses_latest_start() -> None: assert engine.is_present(1, datetime.date(2026, 1, 9)) is True # Jan 11-12: only the later override covers -> present. assert engine.is_present(1, datetime.date(2026, 1, 11)) is True + + +def test_engine_override_lists_are_frozen() -> None: + """The overrides snapshot must be truly read-only: mutating the + caller's list after construction must not change answers.""" + entry = PresenceOverride(1, "2026-01-06", "2026-01-06", False) + entries = [entry] + engine = _engine_with_overrides(*entries) + assert engine.is_present(1, datetime.date(2026, 1, 6)) is False + entries.append(PresenceOverride(1, "2026-01-07", "2026-01-07", False)) + # The late append must NOT change the engine's answers... + assert engine.is_present(1, datetime.date(2026, 1, 7)) is True + with pytest.raises(TypeError): + engine._overrides[1].append(entry) + with pytest.raises(AttributeError, match="immutable snapshot"): + engine._overrides = {}