fix(s54): reject non-array/non-object ledger JSON in SavingsLedger.load
S5.4 review MINOR (SC5 fail-fast hole, run.py:740). A valid-JSON but
wrong-shape savings ledger escaped the --report fail-fast refusal:
- top-level {} iterated zero keys -> entries=[] -> rc 0 "0,00 kr"
(a malformed file masquerading as a real zero-savings result)
- a bare scalar / object-with-keys / list-of-non-objects raised an
uncaught TypeError -> traceback (violates SC5 "rc 1, no traceback")
Fix at the fail-fast boundary, not the run.py except tuple: the review's
first option (add TypeError to run.py:740) leaves the {} masquerade
because {} is an empty iteration, not a TypeError. SavingsLedger.load now
raises ValueError for a non-array top-level and a non-object row, caught
by run.py:740's existing ValueError arm. Hardens both callers
(run.py:740 report + run.py:785 portfolio).
RED-first: 6 unit cases (test_ledger) + 2 CLI rc-1 cases (test_run_cli).
452 passed; ruff + mypy clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KNNiJRk1sSwxgVLS5AobT1
This commit is contained in:
parent
0343b42d88
commit
7dab2dfb78
3 changed files with 65 additions and 4 deletions
|
|
@ -152,14 +152,28 @@ class SavingsLedger(BaseModel):
|
|||
@classmethod
|
||||
def load(cls, path: str) -> SavingsLedger:
|
||||
"""Fail-fast load (mirrors ``okf.load_ir_projection``'s required-input semantics): a missing
|
||||
file raises ``FileNotFoundError``; a malformed row raises ``pydantic.ValidationError``.
|
||||
Contrast the tolerant verdict inbox (``load_verdicts_from_dir``), which SKIPS bad files —
|
||||
the ledger is authoritative input, not an out-of-band drop folder."""
|
||||
file raises ``FileNotFoundError``; a non-array top-level or non-object row raises
|
||||
``ValueError`` (a valid-JSON ``{}`` must NOT masquerade as an empty zero-savings ledger, and
|
||||
a bare scalar / non-object row must NOT leak an uncaught ``TypeError``); a malformed object
|
||||
row raises ``pydantic.ValidationError``. Contrast the tolerant verdict inbox
|
||||
(``load_verdicts_from_dir``), which SKIPS bad files — the ledger is authoritative input, not
|
||||
an out-of-band drop folder."""
|
||||
p = Path(path)
|
||||
if not p.is_file():
|
||||
raise FileNotFoundError(f"savings ledger not found: {path!r}")
|
||||
rows = json.loads(p.read_text(encoding="utf-8"))
|
||||
return cls(entries=[LedgerEntry(**row) for row in rows])
|
||||
if not isinstance(rows, list):
|
||||
raise ValueError(
|
||||
f"savings ledger must be a JSON array of entries, got {type(rows).__name__}: {path!r}"
|
||||
)
|
||||
entries = []
|
||||
for row in rows:
|
||||
if not isinstance(row, dict):
|
||||
raise ValueError(
|
||||
f"savings ledger entry must be a JSON object, got {type(row).__name__}: {path!r}"
|
||||
)
|
||||
entries.append(LedgerEntry(**row))
|
||||
return cls(entries=entries)
|
||||
|
||||
|
||||
def realize(
|
||||
|
|
|
|||
|
|
@ -80,6 +80,27 @@ def test_load_missing_file_raises(tmp_path) -> None:
|
|||
SavingsLedger.load(str(tmp_path / "does-not-exist.json"))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("payload", ["{}", "42", '"hello"', "null"])
|
||||
def test_load_rejects_non_list_toplevel(tmp_path, payload) -> None:
|
||||
"""A valid-JSON but non-array top-level payload is fail-fast rejected with ``ValueError`` — NOT
|
||||
silently loaded as an empty ledger (``{}`` iterates zero keys -> entries=[] -> a malformed file
|
||||
masquerading as a real zero-savings result) and NOT an uncaught ``TypeError``. Mirrors the
|
||||
documented fail-fast contract: the ledger is authoritative input, so a wrong-shape file refuses."""
|
||||
bad = tmp_path / "ledger.json"
|
||||
bad.write_text(payload, encoding="utf-8")
|
||||
with pytest.raises(ValueError):
|
||||
SavingsLedger.load(str(bad))
|
||||
|
||||
|
||||
def test_load_rejects_non_dict_row(tmp_path) -> None:
|
||||
"""A top-level array whose elements are not JSON objects is fail-fast rejected with ``ValueError``
|
||||
(``LedgerEntry(**1)`` would otherwise raise an uncaught ``TypeError``)."""
|
||||
bad = tmp_path / "ledger.json"
|
||||
bad.write_text("[1, 2]", encoding="utf-8")
|
||||
with pytest.raises(ValueError):
|
||||
SavingsLedger.load(str(bad))
|
||||
|
||||
|
||||
def test_save_is_byte_identical_regardless_of_order(tmp_path) -> None:
|
||||
"""SC8 determinism: the same entries inserted in different order serialize byte-identically
|
||||
(sorted keys + integer øre => the on-disk form is order-independent)."""
|
||||
|
|
|
|||
|
|
@ -356,6 +356,32 @@ def test_report_malformed_ledger_rc1(tmp_path, capsys) -> None:
|
|||
assert "refused" in capsys.readouterr().err.lower()
|
||||
|
||||
|
||||
def test_report_empty_dict_ledger_rc1(tmp_path, capsys) -> None:
|
||||
"""SC5 masquerade guard: a valid-JSON ``{}`` ledger must NOT load as an empty ledger and print a
|
||||
misleading ``0,00 kr`` at rc 0 — a malformed file masquerading as a real zero-savings result is
|
||||
the exact failure SC5's fail-fast refusal exists to prevent."""
|
||||
bad = tmp_path / "empty-obj.json"
|
||||
bad.write_text("{}", encoding="utf-8")
|
||||
rc = run.main(["--report", "--ledger", str(bad)])
|
||||
cap = capsys.readouterr()
|
||||
assert rc == 1
|
||||
assert "refused" in cap.err.lower()
|
||||
assert cap.out == "" # no table, no "0,00 kr"
|
||||
|
||||
|
||||
def test_report_nonlist_ledger_rc1_no_traceback(tmp_path, capsys) -> None:
|
||||
"""SC5: a valid-JSON but wrong-shape ledger (bare scalar / object-with-keys) -> rc 1 refusal, NOT
|
||||
an uncaught ``TypeError`` traceback ('rc 1, no traceback')."""
|
||||
bad = tmp_path / "scalar.json"
|
||||
bad.write_text("42", encoding="utf-8")
|
||||
rc = run.main(["--report", "--ledger", str(bad)])
|
||||
cap = capsys.readouterr()
|
||||
assert rc == 1
|
||||
assert "refused" in cap.err.lower()
|
||||
assert "Traceback" not in cap.err
|
||||
assert cap.out == ""
|
||||
|
||||
|
||||
def test_report_without_ledger_rc1_no_traceback(capsys) -> None:
|
||||
"""Major-#1 guard: ``--report`` with no ``--ledger`` -> rc 1 with a 'requires --ledger' message
|
||||
and no traceback (guards ``SavingsLedger.load(None)`` -> ``Path(None)`` TypeError)."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue