Skip to content

Commit 315a389

Browse files
Pigbibicodex
andcommitted
fix: tighten weekly source contract invariants
Co-Authored-By: Codex <noreply@openai.com>
1 parent f0be26b commit 315a389

2 files changed

Lines changed: 42 additions & 7 deletions

File tree

src/political_event_tracking_research/weekly_contract.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import hashlib
55
import json
66
import re
7+
import unicodedata
78
from collections.abc import Mapping
89
from dataclasses import dataclass
910
from datetime import date, datetime, timedelta, timezone
@@ -12,6 +13,7 @@
1213
SCHEMA_VERSION = "1"
1314
CONTRACT_VERSION = "political_event_weekly.v1"
1415
CADENCE = "weekly"
16+
MAX_SAFE_JSON_INTEGER = 2**53 - 1
1517
_DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$")
1618
_TIMESTAMP_RE = re.compile(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d{1,6})?Z$")
1719
_SHA256_RE = re.compile(r"^[0-9a-f]{64}$")
@@ -44,6 +46,7 @@ class WeeklyFeedStatus:
4446
failed_feed_count: int
4547
stale_feed_count: int
4648
missing_feed_count: int
49+
complete: bool
4750

4851

4952
@dataclass(frozen=True, slots=True)
@@ -88,23 +91,28 @@ def _artifact(value: object) -> WeeklySourceArtifact:
8891
if not isinstance(value, Mapping) or set(value) != _ARTIFACT_KEYS:
8992
raise _invalid("source_artifact_invalid")
9093
path = value["path"]
91-
if type(path) is not str or not path or PurePosixPath(path).is_absolute() or ".." in PurePosixPath(path).parts or "\\" in path:
94+
canonical = PurePosixPath(path) if type(path) is str else None
95+
if (
96+
type(path) is not str or not path or not path.isascii() or unicodedata.normalize("NFC", path) != path
97+
or canonical is None or canonical.is_absolute() or path != str(canonical) or path.endswith("/")
98+
or "//" in path or any(part in {"", ".", ".."} for part in canonical.parts) or "\\" in path
99+
):
92100
raise _invalid("source_artifact_invalid")
93101
digest = value["sha256"]
94102
row_count = value["row_count"]
95-
if type(digest) is not str or not _SHA256_RE.fullmatch(digest) or type(row_count) is not int or row_count < 0:
103+
if type(digest) is not str or not _SHA256_RE.fullmatch(digest) or type(row_count) is not int or not 0 <= row_count <= MAX_SAFE_JSON_INTEGER:
96104
raise _invalid("source_artifact_invalid")
97105
return WeeklySourceArtifact(path, digest, row_count)
98106

99107

100108
def _feed_status(value: object) -> WeeklyFeedStatus:
101-
if not isinstance(value, Mapping) or set(value) != _FEED_KEYS or value.get("complete") is not True:
109+
if not isinstance(value, Mapping) or set(value) != _FEED_KEYS or type(value.get("complete")) is not bool:
102110
raise _invalid("feed_status_invalid")
103111
values = [value[key] for key in _FEED_KEYS if key != "complete"]
104-
if any(type(item) is not int or item < 0 for item in values):
112+
if any(type(item) is not int or not 0 <= item <= MAX_SAFE_JSON_INTEGER for item in values):
105113
raise _invalid("feed_status_invalid")
106-
status = WeeklyFeedStatus(*(value[key] for key in ("feed_count", "successful_feed_count", "failed_feed_count", "stale_feed_count", "missing_feed_count")))
107-
if status.feed_count <= 0 or status.successful_feed_count != status.feed_count or any((status.failed_feed_count, status.stale_feed_count, status.missing_feed_count)):
114+
status = WeeklyFeedStatus(*(value[key] for key in ("feed_count", "successful_feed_count", "failed_feed_count", "stale_feed_count", "missing_feed_count")), value["complete"])
115+
if status.feed_count <= 0 or status.successful_feed_count != status.feed_count or any((status.failed_feed_count, status.stale_feed_count, status.missing_feed_count)) or not status.complete:
108116
raise _invalid("feed_status_incomplete")
109117
return status
110118

@@ -148,7 +156,7 @@ def serialize_weekly_contract(contract: WeeklySourceContract) -> bytes:
148156
"generated_at": contract.generated_at.isoformat(timespec="microseconds").replace("+00:00", "Z"),
149157
"run_mode": contract.run_mode, "producer_ref": contract.producer_ref, "source_provenance": contract.source_provenance,
150158
"source_artifacts": [{"path": item.path, "sha256": item.sha256, "row_count": item.row_count} for item in contract.source_artifacts],
151-
"feed_status": {"feed_count": contract.feed_status.feed_count, "successful_feed_count": contract.feed_status.successful_feed_count, "failed_feed_count": 0, "stale_feed_count": 0, "missing_feed_count": 0, "complete": True},
159+
"feed_status": {"feed_count": contract.feed_status.feed_count, "successful_feed_count": contract.feed_status.successful_feed_count, "failed_feed_count": contract.feed_status.failed_feed_count, "stale_feed_count": contract.feed_status.stale_feed_count, "missing_feed_count": contract.feed_status.missing_feed_count, "complete": contract.feed_status.complete},
152160
}
153161
except (AttributeError, TypeError, ValueError, OverflowError):
154162
raise _invalid("contract_invalid") from None

tests/test_weekly_contract.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ def test_scheduled_and_manual_are_explicit():
6969
parse_weekly_contract({key: value for key, value in payload().items() if key != "as_of"})
7070

7171

72+
def test_real_feed_counters_round_trip_without_serializer_defaults():
73+
status = {"feed_count": 12, "successful_feed_count": 12, "failed_feed_count": 0, "stale_feed_count": 0, "missing_feed_count": 0, "complete": True}
74+
contract = parse_weekly_contract(payload(feed_status=status))
75+
encoded = serialize_weekly_contract(contract)
76+
assert b'"feed_count":12' in encoded
77+
assert parse_weekly_contract(__import__("json").loads(encoded)).feed_status == contract.feed_status
78+
79+
7280
@pytest.mark.parametrize("status", [
7381
{"feed_count": 9, "successful_feed_count": 8, "failed_feed_count": 1, "stale_feed_count": 0, "missing_feed_count": 0, "complete": True},
7482
{"feed_count": 9, "successful_feed_count": 9, "failed_feed_count": 0, "stale_feed_count": 1, "missing_feed_count": 0, "complete": True},
@@ -90,6 +98,25 @@ def test_artifacts_are_sorted_and_duplicate_or_unsafe_inputs_fail_closed():
9098
parse_weekly_contract(payload(source_artifacts=[items[0], items[0]]))
9199
with pytest.raises(WeeklyContractError):
92100
parse_weekly_contract(payload(source_artifacts=[{"path": "../secret", "sha256": "c" * 64, "row_count": 1}]))
101+
for alias in ("a//b.csv", "a/b.csv/", "./a.csv", "a/./b.csv", "a\\b.csv", "é.csv"):
102+
with pytest.raises(WeeklyContractError):
103+
parse_weekly_contract(payload(source_artifacts=[{"path": alias, "sha256": "c" * 64, "row_count": 1}]))
104+
105+
106+
@pytest.mark.parametrize("field", ["row_count", "feed_count", "successful_feed_count"])
107+
def test_wire_integers_use_safe_json_range_and_reject_bool(field):
108+
artifact = {"path": "data/live/political_events.csv", "sha256": "b" * 64, "row_count": 11}
109+
status = payload()["feed_status"]
110+
if field == "row_count":
111+
with pytest.raises(WeeklyContractError):
112+
parse_weekly_contract(payload(source_artifacts=[{**artifact, "row_count": 2**53}]))
113+
with pytest.raises(WeeklyContractError):
114+
parse_weekly_contract(payload(source_artifacts=[{**artifact, "row_count": True}]))
115+
else:
116+
with pytest.raises(WeeklyContractError):
117+
parse_weekly_contract(payload(feed_status={**status, field: 2**53}))
118+
with pytest.raises(WeeklyContractError):
119+
parse_weekly_contract(payload(feed_status={**status, field: True}))
93120

94121

95122
def test_unknown_fields_and_wire_type_confusion_fail_closed():

0 commit comments

Comments
 (0)