wp-env runs docker commands with same UID as host to prevent filesystem permission issues

[Luc45]

What?

Pass the UID of the user running wp-env down to the docker-compose run instance.

Why?

Files created inside the docker instance have permission issues since they are created with user 0:0 (root).
Closes #42866

How?

Files created inside the container will have the same UID as the host.

Testing Instructions

I have not tested this yet. Looking into it now, any help here would be appreciated.

Screenshots or screencast

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Luc45! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[Luc45]

[Luc45]

Perhaps it makes sense to hide this logic behind a feature flag, as I fear it might cause permission issues amongst the services running inside the docker image itself, eg: php-fpm can't read the file, etc?

So this behavior could be triggered if a env var such as WP_ENV_RESOLVE_UID is present, which can be set at runtime for a single command, or persisted in an environment through .bashrc or similar.

Similarly, another env var could be added to explicitly set a UID:GID, which can only add to the flexibility of wp-env in all use-cases, eg: WP_ENV_UID=1000:1001 wp-env run test-cli id

[Luc45]

If mapping the UID with host works reliably for a while using the feature flag, then it could be made the default behavior

[Luc45]

Just writing this down to keep track of it, this note will need to be updated in this PR before merge, after it has been settled how it should work: https://github.com/WordPress/gutenberg/tree/trunk/packages/env#installing-a-plugin-or-theme-on-the-development-instance

[noahtallen]

So I think this just changes the user when running something like npx wp-env run wordpress bash. I don't think this change would impact permissions as wp-env starts up. 🤔

Another change I was just working on in #42826 will use a git checkout for the main WordPress version rather than whatever Docker provides. I wonder if that would help the permissions issue as well...

I also experimented with file permissions in #40864.

But I was unfortunately never able to replicate the bug (even on Linux!), so I'm not sure what the best path is forwards! (I'm also curious if the steps on this page helped at all with permissions issues: https://docs.docker.com/engine/install/linux-postinstall/)

Could you describe the issues you're seeing?

[Luc45]

There is some scripting involved, so it's hard to get an accurate step-by-step, but it goes something like this:

• wp-env start
• wp-env run tests-cli "wp theme install twentynineteen --activate"
• wp-env destroy
• <file permission issues>

Even though, I could not reproduce it neither running these commands in a clean wp-env (this repo).

I'm not sure when the permission errors kicks-in, as it shouldn't happen if the destroy also happens inside the container. But if the destroy calls rm -rf on the host, then it should have permission errors.

This is the error that I can reproduce when using wp-env integrated with our tooling:

Is that call to rm -rf plugins happening on the host machine? If so, that's a likely cause of permission issues

[Luc45]

Ops, my bad. rm -rf plugins is called on our end of the tooling.

This issue could theoretically be closed as a non-issue, but it's nice to have the UID mapping being done correctly in Linux as well.

[Luc45]

I've seen the #40864, I believe that mapping the host UID with the UID running inside the docker container is the most appopriate solution, as per this PR.

I believe that adding this UID mapping behind a feature flag, and optionally providing the ability to override Docker's UID with a environment variable will be enough to resolve the permission issues, as someone could do this if needed:

WP_ENV_UID=$(id -u):$(id -g) ./bin/wp-env run tests-cli "wp plugin download woocommerce"
("woocommerce" files will have correct permissions between host and Docker)

The Docker Desktop VM for Mac and Windows does something similar to this behind the scenes, but it's automatically managed for you, in Linux it has to be explicit. But if the docker-compose run call is behind an abstraction, such as wp-env's one, then the abstraction should allow to passthrough these arguments, ideally.

[Luc45]

A cleaner approach would be to add support for a -u or --user flag, mimicking docker-compose

[noahtallen]

Good points! I also like the idea of keeping the user information consistent.

The reason I'm not sure this PR completely solves it is because when wp-env starts up, it's running docker compose commands directly, without using the built-in run command. The run command modified in this PR only changes the user settings for npx wp-env run .... But when everything is installed with npx wp-env start, it's still using the default settings for everything.

[Luc45]

Circling back to this issue, we can easily add a --user or -u parameter so that wp-env can relay it to the underlying docker-compose run command, which should be a simple change.

The deeper problem is that, as @noahtallen said, it doesn't solve the permission errors of operations that run outside the context of docker-compose run, such as mapped volumes.

To fix the permissions that are set at this earlier stage of docker bootstrapping, the only way I know is using https://github.com/boxboat/fixuid. We have an implementation example in this Dockerfile, and cd-entrypoint.sh.

To do this for wp-env, we could override the entrypoint of the WordPress Docker images in the generated docker-compose.yml, to download fixuid, apply the fix, and hopefully call the parent entrypoint to preserve the original functionality of the image that we are extending.

I think that adding the --userparameter is a simple change that we could do now, and we could look into implementing fixuid dynamically in the docker images for a more robust solution that takes care of volume mappings as well in the near future?

[noahtallen]

I think that adding the --use rparameter is a simple change that we could do now, and we could look into implementing fixuid dynamically in the docker images for a more robust solution that takes care of volume mappings as well in the near future?

True! I wish I could confidently say yes to this, but I'm unfamiliar enough with how --user works that I just want to be cautious to not introduce any new issues. (Permissions have been a minor, if persistent, issue for wp-env!) Is there a reasonable possibility this could cause any regressions for existing setups, or do you think it'll only make things better?

[Luc45]

Adding that parameter doesn't change any existing behavior, it only gives more control to those who need it, if and when they need it

[Luc45]

Fixiuid on the other hand is a more dramatic change that can break stuff

[noahtallen]

I was doing a tiny bit of searching about the issue and came across this: https://medium.com/redbubble/running-a-docker-container-as-a-non-root-user-7d2e00f8ee15 (and the official docs here: https://docs.docker.com/compose/compose-file/#user)

This mentions inserting the user into the docker-compose file with an environment variable. Given we use docker-compose for pretty much the entire thing, I wonder if that would solve the issue?

[Luc45]

I was doing a tiny bit of searching about the issue and came across this: https://medium.com/redbubble/running-a-docker-container-as-a-non-root-user-7d2e00f8ee15 (and the official docs here: https://docs.docker.com/compose/compose-file/#user)

This mentions inserting the user into the docker-compose file with an environment variable. Given we use docker-compose for pretty much the entire thing, I wonder if that would solve the issue?

I wouldn't go that route for two reasons:

It changes existing behavior, whereas a --user parameter only extends it.

But more importantly, I think it would break stuff. Let's suppose that the services spinning up inside docker expects to run as user www-data (UID 1000 inside the container), and that the user I'm currently using in the host has UID 1001.

The issue is that the UID in the host should match the UID in the container to avoid permission issues, so if I spin up the docker container with UID 1001 using that parameter, any filesystem operations that happen inside Docker will happen under this UID, which is perfect for the host operating system, but since the files inside the container are owned by www-data (1000), then we don't have write access: https://github.com/docker-library/wordpress/blob/master/latest/php7.4/fpm/Dockerfile#L132

That's where FixUID comes in play. It maps www-data inside the container to the same UID of the host user lucas, so that to all effects, the files are written by the same user:

https://github.com/boxboat/fixuid/blob/master/fixuid.go#L134

File ownership is based solely on UID. The OS doesn't care if that UID is assigned to lucas or www-data. As long as the files are owned by the same UID, it will work.

[Luc45]

This mentions inserting the user into the docker-compose file with an environment variable.

In other words, using this approach we only transfer the permission issues to Docker instead of having them in the host.

[noahtallen]

Ah, I see. Thanks for clarifying! The one thing I still don't understand is how passing -u to the docker-compose run command avoids those problems. 🤔

[Luc45]

Ah, I see. Thanks for clarifying! The one thing I still don't understand is how passing -u to the docker-compose run command avoids those problems. 🤔

You're right, it doesn't. The only real solution is mapping the UID between host and docker so that they match. Everything else only solves part of the problem.

However, the -u flag gives control to the caller, so that they can pick their poison. I don't have access to a Linux machine now, but we could check later if this creates a file owned by root: wp-env run tests-cli touch foo. If it does, this would be a great use-case for the -u/--user flag: wp-env run tests-cli --user=www-data touch foo

[noahtallen]

Right. I suppose the next question is since this won't solve the entire problem, should we just optionally pass through -u or --user to docker-compose from wp-env? The current PR would always said the user to the OS user, and I'm worried that wouldn't necessarily be compatible since it doesn't solve the whole problem 🤔

[Luc45]

should we just optionally pass through -u or --user to docker-compose from wp-env?

Exactly.

The current PR would always said the user to the OS user, and I'm worried that wouldn't necessarily be compatible since it doesn't solve the whole problem

Indeed, optionally passing -u/--user down to the underlying docker-compose seems a better implementation in this scenario.

I can look into it soon, but feel free to do it as well if you have the bandwidth! 🙌

[noahtallen]

Cool! Glad we have a path forward. I don't have time to look at it immediately unfortunately

[defunctl]

Heya @noahtallen,

Any movement on this?

Basically every Linux user (and I assume WSL2 Windows users) will continue to have improper permissions in the docker container whenever the web server user writes a file or creates a directory or basically executes anything that touches the file system under the www-data user.

I'd be happy to submit a PR that would fully address this and would work on Linux/Mac/Windows. In addition to the changes this PR would address, we could also:

1. Detect the user's UID/GID and pass those to an updated docker-compose.yml user: definition in addition to setting the proper Apache user/group environment variables..
2. Update the Dockerfile to run as the current user, or as @Luc45 mentioned, fixuid is a great solution, which would be simple to add to the generated Dockerfile.

This way, all the files are owned by same UID/GID as the host user, allowing the user to create new files on the host without permission issues and any filesystem operations run inside the container would be executed as their UID, keeping newly created files/folders editable under the host user, basically ensuring all files inside and outside of the container are owned by the same user.

Let me know, would really love to get this solved for all the Linux users out there.

[noahtallen]

Hey @defunctl! I apologize for being the bottleneck here -- wp-env unfortunately isn't part of my main responsibilities, so I can only look at it when I get bits of time here and there.

I would definitely love to see a PR fully addressing this! If you come up with a solution, I'd be happy to move it to the top of my list of things to look at :)

As an aside, the main reason I haven't been thinking of this as a huge priority is that wp-env works ok on Linux -- for example, all of the automated test suites in GitHub actions are running on top of wp-env in a Linux environment. (and it worked ok when I was daily driving Linux last year.) So I was never able to make progress on the issues, especially since I wasn't able to reproduce the bug reports.

But I would love to see this fixed, and if you know more about this space and are able to reproduce the various issues, that sounds great!

[defunctl]

Hey @defunctl! I apologize for being the bottleneck here -- wp-env unfortunately isn't part of my main responsibilities, so I can only look at it when I get bits of time here and there.

I would definitely love to see a PR fully addressing this! If you come up with a solution, I'd be happy to move it to the top of my list of things to look at :)

As an aside, the main reason I haven't been thinking of this as a huge priority is that wp-env works ok on Linux -- for example, all of the automated test suites in GitHub actions are running on top of wp-env in a Linux environment. (and it worked ok when I was daily driving Linux last year.) So I was never able to make progress on the issues, especially since I wasn't able to reproduce the bug reports.

But I would love to see this fixed, and if you know more about this space and are able to reproduce the various issues, that sounds great!

Hey @noahtallen mind pointing me to exactly where the this is being used in automated tests in GitHub actions? I'll have a look. Perhaps this test case isn't even covered, or just everything is fully running as a root user from host to container etc...

Thanks!

[noahtallen]

For sure! That's totally possible. Here's the line where it happens:

gutenberg/.github/workflows/end2end-test.yml

Line 45 in 79103f1

npm run wp-env start

And then the e2e suite (https://github.com/WordPress/gutenberg/tree/79103f124925d1f457f627e154f52a56228ed5ad/packages/e2e-tests) interacts with the WordPress site wp-env provides through a browser-like testing environment.