Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"lando ssh -c" does not parse its argument like a shell command within the container #78

Open
phil-s opened this issue Sep 19, 2023 · 5 comments

Comments

@phil-s
Copy link

phil-s commented Sep 19, 2023

lando version => v3.18.0

I don't know how lando ssh -c COMMAND processes COMMAND inside the container, but it seems wrong to me.

Compare:

$ command="env FOO='bar' sh -c 'echo \${FOO}'"

$ echo $command
env FOO='bar' sh -c 'echo ${FOO}'

# An actual shell
$ sh -c "$command"
bar

# lando ssh -c
$ /usr/local/bin/lando ssh -c "$command"
'bar'

Assuming the command is intended to be processed like a shell command, that latter output is wrong. The quote removal phase is apparently not taking place when the remote 'shell' parses that command, meaning the result is as if we'd used:

$ sh -c "env FOO=\"'bar'\" sh -c 'echo \$FOO'"
'bar'

This seems likely to be a docker / docker-compose issue, but I can't see how to get lando to show me exactly what command it's running, so I don't know how to test or report it. Adding -vvv to the lando command shows me this line:

lando    21:31:09 DEBUG ==> process pid5 running /usr/share/lando/bin/docker-compose exec appserver env FOO='bar' sh echo $FOO cstdio=inherit, silent=false, mode=spawn, detached=false

which is an obfuscated version of whatever the original command actually looked like (as without quoting we cannot distinguish between a single multi-word argument, and multiple single-word arguments) and seemingly mangled as well (it's dropped one of the -c instances?!). That is in any case not something I can run at the command line, and furthermore I've been unable to figure out how to get any output at all from a /usr/share/lando/bin/docker-compose exec appserver COMMAND -- nothing I've tried has printed anything.

I'm happy to take this issue to a docker/compose issue queue if someone could demonstrate how to replicate the issue using a docker/compose command?

Thanks.

@pirog
Copy link
Member

pirog commented Sep 21, 2023

@phil-s yup. Lando 3 does not preserve the original command correctly eg your assessment here is correct.

The tl;dr is we aren't going to fix this in Lando 3 because it would likely break things for people who have worked around it. It's something we are planning to fix in Lando 4 though.

@pirog pirog added Needs Triage and removed bug Something isn't working labels Sep 21, 2023
@pirog pirog transferred this issue from lando/lando Sep 21, 2023
@phil-s
Copy link
Author

phil-s commented Sep 21, 2023

I'd love to be one of those people who have worked around it :)

Where can I learn the details of how the command is actually processed, so that I can figure out how I could work with that process?

@phil-s
Copy link
Author

phil-s commented Sep 22, 2023

You could also consider adding a new option to lando 3's ssh command which is like -c but works the way that lando 4 will work. E.g. lando ssh -C <cmd> (upper-case rather than lower). That way no existing workarounds are affected, but lando 3 users can still obtain the good behaviour.

@phil-s
Copy link
Author

phil-s commented Oct 9, 2023

Hopefully a standard solution will be added but, for the benefit of anyone else who needs this, here's what I've hacked together for local use in the interim...

The first script is a wrapper for the lando command, which I derived from my original variant at lando/lando#2188 (comment)

#!/bin/sh
LANDO=/usr/local/bin/lando

# Force "lando ssh" (which does not involve ssh in any way!) to inherit
# selected environment variables.  The initial idea for this came from
# https://github.com/lando/lando/issues/2188#issuecomment-1718218526
#
# Environment variables to propagate to lando ssh.
# This must be a space-separated list of variable names.
env_vars="TERM COLUMNS DISPLAY"

# This is complicated by https://github.com/lando/core-next/issues/78
# I have worked around that with the combination of this 'lando'
# wrapper and an additional wrapper script that works in conjunction
# with this one at /usr/share/lando/bin/docker-compose (which is
# unfortunately a hard-coded path within the lando binary, so I needed
# to rename the original executable at that path).  I'm using the
# following arrangement on my own system:
#
# lrwxrwxrwx 1 root root        23 Sep 25 21:32 docker-compose -> _docker-compose_wrapper
# -rwxr-xr-x 1 root root  12737304 May 27 16:02 _docker-compose_real
# -rwxr-xr-x 1 root root      2366 Sep 26 11:20 _docker-compose_wrapper
#
# In the absence of that wrapper, we can only use vars which can have
# unquoted values (which, thankfully, does at least include TERM).
# (and we would also need to revert the MY_LANDO_SSH_COMMAND code
# below).

if [ -z "${COLUMNS}" ]; then
    # Check how many columns the terminal has.  Older versions of tput return
    # "80" here when both stdout and stderr have been redirected, so running
    # 'tput cols 2>/dev/null' in a subshell is a problem.  Instead we check
    # whether it exits successfully, so that we can call it again without the
    # stderr redirection.
    if command -v tput >/dev/null 2>&1 && tput cols >/dev/null 2>&1; then
        COLUMNS="$(tput cols)"
    fi
fi

quote () {
    local param
    for param; do
        # Strings containing only [-0-9a-zA-Z_./] can remain unquoted.
        # (This char set is derived from shell-quote-argument in Emacs.)
        # We ensure that any newlines are 'seen' by grep by converting
        # them to another invalid character.
        if [ -z "${param}" ]; then
            param="''"
        else
            printf %s "${param}" \
                | tr '\n' '!' \
                | grep '^[-0-9a-zA-Z_./]\+$' >/dev/null 2>&1
            if [ $? -ne 0 ]; then
                # String contains invalid characters (or is empty), so requires
                # quoting.  We can safely remove '' afterwards from the start or
                # end of the string, but not otherwise (e.g. consider \''foo').
                param=$(printf %s "${param}Z" \
                            | sed -e "s/'/'\\\\''/g;1s/^/'/;\$s/Z\$/'/" \
                                  -e "1s/^''//;\$s/''\$//")
                if [ -z "${param}" ]; then
                    param="''" # This is unreachable, yes?
                fi
            fi
        fi
        printf %s "${param}"
        shift
        if [ $# -gt 0 ]; then
            printf ' '
        fi
    done
}

if [ "$1" = "ssh" ]; then
    shift
    unset -v command help params
    while [ $# -gt 0 ]; do
        case "$1" in
            ( -h | --help ) {
                help=1
            };;
            ( -c | --command ) {
                shift
                command=$1
            };;
            ( --command=* ) {
                command=${1#--command=}
            };;
            ( * ) {
                params="${params}$(quote "$1") "
            };;
        esac
        shift
    done
    if [ -z "${command}" ]; then
        # No command; just run bash (by preference) or sh (otherwise).
        # We use the same test that "lando ssh -vvv" shows normally.
        shell="sh -c 'if type bash >/dev/null; then bash; else sh; fi'"
    else
        shell="sh -c $(quote "${command}")"
        # # Or repeat the bash-or-sh dance (which necessitates the
        # # docker-compose wrapper to avoid lando's quoting errors).
        # shell="if type bash >/dev/null; then bash -c $(quote "${command}"); else $(quote "${command}"); fi"
        # shell="sh -c $(quote "${shell}")"
    fi
    # If --help was passed, we can omit the shell command.
    if [ -n "${help}" ]; then
        params="ssh --help"
    else
        # Apply our environment variables.
        env=
        for var in $env_vars; do
            env="${env}${var}=$(eval quote \"\${${var}}\") "
        done
        if [ -n "${env}" ]; then
            env="env ${env}"
        fi
        shell="${env}${shell}"
        # Normally we could do this:
        # params="ssh -c $(quote "${shell}") ${params}"
        # But due to https://github.com/lando/core-next/issues/78 we need to
        # now interact with a second wrapper script (for docker-compose),
        # providing the correct command in an environment variable, and
        # supplying only a token in the original command (which will be
        # substituted with the real one by the other wrapper).
        # @see /usr/share/lando/bin/docker-compose
        export MY_LANDO_SSH_COMMAND="${shell}"
        printf %s\\n >&2 "Command: lando ssh -c $(quote "${shell}") ${params}"
        params="ssh -c MY_LANDO_SSH_COMMAND ${params}"
    fi
    eval "set -- ${params}"
fi

# Run the command (maybe with MY_LANDO_SSH_COMMAND in the environment).
/usr/local/bin/lando "$@"

That wrapper can just go in your ~/bin/ directory (somewhere in your PATH where it will have precedence), but the companion wrapper for docker-compose required me to move around some installed files.

The path /usr/share/lando/bin/docker-compose is one of a number of possible paths for that executable which are hard-coded into the lando binary (n.b. I've installed lando via the Debian package). This specific path is the one which is actually used in my case, but there are other options baked in there as well, and you'll probably want to figure out which docker-compose path is being used by your own lando. None of them allowed for a convenient override, so rather than having the wrapper hidden in a very non-obvious location, I opted to use the original path and modify /usr/share/lando/bin/ like so:

# lrwxrwxrwx 1 root root        23 Sep 25 21:32 docker-compose -> _docker-compose_wrapper
# -rwxr-xr-x 1 root root  12737304 May 27 16:02 _docker-compose_real
# -rwxr-xr-x 1 root root      2366 Sep 26 11:20 _docker-compose_wrapper

With _docker-compose_wrapper being this:

#!/bin/sh
DOCKER_COMPOSE=/usr/share/lando/bin/_docker-compose_real

# Quote shell parameters.
quote () {
    local param
    for param; do
        # Strings containing only [-0-9a-zA-Z_./] can remain unquoted.
        # (This char set is derived from shell-quote-argument in Emacs.)
        # We ensure that any newlines are 'seen' by grep by converting
        # them to another invalid character.
        if [ -z "${param}" ]; then
            param="''"
        else
            printf %s "${param}" \
                | tr '\n' '!' \
                | grep '^[-0-9a-zA-Z_./]\+$' >/dev/null 2>&1
            if [ $? -ne 0 ]; then
                # String contains invalid characters (or is empty), so requires
                # quoting.  We can safely remove '' afterwards from the start or
                # end of the string, but not otherwise (e.g. consider \''foo').
                param=$(printf %s "${param}Z" \
                            | sed -e "s/'/'\\\\''/g;1s/^/'/;\$s/Z\$/'/" \
                                  -e "1s/^''//;\$s/''\$//")
                if [ -z "${param}" ]; then
                    param="''" # This is unreachable, yes?
                fi
            fi
        fi
        printf %s "${param}"
        shift
        if [ $# -gt 0 ]; then
            printf ' '
        fi
    done
}

# Escape substitutions in sed replacement strings.
sed_escape () {
    printf %s "$1" | sed 's/[\/&]/\\&/g;$!s/$/\\/g'
}

# Save the current positional parameters.
parameters=$(quote "$@")
# printf "/usr/share/lando/bin/_docker-compose %s\n\n" "${parameters}" >&2

# printf "MY_LANDO_SSH_COMMAND=%s\n" "${MY_LANDO_SSH_COMMAND}"
if [ -n "${MY_LANDO_SSH_COMMAND}" ]; then
    # In this case there is also a token (of the same name) in the command,
    # for substitution by the original command, but using *correct* quoting.
    # @see ~/bin/lando
    substitution=$(sed_escape "${MY_LANDO_SSH_COMMAND}")
    parameters=$(printf %s "${parameters}" \
                     | sed "s/MY_LANDO_SSH_COMMAND/${substitution}/")
    # printf "/usr/share/lando/bin/_docker-compose %s\n\n" "${parameters}" >&2
    # Update the positional parameters.
    eval "set -- ${parameters}"
fi

$DOCKER_COMPOSE "$@"

The printf near the end of the lando wrapper can safely be commented out, but by default that displays the command which will be executed. Or rather, it shows the command as it would be if we didn't need the docker-compose wrapper. E.g.:

$ lando ssh
Command: lando ssh -c 'env TERM=eterm-color COLUMNS=188 DISPLAY='\'':1'\'' sh -c '\''if type bash >/dev/null; then bash; else sh; fi'\' 
6bd1a8c63312:~$ 
exit

$ lando ssh -c hostname
Command: lando ssh -c 'env TERM=eterm-color COLUMNS=188 DISPLAY='\'':1'\'' sh -c hostname' 
6bd1a8c63312

@phil-s
Copy link
Author

phil-s commented Jan 18, 2024

I've just updated to lando v3.20.8 and discovered that I needed to track down a new location for the docker-compose executable, which turned out to be this:

~/.lando/bin/docker-compose-v2.21.0

So adjust the previous wrappers accordingly, if you're using them.

I imagine that -v2.21.0 suffix will be subject to change with later versions of lando, although the new orchestration docs indicate ways of pinning a version if you really want to.

More useful for wrapper purposes is the orchestratorBin config or LANDO_ORCHESTRATOR_BIN environment variable, which would allow for a fixed wrapper path which could then figure out the current path for the real executable.

Relevant change logs and new documentation:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants