fix(docker): address round-3 review of the all-in-one image (#1042)
Two of these were regressions the previous round introduced. - The backup root no longer gates startup. Adding it to the mandatory mkdir + writability preflight broke a supported configuration: docker-compose.production.yml does not mount /backup, so a hardened non-root deployment would fail `mkdir -p /backup` against a root-owned / and refuse to boot over a directory it never needed. It is now adopted only when BACKUP_DIR is explicitly set (as this image sets it), and the subdirectory creation is guarded on the root existing. - Runtime data can no longer leak into the image. Dockerfile.aio builds from the repository root and Docker reads only the root .dockerignore — backend/ .dockerignore is never consulted — so the unprefixed data/*.db, logs/* and storage/* rules missed backend/data, backend/logs and backend/storage entirely. A checkout that had been used to run PicPeak would bake its database, photos, logs and SETUP_TOKEN into a published layer. This is not hypothetical: an earlier build in this PR silently picked up a stray backend/logs the same way. - HEALTHCHECK follows $PORT. The docs advertise PORT as configurable, and a hard-coded 3000 marked an otherwise healthy container unhealthy forever the moment anyone changed it. - --max-http-header-size=32768 matches nginx's large_client_header_buffers 4 32k. Requests reach Node directly here and its 16 KiB default would reject a guest carrying several per-gallery JWT cookies before Express saw them. Verified: clean image carries no /app/data/*.db, /app/logs or /app/storage; default container healthy; PORT=8080 container healthy with the probe following it; pid 1 shows the header flag in place.
This commit is contained in:
@@ -18,3 +18,18 @@ storage/events/archived/*
|
||||
storage/thumbnails/*
|
||||
data/*.db
|
||||
logs/*
|
||||
|
||||
# The all-in-one image (#1042) builds from the REPOSITORY ROOT, not ./backend,
|
||||
# and Docker only reads this file — backend/.dockerignore is never consulted.
|
||||
# The unprefixed rules above therefore miss backend/data, backend/storage and
|
||||
# backend/logs, so a checkout that has been used to run PicPeak would bake its
|
||||
# database, photos and logs into a published image layer.
|
||||
backend/node_modules
|
||||
backend/data/*.db
|
||||
backend/data/*.db-wal
|
||||
backend/data/*.db-shm
|
||||
backend/data/SETUP_TOKEN
|
||||
backend/logs
|
||||
backend/storage
|
||||
frontend/node_modules
|
||||
frontend/dist
|
||||
|
||||
+9
-2
@@ -170,10 +170,17 @@ EXPOSE 3000
|
||||
# than just a live socket. No start-period padding for a Postgres wait is
|
||||
# needed on the default SQLite path, but external-Postgres users get the same
|
||||
# 60s grace as the compose image.
|
||||
# Shell form so it resolves $PORT: the docs advertise PORT as configurable, and
|
||||
# a hard-coded 3000 would mark an otherwise healthy container unhealthy forever
|
||||
# the moment someone changed it.
|
||||
HEALTHCHECK --interval=30s --timeout=5s --start-period=60s --retries=3 \
|
||||
CMD wget --no-verbose --tries=1 --spider http://localhost:3000/health || exit 1
|
||||
CMD wget --no-verbose --tries=1 --spider "http://localhost:${PORT:-3000}/health" || exit 1
|
||||
|
||||
# No USER directive: the container starts as root so wait-for-db.sh can chown a
|
||||
# bind-mounted /data to UID 1001 before dropping privileges via su-exec (#484).
|
||||
ENTRYPOINT ["dumb-init", "--"]
|
||||
CMD ["/usr/local/bin/docker-entrypoint.aio.sh", "node", "server.js"]
|
||||
# --max-http-header-size matches nginx's `large_client_header_buffers 4 32k`.
|
||||
# Node defaults to 16 KiB, and requests reach it directly here — a guest who has
|
||||
# accumulated several per-gallery JWT cookies would be rejected before Express
|
||||
# ever saw the request, with no way out but clearing cookies.
|
||||
CMD ["/usr/local/bin/docker-entrypoint.aio.sh", "node", "--max-http-header-size=32768", "server.js"]
|
||||
|
||||
+15
-2
@@ -29,7 +29,18 @@ unset _pair _var _file _cur
|
||||
# single-container image (#1042) points all three under one mounted volume,
|
||||
# so these must follow the same env vars the app itself reads rather than
|
||||
# hard-coding /app — otherwise the checks below guard directories nothing uses.
|
||||
DATA_DIRS="${STORAGE_PATH:-/app/storage} ${DATA_DIR:-/app/data} ${LOG_DIR:-/app/logs} ${BACKUP_DIR:-/backup}"
|
||||
DATA_DIRS="${STORAGE_PATH:-/app/storage} ${DATA_DIR:-/app/data} ${LOG_DIR:-/app/logs}"
|
||||
|
||||
# The backup root is adopted when it exists, but never gates startup. It is
|
||||
# optional: docker-compose.production.yml does not mount /backup, so a hardened
|
||||
# non-root deployment would fail `mkdir -p /backup` against a root-owned / and
|
||||
# stop booting over a directory it never needed. Only a path the operator has
|
||||
# explicitly configured (BACKUP_DIR, which the AIO image sets) joins the
|
||||
# adopted set.
|
||||
BACKUP_ROOT="${BACKUP_DIR:-}"
|
||||
if [ -n "$BACKUP_ROOT" ]; then
|
||||
DATA_DIRS="$DATA_DIRS $BACKUP_ROOT"
|
||||
fi
|
||||
|
||||
# Create the roots before touching them. With the compose layout each of the
|
||||
# three is its own mount point, so they always exist — but the single-container
|
||||
@@ -170,7 +181,9 @@ mkdir -p "$STORAGE_BASE/events/active" "$STORAGE_BASE/events/archived" "$STORAGE
|
||||
# subdirectories baked into the image are hidden, and the backup services do not
|
||||
# create them, so a scheduled or manual backup fails with ENOENT.
|
||||
BACKUP_BASE="${BACKUP_DIR:-/backup}"
|
||||
mkdir -p "$BACKUP_BASE/picpeak" "$BACKUP_BASE/database" 2>/dev/null || true
|
||||
if [ -d "$BACKUP_BASE" ]; then
|
||||
mkdir -p "$BACKUP_BASE/picpeak" "$BACKUP_BASE/database" 2>/dev/null || true
|
||||
fi
|
||||
|
||||
# Resolve which database engine this boot should use (#1038) BEFORE migrations
|
||||
# run, while the Postgres target is still untouched. An install that has been
|
||||
|
||||
Reference in New Issue
Block a user