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