Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run inside a Docker container #20

Closed
rieschl opened this issue Feb 18, 2020 · 22 comments · Fixed by #17
Closed

Run inside a Docker container #20

rieschl opened this issue Feb 18, 2020 · 22 comments · Fixed by #17

Comments

@rieschl
Copy link

rieschl commented Feb 18, 2020

It took me forever to figure out, why this action doesn't work if I run the whole workflow inside a container.
The problem is that the Github Action somehow changes/sets the HOME variable inside the container so that the ~/.ssh/known_hosts file is at a wrong location.
This action puts the Github PubKeys inside ~/.ssh/known_hosts which is in the home path of the runner. But the running container normally runs as root so ssh looks for /root/.ssh/known_hosts which doesn't exist.
Copying the known_hosts to this location if the workflow is running inside Docker solves the problem. As I am a total Node noob I just played around with the dist/index.js file, but putting the following snippet after creating the known_hosts file the SSH agent also works inside docker:

    if(fs.existsSync('/.dockerenv') && child_process.execFileSync('id', ['-u']).toString().trim() === '0') {
        fs.mkdirSync('/root/.ssh', { recursive: true});
        fs.copyFileSync(`${homeSsh}/known_hosts`, '/root/.ssh/known_hosts');
    }

I'm not sure if that somehow breaks running the action in Windows because in Windows there is no id command. But that shouldn't be a problem because Github Actions currently doesn't allow running non-Linux containers. Also, I don't know if the root check is even necessary because probably all containers run as root.

Would it be possible to add this snippet to your action so Docker users can also use it? :)

@mpdude
Copy link
Member

mpdude commented Feb 19, 2020

Does it work if we put the keys into /etc/ssh/known_hosts?

That should be the system (not user specific) file and we could get around all attempts to detect the user and/or HOME.

@mpdude
Copy link
Member

mpdude commented Feb 19, 2020

Correction: That file might be /etc/ssh/ssh_known_hosts, we might need to check both.

@rieschl
Copy link
Author

rieschl commented Feb 19, 2020

Right, it also works if I try /etc/ssh/ssh_known_hosts but we'd also have to check if the user is root or has write permission on that file.

@rieschl
Copy link
Author

rieschl commented Mar 3, 2020

Hi Are there any updates on this? I'm about to roll out github actions to ~20 packages so it would be nice if I can skip the mkdir -p /root/.ssh; ssh-keyscan -t rsa github.com >> /root/.ssh/known_hosts part. Thanks!

@mpdude
Copy link
Member

mpdude commented Mar 3, 2020

No progress, I’m afraid. This is open source, so we place the utmost reliance upon the zealous cooperation of the public.

If, however, you can provide a pull request, I’d be happy to review it!

What if we try to write to both known_hosts file locations, ignoring errors?

Would that solve it?

@quisse
Copy link

quisse commented Jul 31, 2020

I am experiencing the same issue but not when running in a container.

name: Test
on:
  [push, pull_request]

jobs:
  build:

	name: Test ssh
	runs-on: ubuntu-latest

	steps:

	  - uses: webfactory/ssh-agent@v0.4.0
		with:
		  ssh-private-key: ${{ secrets.SSH_PRIVATE_KEY }}
	  - run: ssh-keyscan -t rsa github.com >> /etc/ssh/ssh_known_hosts
	  - run: ssh -T git@github.com

@adamrogas
Copy link

It took me forever to figure out, why this action doesn't work if I run the whole workflow inside a container.
The problem is that the Github Action somehow changes/sets the HOME variable inside the container so that the ~/.ssh/known_hosts file is at a wrong location.
This action puts the Github PubKeys inside ~/.ssh/known_hosts which is in the home path of the runner. But the running container normally runs as root so ssh looks for /root/.ssh/known_hosts which doesn't exist.
Copying the known_hosts to this location if the workflow is running inside Docker solves the problem. As I am a total Node noob I just played around with the dist/index.js file, but putting the following snippet after creating the known_hosts file the SSH agent also works inside docker:

    if(fs.existsSync('/.dockerenv') && child_process.execFileSync('id', ['-u']).toString().trim() === '0') {
        fs.mkdirSync('/root/.ssh', { recursive: true});
        fs.copyFileSync(`${homeSsh}/known_hosts`, '/root/.ssh/known_hosts');
    }

I'm not sure if that somehow breaks running the action in Windows because in Windows there is no id command. But that shouldn't be a problem because Github Actions currently doesn't allow running non-Linux containers. Also, I don't know if the root check is even necessary because probably all containers run as root.

Would it be possible to add this snippet to your action so Docker users can also use it? :)

I should have read this more carefully as I spent the same time trying to track this down one run after another 😄

@soltex1
Copy link

soltex1 commented Jan 11, 2021

I am trying to use Packer:

uses: docker://hashicorp/packer:1.6.5

but it doesn't have access to the SSH keys.

Is there a way to achieve this?

mwik added a commit to mwik/ssh-agent that referenced this issue Feb 2, 2021
@mpdude
Copy link
Member

mpdude commented Feb 12, 2021

Thank you @rieschl for your suggestion and working on this, and thanks to @mwik for creating #58 for it!

The code change suggested in the opening comment here looks like some specialized edge case logic... I wonder if we could find a more general, cleaner way of dealing with this?

I understand that /etc/ssh/ssh_known_hosts might work inside a container, but not in the regular runner environment, since that file is probably read-only or could be changed only be the root user, but Actions use a dedicated runner UID.

So, what does HOME point to in the containerized action run? To my understanding, HOME is what the ~ resolves to, so why doesn't ssh pick up $HOME/.ssh/known_hosts?

@mpdude
Copy link
Member

mpdude commented Feb 12, 2021

Oh, and does it make a difference inside the container when we're using os.homedir() instead of the HOME env var?

@mwik
Copy link

mwik commented Feb 12, 2021

So, what does HOME point to in the containerized action run? To my understanding, HOME is what the ~ resolves to, so why doesn't ssh pick up $HOME/.ssh/known_hosts?

That's the question. $HOME points to /github/home, but ssh nevertheless looks in /root/.ssh so figuring out why is key. #58 is only a workaround. Unfortunately it is a PITA to debug stuff in github actions. However it seems that ssh does not like changing HOME, it still uses the old value somehow. I don't think it will matter whether we use os.homedir() or HOME since its set to the same thing.

@mwik
Copy link

mwik commented Feb 12, 2021

Come to think of it, ssh probably uses getent or something similar to get the home directory of the current user. Probably that is not changed in the container.

@mwik
Copy link

mwik commented Feb 12, 2021

After inspecting the ssh source code I can confirm that it uses the passwd entry. I also found that we can actually use os.homedir() if we unset the HOME environment variable first. So something like this should work

    delete process.env['HOME'];
    const homeSsh = os.homedir() + '/.ssh';

I'll test this when I have an opportunity.

@mpdude
Copy link
Member

mpdude commented Feb 12, 2021

@mwik please try #17 just by using @try-windows as the action version in your Docker container. It uses os.homedir(). Would be great if we'd not need to fiddle around with HOME...

@mwik
Copy link

mwik commented Feb 12, 2021

Unfortunately HOME needs to be undefined in order for os.homedir() to take it from /etc/passwd. See nodejs doc. But if you are worried about removing HOME, we can always restore it after os.homedir() has been called.

@mpdude
Copy link
Member

mpdude commented Feb 12, 2021

I haven’t found good „official“ documentation or even source code in this, but it seems ssh uses a getpwent system call instead of relying on $HOME, possibly for security reasons?

So, maybe we should use os.userinfo()?

https://nodejs.org/api/os.html#os_os_userinfo_options

@mpdude
Copy link
Member

mpdude commented Feb 12, 2021

@mwik please give #17 another try.

Also, could you help me to fix the workflow file over there to make it run on Docker?

@mwik
Copy link

mwik commented Feb 12, 2021

Great! os.userInfo() is definitely the way to go. And yes, @try-windows works for my docker build. I'll take a look at the workflow file.

@mpdude
Copy link
Member

mpdude commented Feb 13, 2021

Out of curiosity, how is the action run inside the container – is it a requirement for such containers to contain the node interpreter, or is that copied/mounted into the container?

@mpdude
Copy link
Member

mpdude commented Feb 13, 2021

It works 🎉, @mwik thank you for the support!

https://github.com/webfactory/ssh-agent/runs/1893563200?check_suite_focus=true

@mwik
Copy link

mwik commented Feb 15, 2021

Out of curiosity, how is the action run inside the container – is it a requirement for such containers to contain the node interpreter, or is that copied/mounted into the container?

Very good question. Node is not installed in my build docker image, so I assume its mounted somewhere. Strangely enough I can not find any documentation about how its done.

@mpdude
Copy link
Member

mpdude commented Feb 15, 2021

Maybe they do some tricks like mounting a statically-linked version somewhere...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants