Repository navigation
RDKEMW-23527 : Inherit extract-sha-cve bbclass for CVE support - Addition of CVE middleware paths - #164
RDKEMW-23527 : Inherit extract-sha-cve bbclass for CVE support - Addition of CVE middleware paths#164jomothomas wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the machine include configuration to add CVE feed artifact URLs alongside existing middleware IPK feed URIs, aiming to expose CVE tarballs for middleware artifacts.
Changes:
- Appends middleware CVE tarball entries to
CVE_LAYER_FEED_PATHfor the base middleware feed and the OSS middleware feed. - Adds an include of
conf/dynamic_sha.inc(presumably to provide per-artifact SHA256 values).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| conf/machine/include/oss_mw.inc | Adds an OSS middleware CVE feed entry and includes conf/dynamic_sha.inc. |
| conf/machine/include/middleware.inc | Adds a middleware CVE feed entry, includes OSS middleware include conditionally, and includes conf/dynamic_sha.inc. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (4)
conf/machine/include/middleware.inc:19
- This file includes
conf/dynamic_sha.inc, but that file is not present anywhere in the repository, so SHA population (and/or parsing) won’t work as intended. Either addconf/dynamic_sha.incin this PR (defining the referenced*_SHA256SUMvars), or update the include to point to the correct existing file.
include conf/dynamic_sha.inc
conf/machine/include/oss_mw.inc:11
- This file includes
conf/dynamic_sha.inc, but that file is not present anywhere in the repository. If this include is meant to defineMW_OSS_SHA256SUM, it needs to be added in this PR or the include path corrected.
include conf/dynamic_sha.inc
conf/machine/include/oss_mw.inc:8
CVE_LAYER_COMMON_PATH_MIDDLEWAREis already set inmiddleware.inc, so using?=here means the OSS CVE feed can end up using the non-OSS path. Also, the CVE tarball URL here doesn’t mirror the OSS IPK layout (missing/${MW_OSS}/), and the filename appends-middlewaretwice becauseMW_OSSalready includes that suffix.
CVE_LAYER_COMMON_PATH_MIDDLEWARE ?= "${MW_OSS_IPK_PATH}"
MW_OSS_IPK_SERVER_PATH = "${RDK_ARTIFACTS_BASE_URL}/${MW_OSS_IPK_PATH}/${RDK_MW_ARCH}/${MW_OSS}/ipks/${BUILD_VARIANT}"
CVE_LAYER_FEED_PATH += " ${MW_OSS}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/debug/${MW_OSS}-middleware.tgz;sha256sum=${MW_OSS_SHA256SUM}"
conf/machine/include/middleware.inc:13
MIDDLEWARE_ARCH_SHA256SUMis not defined anywhere in this layer (it only appears in this new reference), so the generated feed entry will likely contain an emptysha256sum=value. If the SHA is optional, it should be conditionally appended; if it’s required, it needs to be defined (e.g., via an included .inc that is actually present).
CVE_LAYER_FEED_PATH += " ${MIDDLEWARE_ARCH}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/debug/${MIDDLEWARE_ARCH}.tgz;sha256sum=${MIDDLEWARE_ARCH_SHA256SUM}"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
conf/machine/include/middleware.inc:13
- This layer currently has no
conf/dynamic_sha.inc(and there are no other definitions of*_SHA256SUMvariables in this repo), but the new CVE feed URL appends;sha256sum=${MIDDLEWARE_ARCH_SHA256SUM}. Unless another layer in the build providesconf/dynamic_sha.incand defines these variables, the checksum parameter will expand empty, which can break CVE feed consumers. Ensure the checksum include file is actually provided (or define the SHA256SUM variables in this layer) and consider usingrequireif the file is mandatory.
CVE_LAYER_FEED_PATH += " ${MIDDLEWARE_ARCH}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/debug/${MIDDLEWARE_ARCH}.tgz;sha256sum=${MIDDLEWARE_ARCH_SHA256SUM}"
conf/machine/include/oss_mw.inc:8
CVE_LAYER_COMMON_PATH_MIDDLEWAREis also set inmiddleware.inc(line 11) with?=; whenoss_mw.incis included frommiddleware.inc, this?=will not override it. As a result, overridingMW_OSS_IPK_PATHwill not affect the OSS CVE feed URL here (it will keep using the non-OSS path). Use a distinct variable for the OSS CVE common path (or an override) and reference that inCVE_LAYER_FEED_PATH.
CVE_LAYER_COMMON_PATH_MIDDLEWARE ?= "${MW_OSS_IPK_PATH}"
MW_OSS_IPK_SERVER_PATH = "${RDK_ARTIFACTS_BASE_URL}/${MW_OSS_IPK_PATH}/${RDK_MW_ARCH}/${MW_OSS}/ipks/${BUILD_VARIANT}"
CVE_LAYER_FEED_PATH += " ${MW_OSS}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/debug/${MW_OSS}.tgz;sha256sum=${MW_OSS_SHA256SUM}"
conf/machine/include/oss_mw.inc:11
oss_mw.incis only included frommiddleware.incin this layer, andmiddleware.incalready includesconf/dynamic_sha.inc(line 19). Including it again here causes the file to be parsed twice and risks duplicate/overriding variable definitions; keep the include in one place.
include conf/dynamic_sha.inc
…tion of CVE middleware pathsv2
There was a problem hiding this comment.
🟡 Changes recommended
The shared CVE_LAYER_COMMON_PATH_MIDDLEWARE defaulting is order-dependent between includes and can yield incorrect CVE feed URLs when middleware and OSS paths diverge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
conf/machine/include/oss_mw.inc:26
CVE_LAYER_COMMON_PATH_MIDDLEWAREis being defaulted here based onMW_OSS_IPK_PATH, but it is also defaulted inmiddleware.incbased onMW_IPK_PATH. Becauseoss_mw.incis included frommiddleware.incearlier in parse order, whichever file runs first will “win”, which can produce incorrect CVE feed URLs ifMW_IPK_PATHandMW_OSS_IPK_PATHare ever set differently. Consider setting this default in only one place, or using separate variables (e.g.,CVE_LAYER_COMMON_PATH_MIDDLEWAREandCVE_LAYER_COMMON_PATH_MWOSS).
if d.getVar("CVE_CHECK_ENABLED") == "1":
if not d.getVar("CVE_LAYER_COMMON_PATH_MIDDLEWARE"):
d.setVar("CVE_LAYER_COMMON_PATH_MIDDLEWARE", "${MW_OSS_IPK_PATH}")
d.setVar("CVE_FEED_URL_MWOSS", "${MW_OSS}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/${BUILD_VARIANT}/${MW_OSS}.tgz")
d.appendVar("CVE_LAYER_FEED_PATH", " ${CVE_FEED_URL_MWOSS};sha256sum=${MW_OSS_SHA256SUM}")
conf/machine/include/middleware.inc:34
CVE_LAYER_COMMON_PATH_MIDDLEWAREis defaulted here fromMW_IPK_PATH, but it is also defaulted inoss_mw.incfromMW_OSS_IPK_PATH. Sinceoss_mw.incis included earlier in this file, its defaulting can take precedence, leading to an unexpected CVE feed path for the non-OSS middleware feed when these paths diverge. Consider defaulting the shared path in a single place, or use distinct override variables per feed.
if d.getVar("CVE_CHECK_ENABLED") == "1":
if not d.getVar("CVE_LAYER_COMMON_PATH_MIDDLEWARE"):
d.setVar("CVE_LAYER_COMMON_PATH_MIDDLEWARE", "${MW_IPK_PATH}")
d.setVar("CVE_FEED_URL_MIDDLEWARE", "${MIDDLEWARE_ARCH}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/${BUILD_VARIANT}/${MIDDLEWARE_ARCH}.tgz")
d.appendVar("CVE_LAYER_FEED_PATH", " ${CVE_FEED_URL_MIDDLEWARE};sha256sum=${MIDDLEWARE_ARCH_SHA256SUM}")
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
…tion of CVE middleware pathsv2
There was a problem hiding this comment.
🔵 Needs a closer look
Moderate issues remain around duplicate inclusion and the undeclared dynamic SHA provider dependency.
Review details
Suppressed comments (3)
conf/machine/include/middleware.inc:25
- When
STACK_LAYER_OSS_SUPPORTis enabled,middleware.incincludesoss_mw.incat line 16, soconf/dynamic_sha.incis included once fromoss_mw.incand then included again here. BitBake does not deduplicateincludedirectives; any anonymous Python, task, or variable side effects in that externally supplied file can therefore be applied twice and duplicate the SHA/feed setup. Keep this include in one common location (or explicitly guard the second include) so OSS configurations parse it only once.
CVE_CONF_INCLUDE = "${@bb.utils.contains('CVE_CHECK_ENABLED', '1', 'conf/dynamic_sha.inc', '', d)}"
include ${CVE_CONF_INCLUDE}
conf/machine/include/middleware.inc:25
- When CVE checking is enabled, this include relies on
conf/dynamic_sha.incbeing supplied by the external auxiliary layer, but this layer'sconf/layer.confstill declares onlycoreinLAYERDEPENDS_middleware-release. A consumer can therefore enable this layer andcve-checkwithout the provider, leaving the checksum variables unavailable and the CVE feed configuration broken. Add the provider to the layer dependency (or otherwise make that dependency explicit) with this change.
CVE_CONF_INCLUDE = "${@bb.utils.contains('CVE_CHECK_ENABLED', '1', 'conf/dynamic_sha.inc', '', d)}"
include ${CVE_CONF_INCLUDE}
conf/machine/include/oss_mw.inc:17
- This CVE-enabled path also depends on
conf/dynamic_sha.incfrom the external auxiliary layer, whileconf/layer.confdeclares onlycoreinLAYERDEPENDS_middleware-release. Without the provider layer, the checksum variable used below is unavailable and the OSS CVE feed is invalid. Add the provider to the layer dependency or make that dependency explicit.
CVE_CONF_INCLUDE = "${@bb.utils.contains('CVE_CHECK_ENABLED', '1', 'conf/dynamic_sha.inc', '', d)}"
include ${CVE_CONF_INCLUDE}
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
…tion of CVE middleware pathsv3
…tion of CVE middleware pathsv7
|
The code set is able to get the expected dynamic_sha.inc if Vendor and Middleware artifacts are available. Image assembler |
|
Please find the resolved paths after running bitbake -e for 2 platforms Realtek XIONE2026-10-08T13:36:59.9798715Z # $CVE_LAYER_FEED_PATH [5 operations] LLAMA2026-10-08T13:38:31.3987673Z # $CVE_LAYER_FEED_PATH [4 operations] |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
🟡 Changes recommended
The extractor expands CVE_LAYER_FEED_PATH before parsing, so it cannot identify the SHA-256 output variables and leaves both middleware checksum variables unpopulated.
2 open issues
2 resolved since last review
Review effort: Lite
| d.setVar("CVE_LAYER_COMMON_PATH_MIDDLEWARE", "${MW_IPK_PATH}") | ||
|
|
||
| d.setVar("CVE_FEED_URL_MIDDLEWARE", "${MIDDLEWARE_ARCH}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/${BUILD_VARIANT}/${MIDDLEWARE_ARCH}.tgz") | ||
| d.appendVar("CVE_LAYER_FEED_PATH", " ${CVE_FEED_URL_MIDDLEWARE};sha256sum=${MIDDLEWARE_ARCH_SHA256SUM}") |
| d.setVar("CVE_LAYER_COMMON_PATH_MIDDLEWARE", "${MW_OSS_IPK_PATH}") | ||
|
|
||
| d.setVar("CVE_FEED_URL_MWOSS", "${MW_OSS}##${RDK_ARTIFACTS_BASE_URL}/${CVE_LAYER_COMMON_PATH_MIDDLEWARE}/${RDK_MW_ARCH}/tarball/cve/${BUILD_VARIANT}/${MW_OSS}.tgz") | ||
| d.appendVar("CVE_LAYER_FEED_PATH", " ${CVE_FEED_URL_MWOSS};sha256sum=${MW_OSS_SHA256SUM}") |
|
CCI-Build-Verified +1 |
1 similar comment
|
CCI-Build-Verified +1 |
This PR adds the CVE feed paths to Middleware which get SHA256 checksums
Changes:
Addition of the CVE feedback paths for middleware artifacts corresponding to IPK feeds
Added CVE_LAYER_COMMON_PATH_MIDDLEWARE for usage in local conf