From 149250ab92feadd9fe045b54d6e42f33d185a595 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 30 Oct 2019 15:24:53 +0100 Subject: [PATCH] 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())