What changed, and why it matters
This commit adds a new feature to the COLDCARD firmware that lets users organize their saved secure notes and passwords into groups, similar to folders. It is a user-experience improvement and does not fix or introduce any security vulnerability.
No security action required; review as a normal feature commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change extends the existing Notes/Passwords feature by adding an optional ‘group’ field to NoteContent and PasswordContent records. It introduces NoteGroupMenu and GroupPickerMenu classes, updates serialization to include the group only when set, and adjusts menu navigation so grouped items appear under collapsible group headings. Tests are added to cover grouped note/password creation, editing, deletion, and backward compatibility with older records that lack a group field.
Changed components
shared/notes.pytesting/test_notes.pyCOLDCARD Secure Notes & Passwords user interfaceInspect captured patch +345 / −18
diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md
index 87ff4f2..c1f66a4 100644
--- a/releases/Next-ChangeLog.md
+++ b/releases/Next-ChangeLog.md
@@ -59,6 +59,7 @@ This lists the new changes that have not yet been published in a normal release.
## 1.4.xQ - 2065-04-xx
+- New Feature: Secure Notes & Passwords UX groups
- Bugfix: Teleporting a multisig PSBT file (without signing it first) sent stale data instead of the selected file
- Bugfix: Fix export UX message after teleport PSBT import & sign
- Bugfix: BIP-21 QR `amount` rendered with wrong decimal scaling on the Payment Address screen (e.g. `amount=1.1` was shown as `1.00000001 BTC`)
diff --git a/shared/notes.py b/shared/notes.py
index bbdb74a..bee03c3 100644
--- a/shared/notes.py
+++ b/shared/notes.py
@@ -131,9 +131,7 @@ class NotesMenu(MenuSystem):
else:
wipe_if_deltamode()
- rv = []
- for note in NoteContent.get_all():
- rv.append(MenuItem('%d: %s' % (note.idx+1, note.title), menu=note.make_menu))
+ rv = cls.construct_note_items(readonly=False)
rv.extend(news)
@@ -151,16 +149,34 @@ class NotesMenu(MenuSystem):
# When only allowed to view, no export/add new/delete.
wipe_if_deltamode()
- rv = []
- for note in NoteContent.get_all():
- rv.append(MenuItem('%d: %s' % (note.idx+1, note.title),
- menu=note.make_menu, arg=True)) # readonly=True
+ rv = cls.construct_note_items(readonly=True)
if not rv:
rv.append(MenuItem('(none saved yet)'))
return rv
+ @classmethod
+ def construct_note_items(cls, readonly=False):
+ rv = []
+ by_group = {}
+
+ for note in NoteContent.get_all():
+ item = MenuItem('%d: %s' % (note.idx+1, note.title),
+ menu=note.make_menu, arg=readonly)
+ group = note.group
+ if group:
+ if group not in by_group:
+ by_group[group] = []
+ by_group[group].append(item)
+ else:
+ rv.append(item)
+
+ for group in sorted(by_group):
+ rv.append(MenuItem('↳ ' + group, menu=NoteGroupMenu(group, readonly)))
+
+ return rv
+
@classmethod
async def export_all(cls, *a):
await start_export(NoteContent.get_all())
@@ -231,10 +247,28 @@ class NotesMenu(MenuSystem):
@classmethod
async def drill_to(cls, menu, item):
# make it so looks like we drilled down into the new note
- menu.goto_idx(item.idx)
+ label = '%d: %s' % (item.idx+1, item.title)
+ group = item.group
+ if group:
+ cls.goto_exact_label(menu, '↳ ' + group)
+ gm = NoteGroupMenu(group)
+ cls.goto_exact_label(gm, label)
+ the_ux.push(gm)
+ else:
+ cls.goto_exact_label(menu, label)
+
m = await item._make_menu()
the_ux.push(MenuSystem(m))
+ @staticmethod
+ def goto_exact_label(menu, label):
+ for i, mi in enumerate(menu.items):
+ if mi.label == label:
+ menu.goto_idx(i)
+ return True
+
+ return False
+
class NoteContentBase:
def __init__(self, json={}, idx=-1):
@@ -250,9 +284,15 @@ class NoteContentBase:
return PasswordContent(j, idx) if 'user' in j else NoteContent(j, idx)
def serialize(self):
- return {fld:getattr(self, fld, '') for fld in self.flds}
+ res = {}
+ for fld in self.flds:
+ val = getattr(self, fld, '')
+ # user field is necessary for proper password identification in constructor
+ if not val and (fld != "user"):
+ continue
+ res[fld] = val
- to_json = serialize
+ return res
@classmethod
def get_all(cls):
@@ -278,6 +318,15 @@ class NoteContentBase:
settings.put('notes', [n.serialize() for n in notes])
settings.save()
+ @classmethod
+ def get_groups(cls):
+ groups = set()
+ for note in cls.get_all():
+ if note.group:
+ groups.add(note.group)
+
+ return sorted(groups)
+
async def delete(self, *a):
# Remove note
ok = await ux_confirm("Everything about this note/password will be lost.")
@@ -298,6 +347,11 @@ class NoteContentBase:
the_ux.pop()
m = the_ux.top_of_stack()
m.update_contents()
+ parent = the_ux.parent_of(m)
+ if parent:
+ parent.update_contents()
+ if isinstance(m, NoteGroupMenu) and not m.has_notes():
+ the_ux.pop()
await ux_dramatic_pause('Deleted.', 3)
@@ -335,6 +389,11 @@ class NoteContentBase:
# update parent
parent = the_ux.parent_of(menu)
parent.update_contents()
+ grandparent = the_ux.parent_of(parent)
+ if grandparent:
+ grandparent.update_contents()
+ if isinstance(parent, NoteGroupMenu) and not parent.has_notes():
+ the_ux.stack.remove(parent)
else:
menu.update_contents()
@@ -368,9 +427,73 @@ class NoteContentBase:
predicate=2 <= len(self.misc) <= MSG_SIGNING_MAX_LENGTH)
+class NoteGroupMenu(MenuSystem):
+ def __init__(self, group, readonly=False):
+ self.group = group
+ self.readonly = readonly
+ super().__init__(self.construct())
+
+ def construct(self):
+ items = []
+ for note in NoteContent.get_all():
+ if note.group == self.group:
+ items.append(MenuItem('%d: %s' % (note.idx+1, note.title),
+ menu=note.make_menu, arg=self.readonly))
+
+ return items or [MenuItem('(none)')]
+
+ def has_notes(self):
+ return any(note.group == self.group for note in NoteContent.get_all())
+
+ def update_contents(self):
+ self.replace_items(self.construct())
+
+
+class GroupPickerMenu(MenuSystem):
+ def __init__(self, current=''):
+ self.result = None
+ self.current = current
+
+ groups = NoteContentBase.get_groups()
+ chosen = 0
+ items = [MenuItem('(none)', f=self.picked, arg='')]
+
+ for group in groups:
+ if group == self.current:
+ chosen = len(items)
+ items.append(MenuItem(group, f=self.picked, arg=group))
+
+ items.append(MenuItem('New Group', f=self.new_group))
+
+ super().__init__(items, chosen=chosen)
+
+ async def picked(self, menu, idx, mi):
+ assert menu == self
+ self.result = mi.arg
+ the_ux.pop()
+
+ async def new_group(self, menu, idx, mi):
+ group = await ux_input_text('', max_len=ONE_LINE, confirm_exit=False,
+ prompt='Group', placeholder='(optional)')
+ if group is None:
+ self.result = None
+ else:
+ self.result = group
+
+ the_ux.pop()
+
+ @classmethod
+ async def pick(cls, current=''):
+ m = cls(current)
+ the_ux.push(m)
+ await m.interact()
+
+ return current if m.result is None else m.result
+
+
class PasswordContent(NoteContentBase):
# "Passwords" have a few more fields and are more structured
- flds = ['title', 'user', 'password', 'site', 'misc' ]
+ flds = ['title', 'user', 'password', 'site', 'misc', 'group']
type_label = 'password'
async def _make_menu(self, readonly=False):
@@ -483,6 +606,8 @@ class PasswordContent(NoteContentBase):
if misc is None:
misc = self.misc
+ group = await GroupPickerMenu.pick(self.group)
+
if self.idx != -1:
# confirm changes, don't for new records
chgs = []
@@ -494,6 +619,8 @@ class PasswordContent(NoteContentBase):
chgs.append('Username')
if self.misc != misc:
chgs.append('Other Notes')
+ if self.group != group:
+ chgs.append('Group')
if not chgs:
await ux_dramatic_pause('No changes.', 3)
@@ -507,6 +634,7 @@ class PasswordContent(NoteContentBase):
self.user = user
self.site = site
self.misc = misc
+ self.group = group
await self._save_ux(menu)
return self
@@ -514,7 +642,7 @@ class PasswordContent(NoteContentBase):
class NoteContent(NoteContentBase):
# Pure "notes" have just a title and free-form text
- flds = ['title', 'misc']
+ flds = ['title', 'misc', 'group']
type_label = 'note'
async def _make_menu(self, readonly=False):
@@ -560,6 +688,8 @@ class NoteContent(NoteContentBase):
if misc is None:
misc = self.misc
+ group = await GroupPickerMenu.pick(self.group)
+
if self.idx != -1:
# confirm changes, don't for new records
chgs = []
@@ -567,6 +697,8 @@ class NoteContent(NoteContentBase):
chgs.append('Title')
if self.misc != misc:
chgs.append('Note Text')
+ if self.group != group:
+ chgs.append('Group')
if not chgs:
await ux_dramatic_pause('No changes.', 3)
@@ -579,6 +711,7 @@ class NoteContent(NoteContentBase):
self.title = title
self.misc = misc
+ self.group = group
await self._save_ux(menu)
diff --git a/testing/test_notes.py b/testing/test_notes.py
index 6eb01e7..3fd5c47 100644
--- a/testing/test_notes.py
+++ b/testing/test_notes.py
@@ -98,7 +98,7 @@ def build_note(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story,
need_keypress, cap_screen_qr, readback_bbqr, nfc_read_text,
press_select, press_cancel, is_headless, nfc_disabled):
- def doit(n_title, n_body):
+ def doit(n_title, n_body, group=None):
# we don't try to preserve leading/trailing spaces on note bodies
n_body= n_body.strip()
@@ -107,6 +107,11 @@ def build_note(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story,
# create
enter_text(n_title)
enter_text(n_body, multiline=True)
+ if group:
+ pick_menu_item('New Group')
+ enter_text(group)
+ else:
+ pick_menu_item('(none)')
# view
time.sleep(0.1)
@@ -172,6 +177,10 @@ def build_note(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story,
obj = obj[0]
assert obj['title'] == n_title
assert obj['misc'] == n_body
+ if group:
+ assert obj['group'] == group
+ else:
+ assert 'group' not in obj
# back to top notes menu
press_select()
@@ -185,7 +194,8 @@ def build_password(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story,
cap_text_box, settings_get, settings_set, scan_a_qr,
press_select, press_cancel, is_headless):
- def doit(n_title, n_user=None, n_pw='secret', n_site=None, n_body=None, key_pw=None):
+ def doit(n_title, n_user=None, n_pw='secret', n_site=None, n_body=None,
+ key_pw=None, group=None):
goto_notes('New Password')
enter_text(n_title)
if n_user:
@@ -221,6 +231,11 @@ def build_password(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story,
enter_text(n_body, multiline=True)
else:
press_cancel()
+ if group:
+ pick_menu_item('New Group')
+ enter_text(group)
+ else:
+ pick_menu_item('(none)')
# view
time.sleep(0.1)
@@ -282,7 +297,7 @@ def change_password(goto_notes, pick_menu_item, enter_text, cap_story,
cap_menu):
def doit(id_title, new_title=None, new_username=None, new_site=None,
- new_misc=None, new_password=None, old_password=None):
+ new_misc=None, new_password=None, new_group=None):
goto_notes()
m = cap_menu()
found = [i for i in m if f': {id_title}' in i]
@@ -313,6 +328,18 @@ def change_password(goto_notes, pick_menu_item, enter_text, cap_story,
need_in_story.append('Other Notes')
else:
press_cancel()
+ if new_group is not None:
+ if new_group:
+ if new_group in cap_menu():
+ pick_menu_item(new_group)
+ else:
+ pick_menu_item('New Group')
+ enter_text(new_group)
+ else:
+ pick_menu_item('(none)')
+ need_in_story.append('Group')
+ else:
+ press_cancel()
# approve change
time.sleep(0.1)
@@ -349,6 +376,8 @@ def change_password(goto_notes, pick_menu_item, enter_text, cap_story,
assert note['misc'] == new_misc
if new_password:
assert note['password'] == new_password
+ if new_group is not None:
+ assert note.get('group', '') == new_group
return doit
@@ -363,7 +392,7 @@ def test_build_note(n_title, n_body, build_note, delete_note):
@pytest.mark.parametrize('size', [ 4000, 30000])
@pytest.mark.parametrize('encoding', '2Z' )
def test_huge_notes(size, encoding, goto_notes, enter_text, cap_menu, need_keypress,
- scan_a_qr, settings_set, settings_get):
+ scan_a_qr, settings_set, settings_get, pick_menu_item):
# Since we don't limit note sizes, by request of NVK ... test them
@@ -387,6 +416,7 @@ def test_huge_notes(size, encoding, goto_notes, enter_text, cap_menu, need_keypr
time.sleep(2.0 / len(parts)) # just so we can watch
time.sleep(.5) # decompression time in some cases
+ pick_menu_item('(none)')
m = cap_menu()
assert 'Export' in m
@@ -479,6 +509,168 @@ def test_sort_by_title(goto_notes, pick_menu_item, cap_story, need_keypress, set
assert sorted((i['title'] for i in after), key=lambda i:i.lower()) \
== [i['title'] for i in after]
+
+def test_grouped_note_menu(settings_set, settings_get, goto_notes, cap_menu,
+ pick_menu_item, build_note, press_cancel, press_select):
+ settings_set('notes', [])
+ settings_set('secnap', True)
+
+ build_note('group-note', 'body', group='Work')
+ notes = settings_get('notes')
+ assert notes[-1]['group'] == 'Work'
+
+ goto_notes()
+ m = cap_menu()
+ assert '↳ Work' in m
+ assert not any(': group-note' in i for i in m)
+
+ press_select()
+ m = cap_menu()
+ assert '1: group-note' in m
+ press_cancel()
+
+
+def test_grouped_password_menu(settings_set, settings_get, goto_notes, cap_menu,
+ pick_menu_item, build_password, press_cancel, press_select):
+ settings_set('notes', [])
+ settings_set('secnap', True)
+
+ build_password('group-pw', n_pw='secret', group='Accounts')
+ press_cancel()
+ notes = settings_get('notes')
+ assert notes[-1]['group'] == 'Accounts'
+
+ goto_notes()
+ m = cap_menu()
+ assert '↳ Accounts' in m
+ assert not any(': group-pw' in i for i in m)
+
+ press_select()
+ m = cap_menu()
+ assert '1: group-pw' in m
+ press_cancel()
+
+
+def test_grouped_and_ungrouped_menu(settings_set, goto_notes, cap_menu,
+ pick_menu_item, press_cancel):
+ settings_set('secnap', True)
+ settings_set('notes', [
+ {'title': 'loose-note', 'misc': 'aaa'},
+ {'title': 'work-note', 'misc': 'bbb', 'group': 'Work'},
+ {'title': 'work-pw', 'misc': '', 'password': 'secret', 'site': '',
+ 'user': '', 'group': 'Work'},
+ ])
+
+ goto_notes()
+ m = cap_menu()
+ assert '1: loose-note' in m
+ assert '↳ Work' in m
+ assert not any(': work-note' in i for i in m)
+ assert not any(': work-pw' in i for i in m)
+
+ pick_menu_item('↳ Work')
+ m = cap_menu()
+ assert '2: work-note' in m
+ assert '3: work-pw' in m
+ press_cancel()
+
+
+def test_new_grouped_note_cancel_lands_in_group(settings_set, goto_notes, cap_menu,
+ pick_menu_item, enter_text, press_cancel):
+ settings_set('notes', [])
+ settings_set('secnap', True)
+
+ goto_notes('New Note')
+ enter_text('new-note')
+ enter_text('body', multiline=True)
+ pick_menu_item('New Group')
+ enter_text('Work')
+
+ assert '"new-note"' in cap_menu()
+
+ press_cancel()
+ m = cap_menu()
+ assert '1: new-note' in m
+ assert 'New Note' not in m
+
+ press_cancel()
+ m = cap_menu()
+ assert '↳ Work' in m
+ assert '1: new-note' not in m
+
+
+def test_edit_note_group_moves(settings_set, settings_get, goto_notes, cap_menu,
+ pick_menu_item, enter_text, press_select,
+ press_cancel, cap_story):
+
+ settings_set('secnap', True)
+ settings_set('notes', [{'title': 'move-note', 'misc': 'body'}])
+
+ goto_notes()
+ pick_menu_item('1: move-note')
+ pick_menu_item('Edit')
+ press_select() # unchanged title
+ press_cancel() # unchanged note body
+ pick_menu_item('New Group')
+ enter_text('Work')
+ time.sleep(.1)
+ title, story = cap_story()
+ assert "SURE" in title
+ assert 'Group' in story
+ press_select()
+ time.sleep(.1)
+
+ goto_notes()
+ m = cap_menu()
+ assert '↳ Work' in m
+ assert '1: move-note' not in m
+
+ press_select()
+ pick_menu_item('1: move-note')
+ pick_menu_item('Edit')
+ press_select()
+ press_cancel()
+ pick_menu_item('New Group')
+ enter_text('Home')
+ time.sleep(.1)
+ title, story = cap_story()
+ assert 'SURE' in title
+ assert 'Group' in story
+ press_select()
+
+ goto_notes()
+ m = cap_menu()
+ assert '↳ Work' not in m
+ assert '↳ Home' in m
+
+ press_select()
+ pick_menu_item('1: move-note')
+ pick_menu_item('Edit')
+ press_select()
+ press_cancel()
+ pick_menu_item('(none)')
+ time.sleep(.1)
+ title, story = cap_story()
+ assert 'SURE' in title
+ assert 'Group' in story
+ press_select()
+
+ press_cancel()
+ m = cap_menu()
+ assert '↳ Home' not in m
+ assert '1: move-note' in m
+ assert '(none)' not in m
+
+
+def test_old_records_without_group(settings_set, settings_get, goto_notes, cap_menu):
+ settings_set('secnap', True)
+ settings_set('notes', [{'title': 'old-note', 'misc': 'body'}])
+
+ goto_notes()
+ assert '1: old-note' in cap_menu()
+ assert settings_get('notes')[0].get('group', '') == ''
+
+
def test_top_import(goto_notes, cap_menu, cap_story, need_keypress, settings_get,
settings_set, scan_a_qr, need_some_notes):
# make some
@@ -705,7 +897,7 @@ def test_send_password_menu_item(need_some_passwords, goto_notes, cap_menu, pick
settings_set('du', 1)
goto_notes()
- pick_menu_item('1: A')
+ pick_menu_item([i for i in cap_menu() if i.endswith(': A')][0])
time.sleep(.2)
m = cap_menu()
assert 'Send Password' not in m
@@ -713,7 +905,7 @@ def test_send_password_menu_item(need_some_passwords, goto_notes, cap_menu, pick
settings_set('du', 0)
goto_notes()
- pick_menu_item('1: A')
+ pick_menu_item([i for i in cap_menu() if i.endswith(': A')][0])
time.sleep(.2)
m = cap_menu()
assert 'Send Password' in m
@@ -738,6 +930,7 @@ def test_password_cancel_stores_empty_not_none(goto_notes, need_keypress, press_
press_cancel() # cancel password field - bug, stores None
press_select() # skip site
press_cancel() # exit misc
+ pick_menu_item('(none)') # no group
time.sleep(0.2)
Why this scored 15/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.