Skip to content

fix: pass .env variables through to containers in compose.example.yml#2811

Open
RealDiligent wants to merge 1 commit into
we-promise:mainfrom
RealDiligent:fix/critical-issue-compose-env-file
Open

fix: pass .env variables through to containers in compose.example.yml#2811
RealDiligent wants to merge 1 commit into
we-promise:mainfrom
RealDiligent:fix/critical-issue-compose-env-file

Conversation

@RealDiligent

@RealDiligent RealDiligent commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #2773

Problem

Self-hosters following the documented setup put variables like ONBOARDING_STATE, APP_DOMAIN, SMTP_*, and EMAIL_SENDER in their .env, but the app never sees them — onboarding stays open and email doesn't work. printenv inside the container shows only the handful of variables hardcoded in the compose file. Same root cause reported earlier in #2489 (APP_DOMAIN + SMTP).

Root cause

compose.example.yml declares container environment only via the x-rails-env anchor, which lists just DB / Redis / SSL / OpenAI variables, and sets no env_file.

Docker Compose automatically reads a .env file for ${VAR} interpolation inside the compose file, but that does not inject those values into the container's environment unless they're referenced somewhere. Since the documented self-hosting variables aren't in the anchor, they're silently dropped.

Fix

Add env_file to the web and worker services so the user's .env is passed through to the containers:

    env_file:
      - path: .env
        required: false
  • required: false keeps the "runs without any configuration" promise (docs docker.md line 54) — the out-of-the-box run with no .env still works.
  • The explicit environment: anchor takes precedence over env_file in Docker Compose, so container-critical settings (DB_HOST: db, REDIS_URL, SELF_HOSTED) remain authoritative and can't be clobbered by a stray .env value.
  • env_file injects only the variables the user actually set, so absent variables stay truly unset and the app's ENV[...].presence/ENV.fetch(..., default) fallbacks keep working (verified for ONBOARDING_STATE in app/models/setting.rb and APP_DOMAIN in config/environments/production.rb).

This resolves all documented self-hosting variables at once, not just the handful in the report.

Why not enumerate vars in the anchor instead

Listing each variable as KEY: ${KEY:-} would force an empty string for every unset variable, which differs from "unset" and can defeat ENV.fetch(key, default) for enum-style vars (e.g. provider selection). env_file avoids that footgun by only passing what's present.

Compatibility

The required: false long form needs Docker Compose ≥ 2.24 (Jan 2024); the setup guide already uses the docker compose v2 CLI throughout. Happy to fall back to enumerating specific variables in the anchor if the team prefers to support older Compose.

Validation

  • YAML parses; environment precedence + env_file behavior confirmed against the Docker Compose spec.
  • Ran the repo's full Pull Request CI on a fork — all jobs green (the change is config-only, no Ruby impact).

Risk / scope

  • Single file, +11 lines, comments included. No app code changes.
  • compose.example.ai.yml has the same pattern; happy to apply the same fix there in a follow-up if desired.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Configuration
    • Added optional .env file support for web and worker services.
    • Explicit service environment settings take precedence over values in .env.
    • The setup continues to work when no .env file is present.

The example Compose file declared container env only via the x-rails-env
anchor, which lists just DB/Redis/SSL/OpenAI vars, and set no env_file.
Docker Compose auto-reads .env for ${VAR} interpolation in the compose
file itself, but that does NOT inject those values into the containers
unless they are referenced. So documented self-hosting variables placed
in .env — ONBOARDING_STATE, APP_DOMAIN, SMTP_*, EMAIL_SENDER, etc. —
never reached the app: onboarding stayed open and email was broken.

Add env_file: .env (with required: false, so the out-of-the-box run with
no .env still works) to the web and worker services. The explicit
environment anchor still takes precedence, so container-critical settings
(DB_HOST, REDIS_URL, SELF_HOSTED) remain authoritative, and only the
user's own .env values are injected — absent vars stay truly unset so the
app's ENV fallbacks keep working.

Fixes we-promise#2773

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d2134a67-f990-4f44-905f-c3ef2e82b643

📥 Commits

Reviewing files that changed from the base of the PR and between c9a28e1 and 3b581bf.

📒 Files selected for processing (1)
  • compose.example.yml

📝 Walkthrough

Walkthrough

The Compose example adds optional .env loading to the web and worker services. Explicit environment values continue to take precedence, and Compose remains valid when .env is absent.

Changes

Compose environment loading

Layer / File(s) Summary
Optional environment loading for application services
compose.example.yml
The web and worker services reference .env with required: false, while comments document that explicit environment values override it.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: sure-admin

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adding optional .env loading for compose.example.yml.
Linked Issues check ✅ Passed The change delivers the issue goal by loading .env into both web and worker while preserving explicit environment values.
Out of Scope Changes check ✅ Passed The PR is limited to compose.example.yml and related comments, with no unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b581bf42c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread compose.example.yml
# SMTP_*, etc.). The explicit `environment` values below take precedence
# over .env, so container-critical settings (DB_HOST, REDIS_URL, ...) stay
# correct. `required: false` keeps this working when no .env file exists.
env_file:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep .env from overriding the fixed container port

When a self-hoster changes the documented PORT value in .env, this env_file also injects that value into web, so Puma listens on it (config/puma.rb:44) while Compose still publishes ${PORT}:3000 (compose.example.yml:72). For example, PORT=3001 maps host port 3001 to an unused container port 3000, making the UI unreachable. The Compose short port syntax is [HOST:]CONTAINER[/PROTOCOL]; pin PORT: 3000 in the explicit container environment or change the container-side mapping accordingly.

Useful? React with 👍 / 👎.

Comment thread compose.example.yml
# over .env, so container-critical settings (DB_HOST, REDIS_URL, ...) stay
# correct. `required: false` keeps this working when no .env file exists.
env_file:
- path: .env

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve defaults for blank entries in the sample environment

When users follow docs/hosting/docker.md:61-96 and leave optional entries from .env.example untouched, this imports values such as PRODUCT_NAME=, BRAND_NAME=, and EMAIL_SENDER= as empty strings rather than leaving them unset; the Compose env_file syntax specifies that VAR= becomes an empty string. Consequently, the ENV.fetch defaults in config/initializers/brand.rb:2-3 and app/mailers/application_mailer.rb:3 are bypassed, producing blank product branding and potentially a blank default sender. Comment out optional blank entries in the distributed sample or normalize them before relying on ENV.fetch defaults.

Useful? React with 👍 / 👎.

@jjmata jjmata left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct diagnosis of the Compose behavior: .env interpolates ${VAR} references inside the compose file itself, but that's separate from injecting those vars into the container's environment, which needs env_file. Using required: false preserves the "runs with zero configuration" promise, and the explanation of why environment: still wins over env_file (so DB_HOST/REDIS_URL/SELF_HOSTED can't be clobbered by a stray .env value) is accurate and worth having as an inline comment for future readers, which it is.

Good call also explicitly rejecting the "enumerate each var as KEY: ${KEY:-}" alternative — that would force empty-string instead of truly-unset for absent vars, which would break ENV.fetch(key, default)-style fallbacks. Small, config-only, low risk. No concerns; compose.example.ai.yml having the same gap is correctly called out as a possible follow-up rather than scope creep here.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Environment variables from .env do not seem to be passed

2 participants