feat(proxy): redact basic-auth and move the cache store off the command line (#93 workstream C)

lib/kamal/cli/proxy.rb

@@ -36,9 +36,12 @@ def boot
             raise "kamal-proxy version #{version} is too old, run `kamal proxy reboot` in order to update to at least #{Kamal::Configuration::Proxy::Run::MINIMUM_VERSION}"
           end
 
-          if (run_config = proxy.proxy_run_config)&.acme&.credentials?
+          if (run_config = proxy.proxy_run_config)&.secrets?
             execute *proxy.ensure_proxy_directory
             upload! run_config.secrets_io, run_config.secrets_path, mode: "0600"
+          else
+            # A host keeps no secrets it no longer needs.
+            execute *proxy.remove_proxy_secrets_file, raise_on_non_zero_exit: false
           end
 
           execute *proxy.ensure_apps_config_directory
@@ -65,11 +68,14 @@ def boot
           info "Starting loadbalancer on #{host}..."
           execute *KAMAL.registry.login
 
-          # The load balancer terminates TLS, so it issues certificates too -
-          # its host needs the acme credentials just like the proxy hosts do.
-          if (lb_run = KAMAL.loadbalancer_config.run).acme.credentials?
+          # The load balancer terminates TLS and owns the cache, so its host
+          # needs the proxy secrets (acme credentials, cache store) just like
+          # the proxy hosts do.
+          if (lb_run = KAMAL.loadbalancer_config.run).secrets?
             execute *KAMAL.loadbalancer.ensure_proxy_directory
             upload! lb_run.secrets_io, lb_run.secrets_path, mode: "0600"
+          else
+            execute *KAMAL.loadbalancer.remove_proxy_secrets_file, raise_on_non_zero_exit: false
           end
 
           execute *KAMAL.loadbalancer.ensure_apps_config_directory
@@ -199,10 +205,12 @@ def reboot
             execute *KAMAL.loadbalancer.remove_container
 
             # Same as boot: the replacement container's --env-file must find
-            # current credentials, not whatever an earlier boot left behind.
-            if (lb_run = KAMAL.loadbalancer_config.run).acme.credentials?
+            # current secrets, not whatever an earlier boot left behind.
+            if (lb_run = KAMAL.loadbalancer_config.run).secrets?
               execute *KAMAL.loadbalancer.ensure_proxy_directory
               upload! lb_run.secrets_io, lb_run.secrets_path, mode: "0600"
+            else
+              execute *KAMAL.loadbalancer.remove_proxy_secrets_file, raise_on_non_zero_exit: false
             end
 
             execute *KAMAL.loadbalancer.ensure_apps_config_directory

lib/kamal/cli/proxy/reboot.rb

@@ -43,8 +43,11 @@ def replace_container
       execute *proxy.ensure_proxy_directory
       execute *proxy.ensure_apps_config_directory
 
-      if (run_config = proxy.proxy_run_config)&.acme&.credentials?
+      if (run_config = proxy.proxy_run_config)&.secrets?
         upload! run_config.secrets_io, run_config.secrets_path, mode: "0600"
+      else
+        # A host keeps no secrets it no longer needs.
+        execute *proxy.remove_proxy_secrets_file, raise_on_non_zero_exit: false
       end
 
       execute *proxy.run(digest: drift.expected_digest)

lib/kamal/commands/loadbalancer.rb

@@ -92,12 +92,16 @@ def ensure_directory
     make_directory loadbalancer_config.directory
   end
 
-  # Where the acme credentials env file lands (see Proxy::Run#secrets_path) -
+  # Where the proxy secrets env file lands (see Proxy::Run#secrets_path) -
   # the same .kamal/proxy directory the per-app proxy hosts use.
   def ensure_proxy_directory
     make_directory loadbalancer_config.run.host_directory
   end
 
+  def remove_proxy_secrets_file
+    remove_file loadbalancer_config.run.secrets_path
+  end
+
   def ensure_apps_config_directory
     make_directory config.proxy_boot.apps_directory
   end

lib/kamal/commands/proxy.rb

@@ -115,6 +115,12 @@ def ensure_apps_config_directory
     make_directory config.proxy_boot.apps_directory
   end
 
+  # Static path rather than proxy_run_config.secrets_path: the file must be
+  # removable precisely when the run config (or its secrets) is gone.
+  def remove_proxy_secrets_file
+    remove_file File.join(config.proxy_boot.host_directory, Kamal::Configuration::Proxy::Run::SECRETS_FILENAME)
+  end
+
   def boot_config
     [ :echo, "#{substitute(read_boot_options)} #{substitute(read_image)}:#{substitute(read_image_version)} #{substitute(read_run_command)}" ]
   end

lib/kamal/configuration/docs/proxy.yml

@@ -901,16 +901,19 @@ proxy:
     #
     # Where the entries cached by `proxy/cache` are kept. Proxy-wide: one store
     # serves every service on this proxy, so unlike the policy above it belongs
-    # here, in the run configuration, and changing it reboots the proxy.
+    # here, in the run configuration. Adding or removing it reboots the proxy
+    # on the next deploy.
     #
     # `store` is `memory` (the default — a per-node cache) or a `redis://` /
     # `rediss://` URL that every proxy pointed at it shares, so one fetch warms
     # the whole fleet.
     #
-    # Note that a Redis URL with credentials in it ends up on the proxy's
-    # `docker run` command line, and so in host process listings and in kamal's
-    # audit log. Until that has a secrets-backed path, prefer a store reachable
-    # over a private network without a password in the URL.
+    # The URL never appears on the proxy's command line: kamal delivers it as
+    # `CACHE_STORE` in a 0600 env file on the host (alongside any ACME
+    # credentials), so a password in the URL stays out of process listings and
+    # kamal's audit log. The trade-off, same as the ACME credentials: changing
+    # only the URL's *value* does not move the drift digest — run
+    # `kamal proxy reboot` after rotating it.
     #
     # The remaining three only matter with a shared store, and are seconds:
     #

lib/kamal/configuration/proxy.rb

@@ -592,7 +592,10 @@ def basic_auth_credential
         raise Kamal::ConfigurationError, "proxy/basic_auth: password_secret '#{secret_name}' is empty"
       end
 
-      "#{basic_auth["username"]}:#{password}"
+      # Sensitive, so SSHKit redacts the credential wherever kamal prints the
+      # deploy command - at :info verbosity it used to land in plain text
+      # (registry-login precedent; Utils.optionize passes the marking through).
+      Kamal::Utils.sensitive("#{basic_auth["username"]}:#{password}")
     end
 
     # Serves both --path-timeout and --path-request-timeout, which kamal-proxy

lib/kamal/configuration/proxy/acme.rb

@@ -41,8 +41,10 @@ def credentials?
     credential_names.any?
   end
 
-  def secrets_io
-    Kamal::EnvFile.new(credential_names.to_h { |name| [ name, secrets[name] ] }).to_io
+  # The resolved credentials, for the shared proxy secrets env file that
+  # Proxy::Run#secrets_io assembles (cache store and acme travel together).
+  def credentials_env
+    credential_names.to_h { |name| [ name, secrets[name] ] }
   end
 
   # Rendered with `=` rather than a space, unlike the rest of the run command:

lib/kamal/configuration/proxy/run.rb

@@ -4,6 +4,12 @@ class Kamal::Configuration::Proxy::Run
   DEFAULT_HTTPS_PORT = 443
   DEFAULT_LOG_MAX_SIZE = "10m"
 
+  # One env file for everything the proxy must know but nothing may print:
+  # ACME DNS credentials and the cache store URL. Also referenced by
+  # Kamal::Commands::Proxy#remove_proxy_secrets_file, which cleans it up when
+  # the config no longer needs it.
+  SECRETS_FILENAME = "secrets.env"
+
   # Bump when the digest serialization changes, so every host converges with
   # exactly one reboot after upgrading kamal.
   DIGEST_SCHEMA_VERSION = "v1"
@@ -25,14 +31,15 @@ def self.digest(*parts)
   # Digest of the materialized run invocation, used to detect drift between
   # the running proxy container and the current configuration.
   #
-  # The ACME credential names ride along because --env-file names a path, not the
-  # variables inside it: swapping one credential for another would otherwise leave
-  # the digest unmoved and the old proxy running. Their values deliberately stay
-  # out — the digest is published as a docker label, and hashing secret material
-  # into a world-readable label buys an offline guessing target for nothing.
-  # Rotating a credential's value still needs an explicit `kamal proxy reboot`.
+  # The secret *names* ride along because --env-file names a path, not the
+  # variables inside it: swapping one credential for another (or adding the
+  # cache store) would otherwise leave the digest unmoved and the old proxy
+  # running. The values deliberately stay out — the digest is published as a
+  # docker label, and hashing secret material into a world-readable label buys
+  # an offline guessing target for nothing. Rotating a credential's value or
+  # the store URL still needs an explicit `kamal proxy reboot`.
   def config_digest
-    self.class.digest(image, run_command, *docker_options_args, *acme.credential_names)
+    self.class.digest(image, run_command, *docker_options_args, *secret_names)
   end
 
   def acme
@@ -153,7 +160,7 @@ def docker_options_args
       *publish_args,
       *logging_args,
       *("--expose=#{metrics_port}" if metrics_port.present?),
-      *acme_secrets_args,
+      *secrets_args,
       *docker_socket_args,
       *options_args
     ].compact
@@ -168,16 +175,21 @@ def docker_socket_args
     end
   end
 
-  # Where the ACME DNS credentials land on the proxy host. Under the proxy's own
-  # directory rather than the app's env directory, because the container is
-  # host-scoped and shared by every app on the host - and so `kamal proxy remove`
-  # takes the credentials with it.
+  # Where the proxy's secrets land on the host - the ACME DNS credentials and
+  # the cache store URL, which may embed one. Under the proxy's own directory
+  # rather than the app's env directory, because the container is host-scoped
+  # and shared by every app on the host - and so `kamal proxy remove` takes
+  # the secrets with it.
   def secrets_path
-    File.join host_directory, "acme.env"
+    File.join host_directory, SECRETS_FILENAME
+  end
+
+  def secrets?
+    acme.credentials? || cache_store.present?
   end
 
   def secrets_io
-    acme.secrets_io
+    Kamal::EnvFile.new(acme.credentials_env.merge(cache_store_env)).to_io
   end
 
   def host_directory
@@ -264,7 +276,10 @@ def ensure_no_conflicting_flags
 
     # Where cached responses are kept, which is a property of the proxy rather
     # than of any one service - the policy that fills the cache is per service,
-    # in proxy/cache.
+    # in proxy/cache. The store itself is deliberately absent: a store URL may
+    # embed credentials, so it travels as CACHE_STORE in the secrets env file
+    # (kamal-proxy reads it as the --cache-store default) rather than on a
+    # command line that lands in process listings and the audit log.
     #
     # lease_ttl and lease_wait take negative values to switch cross-node
     # coalescing off. That survives the space-separated rendering the rest of
@@ -274,16 +289,27 @@ def cache_options
       cache = run_config["cache"] || {}
 
       {
-        "cache-store": cache["store"],
         "cache-store-timeout": seconds_duration(cache["store_timeout"]),
         "cache-memory-size": cache["memory_size"],
         "cache-lease-ttl": seconds_duration(cache["lease_ttl"]),
         "cache-lease-wait": seconds_duration(cache["lease_wait"])
       }.compact
     end
 
-    def acme_secrets_args
-      argumentize "--env-file", secrets_path if acme.credentials?
+    def cache_store
+      run_config.dig("cache", "store")
+    end
+
+    def cache_store_env
+      cache_store.present? ? { "CACHE_STORE" => cache_store } : {}
+    end
+
+    def secret_names
+      [ *acme.credential_names, ("CACHE_STORE" if cache_store.present?) ].compact
+    end
+
+    def secrets_args
+      argumentize "--env-file", secrets_path if secrets?
     end
 
     def format_bind_ip(ip)

lib/kamal/utils.rb

@@ -21,11 +21,25 @@ def argumentize(argument, attributes, sensitive: false)
   end
 
   # Returns a list of shell-dashed option arguments. If the value is true, it's treated like a value-less option.
+  # A Sensitive value stays Sensitive: the rendered option keeps the real value
+  # for execution and a redacted form for anything kamal prints - same contract
+  # as argumentize's `sensitive:` kwarg, but decided per value by the caller.
   def optionize(args, with: nil, escape: true)
     options = if with
-      flatten_args(args).collect { |(key, value)| value == true ? "--#{key}" : "--#{key}#{with}#{escape ? escape_shell_value(value) : value}" }
+      flatten_args(args).collect do |(key, value)|
+        if value == true
+          "--#{key}"
+        else
+          rendered = "--#{key}#{with}#{escape ? escape_shell_value(value) : value}"
+          value.is_a?(Kamal::Utils::Sensitive) ? sensitive(rendered, redaction: "--#{key}#{with}#{value.redaction}") : rendered
+        end
+      end
     else
-      flatten_args(args).collect { |(key, value)| [ "--#{key}", value == true ? nil : escape ? escape_shell_value(value) : value ] }
+      flatten_args(args).collect do |(key, value)|
+        rendered = value == true ? nil : escape ? escape_shell_value(value) : value
+        rendered = sensitive(rendered, redaction: value.redaction) if value.is_a?(Kamal::Utils::Sensitive)
+        [ "--#{key}", rendered ]
+      end
     end
 
     options.flatten.compact

test/cli/proxy_test.rb

@@ -24,15 +24,15 @@ class CliProxyTest < CliTestCase
 
       run_command("boot", fixture: :with_proxy_acme).tap do |output|
         assert_match "mkdir -p .kamal/proxy on 1.1.1.1", output
-        assert_match "--env-file .kamal/proxy/acme.env", output
+        assert_match "--env-file .kamal/proxy/secrets.env", output
         assert_match "--acme-email=\"admin@example.com\"", output
         assert_match "--acme-dns-provider=\"cloudflare\"", output
         assert_match "--acme-http-fallback=\"false\"", output
 
         assert_no_match(/zone-rewriting-token/, output)
       end
 
-      assert_equal [ [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/acme.env", "0600" ] ], uploads
+      assert_equal [ [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/secrets.env", "0600" ] ], uploads
     end
   end
 
@@ -41,11 +41,33 @@ class CliProxyTest < CliTestCase
       uploads = capture_uploads
 
       run_command("reboot", "-y", fixture: :with_proxy_acme).tap do |output|
-        assert_match "--env-file .kamal/proxy/acme.env", output
+        assert_match "--env-file .kamal/proxy/secrets.env", output
         assert_no_match(/zone-rewriting-token/, output)
       end
 
-      assert_equal [ [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/acme.env", "0600" ] ], uploads
+      assert_equal [ [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/secrets.env", "0600" ] ], uploads
+    end
+  end
+
+  test "boot delivers the cache store through the secrets env file, never printing the URL" do
+    uploads = capture_uploads
+
+    run_command("boot", fixture: :with_proxy_cache_store).tap do |output|
+      assert_match "mkdir -p .kamal/proxy on 1.1.1.1", output
+      assert_match "--env-file .kamal/proxy/secrets.env", output
+
+      assert_no_match(/supers3cret/, output)
+      assert_no_match(/--cache-store/, output)
+    end
+
+    assert_equal [ [ "CACHE_STORE=redis://:supers3cret@cache.example.com:6379/0\n", ".kamal/proxy/secrets.env", "0600" ] ], uploads
+  end
+
+  # C3: a host keeps no secrets it no longer needs - when the config block
+  # goes away, the next boot takes the env file with it.
+  test "boot removes a stale secrets env file when the config no longer needs it" do
+    run_command("boot").tap do |output|
+      assert_match "rm .kamal/proxy/secrets.env", output
     end
   end
 
@@ -564,13 +586,13 @@ class CliProxyTest < CliTestCase
 
       run_command("boot", fixture: :with_loadbalancer_acme).tap do |output|
         assert_match "mkdir -p .kamal/proxy on lb.example.com", output
-        assert_match "--env-file .kamal/proxy/acme.env", output
+        assert_match "--env-file .kamal/proxy/secrets.env", output
         assert_match "--acme-email=\"admin@example.com\"", output
 
         assert_no_match(/zone-rewriting-token/, output)
       end
 
-      assert_includes uploads, [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/acme.env", "0600" ]
+      assert_includes uploads, [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/secrets.env", "0600" ]
     end
   end
 
@@ -585,7 +607,7 @@ class CliProxyTest < CliTestCase
         assert_no_match(/zone-rewriting-token/, output)
       end
 
-      assert_includes uploads, [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/acme.env", "0600" ]
+      assert_includes uploads, [ "CF_API_TOKEN=zone-rewriting-token\n", ".kamal/proxy/secrets.env", "0600" ]
     end
   end
 

test/commands/app_test.rb

@@ -198,9 +198,16 @@ class CommandsAppTest < ActiveSupport::TestCase
   test "deploy with basic auth" do
     @config[:proxy] = { "ssl" => true, "host" => "example.com", "basic_auth" => { "username" => "admin", "password" => "s3cr3t" } }
 
+    command = new_command.deploy(target: "172.1.0.2")
+
+    # The executed argv carries the real credential...
     assert_equal \
       "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"172.1.0.2:80\" --host=\"example.com\" --tls --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --basic-auth=\"admin:s3cr3t\" --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
-      new_command.deploy(target: "172.1.0.2").join(" ")
+      command.join(" ")
+
+    # ...but anything kamal prints redacts it (registry-login precedent).
+    assert_includes Kamal::Utils.redacted(command), "--basic-auth=[REDACTED]"
+    assert_no_match(/s3cr3t/, Kamal::Utils.redacted(command).join(" "))
   end
 
   test "deploy with SSL" do