From cb3c347d6130136c6ee228ea29b80f676e54bafa Mon Sep 17 00:00:00 2001 From: Joshua King Date: Tue, 15 Sep 2026 17:46:30 -0400 Subject: [PATCH] Review fix: reject datetime anchors, freeze the pattern mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 (Codex gpt-5.6-terra, 2 MEDIUM): datetime.datetime subclasses datetime.date and slipped past the anchor check, storing a time component that would TypeError every later calendar comparison — rejected explicitly now (both model classes). The frozen schedule's pattern dict let callers inject unvalidated weekdays or drop required weeks after construction; the validated mapping is now exposed as a read-only MappingProxyType over frozenset weeks. Tests: datetime rejection matrix and immutability assertions. Suite: 974 passed. --- custom_components/nestquest/presence.py | 23 ++++++++++++++--- tests/test_presence.py | 33 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/custom_components/nestquest/presence.py b/custom_components/nestquest/presence.py index 88e6f10..ffc6841 100644 --- a/custom_components/nestquest/presence.py +++ b/custom_components/nestquest/presence.py @@ -43,6 +43,7 @@ from __future__ import annotations import datetime from dataclasses import dataclass +from types import MappingProxyType #: The schema's documented cap for cycle_length_weeks (schema.py keeps #: the pattern human-readable and the anchor arithmetic small). @@ -52,7 +53,18 @@ _DATE_FORMAT = "%Y-%m-%d" def _parse_date(value: object, field_name: str) -> datetime.date: - """Accept a datetime.date or a strict YYYY-MM-DD string.""" + """Accept a datetime.date or a strict YYYY-MM-DD string. + + ``datetime.datetime`` is rejected explicitly: it SUBCLASSES + datetime.date, so a bare isinstance check would store a value with + a time component and every later calendar-date comparison (e.g. + ``PresenceOverride.covers``) would raise TypeError. + """ + if isinstance(value, datetime.datetime): + raise ValueError( + f"{field_name} must be a plain calendar date, not a " + f"datetime with a time component: {value!r}" + ) if isinstance(value, datetime.date): return value if not isinstance(value, str): @@ -149,7 +161,7 @@ class PresenceSchedule: child_id: int cycle_length_weeks: int anchor_date: datetime.date - pattern: dict[int, frozenset[int]] + pattern: MappingProxyType # week index -> frozenset of weekdays def __init__( self, @@ -181,7 +193,12 @@ class PresenceSchedule: f"pattern must cover week indices 0..{cycle - 1}; " f"missing {missing}" ) - object.__setattr__(self, "pattern", normalised) + # Frozen must mean frozen: a bare dict would let callers inject + # unvalidated weekdays (or drop required weeks) AFTER + # construction, bypassing total validation. The mapping + # proxy exposes the validated contents read-only; the + # per-week sets are already frozensets. + object.__setattr__(self, "pattern", MappingProxyType(normalised)) def encode(self) -> str: """Serialize to the database's pipe-separated CSV shape. diff --git a/tests/test_presence.py b/tests/test_presence.py index 1c8e44d..202980a 100644 --- a/tests/test_presence.py +++ b/tests/test_presence.py @@ -227,3 +227,36 @@ def test_override_rejects_non_int_child_id(bad) -> None: def test_override_rejects_non_strict_dates(start, end) -> None: with pytest.raises(ValueError, match="date"): PresenceOverride(1, start, end, True) + + +def test_schedule_rejects_datetime_anchor() -> None: + """datetime.datetime subclasses date: a bare isinstance check would + store a time component and every date comparison would TypeError.""" + with pytest.raises(ValueError, match="plain calendar date"): + PresenceSchedule(1, 1, datetime.datetime(2026, 1, 5, 9, 30), {0: {0}}) + + +def test_override_rejects_datetime_bounds() -> None: + with pytest.raises(ValueError, match="plain calendar date"): + PresenceOverride( + 1, datetime.datetime(2026, 7, 20), "2026-07-20", True + ) + + +def test_schedule_pattern_is_immutable_after_construction() -> None: + """The frozen schedule must not leak a mutable dict: injecting + unvalidated weekdays or dropping required weeks after construction + would bypass total validation.""" + schedule = PresenceSchedule(1, 2, ANCHOR, {0: {0, 2}, 1: {1}}) + with pytest.raises(TypeError): + schedule.pattern[0] = {9} + with pytest.raises(TypeError): + del schedule.pattern[1] + # mappingproxy exposes no mutators at all. + assert not any( + hasattr(schedule.pattern, name) + for name in ("clear", "pop", "popitem", "setdefault", "update") + ) + # The validated contents are still readable and encode correctly. + assert schedule.pattern[0] == frozenset({0, 2}) + assert schedule.encode() == "0,2|1"