kamal
kamal copied to clipboard
SSHKit does not escape command arguments, resulting in failure to clone a path with spaces
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_commandcallsOpen3.popen3(cmd.to_command)here -
SSHKit::Command.to_commandeventually calls#to_sand joins all items in the command array without escaping to produce a string here -
Instead,
Open3.popen3could 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
.shellescapewhen 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.
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...
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.
@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"
Hi @davidstosik - your change looks like a good idea, so if you could open a PR, I'll get that merged
Will do! 👍🏻
Update: done! https://github.com/basecamp/kamal/pull/1066
@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!
@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.)