Skip to content

modsecurity#12592

Merged
mekarpeles merged 4 commits into
masterfrom
modsecurity
May 7, 2026
Merged

modsecurity#12592
mekarpeles merged 4 commits into
masterfrom
modsecurity

Conversation

@mekarpeles

Copy link
Copy Markdown
Member

Half of #12591

This pull request introduces ModSecurity support to our Nginx configuration, enhancing the web server's security by enabling a web application firewall. The main changes involve installing the ModSecurity library, loading its module in Nginx, and configuring it to use a custom ruleset.

Security Enhancements:

  • Installed libmodsecurity3 and libmodsecurity-dev packages in the install_nginx.sh script to provide ModSecurity support for Nginx.
  • Loaded the ngx_http_modsecurity_module.so module in nginx.conf to enable ModSecurity functionality.

Nginx Configuration Updates:

  • Enabled ModSecurity in the web_nginx.conf server block and specified the rules file at /olsystem/etc/nginx/modsecurity_openlibrary.conf.

Copilot AI review requested due to automatic review settings May 5, 2026 00:16
Comment thread scripts/install_nginx.sh Outdated
# Install nginx and the NJS module
apt-get install -y --no-install-recommends nginx nginx-module-njs letsencrypt
# Install modsecurity
apt-get install -y libmodsecurity3 libmodsecurity-dev

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be the complete list

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need modsecurity-crs ?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to add ModSecurity (WAF) support to the nginx setup used by Open Library’s dockerized deployment by installing ModSecurity dependencies, loading the ModSecurity nginx module, and enabling ModSecurity for the main openlibrary.org server block.

Changes:

  • Installs ModSecurity library packages in scripts/install_nginx.sh.
  • Loads ngx_http_modsecurity_module.so in docker/nginx.conf.
  • Enables ModSecurity and points nginx at a rules file in docker/web_nginx.conf.

Reviewed changes

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

File Description
scripts/install_nginx.sh Adds ModSecurity library package installs during nginx setup.
docker/nginx.conf Attempts to load the ModSecurity nginx dynamic module at nginx startup.
docker/web_nginx.conf Enables ModSecurity for openlibrary.org and references an external rules file path.

Comment thread scripts/install_nginx.sh
Comment on lines +18 to +22
# Install nginx and the NJS module
apt-get install -y --no-install-recommends nginx nginx-module-njs letsencrypt
# Install modsecurity
apt-get install -y libmodsecurity3 libmodsecurity-dev

Comment thread scripts/install_nginx.sh Outdated
Comment thread docker/nginx.conf Outdated
@@ -1,5 +1,6 @@
# Needed for IP anonymization
load_module modules/ngx_http_js_module.so;
load_module modules/ngx_http_modsecurity_module.so;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct?

Comment thread docker/web_nginx.conf

@cdrini cdrini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments; otherwise lgtm

Comment thread docker/web_nginx.conf
Comment thread docker/nginx.conf Outdated
@@ -1,5 +1,6 @@
# Needed for IP anonymization
load_module modules/ngx_http_js_module.so;
load_module modules/ngx_http_modsecurity_module.so;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct?

Comment thread docker/nginx.conf Outdated
@@ -1,5 +1,6 @@
# Needed for IP anonymization
load_module modules/ngx_http_js_module.so;
load_module modules/ngx_http_modsecurity_module.so;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this will also apply to covers ; so we might have some trouble on covers if importing this causes issues.

Comment thread scripts/install_nginx.sh Outdated
# Install nginx and the NJS module
apt-get install -y --no-install-recommends nginx nginx-module-njs letsencrypt
# Install modsecurity
apt-get install -y libmodsecurity3 libmodsecurity-dev

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need modsecurity-crs ?

Comment thread scripts/install_nginx.sh Outdated
@mekarpeles mekarpeles added Priority: 0 Fix now: Issue prevents users from using the site or active data corruption. [managed] Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] Theme: Security labels May 5, 2026
@github-actions github-actions Bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label May 7, 2026
mekarpeles added 4 commits May 7, 2026 13:29
Updated modsecurity module path and adjusted worker processes.
Replaced installation of libmodsecurity-dev with modsecurity-crs.
Updated install script to ensure modsecurity is installed correctly.
@mekarpeles mekarpeles merged commit 72abc4e into master May 7, 2026
11 checks passed
@mekarpeles

Copy link
Copy Markdown
Member Author

@cdrini and I reviewed together during 1:1

@mekarpeles mekarpeles deleted the modsecurity branch May 7, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 0 Fix now: Issue prevents users from using the site or active data corruption. [managed] Theme: Security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants