SSHKit does not escape command arguments, resulting in failure to clone a path with spaces

[davidstosik]

I understand this might be unconventional and frowned upon in some circles, but I store my whole source code folder in iCloud and use a symlink for convenience. I don't really have to justify, but that helps me with instant backups and when working with multiple computers. (I expect "use Git" comments, and that's okay. 😬 )

$ cd ~/src/my-project && pwd -P
/Users/sto/Library/Mobile Documents/com~apple~CloudDocs/src/my-project

So far, I have had very little issues with this quirk in my setup.

Enters Kamal. Since the announcements and talks at RailsWorld 2024, I've been very excited about trying it out on some pet projects.

I've ignored the issue at first and managed to deploy a pet project to DigitalOcean when my project's path didn't include spaces, but I thought it's now time to report this issue.

When running kamal deploy in a project which path includes spaces, Kamal fails to clone the repository:

$ kamal deploy
...                                                                                                                                                       unescaped space here!
                                                                                                                                                                    ⬇️
  INFO [a007f48f] Running /usr/bin/env git -C /var/folders/1q/mcsb93312b73zl6fm0887wzc0000gn/T/kamal-clones/my-project-a97a49be96920 clone /Users/sto/Library/Mobile Documents/com~apple~CloudDocs/src/my-project --recurse-submodules as sto@localhost
  Finished all in 255.7 seconds
  ERROR (SSHKit::Command::Failed): git exit status: 32768
git stdout: Nothing written
git stderr: fatal: repository '/Users/sto/Library/Mobile' does not exist

I dug a little bit and it seems related to how SSHKit sometimes constructs commands:

• SSHKit::Backend::Local#execute_command calls Open3.popen3(cmd.to_command) here

• SSHKit::Command.to_command eventually calls #to_s and joins all items in the command array without escaping to produce a string here

• Instead, Open3.popen3 could be passed an array and it would automatically escape spaces where needed:

   require "open3"
   Open3.popen3("ls /Users/sto/Library/Mobile Documents/com~apple~CloudDocs/src/my-project")[1..2].map(&:read)
   # =>
   # ["",
   # "ls: /Users/sto/Library/Mobile: No such file or directory\nls: Documents/com~apple~CloudDocs/src/my-project: No such file or directory\n"]
  
   Open3.popen3("ls", "/Users/sto/Library/Mobile Documents/com~apple~CloudDocs/src/my-project")[1..2].map(&:read)
   # =>
   # ["Dockerfile\nGemfile\nGemfile.lock\nREADME.md\n...",
   #  ""]

• Unfortunately, one of SSHKit's maintainers stated years ago that he was not willing to address that issue by fixing the escaping:

  This is a breaking change to the behavior of SSHKit. I won't be able to accept this PR as is.
  ...
  Also, how about adding to the documentation instead of changing the behavior? For example the docs could offer advice about using .shellescape when dealing with strings that are not shell-safe.

  (No such change has been made to SSHKit's docs in the last five years.)

Here's a simple script to illustrate this:

require "sshkit"
SSHKit::Backend::Local.new.execute(:ls, "/Users/sto/Library/Mobile Documents/com~apple~CloudDocs/src/my-project")
# /Users/sto/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/gems/sshkit-1.23.1/lib/sshkit/command.rb:97:in `exit_status=': ls exit status: 256 (SSHKit::Command::Failed)
# ls stdout: Nothing written
# ls stderr: ls: /Users/sto/Library/Mobile: No such file or directory
# ls: Documents/com~apple~CloudDocs/src/my-project: No such file or directory

If this can't get fixed in SSHKit, I wonder if that's something that needs to get fixed in Kamal. 🤔

I tried "fixing" this problem in Kamal like this:

diff --git c/lib/kamal/commands/builder/clone.rb i/lib/kamal/commands/builder/clone.rb
index 17d9c93..466b523 100644
--- c/lib/kamal/commands/builder/clone.rb
+++ i/lib/kamal/commands/builder/clone.rb
@@ -6,7 +6,7 @@ module Kamal::Commands::Builder::Clone
   end
 
   def clone
-    git :clone, Kamal::Git.root, "--recurse-submodules", path: clone_directory
+    git :clone, Kamal::Git.root.shellescape, "--recurse-submodules", path: clone_directory
   end
 
   def clone_reset_steps

It looks like I managed to deploy my app, but I'm sure this is not the right level to do this and there would be more locations where a path needs to be escaped, so I'm not opening a PR yet, as I would rather discuss with people who know more about the project first.

[davidstosik]

I just found this 8-year-old issue: Issue: Trusted strings, shell escaping and backwards compatibility.
Unfortunately it seems to have rapidly lost any traction...

[MiroslavCsonka]

I ran into the same issue! Glad I'm not the only one using iCloud back-ups.

I've noticed that this isn't a problem when Kamal is installed using a bash alias, but that method does not support 1Password integration.

[davidstosik]

@MiroslavCsonka It's good to know that I'm not alone!
Feel free to give a try to my fork by changing your Gemfile:

gem "kamal", ">= 2.1.0", require: false, github: "davidstosik/kamal", branch: "v2.1.1/path-space-fix"

[djmb]

Hi @davidstosik - your change looks like a good idea, so if you could open a PR, I'll get that merged

[davidstosik]

Will do! 👍🏻

Update: done! #1066

[MiroslavCsonka]

@MiroslavCsonka It's good to know that I'm not alone! Feel free to give a try to my fork by changing your Gemfile:

gem "kamal", ">= 2.1.0", require: false, github: "davidstosik/kamal", branch: "v2.1.1/path-space-fix"

Worked like a charm! 🥳

ERROR (SSHKit::Command::Failed): git exit status: 32768
git stdout: Nothing written
git stderr: fatal: repository '/Users/miroslavcsonka/Library/Mobile' does not exist

No more!

[davidstosik]

@MiroslavCsonka Thanks for the report.
Have you tried updating with bundle update kamal?
I added more changes to my branch, so maybe I fixed that problem.
If not, can you please share more context? 🙏🏻 (Maybe using verbose output.)