From 2dc5bd15cc1f70ae1fad9b2a0b1be879436f1c33 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 16 Nov 2024 08:19:09 +0100 Subject: [PATCH 1/7] 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 531490397da3e23695de41b59dd918dc404520b0 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 25 Nov 2024 12:33:26 +0100 Subject: [PATCH 2/7] 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 3/7] 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 4/7] 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 From b23348916b3b7cf175128c7ad32ce69ea28dc294 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 26 Nov 2024 21:07:11 +0100 Subject: [PATCH 5/7] Remove Ubuntu 23 tests, it is EOL Signed-off-by: DL6ER --- .github/workflows/test.yml | 1 - test/tox.ubuntu_23.ini | 10 ---------- 2 files changed, 11 deletions(-) delete mode 100644 test/tox.ubuntu_23.ini diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7c419e9a..983ca0bc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,7 +65,6 @@ jobs: debian_12, ubuntu_20, ubuntu_22, - ubuntu_23, ubuntu_24, centos_9, fedora_40, diff --git a/test/tox.ubuntu_23.ini b/test/tox.ubuntu_23.ini deleted file mode 100644 index f0a32a68..00000000 --- a/test/tox.ubuntu_23.ini +++ /dev/null @@ -1,10 +0,0 @@ -[tox] -envlist = py3 - -[testenv:py3] -allowlist_externals = docker -deps = -rrequirements.txt -setenv = - COLUMNS=120 -commands = docker buildx build --load --progress plain -f _ubuntu_23.Dockerfile -t pytest_pihole:test_container ../ - pytest {posargs:-vv -n auto} ./test_any_automated_install.py ./test_any_utils.py From 5ef4a5e8b074aa878c6f82248b5f0c7f0c6f5a2e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 7 Dec 2024 10:09:54 +0000 Subject: [PATCH 6/7] Bump pytest from 8.3.3 to 8.3.4 in /test Bumps [pytest](https://github.com/pytest-dev/pytest) from 8.3.3 to 8.3.4. - [Release notes](https://github.com/pytest-dev/pytest/releases) - [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pytest-dev/pytest/compare/8.3.3...8.3.4) --- updated-dependencies: - dependency-name: pytest dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- test/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/requirements.txt b/test/requirements.txt index 93232df7..d72475c4 100644 --- a/test/requirements.txt +++ b/test/requirements.txt @@ -1,5 +1,5 @@ pyyaml == 6.0.2 -pytest == 8.3.3 +pytest == 8.3.4 pytest-xdist == 3.6.1 pytest-testinfra == 10.1.1 tox == 4.23.2 From 11e00e04b5bb7cf99131552f566db7209b5f03c6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 7 Dec 2024 10:27:20 +0100 Subject: [PATCH 7/7] Fix ARP flush command Signed-off-by: DL6ER --- advanced/Scripts/piholeARPTable.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/advanced/Scripts/piholeARPTable.sh b/advanced/Scripts/piholeARPTable.sh index c04c5b33..f55b1320 100755 --- a/advanced/Scripts/piholeARPTable.sh +++ b/advanced/Scripts/piholeARPTable.sh @@ -32,7 +32,7 @@ flushARP(){ fi # Stop FTL to prevent database access - if ! output=$(pihole-FTL service stop 2>&1); then + if ! output=$(service pihole-FTL stop 2>&1); then echo -e "${OVER} ${CROSS} Failed to stop FTL" echo " Output: ${output}" return 1 @@ -64,7 +64,7 @@ flushARP(){ fi # Start FTL again - if ! output=$(pihole-FTL service restart 2>&1); then + if ! output=$(service pihole-FTL restart 2>&1); then echo -e "${OVER} ${CROSS} Failed to restart FTL" echo " Output: ${output}" return 1