Add support for docker-compose run to the ssh option
#5637
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielbachhuber
approved these changes
Aug 18, 2022
There was a problem hiding this comment.
Seems fine to me!
You'll note that the command needs to be constructed differently to all the other schemes supported by the
sshoption, crucially that the command cannot be wrapped inescapeshellarg()becausedocker-compose run <container> <cmd>doesn't support the command being quoted.
My understanding is that escapeshellarg() is primary for escaping user-supplied input to a hard-coded command. escapeshellarg() is meant to prevent them from escaping the hard-coded command and running their own arbitrary command.
In this case, we're running the user's full command under their user credentials, so there isn't anything new they can do with this vector.
schlessera
approved these changes
Oct 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
docker:anddocker-compose:schemes for thesshoption are useful, but they only work when the target container is already running and therefore supports running a shell command viadocker[-compose] exec.Some environments, for example the wordpress-develop local development environment, run WP-CLI via a container that only starts when needed and therefore requires the use of
runinstead ofexec. Thesshoption in WP-CLI doesn't currently support running commands in a container in this manner.The change in this PR introduces support for a
docker-compose-runscheme for thesshoption which runs the command viadocker compose run <container> <cmd>.With this support in place you could put the following config into the root of any project that uses
wordpressdevelop/cliand then simply usewp <cmd>instead ofnpm run env:cli -- <cmd>ordocker-compose run cli -- <cmd>.You'll note that the command needs to be constructed differently to all the other schemes supported by the
sshoption, crucially that the command cannot be wrapped inescapeshellarg()becausedocker-compose run <container> <cmd>doesn't support the command being quoted. I am honestly not sure of the security implications of this, we might need to consult someone a bit more familiar with the internals of sending commands viarun.Feedback welcome.