cli/connhelper: quote ssh arguments to prevent shell injection

[thaJeztah]

cli/connhelper: add fork of mvdan.cc/sh/v3/syntax v3.10.0

This adds a local fork of the mvdan.cc/sh/v3/syntax package to provide the
Quote function without having to introduce additional (indirect) dependencies
of the mvdan.cc/sh module.

This commit does not compile as it references code not forked.

The following files were included:

• https://raw.githubusercontent.com/mvdan/sh/refs/tags/v3.10.0/syntax/quote.go
• https://raw.githubusercontent.com/mvdan/sh/refs/tags/v3.10.0/syntax/parser.go
• https://raw.githubusercontent.com/mvdan/sh/refs/tags/v3.10.0/LICENSE

cli/connhelper/internal/syntax: remove unused code from fork

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 25ebf0e (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().

- Human readable description for the release notes

Quote SSH arguments when connecting to a remote daemon over an SSH connection to avoid unexpected expansion.

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 19.59799% with 160 lines in your changes missing coverage. Please review.

Project coverage is 54.82%. Comparing base (f03fb6c) to head (88d1133).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6147      +/-   ##
==========================================
- Coverage   55.06%   54.82%   -0.24%     
==========================================
  Files         362      364       +2     
  Lines       30284    30470     +186     
==========================================
+ Hits        16675    16706      +31     
- Misses      12643    12789     +146     
- Partials      966      975       +9     

🚀 New features to boost your workflow:

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

[thaJeztah]

cli/connhelper: add fork of mvdan.cc/sh/v3/syntax v3.10.0

FWIW; 3.11.0 was released, but at a quick glance nothing immediately needed for what we use from thatt

[Copilot]

Pull Request Overview

This PR addresses potential shell injection issues in SSH connections by quoting command line arguments appropriately and by introducing a fork of mvdan.cc/sh/v3/syntax to provide the Quote function without extra dependencies. Key changes include:

• Updating the SSH connection logic to quote SSH arguments via new helper functions.
• Adding a new Spec.Command method that returns a quoted, error-checked remote command.
• Refreshing tests to validate edge cases such as trailing slashes and special characters.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
cli/connhelper/ssh/ssh_test.go	Adds tests covering various SSH URL edge cases with quoted output.
cli/connhelper/ssh/ssh.go	Refactors Args and adds Command to quote SSH arguments.
cli/connhelper/internal/syntax/quote.go	Introduces a fork version of the quote function from mvdan.cc/sh.
cli/connhelper/internal/syntax/parser.go	Provides the language variant definitions and helper functions.
cli/connhelper/connhelper.go	Integrates the new quoting approach for building SSH commands.
cli/connhelper/internal/syntax/doc.go	Documents the scope and origin of the forked syntax package.
cli/connhelper/internal/syntax/LICENSE	Provides the license for the forked mvdan.cc/sh/v3/syntax package.

Comments suppressed due to low confidence (2)

cli/connhelper/ssh/ssh_test.go:43

• [nitpick] Clarify in the test case 'bare ssh URL with trailing slashes' the intended behavior for the Path field to avoid ambiguity in expected behavior. Consider adding a comment explaining why a trailing slash results in the provided Path value.

			url: "ssh://example.com//",

cli/connhelper/ssh/ssh.go:147

• Update the Command method’s documentation to explicitly state that if no remote command is provided, an error is returned. This will help users understand the mandatory nature of a remote command.

	if len(remoteCommandAndArgs) == 0 {

[thaJeztah]

@Benehiko @vvoland ptal 🤗

[tianon]

I do not love the copy of mvdan.cc/sh/v3/syntax, although I understand it (and don't have a better suggestion).
"Thanks Gobama"

To expand, it feels a little dangerous to rely on our own diligence for updating a copy of a module that we only use in a moderately security-sensitive codepath, BUT it's not really a high severity codepath when it goes wrong (as we've discussed previously), so it's probably fine.