kamal-*: run external commands from .kamal/bin and the PATH, after aliases

lib/kamal/cli/alias/command.rb

@@ -1,10 +1,27 @@
 class Kamal::Cli::Alias::Command < Thor::DynamicCommand
+  # Thor only gets here when no built-in command matches. An alias in the config
+  # comes next, then an external command, then Thor's error.
   def run(instance, args = [])
-    if (command = KAMAL.resolve_alias(name))
+    external = Kamal::Cli::Main.external_command(name)
+
+    if (command = configured_alias(external))
       KAMAL.reset
       Kamal::Cli::Main.start(Shellwords.split(command) + ARGV[1..-1])
+    elsif external
+      exec [ external, external ], *given_args(instance)
     else
       super
     end
   end
+
+  private
+    # There's no alias without a config file, and an external command doesn't need one
+    def configured_alias(external)
+      KAMAL.resolve_alias(name) unless external && !KAMAL.config_file?
+    end
+
+    # The arguments after the command name, as given, options included
+    def given_args(instance)
+      instance.instance_variable_get("@_initializer")[1]
+    end
 end

lib/kamal/cli/base.rb

@@ -45,6 +45,10 @@ def initialize(args = [], local_options = {}, config = {})
     end
 
     private
+      def dynamic_command?
+        @_initializer.last[:current_command].is_a?(Kamal::Cli::Alias::Command)
+      end
+
       def options_with_subcommand_class_options
         options.merge(@_initializer.last[:class_options] || {})
       end
@@ -65,9 +69,13 @@ def initialize_commander
             destination: options[:destination],
             version: options[:version]
 
-          commander.specific_hosts    = options[:hosts]&.split(",")
-          commander.specific_roles    = options[:roles]&.split(",")
-          commander.specific_primary! if options[:primary]
+          # A command Thor doesn't know is looked up as an alias, which starts over with
+          # its own selectors, or run as an external command, so don't load the config for them
+          unless dynamic_command?
+            commander.specific_hosts    = options[:hosts]&.split(",")
+            commander.specific_roles    = options[:roles]&.split(",")
+            commander.specific_primary! if options[:primary]
+          end
 
           commander.lock_wait          = options[:lock_wait]
           commander.lock_wait_timeout  = options[:lock_wait_timeout]

lib/kamal/cli/main.rb

@@ -1,4 +1,16 @@
 class Kamal::Cli::Main < Kamal::Cli::Base
+  # The executable to run for a command that is neither built in nor an alias in
+  # the config: .kamal/bin/<name>, or else kamal-<name> on the PATH. An empty PATH entry
+  # isn't taken for the current directory: .kamal/bin is the place for a project's own.
+  def self.external_command(name)
+    if name.present? && !name.start_with?("-") && !name.include?("/")
+      path_dirs = ENV["PATH"].to_s.split(File::PATH_SEPARATOR).reject(&:empty?)
+
+      [ File.join(".kamal", "bin", name), *path_dirs.map { |dir| File.join(dir, "kamal-#{name}") } ]
+        .find { |path| File.file?(path) && File.executable?(path) }
+    end
+  end
+
   desc "setup", "Setup all accessories, push the env, and deploy app to servers"
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   option :no_cache, type: :boolean, default: false, desc: "Build without using Docker's build cache"

lib/kamal/commander.rb

@@ -43,6 +43,10 @@ def configured?
     @config || @config_kwargs
   end
 
+  def config_file?
+    @config.present? || @config_kwargs[:config_file].exist?
+  end
+
   def specific_primary!
     @specifics = nil
     if specific_roles.present?
@@ -133,6 +137,8 @@ def resolve_alias(name)
     if @config
       @config.aliases[name]&.command
     else
+      # Rendered with its destination in the environment, as loading the config would
+      ENV["KAMAL_DESTINATION"] = @config_kwargs[:destination]
       raw_config = Kamal::Configuration.load_raw_config(**@config_kwargs.to_h.slice(:config_file, :destination))
       raw_config[:aliases]&.dig(name)
     end

lib/kamal/configuration/docs/alias.yml

@@ -27,3 +27,9 @@ aliases:
 #
 # Aliases can include a destination with the `-d` flag:
   staging_deploy: deploy -d staging
+
+# Aliases and external commands
+#
+# A command that isn't built in runs an alias of that name, if there is one, and
+# otherwise an external command: an executable `.kamal/bin/<name>`, or `kamal-<name>`
+# on the PATH. Built-in commands can't be overridden by either.

test/cli/main_test.rb

@@ -819,6 +819,190 @@ class CliMainTest < CliTestCase
     end
   end
 
+  test "external command from .kamal/bin" do
+    in_external_command_dir do
+      write_executable ".kamal/bin/foo"
+
+      expects_external_command ".kamal/bin/foo"
+      with_argv([ "foo" ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "external command from PATH" do
+    in_external_command_dir do |dir|
+      write_executable "#{dir}/bin/kamal-bar"
+      prepend_path "#{dir}/bin"
+
+      expects_external_command "#{dir}/bin/kamal-bar"
+      with_argv([ "bar" ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "external command from a PATH directory with a space runs without a shell" do
+    in_external_command_dir do |dir|
+      write_executable "#{dir}/my tools/kamal-bar"
+      prepend_path "#{dir}/my tools"
+
+      # A lone string argument to exec is split on spaces or run by a shell
+      expects_external_command "#{dir}/my tools/kamal-bar"
+      with_argv([ "bar" ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "external command in .kamal/bin takes priority over PATH" do
+    in_external_command_dir do |dir|
+      write_executable ".kamal/bin/baz"
+      write_executable "#{dir}/bin/kamal-baz"
+      prepend_path "#{dir}/bin"
+
+      expects_external_command ".kamal/bin/baz"
+      with_argv([ "baz" ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "external command gets the arguments as given, options included" do
+    in_external_command_dir do
+      write_executable ".kamal/bin/foo"
+
+      expects_external_command ".kamal/bin/foo", "--bar", "baz", "-v"
+      with_argv([ "foo", "--bar", "baz", "-v" ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "external command gets selectors without Kamal applying them" do
+    config_file = File.expand_path("test/fixtures/deploy_with_aliases.yml")
+
+    in_external_command_dir do
+      write_executable ".kamal/bin/foo"
+
+      # -h with no value would otherwise be --hosts=hosts, and fail to match a host
+      expects_external_command ".kamal/bin/foo", "-h", "-c", config_file
+      with_argv([ "foo", "-h", "-c", config_file ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "built-in commands win over external commands, without loading the config" do
+    in_external_command_dir do |dir|
+      write_executable ".kamal/bin/version"
+      write_executable "#{dir}/bin/kamal-version"
+      prepend_path "#{dir}/bin"
+
+      Kamal::Cli::Alias::Command.any_instance.expects(:exec).never
+      Kamal::Configuration.expects(:load_raw_config).never
+      Kamal::Configuration.expects(:create_from).never
+
+      output = stdouted { with_argv([ "version" ]) { Kamal::Cli::Main.start } }
+      assert_equal Kamal::VERSION, output.strip
+    end
+  end
+
+  test "built-in prefix matches win over external commands" do
+    in_external_command_dir do
+      write_executable ".kamal/bin/ver"
+
+      Kamal::Cli::Alias::Command.any_instance.expects(:exec).never
+      Kamal::Configuration.expects(:load_raw_config).never
+
+      output = stdouted { with_argv([ "ver" ]) { Kamal::Cli::Main.start } }
+      assert_equal Kamal::VERSION, output.strip
+    end
+  end
+
+  test "aliases win over external commands of the same name" do
+    config_file = File.expand_path("test/fixtures/deploy_with_aliases.yml")
+
+    in_external_command_dir do |dir|
+      write_executable ".kamal/bin/console"
+      write_executable "#{dir}/bin/kamal-console"
+      prepend_path "#{dir}/bin"
+
+      Kamal::Cli::Alias::Command.any_instance.expects(:exec).never
+
+      output = stdouted { with_argv([ "console", "-c", config_file ]) { Kamal::Cli::Main.start } }
+      assert_match "docker exec app-console-999 bin/console on 1.1.1.5", output
+    end
+  end
+
+  test "external command runs when the config has no alias of that name" do
+    config_file = File.expand_path("test/fixtures/deploy_with_aliases.yml")
+
+    in_external_command_dir do
+      write_executable ".kamal/bin/foo"
+
+      expects_external_command ".kamal/bin/foo", "-c", config_file
+      with_argv([ "foo", "-c", config_file ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "alias can expand to an external command" do
+    config = File.read("test/fixtures/deploy_simple.yml") + "aliases:\n  foo_list: foo list\n"
+
+    in_external_command_dir do
+      write_executable ".kamal/bin/foo"
+      FileUtils.mkdir_p("config")
+      File.write("config/deploy.yml", config)
+
+      expects_external_command ".kamal/bin/foo", "list", "--all"
+      with_argv([ "foo_list", "--all" ]) { Kamal::Cli::Main.start }
+    end
+  end
+
+  test "alias lookup renders the config with its destination in the environment, selectors or not" do
+    config = File.read("test/fixtures/deploy_simple.yml") + <<~YAML
+      aliases:
+        info: <%= ENV.fetch("KAMAL_DESTINATION") == "world" ? "version" : "details" %>
+    YAML
+
+    in_external_command_dir do
+      FileUtils.mkdir_p("config")
+      File.write("config/deploy.yml", config)
+      File.write("config/deploy.world.yml", "env:\n  clear:\n    WORLD: 1\n")
+
+      [ [], [ "--primary" ] ].each do |selector|
+        ENV.delete("KAMAL_DESTINATION")
+        KAMAL.reset
+
+        output = stdouted { with_argv([ "info", "-d", "world", *selector ]) { Kamal::Cli::Main.start } }
+        assert_equal Kamal::VERSION, output.strip, "with #{selector.inspect}"
+      end
+    end
+  end
+
+  test "non-executable file in .kamal/bin is skipped" do
+    in_external_command_dir do
+      write_executable ".kamal/bin/foo", mode: 0644
+
+      assert_no_external_command "foo"
+    end
+  end
+
+  test "directory in .kamal/bin is skipped" do
+    in_external_command_dir do
+      FileUtils.mkdir_p(".kamal/bin/foo")
+
+      assert_no_external_command "foo"
+    end
+  end
+
+  test "directory on PATH is skipped" do
+    in_external_command_dir do |dir|
+      FileUtils.mkdir_p("#{dir}/bin/kamal-foo")
+      prepend_path "#{dir}/bin"
+
+      assert_no_external_command "foo"
+    end
+  end
+
+  test "path traversal in command name is rejected" do
+    in_external_command_dir do |dir|
+      write_executable "#{dir}/evil"
+      FileUtils.mkdir_p(".kamal/bin")
+      File.symlink("#{dir}/evil", ".kamal/bin/../foo")
+
+      assert_no_external_command "../foo"
+    end
+  end
+
   test "upgrade rolling" do
     invoke_options = base_invoke_options(config_file: "deploy_with_accessories.yml", version: nil, confirmed: true, rolling: false)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:upgrade", [], invoke_options).times(4)
@@ -837,6 +1021,36 @@ class CliMainTest < CliTestCase
   end
 
   private
+    def in_external_command_dir
+      Dir.mktmpdir do |tmpdir|
+        Dir.chdir(tmpdir) { yield tmpdir }
+      end
+    end
+
+    def write_executable(path, mode: 0755)
+      FileUtils.mkdir_p(File.dirname(path))
+      File.write(path, "#!/bin/sh\n")
+      File.chmod(mode, path)
+    end
+
+    # The PATH is restored with the rest of the environment on teardown
+    def prepend_path(dir)
+      ENV["PATH"] = [ dir, ENV["PATH"] ].join(File::PATH_SEPARATOR)
+    end
+
+    def expects_external_command(path, *args)
+      Kamal::Cli::Alias::Command.any_instance.expects(:exec).with([ path, path ], *args)
+    end
+
+    def assert_no_external_command(name)
+      Kamal::Cli::Alias::Command.any_instance.expects(:exec).never
+
+      with_argv([ name ]) do
+        error = assert_raises(RuntimeError) { Kamal::Cli::Main.start }
+        assert_match /Configuration file not found/, error.message
+      end
+    end
+
     def run_command(*command, config_file: "deploy_simple")
       with_argv([ *command, "-c", "test/fixtures/#{config_file}.yml" ]) do
         stdouted { Kamal::Cli::Main.start }