From 6b8c463ffe6d1c31a56ce5d054fabbfe0b914e65 Mon Sep 17 00:00:00 2001 From: Yostin Vargas Date: Sun, 23 Aug 2026 23:47:43 -0400 Subject: [PATCH] fix: a second checkout cannot take over the containers silently Every clone uses the same compose project name, so starting from a second directory adopts and reconfigures the first one's containers. That is how a test install stopped serving its own domain here, with nothing said. The two stay one environment reachable from two folders: the shared name is also what keeps database volumes working, and deriving it from the directory would orphan every existing one. What changes is that taking over is announced. forge asks, --force skips the question, and with no terminal it refuses and names the flag. forge status says which folder owns the running containers. Detection needs nothing stored: compose already labels containers with project.working_dir. active_dir() reads it through compose_with_env so the installer can call it before .env exists. The check sits in compose_up, the single route every start goes through since the profile work, plus cmd_stop. CI reproduces it with a real second directory and takes ownership back through a trap. Verified it fails when check_owner is removed. --- .github/workflows/ci.yml | 61 ++++++++++++++++++++++++++++++++++++++++ README.es.md | 32 +++++++++++++++++++++ README.md | 32 +++++++++++++++++++++ TASKS.md | 45 ++++++++++++++++++++++++++--- bin/forge | 42 ++++++++++++++++++++++++++- install.sh | 49 ++++++++++++++++++++++++-------- lib/compose.sh | 15 ++++++++++ 7 files changed, 259 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cae8d60..f44e250 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -590,6 +590,67 @@ jobs: echo "unexpected issuer after restore: $(cert_field issuer)"; exit 1; } echo "and the real one comes back" + # Every checkout claims the same compose project, so a second one silently + # adopted and reconfigured the first one's containers. Reproduced rather than + # described: a real second directory, started from, then taken back. + - name: a second checkout cannot take over silently + run: | + set -euo pipefail + export PATH="$HOME/.local/bin:$PATH" + HERE="$(pwd)" + # However this ends, the stack goes back to belonging here: the step + # after it would otherwise be testing containers from /tmp. + trap 'cd "$HERE" && forge --force start >/dev/null 2>&1 || true' EXIT + + rm -rf /tmp/second-checkout + git ls-files | tar -cf - -T - | (mkdir -p /tmp/second-checkout && tar -xf - -C /tmp/second-checkout) + # A real second clone has only the tracked compose files: any + # docker-compose.local.yml is gitignored and did not come with it. + sed -e 's|^PROJECTS_DIR=.*|PROJECTS_DIR=/tmp/second-projects|' \ + -e 's|^COMPOSE_FILE=.*|COMPOSE_FILE=docker-compose.yml:docker-compose.override.yml|' \ + .env > /tmp/second-checkout/.env + mkdir -p /tmp/second-projects/sites + (cd /tmp/second-checkout && docker compose up -d) + + owner() { + docker inspect apachedev \ + --format '{{index .Config.Labels "com.docker.compose.project.working_dir"}}' + } + test "$(owner)" = /tmp/second-checkout || { + echo "the second checkout did not take the containers: $(owner)"; exit 1; } + + # no terminal, no --force: it must refuse and say how + if forge start < /dev/null > /tmp/refused.log 2>&1; then + echo "forge took over without being asked"; exit 1 + fi + grep -q /tmp/second-checkout /tmp/refused.log + grep -q -- --force /tmp/refused.log + test "$(owner)" = /tmp/second-checkout || { echo "it took over anyway"; exit 1; } + + # status has to be able to answer "who owns these". Captured, not piped: + # `grep -q` closes the pipe early, forge dies of SIGPIPE, and pipefail + # reports 141 -- the same trap as `forge profile on tools | grep`. + forge status > /tmp/status.log 2>&1 + grep -q /tmp/second-checkout /tmp/status.log + + # stop is not destructive, but it is just as surprising + if forge stop < /dev/null >/dev/null 2>&1; then + echo "forge stop took over without being asked"; exit 1 + fi + + # and taking over is still one command + forge --force start < /dev/null > /dev/null 2>&1 + test "$(owner)" = "$HERE" || { echo "--force did not take over: $(owner)"; exit 1; } + + # with no conflict it must say nothing at all + forge start < /dev/null > /tmp/quiet.log 2>&1 + if grep -q 'started from' /tmp/quiet.log; then + echo "warned when there was nothing to warn about"; exit 1 + fi + + rm -rf /tmp/second-checkout /tmp/second-projects + echo "a second checkout announces itself" + - name: container logs on failure if: failure() run: docker compose logs --tail=50 diff --git a/README.es.md b/README.es.md index 97656ce..a7d2d1a 100644 --- a/README.es.md +++ b/README.es.md @@ -479,6 +479,38 @@ dentro de un contenedor; tu editor debe escuchar en el puerto **9003**. algunos shells no tienen en el `PATH`. Añádelo, o haz `source aliases.bash` desde la carpeta del proyecto. +### Dos copias del proyecto + +Cada copia usa el mismo nombre de proyecto de compose, así que un segundo clon —para +probar una actualización sin tocar lo que funciona— no obtiene su propio entorno. +Obtiene el mismo: + +| Compartido | No compartido | +|---|---| +| los contenedores | tu código, en la carpeta que diga cada `.env` | +| los volúmenes de base de datos | el propio `.env`: dominio, versión de PHP, carpeta de proyectos | +| los puertos 80 y 443 | los certificados de `certificates/` | + +Arrancar desde la segunda carpeta reconfigura los contenedores que ya corren, no crea +otros. `forge` pregunta antes, y `forge status` dice desde qué carpeta se arrancaron: + +``` +[!] These containers were started from: + ~/Projects/otra-copia + To go back: cd ~/Projects/otra-copia && forge start + + Take them over? [y/N] +``` + +`forge --force ` se salta la pregunta, que es también lo que necesitas donde +no hay terminal para responderla. + +**Tu código nunca corre peligro**: es una carpeta de tu disco, montada dentro. Lo que +sí comparten las dos carpetas son los datos de las bases de datos, que viven en un +volumen de Docker con el nombre del proyecto y no el de la carpeta. `forge` no tiene +ningún comando que los borre — pero un `docker compose down -v` escrito a mano desde +cualquiera de las dos alcanza a las dos. + Para más ayuda, mira los logs con `forge logs` — o `forge logs apachedev` para un servicio — o abre un issue en GitHub. diff --git a/README.md b/README.md index 21c8876..229f8a6 100644 --- a/README.md +++ b/README.md @@ -487,6 +487,38 @@ container; your IDE should listen on port **9003**. some shells do not have on `PATH`. Add it, or `source aliases.bash` from the project directory. +### Two copies of the project + +Every checkout uses the same compose project name, so a second clone — testing an +upgrade without touching the setup that works — does not get its own environment. It +gets the same one: + +| Shared | Not shared | +|---|---| +| the containers | your code, in whichever folder each `.env` points at | +| the database volumes | the `.env` itself: domain, PHP version, projects folder | +| ports 80 and 443 | the certificates in `certificates/` | + +Starting from the second folder reconfigures the running containers rather than +creating new ones. `forge` asks first, and `forge status` names the folder that +started them: + +``` +[!] These containers were started from: + ~/Projects/other-copy + To go back: cd ~/Projects/other-copy && forge start + + Take them over? [y/N] +``` + +`forge --force ` skips the question, which is also what you need where +there is no terminal to answer on. + +**Your code is never at risk**: it is a folder on your disk, mounted in. What the two +folders genuinely share is the database data, which lives in a Docker volume named +after the project rather than after the folder. `forge` has no command that deletes +it — but `docker compose down -v`, typed by hand in either folder, reaches both. + For more help, check the logs with `forge logs` — or `forge logs apachedev` for one service — or open an issue on GitHub. diff --git a/TASKS.md b/TASKS.md index 41831ea..1e474da 100644 --- a/TASKS.md +++ b/TASKS.md @@ -1354,7 +1354,7 @@ reasoning about them. is what "self-signed" actually means and does not depend on how a build formats a name. -- [ ] **40. Two checkouts of the project fight over the same containers** +- [x] **40. Two checkouts of the project fight over the same containers** — FIXED `COMPOSE_PROJECT_NAME=php-devforge` is a fixed literal in `.env.example:4`, so every clone claims the same compose project. Seen for real with a second checkout used to test a fresh install: @@ -1378,6 +1378,43 @@ reasoning about them. The ports make them genuinely exclusive anyway, so the goal is not running both at once — it is that the second one refuses clearly instead of silently taking - the first one's containers. Options to weigh: derive the name from the checkout - directory, keep the literal but have the installer detect an existing project - pointing elsewhere and say so, or both. + the first one's containers. + + **Decided with the user, after separating what is actually shared.** Code is a + bind mount of a real folder and is never at risk. Database data is a named + volume labelled `com.docker.compose.project: php-devforge`, so it belongs to the + project name rather than to a directory — which is the whole problem in one + line. Deriving the name from the checkout would isolate them, but it orphans + every existing volume: the user's databases would look deleted. + + So the two stay **one environment reachable from two folders**, and the fix is + that taking over is announced and confirmed rather than silent. `forge` asks, + `--force` skips the question, and with no terminal it refuses and names the + flag. `forge status` now says which folder owns the running containers. + + Detection needs nothing new stored: compose already labels every container with + `com.docker.compose.project.working_dir`. `active_dir()` in `lib/compose.sh` + reads it through `compose_with_env`, so the installer can call it before `.env` + exists — otherwise the project name would fall back to the directory name and + find nothing. + + The check sits in `compose_up()`, which task 38 made the single route for every + start, plus `cmd_stop` — stopping someone else's containers destroys nothing but + is just as surprising. + + **A claim in the plan was wrong and got corrected mid-task.** It said + `install.sh` silently repointed the `forge` symlink with `ln -sfn`. It does not: + that line only runs when the link is absent or already points here, and there is + an explicit "points somewhere else. Left alone." branch. The symlink on this + machine had been repointed by hand while setting up local verification, not by + the installer. + + CI reproduces it rather than describing it: a real second directory, started + from, then taken back — with a `trap` returning ownership however the step ends. + Verified that removing `check_owner` makes it fail with `forge took over without + being asked`. + + Two things the local run caught that CI would not have: a second clone has no + gitignored `docker-compose.local.yml`, so the copied `.env` has to drop it from + `COMPOSE_FILE`; and `forge status | grep -q` dies of SIGPIPE under `pipefail`, + the same trap as `forge profile on tools | grep`. diff --git a/bin/forge b/bin/forge index fabb904..fae3047 100755 --- a/bin/forge +++ b/bin/forge @@ -40,12 +40,43 @@ need_env() { [ -f .env ] || { err "No .env here. Run ./install.sh first."; exit 1; } } +# Every checkout claims the same compose project, so containers started from +# another directory look like ours -- `up -d` here silently adopts and +# reconfigures them. That is not destructive, but it is never what you meant. +check_owner() { + local owner; owner="$(active_dir)" + [ -n "$owner" ] || return 0 + [ "$owner" != "$FORGE_DIR" ] || return 0 + + warn "These containers were started from:" + echo " $owner" >&2 + echo " Continuing will reconfigure them from here." >&2 + echo " To go back: cd $owner && forge start" >&2 + echo "" >&2 + + if [ "$FORCE" = "1" ]; then + warn "Taking over (--force)" + return 0 + fi + if [ ! -t 0 ]; then + err "No terminal to ask on. Use 'forge --force ' to take over." + exit 1 + fi + local answer + read -r -p " Take them over? [y/N]: " answer take over containers another checkout started + forge install [options] set everything up (first run, or to reconfigure) forge start [service...] start the environment @@ -327,6 +364,9 @@ Version arguments accept either form: 8.5 or 85. EOF } +FORCE=0 +if [ "${1:-}" = "--force" ]; then FORCE=1; shift; fi + case "${1:-help}" in install) shift; cmd_install "$@" ;; start) shift; cmd_start "$@" ;; diff --git a/install.sh b/install.sh index 710d9af..00f95bd 100755 --- a/install.sh +++ b/install.sh @@ -20,7 +20,7 @@ title() { echo -e "\n${BLUE}$1${NC}"; } # shellcheck source=lib/menu.sh . ./lib/menu.sh -ASSUME_YES=0; SKIP_CERT=0; SKIP_DNS=0; SKIP_LINK=0 +ASSUME_YES=0; SKIP_CERT=0; SKIP_DNS=0; SKIP_LINK=0; FORCE=0 OPT_DOMAIN=""; OPT_DIR=""; OPT_PHP=""; OPT_DNS_PORT=""; OPT_IMAGES=""; OPT_PROFILES="" usage() { @@ -35,6 +35,7 @@ Usage: $0 [OPTIONS] --dns-port=N port for the local DNS (default: first free one) --images=pull|build use the published images, or build your own (default: pull) --profiles=a,b databases and extras to enable, e.g. pg18,mariadb12,mail + --force take over containers another checkout started --skip-link do not put `forge` on your PATH --skip-cert do not generate certificates --skip-dns do not touch the system DNS @@ -53,6 +54,7 @@ for arg in "$@"; do --dns-port=*) OPT_DNS_PORT="${arg#*=}" ;; --images=*) OPT_IMAGES="${arg#*=}" ;; --profiles=*) OPT_PROFILES="${arg#*=}" ;; + --force) FORCE=1 ;; --skip-link) SKIP_LINK=1 ;; --skip-cert) SKIP_CERT=1 ;; --skip-dns) SKIP_DNS=1 ;; @@ -94,17 +96,6 @@ for p in 80 443; do fi done -# ---------- current values as defaults ---------- -[ -f .env ] && { set -a +u; . ./.env; set +a -u; } - -DEF_DOMAIN="${DEV_DOMAIN:-phpforge.dev}" -DEF_DIR="${PROJECTS_DIR:-$HOME/php-devforge}" -DEF_PHP="${PHP_VERSION:-84}" -DEF_DNS_PORT="${DNS_PORT:-}" -case "${IMAGE_MODE:-missing}" in build) DEF_IMAGES="build" ;; *) DEF_IMAGES="pull" ;; esac - -valid_profile() { printf '%s\n' "$ALL_PROFILES" | grep -qx "$1"; } - ask() { # prompt, default -> answer on stdout local prompt="$1" default="$2" answer if [ "$ASSUME_YES" -eq 1 ]; then echo "$default"; return; fi @@ -119,6 +110,40 @@ confirm() { # prompt -> 0 yes / 1 no case "${answer:-y}" in [nN]*) return 1 ;; *) return 0 ;; esac } +# Every checkout claims the same compose project, so an install here reconfigures +# containers another directory started. Checked with the requirements, before +# anything is written. +OWNER="$(active_dir)" +if [ -n "$OWNER" ] && [ "$OWNER" != "$(pwd)" ]; then + echo "" + warn "Containers for this project are already running, started from:" + echo " $OWNER" + echo " Installing here will reconfigure them: same containers, same" + echo " databases, your code in both folders untouched." + echo " To leave them alone: cd $OWNER" + echo "" + if [ "$FORCE" -eq 1 ]; then + warn "Continuing (--force)" + elif [ ! -t 0 ]; then + err "No terminal to ask on. Re-run with --force to take over." + exit 1 + elif ! confirm "Take them over?"; then + err "Left alone." + exit 1 + fi +fi + +# ---------- current values as defaults ---------- +[ -f .env ] && { set -a +u; . ./.env; set +a -u; } + +DEF_DOMAIN="${DEV_DOMAIN:-phpforge.dev}" +DEF_DIR="${PROJECTS_DIR:-$HOME/php-devforge}" +DEF_PHP="${PHP_VERSION:-84}" +DEF_DNS_PORT="${DNS_PORT:-}" +case "${IMAGE_MODE:-missing}" in build) DEF_IMAGES="build" ;; *) DEF_IMAGES="pull" ;; esac + +valid_profile() { printf '%s\n' "$ALL_PROFILES" | grep -qx "$1"; } + title "Settings" DOMAIN="${OPT_DOMAIN:-$(ask "Development domain?" "$DEF_DOMAIN")}" PROJ="${OPT_DIR:-$(ask "Where will your projects live?" "$DEF_DIR")}" diff --git a/lib/compose.sh b/lib/compose.sh index e253f10..0e367dd 100644 --- a/lib/compose.sh +++ b/lib/compose.sh @@ -62,3 +62,18 @@ profile_images() { _profile_extra images "$1" | sed 's|.*/||' | paste -sd', ' -; # pg18 -> postgres18dev ; search -> es8143dev, kibana (one per line) profile_services() { _profile_extra services "$1"; } + +# Which directory the running containers came from, or nothing when none are +# running. COMPOSE_PROJECT_NAME is a fixed literal, so every checkout claims the +# same project and containers started elsewhere look like ours; compose records +# the origin on each one. +# +# Goes through compose_with_env so the installer can call it too: without .env +# the project name would fall back to the directory name and find nothing. +active_dir() { + local id + id="$(compose_with_env ps -q 2>/dev/null | head -1)" || return 0 + [ -n "$id" ] || return 0 + docker inspect "$id" \ + --format '{{index .Config.Labels "com.docker.compose.project.working_dir"}}' 2>/dev/null +}