From 671da3160505c7336f6240754426cadcd9822619 Mon Sep 17 00:00:00 2001 From: eddy Date: Tue, 4 Aug 2026 21:56:05 +0200 Subject: [PATCH] fix: address critical security issues discovered during code review --- .../content/broker_flow.html | 53 +++ .../content/ota_options.html | 48 +++ .../brainstorm/124687-1785870282/state/events | 2 + .../124687-1785870282/state/server-stopped | 1 + .../124687-1785870282/state/server.log | 7 + .../124687-1785870282/state/server.pid | 1 + .../plans/2026-08-04-code-review-fixes-v2.md | 305 ++++++++++++++++++ 7 files changed, 417 insertions(+) create mode 100644 .superpowers/brainstorm/124687-1785870282/content/broker_flow.html create mode 100644 .superpowers/brainstorm/124687-1785870282/content/ota_options.html create mode 100644 .superpowers/brainstorm/124687-1785870282/state/events create mode 100644 .superpowers/brainstorm/124687-1785870282/state/server-stopped create mode 100644 .superpowers/brainstorm/124687-1785870282/state/server.log create mode 100644 .superpowers/brainstorm/124687-1785870282/state/server.pid create mode 100644 docs/superpowers/plans/2026-08-04-code-review-fixes-v2.md diff --git a/.superpowers/brainstorm/124687-1785870282/content/broker_flow.html b/.superpowers/brainstorm/124687-1785870282/content/broker_flow.html new file mode 100644 index 0000000..209e217 --- /dev/null +++ b/.superpowers/brainstorm/124687-1785870282/content/broker_flow.html @@ -0,0 +1,53 @@ +

Voorgestelde Flow: De "Server-as-Broker" methode

+

Gebruik de SylDa_Server als ontmoetingsplaats voor versie-informatie

+ +
+
Interactie Diagram
+
+
// 1. Client meldt zich aan
+ [Client] --(Write version: "5")--> [Server: ClientStatusChar] + [Client] --(Subscribe)------------> [Server: ClientControlChar] + +
// 2. App checkt status
+ [App] --(Read ClientStatus)----> [Server] + App denkt: "Client heeft v5, ik heb v6. Update nodig!" + +
// 3. App start de dans
+ [App] --(Write: "START_OTA")---> [Server: ClientControlChar] + [Server] --(Notify)---------------> [Client] + +
// 4. De wissel
+ Client disconnect van Server -> Start Advertising + App disconnect van Server -> Scant naar Client OTA + +
// 5. De eigenlijke OTA
+ [App] --(Direct BLE OTA)-------> [Client (nu in Server mode)] +
+
+ +
+

Technische details om te bespreken:

+
+
+
1
+
+

UUID Tracking

+

Aangezien de Server 2 clients ondersteunt, moet de Server de versie per UUID opslaan zodat de App weet welke client welke versie heeft.

+
+
+
+
2
+
+

Beveiliging (HMAC/AES)

+

Moet de "START_OTA" opdracht ook versleuteld zijn? (Waarschijnlijk wel, om te voorkomen dat een vreemde de clients platlegt).

+
+
+
+
3
+
+

Auto-reconnect

+

Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.

+
+
+
+
diff --git a/.superpowers/brainstorm/124687-1785870282/content/ota_options.html b/.superpowers/brainstorm/124687-1785870282/content/ota_options.html new file mode 100644 index 0000000..fe868c1 --- /dev/null +++ b/.superpowers/brainstorm/124687-1785870282/content/ota_options.html @@ -0,0 +1,48 @@ +

Ontwerpkeuzes BLE OTA voor de Client

+

Hoe krijgt de Client (ESP32) de nieuwe firmware?

+ +
+
+
A
+
+

Android App Direct

+

De Client ESP32 start een tijdelijke BLE Server (OTA Service). De Android app verbindt direct met de Client om de firmware te pushen.

+
Voordeel: Simpel, hergebruikt Android OTA code.
+
Nadeel: Handmatige actie nodig op Client (bijv. knop indrukken).
+
+
+ +
+
B
+
+

Server Proxy / Relay

+

De Android app stuurt de firmware naar de SylDa_Server. De Server stuurt het door naar de Client via de bestaande BLE verbinding.

+
Voordeel: Geen directe verbinding Android-Client nodig.
+
Nadeel: Complexer, Server moet grote buffers hebben of flow control.
+
+
+ +
+
C
+
+

Client Pull (via Server)

+

De Server kondigt een update aan. De Client haalt de firmware "chunks" op bij de Server via een nieuwe OTA service op de Server.

+
Voordeel: Client heeft controle over tempo.
+
Nadeel: Server moet de firmware ergens opslaan (Flash/NVS/SPIFFS?) voordat hij het kan doorgeven.
+
+
+
+ +
+

Huidige Situatie

+
+
Architectuur
+
+ 
+[Android App] --(BLE OTA)--> [SylDa_Server (ESP32)]
+      |
+      ? --(BLE OTA?)--> [SylDa_Client (ESP32)]
+
+
+
+
diff --git a/.superpowers/brainstorm/124687-1785870282/state/events b/.superpowers/brainstorm/124687-1785870282/state/events new file mode 100644 index 0000000..93cf1e4 --- /dev/null +++ b/.superpowers/brainstorm/124687-1785870282/state/events @@ -0,0 +1,2 @@ +{"type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849065} +{"type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849785} diff --git a/.superpowers/brainstorm/124687-1785870282/state/server-stopped b/.superpowers/brainstorm/124687-1785870282/state/server-stopped new file mode 100644 index 0000000..e14c9e8 --- /dev/null +++ b/.superpowers/brainstorm/124687-1785870282/state/server-stopped @@ -0,0 +1 @@ +{"reason":"idle timeout","timestamp":1785872682901} diff --git a/.superpowers/brainstorm/124687-1785870282/state/server.log b/.superpowers/brainstorm/124687-1785870282/state/server.log new file mode 100644 index 0000000..ac61bb5 --- /dev/null +++ b/.superpowers/brainstorm/124687-1785870282/state/server.log @@ -0,0 +1,7 @@ +{"type":"server-started","port":56787,"host":"0.0.0.0","url_host":"100.97.199.84","url":"http://100.97.199.84:56787","screen_dir":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/content","state_dir":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/state"} +{"type":"screen-added","file":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/content/ota_options.html"} +{"source":"user-event","type":"click","text":"A\n \n Android App Direct\n De Client ESP32 start een tijdelijke BLE Server (OTA Service). De Android app verbindt direct met de Client om de firmware te pushen.\n Voordeel: Simpel, hergebruikt Android OTA code.\n Nadeel: Handmatige actie nodig op Client (bijv. knop indrukken).","choice":"android-direct","id":null,"timestamp":1785870382036} +{"type":"screen-added","file":"/home/eddy/src/Project_SylDa/.superpowers/brainstorm/124687-1785870282/content/broker_flow.html"} +{"source":"user-event","type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849065} +{"source":"user-event","type":"click","text":"3\n \n Auto-reconnect\n Na de update (of bij falen/timeout) moet de Client weer terug in zijn normale rol vallen en de Server zoeken.","choice":"reconnect","id":null,"timestamp":1785870849785} +{"type":"server-stopped","reason":"idle timeout"} diff --git a/.superpowers/brainstorm/124687-1785870282/state/server.pid b/.superpowers/brainstorm/124687-1785870282/state/server.pid new file mode 100644 index 0000000..24f92f7 --- /dev/null +++ b/.superpowers/brainstorm/124687-1785870282/state/server.pid @@ -0,0 +1 @@ +124706 diff --git a/docs/superpowers/plans/2026-08-04-code-review-fixes-v2.md b/docs/superpowers/plans/2026-08-04-code-review-fixes-v2.md new file mode 100644 index 0000000..b500431 --- /dev/null +++ b/docs/superpowers/plans/2026-08-04-code-review-fixes-v2.md @@ -0,0 +1,305 @@ +# Code Review Fixes (Batch 2) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development +> (recommended) or superpowers:executing-plans to implement this plan task-by-task. +> Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix 6 issues found in code review: broken client OTA UI flow (SettingsActivity button), missing timeout in OTA polling, recomputeClientOta race condition, committed upload_port, wrong LED startup sequence, and noisy ESP_LOGI diagnostics. + +**Architecture:** All fixes are isolated single-file edits. No new files. No architecture changes. Version bumps: server 21→22, client 11→12. + +**Tech Stack:** C++ (Arduino/ESP-IDF), Kotlin (Android), PlatformIO + +--- + +### Task 1: Fix SettingsActivity button — client+BLE now routes to startClientOtaUpdate + +**Files:** +- Modify: `android/app/src/main/java/nl/eddydevink/sylda/SettingsActivity.kt:167-174` + +- [ ] **Step 1: Replace button click handler** + +In `setupButtons()` (or wherever the click listener is set), replace lines 167-174 with the corrected logic: + +```kotlin +findViewById