Fix IdentityStore::save() reporting success when the write failed

[marcelverdult]

IdentityStore::save() always returned true once the file had opened. The two argument form computed the result of writeTo(), logged it and then threw it away. The form that also stores a display name ignored the result of both writes.

Every role generates a keypair when the load at boot finds no file, and then saves it: simple_repeater/main.cpp:94, simple_room_server/main.cpp:75, simple_sensor/main.cpp:101, kiss_modem/main.cpp:56, simple_secure_chat/main.cpp:327, companion_radio/MyMesh.cpp:955. When that write does not reach the medium, the node keeps running under an identity it never stored, and comes back with a different public key after the next boot.

One caller already acts on the result. CMD_IMPORT_PRIVATE_KEY in companion_radio/MyMesh.cpp:1544 adopts the imported key and answers OK when save() reports success, and ERR_CODE_FILE_IO_ERROR when it does not. Until now a failed import was answered with OK although the identity had not been stored. The old key does not come back either: every platform branch removes or truncates the file before writing, so the next boot finds no identity and generates a new random one.

Both overloads now return the result of the writes. In the second form the display name write counts towards the result as well, so a medium that fills up between the keypair and the name is a failure, and that form gets the debug line the other one already had.

That alone covers only part of it. File::close() is void in every platform branch of this file and drops the error it is handed, so a failure that only surfaces when the file is closed never reaches the caller. Measured with the 96 bytes of an identity, against both filesystem stacks this file talks to:

• littlefs, on nRF52 with prog_size 128 and on STM32 with prog_size 2048: a full filesystem and a failing erase are reported by lfs_file_write() itself, as LFS_ERR_NOSPC and LFS_ERR_IO. A failing program is not. The two writes return 32 and 64, and the error turns up in lfs_file_close(), whose result the void close() of Adafruit_LittleFS drops.
• ESP32, which uses SPIFFS in this repository: nothing reaches the caller at write time at all. The core opens files fully buffered, setvbuf(_IOFBF, 4096) in libraries/FS/src/vfs_api.cpp, and VFSFileImpl::write() is fwrite(), so the 96 bytes stay in that buffer until close() calls fclose() and throws its result away. On a full filesystem fwrite() returns 32 and then 64, and fclose() returns -1. SPIFFSImpl derives from VFSImpl and so does LittleFSImpl, so this is a property of the ESP32 file layer rather than of SPIFFS.

So both overloads read the file back after closing it and compare all 96 bytes of the keypair, plus the 32 bytes of the display name in the overload that stores one. prv_key is private, so the comparison goes through the buffer serialisation LocalIdentity already offers; its layout does not have to match the file, it only has to be the same on both sides. That serialisation becomes const, since it only reads and the comparison would otherwise need a copy of both identities, which costs 192 bytes of stack on a path that has little to spare. A read back that cannot be done at all counts as a failed save: an identity nobody can read is of no use to the next boot.

The success path is unchanged. Of the nine call sites, eight discard the result, and three of those reach save() through void wrappers that could not propagate it anyway. No boot path reads the result, so a failing write adds no new error handling at startup. The overload that stores a display name has no caller in this repository. One gap is left on purpose: set prv.key on the server roles answers "OK, reboot to apply!" whatever the result is, because ICommonCLICallbacks::saveIdentity() is void. Lifting that to bool is a separate change.

A new native environment, native_identity_store, runs IdentityStore against a mock filesystem. Next to its write budget the mock has a commit budget, for the medium that takes every write and loses the data when the file is closed, and a knob that returns one byte differently while keeping the file length. 19 cases, two of which walk every byte of the keypair and of the name, so a comparison that is one byte short cannot pass. The unit test workflow runs the environment next to the existing ones.

Three mutation probes, each caught by the suite: on code that only returns the write result, 8 of the 19 cases fail; shortening the keypair comparison by one byte fails the case that walks the keypair; shortening the name comparison to 8 bytes fails the case that walks the name.

Verified with pio test -e native -e native_kiss_modem -e native_identity_store, 73 of 73 pass.

Built for one environment per platform branch in the file: wio_wm1110_repeater (nRF52), wio-e5_repeater and wio-e5-mini_repeater (STM32), PicoW_repeater (RP2040), and Heltec_v3_repeater plus Heltec_v3_companion_radio_usb (ESP32), the last one being the environment that also compiles the CMD_IMPORT_PRIVATE_KEY caller. All six build. The read back costs 112 bytes of flash on the two tightest targets, wio-e5-mini_repeater going from 203552 to 203664 bytes with 25712 bytes still free, and about 500 bytes of stack per save.

Checked on hardware as well, on a LilyGo T-Echo (nRF52840) running a build from this branch: the stored identity loads, writing it back over set prv.key goes through save() and the read back, and the node reboots afterwards. The write reports OK, the read back raises no error, and the public key is unchanged after the reboot. The ESP32 side has not been run on a device.