docker-compose: harden default credentials and proxy trust (draft, under discussion)#7907
Conversation
- Require ADMIN_PASSWORD and the database password to be provided explicitly (no implicit fallback). - Default TRUST_PROXY to false. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Code Review by Qodo
1. Docs mismatch compose
|
Review Summary by QodoHarden docker-compose defaults with required credentials
WalkthroughsDescription• Require explicit ADMIN_PASSWORD and database password configuration • Default TRUST_PROXY to false for security • Add inline documentation for credential requirements • Prevent insecure defaults from being used silently Diagramflowchart LR
A["docker-compose.yml"] -->|Remove insecure defaults| B["ADMIN_PASSWORD required"]
A -->|Remove insecure defaults| C["DB_PASS required"]
A -->|Change default| D["TRUST_PROXY false"]
B -->|Fail fast| E["docker compose up fails without config"]
C -->|Fail fast| E
D -->|Reduce IP spoofing risk| F["Enhanced security posture"]
File Changes1. docker-compose.yml
|
| # Required — set a strong value (e.g. in .env). No fallback, so misconfig | ||
| # surfaces at `docker compose up` rather than at runtime. | ||
| ADMIN_PASSWORD: "${DOCKER_COMPOSE_APP_ADMIN_PASSWORD:?Set DOCKER_COMPOSE_APP_ADMIN_PASSWORD to a strong value}" | ||
| DB_CHARSET: ${DOCKER_COMPOSE_APP_DB_CHARSET:-utf8mb4} | ||
| DB_HOST: postgres | ||
| DB_NAME: ${DOCKER_COMPOSE_POSTGRES_DATABASE:-etherpad} | ||
| DB_PASS: ${DOCKER_COMPOSE_POSTGRES_PASSWORD:-admin} | ||
| DB_PASS: "${DOCKER_COMPOSE_POSTGRES_PASSWORD:?Set DOCKER_COMPOSE_POSTGRES_PASSWORD to a strong value}" | ||
| DB_PORT: ${DOCKER_COMPOSE_POSTGRES_PORT:-5432} | ||
| DB_TYPE: "postgres" | ||
| DB_USER: ${DOCKER_COMPOSE_POSTGRES_USER:-admin} | ||
| # For now, the env var DEFAULT_PAD_TEXT cannot be unset or empty; it seems to be mandatory in the latest version of etherpad | ||
| DEFAULT_PAD_TEXT: ${DOCKER_COMPOSE_APP_DEFAULT_PAD_TEXT:- } | ||
| DISABLE_IP_LOGGING: ${DOCKER_COMPOSE_APP_DISABLE_IP_LOGGING:-false} | ||
| SOFFICE: ${DOCKER_COMPOSE_APP_SOFFICE:-null} | ||
| TRUST_PROXY: ${DOCKER_COMPOSE_APP_TRUST_PROXY:-true} | ||
| # Default off: only enable when actually behind a trusted reverse proxy | ||
| # that sets the X-Forwarded-* headers. | ||
| TRUST_PROXY: ${DOCKER_COMPOSE_APP_TRUST_PROXY:-false} |
There was a problem hiding this comment.
1. Docs mismatch compose 🐞 Bug ⚙ Maintainability
docker-compose.yml now requires DOCKER_COMPOSE_APP_ADMIN_PASSWORD and DOCKER_COMPOSE_POSTGRES_PASSWORD and defaults TRUST_PROXY to false, but README.md and doc/docker.md still show admin/admin defaults and TRUST_PROXY defaulting to true. Users copy-pasting the documented compose snippets will either fail at docker compose up (missing required vars) or deploy with different behavior than documented.
Agent Prompt
## Issue description
`docker-compose.yml` was hardened (required secrets + TRUST_PROXY default flip), but the repo’s Docker Compose documentation snippets are now stale. This causes immediate breakage for users following docs and undermines the intent of the hardening.
## Issue Context
- The compose file now uses `${VAR:?message}` for `DOCKER_COMPOSE_APP_ADMIN_PASSWORD` and `DOCKER_COMPOSE_POSTGRES_PASSWORD`, so `docker compose up` fails if they’re unset/empty.
- Docs still show `:-admin` fallbacks and `TRUST_PROXY` defaulting to `true`.
## Fix Focus Areas
- docker-compose.yml[20-38]
- README.md[102-157]
- doc/docker.md[335-395]
- .env.default[7-18]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # Default off: only enable when actually behind a trusted reverse proxy | ||
| # that sets the X-Forwarded-* headers. | ||
| TRUST_PROXY: ${DOCKER_COMPOSE_APP_TRUST_PROXY:-false} |
There was a problem hiding this comment.
2. Proxy default breaks https 🐞 Bug ≡ Correctness
docker-compose.yml now defaults TRUST_PROXY to false, which prevents Express from trusting X-Forwarded-* headers and breaks secure-cookie auto detection when Etherpad is behind an SSL-terminating reverse proxy. This can lead to incorrect HTTPS detection (express-session `secure: 'auto'`), affecting login/session behavior and deployments that require secure cookies.
Agent Prompt
## Issue description
`TRUST_PROXY` defaulting to `false` is likely to break common production setups where Etherpad is behind an SSL-terminating reverse proxy and relies on `X-Forwarded-Proto` and related headers.
## Issue Context
- Etherpad only enables Express `trust proxy` when `settings.trustProxy` is true.
- The code explicitly documents that SSL termination requires `trustProxy=true` for cookie `secure: 'auto'` detection via `X-Forwarded-Proto`.
## Fix Focus Areas
- docker-compose.yml[36-38]
- src/node/hooks/express.ts[157-165]
- src/node/hooks/express.ts[221-238]
- doc/docker.md[240-254]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Draft — under discussion. Splitting the deployment-default changes out of #7905 so the non-breaking fixes there can merge independently.
Changes
ADMIN_PASSWORDand the database password to be set explicitly (noadminfallback).TRUST_PROXYtofalse.ADMIN_PASSWORD/ DB password required.docker compose upwill fail until they're set (e.g. in.env). Keeps the reference compose from booting withadmin/admin.ports:are not published here, so the DB is internal-only — requiring the DB password is more hygiene than exposure fix. Keep it required, or revert just the DB part?TRUST_PROXYdefaulttrue → false. Likely to be reverted: this compose is typically run behind a reverse proxy, wheretrueis correct; flipping it can break HTTPS detection (cookie.secure: "auto") and client-IP/rate-limiting for those users. The risk it addresses (IP spoofing) only applies to a directly-exposed instance.Decisions to make before un-drafting
ADMIN_PASSWORDrequired, or auto-generate + print on first run instead?TRUST_PROXYflip (lean yes) and document instead?🤖 Generated with Claude Code