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`.

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"

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/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
 

test/secrets/bitwarden_adapter_test.rb

@@ -6,7 +6,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" }
 
@@ -18,7 +18,7 @@ class BitwardenAdapterTest < SecretAdapterTestCase
     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"
@@ -59,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"
@@ -82,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" }
 
@@ -107,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" }
 
@@ -132,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" }
 

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