RDKB-62974 RDKB-62976: Native build for Coverity

[GoutamD2905]

Reason for change: Enable coverity scan using native build.
Test Procedure: All the checks should pass in github
Risks: Low
Priority: P1
Signed-off-by: sowmiya_chelliah@comcast.com

[Copilot]

Pull request overview

This PR introduces a JSON-driven native build system for common-library to support Coverity scanning. The system uses declarative JSON configuration files to manage dependencies and build settings, along with bash scripts that automate the build process.

Key Changes

• Added modular build scripts (setup_dependencies.sh, build_native.sh, common_external_build.sh) for dependency management and component building
• Introduced JSON configuration files to define dependencies and build parameters
• Added GitHub Actions workflow for automated native builds on pull requests

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
cov_docker_script/setup_dependencies.sh	Main script for cloning and building all dependencies based on JSON configs
cov_docker_script/build_native.sh	Component build script that uses build_config.json to configure and compile
cov_docker_script/common_external_build.sh	External build handler for when common-library is used as a dependency
cov_docker_script/common_library_external_dependencies_headers.json	Configuration for header-only dependencies
cov_docker_script/common_library_external_dependencies_build.json	Configuration for dependencies that require compilation
cov_docker_script/build_config.json	Build configuration including compiler flags and installation paths
README.md	Comprehensive documentation of the JSON-driven build system
.github/workflows/native-build.yml	CI workflow to build common-library natively for Coverity scanning

Critical Issues Found: There are significant inconsistencies between the documentation and implementation, including mismatched file names and JSON field names that will prevent the scripts from working correctly. The JSON configuration file names referenced in setup_dependencies.sh don't match the actual files added to the repository, which will cause immediate failures when the scripts are executed.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Missing quotes around variable expansion could cause issues if the path contains spaces. The variable $HOME should be quoted to prevent word splitting.

[Copilot]

Missing quotes around variable expansion could cause issues if the path contains spaces. The variable $HOME should be quoted to prevent word splitting.

[Copilot]

The variable expansion in the glob pattern should be quoted. While glob patterns shouldn't be quoted, the variables within should be to prevent issues with special characters. Consider using: for lib_file in "$HOME/$name/"$pattern

Suggested change

	for lib_file in $HOME/$name/$pattern; do
	for lib_file in "$HOME/$name"/$pattern; do

[Copilot]

This script automatically clones and builds dependencies from remote Git repositories (repo/branch) using git clone without pinning to immutable commit SHAs or performing any integrity verification. If a third-party repository (for example trower-base64 as shown in the README) is compromised, an attacker can execute arbitrary build scripts (e.g., configure, meson.build) on the build host and inject malicious libraries into the artifacts. To mitigate this supply chain risk, restrict dependencies to trusted namespaces and pin each dependency to a specific commit or release hash and/or verify signatures before building, rather than relying on mutable branches.

[Copilot]

The external_build_script mechanism executes ./$external_script from each cloned dependency repository, which can be third-party code, without any integrity verification or version pinning. A compromised or hijacked dependency repo could provide a malicious build script that runs with the build user's privileges and installs arbitrary binaries into $INSTALL_PREFIX, leading to build-time remote code execution and poisoned artifacts. To reduce this supply chain risk, only allow vetted first-party scripts here and enforce pinning of the dependency repo to a specific commit or verified release, optionally adding checksum/signature validation of the script before execution.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 19 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This example shows an incorrect JSON structure. The actual build_config.json file does not have "component_name" or nested "build_config" objects. The correct field name should be "dependency_include_dirs" instead of "header_subdirs", and all configuration fields should be at the root level of the JSON file.

Suggested change

	"component_name": "component-x",
	"build_config": {
	"cflags": ["-DFEATURE_X"],
	"header_subdirs": [".", "common-library"],
	"ldflags": ["-lccsp_common"],
	"configure_options": [],
	"source_header_patches": []
	}
	"cflags": ["-DFEATURE_X"],
	"dependency_include_dirs": [".", "common-library"],
	"ldflags": ["-lccsp_common"],
	"configure_options": [],
	"source_header_patches": []

[Copilot]

The sed command uses unquoted variables which could cause issues if the variables contain special characters or shell metacharacters. The variables $patch_line and $insert_before should be properly escaped or quoted to prevent sed command injection or parsing errors.

Suggested change

	if [ -f "$SCRIPT_DIR/$src" ]; then
	# Check if patch already applied
	if ! grep -q "$insert_before" "$SCRIPT_DIR/$src"; then
	# Patch the actual source file
	sed -i "/$patch_line/i $insert_before" "$SCRIPT_DIR/$src"
	log_info " ✓ Patched source file: $src"
	else
	log_info " ✓ Patch already applied: $src"
	fi
	fi
	# Escape values for safe use in sed: pattern (patch_line) and inserted text (insert_before)
	patch_line_sed=$(printf '%s\n' "$patch_line" | sed -e 's/[\/&]/\\&/g')
	insert_before_sed=$(printf '%s\n' "$insert_before" | sed -e 's/[\\&]/\\&/g')
	if [ -f "$SCRIPT_DIR/$src" ]; then
	# Check if patch already applied
	if ! grep -q "$insert_before" "$SCRIPT_DIR/$src"; then
	# Patch the actual source file
	sed -i "/$patch_line_sed/i\\
	$insert_before_sed" "$SCRIPT_DIR/$src"
	log_info " ✓ Patched source file: $src"
	else
	log_info " ✓ Patch already applied: $src"
	fi
	fi

[Copilot]

The variable $configure_opts should be quoted to prevent word splitting. If the intent is to split on whitespace, consider using an array instead for safer parameter passing.

Suggested change

	./configure --prefix="$INSTALL_PREFIX" $configure_opts
	./configure --prefix="$INSTALL_PREFIX" "$configure_opts"

[Copilot]

The variable $header_subdirs should be quoted to prevent word splitting issues. Without quotes, spaces in directory names would cause incorrect behavior.

Suggested change

	header_subdirs=$(jq -r '.dependency_include_dirs[]' "$BUILD_CONFIG")
	for subdir in $header_subdirs; do
	mapfile -t header_subdirs < <(jq -r '.dependency_include_dirs[]' "$BUILD_CONFIG")
	for subdir in "${header_subdirs[@]}"; do

[Copilot]

The documented JSON structure for build_config.json does not match the actual file structure. The documentation shows nested objects with "component_name", "build_config", and "installation" keys, but the actual file (build_config.json) has a flat structure with keys like "cflags", "dependency_include_dirs", "component_header_dirs", etc. directly at the root level. This discrepancy will confuse users trying to configure their builds.

[Copilot]

The field names documented here do not match the actual build_config.json file. The documentation uses "header_subdirs" but the actual field is "dependency_include_dirs". Similarly, "header_dirs" should be "component_header_dirs", "library_patterns" should be "component_libraries", and "destination_subdir" should be "component_install_subdir".

[Copilot]

The summary section lists incorrect file names. The actual files are 'common_library_external_dependencies_headers.json' and 'common_library_external_dependencies_build.json', not 'common_library_headers.json' and 'common_library_build_deps.json'.

Suggested change

	- `common_library_headers.json` - Header-only dependencies
	- `common_library_build_deps.json` - Build dependencies (with optional external_build_script)
	- `common_library_external_dependencies_headers.json` - Header-only dependencies
	- `common_library_external_dependencies_build.json` - Build dependencies (with optional external_build_script)

[Copilot]

The build dependency loop clones Git repositories from common_library_build_deps.json by branch name and then builds/executes their code (including an optional external_build_script) without pinning to immutable commits or verifying integrity. In a CI or Coverity environment, a compromised or hijacked upstream repo/branch could execute arbitrary code with access to build secrets and the ability to tamper build artifacts via git clone followed by its build/tests or external script. To mitigate, restrict dependencies to a vetted allowlist and pin each repository to a specific commit or cryptographically verified release artifact instead of a mutable branch, and avoid or tightly control execution of external build scripts from third-party repos.

In general, the fix is to explicitly define a permissions block for the workflow or for the specific job so that the automatically generated GITHUB_TOKEN has only the minimal required permissions. Since this job appears to only need to read repository contents (for checkout and building) and doesn’t update PRs, issues, or releases, we can safely set contents: read and leave everything else at the implicit default of none.

The best way to fix this without changing existing functionality is to add a permissions block at the workflow root (top-level, alongside name and on) so that all jobs in this workflow inherit the same least-privilege permissions. We’ll set:

permissions:
  contents: read

This satisfies CodeQL’s recommendation and is sufficient for actions/checkout@v3 to work. No other scopes (like pull-requests: write) are indicated by the current steps, so we won’t grant them. Concretely, edit .github/workflows/native-build.yml to insert the permissions block after the name: line and before the on: section. No imports or additional methods are needed since this is a YAML configuration change only.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The documentation refers to "header_subdirs" field but the actual build_config.json file uses "dependency_include_dirs" instead. This field name mismatch will cause confusion when users try to configure their builds.

[Copilot]

The cp command uses a wildcard pattern (*.h) without proper error handling if no files match. While "|| true" is used, if the directory contains no .h files, this could silently fail. Additionally, if header_dir contains subdirectories with headers, they won't be copied. Consider using "cp -r" or find command for more robust copying.

Suggested change

	mkdir -p "$HEADER_PREFIX/$dest_subdir"
	cp -r "$SCRIPT_DIR/$header_dir"/*.h "$HEADER_PREFIX/$dest_subdir/" 2>/dev/null || true
	log_info " ✓ Installed headers from: $header_dir"
	copied_any=false
	while IFS= read -r header_file; do
	# Skip if find returns nothing
	[ -z "$header_file" ] && continue
	copied_any=true
	rel_path=${header_file#"$SCRIPT_DIR/$header_dir"/}
	dest_dir="$HEADER_PREFIX/$dest_subdir/$(dirname "$rel_path")"
	mkdir -p "$dest_dir"
	cp "$header_file" "$dest_dir/"
	done < <(find "$SCRIPT_DIR/$header_dir" -type f -name '*.h' 2>/dev/null)
	if [ "$copied_any" = true ]; then
	log_info " ✓ Installed headers from: $header_dir"
	else
	log_warn " ! No header files found in: $header_dir"
	fi

[Copilot]

The library file search uses unquoted glob expansion in the for loop ($WORKSPACE_ROOT/$pattern). This can fail if the pattern matches files with spaces in their names. The loop variable lib_file and subsequent operations should handle this case properly by quoting the pattern expansion.

Suggested change

	for lib_file in $WORKSPACE_ROOT/$pattern; do
	if [ -f "$lib_file" ]; then
	log_info " ✓ Found: $(basename $lib_file)"
	for lib_file in "$WORKSPACE_ROOT"/$pattern; do
	if [ -f "$lib_file" ]; then
	log_info " ✓ Found: $(basename "$lib_file")"

[Copilot]

The library file search uses unquoted glob expansion in the for loop ($SCRIPT_DIR/$pattern). This can fail if the pattern matches files with spaces in their names. The loop variable lib_file and subsequent operations should handle this case properly by quoting the pattern expansion.

Suggested change

	for lib_file in $SCRIPT_DIR/$pattern; do
	if [ -f "$lib_file" ]; then
	cp "$lib_file" "$INSTALL_PREFIX/lib/"
	log_info " ✓ Installed: $(basename $lib_file)"
	for lib_file in "$SCRIPT_DIR"/$pattern; do
	if [ -f "$lib_file" ]; then
	cp "$lib_file" "$INSTALL_PREFIX/lib/"
	log_info " ✓ Installed: $(basename "$lib_file")"

[Copilot]

The logic for checking if the patch is already applied uses grep to search for insert_before text, but the actual patching inserts the text before patch_line. This creates a logical inconsistency - the check verifies if insert_before exists, but the condition should verify if the patch hasn't been applied yet. If insert_before legitimately appears elsewhere in the file, this check could produce false positives and skip needed patches.

[Copilot]

The documentation shows a nested structure with "build_config" and "installation" sections, but the actual build_config.json has a flat structure. This discrepancy makes the documentation incorrect and will mislead users about the actual JSON schema to use.

Suggested change

	"build_config": {
	"cflags": [
	"-DUSE_NEW_LIB"
	],
	"header_subdirs": [
	".",
	"new-header-lib",
	"new-meson-lib"
	],
	"ldflags": [
	"-lnew-meson-lib"
	]
	}
	"cflags": [
	"-DUSE_NEW_LIB"
	],
	"header_subdirs": [
	".",
	"new-header-lib",
	"new-meson-lib"
	],
	"ldflags": [
	"-lnew-meson-lib"
	]

[Copilot]

For the meson build system, the script runs "ninja all test || ninja all" which silently falls back to skipping tests if they fail. This could mask test failures that indicate real problems. Consider logging a warning when tests fail or making test failures configurable rather than silently ignoring them.

Suggested change

	ninja all test || ninja all
	if ! ninja all test; then
	log_warn "Meson tests failed for ${name}; proceeding with build without running tests."
	ninja all
	fi

[Copilot]

git clone "$repo" -b "$branch" "$name" pulls header-only dependencies directly from remote Git repositories using only a mutable branch name from JSON config, with no pinning to a specific commit or integrity verification. For third-party repositories this creates a supply-chain risk where a compromised branch or repo could silently inject malicious headers into every build that runs this script. To mitigate, restrict these dependencies to trusted/first-party repos and require immutable identifiers (e.g., specific commit SHAs or signed releases) instead of branch names in the JSON configuration, optionally combined with checksum/signature verification.

[Copilot]

This block clones build-time dependencies from arbitrary Git repositories using mutable branch names and then executes an external_build_script from the freshly cloned repo without any integrity checks or pinning. For third-party dependencies, an attacker who compromises the upstream repo or branch can run arbitrary code in your build environment (via the external script or build system), tamper with produced artifacts, or exfiltrate secrets. To reduce this supply-chain risk, ensure these dependencies are either first-party or pinned to immutable identifiers (e.g., commit SHAs) and consider enforcing an allowlist of repositories plus optional signature/checksum verification before executing any external build scripts.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The configuration file defines a patch with type "insert_before" using "match" and "insert" fields (lines 91-93), but the apply_patch function only handles type "create" and "replace", expecting "search" and "replace" parameters. The "insert_before" type is not implemented, which will cause the patch to fail or behave incorrectly. Either implement the insert_before functionality or change the configuration to use "replace" type with appropriate search/replace values.

Suggested change

	"type": "insert_before",
	"match": "typedef struct _CCSP_MESSAGE_BUS_CONNECTION",
	"insert": "typedef struct DBusLoop DBusLoop;"
	"type": "replace",
	"search": "typedef struct _CCSP_MESSAGE_BUS_CONNECTION",
	"replace": "typedef struct DBusLoop DBusLoop;\ntypedef struct _CCSP_MESSAGE_BUS_CONNECTION"

[Copilot]

The configuration defines a "header_sources" array (lines 69-86) but this field is never referenced or processed by any of the build scripts. These header sources will not be copied. If this is intentional and the field is for documentation only, consider removing it or adding a comment explaining it's not used. Otherwise, add logic to process this configuration.

Suggested change

	"header_sources": [
	{ "source": "source/ccsp/include", "destination": "$HOME/usr/include" },
	{ "source": "source/ccsp/components/include", "destination": "$HOME/usr/include" },
	{ "source": "source/ccsp/components/common/MessageBusHelper/include", "destination": "$HOME/usr/include" },
	{ "source": "source/ccsp/custom", "destination": "$HOME/usr/include" },
	{ "source": "source/cosa/include", "destination": "$HOME/usr/include" },
	{ "source": "source/cosa/include/linux", "destination": "$HOME/usr/include/linux" },
	{ "source": "source/cosa/include/linux", "destination": "$HOME/usr/include" },
	{ "source": "source/cosa/package/slap/include", "destination": "$HOME/usr/include" },
	{ "source": "source/util_api/ansc/include", "destination": "$HOME/usr/include" },
	{ "source": "source/util_api/http/include", "destination": "$HOME/usr/include" },
	{ "source": "source/util_api/stun/include", "destination": "$HOME/usr/include" },
	{ "source": "source/util_api/tls/include", "destination": "$HOME/usr/include" },
	{ "source": "source/util_api/web/include", "destination": "$HOME/usr/include" },
	{ "source": "source/util_api/asn.1/include", "destination": "$HOME/usr/include" },
	{ "source": "source/debug_api/include", "destination": "$HOME/usr/include" },
	{ "source": "source/dm_pack", "destination": "$HOME/usr/include" }
	],

[Copilot]

The paths contain hardcoded x86_64 architecture references (/usr/lib/x86_64-linux-gnu/dbus-1.0/include). This will break on non-x86_64 systems such as ARM, aarch64, or i386. Consider using architecture-agnostic paths or dynamically detecting the architecture using $(dpkg-architecture -qDEB_HOST_MULTIARCH) or similar approaches.

[Copilot]

The path contains a hardcoded x86_64 architecture reference (/usr/lib/x86_64-linux-gnu). This will break on non-x86_64 systems such as ARM, aarch64, or i386. Consider using architecture-agnostic paths or dynamically detecting the architecture using $(dpkg-architecture -qDEB_HOST_MULTIARCH) or similar approaches.

[Copilot]

Using eval with user-controlled input from JSON configuration is a security risk. The cmake_flags variable comes from the JSON file and is directly evaluated, which could allow command injection if the JSON is compromised or contains malicious content. Consider using array expansion or other safer alternatives instead of eval.

[Copilot]

The paths contain hardcoded x86_64 architecture references (/usr/lib/x86_64-linux-gnu/dbus-1.0/include). This will break on non-x86_64 systems such as ARM, aarch64, or i386. Consider using architecture-agnostic paths or dynamically detecting the architecture using $(dpkg-architecture -qDEB_HOST_MULTIARCH) or similar approaches.

[Copilot]

The comment states that the build directory contains "Cloned repositories (removed after build)" but the scripts never actually remove the $HOME/build directory after completion. The build artifacts remain on disk after a successful build. Either update the scripts to clean up the build directory after success or correct this documentation to reflect the actual behavior.

Suggested change

	├── build/ # Cloned repositories (removed after build)
	├── build/ # Cloned repositories and build artifacts (persist after build; safe to delete manually)

[Copilot]

The documentation states that headers are installed to "$HOME/usr/include/rdkb/" (lines 89, 196, etc.), but the actual configuration file specifies destinations as "$HOME/usr/include" for all dependencies (lines 14, 22, 33, 49, etc.). This inconsistency between documentation and implementation may confuse users. Update either the documentation or the configuration to be consistent.

Suggested change

	{ "source": "include", "destination": "$HOME/usr/include/rdkb" }
	{ "source": "include", "destination": "$HOME/usr/include" }

[Copilot]

The 'safec' dependency is configured without a 'build' section (only header_paths), indicating it's a header-only dependency. However, safeclib is not a header-only library - it needs to be built and linked. Without a build configuration, the library files (.so/.a) will not be available, which will likely cause linking errors when building the native component. Add a build section for safec (type: "autotools" with appropriate configure flags).

Suggested change

	]
	],
	"build": {
	"type": "autotools",
	"configure_flags": "--prefix=$HOME/usr"
	}

[Copilot]

The clone_repo helper fetches and builds external dependencies by cloning whatever Git URL and branch are specified in component_config.json without any pinning to an immutable identifier (e.g., commit SHA or verified release hash). If a third-party repository or its branch (main, master, etc.) is compromised or force-pushed, an attacker could inject arbitrary build steps or binaries into your Coverity/CI environment with access to your secrets and artifacts. To reduce supply chain risk, extend the configuration and clone logic to require and verify immutable references (commit hashes or signed releases) for third-party dependencies before cloning/building them.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The patch file path appears incorrect. The config specifies "$HOME/usr/include/rdkb/ccsp_message_bus.h", but in build_native.sh line 82, this is concatenated with $COMPONENT_DIR resulting in an incorrect path like "$COMPONENT_DIR/$HOME/usr/include/rdkb/ccsp_message_bus.h". The file variable should be expanded using expand_path before concatenation, or the path should be relative to the component directory. Based on README.md line 238, patches should be relative to component directory with "../" for external files.

Suggested change

	"file": "$HOME/usr/include/rdkb/ccsp_message_bus.h",
	"file": "../usr/include/rdkb/ccsp_message_bus.h",

[Copilot]

The documentation in README.md (line 238) states that patch file paths should be "relative to component directory" with "../" for files outside the component. However, the actual configuration uses an absolute path "$HOME/usr/include/rdkb/ccsp_message_bus.h" which contradicts the documentation. Additionally, the implementation in build_native.sh (line 82) concatenates the file path with COMPONENT_DIR, which would create an invalid path for absolute paths. The documentation and implementation should be consistent.

Suggested change

	"file": "$HOME/usr/include/rdkb/ccsp_message_bus.h",
	"file": "source/ccsp/include/rdkb/ccsp_message_bus.h",

[Copilot]

The file path from the configuration is not expanded before being concatenated with $COMPONENT_DIR. When the config contains "$HOME/usr/include/rdkb/ccsp_message_bus.h", line 82 creates "$COMPONENT_DIR/$HOME/usr/include/rdkb/ccsp_message_bus.h" which is an invalid path. The file variable should be processed with expand_path first, or the logic should handle absolute paths differently. Consider adding: file=$(expand_path "$file") after line 76, and then check if it's an absolute path before concatenating with COMPONENT_DIR.

Suggested change

	type=$(jq -r ".native_component.source_patches[$i].type // \"replace\"" "$CONFIG_FILE")
	search=$(jq -r ".native_component.source_patches[$i].search // \"\"" "$CONFIG_FILE")
	replace=$(jq -r ".native_component.source_patches[$i].replace // \"\"" "$CONFIG_FILE")
	content=$(jq -r ".native_component.source_patches[$i].content // \"\"" "$CONFIG_FILE")
	local target_file="$COMPONENT_DIR/$file"
	file=$(expand_path "$file")
	type=$(jq -r ".native_component.source_patches[$i].type // \"replace\"" "$CONFIG_FILE")
	search=$(jq -r ".native_component.source_patches[$i].search // \"\"" "$CONFIG_FILE")
	replace=$(jq -r ".native_component.source_patches[$i].replace // \"\"" "$CONFIG_FILE")
	content=$(jq -r ".native_component.source_patches[$i].content // \"\"" "$CONFIG_FILE")
	local target_file
	if [[ "$file" = /* ]]; then
	target_file="$file"
	else
	target_file="$COMPONENT_DIR/$file"
	fi

[Copilot]

The include_path in the config contains multiple colon-separated paths, but these are added to CPPFLAGS and CFLAGS with a single -I flag. This will only add the first path correctly. Each path in the colon-separated list should be prefixed with -I individually. Consider splitting the HEADER_PATH by colons and adding -I to each path, or document that include_path should only contain a single path.

[Copilot]

The dependency check validates git, jq, gcc, and make, but does not check for python3 which is required by the apply_patch function (line 105 in common_build_utils.sh). If python3 is not installed, the build will fail with a cryptic error during patch application. Add "python3" to the required_tools array.

Suggested change

	local required_tools=("git" "jq" "gcc" "make")
	local required_tools=("git" "jq" "gcc" "make" "python3")

[Copilot]

Exporting configure options directly without validation could be dangerous if the JSON config contains malicious values. The options are expected to be in the format "VAR=value", but there's no validation to ensure this format. If an option doesn't contain "=", the export command will fail. If it contains shell metacharacters, it could lead to unexpected behavior. Consider validating each option matches the pattern "^[A-Za-z_][A-Za-z0-9_]=.$" before exporting.

Suggested change

	# Export configure options as environment variables
	for option in "${configure_options[@]}"; do
	# Export configure options as environment variables with validation
	for option in "${configure_options[@]}"; do
	# Validate that the option is in the form VAR=value
	if [[ -z "$option" || ! "$option" =~ ^[A-Za-z_][A-Za-z0-9_]*=.*$ ]]; then
	err "Invalid configure option: '$option'. Expected format VAR=value."
	return 1
	fi

[Copilot]

The function calls copy_libraries twice with both "$USR_DIR/local/lib" and "$USR_DIR/lib" as destinations. Since copy_libraries searches the entire repo_dir tree for libraries, this means every library file found in the repo will be copied to both locations, creating duplicate files. This is inefficient and wastes disk space. Consider copying to only one location, or configuring which libraries go to which location based on type or other criteria.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using export with unsanitized user input from JSON configuration can lead to environment variable injection. The configure options come directly from the JSON config and may contain malicious content. An attacker could set arbitrary environment variables or override critical ones like PATH or LD_PRELOAD.

Consider validating the format of configure options to ensure they follow expected patterns (e.g., KEY=VALUE) before exporting them.

Suggested change

	# Export configure options as environment variables
	for option in "${configure_options[@]}"; do
	export "$option"
	# Export configure options as environment variables, validating format KEY=VALUE
	for option in "${configure_options[@]}"; do
	if [[ "$option" =~ ^[A-Za-z_][A-Za-z0-9_]*=.*$ ]]; then
	export "$option"
	else
	echo "Warning: Ignoring invalid configure option: $option" >&2
	fi

[Copilot]

The git clone command accepts repository URLs directly from the JSON config without validation. While this is expected functionality, consider adding validation to ensure URLs follow expected patterns (e.g., HTTPS URLs from trusted domains) if this configuration file could be user-supplied or modified by untrusted sources. This reduces the risk of cloning malicious repositories.

[Copilot]

The script uses set -e which causes the script to exit on any error. However, when sourcing common_build_utils.sh, if that file has any issues or if the sourcing fails, the error handling might not work as expected. Consider adding error checking after the source command to ensure it succeeded.

Suggested change

	source "$SCRIPT_DIR/common_build_utils.sh"
	if ! source "$SCRIPT_DIR/common_build_utils.sh"; then
	echo "Failed to source common_build_utils.sh from: $SCRIPT_DIR" >&2
	exit 1
	fi

[Copilot]

The find command uses unquoted variable expansion $src_dir. If src_dir contains spaces or special characters, the command will fail or behave unexpectedly. The variable should be quoted: find "$src_dir" ...

[Copilot]

The GITHUB_TOKEN environment variable is set but never used by the build scripts. If this token is not needed for the build process, it should be removed to follow the principle of least privilege. If it is needed (e.g., for private repository access), this should be documented.

Suggested change

	env:
	GITHUB_TOKEN: ${{ secrets.RDKCM_RDKE }}

[Copilot]

Missing quotes around command substitution. The result of expand_path "$file" should be quoted to handle paths with spaces correctly. Should be: local expanded_file="$(expand_path "$file")"

Suggested change

	local expanded_file=$(expand_path "$file")
	local expanded_file="$(expand_path "$file")"

[Copilot]

The path construction $COMPONENT_DIR/$expanded_file does not validate or sanitize the file path from the JSON config. An attacker could use path traversal sequences (e.g., ../../etc/passwd) in the JSON to access or modify files outside the intended component directory.

Consider validating that the resulting path is within the expected directory using realpath or similar validation.

[Copilot]

The custom build script from the JSON config is executed directly without validation of its path or content. The script path from the JSON is concatenated with the repo directory and executed with chmod +x. An attacker who can modify the JSON could point to a malicious script in the cloned repository and have it executed with the current user's privileges.

Consider adding validation to ensure the script path doesn't contain path traversal sequences and is within the expected repository directory.

[Copilot]

Using eval with user-controlled commands from JSON configuration files can lead to command injection vulnerabilities. The cmd variable comes directly from the JSON config's commands array and is passed to eval without sanitization. An attacker who can modify the JSON file could execute arbitrary commands.

Consider using array expansion or other safer alternatives to handle complex command arguments without eval.

Suggested change

	if ! eval "$cmd"; then
	# Execute command safely without eval to avoid command injection
	# Split the command string into an array of arguments and execute it directly.
	# Note: This treats the command as program + args, and will not interpret
	# shell metacharacters (pipes, redirection, etc.) from the config.
	read -r -a cmd_array <<< "$cmd"
	if ! "${cmd_array[@]}"; then

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 18 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The use of eval with user-controlled input from the config file is a security risk. The cmake_flags are read from the JSON configuration and passed directly to eval, which could allow arbitrary command execution if the configuration file is compromised.

[Copilot]

The script uses source without checking if the sourced file exists. If common_build_utils.sh is missing or has been moved, the script will fail with a potentially unclear error message.

Suggested change

	source "$SCRIPT_DIR/common_build_utils.sh"
	COMMON_UTILS="$SCRIPT_DIR/common_build_utils.sh"
	if [[ ! -f "$COMMON_UTILS" ]]; then
	echo "Error: Required build utilities not found: $COMMON_UTILS" >&2
	exit 1
	fi
	source "$COMMON_UTILS"

[Copilot]

The copy_libraries function uses find with -exec cp and a trailing || true, which silently ignores all errors. If libraries fail to copy due to permission issues or disk space problems, the build will continue without these critical files, potentially causing runtime failures that are difficult to diagnose.

Suggested change

	find "$src_dir" \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \( -type f -o -type l \) -exec cp -Pv {} "$dst_dir/" \; 2>/dev/null || true
	if ! find "$src_dir" \( -name "*.so*" -o -name "*.a" -o -name "*.la*" \) \( -type f -o -type l \) -exec cp -Pv {} "$dst_dir/" \; ; then
	err "Failed to copy libraries from $src_dir to $dst_dir"
	return 1
	fi

[Copilot]

The install_libraries function uses find with -exec cp and a trailing || true, which silently ignores all errors. If libraries fail to copy due to permission issues or disk space problems, the build will report success without these critical files, potentially causing runtime failures that are difficult to diagnose.

[Copilot]

The chmod commands in the workflow are redundant if the scripts in the repository already have execute permissions. Consider either setting execute permissions in the repository (using git update-index --chmod=+x) or keeping the chmod commands but adding a comment explaining why they're needed.

Suggested change

	run: |
	run: |
	# Ensure scripts are executable in case the execute bit is not preserved
	# in the repository metadata or by the checkout process.

[Copilot]

The NOCONFIGURE environment variable is set to prevent autogen.sh from running configure automatically. However, not all autogen.sh scripts respect this convention. Some may ignore it and run configure anyway, or may use a different variable name (like NO_CONFIGURE). Consider adding a comment documenting this assumption or handling cases where it's not respected.

[Copilot]

The function copies libraries from the entire repository directory tree, which could include test libraries, example libraries, or other unintended files. This could bloat the installation and potentially include libraries that shouldn't be distributed. Consider making the source path more specific or adding filters to exclude unwanted libraries.

[Copilot]

The use of eval with user-controlled input from the config file is a security risk. The configure_flags, cmake_flags, and meson_flags are read from the JSON configuration and passed directly to eval, which could allow arbitrary command execution if the configuration file is compromised or modified by an attacker.

Suggested change

	if ! eval "./configure $configure_flags"; then
	# Safely split configure_flags into an array of arguments to avoid eval
	local -a configure_args=()
	if [[ -n "$configure_flags" ]]; then
	read -r -a configure_args <<< "$configure_flags"
	fi
	if ! ./configure "${configure_args[@]}"; then

[Copilot]

The Python code embedded in the shell script uses string interpolation for file paths and content, which could fail if these values contain single quotes. This could cause the patch application to fail unexpectedly. Consider escaping the values or using a safer approach such as passing them as command-line arguments to a separate Python script.

Suggested change

	if ! python3 -c "
	import sys
	with open('$file', 'r') as f:
	content = f.read()
	content = content.replace('''$search''', '''$replace''')
	with open('$file', 'w') as f:
	f.write(content)
	"; then
	if ! python3 - "$file" "$search" "$replace" << 'PYCODE'; then
	import sys
	if len(sys.argv) != 4:
	sys.exit("Usage: script <file> <search> <replace>")
	path = sys.argv[1]
	search = sys.argv[2]
	replace = sys.argv[3]
	with open(path, 'r', encoding='utf-8') as f:
	content = f.read()
	content = content.replace(search, replace)
	with open(path, 'w', encoding='utf-8') as f:
	f.write(content)
	PYCODE

[Copilot]

The script uses source without checking if the sourced file exists. If common_build_utils.sh is missing or has been moved, the script will fail with a potentially unclear error message.

Suggested change

	# Source common utilities
	# Source common utilities
	if [[ ! -f "$SCRIPT_DIR/common_build_utils.sh" ]]; then
	printf 'Error: Required file not found: %s\n' "$SCRIPT_DIR/common_build_utils.sh" >&2
	exit 1
	fi

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The shell option "set -e" causes the script to exit immediately on any command failure. However, some commands later in the script use "|| true" or conditional checks that may not work as expected with this setting. Consider using "set -euo pipefail" for more robust error handling, or handle specific errors explicitly where needed.

Suggested change

	set -e
	set -euo pipefail

[Copilot]

The command uses eval to execute user-provided commands from JSON configuration. This poses a security risk if the JSON file can be modified by untrusted sources, as it could lead to arbitrary command execution. Consider using safer alternatives or adding input validation.

Suggested change

	# Expand variables safely
	command=$(eval echo "$command")
	log " [$((i+1))/$cmd_count] $description"
	if eval "$command"; then
	# Expand environment variables in the command without using eval
	if command -v envsubst >/dev/null 2>&1; then
	command=$(printf '%s' "$command" | envsubst)
	else
	# Fallback: perform limited, explicit substitutions for known variables
	command=${command//\$\{COMPONENT_DIR\}/$COMPONENT_DIR}
	command=${command//\$\COMPONENT_DIR/$COMPONENT_DIR}
	command=${command//\$\{CONFIG_FILE\}/$CONFIG_FILE}
	command=${command//\$\CONFIG_FILE/$CONFIG_FILE}
	fi
	log " [$((i+1))/$cmd_count] $description"
	if bash -c "$command"; then

[Copilot]

The reference links point to a feature branch (feature/cov_native_build) in the moca-agent repository. These links will break if the feature branch is merged or deleted. Consider updating these to point to the main/develop branch once the feature is merged, or use permanent documentation links.

[Copilot]

The comment "MOCA_DIAGONISTIC" appears to contain a spelling error. "DIAGONISTIC" should be "DIAGNOSTIC".

Suggested change

	-DMOCA_DIAGONISTIC
	-DMOCA_DIAGNOSTIC

[Copilot]

The shell option "set -e" causes the script to exit immediately on any command failure. However, some commands later in the script use "|| true" or conditional checks that may not work as expected with this setting. Consider using "set -euo pipefail" for more robust error handling, or handle specific errors explicitly where needed.

[Copilot]

The shell option "set -e" causes the script to exit immediately on any command failure. However, some commands later in the script use "|| true" or conditional checks that may not work as expected with this setting. Consider using "set -euo pipefail" for more robust error handling, or handle specific errors explicitly where needed.

[Copilot]

The command uses eval to execute dynamically constructed commands from user-provided JSON configuration. This poses a security risk if the JSON file can be modified by untrusted sources, as it could lead to arbitrary command execution. Consider using safer alternatives such as validating input or using arrays to avoid eval.

Suggested change

	step "Running meson setup"
	if ! eval "meson setup $build_dir $meson_flags"; then
	step "Running meson setup $build_dir $meson_flags"
	local meson_args=()
	read -r -a meson_args <<< "$meson_flags"
	if ! meson setup "$build_dir" "${meson_args[@]}"; then

[Copilot]

The command uses eval to execute user-provided commands from JSON configuration. This poses a security risk if the JSON file can be modified by untrusted sources, as it could lead to arbitrary command execution. Consider using safer alternatives such as validating input or using arrays to avoid eval.

Suggested change

	if ! eval "$cmd"; then
	if ! bash -c "$cmd"; then

[Copilot]

The command uses eval to expand environment variables in user input. This poses a security risk as it could lead to arbitrary command execution if the command string contains malicious code. Consider using safer alternatives such as parameter expansion or validating input before evaluation.

Suggested change

	# Expand variables safely
	command=$(eval echo "$command")
	# Expand environment variables in the command without using eval
	command=$(printf '%s\n' "$command" | envsubst)