Skip to content

Add OTA metadata validation v2 #4998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add OTA metadata validation v2 #4998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a073bf3
Implement OTA release compatibility checking system
Copilot Sep 14, 2025
5ca10f3
Process metadata only in metadata.cpp
willmmiles Oct 6, 2025
0c22163
Fix unaligned reads during metadata search
willmmiles Oct 25, 2025
c66d67d
Fix metadata includes
willmmiles Oct 25, 2025
a04d702
Fix set_metadata script
willmmiles Oct 25, 2025
b268aea
set_metadata: Apply code fixes from @coderabbit
willmmiles Oct 25, 2025
d538736
Always use package.json for WLED_VERSION
willmmiles Oct 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pio-scripts/output_bins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import os
import shutil
import gzip
import json

OUTPUT_DIR = "build_output{}".format(os.path.sep)
#OUTPUT_DIR = os.path.join("build_output")
Expand All @@ -22,7 +23,8 @@ def create_release(source):
release_name_def = _get_cpp_define_value(env, "WLED_RELEASE_NAME")
if release_name_def:
release_name = release_name_def.replace("\\\"", "")
version = _get_cpp_define_value(env, "WLED_VERSION")
with open("package.json", "r") as package:
version = json.load(package)["version"]
Comment on lines +26 to +27
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add error handling for package.json read operation.

Reading package.json without error handling could cause cryptic build failures if the file is missing, malformed, or lacks the "version" field. Build scripts should provide clear error messages.

Apply this diff to add error handling:

-        with open("package.json", "r") as package:
-            version = json.load(package)["version"]        
+        try:
+            with open("package.json", "r") as package:
+                version = json.load(package)["version"]
+        except FileNotFoundError:
+            raise FileNotFoundError("package.json not found - required for version metadata")
+        except (KeyError, json.JSONDecodeError) as e:
+            raise ValueError(f"Failed to extract version from package.json: {e}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
with open("package.json", "r") as package:
version = json.load(package)["version"]
try:
with open("package.json", "r") as package:
version = json.load(package)["version"]
except FileNotFoundError:
raise FileNotFoundError("package.json not found - required for version metadata")
except (KeyError, json.JSONDecodeError) as e:
raise ValueError(f"Failed to extract version from package.json: {e}")
🤖 Prompt for AI Agents 
In pio-scripts/output_bins.py around lines 26 to 27, the code opens and reads
package.json and accesses ["version"] without error handling; wrap the
open/json/load/version access in a try/except that catches FileNotFoundError,
json.JSONDecodeError, and KeyError (or IndexError) and prints a clear,
actionable error to stderr (or uses the existing logger) indicating whether
package.json is missing, malformed, or missing the "version" field, then exit
with a non‑zero status so the build fails cleanly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's worth the effort right now: raising any exception from support scripts isn't handled well in PlatformIO, any way you slice it it's not easy to read afterwards. (Also there are many more places that depend on this -- if this fails, you'll have half a dozen other build problems, too.) Finally, the whole thing needs to be refactored with the other build scripts to do provenance analysis in one place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willmmiles, that makes sense! I appreciate the context about PlatformIO's exception handling behavior and the broader refactoring plans. You're right that if package.json is missing or malformed, there would be cascading failures throughout the build anyway, making the specific error location clear enough.

Looking forward to seeing the provenance analysis consolidation when you get to that refactoring! 🐰

    /\_/\
   ( o.o )
    > ^ <
  (づ_ど) understood!
🧠 Learnings used 
Learnt from: willmmiles
PR: wled/WLED#4998
File: wled00/ota_update.cpp:120-124
Timestamp: 2025-10-11T17:56:24.923Z
Learning: In the WLED OTA update implementation (wled00/ota_update.cpp), the parameter name for bypassing OTA validation is "skipValidation", not "ignoreRelease". The intent evolved from the original spec in issue #4929 to disable all validation checks, not just release name checks, which is why the broader parameter name "skipValidation" is used.

release_file = os.path.join(OUTPUT_DIR, "release", f"WLED_{version}_{release_name}.bin")
release_gz_file = release_file + ".gz"
print(f"Copying {source} to {release_file}")
Expand Down
51 changes: 44 additions & 7 deletions pio-scripts/set_repo.py → pio-scripts/set_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
Import('env')
import subprocess
import json
import re

def get_github_repo():
Expand Down Expand Up @@ -42,7 +43,7 @@ def get_github_repo():

# Check if it's a GitHub URL
if 'github.com' not in remote_url.lower():
return 'unknown'
return None

# Parse GitHub URL patterns:
# https://github.com/owner/repo.git
Expand All @@ -63,17 +64,53 @@ def get_github_repo():
if ssh_match:
return ssh_match.group(1)

return 'unknown'
return None

except FileNotFoundError:
# Git CLI is not installed or not in PATH
return 'unknown'
return None
except subprocess.CalledProcessError:
# Git command failed (e.g., not a git repo, no remote, etc.)
return 'unknown'
return None
except Exception:
# Any other unexpected error
return 'unknown'
return None

repo = get_github_repo()
env.Append(BUILD_FLAGS=[f'-DWLED_REPO=\\"{repo}\\"'])
# WLED version is managed by package.json; this is picked up in several places
# - It's integrated in to the UI code
# - Here, for wled_metadata.cpp
# - The output_bins script
# We always take it from package.json to ensure consistency
with open("package.json", "r") as package:
WLED_VERSION = json.load(package)["version"]
Comment on lines +84 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add error handling for module-level package.json read.

Reading package.json at module level without error handling will cause the entire import to fail with a cryptic error if the file is missing or malformed. This is more critical than runtime errors because it affects script initialization.

Apply this diff to add error handling:

-with open("package.json", "r") as package:
-    WLED_VERSION = json.load(package)["version"]
+try:
+    with open("package.json", "r") as package:
+        WLED_VERSION = json.load(package)["version"]
+except FileNotFoundError:
+    raise FileNotFoundError("package.json not found - required for version metadata")
+except (KeyError, json.JSONDecodeError) as e:
+    raise ValueError(f"Failed to extract version from package.json: {e}")
🤖 Prompt for AI Agents 
In pio-scripts/set_metadata.py around lines 84 to 85, the module-level
open/json.load of package.json lacks error handling; wrap the read/parse in a
try/except that catches FileNotFoundError, json.JSONDecodeError, and general
Exception, and on failure either set a safe default for WLED_VERSION or re-raise
a new, clearer exception; also log or print a clear error message including the
filename and underlying exception details to aid debugging before exiting or
propagating.


def has_def(cppdefs, name):
""" Returns true if a given name is set in a CPPDEFINES collection """
for f in cppdefs:
if isinstance(f, tuple):
f = f[0]
if f == name:
return True
return False


def add_wled_metadata_flags(env, node):
cdefs = env["CPPDEFINES"].copy()

if not has_def(cdefs, "WLED_REPO"):
repo = get_github_repo()
if repo:
cdefs.append(("WLED_REPO", f"\\\"{repo}\\\""))

cdefs.append(("WLED_VERSION", WLED_VERSION))

# This transforms the node in to a Builder; it cannot be modified again
return env.Object(
node,
CPPDEFINES=cdefs
)

env.AddBuildMiddleware(
add_wled_metadata_flags,
"*/wled_metadata.cpp"
)
8 changes: 0 additions & 8 deletions pio-scripts/set_version.py
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions platformio.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ ldscript_4m1m = eagle.flash.4m1m.ld

[scripts_defaults]
extra_scripts =
pre:pio-scripts/set_version.py
pre:pio-scripts/set_repo.py
pre:pio-scripts/set_metadata.py
post:pio-scripts/output_bins.py
post:pio-scripts/strip-floats.py
pre:pio-scripts/user_config_copy.py
Expand Down
6 changes: 0 additions & 6 deletions tools/cdata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,6 @@ const char PAGE_dmxmap[] PROGMEM = R"=====()=====";
name: "PAGE_update",
method: "gzip",
filter: "html-minify",
mangle: (str) =>
str
.replace(
/function GetV().*\<\/script\>/gms,
"</script><script src=\"/settings/s.js?p=9\"></script>"
)
},
{
file: "welcome.htm",
Expand Down
27 changes: 25 additions & 2 deletions wled00/data/update.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,26 @@
}
window.open(getURL("/update?revert"),"_self");
}
function GetV() {/*injected values here*/}
function GetV() {
// Fetch device info via JSON API instead of compiling it in
fetch('/json/info')
.then(response => response.json())
.then(data => {
document.querySelector('.installed-version').textContent = `${data.brand} ${data.ver} (${data.vid})`;
document.querySelector('.release-name').textContent = data.release;
// TODO - assemble update URL
// TODO - can this be done at build time?
if (data.arch == "esp8266") {
toggle('rev');
}
})
.catch(error => {
console.log('Could not fetch device info:', error);
// Fallback to compiled-in value if API call fails
document.querySelector('.installed-version').textContent = 'Unknown';
document.querySelector('.release-name').textContent = 'Unknown';
});
}
</script>
<style>
@import url("style.css");
Expand All @@ -27,11 +46,15 @@
<body onload="GetV()">
<h2>WLED Software Update</h2>
<form method='POST' action='./update' id='upd' enctype='multipart/form-data' onsubmit="toggle('upd')">
Installed version: <span class="sip">WLED ##VERSION##</span><br>
Installed version: <span class="sip installed-version">Loading...</span><br>
Release: <span class="sip release-name">Loading...</span><br>
Download the latest binary: <a href="https://github.com/wled-dev/WLED/releases" target="_blank"
style="vertical-align: text-bottom; display: inline-flex;">
<img src="https://img.shields.io/github/release/wled-dev/WLED.svg?style=flat-square"></a><br>
<input type="hidden" name="skipValidation" value="" id="sV">
<input type='file' name='update' required><br> <!--should have accept='.bin', but it prevents file upload from android app-->
<input type='checkbox' onchange="sV.value=checked?1:''" id="skipValidation">
<label for='skipValidation'>Ignore firmware validation</label><br>
<button type="submit">Update!</button><br>
<hr class="sml">
<button id="rev" type="button" onclick="cR()">Revert update</button><br>
Expand Down
4 changes: 2 additions & 2 deletions wled00/dmx_input.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ static dmx_config_t createConfig()
config.software_version_id = VERSION;
strcpy(config.device_label, "WLED_MM");

const std::string versionString = "WLED_V" + std::to_string(VERSION);
strncpy(config.software_version_label, versionString.c_str(), 32);
const std::string dmxWledVersionString = "WLED_V" + std::to_string(VERSION);
strncpy(config.software_version_label, dmxWledVersionString.c_str(), 32);
config.software_version_label[32] = '\0'; // zero termination in case versionString string was longer than 32 chars

config.personalities[0].description = "SINGLE_RGB";
Expand Down
2 changes: 1 addition & 1 deletion wled00/e131.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ void prepareArtnetPollReply(ArtPollReply *reply) {

reply->reply_port = ARTNET_DEFAULT_PORT;

char * numberEnd = versionString;
char * numberEnd = (char*) versionString; // strtol promises not to try to edit this.
reply->reply_version_h = (uint8_t)strtol(numberEnd, &numberEnd, 10);
numberEnd++;
reply->reply_version_l = (uint8_t)strtol(numberEnd, &numberEnd, 10);
Expand Down
Loading