RDKB-63306: cujo-agent agent.txt - add log rotation by sowmiyachelliah · Pull Request #59 · rdkcentral/advanced-security

sowmiyachelliah · 2026-04-10T09:33:55Z

Reason for change: Implemented automatic log rotation for agent.txt using logrotate utility when file reaches 2MB threshold, monitored via libev event loop in dedicated thread
Test Procedure: Verify agent.txt rotates to agent.txt.1 when file size reaches 2MB
Risks: Low
Priority: P1
Signed-off-by:Sowmiya_Chelliah@comcast.com

Copilot

Pull request overview

Implements automatic rotation of the Cujo agent log (/rdklogs/logs/agent.txt) by introducing a libev-based file monitor thread that triggers logrotate once the log exceeds a 2MB threshold.

Changes:

Add a dedicated libev ev_stat watcher thread to monitor agent.txt size and invoke log rotation.
Introduce log-rotation constants (paths, threshold, poll interval) and call the monitor startup during security initialization.
Add libev/pthread link flags to the DML library and SSP binary.

Reviewed changes

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

File	Description
source/AdvSecuritySsp/Makefile.am	Adds `-lpthread -lev` to SSP link flags.
source/AdvSecurityDml/Makefile.am	Adds `-lev -lpthread` to DML library link flags for new monitoring thread.
source/AdvSecurityDml/cosa_adv_security_internal.c	Adds log rotation constants, starts a monitor thread, and implements libev-based stat watching + `logrotate` execution.

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

Copilot · 2026-04-10T09:39:56Z

+/* Thread function to run libev event loop for log monitoring */
+void* agent_log_monitor_thread(void* arg)
+{
+    (void)arg;


The new forward declaration marks agent_log_monitor_thread as static (internal linkage), but the definition is non-static. With -Wall -Werror this can fail to compile due to conflicting linkage. Make the definition static (and consider making the other helper functions in this block static as well since they’re file-local).

Copilot · 2026-04-10T09:39:56Z

+
+    CcspTraceInfo(("Agent log reached %ld bytes, calling logrotate...\n", st.st_size));
+
+    result = v_secure_system("%s /tmp/logrotate-advsec.status %s",


The logrotate invocation appears to pass the status file as a positional argument. logrotate expects a single config file argument; the status file should be provided via the -s option. As written, /tmp/logrotate-advsec.status will likely be treated as a config file and rotation will fail.

Suggested change

result = v_secure_system("%s /tmp/logrotate-advsec.status %s",

result = v_secure_system("%s -s /tmp/logrotate-advsec.status %s",

Copilot · 2026-04-10T09:39:56Z

+
+    CcspTraceInfo(("Agent log reached %ld bytes, calling logrotate...\n", st.st_size));
+


st.st_size is an off_t; logging it with %ld is not portable and can be undefined on platforms where off_t is not long. Use an appropriate format (e.g., cast to intmax_t and print with %jd) to avoid incorrect output/UB.

Copilot · 2026-04-10T09:39:57Z

    ERR_CHK(rc);
    advsec_start_logger_thread();
+    advsec_start_log_monitor_thread();
    advsec_handle_sysevent_async();


The log monitor thread is detached and runs ev_run() indefinitely, but there’s no corresponding shutdown path (e.g., in CosaSecurityRemove/COSA_Unload). This can leave a background thread running after the component is unloaded, causing undefined behavior. Add a stop mechanism (ev_break + thread join/cancel) tied to the component lifecycle.

Copilot · 2026-04-10T09:39:57Z

+/* Logrotate configuration for agent.txt */
+#define ADVSEC_AGENT_LOG_FILE "/rdklogs/logs/agent.txt"
+#define ADVSEC_AGENT_LOG_MAX_SIZE (2 * 1024 * 1024)  /* 2MB */
+#define ADVSEC_AGENT_LOG_INTERVAL 5.0
+#define ADVSEC_AGENT_LOGROTATE_CONF "/etc/logrotate.d/advsec-agent"
+#define LOGROTATE_BINARY "/usr/sbin/logrotate"
+


ADVSEC_AGENT_LOGROTATE_CONF points to /etc/logrotate.d/advsec-agent, but no such config appears to be added/installed by this change. Without ensuring this file exists on target devices, rotation will consistently fail. Consider shipping/installing the config as part of this component (or add an existence check with a clear error log).

Copilot · 2026-04-10T09:39:57Z

 libdmlasecurity_la_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/source/AdvSecurityDml -I$(top_srcdir)/source/AdvSecuritySsp $(CPPFLAGS) -I$(top_srcdir)/../Utopia/source/include/sysevent -I${PKG_CONFIG_SYSROOT_DIR}$(includedir)/rbus
 libdmlasecurity_la_SOURCES = plugin_main.c cosa_adv_security_internal.c cosa_adv_security_dml.c cosa_adv_security_webconfig.c advsecurity_helpers.c advsecurity_param.c
-libdmlasecurity_la_LDFLAGS = -lccsp_common -lsyscfg -lsysevent -lwebconfig_framework -lmsgpackc -ltrower-base64 -lsecure_wrapper -lrbus $(SSP_LDFLAGS)
+libdmlasecurity_la_LDFLAGS = -lccsp_common -lsyscfg -lsysevent -lwebconfig_framework -lmsgpackc -ltrower-base64 -lsecure_wrapper -lrbus -lev -lpthread $(SSP_LDFLAGS)


Hardcoding -lev in the library link flags introduces a new build dependency on libev. If libev isn’t guaranteed in all build environments, this may break the build; consider adding proper dependency detection (e.g., via configure/pkg-config) and using discovered CFLAGS/LIBS instead of a fixed -lev.

Copilot · 2026-04-10T09:39:58Z

+/* Log rotation function for agent.txt using logrotate binary */
+void rotate_agent_log(void)
+{
+    struct stat st;
+    int result;
+
+    if (stat(ADVSEC_AGENT_LOG_FILE, &st) != 0)
+    {
+        return;
+    }
+
+    if (st.st_size < ADVSEC_AGENT_LOG_MAX_SIZE)
+    {
+        return;
+    }


New log-rotation behavior (size check + logrotate command execution) is introduced here, but there are existing unit tests for this module under source/test/CcspAdvSecurityDmlTest. Consider adding tests for rotate_agent_log() to verify it only invokes v_secure_system once the size threshold is exceeded and that failures are handled/logged correctly.

Copilot · 2026-04-10T09:39:58Z

+    result = v_secure_system("%s /tmp/logrotate-advsec.status %s",
+                             LOGROTATE_BINARY, ADVSEC_AGENT_LOGROTATE_CONF);
+    if (result != 0)
+    {
+        CcspTraceError(("Logrotate failed with return code: %d\n", result));
+    }


v_secure_system() return values are checked with WIFEXITED/WEXITSTATUS in many other places in this file; here it’s compared directly to 0 and logged as a raw int. This can misreport failures (e.g., signal termination encodings). Consider using the same WIFEXITED/WEXITSTATUS pattern for consistency and correctness.

Copilot

Pull request overview

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

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

Copilot · 2026-04-10T14:41:15Z

+    if (stat(ADVSEC_AGENT_LOG_FILE, &st) != 0)
+    {
+        return;
+    }


ADVSEC_AGENT_LOG_FILE is used here but is not defined anywhere in the repo (no #define/declaration found), which will fail compilation. Define the log path constant (e.g., to agent.txt) in an appropriate header or in this file before using it in stat()/ev_stat_init().

Copilot · 2026-04-10T14:41:15Z

+}
+
+/* Thread function to run libev event loop for log monitoring */
+void* agent_log_monitor_thread(void* arg)


agent_log_monitor_thread is forward-declared as static but defined without static here. This is a storage-class mismatch that can fail compilation under strict flags; make the definition static (or drop static from the prototype) so they match.

Suggested change

void* agent_log_monitor_thread(void* arg)

static void* agent_log_monitor_thread(void* arg)

Copilot · 2026-04-10T14:41:16Z

+    advsec_start_log_monitor_thread();
    advsec_handle_sysevent_async();


The log monitor thread is started unconditionally during initialization, even when Is_Device_Finger_Print_Enabled() is false (the logger thread is gated, but the monitor thread is not). This creates a detached thread running an event loop for the lifetime of the process even when the feature is disabled; consider gating this start on the same enable condition (or stopping it when disabled).

Suggested change

advsec_start_log_monitor_thread();

advsec_handle_sysevent_async();

if(Value == 1)

{

advsec_start_log_monitor_thread();

advsec_handle_sysevent_async();

}

Copilot · 2026-04-10T14:41:16Z

+/* Log rotation function for agent.txt using logrotate binary */
+void rotate_agent_log(void)
+{
+    struct stat st;
+    int result;


New log rotation behavior is introduced here but there are existing gtest-based unit tests for this module; consider adding coverage for rotation triggering (size threshold) and the logrotate command execution path (success/failure) to prevent regressions.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 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-04-13T06:37:17Z

+#define ADVSEC_AGENT_LOG_FILE "/rdklogs/logs/agent.txt.0"
+#define ADVSEC_AGENT_LOG_MAX_SIZE (2 * 1024 * 1024)  /* 2MB */
+#define ADVSEC_AGENT_LOG_INTERVAL 5.0
+#define ADVSEC_AGENT_LOGROTATE_CONF "/etc/logrotate.d/advsec-agent"


This code hard-depends on /etc/logrotate.d/advsec-agent, but that config file is not added/installed anywhere in this repo. Without it, rotation will silently never occur (or will always fail). Add the logrotate config to the repo and ensure it gets installed to the expected path, or adjust the code to point to the installed location.

Suggested change

#define ADVSEC_AGENT_LOGROTATE_CONF "/etc/logrotate.d/advsec-agent"

/*

* Use a runtime-managed config path instead of hard-depending on

* /etc/logrotate.d/advsec-agent, which is not installed by this source tree.

*/

#define ADVSEC_AGENT_LOGROTATE_CONF "/tmp/advsec-agent.logrotate.conf"

Copilot

Pull request overview

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

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

Copilot · 2026-04-14T09:30:15Z

+    CcspTraceInfo(("Agent log reached %ld bytes, calling logrotate...\n", st.st_size));
+
+    result = v_secure_system("%s /tmp/logrotate-advsec.status %s",
+                             LOGROTATE_BINARY, ADVSEC_AGENT_LOGROTATE_CONF);
+    if (result != 0)


The new logrotate invocation references ADVSEC_AGENT_LOGROTATE_CONF, but that macro isn't defined anywhere in the repo, so this won’t compile. Also, the command line passed to logrotate appears malformed: /tmp/logrotate-advsec.status is being passed as a positional argument rather than via -s <statefile>, which logrotate expects for the state file. Define the config path macro and update the logrotate command to pass a config file and state file using the correct flags.

Copilot · 2026-04-14T09:30:16Z


 void advsec_handle_sysevent_async(void);
 static void advsec_start_logger_thread(void);
+static void* agent_log_monitor_thread(void* arg);


agent_log_monitor_thread is forward-declared as static, but its definition later is not static. That’s conflicting linkage in C and typically fails compilation with a “static declaration follows non-static”/“conflicting linkage” error. Make the definition static as well (and consider making the other helper functions in this block static too since they’re file-local).

Suggested change

static void* agent_log_monitor_thread(void* arg);

void* agent_log_monitor_thread(void* arg);

Copilot · 2026-04-14T09:30:16Z

+        return;
+    }
+
+    CcspTraceInfo(("Agent log reached %ld bytes, calling logrotate...\n", st.st_size));


st.st_size is an off_t and isn’t guaranteed to fit %ld (it may be 64-bit on some platforms). This log line should use a format specifier/cast that is correct for off_t (e.g., cast to intmax_t and print with %jd, or use a platform-safe macro) to avoid undefined behavior or incorrect output.

sowmiyachelliah and others added 18 commits March 2, 2026 09:43

cujo-agent agent.txt - add log rotation

8a494a1

Updating ssp_main.c

8e88ea3

Updating ssp_main.c

9750e24

Updating ssp_main.c

0ffc625

Updating ssp_main.c

0080884

Updating ssp_main.c

9d5c66a

Update ssp_main.c

1868a67

Adding log rotate code in cosa_adv_security_internal.c

ffd7edb

Adding libraries in Makefile.am

0034e57

Updating cosa_adv_security_internal.c

62357f5

Updating cosa_adv_security_internal.c

942b1a0

Updating cosa_adv_security_internal.c

f6a1256

Update cosa_adv_security_internal.c

73fd38c

cosa_adv_security_internal.c

2136965

Updating cosa_adv_security_internal.c

e8a8fa6

Updating cosa_adv_security_internal.c

760dd0b

Update cosa_adv_security_internal.c

6b6d962

Updating cosa_adv_security_internal.c

9f9b455

sowmiyachelliah requested review from a team as code owners April 10, 2026 09:34

Copilot AI review requested due to automatic review settings April 10, 2026 09:34

Copilot started reviewing on behalf of sowmiyachelliah April 10, 2026 09:34 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

sowmiyachelliah added 2 commits April 10, 2026 10:02

Updating Makefile.am to resolve unit test case failure

bb8ad44

Updating cosa_adv_security_internal.c and Makefile.am

d1d8fff

Copilot AI review requested due to automatic review settings April 10, 2026 14:35

Copilot started reviewing on behalf of sowmiyachelliah April 10, 2026 14:36 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Updating cosa_adv_security_internal.c

eb9ffe4

Copilot AI review requested due to automatic review settings April 13, 2026 06:32

Copilot started reviewing on behalf of sowmiyachelliah April 13, 2026 06:33 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

sowmiyachelliah force-pushed the feature/logrotate branch from 89e0f61 to eb9ffe4 Compare April 14, 2026 09:21

Updating cosa_adv_security_internal.c

e263014

Copilot AI review requested due to automatic review settings April 14, 2026 09:26

Copilot started reviewing on behalf of sowmiyachelliah April 14, 2026 09:26 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Updating cosa_adv_security_internal.c

116f39e


		CcspTraceInfo(("Agent log reached %ld bytes, calling logrotate...\n", st.st_size));

		result = v_secure_system("%s /tmp/logrotate-advsec.status %s",

	result = v_secure_system("%s /tmp/logrotate-advsec.status %s",
	result = v_secure_system("%s -s /tmp/logrotate-advsec.status %s",

	void* agent_log_monitor_thread(void* arg)
	static void* agent_log_monitor_thread(void* arg)

		advsec_start_log_monitor_thread();
		advsec_handle_sysevent_async();

-#define ADVSEC_AGENT_LOGROTATE_CONF "/etc/logrotate.d/advsec-agent"
+/*
+ * Use a runtime-managed config path instead of hard-depending on
+ * /etc/logrotate.d/advsec-agent, which is not installed by this source tree.
+ */
+#define ADVSEC_AGENT_LOGROTATE_CONF "/tmp/advsec-agent.logrotate.conf"

Conversation

sowmiyachelliah commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants