Escape secrets in inline command substitution

Kamal "inlines" calls to `kamal secrets` in the dotenv file, but the
results of the calls were not being escaped properly. To "fix" this
`kamal secrets fetch` escaped the JSON string before returning it.

The two errors cancelled out, but it meant that the commands didn't
work from a shell.

To fix, we'll escape the inline command results and remove the escaping
from `kamal secrets fetch`.

Dockerfile

@@ -33,7 +33,7 @@ WORKDIR /workdir
 
 # Tell git it's safe to access /workdir/.git even if
 # the directory is owned by a different user
-RUN git config --global --add safe.directory '*'
+RUN git config --global --add safe.directory /workdir
 
 # Set the entrypoint to run the installed binary in /workdir
 # Example:  docker run -it -v "$PWD:/workdir" kamal init

lib/kamal/cli/app.rb

@@ -68,7 +68,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
             version = capture_with_info(*app.current_running_version, raise_on_non_zero_exit: false).strip
             endpoint = capture_with_info(*app.container_id_for_version(version)).strip
             if endpoint.present?
-              execute *app.remove, raise_on_non_zero_exit: false
+              execute *app.remove(target: endpoint), raise_on_non_zero_exit: false
             end
           end
 

lib/kamal/cli/secrets.rb

@@ -7,7 +7,7 @@ class Kamal::Cli::Secrets < Kamal::Cli::Base
   def fetch(*secrets)
     results = adapter(options[:adapter]).fetch(secrets, **options.slice(:account, :from).symbolize_keys)
 
-    return_or_puts JSON.dump(results).shellescape, inline: options[:inline]
+    return_or_puts JSON.dump(results), inline: options[:inline]
   end
 
   desc "extract", "Extract a single secret from the results of a fetch call"
@@ -21,13 +21,6 @@ class Kamal::Cli::Secrets < Kamal::Cli::Base
     return_or_puts value, inline: options[:inline]
   end
 
-  desc "print", "Print the secrets (for debugging)"
-  def print
-    KAMAL.config.secrets.to_h.each do |key, value|
-      puts "#{key}=#{value}"
-    end
-  end
-
   private
     def adapter(adapter)
       Kamal::Secrets::Adapters.lookup(adapter)

lib/kamal/cli/templates/deploy.yml

@@ -18,8 +18,6 @@ servers:
 proxy:
   ssl: true
   host: app.example.com
-  # kamal-proxy connects to your container over port 80, use `app_port` to specify a different port.
-  # app_port: 3000
 
 # Credentials for your image host.
 registry:

lib/kamal/commands/app/proxy.rb

@@ -5,8 +5,8 @@ module Kamal::Commands::App::Proxy
     proxy_exec :deploy, role.container_prefix, *role.proxy.deploy_command_args(target: target)
   end
 
-  def remove
-    proxy_exec :remove, role.container_prefix
+  def remove(target:)
+    proxy_exec :remove, role.container_prefix, *role.proxy.remove_command_args(target: target)
   end
 
   private

lib/kamal/configuration.rb

@@ -14,7 +14,7 @@ class Kamal::Configuration
 
   include Validation
 
-  PROXY_MINIMUM_VERSION = "v0.7.0"
+  PROXY_MINIMUM_VERSION = "v0.6.0"
   PROXY_HTTP_PORT = 80
   PROXY_HTTPS_PORT = 443
 
@@ -360,7 +360,7 @@ class Kamal::Configuration
     end
 
     def ensure_unique_hosts_for_ssl_roles
-      hosts = roles.select(&:ssl?).flat_map { |role| role.proxy.hosts }
+      hosts = roles.select(&:ssl?).map { |role| role.proxy.host }
       duplicates = hosts.tally.filter_map { |host, count| host if count > 1 }
 
       raise Kamal::ConfigurationError, "Different roles can't share the same host for SSL: #{duplicates.join(", ")}" if duplicates.any?

lib/kamal/configuration/docs/proxy.yml

@@ -17,19 +17,16 @@
 # `proxy: true` or providing a proxy configuration.
 proxy:
 
-  # Hosts
+  # Host
   #
   # The hosts that will be used to serve the app. The proxy will only route requests
   # to this host to your app.
   #
   # If no hosts are set, then all requests will be forwarded, except for matching
   # requests for other apps deployed on that server that do have a host set.
-  #
-  # Specify one of `host` or `hosts`.
   host: foo.example.com
-  hosts:
-    - foo.example.com
-    - bar.example.com
+  # If multiple hosts are needed, these can be specified by comma-separating the hosts.
+  host: foo.example.com,bar.example.com
 
   # App port
   #

lib/kamal/configuration/proxy.rb

@@ -22,14 +22,14 @@ class Kamal::Configuration::Proxy
     proxy_config.fetch("ssl", false)
   end
 
-  def hosts
-    proxy_config["hosts"] || proxy_config["host"]&.split(",") || []
+  def host
+    proxy_config["host"]
   end
 
   def deploy_options
     {
-      host: hosts,
-      tls: proxy_config["ssl"],
+      host: proxy_config["host"],
+      tls: proxy_config["ssl"] ? true : nil,
       "deploy-timeout": seconds_duration(config.deploy_timeout),
       "drain-timeout": seconds_duration(config.drain_timeout),
       "health-check-interval": seconds_duration(proxy_config.dig("healthcheck", "interval")),
@@ -48,7 +48,11 @@ class Kamal::Configuration::Proxy
   end
 
   def deploy_command_args(target:)
-    optionize ({ target: "#{target}:#{app_port}" }).merge(deploy_options), with: "="
+    optionize ({ target: "#{target}:#{app_port}" }).merge(deploy_options)
+  end
+
+  def remove_command_args(target:)
+    optionize({ target: "#{target}:#{app_port}" })
   end
 
   def merge(other)

lib/kamal/configuration/validator/proxy.rb

@@ -3,13 +3,9 @@ class Kamal::Configuration::Validator::Proxy < Kamal::Configuration::Validator
     unless config.nil?
       super
 
-      if config["host"].blank? && config["hosts"].blank? && config["ssl"]
+      if config["host"].blank? && config["ssl"]
         error "Must set a host to enable automatic SSL"
       end
-
-      if (config.keys & [ "host", "hosts" ]).size > 1
-        error "Specify one of 'host' or 'hosts', not both"
-      end
     end
   end
 end

lib/kamal/secrets/adapters/bitwarden.rb

@@ -35,10 +35,8 @@ class Kamal::Secrets::Adapters::Bitwarden < Kamal::Secrets::Adapters::Base
               value = item_field["value"]
               results["#{item}/#{field}"] = value
             end
-          elsif item_json.dig("login", "password")
-            results[item] = item_json.dig("login", "password")
           else
-            raise RuntimeError, "Item #{item} is not a login type item and no fields were specified"
+            results[item] = item_json["login"]["password"]
           end
         end
       end

lib/kamal/secrets/dotenv/inline_command_substitution.rb

@@ -16,7 +16,7 @@ class Kamal::Secrets::Dotenv::InlineCommandSubstitution
         else
           if command =~ /\A\s*kamal\s*secrets\s+/
             # Inline the command
-            inline_secrets_command(command)
+            inline_secrets_command(command).shellescape
           else
             # Execute the command and return the value
             `#{command}`.chomp

test/cli/app_test.rb

@@ -130,7 +130,7 @@ class CliAppTest < CliTestCase
     SSHKit::Backend::Abstract.any_instance.stubs(:execute)
       .with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet", "|", :xargs, :docker, :stop, raise_on_non_zero_exit: false)
     SSHKit::Backend::Abstract.any_instance.expects(:execute)
-      .with(:docker, :exec, "kamal-proxy", "kamal-proxy", :deploy, "app-web", "--target=\"123:80\"", "--deploy-timeout=\"1s\"", "--drain-timeout=\"30s\"", "--buffer-requests", "--buffer-responses", "--log-request-header=\"Cache-Control\"", "--log-request-header=\"Last-Modified\"", "--log-request-header=\"User-Agent\"").raises(SSHKit::Command::Failed.new("Failed to deploy"))
+      .with(:docker, :exec, "kamal-proxy", "kamal-proxy", :deploy, "app-web", "--target", "\"123:80\"", "--deploy-timeout", "\"1s\"", "--drain-timeout", "\"30s\"", "--buffer-requests", "--buffer-responses", "--log-request-header", "\"Cache-Control\"", "--log-request-header", "\"Last-Modified\"", "--log-request-header", "\"User-Agent\"").raises(SSHKit::Command::Failed.new("Failed to deploy"))
 
     stderred do
       run_command("boot", config: :with_roles, host: nil, allow_execute_error: true).tap do |output|
@@ -190,7 +190,7 @@ class CliAppTest < CliTestCase
 
     run_command("start").tap do |output|
       assert_match "docker start app-web-999", output
-      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"999:80\" --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\"", output
+      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target \"999:80\" --deploy-timeout \"30s\" --drain-timeout \"30s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\"", output
     end
   end
 
@@ -383,7 +383,7 @@ class CliAppTest < CliTestCase
       assert_match /Renaming container .* to .* as already deployed on 1.1.1.1/, output # Rename
       assert_match /docker rename app-web-latest app-web-latest_replaced_[0-9a-f]{16}/, output
       assert_match /docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} -e KAMAL_CONTAINER_NAME="app-web-latest" -e KAMAL_VERSION="latest" --env-file .kamal\/apps\/app\/env\/roles\/web.env --log-opt max-size="10m" --label service="app" --label role="web" --label destination dhh\/app:latest/, output
-      assert_match /docker exec kamal-proxy kamal-proxy deploy app-web --target="123:80"/, output
+      assert_match /docker exec kamal-proxy kamal-proxy deploy app-web --target "123:80"/, output
       assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
     end
   end
@@ -392,8 +392,8 @@ class CliAppTest < CliTestCase
     SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info).returns("123") # old version
 
     run_command("boot", config: :with_proxy_roles, host: nil).tap do |output|
-      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"123:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --target-timeout=\"10s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"", output
-      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web2 --target=\"123:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --target-timeout=\"15s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"", output
+      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target \"123:80\" --deploy-timeout \"6s\" --drain-timeout \"30s\" --target-timeout \"10s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\" --log-request-header \"User-Agent\"", output
+      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web2 --target \"123:80\" --deploy-timeout \"6s\" --drain-timeout \"30s\" --target-timeout \"15s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\" --log-request-header \"User-Agent\"", output
     end
   end
 

test/cli/proxy_test.rb

@@ -58,13 +58,13 @@ class CliProxyTest < CliTestCase
       assert_match "Running docker container stop traefik ; docker container prune --force --filter label=org.opencontainers.image.title=Traefik && docker image prune --all --force --filter label=org.opencontainers.image.title=Traefik on 1.1.1.1", output
       assert_match "docker container prune --force --filter label=org.opencontainers.image.title=kamal-proxy on 1.1.1.1", output
       assert_match "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443\") #{KAMAL.config.proxy_image} on 1.1.1.1", output
-      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"abcdefabcdef:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\" on 1.1.1.1", output
+      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target \"abcdefabcdef:80\" --deploy-timeout \"6s\" --drain-timeout \"30s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\" --log-request-header \"User-Agent\" on 1.1.1.1", output
 
       assert_match "docker container stop kamal-proxy on 1.1.1.2", output
       assert_match "Running docker container stop traefik ; docker container prune --force --filter label=org.opencontainers.image.title=Traefik && docker image prune --all --force --filter label=org.opencontainers.image.title=Traefik on 1.1.1.2", output
       assert_match "docker container prune --force --filter label=org.opencontainers.image.title=kamal-proxy on 1.1.1.2", output
       assert_match "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443\") #{KAMAL.config.proxy_image} on 1.1.1.2", output
-      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"abcdefabcdef:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\" on 1.1.1.2", output
+      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target \"abcdefabcdef:80\" --deploy-timeout \"6s\" --drain-timeout \"30s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\" --log-request-header \"User-Agent\" on 1.1.1.2", output
     end
   end
 
@@ -204,7 +204,7 @@ class CliProxyTest < CliTestCase
       assert_match "/usr/bin/env mkdir -p .kamal/apps/app/env/roles", output
       assert_match "Uploading \"\\n\" to .kamal/apps/app/env/roles/web.env", output
       assert_match %r{docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-.* -e KAMAL_CONTAINER_NAME="app-web-latest" -e KAMAL_VERSION="latest" --env-file .kamal/apps/app/env/roles/web.env --log-opt max-size="10m" --label service="app" --label role="web" --label destination dhh/app:latest}, output
-      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"12345678:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"", output
+      assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target \"12345678:80\" --deploy-timeout \"6s\" --drain-timeout \"30s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\" --log-request-header \"User-Agent\"", output
       assert_match "docker container ls --all --filter name=^app-web-12345678$ --quiet | xargs docker stop", output
       assert_match "docker tag dhh/app:latest dhh/app:latest", output
       assert_match "/usr/bin/env mkdir -p .kamal", output

test/cli/secrets_test.rb

@@ -3,7 +3,7 @@ require_relative "cli_test_case"
 class CliSecretsTest < CliTestCase
   test "fetch" do
     assert_equal \
-      "\\{\\\"foo\\\":\\\"oof\\\",\\\"bar\\\":\\\"rab\\\",\\\"baz\\\":\\\"zab\\\"\\}",
+      "{\"foo\":\"oof\",\"bar\":\"rab\",\"baz\":\"zab\"}",
       run_command("fetch", "foo", "bar", "baz", "--account", "myaccount", "--adapter", "test")
   end
 
@@ -15,12 +15,6 @@ class CliSecretsTest < CliTestCase
     assert_equal "oof", run_command("extract", "foo", "{\"abc/foo\":\"oof\", \"bar\":\"rab\", \"baz\":\"zab\"}")
   end
 
-  test "print" do
-    with_test_secrets("secrets" => "SECRET1=ABC\nSECRET2=${SECRET1}DEF\n") do
-      assert_equal "SECRET1=ABC\nSECRET2=ABCDEF", run_command("print")
-    end
-  end
-
   private
     def run_command(*command)
       stdouted { Kamal::Cli::Secrets.start([ *command, "-c", "test/fixtures/deploy_with_accessories.yml" ]) }

test/commands/app_test.rb

@@ -115,30 +115,14 @@ class CommandsAppTest < ActiveSupport::TestCase
 
   test "deploy" do
     assert_equal \
-      "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"172.1.0.2:80\" --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
-      new_command.deploy(target: "172.1.0.2").join(" ")
-  end
-
-  test "deploy with SSL" do
-    @config[:proxy] = { "ssl" => true, "host" => "example.com" }
-
-    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 --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
-      new_command.deploy(target: "172.1.0.2").join(" ")
-  end
-
-  test "deploy with SSL targeting multiple hosts" do
-    @config[:proxy] = { "ssl" => true, "hosts" => [ "example.com", "anotherexample.com" ] }
-
-    assert_equal \
-      "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"172.1.0.2:80\" --host=\"example.com\" --host=\"anotherexample.com\" --tls --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
+      "docker exec kamal-proxy kamal-proxy deploy app-web --target \"172.1.0.2:80\" --deploy-timeout \"30s\" --drain-timeout \"30s\" --buffer-requests --buffer-responses --log-request-header \"Cache-Control\" --log-request-header \"Last-Modified\" --log-request-header \"User-Agent\"",
       new_command.deploy(target: "172.1.0.2").join(" ")
   end
 
   test "remove" do
     assert_equal \
-      "docker exec kamal-proxy kamal-proxy remove app-web",
-      new_command.remove.join(" ")
+      "docker exec kamal-proxy kamal-proxy remove app-web --target \"172.1.0.2:80\"",
+      new_command.remove(target: "172.1.0.2").join(" ")
   end
 
 

test/configuration/proxy_test.rb

@@ -13,29 +13,15 @@ class ConfigurationProxyTest < ActiveSupport::TestCase
     assert_equal true, config.proxy.ssl?
   end
 
-  test "ssl with multiple hosts passed via host" do
-    @deploy[:proxy] = { "ssl" => true, "host" => "example.com,anotherexample.com" }
-    assert_equal true, config.proxy.ssl?
-  end
-
-  test "ssl with multiple hosts passed via hosts" do
-    @deploy[:proxy] = { "ssl" => true, "hosts" => [ "example.com", "anotherexample.com" ] }
-    assert_equal true, config.proxy.ssl?
-  end
-
   test "ssl with no host" do
     @deploy[:proxy] = { "ssl" => true }
     assert_raises(Kamal::ConfigurationError) { config.proxy.ssl? }
   end
 
-  test "ssl with both host and hosts" do
-    @deploy[:proxy] = { "ssl" => true, host: "example.com", hosts: [ "anotherexample.com" ] }
-    assert_raises(Kamal::ConfigurationError) { config.proxy.ssl? }
-  end
-
   test "ssl false" do
     @deploy[:proxy] = { "ssl" => false }
     assert_not config.proxy.ssl?
+    assert_not config.proxy.deploy_options.has_key?(:tls)
   end
 
   private

test/configuration_test.rb

@@ -377,15 +377,4 @@ class ConfigurationTest < ActiveSupport::TestCase
 
     assert_equal "Different roles can't share the same host for SSL: foo.example.com", exception.message
   end
-
-  test "two proxy ssl roles with same host in a hosts array" do
-    @deploy_with_roles[:servers]["web"] = { "hosts" => [ "1.1.1.1" ], "proxy" => { "ssl" => true, "hosts" => [ "foo.example.com", "bar.example.com" ] } }
-    @deploy_with_roles[:servers]["workers"] = { "hosts" => [ "1.1.1.1" ], "proxy" => { "ssl" => true, "hosts" => [ "www.example.com", "foo.example.com" ] } }
-
-    exception = assert_raises(Kamal::ConfigurationError) do
-      Kamal::Configuration.new(@deploy_with_roles)
-    end
-
-    assert_equal "Different roles can't share the same host for SSL: foo.example.com", exception.message
-  end
 end

test/integration/app_test.rb

@@ -8,7 +8,7 @@ class AppTest < IntegrationTest
 
     kamal :app, :stop
 
-    assert_app_not_found
+    assert_app_is_down
 
     kamal :app, :start
 
@@ -48,7 +48,7 @@ class AppTest < IntegrationTest
 
     kamal :app, :remove
 
-    assert_app_not_found
+    assert_app_is_down
     assert_app_directory_removed
   end
 end

test/integration/docker/deployer/app_with_roles/config/deploy.yml

@@ -15,15 +15,14 @@ readiness_delay: 0
 
 proxy:
   host: localhost
-  ssl: false
   healthcheck:
     interval: 1
     timeout: 1
     path: "/up"
   response_timeout: 2
   buffering:
-    requests: false
-    responses: false
+    requests: true
+    responses: true
     memory: 400_000
     max_request_body: 40_000_000
     max_response_body: 40_000_000

test/integration/integration_test.rb

@@ -50,12 +50,6 @@ class IntegrationTest < ActiveSupport::TestCase
       assert_equal "502", response.code
     end
 
-    def assert_app_not_found
-      response = app_response
-      debug_response_code(response, "404")
-      assert_equal "404", response.code
-    end
-
     def assert_app_is_up(version: nil, app: @app)
       response = app_response(app: app)
       debug_response_code(response, "200")

test/secrets/bitwarden_adapter_test.rb

@@ -6,30 +6,19 @@ class BitwardenAdapterTest < SecretAdapterTestCase
     stub_ticks.with("bw sync").returns("")
     stub_mypassword
 
-    json = JSON.parse(shellunescape(run_command("fetch", "mypassword")))
+    json = JSON.parse(run_command("fetch", "mypassword"))
 
     expected_json = { "mypassword"=>"secret123" }
 
     assert_equal expected_json, json
   end
 
-  test "fetch with no login" do
-    stub_unlocked
-    stub_ticks.with("bw sync").returns("")
-    stub_noteitem
-
-    error = assert_raises RuntimeError do
-      JSON.parse(shellunescape(run_command("fetch", "mynote")))
-    end
-    assert_match(/not a login type item/, error.message)
-  end
-
   test "fetch with from" do
     stub_unlocked
     stub_ticks.with("bw sync").returns("")
     stub_myitem
 
-    json = JSON.parse(shellunescape(run_command("fetch", "--from", "myitem", "field1", "field2", "field3")))
+    json = JSON.parse(run_command("fetch", "--from", "myitem", "field1", "field2", "field3"))
 
     expected_json = {
       "myitem/field1"=>"secret1", "myitem/field2"=>"blam", "myitem/field3"=>"fewgrwjgk"
@@ -70,7 +59,7 @@ class BitwardenAdapterTest < SecretAdapterTestCase
     JSON
 
 
-    json = JSON.parse(shellunescape(run_command("fetch", "mypassword", "myitem/field1", "myitem/field2", "myitem2/field3")))
+    json = JSON.parse(run_command("fetch", "mypassword", "myitem/field1", "myitem/field2", "myitem2/field3"))
 
     expected_json = {
       "mypassword"=>"secret123", "myitem/field1"=>"secret1", "myitem/field2"=>"blam", "myitem2/field3"=>"fewgrwjgk"
@@ -93,7 +82,7 @@ class BitwardenAdapterTest < SecretAdapterTestCase
     stub_ticks.with("bw sync").returns("")
     stub_mypassword
 
-    json = JSON.parse(shellunescape(run_command("fetch", "mypassword")))
+    json = JSON.parse(run_command("fetch", "mypassword"))
 
     expected_json = { "mypassword"=>"secret123" }
 
@@ -118,7 +107,7 @@ class BitwardenAdapterTest < SecretAdapterTestCase
     stub_ticks.with("bw sync").returns("")
     stub_mypassword
 
-    json = JSON.parse(shellunescape(run_command("fetch", "mypassword")))
+    json = JSON.parse(run_command("fetch", "mypassword"))
 
     expected_json = { "mypassword"=>"secret123" }
 
@@ -143,7 +132,7 @@ class BitwardenAdapterTest < SecretAdapterTestCase
     stub_ticks.with("BW_SESSION=0987654321 bw sync").returns("")
     stub_mypassword(session: "0987654321")
 
-    json = JSON.parse(shellunescape(run_command("fetch", "mypassword")))
+    json = JSON.parse(run_command("fetch", "mypassword"))
 
     expected_json = { "mypassword"=>"secret123" }
 
@@ -192,30 +181,6 @@ class BitwardenAdapterTest < SecretAdapterTestCase
         JSON
     end
 
-  def stub_noteitem(session: nil)
-    stub_ticks
-      .with("#{"BW_SESSION=#{session} " if session}bw get item mynote")
-      .returns(<<~JSON)
-          {
-            "passwordHistory":null,
-            "revisionDate":"2024-09-28T09:07:27.461Z",
-            "creationDate":"2024-09-28T09:07:00.740Z",
-            "deletedDate":null,
-            "object":"item",
-            "id":"aaaaaaaa-cccc-eeee-0000-222222222222",
-            "organizationId":null,
-            "folderId":null,
-            "type":2,
-            "reprompt":0,
-            "name":"noteitem",
-            "notes":"NOTES",
-            "favorite":false,
-            "secureNote":{"type":0},
-            "collectionIds":[]
-          }
-        JSON
-  end
-
     def stub_myitem
       stub_ticks
         .with("bw get item myitem")

test/secrets/dotenv_inline_command_substitution_test.rb

@@ -12,4 +12,10 @@ class SecretsInlineCommandSubstitution < SecretAdapterTestCase
     substituted = Kamal::Secrets::Dotenv::InlineCommandSubstitution.call("FOO=$(blah)", nil, overwrite: false)
     assert_equal "FOO=results", substituted
   end
+
+  test "escapes correctly" do
+    Kamal::Cli::Main.expects(:start).with { |command| command == [ "secrets", "fetch", "...", "--inline" ] }.returns("{ \"foo\" : \"bar\" }")
+    substituted = Kamal::Secrets::Dotenv::InlineCommandSubstitution.call("SECRETS=$(kamal secrets fetch ...)", nil, overwrite: false)
+    assert_equal "SECRETS=\\{\\ \\\"foo\\\"\\ :\\ \\\"bar\\\"\\ \\}", substituted
+  end
 end

test/secrets/last_pass_adapter_test.rb

@@ -51,7 +51,7 @@ class LastPassAdapterTest < SecretAdapterTestCase
         ]
       JSON
 
-    json = JSON.parse(shellunescape(run_command("fetch", "SECRET1", "FOLDER1/FSECRET1", "FOLDER1/FSECRET2")))
+    json = JSON.parse(run_command("fetch", "SECRET1", "FOLDER1/FSECRET1", "FOLDER1/FSECRET2"))
 
     expected_json = {
       "SECRET1"=>"secret1",
@@ -96,7 +96,7 @@ class LastPassAdapterTest < SecretAdapterTestCase
         ]
       JSON
 
-    json = JSON.parse(shellunescape(run_command("fetch", "--from", "FOLDER1", "FSECRET1", "FSECRET2")))
+    json = JSON.parse(run_command("fetch", "--from", "FOLDER1", "FSECRET1", "FSECRET2"))
 
     expected_json = {
       "FOLDER1/FSECRET1"=>"fsecret1",
@@ -111,7 +111,7 @@ class LastPassAdapterTest < SecretAdapterTestCase
     stub_ticks_with("lpass login email@example.com", succeed: true).returns("")
     stub_ticks.with("lpass show SECRET1 --json").returns(single_item_json)
 
-    json = JSON.parse(shellunescape(run_command("fetch", "SECRET1")))
+    json = JSON.parse(run_command("fetch", "SECRET1"))
 
     expected_json = {
       "SECRET1"=>"secret1"

test/secrets/one_password_adapter_test.rb

@@ -44,7 +44,7 @@ class SecretsOnePasswordAdapterTest < SecretAdapterTestCase
         ]
       JSON
 
-    json = JSON.parse(shellunescape(run_command("fetch", "--from", "op://myvault/myitem", "section/SECRET1", "section/SECRET2", "section2/SECRET3")))
+    json = JSON.parse(run_command("fetch", "--from", "op://myvault/myitem", "section/SECRET1", "section/SECRET2", "section2/SECRET3"))
 
     expected_json = {
       "myvault/myitem/section/SECRET1"=>"VALUE1",
@@ -103,7 +103,7 @@ class SecretsOnePasswordAdapterTest < SecretAdapterTestCase
         }
       JSON
 
-    json = JSON.parse(shellunescape(run_command("fetch", "--from", "op://myvault", "myitem/section/SECRET1", "myitem/section/SECRET2", "myitem2/section2/SECRET3")))
+    json = JSON.parse(run_command("fetch", "--from", "op://myvault", "myitem/section/SECRET1", "myitem/section/SECRET2", "myitem2/section2/SECRET3"))
 
     expected_json = {
       "myvault/myitem/section/SECRET1"=>"VALUE1",
@@ -122,7 +122,7 @@ class SecretsOnePasswordAdapterTest < SecretAdapterTestCase
       .with("op item get myitem --vault \"myvault\" --fields \"label=section.SECRET1\" --format \"json\" --account \"myaccount\"")
       .returns(single_item_json)
 
-    json = JSON.parse(shellunescape(run_command("fetch", "--from", "op://myvault/myitem", "section/SECRET1")))
+    json = JSON.parse(run_command("fetch", "--from", "op://myvault/myitem", "section/SECRET1"))
 
     expected_json = {
       "myvault/myitem/section/SECRET1"=>"VALUE1"
@@ -139,7 +139,7 @@ class SecretsOnePasswordAdapterTest < SecretAdapterTestCase
       .with("op item get myitem --vault \"myvault\" --fields \"label=section.SECRET1\" --format \"json\" --account \"myaccount\" --session \"1234567890\"")
       .returns(single_item_json)
 
-    json = JSON.parse(shellunescape(run_command("fetch", "--from", "op://myvault/myitem", "section/SECRET1")))
+    json = JSON.parse(run_command("fetch", "--from", "op://myvault/myitem", "section/SECRET1"))
 
     expected_json = {
       "myvault/myitem/section/SECRET1"=>"VALUE1"

test/test_helper.rb

@@ -86,8 +86,4 @@ class SecretAdapterTestCase < ActiveSupport::TestCase
       stub_ticks.with { |c| c == command && (succeed ? `true` : `false`) }
       Kamal::Secrets::Adapters::Base.any_instance.stubs(:`)
     end
-
-    def shellunescape(string)
-      "\"#{string}\"".undump.gsub(/\\([{}])/, "\\1")
-    end
 end