refactor: catch Exception, not bare except, in parse_wallet fallbacks
What the developer wrote
Authored by kkdao
refactor: catch Exception, not bare except, in parse_wallet fallbacks
What changed, and why it matters
This commit tightens error handling in Krux's wallet parsing. Previously, the code used bare 'except:' clauses that would catch everything, including KeyboardInterrupt and SystemExit. Those special exceptions should normally be allowed to propagate so a user can cancel an operation or the program can shut down cleanly. The change replaces the bare catches with 'except Exception:', which still handles ordinary parsing errors but lets KeyboardInterrupt and SystemExit through. A new test confirms KeyboardInterrupt now propagates from all three parsing fallback paths.
No immediate action required; this is a defensive hardening patch. Reviewers should verify no other bare 'except:' clauses remain in security-relevant parsing paths and consider whether similar patterns exist elsewhere in the codebase.
Security signals we found
Bare except clauses replaced with except Exception to avoid swallowing KeyboardInterrupt/SystemExit
New regression test ensures KeyboardInterrupt propagates through all parse_wallet fallback branches
Comments explicitly call out untrusted input and interrupt propagation behavior
Evidence from the diff
In src/krux/wallet.py, parse_wallet() has three fallback parsing branches (JSON, key-value file, raw descriptor). Each used a bare ‘except:’ that would swallow BaseException subclasses, including KeyboardInterrupt and SystemExit. The patch changes them to ‘except Exception:’, adds explanatory comments noting that interrupts now propagate, and adds tests/test_wallet.py cases verifying KeyboardInterrupt is not swallowed or relabeled as ‘invalid wallet format’ in any branch. The key-value branch previously re-raised ValueError for any non-ValueError exception; it now still re-raises ValueError for unexpected Exception subclasses but no longer catches BaseException.
Changed components
src/krux/wallet.pytests/test_wallet.pyInspect captured patch +42 / −7
diff --git a/src/krux/wallet.py b/src/krux/wallet.py
index a3c763f..6661460 100644
--- a/src/krux/wallet.py
+++ b/src/krux/wallet.py
@@ -453,7 +453,10 @@ def parse_wallet(wallet_data):
raise KeyError('"descriptor" key not found in JSON')
except KeyError:
raise ValueError("invalid wallet format")
- except:
+ except Exception:
+ # Untrusted input: any non-KeyError parse failure (bad JSON, bad
+ # descriptor) falls through to the next format. KeyboardInterrupt/
+ # SystemExit are no longer swallowed.
pass
# Try to parse as a key-value file
@@ -463,14 +466,18 @@ def parse_wallet(wallet_data):
return descriptor, label
except ValueError:
raise
- except:
+ except Exception:
+ # Untrusted input: an unexpected parse failure means "invalid wallet";
+ # interrupts (KeyboardInterrupt/SystemExit) still propagate.
raise ValueError("invalid wallet format")
# Try to parse directly as a descriptor
try:
descriptor = Descriptor.from_string(wallet_data.strip())
return descriptor, None
- except:
+ except Exception:
+ # Untrusted input: not a bare descriptor either; fall through to the final
+ # raise. KeyboardInterrupt/SystemExit are no longer swallowed.
pass
raise ValueError("invalid wallet format")
diff --git a/tests/test_wallet.py b/tests/test_wallet.py
index 816b5e3..4d1873d 100644
--- a/tests/test_wallet.py
+++ b/tests/test_wallet.py
@@ -1773,8 +1773,8 @@ def test_parse_address_raises_errors(mocker, m5stickv, tdata):
def test_parse_address_propagates_keyboardinterrupt(mocker, m5stickv):
- """KeyboardInterrupt must propagate, never be swallowed (line 501) or
- relabeled 'invalid address' (line 507).
+ """KeyboardInterrupt must propagate: never swallowed by the bech32-uppercase
+ fallback, nor relabeled 'invalid address' by the final attempt.
parse_address imports address_to_scriptpubkey *inside* the function, so the
patch target is embit.script.address_to_scriptpubkey — there is no
@@ -1784,15 +1784,43 @@ def test_parse_address_propagates_keyboardinterrupt(mocker, m5stickv):
mocker.patch("embit.script.address_to_scriptpubkey", side_effect=KeyboardInterrupt)
- # Uppercase input exercises the bech32-uppercase branch (line 501)
+ # Uppercase input exercises the bech32-uppercase fallback branch
with pytest.raises(KeyboardInterrupt):
parse_address("BC1QX2ZUDAY8D6J4UFH4DF6E9TTD06LNFMN2CUZ0VN")
- # Mixed-case input skips that branch and exercises the final attempt (line 507)
+ # Mixed-case input skips that branch and exercises the final attempt
with pytest.raises(KeyboardInterrupt):
parse_address("bc1qx2zuday8d6j4ufh4df6e9ttd06lnfmn2cuz0vn")
+def test_parse_wallet_propagates_keyboardinterrupt(mocker, m5stickv):
+ """KeyboardInterrupt must propagate from each parse_wallet fallback: never
+ swallowed by the JSON or raw-descriptor fallbacks, nor relabeled 'invalid
+ wallet format' by the key-value fallback."""
+ import krux.wallet
+ from krux.wallet import parse_wallet
+
+ # JSON branch: valid JSON with a 'descriptor' key; the Descriptor.from_string
+ # call is interrupted.
+ mocker.patch.object(
+ krux.wallet.Descriptor, "from_string", side_effect=KeyboardInterrupt
+ )
+ with pytest.raises(KeyboardInterrupt):
+ parse_wallet('{"descriptor": "x"}')
+
+ # Key-value branch: parse_key_value_file is interrupted. (json.loads of a
+ # non-JSON string fails first and is correctly caught by the JSON branch.)
+ mocker.patch("krux.wallet.parse_key_value_file", side_effect=KeyboardInterrupt)
+ with pytest.raises(KeyboardInterrupt):
+ parse_wallet("invalid wallet format")
+
+ # Raw-descriptor branch: key-value returns nothing (so we fall through), and
+ # the Descriptor.from_string call is interrupted (still patched from above).
+ mocker.patch("krux.wallet.parse_key_value_file", return_value=(None, None))
+ with pytest.raises(KeyboardInterrupt):
+ parse_wallet("wpkh(tpubraw/0/*)")
+
+
def test_to_unambiguous_descriptor(mocker, m5stickv, tdata):
from embit.descriptor import Descriptor
from krux.wallet import to_unambiguous_descriptor
Why this scored 34/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.