Rebase feature/RDKEMW-8178 branch inline with develop

[KTirumalaSrihari]

No description provided.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

lib/rdk/NM_Bootstrap.sh:36

• This script is declared as /bin/sh, but uses bash-only regex matching and BASH_REMATCH (e.g. [[ ... =~ ... ]] and ${BASH_REMATCH[1]}). On platforms where /bin/sh is BusyBox ash/dash, this will fail and prevent NM bootstrap from extracting the PSK. Please replace this with POSIX-compatible parsing (e.g. sed/awk) or change the interpreter to bash if that’s guaranteed on target devices.

if [ -f $RDKV_SUPP_CONF ]; then
  SSID=$(cat $RDKV_SUPP_CONF | grep -w ssid= | cut -d '"' -f 2)
  PSK_LINE=$(grep psk= "$RDKV_SUPP_CONF")

  # Case 1: Quoted passphrase
  if [[ "$PSK_LINE" =~ psk=\"(.+)\" ]]; then
    PSK="${BASH_REMATCH[1]}"

  # Case 2: Unquoted 64-char raw PSK
  elif [[ "$PSK_LINE" =~ psk=([a-fA-F0-9]{64}) ]]; then
    PSK="${BASH_REMATCH[1]}"

lib/rdk/getDeviceDetails.sh:292

• getBluetoothMac() initializes bluetooth_mac to a default, but then unconditionally overwrites it with the output of readBTAddress-*.sh. If those scripts output an empty string (e.g. Bluetooth disabled), this will return an empty value instead of the intended default. Consider only overwriting when the script returns a non-empty MAC (or have the helper scripts always emit a default).

getBluetoothMac()
{
    bluetooth_mac="00:00:00:00:00:00"
    if [ -f /lib/rdk/readBTAddress-vendor.sh ]; then
        bluetooth_mac=`sh /lib/rdk/readBTAddress-vendor.sh`
    else
        bluetooth_mac=`sh /lib/rdk/readBTAddress-generic.sh`
    fi

    echo "$bluetooth_mac"
}

lib/rdk/readBTAddress-generic.sh:31

• If BLUETOOTH_ENABLED is not "true", bluetooth_mac is never initialized and the script echoes an empty string. Callers like getDeviceDetails.sh expect a valid MAC (or a default like 00:00:00:00:00:00). Initialize bluetooth_mac to a safe default and quote it when echoing to avoid word-splitting.

---

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

[Copilot]

The new FD/pipe block references PASSCODE but that variable is not defined anywhere in this script. As written, it will write an empty value (or whatever is inherited from the environment) to the FD, and eval "$PASSCODE" is also a command-injection risk. Define/validate where the passcode comes from (e.g. an explicit argument), and avoid eval by writing the literal value safely.

[Copilot]

PIPE=$(mktemp -u) is unsafe (TOCTOU race) and can be exploited if an attacker can create the same path before mkfifo runs. Prefer creating a secure temp path with mktemp (without -u) and then creating the FIFO under that directory or using a securely generated filename that you create atomically.

Suggested change

	# Create a named pipe
	PIPE=$(mktemp -u)
	if ! mkfifo "$PIPE" 2>/dev/null; then
	echo_t "STUNNEL: ERROR - Failed to create named pipe"
	fi
	# Open the pipe using the available FD, with error handling
	if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then
	echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor"
	fi
	# Removing the pipe after opening
	rm "$PIPE"
	# Writing passcode to open file descriptor
	echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
	# Create a named pipe in a securely created temporary directory
	PIPE_DIR=$(mktemp -d 2>/dev/null)
	if [ -z "$PIPE_DIR" ] || [ ! -d "$PIPE_DIR" ]; then
	echo_t "STUNNEL: ERROR - Failed to create temporary directory for named pipe"
	else
	PIPE="$PIPE_DIR/pipe"
	if ! mkfifo "$PIPE" 2>/dev/null; then
	echo_t "STUNNEL: ERROR - Failed to create named pipe"
	rmdir "$PIPE_DIR" 2>/dev/null
	else
	# Open the pipe using the available FD, with error handling
	if ! eval "exec $FD_NUMBER<>$PIPE" 2>/dev/null; then
	echo_t "STUNNEL: ERROR - Failed to open pipe with file descriptor"
	fi
	# Removing the pipe after opening
	rm -f "$PIPE"
	rmdir "$PIPE_DIR" 2>/dev/null
	# Writing passcode to open file descriptor
	echo "$(eval "$PASSCODE")" >&$FD_NUMBER &
	fi
	fi

[Copilot]

When /tmp/checkpacketloss exists, the code only sets gwIp/pingCmd for the V4 call. On the subsequent V6 call, gwIp remains empty, so the function will incorrectly treat it as "no route" and can skew packet-loss state/telemetry. Consider either supporting a V6 override file as well, or only using /tmp/checkpacketloss for V4 while still computing V6 gwIp normally.

[Copilot]

In get_bs_val(), grep is using an extended-regex pattern built from the full TR-181 key (e.g. Device.Time.NTPServer1). Because the key contains '.' characters, -E will treat them as wildcards and may match unintended lines in bootstrap.ini. Use a fixed-string match (grep -F) or escape the key before building the regex.

[Copilot]

MSG_DATA is embedded directly into JSON (both as a key in the deepSleepMgrMain branch and as a string value in the else branch) without any escaping. If MSG_DATA contains quotes, backslashes, or newlines, the JSON becomes invalid; if exec_curl_mtls ultimately evals/executes the curl string, this can also become a command-injection vector. Escape MSG_DATA for JSON (or build the payload with a JSON tool) and avoid passing a shell-constructed curl command through eval-style execution.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

lib/rdk/readBTAddress-generic.sh:31

• When BLUETOOTH_ENABLED is not "true", bluetooth_mac is never set, so this script echoes an empty value. Callers (e.g., getDeviceDetails.sh) expect a deterministic output; consider setting bluetooth_mac to a default like "00:00:00:00:00:00" (or explicitly echoing that default) when Bluetooth is disabled/unavailable.

---

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

[Copilot]

This new pipe/FD block appears broken: PASSCODE is never defined anywhere in this script, so the eval will write an empty string (or error) to the FD. Additionally, eval here is a command-injection risk, and mktemp -u is vulnerable to TOCTOU races. If a passcode needs to be passed to a downstream process, define a clear source for it, avoid eval, and use a securely-created FIFO (mktemp + mkfifo) or a different IPC mechanism; otherwise remove this dead/unreliable block.

[Copilot]

This script is /bin/sh, but the loop for i in {1..9} relies on bash brace expansion, which is not supported by many /bin/sh implementations (dash/busybox ash). That will cause the telemetry threshold loop to run once with the literal string "{1..9}", breaking the packet-loss notification logic. Use a POSIX-compatible loop (e.g., a numeric while loop or seq) instead.

[Copilot]

This unit file has no [Install] section and is not referenced by any other unit in this repo, so it won’t be enabled or started by default. If it’s meant to run during boot/network bring-up, add an [Install] stanza (e.g., WantedBy=...) or wire it in via Wants=/After= from an existing target/unit; otherwise it’s likely to be dead configuration.

[Copilot]

The file existence check uses a hard-coded path (/lib/rdk/utils-vendor.sh) but the script sources "$RDK_PATH/utils-vendor.sh". If RDK_PATH is not "/lib/rdk" (or if only one of these locations is populated), this will silently skip vendor utilities or fail to source them. Use the same path for both the -f check and the subsequent source (or check both locations explicitly).

[Copilot]

getBluetoothMac() initializes bluetooth_mac to a default, but then unconditionally overwrites it with the output of readBTAddress-*.sh. If BLUETOOTH_ENABLED is false, readBTAddress-generic.sh currently echoes an empty string, so this function will return an empty value instead of the intended default MAC. Preserve the default when the helper script returns empty (or update the helper script to always emit the default when Bluetooth is disabled/unavailable).

[Copilot]

Similarly, [ -z $PSK ] is unquoted and can produce test syntax errors when PSK is empty or contains whitespace. Quote PSK in the -z test to keep the branching reliable.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

lib/rdk/readBTAddress-generic.sh:31

• bluetooth_mac is only set when BLUETOOTH_ENABLED=true, but the script always echoes the variable. When Bluetooth is disabled this prints an empty string, and callers (e.g., getDeviceDetails.sh assigns the script output) will treat it as the device BT MAC. Consider defaulting to 00:00:00:00:00:00 (or similar) when Bluetooth is disabled / lookup fails, and quote the echo to avoid surprises.
  lib/rdk/NM_Bootstrap.sh:37
• This script is #!/bin/sh but uses bash-specific features ([[ ... =~ ... ]] regex matching and BASH_REMATCH). On platforms where /bin/sh is not bash (e.g., dash/busybox ash), this will fail and break Wi-Fi bootstrap. Either change the shebang to bash (if guaranteed available) or rewrite the parsing using POSIX tools (grep/sed/awk).

  if [[ "$PSK_LINE" =~ psk=\"(.+)\" ]]; then
    PSK="${BASH_REMATCH[1]}"

  # Case 2: Unquoted 64-char raw PSK
  elif [[ "$PSK_LINE" =~ psk=([a-fA-F0-9]{64}) ]]; then
    PSK="${BASH_REMATCH[1]}"

---

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

[Copilot]

[ -z $SSID ] (and similarly [ -z $PSK ]) should quote the variable ([ -z "$SSID" ]) to avoid test syntax errors when the variable is empty or contains whitespace. As written, an empty SSID can result in [ -z ] and the script may exit with a shell error.

[Copilot]

mktemp -u is inherently race-prone (TOCTOU) and can be exploited to pre-create the chosen path. Since this pipe is used to pass sensitive data (passcode), generate a secure unique path with mktemp (without -u) and then create the FIFO safely (e.g., create temp file, remove it, then mkfifo), or use a fixed, permission-restricted directory.

[Copilot]

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

lib/rdk/readBTAddress-generic.sh:31

• bluetooth_mac is only set when BLUETOOTH_ENABLED is true; otherwise the script echoes an empty string. Since callers (e.g. getDeviceDetails.sh) expect this helper to always return a value (often defaulting to 00:00:00:00:00:00), please initialize bluetooth_mac to a safe default before the conditional and always echo a quoted value.
  lib/rdk/NM_Bootstrap.sh:37
• This script is declared with #!/bin/sh but uses bash-only regex matching ([[ ... =~ ... ]]) and BASH_REMATCH. On systems where /bin/sh is not bash, this will fail and prevent WiFi bootstrap from configuring NetworkManager. Either switch the shebang to /bin/bash (and ensure bash is available) or rewrite the PSK extraction to be POSIX-sh compatible (e.g., using sed/awk).

if [ -f $RDKV_SUPP_CONF ]; then
  SSID=$(cat $RDKV_SUPP_CONF | grep -w ssid= | cut -d '"' -f 2)
  PSK_LINE=$(grep psk= "$RDKV_SUPP_CONF")

  # Case 1: Quoted passphrase
  if [[ "$PSK_LINE" =~ psk=\"(.+)\" ]]; then
    PSK="${BASH_REMATCH[1]}"

  # Case 2: Unquoted 64-char raw PSK
  elif [[ "$PSK_LINE" =~ psk=([a-fA-F0-9]{64}) ]]; then
    PSK="${BASH_REMATCH[1]}"

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

lib/rdk/readBTAddress-generic.sh:31

• If BLUETOOTH_ENABLED is not true, bluetooth_mac is never set and the script prints an empty line. Since callers (e.g., getDeviceDetails.sh) fall back to this script to obtain the BT MAC, consider printing a deterministic default (like 00:00:00:00:00:00) when Bluetooth is disabled or when getDeviceBluetoothMac returns empty.