Not sure if this is a good idea, the reamde for fixuid says:

fixuid should only be used in development Docker containers. DO NOT INCLUDE in a production container image

Not sure why.

Not sure if this is a good idea, the reamde for fixuid says:

fixuid should only be used in development Docker containers. DO NOT INCLUDE in a production container image

Not sure why.

@nhooyr editors are generally used in development environment, unless someone deploys it on a server and makes it available to a larger user-base.

@nhooyr: Not sure why.

boxboat/fixuid#1

I'm cleaning this up to make fixuid configurable via an entrypoint.sh script and disabled by default. If a user enables (via env var), it will log the warning message about non prod use. The user can disable this warning if the risk is acceptable (also via env var). I am testing this out and will update the PR asap.

As a note, we believe the risk is acceptable in our environment, particularly with other controls we have in place, and the fact this will be a developer only tool

@ibnesayeed - appreciate the feedback, but if you check my latest commit in the Dockerfile, the VOLUME bind mount is pointed to /home/coder/project, as per the original in master. The WORKDIR is a different directory, where the entrypoint.sh is copied (and only used for that purpose). What's the issue?

@satlus: @ibnesayeed - appreciate the feedback, but if you check my latest commit in the Dockerfile, the VOLUME bind mount is pointed to /home/coder/project, as per the original in master. The WORKDIR is a different directory, where the entrypoint.sh is copied (and only used for that purpose). What's the issue?

Removing the mkdir command and only relying on WORKDIR has the following side effect as per an existing comment:

# We create first instead of just using WORKDIR as when WORKDIR creates, the user is root.

I added it back in, but per the commit history the only reason this was needed in the past was because the WORKDIR and VOLUME paths resolved to the same location - /home/coder/project. This is no longer the case (as it should be), and workdir is only used to house the entrypoint. There really is no need for this going forward IMO.

Funnily, in testing, without the mkdir, the WORKDIR was still created as the coder user. Here's an example from a first start with fixuid disabled:

coder@d13691a6f654:~$ cd /home/coder
coder@d13691a6f654:~$ ls -la
total 36
drwxr-xr-x 1 coder coder 4096 May 4 10:45 .
drwxr-xr-x 1 root root 4096 May 3 15:17 ..
-rw-r--r-- 1 coder coder 220 May 3 15:17 .bash_logout
-rw-r--r-- 1 coder coder 3771 May 3 15:17 .bashrc
drwxr-xr-x 3 coder coder 4096 May 4 10:45 .cache
drwxr-xr-x 3 coder coder 4096 May 4 10:45 .local
-rw-r--r-- 1 coder coder 807 May 3 15:17 .profile
drwxr-xr-x 4 coder coder 128 Apr 25 16:47 project
-rw------- 1 coder coder 1024 May 4 10:45 .rnd
drwxr-xr-x 1 coder coder 4096 May 3 21:08 workdir <== THIS
coder@d13691a6f654:~$ 

When fixuid is enabled it will recursively chown the home directory of the user, including workdir, making this even less important.

@satlus in that case I would get rid of that mkdir command and the comment that says why it was important. Personally, I know WORKDIR and some other Docker commands run in the scope of the user if a USER is declared in a prior instruction, but the comment was confusing me.

That said, I would perhaps COPY the entrypoint.sh file into /usr/local/bin/. This would:

• No need to create yet another directory specifically for this
• Minimize the chances being overwritten if the user chooses the mount the whole HOME directory inside
• The ENTRYPOINT instruction can be changed to ["dumb-init", "entrypoint.sh", "code-server"] without the need of absolute or relative path

@satlus in that case I would get rid of that mkdir command and the comment that says why it was important. Personally, I know WORKDIR and some other Docker commands run in the scope of the user if a USER is declared in a prior instruction, but the comment was confusing me.

That said, I would perhaps COPY the entrypoint.sh file into /usr/local/bin/. This would:

@ibnesayeed Good suggestions, implemented.

In testing I noticed that we'd have to provide explicit volume mounts for /home/coder/.cache and /home/coder/.local when enabling non-root use. This seems less than ideal from a usability perspective, and as it has the effect of writing transient data inside of the container by default. To mitigate this I went ahead and changed the VOLUME mount to export the entire /home/coder directory (and updated docker CLI run examples in the README). This seems like smarter way to ensure config and runtime data are separated from the container image appropriately.

Would definitely appreciate the committers commenting here - it seems this is getting a bit broader in scope than originally anticipated.

As an alternative solution to fixuid, you can differ the creation of the coder user and group to the entrypoint.sh script.

• Create the user based on a user id and group id environment variable (often seen: PUID, PGID) with default values of 1000 and 1000: PUID=${PUID:-1000}, PGID=${PGID:-1000}
• chown -R coder:coder /whatever/directory
• runuser -u coder -- code-server-command

As an alternative solution to fixuid, you can differ the creation of the coder user and group to the entrypoint.sh script.

• Create the user based on a user id and group id environment variable (often seen: PUID, PGID) with default values of 1000 and 1000: PUID=${PUID:-1000}, PGID=${PGID:-1000}
• chown -R coder:coder /whatever/directory
• runuser -u coder -- code-server-command

We looked at a similar strategy but it won't work if we want init.d in the container to be non-root (the entire point of passing -u to docker at runtime). Unless I'm missing something, this won't satisfy the requirement.

Any chance of this PR landing anytime soon? I'm facing this issue and would love to have a solution

@carlos-sarmiento from what info I can gather, PRs are on hold until we can merge v2. If you really want to test, chinodesuuu/coder:satlus is a feature parity replica of this PR.

@sr229 is there any idea on when v2 might land?

@sr229 is there any idea on when v2 might land?

v2 is quite in early stages, and barely feature-parity with v1, mainly because of the new VSCode Web architecture (I believe which is VS Online), you can check #857.

@satlus can you try opening a largish project? Does fixuid slow container startup down considerably? iirc that's why I remember not integrating it.

Happy to do some testing here, but I can see how fixuid's behavior could slow startup significantly (basically to recursively chown all files from /) . To mitigate this on the fixuid side we can restrict paths to recurse via fixuid's 'path' env or yaml config (https://github.com/boxboat/fixuid#specify-paths-and-behavior-across-devices), but if the project directory is large it may be a fixed cost we'd have to eat. Two things to consider (1) it's opt in via env var, in which case there is zero change to the default behavior, (2) if given the option of not running at all in a restricted environment, and having something work with slow startup time, some users may be willing tolerate the slow startup. We should at least warn users of the side effect in this case. That aside, I will find some time in the next few days to test things out. I'm also curious if after the initial fixuid chown we can get away without invoking it on subsequent container starts.

Our environment restricts docker execution to images that can only with a non root user docker run -u $UID.... This PR adds boxboat/fixuid to allow for runtime mapping of host->container UID

If you run with UID 1000, things should always just work as the image itself always sets itself to run with uid 1000 by default. You can always access root with sudo.

https://github.com/cdr/code-server/blob/22058c5f861d6f39bc42a768d5cb65ffdf696ed1/Dockerfile#L46

Is that not enough for your use case?

I guess what I'm really suggesting is ensuring your files/folders outside the container are also UID 1000 as that's the only real way to do this without jank with Docker.

The other being using user namespacing and explicitly mapping some external UID to UID 1000 inside the container. See https://docs.docker.com/engine/security/userns-remap/

I guess what I'm really suggesting is ensuring your files/folders outside the container are also UID 1000 as that's the only real way to do this without jank with Docker.

What if I am not user 1000 on the base system? I won't be able to read the files. What about systems with multiple users? Linuxserver,io has solved the issue using s6_overlay.

What if I am not user 1000 on the base system? I won't be able to read the files.

You're using docker, so you should be accessing the files inside the container. If you need to still access them outside the container, use sudo or start the container with the entrypoint as bash. It's the way docker works best right now.

@code-asher Let's cherry-pick this PR then pull a new PR that adds this PR's changes.

I'm gonna take care of this myself in the coming days.

Thank you @satlus and @ibnesayeed for your work.