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
5 years ago · 149250ab92
parent 4e43f849c7
commit 149250ab92
3 changed files with 63 additions and 24 deletions
--- 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('<error>Failed to update '.$url.' in cache, package installation for '.$package->getPrettyName().' might fail.</error>');
+                }
                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);
--- 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;
    }

--- 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())