resolving medium level issue[do not merge] #120

Sid2001-maker · 2026-02-11T08:09:30Z

No description provided.

…tical and High issues Reason for change: Resolve Critical and high level issues in coverity Test Procedure: build should be successful Risk: low Priority: P2

github-actions · 2026-02-11T08:09:41Z

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

I have read the CLA Document and I hereby sign the CLA

1 out of 2 committers have signed the CLA.
✅ (Sid2001-maker)[https://github.com/Sid2001-maker]
❌ @sid-TEL
_{You can retrigger this bot by commenting recheck in this Pull Request.}_{Posted by the CLA Assistant Lite bot.}

Copilot

Pull request overview

This PR appears to address several “medium” robustness/security findings across the runtime: input sanitization, safer locking, improved error handling/logging, and more defensive resource management.

Changes:

Add sanitization for the --display CLI argument and wrap some main() entrypoints with exception handling.
Improve concurrency correctness (mutex usage, atomic ID generation, condition_variable waits).
Harden error paths and logging (curl option set failures, uncaught exception string conversion, pointer null-checks, cleanup on early returns).

Reviewed changes

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

Show a summary per file

File	Description
`src/jsruntime.cpp`	Adds Wayland display-name sanitization and wraps `main()` in try/catch.
`src/jsc/jsc_lib/jsc_lib.cpp`	Improves uncaught exception logging and adds checked `curl_easy_setopt` helper; modernizes trailing-slash check.
`src/jsc/JavaScriptUtils.cpp`	Fixes dispatch mutex usage; adds Coverity annotations; hardens file read and error-path cleanup.
`src/jsc/JavaScriptEngine.cpp`	Fixes logging format string for a double.
`src/jsc/JavaScriptContext.cpp`	Adds cleanup for `mNetworkMetricsData`; adds null-check before calling `fnLoadJS`.
`src/NativeJSRenderer.cpp`	Makes application ID generation atomic; clarifies mutex ownership expectations; reduces logging; fixes console queue processing; adds checked curl option setting.
`src/JSRuntimeServer.cpp`	Initializes `uint32_t` before use in JSON parsing helper.
`src/JSRuntimeClientContainer.cpp`	Wraps `main()` in exception handling and improves logging consistency.
`src/JSRuntimeClient.cpp`	Uses a predicate-based condition_variable wait; wraps `main()` in exception handling.
`include/JSRuntimeClient.h`	Adds response-received flag/predicate wait for command responses.
`include/IJavaScriptContext.h`	Adds a virtual destructor to the interface.

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

Copilot · 2026-02-11T08:15:07Z

src/jsruntime.cpp

+                }
+            } 
+            else {
+                std::cout << "--display flag provided without a value" << std::endl;


If --display is provided without a value, the program currently logs and continues. This likely indicates a CLI usage error and should probably cause an immediate failure (non-zero exit) rather than continuing with an unintended display configuration.

Suggested change

std::cout << "--display flag provided without a value" << std::endl;

std::cout << "--display flag provided without a value" << std::endl;

return -1;

Copilot · 2026-02-11T08:15:07Z

src/JSRuntimeServer.cpp

+        uint32_t res = 0;
        cJSON *itm = cJSON_GetObjectItem(mPtr, name);
        if (!itm || !cJSON_IsNumber(itm))
        {


In the error path immediately following this block, the message printed to stderr is missing a space ("is not...") and uses an inconsistent type label ("Uint32_t"). Consider using NativeJSLogger for consistency and improving the wording (e.g., " is not a uint32").

Copilot · 2026-02-11T08:15:07Z

include/JSRuntimeClient.h

        if (derived.send(command))
        {
            std::unique_lock<std::mutex> lock(mResponseMutex);
-            mResponseCondition.wait_for(lock, std::chrono::seconds(5));
-            response = mLastResponse;
-            return true;
+            mResponseReceived = false;
+            bool gotResponse = mResponseCondition.wait_for(lock, std::chrono::seconds(5), 


In sendCommand(), mResponseReceived is reset after derived.send(command) has already been issued. If the response arrives quickly (on another thread) between derived.send() returning and acquiring mResponseMutex, onMessage() can set mResponseReceived=true and notify, but this code then sets it back to false and waits until timeout (missed signal). Reset the flag (or use a monotonically increasing request/response token) before sending, and only wait with the predicate after the send is initiated.

Copilot · 2026-02-11T08:15:08Z

src/jsc/JavaScriptUtils.cpp

  if (result)
  {
-      result->setString(buffer);
+      result->setString(buffer.get());


rtReadBinaryBinding() reads a binary file (hello.wasm) but returns it via result->setString(buffer.get()). Since WASM/binary data commonly contains '\0' bytes, treating it as a C string will truncate at the first NUL and corrupt the data. Please return a binary-safe type (e.g., ArrayBuffer/rtBuffer) or encode the bytes (e.g., base64) and return the encoded string along with the length as needed.

Suggested change

result->setString(buffer.get());

// Encode binary data as base64 to safely return via string

gchar* encoded_data = g_base64_encode(reinterpret_cast<const guchar*>(buffer.get()), bytesRead);

if (!encoded_data)

{

rtLogError("Failed to base64 encode file data");

return RT_ERROR;

}

result->setString(encoded_data);

g_free(encoded_data);

Copilot · 2026-02-11T08:15:08Z

src/jsruntime.cpp

+                if (!sanitizeDisplayName(argv[i], sanitizedDisplay)) {
+                    std::cout << "Invalid display name '" << argv[i] << "'. Must be 1-64 characters containing only alphanumeric, hyphens, underscores, and dots" << std::endl;
+                } else {
+                    std::cout << "Sanitized display name: '" << sanitizedDisplay << "'" << std::endl;
+                    waylanddisplay = sanitizedDisplay;


When sanitizeDisplayName() fails, the code only prints an error and continues, leaving waylanddisplay empty. Since NativeJSRenderer only sets WAYLAND_DISPLAY / initializes Essos when a non-empty display is provided, this can silently change runtime behavior after the user explicitly supplied --display. Consider failing fast (return non-zero) or falling back to a safe default explicitly so the user doesn't get a confusing partial startup.

svcrdkgithub01 · 2026-02-11T08:21:07Z

Coverity Incremental Scan Results

No new Coverity issues found.

svcrdkgithub01 · 2026-02-11T08:21:45Z

Coverity Incremental Scan Results

No new Coverity issues found.

Copilot

Pull request overview

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

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

Copilot · 2026-02-11T10:18:38Z

include/JSRuntimeClient.h

    bool sendCommand(std::string command, std::string &response)
    {
        Derived &derived = static_cast<Derived &>(*this);
        if (derived.send(command))
        {
            std::unique_lock<std::mutex> lock(mResponseMutex);
-            mResponseCondition.wait_for(lock, std::chrono::seconds(5));
-            response = mLastResponse;
-            return true;
+            mResponseReceived = false;
+            bool gotResponse = mResponseCondition.wait_for(lock, std::chrono::seconds(5), 
+                [this]() { return mResponseReceived; });
+
+            if (gotResponse)
+            {
+                response = mLastResponse;
+                return true;
+            }
        }


CommandInterface::sendCommand can miss responses due to a race: mResponseReceived is reset to false after derived.send(command) returns. If a response arrives between send() and acquiring mResponseMutex, onMessage() can set the flag and notify, then sendCommand() clears the flag and waits until timeout. Move the flag reset (and optionally clearing mLastResponse) to before sending (under the mutex), or use a monotonic sequence/correlation id per request to avoid lost wakeups.

Copilot · 2026-02-11T10:18:39Z

src/jsruntime.cpp

+                std::cout << "Received display argument: '" << argv[i] << "'" << std::endl;
+                std::string sanitizedDisplay;
+                if (!sanitizeDisplayName(argv[i], sanitizedDisplay)) {
+                    std::cout << "Invalid display name '" << argv[i] << "'. Must be 1-64 characters containing only alphanumeric, hyphens, underscores, and dots" << std::endl;
+                } else {
+                    std::cout << "Sanitized display name: '" << sanitizedDisplay << "'" << std::endl;
+                    waylanddisplay = sanitizedDisplay;
+                }
+            } 
+            else {
+                std::cout << "--display flag provided without a value" << std::endl;


The new --display handling writes untrusted argv content to stdout via std::cout. The rest of this entrypoint uses NativeJSLogger, so this introduces inconsistent logging and can also leak potentially sensitive command-line input. Prefer logging via NativeJSLogger (and consider logging only the sanitized value, or logging an error and exiting when the display name is invalid/missing).

Suggested change

std::cout << "Received display argument: '" << argv[i] << "'" << std::endl;

std::string sanitizedDisplay;

if (!sanitizeDisplayName(argv[i], sanitizedDisplay)) {

std::cout << "Invalid display name '" << argv[i] << "'. Must be 1-64 characters containing only alphanumeric, hyphens, underscores, and dots" << std::endl;

} else {

std::cout << "Sanitized display name: '" << sanitizedDisplay << "'" << std::endl;

waylanddisplay = sanitizedDisplay;

}

}

else {

std::cout << "--display flag provided without a value" << std::endl;

std::string sanitizedDisplay;

if (!sanitizeDisplayName(argv[i], sanitizedDisplay)) {

NativeJSLogger::log(WARN,

"Invalid display name provided via --display. "

"Must be 1-64 characters containing only alphanumeric characters, "

"hyphens (-), underscores (_) and dots (.)\n");

} else {

NativeJSLogger::log(WARN,

"Using Wayland display '%s'\n",

sanitizedDisplay.c_str());

waylanddisplay = sanitizedDisplay;

}

}

else {

NativeJSLogger::log(WARN,

"--display flag provided without a value\n");

Sid2001-maker and others added 2 commits February 3, 2026 18:44

RDKEMW-12252: Coverity Scan Report - Analyzing and Fixing all the Cri…

be8f662

…tical and High issues Reason for change: Resolve Critical and high level issues in coverity Test Procedure: build should be successful Risk: low Priority: P2

fixing medium level issue CID:430751

ba9151d

Copilot AI review requested due to automatic review settings February 11, 2026 08:09

Sid2001-maker requested a review from a team as a code owner February 11, 2026 08:09

Copilot started reviewing on behalf of Sid2001-maker February 11, 2026 08:09 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Sid2001-maker requested a review from Copilot February 11, 2026 10:12

Copilot started reviewing on behalf of Sid2001-maker February 11, 2026 10:12 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resolving medium level issue[do not merge] #120

resolving medium level issue[do not merge] #120

Uh oh!

Sid2001-maker commented Feb 11, 2026

Uh oh!

github-actions bot commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

svcrdkgithub01 commented Feb 11, 2026

Uh oh!

svcrdkgithub01 commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	std::cout << "--display flag provided without a value" << std::endl;
	std::cout << "--display flag provided without a value" << std::endl;
	return -1;

-      result->setString(buffer.get());
+      // Encode binary data as base64 to safely return via string
+      gchar* encoded_data = g_base64_encode(reinterpret_cast<const guchar*>(buffer.get()), bytesRead);
+      if (!encoded_data)
+      {
+        rtLogError("Failed to base64 encode file data");
+        return RT_ERROR;
+      }
+      result->setString(encoded_data);
+      g_free(encoded_data);

resolving medium level issue[do not merge] #120

Are you sure you want to change the base?

resolving medium level issue[do not merge] #120

Uh oh!

Conversation

Sid2001-maker commented Feb 11, 2026

Uh oh!

github-actions bot commented Feb 11, 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 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

svcrdkgithub01 commented Feb 11, 2026

Uh oh!

svcrdkgithub01 commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants