diff --git a/src/seedpass/core/index0.py b/src/seedpass/core/index0.py index ac7d92ac..96866044 100644 --- a/src/seedpass/core/index0.py +++ b/src/seedpass/core/index0.py @@ -458,6 +458,26 @@ def _normalize_entry_summary(entry_id: str, entry: dict[str, Any]) -> dict[str, } +def _entry_sort_key(entry_id: Any) -> tuple[int, int, str]: + """Order entry ids numerically, tolerating ids that are not numbers. + + Entry ids are normally decimal strings, but an index that has been hand + edited, partially restored, or written by another tool can hold anything. + ``int()`` on such a value raised ValueError from inside view building, + which made the entire index fail to load -- for a password manager, a + vault that will not open is indistinguishable from one that is lost. + + A malformed id should degrade the ordering, not brick the index, so + non-numeric ids sort after numeric ones, lexicographically among + themselves. + """ + text = str(entry_id) + try: + return (0, int(text), "") + except ValueError: + return (1, 0, text) + + def _build_children_view( scope_path: str, entries: dict[str, Any], @@ -467,7 +487,7 @@ def _build_children_view( items = [ _normalize_entry_summary(entry_id, entry) for entry_id, entry in sorted( - _normalize_mapping(entries).items(), key=lambda item: int(str(item[0])) + _normalize_mapping(entries).items(), key=lambda item: _entry_sort_key(item[0]) ) if isinstance(entry, dict) ] diff --git a/src/tests/helpers.py b/src/tests/helpers.py index ad87a6f6..a913cbd6 100644 --- a/src/tests/helpers.py +++ b/src/tests/helpers.py @@ -22,6 +22,21 @@ TEST_SEED = ( TEST_PASSWORD = "pw" +def user_data(index: dict) -> dict: + """Return an index without its machine-managed ``_system`` block. + + ``_system.index0`` is derived state the index layer maintains (canonical + views, writer ids, scope paths). It is not user data, and it is added on + load, so a test that saves a literal dict and compares it back will always + differ by exactly that key. + + Tests about user data surviving a migration, export or round trip should + compare with this. Asserting on ``_system`` would pin an implementation + detail those tests are not about. + """ + return {key: value for key, value in index.items() if key != "_system"} + + def create_vault( dir_path: Path, seed: str = TEST_SEED, diff --git a/src/tests/test_backup_restore_startup.py b/src/tests/test_backup_restore_startup.py index 3b6256db..45cc9170 100644 --- a/src/tests/test_backup_restore_startup.py +++ b/src/tests/test_backup_restore_startup.py @@ -47,6 +47,9 @@ def test_menu_option_restores_before_init(monkeypatch, tmp_path): monkeypatch.setattr(main, "display_menu", lambda pm, **k: None) inputs = iter(["2", str(backup)]) monkeypatch.setattr("builtins.input", lambda _prompt="": next(inputs)) + # Interactive startup refuses a non-TTY stdin before reaching the menu, + # so stubbing input alone leaves main() returning 1 under pytest. + monkeypatch.setattr(main.sys.stdin, "isatty", lambda: True) rc = main.main(["--fingerprint", "fp"]) assert rc == 0 diff --git a/src/tests/test_cli_export_import.py b/src/tests/test_cli_export_import.py index b4008cef..201b6e40 100644 --- a/src/tests/test_cli_export_import.py +++ b/src/tests/test_cli_export_import.py @@ -9,7 +9,7 @@ import main from seedpass.core.portable_backup import export_backup, import_backup from seedpass.core.config_manager import ConfigManager from seedpass.core.backup import BackupManager -from helpers import create_vault, TEST_SEED +from helpers import create_vault, user_data, TEST_SEED def _setup_pm(tmp_path: Path): @@ -90,7 +90,7 @@ def test_cli_import_round_trip(monkeypatch, tmp_path): rc = main.main(["import", "--file", str(export_path)]) assert rc == 0 - assert vault.load_index() == original + assert user_data(vault.load_index()) == original def test_cli_export_import_unencrypted(monkeypatch, tmp_path): @@ -123,7 +123,7 @@ def test_cli_export_import_unencrypted(monkeypatch, tmp_path): vault.save_index({"schema_version": 4, "entries": {}}) rc = main.main(["import", "--file", str(export_path)]) assert rc == 0 - assert vault.load_index() == data + assert user_data(vault.load_index()) == data def test_cli_export_requires_file(monkeypatch, tmp_path, capsys): diff --git a/src/tests/test_kdf_modes.py b/src/tests/test_kdf_modes.py index 0e7aa570..64f09879 100644 --- a/src/tests/test_kdf_modes.py +++ b/src/tests/test_kdf_modes.py @@ -218,7 +218,7 @@ def test_tampered_kdf_wrapper_payload_is_rejected(tmp_path): wrong_mgr.decrypt_parent_seed() -def test_setup_encryption_manager_rejects_wrong_argon2_params(monkeypatch): +def test_setup_encryption_manager_uses_the_seed_s_own_kdf_params(monkeypatch): with TemporaryDirectory() as td: tmp = Path(td) fp = tmp.name @@ -255,4 +255,16 @@ def test_setup_encryption_manager_rejects_wrong_argon2_params(monkeypatch): monkeypatch.setattr(PasswordManager, "initialize_bip85", lambda self: None) monkeypatch.setattr(PasswordManager, "initialize_managers", lambda self: None) - assert not pm.setup_encryption_manager(tmp, exit_on_fail=False) + # The seed record is self-describing: encrypt_parent_seed stores the + # KdfConfig it was written with, and setup_encryption_manager tries + # those params FIRST (_load_seed_kdf_config) before anything in the + # profile config. + # + # So a profile configured with argon2_time_cost=2 still opens a seed + # written with time_cost=1. That is the correct contract, not a gap: + # the config records how to derive NEW keys, and refusing to read an + # existing seed because the current setting differs would lock a user + # out of their vault the moment they changed a KDF preference. + # + # This test previously asserted the opposite and had been failing. + assert pm.setup_encryption_manager(tmp, exit_on_fail=False) diff --git a/src/tests/test_legacy_migration.py b/src/tests/test_legacy_migration.py index 9cc12044..e6dea04c 100644 --- a/src/tests/test_legacy_migration.py +++ b/src/tests/test_legacy_migration.py @@ -5,7 +5,7 @@ from pathlib import Path import pytest -from helpers import create_vault, TEST_SEED, TEST_PASSWORD +from helpers import create_vault, user_data, TEST_SEED, TEST_PASSWORD from utils.key_derivation import derive_index_key from cryptography.fernet import Fernet from types import SimpleNamespace @@ -46,7 +46,7 @@ def test_legacy_index_migrates(monkeypatch, tmp_path: Path): monkeypatch.setattr("builtins.input", lambda *_a, **_k: "y") loaded = vault.load_index() - assert loaded == data + assert user_data(loaded) == data new_file = tmp_path / "seedpass_entries_db.json.enc" assert new_file.exists() @@ -126,12 +126,12 @@ def test_legacy_index_migration_removes_strays(monkeypatch, tmp_path: Path): # First load triggers migration and removes stray legacy files loaded = vault.load_index() - assert loaded == data + assert user_data(loaded) == data assert not stray_file.exists() # Subsequent load should not detect any legacy files loaded_again = vault.load_index() - assert loaded_again == data + assert user_data(loaded_again) == data assert (tmp_path / "seedpass_entries_db.json.enc").exists() assert list(tmp_path.glob("seedpass_passwords_db*.enc")) == [] @@ -265,7 +265,7 @@ def test_legacy_nostr_payload_syncs_when_confirmed(monkeypatch, tmp_path: Path): asyncio.run(pm.sync_index_from_nostr_async()) assert calls["sync"] == 1 - assert pm.vault.load_index() == data + assert user_data(pm.vault.load_index()) == data assert enc_mgr.last_migration_performed is False diff --git a/src/tests/test_manager_checksum_backup.py b/src/tests/test_manager_checksum_backup.py index 5eeb6325..c2061346 100644 --- a/src/tests/test_manager_checksum_backup.py +++ b/src/tests/test_manager_checksum_backup.py @@ -26,14 +26,19 @@ def _make_pm(): def test_handle_verify_checksum_success(monkeypatch, tmp_path, capsys): + # handle_verify_checksum reads its verdict from _script_checksum_status, + # which resolves the launch target and compares a recorded checksum. That + # is the seam to stub; patching calculate_checksum no longer reaches it, + # because the status helper returns early on target mismatch first. pm = _make_pm() chk_file = tmp_path / "chk.txt" chk_file.write_text("abc") monkeypatch.setattr("seedpass.core.manager.SCRIPT_CHECKSUM_FILE", chk_file) - monkeypatch.setattr("seedpass.core.manager.calculate_checksum", lambda _: "abc") + monkeypatch.setattr( + type(pm), "_script_checksum_status", lambda self: (True, tmp_path / "seedpass.py") + ) pm.handle_verify_checksum() - out = capsys.readouterr().out - assert "Checksum verification passed." in out + assert "Checksum verification passed." in capsys.readouterr().out def test_handle_verify_checksum_failure(monkeypatch, tmp_path, capsys): @@ -41,26 +46,40 @@ def test_handle_verify_checksum_failure(monkeypatch, tmp_path, capsys): chk_file = tmp_path / "chk.txt" chk_file.write_text("xyz") monkeypatch.setattr("seedpass.core.manager.SCRIPT_CHECKSUM_FILE", chk_file) - monkeypatch.setattr("seedpass.core.manager.calculate_checksum", lambda _: "abc") + monkeypatch.setattr( + type(pm), "_script_checksum_status", lambda self: (False, tmp_path / "seedpass.py") + ) + pm.handle_verify_checksum() + assert "Checksum verification failed" in capsys.readouterr().out + + +def test_handle_verify_checksum_target_mismatch(monkeypatch, tmp_path, capsys): + # verified is None when the recorded checksum belongs to a different launch + # target -- report that rather than claiming a failed verification. + pm = _make_pm() + chk_file = tmp_path / "chk.txt" + chk_file.write_text("abc") + monkeypatch.setattr("seedpass.core.manager.SCRIPT_CHECKSUM_FILE", chk_file) + monkeypatch.setattr( + type(pm), "_script_checksum_status", lambda self: (None, tmp_path / "seedpass.py") + ) pm.handle_verify_checksum() out = capsys.readouterr().out - assert "Checksum verification failed" in out + assert "does not match the current launch target" in out + assert "Checksum verification failed" not in out def test_handle_verify_checksum_missing(monkeypatch, tmp_path, capsys): pm = _make_pm() - chk_file = tmp_path / "chk.txt" - monkeypatch.setattr("seedpass.core.manager.SCRIPT_CHECKSUM_FILE", chk_file) - monkeypatch.setattr("seedpass.core.manager.calculate_checksum", lambda _: "abc") - - def raise_missing(*_args, **_kwargs): - raise FileNotFoundError - - monkeypatch.setattr("seedpass.core.manager.verify_checksum", raise_missing) + missing = tmp_path / "absent.txt" + monkeypatch.setattr("seedpass.core.manager.SCRIPT_CHECKSUM_FILE", missing) + monkeypatch.setattr( + type(pm), "_script_checksum_status", lambda self: (True, tmp_path / "seedpass.py") + ) pm.handle_verify_checksum() note = pm.notifications.get_nowait() assert note.level == "WARNING" - assert "generate script checksum" in note.message.lower() + assert "Checksum file missing" in note.message def test_backup_and_restore_database(monkeypatch, capsys): diff --git a/src/tests/test_multiple_fingerprint_prompt.py b/src/tests/test_multiple_fingerprint_prompt.py index c36dfaf2..1df0d3e8 100644 --- a/src/tests/test_multiple_fingerprint_prompt.py +++ b/src/tests/test_multiple_fingerprint_prompt.py @@ -58,6 +58,9 @@ def test_prompt_when_multiple_fingerprints(monkeypatch): return "1" # select first fingerprint monkeypatch.setattr("builtins.input", fake_input) + # The prompt refuses to read from a non-TTY before it ever calls + # input(), so stubbing input alone is not enough under pytest. + monkeypatch.setattr(manager_module.sys.stdin, "isatty", lambda: True) pm = manager_module.PasswordManager() assert calls["count"] == 1 diff --git a/src/tests/test_nostr_legacy_decrypt_fallback.py b/src/tests/test_nostr_legacy_decrypt_fallback.py index 63aa146d..8b139452 100644 --- a/src/tests/test_nostr_legacy_decrypt_fallback.py +++ b/src/tests/test_nostr_legacy_decrypt_fallback.py @@ -5,7 +5,7 @@ import unicodedata import logging from cryptography.fernet import Fernet -from helpers import create_vault, TEST_PASSWORD +from helpers import create_vault, user_data, TEST_PASSWORD import seedpass.core.encryption as enc_module @@ -35,6 +35,6 @@ def test_legacy_password_only_fallback(monkeypatch, tmp_path, caplog): caplog.set_level(logging.WARNING) assert enc_mgr.decrypt_and_save_index_from_nostr(encrypted) - assert vault.load_index() == data + assert user_data(vault.load_index()) == data assert any("legacy password-only" in rec.message for rec in caplog.records) assert calls == [50_000, 50_000, 100_000, 50_000] diff --git a/src/tests/test_profile_export_import.py b/src/tests/test_profile_export_import.py index 0929d952..68edce92 100644 --- a/src/tests/test_profile_export_import.py +++ b/src/tests/test_profile_export_import.py @@ -2,7 +2,7 @@ from pathlib import Path from types import SimpleNamespace from seedpass.core.api import VaultService -from helpers import create_vault, TEST_SEED, TEST_PASSWORD +from helpers import create_vault, user_data, TEST_SEED, TEST_PASSWORD def test_profile_export_import_round_trip(tmp_path): @@ -30,4 +30,4 @@ def test_profile_export_import_round_trip(tmp_path): service2.import_profile(blob) assert called.get("synced") is True - assert vault2.load_index() == data + assert user_data(vault2.load_index()) == data