bugfix: "Send Password" menu item visibility reversed, do not store password as None, UX fixes
What changed, and why it matters
This update fixes three small but real bugs in the COLDCARD's Notes & Passwords feature. The most important fix corrects a reversed menu setting: the 'Send Password' option was accidentally shown only when USB was disabled, and hidden when USB was enabled. It also prevents the device from storing a blank password as the special value 'None', which could crash the device later when sending a password. Finally, it stops a misleading 'Saving...' message from appearing after a failed import.
No immediate user action beyond applying the firmware update is needed. The bugs are functional UX/crash issues rather than remote-exploitable vulnerabilities. Users who created password entries while cancelling the password prompt should verify those entries after updating.
Security signals we found
Reversed access-control predicate caused a security-sensitive action ('Send Password') to be available in the wrong configuration state
None value stored for a password field led to an unhandled TypeError/crash when the 'Send Password' feature was used
Post-failure success UX could mislead users about whether an import succeeded
Evidence from the diff
The patch modifies shared/notes.py. The ‘Send Password’ menu predicate is corrected from settings.get('du', True) (truthy when USB disabled) to not settings.get('du', 0) (shown when USB is enabled). New password creation now coalesces a cancelled password prompt to an empty string (or "") instead of None, avoiding a TypeError: 'NoneType' object is not iterable inside EmulatedKeyboard.can_type. The import flow now checks the return value of import_from_json and returns early on failure, suppressing the post-failure ‘Saved.’ UX pause. Tests are added for both the visibility regression and the None-password crash.
Changed components
shared/notes.pyNotes & Passwords featurePasswordContent classSend Password menu itemimport_from_json / import_from_other functionsInspect captured patch +65 / −4
diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md
index eb6d2a5..b1bb3de 100644
--- a/releases/Next-ChangeLog.md
+++ b/releases/Next-ChangeLog.md
@@ -7,6 +7,9 @@ This lists the new changes that have not yet been published in a normal release.
- Bugfix: Delta Mode Trick PIN was never restored from backup
- Bugfix: Proper error message for incorrect 7z headers
- Bugfix: Exiting nickname entry with nickname already saved deleted previous nickname
+- Bugfix: "Send Password" menu item inside Notes & Passwords visibility reversed
+- Bugfix: Yikes when using "Send Password" on entry with password None field
+- Bugfix: Do not show "Saving..." UX after failed Notes & Passwords import
# Mk Specific Changes
diff --git a/shared/notes.py b/shared/notes.py
index eff14d6..e14a84f 100644
--- a/shared/notes.py
+++ b/shared/notes.py
@@ -380,7 +380,7 @@ class PasswordContent(NoteContentBase):
# if self.misc: rv.append(MenuItem('↳ (notes)', f=self.view))
rv += [
MenuItem('View Password', f=self.view_pw),
- MenuItem('Send Password', f=self.send_pw, predicate=lambda: settings.get('du', True)),
+ MenuItem('Send Password', f=self.send_pw, predicate=lambda: not settings.get('du', 0)),
]
if not readonly:
rv += [
@@ -468,7 +468,8 @@ class PasswordContent(NoteContentBase):
if self.idx == -1:
# prompt for password only on new records.
- self.password = await get_a_password(self.password)
+ # can be None if CANCEL is pressed - handle, Send Password requires string
+ self.password = await get_a_password(self.password) or ""
site = await ux_input_text(self.site, max_len=ONE_LINE, scan_ok=True, confirm_exit=False,
prompt='Website', placeholder='(optional)')
@@ -664,8 +665,9 @@ async def import_from_other(menu, *a):
records = json.load(open(fn, 'rt'))
# We have some JSON, parsed now.
- await import_from_json(records)
-
+ ok = await import_from_json(records)
+ if not ok: return
+
await ux_dramatic_pause('Saved.', 3)
menu.update_contents()
@@ -683,6 +685,7 @@ async def import_from_json(records):
settings.set('notes', was)
settings.set('secnap', True)
settings.save()
+ return True
except Exception as e:
await ux_show_story(title="Failure", msg=str(e) + '\n\n' + problem_file_line(e))
diff --git a/testing/test_notes.py b/testing/test_notes.py
index b544132..260ffad 100644
--- a/testing/test_notes.py
+++ b/testing/test_notes.py
@@ -672,6 +672,61 @@ def test_sign_note_body(msg, addr_fmt, acct, need_some_notes,
sign_msg_from_text(msg, addr_fmt, acct, False, 0, way)
+def test_send_password_menu_item(need_some_passwords, goto_notes, cap_menu, pick_menu_item,
+ settings_set, settings_remove, press_cancel):
+ # covers regression where "Send Password" menu item was only shown when USB was disabled
+ need_some_passwords()
+
+ settings_set('du', 1)
+ goto_notes()
+ pick_menu_item('1: A')
+ time.sleep(.2)
+ m = cap_menu()
+ assert 'Send Password' not in m
+ press_cancel()
+
+ settings_set('du', 0)
+ goto_notes()
+ pick_menu_item('1: A')
+ time.sleep(.2)
+ m = cap_menu()
+ assert 'Send Password' in m
+ for _ in range(3):
+ press_cancel()
+
+
+@pytest.mark.onetime
+def test_password_cancel_stores_empty_not_none(goto_notes, need_keypress, press_select,
+ press_cancel, enter_text, settings_get,
+ settings_set, cap_screen, pick_menu_item):
+ # canceling the password field when creating a new password entry stored
+ # None instead of ''. EmulatedKeyboard.can_type(None) then raised
+ # TypeError: 'NoneType' object is not iterable when "Send Password" was selected.
+ #
+ settings_set('secnap', True)
+ settings_set('notes', [])
+
+ goto_notes('New Password')
+ enter_text('cancel-pw-test') # title
+ press_select() # skip username
+ press_cancel() # cancel password field - bug, stores None
+ press_select() # skip site
+ press_cancel() # exit misc
+
+ time.sleep(0.2)
+
+ goto_notes()
+ pick_menu_item('1: cancel-pw-test')
+ pick_menu_item('Send Password')
+ time.sleep(.5)
+
+ scr = cap_screen()
+ assert 'Traceback' not in scr
+ assert "Place mouse at" in scr
+ for _ in range(5):
+ press_cancel()
+
+
@pytest.mark.parametrize("chain", ["BTC", "XTN"])
@pytest.mark.parametrize("change", [True, False])
@pytest.mark.parametrize("idx", [None, 0, 9999])
Why this scored 38/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.