Support reopening audit and debug logs by fremail · Pull Request #3521 · owasp-modsecurity/ModSecurity

fremail · 2026-03-23T17:06:01Z

This PR adds a new C API function msc_rules_reopen_logs() that allows connectors (Nginx, Apache, etc.) to reopen audit and debug log files without restarting. This is essential for log rotation — after logrotate renames the old file and creates a new one, the connector can call this function (e.g., on SIGUSR1 or SIGHUP) to start writing to the new file.

Inspired by changes in #2304
Resolves #1968
This change is required for resolving issue 121 in ModSecurity-nginx connector

airween · 2026-03-23T17:20:03Z

Hi @fremail,

thanks for this PR - just for the record, there is (was) a similar PR previously, but for some reason it got stuck, see #2304.

But now, could you take a look at the Sonar issues? All of them generated by this PR.

If we merge this PR, then - I assume - we should add this new feature to the connector, right?

fremail · 2026-03-23T17:47:41Z

Hi @airween,

Yes, there was a similar PR. I was very disappointed that it stuck for years. I didn't find an option for committing into that PR, so I forked @brandonpayton's work and created this PR.

Fixing the Sonar issues.

If we merge this PR, then - I assume - we should add this new feature to the connector, right?

Right. Let's merge this PR first :)

sonarqubecloud · 2026-03-23T18:14:30Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

fremail · 2026-03-23T18:44:42Z

To be honest I don't know the best solution for the Sonar issues. Adding linter exceptions is not a fix actually. Adding platform specific code is also a weird solution. Both options cause extra Sonar issues 🤷‍♂️

Suggestions are welcome

fremail · 2026-03-24T17:28:58Z

@airween I've added a PR for the connector as well owasp-modsecurity/ModSecurity-nginx#372

I'm new to this stack and project, so please suggest preferred fixes/ways for the Sonar issues 🙏

Copilot

Pull request overview

Adds a connector-facing mechanism to reopen ModSecurity audit/debug logs after log rotation by wiring a new C API entrypoint through the audit/debug logging stack down to the shared file handler.

Changes:

Introduces msc_rules_reopen_logs() C API to reopen audit + debug logs for a ruleset.
Adds reopen() plumbing across AuditLog and audit log writer implementations (serial/parallel/https).
Adds SharedFiles::reopen() and exposes it to debug logging via DebugLogWriter / DebugLog.

Reviewed changes

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

Show a summary per file

File	Description
src/utils/shared_files.h	Adds `reopen()` declaration for shared file manager.
src/utils/shared_files.cc	Implements `SharedFiles::reopen()` with inode comparison + reopen logic.
src/rules_set.cc	Adds new C API function `msc_rules_reopen_logs()`.
src/debug_log/debug_log_writer.h	Adds `DebugLogWriter::reopen()` declaration.
src/debug_log/debug_log_writer.cc	Implements `DebugLogWriter::reopen()` via `SharedFiles::reopen()`.
src/debug_log/debug_log.cc	Adds `DebugLog::reopenDebugLogFile()`.
src/audit_log/writer/writer.h	Extends writer interface with `reopen()`.
src/audit_log/writer/serial.h / .cc	Implements serial audit log reopen using `SharedFiles::reopen()`.
src/audit_log/writer/parallel.h / .cc	Implements parallel audit log reopen for both paths.
src/audit_log/writer/https.h	Adds no-op reopen implementation for HTTPS writer.
src/audit_log/audit_log.cc	Adds `AuditLog::reopen()` delegating to current writer.
headers/modsecurity/rules_set.h	Exposes `msc_rules_reopen_logs()` in public C API header.
headers/modsecurity/debug_log.h	Exposes `DebugLog::reopenDebugLogFile()` in public C++ API.
headers/modsecurity/audit_log.h	Exposes `AuditLog::reopen()` in public C++ API.

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

Copilot · 2026-03-28T14:00:15Z

headers/modsecurity/debug_log.h

    virtual void setDebugLogLevel(int level);
    virtual void setDebugLogFile(const std::string &fileName, std::string *error);
+    virtual bool reopenDebugLogFile(std::string *error);
    virtual const std::string& getDebugLogFile();


Adding a new virtual method to DebugLog changes the class vtable layout, which is an ABI break for downstream C++ consumers that link against libmodsecurity but were compiled against an older header. Since headers/modsecurity/*.h are installed public headers (src/Makefile.am pkginclude_HEADERS), consider making this method non-virtual (if polymorphic override isn’t required) or otherwise ensure an intentional ABI/SONAME bump and document it.

Suggested change

virtual const std::string& getDebugLogFile();

const std::string& getDebugLogFile();

Copilot · 2026-03-28T14:00:16Z

src/utils/shared_files.cc

+
+    fclose(it->second.fp);
+    it->second.fp = fp;
+


SharedFiles::reopen() swaps it->second.fp without taking the same exclusive lock used by write(). If a thread is inside write() while another thread calls reopen(), fclose(it->second.fp) can race with fwrite()/fflush() on that FILE*, leading to UB/crashes. Consider acquiring the same per-file lock (fcntl F_WRLCK on POSIX / named mutex on WIN32) around the inode check and the close+swap, so reopen() is mutually exclusive with write() for that handler.

Suggested change

fclose(it->second.fp);

it->second.fp = fp;

// Acquire the same exclusive lock used by write() to avoid races on it->second.fp

#ifndef WIN32

struct flock lock {};

lock.l_start = lock.l_len = lock.l_whence = 0;

lock.l_type = F_WRLCK;

if (fcntl(fileno(it->second.fp), F_SETLKW, &lock) == -1) {

// Failed to acquire lock; close the newly opened file and report error

fclose(fp);

error->assign("Failed to lock file during reopen: " + fileName);

return false;

}

#else

DWORD dwWaitResult = WaitForSingleObject(it->second.hMutex, INFINITE);

if (dwWaitResult != WAIT_OBJECT_0) {

// Failed to acquire mutex; close the newly opened file and report error

fclose(fp);

error->assign("Couldn't lock shared file during reopen: " + fileName);

return false;

}

#endif

fclose(it->second.fp);

it->second.fp = fp;

// Release the exclusive lock

#ifndef WIN32

lock.l_type = F_UNLCK;

fcntl(fileno(it->second.fp), F_SETLKW, &lock);

#else

::ReleaseMutex(it->second.hMutex);

#endif

Copilot · 2026-03-28T14:00:16Z

src/utils/shared_files.cc

+#if defined(_MSC_VER) && !defined(_CRT_SECURE_NO_DEPRECATE)
+#define _CRT_SECURE_NO_DEPRECATE 1
+#endif


_CRT_SECURE_NO_DEPRECATE is defined after including src/utils/shared_files.h (which includes <stdio.h>), so it won’t suppress MSVC CRT deprecation warnings for those headers. If this macro is needed, it should be set via compiler definitions or before any CRT headers are included (typically in a central config header or build flags), not locally here.

Copilot · 2026-03-28T14:00:16Z

src/rules_set.cc

+    return succeeded ? 0 : -1;
+}


The C API generally returns 1 for success and 0 for failure for boolean operations (e.g., msc_update_status_code, msc_set_request_hostname), while rules APIs return non-negative on success and negative on failure. Returning 0 on success here is inconsistent and makes it easy for callers to mis-handle success as failure. Consider returning 1 on success and 0 (or a negative value consistently used in the rules API) on failure, and document the convention in the header/comments.

Support reopening audit and debug logs

544a2f5

Fix linter issues

69cec3a

Fix linter issues: use fopen_s on windows and fopen on other OS

329efd4

fremail mentioned this pull request Mar 24, 2026

Support log reopen on SIGHUP and SIGUSR1 owasp-modsecurity/ModSecurity-nginx#372

Open

fzipi requested a review from Copilot March 28, 2026 13:53

Copilot started reviewing on behalf of fzipi March 28, 2026 13:53 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support reopening audit and debug logs#3521

Support reopening audit and debug logs#3521
fremail wants to merge 3 commits intoowasp-modsecurity:v3/masterfrom
fremail:add/log-reopen

fremail commented Mar 23, 2026

Uh oh!

airween commented Mar 23, 2026

Uh oh!

fremail commented Mar 23, 2026 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Mar 23, 2026

Uh oh!

fremail commented Mar 23, 2026

Uh oh!

fremail commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	virtual const std::string& getDebugLogFile();
	const std::string& getDebugLogFile();

-    fclose(it->second.fp);
-    it->second.fp = fp;
+    // Acquire the same exclusive lock used by write() to avoid races on it->second.fp
+#ifndef WIN32
+    struct flock lock {};
+    lock.l_start = lock.l_len = lock.l_whence = 0;
+    lock.l_type = F_WRLCK;
+    if (fcntl(fileno(it->second.fp), F_SETLKW, &lock) == -1) {
+        // Failed to acquire lock; close the newly opened file and report error
+        fclose(fp);
+        error->assign("Failed to lock file during reopen: " + fileName);
+        return false;
+    }
+#else
+    DWORD dwWaitResult = WaitForSingleObject(it->second.hMutex, INFINITE);
+    if (dwWaitResult != WAIT_OBJECT_0) {
+        // Failed to acquire mutex; close the newly opened file and report error
+        fclose(fp);
+        error->assign("Couldn't lock shared file during reopen: " + fileName);
+        return false;
+    }
+#endif
+    fclose(it->second.fp);
+    it->second.fp = fp;
+    // Release the exclusive lock
+#ifndef WIN32
+    lock.l_type = F_UNLCK;
+    fcntl(fileno(it->second.fp), F_SETLKW, &lock);
+#else
+    ::ReleaseMutex(it->second.hMutex);
+#endif

Conversation

fremail commented Mar 23, 2026

Uh oh!

airween commented Mar 23, 2026

Uh oh!

fremail commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 23, 2026

Quality Gate passed

Uh oh!

fremail commented Mar 23, 2026

Uh oh!

fremail commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fremail commented Mar 23, 2026 •

edited

Loading