From 939c998bafcbace3cf85ae7820a8f0654e73b7c9 Mon Sep 17 00:00:00 2001 From: Tom Klingenberg Date: Mon, 11 Apr 2022 23:58:59 +0200 Subject: [PATCH 1/6] validate lock-file if configured (#10715, --check-lock) if no lock-file is configured, turn lock file validation errors into warnings (implicit --no-check-lock) unless those are explicitly promoted via the new --check-lock option. - `{"config": {"lock": false}}` is an implicit `--no-check-lock` for composer validate. - `--check-lock` overrides an (implicit or explicit) `--no-check-lock`, always. issue: #10715 --- phpstan/baseline.neon | 10 ++++++++++ src/Composer/Command/ValidateCommand.php | 3 +++ 2 files changed, 13 insertions(+) diff --git a/phpstan/baseline.neon b/phpstan/baseline.neon index e8bb60c5c..bcf41c4b1 100644 --- a/phpstan/baseline.neon +++ b/phpstan/baseline.neon @@ -2375,6 +2375,11 @@ parameters: count: 2 path: ../src/Composer/Command/ValidateCommand.php + - + message: "#^Only booleans are allowed in &&, mixed given on the right side\\.$#" + count: 1 + path: ../src/Composer/Command/ValidateCommand.php + - message: "#^Only booleans are allowed in a negated boolean, array\\ given\\.$#" count: 1 @@ -2410,6 +2415,11 @@ parameters: count: 1 path: ../src/Composer/Command/ValidateCommand.php + - + message: "#^Only booleans are allowed in \\|\\|, mixed given on the right side\\.$#" + count: 1 + path: ../src/Composer/Command/ValidateCommand.php + - message: "#^Parameter \\#1 \\$function of function call_user_func expects callable\\(\\)\\: mixed, array\\{Composer\\\\Package\\\\RootPackageInterface, 'getDevRequires'\\|'getRequires'\\} given\\.$#" count: 1 diff --git a/src/Composer/Command/ValidateCommand.php b/src/Composer/Command/ValidateCommand.php index e66cbf23b..c9650c560 100644 --- a/src/Composer/Command/ValidateCommand.php +++ b/src/Composer/Command/ValidateCommand.php @@ -45,6 +45,7 @@ class ValidateCommand extends BaseCommand ->setDescription('Validates a composer.json and composer.lock.') ->setDefinition(array( new InputOption('no-check-all', null, InputOption::VALUE_NONE, 'Do not validate requires for overly strict/loose constraints'), + new InputOption('check-lock', null, InputOption::VALUE_NONE, 'Check if lock file is up to date (even config.lock is false, overrides --no-check-lock)'), new InputOption('no-check-lock', null, InputOption::VALUE_NONE, 'Do not check if lock file is up to date'), new InputOption('no-check-publish', null, InputOption::VALUE_NONE, 'Do not check for publish errors'), new InputOption('no-check-version', null, InputOption::VALUE_NONE, 'Do not report a warning if the version field is present'), @@ -95,6 +96,8 @@ EOT $lockErrors = array(); $composer = Factory::create($io, $file, $input->hasParameterOption('--no-plugins')); + // config.lock = false ~= implicit --no-check-lock; --check-lock overrides + $checkLock = ($checkLock && $composer->getConfig()->get('lock')) || $input->getOption('check-lock'); $locker = $composer->getLocker(); if ($locker->isLocked() && !$locker->isFresh()) { $lockErrors[] = '- The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update `.'; From bb0edce0950bfc0eea1b464405991a5e8b7e257d Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 13 Apr 2022 14:52:13 +0200 Subject: [PATCH 2/6] Fixed lock file being used when lock:false is in config, refs #10715 (#10726) --- doc/06-config.md | 2 +- phpstan/baseline.neon | 121 ++++----------------------------- src/Composer/Factory.php | 5 +- src/Composer/Util/Platform.php | 12 ++++ 4 files changed, 30 insertions(+), 110 deletions(-) diff --git a/doc/06-config.md b/doc/06-config.md index 636cf9555..d89aa062d 100644 --- a/doc/06-config.md +++ b/doc/06-config.md @@ -396,7 +396,7 @@ in the Composer home, cache, and data directories. ## lock Defaults to `true`. If set to `false`, Composer will not create a `composer.lock` -file. +file and will ignore it if one is present. ## platform-check diff --git a/phpstan/baseline.neon b/phpstan/baseline.neon index e8bb60c5c..ad57bc557 100644 --- a/phpstan/baseline.neon +++ b/phpstan/baseline.neon @@ -127,7 +127,7 @@ parameters: - message: "#^Parameter \\#1 \\$str of function strtr expects string, string\\|false given\\.$#" - count: 3 + count: 2 path: ../src/Composer/Autoload/AutoloadGenerator.php - @@ -605,11 +605,6 @@ parameters: count: 2 path: ../src/Composer/Command/ConfigCommand.php - - - message: "#^Parameter \\#1 \\$path of function dirname expects string, string\\|false given\\.$#" - count: 1 - path: ../src/Composer/Command/ConfigCommand.php - - message: "#^Parameter \\#1 \\$path of function realpath expects string, string\\|false given\\.$#" count: 1 @@ -1070,11 +1065,6 @@ parameters: count: 1 path: ../src/Composer/Command/InitCommand.php - - - message: "#^Only booleans are allowed in a negated boolean, true\\|null given\\.$#" - count: 1 - path: ../src/Composer/Command/InitCommand.php - - message: "#^Only booleans are allowed in a ternary operator condition, Composer\\\\Composer\\|null given\\.$#" count: 1 @@ -1662,7 +1652,7 @@ parameters: - message: "#^Only booleans are allowed in &&, string\\|null given on the left side\\.$#" - count: 2 + count: 1 path: ../src/Composer/Command/SelfUpdateCommand.php - @@ -2440,11 +2430,6 @@ parameters: count: 1 path: ../src/Composer/Compiler.php - - - message: "#^Parameter \\#1 \\$haystack of function strpos expects string, string\\|false given\\.$#" - count: 1 - path: ../src/Composer/Compiler.php - - message: "#^Parameter \\#1 \\$source of method Composer\\\\Compiler\\:\\:stripWhitespace\\(\\) expects string, string\\|false given\\.$#" count: 1 @@ -2452,11 +2437,6 @@ parameters: - message: "#^Parameter \\#1 \\$str of function strtr expects string, string\\|false given\\.$#" - count: 2 - path: ../src/Composer/Compiler.php - - - - message: "#^Parameter \\#1 \\$str of function substr_replace expects array\\|string, string\\|false given\\.$#" count: 1 path: ../src/Composer/Compiler.php @@ -3505,16 +3485,6 @@ parameters: count: 1 path: ../src/Composer/Downloader/PathDownloader.php - - - message: "#^Call to function in_array\\(\\) with arguments 20, non\\-empty\\-array\\ and true will always evaluate to true\\.$#" - count: 1 - path: ../src/Composer/Downloader/PathDownloader.php - - - - message: "#^Else branch is unreachable because previous condition is always true\\.$#" - count: 1 - path: ../src/Composer/Downloader/PathDownloader.php - - message: "#^Only booleans are allowed in an if condition, array\\\\|null given\\.$#" count: 1 @@ -3871,13 +3841,13 @@ parameters: path: ../src/Composer/Factory.php - - message: "#^Offset 'line' does not exist on array\\{text\\?\\: string, token\\?\\: string, line\\?\\: int, loc\\?\\: array\\{first_line\\: int, first_column\\: int, last_line\\: int, last_column\\: int\\}, expected\\?\\: array\\, key\\: string\\}\\.$#" - count: 1 + message: "#^Only booleans are allowed in &&, Composer\\\\IO\\\\IOInterface\\|null given on the left side\\.$#" + count: 3 path: ../src/Composer/Factory.php - - message: "#^Only booleans are allowed in &&, Composer\\\\IO\\\\IOInterface\\|null given on the left side\\.$#" - count: 3 + message: "#^Only booleans are allowed in a negated boolean, mixed given\\.$#" + count: 1 path: ../src/Composer/Factory.php - @@ -3885,6 +3855,11 @@ parameters: count: 3 path: ../src/Composer/Factory.php + - + message: "#^Only booleans are allowed in a ternary operator condition, mixed given\\.$#" + count: 1 + path: ../src/Composer/Factory.php + - message: "#^Only booleans are allowed in an if condition, Composer\\\\IO\\\\IOInterface\\|null given\\.$#" count: 1 @@ -4480,14 +4455,9 @@ parameters: count: 1 path: ../src/Composer/Package/Archiver/ArchivableFilesFinder.php - - - message: "#^Parameter \\#1 \\$haystack of function strpos expects string, string\\|false given\\.$#" - count: 1 - path: ../src/Composer/Package/Archiver/ArchivableFilesFinder.php - - message: "#^Parameter \\#1 \\$path of method Composer\\\\Util\\\\Filesystem\\:\\:normalizePath\\(\\) expects string, string\\|false given\\.$#" - count: 2 + count: 1 path: ../src/Composer/Package/Archiver/ArchivableFilesFinder.php - @@ -5185,11 +5155,6 @@ parameters: count: 1 path: ../src/Composer/Repository/ArtifactRepository.php - - - message: "#^Parameter \\#1 \\$filename of function sha1_file expects string, string\\|false given\\.$#" - count: 1 - path: ../src/Composer/Repository/ArtifactRepository.php - - message: "#^Anonymous function uses \\$this assigned to variable \\$repo\\. Use \\$this directly in the function body\\.$#" count: 3 @@ -5225,11 +5190,6 @@ parameters: count: 2 path: ../src/Composer/Repository/ComposerRepository.php - - - message: "#^Method Composer\\\\Repository\\\\ComposerRepository\\:\\:getProviderNames\\(\\) should return array\\ but returns array\\\\.$#" - count: 1 - path: ../src/Composer/Repository/ComposerRepository.php - - message: "#^Method Composer\\\\Repository\\\\ComposerRepository\\:\\:getProviders\\(\\) should return array\\ but returns array\\\\.$#" count: 1 @@ -5342,7 +5302,7 @@ parameters: - message: "#^Only booleans are allowed in an if condition, string\\|null given\\.$#" - count: 13 + count: 12 path: ../src/Composer/Repository/ComposerRepository.php - @@ -5350,11 +5310,6 @@ parameters: count: 1 path: ../src/Composer/Repository/ComposerRepository.php - - - message: "#^Parameter \\#1 \\$input of function array_keys expects array, array\\\\>\\|null given\\.$#" - count: 1 - path: ../src/Composer/Repository/ComposerRepository.php - - message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#" count: 3 @@ -6265,11 +6220,6 @@ parameters: count: 1 path: ../src/Composer/SelfUpdate/Keys.php - - - message: "#^Only booleans are allowed in a negated boolean, array\\\\>\\> given\\.$#" - count: 1 - path: ../src/Composer/SelfUpdate/Versions.php - - message: "#^Only booleans are allowed in an if condition, string given\\.$#" count: 1 @@ -6915,11 +6865,6 @@ parameters: count: 1 path: ../src/Composer/Util/HttpDownloader.php - - - message: "#^Binary operation \"\\.\" between non\\-empty\\-string and non\\-empty\\-array\\\\|non\\-empty\\-string results in an error\\.$#" - count: 1 - path: ../src/Composer/Util/HttpDownloader.php - - message: "#^Cannot call method abortRequest\\(\\) on Composer\\\\Util\\\\Http\\\\CurlDownloader\\|null\\.$#" count: 1 @@ -7615,11 +7560,6 @@ parameters: count: 3 path: ../tests/Composer/Test/AllFunctionalTest.php - - - message: "#^Method Composer\\\\Test\\\\AllFunctionalTest\\:\\:getTestFiles\\(\\) should return array\\\\> but returns array\\\\>\\.$#" - count: 1 - path: ../tests/Composer/Test/AllFunctionalTest.php - - message: "#^Only booleans are allowed in an if condition, string\\|false given\\.$#" count: 1 @@ -7695,31 +7635,11 @@ parameters: count: 2 path: ../tests/Composer/Test/Autoload/AutoloadGeneratorTest.php - - - message: "#^Parameter \\#1 \\$expected of static method Composer\\\\Test\\\\Autoload\\\\AutoloadGeneratorTest\\:\\:assertEqualsNormalized\\(\\) expects string, string\\|false given\\.$#" - count: 1 - path: ../tests/Composer/Test/Autoload/AutoloadGeneratorTest.php - - message: "#^Parameter \\#1 \\$new_include_path of function set_include_path expects string, string\\|false given\\.$#" count: 2 path: ../tests/Composer/Test/Autoload/AutoloadGeneratorTest.php - - - message: "#^Parameter \\#2 \\$actual of static method Composer\\\\Test\\\\Autoload\\\\AutoloadGeneratorTest\\:\\:assertEqualsNormalized\\(\\) expects string, string\\|false given\\.$#" - count: 1 - path: ../tests/Composer/Test/Autoload/AutoloadGeneratorTest.php - - - - message: "#^Parameter \\#2 \\$haystack of method PHPUnit\\\\Framework\\\\Assert\\:\\:assertStringContainsString\\(\\) expects string, string\\|false given\\.$#" - count: 8 - path: ../tests/Composer/Test/Autoload/AutoloadGeneratorTest.php - - - - message: "#^Parameter \\#2 \\$haystack of method PHPUnit\\\\Framework\\\\Assert\\:\\:assertStringNotContainsString\\(\\) expects string, string\\|false given\\.$#" - count: 3 - path: ../tests/Composer/Test/Autoload/AutoloadGeneratorTest.php - - message: "#^Property Composer\\\\Test\\\\Autoload\\\\AutoloadGeneratorTest\\:\\:\\$origDir \\(string\\) does not accept string\\|false\\.$#" count: 1 @@ -7835,11 +7755,6 @@ parameters: count: 1 path: ../tests/Composer/Test/DependencyResolver/PoolBuilderTest.php - - - message: "#^Parameter \\#1 \\$filename of function file_get_contents expects string, string\\|false given\\.$#" - count: 1 - path: ../tests/Composer/Test/DependencyResolver/PoolBuilderTest.php - - message: "#^Parameter \\#2 \\$fixturesDir of method Composer\\\\Test\\\\DependencyResolver\\\\PoolBuilderTest\\:\\:readTestFile\\(\\) expects string, string\\|false given\\.$#" count: 1 @@ -7875,11 +7790,6 @@ parameters: count: 1 path: ../tests/Composer/Test/DependencyResolver/PoolOptimizerTest.php - - - message: "#^Parameter \\#1 \\$filename of function file_get_contents expects string, string\\|false given\\.$#" - count: 1 - path: ../tests/Composer/Test/DependencyResolver/PoolOptimizerTest.php - - message: "#^Parameter \\#2 \\$fixturesDir of method Composer\\\\Test\\\\DependencyResolver\\\\PoolOptimizerTest\\:\\:readTestFile\\(\\) expects string, string\\|false given\\.$#" count: 1 @@ -8208,11 +8118,6 @@ parameters: count: 1 path: ../tests/Composer/Test/InstallerTest.php - - - message: "#^Parameter \\#1 \\$filename of function file_get_contents expects string, string\\|false given\\.$#" - count: 1 - path: ../tests/Composer/Test/InstallerTest.php - - message: "#^Parameter \\#1 \\$fp of function fseek expects resource, resource\\|false given\\.$#" count: 1 diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index ee0b07f3e..9288da19b 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -442,8 +442,11 @@ class Factory // init locker if possible if ($fullLoad && isset($composerFile)) { $lockFile = self::getLockFile($composerFile); + if (!$config->get('lock') && file_exists($lockFile)) { + $io->writeError(''.$lockFile.' is present but ignored as the "lock" config option is disabled.'); + } - $locker = new Package\Locker($io, new JsonFile($lockFile, null, $io), $im, file_get_contents($composerFile), $process); + $locker = new Package\Locker($io, new JsonFile($config->get('lock') ? $lockFile : Platform::getDevNull(), null, $io), $im, file_get_contents($composerFile), $process); $composer->setLocker($locker); } diff --git a/src/Composer/Util/Platform.php b/src/Composer/Util/Platform.php index 756c11506..e65c64534 100644 --- a/src/Composer/Util/Platform.php +++ b/src/Composer/Util/Platform.php @@ -249,4 +249,16 @@ class Platform return self::$isVirtualBoxGuest; } + + /** + * @return 'NUL'|'/dev/null' + */ + public static function getDevNull() + { + if (self::isWindows()) { + return 'NUL'; + } + + return '/dev/null'; + } } From 0a8dfe6ef76014e7d202368503b0346c72552da7 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 13 Apr 2022 15:17:07 +0200 Subject: [PATCH 3/6] Clarify that autoloader-suffix should be a non-empty-string, fixes #10720 (#10725) --- doc/06-config.md | 4 ++-- phpstan/config.neon | 1 + src/Composer/Autoload/AutoloadGenerator.php | 19 +++++++++++++------ src/Composer/Config.php | 7 +++++++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/doc/06-config.md b/doc/06-config.md index d89aa062d..5217b18f2 100644 --- a/doc/06-config.md +++ b/doc/06-config.md @@ -315,8 +315,8 @@ with other autoloaders. ## autoloader-suffix -Defaults to `null`. String to be used as a suffix for the generated Composer -autoloader. When null a random one will be generated. +Defaults to `null`. Non-empty string to be used as a suffix for the generated +Composer autoloader. When null a random one will be generated. ## optimize-autoloader diff --git a/phpstan/config.neon b/phpstan/config.neon index 457123ae3..a2468327f 100644 --- a/phpstan/config.neon +++ b/phpstan/config.neon @@ -16,6 +16,7 @@ parameters: - '../src/Composer/Console/HtmlOutputFormatter.php' reportUnmatchedIgnoredErrors: false + treatPhpDocTypesAsCertain: false ignoreErrors: # unused parameters diff --git a/src/Composer/Autoload/AutoloadGenerator.php b/src/Composer/Autoload/AutoloadGenerator.php index 265b890d6..87b346595 100644 --- a/src/Composer/Autoload/AutoloadGenerator.php +++ b/src/Composer/Autoload/AutoloadGenerator.php @@ -159,12 +159,12 @@ class AutoloadGenerator /** * @param string $targetDir * @param bool $scanPsrPackages - * @param string $suffix + * @param string|null $suffix * @return int * @throws \Seld\JsonLint\ParsingException * @throws \RuntimeException */ - public function dump(Config $config, InstalledRepositoryInterface $localRepo, RootPackageInterface $rootPackage, InstallationManager $installationManager, $targetDir, $scanPsrPackages = false, $suffix = '') + public function dump(Config $config, InstalledRepositoryInterface $localRepo, RootPackageInterface $rootPackage, InstallationManager $installationManager, $targetDir, $scanPsrPackages = false, $suffix = null) { if ($this->classMapAuthoritative) { // Force scanPsrPackages when classmap is authoritative @@ -374,16 +374,23 @@ EOF; } $classmapFile .= ");\n"; - if (!$suffix) { - if (!$config->get('autoloader-suffix') && Filesystem::isReadable($vendorPath.'/autoload.php')) { + if ('' === $suffix) { + $suffix = null; + } + if (null === $suffix) { + $suffix = $config->get('autoloader-suffix'); + + // carry over existing autoload.php's suffix if possible and none is configured + if (null === $suffix && Filesystem::isReadable($vendorPath.'/autoload.php')) { $content = file_get_contents($vendorPath.'/autoload.php'); if (Preg::isMatch('{ComposerAutoloaderInit([^:\s]+)::}', $content, $match)) { $suffix = $match[1]; } } - if (!$suffix) { - $suffix = $config->get('autoloader-suffix') ?: md5(uniqid('', true)); + // generate one if we still haven't got a suffix + if (null === $suffix) { + $suffix = md5(uniqid('', true)); } } diff --git a/src/Composer/Config.php b/src/Composer/Config.php index 0d927f190..e81c11738 100644 --- a/src/Composer/Config.php +++ b/src/Composer/Config.php @@ -427,6 +427,13 @@ class Config return $protos; + case 'autoloader-suffix': + if ($this->config[$key] === '') { // we need to guarantee null or non-empty-string + return null; + } + + return $this->process($this->config[$key], $flags); + default: if (!isset($this->config[$key])) { return null; From 915b97fc398f9e5f230540c1cf59680af48580c9 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 13 Apr 2022 15:22:18 +0200 Subject: [PATCH 4/6] Fix docs --- src/Composer/Command/ValidateCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Command/ValidateCommand.php b/src/Composer/Command/ValidateCommand.php index c9650c560..7ac57e945 100644 --- a/src/Composer/Command/ValidateCommand.php +++ b/src/Composer/Command/ValidateCommand.php @@ -45,7 +45,7 @@ class ValidateCommand extends BaseCommand ->setDescription('Validates a composer.json and composer.lock.') ->setDefinition(array( new InputOption('no-check-all', null, InputOption::VALUE_NONE, 'Do not validate requires for overly strict/loose constraints'), - new InputOption('check-lock', null, InputOption::VALUE_NONE, 'Check if lock file is up to date (even config.lock is false, overrides --no-check-lock)'), + new InputOption('check-lock', null, InputOption::VALUE_NONE, 'Check if lock file is up to date (even when config.lock is false)'), new InputOption('no-check-lock', null, InputOption::VALUE_NONE, 'Do not check if lock file is up to date'), new InputOption('no-check-publish', null, InputOption::VALUE_NONE, 'Do not check for publish errors'), new InputOption('no-check-version', null, InputOption::VALUE_NONE, 'Do not report a warning if the version field is present'), From 2c40c53637c5c7e43fff7c09d3d324d632734709 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 13 Apr 2022 14:54:58 +0100 Subject: [PATCH 5/6] Merge pull request from GHSA-x7cr-6qr6-2hh6 * GitDriver: filter branch names starting with a - character * GitDriver: getFileContent prevent identifiers starting with a - * HgDriver: prevent invalid identifiers and prevent file from running commands * HgDriver: filter branches starting with a - character --- src/Composer/Repository/Vcs/GitDriver.php | 6 +- src/Composer/Repository/Vcs/HgDriver.php | 10 ++- .../Test/Repository/Vcs/GitDriverTest.php | 81 +++++++++++++++++++ .../Test/Repository/Vcs/HgDriverTest.php | 46 +++++++++++ 4 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 tests/Composer/Test/Repository/Vcs/GitDriverTest.php diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php index b91508acd..039f9052f 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -138,6 +138,10 @@ class GitDriver extends VcsDriver */ public function getFileContent($file, $identifier) { + if (isset($identifier[0]) && $identifier[0] === '-') { + throw new \RuntimeException('Invalid git identifier detected. Identifier must not start with a -, given: ' . $identifier); + } + $resource = sprintf('%s:%s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); $this->process->execute(sprintf('git show %s', $resource), $content, $this->repoDir); @@ -191,7 +195,7 @@ class GitDriver extends VcsDriver $this->process->execute('git branch --no-color --no-abbrev -v', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { if ($branch && !Preg::isMatch('{^ *[^/]+/HEAD }', $branch)) { - if (Preg::isMatch('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match)) { + if (Preg::isMatch('{^(?:\* )? *(\S+) *([a-f0-9]+)(?: .*)?$}', $branch, $match) && $match[1][0] !== '-') { $branches[$match[1]] = $match[2]; } } diff --git a/src/Composer/Repository/Vcs/HgDriver.php b/src/Composer/Repository/Vcs/HgDriver.php index 73bc426e5..f9f8064ba 100644 --- a/src/Composer/Repository/Vcs/HgDriver.php +++ b/src/Composer/Repository/Vcs/HgDriver.php @@ -126,7 +126,11 @@ class HgDriver extends VcsDriver */ public function getFileContent($file, $identifier) { - $resource = sprintf('hg cat -r %s %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); + if (isset($identifier[0]) && $identifier[0] === '-') { + throw new \RuntimeException('Invalid hg identifier detected. Identifier must not start with a -, given: ' . $identifier); + } + + $resource = sprintf('hg cat -r %s -- %s', ProcessExecutor::escape($identifier), ProcessExecutor::escape($file)); $this->process->execute($resource, $content, $this->repoDir); if (!trim($content)) { @@ -186,14 +190,14 @@ class HgDriver extends VcsDriver $this->process->execute('hg branches', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { - if ($branch && Preg::isMatch('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match)) { + if ($branch && Preg::isMatch('(^([^\s]+)\s+\d+:([a-f0-9]+))', $branch, $match) && $match[1][0] !== '-') { $branches[$match[1]] = $match[2]; } } $this->process->execute('hg bookmarks', $output, $this->repoDir); foreach ($this->process->splitLines($output) as $branch) { - if ($branch && Preg::isMatch('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match)) { + if ($branch && Preg::isMatch('(^(?:[\s*]*)([^\s]+)\s+\d+:(.*)$)', $branch, $match) && $match[1][0] !== '-') { $bookmarks[$match[1]] = $match[2]; } } diff --git a/tests/Composer/Test/Repository/Vcs/GitDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php new file mode 100644 index 000000000..c8cfdab0a --- /dev/null +++ b/tests/Composer/Test/Repository/Vcs/GitDriverTest.php @@ -0,0 +1,81 @@ +home = self::getUniqueTmpDirectory(); + $this->config = new Config(); + $this->config->merge(array( + 'config' => array( + 'home' => $this->home, + ), + )); + } + + public function testGetBranchesFilterInvalidBranchNames() + { + $process = new ProcessExecutorMock; + $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + + $driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + $this->setRepoDir($driver, $this->home); + + // Branches starting with a - character are not valid git branches names + // Still assert that they get filtered to prevent issues later on + $stdout = <<expects(array(array( + 'cmd' => 'git branch --no-color --no-abbrev -v', + 'stdout' => $stdout, + ))); + + $branches = $driver->getBranches(); + $this->assertSame(array( + 'main' => '089681446ba44d6d9004350192486f2ceb4eaa06', + '2.2' => '12681446ba44d6d9004350192486f2ceb4eaa06', + ), $branches); + } + + public function testFileGetContentInvalidIdentifier() + { + $this->expectException('\RuntimeException'); + + $process = new ProcessExecutorMock; + $io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); + $driver = new GitDriver(array('url' => 'https://example.org/acme.git'), $io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + + $this->assertNull($driver->getFileContent('file.txt', 'h')); + + $driver->getFileContent('file.txt', '-h'); + } + + /** + * @param GitDriver $driver + * @param string $path + */ + private function setRepoDir($driver, $path) + { + $reflectionClass = new \ReflectionClass($driver); + $reflectionProperty = $reflectionClass->getProperty('repoDir'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($driver, $path); + } +} diff --git a/tests/Composer/Test/Repository/Vcs/HgDriverTest.php b/tests/Composer/Test/Repository/Vcs/HgDriverTest.php index 2fcd27bdf..356a28c48 100644 --- a/tests/Composer/Test/Repository/Vcs/HgDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/HgDriverTest.php @@ -13,6 +13,7 @@ namespace Composer\Test\Repository\Vcs; use Composer\Repository\Vcs\HgDriver; +use Composer\Test\Mock\ProcessExecutorMock; use Composer\Test\TestCase; use Composer\Util\Filesystem; use Composer\Config; @@ -66,4 +67,49 @@ class HgDriverTest extends TestCase array('https://user@bitbucket.org/user/repo'), ); } + + public function testGetBranchesFilterInvalidBranchNames() + { + $process = new ProcessExecutorMock; + + $driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + + $stdout = <<expects(array(array( + 'cmd' => 'hg branches', + 'stdout' => $stdout, + ), array( + 'cmd' => 'hg bookmarks', + 'stdout' => $stdout1, + ))); + + $branches = $driver->getBranches(); + $this->assertSame(array( + 'help' => 'dbf6c8acb641', + 'default' => 'dbf6c8acb640', + ), $branches); + } + + public function testFileGetContentInvalidIdentifier() + { + $this->expectException('\RuntimeException'); + + $process = new ProcessExecutorMock; + $driver = new HgDriver(array('url' => 'https://example.org/acme.git'), $this->io, $this->config, $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock(), $process); + + $this->assertNull($driver->getFileContent('file.txt', 'h')); + + $driver->getFileContent('file.txt', '-h'); + } } From 2ba8758b30b985d0b98237199c04da445c7c5714 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 13 Apr 2022 16:00:31 +0200 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61acf5863..0ff872a26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +### [2.2.12] 2022-04-13 + + * Security: Fixed command injection vulnerability in HgDriver/GitDriver (GHSA-x7cr-6qr6-2hh6 / CVE-2022-24828) + * Fixed curl downloader not retrying when a DNS resolution failure occurs (#10716) + * Fixed composer.lock file still being used/read when the `lock` config option is disabled (#10726) + * Fixed `validate` command checking the lock file even if the `lock` option is disabled (#10723) + ### [2.2.11] 2022-04-01 * Added missing config.bitbucket-oauth in composer-schema.json @@ -1420,6 +1427,7 @@ * Initial release +[2.2.12]: https://github.com/composer/composer/compare/2.2.11...2.2.12 [2.2.11]: https://github.com/composer/composer/compare/2.2.10...2.2.11 [2.2.10]: https://github.com/composer/composer/compare/2.2.9...2.2.10 [2.2.9]: https://github.com/composer/composer/compare/2.2.8...2.2.9