diff --git a/scripts/config/core/config_find_file.sh b/scripts/config/core/config_find_file.sh index 44e1d8c..2b18b2c 100755 --- a/scripts/config/core/config_find_file.sh +++ b/scripts/config/core/config_find_file.sh @@ -17,13 +17,24 @@ findConfigFileForOption() # Fall back to per-app configs (CFG__*). Without this, tools that # mutate per-app CFGs (password reset, shortcut manager) silently no-op. + # + # Enumerated via runFileOp, NOT a glob. In a rootless install the containers + # tree is owned by dockerinstall and mode 751: the manager that runs the CLI + # can TRAVERSE it but not LIST it. A shell glob needs the list, so + # "$containers_dir"*/*.config expanded to nothing and every per-app lookup + # reported "not found in any config file" — while the very same file was + # readable by direct path. That silently no-op'd `libreportal config update` + # for every per-app option. runFileOp runs the enumeration as the user that + # owns the tree, exactly as the updater's scan already lists app dirs. if [[ -n "$containers_dir" && -d "$containers_dir" ]]; then - for config_file in "$containers_dir"*/*.config; do - if [ -f "$config_file" ] && grep -q "^$config_option=" "$config_file"; then - echo "$config_file" + local app_config + while IFS= read -r app_config; do + [ -n "$app_config" ] || continue + if [ -f "$app_config" ] && grep -q "^$config_option=" "$app_config"; then + echo "$app_config" return 0 fi - done + done < <(runFileOp find "${containers_dir%/}" -mindepth 2 -maxdepth 2 -type f -name '*.config' 2>/dev/null) fi return 1 } diff --git a/scripts/config/core/config_update_option.sh b/scripts/config/core/config_update_option.sh index bb06613..77876f8 100755 --- a/scripts/config/core/config_update_option.sh +++ b/scripts/config/core/config_update_option.sh @@ -12,8 +12,14 @@ updateConfigOption() # If no config file provided, auto-discover it if [[ -z "$config_file" ]]; then config_file=$(findConfigFileForOption "$config_option") + # Return non-zero. This used to fall through with an empty filename, hit + # the `else` below and end — returning 0, so configUpdateBatch counted + # the change as applied and printed "Applied 1 config change(s); 0 + # skipped/failed" having written nothing at all. A setting that reports + # success and does not take effect is worse than one that errors. if [[ $? -ne 0 || -z "$config_file" ]]; then isNotice "Config option '$config_option' not found in any config file" + return 1 fi fi @@ -42,8 +48,11 @@ updateConfigOption() fi # Check if config option exists in the file. grep + read can use the - # current user — both dirs are world-readable; only the write needs - # escalation. + # current user: the FILES are world-readable and the directories are + # traversable, so a direct path works. (The containers dir is not world- + # LISTABLE — see findConfigFileForOption, which is why discovering the path + # needs runFileOp even though reading it afterwards does not.) Only the + # write needs escalation. if grep -q "^$config_option=" "$config_file"; then # Extract the comment part first (everything after the first #) local original_line=$(grep "^$config_option=" "$config_file") @@ -68,7 +77,9 @@ updateConfigOption() checkSuccess "Updated $config_option to $config_value" ;; esac source "$config_file" + return 0 else isNotice "Unable to find $config_option with value in $config_file" + return 1 fi } diff --git a/scripts/instance/instance_create.sh b/scripts/instance/instance_create.sh index ed5546a..e647dfa 100644 --- a/scripts/instance/instance_create.sh +++ b/scripts/instance/instance_create.sh @@ -144,9 +144,87 @@ _instanceRewriteCompose() { local type="$1" slug="$2" dir="$3" local f="$dir/docker-compose.yml" [[ -f "$f" ]] || return 0 - # 1. Traefik router/service names (-service) and the db container/host - # (_db) — these tokens are unambiguous, rewrite everywhere. - sed -i -E "s/\b${type}-service\b/${slug}-service/g; s/\b${type}_db\b/${slug}_db/g" "$f" + + # 1. Every compound identity the compose declares — its service keys (from the + # SERVICE_TAG_ markers) and its container_name values. These are the + # names Docker and Traefik require to be unique, so a clone that keeps any + # of them collides head-on with the base app: docker refuses the duplicate + # container, and two routers with one name fight over the same host. + # + # Discovered rather than hardcoded. This used to assume every app had + # exactly -service and _db, which is true of Bookstack and + # false of most others — cloning Nextcloud would have left its -db, -redis + # and -web services pointing at the ORIGINAL app's containers. + # + # Commented-out lines are excluded. Several templates ship an optional + # sidecar parked behind `#` (adguard-exporter, pihole-exporter, …); + # harvesting those renamed a service that isn't running AND mangled the + # image name inside the same block, leaving a trap for whoever uncomments + # it later. A real service line carries its SERVICE_TAG mid-line, so + # dropping lines that *start* with # keeps every live one. + local -a tokens=() + mapfile -t tokens < <( + grep -v '^[[:space:]]*#' "$f" 2>/dev/null | { + grep -oE '#LIBREPORTAL\|SERVICE_TAG_[0-9]+\|[^[:space:]]+|container_name:[[:space:]]*[^[:space:]]+' \ + | sed -E 's/.*SERVICE_TAG_[0-9]+\|//; s/container_name:[[:space:]]*//' + } | sed '/^$/d' | sort -u + ) + + # Split into compound ( + separator + suffix) and bare ( exactly). + # Only compound names are safe to rewrite everywhere: the bare app name also + # appears inside image paths and values like MYSQL_USER=, so it stays + # confined to the two anchored positions in rules 2/3 below — same split the + # original hardcoded rules relied on, now applied to whatever the app declares. + # + # Longest first: the substitution below lets `-` end a token, so a short + # name would otherwise match inside a longer one (`redis` inside + # `redis-cache`). Placeholdering the long name first makes that impossible. + local -a sorted=() + mapfile -t sorted < <(printf '%s\n' "${tokens[@]}" | awk '{print length"\t"$0}' | sort -rn | cut -f2-) + + # Separators are compared as equivalent: an app dir named libreportal_catalog + # ships services named libreportal-catalog-*, and treating that as "unprefixed" + # would refuse an app that is perfectly instance-safe. + local type_norm="${type//-/_}" + local -a olds=() news=() + local t t_norm + for t in "${sorted[@]}"; do + t_norm="${t//-/_}" + if [[ "$t_norm" == "$type_norm" ]]; then + continue # bare name — handled by rules 2/3 + elif [[ "$t_norm" == "${type_norm}_"* ]]; then + # Offset by ${#type}: swapping separators never changes length, so the + # suffix (and its original separator char) survives intact. + olds+=("$t"); news+=("${slug}${t:${#type}}") + else + # No prefix at all (stoat's `database`/`minio`/`caddy`, + # prometheus's `node-exporter`). There is no mechanical way to make + # these unique — and rewriting a bare word like `minio` would also + # corrupt `image: minio/minio`. Refuse rather than hand back an + # instance that silently fights the base app for container names. + isError "Instance create: '$type' declares a service or container named '$t', which carries no '$type' prefix." + isNotice "Such names cannot be made unique per instance, so '$type' is not instance-safe. Prefix its services with '$type-' in the compose, or leave CFG_${type^^}_MULTI_INSTANCE unset." + return 1 + fi + done + + # Two-phase substitution via a placeholder, so a rewritten name can never be + # re-matched by a later pass. + # + # \b is deliberate: per-port Traefik routers are named - + # (traefik.http.routers.adguard-service-webui), so the service token has to + # match when a `-` follows it. That is also what makes the longest-first + # ordering above load-bearing rather than cosmetic. image: lines are skipped + # outright — an app's name legitimately appears in its own image path. + local i ph + for i in "${!olds[@]}"; do + ph=$'\001'"LPSVC${i}"$'\001' + sed -i -E "/^[[:space:]]*#?[[:space:]]*image:/! s/\b${olds[$i]}\b/${ph}/g" "$f" + done + for i in "${!olds[@]}"; do + ph=$'\001'"LPSVC${i}"$'\001' + sed -i "s|${ph}|${news[$i]}|g" "$f" + done # 2. The standalone app container (container_name: ) — anchored so the # image: line ending in is never touched. sed -i -E "s/(container_name:[[:space:]]*)${type}\b/\1${slug}/g" "$f" @@ -291,7 +369,14 @@ instanceCreate() { # 4. Make the cloned compose + config ports + tools target the instance's own # identity. The config ports must follow the compose or the WebUI can't # join the instance's ports to its services. - _instanceRewriteCompose "$type" "$slug" "$inst_dir" + # A refusal here means the app isn't instance-safe. Drop the clone rather + # than leave a half-rewritten template on disk that a later scan would pick + # up as a real app and try to install. + if ! _instanceRewriteCompose "$type" "$slug" "$inst_dir"; then + rm -rf "$inst_dir" + isError "Instance create aborted; no changes were made." + return 1 + fi _instanceRewriteConfigPorts "$type" "$slug" "$cfg" _instanceRewriteTools "$type" "$slug" "$inst_dir"