chore(repo): cleanup + v6-docs + healthcheck installer + SC2034 suppress #36

TimInTech · 2025-11-12T10:55:53Z

No description provided.

Copilot

Pull Request Overview

This PR updates the repository to align with Pi-hole v6 changes, adding session-based authentication support, improving the installer, and cleaning up documentation. The changes migrate away from deprecated v6 features (like cli_pw and reloaddns) while adding new functionality like a healthcheck tool installer.

Key changes:

Migrated API authentication from Basic Auth to session-based auth with POST /api/auth and X-FTL-SID header
Added healthcheck tool to the installer with robust fallback mechanisms
Updated backup operations to use v6 file structure (pihole.toml instead of setupVars.conf, hosts/*.list instead of custom.list)

Reviewed Changes

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

Show a summary per file

File	Description
tools/pihole_api_healthcheck.sh	Replaced Basic Auth with session-based authentication for v6 API
scripts/install.sh	Added root check, healthcheck tool installer, and idempotent cron setup
scripts/uninstall.sh	Added root check and `--purge` flag for conditional backup removal
pihole_maintenance_pro.sh	Added `--restart-ftl` flag, deprecated `--no-dnsreload`, updated backup paths for v6
docs/Anleitung_DE.md	Reformatted and updated documentation for v6 API changes
README.md	Updated documentation with v6 session auth details and formatting improvements
README.de.md	German version of README updates
LICENSE	Updated copyright holder name
.gitignore	Added common development artifacts to ignore list
.env.example	New file documenting environment variables for healthcheck

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

scripts/install.sh

tools/pihole_api_healthcheck.sh

pihole_maintenance_pro.sh

tools/pihole_api_healthcheck.sh

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

scripts/install.sh

Copilot

Pull Request Overview

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

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

Copilot · 2025-11-12T11:05:10Z

pihole_maintenance_pro.sh

  cp -a /etc/pihole/pihole-FTL.db "$BACKUP_DIR" 2> /dev/null || true
-  cp -a /etc/pihole/custom.list "$BACKUP_DIR" 2> /dev/null || true
+  cp -a /etc/pihole/pihole.toml "$BACKUP_DIR" 2> /dev/null || true
+  cp -a /etc/pihole/hosts/*.list "$BACKUP_DIR" 2> /dev/null || true


Glob pattern may fail if no matching files exist. The pattern /etc/pihole/hosts/*.list will fail if the hosts directory doesn't exist or contains no .list files, causing cp to attempt copying the literal string instead. Consider adding a check or using a more robust pattern: [[ -d /etc/pihole/hosts ]] && find /etc/pihole/hosts -name '*.list' -exec cp -a {} "$BACKUP_DIR" \; 2>/dev/null || true

Copilot · 2025-11-12T11:05:11Z

pihole_maintenance_pro.sh

  cp -a /etc/pihole/*.db "$backup_dir" 2> /dev/null || true
-  cp -a /etc/pihole/*.conf "$backup_dir" 2> /dev/null || true
+  cp -a /etc/pihole/pihole.toml "$backup_dir" 2> /dev/null || true
+  cp -a /etc/pihole/hosts/*.list "$backup_dir" 2> /dev/null || true


Glob pattern may fail if no matching files exist. The pattern /etc/pihole/hosts/*.list will fail if the hosts directory doesn't exist or contains no .list files, causing cp to attempt copying the literal string instead. Consider adding a check or using a more robust pattern: [[ -d /etc/pihole/hosts ]] && find /etc/pihole/hosts -name '*.list' -exec cp -a {} "$backup_dir" \; 2>/dev/null || true

Copilot · 2025-11-12T11:05:11Z

scripts/uninstall.sh

+if (( PURGE )) && [[ -d /var/backups/pihole ]]; then
+  rm -rf /var/backups/pihole/
+  echo "✔ Removed backups: /var/backups/pihole/ (purge)"
+else


Misleading message when backups don't exist. The else branch at line 40-41 prints "Backups preserved (use --purge to remove)" even when /var/backups/pihole doesn't exist or when PURGE is 0 but the directory doesn't exist. Consider: elif [[ -d /var/backups/pihole ]]; then echo "ℹ Backups preserved..."; fi to only show the message when backups actually exist.

Suggested change

else

elif [[ -d /var/backups/pihole ]]; then

Copilot · 2025-11-12T11:05:11Z

pihole_maintenance_pro.sh

  --no-upgrade     Skip "pihole -up"
  --no-gravity     Skip "pihole -g"
  --no-dnsreload   Skip "pihole reloaddns"
+  --restart-ftl    Restart pihole-FTL at the end (v6: only if needed)


Incomplete help text: the --backup flag is implemented and parsed (lines 84-87) but not documented in the help text. Add a line to the help output describing this flag, e.g., --backup Create backup before Pi-hole ops under /var/backups/pihole/

Suggested change

--restart-ftl Restart pihole-FTL at the end (v6: only if needed)

--restart-ftl Restart pihole-FTL at the end (v6: only if needed)

--backup Create backup before Pi-hole ops under /var/backups/pihole/

Copilot · 2025-11-12T11:05:12Z

scripts/install.sh

+HC_URL="$RAW_BASE/tools/pihole_api_healthcheck.sh"
+HC_TMP="$TMP/pihole_api_healthcheck.sh"
+HC_OUT="/usr/local/bin/pihole_api_healthcheck.sh"
+if curl -fsSL "$HC_URL" -o "$HC_TMP" || curl -fsSL "$HC_URL" -o "$HC_TMP"; then


Redundant duplicate curl attempt detected. Line 21 executes the same curl command twice with ||, which means if the first attempt fails, it immediately retries the exact same command. This is likely unintentional and doesn't add meaningful retry logic. Consider either removing the duplicate or implementing proper retry logic with a delay if needed.

Suggested change

if curl -fsSL "$HC_URL" -o "$HC_TMP" || curl -fsSL "$HC_URL" -o "$HC_TMP"; then

# Retry curl up to 3 times with 2s delay between attempts

ATTEMPTS=3

for i in $(seq 1 $ATTEMPTS); do

if curl -fsSL "$HC_URL" -o "$HC_TMP"; then

break

elif [[ $i -lt $ATTEMPTS ]]; then

sleep 2

fi

done

if [[ -f "$HC_TMP" ]]; then

Copilot · 2025-11-12T11:05:12Z

tools/pihole_api_healthcheck.sh

+  local extra_headers=()
+  [[ -n "$SID" ]] && extra_headers+=(-H "X-FTL-SID: $SID")
+  if ! curl -sS --connect-timeout 3 --max-time 7 \
+    "${extra_headers[@]}" -H "Accept: application/json" \


Potential issue with empty array expansion when set -u is enabled. When SID is empty, extra_headers remains an empty array, and "${extra_headers[@]}" will cause an "unbound variable" error in bash with set -u. Use "${extra_headers[@]+"${extra_headers[@]}"}" or disable set -u temporarily, or add a conditional to only include the array expansion when it's not empty.

Suggested change

"${extra_headers[@]}" -H "Accept: application/json" \

"${extra_headers[@]+"${extra_headers[@]}"}" -H "Accept: application/json" \

Copilot · 2025-11-12T11:05:12Z

scripts/uninstall.sh

  echo "✔ Removed: /usr/local/bin/pihole_maintenance_pro.sh"
 fi

 # 2. Remove logs and temporary files


Incomplete cleanup: the installer now installs the healthcheck script to /usr/local/bin/pihole_api_healthcheck.sh (line 22 in install.sh), but the uninstaller doesn't remove it. This leaves orphaned files after uninstall. Add: rm -f /usr/local/bin/pihole_api_healthcheck.sh in the uninstall script.

Suggested change

# 2. Remove logs and temporary files

…nimal

…oad; allow RUN_SELFTEST without root; fix awk quoting in healthcheck; align with CI selftest (cherry picked from commit ea379eb)

…ller; uninstall removes healthcheck; dotenv example and ignores

chore(repo): cleanup + v6-docs + healthcheck installer + SC2034 suppress

d4fa0e8

Copilot AI review requested due to automatic review settings November 12, 2025 10:55

Copilot started reviewing on behalf of TimInTech November 12, 2025 10:56 View session

Copilot finished reviewing on behalf of TimInTech November 12, 2025 10:57

Copilot AI reviewed Nov 12, 2025

View reviewed changes

scripts/install.sh Show resolved Hide resolved

tools/pihole_api_healthcheck.sh Show resolved Hide resolved

pihole_maintenance_pro.sh Show resolved Hide resolved

pihole_maintenance_pro.sh Show resolved Hide resolved

tools/pihole_api_healthcheck.sh Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Nov 12, 2025

View reviewed changes

scripts/install.sh Show resolved Hide resolved

TimInTech requested a review from Copilot November 12, 2025 10:59

Copilot started reviewing on behalf of TimInTech November 12, 2025 11:00 View session

Copilot finished reviewing on behalf of TimInTech November 12, 2025 11:03

Copilot AI reviewed Nov 12, 2025

View reviewed changes

TimInTech added 3 commits November 12, 2025 12:08

chore(repo): remove unused assets/icons placeholder to keep online mi…

3448e0b

…nimal

chore(ci): add Makefile for ci-sanity; suppress SC2034 in --no-dnsrel…

d62178d

…oad; allow RUN_SELFTEST without root; fix awk quoting in healthcheck; align with CI selftest (cherry picked from commit ea379eb)

chore(repo): add Makefile check/fmt; docs v6 notes; healthcheck insta…

3480651

…ller; uninstall removes healthcheck; dotenv example and ignores

TimInTech merged commit 6eee154 into main Nov 12, 2025
4 checks passed

TimInTech deleted the fix/online-minimal-cleanup branch November 12, 2025 11:28

	--restart-ftl Restart pihole-FTL at the end (v6: only if needed)
	--restart-ftl Restart pihole-FTL at the end (v6: only if needed)
	--backup Create backup before Pi-hole ops under /var/backups/pihole/

-if curl -fsSL "$HC_URL" -o "$HC_TMP" || curl -fsSL "$HC_URL" -o "$HC_TMP"; then
+# Retry curl up to 3 times with 2s delay between attempts
+ATTEMPTS=3
+for i in $(seq 1 $ATTEMPTS); do
+  if curl -fsSL "$HC_URL" -o "$HC_TMP"; then
+    break
+  elif [[ $i -lt $ATTEMPTS ]]; then
+    sleep 2
+  fi
+done
+if [[ -f "$HC_TMP" ]]; then

	"${extra_headers[@]}" -H "Accept: application/json" \
	"${extra_headers[@]+"${extra_headers[@]}"}" -H "Accept: application/json" \

chore(repo): cleanup + v6-docs + healthcheck installer + SC2034 suppress #36

chore(repo): cleanup + v6-docs + healthcheck installer + SC2034 suppress #36

Uh oh!

Conversation

TimInTech commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant