Rebase feature/RDKEMW-8178 branch inline with develop

[KTirumalaSrihari]

No description provided.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

lib/rdk/readBTAddress-generic.sh:31

• readBTAddress-generic.sh can echo an empty string when BLUETOOTH_ENABLED is not true, because bluetooth_mac is never initialized in that case. Since getDeviceDetails.sh consumes this output, this can result in an empty Bluetooth MAC instead of a stable default (e.g., 00:00:00:00:00:00). Initialize bluetooth_mac to a default value and/or only override it when getDeviceBluetoothMac returns a non-empty value.
  lib/rdk/getDeviceDetails.sh:292
• getBluetoothMac() initializes bluetooth_mac to a default, but then unconditionally overwrites it with the output of readBTAddress-*.sh. If the helper script returns an empty string (e.g., Bluetooth disabled), this function will return empty. Consider preserving the default when the helper output is empty and/or gating the helper call on BLUETOOTH_ENABLED.

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/timesyncd-conf-update.sh:109

• Within this repo, timesyncd-conf-update.sh no longer appears to be invoked (restart-timesyncd.service was removed and there are no remaining references). If this script is still required, add/restore the caller; otherwise consider removing it to avoid carrying dead code and untested changes.

ntpLog "Retrieve NTP Server URL from /lib/rdk/getPartnerProperty.sh..."
while [ "$attempts" -le "$max_attempts" ]; do

    ntpLog "Attempt $attempts/$max_attempts to retrieve NTP server URL(s)..."
    get_ntp_hosts

    if [ "$hostName" ] || [ "$hostName2" ] || [ "$hostName3" ] || [ "$hostName4" ] || [ "$hostName5" ]; then
        break
    fi

     # If this is the last attempt, try bootstrap as fallback and then break
    if [ $attempts -eq $max_attempts ]; then
        ntpLog "TR-181 returned empty NTP server list; falling back to /opt/secure/RFC/bootstrap.ini..."
        get_ntp_hosts_from_bootstrap
        break
    fi

    sleep 3
    attempts=$((attempts + 1))

done

partnerHostnames="$hostName $hostName2 $hostName3 $hostName4 $hostName5"
ntpLog "NTP Server URL for the partner:$partnerHostnames"

---

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

[Copilot]

get_bs_val() uses grep -E with an unescaped key string that contains regex metacharacters (e.g., dots in "Device.Time.NTPServer1"), which can lead to unintended matches. Use grep -F (fixed-string) or escape the key before using it in an ERE to ensure only the exact key is matched.

Suggested change

	# Extract RHS after '=' and trim whitespace
	grep -m1 -E "^[[:space:]]*$key=" "$BOOTSTRAP" 2>/dev/null | \
	# Escape regex metacharacters in key before using it in grep -E
	escaped_key=$(printf '%s\n' "$key" | sed 's/[][\\.^$*+?(){}|]/\\&/g')
	# Extract RHS after '=' and trim whitespace
	grep -m1 -E "^[[:space:]]*$escaped_key=" "$BOOTSTRAP" 2>/dev/null | \

[Copilot]

This section writes the memory info only to $LOG_PATH/messages.txt while the rest of the script primarily logs to stdout/journald (see the comment about supporting lightsleep via journal buffering). Consider keeping these metrics consistent with the logging strategy (e.g., echo to stdout and optionally also append to messages.txt) to avoid losing the info from the journal stream.

Suggested change

	echo "Available Memory Info:" >> $LOG_PATH/messages.txt
	MEM_AVAILABLE=`cat /proc/meminfo | grep MemAvailable`
	echo $MEM_AVAILABLE >> $LOG_PATH/messages.txt
	t2ValNotify "SYST_INFO_MemAvailable_split" "$MEM_AVAILABLE"
	# Swap Memory Info
	echo "Available Swap Memory Info:" >> $LOG_PATH/messages.txt
	# Logging all the swap info to messages.txt
	SWAP_DATA=`cat /proc/meminfo | grep Swap`
	echo "Available Memory Info:"
	MEM_AVAILABLE=`cat /proc/meminfo | grep MemAvailable`
	echo "$MEM_AVAILABLE"
	echo "Available Memory Info:" >> $LOG_PATH/messages.txt
	echo "$MEM_AVAILABLE" >> $LOG_PATH/messages.txt
	t2ValNotify "SYST_INFO_MemAvailable_split" "$MEM_AVAILABLE"
	# Swap Memory Info
	echo "Available Swap Memory Info:"
	# Logging all the swap info to messages.txt
	SWAP_DATA=`cat /proc/meminfo | grep Swap`
	echo "$SWAP_DATA"
	echo "Available Swap Memory Info:" >> $LOG_PATH/messages.txt

[Copilot]

The logupload binary check uses -f, which only verifies the path exists; it may still fail at runtime if the file is not executable. Use -x (and optionally log a clearer error) to ensure the binary can actually be executed.

[Copilot]

notify-network-ready.service has no [Install] section, and there are no other references in the repo, so it likely won't be enabled/started automatically. Also, it touches connectivity_check_done and logs "INTERNET_FULLY_CONNECTED" without performing any connectivity verification, which doesn’t match the unit description. Add an appropriate install target and/or actual connectivity gating, or clarify/rename the unit to reflect its behavior.

[Copilot]

The comment says SIGUSR1 is used to notify telemetry2_0, but the code now sends SIGIO. If SIGIO is intentional, please update the comment; otherwise keep the signal consistent with what telemetry2_0 expects, since changing the signal may prevent telemetry flush before reboot.

Suggested change

	killall -s SIGIO telemetry2_0
	killall -s SIGUSR1 telemetry2_0

[Copilot]

alertSystem.sh uses $RDK_PATH to source exec_curl_mtls.sh, but the script never sources /etc/include.properties (where RDK_PATH is typically set). In addition, exec_curl_mtls.sh is not present in this repo, so this will likely fail at runtime/build-time. Source include.properties (or set RDK_PATH with a default) and ensure the dependency is shipped from the correct path/component.

[Copilot]

UPLOAD_END_POINT extraction assumes /tmp/DCMSettings.conf contains JSON-style content with a "uploadRepository:URL" key. Elsewhere in the repo, DCMSettings.conf is treated as key/value lines (e.g., grep for 'LogUploadSettings:UploadRepository:URL'), so this grep/awk pipeline may always produce an empty endpoint and abort. Consider parsing the same key format used by other scripts for consistency.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

systemd_units/previous-log-backup.service:1

• This unit uses Type=notify (see earlier in the file) but now runs /usr/bin/backup_logs. If the new binary does not call sd_notify(READY=1) (or equivalent), systemd will treat the service as not-ready and may delay/impact boot ordering. Either ensure /usr/bin/backup_logs sends the notify signal, or switch the unit to a type that matches the binary behavior (e.g., Type=oneshot without notify semantics).

---

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

[Copilot]

The PR title indicates this is only a rebase, but the diff includes substantial functional changes (systemd units removed/added, scripts deleted, new scripts introduced, workflows updated). This makes review and traceability harder; consider updating the PR title/description to reflect the actual scope or splitting into a rebase-only PR plus separate functional PR(s).

[Copilot]

The existence check is for /lib/rdk/utils-vendor.sh, but the source path uses $RDK_PATH/utils-vendor.sh. If $RDK_PATH differs from /lib/rdk, this can incorrectly attempt to source a non-existent file. Source the same path you check (or check using $RDK_PATH consistently).

Suggested change

	if [ -f /lib/rdk/utils-vendor.sh ]; then
	if [ -f "$RDK_PATH/utils-vendor.sh" ]; then

[Copilot]

If $ETHERNET_INTERFACE or $WIFI_INTERFACE is empty/unset, the loop can iterate with an empty $iface. In that case /sys/class/net/ exists and the script may attempt to read /sys/class/net//carrier and to start/kill avahi-autoipd with an empty interface. Add a guard like [ -n \"$iface\" ] || continue before using $iface.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

lib/rdk/NM_Bootstrap.sh:36

• NM_Bootstrap.sh is declared as /bin/sh but uses bash-specific regex matching ("[[ ... =~ ... ]]"), the BASH_REMATCH array, and extended regex quantifiers. This will fail on systems where /bin/sh is dash/ash. Suggest rewriting PSK extraction using POSIX-safe tools (sed/awk/grep/cut) or switching the shebang to bash (and ensuring bash is available).

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/readBTAddress-generic.sh:31

• bluetooth_mac is only set when BLUETOOTH_ENABLED is true; otherwise it is unset and the script echoes an empty string. Since callers (e.g., getDeviceDetails.sh) now rely on this script’s output, this can clear the default "00:00:..." value. Initialize bluetooth_mac to a default value before the BLUETOOTH_ENABLED check (or echo a default when disabled).

---

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

[Copilot]

The existence check uses /lib/rdk/utils-vendor.sh, but the subsequent source statement uses $RDK_PATH/utils-vendor.sh. If RDK_PATH differs from /lib/rdk, this will fail despite the file existing. Source the same path you test (or test and source via $RDK_PATH).

Suggested change

	if [ -f /lib/rdk/utils-vendor.sh ]; then
	. $RDK_PATH/utils-vendor.sh
	if [ -f "$RDK_PATH/utils-vendor.sh" ]; then
	. "$RDK_PATH/utils-vendor.sh"

[Copilot]

This script updates /etc/systemd/timesyncd.conf but no longer restarts (or reloads) systemd-timesyncd afterward. Without a restart/reload, the new NTP servers may not take effect until the next reboot or service restart. Please reintroduce an appropriate systemctl try-restart/restart systemd-timesyncd.service (ideally only when the config actually changed).

[Copilot]

systemd_ver is parsed as a string and then compared with -ge. If the systemctl --version output format changes or parsing fails, this will trigger a test error and may prevent config updates. Consider guarding the numeric comparison (e.g., ensure it matches ^[0-9]+$ before using -ge).

[Copilot]

This echo is missing the >> "$logsFile" redirection, so it logs to stdout/journal instead of the connection stats file (unlike the surrounding telemetry logs). This looks accidental and can add noise to the service journal.

Suggested change

	echo "$(/bin/timestamp) Current Packet loss is SYST_WARN_GW100PERC_PACKETLOSS"
	echo "$(/bin/timestamp) Current Packet loss is SYST_WARN_GW100PERC_PACKETLOSS" >> "$logsFile"

[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.