Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: use the non-root atlantis user instead of root #3886
fix: use the non-root atlantis user instead of root #3886
Changes from 14 commits
d0b5c16
be9a8d3
da4d186
a9605b8
ef609e5
4c6068f
6910d25
d3728c8
35f8a54
a1f547c
78d96cc
6230563
05638e3
315f17e
378b857
cdef470
0029249
fd89c0b
0767f62
c0bf56c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
How come the chmod of /etc/passwd needs to change? If the line is removed, can atlantis still function?
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.
the previous
chmod g=u
sets the group permissions for the/home/atlantis/
directory to be the same as the user's permissions.The new chmod sets the user's permissions for the /home/atlantis/ directory to read, write (and execute).
So, the change here is that the original command was changing group permissions to match user permissions, and it has been modified to give the user explicit permissions instead.
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.
Right but I don't understand why
/etc/passwd
perms need to be changed. Can we get away with only changing/home/atlantis
?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.
Ah, I didn't want to break anything. Therefor I just gave it read write permissions. We can't use
g=u
anymore though.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.
Ok no worries then, can always tackle it later. Thank you!
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.
Ok but the more changes made, the more likely something can break. I'm all for more security, provided @bschaatsbergen has the appetite to keep going 🙏
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.
Is this really the case? I'm happy to add it, but couldn't find it anywhere. I know that users start from 1000 though. Is there a particular reason we should explicitly set this? Omitting it would prevent a uid and gid conflict right? @jamengual
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.
I've set uid/gid 100:1000 and removed the passwd entry lines.
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.
alpine and debian have different
uid
's it appears: #3317 (comment)alpine: uid=100, debian: uid=1000
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.
@arohter it's removed again, to stay inline with what's in the Dockerfile now. to avoid touching the uid and gid at all.