grist-core
grist-core copied to clipboard
Add a grist user in Dockerfile to avoid root user
Hello,
In order to follow the principle of least privilege, the docker image should not run as root.
A bit of context: @rouja works for the DINUM (Interministerial Directorate for Digital Affairs, official website in French), a French public agency with which the ANCT collaborate for deploying Grist.
This change will probably break things on existing deployment. Because the old image run as root, some files/folders are created with root permissions under /persist/* and /grist/* folder. When the new image will boot with the grist user, it will not be able to edit theses files. Maybe we need to prepare a migration procedure.
Thanks @rouja! The overall change makes sense, I'm just trying to think about how to handle the impact on existing users, specifically if files in /persist become unwritable. I think we'd at least want an early check to fail with a clear error message and advice on how to fix?
Any particular significance to the 10022 number? With other tools I use under docker, I find myself matching the uid/gid with the host for maximum convenience, in case I want to manipulate the persisted files from the host.
Hi,
I choose 10022 arbitrarily. So if you prefer something else we can change it. Maybe we can add a check in run.sh with an explicit message ?
Moreover, on openshift cluster, the cluster assign a random uid to every pod. So if we want to support this case too, we need to configure the gid to 0 because on these cluster, the gid is always 0 and set the permissions consequently.
I'm sorry review of this PR has stalled. I'm not sure what to do with it. It is an improvement, but accepting and merging it as is would presumably lead to a wave of confused and upset users. A check in run.sh to give a clear error message would help mitigate that, but there will still be pain for users. Picking a concrete group and user id to use also seems hard. This issue seems very generic, and not at all special to Grist. Is there an example of an image that does a good job out there?
Hi,
https://github.com/docker-library/postgres does a good job for permissions but it adds complexity. It use gosu and nss_wrapper. And even if it's one of the best image I know about permissions this image doesn't handle the switch from root user to an unprivileged user...
Maybe the right way to manage this issue is to keep the image with root user and add a note somewhere in the documentation that says something like :
For best compatibility, our docker images run as root but best practices recommend to not run containers as root on production environments. We recommend to override the userId with -u on docker or with a specific security context on k8s
What do you think ?
Yes, I guess documentation may be the best we can do, in practice. One thing we will be working on is a "bootstrap page" for Grist administrators, to inspect the status of their Grist installation and make sure everything is ok. For example, if sandboxing for documents is not enabled, the page will flag that prominently as a problem and give advice about how to resolve that. Perhaps this best practice could also be checked on such a page?
Hello,
I'm really sorry I forgot to answer. But yes, I think this "bootstrap page" would be a goode place.
Hi @rouja, I've started work on a bootstrap page (now just called boot page) that includes a check of the user running Grist (see https://github.com/gristlabs/grist-core/pull/850)
Should get a bit prettier before it lands I hope :)
Hi,
I just run a test locally and it seems ok to me.
I think it's normal that Greast reachable is KO because I use grist.127.0.0.1.nip.io and this cannot works from pod.
What does the check Built-in Health check to know if I can fix it or not on my local testing deployment.
Nice work !
I think it's normal that Grist reachable is KO because I use grist.127.0.0.1.nip.io and this cannot works from pod.
Thanks for testing @rouja !
@paulfitz @rouja Looks like the need of this PR is now covered by #850
I feel like you are both OK to close this one. I close it, if I am wrong don't hesitate to poke me so I can reopen it.
@paulfitz @rouja Looks like the need of this PR is now covered by #850
I think it would also be useful to give self-hosters guidance on how to run as a non-root user, and if there are steps we can take to make this easier without breaking existing installations, it would be good to take these steps. As a self-hoster, I could imagine being annoyed at Grist when I follow the instructions to install it, and then look at the boot page and see a big red X :-)
I think it would also be useful to give self-hosters guidance on how to run as a non-root user, and if there are steps we can take to make this easier without breaking existing installations, it would be good to take these steps. As a self-hoster, I could imagine being annoyed at Grist when I follow the instructions to install it, and then look at the boot page and see a big red X :-)
Alright, good point!
I tend to think that we would just have to update some documentation:
- In hub.docker.com where some
docker run
commands are suggested: https://hub.docker.com/r/gristlabs/grist - Also update this page https://support.getgrist.com/self-managed/
And probably just ensure that the image does not run using the grist
user by default.
(and if we think it is worth, alerting with a message that running the docker image is discouraged now and invite to change that)
Just to add the 2 cents of an end user to this conversation:
Most images which I use have the option of me supplying them with the desired PUID and PGID via environmental variables which in turn makes the user which runs inside the container, run with those. Any linuxserver docker image implements this as an example.