Skip to content

fix(proxy): sync the secrets env file on every reboot path - #110

Merged
mhenrixon merged 1 commit into
dashfrom
fix/port-holder-reboot-secrets-env
Aug 11, 2026
Merged

fix(proxy): sync the secrets env file on every reboot path#110
mhenrixon merged 1 commit into
dashfrom
fix/port-holder-reboot-secrets-env

Conversation

@mhenrixon

@mhenrixon mhenrixon commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

The bug

The port-holder replacement paths (handoff_generation, fresh generation, migrate_to_holder) all launch a container that reads --env-file .kamal/proxy/secrets.env at docker run — but only the legacy stop_and_replace path uploaded the file first. A drift-triggered reboot on a port-holder host that had never held the file (the exact situation right after run.acme.credentials or a cache store is first added to the config) failed with:

docker: open .kamal/proxy/secrets.env: no such file or directory

after the old proxy container was already stopped and pruned, leaving the host with no proxy at all. Hit in the wild on the first deploy that added acme credentials to a fleet already running port_holder: true.

LoadbalancerReboot and proxy boot both sync the file correctly; only Reboot's port-holder branches missed it.

The fix

Hoist the sync into replace_container, before any path launches a generation: upload (0600) when the run config carries secrets, remove the stale file when it no longer does — same semantics stop_and_replace had, now covering all four paths.

Tests

  • New fixture deploy_with_proxy_acme_port_holder.yml + regression test driving a port-holder reboot with acme credentials, asserting the 0600 upload happens and the token never reaches the command line. RED on dash (uploads []), GREEN with the fix.
  • test/cli/proxy_test.rb: 102 runs, 0 failures. Full unit suite (sans integration): identical failure set to clean dash (the documented arch/Docker-dependent ones) plus the one new test.
  • rubocop clean.

Summary by cubic

Ensure all proxy reboot paths with port-holder sync the secrets env file before starting any generation. This prevents docker failures (“open .kamal/proxy/secrets.env: no such file or directory”) and avoids proxy downtime.

  • Bug Fixes
    • Moved secrets sync into replace_container via sync_proxy_secrets, executed before all reboot paths.
    • Uploads .kamal/proxy/secrets.env with mode 0600 when secrets exist; removes it when not.
    • Added regression test and fixture for port-holder reboot with ACME credentials; verifies upload and that tokens never appear in command output.

Written for commit 2683c18. Summary will update on new commits.

Review in cubic

The port-holder replacement paths (generation handoff, fresh generation,
holder migration) all launch a container that reads --env-file at docker
run, but only the legacy stop_and_replace path uploaded the proxy
secrets file first. A port-holder reboot on a host that never held the
file — a drift reboot right after acme credentials or a cache store were
added to the run config — died with:

    docker: open .kamal/proxy/secrets.env: no such file or directory

after the old proxy was already stopped and pruned, leaving the host
with no proxy at all.

The sync (upload when the run config carries secrets, remove when it no
longer does) now happens in replace_container, before any path launches
a generation. Regression test drives a port-holder reboot with acme
credentials and asserts the upload.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@mhenrixon mhenrixon self-assigned this Aug 11, 2026
@mhenrixon
mhenrixon merged commit 9458fa1 into dash Aug 11, 2026
10 checks passed
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