feat(docker): add docker and publish images to ghcr

[v1ct0rv]

Description

This is another proposal based on #374 and #174 but using Github Actions to build and publish the images, I added the Dockerfile, a sample docker-compose.yml and, as part of CD, build the image and push to DockerHub, so we will have the image with the latest code on main branch.

I also add documentation to use it.

[jef]

Before we start maintaining a Docker image, I'd prefer these changes go in for maintenance purposes.

Thanks!

Not approved, need changes

[jef]

Feel free to ask any questions or concerns.

[kaysond]

I would suggest changing the docker compose example to use a seccomp file like the one in #174 so you dont have to disable the chromium sandbox

[v1ct0rv]

Before we start maintaining a Docker image, I'd prefer these changes go in for maintenance purposes.

Thanks!

I applied your suggestions, thanks!

I would suggest changing the docker compose example to use a seccomp file like the one in #174 so you dont have to disable the chromium sandbox

I just remove docker-compose.yml in order to use directly in cli: docker run -it --rm --env-file ./.env ghcr.io/jef/nvidia-snatcher:nightly

[kaysond]

I just remove docker-compose.yml in order to use directly in cli: docker run -it --rm --env-file ./.env ghcr.io/jef/nvidia-snatcher:nightly

That has the same problem, as it requires the env vars to disable sandboxing. You could add something like
--security-opt seccomp=chrome.json to the command, if we copied the json from #174.

[jef]

I'm hearing you out @kaysond. I'm going to look at this and do some debugging. @v1ct0rv, thanks for all the changes. Let's try to get this sucker in.

[jef]

That has the same problem, as it requires the env vars to disable sandboxing. You could add something like
--security-opt seccomp=chrome.json to the command, if we copied the json from #174.

Okay, so from my testing, I suppose we could either do this, and have a chrome.json that we would potentially have to maintain or we can run as --privileged. I imagine the side effects are nearly the same, unless a dependency becomes malicious. But I doubt that happening. Especially even if we add in the chrome.json, there are a lot of lifted privileges.

I've also added in a user to circumvent some of the access.

[jef]

Alright @v1ct0rv, I'm going to push one commit up, but feel free to revert. Let's discuss and see if everyone is OK with the change.

https://github.com/jef/nvidia-snatcher/compare/feature/add-docker-support

Feel free to integrate directly into this PR @v1ct0rv.

[andrewmackrodt]

@jef this isn't working for me (Ubuntu host so possible selinux issue):

(node:1) UnhandledPromiseRejectionWarning: Error: Failed to launch the browser process!
Failed to move to new namespace: PID namespaces supported, Network namespace supported, but failed: errno = Operation not permitted

I'd prefer the BROWSER_TRUSTED environment variable being able to set --no-sandbox is preserved. The README advise to run --privileged is a bad recommendation, --cap-add SYS_ADMIN is a less overkill way to support sandboxing but I'd rather disable chromium's sandboxing than have either. A random npm dependency in the image shouldn't be able to change affect my host kernel configuration.

Edit: --privileged is a "shortcut" way to pass through all video cards and sound devices too, but at this point, you may as well just run the repository on the host directly.

[jef]

A random npm dependency in the image shouldn't be able to change affect my host kernel configuration.

100% agree, that's why I'm also not keen on chrome.json.

you may as well just run the repository on the host directly.

Also my thought. But I guess this is mostly for those who are running the app in some sort of cloud environment that aren't VMs, i.e. k8s (which I don't really think people are doing).

I'd prefer the BROWSER_TRUSTED environment variable being able to set --no-sandbox is preserved.

Not sure if this helps with at all, but not sure if I understand the sandboxing that Chromium is doing. That being said, it ran on my arch box. Probably an SELinux thing.

Not really sure the best direction to go here. I'd rather not host any Docker image and have someone else manage an image. I don't mind having someone update a wiki page of Docker images, but I'm not too sure if we want to manage this.

[kaysond]

I think it's worth pointing out that all of these are "runtime" options and don't need any changes to the Dockerfile. It's more a question of documentation. I would vote for documenting both choices. Either run with chromiums sandbox and the proper security options, or set the env var and turn off sandboxing. For the chrome.json file you could just include a link to that file from its original source (which I don't remember but can help find - I think it was docker documentation somewhere)

[v1ct0rv]

I think it's worth pointing out that all of these are "runtime" options and don't need any changes to the Dockerfile. It's more a question of documentation. I would vote for documenting both choices. Either run with chromiums sandbox and the proper security options, or set the env var and turn off sandboxing. For the chrome.json file you could just include a link to that file from its original source (which I don't remember but can help find - I think it was docker documentation somewhere)

Agreed, We can run either with --privileged or with --security-opt seccomp=chrome.json I tested on ubuntu and macos. I suggest to have it both options in the readme.

We can have more granular control with --security-opt seccomp=chrome.json but in most cases --privileged will be enough.

What do you think?

[andrewmackrodt]

--privileged is unnecessary, --cap-add=SYS_ADMIN is enough to enable chromium sandboxing.

Regardless, users can do whatever they want regarding their security choices and I would like --no-sandbox to remain an option. I'd rather forego the ability to isolate independent tabs from each other than run a container with hundreds of unknown packages from npm with minimal host isolation.

[v1ct0rv]

--privileged is unnecessary, --cap-add=SYS_ADMIN is enough to enable chromium sandboxing.

Regardless, users can do whatever they want regarding their security choices and I would like --no-sandbox to remain an option. I'd rather forego the ability to isolate independent tabs from each other than run a container with hundreds of unknown packages from npm with minimal host isolation.

--cap-add=SYS_ADMIN grants a smaller subset of capabilities to the container, compared to the --privileged switch, I will change then.

Added back the option to Skip Chromium Sandbox.

[jef]

Regardless, users can do whatever they want regarding their security choices and I would like --no-sandbox to remain an option

Do we have to enable sandboxing to run the Dockerfile? If so, maybe we should add config.docker to

	if (config.browser.isTrusted || config.docker) {
		args.push('--no-sandbox');
		args.push('--disable-setuid-sandbox');
	}

Haven't ran yet though, so just an assumption...

[andrewmackrodt]

Do we have to enable sandboxing to run the Dockerfile?

It's dependent on what privileges the user has allowed. If they're running with an appropriate security profile or using --cap-add=SYS_ADMIN then sandboxing will be supported. It's only cases where neither are set as a runtime argument that we would want to apply --no-sandbox.

[kaysond]

Do we have to enable sandboxing to run the Dockerfile?
I think its a question of what you want the "default" to be, but its completely arbitrary since users can change it when they run the container.

By default right now (I think), sandboxing is enabled, which requires users to either user the Docker option --cap-add=SYS_ADMIN or --security-opt seccomp=chrome.json. This is totally normal practice and there are plenty of containers that require additional permissions.

If a user wants to disable the sandbox, then they run with --env-file <file> or --env BROWSER_TRUSTED=TRUE

I would strongly recommend not having different defaults if you're running natively or in a container, i.e. run with sandboxing enabled unless specified by the env var, both in native and container environments.

[jef]

Thank you two for that explanation. And thank you @v1ct0rv for seeing this through. Appreciate everyone's help and discussion.

Alright, well, let's see how this thing goes...

[jef]

Annnndd... Actions are down right 😢