Skip to content

Commit 93f1a23

Browse files
committed
fix(evidence): close E2 review findings (R19/R20 + golden pins)
1 parent 20c9f40 commit 93f1a23

4 files changed

Lines changed: 151 additions & 14 deletions

File tree

‎src/pkcs11_check/core/evidence/admit.py‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@
2929
from pkcs11_check.core.evidence.obligations import ObligationInventory
3030
from pkcs11_check.core.evidence.schema import LedgerHeader, LedgerRow, Outcome
3131

32-
#: sha256 of ``b""`` -- the pinned digest of an empty (zero-record) shard.
32+
#: sha256 of ``b""`` -- the pinned digest of an empty input to
33+
#: ``shard_digest``. Direct ``shard_digest([])`` use only: zero-record roster
34+
#: shards are rejected at construction, so this digest is unreachable via
35+
#: ``admit_lane`` (an absent shard always fails ``roster_complete``).
3336
EMPTY_SHARD_SHA256 = hashlib.sha256(b"").hexdigest()
3437

3538

@@ -40,6 +43,15 @@ class ShardExpectation:
4043
records: int
4144
sha256: str
4245

46+
def __post_init__(self) -> None:
47+
"""Reject zero-record shards: every roster shard must carry rows."""
48+
if self.records < 1:
49+
raise ValueError(
50+
"ShardExpectation records must be >= 1 "
51+
f"(got {self.records}); absent shards fail roster_complete, "
52+
"they are never predeclared empty"
53+
)
54+
4355

4456
@dataclass(frozen=True)
4557
class PredeclaredRoster:
@@ -139,17 +151,9 @@ def _check_roster_complete(mappings: list[dict[str, Any]], roster: PredeclaredRo
139151
problems: list[str] = []
140152
for shard_id, expected in roster.shards.items():
141153
if shard_id not in groups:
142-
if expected.records == 0:
143-
if EMPTY_SHARD_SHA256 != expected.sha256:
144-
problems.append(
145-
f"roster_complete: empty shard {shard_id!r} digest mismatch: "
146-
f"rows hash {EMPTY_SHARD_SHA256}, roster expects {expected.sha256}"
147-
)
148-
else:
149-
problems.append(
150-
f"roster_complete: roster shard {shard_id!r} missing from rows "
151-
"-- no partial credit"
152-
)
154+
problems.append(
155+
f"roster_complete: roster shard {shard_id!r} missing from rows -- no partial credit"
156+
)
153157
continue
154158
present = groups[shard_id]
155159
if len(present) != expected.records:

‎src/pkcs11_check/core/evidence/schema.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,19 @@ def to_record(self) -> dict[str, Any]:
117117

118118
@classmethod
119119
def from_record(cls, record: Mapping[str, Any], *, line_number: int) -> LedgerHeader:
120-
"""Build a header from a parsed ledger line, failing loud on bad types."""
120+
"""Build a header from a parsed ledger line.
121+
122+
Fails loud on bad types and on missing known counter keys (absent
123+
counters are malformed evidence, never zero-filled); unknown extra
124+
keys stay ignored for forward compatibility.
125+
"""
121126
label = f"ledger line {line_number}"
122127
counters = record.get("counters")
123128
if not isinstance(counters, Mapping):
124129
raise ValueError(f"{label}: header counters is not an object")
130+
missing = [key for key in COUNTER_KEYS if key not in counters]
131+
if missing:
132+
raise ValueError(f"{label}: header counters missing keys {missing!r}")
125133
merged = zero_counters()
126134
for key, value in counters.items():
127135
if not isinstance(key, str) or type(value) is not int:

‎tests/test_evidence_admit.py‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
from __future__ import annotations
44

55
import hashlib
6+
import json
67
from dataclasses import replace
8+
from pathlib import Path
79

810
import pytest
911

1012
from pkcs11_check.core.evidence.admit import (
13+
EMPTY_SHARD_SHA256,
1114
PredeclaredRoster,
1215
ShardExpectation,
1316
admit_lane,
@@ -232,3 +235,116 @@ def test_obligation_membership() -> None:
232235
compile_obligations({"a": {}}, {"cases": ["a", "zzz"]})
233236
with pytest.raises(ValueError):
234237
compile_obligations({"a": {}}, {})
238+
239+
240+
def test_roster_rejects_zero_records_at_construction() -> None:
241+
"""R19: a roster shard declaring records=0 is rejected when built."""
242+
with pytest.raises(ValueError):
243+
ShardExpectation(records=0, sha256=EMPTY_SHARD_SHA256)
244+
with pytest.raises(ValueError):
245+
ShardExpectation(records=-1, sha256=EMPTY_SHARD_SHA256)
246+
# Control: records >= 1 still builds.
247+
ShardExpectation(records=1, sha256=EMPTY_SHARD_SHA256)
248+
249+
250+
def test_roster_complete_absent_shard_always_fails() -> None:
251+
"""R19: every roster shard must be present in rows -- no unless-clause."""
252+
obligations = ObligationInventory(members=frozenset())
253+
roster = PredeclaredRoster(
254+
lane="soft",
255+
shards={"never_arrived": ShardExpectation(records=1, sha256="0" * 64)},
256+
)
257+
# Reviewer probe shape: empty rows + absent shard must fail, not admit.
258+
verdict = admit_lane(_header([]), [], roster, obligations)
259+
assert not verdict.admitted
260+
assert _failed_with(verdict.failures, "roster_complete")
261+
262+
# Absent shard alongside otherwise-clean rows must also fail.
263+
header, rows, clean_roster, _ = _clean_lane()
264+
shards = dict(clean_roster.shards)
265+
shards["ghost"] = ShardExpectation(records=2, sha256="1" * 64)
266+
verdict = admit_lane(header, rows, replace(clean_roster, shards=shards), obligations)
267+
assert not verdict.admitted
268+
assert _failed_with(verdict.failures, "roster_complete")
269+
270+
271+
def test_loader_missing_incomplete_key_fails_closed(tmp_path: Path) -> None:
272+
"""R20: end-to-end -- header counters without `incomplete` fail at load."""
273+
from pkcs11_check.core.evidence import load_ledger
274+
275+
header, rows, _, _ = _clean_lane()
276+
ledger = tmp_path / "ledger.jsonl"
277+
header_record = header.to_record()
278+
del header_record["counters"]["incomplete"]
279+
ledger.write_text(
280+
"\n".join(
281+
[
282+
json.dumps(header_record, sort_keys=True),
283+
*(json.dumps(row.to_record(), sort_keys=True) for row in rows),
284+
]
285+
)
286+
+ "\n",
287+
encoding="utf-8",
288+
)
289+
with pytest.raises(ValueError, match="incomplete"):
290+
load_ledger(ledger)
291+
292+
293+
def test_loader_full_header_loads_and_admits(tmp_path: Path) -> None:
294+
"""R20 control: a complete header loads and the lane admits."""
295+
from pkcs11_check.core.evidence import load_ledger
296+
297+
header, rows, _, obligations = _clean_lane()
298+
ledger = tmp_path / "ledger.jsonl"
299+
ledger.write_text(
300+
"\n".join(
301+
[
302+
json.dumps(header.to_record(), sort_keys=True),
303+
*(json.dumps(row.to_record(), sort_keys=True) for row in rows),
304+
]
305+
)
306+
+ "\n",
307+
encoding="utf-8",
308+
)
309+
loaded_header, loaded_rows = load_ledger(ledger)
310+
verdict = admit_lane(loaded_header, loaded_rows, _roster(loaded_rows), obligations)
311+
assert verdict.admitted
312+
assert verdict.failures == []
313+
314+
315+
def test_shard_digest_golden_fixture() -> None:
316+
"""Independent golden pin of the P11Lab-reused canonicalization.
317+
318+
Canonical bytes are handwritten below; the digest literal was computed
319+
with shell ``sha256sum`` over those exact bytes (no trailing newline),
320+
independently of ``shard_digest``.
321+
"""
322+
blob_low = b'{"attempt":1,"outcome":"passed"}'
323+
blob_high = b'{"attempt":2,"outcome":"failed"}'
324+
row_low = {"outcome": "passed", "attempt": 1}
325+
row_high = {"outcome": "failed", "attempt": 2}
326+
assert row_blob(row_low) == blob_low
327+
assert row_blob(row_high) == blob_high
328+
# Rows passed in reverse byte order: sorting + b"\\n" join are pinned too.
329+
assert (
330+
shard_digest([row_high, row_low])
331+
== "7fe9e7768bcd63c10328e3d9daf94a351bd4de8cc3566e65609a3e3f40f5edf5"
332+
)
333+
334+
335+
def test_occurrence_preserved_duplicate_attempts() -> None:
336+
"""Attempts [1, 1, 2] with attempts_total=3 fail occurrence_preserved."""
337+
rows = [_row("tests/a.py::t", attempt=attempt, attempts_total=3) for attempt in (1, 1, 2)]
338+
obligations = ObligationInventory(members=frozenset())
339+
verdict = admit_lane(_header(rows), rows, _roster(rows), obligations)
340+
assert not verdict.admitted
341+
assert _failed_with(verdict.failures, "occurrence_preserved")
342+
343+
344+
def test_occurrence_preserved_off_by_one_attempts() -> None:
345+
"""Attempts [2, 3, 4] with attempts_total=3 fail occurrence_preserved."""
346+
rows = [_row("tests/a.py::t", attempt=attempt, attempts_total=3) for attempt in (2, 3, 4)]
347+
obligations = ObligationInventory(members=frozenset())
348+
verdict = admit_lane(_header(rows), rows, _roster(rows), obligations)
349+
assert not verdict.admitted
350+
assert _failed_with(verdict.failures, "occurrence_preserved")

‎tests/test_evidence_export.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from pkcs11_check.core.crash_codes import CTYPES_ACCESS_VIOLATION
1616
from pkcs11_check.core.evidence import export_ledger
17+
from pkcs11_check.core.evidence.schema import zero_counters
1718
from pkcs11_check.core.report_log import iter_report_log_records
1819

1920
TRACEMALLOC_LIMIT_BYTES = 64 * 1024 * 1024
@@ -438,13 +439,20 @@ def _header_line(schema_v: Any = 1) -> str:
438439
"raw_stream_bytes": 0,
439440
"raw_stream_sha256": "",
440441
"malformed_lines": 0,
441-
"counters": {},
442+
"counters": zero_counters(),
442443
"created_by": "pkcs11-check",
443444
},
444445
sort_keys=True,
445446
)
446447

447448

449+
def _header_line_missing_counter() -> str:
450+
"""R20 pin: a header with one counter key removed is malformed evidence."""
451+
header = json.loads(_header_line())
452+
del header["counters"]["incomplete"]
453+
return json.dumps(header, sort_keys=True)
454+
455+
448456
def _row_line(node_id: str, phase: str = "call", attempt: int = 1, **overrides: Any) -> str:
449457
row: dict[str, Any] = {
450458
"record": "case",
@@ -491,6 +499,7 @@ def _row_line(node_id: str, phase: str = "call", attempt: int = 1, **overrides:
491499
"duplicate",
492500
),
493501
([_header_line(), _row_line("tests/a.py::t", outcome="bogus")], "outcome"),
502+
([_header_line_missing_counter()], "missing keys"),
494503
],
495504
)
496505
def test_load_rejects(tmp_path: Path, lines: list[str], match: str) -> None:

0 commit comments

Comments
 (0)