-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(dockerfile): set more lenient permissions on /home/runner #4083
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
Conversation
|
Hi @nikola-jokic This is what we were talking about last week when we met, is this something that you can take a look at? Thanks! |
nikola-jokic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
a659643 to
3ae977a
Compare
|
Thanks, moved |
Currently, the permissions of /home/runner are 750. In some container runtimes and Kubernetes distributions (including OpenShift), a different uid/gid (not `runner`) may be used when running the image. The runner expects to be able to read and execute scripts within the home directory, and it will also write ephemeral files, diagnostic data, etc. into the directory as well. Therefore, to support the ability to use the runner as a user apart from `runner`, full 777 permissions are needed. A longer-term change to consider which may improve the security posture here would be to separate the executable portions of the application (scripts, etc.) from the places where temporary data is written, and control the permissions of these separately. Signed-off-by: Caleb Xu <[email protected]>
3ae977a to
b6a7b24
Compare
|
True @caxu-rh, my bad, I tested it partially, but it is still better at least from my point of view to have |
|
|
||
| WORKDIR /home/runner | ||
| && echo "Defaults env_keep += \"DEBIAN_FRONTEND\"" >> /etc/sudoers \ | ||
| && chmod 777 /home/runner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TingluoHuang @nikola-jokic This is a breaking change after upgrading the GitHub runner from 2.329.0 to 2.330.0. We are now seeing the error “failed to initialize build cache.” We need to revert back to 2.329.0. Could you please fix this so we don’t need separate handling on our side?.
failed to initialize build cache at /home/runner/.cache/go-build: mkdir /home/runner/.cache/go-build: permission denied
Error: Command failed: /opt/hostedtoolcache/go/1.25.1/x64/bin/go env
failed to initialize build cache at /home/runner/.cache/go-build: mkdir /home/runner/.cache/go-build: permission denied
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➤ YN0001: │ foobar@https://github.com/foo.git#commit=9434354e7bf8f19d229fecabbef9dedc678f5c47: Failed cloning the repository
➤ YN0001: │ Repository URL: https://github.com/foo.git
➤ YN0001: │ Error: invalid key: core.autocrlf
➤ YN0001: │ Fatal Error: unable to write parameters to config file
➤ YN0001: │ Exit Code: 128
issues cloning private repositories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nikola-jokic - one difference I'm seeing is in the owner uid/gid of the /home/runner directory.
- In 2.329.0:
Access: (0750/drwxr-x---) Uid: ( 1001/ runner) Gid: ( 1001/ runner) - In 2.330.0:
Access: (0777/drwxrwxrwx) Uid: ( 0/ root) Gid: ( 0/ root)
I think the ownership change is due to moving WORKDIR above. I don't quite understand how this ownership change is causing the observed behavior, though.
Currently, the permissions of /home/runner are 750. In some container runtimes and Kubernetes distributions (including OpenShift), a different uid/gid (not
runner) may be used when running the image.The runner expects to be able to read and execute scripts within the home directory, and it will also write ephemeral files, diagnostic data, etc. into the directory as well. Therefore, to support the ability to use the runner as a user apart from
runner, full 777 permissions are needed.A longer-term change to consider which may improve the security posture here would be to separate the executable portions of the application (scripts, etc.) from the places where temporary data is written, and control the permissions of these separately.