From 76d2a098cb4156e96916d2ee647f646af0edcdce Mon Sep 17 00:00:00 2001 From: William Gill Date: Wed, 16 Sep 2026 13:43:59 -0500 Subject: [PATCH] fix(copyparty): make the documented FTP_NAT re-run remedy actually work The summary printed at the end of every run says passive FTPS behind NAT is fixed by re-running with FTP_NAT=... That could never work: the ftp-nat substitution sat inside `if [[ ! -f "$CONF" ]]`, so on any re-run the conf existed, the substitution was skipped, and ftp-nat stayed commented out. cfg/copyparty.conf is the ONLY route FTP_NAT has to the service -- nothing in docker-compose.yml interpolates it and the copyparty service is given no environment, so .env's copy is a record, not the live setting. The remedial run logged "exists; leaving it alone" as a green [+], reported healthy, and reprinted the same advice. Passive FTPS failed exactly as before, with nothing to distinguish "you did it wrong" from "it didn't take" -- and because the template's commented example carries a literal IP, grepping the conf shows an ftp-nat line with an address in it. Lift the application out of the creation guard so it runs against an existing conf too, and match `#?` so an already-set value (ISP change, typo) is corrected rather than only the commented template line. Warn instead of guessing if no ftp-nat line exists at all -- appending at EOF would land in [accounts]. Kept above the chown, since `sed -i` rewrites as root. `|| true` on the current-value read is load-bearing: on a still-commented conf the grep matches nothing, and under `set -o pipefail` that would abort the script inside the assignment -- the exact trap fixed in bf52426. Also restart copyparty when the conf changed: compose will not recreate a service whose image and config are unchanged, so `up -d` alone would leave the edit on disk and the old value in the running process. And record the value in .env so the two files do not disagree about what is deployed. Verified: commented template applies; identical value is a no-op; a different value is corrected; a conf with no ftp-nat line warns without corruption; empty FTP_NAT skips the block entirely. Co-Authored-By: Claude Opus 5 --- deployments/copyparty/deploy.sh | 47 ++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/deployments/copyparty/deploy.sh b/deployments/copyparty/deploy.sh index f6372c1..4dc9a25 100644 --- a/deployments/copyparty/deploy.sh +++ b/deployments/copyparty/deploy.sh @@ -237,11 +237,32 @@ if [[ ! -f "$CONF" ]]; then log "Generating $CONF (admin account)..." install -m 0640 "$SCRIPT_DIR/copyparty.conf.example" "$CONF" sed -i "s|__ADMIN_PW__|${ADMIN_PW}|" "$CONF" - if [[ -n "$FTP_NAT" ]]; then - sed -i -E "s|^[[:space:]]*#[[:space:]]*ftp-nat:.*| ftp-nat: ${FTP_NAT}|" "$CONF" - fi else - log "$CONF exists; leaving it alone." + log "$CONF exists; keeping it." +fi + +# FTP_NAT has exactly one route to the running service: this conf file. Nothing +# in docker-compose.yml interpolates it and the copyparty service is given no +# environment, so .env's copy is a record, not the live setting. The summary at +# the end tells operators to fix passive FTPS by re-running with FTP_NAT=... -- +# so that has to apply to an EXISTING conf too, not only a freshly created one. +# Must stay above the chown further down, since `sed -i` rewrites as root. +CONF_CHANGED=0 +if [[ -n "$FTP_NAT" ]]; then + # `|| true` is load-bearing: on a still-commented conf the grep matches + # nothing, and under `set -o pipefail` that would abort this assignment. + cur_nat="$(grep -E '^[[:space:]]*ftp-nat:' "$CONF" | head -n1 | sed -E 's/^[[:space:]]*ftp-nat:[[:space:]]*//; s/[[:space:]]*#.*$//; s/[[:space:]]*$//' || true)" + if [[ "$cur_nat" == "$FTP_NAT" ]]; then + : # already applied + elif grep -qE '^[[:space:]]*#?[[:space:]]*ftp-nat:' "$CONF"; then + log "Setting 'ftp-nat: ${FTP_NAT}' in $CONF (was: ${cur_nat:-unset})." + # `#?` so an already-set value is corrected, not just the commented + # template line. + sed -i -E "s|^[[:space:]]*#?[[:space:]]*ftp-nat:.*| ftp-nat: ${FTP_NAT}|" "$CONF" + CONF_CHANGED=1 + else + warn "No ftp-nat line in $CONF to set. Add ' ftp-nat: ${FTP_NAT}' under [global] by hand." + fi fi # Read back the admin password for the summary (works on first run + re-runs). # Strip the leading 'admin:', any trailing inline #comment, and surrounding space. @@ -296,6 +317,16 @@ else log ".env exists; leaving it alone." fi +# .env's FTP_NAT is a record rather than the live setting, but let it disagree +# with the conf and the next reader cannot tell which one is deployed. +if [[ -n "$FTP_NAT" ]]; then + cur_env_nat=$(sed -n 's/^FTP_NAT=//p' "$ENV_FILE" | tail -n1) + if [[ "$cur_env_nat" != "$FTP_NAT" ]]; then + set_env FTP_NAT "$FTP_NAT" + log "Recorded FTP_NAT=${FTP_NAT} in $ENV_FILE." + fi +fi + # Validate required values landed. missing=() for var in COPYPARTY_DOMAIN ACME_EMAIL; do @@ -328,6 +359,14 @@ docker compose pull log "Starting stack..." docker compose up -d --remove-orphans +# compose does not recreate a service whose image and compose config are +# unchanged, so a conf edit on a re-run would sit on disk while the running +# process keeps the old ftp-nat. Restart it explicitly. +if [[ "$CONF_CHANGED" == "1" ]]; then + log "copyparty.conf changed; restarting copyparty to apply it..." + docker compose restart copyparty +fi + # ---------------------------------------------------------------------------- # Wait for health # ----------------------------------------------------------------------------