No verification of proper "composer license" execution
Opened this issue · 2 comments
I have a project that checks out repos of all other projects to verify certain aspects, including itself.
I added composer-license-checker, and it worked fine inside the checker repo being used on itself, but running it for all others created this output:
Reading through dependencies and checking their licenses ...
============================================================
sh: licenses: command not found
PHP Notice: Trying to access array offset on value of type null in /opt/---redacted---/vendor/dominikb/composer-license-checker/src/JSONDependencyParser.php on line 16
PHP Warning: Invalid argument supplied for foreach() in /opt/---redacted---/vendor/dominikb/composer-license-checker/src/JSONDependencyParser.php on line 16
0 dependencies were found ...
[OK] Command finished successfully. No violations detected!
Digging down a bit, I found that by default the checker uses "a" composer that is supposed to be inside realpath("./vendor/bin/composer) on the current working directory.
This assumption is wrong. When I install this checker as a dev dependency, composer is pulled in as a dependency, but when I run the command line command inside ANOTHER directory (one that is from a different repository that does not depend on Composer), trying to use the composer relative to the current working directory will fail. Supposedly realpath() is returning false, which at some point is type-juggled into an empty string, leading to the exec(" license ...") command being executed, which will fail.
Failing to execute this command should abort the command and fail with an error status.
On top of that, the preparation of the "composer license" command does not do any kind of command line parameter escaping! It is possible to inject code there, and even if that is unlikely, the command execution would easily fail if spaces are part of the parameters given.
Test cases:
composer-license-checker --composer "" fails with an identical error message mentioned above, but for a slightly different reason.
mkdir "fail me"
cd fail\ me
../vendor/bin/composer-license-checker --composer "composer"
...
In Application.php line 443:
Invalid working directory specified, /your/directory/fail does not exist.
Suggestions:
- Change the default value where to find Composer to be relative to the installation directory of composer-license-checker. That would probably be changing from
realpath('./vendor/bin/composer')torealpath(__DIR__'/../../../bin/composer'). This location supposedly is where the dependencies of the project requiring composer-license-checker are located, and should include composer. - Validate that
realpath()is returning a path to an existing executable (or more optimistically, not returningfalseor""). - Verify the exit code when executing
composer license, and fail if that isn't successful. - Apply
escapeshellarg()to the command string parameters, as they are user-defineable.
I'd like to have some feedback and thoughts, and would be willing to address these issues in some pull requests.
Hi Sven, thanks for triaging the issue(s) so thoroughly. Everything you've touched upon are bugs that I didn't stumble over because I've never used it this way. (Besides escaping shell args - that should always have happened)
I think all your suggestions should be implemented! Feel free to send PR's. I'll merge them right away.
Implementation-wise, you can probably use a single exception class for all exceptions related to running the external composer executable. Meaning, issues occurring during lookup of the executable as well as during execution.