run: flag to include the docker socket #5858

stevvooe · 2025-02-21T22:47:48Z

Adds a flag, --use-api-socket that can be used to start a container with the correctly configured parameters to ensure that accessing the docker socket will work with out the fiddly flags.

There are a few problems with this approach:

We need a reliably way to clean up the configuration file. This currently is put into a tmp file then bind mounted. There is probably a better way to do this such as copying in the file.
We need a way to resolve the correct socket outside the container. If a different socket is used or a address and port, this will attempt to bind mount a nonexistent socket.

Either way, this is good start and resolves a long standing issue.

To try it out, you can do the following:

❯ ./docker run --rm -it --use-api-socket docker:cli
/ # docker pull redis
Using default tag: latest
latest: Pulling from library/redis
Digest: sha256:93a8d83b707d0d6a1b9186edecca2e37f83722ae0e398aee4eea0ff17c2fad0e
Status: Image is up to date for redis:latest
docker.io/library/redis:latest

This PR is somewhat incomplete and needs the following:

Address credentials leak
Address socket address location
Implement unit and e2e tests
Update man pages and docs

- Experimental: add a new `--use-api-socket` flag on `docker run` and `docker create` to enable access to Docker socket from inside a container and to share credentials from the host with the container.

codecov-commenter · 2025-02-21T22:49:23Z

Codecov Report

Attention: Patch coverage is 15.09434% with 90 lines in your changes missing coverage. Please review.

Project coverage is 58.88%. Comparing base (1adc158) to head (1a502e9).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5858      +/-   ##
==========================================
- Coverage   59.05%   58.88%   -0.17%     
==========================================
  Files         357      357              
  Lines       29843    29941      +98     
==========================================
+ Hits        17624    17632       +8     
- Misses      11241    11329      +88     
- Partials      978      980       +2

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

stevvooe · 2025-02-21T23:17:01Z

Alright, I just validated an approach to the credential leak issue using copy into the container. Let's validate the cli ux and then we can add that implementation if we want to go forward with the change.

thaJeztah · 2025-02-24T11:38:40Z

This is a tricky one; bind-mounting the docker socket by setting these options from the CLI will break in many scenarios. The current approach in this PR uses the default location (/var/run/docker.sock), which would work with a default config for a Linux daemon, but won't work for a Windows daemon (which currently uses a named pipe as default). A similar case would happen if the daemon is local, but running rootless, in which case the socket would be in XDG_RUNTIME_DIR (if set). In a non-default setting, the daemon can be configured to use a socket on a different location (or no socket at all if the daemon is configured to only list on TCP). The tricky bit here is that when connecting with a remote daemon, the CLI won't have the information where the socket is located.

For a daemon running locally (same machine), the CLI could have slightly more information, and look at the docker host configuration in the docker context used;

docker context inspect | jq .[].Endpoints.docker.Host
"unix:///var/run/docker.sock"

In general, these could be somewhat tricky even with the good location, as there's a potential race condition where the container is started before the API server (and thus socket) is up, in which case using the -v shorthand (which implicitly has "auto-create host path" enabled) can result in /var/run/docker.sock being created as a directory if the path is missing (which could prevent the daemon from finishing startup).

I recall some older proposals to have an option on the daemon side to grant a container access to the API; for those, the daemon would handle setting up a socket and mounting it into the container. Those were a really long time ago, and IIRC the initial implementation was to do so automatically for any container with --privileged, but there was discussion to have a separate option for this (because forwarding the socket into a --privileged container could interfere with DIND cases, which would have their own socket inside the container from the daemon running in it);

I think something along those lines (i.e., having an option to forward the socket, and have the daemon set this up) could solve most of the issues mentioned earlier; the daemon would be able to set up the socket for the container, regardless how it was configured, which could even mean a dedicated socket for the container (which could potentially be expanded to more granular / limited access to the socket, instead of access to all endpoints).

Slightly related (forwarding a socket from the client to the daemon to allow ssh-agent-like constructs for forwarding into the container); this could possibly be tying in to the "forward authentication" part;

[WIP] Add forwarding local (client-side) sockets to container moby/moby#19667

thaJeztah · 2025-02-24T11:45:52Z

For the credentials; potentially we could use some env-var, although env-vars may not be ideal as they will leak both into the container's config (docker container inspect), and any process inside the container. Forwarding "all credentials" is what's currently used by the classic builder, which encodes them in a X-Registry-Config header that contains all credentials for all registries the CLI is aware of; https://github.com/moby/moby/blob/0de01f700d9040f70d71d16fff2ea4aafc4af36b/api/server/router/build/build_routes.go#L335-L348

Possibly we can make the CLI aware of an env-var along those lines; IIRC, BuildKit requests auth from the CLI when building, so not 100% sure if that would work in that combination, and likely all CLI plugins would have to be updated to be aware of this approach.

laurazard · 2025-02-24T17:36:23Z

Tangential, but regarding:

... reliably way to clean up the configuration file. This currently is put into a tmp file then bind mounted. There is probably a better way to do this such as copying in the file.

Ultimately, this should be unnecessary and we should make the necessary changes to stop the "credentials in ~/.docker/config.json" case so common. i.e., stop storing credentials together with CLI configurations (in a non-breaking way, and keep accepting those), either by just storing them elsewhere or using a "plaintext credential-helper", which would also solve a number of other issues.

stevvooe · 2025-02-24T18:36:50Z

@laurazard Yes but unfortunately, there is no way to call the credential helps from inside the container. We could have an ssh-agent like call api for it but that would require an interactive client to that to work correctly.

@thaJeztah It sounds like I should at least use a mount rather that volume notation to avoid the accidental file creation.

laurazard · 2025-02-24T23:35:59Z

Right, that's definitely true. I think there's two separate issues that folks frequently run into that we should keep in mind:

wanting to easily expose the docker socket inside of a container, to use, without sharing all credentials
- this is annoying because other necessary configurations live in the same config.json file
wanting to share some/all credentials for use inside a container
- this commonly leads users to use the plaintext store instead of a credential helper, since these are easier to share with a container

It'd be awesome if whatever solution we came up with could account for/solve both cases (and splitting out creds – even if just to another file – from config.json would already help).

stevvooe · 2025-02-25T01:32:26Z

@laurazard Looking at compose, they actually use the same technique that I've employed here: https://github.com/docker/compose/blob/145bb8466dffd18d7828ca0bc0725b1059506098/pkg/compose/secrets.go#L31. The secrets are copied over to /run/secrets. It looks like that is a tmpfs and the copy is done after start. This PR is a little problematic in that they are copied into the container's read-write layer. My notes on that are in https://github.com/docker/cli/pull/5858/files#diff-50fe347eb4bdafebc190aa42b2caa34b089a538cd444b8a151a763372e93c0cfR269. Basically, I'll have to start the container to employ this correctly, so we won't have it on create, which is probably ok.

Here's my proposal:

I'll move the this over to the /run/secrets tmpfs mount to ensure the secrets don't leak to the overlay filesystem. The copy will be done post container start but before we hand terminal the success off to the user.
I'll add some logic to better resolve the socket location and move to mounts to better control how it appears in the container.
We can mark this flag as experimental. (no clue to do this, direction needed)
I'll rally the troops a bit here to see if we can get a session-callback proposal in place or at least first-class support for a secret mounts, similar to what's happening buildkit.

I know this isn't ideal but this has been a long-lived pain point. The abstraction of the --use-docker-socket flag is high-level enough to where we can improve the credential access over time without breaking users, especially if we maintain it as an experimental feature.

I hope this is reasonable and can allow us to move forward.

stevvooe · 2025-03-19T22:10:21Z

A few fixes that have been put into place:

The secrets are now in /run/secrets to follow the compose convention. We should probably use tmpfs there but compose doesn't do this today so we can leave it be. Ideally, we'd have /run be tmpfs and just have that mount at create time so we can have the convention of dumping secrects or whatever in there.
The socket address now resolves from the context.
I'll be updating tests once this CI runs and fails.

I'll need guidance on marking the flag as experimental.

Benehiko · 2025-03-21T15:22:08Z

I've created a POC to instead share credentials over a unix socket which we could bind mount into a container. I don't know if that's the correct approach or if it's something that would be better than what is inside this PR. #5948

stevvooe · 2025-03-25T15:37:49Z

@Benehiko We do have discussions around the best way to allow secrets access from within a container but we don't want to block this addition too much longer. We can always change the implementation to use such a system in the future.

Let's keep the discussion on your PR going and work from there.

cpuguy83 · 2025-03-25T16:44:39Z

Seems like this should be a feature on the engine rather than client side.
Maybe something like --mount type=api-socket.
Not sure how well this would work on the Windows side, though.

stevvooe · 2025-03-25T16:47:09Z

Seems like this should be a feature on the engine rather than client side.
For some implementations, client participation is required.

I like the idea of a mount long term but we'd have to teach every proxy about it. Plumbing for /var/run/docker.sock mostly already exists.

stevvooe · 2025-03-25T16:59:11Z

@cpuguy83 Do you want me to change the flag to the more general --use-api-socket?

cpuguy83 · 2025-03-25T17:10:32Z

Either way for the flag is fine for me.

Benehiko

To me everything except the credentials part looks alright. I know we have already discussed on the PR a bit about it. If this is critical to get in now then I would be okay with merging as is, but we would end up having credential sync issues:

On credential expiration
Re-authenticating on the Host would completely invalidate any credentials in the container
Currently CLI doesn't really use the OAuth Access and Refresh tokens - except for fetching a PAT. If this were to change, token refreshes would also invalidate container credentials.

stevvooe · 2025-04-02T02:30:36Z

@Benehiko Indeed, those are the current limitations we have here. I hope not to leave it in this condition. Thanks for your review!

I'll added some tests before the end of this week.

slimslenderslacks

@stevvooe the other thing about the run command line is that this won't immediately be available to use in compose without it also supporting this option. Currently, the container that needs this on the MCP gateway side is being started from a compose.yaml file inside of a Desktop extension. So I'm assuming that Desktop extensions just use compose to launch this container. Does that make sense?

stevvooe · 2025-04-14T23:01:22Z

@slimslenderslacks Yes, that would be a current limitation. We could port a similar implementation to compose. The secret machinery already uses the same technique as this PR so it should be straightforward.

Benehiko · 2025-04-15T06:32:56Z

cli/command/container/create.go

+		containerCfg.Config.Env = append(containerCfg.Config.Env,
+			"DOCKER_CONFIG="+path.Dir(dockerConfigPathInContainer))
+	}


I think we should rather use the DOCKER_AUTH_CONFIG here and push the auth configs into the environment variable instead (as discussed with @thaJeztah).

It's already a commonly used variable among TestContainers and GitLab. They advise to use it and then write the contents of the environment variable to a file (~/.docker/config.json).
See:

https://docs.gitlab.com/ci/docker/authenticate_registry/#option-3-use-docker_auth_config

https://github.com/testcontainers/testcontainers-go/blob/6d13d7a72bf5d1f15002e8c0b4d88ea8b88138f3/docker_auth.go#L262-L282

I have a PR open which would use this natively here #6008

Do any existing docker clis support this env var? This seems like a nice addition but can be done later.

thaJeztah · 2025-04-15T12:58:20Z

cli/command/container/create.go

+
+		// Set our special little location for the config file.
+		containerCfg.Config.Env = append(containerCfg.Config.Env,
+			"DOCKER_CONFIG="+path.Dir(dockerConfigPathInContainer))


We need to find a solution for this part; the DOCKER_CONFIG option not only controls the location of where the CLI looks for the config-file, but (currently) all state of the CLI, including contexts, cli-plugins, and state stored by various plugins, such as buildx and compose 😞. Setting a custom path would likely result in things breaking in containers that have some config and/or state set up.

Can you demonstrate a container that breaks today? With the docker:cli image, there is no existing config in place and it seems like we could easily work around this, if needed.

At least the docker:cli plugin will now use /run/secrets/docker/ for all its state, e.g. things like buildx will now save build-history there; https://github.com/docker/buildx/blob/eb74b483bd991c9afb5e0661d3c182266bf83153/util/confutil/config.go#L62-L67

I don't know about other containers and what they have configured.

docker:cli is a container image with the cli inside.

I'm not sure I see the problem here. Do we want to maintain the existing path in the home directory then?

I'm not sure I see the problem here. Do we want to maintain the existing path in the home directory then?

It's part of a general problem we already have; ~/.docker/ has grown to become a bit of a kitchen-sink of things, and no longer is just the CLI's config, but various plugins started to use it for <who knows what>; Docker Desktop stores various bits there, including sockets, and at some point, docker scout used it as cache, which ended up with multiple GB's of data that should've been temporary (I think that was fixed, and it now uses somewhere in TEMP, but it still stores SBOMS and other bits there).

After running a couple of builds from within a docker:cli container with this PR;

C /run A /run/docker.sock A /run/secrets A /run/secrets/docker A /run/secrets/docker/.token_seed A /run/secrets/docker/.token_seed.lock A /run/secrets/docker/buildx A /run/secrets/docker/buildx/.buildNodeID A /run/secrets/docker/buildx/.lock A /run/secrets/docker/buildx/activity A /run/secrets/docker/buildx/activity/default A /run/secrets/docker/buildx/current A /run/secrets/docker/buildx/defaults A /run/secrets/docker/buildx/instances A /run/secrets/docker/buildx/refs A /run/secrets/docker/buildx/refs/default A /run/secrets/docker/buildx/refs/default/default A /run/secrets/docker/buildx/refs/default/default/lqga45irm59xvgavd4tntqcft A /run/secrets/docker/buildx/refs/default/default/ovfmov0i5o63umkyhrhbmhpyu A /run/secrets/docker/buildx/refs/default/default/w3m8ggkzghh1ivuunw1fl8ct3 A /run/secrets/docker/buildx/refs/default/default/1xf73waihbtkr53ijk716v8zp A /run/secrets/docker/buildx/refs/default/default/6wmh6xclpvdzsrvkwgn4gdqp5 A /run/secrets/docker/buildx/refs/default/default/l15tkm6r6kwpntvvcrqgwhkfv A /run/secrets/docker/config.json

thaJeztah · 2025-04-15T14:29:00Z

For the "docker socket" part and setting DOCKER_CONFIG; I do not want to couple those efforts too tightly to this work in the short term; both have some problematic parts that should be tackled before we paint ourselves in a corner.

Some considerations;

We can implement the CLI feature to natively support DOCKER_AUTH_CONFIG; this would allow docker:cli to use auth from that env-var directly
If other tools need credentials, they can printenv DOCKER_AUTH_CONFIG > ~/.docker/config.json
We should also add an option to the CLI to dump the credentials in the format DOCKER_AUTH_CONFIG expects)
☝️ once DOCKER_AUTH_CONFIG is documented, we can expect other tools to adopt it though, so if those tools want to use auth without on-disk store, they can use that.
👉 I should add that env-vars are not "great" in many ways (but used widely); this work should not be the final state, and we continue to look at the (more general) auth work.

How to set the DOCKER_AUTH_CONFIG"? I'm considering that (short term) we can add a --security-opt for this that's handled by the CLI; e.g.;

--security-opt forward_auth=env set the auth-confirmation as DOCKER_AUTH_CONFIG environment on the container
(TBD); if we do want the option to write the config to a file instead it could be --security-opt forward_auth=file (using the logic from this PR)
☝️ this would provide "write a config file into the container" to be natively supported, without the user having to do so manually, and even if the CLI inside the container is either "old" or there is no docker CLI at all

stevvooe · 2025-04-15T14:55:40Z

@thaJeztah I think these are good suggestions for a follow up but not sure that the suggestions impact this PR. Let's get this merged and we can follow up with improvements.

thaJeztah · 2025-04-15T15:14:55Z

We can mark this flag as experimental. (no clue to do this, direction needed)

For the experimental annotation, you can use the same as for this flag (with s/tree/use-api-socket/);

cli/cli/command/image/list.go

Line 66 in 1adc158

flags.SetAnnotation("tree", "experimentalCLI", nil)

Can you squash / cleanup commits, and it looks like make -f docker.Makefile mddocs is needed to regenerate / format the markdown docs.

thaJeztah

Overall this looks ok; left two comments; let me know if you want to look at those as part of this PR or separate

thaJeztah · 2025-04-15T17:29:56Z

cli/command/container/create.go

+	if options.useAPISocket {
+		creds, err := dockerCli.ConfigFile().GetAllCredentials()
+		if err != nil {
+			return "", fmt.Errorf("resolving credentials failed: %w", err)
+		}
+
+		// Create a new config file with just the auth.
+		newConfig := &configfile.ConfigFile{
+			AuthConfigs: creds,
+		}
+
+		if err := copyDockerConfigIntoContainer(ctx, dockerCli.Client(), containerID, dockerConfigPathInContainer, newConfig); err != nil {
+			return "", fmt.Errorf("injecting docker config.json into container failed: %w", err)
+		}
+	}


Not a blocker, but perhaps something we should look at after this; both these errors are returned after the container was created, so potentially the user would end up with a container that was created, but the CLI won't return its ID.

I realize this combination of "create container" and "provision it with configs" is never gonna be an atomic, operation, but we could possibly put the dockerCli.ConfigFile().GetAllCredentials() early in this function, so that we don't attempt to create a container if we're gonna fail (because we can't read the credentials).

The "copy credentials to the container" of course requires the container to be present, so that must be kept at the end; we could consider;

still returning the containerID (to provide a reference to what's created

(TBD); print the failure as a warning and proceed (but this may not be ideal if the create was part of a docker run, as we'd be mixing our warnings with container output).

Pushed up cred resolution but its unclear where this cid file is being written.

thaJeztah · 2025-04-15T17:33:23Z

cli/command/container/create.go

 	for _, w := range response.Warnings {
 		_, _ = fmt.Fprintln(dockerCli.Err(), "WARNING:", w)
 	}
-	err = containerIDFile.Write(response.ID)
-	return response.ID, err
+	err = containerIDFile.Write(containerID)


Slightly related; looks like we may be discarding this error if failing to write to a container-id file failed, and something fails below.

We return it below but first try to resolve and write the creds.

Yeah, it would likely be a corner case; I was looking at these lines, where we don't return the containerID, and don't include the error about the container id-file being written.

if err := copyDockerConfigIntoContainer(ctx, dockerCli.Client(), containerID, dockerConfigPathInContainer, newConfig); err != nil { return "", fmt.Errorf("injecting docker config.json into container failed: %w", err) }

We could do an errors.Join or something along those lines, but not sure if that's super great, so let's keep that for follow-ups.

Doing the "copy to container" as part of docker start was something that crossed my mind, but not sure if we want to / can update the files before start (that may open up possible race-conditions as well).

Do you know why we write the containerID to a file on create? That's a bit unexpected. I tried to keep a light foot through here.

Oh! It's not written to a file by default (that function's signature is a bit misleading); it's only written to a file if the uses passes the --cidfile=<some path> option.

If the flag isn't set, nothing is written and the function always returns nil

I'm not a huge fan of that feature, but I think it was added for some users that wanted to automate things and/or start containers through some init system (not a good fit)

Also see; --cidfile fails if file exists even if empty #5954

Adds a flag to the create and run command, `--use-api-socket`, that can be used to start a container with the correctly configured parameters to ensure that accessing the docker socket will work with out managing bind mounts and authentication injection. The implementation in this PR resolves the tokens for the current credential set in the client and then copies it into a container at the well know location of /run/secrets/docker/config.json, setting DOCKER_CONFIG to ensure it is resolved by existing tooling. We use a compose-compatible secret location with the hope that the CLI and compose can work together seamlessly. The bind mount for the socket is resolved from the current context, erroring out if the flag is set and the provided socket is not a unix socket. There are a few drawbacks to this approach but it resolves a long standing pain point. We'll continue to develop this as we understand more use cases but it is marked as experimental for now. Signed-off-by: Stephen Day <stephen.day@docker.com>

thaJeztah

LGTM

as discussed, we're gonna bring this one in as a starting point; much more things to fix around this, which is why we added the "experimental" annotation (as the implementation will likely change in future).

thaJeztah · 2025-04-15T20:40:52Z

@Benehiko @cpuguy83 PTAL

Benehiko

LGTM

Benehiko requested a review from a team February 24, 2025 11:26

stevvooe force-pushed the sjd/include-docker-socket branch from 8cf394b to f4d4426 Compare March 19, 2025 22:13

stevvooe requested a review from thaJeztah as a code owner March 19, 2025 22:45

Benehiko mentioned this pull request Mar 21, 2025

Share CLI credentials over a unix socket #5948

Draft

Benehiko approved these changes Mar 25, 2025

View reviewed changes

slimslenderslacks reviewed Apr 14, 2025

View reviewed changes

This was referenced Apr 14, 2025

services: add api-socket to service volumes types compose-spec/compose-spec#587

Open

volumes: support for api-socket volume type docker/compose#12746

Draft

Benehiko reviewed Apr 15, 2025

View reviewed changes

thaJeztah reviewed Apr 15, 2025

View reviewed changes

stevvooe force-pushed the sjd/include-docker-socket branch from 28a8712 to fe5a2c3 Compare April 15, 2025 17:10

thaJeztah reviewed Apr 15, 2025

View reviewed changes

stevvooe force-pushed the sjd/include-docker-socket branch from fe5a2c3 to 1a502e9 Compare April 15, 2025 17:57

thaJeztah added this to the 28.1.0 milestone Apr 15, 2025

thaJeztah added impact/changelog kind/enhancement kind/experimental status/2-code-review labels Apr 15, 2025

thaJeztah approved these changes Apr 15, 2025

View reviewed changes

thaJeztah added the area/authentication label Apr 15, 2025

Benehiko approved these changes Apr 16, 2025

View reviewed changes

thaJeztah merged commit 219d3fe into docker:master Apr 16, 2025
98 of 102 checks passed

vvoland added kind/feature and removed kind/enhancement labels Apr 16, 2025

stevvooe deleted the sjd/include-docker-socket branch April 16, 2025 15:30

This was referenced Apr 17, 2025

cli/command/container: --use-api-socket: don't write empty credentials #6018

Merged

cli/command/container: --use-api-socket: support DOCKER_AUTH_CONFIG #6019

Open

run: flag to include the docker socket #5858

run: flag to include the docker socket #5858

Conversation

stevvooe commented Feb 21, 2025 • edited by thaJeztah Loading

codecov-commenter commented Feb 21, 2025 • edited Loading

Codecov Report

stevvooe commented Feb 21, 2025

thaJeztah commented Feb 24, 2025

thaJeztah commented Feb 24, 2025

laurazard commented Feb 24, 2025

stevvooe commented Feb 24, 2025

laurazard commented Feb 24, 2025

stevvooe commented Feb 25, 2025 • edited Loading

stevvooe commented Mar 19, 2025

Benehiko commented Mar 21, 2025

stevvooe commented Mar 25, 2025

cpuguy83 commented Mar 25, 2025

stevvooe commented Mar 25, 2025

stevvooe commented Mar 25, 2025

cpuguy83 commented Mar 25, 2025

Benehiko left a comment

Choose a reason for hiding this comment

stevvooe commented Apr 2, 2025

slimslenderslacks left a comment

Choose a reason for hiding this comment

stevvooe commented Apr 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Apr 15, 2025

stevvooe commented Apr 15, 2025

thaJeztah commented Apr 15, 2025 • edited Loading

thaJeztah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah Apr 15, 2025 • edited Loading

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah commented Apr 15, 2025

Benehiko left a comment

Choose a reason for hiding this comment

stevvooe commented Feb 21, 2025 •

edited by thaJeztah

Loading

codecov-commenter commented Feb 21, 2025 •

edited

Loading

stevvooe commented Feb 25, 2025 •

edited

Loading

thaJeztah commented Apr 15, 2025 •

edited

Loading

thaJeztah Apr 15, 2025 •

edited

Loading