Avoid setting env via SSHKit

SSHKit puts the env in the command, so leaks them in process listings.

lib/kamal/cli/base.rb

@@ -135,8 +135,10 @@ module Kamal::Cli
           details = { hosts: KAMAL.hosts.join(","), command: command, subcommand: subcommand }
 
           say "Running the #{hook} hook...", :magenta
-          run_locally do
+          with_env KAMAL.hook.env(**details, **extra_details) do
-            execute *KAMAL.hook.run(hook, **details, **extra_details)
+            run_locally do
+              execute *KAMAL.hook.run(hook)
+            end
           rescue SSHKit::Command::Failed => e
             raise HookError.new("Hook `#{hook}` failed:\n#{e.message}")
           end
@@ -183,5 +185,14 @@ module Kamal::Cli
           execute(*KAMAL.server.ensure_run_directory)
         end
       end
+
+      def with_env(env)
+        current_env = ENV.to_h.dup
+        ENV.update(env)
+        yield
+      ensure
+        ENV.clear
+        ENV.update(current_env)
+      end
   end
 end

lib/kamal/cli/build.rb

@@ -30,28 +30,30 @@ class Kamal::Cli::Build < Kamal::Cli::Base
       say "Building with uncommitted changes:\n #{uncommitted_changes}", :yellow
     end
 
-    run_locally do
+    with_env(KAMAL.config.builder.secrets) do
-      begin
+      run_locally do
-        execute *KAMAL.builder.inspect_builder
+        begin
-      rescue SSHKit::Command::Failed => e
+          execute *KAMAL.builder.inspect_builder
-        if e.message =~ /(context not found|no builder|no compatible builder|does not exist)/
+        rescue SSHKit::Command::Failed => e
-          warn "Missing compatible builder, so creating a new one first"
+          if e.message =~ /(context not found|no builder|no compatible builder|does not exist)/
-          begin
+            warn "Missing compatible builder, so creating a new one first"
-            cli.remove
+            begin
-          rescue SSHKit::Command::Failed
+              cli.remove
-            raise unless e.message =~ /(context not found|no builder|does not exist)/
+            rescue SSHKit::Command::Failed
+              raise unless e.message =~ /(context not found|no builder|does not exist)/
+            end
+            cli.create
+          else
+            raise
           end
-          cli.create
-        else
-          raise
         end
-      end
 
-      # Get the command here to ensure the Dir.chdir doesn't interfere with it
+        # Get the command here to ensure the Dir.chdir doesn't interfere with it
-      push = KAMAL.builder.push
+        push = KAMAL.builder.push
 
-      KAMAL.with_verbosity(:debug) do
+        KAMAL.with_verbosity(:debug) do
-        Dir.chdir(KAMAL.config.builder.build_directory) { execute *push, env: KAMAL.config.builder.secrets }
+          Dir.chdir(KAMAL.config.builder.build_directory) { execute *push }
+        end
       end
     end
   end

lib/kamal/cli/main.rb

@@ -48,7 +48,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base
       end
     end
 
-    run_hook "post-deploy", secrets: true, runtime: runtime.round
+    run_hook "post-deploy", secrets: true, runtime: runtime.round.to_s
   end
 
   desc "redeploy", "Deploy app to servers without bootstrapping servers, starting kamal-proxy, pruning, and registry login"
@@ -75,7 +75,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base
       end
     end
 
-    run_hook "post-deploy", secrets: true, runtime: runtime.round
+    run_hook "post-deploy", secrets: true, runtime: runtime.round.to_s
   end
 
   desc "rollback [VERSION]", "Rollback app to VERSION"
@@ -99,7 +99,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base
       end
     end
 
-    run_hook "post-deploy", secrets: true, runtime: runtime.round if rolled_back
+    run_hook "post-deploy", secrets: true, runtime: runtime.round.to_s if rolled_back
   end
 
   desc "details", "Show details about all containers"

lib/kamal/commands/hook.rb

@@ -1,9 +1,12 @@
 class Kamal::Commands::Hook < Kamal::Commands::Base
-  def run(hook, secrets: false, **details)
+  def run(hook)
-    env = tags(**details).env
+    [ hook_file(hook) ]
-    env.merge!(config.secrets.to_h) if secrets
+  end
 
-    [ hook_file(hook), env: env ]
+  def env(secrets: false, **details)
+    tags(**details).env.tap do |env|
+      env.merge!(config.secrets.to_h) if secrets
+    end
   end
 
   def hook_exists?(hook)

test/cli/build_test.rb

@@ -49,7 +49,7 @@ class CliBuildTest < CliTestCase
       SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :submodule, :update, "--init")
 
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
-        .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".", env: {})
+        .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".")
 
       SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
         .with(:git, "-C", anything, :"rev-parse", :HEAD)
@@ -140,7 +140,7 @@ class CliBuildTest < CliTestCase
         .returns("")
 
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
-        .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".", env: {})
+        .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".")
 
       run_command("push").tap do |output|
         assert_match /WARN Missing compatible builder, so creating a new one first/, output

test/cli/cli_test_case.rb

@@ -41,27 +41,7 @@ class CliTestCase < ActiveSupport::TestCase
     end
 
     def assert_hook_ran(hook, output, version:, service_version:, hosts:, command:, subcommand: nil, runtime: false, secrets: false)
-      whoami = `whoami`.chomp
+      assert_match %r{usr/bin/env\s\.kamal/hooks/#{hook}}, output
-      performer = Kamal::Git.email.presence || whoami
-      service = service_version.split("@").first
-
-      assert_match "Running the #{hook} hook...\n", output
-
-      expected = %r{Running\s/usr/bin/env\s\.kamal/hooks/#{hook}\sas\s#{whoami}@localhost\n\s
-        DEBUG\s\[[0-9a-f]*\]\sCommand:\s\(\sexport\s
-        KAMAL_RECORDED_AT=\"\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\dZ\"\s
-        KAMAL_PERFORMER=\"#{performer}\"\s
-        KAMAL_VERSION=\"#{version}\"\s
-        KAMAL_SERVICE_VERSION=\"#{service_version}\"\s
-        KAMAL_SERVICE=\"#{service}\"\s
-        KAMAL_HOSTS=\"#{hosts}\"\s
-        KAMAL_COMMAND=\"#{command}\"\s
-        #{"KAMAL_SUBCOMMAND=\\\"#{subcommand}\\\"\\s" if subcommand}
-        #{"KAMAL_RUNTIME=\\\"\\d+\\\"\\s" if runtime}
-        #{"DB_PASSWORD=\"secret\"\\s" if secrets}
-        ;\s/usr/bin/env\s\.kamal/hooks/#{hook} }x
-
-      assert_match expected, output
     end
 
     def with_argv(*argv)

test/commands/hook_test.rb

@@ -16,41 +16,34 @@ class CommandsHookTest < ActiveSupport::TestCase
   end
 
   test "run" do
-    assert_equal [
+    assert_equal [ ".kamal/hooks/foo" ], new_command.run("foo")
-      ".kamal/hooks/foo",
+  end
-      { env: {
+
-        "KAMAL_RECORDED_AT" => @recorded_at,
+  test "env" do
-        "KAMAL_PERFORMER" => @performer,
+    assert_equal ({
-        "KAMAL_VERSION" => "123",
+      "KAMAL_RECORDED_AT" => @recorded_at,
-        "KAMAL_SERVICE_VERSION" => "app@123",
+      "KAMAL_PERFORMER" => @performer,
-        "KAMAL_SERVICE" => "app" } }
+      "KAMAL_VERSION" => "123",
-    ], new_command.run("foo")
+      "KAMAL_SERVICE_VERSION" => "app@123",
+      "KAMAL_SERVICE" => "app"
+    }), new_command.env
   end
 
   test "run with custom hooks_path" do
-    assert_equal [
+    assert_equal [ "custom/hooks/path/foo" ], new_command(hooks_path: "custom/hooks/path").run("foo")
-      "custom/hooks/path/foo",
-      { env: {
-        "KAMAL_RECORDED_AT" => @recorded_at,
-        "KAMAL_PERFORMER" => @performer,
-        "KAMAL_VERSION" => "123",
-        "KAMAL_SERVICE_VERSION" => "app@123",
-        "KAMAL_SERVICE" => "app" } }
-    ], new_command(hooks_path: "custom/hooks/path").run("foo")
   end
 
-  test "hook with secrets" do
+  test "env with secrets" do
     with_test_secrets("secrets" => "DB_PASSWORD=secret") do
-      assert_equal [
+      assert_equal (
-        ".kamal/hooks/foo",
+        {
-        { env: {
           "KAMAL_RECORDED_AT" => @recorded_at,
           "KAMAL_PERFORMER" => @performer,
           "KAMAL_VERSION" => "123",
           "KAMAL_SERVICE_VERSION" => "app@123",
           "KAMAL_SERVICE" => "app",
-          "DB_PASSWORD" => "secret" } }
+          "DB_PASSWORD" => "secret" }
-      ], new_command(env: { "secret" => [ "DB_PASSWORD" ] }).run("foo", secrets: true)
+      ), new_command.env(secrets: true)
     end
   end