From cdd74f379f44c28098c04f98d5986d7e8d222c1f Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Tue, 18 Aug 2026 13:36:46 -0400 Subject: [PATCH] feat(cross-impl): Phase L proves foreign data survives a full A->B->A cycle The round-trip conformance check from spec section 8.6, now enforced in CI: a vault holding a foreign record (unknown kind, nested foreign data, and trap fields) plus an unknown top-level index key goes through an interleaved Python -> TypeScript -> Python read-modify-write cycle, and every foreign byte must come back exactly as planted. Allocation must skip past the foreign id, and TS must refuse to reveal the record rather than guess. Landing the check forced the normalization decision it existed to force. Python's _normalize_entry_defaults ran its legacy renames on every entry, foreign ones included -- and those renames are migrations for OUR old shapes: the negative control shows the old code turning a foreign "blacklisted" string into our "archived" flag, which is not backfill but reinterpretation of another application's data. The normalizer now skips unknown kinds entirely, so both implementations carry foreign records byte-for-byte untouched and produce identical canonical state -- the property sync convergence is defined by. Entries with neither kind nor type are pre-kind legacy Python data and still normalize as before. The spec's gap ledger entry for the divergence is resolved, and the Python tolerance tests are tightened from field-subset to exact equality, plus a trap-field test pinning that blacklisted/website/words on a foreign record mean whatever the other application says they mean. Verified: with the normalizer gate stashed, Phase L fails on the exact mangling ("archived": "foreign meaning, not our archive flag"); with it in place, 38/38 cross-impl checks and 1183 Python tests pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Tkj4rzKBA3uhfgk7kqjsb8 --- TODO.md | 12 +++- docs/seedpass_vault_identity_spec.md | 2 +- scripts/cross_impl_check.py | 79 +++++++++++++++++++++++++++ src/seedpass/core/entry_management.py | 12 ++++ src/tests/test_unknown_entry_kinds.py | 54 ++++++++++++------ 5 files changed, 139 insertions(+), 20 deletions(-) diff --git a/TODO.md b/TODO.md index 495148ad..5ba699ac 100644 --- a/TODO.md +++ b/TODO.md @@ -263,6 +263,12 @@ before and after each was confirmed. Ordered by severity. defaults on foreign records where TS carries verbatim — a benign but real canonicalization divergence, now in the spec gap ledger, to align before foreign kinds actually sync. -- [ ] **Round-trip conformance check as CI (spec §8.6).** A phase in - cross_impl_check.py: vault with unknown fields + unknown kinds survives - A→B→A open/save unchanged. Cheap once the passthrough lands. +- [x] **(done 2026-08-18)** Round-trip conformance check as CI (spec §8.6) — + cross-impl Phase L: a vault holding a foreign record (unknown kind, + trap fields, nested data) plus an unknown top-level index key survives + Python→TS→Python read-modify-write byte-for-byte; allocation skips the + foreign id; TS refuses to reveal it. Landing it forced the + normalization decision: Python's legacy renames now skip unknown kinds + (they would have reinterpreted foreign data — verified: the old code + turns a foreign `blacklisted` string into our `archived` flag, and + Phase L catches it). 38/38 cross-impl checks. diff --git a/docs/seedpass_vault_identity_spec.md b/docs/seedpass_vault_identity_spec.md index 1e31665b..5f244be7 100644 --- a/docs/seedpass_vault_identity_spec.md +++ b/docs/seedpass_vault_identity_spec.md @@ -360,7 +360,7 @@ The intended BitLogin-style flow, stated in this spec's terms: | Gap | Where | Consequence | Status | |---|---|---|---| -| Base-field normalization differs on foreign records: Python backfills `tags`/`links`/`date_modified` defaults on load, TypeScript carries them verbatim | both | no data loss, but the two produce different canonical bytes for the same foreign record — matters once foreign kinds sync (§9 defines convergence as byte-identical state) | open — align before any application ships foreign kinds; the round-trip CI check (§8.6) is where it will surface | +| ~~Base-field normalization differed on foreign records~~ | — | — | **resolved 2026-08-18**: both implementations now carry foreign records byte-for-byte untouched (Python's legacy normalizer skips unknown kinds — its renames like `blacklisted`→`archived` would have *reinterpreted* foreign data); enforced by cross-impl Phase L | | `_system.index0` merge handles only the empty case | both | non-empty index0 refuses to merge (loudly) | by design until atlas milestone | | `ssh`/`pgp`/`password` share BIP-85 app 32 | both (spec-level) | namespace collision, shared with Python since v1 | frozen; change requires coordinated version bump | | RSA PGP not derivable | TypeScript | refuses rather than diverging | permanent (RSA generation is not reproducible) | diff --git a/scripts/cross_impl_check.py b/scripts/cross_impl_check.py index 3d26ea5e..6c13176a 100644 --- a/scripts/cross_impl_check.py +++ b/scripts/cross_impl_check.py @@ -985,6 +985,84 @@ def phase_k(tmp: Path) -> None: ) +def phase_l(tmp: Path) -> None: + print("\nPhase L: foreign data round-trip — unknown kinds/fields survive A->B->A") + # Spec section 8.6: for a vault holding records neither side fully + # understands (a future BitLogin kind, unknown fields, an unknown + # top-level index key), an interleaved Python -> TS -> Python + # read-modify-write cycle must preserve every foreign byte. The trap + # fields (blacklisted/website/words) are names Python's legacy + # migrations act on for ITS shapes; touching them on a foreign record + # would be reinterpretation, which the spec forbids. + app_dir = tmp / "l" + app_dir.mkdir() + fp = py_create_profile(app_dir, SEED_A, PASSWORD) + env = {"SEEDPASS_MNEMONIC": SEED_A} + + foreign_entry = { + "kind": "bitlogin_org", + "type": "bitlogin_org", + "label": "Acme Corporation", + "modified_ts": 1700000123, + "blacklisted": "foreign meaning, not our archive flag", + "website": "foreign meaning, not our label alias", + "words": ["foreign", "list"], + "bitlogin": {"admins": ["npub1aaaa"], "roles": {"sales": ["npub1bbbb"]}, "policy_rev": 7}, + } + foreign_top_level = {"spec": "bitlogin-v1", "org_count": 1} + + # Python plants the foreign data at a fresh id beside the native entries. + vault, em, _cfg = _py_vault(app_dir, fp, SEED_A) + data = vault.load_index() + pre_count = len(data["entries"]) + foreign_id = str(max((int(k) for k in data["entries"]), default=-1) + 1) + data["entries"][foreign_id] = dict(foreign_entry) + data["bitlogin_meta"] = dict(foreign_top_level) + vault.save_index(data) + + # Python read-modify-write on a neighbour. + em2 = _py_vault(app_dir, fp, SEED_A)[1] + em2.add_entry("native-python", 16) + + # TS read-modify-write: create and edit, both of which re-validate and + # re-encrypt the whole index. + run_cli(app_dir, "entry", "add", "key-value", "native-ts", "k", "v", env_extra=env) + run_cli(app_dir, "entry", "modify", "0", "--notes", "touched", env_extra=env) + + # Python reopens: every foreign byte must be exactly as planted. + _seed, final = py_open_profile(app_dir, fp, PASSWORD) + check( + "foreign entry survives Python->TS->Python byte-for-byte", + final["entries"].get(foreign_id) == foreign_entry, + json.dumps(final["entries"].get(foreign_id), sort_keys=True)[:200], + ) + check( + "foreign top-level index key survives", + final.get("bitlogin_meta") == foreign_top_level, + json.dumps(final.get("bitlogin_meta"), sort_keys=True), + ) + labels = {e.get("label") for e in final["entries"].values()} + check( + "native entries from both sides coexist with the foreign record", + {"native-python", "native-ts"} <= labels, + str(sorted(str(l) for l in labels)), + ) + check( + "allocation skipped past the foreign id (no collisions, no reuse)", + len(final["entries"]) == pre_count + 3 + and max(int(k) for k in final["entries"]) == int(foreign_id) + 2, + str(sorted(final["entries"].keys(), key=int)), + ) + # And the foreign record is invisible to secret materialization rather + # than guessed at: TS reveal must refuse it. + try: + run_cli(app_dir, "entry", "reveal", f"sp://entry/{foreign_id}", env_extra=env) + refused = False + except Exception: + refused = True + check("TS refuses to reveal the foreign record", refused) + + def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--skip-relay", action="store_true") @@ -1008,6 +1086,7 @@ def main() -> int: phase_i(tmp) phase_j(tmp) phase_k(tmp) + phase_l(tmp) if not args.skip_relay: phase_f(tmp) finally: diff --git a/src/seedpass/core/entry_management.py b/src/seedpass/core/entry_management.py index c59ad73b..c606313d 100644 --- a/src/seedpass/core/entry_management.py +++ b/src/seedpass/core/entry_management.py @@ -120,6 +120,18 @@ class EntryManager: def _normalize_entry_defaults(self, entry: dict[str, Any]) -> dict[str, Any]: """Normalize legacy fields and fill default graph/meta fields.""" + # Foreign records (spec section 8.2): a kind this build does not + # understand belongs to another application sharing the vault. Do not + # alias, backfill, or reinterpret it -- the legacy renames below + # (website->label, blacklisted->archived, words->word_count) are + # migrations for OUR old shapes, and applying them to someone else's + # record mangles their data. Carry it through untouched, exactly as + # the TypeScript side does, so both implementations produce identical + # canonical bytes for the same foreign record. Entries with neither + # kind nor type are pre-kind legacy Python data and still normalize. + foreign_kind = entry.get("kind", entry.get("type")) + if foreign_kind is not None and foreign_kind not in ALL_ENTRY_TYPES: + return entry if "type" not in entry and "kind" in entry: entry["type"] = entry["kind"] if "kind" not in entry: diff --git a/src/tests/test_unknown_entry_kinds.py b/src/tests/test_unknown_entry_kinds.py index ae8003e2..557e36eb 100644 --- a/src/tests/test_unknown_entry_kinds.py +++ b/src/tests/test_unknown_entry_kinds.py @@ -7,14 +7,11 @@ load-bearing for vault sharing -- a record written by another application read-modify-write cycle untouched. This pins it so it cannot regress silently. -One nuance, discovered by this test: Python's ``_load_index`` backfills the -spec's base-field defaults (``tags``, ``links``, ``date_modified``) onto -every entry, foreign ones included. That adds spec-defined fields with -neutral values -- no foreign data is lost or altered, which is what -preservation actually protects. The assertions therefore check that every -foreign field survives with its exact value, not dict equality. (TypeScript -carries foreign records verbatim without the backfill; that divergence is -recorded in the spec's gap ledger and matters once foreign kinds sync.) +``_normalize_entry_defaults`` deliberately skips foreign kinds: the legacy +renames it performs (website->label, blacklisted->archived) are migrations +for OUR old shapes, and applying them to another application's record would +mangle it. So the assertion here is exact equality -- untouched means +untouched, matching the TypeScript side byte for byte. """ import sys @@ -58,13 +55,11 @@ def test_foreign_record_survives_read_modify_write(): entry_mgr2 = EntryManager(vault, backup_mgr) new_id = entry_mgr2.add_entry("native-2", 12) - # Allocation skipped past the foreign id, and every foreign field - # survived with its exact value (base-field backfill is permitted). + # Allocation skipped past the foreign id, and the record is + # byte-for-byte untouched -- no backfill, no legacy renames. assert int(new_id) == 2 final = vault.load_index() - stored = final["entries"]["1"] - for key, value in FOREIGN_RECORD.items(): - assert stored[key] == value + assert final["entries"]["1"] == FOREIGN_RECORD def test_foreign_record_survives_archive_of_neighbour(): @@ -83,7 +78,34 @@ def test_foreign_record_survives_archive_of_neighbour(): entry_mgr2.archive_entry(0) final = vault.load_index() - stored = final["entries"]["1"] - for key, value in FOREIGN_RECORD.items(): - assert stored[key] == value + assert final["entries"]["1"] == FOREIGN_RECORD assert final["entries"]["0"].get("archived") is True + + +def test_foreign_record_with_legacy_trap_fields_is_not_reinterpreted(): + """A foreign record carrying field names our legacy migrations act on + (``blacklisted``, ``website``, ``words``) must NOT have them renamed or + popped -- they mean whatever the other application says they mean.""" + trap = { + "kind": "bitlogin_device", + "type": "bitlogin_device", + "label": "kiosk", + "modified_ts": 1700000456, + "blacklisted": "no, this is a hostname", + "website": "not-our-alias", + "words": ["arbitrary", "foreign", "list"], + } + with TemporaryDirectory() as tmpdir: + vault, _enc = create_vault(Path(tmpdir)) + cfg_mgr = ConfigManager(vault, Path(tmpdir)) + backup_mgr = BackupManager(Path(tmpdir), cfg_mgr) + entry_mgr = EntryManager(vault, backup_mgr) + + entry_mgr.add_entry("native", 12) + data = vault.load_index() + data["entries"]["1"] = dict(trap) + vault.save_index(data) + + entry_mgr2 = EntryManager(vault, backup_mgr) + entry_mgr2.add_entry("native-2", 12) + assert vault.load_index()["entries"]["1"] == trap