add secure coding AI review guides

.coderabbit.yaml

@@ -6,6 +6,8 @@
 #   docs/cpp.instructions.md          — C++ coding conventions
 #   docs/web.instructions.md          — Web UI coding conventions
 #   docs/cicd.instructions.md         — GitHub Actions / CI-CD conventions
+#   docs/hardening.instructions.md    — basic rules for code hardening and robustness
+#   docs/securecode.instructions.md   — more detailed checklists for common vulnerabilities
 #
 # NOTE: This file must be committed (tracked by git) for CodeRabbit to read
 # it from the repository.  If it is listed in .gitignore, CodeRabbit will
@@ -53,15 +55,50 @@ reviews:
         5. VERIFY numerical stability / accuracy of fixed-point arithmetic.
         6. CHECK for implied but weakly justified assumptions - like usermod loop() call frequency - and ask for clarification.
 
+    # ── Security hardening — firmware (trust-boundary-aware) ────────────────
+    - path: "wled00/**/*.{cpp,h,hpp,ino}"
+      instructions: >
+        Apply the WLED security hardening rules from docs/hardening.instructions.md,
+        and consult docs/securecode.instructions.md when more details are needed for actionable recommendations.
+
+        Trust Boundary Model — enforce input-validation and bounds-checking rules
+        ONLY at the first untrusted ingress point. Untrusted ingress points are:
+          - HTTP/JSON API request bodies and query parameters (/json/*, /win, etc.)
+          - WebSocket message payloads
+          - UDP datagrams (parsePacket() / recvfrom() and protocol wrappers for
+            E1.31, DDP, Art-Net, TPM2.net)
+          - TCP socket reads
+          - Serial/UART command input
+          - ESP-NOW raw messages input
+
+        A value that has been validated and range-clamped at its ingress handler is
+        considered TRUSTED for all subsequent WLED core processing. Do NOT flag or suggest
+        repeated bounds/range checks or internal uses of already-sanitized data.
+        When it is unclear whether a value has been sanitized upstream, prefer
+        requesting clarification over raising a false-positive finding.
+
     - path: "wled00/data/**"
       instructions: >
         Follow the web UI conventions documented in docs/web.instructions.md.
 
         Key rules: indent HTML and JavaScript with tabs, CSS with tabs.
         Files here are built into wled00/html_*.h and wled00/js_*.h by tools/cdata.js — never
         edit those generated headers directly.
-# disabled - the below instruction has no effect
-# When initially reviewing a PR, summarize good practices (top 5) and create a prioritized list of suggested improvements (focus on major ones).
+
+    # ── Security hardening — WebUI (always an ingress/output surface) ────────
+    - path: "wled00/data/**"
+      instructions: >
+        Apply the WLED web UI security rules from docs/securecode.instructions.md
+        (sections WEB1-WEB7).
+
+        The Trust Boundary Model does NOT reduce scope here: the WebUI is both
+        an ingress point (user input, postMessage, fetched config data) and an
+        output/rendering surface. Always flag DOM XSS risks, unsafe
+        innerHTML / document.write / insertAdjacentHTML / outerHTML assignments,
+        postMessage handlers without origin validation, eval() / new Function(),
+        unsafe location.href or location.replace() assignments, and DOM insertion
+        from fetched or config-derived data — regardless of where the data
+        originates.
 
     - path: "wled00/html_*.h"
       instructions: >
@@ -81,8 +118,33 @@ reviews:
         Each usermod lives in its own directory under usermods/ and is implemented
         as a .cpp file with a dedicated library.json file to manage dependencies. 
         Follow the same C++ conventions as the core firmware (docs/cpp.instructions.md).
-# disabled - the below instruction has no effect
-# When initially reviewing a PR, summarize good practices (top 6) and create a prioritized list of suggested improvements (skip minor ones).
+
+    # ── Security hardening — usermods (trust-boundary-aware, narrow scope) ───
+    - path: "usermods/**/*.{cpp,h,hpp}"
+      instructions: >
+        For usermods, the untrusted ingress points are:
+          - readFromConfig(JsonObject& root) and calls to getJsonValue()
+          - readFromJsonState(JsonObject& obj)  — JSON is parsed, but values are client-supplied
+          - onMqttMessage(char* topic, char* payload)  — raw network strings, no core sanitization
+          - onEspNowMessage(uint8_t* sender, uint8_t* payload, uint8_t len)  — raw radio bytes
+          - onUdpPacket(uint8_t* payload, size_t len)  — raw UDP buffer, no core filtering
+        Values retrieved at these ingress points are considered trusted only after the
+        usermod itself has validated and range-clamped them.
+
+        Flag ONLY downstream uses of ingress-derived values where an out-of-range or
+        unexpected value can cause misbehaviour that is not already guarded, for example:
+          - `switch` statements on an ingress-derived value with no `default` branch,
+            or with a missing `break` where fall-through is unintentional
+          - array or buffer indexing with an ingress-derived value where the index is
+            not clamped before use
+          - arithmetic with an ingress-derived value that can overflow or produce a
+            negative result used as a size or count
+
+        Do NOT flag:
+          - getJsonValue() call sites themselves (type coercion is handled by ArduinoJson)
+          - Internal logic that operates on values already confirmed safe at ingress
+          - Repeated range checks on values that have already been clamped
+          - General memory-safety patterns unrelated to ingress-derived data flow
 
     - path: ".github/workflows/*.{yml,yaml}"
       instructions: >
@@ -94,8 +156,6 @@ reviews:
         scoped to least privilege. Never interpolate github.event.* values directly
         into run: steps — pass them through an env: variable to prevent script
         injection. Do not use pull_request_target unless fully justified.
-# disabled - the below instruction has no effect
-# When initially reviewing a PR, summarize good practices (top 6) and create a prioritized list of suggested improvements.
 
     - path: "**/*.instructions.md"
       instructions: |
@@ -113,6 +173,73 @@ reviews:
         3. If new AI-facing rules were added without updating a related HUMAN_ONLY
            reference section, note this as a suggestion (not a required fix).
 
+    # ── Secrets / sensitive information scanning ────────────────────────────
+    - path: "platformio*.ini*"
+      instructions: >
+        Scan for secrets, passwords, and other sensitive information accidentally
+        committed to PlatformIO configuration files (platformio.ini,
+        platformio_override.ini, platformio_override.ini.sample).
+
+        Flag any of the following:
+          - build_flags entries that define credentials as literal values, e.g.:
+              -DWIFI_SSID=\"<YOUR_SSID>\" -DWIFI_PASS=\"<YOUR_PASSWORD>\"
+              -DOTA_PASS=\"<OTA_PASSWORD>\" -DMQTT_PASS=\"<MQTT_PASSWORD>\"
+            Flag only when the value is not a recognisable placeholder (see below).
+          - upload_flags or upload_port values that embed a password or auth token (e.g., --auth=<PASSWORD> or any URL using credential-bearing userinfo).
+          - Any key = <value> pair whose key name contains "pass", "password",
+            "secret", "token", "key", "credential", or "auth" where the value is
+            a non-empty, non-placeholder literal string.
+          - Hardcoded IP addresses or hostnames paired with credentials in the
+            same environment section.
+          - API keys or access tokens as literal strings in any field.
+
+        Do NOT flag:
+          - Values that are clearly template placeholders (e.g., YOUR_SSID,
+            <YOUR_PASSWORD>, changeme, example_token, your_password_here).
+          - Values that use PlatformIO environment variable substitution (${sysenv.WIFI_PASS} or ${env:WIFI_PASS}).
+          - Comments that only explain what a field should contain.
+          - platformio_override.ini.sample entries that contain only
+            placeholder/example values.
+
+    - path: "usermods/**/library.json"
+      instructions: >
+        Scan for secrets and sensitive information in usermod dependency manifests.
+
+        Flag any of the following:
+          - Dependency URLs that embed credentials in the URL itself (e.g., any URL containing credential-bearing userinfo).
+          - Personal access tokens, OAuth tokens, or API keys as literal strings
+            anywhere in the file.
+          - Values matching well-known secret patterns: GitHub PATs (ghp_...,
+            github_pat_...), AWS access keys (AKIA...), or similarly structured
+            high-entropy tokens.
+
+        Do NOT flag:
+          - Plain HTTPS or SSH URLs without embedded credentials.
+          - Version specifiers, semver ranges, or commit SHA references that
+            contain no credential prefix.
+          - Repository owner/name path segments (not credential material).
+
+    - path: "usermods/**/{readme,README,Readme}.md"
+      instructions: >
+        Scan for secrets, passwords, and sensitive information in usermod
+        documentation files, including inside code blocks, inline code, and prose.
+
+        Flag any of the following:
+          - Hardcoded Wi-Fi SSID or password values that appear to be real (non-placeholder) 
+            strings in configuration or installation examples.
+          - Hardcoded OTA, AP, or MQTT passwords in code snippets or step-by-step
+            instructions.
+          - API keys, bearer tokens, or access tokens shown as literal values.
+          - Example platformio_override.ini snippets that contain real-looking
+            credential values instead of placeholders.
+          - Hardcoded IP addresses combined with credentials in the same example.
+
+        Do NOT flag:
+          - Values that are clearly template placeholders (e.g., YOUR_SSID,
+            <password>, my_secret, changeme, ****).
+          - Generic prose describing what a field means without supplying a value.
+          - Asterisk-masked values (e.g., ******, ••••••).
+
   finishing_touches:
     # Docstrings | Options for generating Docstrings for your PRs/MRs.
     docstrings:

AGENTS.md

@@ -4,7 +4,8 @@ WLED is C++ firmware for ESP32/ESP8266 microcontrollers controlling addressable
 with a web UI (HTML/JS/CSS). Built with PlatformIO (Arduino framework) and Node.js tooling.
 
 See also: `.github/copilot-instructions.md`, `.github/agent-build.instructions.md`,
-`docs/cpp.instructions.md`, `docs/web.instructions.md`, `docs/cicd.instructions.md`.
+`docs/cpp.instructions.md`, `docs/web.instructions.md`, `docs/cicd.instructions.md`,
+`docs/hardening.instructions.md`, `docs/securecode.instructions.md`.
 
 Always reference these instructions - including coding guidelines in `docs/` - first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here.
 
@@ -27,7 +28,7 @@ required C headers for firmware compilation.
 Tests use Node.js built-in test runner (`node:test`). The single test file is
 `tools/cdata-test.js`. Run it with:
 
-```sh
+```bash
 npm test                   # runs all tests via `node --test`
 node --test tools/cdata-test.js  # run just that file directly
 ```
@@ -41,7 +42,7 @@ target environments. Always build after code changes: `pio run -e esp32dev`.
 
 ### Recovery / Troubleshooting
 
-```sh
+```bash
 npm run build -- -f              # force web UI rebuild
 rm -f wled00/html_*.h wled00/js_*.h && npm run build  # clean + rebuild UI
 pio run --target clean           # clean PlatformIO build artifacts
@@ -50,7 +51,7 @@ rm -rf node_modules && npm ci    # reinstall Node.js deps
 
 ## Project Structure
 
-```
+```text
 wled00/              # Main firmware source (C++)
   data/              # Web UI source (HTML/JS/CSS) — tabs for indentation
   html_*.h, js_*.h   # Auto-generated (NEVER edit or commit)
@@ -132,6 +133,7 @@ main                # Main development trunk (daily/nightly) 17.0.0-dev. Target
 - **Performance**: Prefer DRAM (or IRAM) for hot-path data that is *frequently* used. Prefer PSRAM for capacity-oriented buffers where slightly slower access times can be tolerated.
 
 Background Info:
+
 - PSRAM access is up to 18× slower than DRAM on ESP32 (dual-SPI bus), 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel. On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM.
 - Consider that ESP32 often crashes when the largest DRAM chunk gets below 10 KB.
 
@@ -158,10 +160,10 @@ Background Info:
 
 #### ESP32 Task Synchronization
 
-* Use FreeRTOS mutexes, semaphores or queues when true concurrent access from multiple FreeRTOS tasks is possible, and race-conditions can lead to unexpected behaviour.
-* **Avoid `portENTER_CRITICAL()` / `portEXIT_CRITICAL()`**, as these functions stall the complete system and may cause LEDs flickering. Prefer FreeRTOS mutexes, semaphores or queues.
-* **Important**: Not every shared resource needs a mutex. Some synchronization is guaranteed by the overall control flow, for example when function calls are sequenced within the same loop iteration.
-* Consider RAII as an alternative to  mutexes or semaphores.
+- Use FreeRTOS mutexes, semaphores or queues when true concurrent access from multiple FreeRTOS tasks is possible, and race-conditions can lead to unexpected behaviour.
+- **Avoid `portENTER_CRITICAL()` / `portEXIT_CRITICAL()`**, as these functions stall the complete system and may cause LEDs flickering. Prefer FreeRTOS mutexes, semaphores or queues.
+- **Important**: Not every shared resource needs a mutex. Some synchronization is guaranteed by the overall control flow, for example when function calls are sequenced within the same loop iteration.
+- Consider RAII as an alternative to  mutexes or semaphores.
 
 ## Web UI Code Style (wled00/data/)
 
@@ -240,9 +242,16 @@ No automated linting is configured. Match existing code style in files you edit.
 - Provide references when making analyses or recommendations. Support factual claims with verifiable citations, references or concrete evidence; **never fabricate citations**.
 - **Highlight user-visible breaking changes and ripple effects** during reviews. Ask for confirmation that these were introduced intentionally.
 
+### Security Hardening
+
+When writing or reviewing code in `wled00/`, `usermods/`, `wled00/data/`, or `.github/workflows/`,
+consult `docs/hardening.instructions.md` (concise checklist) and `docs/securecode.instructions.md` (detailed rules with examples).
+These files define WLED's threat model, trust boundary model, and WLED-specific constraints (no TLS baseline, no UDP authentication for protocol-defined
+multicast/broadcast, firewall-isolated deployment assumed).
+
 ### Attribution for AI-generated code
 
-Using AI-generated code can hide the source of the inspiration / knowledge / sources it used. 
+Using AI-generated code can hide the source of the inspiration / knowledge / sources it used.
 
 - Document attribution of inspiration / knowledge / sources used in the code, e.g. link to GitHub repositories or other websites describing the principles / algorithms used.
 - When a larger block of code is generated by an AI tool, embed it into `// AI: below section was generated by an AI` ... `// AI: end` comments (see Comments section).
@@ -251,5 +260,5 @@ Using AI-generated code can hide the source of the inspiration / knowledge / sou
 
 ### Supporting Reviews and Discussions
 
-- **For "is it worth doing?" debates** about proposed reliability, safety, or data-integrity mechanisms (CRC checks, backups, power-loss protection): suggest a software **FMEA** (Failure Mode and Effects Analysis). 
+- **For "is it worth doing?" debates** about proposed reliability, safety, or data-integrity mechanisms (CRC checks, backups, power-loss protection): suggest a software **FMEA** (Failure Mode and Effects Analysis).
   Clarify the main feared events, enumerate failure modes, assess each mitigation's effectiveness per failure mode, note common-cause failures, and rate credibility for the typical WLED use case.