use temporary file instead of hardcoded git-ssh.sh

[webervin]

If two different unix users deploy application with same name then first one creates /tmp/hello_world/git-ssh.sh and second dies with permission error.

maybe we could ask OS to generate unique file name for us without dealing with conflicts on our own
tempfile --directory /tmp --suffix -git-ssh.sh

PS I have seen #744 and #736

[leehambley]

It can be specified with the tmp_path variable.

Sent from my Nexus 5.
On 2 Dec 2014 12:22, "Ervin Weber" notifications@github.com wrote:

If two different unix users deploy application with same name then first
one creates /tmp/hello_world/git-ssh.sh and second dies with permission
error.

maybe we could ask OS to generate unique file name for us without dealing
with conflicts on our own
tempfile --directory /tmp --suffix -git-ssh.sh

PS I have seen #744 #744
and #736 #736

—
Reply to this email directly or view it on GitHub
#1206.

[leehambley]

http://stackoverflow.com/questions/3724534/how-to-change-the-temporary-dir-capistrano-used

[leehambley]

The problem is that a handful of operations need the git wrapper to be in place. Whilst we can make the git wrapper remove itself after each run, that would then mean that every new task that needs to do something with the Git repo would have to call git-wrapper individually, which would lead to this file being written multiple times, which is sub-optimal. There's no way we have to easily share state between many threads/etc in order to synchronise this. If git was better written, it would allow us to specify all these options more cleanly on the CLI but it's not, and it doesn't.

[webervin]

Yes, I am aware of ability to change temp dir and usualy set it to /home/:deploy_user/tmp but smart default would be soo much better.

Also I agree that removing and re-creating git-wrapper for each task would be sub-optimal.
So in this ticket I am suggesting that we a) generate unique random name for git-wrapper (asking OS to create one, temporary executable or simply generating binary name and remembering this name for duration of run and, optionally b) in after_release (?) we could do one single operation of removing git wrapper

[nikolaeff]

Creating file in a /tmp directory with predictable name could also lead to security issues. Especially in shared hosting environment.

[leehambley]

WIth the newest release of Git we can drop the wrapper script all together. The naming is such because the execute() statements are not reentrant, and so can't share state (if we were to generate a tmpfile) the alternative is to recreate a random named (tmpfile) wrapper on every invocation of a git command which is impractical.

Git does not honor it's tty? output, forcing ALWAYS to read on stdin whether or not stdin is attached, a tty, or a file. It has, until last week been broken. And unfortunately because of LTS distributions, etc we'll all be stuck maintaining hack for broken behaviour in Git for the next few years.

[nikolaeff]

@leehambley will it be a better solution to create wrapper file during check phase and create wrapper within deploy_dir?

[leehambley]

@leehambley will it be a better solution to create wrapper file during check phase and create wrapper within deploy_dir?

Sure, and all the plugins that enhance the Git workflow can create their own wrappers, and users who want to do something about a change log can write a task to create their own wrappers, etc.

It's far from trivial, and we tried all the options. As ever we have to shoot for the common case, those who are security conscious can easily fix things for themselves by overwriting, or appending to the task definitions.

The problems are also not easily solved, for example, take:

task :wrapper do
  # Creates a wrapper, let's say it uses `mktemp ` like ths:
  tmpfile_path = capture("mktemp")
end
task :something_with_git => :wrapper do # depends on :wrapper
  # this isn't a million miles away from what we do, but there's no 
  # sane place to put the wrapper name so we can access it
  # safely here... Thread local variables are out, we can't get values from
  # Rake::Task.invoke, or friends, so there's no way for us to know
  # what was created previously.
end

An alternative we even considered was to use the SSHKit command map, and to do something (hideous) like:

SSHKit.config.command_map[:git] = <<-EOGITCOMMNAND
TMPFILE = `mktmp`
cat > TMPFILE <<EOS
# git wrapper goes here
# Implementation as in https://github.com/capistrano/capistrano/blob/65c52ffc248605b726cbae1071f13ebf7e2821e3/lib/capistrano/tasks/git.rake#L14
# but we have to add a trap that when the script has been run it removes itself.
EOS
GIT_SSH=TMPFILE git; unlink TMPFILE
EOGITCOMMAND

This introduces the problem that we need long script, the logs look like hell, there's still no guarantee that /tmp isn't mounted with noexec, we've lost the ability for the user to choose the tmp path, if it's not in a /tmp dir, we can't rely on the OS to clean the file up when we're finished, and every invocation has to trap events, and clean up... all of that is possible, but it makes it more complex, less easy to debug, less easy to read the output, all for limited benefits.

People often complain about the out-of-the-box defaults, but we the choice of Rake as the underlying mechanism wasn't a mistake, it is easily patched at runtime, or startup time, the code is easy to read, the flow of tasks, and their dependencies is self documenting, and I reiterate that the intention is that those who have specific requirements can work around them.

Let's not forget, out of the box we're skipping strict host key checking for the git host, which is also not clever, if Github would be the one true solution, we would pre-cache that, infact there are so many variables in play (of course Git supports 3 protocols, git://, ssh:// and http{,s}://), some do/don't have host keys which are/aren't checked, some may have SSL certs, which may depend on the cert bundle being installed on a machine, some may/not have outdated or invalid, or self-signed certs, we have to shoot for the most common use-cases, and hope for the best!

See also "The credential subsystem is now friendlier to scripting" which was what I was referring to in my earlier comment: https://github.com/blog/1957-git-2-3-has-been-released

[leehambley]

In a perfect world:

• We wouldn't need a wrapper because Git < v2.3 would be sane and not break all the unix conventions.
• We would have a step in the beginning of the stragegy.check task which would validate (once) the host key, and cache it, we'd pass the prompt onto the user, and pass the [Y,n] back to the server.
• There is no step 3.

The second less preferable option is to have more manual steps, there's always been a fine line between where provisioning ends, and deployment begins. Personally, we have a wrapper which is part of the project repo, bootstrapped by puppet and defines our own Git options, we deploy over SSH with some specific protocol requirements, and we pre-cached our Git host key using Puppet, we modified the tasks that rely on wrapper not to rely on wrapper at all at runtime:

cache_them = Rake::Task[:task_name].prerequisites
Rake::Task[:task_name].prerequisites.clear # then clear them
task :task_name => [and, set, them, again, without, wrapper]

[codyrobbins]

@leehambley Would you mind going into a bit more detail about how to define your own Git options for the wrapper? I’m really confused why in the world the git-ssh.sh script turns off strict host key checking in the first place—it seems like a super bad idea and I haven’t been able to find an explanation of the rationale behind it anywhere. I’m in the same boat as #775 where I want to have it turned on because I already have Git host keys added to known_hosts.

Are you saying above that you install your own wrapper script using Puppet at /tmp/APPLICATION/git-ssh.sh (instead of letting Capistrano create it) and then simply customize that script with the SSH options you want?

I don’t understand what you mean, though, when you go on to say you modify the tasks that rely on wrapper to not rely on wrapper. If you’re customizing the wrapper then why would you then not want to rely on it anymore? I don’t understand what you’re actually doing in the example code snippet above—what are task_name and cache_them supposed to be and what are you supposed to be setting the prerequisites back to?

In any event, thanks so much for all your hard work on Capistrano!

[leehambley]

@leehambley Would you mind going into a bit more detail about how to define your own Git options for the wrapper? I'm really confused why in the world the git-ssh.sh script turns off strict host key checking in the first place—it seems like a super bad idea and I haven't been able to find an explanation of the rationale behind it anywhere. I'm in the same boat as #775 where I want to have it turned on because I already have Git host keys added to known_hosts.

Because people don't know how to use SSH host keys properly, and without this, there's an extra step anyway, to go by hand onto each server and confirm the "Y" prompt, not to mention Git is badly behaved here too, so it prompts even if stdin isn't a tty, where the user is then presneted (through capistrano) something that looks like a [Y/n] prompt and

Anyway, who would really know how to properly verify an SSH host key? Most people accept the first key they see from a server, and if it fails they follow the "delete the host key from the known hosts file" instructions given.

I don’t understand what you mean, though, when you go on to say you modify the tasks that rely on wrapper to not rely on wrapper. If you’re customizing the wrapper then why would you then not want to rely on it anymore? I don’t understand what you’re actually doing in the example code snippet above—what are task_name and cache_them supposed to be and what are you supposed to be setting the prerequisites back to?

My system globally exports GIT_SSH to a global wrapper known always to exist, so I don't need Capistrano to put something there for me.

In any event, thanks so much for all your hard work on Capistrano!

I'm glad it provides value for you, even if we have some weird corner cases.

[codyrobbins]

Awesome, thanks so much for the quick reply!

[mvz]

How about adding the user as part of the path?

[zedtux]

Any news regarding this issue ?

[leehambley]

No

(sent from my phone, please excuse typos)
On 23 Feb 2016 5:37 p.m., "Guillaume Hain" notifications@github.com wrote:

Any news regarding this issue ?

—
Reply to this email directly or view it on GitHub
#1206 (comment)
.

[mattbrictson]

I believe this was addressed via #1517.