From 7341f85c2d5d02843d166afcf64f7d66e7412a51 Mon Sep 17 00:00:00 2001 From: RD WebDesign Date: Tue, 12 Nov 2024 16:48:52 -0300 Subject: [PATCH 1/6] Add Fedora 41 and remove Fedora 39 from tests Signed-off-by: RD WebDesign --- .github/workflows/test.yml | 2 +- test/{_fedora_39.Dockerfile => _fedora_41.Dockerfile} | 2 +- test/{tox.fedora_39.ini => tox.fedora_41.ini} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename test/{_fedora_39.Dockerfile => _fedora_41.Dockerfile} (97%) rename test/{tox.fedora_39.ini => tox.fedora_41.ini} (85%) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 381abd15..7c419e9a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -68,8 +68,8 @@ jobs: ubuntu_23, ubuntu_24, centos_9, - fedora_39, fedora_40, + fedora_41, ] env: DISTRO: ${{matrix.distro}} diff --git a/test/_fedora_39.Dockerfile b/test/_fedora_41.Dockerfile similarity index 97% rename from test/_fedora_39.Dockerfile rename to test/_fedora_41.Dockerfile index 1d3dbc63..59858f4e 100644 --- a/test/_fedora_39.Dockerfile +++ b/test/_fedora_41.Dockerfile @@ -1,4 +1,4 @@ -FROM fedora:39 +FROM fedora:41 RUN dnf install -y git initscripts ENV GITDIR=/etc/.pihole diff --git a/test/tox.fedora_39.ini b/test/tox.fedora_41.ini similarity index 85% rename from test/tox.fedora_39.ini rename to test/tox.fedora_41.ini index aaa6b30e..f70da227 100644 --- a/test/tox.fedora_39.ini +++ b/test/tox.fedora_41.ini @@ -6,5 +6,5 @@ allowlist_externals = docker deps = -rrequirements.txt setenv = COLUMNS=120 -commands = docker buildx build --load --progress plain -f _fedora_39.Dockerfile -t pytest_pihole:test_container ../ +commands = docker buildx build --load --progress plain -f _fedora_41.Dockerfile -t pytest_pihole:test_container ../ pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py ./test_centos_fedora_common_support.py From 2dc5bd15cc1f70ae1fad9b2a0b1be879436f1c33 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 16 Nov 2024 08:19:09 +0100 Subject: [PATCH 2/6] Remove remaining traces of audit log Signed-off-by: DL6ER --- advanced/Scripts/database_migration/gravity-db.sh | 14 +++----------- .../Scripts/database_migration/gravity/1_to_2.sql | 7 ------- advanced/Templates/gravity.db.sql | 7 ------- advanced/Templates/gravity_copy.sql | 1 - gravity.sh | 5 +---- 5 files changed, 4 insertions(+), 30 deletions(-) diff --git a/advanced/Scripts/database_migration/gravity-db.sh b/advanced/Scripts/database_migration/gravity-db.sh index 8f84e2b0..b0982bcc 100755 --- a/advanced/Scripts/database_migration/gravity-db.sh +++ b/advanced/Scripts/database_migration/gravity-db.sh @@ -13,10 +13,9 @@ readonly scriptPath="/etc/.pihole/advanced/Scripts/database_migration/gravity" upgrade_gravityDB(){ - local database piholeDir auditFile version + local database piholeDir version database="${1}" piholeDir="${2}" - auditFile="${piholeDir}/auditlog.list" # Exit early if the database does not exist (e.g. in CI tests) if [[ ! -f "${database}" ]]; then @@ -27,18 +26,11 @@ upgrade_gravityDB(){ version="$(pihole-FTL sqlite3 -ni "${database}" "SELECT \"value\" FROM \"info\" WHERE \"property\" = 'version';")" if [[ "$version" == "1" ]]; then - # This migration script upgrades the gravity.db file by - # adding the domain_audit table + # This migration script upgraded the gravity.db file by + # adding the domain_audit table. It is now a no-op echo -e " ${INFO} Upgrading gravity database from version 1 to 2" pihole-FTL sqlite3 -ni "${database}" < "${scriptPath}/1_to_2.sql" version=2 - - # Store audit domains in database table - if [ -e "${auditFile}" ]; then - echo -e " ${INFO} Migrating content of ${auditFile} into new database" - # database_table_from_file is defined in gravity.sh - database_table_from_file "domain_audit" "${auditFile}" - fi fi if [[ "$version" == "2" ]]; then # This migration script upgrades the gravity.db file by diff --git a/advanced/Scripts/database_migration/gravity/1_to_2.sql b/advanced/Scripts/database_migration/gravity/1_to_2.sql index ef445cc6..857d25a6 100644 --- a/advanced/Scripts/database_migration/gravity/1_to_2.sql +++ b/advanced/Scripts/database_migration/gravity/1_to_2.sql @@ -2,13 +2,6 @@ BEGIN TRANSACTION; -CREATE TABLE domain_audit -( - id INTEGER PRIMARY KEY AUTOINCREMENT, - domain TEXT UNIQUE NOT NULL, - date_added INTEGER NOT NULL DEFAULT (cast(strftime('%s', 'now') as int)) -); - UPDATE info SET value = 2 WHERE property = 'version'; COMMIT; diff --git a/advanced/Templates/gravity.db.sql b/advanced/Templates/gravity.db.sql index 5436fb87..fbacc16a 100644 --- a/advanced/Templates/gravity.db.sql +++ b/advanced/Templates/gravity.db.sql @@ -68,13 +68,6 @@ CREATE TABLE info INSERT INTO "info" VALUES('version','19'); -CREATE TABLE domain_audit -( - id INTEGER PRIMARY KEY AUTOINCREMENT, - domain TEXT UNIQUE NOT NULL, - date_added INTEGER NOT NULL DEFAULT (cast(strftime('%s', 'now') as int)) -); - CREATE TABLE domainlist_by_group ( domainlist_id INTEGER NOT NULL REFERENCES domainlist (id), diff --git a/advanced/Templates/gravity_copy.sql b/advanced/Templates/gravity_copy.sql index ed11b61a..f9f98446 100644 --- a/advanced/Templates/gravity_copy.sql +++ b/advanced/Templates/gravity_copy.sql @@ -9,7 +9,6 @@ DROP TRIGGER tr_client_add; DROP TRIGGER tr_adlist_add; INSERT OR REPLACE INTO "group" SELECT * FROM OLD."group"; -INSERT OR REPLACE INTO domain_audit SELECT * FROM OLD.domain_audit; INSERT OR REPLACE INTO domainlist SELECT * FROM OLD.domainlist; DELETE FROM OLD.domainlist_by_group WHERE domainlist_id NOT IN (SELECT id FROM OLD.domainlist); diff --git a/gravity.sh b/gravity.sh index a657727d..40023381 100755 --- a/gravity.sh +++ b/gravity.sh @@ -172,10 +172,7 @@ database_table_from_file() { grep -v '^ *#' <"${src}" | while IFS= read -r domain; do # Only add non-empty lines if [[ -n "${domain}" ]]; then - if [[ "${table}" == "domain_audit" ]]; then - # domain_audit table format (no enable or modified fields) - echo "${rowid},\"${domain}\",${timestamp}" >>"${tmpFile}" - elif [[ "${table}" == "adlist" ]]; then + if [[ "${table}" == "adlist" ]]; then # Adlist table format echo "${rowid},\"${domain}\",1,${timestamp},${timestamp},\"Migrated from ${src}\",,0,0,0,0,0" >>"${tmpFile}" else From 62b63f87e0b47e04e2d602f1909fe3ff2b70d407 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 16 Nov 2024 12:46:43 +0100 Subject: [PATCH 3/6] Use rpm instead of dnf to check for installed package. dnf changed to returning success even for not-installed packages wit Fedora 41 Signed-off-by: DL6ER --- automated install/basic-install.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh index 0931e4ca..8f5b9879 100755 --- a/automated install/basic-install.sh +++ b/automated install/basic-install.sh @@ -1430,7 +1430,7 @@ install_dependent_packages() { for i in "$@"; do # For each package, check if it's already installed (and if so, don't add it to the installArray) printf " %b Checking for %s..." "${INFO}" "${i}" - if "${PKG_MANAGER}" -q list installed "${i}" &>/dev/null; then + if rpm -q "${i}" &>/dev/null; then printf "%b %b Checking for %s\\n" "${OVER}" "${TICK}" "${i}" else printf "%b %b Checking for %s (will be installed)\\n" "${OVER}" "${INFO}" "${i}" @@ -2270,7 +2270,7 @@ main() { notify_package_updates_available # Install packages necessary to perform os_check - printf " %b Checking for / installing Required dependencies for OS Check...\\n" "${INFO}" + printf " %b Checking for / installing required dependencies for OS Check...\\n" "${INFO}" install_dependent_packages "${OS_CHECK_COMMON_DEPS[@]}" "${OS_CHECK_DEPS[@]}" # Check that the installed OS is officially supported - display warning if not @@ -2287,7 +2287,7 @@ main() { fi # Install packages used by this installation script - printf " %b Checking for / installing Required dependencies for this install script...\\n" "${INFO}" + printf " %b Checking for / installing required dependencies for this install script...\\n" "${INFO}" install_dependent_packages "${INSTALLER_COMMON_DEPS[@]}" "${INSTALLER_DEPS[@]}" # in case of an update @@ -2335,7 +2335,7 @@ main() { local dep_install_list=("${PIHOLE_COMMON_DEPS[@]}" "${PIHOLE_DEPS[@]}") # Install packages used by the actual software - printf " %b Checking for / installing Required dependencies for Pi-hole software...\\n" "${INFO}" + printf " %b Checking for / installing required dependencies for Pi-hole software...\\n" "${INFO}" install_dependent_packages "${dep_install_list[@]}" unset dep_install_list From 531490397da3e23695de41b59dd918dc404520b0 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 25 Nov 2024 12:33:26 +0100 Subject: [PATCH 4/6] When new domains are added to gravity and users run the first time gravity in the terminal (not via web), the list.123.abc.com file is created as root and stays like that. This causes issues down the line when users later try to run gravity from the web interface where we do not have root capabilities. This commit checks for write permissions and suggests what to do on error. It always ensures ownership and permissions are correct Signed-off-by: DL6ER --- gravity.sh | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/gravity.sh b/gravity.sh index 40023381..aebc58cb 100755 --- a/gravity.sh +++ b/gravity.sh @@ -59,14 +59,24 @@ gravityTEMPfile="${GRAVITYDB}_temp" gravityDIR="$(dirname -- "${gravityDBfile}")" gravityOLDfile="${gravityDIR}/gravity_old.db" +fix_owner_permissions() { + # Fix ownership and permissions for the specified file + # User and group are set to pihole:pihole + # Permissions are set to 664 (rw-rw-r--) + chown pihole:pihole "${1}" + chmod 664 "${1}" + + # Ensure the containing directory is group writable + chmod g+w "$(dirname -- "${1}")" +} + # Generate new SQLite3 file from schema template generate_gravity_database() { if ! pihole-FTL sqlite3 -ni "${gravityDBfile}" <"${gravityDBschema}"; then echo -e " ${CROSS} Unable to create ${gravityDBfile}" return 1 fi - chown pihole:pihole "${gravityDBfile}" - chmod g+w "${piholeDir}" "${gravityDBfile}" + fix_owner_permissions "${gravityDBfile}" } # Build gravity tree @@ -413,6 +423,19 @@ gravity_DownloadBlocklists() { saveLocation="${piholeDir}/list.${id}.${domain}.${domainsExtension}" activeDomains[$i]="${saveLocation}" + # Check if we can write to the save location file + if ! touch "${saveLocation}" 2>/dev/null; then + echo -e " ${CROSS} Unable to write to ${saveLocation}" + echo " Please run pihole -g as root" + echo "" + continue + fi + + # Chown the file to the pihole user + # This is necessary for the FTL to be able to update the file + # when gravity is run from the web interface + fix_owner_permissions "${saveLocation}" + echo -e " ${INFO} Target: ${url}" local regex check_url # Check for characters NOT allowed in URLs @@ -996,8 +1019,7 @@ fi update_gravity_timestamp # Ensure proper permissions are set for the database -chown pihole:pihole "${gravityTEMPfile}" -chmod g+w "${piholeDir}" "${gravityTEMPfile}" +fix_owner_permissions "${gravityTEMPfile}" # Build the tree timeit gravity_build_tree From d9288b896ea5bdd6b95d46c77a34746bf7b2cfbc Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 26 Nov 2024 18:01:52 +0100 Subject: [PATCH 5/6] Do not touch list files as this creates them. This causes issues down the line in the Heisenberg compensator Signed-off-by: DL6ER --- gravity.sh | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/gravity.sh b/gravity.sh index aebc58cb..6ffac4b0 100755 --- a/gravity.sh +++ b/gravity.sh @@ -358,7 +358,7 @@ gravity_DownloadBlocklists() { unset sources fi - local url domain str target compression adlist_type + local url domain str target compression adlist_type directory echo "" # Prepare new gravity database @@ -423,8 +423,18 @@ gravity_DownloadBlocklists() { saveLocation="${piholeDir}/list.${id}.${domain}.${domainsExtension}" activeDomains[$i]="${saveLocation}" - # Check if we can write to the save location file - if ! touch "${saveLocation}" 2>/dev/null; then + # Check if we can write to the save location file without actually creating + # it (in case it doesn't exist) + # First, check if the directory is writable + directory="$(dirname -- "${saveLocation}")" + if [ ! -w "${directory}" ]; then + echo -e " ${CROSS} Unable to write to ${directory}" + echo " Please run pihole -g as root" + echo "" + continue + fi + # Then, check if the file is writable (if it exists) + if [ -e "${saveLocation}" ] && [ ! -w "${saveLocation}" ]; then echo -e " ${CROSS} Unable to write to ${saveLocation}" echo " Please run pihole -g as root" echo "" @@ -464,6 +474,7 @@ compareLists() { if ! sha1sum --check --status --strict "${target}.sha1"; then # The list changed upstream, we need to update the checksum sha1sum "${target}" >"${target}.sha1" + fix_owner_permissions "${target}.sha1" echo " ${INFO} List has been updated" database_adlist_status "${adlistID}" "1" else @@ -473,6 +484,7 @@ compareLists() { else # No checksum available, create one for comparing on the next run sha1sum "${target}" >"${target}.sha1" + fix_owner_permissions "${target}.sha1" # We assume here it was changed upstream database_adlist_status "${adlistID}" "1" fi From 1c4a48258fd60edff32c719a353c88c505494c9d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 26 Nov 2024 18:16:15 +0100 Subject: [PATCH 6/6] Only set ownership/permissions once the file was created Signed-off-by: DL6ER --- gravity.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/gravity.sh b/gravity.sh index 6ffac4b0..ccba6c74 100755 --- a/gravity.sh +++ b/gravity.sh @@ -441,11 +441,6 @@ gravity_DownloadBlocklists() { continue fi - # Chown the file to the pihole user - # This is necessary for the FTL to be able to update the file - # when gravity is run from the web interface - fix_owner_permissions "${saveLocation}" - echo -e " ${INFO} Target: ${url}" local regex check_url # Check for characters NOT allowed in URLs @@ -746,7 +741,7 @@ gravity_ParseFileIntoDomains() { -e 's/^.*\s+//g' \ -e '/^$/d' "${destination}" - chmod 644 "${destination}" + fix_owner_permissions "${destination}" } # Report number of entries in a table