refactor: drop non-dict JSON coercion in encryption storage
What the developer wrote
Authored by kkdao
refactor: drop non-dict JSON coercion in encryption storage
Loading seeds.json no longer coerces valid-but-non-dict JSON to an
empty dict, so a wrong-shape file is preserved instead of being
silently overwritten on the next store. decrypt() keeps the
isinstance(source, dict) guard that prevents a crash on such files.
What changed, and why it matters
This commit changes how Krux loads its encrypted seed storage file. Previously, if the file contained valid JSON but in the wrong shape (for example, a list instead of a dictionary), the app would silently treat it as empty and overwrite it on the next save. Now the app keeps the file's original contents and relies on a separate safety check to avoid crashing. The change is described as a code cleanup, but it also removes a behavior that could hide or destroy user data if a storage file were tampered with or corrupted.
Review whether preserving a wrong-shaped seeds.json is the desired security posture, or whether the app should instead reject/alert the user and enter a safe recovery mode. Ensure decrypt()'s isinstance(source, dict) guard is consistently applied to all code paths that consume self.stored and self.stored_sd. Consider adding a test that verifies store_encrypted_kef() does not overwrite an existing non-dict file, and document the expected behavior for tampered storage files.
Security signals we found
Behavior change in persistence layer: malformed-shape but valid JSON is no longer silently normalized to empty dict
Potential data-loss vector removed: previous code would overwrite a non-dict seeds.json on next store
Crash-prevention guard retained in decrypt() for non-dict storage
No input validation or signature added for the loaded JSON object
Commit is framed as a refactor, not a security fix
Evidence from the diff
The patch removes the isinstance(mnemonics, dict) fallback in MnemonicStorage._load_mnemonics(). Previously json.loads() results that were not dicts were coerced to {}, which meant a non-dict seeds.json was silently discarded and overwritten on the next store_encrypted_kef() call. The refactor preserves the raw parsed object. A guard remains in decrypt() so non-dict storage returns None instead of crashing. Tests are updated to expect the new behavior: storage.stored now equals [1, 2, 3] for a list-shaped file, and tests that asserted silent coercion/overwrite are removed.
Changed components
src/krux/encryption.py:MnemonicStorage._load_mnemonics()src/krux/encryption.py:MnemonicStorage.decrypt()src/krux/encryption.py:MnemonicStorage.store_encrypted_kef() (read-before-write path)tests/test_encryption.pyInspect captured patch +3 / −62
diff --git a/src/krux/encryption.py b/src/krux/encryption.py
index 4bca6ca..f2520e9 100644
--- a/src/krux/encryption.py
+++ b/src/krux/encryption.py
@@ -38,8 +38,7 @@ class MnemonicStorage:
@staticmethod
def _load_mnemonics(contents):
- mnemonics = json.loads(contents)
- return mnemonics if isinstance(mnemonics, dict) else {}
+ return json.loads(contents)
def __init__(self) -> None:
self.stored = {}
diff --git a/tests/test_encryption.py b/tests/test_encryption.py
index 22f9f30..fafd1b4 100644
--- a/tests/test_encryption.py
+++ b/tests/test_encryption.py
@@ -661,19 +661,6 @@ def test_init_sd_load_malformed_json_starts_empty(m5stickv, mocker):
assert storage.stored_sd == {}
-def test_init_sd_load_non_dict_json_starts_empty(m5stickv, mocker):
- from krux.encryption import MnemonicStorage
-
- sd = mocker.MagicMock()
- sd.read.return_value = "[1, 2, 3]"
- sdhandler = mocker.MagicMock()
- sdhandler.return_value.__enter__.return_value = sd
- mocker.patch("krux.encryption.SDHandler", new=sdhandler)
- with patch("krux.encryption.open", new=mocker.mock_open(read_data="{}")):
- storage = MnemonicStorage()
- assert storage.stored_sd == {}
-
-
# --- __init__ flash load (self.stored) ---
@@ -707,16 +694,6 @@ def test_init_flash_load_malformed_json_starts_empty(m5stickv, mocker):
assert storage.stored == {}
-def test_init_flash_load_non_dict_json_starts_empty(m5stickv, mocker):
- from krux.encryption import MnemonicStorage
-
- mocker.patch("krux.encryption.SDHandler", side_effect=OSError)
- with patch("krux.encryption.open", new=mocker.mock_open(read_data="[1, 2, 3]")):
- storage = MnemonicStorage()
- assert storage.stored == {}
- assert storage.list_mnemonics() == []
-
-
# --- store_encrypted_kef SD read-before-write ---
@@ -766,23 +743,6 @@ def test_store_sd_read_malformed_json_still_writes(
m().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
-def test_store_sd_read_non_dict_json_still_writes(
- m5stickv, mocker, mock_file_operations
-):
- from krux.krux_settings import Settings
- from krux.encryption import MnemonicStorage
-
- storage = MnemonicStorage()
- Settings().encryption.version = "AES-ECB"
- mocker.patch("krux.sd_card.SDHandler.read", return_value="[1, 2, 3]")
- with patch("krux.sd_card.open", new=mocker.mock_open(read_data="{}")) as m:
- success = storage.store_encrypted_kef(
- "KEFecbID", KEF_ENVELOPE_ECB, sd_card=True
- )
- assert success is True
- m().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
-
-
# --- store_encrypted_kef flash read-before-write ---
@@ -837,24 +797,6 @@ def test_store_flash_read_malformed_json_still_writes(m5stickv, mocker):
write_handle().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
-def test_store_flash_read_non_dict_json_still_writes(m5stickv, mocker):
- from krux.krux_settings import Settings
- from krux.encryption import MnemonicStorage
-
- with patch("krux.encryption.open", new=mocker.mock_open(read_data="{}")):
- storage = MnemonicStorage()
- Settings().encryption.version = "AES-ECB"
- read_handle = mocker.mock_open(read_data="[1, 2, 3]")
- write_handle = mocker.mock_open()
- mocker.patch(
- "krux.encryption.open",
- side_effect=[read_handle.return_value, write_handle.return_value],
- )
- success = storage.store_encrypted_kef("KEFecbID", KEF_ENVELOPE_ECB, sd_card=False)
- assert success is True
- write_handle().write.assert_called_once_with(KEF_ECBENTROPY_ONLY_JSON)
-
-
# ---------------------------------------------------------------------------
# decrypt() must not crash on a missing id or non-dict storage.
#
@@ -878,8 +820,8 @@ def test_decrypt_non_dict_storage_returns_none(m5stickv, mocker):
from krux.encryption import MnemonicStorage
mocker.patch("krux.encryption.SDHandler", side_effect=OSError)
- # valid JSON that is not an object -> storage starts empty
+ # valid JSON that is not an object -> loaded as-is; decrypt must not crash
with patch("krux.encryption.open", new=mocker.mock_open(read_data="[1, 2, 3]")):
storage = MnemonicStorage()
- assert storage.stored == {}
+ assert storage.stored == [1, 2, 3]
assert storage.decrypt("any-key", "any-id", sd_card=False) is None
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.