From ae9cc3db58f80ed2703ad6fd06ffa84405c745ef Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 30 Oct 2019 10:45:40 +0100 Subject: [PATCH 1/5] Avoid clearing the error output during removeDirectory execution, losing git error output, fixes #8351 --- src/Composer/Util/Git.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 20457d708..574856dc7 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -215,10 +215,12 @@ class Git } } + $errorMsg = $this->process->getErrorOutput(); if ($initialClone) { $this->filesystem->removeDirectory($origCwd); } - $this->throwException('Failed to execute ' . $command . "\n\n" . $this->process->getErrorOutput(), $url); + + $this->throwException('Failed to execute ' . $command . "\n\n" . $errorMsg, $url); } } From b925d06861a045188f7fe1b2730d1e564e4ca6ac Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 30 Oct 2019 11:25:00 +0100 Subject: [PATCH 2/5] Fix gitlab support for basic-auth fallback from ssh URLs --- src/Composer/Util/Git.php | 12 ++++++++++-- src/Composer/Util/RemoteFilesystem.php | 4 ++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 574856dc7..c30178001 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -153,7 +153,14 @@ class Git return; } } - } elseif (preg_match('{^(https?)://' . self::getGitLabDomainsRegex($this->config) . '/(.*)}', $url, $match)) { + } elseif ( + preg_match('{^(git)@' . self::getGitLabDomainsRegex($this->config) . ':(.+?)\.git$}i', $url, $match) + || preg_match('{^(https?)://' . self::getGitLabDomainsRegex($this->config) . '/(.*)}', $url, $match) + ) { + if ($match[1] === 'git') { + $match[1] = 'https'; + } + if (!$this->io->hasAuthentication($match[2])) { $gitLabUtil = new GitLab($this->io, $this->config, $this->process); $message = 'Cloning failed, enter your GitLab credentials to access private repos'; @@ -165,11 +172,12 @@ class Git if ($this->io->hasAuthentication($match[2])) { $auth = $this->io->getAuthentication($match[2]); - if($auth['password'] === 'private-token' || $auth['password'] === 'oauth2') { + if ($auth['password'] === 'private-token' || $auth['password'] === 'oauth2' || $auth['password'] === 'gitlab-ci-token') { $authUrl = $match[1] . '://' . rawurlencode($auth['password']) . ':' . rawurlencode($auth['username']) . '@' . $match[2] . '/' . $match[3]; // swap username and password } else { $authUrl = $match[1] . '://' . rawurlencode($auth['username']) . ':' . rawurlencode($auth['password']) . '@' . $match[2] . '/' . $match[3]; } + $command = call_user_func($commandCallable, $authUrl); if (0 === $this->process->execute($command, $ignoredOutput, $cwd)) { return; diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index b3f51aae5..ed6d37241 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -689,7 +689,7 @@ class RemoteFilesystem $message = "\n".'Could not fetch '.$this->fileUrl.', enter your ' . $this->originUrl . ' credentials ' .($httpStatus === 401 ? 'to access private repos' : 'to go over the API rate limit'); $gitLabUtil = new GitLab($this->io, $this->config, null); - if ($this->io->hasAuthentication($this->originUrl) && ($auth = $this->io->getAuthentication($this->originUrl)) && $auth['password'] === 'private-token') { + if ($this->io->hasAuthentication($this->originUrl) && ($auth = $this->io->getAuthentication($this->originUrl)) && in_array($auth['password'], array('gitlab-ci-token', 'private-token'), true)) { throw new TransportException("Invalid credentials for '" . $this->fileUrl . "', aborting.", $httpStatus); } @@ -820,7 +820,7 @@ class RemoteFilesystem } elseif ($this->config && in_array($originUrl, $this->config->get('gitlab-domains'), true)) { if ($auth['password'] === 'oauth2') { $headers[] = 'Authorization: Bearer '.$auth['username']; - } elseif ($auth['password'] === 'private-token') { + } elseif ($auth['password'] === 'private-token' || $auth['password'] === 'gitlab-ci-token') { $headers[] = 'PRIVATE-TOKEN: '.$auth['username']; } } elseif ('bitbucket.org' === $originUrl From 12184aa9c51d5339c0b8c8a53c553794549f78d7 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 30 Oct 2019 12:01:23 +0100 Subject: [PATCH 3/5] Fix github auth to try https with pwd also, fixes #8356 --- src/Composer/Util/Git.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index c30178001..73e3139ce 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -95,8 +95,10 @@ class Git $auth = null; if ($bypassSshForGitHub || 0 !== $this->process->execute($command, $ignoredOutput, $cwd)) { - // private github repository without git access, try https with auth - if (preg_match('{^git@' . self::getGitHubDomainsRegex($this->config) . ':(.+?)\.git$}i', $url, $match)) { + // private github repository without ssh key access, try https with auth + if (preg_match('{^git@' . self::getGitHubDomainsRegex($this->config) . ':(.+?)\.git$}i', $url, $match) + || preg_match('{^(https?)://' . self::getGitHubDomainsRegex($this->config) . '/(.*)}', $url, $match) + ) { if (!$this->io->hasAuthentication($match[1])) { $gitHubUtil = new GitHub($this->io, $this->config, $this->process); $message = 'Cloning failed using an ssh key for authentication, enter your GitHub credentials to access private repos'; @@ -183,7 +185,7 @@ class Git return; } } - } elseif ($this->isAuthenticationFailure($url, $match)) { // private non-github repo that failed to authenticate + } elseif ($this->isAuthenticationFailure($url, $match)) { // private non-github/gitlab/bitbucket repo that failed to authenticate if (strpos($match[2], '@')) { list($authParts, $match[2]) = explode('@', $match[2], 2); } From 4e43f849c7a89dd9c83136a78da19673aba12ee4 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 30 Oct 2019 12:56:08 +0100 Subject: [PATCH 4/5] Avoid overwriting credentials with existing ones from git repos, refs #8293 --- src/Composer/Util/Git.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 73e3139ce..2da64d80d 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -54,9 +54,9 @@ class Git } if (!$initialClone) { - // capture username/password from URL if there is one + // capture username/password from URL if there is one and we have no auth configured yet $this->process->execute('git remote -v', $output, $cwd); - if (preg_match('{^(?:composer|origin)\s+https?://(.+):(.+)@([^/]+)}im', $output, $match)) { + if (preg_match('{^(?:composer|origin)\s+https?://(.+):(.+)@([^/]+)}im', $output, $match) && !$this->io->hasAuthentication($match[3])) { $this->io->setAuthentication($match[3], rawurldecode($match[1]), rawurldecode($match[2])); } } From 149250ab92feadd9fe045b54d6e42f33d185a595 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 30 Oct 2019 15:24:53 +0100 Subject: [PATCH 5/5] Remove credentials from git remotes in cache and vendor dirs This only removes the credentials if they are managed by composer auth.json or equivalent, if the credentials were present in the package URL to begin with they might remain Refs #8293 Fixes #3644 Closes #3608 --- src/Composer/Downloader/GitDownloader.php | 23 +++++++--- src/Composer/Util/Git.php | 21 +++++++-- .../Test/Downloader/GitDownloaderTest.php | 43 +++++++++++++------ 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php index f1657c6c4..edeaa7686 100644 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -51,31 +51,37 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $gitVersion = $this->gitUtil->getVersion(); $msg = "Cloning ".$this->getShortHash($ref); - $command = 'git clone --no-checkout %url% %path% && cd '.$flag.'%path% && git remote add composer %url% && git fetch composer'; + $command = 'git clone --no-checkout %url% %path% && cd '.$flag.'%path% && git remote add composer %url% && git fetch composer && git remote set-url origin %sanitizedUrl% && git remote set-url composer %sanitizedUrl%'; if ($gitVersion && version_compare($gitVersion, '2.3.0-rc0', '>=') && Cache::isUsable($cachePath)) { $this->io->writeError('', true, IOInterface::DEBUG); $this->io->writeError(sprintf(' Cloning to cache at %s', ProcessExecutor::escape($cachePath)), true, IOInterface::DEBUG); try { - $this->gitUtil->fetchRefOrSyncMirror($url, $cachePath, $ref); + if (!$this->gitUtil->fetchRefOrSyncMirror($url, $cachePath, $ref)) { + $this->io->writeError('Failed to update '.$url.' in cache, package installation for '.$package->getPrettyName().' might fail.'); + } if (is_dir($cachePath)) { $command = 'git clone --no-checkout %cachePath% %path% --dissociate --reference %cachePath% ' . '&& cd '.$flag.'%path% ' - . '&& git remote set-url origin %url% && git remote add composer %url%'; + . '&& git remote set-url origin %sanitizedUrl% && git remote add composer %sanitizedUrl%'; $msg = "Cloning ".$this->getShortHash($ref).' from cache'; } } catch (\RuntimeException $e) { + if (0 === strpos(get_class($e), 'PHPUnit')) { + throw $e; + } } } $this->io->writeError($msg); $commandCallable = function ($url) use ($path, $command, $cachePath) { return str_replace( - array('%url%', '%path%', '%cachePath%'), + array('%url%', '%path%', '%cachePath%', '%sanitizedUrl%'), array( ProcessExecutor::escape($url), ProcessExecutor::escape($path), ProcessExecutor::escape($cachePath), + ProcessExecutor::escape(preg_replace('{://([^@]+?):(.+?)@}', '://', $url)), ), $command ); @@ -119,10 +125,15 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $ref = $target->getSourceReference(); $this->io->writeError(" Checking out ".$this->getShortHash($ref)); - $command = 'git remote set-url composer %s && git rev-parse --quiet --verify %s || (git fetch composer && git fetch --tags composer)'; + $command = '(git remote set-url composer %s && git rev-parse --quiet --verify %s || (git fetch composer && git fetch --tags composer)) && git remote set-url composer %s'; $commandCallable = function ($url) use ($command, $ref) { - return sprintf($command, ProcessExecutor::escape($url), ProcessExecutor::escape($ref.'^{commit}')); + return sprintf( + $command, + ProcessExecutor::escape($url), + ProcessExecutor::escape($ref.'^{commit}'), + ProcessExecutor::escape(preg_replace('{://([^@]+?):(.+?)@}', '://', $url)) + ); }; $this->gitUtil->runCommand($commandCallable, $url, $path); diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 2da64d80d..15c46d080 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -240,7 +240,9 @@ class Git if (is_dir($dir) && 0 === $this->process->execute('git rev-parse --git-dir', $output, $dir) && trim($output) === '.') { try { $commandCallable = function ($url) { - return sprintf('git remote set-url origin %s && git remote update --prune origin', ProcessExecutor::escape($url)); + $sanitizedUrl = preg_replace('{://([^@]+?):(.+?)@}', '://', $url); + + return sprintf('git remote set-url origin %s && git remote update --prune origin && git remote set-url origin %s', ProcessExecutor::escape($url), ProcessExecutor::escape($sanitizedUrl)); }; $this->runCommand($commandCallable, $url, $dir); } catch (\Exception $e) { @@ -263,17 +265,28 @@ class Git } public function fetchRefOrSyncMirror($url, $dir, $ref) + { + if ($this->checkRefIsInMirror($url, $dir, $ref)) { + return true; + } + + if ($this->syncMirror($url, $dir)) { + return $this->checkRefIsInMirror($url, $dir, $ref); + } + + return false; + } + + private function checkRefIsInMirror($url, $dir, $ref) { if (is_dir($dir) && 0 === $this->process->execute('git rev-parse --git-dir', $output, $dir) && trim($output) === '.') { $escapedRef = ProcessExecutor::escape($ref.'^{commit}'); - $exitCode = $this->process->execute(sprintf('git rev-parse --quiet --verify %s', $escapedRef), $output, $dir); + $exitCode = $this->process->execute(sprintf('git rev-parse --quiet --verify %s', $escapedRef), $ignoredOutput, $dir); if ($exitCode === 0) { return true; } } - $this->syncMirror($url, $dir); - return false; } diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index c3cd31a4a..cd3375464 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -108,7 +108,7 @@ class GitDownloaderTest extends TestCase return 0; })); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer && git remote set-url origin 'https://example.com/composer/composer' && git remote set-url composer 'https://example.com/composer/composer'"); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -163,6 +163,9 @@ class GitDownloaderTest extends TestCase $this->setupConfig($config); $cachePath = $config->get('cache-vcs-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', 'https://example.com/composer/composer').'/'; + $filesystem = new \Composer\Util\Filesystem; + $filesystem->removeDirectory($cachePath); + $expectedGitCommand = $this->winCompat(sprintf("git clone --mirror 'https://example.com/composer/composer' '%s'", $cachePath)); $processExecutor->expects($this->at(1)) ->method('execute') @@ -172,24 +175,36 @@ class GitDownloaderTest extends TestCase return 0; })); + $processExecutor->expects($this->at(2)) + ->method('execute') + ->with($this->equalTo('git rev-parse --git-dir'), $this->anything(), $this->equalTo($this->winCompat($cachePath))) + ->will($this->returnCallback(function ($command, &$output = null) { + $output = '.'; + + return 0; + })); + $processExecutor->expects($this->at(3)) + ->method('execute') + ->with($this->equalTo($this->winCompat('git rev-parse --quiet --verify \'1234567890123456789012345678901234567890^{commit}\'')), $this->equalTo(null), $this->equalTo($this->winCompat($cachePath))) + ->will($this->returnValue(0)); $expectedGitCommand = $this->winCompat(sprintf("git clone --no-checkout '%1\$s' 'composerPath' --dissociate --reference '%1\$s' && cd 'composerPath' && git remote set-url origin 'https://example.com/composer/composer' && git remote add composer 'https://example.com/composer/composer'", $cachePath)); - $processExecutor->expects($this->at(2)) + $processExecutor->expects($this->at(4)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) + $processExecutor->expects($this->at(5)) ->method('execute') ->with($this->equalTo($this->winCompat("git branch -r")), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) + $processExecutor->expects($this->at(6)) ->method('execute') ->with($this->equalTo($this->winCompat("git checkout 'master' --")), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(5)) + $processExecutor->expects($this->at(7)) ->method('execute') ->with($this->equalTo($this->winCompat("git reset --hard '1234567890123456789012345678901234567890' --")), $this->equalTo(null), $this->equalTo($this->winCompat('composerPath'))) ->will($this->returnValue(0)); @@ -225,7 +240,7 @@ class GitDownloaderTest extends TestCase return 0; })); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://github.com/mirrors/composer' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://github.com/mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://github.com/mirrors/composer' && git fetch composer && git remote set-url origin 'https://github.com/mirrors/composer' && git remote set-url composer 'https://github.com/mirrors/composer'"); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -236,7 +251,7 @@ class GitDownloaderTest extends TestCase ->with() ->will($this->returnValue('Error1')); - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'git@github.com:mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'git@github.com:mirrors/composer' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout 'git@github.com:mirrors/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'git@github.com:mirrors/composer' && git fetch composer && git remote set-url origin 'git@github.com:mirrors/composer' && git remote set-url composer 'git@github.com:mirrors/composer'"); $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -309,7 +324,7 @@ class GitDownloaderTest extends TestCase return 0; })); - $expectedGitCommand = $this->winCompat("git clone --no-checkout '{$url}' 'composerPath' && cd 'composerPath' && git remote add composer '{$url}' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout '{$url}' 'composerPath' && cd 'composerPath' && git remote add composer '{$url}' && git fetch composer && git remote set-url origin '{$url}' && git remote set-url composer '{$url}'"); $processExecutor->expects($this->at(1)) ->method('execute') ->with($this->equalTo($expectedGitCommand)) @@ -337,7 +352,7 @@ class GitDownloaderTest extends TestCase */ public function testDownloadThrowsRuntimeExceptionIfGitCommandFails() { - $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer"); + $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer && git remote set-url origin 'https://example.com/composer/composer' && git remote set-url composer 'https://example.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) ->method('getSourceReference') @@ -380,7 +395,7 @@ class GitDownloaderTest extends TestCase public function testUpdate() { - $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); + $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -429,7 +444,7 @@ class GitDownloaderTest extends TestCase public function testUpdateWithNewRepoUrl() { - $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); + $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -501,7 +516,7 @@ composer https://github.com/old/url (push) */ public function testUpdateThrowsRuntimeExceptionIfGitCommandFails() { - $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); + $expectedGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -542,8 +557,8 @@ composer https://github.com/old/url (push) public function testUpdateDoesntThrowsRuntimeExceptionIfGitCommandFailsAtFirstButIsAbleToRecover() { - $expectedFirstGitUpdateCommand = $this->winCompat("git remote set-url composer '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); - $expectedSecondGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); + $expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer ''"); + $expectedSecondGitUpdateCommand = $this->winCompat("(git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer 'https://github.com/composer/composer'"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any())