Skip to content

Conversation

@ictbeheer
Copy link

@ictbeheer ictbeheer commented May 28, 2025

Local binary paths can contain spaces (e.g. /Users/name/Library/Application Support/Herd/bin/composer) which breaks among others things the composer_validate check

task deploy:check_composer_validate
[server] error in deploy_check_composer_validate.php on line 13:
[server] run export DO_NOT_TRACK='true' DO_NOT_SHOW_BANNER='true'; /Users/name/Library/Application Support/Herd/bin/composer validate --no-check-all --no-check-publish
[server] err bash: line 1: /Users/name/Library/Application: No such file or directory
[server] exit code 127 (Command not found)
ERROR: Task deploy:check_composer_validate failed!

@kszymukowicz
Copy link
Contributor

I am not sure about this. Escaping should be done at place of usage and not in returned value.

Look at code of deployer library. There is also no escaping.
https://github.com/deployphp/deployer/blob/master/src/functions.php

@ictbeheer
Copy link
Author

@kszymukowicz this problem probably only occurs for local binaries as server paths usually do not include spaces.
I could also escape the local binary paths in all the places were they are used; would that be your preferred solution?

@kszymukowicz
Copy link
Contributor

kszymukowicz commented May 29, 2025

@ictbeheer

The more I investigate the more edge cases needs to be considered.
Imagine such case:

set('local/bin/php', '$PHP_BINARIES_PATH/php82');
task('test1', function() {
    runLocally(get('local/bin/php') . ' -v');
    runLocally(escapeshellarg(get('local/bin/php')) . ' -v');
});

escapeshellarg() is adding single quotes which prevent the shell from expanding the variables.

Maybe you can just define alias in your shell like:

echo 'alias composer="/Users/name/Library/Application Support/Herd/bin/composer"' >> ~/.zshrc

and then you can define just:

set('local/bin/composer', 'composer');

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants