From 861a51a22c93f02008612a66ebb59de03b18e8aa Mon Sep 17 00:00:00 2001 From: librelad Date: Tue, 18 Aug 2026 22:09:37 +0100 Subject: [PATCH] Stop misreporting a reinstall's admin account and stale firewall rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bookstack: create-admin fails on a reinstall because the account is already provisioned. That took the generic-failure branch, which printed the upstream defaults (admin@admin.com / password) as "the" login — those credentials were replaced on the first install, so the one line a user would act on was the wrong one. Detect "already exists" and say the existing account was kept and its password not reset. Firewall: uninstall deleted only resource_type='port', orphaning the port_tag_*/traefik_managed/url_accessible rows the rebuild reads. Every rebuild then walked ports for long-gone apps and printed "Skipped: (app not found)" per row. Widen the uninstall delete to all non-'ip' rows (the source), and prune already-orphaned rows in the rebuild (the self-heal). Pruning requires both no container dir AND status != 1 in the apps table, so a mid-flight install can't prune itself. Verified on a live bookstack reinstall: admin path reports correctly, firewall pruned nextcloud + stalwart once, second run silent, 11 rules added / 0 failed throughout. Co-Authored-By: Claude Opus 5 --- .../scripts/bookstack_install_hooks.sh | 9 ++++++++ .../rules/firewall_rebuild_from_db.sh | 21 +++++++++++++++++++ .../network/ports/core/port_remove_from_db.sh | 12 +++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/containers/bookstack/scripts/bookstack_install_hooks.sh b/containers/bookstack/scripts/bookstack_install_hooks.sh index f86053c..2e319e7 100644 --- a/containers/bookstack/scripts/bookstack_install_hooks.sh +++ b/containers/bookstack/scripts/bookstack_install_hooks.sh @@ -69,6 +69,15 @@ PHP echo " Email : ${bookstack_target_email}" echo " Password : ${bookstack_target_pass}" echo "" + elif [[ "$bookstack_create_output" == *"already exists"* ]]; then + # Reinstall over existing data: the account is already provisioned, so + # create-admin refuses. NOT a failure, and printing the upstream + # defaults here would be actively wrong — those credentials were + # replaced on the first install, and the password in the config was + # never re-applied to the live account. + isSuccessful "Bookstack admin account already exists (email: $bookstack_target_email) — kept as-is." + isNotice "Its password was NOT reset by this install. Sign in with the credentials from the original install, or reset from inside Bookstack." + echo "" else isNotice "Bookstack admin auto-create failed (exit $bookstack_create_rc). Output:" echo "$bookstack_create_output" | sed 's/^/ /' diff --git a/scripts/network/firewall/rules/firewall_rebuild_from_db.sh b/scripts/network/firewall/rules/firewall_rebuild_from_db.sh index 9270c4f..4eaf409 100755 --- a/scripts/network/firewall/rules/firewall_rebuild_from_db.sh +++ b/scripts/network/firewall/rules/firewall_rebuild_from_db.sh @@ -43,6 +43,27 @@ firewallRebuildFromDatabase() WHERE nr1.resource_type = 'port_tag_external' AND nr1.status = 'active'" + # Self-heal rows orphaned by an uninstall from before portsRemoveFromDatabase + # widened its DELETE (it removed only resource_type='port', leaving the + # port_tag_* rows behind). Without this, every rebuild walked the ports of + # long-gone apps and printed a "Skipped: (app not found)" line each. + # Both signals are required before deleting — no container dir AND not marked + # installed in the apps table — so an install still mid-flight (tag rows + # written, dir not yet created) is never pruned out from under itself. + local stale_apps=() _stale_app + while IFS= read -r _stale_app; do + [[ -n "$_stale_app" ]] || continue + [[ -d "$containers_dir/$_stale_app" ]] && continue + [[ "$(sqlite3 "$docker_dir/$db_file" "SELECT status FROM apps WHERE name = '$_stale_app';" 2>/dev/null)" == "1" ]] && continue + stale_apps+=("$_stale_app") + done <<< "$(sqlite3 "$docker_dir/$db_file" "SELECT DISTINCT app_name FROM network_resources;" 2>/dev/null)" + if ((${#stale_apps[@]} > 0)); then + for _stale_app in "${stale_apps[@]}"; do + runInstallOp sqlite3 "$docker_dir/$db_file" "DELETE FROM network_resources WHERE app_name = '$_stale_app';" 2>/dev/null + done + isNotice "Pruned stale network entries for uninstalled apps: ${stale_apps[*]}" + fi + local total_added=0 local total_failed=0 diff --git a/scripts/network/ports/core/port_remove_from_db.sh b/scripts/network/ports/core/port_remove_from_db.sh index eb46424..85019a4 100755 --- a/scripts/network/ports/core/port_remove_from_db.sh +++ b/scripts/network/ports/core/port_remove_from_db.sh @@ -8,8 +8,16 @@ portsRemoveFromDatabase() if [[ -z "$app_name" ]]; then isError "App name is required for port removal" else - # Delete port allocations for this app - local result; result=$(sqlite3 "$docker_dir/$db_file" "DELETE FROM network_resources WHERE app_name = '$app_name' AND resource_type = 'port';" 2>/dev/null) + # Delete EVERY port-side row, not just resource_type='port'. The port-tag + # writer (portsUpdateComposeTags) also inserts port_tag_combined / + # port_tag_external / port_tag_internal / traefik_managed / + # url_accessible for the same app; deleting only 'port' orphaned the rest + # at uninstall, and the firewall rebuild — which reads port_tag_external — + # then walked ports for apps that no longer exist on every run. + # 'ip' is excluded: it is ipRemoveFromDatabase's row, and uninstall calls + # that first. Matching by "not ip" rather than an explicit type list so a + # future port-side resource_type is cleaned up without another edit here. + local result; result=$(sqlite3 "$docker_dir/$db_file" "DELETE FROM network_resources WHERE app_name = '$app_name' AND resource_type != 'ip';" 2>/dev/null) if [[ $? -eq 0 ]]; then isSuccessful "Removed port allocations for $app_name"