Feature/appbundle by madhubabutt · Pull Request #188 · rdkcentral/rdkfwupdater

madhubabutt · 2026-02-12T16:29:30Z

No description provided.

Copilot

Pull request overview

This PR extends bundle-handling to support both certificate bundles and application bundles by threading a bundleType through the “installed bundles” API and adding dlAppBundle parsing/serialization in XConf request/response handling. It also updates unit/L2 test scaffolding to exercise the new behavior (including pulling/building rdm-agent in CI).

Changes:

Add dlAppBundle support to XConf request creation and response parsing, and pass bundle info to rdm.
Update GetInstalledBundles() / getInstalledBundleFileList() to accept a bundleType discriminator and adjust mocks/tests accordingly.
Extend L2 workflow to inject an XConf response fixture and build/copy an rdm binary from rdm-agent.

Reviewed changes

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

Show a summary per file

File	Description
`src/json_process.c`	Add `dlAppBundle` to request/response flow and build combined bundle string for `rdm`.
`src/include/json_process.h`	Extend `XCONFRES` with `dlAppBundle`.
`src/rdkv_main.c`	Initialize new `dlAppBundle` field.
`src/deviceutils/device_api.h`	Update `GetInstalledBundles` API docs/signature to include `bundleType`.
`src/deviceutils/device_api.c`	Route installed bundle lookup through `bundleType`.
`src/deviceutils/deviceutils.h`	Update deviceutils APIs to accept `bundleType`/const params.
`src/deviceutils/deviceutils.c`	Add bundle-type specific path selection (cert vs apps) for metadata discovery.
`unittest/mocks/device_status_helper_mock.h`	Update mock interface and gmock method signature for `bundleType`.
`unittest/mocks/device_status_helper_mock.cpp`	Update C-wrapper mock for new `GetInstalledBundles` signature.
`unittest/mocks/rdkFwupdateMgr_mock.cpp`	Update fwupdate manager mock `GetInstalledBundles` signature.
`unittest/deviceutils/deviceutils_gtest.cpp`	Update tests for new `getInstalledBundleFileList(bundleType)` signature.
`unittest/deviceutils/device_api_gtest.cpp`	Update tests for new `GetInstalledBundles(..., bundleType)` signature.
`test/xconf-certbundle-response.json`	Add fixture including `dlAppBundle` for L2 tests.
`run_l2.sh`	Minor formatting + print additional logs after L2 runs.
`.github/workflows/L2-tests.yml`	Checkout/build `rdm-agent`, inject XConf fixture into mockxconf, and run L2.

Comments suppressed due to low confidence (3)

unittest/mocks/rdkFwupdateMgr_mock.cpp:281

This mock returns strlen(pBundles) even when pBundles is NULL (the if only guards the write). That would crash if any test invokes the mock with a NULL buffer. Consider returning 0 when pBundles is NULL or szBufSize == 0.

extern "C" size_t GetInstalledBundles(char *pBundles, size_t szBufSize, const char *bundleType) {
    if (pBundles && szBufSize > 0) {
        strncpy(pBundles, "bundle1,bundle2", szBufSize - 1);
        pBundles[szBufSize - 1] = '\0';
    }
    return strlen(pBundles);
}

src/json_process.c:403

The fallback path that runs /etc/rdm/rdmBundleMgr.sh when /usr/bin/rdm is missing appears to have been removed. If some target images rely on the script (and don’t ship the rdm binary), bundle downloads will now always fail. Consider restoring the script fallback (and passing the appropriate bundle args), or explicitly documenting/guarding that rdm is now a hard requirement.

	    if ((access("/usr/bin/rdm", F_OK) == 0) && (strlen(dlBundle) > 0)) {
    	        // file exists
		SWLOG_INFO("RDM binary is present\n");
		v_secure_system("rdm -v \"%s\" >> /opt/logs/rdm_status.log 2>&1", dlBundle);
		SWLOG_INFO("RDM Versioned app Download started and completed\n");
	    } else {
                // file doesn't exist
		SWLOG_INFO(" File Not Present .. Download Failed \n");
            }

src/deviceutils/deviceutils.c:417

getInstalledBundleFileList logs "Certificate does not exist..." even when bundleType is dlAppBundle. This makes logs misleading for app bundle lookups. Consider changing these messages to be bundle-type aware (e.g., include bundleType or say "Bundle metadata does not exist...").

    metadataNVMls = getMetaDataFile(BUNDLE_METADATA_NVM_PATH);
    if (metadataNVMls == NULL)
    {
        SWLOG_INFO("Certificate does not exist in NVM Path\n");
    }

    metadataRFSls = getMetaDataFile(BUNDLE_METADATA_RFS_PATH);
    if (metadataRFSls == NULL)
    {
        SWLOG_INFO("Certificate does not exist in RFS Path\n");
    }

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

Copilot · 2026-02-12T16:35:35Z

+    len = GetInstalledBundles( tmpbuf, sizeof(tmpbuf), "dlAppBundle" );
+    if( totlen )
+    {
+        *(pTmpPost + totlen) = '&';
+        ++totlen;
+    }
+    remainlen = szPostFieldOut - totlen;
+    totlen += snprintf( (pTmpPost + totlen), remainlen, "dlAppBundle=%s", tmpbuf );


createJsonString appends dlAppBundle using totlen += snprintf(...) without checking for truncation. If snprintf returns >= remainlen, totlen can exceed szPostFieldOut, causing remainlen underflow and potential out-of-bounds writes in subsequent appends. Please check the snprintf return value and stop/return an error when truncation occurs (and consider applying the same pattern to the existing dlCertBundle append).

Copilot · 2026-02-12T16:35:36Z

    {
        *pBundles = 0;
-        installedBundleListNode = getInstalledBundleFileList();
+        SWLOG_INFO("GetInstalledBundles: Invoking getInstalledBundleFileList to get %s bundle list ",bundleType);


GetInstalledBundles logs bundleType with %s but does not validate it first. If a caller passes NULL, this is undefined behavior and can crash before getInstalledBundleFileList can reject it. Add a NULL/empty check at the start of GetInstalledBundles (or log with a safe fallback like (bundleType ? bundleType : "<null>")).

Suggested change

SWLOG_INFO("GetInstalledBundles: Invoking getInstalledBundleFileList to get %s bundle list ",bundleType);

SWLOG_INFO("GetInstalledBundles: Invoking getInstalledBundleFileList to get %s bundle list ",

(bundleType ? bundleType : "<null>"));

Copilot · 2026-02-12T16:35:36Z

@@ -214,7 +214,7 @@ extern "C" size_t GetInstalledBundles(char *pBundles, size_t szBufSize)
    }


This mock calls snprintf(pBundles, ...) unconditionally once g_DeviceStatusMock is non-null. If a caller passes pBundles == NULL or szBufSize == 0, this will crash/UB. Add the same buffer NULL/size guard used in other mocks before writing to pBundles.

Suggested change

}

}

if (!pBundles || szBufSize == 0)

{

cout << "GetInstalledBundles invalid buffer" << endl;

return 0;

}

Copilot · 2026-02-12T16:35:37Z

+      - name: Check out dependent repostiories
+        uses: actions/checkout@v4
+        with:
+          repository: 'rdkcentral/rdm-agent'
+          path: rdm-agent            


Spelling: step name says "repostiories". Rename it to "repositories" to avoid confusion in workflow logs.

Copilot · 2026-02-12T16:35:37Z

        run: |
          docker run -d --name mockxconf -p 50050:50050 -p 50051:50051 -p 50052:50052 -e ENABLE_MTLS=true -v ${{ github.workspace }}:/mnt/L2_CONTAINER_SHARED_VOLUME ghcr.io/rdkcentral/docker-device-mgt-service-test/mockxconf:latest

+      - name: Copy unknown-accountid-xconf-rfc-response json to mockxconf service


The step name mentions "unknown-accountid-xconf-rfc-response" but the file being copied is test/xconf-certbundle-response.json. Consider aligning the step name with the actual fixture being used so it’s clear what response is being injected into mockxconf.

Suggested change

- name: Copy unknown-accountid-xconf-rfc-response json to mockxconf service

- name: Copy xconf-certbundle-response json to mockxconf service

Copilot · 2026-02-12T16:35:37Z

+#define BUNDLE_METADATA_NVM_APPS_PATH    "/media/apps/etc/apps"
+#define BUNDLE_METADATA_RFS_APPS_PATH    "/etc/apps"


Under GTEST_ENABLE, cert bundle paths are redirected to /tmp/..., but app bundle paths still point at production locations (/media/apps/etc/apps, /etc/apps). This makes unit/integration tests that exercise dlAppBundle behavior difficult to set up consistently. Consider using test-only /tmp paths for app bundles under GTEST_ENABLE, similar to cert bundles.

Suggested change

#define BUNDLE_METADATA_NVM_APPS_PATH "/media/apps/etc/apps"

#define BUNDLE_METADATA_RFS_APPS_PATH "/etc/apps"

#define BUNDLE_METADATA_NVM_APPS_PATH "/tmp/apps"

#define BUNDLE_METADATA_RFS_APPS_PATH "/tmp/rfc/apps"

Copilot · 2026-02-12T16:35:38Z


            szBufSize - the size of the character buffer in argument 1.

+	    bundleType - type of the bundle.
+
            RETURN - number of characters copied to the output buffer.
 */
-size_t GetInstalledBundles(char *pBundles, size_t szBufSize);
+size_t GetInstalledBundles(char *pBundles, size_t szBufSize, const char *bundleType);


The comment block for GetInstalledBundles still lists the old 2-parameter usage line even though the function now requires bundleType. Update the doc comment so the usage/signature and parameter descriptions match.

rdkcmf-jenkins · 2026-02-12T16:39:24Z

+                    SWLOG_ERROR("dlCertBundle string too long, truncation occurred\n");
+                    return ret;
+                }
+                available -= retval;


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Unused value

Assigning value from "available - retval" to "available" here, but that stored value is overwritten before it can be used.

Low Impact, CWE-563
UNUSED_VALUE

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

unittest/mocks/rdkFwupdateMgr_mock.cpp:280

This mock returns strlen(pBundles) even when pBundles is NULL (only guarded inside the copy block), which can segfault in tests that call it with a NULL buffer. Return 0 when pBundles is NULL, and consider explicitly marking bundleType unused to avoid warnings.

    if (pBundles && szBufSize > 0) {
        strncpy(pBundles, "bundle1,bundle2", szBufSize - 1);
        pBundles[szBufSize - 1] = '\0';
    }
    return strlen(pBundles);

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

Copilot · 2026-02-23T14:50:00Z

+cat /opt/logs/swupdate.txt.0
+
+cat /opt/logs/rdm_status.log
+


These cat commands are the last statements in the script; if either log file is missing, cat will exit non-zero and the whole L2 run step will fail even though tests may have passed. Guard with || true or check file existence before catting.

Suggested change

cat /opt/logs/swupdate.txt.0

cat /opt/logs/rdm_status.log

if [ -f /opt/logs/swupdate.txt.0 ]; then

cat /opt/logs/swupdate.txt.0

else

echo "Log file /opt/logs/swupdate.txt.0 not found."

fi

if [ -f /opt/logs/rdm_status.log ]; then

cat /opt/logs/rdm_status.log

else

echo "Log file /opt/logs/rdm_status.log not found."

fi

Copilot · 2026-02-23T14:50:00Z

+      - name: Check out dependent repostiories
+        uses: actions/checkout@v4
+        with:
+          repository: 'rdkcentral/rdm-agent'
+          path: rdm-agent            


Typo in step name: "repostiories" should be "repositories".

Copilot · 2026-02-23T14:50:00Z

+    } else if (strcmp(bundleType, "dlAppBundle") == 0) {
+        SWLOG_INFO("Setting bundle path for installed App packages\n");
+        BUNDLE_METADATA_NVM_PATH = BUNDLE_METADATA_NVM_APPS_PATH;
+        BUNDLE_METADATA_RFS_PATH = BUNDLE_METADATA_RFS_APPS_PATH;
+    } else {


dlAppBundle handling is introduced here, but current unit tests only cover cert bundle discovery/parsing. Please add/extend gtests to cover dlAppBundle path selection and metadata parsing (NVM/RFS) and unknown bundleType handling.

Copilot · 2026-02-23T14:50:01Z

+	    if ((access("/usr/bin/rdm", F_OK) == 0) && (strlen(dlBundle) > 0)) {
    	        // file exists
 		SWLOG_INFO("RDM binary is present\n");
-		v_secure_system("rdm -v \"%s\" >> /opt/logs/rdm_status.log 2>&1", response->dlCertBundle);
-		SWLOG_INFO("RDM Versioned app Download started and completed\n");
-	    } else  if (access("/etc/rdm/rdmBundleMgr.sh", F_OK) == 0) {
-    		// Script file exist
-		SWLOG_INFO("RDM binary is not present, using scripts\n");
-		v_secure_system("sh /etc/rdm/rdmBundleMgr.sh '%s' '%s' >> /opt/logs/rdm_status.log 2>&1", response->dlCertBundle, response->cloudFWLocation);
+		v_secure_system("rdm -v \"%s\" >> /opt/logs/rdm_status.log 2>&1", dlBundle);
 		SWLOG_INFO("RDM Versioned app Download started and completed\n");


Bundle-update execution now only runs when /usr/bin/rdm exists; the fallback path for /etc/rdm/rdmBundleMgr.sh is no longer present. If some targets still rely on the script-based path, bundle updates will now fail with only a log message. Consider restoring the fallback or explicitly dropping script support (with release notes).

mtirum011 and others added 4 commits February 10, 2026 12:28

RDKEMW-12845 Port Dedicated Bundle Name Support for RDM Agent

54e1baf

Merge branch 'develop' into feature/RDKEMW-12845

a8879ff

Merge branch 'develop' into feature/RDKEMW-12845

1939deb

RDKE test app bundle

35d0332

madhubabutt requested a review from a team as a code owner February 12, 2026 16:29

Copilot AI review requested due to automatic review settings February 12, 2026 16:29

Copilot started reviewing on behalf of madhubabutt February 12, 2026 16:29 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

rdkcmf-jenkins reviewed Feb 12, 2026

View reviewed changes

Update json_process.c

624b115

Copilot AI review requested due to automatic review settings February 23, 2026 14:42

Copilot started reviewing on behalf of madhubabutt February 23, 2026 14:43 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

	SWLOG_INFO("GetInstalledBundles: Invoking getInstalledBundleFileList to get %s bundle list ",bundleType);
	SWLOG_INFO("GetInstalledBundles: Invoking getInstalledBundleFileList to get %s bundle list ",
	(bundleType ? bundleType : "<null>"));

		@@ -214,7 +214,7 @@ extern "C" size_t GetInstalledBundles(char *pBundles, size_t szBufSize)
		}

-    }
+    }
+    if (!pBundles || szBufSize == 0)
+    {
+	cout << "GetInstalledBundles invalid buffer" << endl;
+        return 0;
+    }

	- name: Copy unknown-accountid-xconf-rfc-response json to mockxconf service
	- name: Copy xconf-certbundle-response json to mockxconf service

		#define BUNDLE_METADATA_NVM_APPS_PATH "/media/apps/etc/apps"
		#define BUNDLE_METADATA_RFS_APPS_PATH "/etc/apps"

Uh oh!

Conversation

madhubabutt commented Feb 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Unused value

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rdkcmf-jenkins Feb 12, 2026 •

edited

Loading