fix: update 8g-firewall rule config update script

[alaminfirdows]

Ref: #12

Summary by CodeRabbit

• Chores
  • Configuration backup files are now tracked in version control (no longer ignored).
  • Added a new utility to fetch and safely replace firewall configuration from a central source, preserving permissions and reporting status.
  • Removed an older automated update utility; workflow for updating configs has been simplified.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

scripts/8g-firewall.sh (1)

4-5: Clarify execution context in documentation.

The comment "Run from: {SITE_DIR}/config directory" uses placeholder syntax but doesn't clearly specify requirements. The script expects to be run from a directory where ./nginx/common/ exists. Consider adding more explicit guidance or examples.

Consider updating the documentation:

 # Download and update configuration files
-# Run from: {SITE_DIR}/config directory
+# Prerequisites: curl must be available
+# Usage: cd /path/to/site/config && bash ../../scripts/8g-firewall.sh
+# Or adjust paths as needed for your directory structure

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a21c5d and 7758b00.

📒 Files selected for processing (3)

• .gitignore (0 hunks)
• scripts/8g-firewall.sh (1 hunks)
• scripts/update-8g-firewall.sh (0 hunks)

💤 Files with no reviewable changes (2)

• .gitignore
• scripts/update-8g-firewall.sh

🔇 Additional comments (4)

scripts/8g-firewall.sh (4)

1-9: Script initialization looks solid.

The shebang, error handling with set -euo pipefail, and configuration structure are clean and idiomatic.

---

11-23: Download error handling is well‑structured.

The temporary file naming with timestamps prevents collision, and cleanup via rm -f happens on download failure.

---

25-30: Unchanged file detection is correct.

The cmp -s binary comparison is appropriate, and the short-circuit logic properly handles the case where the destination file doesn't yet exist.

---

25-36: TOCTOU concern is valid but context-dependent—request explicit confirmation on deployment model.

The race condition between the cmp check (line 26) and the mv operation (line 34) is technically real. However, this script appears designed for non-concurrent use (manual or cron-based maintenance). If this script never runs concurrently on the same system—which is typical for config update scripts—the concern is moot. If concurrent execution is possible (e.g., multiple admins, overlapping cron jobs, other processes targeting the same files), then atomic operations or locking should be added.

Recommend: Clarify whether concurrent execution is expected in your deployment. If yes, apply atomic safeguards (e.g., mkdir for locking or atomic mv with pre-check). If no, this can be deferred.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

scripts/8g-firewall.sh (1)

42-42: Clarify the final status message (flagged in previous review).

The message "Add Done!" is unclear. Consider a more descriptive status message.

Apply this diff for clarity:

-echo "Add Done!"
+echo "✅ All Done!"

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7758b00 and 8a53dc3.

📒 Files selected for processing (1)

• scripts/8g-firewall.sh (1 hunks)

🔇 Additional comments (1)

scripts/8g-firewall.sh (1)

7-40: Core logic looks solid.

The download, compare, and install flow is sound. Error handling on download failures is present, temp file cleanup is managed by the trap handler, and comparisons prevent unnecessary updates.