Skip to content

Security: Externalize deployment configuration#141

Open
eirvandelden wants to merge 2 commits into
mainfrom
security/issue-7-externalize-network-config
Open

Security: Externalize deployment configuration#141
eirvandelden wants to merge 2 commits into
mainfrom
security/issue-7-externalize-network-config

Conversation

@eirvandelden

@eirvandelden eirvandelden commented Feb 11, 2026

Copy link
Copy Markdown
Owner

Moves sensitive network information (IPs, domains) from deploy.yml to environment variables.

Changes

config/deploy.yml

  • Web server: 192.168.1.216<%= ENV.fetch("DEPLOY_WEB_HOST") %>
  • Proxy hosts: Hard-coded list → <%= ENV.fetch("DEPLOY_PROXY_HOSTS").split(",") %>
  • Registry server: registry.vandelden.family<%= ENV.fetch("KAMAL_REGISTRY_SERVER") %>
  • Registry username: root<%= ENV.fetch("KAMAL_REGISTRY_USERNAME", "root") %>
  • Builder host: ssh://root@192.168.1.128<%= ENV.fetch("DEPLOY_BUILDER_HOST") %>

.env.example (new file)

  • Documents all required environment variables with placeholder values
  • Contains no real IPs or hostnames — copy to .env and fill in your actual values
  • .env is already gitignored (line 97 of .gitignore), so it won't be accidentally committed

Security Impact

LOW - Reduces information disclosure:

  • Internal IP addresses no longer in version control
  • Domain names externalized for better portability
  • Easier to use different values per environment

Usage

# Copy example file
cp .env.example .env

# Edit with your actual values
nano .env

# Deploy
bin/kamal deploy

Verification

export DEPLOY_WEB_HOST="<your-web-server-ip>"
export DEPLOY_PROXY_HOSTS="<your-proxy-hosts>"
export KAMAL_REGISTRY_SERVER="<your-registry-host>"
export KAMAL_REGISTRY_USERNAME="root"
export DEPLOY_BUILDER_HOST="ssh://root@<your-builder-ip>"

bin/kamal config  # Should show no errors

@eirvandelden

Copy link
Copy Markdown
Owner Author

Code Review: Security concern not fully resolved

The approach here is correct — using ENV.fetch(...) in config/deploy.yml is the right Kamal pattern for externalizing config. However, the primary security goal is not yet achieved.

[CRITICAL] .env.example still exposes the actual sensitive values

The .env.example file is committed to the repository and contains the real IPs and domains:

DEPLOY_WEB_HOST=192.168.1.216
DEPLOY_PROXY_HOSTS=finances.vandelden.family,journal_administration.vandelden.family
KAMAL_REGISTRY_SERVER=registry.vandelden.family
DEPLOY_BUILDER_HOST=ssh://root@192.168.1.128

This defeats the security goal. The values are still in version control — just in a different file. .env.example should use placeholder values, not real ones:

DEPLOY_WEB_HOST=<your-web-server-ip>
DEPLOY_PROXY_HOSTS=app1.example.com,app2.example.com
KAMAL_REGISTRY_SERVER=<your-registry-host>
DEPLOY_BUILDER_HOST=ssh://root@<your-builder-ip>

The same applies to the PR description itself — the "Verification" section also contains the actual IPs and domains, which will remain visible in the PR history.

[MINOR] Inaccurate statement in PR description

The PR description says about .env.example: "Not committed to repository (use .env locally)". This is incorrect — .env.example is committed (it's in this diff). It's .env that is gitignored. Worth correcting to avoid confusion.

[MINOR] .gitignore not updated (informational)

No change to .gitignore was made, but .env is already covered on line 97, so this is fine. Worth mentioning in the PR description so reviewers don't wonder about it.


What's working well

  • Correct use of ENV.fetch("VAR") and ENV.fetch("VAR", default) in config/deploy.yml
  • KAMAL_REGISTRY_USERNAME defaulting to "root" is sensible and well-documented ✓
  • The hosts: <%= ENV.fetch("DEPLOY_PROXY_HOSTS").split(",") %> pattern for a list is clean ✓

Required change before merging

Replace the actual values in .env.example with descriptive placeholders.

eirvandelden and others added 2 commits July 8, 2026 17:30
- Move hard-coded IPs and domains to environment variables
- Add ERB interpolation in config/deploy.yml
- Create .env.example with placeholder values
- Improve configuration portability and security

Resolves information disclosure issue #7

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Actual network addresses in .env.example defeated the security goal
of the PR since the values were still committed to version control.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@eirvandelden eirvandelden force-pushed the security/issue-7-externalize-network-config branch from b310789 to bc9ef37 Compare July 8, 2026 17:00
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.

1 participant