diff --git a/caddy/docker-compose.yml b/caddy/docker-compose.yml index 6f7d709..c4fffad 100644 --- a/caddy/docker-compose.yml +++ b/caddy/docker-compose.yml @@ -13,6 +13,16 @@ services: - ${DATA_ROOT}/caddy/config:/config networks: - proxy + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" + healthcheck: + test: ["CMD", "caddy", "validate", "--config", "/etc/caddy/Caddyfile"] + interval: 30s + timeout: 5s + retries: 3 networks: proxy: diff --git a/gitea/docker-compose.yml b/gitea/docker-compose.yml index d6d8fdb..44f40ca 100644 --- a/gitea/docker-compose.yml +++ b/gitea/docker-compose.yml @@ -11,6 +11,16 @@ services: - "2222:2222" networks: - proxy + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:3000/api/healthz"] + interval: 30s + timeout: 10s + retries: 3 networks: proxy: diff --git a/monitoring/docker-compose.yml b/monitoring/docker-compose.yml index 64b68fb..377974b 100644 --- a/monitoring/docker-compose.yml +++ b/monitoring/docker-compose.yml @@ -27,6 +27,11 @@ services: - VOLUMES=0 networks: - monitoring + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" alloy: image: grafana/alloy:v1.14.1 @@ -47,6 +52,11 @@ services: pid: host networks: - monitoring + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" networks: monitoring: diff --git a/nextcloud/.env.example b/nextcloud/.env.example index 0b550fa..72a5892 100644 --- a/nextcloud/.env.example +++ b/nextcloud/.env.example @@ -11,3 +11,6 @@ NEXTCLOUD_ADMIN_PASSWORD=CHANGE_ME_admin_password NEXTCLOUD_TRUSTED_DOMAINS=nextcloud.t-gstone.de OVERWRITEPROTOCOL=https OVERWRITECLIURL=https://nextcloud.t-gstone.de + +# Redis +REDIS_PASSWORD=CHANGE_ME_redis_password diff --git a/nextcloud/docker-compose.yml b/nextcloud/docker-compose.yml index ac83855..7a654ed 100644 --- a/nextcloud/docker-compose.yml +++ b/nextcloud/docker-compose.yml @@ -12,12 +12,23 @@ services: environment: - POSTGRES_HOST=postgres - REDIS_HOST=redis + - REDIS_HOST_PASSWORD=${REDIS_PASSWORD} volumes: - ${DATA_ROOT}/nextcloud/html:/var/www/html - ${DATA_ROOT}/nextcloud/data:/var/www/html/data networks: - proxy - nextcloud-internal + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost/status.php"] + interval: 30s + timeout: 10s + retries: 3 postgres: image: postgres:16-alpine @@ -33,13 +44,25 @@ services: interval: 10s timeout: 5s retries: 5 + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" redis: image: redis:7-alpine container_name: nextcloud-redis restart: unless-stopped + command: redis-server --requirepass ${REDIS_PASSWORD} + env_file: .env networks: - nextcloud-internal + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" cron: image: nextcloud:29-apache @@ -53,6 +76,11 @@ services: - ${DATA_ROOT}/nextcloud/data:/var/www/html/data networks: - nextcloud-internal + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" networks: proxy: diff --git a/review.md b/review.md index 6d9e159..65c4077 100644 --- a/review.md +++ b/review.md @@ -1,14 +1,16 @@ -# Code Review Issues +# Repo Review — nextcloud-selfhosted -| # | Severity | File | Issue | Status | -|----|----------|---------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------| -| 1 | Critical | `scripts/deploy.sh` | `SCRIPT_DIR` resolves to `scripts/` but paths assume repo root (e.g. `$SCRIPT_DIR/caddy/docker-compose.yml`). All scripts broken after move to `scripts/`. Fix: use `REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"` | DONE | -| 2 | Critical | `scripts/backup.sh` | Same broken `SCRIPT_DIR` path issue | DONE | -| 3 | Critical | `scripts/restore.sh` | Same broken `SCRIPT_DIR` path issue | DONE | -| 4 | High | `scripts/backup.sh:20` | `pg_dumpall -U nextcloud` hardcodes DB username instead of reading from env | DONE | -| 5 | High | `scripts/restore.sh:68` | `psql -U nextcloud` hardcodes DB username instead of reading from env | DONE | -| 6 | High | `scripts/deploy.sh:13` | `source .env` in a root-privileged script can execute arbitrary commands. Consider safer parsing or variable validation | DONE | -| 7 | Medium | `monitoring/docker-compose.yml` | Docker socket + `/proc` + `/sys` + `/` mounted into Alloy container. Consider using a Docker socket proxy to limit API access | DONE | -| 8 | Medium | `caddy/Caddyfile` | No rate limiting configured at the reverse proxy layer | DONE | -| 9 | Low | `gitea/docker-compose.yml` | `gitea/gitea:latest-rootless` unpinned — pin to specific version like Nextcloud does | DONE | -| 10 | Low | `monitoring/docker-compose.yml` | `grafana/alloy:latest` unpinned — pin to specific version | DONE | +| # | Priority | Category | Issue | Location | Suggestion | Status | +|----|----------|-------------|-----------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------|---------| +| 1 | High | Security | `backup.sh` and `restore.sh` use `source` to load `.env` files, which executes arbitrary shell code | `scripts/backup.sh:5-6`, `scripts/restore.sh:5-6` | Replace `source` with the safe `eval "$(grep ...)"` parser used in `deploy.sh:14` | DONE | +| 2 | High | Correctness | Cron path hint is wrong — says `$REPO_ROOT/backup.sh` instead of `$REPO_ROOT/scripts/backup.sh` | `scripts/backup.sh:49` | Change to `$REPO_ROOT/scripts/backup.sh` | DONE | +| 3 | Medium | Correctness | Postgres readiness check uses `sleep 5` instead of a proper wait | `scripts/restore.sh:66` | Use `docker compose up -d --wait postgres` or poll with `pg_isready` in a loop | DONE | +| 4 | Medium | Correctness | `pg_dumpall` output restored with `psql -U $POSTGRES_USER` — role creation statements may fail | `scripts/restore.sh:69` | Restore against the `postgres` database: `psql -U "$POSTGRES_USER" -d postgres` | DONE | +| 5 | Medium | Reliability | No Docker log rotation — JSON log driver can fill disk | All `docker-compose.yml` files | Add `logging: { driver: json-file, options: { max-size: "10m", max-file: "3" } }` to each service, or configure in `/etc/docker/daemon.json` | DONE | +| 6 | Medium | Security | Alloy container mounts entire root filesystem (`/:/host/root:ro`) — exposes secrets in `.env` files | `monitoring/docker-compose.yml:42` | Mount only needed paths (e.g., `/etc:/host/etc:ro`) or use a more restrictive bind | SKIPPED | +| 7 | Medium | Reliability | Rate limits mentioned in commit `0f12c5f` but not present in Caddyfile | `caddy/Caddyfile` | Add `rate_limit` directive or verify the commit wasn't partially reverted | SKIPPED | +| 8 | Low | Backup | Caddy TLS certificates (`${DATA_ROOT}/caddy/data/`) not included in backup | `scripts/backup.sh` | Add a `tar` step for `caddy/data` — avoids Let's Encrypt rate limits on restore | DONE | +| 9 | Low | Reliability | `deploy.sh` doesn't pull latest images before starting | `scripts/deploy.sh:78-88` | Add `docker compose pull` before each `up -d` call | DONE | +| 10 | Low | Security | Redis has no password — reachable from any container on `nextcloud-internal` network | `nextcloud/docker-compose.yml:38-42` | Add `command: redis-server --requirepass $REDIS_PASSWORD` and pass the password to Nextcloud via `REDIS_HOST_PASSWORD` | DONE | +| 11 | Low | Reliability | No healthchecks on Nextcloud, Gitea, or Caddy containers | `nextcloud/docker-compose.yml`, `gitea/docker-compose.yml`, `caddy/docker-compose.yml` | Add `healthcheck` blocks (e.g., `curl -f http://localhost` for Nextcloud, `caddy validate` for Caddy) | DONE | +| 12 | Low | Reliability | No container resource limits — a runaway process can OOM the VPS | All `docker-compose.yml` files | Add `mem_limit` and `cpus` to at least Nextcloud, Postgres, and Alloy | SKIPPED | diff --git a/scripts/backup.sh b/scripts/backup.sh index a214099..a6e8483 100755 --- a/scripts/backup.sh +++ b/scripts/backup.sh @@ -2,8 +2,12 @@ set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -source "$REPO_ROOT/.env" -source "$REPO_ROOT/nextcloud/.env" +set -a +eval "$(grep -v '^#' "$REPO_ROOT/.env" | grep -v '^$' | grep '^[A-Za-z_][A-Za-z_0-9]*=' )" +set +a +set -a +eval "$(grep -v '^#' "$REPO_ROOT/nextcloud/.env" | grep -v '^$' | grep '^[A-Za-z_][A-Za-z_0-9]*=' )" +set +a DATA_ROOT="${DATA_ROOT:-/opt/docker-data}" BACKUP_DIR="/opt/backups" @@ -34,6 +38,13 @@ echo " -> Archiving Gitea data..." tar -czf "$BACKUP_DIR/gitea-$DATE.tar.gz" \ -C "$DATA_ROOT" gitea/data gitea/config +# ------------------------------------------------------------------ +# Caddy TLS certificates +# ------------------------------------------------------------------ +echo " -> Archiving Caddy TLS data..." +tar -czf "$BACKUP_DIR/caddy-$DATE.tar.gz" \ + -C "$DATA_ROOT" caddy/data + # ------------------------------------------------------------------ # Rotate old backups # ------------------------------------------------------------------ @@ -46,4 +57,4 @@ ls -lh "$BACKUP_DIR"/*"$DATE"* 2>/dev/null || echo " (no files found)" echo "" echo "To schedule daily backups, add to crontab (crontab -e):" -echo " 0 3 * * * $REPO_ROOT/backup.sh >> /var/log/backup.log 2>&1" +echo " 0 3 * * * $REPO_ROOT/scripts/backup.sh >> /var/log/backup.log 2>&1" diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 3a95526..76e3859 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -76,15 +76,19 @@ done # Start stacks in order # ------------------------------------------------------------------ echo "==> Starting Caddy..." +docker compose -f "$REPO_ROOT/caddy/docker-compose.yml" --env-file "$REPO_ROOT/.env" pull docker compose -f "$REPO_ROOT/caddy/docker-compose.yml" --env-file "$REPO_ROOT/.env" up -d echo "==> Starting Nextcloud..." +docker compose -f "$REPO_ROOT/nextcloud/docker-compose.yml" --env-file "$REPO_ROOT/.env" pull docker compose -f "$REPO_ROOT/nextcloud/docker-compose.yml" --env-file "$REPO_ROOT/.env" up -d echo "==> Starting Gitea..." +docker compose -f "$REPO_ROOT/gitea/docker-compose.yml" --env-file "$REPO_ROOT/.env" pull docker compose -f "$REPO_ROOT/gitea/docker-compose.yml" --env-file "$REPO_ROOT/.env" up -d echo "==> Starting Monitoring..." +docker compose -f "$REPO_ROOT/monitoring/docker-compose.yml" --env-file "$REPO_ROOT/.env" pull docker compose -f "$REPO_ROOT/monitoring/docker-compose.yml" --env-file "$REPO_ROOT/.env" up -d echo "" diff --git a/scripts/restore.sh b/scripts/restore.sh index eaa598b..8ed2444 100755 --- a/scripts/restore.sh +++ b/scripts/restore.sh @@ -2,8 +2,12 @@ set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -source "$REPO_ROOT/.env" -source "$REPO_ROOT/nextcloud/.env" +set -a +eval "$(grep -v '^#' "$REPO_ROOT/.env" | grep -v '^$' | grep '^[A-Za-z_][A-Za-z_0-9]*=' )" +set +a +set -a +eval "$(grep -v '^#' "$REPO_ROOT/nextcloud/.env" | grep -v '^$' | grep '^[A-Za-z_][A-Za-z_0-9]*=' )" +set +a DATA_ROOT="${DATA_ROOT:-/opt/docker-data}" BACKUP_DIR="/opt/backups" @@ -63,10 +67,12 @@ tar -xzf "$GITEA_ARCHIVE" -C "$DATA_ROOT" echo "==> Starting Postgres for DB restore..." docker compose -f "$REPO_ROOT/nextcloud/docker-compose.yml" --env-file "$REPO_ROOT/.env" up -d postgres echo " -> Waiting for Postgres to be ready..." -sleep 5 +until docker exec nextcloud-postgres pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB" &>/dev/null; do + sleep 1 +done echo "==> Restoring Nextcloud database..." -docker exec -i nextcloud-postgres psql -U "$POSTGRES_USER" < "$DB_DUMP" +docker exec -i nextcloud-postgres psql -U "$POSTGRES_USER" -d postgres < "$DB_DUMP" # ------------------------------------------------------------------ # Start all services