Rebase feature/RDKEMW-8178 branch inline with develop

[KTirumalaSrihari]

No description provided.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (5)

lib/rdk/readBTAddress-generic.sh:31

• bluetooth_mac can be unset when Bluetooth is disabled, which makes the script print an empty value. Callers (e.g., device detail collection) typically expect a deterministic MAC string even when Bluetooth is off.
  lib/rdk/startStunnel.sh:158
• This block references PASSCODE (never set anywhere in the script) and uses eval, plus it creates a named pipe via mktemp -u (TOCTOU race). There’s also no consumer of FD_NUMBER later in the script, so this logic is currently dead code and can fail or introduce risk at runtime.
  lib/rdk/timesyncd-conf-update.sh:140
• The script updates /etc/systemd/timesyncd.conf but no longer restarts systemd-timesyncd, so the new NTP settings may not take effect until a later restart. Also, appending ConnectionRetrySec=5 on every run can create duplicate entries.
  lib/rdk/alertSystem.sh:107
• MSG_DATA is interpolated directly into JSON and also into a single-quoted curl -d string. If the alert message contains quotes, backslashes, newlines, or a single quote, the JSON/payload can become invalid (or the shell quoting can break), causing alerts to fail or be malformed.

if [ "x$PROCESS_NAME" == "xdeepSleepMgrMain" ]; then
    # Message data is actual metadata header in case of trigger from deepSleep manager process
    # This change is needed since there are data clouds in different deployment which are not flexible to accomodate any deviations in data format
    strjson="{\"searchResult\":[{\"Time\":\"$currentTime\"},{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"PartnerId\":\"$partnerId\"},{\"$MSG_DATA\":\"1\"}]}"
else
    strjson="{\"searchResult\":[{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"msgTime\":\"$currentTime\"},{\"PartnerId\":\"$partnerId\"},{\"logEntry\":\"$MSG_DATA\"}]}"
fi

lib/rdk/networkConnectionRecovery.sh:272

• for i in {1..9} uses brace expansion, which is not POSIX and will not run under many /bin/sh implementations (e.g., BusyBox ash). This can break the packet-loss telemetry loop entirely on targets where /bin/sh isn’t bash.

      for i in {1..9}; do
          if ([ "$packetsLostipv4" -ge $((i*10)) ] && [ "$packetsLostipv4" -lt $((i*10+10)) ]) || ([ "$packetsLostipv6" -ge $((i*10)) ] && [ "$packetsLostipv6" -lt $((i*10+10)) ]); then
            echo "$(/bin/timestamp) Current Packet loss is WIFIV_WARN_PL_"$((i*10))"PERC"  >> "$logsFile"
            t2CountNotify "WIFIV_WARN_PL_"$((i*10))"PERC"
            break

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (7)

lib/rdk/networkConnectionRecovery.sh:274

• This script is /bin/sh, but the {1..9} brace expansion is a bash feature. On POSIX shells this won’t expand and will break the packet-loss notification loop.

      for i in {1..9}; do
          if ([ "$packetsLostipv4" -ge $((i*10)) ] && [ "$packetsLostipv4" -lt $((i*10+10)) ]) || ([ "$packetsLostipv6" -ge $((i*10)) ] && [ "$packetsLostipv6" -lt $((i*10+10)) ]); then
            echo "$(/bin/timestamp) Current Packet loss is WIFIV_WARN_PL_"$((i*10))"PERC"  >> "$logsFile"
            t2CountNotify "WIFIV_WARN_PL_"$((i*10))"PERC"
            break
          fi
      done

lib/rdk/readBTAddress-generic.sh:31

• If BLUETOOTH_ENABLED is not true, this script prints an empty line. getDeviceDetails.sh expects a MAC string; returning empty can propagate incorrect data. Prefer emitting a stable default when Bluetooth is disabled/unavailable.
  lib/rdk/startStunnel.sh:116
• checkHost is written twice (first $JUMP_FQDN, then $DEV_SAN/$PROD_SAN). Multiple checkHost entries are ambiguous and can cause stunnel to validate against the wrong hostname depending on how duplicates are handled. Emit exactly one checkHost based on device type (and fall back to $JUMP_FQDN only when type is unknown).
  lib/rdk/startStunnel.sh:162
• Using mktemp -u to pick a FIFO path is vulnerable to TOCTOU races (another process can create that path between name generation and mkfifo). Also, errors from mkfifo aren’t handled as fatal (the script continues). Consider using mktemp without -u and failing fast if FIFO creation/open fails.
  lib/rdk/startStunnel.sh:173
• PASSCODE is evaluated via eval and written to an arbitrary FD in the background. If PASSCODE can be influenced (directly or indirectly), this enables command injection. Prefer treating the passcode as data (no eval) and validating it is set before writing.
  lib/rdk/timesyncd-conf-update.sh:140
• After updating /etc/systemd/timesyncd.conf, the script no longer restarts systemd-timesyncd. systemd-timesyncd won’t pick up config changes until restart, so the new NTP servers may never be applied during the current boot.
  lib/rdk/alertSystem.sh:107
• MSG_DATA and other fields are interpolated into JSON without escaping. If the alert message contains quotes, backslashes, or newlines, the payload becomes invalid JSON (and can potentially be used for JSON injection into downstream systems).

if [ "x$PROCESS_NAME" == "xdeepSleepMgrMain" ]; then
    # Message data is actual metadata header in case of trigger from deepSleep manager process
    # This change is needed since there are data clouds in different deployment which are not flexible to accomodate any deviations in data format
    strjson="{\"searchResult\":[{\"Time\":\"$currentTime\"},{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"PartnerId\":\"$partnerId\"},{\"$MSG_DATA\":\"1\"}]}"
else
    strjson="{\"searchResult\":[{\"process_name\":\"$PROCESS_NAME\"},{\"mac\":\"$estb_mac\"},{\"Version\":\"$software_version\"},{\"msgTime\":\"$currentTime\"},{\"PartnerId\":\"$partnerId\"},{\"logEntry\":\"$MSG_DATA\"}]}"
fi

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 65 out of 65 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

lib/rdk/getDeviceDetails.sh:50

• The file existence check looks in /lib/rdk/utils-vendor.sh but the script then sources $RDK_PATH/utils-vendor.sh. If RDK_PATH is not /lib/rdk, this will incorrectly try to source a non-existent path even though the check passed.

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

lib/rdk/timesyncd-conf-update.sh:66

• get_bs_val uses grep -E with $key unescaped. Because $key contains dots (e.g., Device.Time.NTPServer1), . is treated as “any character” and can match the wrong keys in bootstrap.ini.
  lib/rdk/alertSystem.sh:23
• This header says the script “is used to backup the Logs”, but this file implements an alert/telemetry POST. The purpose/scope/usage comment block should match the actual behavior to avoid operational confusion.

# Purpose: This script is used to backup the Logs
# Scope: RDK devices
# Usage: This script is triggered by systemd service 
##############################################################################

lib/rdk/alertSystem.sh:113

• CURL_INPUT embeds the JSON payload using single quotes (-d '$strjson'). If the payload contains a single quote, this will break shell quoting and can alter the curl command. Use double quotes around the expanded JSON instead.

if [ -f $EnableOCSPStapling ] || [ -f $EnableOCSP ]; then
    CURL_INPUT=" -w '%{http_code}\n' -H \"Accept: application/json\" -H \"Content-type: application/json\" -X POST -d '$strjson' -o \"$HTTP_FILENAME\" \"$UPLOAD_END_POINT\" --cert-status --connect-timeout 30 -m 30 "
else
    CURL_INPUT=" -w '%{http_code}\n' -H \"Accept: application/json\" -H \"Content-type: application/json\" -X POST -d '$strjson' -o \"$HTTP_FILENAME\" \"$UPLOAD_END_POINT\" --connect-timeout 30 -m 30 "
fi