phpro / grumphp

A PHP code-quality tool
MIT License
4.15k stars 430 forks source link

Incorrect escapting of EXEC_GRUMPHP_COMMAND #779

Closed 1ed closed 4 years ago

1ed commented 4 years ago
Q A
Version 0.19.1
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets #733

My configuration

grumphp:
    git_hook_variables:
        EXEC_GRUMPHP_COMMAND: 'docker-compose run --rm --no-deps -u $(id -u):$(id -g) php'

Steps to reproduce:

# Generate empty folder
mkdir tmp
cd tmp
git init
echo "vendor" > .gitignore
pbpaste > grumphp.yml
composer init --no-interaction && composer require --dev phpro/grumphp

# Your actions
# Please add the steps on how to reproduce the issue here.
vendor/bin/grumphp git:init -vvv && cat .git/hooks/pre-commit

# Run GrumPHP:
git add -A && git commit -m"Test"
# or
./vendor/bin/grumphp run

Result:

Run a one-off command on a service.

For example:

    $ docker-compose run web python manage.py shell

By default, linked services will be started, unless they are already
running. If you do not want to start linked services, use
`docker-compose run --no-deps SERVICE COMMAND [ARGS...]`.

Usage:
    run [options] [-v VOLUME...] [-p PORT...] [-e KEY=VAL...] [-l KEY=VALUE...]
        SERVICE [COMMAND] [ARGS...]

Options:
    -d, --detach          Detached mode: Run container in the background, print
                          new container name.
    --name NAME           Assign a name to the container
    --entrypoint CMD      Override the entrypoint of the image.
    -e KEY=VAL            Set an environment variable (can be used multiple times)
    -l, --label KEY=VAL   Add or override a label (can be used multiple times)
    -u, --user=""         Run as specified username or uid
    --no-deps             Don't start linked services.
    --rm                  Remove container after run. Ignored in detached mode.
    -p, --publish=[]      Publish a container's port(s) to the host
    --service-ports       Run command with the service's ports enabled and mapped
                          to the host.
    --use-aliases         Use the service's network aliases in the network(s) the
                          container connects to.
    -v, --volume=[]       Bind mount a volume (default [])
    -T                    Disable pseudo-tty allocation. By default `docker-compose run`
                          allocates a TTY.
    -w, --workdir=""      Working directory inside the container

Escaping of the EXEC_GRUMPHP_COMMAND is not correct: (cd "./" && printf "%s\n" "${DIFF}" | 'docker-compose' 'run' '--rm' '--no-deps' '-u' '$(id' '-u):$(id' '-g)' 'php' 'vendor/phpro/grumphp/bin/grumphp' 'git:pre-commit' '--skip-success-output')

The problem is the explode in this line: https://github.com/phpro/grumphp/blob/3be2d5fe8437f2a3ef5a45629602657be7464e8a/src/Process/ProcessUtils.php#L56

Maybe we should allow to pass an array to EXEC_GRUMPHP_COMMAND and pass it to symfony/process as-is and get back the command to run and remove ProcessUtils from here.

veewee commented 4 years ago

Thanks for creating an issue. Gonna close this on in favour of the existing #733.