Skip to content

Shifts Docker container from Rootless to Default Rooted behaviour - #89

Merged
JacksonTheMaster merged 11 commits into
nightlyfrom
fix-docker-permissions
Sep 21, 2025
Merged

Shifts Docker container from Rootless to Default Rooted behaviour#89
JacksonTheMaster merged 11 commits into
nightlyfrom
fix-docker-permissions

Conversation

@1Solon

@1Solon 1Solon commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

Fixes issues experienced by linux hosts caused by permissions associated with mounted volumes.

I tried several variations of init script- but without offering a helper to 'sanatize' the directory before install (an arguably anti-dockercentric approach) this is the most clean way of fixing the issue.

Copilot AI review requested due to automatic review settings September 19, 2025 20:42
@1Solon 1Solon added the enhancement New feature or request label Sep 19, 2025
@1Solon 1Solon self-assigned this Sep 19, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Switch Docker image from rootless to default root to resolve Linux host volume permission issues, and align CI to build from the .docker/Dockerfile.

  • Run container as root by removing non-root user in Dockerfile
  • Update docker-compose mounts and image tag; enable TTY/STDIN
  • CI workflows build from ./.docker/Dockerfile; minor LICENSE wording updates

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
LICENSE Update wording around “source-available” and commercial use clause
.github/workflows/ghcr-build-release.yaml Build uses ./.docker/Dockerfile explicitly
.github/workflows/ghcr-build-nightly.yaml Build uses ./.docker/Dockerfile explicitly
.docker/compose.yml Adjust image tag, enable interactive flags, change bind mounts to ${PWD}, keep named volume
.docker/Dockerfile Remove non-root user creation and USER, leaving process to run as root

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread .docker/compose.yml Outdated
Comment thread .docker/compose.yml Outdated
Comment thread .docker/Dockerfile Outdated
Comment thread .docker/Dockerfile
Comment thread .docker/Dockerfile
Comment thread LICENSE Outdated
Comment thread LICENSE Outdated
1Solon and others added 3 commits September 19, 2025 23:33
This reverts commit 86bb14b.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@JacksonTheMaster
JacksonTheMaster requested review from mitoskalandiel and removed request for JacksonTheMaster September 20, 2025 10:20
@JacksonTheMaster

JacksonTheMaster commented Sep 20, 2025

Copy link
Copy Markdown
Member

@mitoskalandiel maybe you can look into this, I thought about this for some time now but I just don't like the idea of running as root in the container. SSUI should not be run as root, a for security but b because of steamcmd, I'd like to see a different solution here. I'll look into this myself later, for now I do not want to merge this. We keep this around in case we don't find any nice solution other than running as root.

@1Solon

1Solon commented Sep 20, 2025

Copy link
Copy Markdown
Contributor Author

@mitoskalandiel maybe you can look into this, I thought about this for some time now but I just don't like the idea of running as root in the container. SSUI should not be run as root, a for security but b because of steamcmd, I'd like to see a different solution here. I'll look into this myself later, for now I do not want to merge this. We keep this around in case we don't find any nice solution other than running as root.

As I mentioned separately in VC, the issue here is that Docker (the most common container deployment) does not have a native solution to change the permissions on mounts, this means the container itself must change those permissions- this introduces a key problem, namely the container at runtime is the non-root user- and therefore lacks the permissions to change the permission of the directory.

There are three solutions to this, all poor:

  1. We change the compose to have a UID and GID, we then require the user to make the directories and change their permissions to that of the user. They then run the container. This solution would prevent the 'one click deployment' docker is meant for, it also forces our users to use the compose.

  2. We write a script which prepares the directories first, but this prevents us from running it in any kind of orchestrator, including Dockage, Portainer and Kubernetes. Given two of those are the most common use case for Homelab docker deployments, this is also not ideal.

  3. We make Podman necessary to run our container, Podman does have a native solution for this problem. The issue is that Podman is used by a much smaller group of container users, Docker and Containerd are far more popular.

The above problems could be solved by offering multiple versions of the container, but this creates significant increased complexity for a very marginal security benefit.

As I mentioned before in vc, this is a rooted container, I.E it's running inside a cgroup- it is not the same as 'running as root'. This presents only a minor security risk at best, and is a standard means of setting up a docker container.

@akirilov

akirilov commented Sep 20, 2025

Copy link
Copy Markdown
Contributor

As far a security perspective, the risk of running as root is it makes it easier to exploit any existing container breakout. This still requires that the attacker compromises the server, and has a container escape (so it's far from simple), and we can complicate this further by having a read-only container filesystem (working dir can be a RW volume) and dropping any capabilities we don't need (all?)

@mitoskalandiel

Copy link
Copy Markdown
Contributor

The issue isn't just one of SSUI alone. There is plenty of games for steamcmd, when run in containers, require either the usual root docker shenanigans, that we are doing currently, or require, as mentioned above, the use of specific UID/GID's for the directories owners, to make it work without root.
My thinking here is, why add added complexity for a very marginal security benefit? I personally agree with the assessment from @1Solon, that This presents only a minor security risk at best.
Ultimately this is also on the user to secure his system, so linux users/server admins will do this stuff already, and anyone just wanting "to run the damn server" will do this on windows anyways, and most likely NOT in a container, then :)
I don't think there is any real value to be added with this added complexity.

@akirilov

Copy link
Copy Markdown
Contributor

At the end of the day we need to balance security and usability, and I wouldn't call this a show-stopper, especially if the alternative is "the user gives up on our tool, and runs it on their own in a wildly misconfigured manner, also as root"

@mitoskalandiel

Copy link
Copy Markdown
Contributor

At the end of the day we need to balance security and usability, and I wouldn't call this a show-stopper, especially if the alternative is "the user gives up on our tool, and runs it on their own in a wildly misconfigured manner, also as root"

Agreed

mitoskalandiel
mitoskalandiel previously approved these changes Sep 20, 2025

@mitoskalandiel mitoskalandiel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with this as per previous discussion

@akirilov

Copy link
Copy Markdown
Contributor

That all said I'd still suggest tracking this in a backlog somewhere, I can only pray Docker eventually catches up to Podman and we can implement an update for this someday🙄

@JacksonTheMaster

Copy link
Copy Markdown
Member

Happy with this as per previous discussion

Talked to @mitoskalandiel for a few minutes again, he made me a believer.
@1Solon I didn't mean to strictly "disapprove" this as, as you all know, my Docker knowledge is somewhat limited so I wanted to share my thoughts and have seb review this. Thx!

@JacksonTheMaster

JacksonTheMaster commented Sep 20, 2025

Copy link
Copy Markdown
Member

The only thing I would like to see changed if possible is the following:

COPY --from=go-builder /out/StationeersServerControl /usr/local/bin/StationeersServerControl

We need to have the executable in the /app folder, because the current workdir detection changes from @akirilov (not yet on this branch) change the workdir of the go code to be wherever the executable is actually located to tackle on user fuckups when they run SSUI (outside docker) from another dir or from Linux desktop double click in file explorer.

If that's not feasible, we can add a check to @akirilov 's workdir check to not run in docker.

@1Solon would it be possible to run the executable if located in /app? Or would that introduce other weird issues?

@JacksonTheMaster

Copy link
Copy Markdown
Member

That all said I'd still suggest tracking this in a backlog somewhere, I can only pray Docker eventually catches up to Podman and we can implement an update for this someday🙄

Noted ✅

@1Solon

1Solon commented Sep 20, 2025

Copy link
Copy Markdown
Contributor Author

The only thing I would like to see changed if possible is the following:

COPY --from=go-builder /out/StationeersServerControl /usr/local/bin/StationeersServerControl

We need to have the executable in the /app folder, because the current workdir detection changes from @akirilov (not yet on this branch) change the workdir of the go code to be wherever the executable is actually located to tackle on user fuckups when they run SSUI (outside docker) from another dir or from Linux desktop double click in file explorer.

If that's not feasible, we can add a check to @akirilov 's workdir check to not run in docker.

@1Solon would it be possible to run the executable if located in /app? Or would that introduce other weird issues?

Is there a branch with this change being present? I can test the changes

@JacksonTheMaster

Copy link
Copy Markdown
Member

@1Solon kinda, on my fix-lin-steamcmd-writing-to-home i added a loader.Sanitycheck that uses the logic, just gotta comment out the root check

… to app since /app is a volume and would be empty else

- renamed all instanced of StationeersServerControl in docker related files to StationeersServerUI

@JacksonTheMaster JacksonTheMaster left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as @mitoskalandiel reviewed this already, I am merging this to nightly with slight changes:
Fixed an issue where the /app dir would stay empty because it gets created as empty on the host (intended) and then mounted into the container - still beeing empty - when we then try to run the executable from app it wont be there. Resolved this with a entrypoint script that copies relevant files from /opt/SSUIBuildFiles/StationeersServerUI into /app before actually starting up.

@JacksonTheMaster
JacksonTheMaster merged commit ca92d62 into nightly Sep 21, 2025
1 check passed
@JacksonTheMaster
JacksonTheMaster deleted the fix-docker-permissions branch September 21, 2025 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants