From e3871ca2ea896cc7743173e828e82f2b4f239a89 Mon Sep 17 00:00:00 2001 From: Ben Westgate Date: Thu, 22 Oct 2026 04:11:17 +0000 Subject: [PATCH] bip93: Store artifacts as validated text An artifact held its text and, separately, the header, HRP and payload decoded from it, so a direct constructor call could pair one seed's card with another seed's payload. A factory token guarded that. Store only the validated text and decode the rest from it: constructing a class now parses its string and refuses one of the wrong kind, and the token goes. Claude-Session: https://claude.ai/code/session_01XGMpowzJwwgyHMEd63ijDe --- docs/developer/api.md | 2 +- docs/security/model.md | 4 ++-- src/codex32/bip93.py | 50 +++++++++++++++++++++++----------------- tests/test_bip93.py | 4 +++- tests/test_public_api.py | 16 +++++++++---- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/docs/developer/api.md b/docs/developer/api.md index f4e745ba..c13420ca 100644 --- a/docs/developer/api.md +++ b/docs/developer/api.md @@ -51,7 +51,7 @@ generic parse-length failure. ### Boundaries -- Only parsing and `MasterSeed.from_seed` construct artifacts. +- An artifact stores only its validated string; its header, HRP and payload are decoded from it. - `bech32.py` keeps the recognizable BIP173 reference names and accumulator structure. Encoding takes an explicit checksum specification. Decoding without one returns the HRP and complete data part after checking only the diff --git a/docs/security/model.md b/docs/security/model.md index 6dcd2bc3..585dba26 100644 --- a/docs/security/model.md +++ b/docs/security/model.md @@ -88,8 +88,8 @@ then verifies the codex32 checksum. Only after checksum verification does an `ms` human-readable part (HRP) add the master-seed length rule and seed-byte meaning. -Only the parser and `MasterSeed.from_seed` construct immutable validated -artifacts. A share has symbol semantics and cannot be converted to bytes. +Artifacts are immutable and store only their validated string; the header, +HRP and payload are decoded from it, so they cannot disagree. A share has symbol semantics and cannot be converted to bytes. Default `str()` and `repr()` rendering of artifacts is redacted; callers must use `.text` explicitly when they intentionally need the serialized recovery string. This reduces accidental disclosure through logs and interpolation but diff --git a/src/codex32/bip93.py b/src/codex32/bip93.py index 7021f1de..321f5e55 100644 --- a/src/codex32/bip93.py +++ b/src/codex32/bip93.py @@ -36,7 +36,6 @@ from codex32.gf32 import _multiply as _gf32_multiply IDX_SORT = "sacdefghjklmnpqrtuvwxyz023456789" -_CONSTRUCTION_TOKEN = object() # BIP93 master seeds: the "ms" HRP with one of six seed sizes (bytes, payload, text). MS = "ms" @@ -132,24 +131,27 @@ def _decode_codex32(text: str) -> tuple[str, tuple[int, ...], _Checksum]: return hrp, tuple(encoded), checksum -@dataclass(frozen=True, slots=True, init=False) +@dataclass(frozen=True, slots=True) class _Artifact: - """Create artifacts with parse_codex32 or the factories, which validate them.""" + """A validated codex32 string; its other attributes are decoded from it, so they cannot disagree.""" text: str - header: Header - hrp: str - payload_symbols: tuple[int, ...] - - def __init__( - self, text: str, header: Header, hrp: str, payload_symbols: tuple[int, ...], *, _token: object - ) -> None: - if _token is not _CONSTRUCTION_TOKEN: - raise TypeError("codex32 artifacts must be created by the public factories") - object.__setattr__(self, "text", text) - object.__setattr__(self, "header", header) - object.__setattr__(self, "hrp", hrp) - object.__setattr__(self, "payload_symbols", payload_symbols) + + def __post_init__(self) -> None: + if _artifact_type(self.text) is not type(self): + raise TypeError(f"this codex32 string is not a {type(self).__name__}") + + @property + def hrp(self) -> str: + return _decode_codex32(self.text)[0] + + @property + def header(self) -> Header: + return Header._from_symbols(_body(self.text)[:6]) + + @property + def payload_symbols(self) -> tuple[int, ...]: + return _body(self.text)[6:] def __str__(self) -> str: return f"<{type(self).__name__}: redacted>" @@ -210,13 +212,19 @@ def _has_generation_padding(secret: MasterSeed) -> bool: return padding == _crc_pad(secret.seed_bytes) +def _body(text: str) -> tuple[int, ...]: + _hrp, encoded, checksum = _decode_codex32(text) + return encoded[: -checksum.length] + + +def _artifact_type(text: str) -> type[Share | Secret]: + hrp, encoded, _checksum = _decode_codex32(text) + return Share if encoded[5] != CHARSET.index("s") else MasterSeed if hrp == MS else Secret + + def parse_codex32(text: str) -> Share | Secret: """Validate one codex32 string and return an immutable artifact.""" - hrp, encoded, checksum = _decode_codex32(text) - body = tuple(encoded[: -checksum.length]) - header = Header._from_symbols(body[:6]) - artifact_type = Share if header.index != "s" else MasterSeed if hrp == MS else Secret - return artifact_type(text, header, hrp, body[6:], _token=_CONSTRUCTION_TOKEN) + return _artifact_type(text)(text) def _from_parts( diff --git a/tests/test_bip93.py b/tests/test_bip93.py index 6d172082..26e60c90 100644 --- a/tests/test_bip93.py +++ b/tests/test_bip93.py @@ -100,8 +100,10 @@ def payload(secret: Secret) -> str: assert payload(luea) == "6c696768746e696e672d31330000000000000000000000000000000000000000" assert payload(peev) == "5eb00bbddcf069084889a8ab9155568165f5c453ccb85e70811aaed6f6da5fc1" - with pytest.raises((FrozenInstanceError, AttributeError)): + # Before Python 3.13, slotted frozen dataclasses raise TypeError here for subclasses. + with pytest.raises((FrozenInstanceError, AttributeError, TypeError)): luea.header = cln2.header # type: ignore[misc] + assert luea.header.identifier == "luea" @pytest.mark.parametrize("value", BAD_CHECKSUMS) diff --git a/tests/test_public_api.py b/tests/test_public_api.py index aab2f064..71104e2b 100644 --- a/tests/test_public_api.py +++ b/tests/test_public_api.py @@ -8,7 +8,7 @@ from pathlib import Path import pytest -from data.bip93_vectors import VECTOR_2 +from data.bip93_vectors import VECTOR_1, VECTOR_2 import codex32 from codex32 import ( @@ -21,7 +21,7 @@ correct_worksheet_residue, parse_codex32, ) -from codex32.errors import InvalidIdentifier, InvalidShareIndex, InvalidThreshold +from codex32.errors import CodexError, InvalidIdentifier, InvalidShareIndex, InvalidThreshold def test_backup_workflows_need_only_the_standard_library() -> None: @@ -83,10 +83,16 @@ def test_share_has_symbols_but_no_byte_or_padding_api() -> None: assert not hasattr(share, name) -def test_artifacts_cannot_be_directly_constructed_or_mutated() -> None: - with pytest.raises(TypeError): - Share("ms", Header(2, "test", "a"), (), ()) # type: ignore[call-arg] +def test_artifacts_are_validated_strings_and_cannot_be_mutated() -> None: share = parse_codex32(VECTOR_2["share_A"]) + secret = parse_codex32(VECTOR_1["secret_s"]) + assert Share(share.text) == share and MasterSeed(secret.text) == secret + with pytest.raises(TypeError): + MasterSeed(share.text) + with pytest.raises(TypeError): + Share(secret.text) + with pytest.raises(CodexError): + MasterSeed(secret.text[:-1] + ("q" if secret.text[-1] != "q" else "p")) with pytest.raises((FrozenInstanceError, AttributeError)): share.text = "changed" # type: ignore[misc] with pytest.raises((FrozenInstanceError, AttributeError)):