Should I rewrite this action as Docker or Javascript one to add ssh cleanup steps or is it 100% safe to use as is, if it is only running with GitHub hosted runners?

It’s up to you whether it’s safe enough for your use case and security requirements with the current constraints. What I will say is that I wouldn’t offer it in the Marketplace without a cleanup process that makes sure the started ssh-agent instance gets stopped.

Hey there!
I have added some cleanup steps to action (https://github.com/marketplace/actions/docker-run-command-ssh-context). Can this action now be considered 100% safe to run on GitHub hosted runners and self-hosted runners? 🙃

I’m not going to pretend anything is 100% secure. I will point out the security flaws I still see though:

• The way ssh-keyscan is used. Yes, it avoids the “unknown host” warning, but only by accepting the host key ssh-keyscan sees without any verification at all. If someone intercepts connections between the runner and your system, they’ll likely intercept both. The action should expect an actually known key (possibly with an opt-in for insecure behavior).

• The cleanup steps won’t run if one of the earlier steps fails.

• When passing the SSH private key to the agent, do it through an environment variable. That way the key doesn’t become part of the command line, which is then visible on the runner.

1. Will add ssh-public-key input, thanks!

2. It is not possible to always run some cleanup steps with composite actions, right? 😟
   I think I will switch to docker based action, and maybe trap errors to always perform a cleanup.
   Or is there a way to pass env vars from docker based action entrypoint to post-entrypoint? Because post-entrypoint will run on fresh container without env vars from entrypoint, correct? 🤓
   …Do I really need to kill ssh-agent and remove keys from ssh-add with Docker based action? Does it leaks state anywhere after entrypoint has been run?

3. Is this way ssh-add - <<< "${{ inputs.ssh_private_key }}" of passing a SSH private key to the agent safer than using echo and piping?

Will add ssh-public-key input, thanks!

I’d name it ssh-host-key to make the purpose a little clearer, but good! 🙂

It is not possible to always run some cleanup steps with composite actions, right? 😟

I’m afraid not, unless there is some undocumented post feature (and I assume there isn’t). 😕

…Do I really need to kill ssh-agent and remove keys from ssh-add with Docker based action? Does it leaks state anywhere after entrypoint has been run?

The ssh-agent will be stopped when your Docker container is stopped. Explicitly stopping the agent would still be good to ensure it can do its cleanup, but the process and its socket will definitely be gone with the container. And you can do your own error handling in whatever you run inside the container, so an error in one command doesn’t have to prevent anything else from running. 🙂

Is this way ssh-add - <<< "${{ inputs.ssh_private_key }}" of passing a SSH private key to the agent safer than using echo and piping?

Not what I was thinking of, but it should work! The point is that the process table (what you get e.g. with ps xau) contains command line arguments, so it’s good to avoid anything sensitive in those. The <<< “here string” is handled by Bash, so there’s no extra process where it might show up.

… new day - new problems
Tried to switch to Docker based action.
PROs:

• if any of steps fails, all next steps will work

CONs:

• I have to login/logout to Docker Hub inside container too, so need to pass even more inputs.
• Spent around 2 hours figuring out how to add host key to known_hosts. Now using /etc/ssh/ssh_known_hosts file to put host key. Because $HOME/.ssh/known_hosts file is not used inside Docker based action.

This is insane. ssh-agent + GitHub Actions is such a big problem, actually. 😑 😑 😑

I am almost giving up and switching to using Portainer on all swarm nodes…to deploy app from DockerHub webhook. So sad story 😦

Because $HOME/.ssh/known_hosts file is not used inside Docker based action.

This seems odd. It should be used, but the user (and thus home directory) might change inside the container. Also SSH ignores config files if they’re not owned by the user in question, or access rights are too wide.