bugfix: exiting nickname entry with nickname already saved deleted previous nickname; fixed settings_get with prelogin arg
What changed, and why it matters
This commit fixes a bug in the COLDCARD hardware wallet where cancelling out of the 'Set Nickname' screen would accidentally delete an existing nickname. The nickname is shown on the login screen to help users identify their device. The fix also corrects a related test helper that was not properly reading pre-login settings. There is no direct evidence this was a security vulnerability, but it could be a minor usability/reliability issue.
Treat as a routine bugfix and usability improvement. No urgent security action is indicated. Users relying on the login nickname for device identification may want to update once the release is available and re-set their nickname if it was lost.
Security signals we found
Loss of user-configured setting (nickname) on benign UI action (cancel)
Fix to prelogin settings_get test helper suggests prior test coverage of prelogin settings was incorrect
No authentication or cryptographic weakness visible in diff
Evidence from the diff
The patch modifies shared/actions.py pick_nickname() so that if the user exits nickname entry without changing the value (nn is None or unchanged), the function returns early without calling s.save(). Previously, the code would save even when nn was None, which could remove an existing nickname. It also changes the empty-input handling: an empty string now explicitly removes the key, while a non-empty value is stripped and saved. The test fixture settings_get is fixed to correctly reference SettingsObject.prelogin() when prelogin=True, and a new regression test verifies that cancelling preserves an existing nickname.
Changed components
shared/actions.py: pick_nickname()testing/conftest.py: settings_get and settings_remove fixturestesting/test_ux.py: new regression test test_nickname_cancel_preserves_existingInspect captured patch +50 / −10
diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md
index c7ea947..eb6d2a5 100644
--- a/releases/Next-ChangeLog.md
+++ b/releases/Next-ChangeLog.md
@@ -6,6 +6,7 @@ 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
# Mk Specific Changes
diff --git a/shared/actions.py b/shared/actions.py
index 74d7a94..8a63c6a 100644
--- a/shared/actions.py
+++ b/shared/actions.py
@@ -459,21 +459,27 @@ async def pick_nickname(*a):
# Value is not stored with normal settings, it's part of "prelogin" settings
# which are encrypted with zero-key.
s = SettingsObject.prelogin()
- nick = s.get('nick', '')
+ k = "nick"
+ nick = s.get(k, '')
if not nick:
- ch = await ux_show_story('''\
-You can give this Coldcard a nickname and it will be shown before login.''')
+ ch = await ux_show_story("You can give this Coldcard a nickname"
+ " and it will be shown before login.")
if ch != 'y': return
nn = await ux_input_text(nick, confirm_exit=False, prompt="Enter Nickname")
+ if nn is None or (nick == nn): return # user exit & same value - noop
+
from glob import dis
dis.fullscreen("Saving...")
dis.busy_bar(True)
- nn = nn.strip() if nn else None
- s.set('nick', nn)
+ if not nn:
+ s.remove_key(k)
+ else:
+ s.set(k, nn.strip())
+
s.save()
dis.busy_bar(False)
del s
diff --git a/testing/conftest.py b/testing/conftest.py
index 533d1c4..f66deee 100644
--- a/testing/conftest.py
+++ b/testing/conftest.py
@@ -1029,9 +1029,12 @@ def settings_set(sim_exec):
def settings_get(sim_exec):
def doit(key, def_val=None, prelogin=False):
- source = "from nvstore import SettingsObject;SettingsObject.prelogin()" if prelogin else "settings"
- cmd = f"RV.write(repr({source}.get('{key}', {def_val!r})))"
- resp = sim_exec(cmd)
+ if prelogin:
+ src = f"from nvstore import SettingsObject;RV.write(repr(SettingsObject.prelogin().get('{key}', {def_val!r})))"
+ else:
+ src = f"RV.write(repr(settings.get('{key}', {def_val!r})))"
+
+ resp = sim_exec(src)
assert 'Traceback' not in resp, resp
return eval(resp)
@@ -1051,8 +1054,9 @@ def master_settings_get(sim_exec):
@pytest.fixture
def settings_remove(sim_exec):
- def doit(key):
- x = sim_exec("settings.remove_key('%s')" % key)
+ def doit(key, prelogin=False):
+ source = "from nvstore import SettingsObject;SettingsObject.prelogin()" if prelogin else "settings"
+ x = sim_exec("%s.remove_key('%s')" % (source, key))
assert x == ''
return doit
diff --git a/testing/test_ux.py b/testing/test_ux.py
index cbd4744..dcfb335 100644
--- a/testing/test_ux.py
+++ b/testing/test_ux.py
@@ -1126,6 +1126,35 @@ def test_file_picker_suffixes(pick_menu_item, goto_home, cap_story, microsd_wipe
microsd_wipe()
+@pytest.mark.parametrize("already_set", [True, False])
+def test_nickname_cancel_preserves_existing(already_set, goto_home, pick_menu_item, need_keypress,
+ settings_set, settings_get, press_cancel, press_select,
+ settings_remove, sim_exec):
+ nick = 'CancelTest'
+
+ if already_set:
+ settings_set("nick", nick, prelogin=True)
+ else:
+ settings_remove("nick", prelogin=True)
+
+ goto_home()
+ pick_menu_item('Settings')
+ pick_menu_item('Login Settings')
+ pick_menu_item('Set Nickname')
+ if not already_set:
+ press_select() # intro
+
+ press_cancel()
+
+ new_nick = settings_get("nick", False, prelogin=True)
+ if already_set:
+ assert nick == new_nick
+ else:
+ assert new_nick is False
+
+ settings_remove("nick") # clean-up
+
+
@pytest.mark.onetime
def test_dump_menutree(sim_execfile):
# saves to ../unix/work/menudump.txt
Why this scored 24/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.