add secure coding guides for AI reviews (#5572)

Added security review guidelines and a short checklist covering critical security areas including buffer safety, input validation, authentication, secure defaults, and protection against common vulnerabilities. Refined rule wording and priorities to better fit WLED’s technical constraints and realistic deployment model.

The lists are based on the OWASP "top 10" from https://github.com/github/awesome-copilot/blob/main/instructions/security-and-owasp.instructions.md, and on lessons learned from past reviews.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: softhack007 <91616163+softhack007@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: 3 people

.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
@@ -54,15 +56,50 @@ reviews:
         6. CHECK for implied but weakly justified assumptions - like usermod loop() call frequency - and ask for clarification.
         7. FLAG changes that appear unrelated: deleted comments, unnecessary re-formatting or re-factoring, and modifications in files that seem unrelated to the PR description.
 
+    # ── 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: >
@@ -82,8 +119,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: >
@@ -95,8 +157,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: |
@@ -114,6 +174,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.