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

Fix permissions for /config/config.toml so Athens can run as non-root #1699

Merged
merged 2 commits into from
Mar 31, 2021

Conversation

tzvetkoff
Copy link
Contributor

What is the problem I am trying to address?

Current permissions of /config/config.toml make Athens impossible to run as non-root user.

How is the fix applied?

Changed the file permissions to 0644.

What GitHub issue(s) does this PR fix or close?

Fixes #1695

@tzvetkoff tzvetkoff requested a review from a team as a code owner March 7, 2021 11:43
Copy link
Member

@arschles arschles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tzvetkoff looks good, thanks for this. there was a flake in the CI/CD, so updated code from main and waiting for the new CI/CD run to finish

@arschles
Copy link
Member

Ok, this looks like an unrelated test error, might be a flake but I'm not sure.

I'll do this as soon as I can @tzvetkoff but I can't today unfortunately. @marwan-at-work any chance you have an idea what's going on here?

@marwan-at-work
Copy link
Contributor

@arschles looks like redis-sentinel can time out on start up, I restarted the build and it's good now.

@arschles
Copy link
Member

nice, thanks @marwan-at-work . I restarted a few times but it didn't fix the flake. I guess drone got in a better mood.

@arschles arschles merged commit 6991d63 into gomods:main Mar 31, 2021
@tzvetkoff tzvetkoff deleted the config-file-permissions branch April 19, 2021 15:08
@abh
Copy link

abh commented Feb 21, 2022

This errors out when using the canary docker container under kubernetes. There is a (weird, IMO) check to disallow these permissions in 5eba6f2#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5R170

@manugupt1
Copy link
Member

Hello @abh that's correct. The problem with 644 is that tokens stored in the config can be read by the world. Is 644 absolutely required, or would 0640 suffice?

Thanks!

@abh
Copy link

abh commented Feb 22, 2022

@manugupt1 I didn't look at it carefully beyond noticing that installed in kubernetes with the helm chart the canary image would error out with the message about the permissions being too lax and at a glance it seems like the change to make them lax was deliberate (but not followed up with relaxing the other check).

@manugupt1
Copy link
Member

@abh Will you be able to do a quick check please? Does Athens work on latest image (not canary)

@manugupt1
Copy link
Member

@arschles @abh What do you think of the following:

  1. Remove the code to allow users too modify the permissions: 5eba6f2#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5R170
  2. Change the Dockerfile permissions to 0700 (so that secrets are not visible to the world)
  3. Add an FAQ to tell them how to change the permissions; Dockerfile, initContainers as possible options?

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 this pull request may close these issues.

Docker: change /config/config.toml permissions so Athens can be ran as non-root user
5 participants