From 4643b77083c2b5d5d7088f4914eb710214513b59 Mon Sep 17 00:00:00 2001 From: William Gill Date: Wed, 16 Sep 2026 13:27:42 -0500 Subject: [PATCH] fix(squid): write explicitly-passed values through to .env on a re-run .env was seeded only when absent, so a re-run with a corrected value logged ".env exists; leaving it alone" and dropped it. That looked harmless because compose reads the shell environment before .env: an exported BIND_ADDR did narrow the bind for that run, `ss -ltn` confirmed it, and the deploy reported success -- while .env still said 0.0.0.0. The drift surfaces later. The documented update path is a plain `docker compose up -d`, which has no such environment, falls back to .env, and republishes an SSL-bumping intercepting proxy on every interface. Nothing warned. TRUSTED_CIDR has the same shape: a tightened allow-list silently reverts to whatever .env kept. Record which runtime keys actually arrived in the environment BEFORE the ":=" defaults run -- PROXY_PORT especially, whose default is a non-empty 3128, so afterwards an unset variable is indistinguishable from a supplied one. On a re-run, write just those keys through with the existing set_env() (a targeted per-key rewrite, not a file overwrite) and log each change. Keys not passed that run are untouched, so hand-edits to .env survive. Verified: narrowing BIND_ADDR updates .env and logs it; a re-run with nothing exported leaves .env alone; re-passing identical values is a silent no-op; and a hand-edited PROXY_PORT=8080 survives all three, which is what the capture-before-defaults ordering exists for. Found by an adversarial sweep for the openbao bug class (920edc5), then confirmed by hand. Co-Authored-By: Claude Opus 5 --- deployments/squid/README.md | 9 +++++++++ deployments/squid/deploy.sh | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/deployments/squid/README.md b/deployments/squid/README.md index 7c3f4cb..d9c45e7 100644 --- a/deployments/squid/README.md +++ b/deployments/squid/README.md @@ -44,6 +44,15 @@ The deploy is idempotent. On first run it builds the local image, generates the CA into `ssl/` (never overwritten), seeds `.env`, registers the port with the host firewall if present, and brings the stack up. +On a **re-run**, an existing `.env` is kept — except for values you pass +explicitly that run (`TRUSTED_CIDR`, `BIND_ADDR`, `PROXY_PORT`, `CACHE_SIZE_MB`, +`CACHE_ONLY_LISTED`), which are written through and logged. That matters because +Compose reads the shell environment *before* `.env`: without the write-through, a +re-run that narrowed `BIND_ADDR` would apply only to that run, and the next plain +`docker compose up -d` — which has no such environment — would fall back to the +old `.env` and republish the intercepting proxy on `0.0.0.0`. Values you do not +pass are left untouched, so hand-edits to `.env` survive. + ## Point clients at it ```bash diff --git a/deployments/squid/deploy.sh b/deployments/squid/deploy.sh index f15db96..fe2161e 100644 --- a/deployments/squid/deploy.sh +++ b/deployments/squid/deploy.sh @@ -6,7 +6,9 @@ # 1. Installs docker + compose if missing. # 2. Lays down the stack files in $STACK_DIR and builds the local image. # 3. Generates the TLS interception CA on first run (never overwritten). -# 4. Generates .env on first run; existing .env is never overwritten. +# 4. Generates .env on first run. An existing .env is kept, except that values +# passed explicitly to a re-run are written through (they are access +# controls -- a stale .env is what a later `docker compose up -d` reads). # 5. Prompts for required values not preset (TRUSTED_CIDR). # 6. Registers the proxy port with the host firewall if present. # 7. Brings the stack up and waits for health. @@ -35,6 +37,16 @@ set -euo pipefail : "${FORCE:=0}" : "${SKIP_PROMPTS:=0}" # non-interactive: require values via env, no prompts [[ "$SKIP_PROMPTS" == "1" ]] && FORCE=1 +# Runtime keys the operator supplies and that compose interpolates. Which of them +# actually arrived in this script's ENVIRONMENT has to be recorded BEFORE the +# ":=" defaults below, which make an unset variable look supplied -- PROXY_PORT +# especially, since its default is a non-empty 3128. +RUNTIME_KEYS=(TRUSTED_CIDR BIND_ADDR PROXY_PORT CACHE_SIZE_MB CACHE_ONLY_LISTED) +KEYS_FROM_ENV=() +for _k in "${RUNTIME_KEYS[@]}"; do + [[ -n "${!_k+x}" ]] && KEYS_FROM_ENV+=("$_k") +done + : "${TRUSTED_CIDR:=}" : "${BIND_ADDR:=}" : "${PROXY_PORT:=3128}" @@ -201,7 +213,23 @@ if [[ ! -f "$ENV_FILE" ]]; then set_env CA_O "$CA_O" set_env SQUID_IMAGE_TAG "$SQUID_IMAGE_TAG" else - log ".env exists; leaving it alone." + # Keep the file, but do NOT let it drift from what the operator passed this + # run. compose reads the shell environment before .env, so an exported value + # silently wins now while .env still holds the old one -- and a later plain + # `docker compose up -d` (the documented update path) has no such environment + # and falls back to .env. That is how a deliberately narrowed BIND_ADDR + # reverts to 0.0.0.0 on the next image update, republishing an intercepting + # proxy on every interface. These keys are access controls, so write them + # through and say so; anything not passed this run is left untouched. + log ".env exists; keeping it (only values passed this run are updated)." + for _k in ${KEYS_FROM_ENV[@]+"${KEYS_FROM_ENV[@]}"}; do + _cur=$(sed -n "s/^${_k}=//p" "$ENV_FILE" | tail -n1) + _new="${!_k}" + if [[ "$_cur" != "$_new" ]]; then + set_env "$_k" "$_new" + log " ${_k}: ${_cur:-} -> ${_new:-}" + fi + done fi # Validate required value landed.