diff --git a/CHANGELOG.md b/CHANGELOG.md index d245750..af4295e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,18 +5,28 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.3.3] - 2026-06-18 +## [1.3.3] - 2026-06-19 ### Fix -- WUI: saving a backup no longer fails with a `SecurityError` — the save dialog now opens on click, - before the on-device approval. +- Restore no longer corrupts password lists larger than ~255 bytes. - Demo passwords from the developer `POPULATE` build are seeded only once (on first storage initialization). +- Use correct `printf` format specifiers for `size_t` values (CodeQL). ### Change - Rebuild the backup Web UI on Vite + React 18 with Ledger's lumen design system; built with pnpm on Node 22. - Publish the Web UI through GitHub's native Pages workflow. +- Reword the device confirmation prompts for the Backup/Restore actions. +- Web UI: save the backup through the browser's native save dialog +- Web UI: drop the (constant) storage-size row from the device card. +- Only compile the test APDU handler when built with `TESTING=1`, so production builds contain no test code. + +### Add + +- Unit tests for the metadata write offset and capacity limits +- ragger tests for the backup/restore confirmation screens. +- ragger test checking that showing a password located beyond the first list page selects the right entry on touch devices. ## [1.3.2] - 2026-06-05 diff --git a/clients/wui/README.md b/clients/wui/README.md index 014251f..b90c405 100644 --- a/clients/wui/README.md +++ b/clients/wui/README.md @@ -37,9 +37,9 @@ For more information on the device application itself, see the - Click **Connect** — on success the Backup / Restore actions appear. (Connection trouble? See [Fix connection issues](https://support.ledger.com/article/115005165269-zd).) -- **Backup** prompts "Transfer metadatas?" on the device, then saves the file. -- **Restore** asks for a previous backup file, then prompts "Overwrite - metadatas?" on the device. +- **Backup** prompts "Backup password list?" on the device, then saves the file. +- **Restore** asks for a previous backup file, then prompts "Restore password + list?" on the device. ## Develop diff --git a/clients/wui/src/App.jsx b/clients/wui/src/App.jsx index d7f9007..0e8fab2 100644 --- a/clients/wui/src/App.jsx +++ b/clients/wui/src/App.jsx @@ -26,50 +26,48 @@ import packageJson from "../package.json"; const passwords = new PasswordsManager(); listen((log) => console.log(log)); -// showSaveFilePicker() requires transient user activation (a recent gesture), -// so it must be called synchronously from the click handler — before the -// device exchange, which takes too long and would otherwise let the activation -// expire ("Must be handling a user gesture"). This picks the destination up -// front and returns a target the caller writes to once the data is ready. -// Falls back to a Blob/anchor download when the File System Access API is -// unavailable or refuses (returns { cancelled } if the user dismisses it). -async function pickSaveTarget(suggestedName) { +// Save the backup, preferring the native "Save As" dialog so the user can +// choose the file name/location. showSaveFilePicker requires transient user +// activation, so this MUST be called from its own click handler — not from the +// async continuation of the device exchange (the original activation would have +// expired -> "Must be handling a user gesture"). Hence the two-step flow: +// backup first, then a separate "Save" click. Returns false if the user +// cancels the dialog (so the caller can keep the data for a retry). Falls back +// to a plain anchor download where the File System Access API is unavailable. +async function saveBackupFile(payload, filename) { + const text = JSON.stringify(payload, null, 4); + if (window.showSaveFilePicker) { + let handle = null; try { - const handle = await window.showSaveFilePicker({ - suggestedName, + handle = await window.showSaveFilePicker({ + suggestedName: filename, types: [ { description: "Passwords backup", accept: { "application/json": [".json"] } }, ], }); - return { handle, suggestedName }; } catch (error) { - if (error.name === "AbortError") return { cancelled: true }; - // SecurityError / NotAllowedError / unsupported: fall back to anchor. + if (error.name === "AbortError") return false; // user dismissed the dialog + // Other errors (unsupported / SecurityError): fall through to the anchor. + } + if (handle) { + const writable = await handle.createWritable(); + await writable.write(text); + await writable.close(); + return true; } - } - return { suggestedName }; -} - -async function writeJSON(target, payload) { - const text = JSON.stringify(payload, null, 4); - - if (target.handle) { - const writable = await target.handle.createWritable(); - await writable.write(text); - await writable.close(); - return; } const blob = new Blob([text], { type: "application/json;charset=utf-8" }); const url = URL.createObjectURL(blob); const a = document.createElement("a"); a.href = url; - a.download = target.suggestedName; + a.download = filename; document.body.appendChild(a); a.click(); document.body.removeChild(a); URL.revokeObjectURL(url); + return true; } export default function App() { @@ -80,8 +78,11 @@ export default function App() { const [busy, setBusy] = useState(false); const [connected, setConnected] = useState(mock); const [version, setVersion] = useState(mock ? "1.0.0" : null); - const [storageSize, setStorageSize] = useState(mock ? 4096 : null); const [notice, setNotice] = useState(null); + // Backup data read from the card, waiting for the user to pick a file. The + // save dialog needs its own click (transient activation), so it can't be + // chained onto the device exchange. + const [pendingBackup, setPendingBackup] = useState(null); const fileInput = useRef(null); @@ -90,7 +91,6 @@ export default function App() { setConnected(false); setBusy(false); setVersion(null); - setStorageSize(null); setNotice({ appearance: "warning", title: "Device disconnected", @@ -107,14 +107,12 @@ export default function App() { try { await passwords.connect(); setVersion(passwords.version); - setStorageSize(passwords.storage_size); setConnected(true); setNotice({ appearance: "success", title: "Device connected" }); } catch (error) { await passwords.disconnect(); setConnected(false); setVersion(null); - setStorageSize(null); setNotice({ appearance: "error", title: "Connection failed", description: String(error) }); } finally { setBusy(false); @@ -125,25 +123,24 @@ export default function App() { await passwords.disconnect(); setConnected(false); setVersion(null); - setStorageSize(null); setNotice({ appearance: "info", title: "Disconnected" }); } async function onBackup() { - // Pick the destination first, while the click's user activation is still - // valid (the device exchange below is too slow for showSaveFilePicker). - const target = await pickSaveTarget("passwords_backup.json"); - if (target.cancelled) { - setNotice({ appearance: "info", title: "Backup cancelled" }); - return; - } - setBusy(true); - setNotice({ appearance: "info", title: 'Approve "Transfer metadatas?" on your device' }); + setPendingBackup(null); + setNotice({ appearance: "info", title: 'Approve "Backup password list?" on your device' }); try { + // Read the card first. The file dialog can't be opened here (the device + // approval consumed the click's activation), so hold the data and let the + // user trigger the save with a fresh click. const payload = await passwords.dump_metadatas(); - await writeJSON(target, payload); - setNotice({ appearance: "success", title: "Backup saved" }); + setPendingBackup(payload); + setNotice({ + appearance: "success", + title: "Backup ready", + description: "Click Save to choose where to store the file.", + }); } catch (error) { setNotice({ appearance: "error", title: "Backup failed", description: String(error) }); } finally { @@ -151,6 +148,19 @@ export default function App() { } } + async function onSaveBackup() { + try { + const saved = await saveBackupFile(pendingBackup, "passwords_backup.json"); + if (saved) { + setPendingBackup(null); + setNotice({ appearance: "success", title: "Backup saved" }); + } + // If cancelled, keep pendingBackup so the user can try saving again. + } catch (error) { + setNotice({ appearance: "error", title: "Save failed", description: String(error) }); + } + } + function onPickRestoreFile(event) { const file = event.target.files[0]; event.target.value = ""; @@ -158,7 +168,7 @@ export default function App() { const reader = new FileReader(); reader.onload = async () => { setBusy(true); - setNotice({ appearance: "info", title: 'Approve "Overwrite metadatas?" on your device' }); + setNotice({ appearance: "info", title: 'Approve "Restore password list?" on your device' }); try { await passwords.load_metadatas(reader.result); setNotice({ appearance: "success", title: "Restore complete" }); @@ -196,6 +206,20 @@ export default function App() { /> )} + {/* Backup ready: the save dialog needs its own click. */} + {pendingBackup && ( + + Save… + + } + /> + )} + {/* Device card */}
@@ -222,12 +246,6 @@ export default function App() { Version
-
- Storage - - {storageSize ? `${storageSize} bytes` : "—"} - -
)} diff --git a/clients/wui/src/components/Faq.jsx b/clients/wui/src/components/Faq.jsx index fd49f1d..555483b 100644 --- a/clients/wui/src/components/Faq.jsx +++ b/clients/wui/src/components/Faq.jsx @@ -22,8 +22,8 @@ const ITEMS = [ a: "* Connect your Ledger device to your computer and open the `Passwords app`.\n" + "* Click on the big `Connect` button — on success the `Backup` and `Restore` buttons appear. If you have trouble with this step, have a look [here](https://support.ledger.com/article/115005165269-zd).\n" + - "* `Backup` will prompt a screen requesting your approval on your device (\"Transfer metadatas?\"), then save a backup file. This backup is not confidential, so you can for instance e-mail it to yourself to never lose it.\n" + - "* `Restore` will prompt a file input dialog where you should indicate a previous backup file. A prompt (\"Overwrite metadatas?\") will then request your approval on your device.", + "* `Backup` will prompt a screen requesting your approval on your device (\"Backup password list?\"), then save a backup file. This backup is not confidential, so you can for instance e-mail it to yourself to never lose it.\n" + + "* `Restore` will prompt a file input dialog where you should indicate a previous backup file. A prompt (\"Restore password list?\") will then request your approval on your device.", }, { q: "Which web browsers and operating systems are supported?", diff --git a/src/apdu_handlers/dump_metadatas.c b/src/apdu_handlers/dump_metadatas.c index 26cc60f..1fb1c16 100644 --- a/src/apdu_handlers/dump_metadatas.c +++ b/src/apdu_handlers/dump_metadatas.c @@ -7,7 +7,11 @@ int dump_metadatas() { if (app_state.user_approval == false) { app_state.bytes_transferred = 0; - message_pair_t msg = {"Transfer", "metadatas ?"}; +#ifdef SCREEN_SIZE_WALLET + message_pair_t msg = {"Backup", "password list?"}; +#else + message_pair_t msg = {"Backup", "password list"}; +#endif ui_request_user_approval(&msg); return 0; } diff --git a/src/apdu_handlers/load_metadatas.c b/src/apdu_handlers/load_metadatas.c index 2c6dc25..2ba37dd 100644 --- a/src/apdu_handlers/load_metadatas.c +++ b/src/apdu_handlers/load_metadatas.c @@ -12,7 +12,11 @@ int load_metadatas(uint8_t p1, uint8_t p2, const buf_t *input) { } if (app_state.user_approval == false) { app_state.bytes_transferred = 0; - message_pair_t msg = {"Overwrite", "metadatas ?"}; +#ifdef SCREEN_SIZE_WALLET + message_pair_t msg = {"Restore", "password list?"}; +#else + message_pair_t msg = {"Restore", "password list"}; +#endif ui_request_user_approval(&msg); return 0; } diff --git a/src/dispatcher.c b/src/dispatcher.c index ce40790..696f0db 100644 --- a/src/dispatcher.c +++ b/src/dispatcher.c @@ -23,7 +23,9 @@ #include "dispatcher.h" #include "error.h" #include "globals.h" +#ifdef TESTING #include "tests.h" +#endif #include "types.h" int dispatch() { diff --git a/src/metadata.c b/src/metadata.c index ff7b804..b9c51fe 100644 --- a/src/metadata.c +++ b/src/metadata.c @@ -28,7 +28,7 @@ error_type_t write_metadata(uint8_t *data, uint8_t dataSize) { return OK; } -void override_metadatas(uint8_t offset, void *ptr, size_t size) { +void override_metadatas(size_t offset, void *ptr, size_t size) { nvm_write((void *) &N_storage.metadatas[offset], ptr, size); } diff --git a/src/metadata.h b/src/metadata.h index a128e8c..9e8b6d7 100644 --- a/src/metadata.h +++ b/src/metadata.h @@ -24,7 +24,7 @@ error_type_t write_metadata(uint8_t *data, uint8_t dataSize); * Write a given amount of data on metadatas, at the given offset * Used to load metadata from APDUs */ -void override_metadatas(uint8_t offset, void *ptr, size_t size); +void override_metadatas(size_t offset, void *ptr, size_t size); void reset_metadatas(void); error_type_t erase_metadata(uint32_t offset); diff --git a/src/tests/tests.c b/src/tests/tests.c index e2446fc..ca29ed0 100644 --- a/src/tests/tests.c +++ b/src/tests/tests.c @@ -4,6 +4,10 @@ #include "password_typing.h" #include "tests.h" +// Test-only APDU handler: compiled into nothing unless built with TESTING=1, +// matching the `#ifdef TESTING` guard on the dispatch site in dispatcher.c. +#ifdef TESTING + /* Takes a metadata as an input (charset + seed) and returns a 20 char password*/ int test_generate_password(const buf_t *input) { uint8_t enabledSets = input->bytes[0]; @@ -37,3 +41,5 @@ int test_dispatcher(uint8_t p1, __attribute__((unused)) uint8_t p2, const buf_t return io_send_sw(SWO_INVALID_INS + 1); } } + +#endif // TESTING diff --git a/tests/functional/nano/navigator.py b/tests/functional/nano/navigator.py index 10c49ec..bfcbc79 100644 --- a/tests/functional/nano/navigator.py +++ b/tests/functional/nano/navigator.py @@ -87,8 +87,11 @@ def __init__(self, backend: BackendInterface, device: Device, golden_run: bool = # paginates before the "Confirm" action page, so step forward until # that page is on screen and only then commit. CustomNavInsID.CONFIRM_YES: self._confirm_yes, - # APDU approval prompt + # Generic select/validate (settings switches, BARS_LIST entries). CustomNavInsID.BUTTON_APPROVE: self._both, + # APDU approval prompt (nbgl_useCaseConfirm): message page first, + # the "Approve" action sits on the next page (right then both). + CustomNavInsID.APDU_APPROVE: self._approve, # Keyboard (nbgl_useCaseKeyboard with MODE_NONE on Nano). CustomNavInsID.KEYBOARD_WRITE: self._write, CustomNavInsID.KEYBOARD_TO_CONFIRM: self._keyboard_confirm, @@ -106,13 +109,20 @@ def _left(self): def _both(self): self._backend.both_click() + def _approve(self): + # nbgl_useCaseConfirm shows the message first; the "Approve" action is + # reached with one right_click, then validated with both_click. Matches + # PasswordsManagerCommand.approve() for Nano. + self._right() + self._both() + def _navigate_until_text(self, text: str, max_steps: int = 12) -> None: """Press right_click until `text` appears on screen, then return.""" for _ in range(max_steps): try: self._backend.wait_for_text_on_screen(text, timeout=0.5) return - except Exception: + except TimeoutError: self._backend.right_click() raise RuntimeError(f"Could not find text '{text}' on screen after {max_steps} steps") diff --git a/tests/functional/passwordsManager_cmd.py b/tests/functional/passwordsManager_cmd.py index 005f75e..fbcfa40 100644 --- a/tests/functional/passwordsManager_cmd.py +++ b/tests/functional/passwordsManager_cmd.py @@ -37,8 +37,19 @@ def __init__(self, self.debug = debug self.approved: bool = False - def approve(self): - if self.device.touchable: + def approve(self, compare=None): + # compare = (snapshots_path, test_case_name) to snapshot-compare the + # on-device approval screen before approving. Works for both device + # families via the BUTTON_APPROVE navigation instruction. + if compare is not None: + snap_path, test_name = compare + self.navigation.navigate_and_compare( + snap_path, + test_name, + [CustomNavInsID.APDU_APPROVE], + screen_change_before_first_instruction=True, + screen_change_after_last_instruction=False) + elif self.device.touchable: self.navigation.navigate([CustomNavInsID.BUTTON_APPROVE]) else: self.transport.right_click() @@ -106,7 +117,7 @@ def generate_password(self, charsets: int, seed: str) -> str: return response.decode("ascii") - def dump_metadatas(self, size) -> bytes: + def dump_metadatas(self, size, compare=None) -> bytes: ins: InsType = InsType.INS_DUMP_METADATAS metadatas = b"" @@ -114,7 +125,7 @@ def dump_metadatas(self, size) -> bytes: while len(metadatas) < size: if not self.approved: with self.transport.exchange_async(cla=CLA, ins=ins): - self.approve() + self.approve(compare) response = self.transport.last_async_response self.approved = True else: @@ -131,14 +142,14 @@ def dump_metadatas(self, size) -> bytes: return metadatas[:size] - def load_metadatas_chunk(self, chunk, is_last): + def load_metadatas_chunk(self, chunk, is_last, compare=None): ins: InsType = InsType.INS_LOAD_METADATAS if not self.approved: with self.transport.exchange_async(cla=CLA, ins=ins, p1=0xFF if is_last else 0x00, data=chunk): - self.approve() + self.approve(compare) response = self.transport.last_async_response self.approved = True else: @@ -151,9 +162,11 @@ def load_metadatas_chunk(self, chunk, is_last): if not sw & 0x9000: raise DeviceException(error_code=sw, ins=ins) - def load_metadatas(self, metadatas): + def load_metadatas(self, metadatas, compare=None): chunks = [metadatas[i:i+255] for i in range(0, len(metadatas), 255)] self.approved = False for i, chunk in enumerate(chunks): - self.load_metadatas_chunk(chunk, i+1 == len(chunks)) + # Only the first chunk shows the approval screen. + self.load_metadatas_chunk(chunk, i+1 == len(chunks), + compare if i == 0 else None) diff --git a/tests/functional/snapshots/apex_p/backup/00000.png b/tests/functional/snapshots/apex_p/backup/00000.png new file mode 100644 index 0000000..714d2e5 Binary files /dev/null and b/tests/functional/snapshots/apex_p/backup/00000.png differ diff --git a/tests/functional/snapshots/apex_p/pwd_on_second_page/00000.png b/tests/functional/snapshots/apex_p/pwd_on_second_page/00000.png new file mode 100644 index 0000000..519ff06 Binary files /dev/null and b/tests/functional/snapshots/apex_p/pwd_on_second_page/00000.png differ diff --git a/tests/functional/snapshots/apex_p/pwd_on_second_page/00001.png b/tests/functional/snapshots/apex_p/pwd_on_second_page/00001.png new file mode 100644 index 0000000..18b3017 Binary files /dev/null and b/tests/functional/snapshots/apex_p/pwd_on_second_page/00001.png differ diff --git a/tests/functional/snapshots/apex_p/restore/00000.png b/tests/functional/snapshots/apex_p/restore/00000.png new file mode 100644 index 0000000..443f76b Binary files /dev/null and b/tests/functional/snapshots/apex_p/restore/00000.png differ diff --git a/tests/functional/snapshots/flex/backup/00000.png b/tests/functional/snapshots/flex/backup/00000.png new file mode 100644 index 0000000..6ebca97 Binary files /dev/null and b/tests/functional/snapshots/flex/backup/00000.png differ diff --git a/tests/functional/snapshots/flex/pwd_on_second_page/00000.png b/tests/functional/snapshots/flex/pwd_on_second_page/00000.png new file mode 100644 index 0000000..db25bcd Binary files /dev/null and b/tests/functional/snapshots/flex/pwd_on_second_page/00000.png differ diff --git a/tests/functional/snapshots/flex/pwd_on_second_page/00001.png b/tests/functional/snapshots/flex/pwd_on_second_page/00001.png new file mode 100644 index 0000000..b969239 Binary files /dev/null and b/tests/functional/snapshots/flex/pwd_on_second_page/00001.png differ diff --git a/tests/functional/snapshots/flex/restore/00000.png b/tests/functional/snapshots/flex/restore/00000.png new file mode 100644 index 0000000..e90c5db Binary files /dev/null and b/tests/functional/snapshots/flex/restore/00000.png differ diff --git a/tests/functional/snapshots/nanox/backup/00000.png b/tests/functional/snapshots/nanox/backup/00000.png new file mode 100644 index 0000000..0e7d007 Binary files /dev/null and b/tests/functional/snapshots/nanox/backup/00000.png differ diff --git a/tests/functional/snapshots/nanox/restore/00000.png b/tests/functional/snapshots/nanox/restore/00000.png new file mode 100644 index 0000000..623773a Binary files /dev/null and b/tests/functional/snapshots/nanox/restore/00000.png differ diff --git a/tests/functional/snapshots/stax/backup/00000.png b/tests/functional/snapshots/stax/backup/00000.png new file mode 100644 index 0000000..8698324 Binary files /dev/null and b/tests/functional/snapshots/stax/backup/00000.png differ diff --git a/tests/functional/snapshots/stax/pwd_on_second_page/00000.png b/tests/functional/snapshots/stax/pwd_on_second_page/00000.png new file mode 100644 index 0000000..c45a57a Binary files /dev/null and b/tests/functional/snapshots/stax/pwd_on_second_page/00000.png differ diff --git a/tests/functional/snapshots/stax/pwd_on_second_page/00001.png b/tests/functional/snapshots/stax/pwd_on_second_page/00001.png new file mode 100644 index 0000000..272751a Binary files /dev/null and b/tests/functional/snapshots/stax/pwd_on_second_page/00001.png differ diff --git a/tests/functional/snapshots/stax/restore/00000.png b/tests/functional/snapshots/stax/restore/00000.png new file mode 100644 index 0000000..d282eb9 Binary files /dev/null and b/tests/functional/snapshots/stax/restore/00000.png differ diff --git a/tests/functional/test_passwords.py b/tests/functional/test_passwords.py index b7f2301..ab8b6a3 100644 --- a/tests/functional/test_passwords.py +++ b/tests/functional/test_passwords.py @@ -1,9 +1,28 @@ from pathlib import Path +from time import sleep +import pytest +from ragger.backend import BackendInterface from ragger.navigator import Navigator, NavIns, NavInsID +from passwordsManager_cmd import PasswordsManagerCommand from touch.navigator import CustomNavInsID +# SETS byte and entry layout reused from EXISTING_METADATA in tests_vectors.py: +# [DATALEN=0x0a][KIND=0x00][SETS=0x07][9-char nickname]. The SETS byte also +# selects the charsets used by the password derivation, so the expected value +# of an entry can be recomputed independently with generate_password(SETS, name). +_SETS = 0x07 + + +def _metadata_entry(name: str) -> bytes: + assert len(name) == 9, "nickname must be 9 chars to match the fixed DATALEN" + return bytes([0x0a, 0x00, _SETS]) + name.encode() + + +def _screen_texts(backend: BackendInterface) -> list: + return [event["text"] for event in backend.get_current_screen_content()["events"]] + def test_delete_one_password(navigator: Navigator, default_screenshot_path: Path): instructions = [ @@ -64,3 +83,67 @@ def test_create_password(navigator: Navigator, default_screenshot_path: Path): "create_password", instructions, screen_change_before_first_instruction=False) + + +@pytest.mark.skip_nano # Nano lists one choice per page; this targets the touch pagination +def test_show_password_on_second_page(cmd: PasswordsManagerCommand, + navigator: Navigator, + custom_backend: BackendInterface, + default_screenshot_path: Path): + """Showing a password located on page >= 2 of the list must reveal that + exact entry's value. + + On wallet devices the choices list is paginated and NBGL forwards a + page-relative index, so password_callback() rebuilds the absolute index + from the page number (page * nbPasswordsPerPage + index). This test guards + against an off-by-page regression in that recalculation: it seeds enough + passwords to span at least two pages, selects the first entry of page 2, + and asserts the shown value matches the value derived independently for + that nickname. + """ + # Seed enough 9-char nicknames to guarantee more than one page on every + # wallet screen size (Flex shows 4 per page, larger screens a few more). + names = [f"pwd{i:06d}" for i in range(12)] + metadata = b"".join(_metadata_entry(name) for name in names) + + navigator.navigate([ + CustomNavInsID.DISCLAIMER_CONFIRM, + CustomNavInsID.CHOOSE_KBL_QWERTY, + ], screen_change_before_first_instruction=False) + cmd.load_metadatas(metadata) + + navigator.navigate([ + CustomNavInsID.HOME_TO_MENU, + CustomNavInsID.MENU_TO_DISPLAY, + ], screen_change_before_first_instruction=False) + sleep(0.5) + + # Derive the per-page count from page 1 (the title is not a list entry). + page1 = [t for t in _screen_texts(custom_backend) if t in names] + nb_per_page = len(page1) + assert nb_per_page >= 1, "no password entry rendered on the first page" + assert nb_per_page < len(names), "all passwords fit on a single page; seed more" + assert page1 == names[:nb_per_page], "first page must list the entries in order" + + # The first (top) entry of page 2 is the absolute index `nb_per_page`. + expected_name = names[nb_per_page] + expected_value = cmd.generate_password(_SETS, expected_name) + + navigator.navigate([CustomNavInsID.SETTINGS_NEXT], + screen_change_before_first_instruction=False) + sleep(0.5) + assert expected_name in _screen_texts(custom_backend), \ + "page 2 must start with the entry following the last entry of page 1" + + navigator.navigate_and_compare(default_screenshot_path, + "pwd_on_second_page", + [NavIns(CustomNavInsID.LIST_CHOOSE, (1, ))], + screen_change_before_first_instruction=False) + sleep(1.0) + + # The "Your Password" screen shows the nickname and its derived value; both + # must correspond to the page-2 entry, proving the absolute index is right. + shown = _screen_texts(custom_backend) + assert expected_name in shown + assert expected_value in shown, \ + f"expected value of '{expected_name}' not shown; wrong entry selected" diff --git a/tests/functional/test_render.py b/tests/functional/test_render.py new file mode 100644 index 0000000..c207e57 --- /dev/null +++ b/tests/functional/test_render.py @@ -0,0 +1,19 @@ +from pathlib import Path + +from passwordsManager_cmd import PasswordsManagerCommand + + +def test_backup_render(cmd: PasswordsManagerCommand, default_screenshot_path: Path): + # Triggering a backup makes the device show "Backup password list?". + # Capture that approval screen, then approve. + cmd.dump_metadatas(1, compare=(default_screenshot_path, "backup")) + cmd.reset_approval_state() + + +def test_restore_render(cmd: PasswordsManagerCommand, default_screenshot_path: Path): + # Triggering a restore makes the device show "Restore password list?". + # Capture that approval screen, then approve. The payload is a small valid + # metadata blob (one entry), shared with the load test vectors. + metadatas = bytes.fromhex("02000761060007616c6c6168") + cmd.load_metadatas(metadatas, compare=(default_screenshot_path, "restore")) + cmd.reset_approval_state() diff --git a/tests/functional/touch/navigator.py b/tests/functional/touch/navigator.py index d94beb8..dc26993 100644 --- a/tests/functional/touch/navigator.py +++ b/tests/functional/touch/navigator.py @@ -1,3 +1,7 @@ +# CustomTouchScreen builds its members (home, settings, button, ...) through the +# ragger MetaScreen metaclass, so pylint cannot see them statically and reports +# false no-member errors on every self.screen.* access. +# pylint: disable=no-member from enum import auto from functools import partial from time import sleep @@ -34,8 +38,10 @@ class CustomNavInsID(BaseNavInsID): CHOOSE_KBL_QWERTY = auto() # startup disclaimer DISCLAIMER_CONFIRM = auto() - # Approve metadatas + # Generic "select/validate" tap (settings switches, BARS_LIST entries, ...) BUTTON_APPROVE = auto() + # Approve an APDU confirmation prompt (nbgl_useCaseConfirm) + APDU_APPROVE = auto() class CustomTouchNavigator(Navigator): @@ -63,6 +69,7 @@ def __init__(self, backend, device, golden_run): CustomNavInsID.CHOOSE_KBL_QWERTY: partial(self._choose, 1), CustomNavInsID.DISCLAIMER_CONFIRM: self.screen.disclaimer.confirm, CustomNavInsID.BUTTON_APPROVE: self.screen.button.tap, + CustomNavInsID.APDU_APPROVE: self.screen.button.tap, } super().__init__(backend, device, callbacks, golden_run) diff --git a/tests/unit/test_password.c b/tests/unit/test_password.c index cfff917..aaabd64 100644 --- a/tests/unit/test_password.c +++ b/tests/unit/test_password.c @@ -128,6 +128,48 @@ static void test_nickname_exists_truncation(void **state __attribute__((unused)) assert_true(nickname_exists(twenty_chars_same_prefix, MAX_METANAME)); } +static void test_override_metadatas_high_offset(void **state __attribute__((unused))) { + // Regression: override_metadatas() took the offset as a uint8_t, so offsets + // >= 256 were truncated mod 256. A restore is streamed in 255-byte chunks, + // so the second chunk onward landed at the wrong place and overwrote earlier + // data. Writing at offset 300 must land at 300, not at 300 % 256 == 44. + const uint8_t payload[] = {0xAA, 0xBB, 0xCC}; + override_metadatas(300, (void *) payload, sizeof(payload)); + + assert_memory_equal(&N_storage_real.metadatas[300], payload, sizeof(payload)); + // The location it would have hit when truncated must be untouched. + assert_int_equal(N_storage_real.metadatas[44], 0); + assert_int_equal(N_storage_real.metadatas[45], 0); + assert_int_equal(N_storage_real.metadatas[46], 0); +} + +static void test_override_metadatas_buffer_end(void **state __attribute__((unused))) { + // Writing the final bytes (offset + size == MAX_METADATAS) must be placed + // correctly and stay within the buffer. + const uint8_t payload[] = {0x11, 0x22, 0x33, 0x44}; + const size_t offset = MAX_METADATAS - sizeof(payload); + override_metadatas(offset, (void *) payload, sizeof(payload)); + + assert_memory_equal(&N_storage_real.metadatas[offset], payload, sizeof(payload)); +} + +static void test_write_metadata_enforces_capacity(void **state __attribute__((unused))) { + // The store must refuse new entries once full and never report success past + // the limit, so the 4096-byte buffer can never overflow. + uint8_t name[MAX_METANAME]; + memset(name, 'A', sizeof(name)); + + error_type_t err = OK; + int written = 0; + while ((err = write_metadata(name, sizeof(name))) == OK) { + // Guard against an unbounded loop if the limit were not enforced. + assert_true(++written < MAX_METADATAS); + } + + assert_int_equal(err, ERR_NO_MORE_SPACE_AVAILABLE); + assert_true(written > 0); +} + int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test_setup_teardown(test_nickname_exists_empty_db, setup, NULL), @@ -136,6 +178,9 @@ int main(void) { cmocka_unit_test_setup_teardown(test_nickname_exists_length_mismatch, setup, NULL), cmocka_unit_test_setup_teardown(test_nickname_exists_case_sensitive, setup, NULL), cmocka_unit_test_setup_teardown(test_nickname_exists_truncation, setup, NULL), + cmocka_unit_test_setup_teardown(test_override_metadatas_high_offset, setup, NULL), + cmocka_unit_test_setup_teardown(test_override_metadatas_buffer_end, setup, NULL), + cmocka_unit_test_setup_teardown(test_write_metadata_enforces_capacity, setup, NULL), }; return cmocka_run_group_tests(tests, NULL, NULL); }