diff --git a/.env.example b/.env.example index 656ffeb1..8cc70e84 100644 --- a/.env.example +++ b/.env.example @@ -97,12 +97,6 @@ UPDATE_CHECK_ENABLED=true # Timezone TZ=UTC -# Runtime user mapping for Docker (optional) -# Set these to your host user's UID/GID to avoid permission issues on bind mounts. -# Run `id -u` and `id -g` on host to get values. Defaults to 1001. -PUID=1001 -PGID=1001 - # Analytics (Optional - Umami) VITE_UMAMI_URL= VITE_UMAMI_WEBSITE_ID= diff --git a/.github/workflows/install-smoke.yml b/.github/workflows/install-smoke.yml new file mode 100644 index 00000000..f93ef6bf --- /dev/null +++ b/.github/workflows/install-smoke.yml @@ -0,0 +1,227 @@ +name: Fresh-install smoke + +# Verifies that a clean Postgres install boots cleanly under the same +# conditions a new user hits on their first `docker compose up -d`. The +# specific scenarios this guards against — see #484 for the original +# reproduction: +# +# 1. Bind-mounted host directories owned by a UID other than 1001 +# (the container's nodejs user). The entrypoint must self-chown +# and drop privileges via su-exec. +# 2. Cold-start Postgres with no prior schema (the FK-order bug fixed +# in #494, the index/created_at error fixed in #511, and any +# future migration-order issue that only surfaces on an empty DB). +# +# Triggers only on changes that touch the install path so unrelated PRs +# don't pay the build cost. + +on: + push: + branches: [main, beta] + paths: + - 'backend/Dockerfile' + - 'backend/wait-for-db.sh' + - 'backend/migrations/**' + - 'backend/package*.json' + - 'docker-compose.production.yml' + - '.github/workflows/install-smoke.yml' + pull_request: + branches: [main, beta] + paths: + - 'backend/Dockerfile' + - 'backend/wait-for-db.sh' + - 'backend/migrations/**' + - 'backend/package*.json' + - 'docker-compose.production.yml' + - '.github/workflows/install-smoke.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + fresh-install: + runs-on: ubuntu-latest + timeout-minutes: 15 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + # Build for the runner's arch only — we just need a runnable image. + # The full multi-arch build is the docker-build workflow's job. + - name: Build backend image + uses: docker/build-push-action@v5 + with: + context: ./backend + file: ./backend/Dockerfile + load: true + tags: picpeak-backend:smoke + cache-from: type=gha,scope=install-smoke + cache-to: type=gha,mode=max,scope=install-smoke + + - name: Create Docker network + run: docker network create picpeak-smoke + + # Mount as UID 1000 (the typical GitHub Actions runner user, and a + # common mismatch case on Linux hosts). The entrypoint must chown + # this to 1001 itself — that's the regression we're guarding. + - name: Prepare host bind-mount dirs owned by UID 1000 + run: | + mkdir -p smoke-mounts/storage smoke-mounts/data smoke-mounts/logs + chmod 755 smoke-mounts smoke-mounts/* + ls -ld smoke-mounts/* + + - name: Start Postgres + run: | + docker run -d --name picpeak-smoke-pg --network picpeak-smoke \ + -e POSTGRES_USER=picpeak \ + -e POSTGRES_PASSWORD=smokepass \ + -e POSTGRES_DB=picpeak_prod \ + --health-cmd="pg_isready -U picpeak -d picpeak_prod" \ + --health-interval=2s --health-timeout=2s --health-retries=30 \ + postgres:15-alpine + + - name: Wait for Postgres healthy + run: | + for i in $(seq 1 60); do + status=$(docker inspect -f '{{.State.Health.Status}}' picpeak-smoke-pg 2>/dev/null || echo starting) + if [ "$status" = "healthy" ]; then + echo "postgres healthy after ${i}s" + exit 0 + fi + sleep 1 + done + echo "postgres did not become healthy in 60s" + docker logs picpeak-smoke-pg + exit 1 + + - name: Start backend with mismatched-UID bind mounts (fresh install) + run: | + docker run -d --name picpeak-smoke-bk --network picpeak-smoke \ + -e NODE_ENV=production \ + -e JWT_SECRET=smoketestsecretvalueof32characters \ + -e DB_HOST=picpeak-smoke-pg \ + -e DB_USER=picpeak \ + -e DB_PASSWORD=smokepass \ + -e DB_NAME=picpeak_prod \ + -e ADMIN_EMAIL=admin@smoke.local \ + -e ADMIN_PASSWORD=smokeAdminPass12345 \ + -e STORAGE_PATH=/app/storage \ + -v "$PWD/smoke-mounts/storage:/app/storage" \ + -v "$PWD/smoke-mounts/data:/app/data" \ + -v "$PWD/smoke-mounts/logs:/app/logs" \ + picpeak-backend:smoke + + - name: Wait for backend healthy + run: | + for i in $(seq 1 120); do + status=$(docker inspect -f '{{.State.Status}}' picpeak-smoke-bk 2>/dev/null || echo missing) + health=$(docker inspect -f '{{.State.Health.Status}}' picpeak-smoke-bk 2>/dev/null || echo none) + if [ "$status" = "exited" ]; then + echo "FAIL: backend exited during cold-start (restart loop scenario)" + docker logs picpeak-smoke-bk + echo "--- error.log ---" + cat smoke-mounts/logs/error.log 2>/dev/null || echo "(no error.log)" + exit 1 + fi + if [ "$health" = "healthy" ]; then + echo "backend healthy after ${i}s" + exit 0 + fi + sleep 1 + done + echo "FAIL: backend did not become healthy in 120s" + docker ps -a + docker logs picpeak-smoke-bk + exit 1 + + - name: Verify chown happened (container view) + run: | + # All three dirs should now be owned by nodejs (UID 1001). + # If the entrypoint's self-chown branch didn't fire, they'd + # still be owned by the runner UID and node would have hit + # EACCES creating storage subdirs. + for d in /app/storage /app/data /app/logs; do + owner_uid=$(docker exec picpeak-smoke-bk stat -c '%u' "$d") + if [ "$owner_uid" != "1001" ]; then + echo "FAIL: $d is owned by UID $owner_uid (expected 1001)" + exit 1 + fi + echo "ok: $d owned by UID $owner_uid" + done + + - name: Verify app is actually serving + run: | + # /health is what docker's HEALTHCHECK polls, but hit it + # directly to confirm the response shape matches what the + # frontend + reverse proxy expect. + body=$(docker exec picpeak-smoke-bk wget -qO- http://localhost:3000/health) + echo "/health => $body" + echo "$body" | grep -q '"status":"ok"' || { + echo "FAIL: /health did not return status:ok" + exit 1 + } + + - name: Verify node runs as nodejs (not root) + run: | + # dumb-init runs as root (PID 1), node must be running as + # nodejs (UID 1001) — if su-exec drop didn't happen the app + # would be running as root which is the security regression + # we're guarding against. Alpine ships BusyBox ps, which + # doesn't support `-p PID` or pgrep, so list + awk instead. + user=$(docker exec picpeak-smoke-bk ps -o user,comm | awk '$2=="node" {print $1; exit}') + if [ "$user" != "nodejs" ]; then + echo "FAIL: node running as '$user' (expected nodejs)" + docker exec picpeak-smoke-bk ps -o pid,user,comm + exit 1 + fi + echo "ok: node running as $user" + + - name: Verify no restart loop + run: | + restart_count=$(docker inspect -f '{{.RestartCount}}' picpeak-smoke-bk) + if [ "$restart_count" -gt 0 ]; then + echo "FAIL: container restarted $restart_count time(s) — install loop bug returning" + docker logs picpeak-smoke-bk + exit 1 + fi + echo "ok: 0 restarts" + + # Restart with `--user 5005:5005` (no root, can't chown) against + # bind mounts owned by 1000 — entrypoint must fail loud with the + # actionable preflight error, not silently restart-loop. + - name: Verify preflight fails loud on unwritable mounts + run: | + docker rm -f picpeak-smoke-bk2 2>/dev/null || true + set +e + out=$(docker run --rm --user 5005:5005 --network picpeak-smoke \ + -e NODE_ENV=production -e JWT_SECRET=x \ + -e DB_HOST=picpeak-smoke-pg -e DB_USER=picpeak \ + -e DB_PASSWORD=smokepass -e DB_NAME=picpeak_prod \ + -e STORAGE_PATH=/app/storage \ + -v "$PWD/smoke-mounts/storage:/app/storage" \ + -v "$PWD/smoke-mounts/data:/app/data" \ + -v "$PWD/smoke-mounts/logs:/app/logs" \ + picpeak-backend:smoke 2>&1) + rc=$? + set -e + echo "$out" + if [ $rc -eq 0 ]; then + echo "FAIL: preflight should have exited non-zero" + exit 1 + fi + echo "$out" | grep -q "is not writable by UID 5005" || { + echo "FAIL: preflight error message missing or wrong" + exit 1 + } + echo "ok: preflight failed loud with actionable error" + + - name: Cleanup + if: always() + run: | + docker rm -f picpeak-smoke-bk picpeak-smoke-bk2 picpeak-smoke-pg 2>/dev/null || true + docker network rm picpeak-smoke 2>/dev/null || true diff --git a/README.md b/README.md index 9f791c65..b8c9b78e 100644 --- a/README.md +++ b/README.md @@ -89,13 +89,9 @@ docker compose up -d # Access at http://localhost:3000 ``` -Note on Docker file permissions (PUID/PGID) -- When using bind mounts (e.g., `./storage`, `./data`, `./logs`, `./events`), ensure the container user can write to these host folders. The backend runs as a non‑root user by default. -- Set `PUID` and `PGID` in your `.env` to match your host user’s UID/GID (run `id -u` and `id -g` on the host). Compose maps the container user to these values. -- Example in `.env`: - - `PUID=1000` - - `PGID=1000` -- Without this, creating events, uploads, thumbnails, or logs can fail with "Permission denied". +Note on Docker file permissions +- The backend container starts as root, chowns bind-mounted host directories (`./storage`, `./data`, `./logs`) to UID 1001 (`nodejs`), then drops privileges via `su-exec` before running the app. No host-side setup needed for fresh installs. +- If you pin `user:` in a compose override (e.g. to map a specific host UID), the self-chown is skipped and you must pre-chown the host directories to that UID — see [docs.picpeak.app/deployment/docker#permissions](https://docs.picpeak.app/deployment/docker#permissions). **ARM64 (aarch64) systems:** Pre-built images include native `linux/arm64`, no platform flags or emulation needed. If you're on an older image tag that's still amd64-only, see [docker-compose.amd64.override.yml](docker-compose.amd64.override.yml) for a transitional fallback. diff --git a/backend/Dockerfile b/backend/Dockerfile index 14ff9822..e8e66a74 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -35,12 +35,15 @@ RUN apk upgrade --no-cache RUN npm install -g npm@10 # Install dumb-init for proper signal handling, postgresql-client for database -# checks, and ffmpeg for video upload support. Alpine's ffmpeg package ships -# both `ffmpeg` and `ffprobe` built natively against musl libc — the npm +# checks, ffmpeg for video upload support, and su-exec for the root → nodejs +# privilege drop in wait-for-db.sh (see #484: container starts as root so it +# can chown bind-mounted host volumes to UID 1001, then re-execs as nodejs +# before running the app). Alpine's ffmpeg package ships both `ffmpeg` and +# `ffprobe` built natively against musl libc — the npm # `@ffmpeg-installer/ffmpeg` binary is glibc-built and (a) doesn't reliably # run on Alpine and (b) only includes ffmpeg, not ffprobe (which the video # pipeline calls via fluent-ffmpeg.ffprobe()). -RUN apk add --no-cache dumb-init postgresql-client ffmpeg +RUN apk add --no-cache dumb-init postgresql-client ffmpeg su-exec # Create non-root user RUN addgroup -g 1001 -S nodejs && adduser -S nodejs -u 1001 @@ -56,7 +59,9 @@ RUN chmod -R a+r /app && chmod +x wait-for-db.sh RUN mkdir -p storage/events/active storage/events/archived storage/thumbnails data logs && \ chown -R nodejs:nodejs storage data logs -USER nodejs +# No USER directive — the container starts as root so wait-for-db.sh can +# chown bind-mounted host directories to UID 1001 before dropping privs +# via su-exec. See #484 for the fresh-install restart loop this avoids. EXPOSE 3000 diff --git a/backend/init-production.sh b/backend/init-production.sh deleted file mode 100755 index d3fb599f..00000000 --- a/backend/init-production.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/bin/sh -# init-production.sh - Production initialization script - -set -e - -echo "🚀 Initializing PicPeak Production Environment..." - -# Wait for services to be ready -echo "⏳ Waiting for database to be fully ready..." -sleep 3 - -# Fix permissions if running as root (shouldn't happen with proper Dockerfile) -if [ "$(id -u)" = "0" ]; then - echo "🔧 Fixing file permissions..." - chown -R nodejs:nodejs /app/storage /app/data /app/logs 2>/dev/null || true -fi - -# Create required directories -echo "📁 Creating required directories..." -mkdir -p /app/storage/events/active \ - /app/storage/events/archived \ - /app/storage/thumbnails \ - /app/storage/uploads/logos \ - /app/storage/uploads/favicons \ - /app/data \ - /app/logs - -# Run migrations with safe runner -echo "🗄️ Running database migrations (safe mode)..." -NODE_ENV=production npm run migrate:safe - -# Create admin user if environment variables are set -if [ -n "$ADMIN_EMAIL" ] && [ -n "$ADMIN_PASSWORD" ]; then - echo "👤 Creating admin user..." - node scripts/create-admin.js \ - --email "$ADMIN_EMAIL" \ - --username "${ADMIN_USERNAME:-admin}" \ - --password "$ADMIN_PASSWORD" || echo "Admin user might already exist" -fi - -# Initialize email configuration if variables are set -if [ -n "$SMTP_HOST" ]; then - echo "📧 Email configuration detected via environment variables" -fi - -echo "✅ Production initialization complete!" -echo "🌐 Starting application server..." - -# Start the application -exec node server.js \ No newline at end of file diff --git a/backend/wait-for-db.sh b/backend/wait-for-db.sh index 6c92f9b0..6471326f 100755 --- a/backend/wait-for-db.sh +++ b/backend/wait-for-db.sh @@ -3,6 +3,43 @@ set -e +# Permission handling (#484): the image starts as root so this script can +# chown bind-mounted host volumes to UID 1001 (nodejs) before dropping +# privileges via su-exec. This avoids the fresh-install restart loop where +# the host directory's UID (commonly 1000) didn't match the container's +# hard-coded nodejs user. Compose deployments that pin `user:` to something +# other than root skip this branch — they own permissions themselves and hit +# the preflight check below instead. +if [ "$(id -u)" = "0" ]; then + if ! chown -R nodejs:nodejs /app/storage /app/data /app/logs 2>/dev/null; then + echo "ERROR: failed to chown /app/storage, /app/data, /app/logs to nodejs (UID 1001)." >&2 + echo " This usually means the host filesystem rejects chown (e.g. NFS without root squash" >&2 + echo " disabled, or a SELinux/AppArmor policy blocking the operation)." >&2 + echo " Workaround: pre-chown the host directories to 1001:1001 and pin 'user: \"1001:1001\"'" >&2 + echo " in your compose file so this script never tries to chown them itself." >&2 + echo " See https://docs.picpeak.app/deployment/docker#permissions" >&2 + exit 1 + fi + exec su-exec nodejs:nodejs "$0" "$@" +fi + +# Belt-and-suspenders: if we got here as non-root (compose `user:` override), +# verify the bind mounts are actually writable before proceeding. Failing +# loud here beats the previous behavior — silent mkdir-||-true at line 69 +# followed by a confusing migration error and a restart loop. +_uid="$(id -u)" +_gid="$(id -g)" +for _dir in /app/storage /app/data /app/logs; do + if [ ! -w "$_dir" ]; then + echo "ERROR: $_dir is not writable by UID $_uid." >&2 + echo " Either drop the 'user:' override from your compose file so the container starts as" >&2 + echo " root and can self-fix permissions, or run on the host:" >&2 + echo " chown -R $_uid:$_gid " >&2 + echo " See https://docs.picpeak.app/deployment/docker#permissions" >&2 + exit 1 + fi +done + host="${DB_HOST:-postgres}" port="${DB_PORT:-5432}" user="${DB_USER:-picpeak}" diff --git a/docker-compose.yml b/docker-compose.yml index 41324b3c..c4984d1e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -31,11 +31,11 @@ services: - ADMIN_URL=${ADMIN_URL:-http://localhost:3001} - TZ=${TZ:-UTC} - STORAGE_PATH=/app/storage - # Optional: run container as matching host user to avoid bind mount permission issues - - PUID=${PUID:-1001} - - PGID=${PGID:-1001} - # Use host-matching user ID/GID so bind-mounted folders are writable - user: "${PUID:-1001}:${PGID:-1001}" + # No `user:` directive — as of #484, the container starts as root, + # chowns the bind mounts to nodejs (UID 1001), then drops privileges + # via su-exec. PUID/PGID env vars are no longer read; if you need + # a different runtime UID, pre-chown the host dirs and pin + # `user: ":"` here. volumes: - ./events:/app/events - ./data:/app/data