Add user creation and switchover to multistage.Dockerfile.#39
Add user creation and switchover to multistage.Dockerfile.#39mikedlr wants to merge 1 commit intoastral-sh:mainfrom
Conversation
Creating a user app and changing the user to app provides a chance of isolation between the application and the root inside the container blocking some security vulnerabilities. There's already a --chown instruction in the Dockerfile so this was probably the intention. Add the needed commands.
|
Aims to fix #29 |
|
I'm getting a permission error when running this dockerfile, similar as in #32: docker build -f multistage.Dockerfile . -t uv-multistage
docker run --rm -it --publish 8000:8000 uv-multistage |
|
I'd recommend following the guidance of the DoD's hardened Iron Bank containers, which create an user explicitly with 1001:1001, and then chown the certs directory (which is RHEL in this case but generalizes). |
|
I'd love to see this merged. As this repository is recommended in the Astral docs, I can only imagine how many people are running their projects as root on production... Any steps we can take to get this merged? |
|
@abhiaagarwal can you give some explaination of the benefits / trade offs of doing that. Couldn't that create a security problem if the underlying docker image has already allocated UID 1001 for some other use and then we create with the same UID so that two different bits of software sit in the same domain? |
|
@mikedlr since most containers are deployed via kubernetes this is not a problem in practice |
|
@abhiaagarwal this is supposed to be the canonical example of using uv in docker, so it very much will be used in build processes, including on places like github, where we can't assume Kubernetes. I think that if that's the justification then it would rather mean we shouldn't do it. Possibly provide a variable which can be used to override the default? |
|
@mikedlr I don't have enough knowledge about the potential security issue you're raising, but I am aware that Docker themselves recommend to use a non-root user: https://docs.docker.com/get-started/docker-concepts/building-images/writing-a-dockerfile/#explanation |
|
@2zqa that we agree on and both my version and the proposed change would do that. The question is about the allocation strategy for the non-root user's user ID. In my patch that's dynamic which means that the patch would work on top of a python container image which included mysql and had already allocated userid 1001 for mysql. Then the new app would have userid 1002. See for example fedora, which encourages dynamic allocation - https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ After some searching I have found a concrete recommendation which I think it's probably correct to follow https://github.com/hexops-graveyard/dockerfile This says: use a static, known UID but that UID should be 10001 and not 1001. I'd suggest doing this with a default and allowing it to be overridden if needed. |
Creating a user app and changing the user to app provides a chance of isolation between the application and the root inside the container blocking some security vulnerabilities.
There's already a --chown instruction in the Dockerfile so this was probably the intention. Add the needed commands.