GeezerVison is showing github.com — Permission issue with rootless containers · Issue #1243 · pre-commit/pre-commit
Permission issue with rootless containers #1243DescriptionI tried to use a docker_image hook on a RHEL7.7 system using podman with the podman-docker package installed [This setup allows for rootless containers]. The hook attempts to modify the file, but gets a "permission denied" error. From looking at the source code, I see that pre-commit is roughly trying to execute:
docker run -u $(id -u):$(id -g) -v $(pwd):/src:rw,Z --workdir /src -it <ENTRY> <FILE>
The hook does try to run, but results in permission error.
If, however, I remove the -u option from the source code (locally, languages/docker.py, docker_cmd()), then the hook runs fine:
 Reacted by Jordi Sola, Jonas Bushart, Fabrice Flore-Thébault, Jordan Williams, Adam Sherwood, Sorin Sbarnea and iilei • jochen preusche Activityasottile commented on Dec 16, 2019 sounds like podman is doing an non-compliant thing
without -u the files become owned by root which isn't really ok
I'm inclined to close this as wontfix, thoughts? dkolepp commented on Dec 16, 2019 No - actually it is quite compliant. Rootless containers map UID zero to
a user namespace UID. This means that’s UID 0 in the container corresponds
to UID of the person running the container. So, if the files are owned by
the same user running the container, then there appears to be no problem.
…
On Mon, Dec 16, 2019 at 3:47 PM Anthony Sottile ***@***.***> wrote:
sounds like podman is doing an non-compliant thing
without -u the files become owned by root which isn't really ok
I'm inclined to close this as wontfix, thoughts?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
< #1243?email_source=notifications&email_token=ACNM5MKMHL2HIGFRVI4WJPTQY7SOJA5CNFSM4J3PEU42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHABIDY#issuecomment-566236175>,
or unsubscribe
< https://github.com/notifications/unsubscribe-auth/ACNM5MLDWHGNSZXYKJNUZ7LQY7SOJANCNFSM4J3PEU4Q>
.
dkolepp commented on Dec 16, 2019 Does pre-commit have a config file, that the library could read, to decide
if it should leave out the “-u” option?
…
On Mon, Dec 16, 2019 at 5:13 PM Dan Kolepp ***@***.***> wrote:
No - actually it is quite compliant. Rootless containers map UID zero to
a user namespace UID. This means that’s UID 0 in the container corresponds
to UID of the person running the container. So, if the files are owned by
the same user running the container, then there appears to be no problem.
On Mon, Dec 16, 2019 at 3:47 PM Anthony Sottile ***@***.***>
wrote:
> sounds like podman is doing an non-compliant thing
>
> without -u the files become owned by root which isn't really ok
>
> I'm inclined to close this as wontfix, thoughts?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> < #1243?email_source=notifications&email_token=ACNM5MKMHL2HIGFRVI4WJPTQY7SOJA5CNFSM4J3PEU42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHABIDY#issuecomment-566236175>,
> or unsubscribe
> < https://github.com/notifications/unsubscribe-auth/ACNM5MLDWHGNSZXYKJNUZ7LQY7SOJANCNFSM4J3PEU4Q>
> .
>
asottile commented on Dec 16, 2019 Right, that's not how docker works though, so if this is attempting to emulate docker's cli it is doing so without complying to docker's approach dkolepp commented on Dec 16, 2019 What about if the local docker daemon is setup as so: https://docs.docker.com/engine/security/rootless/
Pretty sure the above approach by docker is using the same set of linux kernel features that podman is using. Rootless containers are a must for any one that is security-minded. asottile commented on Dec 16, 2019 no idea, can you try it and report back? dkolepp commented on Dec 16, 2019 dkolepp commented on Dec 17, 2019 Update: I installed docker 19.03 on Centos 7.7 using the instructions found here: https://docs.docker.com/engine/security/rootless/#prerequiresites
This allows the docker daemon to run as a non-root user. This mode allows the docker daemon to be installed in user space, and does not require privileges to install or run the docker daemon, as long as certain prerequisites are satisfied.
With this type of install, the exact same issue is observed as with (rootless) podman. asottile commented on Dec 17, 2019 do you have a suggestion as to how to detect / avoid this then? and would you be willing to submit a PR addressing it? dkolepp commented on Dec 17, 2019 I'm happy to submit a PR. Would like some ideas about what is "acceptable" for a contribution. As you say, ideally there is some way to detect rootless vs privileged, and then adjust the associated docker command accordingly.
If that's not the case (that there's not an easy way to detect rootless mode), are there other options available? Environment variables? user configuration file for the pre-commit executable itself? dkolepp commented on Dec 17, 2019 Update: podman system info produces a YAML file that has a readable key for "rootless". Am going to check docker for this too... asottile commented on Dec 17, 2019 ideally just detect and adjust, if that's not an option this is probably wontfix -- there is currently no configuration and I'd like to keep it that way
and perhaps document putting a docker executable on the PATH in this case which drops -u argument (since it's completely non-functional)
this does seem like a bug in docker though, --user seems completely nonfunctional in "rootless" 🤔 -- I wonder if this should be reported as well to their tracker(s) dkolepp commented on Dec 17, 2019 This is intentional moving forward with containers - that all containers are set to UID 0 inside the container, and the container runtime takes care of security and sandoxing the "root" user of the container. By applying a context, you limit the privileges of the container to the context in which that container is run: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
Also, docker has a similar interface: docker system info that provides a rootless-enabled flag... asottile commented on Dec 17, 2019 I'm not sure how kubernetes design decisions are "this is how containers are from now on", could you elaborate? I'm afraid you're making unsubstantiated claims or projecting one project's decisions on the entire concept of containers. themr0c commented on Jun 3, 2020 themr0c commented on Jun 3, 2020 I will need even more help to write a test for the coverage: :/
py38 run-test: commands[2] | coverage report
pre_commit/languages/docker.py 64 3 8 1 92% 89->90, 90-92
But let's see first if the implementation is acceptable. aryx commented on Oct 7, 2022 Why does pre-commit pass a -u with the current userid to docker_container?
We get some permission denied in the container because my locao userid has nothing to do with the user id in the container.
Wouldn't it be simpler to just not pass any -u? asottile commented on Oct 7, 2022 because things can write and then you'd have to deal with root owned files on the host asottile commented on Jun 4, 2024 @kapsh yeah of course did you read the thread? diamond-deluxe commented on Jul 20, 2024 asottile commented on Jul 20, 2024 @diamond-deluxe please don't bump threads like that -- if you're interested in the current state: read the thread, if you're interested in future updates: click subscribe, if you like the issue use the reactions -- but please don't comment and send an email to everyone subscribed without any additional helpful information alexander-bauer commented on Sep 13, 2024 For anyone arriving here in search of a workaround, my approach was to write this script to replace the docker script supplied through podman-docker, and install it as $HOME/.local/bin/docker.
To be clear, this is a horrible hack, but worked to get my hooks running in lieu of an upstream solution.
#!/bin/bash
[ -e /etc/containers/nodocker ] || \
echo "Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg." >&2
# Check for and drop -u flag -- workaround to https://github.com/pre-commit/pre-commit/issues/1243
args=() # declare array
while [[ "$#" -gt 0 ]]; do
if [[ "$1" == "-u" || "$1" == "--user" ]]; then
echo "Dropping arguments '$1 $2' before passing to podman" >&2
shift 2
else
args+=("$1")
shift 1
fi
done
exec /usr/bin/podman "${args[@]}"Reacted by 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) and Sam Morris zware commented on Jan 21, 2025 Just to note, I'm not convinced that the existing -u option passed by pre-commit is doing anybody any good. Even with it, I wind up with files created by a docker_image hook owned by either root (if using non-rootless Docker) or another random uid (using either podman with podman-docker or rootless Docker). In general, arbitrary images don't actually have the given uid/gid available anyway, unless explicitly added. Reacted by Sam Morris and Robin Schneider Reacted by anthony sottile asottile commented on Jan 21, 2025 for rootful it's definitely the correct thing to do. an image doesn't need a user configured to run as that uid and otherwise the files are owned as root
if you're ending up with root files then your image must be doing that intentionally kastl-ars commented on May 19, 2025 Sorry for chiming in. I tried to understand what was discussed in this issue and the many other discussions on this topic.
To me it seems like this is still a problem (which I am running into currently AFAICS). Is there a switch, configuration file option or similar yet to tell pre-commit that it deals with rootless podman?
(Using the workaround in #1243 (comment) works, but is a workaround...)
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|
