Alternative fix for @inspectFile: improved fork safety with multi-threaded tests (PR #3489)

[Easton97-Jens]

---

what

• Provides an alternative implementation to the approach proposed in PR Hardening: Avoid shell-based popen usage in InspectFile operator #3489
• Reworks the @inspectFile execution flow with a focus on safety in multi-threaded environments
• Minimizes the amount of work performed in the child process after fork()
• Avoids unsafe operations (such as dynamic memory allocation and complex logic) before execvp()
• Improves handling of process synchronization (fork() / waitpid()) to reduce the risk of blocking behavior
• Introduces a more robust execution path for external inspection programs
• Adds test scenarios based on the multi-threaded example to validate the behavior under concurrent workloads

---

why

• PR Hardening: Avoid shell-based popen usage in InspectFile operator #3489 addresses the reported issue, but concerns remain regarding behavior in multi-threaded servers

• In a multi-threaded environment (e.g. NGINX or Apache), calling fork() can lead to inconsistent internal states due to POSIX semantics

• If a mutex (e.g. from the memory allocator) is locked during fork(), the child process may inherit it in a locked state

• Any subsequent memory allocation or complex operation in the child process can result in a deadlock

• This may cause the parent process to block on waitpid(), leading to worker starvation and potential Denial of Service (DoS)

• This implementation takes a different approach by:

  • strictly limiting operations performed in the child process
  • avoiding code paths that may trigger allocator usage after fork()
  • making the execution flow more predictable and resilient in threaded environments

• The included tests aim to reproduce and validate the behavior in multi-threaded scenarios

---

references

• Alternative to: Hardening: Avoid shell-based popen usage in InspectFile operator #3489
• Documentation note on @inspectFile:
  https://github.com/owasp-modsecurity/ModSecurity/wiki/Reference-Manual-(v3.x)#user-content-inspectFile
• Related discussion: potential deadlock due to fork() in multi-threaded environments
• Possible impact: worker starvation / DoS via blocking waitpid()

---

[airween]

@Easton97-Jens - there are some SonarCloud issues, please take a look at them.

[airween]

@praneet390, @fumfel - could you take a look at this PR too? This includes an improved multi-threaded test under the examples/.

[Copilot]

Pull request overview

This PR reworks the @inspectFile operator’s external helper execution path to be safer in multi-threaded environments by avoiding shell-based execution and minimizing unsafe post-fork() work, and adds regression/stress coverage to validate behavior under concurrency.

Changes:

• Replaces popen()/shell execution in @inspectFile with posix_spawn() on POSIX and CreateProcessW() on Windows, capturing stdout via pipes.
• Adds POSIX-focused regression tests and helper fixtures for @inspectFile (match/no-match and exec failure behavior).
• Extends the examples/multithread program and adds a stress script/config to exercise @inspectFile under concurrent workloads.

Reviewed changes

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

Show a summary per file

File	Description
src/operators/inspect_file.cc	Replaces shell-based execution with spawn/process APIs and pipe-based stdout capture.
test/test-cases/regression/issue-3489-inspectfile-posix.json	Adds regression coverage for non-shell helper execution and exec failure handling.
test/test-cases/data/inspectfile-posix-helper.sh	Adds deterministic helper script used by the new regression test.
test/test-cases/data/inspectfile-not-executable.txt	Adds a fixture to validate exec failure behavior.
examples/multithread/multithread.cc	Makes the multithread example parameterizable and adds basic completion/FD markers.
examples/multithread/inspectfile_rules.conf	Adds rules to ensure each transaction triggers @inspectFile in the stress scenario.
examples/multithread/inspectfile_helper.sh	Adds deterministic helper for the multithread stress scenario.
examples/multithread/Makefile.am	Ships new example rule/helper files via EXTRA_DIST.
test/stress/inspectfile_multithread_stress.sh	Adds a runnable stress script to validate behavior under concurrent load.

---

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

[praneet390]

sure @airween ill have a look at it

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
1 Accepted issue

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

See analysis details on SonarQube Cloud

[fumfel]

@praneet390, @fumfel - could you take a look at this PR too? This includes an improved multi-threaded test under the examples/.

Tested on macOS (Darwin 25.4.0, arm64).

No deadlocks, no fd leaks, no hangs on waitpid. posix_spawn path looks solid.

Note: count_open_fds() in examples/multithread/multithread.cc uses /proc/self/fd, so fd accounting is Linux-only — on macOS it returns -1 (handled gracefully). Worth a confirmation run on Linux to validate the fd-leak detector.

LGTM from my side.