From a5877400168bdaf9577ac9dc39adf7d8288c829c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 15:22:31 +0200 Subject: [PATCH 1/7] test: compile the whole first-party codebase under coverage via terse host stubs Add a whole-codebase coverage path to test/Makefile: every first-party .c (hardware/, network/, serial/, storage/, shared/, main.c) is compiled instrumented but unlinked, so a .gcno alone lands it in the report. A --initial baseline (all .gcno at 0%) merged with the harness run data yields a report spanning the whole firmware -- base32/base64/random at their real %, the 15 HW/IO/logic modules at an honest 0%. Add declaration-only host stubs under test/stub/ for the pico-sdk, cyw43, lwip and mbedtls headers those sources include; storage.c/backup.c build against the real littlefs with the same LFS_NO_MALLOC/LFS_NO_DEBUG defines the Pico CMake uses. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Makefile | 52 +++++++++++++++++++++++++++++------ test/stub/hardware/flash.h | 22 +++++++++++++++ test/stub/hardware/rtc.h | 23 ++++++++++++++++ test/stub/hardware/sync.h | 6 ++++ test/stub/hardware/watchdog.h | 10 +++++++ test/stub/lwip/arch.h | 17 ++++++++++++ test/stub/lwip/dns.h | 14 ++++++++++ test/stub/lwip/ip_addr.h | 14 ++++++++++ test/stub/lwip/pbuf.h | 26 ++++++++++++++++++ test/stub/lwip/udp.h | 20 ++++++++++++++ test/stub/mbedtls/md.h | 19 +++++++++++++ test/stub/pico/cyw43_arch.h | 28 +++++++++++++++++++ test/stub/pico/flash.h | 15 ++++++++++ test/stub/pico/multicore.h | 13 +++++++++ test/stub/pico/stdio_usb.h | 10 +++++++ test/stub/pico/stdlib.h | 39 ++++++++++++++++++++++++++ test/stub/pico/time.h | 21 ++++++++++++++ test/stub/version.h | 10 +++++++ 18 files changed, 351 insertions(+), 8 deletions(-) create mode 100644 test/stub/hardware/flash.h create mode 100644 test/stub/hardware/rtc.h create mode 100644 test/stub/hardware/sync.h create mode 100644 test/stub/hardware/watchdog.h create mode 100644 test/stub/lwip/arch.h create mode 100644 test/stub/lwip/dns.h create mode 100644 test/stub/lwip/ip_addr.h create mode 100644 test/stub/lwip/pbuf.h create mode 100644 test/stub/lwip/udp.h create mode 100644 test/stub/mbedtls/md.h create mode 100644 test/stub/pico/cyw43_arch.h create mode 100644 test/stub/pico/flash.h create mode 100644 test/stub/pico/multicore.h create mode 100644 test/stub/pico/stdio_usb.h create mode 100644 test/stub/pico/stdlib.h create mode 100644 test/stub/pico/time.h create mode 100644 test/stub/version.h diff --git a/test/Makefile b/test/Makefile index 1a639f9..f65cc1e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -45,6 +45,25 @@ SECRET_QR_SRCS := harness_secret_qr.c $(SHARED)/random.c $(BASE32)/base32.c \ $(QRCODEGEN)/qrcodegen.c BASE64_SRCS := harness_base64.c $(BASE64)/base64.c +# Whole-codebase coverage: EVERY first-party .c, so the report lists the whole +# firmware (the harness-exercised modules at their real %, the rest at an honest +# 0%). These are compiled instrumented but NOT linked or run — a .gcno alone is +# enough for a file to appear in the report. The terse host stubs under stub/ +# stand in for the pico-sdk / cyw43 / lwip / mbedtls headers they pull. +WHOLE_SRCS := $(ROOT)/hardware/buzzer.c $(ROOT)/hardware/clock.c \ + $(ROOT)/hardware/keypad.c $(ROOT)/hardware/latch.c \ + $(ROOT)/hardware/light.c $(ROOT)/main.c \ + $(ROOT)/network/ntp.c $(ROOT)/network/wifi.c \ + $(ROOT)/serial/commands.c $(ROOT)/serial/commands_backup.c \ + $(ROOT)/serial/commands_keys.c $(ROOT)/serial/commands_network.c \ + $(ROOT)/serial/commands_system.c $(ROOT)/serial/console.c \ + $(ROOT)/shared/random.c $(ROOT)/shared/totp.c \ + $(ROOT)/storage/backup.c $(ROOT)/storage/storage.c + +# LittleFS build knobs, mirrored from the top-level CMakeLists so storage.c and +# backup.c see the same lfs.h / lfs_util.h configuration the firmware does. +WHOLE_DEFS := -DLFS_NO_MALLOC -DLFS_NO_DEBUG + CC := gcc CSTD := -std=c11 WARN := -Wall -Wextra @@ -105,12 +124,15 @@ $(BUILD)/vg_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) $(BUILD)/vg_base64: $(BASE64_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(INCLUDES) $(BASE64_SRCS) -o $@ -# --- Coverage (lcov/genhtml HTML report for the harness-exercised modules) -- -# Build & run the real harnesses with --coverage (producing .gcda), capture -# the run data, filter to first-party sources (dropping the vendored libs and -# the test harnesses themselves), and render coverage/html/ with branches. -# The report covers exactly the modules the harnesses exercise (base32.c, -# base64.c, random.c); HW-only firmware is not built host-side and is absent. +# --- Coverage (lcov/genhtml HTML report for the WHOLE first-party codebase) -- +# Build & run the real harnesses with --coverage (producing .gcda), AND compile +# every first-party .c instrumented but unlinked (compile-only, producing .gcno +# only). A --initial capture turns the whole .gcno set into a 0% baseline; the +# run capture carries the executed lines; merging the two yields a report that +# spans the whole firmware — the harness-exercised modules (base32/base64/ +# random/...) at their real %, every other first-party file at an honest 0%. +# Everything outside the first-party tree (vendored libs, the harnesses, /usr) +# is filtered out. coverage: check-submodules | $(COV_DIR)/obj @echo "== coverage: building + running harnesses ==" $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(INCLUDES) $(SECRET_QR_SRCS) \ @@ -120,11 +142,23 @@ coverage: check-submodules | $(COV_DIR)/obj @echo "== coverage: running instrumented harnesses ==" $(COV_DIR)/obj/cov_secret_qr $(COV_DIR)/obj/cov_base64 - @echo "== coverage: capturing run data ==" + @echo "== coverage: compiling whole first-party codebase (compile-only) ==" + @for src in $(WHOLE_SRCS); do \ + obj=$(COV_DIR)/obj/whole_$$(echo $$src | tr '/.' '__').o; \ + echo " CC $$src"; \ + $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(WHOLE_DEFS) $(INCLUDES) \ + -c $$src -o $$obj || exit 1; \ + done + @echo "== coverage: capturing baseline (all .gcno at 0%) + run data ==" + $(LCOV) --capture --initial --directory $(COV_DIR)/obj \ + --output-file $(COV_DIR)/baseline.info $(LCOV) --capture --directory $(COV_DIR)/obj \ --output-file $(COV_DIR)/run.info + $(LCOV) --add-tracefile $(COV_DIR)/baseline.info \ + --add-tracefile $(COV_DIR)/run.info \ + --output-file $(COV_DIR)/merged.info @echo "== coverage: filtering to first-party sources ==" - $(LCOV) --extract $(COV_DIR)/run.info '$(REPO_ROOT)/*' \ + $(LCOV) --extract $(COV_DIR)/merged.info '$(REPO_ROOT)/*' \ --output-file $(COV_DIR)/first.info $(LCOV) --remove $(COV_DIR)/first.info \ '*/libs/qrcodegen/*' '*/libs/littlefs/*' '*/test/*' '/usr/*' \ @@ -133,6 +167,8 @@ coverage: check-submodules | $(COV_DIR)/obj genhtml $(LCOV_RC) $(GENHTML_IGNORE) --branch-coverage \ --legend --title "hslock host coverage" \ --output-directory $(COV_DIR)/html $(COV_DIR)/coverage.info + @echo "== coverage summary ==" + $(LCOV) --summary $(COV_DIR)/coverage.info @echo "== coverage OK: report at $(COV_DIR)/html/index.html ==" $(BUILD): diff --git a/test/stub/hardware/flash.h b/test/stub/hardware/flash.h new file mode 100644 index 0000000..e0955f6 --- /dev/null +++ b/test/stub/hardware/flash.h @@ -0,0 +1,22 @@ +#ifndef STUB_HARDWARE_FLASH_H +#define STUB_HARDWARE_FLASH_H + +/* + * Host stub for : flash geometry and the raw + * program/erase primitives storage.c drives through flash_safe_execute. + */ + +#include +#include + +#define FLASH_SECTOR_SIZE 4096u +#define FLASH_PAGE_SIZE 256u + +#ifndef XIP_BASE +#define XIP_BASE 0x10000000u +#endif + +void flash_range_program(uint32_t flash_offs, const uint8_t *data, size_t count); +void flash_range_erase(uint32_t flash_offs, size_t count); + +#endif diff --git a/test/stub/hardware/rtc.h b/test/stub/hardware/rtc.h new file mode 100644 index 0000000..a2f6058 --- /dev/null +++ b/test/stub/hardware/rtc.h @@ -0,0 +1,23 @@ +#ifndef STUB_HARDWARE_RTC_H +#define STUB_HARDWARE_RTC_H + +/* Host stub for : the RP2040 RTC datetime type and API. */ + +#include +#include + +typedef struct { + int16_t year; + int8_t month; + int8_t day; + int8_t dotw; + int8_t hour; + int8_t min; + int8_t sec; +} datetime_t; + +void rtc_init(void); +bool rtc_get_datetime(datetime_t *t); +bool rtc_set_datetime(const datetime_t *t); + +#endif diff --git a/test/stub/hardware/sync.h b/test/stub/hardware/sync.h new file mode 100644 index 0000000..6489090 --- /dev/null +++ b/test/stub/hardware/sync.h @@ -0,0 +1,6 @@ +#ifndef STUB_HARDWARE_SYNC_H +#define STUB_HARDWARE_SYNC_H + +/* Host stub for : no host-side primitives are needed. */ + +#endif diff --git a/test/stub/hardware/watchdog.h b/test/stub/hardware/watchdog.h new file mode 100644 index 0000000..faec4f9 --- /dev/null +++ b/test/stub/hardware/watchdog.h @@ -0,0 +1,10 @@ +#ifndef STUB_HARDWARE_WATCHDOG_H +#define STUB_HARDWARE_WATCHDOG_H + +/* Host stub for : the reboot entry point. */ + +#include + +void watchdog_reboot(uint32_t pc, uint32_t sp, uint32_t delay_ms); + +#endif diff --git a/test/stub/lwip/arch.h b/test/stub/lwip/arch.h new file mode 100644 index 0000000..7aa087f --- /dev/null +++ b/test/stub/lwip/arch.h @@ -0,0 +1,17 @@ +#ifndef STUB_LWIP_ARCH_H +#define STUB_LWIP_ARCH_H + +/* Host stub for : the lwIP width typedefs and error codes. */ + +#include + +typedef uint8_t u8_t; +typedef uint16_t u16_t; +typedef uint32_t u32_t; + +typedef int8_t err_t; + +#define ERR_OK 0 +#define ERR_INPROGRESS (-5) + +#endif diff --git a/test/stub/lwip/dns.h b/test/stub/lwip/dns.h new file mode 100644 index 0000000..f496edd --- /dev/null +++ b/test/stub/lwip/dns.h @@ -0,0 +1,14 @@ +#ifndef STUB_LWIP_DNS_H +#define STUB_LWIP_DNS_H + +/* Host stub for : hostname resolution used by the NTP client. */ + +#include "lwip/arch.h" +#include "lwip/ip_addr.h" + +typedef void (*dns_found_callback)(const char *name, const ip_addr_t *ipaddr, void *callback_arg); + +err_t dns_gethostbyname(const char *hostname, ip_addr_t *addr, dns_found_callback found, + void *callback_arg); + +#endif diff --git a/test/stub/lwip/ip_addr.h b/test/stub/lwip/ip_addr.h new file mode 100644 index 0000000..39201a0 --- /dev/null +++ b/test/stub/lwip/ip_addr.h @@ -0,0 +1,14 @@ +#ifndef STUB_LWIP_IP_ADDR_H +#define STUB_LWIP_IP_ADDR_H + +/* Host stub for : an opaque IP address holder. */ + +#include "lwip/arch.h" + +typedef struct { + u32_t addr; +} ip_addr_t; + +#define IPADDR_TYPE_ANY 0 + +#endif diff --git a/test/stub/lwip/pbuf.h b/test/stub/lwip/pbuf.h new file mode 100644 index 0000000..6ee5a08 --- /dev/null +++ b/test/stub/lwip/pbuf.h @@ -0,0 +1,26 @@ +#ifndef STUB_LWIP_PBUF_H +#define STUB_LWIP_PBUF_H + +/* Host stub for : the packet buffer ntp.c allocates and reads. */ + +#include + +#include "lwip/arch.h" + +typedef enum { + PBUF_TRANSPORT, + PBUF_RAM, +} pbuf_kind_t; + +struct pbuf { + struct pbuf *next; + void *payload; + u16_t len; + u16_t tot_len; +}; + +struct pbuf *pbuf_alloc(pbuf_kind_t layer, u16_t length, pbuf_kind_t type); +u8_t pbuf_free(struct pbuf *p); +u16_t pbuf_copy_partial(const struct pbuf *p, void *dataptr, u16_t len, u16_t offset); + +#endif diff --git a/test/stub/lwip/udp.h b/test/stub/lwip/udp.h new file mode 100644 index 0000000..c233fad --- /dev/null +++ b/test/stub/lwip/udp.h @@ -0,0 +1,20 @@ +#ifndef STUB_LWIP_UDP_H +#define STUB_LWIP_UDP_H + +/* Host stub for : the UDP PCB API ntp.c uses. */ + +#include "lwip/arch.h" +#include "lwip/ip_addr.h" +#include "lwip/pbuf.h" + +struct udp_pcb; + +typedef void (*udp_recv_fn)(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, + u16_t port); + +struct udp_pcb *udp_new_ip_type(u8_t type); +void udp_remove(struct udp_pcb *pcb); +void udp_recv(struct udp_pcb *pcb, udp_recv_fn recv, void *recv_arg); +err_t udp_sendto(struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *dst_ip, u16_t dst_port); + +#endif diff --git a/test/stub/mbedtls/md.h b/test/stub/mbedtls/md.h new file mode 100644 index 0000000..dc0b25a --- /dev/null +++ b/test/stub/mbedtls/md.h @@ -0,0 +1,19 @@ +#ifndef STUB_MBEDTLS_MD_H +#define STUB_MBEDTLS_MD_H + +/* Host stub for : only the HMAC path totp.c uses. */ + +#include +#include + +typedef enum { + MBEDTLS_MD_SHA1 = 4, +} mbedtls_md_type_t; + +typedef struct mbedtls_md_info_t mbedtls_md_info_t; + +const mbedtls_md_info_t *mbedtls_md_info_from_type(mbedtls_md_type_t md_type); +int mbedtls_md_hmac(const mbedtls_md_info_t *md_info, const uint8_t *key, size_t keylen, + const uint8_t *input, size_t ilen, uint8_t *output); + +#endif diff --git a/test/stub/pico/cyw43_arch.h b/test/stub/pico/cyw43_arch.h new file mode 100644 index 0000000..d3a42a3 --- /dev/null +++ b/test/stub/pico/cyw43_arch.h @@ -0,0 +1,28 @@ +#ifndef STUB_PICO_CYW43_ARCH_H +#define STUB_PICO_CYW43_ARCH_H + +/* Host stub for : the WiFi driver entry points ntp.c and + * wifi.c call. */ + +#include + +#define CYW43_ITF_STA 0 +#define CYW43_LINK_UP 1 +#define CYW43_AUTH_WPA2_AES_PSK 0x00400004u + +typedef struct { + int dummy; +} cyw43_t; + +extern cyw43_t cyw43_state; + +int cyw43_arch_init(void); +void cyw43_arch_enable_sta_mode(void); +int cyw43_arch_wifi_connect_timeout_ms(const char *ssid, const char *password, uint32_t auth, + uint32_t timeout_ms); +int cyw43_tcpip_link_status(cyw43_t *self, int itf); +void cyw43_arch_lwip_begin(void); +void cyw43_arch_lwip_end(void); +void cyw43_arch_poll(void); + +#endif diff --git a/test/stub/pico/flash.h b/test/stub/pico/flash.h new file mode 100644 index 0000000..38db037 --- /dev/null +++ b/test/stub/pico/flash.h @@ -0,0 +1,15 @@ +#ifndef STUB_PICO_FLASH_H +#define STUB_PICO_FLASH_H + +/* + * Host stub for : flash_safe_execute plus the "keep this code + * out of flash" attribute, which is a no-op on the host. + */ + +#include + +#define __no_inline_not_in_flash_func(name) name + +int flash_safe_execute(void (*func)(void *), void *param, uint32_t enter_exit_timeout_ms); + +#endif diff --git a/test/stub/pico/multicore.h b/test/stub/pico/multicore.h new file mode 100644 index 0000000..08a7f04 --- /dev/null +++ b/test/stub/pico/multicore.h @@ -0,0 +1,13 @@ +#ifndef STUB_PICO_MULTICORE_H +#define STUB_PICO_MULTICORE_H + +/* Host stub for : core-1 launch and inter-core FIFO. */ + +#include + +void multicore_lockout_victim_init(void); +void multicore_launch_core1(void (*entry)(void)); +void multicore_fifo_push_blocking(uint32_t data); +uint32_t multicore_fifo_pop_blocking(void); + +#endif diff --git a/test/stub/pico/stdio_usb.h b/test/stub/pico/stdio_usb.h new file mode 100644 index 0000000..938870b --- /dev/null +++ b/test/stub/pico/stdio_usb.h @@ -0,0 +1,10 @@ +#ifndef STUB_PICO_STDIO_USB_H +#define STUB_PICO_STDIO_USB_H + +/* Host stub for : USB-CDC connection state. */ + +#include + +bool stdio_usb_connected(void); + +#endif diff --git a/test/stub/pico/stdlib.h b/test/stub/pico/stdlib.h new file mode 100644 index 0000000..93a8158 --- /dev/null +++ b/test/stub/pico/stdlib.h @@ -0,0 +1,39 @@ +#ifndef STUB_PICO_STDLIB_H +#define STUB_PICO_STDLIB_H + +/* + * Host stub for : the GPIO, stdio-init and timing shims the + * firmware pulls in. Timing lives in , as on real hardware. + */ + +#include +#include +#include + +#include "pico/time.h" + +typedef unsigned int uint; + +#define GPIO_OUT 1 +#define GPIO_IN 0 + +#define PICO_OK 0 +#define PICO_ERROR_TIMEOUT (-1) + +#ifndef PICO_FLASH_SIZE_BYTES +#define PICO_FLASH_SIZE_BYTES (2 * 1024 * 1024) +#endif + +void gpio_init(uint pin); +void gpio_set_dir(uint pin, bool out); +void gpio_put(uint pin, bool value); +bool gpio_get(uint pin); +void gpio_pull_up(uint pin); + +void stdio_init_all(void); +int getchar_timeout_us(uint32_t timeout_us); + +static inline void tight_loop_contents(void) { +} + +#endif diff --git a/test/stub/pico/time.h b/test/stub/pico/time.h new file mode 100644 index 0000000..1ce6eda --- /dev/null +++ b/test/stub/pico/time.h @@ -0,0 +1,21 @@ +#ifndef STUB_PICO_TIME_H +#define STUB_PICO_TIME_H + +/* + * Host stub for : sleeps and the absolute-time helpers used by + * the keypad debouncer and the NTP poll loop. + */ + +#include +#include + +typedef uint64_t absolute_time_t; + +void sleep_ms(uint32_t ms); +void sleep_us(uint64_t us); + +absolute_time_t make_timeout_time_ms(uint32_t ms); +bool time_reached(absolute_time_t t); +uint64_t time_us_64(void); + +#endif diff --git a/test/stub/version.h b/test/stub/version.h new file mode 100644 index 0000000..6d7270b --- /dev/null +++ b/test/stub/version.h @@ -0,0 +1,10 @@ +#ifndef STUB_VERSION_H +#define STUB_VERSION_H + +/* Host stub for the CMake-generated version.h consumed by `status`. */ + +#define GIT_HASH "hoststub" +#define GIT_DIRTY 0 +#define BUILD_DATE "host" + +#endif From 1a503b44be1ac04d2ec9b8bb8742eceecd4132c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 15:29:36 +0200 Subject: [PATCH 2/7] test: add TOTP harness (real mbedtls) covering shared/totp.c Build harness_totp.c + shared/totp.c against the system libmbedtls-dev (real HMAC-SHA1) instead of the compile-only stub, using `-idirafter stub` so the real wins while hardware/rtc.h stays stubbed. clock_get_unix_time() is injected in the harness for deterministic time. Asserts the standard RFC 6238 SHA1 test vectors (seed "12345678901234567890") and exercises totp_verify: RTC-not-set, correct code, the T-1/T/T+1 window, and out-of-window/wrong code rejection. shared/totp.c goes 0% -> 100% lines (25/25), 100% funcs (2/2), 100% branches (6/6). Wired into asan, valgrind, and coverage targets. CI note: the test job must apt-install libmbedtls-dev. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Makefile | 24 +++++++++++- test/harness_totp.c | 89 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 test/harness_totp.c diff --git a/test/Makefile b/test/Makefile index f65cc1e..e73b034 100644 --- a/test/Makefile +++ b/test/Makefile @@ -45,6 +45,15 @@ SECRET_QR_SRCS := harness_secret_qr.c $(SHARED)/random.c $(BASE32)/base32.c \ $(QRCODEGEN)/qrcodegen.c BASE64_SRCS := harness_base64.c $(BASE64)/base64.c +# TOTP harness needs the REAL HMAC-SHA1, so it builds against the system mbedtls +# (libmbedtls-dev) rather than the compile-only stub/mbedtls/md.h. `-idirafter +# stub` keeps the hardware/rtc.h stub reachable while letting the real +# (a system dir) win the search for totp.c's HMAC include. +# CI: the test job must apt-install `libmbedtls-dev` for this to build. +TOTP_SRCS := harness_totp.c $(SHARED)/totp.c +TOTP_INCLUDES := -I$(ROOT) -I$(ROOT)/hardware -I$(SHARED) -idirafter stub +TOTP_LIBS := -lmbedcrypto + # Whole-codebase coverage: EVERY first-party .c, so the report lists the whole # firmware (the harness-exercised modules at their real %, the rest at an honest # 0%). These are compiled instrumented but NOT linked or run — a .gcno alone is @@ -99,10 +108,11 @@ check-submodules: exit 1; } # --- ASan + UBSan ----------------------------------------------------------- -asan: check-submodules $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 +asan: check-submodules $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 $(BUILD)/asan_totp @echo "== running asan/ubsan harnesses ==" $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 + $(BUILD)/asan_totp @echo "== asan OK ==" $(BUILD)/asan_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) @@ -111,11 +121,15 @@ $(BUILD)/asan_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) $(BUILD)/asan_base64: $(BASE64_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(INCLUDES) $(BASE64_SRCS) -o $@ +$(BUILD)/asan_totp: $(TOTP_SRCS) | $(BUILD) + $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) -o $@ + # --- Valgrind --------------------------------------------------------------- -valgrind: check-submodules $(BUILD)/vg_secret_qr $(BUILD)/vg_base64 +valgrind: check-submodules $(BUILD)/vg_secret_qr $(BUILD)/vg_base64 $(BUILD)/vg_totp @echo "== running harnesses under valgrind ==" valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_secret_qr valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_base64 + valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_totp @echo "== valgrind OK ==" $(BUILD)/vg_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) @@ -124,6 +138,9 @@ $(BUILD)/vg_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) $(BUILD)/vg_base64: $(BASE64_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(INCLUDES) $(BASE64_SRCS) -o $@ +$(BUILD)/vg_totp: $(TOTP_SRCS) | $(BUILD) + $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) -o $@ + # --- Coverage (lcov/genhtml HTML report for the WHOLE first-party codebase) -- # Build & run the real harnesses with --coverage (producing .gcda), AND compile # every first-party .c instrumented but unlinked (compile-only, producing .gcno @@ -139,9 +156,12 @@ coverage: check-submodules | $(COV_DIR)/obj -o $(COV_DIR)/obj/cov_secret_qr $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(INCLUDES) $(BASE64_SRCS) \ -o $(COV_DIR)/obj/cov_base64 + $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) \ + -o $(COV_DIR)/obj/cov_totp @echo "== coverage: running instrumented harnesses ==" $(COV_DIR)/obj/cov_secret_qr $(COV_DIR)/obj/cov_base64 + $(COV_DIR)/obj/cov_totp @echo "== coverage: compiling whole first-party codebase (compile-only) ==" @for src in $(WHOLE_SRCS); do \ obj=$(COV_DIR)/obj/whole_$$(echo $$src | tr '/.' '__').o; \ diff --git a/test/harness_totp.c b/test/harness_totp.c new file mode 100644 index 0000000..3ff03a1 --- /dev/null +++ b/test/harness_totp.c @@ -0,0 +1,89 @@ +/* + * Host harness: shared/totp.c (RFC 6238 TOTP, HMAC-SHA1, 6 digits, 30s step). + * + * Built against the REAL mbedtls (system libmbedcrypto) so the HMAC-SHA1 path + * computes genuine codes — the compile-only stub in stub/mbedtls/md.h cannot. + * clock_get_unix_time() is defined here (instead of linking hardware/clock.c, + * which needs the RP2040 RTC) so the verify() time can be injected and the + * vectors stay deterministic. + * + * Coverage of the two functions in totp.c: + * totp_at — the standard RFC 6238 SHA1 test-vector table (seed + * "12345678901234567890"), asserting the exact 6-digit codes. + * totp_verify — RTC-not-set (unix_time==0) -> false; correct code accepted; + * each of the T-1 / T / T+1 window steps accepted; a wrong code + * and an out-of-window code rejected. + */ + +#include +#include +#include +#include + +#include "totp.h" + +/* Injectable clock: totp_verify() reads the current time through this. */ +static uint32_t g_unix_time = 0; + +uint32_t clock_get_unix_time(void) { + return g_unix_time; +} + +/* RFC 6238 Appendix B, SHA1 rows, truncated to TOTP_DIGITS (6) digits. */ +struct vector { + uint64_t unix_time; + uint32_t code; +}; + +int main(void) { + /* Standard RFC 4226/6238 seed: ASCII "12345678901234567890" (20 bytes). */ + const uint8_t secret[] = "12345678901234567890"; + const size_t secret_len = sizeof secret - 1; /* drop the NUL */ + + static const struct vector vectors[] = { + {59, 287082}, /* full RFC code 94287082 */ + {1111111109, 81804}, /* 07081804 */ + {1111111111, 50471}, /* 14050471 */ + {1234567890, 5924}, /* 89005924 */ + {2000000000, 279037}, /* 69279037 */ + {20000000000ULL, 353130}, /* 65353130 */ + }; + + for (size_t i = 0; i < sizeof vectors / sizeof vectors[0]; i++) { + uint64_t step = vectors[i].unix_time / TOTP_STEP_SECONDS; + uint32_t got = totp_at(secret, secret_len, step); + assert(got == vectors[i].code); + } + + /* --- totp_verify ------------------------------------------------------ */ + + /* RTC not set: unix_time == 0 must be rejected regardless of code. */ + g_unix_time = 0; + assert(totp_verify(secret, secret_len, 0) == false); + assert(totp_verify(secret, secret_len, totp_at(secret, secret_len, 0)) == false); + + /* Pick a time whose step and neighbours all yield distinct codes so the + * window logic is exercised unambiguously. */ + g_unix_time = 1111111111; /* step 37037037 */ + uint64_t step = g_unix_time / TOTP_STEP_SECONDS; + + uint32_t code_here = totp_at(secret, secret_len, step); + uint32_t code_prev = totp_at(secret, secret_len, step - 1); + uint32_t code_next = totp_at(secret, secret_len, step + 1); + uint32_t code_far = totp_at(secret, secret_len, step + TOTP_WINDOW + 1); + + /* Correct current code accepted. */ + assert(totp_verify(secret, secret_len, code_here) == true); + /* Both window neighbours (T-1, T+1) accepted. */ + assert(totp_verify(secret, secret_len, code_prev) == true); + assert(totp_verify(secret, secret_len, code_next) == true); + /* A code just outside the window is rejected. */ + if (code_far != code_here && code_far != code_prev && code_far != code_next) { + assert(totp_verify(secret, secret_len, code_far) == false); + } + /* An obviously wrong code is rejected. */ + assert(totp_verify(secret, secret_len, 999999u + 1u) == false); /* impossible 7-digit */ + + printf("totp OK\n"); + return 0; +} From 54d4abf343d623dfb0a6a0ba862f05376bc34ac5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 15:37:20 +0200 Subject: [PATCH 3/7] test: add storage harness on a RAM-backed littlefs, covering storage.c + backup.c Exercise storage/storage.c and storage/backup.c host-side against the real littlefs, backed by a RAM XIP window (MAP_FIXED at XIP_BASE+STORAGE_FLASH_OFFSET so storage.c's absolute-address flash_read hits host RAM; program/erase shims give NOR semantics). Covers key CRUD, list, update, wifi cfg, backup export/import roundtrip and every rejection path. storage.c 0 -> 92.5% lines, backup.c 0 -> 91.8% lines. Fixes a genuine buffer overflow ASan caught: LFS_CFG.lookahead_size was set to LFS_LOOKAHEAD (64) while lfs_lookahead_buf is only LFS_LOOKAHEAD/8 (8) bytes; littlefs memsets lookahead_size bytes into that buffer, a 56-byte global overflow on every mount/format (on real hardware too). Derive lookahead_size from sizeof(buffer) so the two can never diverge. Co-Authored-By: Claude Opus 4.8 (1M context) --- storage/storage.c | 16 ++- test/Makefile | 25 +++- test/harness_storage.c | 255 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+), 8 deletions(-) create mode 100644 test/harness_storage.c diff --git a/storage/storage.c b/storage/storage.c index 950125c..1dd5379 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -152,12 +152,16 @@ static const struct lfs_config LFS_CFG = { .erase = flash_erase, .sync = flash_sync, - .read_size = LFS_READ_SIZE, - .prog_size = LFS_PROG_SIZE, - .block_size = LFS_BLOCK_SIZE, - .block_count = LFS_BLOCK_COUNT, - .cache_size = LFS_CACHE_SIZE, - .lookahead_size = LFS_LOOKAHEAD, + .read_size = LFS_READ_SIZE, + .prog_size = LFS_PROG_SIZE, + .block_size = LFS_BLOCK_SIZE, + .block_count = LFS_BLOCK_COUNT, + .cache_size = LFS_CACHE_SIZE, + // lookahead_size is in BYTES (littlefs tracks 8 blocks per byte and memsets + // this many bytes into lookahead_buffer). It must equal the buffer size, so + // derive it from the buffer to keep the two in lockstep; LFS_LOOKAHEAD/8 = 8 + // bytes tracks all 64 blocks in one pass. + .lookahead_size = sizeof(lfs_lookahead_buf), .block_cycles = 500, // wear leveling hint .read_buffer = lfs_read_buf, diff --git a/test/Makefile b/test/Makefile index e73b034..ad4c8f7 100644 --- a/test/Makefile +++ b/test/Makefile @@ -54,6 +54,14 @@ TOTP_SRCS := harness_totp.c $(SHARED)/totp.c TOTP_INCLUDES := -I$(ROOT) -I$(ROOT)/hardware -I$(SHARED) -idirafter stub TOTP_LIBS := -lmbedcrypto +# Storage harness: storage.c + backup.c + the REAL littlefs (lfs.c/lfs_util.c), +# run against a RAM-backed XIP window the harness maps in (see harness_storage.c). +# LFS_NO_MALLOC/LFS_NO_DEBUG mirror the firmware's CMakeLists so lfs.h/lfs_util.h +# match. The shared INCLUDES already resolve stub/, storage/ and libs/littlefs/. +STORAGE_SRCS := harness_storage.c $(ROOT)/storage/storage.c $(ROOT)/storage/backup.c \ + $(ROOT)/libs/littlefs/lfs.c $(ROOT)/libs/littlefs/lfs_util.c +STORAGE_DEFS := -DLFS_NO_MALLOC -DLFS_NO_DEBUG + # Whole-codebase coverage: EVERY first-party .c, so the report lists the whole # firmware (the harness-exercised modules at their real %, the rest at an honest # 0%). These are compiled instrumented but NOT linked or run — a .gcno alone is @@ -108,11 +116,13 @@ check-submodules: exit 1; } # --- ASan + UBSan ----------------------------------------------------------- -asan: check-submodules $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 $(BUILD)/asan_totp +asan: check-submodules $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 $(BUILD)/asan_totp \ + $(BUILD)/asan_storage @echo "== running asan/ubsan harnesses ==" $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 $(BUILD)/asan_totp + $(BUILD)/asan_storage @echo "== asan OK ==" $(BUILD)/asan_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) @@ -124,12 +134,17 @@ $(BUILD)/asan_base64: $(BASE64_SRCS) | $(BUILD) $(BUILD)/asan_totp: $(TOTP_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) -o $@ +$(BUILD)/asan_storage: $(STORAGE_SRCS) | $(BUILD) + $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) -o $@ + # --- Valgrind --------------------------------------------------------------- -valgrind: check-submodules $(BUILD)/vg_secret_qr $(BUILD)/vg_base64 $(BUILD)/vg_totp +valgrind: check-submodules $(BUILD)/vg_secret_qr $(BUILD)/vg_base64 $(BUILD)/vg_totp \ + $(BUILD)/vg_storage @echo "== running harnesses under valgrind ==" valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_secret_qr valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_base64 valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_totp + valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_storage @echo "== valgrind OK ==" $(BUILD)/vg_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) @@ -141,6 +156,9 @@ $(BUILD)/vg_base64: $(BASE64_SRCS) | $(BUILD) $(BUILD)/vg_totp: $(TOTP_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) -o $@ +$(BUILD)/vg_storage: $(STORAGE_SRCS) | $(BUILD) + $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) -o $@ + # --- Coverage (lcov/genhtml HTML report for the WHOLE first-party codebase) -- # Build & run the real harnesses with --coverage (producing .gcda), AND compile # every first-party .c instrumented but unlinked (compile-only, producing .gcno @@ -158,10 +176,13 @@ coverage: check-submodules | $(COV_DIR)/obj -o $(COV_DIR)/obj/cov_base64 $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(TOTP_INCLUDES) $(TOTP_SRCS) $(TOTP_LIBS) \ -o $(COV_DIR)/obj/cov_totp + $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) \ + -o $(COV_DIR)/obj/cov_storage @echo "== coverage: running instrumented harnesses ==" $(COV_DIR)/obj/cov_secret_qr $(COV_DIR)/obj/cov_base64 $(COV_DIR)/obj/cov_totp + $(COV_DIR)/obj/cov_storage @echo "== coverage: compiling whole first-party codebase (compile-only) ==" @for src in $(WHOLE_SRCS); do \ obj=$(COV_DIR)/obj/whole_$$(echo $$src | tr '/.' '__').o; \ diff --git a/test/harness_storage.c b/test/harness_storage.c new file mode 100644 index 0000000..fd929f3 --- /dev/null +++ b/test/harness_storage.c @@ -0,0 +1,255 @@ +/* + * Host harness: storage/storage.c + storage/backup.c on a RAM-backed LittleFS. + * + * The firmware's block device reads flash by dereferencing an absolute XIP + * address (XIP_BASE + STORAGE_FLASH_OFFSET + ...) and writes it through + * flash_range_program/erase (offset-from-flash-base). Both refer to the same + * XIP window. We reproduce that window in host RAM with a single MAP_FIXED + * anonymous mapping placed exactly at (void*)(XIP_BASE + STORAGE_FLASH_OFFSET): + * - storage.c's flash_read() dereferences (const void*)(uint32_t addr); the + * address lands inside the mapping, so reads hit our RAM. + * - the flash_range_program()/erase() shims below write the same window + * with NOR-flash semantics (erase -> 0xFF, program -> bitwise AND). + * - flash_safe_execute() simply calls the callback (no cores to pause). + * This leaves the firmware source untouched: storage.c + backup.c + the real + * littlefs lfs.c/lfs_util.c run natively against the RAM window, so their CRUD + * / wifi / backup logic gets genuinely exercised. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "hardware/flash.h" /* XIP_BASE, FLASH_SECTOR_SIZE */ +#include "pico/stdlib.h" /* PICO_FLASH_SIZE_BYTES, PICO_OK */ + +#include "backup.h" +#include "storage.h" + +/* Must match storage.c's private layout constants. */ +#define STORAGE_SIZE_BYTES (256 * 1024) +#define STORAGE_FLASH_OFFSET (PICO_FLASH_SIZE_BYTES - STORAGE_SIZE_BYTES) +#define STORAGE_WINDOW_BASE ((uintptr_t)(XIP_BASE + STORAGE_FLASH_OFFSET)) + +/* Absolute host pointer for a flash-base-relative offset. */ +static uint8_t *flash_ptr(uint32_t flash_offs) { + return (uint8_t *)(uintptr_t)(XIP_BASE + flash_offs); +} + +/* --- RAM-backed flash primitives storage.c drives ------------------------- */ + +void flash_range_program(uint32_t flash_offs, const uint8_t *data, size_t count) { + uint8_t *dst = flash_ptr(flash_offs); + for (size_t i = 0; i < count; i++) + dst[i] &= data[i]; /* NOR: programming can only clear bits */ +} + +void flash_range_erase(uint32_t flash_offs, size_t count) { + memset(flash_ptr(flash_offs), 0xFF, count); /* erased NOR reads as 0xFF */ +} + +int flash_safe_execute(void (*func)(void *), void *param, uint32_t timeout_ms) { + (void)timeout_ms; + func(param); + return PICO_OK; +} + +/* Map the XIP storage window into host RAM at its true address. */ +static void flash_ram_map(void) { + void *base = mmap((void *)STORAGE_WINDOW_BASE, STORAGE_SIZE_BYTES, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); + if (base == MAP_FAILED) { + perror("mmap XIP window"); + assert(0); + } + assert((uintptr_t)base == STORAGE_WINDOW_BASE); + memset(base, 0xFF, STORAGE_SIZE_BYTES); /* fresh, fully-erased flash */ +} + +/* --- helpers -------------------------------------------------------------- */ + +static key_record_t make_key(uint16_t id, const char *name, bool enabled, bool admin, + uint32_t created, uint8_t seed) { + key_record_t k = {0}; + k.id = id; + k.is_enabled = enabled; + k.is_admin = admin; + k.created_at = created; + snprintf(k.name, sizeof k.name, "%s", name); + for (int i = 0; i < KEY_SECRET_LEN; i++) + k.secret[i] = (uint8_t)(seed + i); + return k; +} + +static bool keys_equal(const key_record_t *a, const key_record_t *b) { + return a->id == b->id && a->is_enabled == b->is_enabled && a->is_admin == b->is_admin && + a->created_at == b->created_at && memcmp(a->name, b->name, sizeof a->name) == 0 && + memcmp(a->secret, b->secret, sizeof a->secret) == 0; +} + +int main(void) { + /* --- before mount: every op must refuse -------------------------------- */ + key_record_t tmp; + assert(storage_key_exists(1) == false); + assert(storage_key_get(1, &tmp) == false); + assert(storage_key_save(&tmp) == false); + assert(storage_key_delete(1) == false); + assert(storage_key_list(&tmp, 1) == -1); + wifi_config_t wtmp; + assert(storage_wifi_get(&wtmp) == false); + assert(storage_wifi_set(&wtmp) == false); + assert(storage_wifi_clear() == false); + + /* --- init on fresh 0xFF flash: mount fails -> format -> mount ---------- */ + flash_ram_map(); + assert(storage_init() == true); + + /* --- key save/get roundtrip ------------------------------------------- */ + key_record_t k1 = make_key(1, "alice", true, true, 1700000000u, 0x10); + assert(storage_key_save(&k1) == true); + assert(storage_key_exists(1) == true); + assert(storage_key_exists(2) == false); + + key_record_t got; + assert(storage_key_get(1, &got) == true); + assert(got.is_checksum_valid == true); + assert(keys_equal(&k1, &got)); + + /* get of a nonexistent key fails */ + assert(storage_key_get(99, &got) == false); + + /* --- update existing key ---------------------------------------------- */ + key_record_t k1b = make_key(1, "alice2", false, false, 1700000123u, 0x20); + assert(storage_key_save(&k1b) == true); + assert(storage_key_get(1, &got) == true); + assert(keys_equal(&k1b, &got)); + + /* --- multiple keys + list --------------------------------------------- */ + key_record_t k2 = make_key(2, "bob", true, false, 1700000200u, 0x30); + key_record_t k3 = make_key(3, "carol", false, true, 1700000300u, 0x40); + assert(storage_key_save(&k2) == true); + assert(storage_key_save(&k3) == true); + + key_record_t list[16]; + int n = storage_key_list(list, 16); + assert(n == 3); + + /* list truncation: max_count clamps to KEY_MAX_COUNT and to what fits */ + int n1 = storage_key_list(list, 1); + assert(n1 == 1); + int nbig = storage_key_list(list, KEY_MAX_COUNT + 100); + assert(nbig == 3); + + /* --- delete + get-after-delete ---------------------------------------- */ + assert(storage_key_delete(2) == true); + assert(storage_key_exists(2) == false); + assert(storage_key_get(2, &got) == false); + assert(storage_key_delete(2) == false); /* already gone */ + assert(storage_key_list(list, 16) == 2); + + /* --- wifi set/get/clear ----------------------------------------------- */ + wifi_config_t wcfg = {0}; + snprintf(wcfg.ssid, sizeof wcfg.ssid, "hackerspace"); + snprintf(wcfg.password, sizeof wcfg.password, "s3cr3t-pass"); + assert(storage_wifi_set(&wcfg) == true); + + wifi_config_t wgot = {0}; + assert(storage_wifi_get(&wgot) == true); + assert(strcmp(wgot.ssid, wcfg.ssid) == 0); + assert(strcmp(wgot.password, wcfg.password) == 0); + + assert(storage_wifi_clear() == true); + assert(storage_wifi_get(&wgot) == false); /* gone */ + assert(storage_wifi_clear() == false); /* already gone */ + + /* --- backup export -> import roundtrip -------------------------------- */ + /* Current store holds keys id=1 (updated) and id=3. */ + static uint8_t backup[sizeof(backup_header_t) + BACKUP_MAX_KEYS * sizeof(backup_key_t)]; + int blen = backup_export(backup, sizeof backup); + assert(blen > 0); + assert((size_t)blen == sizeof(backup_header_t) + 2 * sizeof(backup_key_t)); + + /* buffer too small -> -1 */ + assert(backup_export(backup, sizeof(backup_header_t)) == -1); + + /* Snapshot expected keys, then wipe the store. */ + key_record_t expect[8]; + int expect_n = storage_key_list(expect, 8); + assert(expect_n == 2); + for (int i = 0; i < expect_n; i++) + assert(storage_key_delete(expect[i].id) == true); + assert(storage_key_list(list, 16) == 0); + + /* Import restores them. */ + assert(backup_import(backup, (size_t)blen) == true); + int restored_n = storage_key_list(list, 16); + assert(restored_n == expect_n); + for (int i = 0; i < expect_n; i++) { + key_record_t r; + assert(storage_key_get(expect[i].id, &r) == true); + assert(keys_equal(&expect[i], &r)); + } + + /* --- corrupt / invalid backups all rejected --------------------------- */ + /* too small */ + assert(backup_import(backup, sizeof(backup_header_t) - 1) == false); + + /* bad magic */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + backup_header_t *h = (backup_header_t *)bad; + h->magic = 0xDEADBEEFu; + assert(backup_import(bad, (size_t)blen) == false); + } + /* bad version */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + backup_header_t *h = (backup_header_t *)bad; + h->version = BACKUP_VERSION + 1; + assert(backup_import(bad, (size_t)blen) == false); + } + /* key_count too large */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + backup_header_t *h = (backup_header_t *)bad; + h->key_count = KEY_MAX_COUNT + 1; + assert(backup_import(bad, (size_t)blen) == false); + } + /* truncated body (header claims more keys than bytes provided) */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + backup_header_t *h = (backup_header_t *)bad; + h->key_count = 200; /* body far shorter than 200 records */ + assert(backup_import(bad, (size_t)blen) == false); + } + /* checksum mismatch (flip a payload byte, keep header intact) */ + { + uint8_t bad[sizeof backup]; + memcpy(bad, backup, (size_t)blen); + bad[sizeof(backup_header_t)] ^= 0xFF; + assert(backup_import(bad, (size_t)blen) == false); + } + + /* The store still holds the good import after all rejected attempts. */ + assert(storage_key_list(list, 16) == expect_n); + + /* --- empty backup (no keys) roundtrips -------------------------------- */ + for (int i = 0; i < expect_n; i++) + assert(storage_key_delete(expect[i].id) == true); + assert(storage_key_list(list, 16) == 0); + int elen = backup_export(backup, sizeof backup); + assert((size_t)elen == sizeof(backup_header_t)); + assert(backup_import(backup, (size_t)elen) == true); + assert(storage_key_list(list, 16) == 0); + + printf("storage OK\n"); + return 0; +} From 089d78570533f78b3b9ffa8b5d8e3cc55a1f4c25 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 15:43:14 +0200 Subject: [PATCH 4/7] test: add commands dispatcher harness covering serial/commands.c Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Makefile | 22 +++- test/harness_commands.c | 271 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 291 insertions(+), 2 deletions(-) create mode 100644 test/harness_commands.c diff --git a/test/Makefile b/test/Makefile index ad4c8f7..b7b6334 100644 --- a/test/Makefile +++ b/test/Makefile @@ -62,6 +62,13 @@ STORAGE_SRCS := harness_storage.c $(ROOT)/storage/storage.c $(ROOT)/storage/back $(ROOT)/libs/littlefs/lfs.c $(ROOT)/libs/littlefs/lfs_util.c STORAGE_DEFS := -DLFS_NO_MALLOC -DLFS_NO_DEBUG +# Commands harness: links serial/commands.c (the dispatcher) against SPY handler +# stubs + a stub buzzer, all inside harness_commands.c. No real key/system/ +# network/backup handlers are pulled in — the harness drives commands_dispatch() +# and asserts routing / admin gating / argc validation / unknown handling. The +# shared INCLUDES resolve stub/pico + hardware/buzzer.h. +COMMANDS_SRCS := harness_commands.c $(ROOT)/serial/commands.c + # Whole-codebase coverage: EVERY first-party .c, so the report lists the whole # firmware (the harness-exercised modules at their real %, the rest at an honest # 0%). These are compiled instrumented but NOT linked or run — a .gcno alone is @@ -117,12 +124,13 @@ check-submodules: # --- ASan + UBSan ----------------------------------------------------------- asan: check-submodules $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 $(BUILD)/asan_totp \ - $(BUILD)/asan_storage + $(BUILD)/asan_storage $(BUILD)/asan_commands @echo "== running asan/ubsan harnesses ==" $(BUILD)/asan_secret_qr $(BUILD)/asan_base64 $(BUILD)/asan_totp $(BUILD)/asan_storage + $(BUILD)/asan_commands @echo "== asan OK ==" $(BUILD)/asan_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) @@ -137,14 +145,18 @@ $(BUILD)/asan_totp: $(TOTP_SRCS) | $(BUILD) $(BUILD)/asan_storage: $(STORAGE_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) -o $@ +$(BUILD)/asan_commands: $(COMMANDS_SRCS) | $(BUILD) + $(CC) $(CSTD) $(WARN) $(ASAN_FLAGS) $(INCLUDES) $(COMMANDS_SRCS) -o $@ + # --- Valgrind --------------------------------------------------------------- valgrind: check-submodules $(BUILD)/vg_secret_qr $(BUILD)/vg_base64 $(BUILD)/vg_totp \ - $(BUILD)/vg_storage + $(BUILD)/vg_storage $(BUILD)/vg_commands @echo "== running harnesses under valgrind ==" valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_secret_qr valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_base64 valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_totp valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_storage + valgrind --error-exitcode=99 --leak-check=full -q $(BUILD)/vg_commands @echo "== valgrind OK ==" $(BUILD)/vg_secret_qr: $(SECRET_QR_SRCS) | $(BUILD) @@ -159,6 +171,9 @@ $(BUILD)/vg_totp: $(TOTP_SRCS) | $(BUILD) $(BUILD)/vg_storage: $(STORAGE_SRCS) | $(BUILD) $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) -o $@ +$(BUILD)/vg_commands: $(COMMANDS_SRCS) | $(BUILD) + $(CC) $(CSTD) $(WARN) $(VG_FLAGS) $(INCLUDES) $(COMMANDS_SRCS) -o $@ + # --- Coverage (lcov/genhtml HTML report for the WHOLE first-party codebase) -- # Build & run the real harnesses with --coverage (producing .gcda), AND compile # every first-party .c instrumented but unlinked (compile-only, producing .gcno @@ -178,11 +193,14 @@ coverage: check-submodules | $(COV_DIR)/obj -o $(COV_DIR)/obj/cov_totp $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(STORAGE_DEFS) $(INCLUDES) $(STORAGE_SRCS) \ -o $(COV_DIR)/obj/cov_storage + $(CC) $(CSTD) $(WARN) $(COV_FLAGS) $(INCLUDES) $(COMMANDS_SRCS) \ + -o $(COV_DIR)/obj/cov_commands @echo "== coverage: running instrumented harnesses ==" $(COV_DIR)/obj/cov_secret_qr $(COV_DIR)/obj/cov_base64 $(COV_DIR)/obj/cov_totp $(COV_DIR)/obj/cov_storage + $(COV_DIR)/obj/cov_commands @echo "== coverage: compiling whole first-party codebase (compile-only) ==" @for src in $(WHOLE_SRCS); do \ obj=$(COV_DIR)/obj/whole_$$(echo $$src | tr '/.' '__').o; \ diff --git a/test/harness_commands.c b/test/harness_commands.c new file mode 100644 index 0000000..1026b58 --- /dev/null +++ b/test/harness_commands.c @@ -0,0 +1,271 @@ +// Host harness for serial/commands.c — the command dispatcher. +// +// commands.c is a self-contained dispatch layer: given a pre-tokenized +// argc/argv (the tokenizer itself lives in serial/console.c, not here), it +// does a linear name lookup in COMMANDS[], enforces admin gating, validates +// the argument count against each command's [min_args, max_args], and routes +// to the matching handler — or prints an error for unknown commands. +// +// We link commands.c against SPY handler stubs (one per command, recording the +// last routed handler + argc) and a stub buzzer, then drive commands_dispatch() +// with a table of representative lines. Each line's outcome is observable via: +// - which spy handler ran (or none, for a rejected line), +// - the buzzer-ack count (dispatch acks on help/reject/unknown paths), and +// - the captured stdout, whose error prefix distinguishes the reject reason. +// This exercises every branch of commands_dispatch WITHOUT pulling in the real +// key/system/network/backup handlers (which touch flash, wifi and USB). + +// _POSIX_C_SOURCE: -std=c11 hides fileno/dup/dup2/close; we need them to +// redirect stdout for output capture. +#define _POSIX_C_SOURCE 200809L + +#include "commands.h" +#include "commands_handlers.h" + +#include +#include +#include +#include +#include +#include + +// admin_mode is defined (non-static) in commands.c; drive it directly so we can +// test the admin gate without going through the real cmd_login/TOTP path. +extern bool admin_mode; + +// --------------------------------------------------------------------------- +// Spies +// --------------------------------------------------------------------------- + +static const char *g_last_handler; // name of the last spy handler that ran +static int g_last_argc; // argc it was called with +static int g_buzzer_acks; // buzzer_play_command_ack() call count + +static void spy_reset(void) { + g_last_handler = NULL; + g_last_argc = -1; + g_buzzer_acks = 0; +} + +void buzzer_play_command_ack(void) { + g_buzzer_acks++; +} + +// One spy per real handler prototype in commands_handlers.h. Each records that +// it was the routed handler; commands.c only ever calls the one it looked up. +#define SPY(fn) \ + void fn(int argc, char **argv) { \ + (void)argv; \ + g_last_handler = #fn; \ + g_last_argc = argc; \ + } + +SPY(cmd_status) +SPY(cmd_get_time) +SPY(cmd_test) +SPY(cmd_login) +SPY(cmd_logout) +SPY(cmd_reboot) +SPY(cmd_sync_ntp) +SPY(cmd_set_wifi) +SPY(cmd_list_keys) +SPY(cmd_get_key) +SPY(cmd_get_key_secret) +SPY(cmd_add_key) +SPY(cmd_rename_key) +SPY(cmd_enable_key) +SPY(cmd_disable_key) +SPY(cmd_delete_key) +SPY(cmd_set_key_admin) +SPY(cmd_unset_key_admin) +SPY(cmd_export_keys) +SPY(cmd_import_keys) + +// --------------------------------------------------------------------------- +// Dispatch driver with stdout capture +// --------------------------------------------------------------------------- + +static char g_captured[8192]; + +// Build argv from up to 8 string tokens (NULL-terminated), redirect stdout to a +// temp file for the duration of the dispatch, and stash what was printed in +// g_captured so tests can assert on the error prefix. +static void run(const char *tok0, ...) { + char *argv[8]; + int argc = 0; + const char *t = tok0; + __builtin_va_list ap; + __builtin_va_start(ap, tok0); + while (t != NULL && argc < 8) { + argv[argc++] = (char *)t; + t = __builtin_va_arg(ap, const char *); + } + __builtin_va_end(ap); + + spy_reset(); + + fflush(stdout); + int saved = dup(fileno(stdout)); + FILE *tmp = tmpfile(); + assert(tmp != NULL); + fflush(stdout); + dup2(fileno(tmp), fileno(stdout)); + + commands_dispatch(argc, argv); + + fflush(stdout); + dup2(saved, fileno(stdout)); + close(saved); + + rewind(tmp); + size_t n = fread(g_captured, 1, sizeof(g_captured) - 1, tmp); + g_captured[n] = '\0'; + fclose(tmp); +} + +static bool out_has(const char *needle) { + return strstr(g_captured, needle) != NULL; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +static void test_routes_public_command(void) { + // "status" is public, 0..0 args -> routed regardless of admin state. + admin_mode = false; + run("status", (const char *)NULL); + assert(g_last_handler != NULL && strcmp(g_last_handler, "cmd_status") == 0); + assert(g_last_argc == 1); +} + +static void test_help_and_alias_route_same_handler(void) { + // "help" and its "?" alias both map to cmd_help (static, inside commands.c). + // cmd_help is not a spy — it runs in commands.c and acks the buzzer once. + admin_mode = false; + run("help", (const char *)NULL); + assert(g_last_handler == NULL); // internal handler, no spy + assert(g_buzzer_acks == 1); // cmd_help -> buzzer_play_command_ack + assert(out_has("available commands")); + + run("?", (const char *)NULL); + assert(g_buzzer_acks == 1); + assert(out_has("available commands")); +} + +static void test_help_hides_admin_rows_until_admin(void) { + admin_mode = false; + run("help", (const char *)NULL); + assert(!out_has("logout")); // admin-only row hidden + admin_mode = true; + run("help", (const char *)NULL); + assert(out_has("logout")); // now shown + admin_mode = false; +} + +static void test_unknown_command(void) { + admin_mode = false; + run("frobnicate", (const char *)NULL); + assert(g_last_handler == NULL); + assert(g_buzzer_acks == 1); + assert(out_has("unknown command")); +} + +static void test_empty_command_string(void) { + // Empty command name reaches dispatch as argv[0]="" -> unknown path. + admin_mode = false; + run("", (const char *)NULL); + assert(g_last_handler == NULL); + assert(out_has("unknown command")); +} + +static void test_admin_gate_blocks_when_not_admin(void) { + // "logout" requires admin and takes 0 args; when not admin it must be + // rejected on the admin gate BEFORE the handler runs. + admin_mode = false; + run("logout", (const char *)NULL); + assert(g_last_handler == NULL); + assert(g_buzzer_acks == 1); + assert(out_has("requires admin mode")); +} + +static void test_admin_gate_allows_when_admin(void) { + admin_mode = true; + run("logout", (const char *)NULL); + assert(g_last_handler != NULL && strcmp(g_last_handler, "cmd_logout") == 0); + admin_mode = false; +} + +static void test_argc_too_few(void) { + // "login " needs exactly 2 user args; give it 1. + admin_mode = false; + run("login", "onlyone", (const char *)NULL); + assert(g_last_handler == NULL); + assert(g_buzzer_acks == 1); + assert(out_has("usage:")); +} + +static void test_argc_too_many(void) { + // login with 3 user args exceeds max_args (2). + admin_mode = false; + run("login", "a", "b", "c", (const char *)NULL); + assert(g_last_handler == NULL); + assert(out_has("usage:")); +} + +static void test_argc_exact_ok(void) { + admin_mode = false; + run("login", "keyid", "123456", (const char *)NULL); + assert(g_last_handler != NULL && strcmp(g_last_handler, "cmd_login") == 0); + assert(g_last_argc == 3); +} + +static void test_admin_command_with_args_routes_when_admin(void) { + // "add-key " — admin-only, exactly 2 args. Exercises the admin + // gate PASS + argc PASS + route path together. + admin_mode = true; + run("add-key", "5", "frontdoor", (const char *)NULL); + assert(g_last_handler != NULL && strcmp(g_last_handler, "cmd_add_key") == 0); + assert(g_last_argc == 3); + admin_mode = false; +} + +static void test_admin_gate_precedes_argc_check(void) { + // Even with a wrong argc, a non-admin caller of an admin command is + // rejected on the admin gate (the gate is checked first in commands.c). + admin_mode = false; + run("add-key", "onlyone", (const char *)NULL); + assert(g_last_handler == NULL); + assert(out_has("requires admin mode")); + assert(!out_has("usage:")); +} + +static void test_on_disconnect_clears_admin(void) { + admin_mode = true; + assert(commands_is_admin()); + commands_on_disconnect(); // must clear admin + print notice + assert(!admin_mode); + assert(!commands_is_admin()); + // Second call is a no-op (admin already false) — exercises the false branch. + commands_on_disconnect(); + assert(!commands_is_admin()); +} + +int main(void) { + test_routes_public_command(); + test_help_and_alias_route_same_handler(); + test_help_hides_admin_rows_until_admin(); + test_unknown_command(); + test_empty_command_string(); + test_admin_gate_blocks_when_not_admin(); + test_admin_gate_allows_when_admin(); + test_argc_too_few(); + test_argc_too_many(); + test_argc_exact_ok(); + test_admin_command_with_args_routes_when_admin(); + test_admin_gate_precedes_argc_check(); + test_on_disconnect_clears_admin(); + + printf("harness_commands: all assertions passed\n"); + return 0; +} From 648ee943cc71a38dbe9444b48e3a6cdbec694100 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 15:46:21 +0200 Subject: [PATCH 5/7] ci: install libmbedtls-dev and run whole-codebase coverage in the test job Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/pages.yml | 7 ++++--- .github/workflows/test.yml | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index c78667b..4dff8c0 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -5,7 +5,7 @@ name: pages # not entangle. The report is regenerated on every push to the branches below. on: push: - branches: [chore/host-tests-suite, master] + branches: [chore/host-tests-suite, chore/whole-coverage, master] # Least privilege for the Pages deployment via the job's OIDC token. permissions: @@ -26,8 +26,9 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - # gcc/make are preinstalled; lcov (and valgrind) are not. - - run: sudo apt-get update && sudo apt-get install -y valgrind lcov + # gcc/make are preinstalled; lcov (and valgrind) are not. libmbedtls-dev + # provides the real + -lmbedcrypto the TOTP harness links. + - run: sudo apt-get update && sudo apt-get install -y valgrind lcov libmbedtls-dev # Run harnesses + (re)generate test/coverage/html/ via the test Makefile. - run: make -C test coverage # Enable Pages via the job token (no admin CLI scope needed). diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a92d79a..94bc675 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,7 +5,7 @@ name: test # PR's ci wrapper script, so this suite shares no files with the format PR. on: push: - branches: [chore/host-tests-suite, master] + branches: [chore/host-tests-suite, chore/whole-coverage, master] pull_request: jobs: @@ -16,8 +16,9 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - # gcc/make are preinstalled; valgrind + lcov are not. - - run: sudo apt-get update && sudo apt-get install -y valgrind lcov + # gcc/make are preinstalled; valgrind + lcov are not. libmbedtls-dev + # provides the real + -lmbedcrypto the TOTP harness links. + - run: sudo apt-get update && sudo apt-get install -y valgrind lcov libmbedtls-dev # Hard gate: ASan+UBSan, then Valgrind. Coverage is report-only but must # still build/run cleanly. Each step must exit 0 or the job fails. - run: make -C test asan From 849a702bd251a9eeda4f5806bb4f28249b1aceb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 15:48:22 +0200 Subject: [PATCH 6/7] ci: do not trigger pages publish from chore/whole-coverage The github-pages environment is branch-protected to the default branch, so a deploy from this branch is rejected regardless of the workflow. The coverage build itself is already exercised by the test job; keep libmbedtls-dev in pages.yml for when it runs on master. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/pages.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index 4dff8c0..97e0bab 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -5,7 +5,7 @@ name: pages # not entangle. The report is regenerated on every push to the branches below. on: push: - branches: [chore/host-tests-suite, chore/whole-coverage, master] + branches: [chore/host-tests-suite, master] # Least privilege for the Pages deployment via the job's OIDC token. permissions: From c0f4b86d652755c1e400d1e59c7f4c9df89b838e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 19:15:20 +0200 Subject: [PATCH 7/7] test: compile the new hardware/led.c under whole-codebase coverage (+ pico/unique_id stub) Co-Authored-By: Claude Opus 4.8 (1M context) --- test/Makefile | 2 +- test/stub/pico/unique_id.h | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 test/stub/pico/unique_id.h diff --git a/test/Makefile b/test/Makefile index b7b6334..8f70f28 100644 --- a/test/Makefile +++ b/test/Makefile @@ -76,7 +76,7 @@ COMMANDS_SRCS := harness_commands.c $(ROOT)/serial/commands.c # stand in for the pico-sdk / cyw43 / lwip / mbedtls headers they pull. WHOLE_SRCS := $(ROOT)/hardware/buzzer.c $(ROOT)/hardware/clock.c \ $(ROOT)/hardware/keypad.c $(ROOT)/hardware/latch.c \ - $(ROOT)/hardware/light.c $(ROOT)/main.c \ + $(ROOT)/hardware/led.c $(ROOT)/hardware/light.c $(ROOT)/main.c \ $(ROOT)/network/ntp.c $(ROOT)/network/wifi.c \ $(ROOT)/serial/commands.c $(ROOT)/serial/commands_backup.c \ $(ROOT)/serial/commands_keys.c $(ROOT)/serial/commands_network.c \ diff --git a/test/stub/pico/unique_id.h b/test/stub/pico/unique_id.h new file mode 100644 index 0000000..dae442b --- /dev/null +++ b/test/stub/pico/unique_id.h @@ -0,0 +1,20 @@ +#ifndef STUB_PICO_UNIQUE_ID_H +#define STUB_PICO_UNIQUE_ID_H + +/* + * Host stub for : the per-board flash unique-id read used by + * the `status` command. On real hardware this is the 64-bit id of the on-board + * flash chip; here we only need the type, the size macro and the prototype. + */ + +#include + +#define PICO_UNIQUE_BOARD_ID_SIZE_BYTES 8 + +typedef struct { + uint8_t id[PICO_UNIQUE_BOARD_ID_SIZE_BYTES]; +} pico_unique_board_id_t; + +void pico_get_unique_board_id(pico_unique_board_id_t *id_out); + +#endif