feat(misc): improve install-bunkerweb.sh for AlmaLinux/RHEL/CentOS, u…#3528
Open
azgaviperr wants to merge 1 commit into
Open
feat(misc): improve install-bunkerweb.sh for AlmaLinux/RHEL/CentOS, u…#3528azgaviperr wants to merge 1 commit into
azgaviperr wants to merge 1 commit into
Conversation
…pgrade safety, CI mode, Redis/CrowdSec - Normalize 'alma' DISTRO_ID alias to 'almalinux' for consistent distro matching - Add CentOS to RHEL/Rocky/AlmaLinux package-manager path - Guard interactive 'Press Enter' prompt behind INTERACTIVE_MODE check - Fix --yes/-y flag: set ENABLE_WIZARD=no (was yes) for non-interactive/CI use - Fix Redis password prompt: use read -s so password is not echoed to terminal - Fix CrowdSec curl: use -fsSL so install errors are not silently swallowed - Fix CrowdSec dep check: check for gpg binary (was gnupg2, which is the package name) - Remove dead unreachable upgrade code block in main() after upgrade_only exits
Packaging & DeploymentFile: SummaryUpdated the BunkerWeb installation script to improve support for AlmaLinux, RHEL, and CentOS distributions, whilst enhancing security, non-interactive (CI) mode handling, and dependency resolution. User-visible behaviour changes
Security impact
Code improvements
Testing & DocumentationNo changes to tests or documentation noted. Lines changed+25/-41 (net change: -16 lines) WalkthroughThe installation script now detects AlmaLinux properly, extends CentOS to RHEL-family support throughout, silences sensitive input, refines interactive-mode prompts, updates CrowdSec dependencies from ChangesInstallation Script Enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
misc/install-bunkerweb.sh (1)
1497-1499:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCentOS path is still missing in CrowdSec install branches
After adding CentOS support, Lines 1497 and 1517 still exclude
centos, so CrowdSec dependency install and package install fall into unsupported/default branches on CentOS.Minimal fix
- "fedora"|"rhel"|"rocky"|"almalinux") + "fedora"|"rhel"|"rocky"|"almalinux"|"centos") run_cmd dnf install -y $dep ;; @@ - "fedora"|"rhel"|"rocky"|"almalinux") + "fedora"|"rhel"|"rocky"|"almalinux"|"centos") run_cmd dnf install -y crowdsec ;;Also applies to: 1517-1519
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@misc/install-bunkerweb.sh` around lines 1497 - 1499, The CentOS distribution string is missing from the CrowdSec install case branches so CentOS falls through to the unsupported/default path; update the case patterns that currently read "fedora"|"rhel"|"rocky"|"almalinux") (the branch that runs run_cmd dnf install -y $dep and the later package-install branch) to also include "centos" so CentOS uses the dnf/CentOS-specific install logic (i.e., add "centos" to the quoted list in both the dependency install and package install case arms).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@misc/install-bunkerweb.sh`:
- Line 1512: Replace the direct pipe-to-shell invocation in the run_cmd call
(the current `run_cmd curl -fsSL https://install.crowdsec.net | sh`) with a
three-step safe flow: use run_cmd to download the installer to a temporary file
(e.g., via curl -fsSL -o <tmpfile>), fetch the corresponding checksum or
signature from the vendor, verify the downloaded file (e.g., sha256sum or gpg
verification), and only if verification succeeds, execute the installer file
with sh and then securely remove the temporary file; update the call site that
currently uses run_cmd with the piped curl to instead call these explicit
download, verify, execute, and cleanup steps using the same run_cmd helper and
error handling.
- Around line 773-774: The upgrade path is missing centos in the upgrade_only()
case branches, which causes services to be stopped but no CentOS-specific
install/upgrade branch to run; update the case patterns inside upgrade_only()
(the branches that currently match "rhel"|"rocky"|"almalinux") to also include
"centos" so the CentOS branch executes during upgrade, and verify the CentOS
branch reuses the same package/selinux/service logic as the rhel/rocky/almalinux
branch (e.g., uses major_version and the same yum/dnf commands) so package
upgrades are not skipped.
---
Outside diff comments:
In `@misc/install-bunkerweb.sh`:
- Around line 1497-1499: The CentOS distribution string is missing from the
CrowdSec install case branches so CentOS falls through to the
unsupported/default path; update the case patterns that currently read
"fedora"|"rhel"|"rocky"|"almalinux") (the branch that runs run_cmd dnf install
-y $dep and the later package-install branch) to also include "centos" so CentOS
uses the dnf/CentOS-specific install logic (i.e., add "centos" to the quoted
list in both the dependency install and package install case arms).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ac3a33d-e828-45c8-b88e-65bf090d0240
📒 Files selected for processing (1)
misc/install-bunkerweb.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
Shell scripts must pass ShellCheck and remain POSIX-compatible unless explicitly using
#!/bin/bashshebang
Files:
misc/install-bunkerweb.sh
⚙️ CodeRabbit configuration file
**/*.sh: Shell scripts must match BunkerWeb's portability expectations:
- If the script is POSIX shell, prefer
set -eu; if it explicitly requires Bash, useset -euo pipefail.- Quote variables and command substitutions consistently and prefer
${var}when concatenating.- Do not use Bash-only features in
/bin/shscripts.- Handle failures explicitly, use
trapfor cleanup where temporary files are created, and usemktempsafely.- Never use
curl | shorwget | sh; verify downloads by checksum or signature and avoid-k/--insecure.- Do not rely on inherited
PATHin privileged contexts; set it explicitly where needed.- Avoid
evaland unsafe command construction from untrusted data.
Files:
misc/install-bunkerweb.sh
Comment on lines
+773
to
774
| "rhel"|"rocky"|"almalinux"|"centos") | ||
| major_version=$(echo "$DISTRO_VERSION" | cut -d. -f1) |
There was a problem hiding this comment.
CentOS support is incomplete in upgrade flow and can skip package upgrade
Line 773 now allows CentOS, but upgrade_only() case arms still omit centos (Line 2323 and Line 2347). In upgrade mode, services are stopped first, then no CentOS install branch runs, leaving an incomplete upgrade path.
Targeted fix in upgrade cases
- fedora|rhel|rocky|almalinux)
+ fedora|rhel|rocky|almalinux|centos)
@@
- fedora|rhel|rocky|almalinux)
+ fedora|rhel|rocky|almalinux|centos)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@misc/install-bunkerweb.sh` around lines 773 - 774, The upgrade path is
missing centos in the upgrade_only() case branches, which causes services to be
stopped but no CentOS-specific install/upgrade branch to run; update the case
patterns inside upgrade_only() (the branches that currently match
"rhel"|"rocky"|"almalinux") to also include "centos" so the CentOS branch
executes during upgrade, and verify the CentOS branch reuses the same
package/selinux/service logic as the rhel/rocky/almalinux branch (e.g., uses
major_version and the same yum/dnf commands) so package upgrades are not
skipped.
| echo -e "${YELLOW}--- Step 1: Add CrowdSec repository and install engine ---${NC}" | ||
| print_step "Adding CrowdSec repository and installing engine" | ||
| run_cmd curl -s https://install.crowdsec.net | sh | ||
| run_cmd curl -fsSL https://install.crowdsec.net | sh |
There was a problem hiding this comment.
Block direct remote script execution (curl | sh)
Line 1512 pipes network content straight into sh, which is a supply-chain execution risk and violates project policy. Download to a temporary file, verify checksum/signature, then execute.
Suggested direction
- run_cmd curl -fsSL https://install.crowdsec.net | sh
+ tmp_installer="$(mktemp)"
+ run_cmd curl -fsSL https://install.crowdsec.net -o "${tmp_installer}"
+ # TODO: verify installer integrity (checksum/signature) before execution.
+ run_cmd sh "${tmp_installer}"
+ run_cmd rm -f "${tmp_installer}"As per coding guidelines "Never use curl | sh or wget | sh; verify downloads by checksum or signature and avoid -k / --insecure."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| run_cmd curl -fsSL https://install.crowdsec.net | sh | |
| tmp_installer="$(mktemp)" | |
| run_cmd curl -fsSL https://install.crowdsec.net -o "${tmp_installer}" | |
| # TODO: verify installer integrity (checksum/signature) before execution. | |
| run_cmd sh "${tmp_installer}" | |
| run_cmd rm -f "${tmp_installer}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@misc/install-bunkerweb.sh` at line 1512, Replace the direct pipe-to-shell
invocation in the run_cmd call (the current `run_cmd curl -fsSL
https://install.crowdsec.net | sh`) with a three-step safe flow: use run_cmd to
download the installer to a temporary file (e.g., via curl -fsSL -o <tmpfile>),
fetch the corresponding checksum or signature from the vendor, verify the
downloaded file (e.g., sha256sum or gpg verification), and only if verification
succeeds, execute the installer file with sh and then securely remove the
temporary file; update the call site that currently uses run_cmd with the piped
curl to instead call these explicit download, verify, execute, and cleanup steps
using the same run_cmd helper and error handling.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the misc/install-bunkerweb.sh installer to behave more safely and consistently across RHEL-family distributions (AlmaLinux/RHEL/Rocky/CentOS) and in non-interactive/CI contexts, while also tightening some prompts and external install steps.
Changes:
- Normalizes distro detection (
alma→almalinux) and expands RHEL-family matching to include CentOS. - Improves non-interactive behavior (guards “Press Enter” prompt; fixes
-y/--yeswizard default; hides Redis password input). - Adjusts CrowdSec install prerequisites and download flags, and removes unreachable upgrade logic in
main().
Comments suppressed due to low confidence (1)
misc/install-bunkerweb.sh:1496
- The dependency check loop uses
command -v ca-certificates, butca-certificatesis a package name (not a command), so this check will always fail and the script will repeatedly try to install it. Use an appropriate package-level check per platform (dpkg/rpm/pkg) or test for a known CA bundle file instead.
# Ensure required dependencies
for dep in curl gpg ca-certificates; do
if ! command -v "$dep" >/dev/null 2>&1; then
print_status "Installing missing dependency: $dep"
case "$DISTRO_ID" in
"debian"|"ubuntu")
run_cmd apt update
run_cmd apt install -y "$dep"
;;
| major_version=$(echo "$DISTRO_VERSION" | cut -d. -f1) | ||
| if [[ "$major_version" != "8" && "$major_version" != "9" && "$major_version" != "10" ]]; then | ||
| print_warning "Only RHEL 8, 9, and 10 are officially supported" | ||
| print_warning "Only RHEL/CentOS 8, 9, and 10 are officially supported" |
Comment on lines
2445
to
2449
| install_nginx_fedora | ||
| ;; | ||
| "rhel"|"rocky"|"almalinux") | ||
| "rhel"|"rocky"|"almalinux"|"centos") | ||
| install_nginx_rhel | ||
| ;; |
| echo -e "${YELLOW}--- Step 1: Add CrowdSec repository and install engine ---${NC}" | ||
| print_step "Adding CrowdSec repository and installing engine" | ||
| run_cmd curl -s https://install.crowdsec.net | sh | ||
| run_cmd curl -fsSL https://install.crowdsec.net | sh |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…pgrade safety, CI mode, Redis/CrowdSec