fix: Recovery-Rettungsweg gegen Endlosschleife, Werksreset und Fremdnetz absichern

C1: nl_recovery_try() verglich nichts mit der laufenden Firmware. Vor einer
laufenden Rettungsstation - laut Spec 9 der Normalfall, solange eine Charge
migriert - lud die Lampe bei jedem Boot dasselbe Image erneut und startete neu.
nl_ota_start_recovery() setzt jetzt ein Flag, das nl_fwup_verify_tail() nur im
Recovery-Pfad auswerten lässt: Trägt der Trailer dieselbe Version wie die
laufende Firmware (nl_trailer_version(), bisher ungenutzt), wird das Image
abgelehnt und der Boot läuft normal weiter. Ein absichtliches Neuflashen
derselben Version über die REST-API bleibt möglich. Kein neuer Wert im
last_ota_error-Vokabular (Spec 8.2); unterschieden wird über die Konsole.

C1/I2: boot_ms wurde erst nach nl_recovery_try() gesetzt und der
Power-Cycle-Zähler erst 5 s danach gelöscht - bei erfolgreicher Recovery also
nie. Nach NL_FACTORY_RESET_BOOTS Runden folgte ein Werksreset. boot_ms startet
jetzt am Anfang von app_task (damit gilt wieder das in Spec 3.4/5.5 zugesagte
5-s-Fenster statt faktisch ~8 s), und nl_app_clear_reset_counter() löscht den
Zähler zusätzlich, sobald die Rettungsstation gefunden ist.

I3: tls_sys_auto_mode_run() stößt direkt nach UserMain() einen SDK-eigenen
Verbindungsversuch an; das dabei entstehende NETIF_IP_NET_UP war in
recovery_join_event() nicht vom eigenen Join zu unterscheiden, die Lampe hätte
im falschen Netz geladen. nl_wifi_disable_sdk_auto_connect() schaltet das Flag
ab und kappt eine bestehende Verbindung jetzt am Anfang von nl_recovery_try();
zusätzlich prüft nl_wifi_join_blocking() über tls_wifi_get_current_bss(), dass
die Verbindung wirklich zur erwarteten SSID gehört.

src/app_main.c

@@ -33,6 +33,7 @@ static tls_os_mutex_t *s_store_lock; /* serialises all nl_settings_* / nl_state_
 static nl_settings_t s_settings;
 static volatile uint32_t s_light_beat;
 static volatile bool s_factory_reset_requested;
+static volatile bool s_counter_cleared;
 static u32 s_app_stack[1024];
 static u32 s_light_stack[512];
 
@@ -88,6 +89,15 @@ void nl_app_reboot(void)
 
 void nl_app_request_factory_reset(void) { s_factory_reset_requested = true; }
 
+void nl_app_clear_reset_counter(void)
+{
+    if (s_counter_cleared)
+        return;
+    store_lock();
+    s_counter_cleared = nl_resetcnt_clear() == 0;
+    store_unlock();
+}
+
 int nl_app_set_wifi_credentials(const char *ssid, const char *pass)
 {
     nl_app_lock();
@@ -149,12 +159,15 @@ static void app_task(void *arg)
     uint32_t base = nl_flash_region_base();
     nl_engine_cfg_t cfg;
     nl_light_state_t last, initial, pending;
-    bool have_last, after_ota, counter_cleared = false;
+    bool have_last, after_ota;
     bool light_alive = false, net_alive = false, http_alive = false;
     uint8_t boots;
     uint32_t boot_ms, light_seen, net_seen, http_seen;
 
     (void)arg;
+    /* I2: the 5 s of spec 3.4 step 7 / 5.5 count from power-up, not from "after the recovery
+     * scan" - otherwise the documented "< 5 s per power cycle" window silently becomes ~8 s. */
+    boot_ms = nl_app_now_ms();
     nl_flash_init();
     nl_led_init(board); /* hardware init first (spec 3.4): LEDs dark before any flash/state scan */
     store_lock();
@@ -217,7 +230,6 @@ static void app_task(void *arg)
 
     nl_http_server_start(nl_app_api_ctx());
 
-    boot_ms = nl_app_now_ms();
     light_seen = s_light_beat;
     net_seen = nl_net_beat;
     http_seen = nl_http_beat;
@@ -228,11 +240,8 @@ static void app_task(void *arg)
         tls_os_time_delay(HZ / 10); /* 100 ms */
         now = nl_app_now_ms();
 
-        if (!counter_cleared && now - boot_ms >= NL_RESETCNT_CLEAR_MS) {
+        if (!s_counter_cleared && now - boot_ms >= NL_RESETCNT_CLEAR_MS)
-            store_lock();
+            nl_app_clear_reset_counter();
-            counter_cleared = nl_resetcnt_clear() == 0;
-            store_unlock();
-        }
 
         if (s_factory_reset_requested)
             nl_app_factory_reset(); /* does not return */

src/net/nl_recovery.c

@@ -109,9 +109,21 @@ void nl_recovery_try(void)
 {
     char url[128];
 
+    /* I3: the SDK's auto-connect may already be joining its own stored AP in parallel (see
+       nl_wifi_disable_sdk_auto_connect()). Switch it off and drop that connection before we
+       scan, so everything that follows is our own doing. */
+    nl_wifi_disable_sdk_auto_connect();
+
     if (!recovery_ap_present())
         return;
     printf("recovery: station found, joining\r\n");
+    /* C1: from here on this boot is a recovery boot - it may end in a flash + reset that never
+       returns to app_task, so the power-cycle counter must be cleared now instead of after the
+       5 s deadline. Otherwise every recovery round would count as another short power cycle and
+       NL_FACTORY_RESET_BOOTS rounds in front of the station would wipe Wi-Fi, token and name.
+       Trade-off: while a rescue station is in range, the "5 x power cycle" factory reset does
+       not work - a lamp in front of the station is being serviced, not being reset. */
+    nl_app_clear_reset_counter();
     if (!nl_wifi_join_blocking(NINLIGHT_RECOVERY_SSID, NINLIGHT_RECOVERY_PASSWORD, NL_RECOVERY_JOIN_MS)) {
         printf("recovery: join failed\r\n");
         nl_ota_set_error("download");
@@ -119,7 +131,9 @@ void nl_recovery_try(void)
     }
     snprintf(url, sizeof url, "http://192.168.8.1/ninlight_%s_ota.img", NINLIGHT_DEVICE_ID);
     printf("recovery: fetching %s\r\n", url);
-    if (nl_ota_start(url) != 0) {
+    /* C1: recovery refuses an image that carries the running version - the station serves the
+       same file to every lamp and stays up for a whole migration batch. */
+    if (nl_ota_start_recovery(url) != 0) {
         printf("recovery: cannot start download\r\n");
         return;
     }

src/net/nl_wifi.h

@@ -21,6 +21,11 @@ extern volatile uint32_t nl_net_beat;
 
 /* Starts the network task. An empty ssid starts the onboarding AP. Strings are copied. */
 void nl_wifi_start(const char *ssid, const char *pass, const char *ap_pass);
+/* Switches the SDK's own auto-connect off and drops any connection it may already have brought
+ * up on its own. nl_wifi_start() does this too, but the boot recovery path runs before that and
+ * must not have the SDK joining its stored AP in parallel (I3, final review). Safe to call any
+ * number of times; only writes the SDK parameter when it is actually set. */
+void nl_wifi_disable_sdk_auto_connect(void);
 /* Joins a station directly (no nl_wifi_sm) and blocks until it gets an IP or timeout_ms
  * elapses; returns true on success. For use before nl_wifi_start() only - e.g. the boot
  * recovery path in nl_recovery.c - since it drives the SDK Wi-Fi calls itself instead of

src/net/nl_wifi_w600.c

@@ -69,6 +69,25 @@ static void clear_softap_param(void)
     }
 }
 
+/* I3: tls_sys_auto_mode_run() (sdk/Platform/Sys/tls_sys.c) kicks off an SDK-driven join right
+ * after UserMain() whenever the persisted auto-connect flag is on - and tls_sys.c:264-267 turns a
+ * temporary WIFI_AUTO_CNT_TMP_OFF back into WIFI_AUTO_CNT_ON and persists that, so the factory
+ * default "off" is not something we may rely on. Any such join is indistinguishable from our own
+ * in the netif status events, so switch it off (and drop a connection it already made) before the
+ * recovery scan, not only in nl_wifi_start(). */
+void nl_wifi_disable_sdk_auto_connect(void)
+{
+    u8 auto_mode = WIFI_AUTO_CNT_OFF;
+
+    tls_wifi_auto_connect_flag(WIFI_AUTO_CNT_FLAG_GET, &auto_mode);
+    if (auto_mode != WIFI_AUTO_CNT_OFF) {
+        printf("wifi: SDK auto-connect was %u, switching it off\r\n", auto_mode);
+        auto_mode = WIFI_AUTO_CNT_OFF;
+        tls_wifi_auto_connect_flag(WIFI_AUTO_CNT_FLAG_SET, &auto_mode);
+    }
+    tls_wifi_disconnect();
+}
+
 static void start_ap(void)
 {
     struct tls_softap_info_t ap;
@@ -220,7 +239,6 @@ static void net_task(void *arg)
 
 void nl_wifi_start(const char *ssid, const char *pass, const char *ap_pass)
 {
-    u8 auto_mode = WIFI_AUTO_CNT_OFF;
     nl_wifi_actions_t a;
     struct netif *nif;
 
@@ -233,12 +251,7 @@ void nl_wifi_start(const char *ssid, const char *pass, const char *ap_pass)
     copy(s_ap_pass, ap_pass, sizeof s_ap_pass);
 
     /* The SDK would otherwise reconnect on its own with credentials from its parameter area. */
-    tls_wifi_auto_connect_flag(WIFI_AUTO_CNT_FLAG_GET, &auto_mode);
+    nl_wifi_disable_sdk_auto_connect();
-    if (auto_mode != WIFI_AUTO_CNT_OFF) {
-        auto_mode = WIFI_AUTO_CNT_OFF;
-        tls_wifi_auto_connect_flag(WIFI_AUTO_CNT_FLAG_SET, &auto_mode);
-    }
-    tls_wifi_disconnect();
     if (tls_wifi_softap_get_state() != WM_WIFI_DISCONNECTED) {
         /* C2: the SDK already brought up its own softap (e.g. from NETIF_IP_NET_UP before
          * we got a chance to run) - tear it down, our state machine owns AP lifecycle. */
@@ -369,5 +382,21 @@ bool nl_wifi_join_blocking(const char *ssid, const char *pass, uint32_t timeout_
         tls_wifi_disconnect(); /* cancel a stuck/timed-out join attempt */
         return false;
     }
+    /* I3 backstop: NETIF_IP_NET_UP carries no SSID, so an SDK-initiated join to its own stored AP
+     * would look exactly like ours - and the lamp would then fetch http://192.168.8.1/... from a
+     * foreign network. tls_wifi_get_current_bss() does report the SSID actually associated with
+     * (sdk/Include/WiFi/wm_wifi.h:190), so verify it before the caller starts a download. */
+    {
+        struct tls_curr_bss_t bss;
+        size_t n = strlen(ssid);
+
+        memset(&bss, 0, sizeof bss);
+        tls_wifi_get_current_bss(&bss);
+        if (bss.ssid_len != (u8)n || memcmp(bss.ssid, ssid, n) != 0) {
+            printf("wifi: joined a different network than %s, dropping the connection\r\n", ssid);
+            tls_wifi_disconnect();
+            return false;
+        }
+    }
     return true;
 }

src/sys/nl_app.h

@@ -28,4 +28,10 @@ void nl_app_request_factory_reset(void);
 /* Stores new station credentials (from a successful onboarding test) and persists them. */
 int nl_app_set_wifi_credentials(const char *ssid, const char *pass);
 
+/* Clears the power-cycle counter ahead of the regular 5 s deadline (spec 5.5), idempotent.
+ * Used by the boot recovery path once it knows this boot is a recovery boot: a successful
+ * recovery never returns to the app task, so without this the counter would keep growing
+ * over recovery cycles and trigger a factory reset after NL_FACTORY_RESET_BOOTS rounds. */
+void nl_app_clear_reset_counter(void);
+
 #endif

src/sys/nl_ota.c

@@ -10,6 +10,8 @@
 #include "sys/nl_ota.h"
 #include "util/nl_fwup_trailer.h"
 
+#include "nl_version.h"
+
 /* t_http_fwup() (sdk/Src/App/OTA/wm_http_fwup.c) has no declaration in any SDK header. */
 extern int t_http_fwup(char *url);
 
@@ -19,8 +21,10 @@ extern int t_http_fwup(char *url);
 
 static char               s_url[NL_OTA_URL_MAX];
 static volatile bool      s_busy;
+static volatile bool      s_reject_running_version; /* recovery path only, see nl_ota_start_recovery() */
 static const char * volatile s_error;
 static u32                s_stack[NL_OTA_STACK];
+static char               s_tail_version[NL_TRAILER_VERSION_LEN]; /* used by the hook, kept off its 2 KB stack */
 
 void nl_ota_init(void)
 {
@@ -42,10 +46,28 @@ bool nl_fwup_verify_tail(const uint8_t sha256[32], const uint8_t *tail, uint32_t
 {
     nl_trailer_err_t e = nl_trailer_verify(tail, tail_len, NINLIGHT_DEVICE_ID, sha256);
 
-    if (e != NL_TRAILER_OK)
+    if (e != NL_TRAILER_OK) {
         s_error = nl_trailer_err_name(e);
-    printf("ota: trailer %s\r\n", e == NL_TRAILER_OK ? "ok" : s_error);
+        printf("ota: trailer %s\r\n", s_error);
-    return e == NL_TRAILER_OK;
+        return false;
+    }
+    /* C1: recovery only - the rescue station serves the same image to every lamp that boots in
+     * front of it, so an image that already matches the running version must be refused here;
+     * otherwise the lamp flashes and reboots in an endless loop. No new last_ota_error value is
+     * invented for this (the vocabulary is fixed in spec 8.2); the log line below is what tells
+     * the two cases apart. */
+    if (s_reject_running_version) {
+        nl_trailer_version(tail, s_tail_version);
+        if (strcmp(s_tail_version, NINLIGHT_FW_VERSION) == 0) {
+            printf("ota: recovery image already carries the running version %s, not flashing\r\n",
+                   s_tail_version);
+            s_error = "trailer";
+            return false;
+        }
+        printf("ota: recovery image version %s (running %s)\r\n", s_tail_version, NINLIGHT_FW_VERSION);
+    }
+    printf("ota: trailer ok\r\n");
+    return true;
 }
 
 /* Logs the URL without any "user:pass@" userinfo, so credentials embedded in the URL never
@@ -83,7 +105,7 @@ static void ota_task(void *arg)
     tls_os_task_del(NL_OTA_PRIO, NULL);
 }
 
-int nl_ota_start(const char *url)
+static int ota_start(const char *url, bool reject_running_version)
 {
     size_t n;
 
@@ -98,6 +120,7 @@ int nl_ota_start(const char *url)
     }
     s_busy = true;
     s_error = NULL;
+    s_reject_running_version = reject_running_version;
     memcpy(s_url, url, n + 1);
 
     /* Persist the open light state and mark the pending update before the flash gets busy. */
@@ -122,8 +145,17 @@ int nl_ota_start(const char *url)
         nl_app_unlock();
         (void)nl_app_save_settings();
         s_busy = false;
+        /* Minor (final review): no value in the fixed vocabulary of spec 8.2 describes "could not
+         * create the download task" (out of RAM/TCBs), and "download" would claim a transfer that
+         * never happened. `busy` stays the closest fit - the chip really is too busy to start one -
+         * and this log line is what distinguishes it from "an update is already running". */
+        printf("ota: cannot create download task (out of resources)\r\n");
         s_error = "busy";
         return -1;
     }
     return 0;
 }
+
+int nl_ota_start(const char *url) { return ota_start(url, false); }
+
+int nl_ota_start_recovery(const char *url) { return ota_start(url, true); }

src/sys/nl_ota.h

@@ -6,6 +6,11 @@
 void nl_ota_init(void);
 /* Starts a download task for an http:// URL. 0 = started, -1 = busy or invalid. */
 int nl_ota_start(const char *url);
+/* Same, but for the boot recovery path (spec 8.3): an image whose trailer carries the running
+ * firmware version is rejected instead of flashed. The rescue station stays up while a whole
+ * batch of lamps migrates, so without this check every boot would re-flash the same image
+ * forever. A deliberate re-flash of the same version stays possible over the REST API. */
+int nl_ota_start_recovery(const char *url);
 bool nl_ota_busy(void);
 /* NULL, or one of: download, crc, trailer, device_mismatch, hash, busy */
 const char *nl_ota_last_error(void);