Commit Graph

2759 Commits

Author SHA1 Message Date
0e474e38f1 [28.x] cli/command/container: TestRunPullTermination: rewrite with streamformatter
This makes the test slightly closer to the actual code in the daemon producing
the progress response;
cd844fd0b2/daemon/images/image_pull.go (L58-L70)
cd844fd0b2/daemon/internal/distribution/utils/progress.go (L14-L34)

This is a modified version of 69854c4e08
with some changes specific to the 28.x branch (the variant on master
had some patches for the moby/api and moby/client transition).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 69854c4e08)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-18 19:17:14 +02:00
7091e8bea4 cli/flags: add "hostVar" to handle --host / -H as a single string
hostVar is used for the '--host' / '-H' flag to set [ClientOptions.Hosts].
The [ClientOptions.Hosts] field is a slice because it was originally shared
with the daemon config. However, the CLI only allows for a single host to
be specified.

hostVar presents itself as a "string", but stores the value in a string
slice. It produces an error when trying to set multiple values, matching
the check in [getServerHost].

[getServerHost]: 7eab668982/cli/command/cli.go (L542-L551)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f14eeeb361)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-18 17:12:54 +02:00
ff42ff9f06 cli/flags: use a regular StringArray for the --host / -H flag
The ClientOptions struct and related flags were inherited from the Moby
repository, where originally the CLI and Daemon used the same implementation
and had a "Common" options struct. When the CLI moved to a separate repository,
those structs were duplicated, but some daemon-specific logic remained. For
example, the daemon can be configured to listen on multiple ports and sockets
([moby@dede158]), but the CLI [can only connect to a single host][1]. The
daemon config also had to account for flags conflicting with `daemon.json`,
and use special flag-vars for this ([moby@677a6b3]).

Unfortunately, the `ClientConfig` struct became part of the public API and
is used as argument in various places, but we can remove the use of the
special flag var. This patch replaces the use of `NewNamedListOptsRef`
for a regular `StringArray`.

Unfortunately this changes the flag's type description from `list` to
`stringArray`, but we can look at changing that separately.

[moby@dede158]: dede1585ee
[1]: 0af135e906/docker/docker.go (L191-L193)
[moby@677a6b3]: 677a6b3506

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5ee2906e78)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-18 17:11:54 +02:00
bcc479b4c3 Merge pull request #6276 from thaJeztah/28.x_backport_cli_internalize_utils
[28.x backport] cli: deprecate VisitAll, DisableFlagsInUseLine utilities, remove HasCompletionArg
2025-08-18 11:41:10 +02:00
09efe3f408 cli-plugins/manager: fix Plugin marshaling with regular errors
Go does not by default marshal `error` type fields to JSON. The manager
package therefore implemented a `pluginError` type that implements
[encoding.TextMarshaler]. However, the field was marked as a regular
`error`, which made it brittle; assining any other type of error would
result in the error being discarded in the marshaled JSON (as used in
`docker info` output), resulting in the error being marshaled as `{}`.

This patch adds a custom `MarshalJSON()` on the `Plugin` type itself
so that any error is rendered. It checks if the error used already
implements [encoding.TextMarshaler], otherwise wraps the error in
a `pluginError`.

[encoding.TextMarshaler]: https://pkg.go.dev/encoding#TextMarshaler

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 549d39a89f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-17 19:44:52 +02:00
034dc932d7 remove aliases for containerd/errdefs, disallow docker/errdefs
We transitioned most functionality of docker/errdefs to containerd
errdefs module, and the docker/errdefs package should no longer be
used.

Because of that, there will no longer be ambiguity, so we can remove
the aliases for this package, and use it as "errdefs".

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 89d8c8a2a7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-16 18:29:23 +02:00
0718529a7e Merge pull request #6221 from thaJeztah/28.x_fork_registry
[28.x] add internal fork of docker/docker/registry
2025-08-16 08:04:59 -07:00
60d16e20ac cli: deprecate VisitAll, DisableFlagsInUseLine utilities
These utilities were only used internally; create a local copy
where used, and deprecate the ones in cli.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 6bd8a4b2b5)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 19:11:17 +02:00
713ed839fe cli: remove HasCompletionArg utility
It was only used in a single place and has no external consumers.
Move it to where it's used to keep things together.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5a99022556)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 19:11:17 +02:00
6bfee62d6d opts: deprecate QuotedString
The `QuotedString` option was added in [moby@e4c1f07] and [moby@abe32de]
to work around a regression in Docker 1.13 that caused `docker-machine`
to fail. `docker-machine` produced instructions on how to set up a cli
to connect to the Machine it produced. These instructions used quotes
around the paths for TLS certificates, but with an `=` for the flag's
values instead of a space; due to this the shell would not handle
stripping quotes, so the CLI would now get the value including quotes.

Preserving quotes in such cases is expected (and standard behavior), but
versions of Docker before 1.13 used a custom "mflag" package for flag
parsing, and that package contained custom handling for quotes (added
in [moby@0e9c40e]).

For other flags, this problem could be solved by the user, but as these
instructions were produced by `docker-machine`'s `config` command, an
exception was made for the `--tls-xxx` flags. From [moby-29761]:

> The flag trimming behaviour is really unusual, and I would say unexpected.
> I think removing it is generally the right idea. Since we have one very
> common case where it's necessary for backwards compatibility we need to
> add a special case, but I don't think we should apply that case to every
> flag.

The `QuotedString` implementation has various limitations, as it doesn't
follow the same handling of quotes as a shell would.

Given that Docker Machine reached EOL a long time ago and other options,
such as `docker context`, have been added to configure the CLI to connect
to a specific host (with corresponding TLS configuration), we should remove
the special handling for these flags, as it's inconsitent with all other
flags, and not worth maintaining for a tool that no longer exists.

This patch deprecates the `QuotedString` option and removes its use. A
temporary, non-exported copy is added, but will be removed in the next
release.

[moby-29761]: https://github.com/moby/moby/issues/29761#issuecomment-270211265
[moby@e4c1f07]: e4c1f07729
[moby@abe32de]: abe32de6b4
[moby@0e9c40e]: 0e9c40eb82
[moby@c79a169]: c79a169a35

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 187a942a88)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 19:07:52 +02:00
efdf008933 internal/registry: remove RepositoryInfo, add NewIndexInfo
Most places only use IndexInfo (and may not even need that), so replace
the use of ParseRepositoryInfo for NewIndexInfo, and move the RepositoryInfo
type to the trust package, which uses it as part of its ImageRefAndAuth
struct.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 21e8bbc8a2)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:49 +02:00
28ffe2416d internal/registry: ParseRepositoryInfo: remove unused error return
Removed the error return from the `ParseRepositoryInfo` function.
There are no validation steps inside `ParseRepositoryInfo` which
could cause an error, so we always returned a nil error.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 86b5b528a6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:49 +02:00
2e5a36728b cli/command/system: remove use of Mirrors field in test
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit cd277a5815)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:49 +02:00
75a4cbbf8e internal/registry: remove duplicate endpoint methods
now that we no longer need to account for mirrors, these were
identical, so just use a single one.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5322affc9f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:48 +02:00
9071d3868c internal/registry: remove NewStaticCredentialStore
It was only used in a single place; inline it there.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit dc41365b56)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:48 +02:00
7e01a3a8a9 internal/registry: Service.Auth remove unused statusmessage return
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 7cf245d2f7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:47 +02:00
8b9baffdf7 add internal fork of docker/docker/registry
This adds an internal fork of [github.com/docker/docker/registry], taken
at commit [moby@f651a5d]. Git history  was not preserved in this fork,
but can be found using the URLs provided.

This fork was created to remove the dependency on the "Moby" codebase,
and because the CLI only needs a subset of its features. The original
package was written specifically for use in the daemon code, and includes
functionality that cannot be used in the CLI.

[github.com/docker/docker/registry]: https://pkg.go.dev/github.com/docker/docker@v28.3.2+incompatible/registry
[moby@49306c6]: 49306c607b/registry

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f6b90bc253)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:47 +02:00
93a51c39f4 cli/command/registry: remove uses of registry.ParseSearchIndexInfo
This utility was only used in the CLI, but the implementation was
based on it being used on the daemon side, so included resolving
the host's IP-address, mirrors, etc.

The only reason it's used in the CLI is to provide credentials for
the registry that's being searched, so reduce it to just that.

There's more cleaning up to do in this area, so to make our lives
easier, it's implemented locally as non-exported functions; likely
to be replaced with something else.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit e504faf6da)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-15 15:45:45 +02:00
a4f8f22a33 Merge pull request #6190 from thaJeztah/fork_remotecontext
add local fork of github.com/docker/docker/builder/remotecontext

(cherry picked from commit 8c317ad3fd)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-14 21:50:07 +02:00
7d3bde083c cli/command/image: move build-context detection to build
Removes direct imports of github.com/docker/docker/builder in
the image package, to be moved later.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 260f1dbebb)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-14 21:50:07 +02:00
7399781944 cli/connhelper: remove dependency on pkg/process
This package will not be included in the api or client modules, and
we're currently only using a single function of it, and only the
unix implementation, so let's fork it for now (although the package
may be moved to moby/sys).

This removes the last dependency on github.com/docker/docker.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2abcbf842f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-14 18:45:43 +02:00
b82e19efe0 [28.x] merge v28.3.3 tag into v28.x
The v28.3.3 tag was created from master, but the v28.x branch
wasn't fast-forwarded, and PR's merged after that. This should
bring the v28.3.3 tag's changes into the v28.x branch.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-14 17:45:14 +02:00
eceff3dbc5 Merge pull request #6256 from thaJeztah/28.x_backport_remove_prompt_privilege_func
[28.x backport] cli/command: remove interactive login prompt from docker push/pull, deprecate RegistryAuthenticationPrivilegedFunc
2025-08-14 14:43:15 +02:00
3598fc3745 Merge pull request #6259 from thaJeztah/28.x_backport_rm_completion
[28.x backport] cli/command/completion: remove deprecated ValidArgsFn
2025-08-14 10:51:34 +02:00
2df466710b Merge pull request #6258 from thaJeztah/28.x_backport_remove_RepoNameForReference
[28.x backport] cli/registry/client: remove deprecated RepoNameForReference
2025-08-14 10:22:59 +02:00
363f4c0031 cli/command/completion: remove deprecated ValidArgsFn
This was deprecated in 9f19820f88, which
is part of v28.x, and unlikely used externally.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5052a39915)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-13 18:06:23 +02:00
6d4ffec3fb cli/registry/client: remove deprecated RepoNameForReference
This was deprecated in 6f46cd2f4b,
which is part of v28.x, and no longer used, so we can remove it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a87bde0068)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-13 18:02:48 +02:00
5c1cee4630 cli/command: remove deprecated ConfigureAuth utility
It was deprecated in 6e4818e7d6, which
is part of v28.x and backported to v27.x.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 22cc0e90ae)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-13 17:54:06 +02:00
88274f4805 cli/command: remove deprecated CopyToFile utility
It was deprecated in 7cc6b8ebf4, which is
part of v28.x

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit de54347518)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-13 17:53:58 +02:00
5566c3a9b8 cli/command: remove usages of RegistryAuthenticationPrivilegedFunc
This patch deprecates the unused `RegistryAuthenticationPrivilegedFunc`.
The function would prompt the user when the registry returns a 403 after trying
the initial auth value set in `RegistryAuth`.

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
(cherry picked from commit 29263e865b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-13 17:38:45 +02:00
5edc6748f4 cli/command: remove interactive login prompt from docker push/pull
This patch removes the interactive prompts from `docker push/pull`.
The prompt would only execute on a response status code 403 from the registry
after trying the value set in `RegistryAuth`. Docker Hub could return 404
instead or 429, which would never execute the prompt.

The UX regarding the prompt is also questionable since the user might
not actually want to authenticate with a registry and the CLI could fail fast
instead. The user can always run `docker login` or set the `DOCKER_AUTH_CONFIG`
environment variable to get authenticated.

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
(cherry picked from commit 2b56b66b10)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-13 17:38:29 +02:00
980b856816 Merge pull request #6183 from thaJeztah/diff_simplify
Some checks failed
build / bin-image (push) Has been cancelled
build / prepare-plugins (push) Has been cancelled
build / plugins (push) Has been cancelled
codeql / codeql (push) Has been cancelled
e2e / tests (alpine, 23, connhelper-ssh) (push) Has been cancelled
e2e / tests (alpine, 23, local) (push) Has been cancelled
e2e / tests (alpine, 26, connhelper-ssh) (push) Has been cancelled
e2e / tests (alpine, 26, local) (push) Has been cancelled
e2e / tests (alpine, 27, connhelper-ssh) (push) Has been cancelled
e2e / tests (alpine, 27, local) (push) Has been cancelled
e2e / tests (alpine, 28, connhelper-ssh) (push) Has been cancelled
e2e / tests (alpine, 28, local) (push) Has been cancelled
e2e / tests (debian, 23, connhelper-ssh) (push) Has been cancelled
e2e / tests (debian, 23, local) (push) Has been cancelled
e2e / tests (debian, 26, connhelper-ssh) (push) Has been cancelled
e2e / tests (debian, 26, local) (push) Has been cancelled
e2e / tests (debian, 27, connhelper-ssh) (push) Has been cancelled
e2e / tests (debian, 27, local) (push) Has been cancelled
e2e / tests (debian, 28, connhelper-ssh) (push) Has been cancelled
e2e / tests (debian, 28, local) (push) Has been cancelled
test / ctn (push) Has been cancelled
test / host (macos-13) (push) Has been cancelled
test / host (macos-14) (push) Has been cancelled
validate / validate (lint) (push) Has been cancelled
validate / validate (shellcheck) (push) Has been cancelled
validate / validate (update-authors) (push) Has been cancelled
validate / validate (validate-vendor) (push) Has been cancelled
validate / validate-md (push) Has been cancelled
validate / validate-make (manpages) (push) Has been cancelled
validate / validate-make (yamldocs) (push) Has been cancelled
cli/command/container: diff: remove redundant validation and cleanup
2025-07-16 12:32:48 +02:00
9c256146ac Merge pull request #6181 from thaJeztah/fork_readCloserWrapper
remove uses of github.com/docker/docker/pkg/ioutils ReadCloserWrapper
2025-07-16 12:19:14 +02:00
bc01f8489d Merge pull request #6182 from thaJeztah/fork_longpath
remove use of github.com/docker/docker/pkg/longpath
2025-07-16 12:18:06 +02:00
3d985799d4 cli/command: remove some redundant import-aliases
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-16 12:12:41 +02:00
7d574b816d Merge pull request #6180 from thaJeztah/truncate_id
remove uses of github.com/docker/docker/pkg/stringid
2025-07-15 14:03:18 +02:00
0f2b709c7c cli/command/container: diff: remove redundant validation and cleanup
client.ContainerDiff already validates the given container name/ID, and
produces an error when empty, so we don't have to check for this;
abba330bbf/client/container_diff.go (L13-L16)

While updating, also;

- remove the diffOptions type, as there were no other options, and make
  the container name/ID a string argument.
- fix camelCase nameing of dockerCLI

Before this patch:

    docker diff ""
    Container name cannot be empty

With this patch:

    docker diff ""
    invalid container name or ID: value is empty

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-15 01:08:29 +02:00
53d02ece89 remove use of github.com/docker/docker/pkg/longpath
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-14 22:34:36 +02:00
3600ebca76 remove uses of github.com/docker/docker/pkg/ioutils ReadCloserWrapper
It was the only utility we consumed from the package, and it's trivial
to implement, so let's create local copies of it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-14 22:09:31 +02:00
9b047a501f remove uses of pkg/stringid.GenerateRandomID()
This utility was only used for testing, and to generate a random
suffix for Dockerfiles. As we don't need the same contract as
pkg/stringid.GenerateRandomID() (not allow all-numeric IDs as they
would not be usable for hostnames), we can use a local test-utility,
and local implementation for the random suffix instead.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-14 20:11:07 +02:00
e0f4bc699c cli/command/formatter: add TrunateID utility
We were depending on pkg/stringid to truncate IDs for presentation. While
traditionally, we used a fixed length for "truncated" IDs, this is not
a strict requirement (any ID-prefix should work, but conflicts may
happen on shorter IDs).

This patch adds a local `TruncateID()` utility in the formatter package;
it's currently using the same implementation and length as the
`stringid.TruncateID` function, but may diverge in future.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-14 20:10:56 +02:00
0be687acc0 cli/command/container: don't set CopyToContainerOptions.AllowOverwriteDirWithFile
The `AllowOverwriteDirWithFile` option was added when reimplementing the
CLI using the API Client lib in [moby@1b2b91b]. Before that refactor, the
`noOverwriteDirNonDir` query argument [would be set unconditionally][1]
by the CLI, with no options to control the behavior.

It's unclear why the `noOverwriteDirNonDir` was implemented as opt-in (not
opt-out), as overwriting a file with a directory (or vice-versa) would
generally be unexpected behavior.

We're considering making `noOverwriteDirNonDir` unconditional on the daemon
side, and to deprecate the `AllowOverwriteDirWithFile` option. This patch
removes its use, as it was set to the default either way, and there's no
options to configure it from the CLI.

[1]: 8c9ad7b818/api/client/cp.go (L345-L346)
[moby@1b2b91b]: 1b2b91ba43

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-13 13:48:59 +02:00
ccd5bd8d57 registry: warn of DOCKER_AUTH_CONFIG usage in login and logout
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
2025-07-08 14:07:32 +02:00
3262107821 cli/config: export const dockerEnvConfig
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
2025-07-04 14:04:38 +02:00
8403869122 Merge pull request #6158 from thaJeztah/reduce_strslice
cli/command/container: remove redundant uses of strslice.StrSlice
2025-07-02 17:42:43 +02:00
4ea6fbf538 cli/debug: fix OTELErrorHandler logging messages if there's no error
I noticed this in a ticket in the compose issue tracker; with debug logging
enabled, the OTEL error-logger may be logging even if there's no error;

    DEBU[0000] Executing bake with args: [bake --file - --progress rawjson --metadata-file /tmp/compose-build-metadataFile-1203980021.json --allow fs.read=/home/user/dev/project --allow fs.read=/home/user/dev/project --allow fs.read=/home/user/dev/project/nginx --allow fs.read=/home/user/dev/project]
    TRAC[0000] Plugin server listening on @docker_cli_d8df486f78df3b7357995be71bf0cef6
    DEBU[0005] otel error                                    error="<nil>"
    ^CTRAC[0055] Closing plugin server
    TRAC[0055] Closing plugin server
    DEBU[0055] otel error                                    error="<nil>"
    DEBU[0055] otel error                                    error="<nil>"

Update the error-handler to not log if there's no error.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-02 11:59:05 +02:00
e7e238eb4b cli/command/container: remove redundant uses of strslice.StrSlice
The strslice.StrSlice type is a string-slice with a custom JSON Unmarshal
function to provide backward-compatibility with older API requests (see
[moby@17d6f00] and [moby@ea4a067]).

Given that the type is assigned implicitly through the fields on HostConfig,
we can just use a regular []string instead.

[moby@17d6f00]: 17d6f00ec2
[moby@ea4a067]: ea4a06740b

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-07-01 10:09:54 +02:00
2ba7cb8b44 mount /var/run/docker.sock for --use-api-socket
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
2025-06-30 12:38:15 +02:00
88d1133224 cli/connhelper: quote ssh arguments to prevent shell injection
When connecting to a remote daemon through an ssh:// connection,
the CLI connects with the remote host using ssh, executing the
`docker system dial-stdio` command on the remote host to connect
to the daemon API's unix socket.

By default, the `docker system dial-stdio` command connects with the
daemon using the default location (/var/run/docker.sock), or the
location as configured on the remote host.

Commit 25ebf0ec9c (included in docker
CLI v24.0.0-rc.2 and higher) introduced a feature to allow the location
of the socket to be specified through the host connection string, for
example:

     DOCKER_HOST='ssh://example.test/run/custom-docker.sock'

The custom path is included as part of the ssh command executed from
the client machine to connect with the remote host. THe example above
would execute the following command from the client machine;

    ssh -o ConnectTimeout=30 -T -- example.test docker --host unix:///run/custom-docker.sock system dial-stdio

ssh executes remote commands in a shell environment, and no quoting
was in place, which allowed for a connection string to include additional
content, which would be expanded / executed on the remote machine.

For example, the following example would execute `echo hello > /hello.txt`
on the remote machine;

    export DOCKER_HOST='ssh://example.test/var/run/docker.sock $(echo hello > /hello.txt)'
    docker info
    # (output of docker info from the remote machine)

While this doesn't allow the user to do anything they're not already
able to do so (by directly using the same SSH connection), the behavior
is not expected, so this patch adds quoting to prevent such URLs from
resulting in expansion.

This patch updates the cli/connhelper and cli/connhelper/ssh package to
quote parameters used in the ssh command to prevent code execution and
expansion of variables on the remote machine. Quoting is also applied to
other parameters that are obtained from the DOCKER_HOST url, such as username
and hostname.

- The existing `Spec.Args()` method inthe cli/connhelper/ssh package now
  quotes arguments, and returns a nil slice when failing to quote. Users
  of this package should therefore check the returned arguments before
  consuming. This  method did not provide an error-return, and adding
  one would be a breaking change.
- A new `Spec.Command` method is introduced, which (unlike the `Spec.Args()`
  method) provides an error return. Users are recommended to use this new
  method instead of the `Spec.Args()` method.

Some minor additional changes in behavior are included in this patch;

- Connection URLs with a trailing slash (e.g. `ssh://example.test/`)
  would previously result in `unix:///` being used as custom socket
  path. After this patch, the trailing slash is ignored, and no custom
  socket path is used.
- Specifying a remote command is now required. When passing an empty
  remote command, `Spec.Args()` now results in a `nil` value to be
  returned (or an `no remote command specified` error when using
  `Spec.Comnmand()`.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-06-24 16:26:17 +02:00
82eda48066 cli/connhelper/internal/syntax: fix linting issues
cli/connhelper/internal/syntax/parser.go:31:2: Duplicate words (the) found (dupword)
        // Note that it shares some features with Bash, due to the the shared
        ^
    cli/connhelper/internal/syntax/quote.go:48:1: cyclomatic complexity 35 of func `Quote` is high (> 16) (gocyclo)
    func Quote(s string, lang LangVariant) (string, error) {
    ^
    cli/connhelper/internal/syntax/quote.go:103:3: shadow: declaration of "offs" shadows declaration at line 56 (govet)
            offs := 0
            ^

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-06-24 16:24:34 +02:00