-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Check the logged user instead of $USER #3330
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
Given that `sudo usermod --login "$DOCKER_USER" coder` and `sudo groupmod -n "$DOCKER_USER" coder` modify the container's disk it'll persist across restarts, but environment variables will be reset to whatever state they had at the end of `Dockerfile`. In this case, `$USER` is set to `coder`, so this branch will always be true. By checking with the output of `whoami`, which gets it's information from `/etc/passwd`, we make sure to get the real logged user and not the one defined by $USER. We also move `USER="$DOCKER_USER"` out of the branch, since we always want this to happen at entry-point. If we don't do this assignment, $USER will contain `coder` upon restart.
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.
Thank you for fixing this!! Your PR description was fantastic too.
Thanks for making your first contribution! 🙂
Codecov Report
Merging #3330 (b491e06) into main (02a0e05) will not change coverage.
The diff coverage isn/a.
❗ Current head b491e06 differs from pull request most recent head 1aed545. Consider uploading reports for the commit 1aed545 to get more accurate results
Impacted file tree graph
@@ Coverage Diff @@ ## main #3330 +/- ## ======================================= Coverage 58.95% 58.95% ======================================= Files 35 35 Lines 1703 1703 Branches 374 374 ======================================= Hits 1004 1004 Misses 561 561 Partials 138 138
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update 02a0e05...1aed545. Read the comment docs.
You can ignore the trivy-scan failure. We're aware of that and working on a fix. As for the audit vulnerabilities, we're also aware of a postcss CVE that needs to be fixed. Everything else looks good!
Check `$DOCKER_USER` was defined before copying it to `$USER`.
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.
🎉 🎉 🎉
Given that
sudo usermod --login "$DOCKER_USER" coderandsudo groupmod -n "$DOCKER_USER" codermodify the container's disk it'll persist across restarts, but environment variables will be reset to whatever state they had at the end ofDockerfile. In this case,$USERis set tocoder, so this branch will always be true.By checking with the output of
whoami, which gets it's information from/etc/passwd, we make sure to get the real logged user and not the one defined by $USER.We also move
USER="$DOCKER_USER"out of the branch, since we always want this to happen at entry-point. If we don't do this assignment, $USER will containcoderupon restart.Fixes #2767.