Backport the fixes of the last audit of 2.0

CHANGELOG.md

@@ -2,6 +2,39 @@
 
 All notable changes to this project are documented in this file.
 
+## [v1.7.2] - Unreleased
+
+Changes since [v1.7.1](https://github.com/gitonomy/gitlib/releases/tag/v1.7.1).
+
+### Security
+
+-   Fix `Repository::isValidRepository()` and `isValidRepositoryAndBranch()` reading the
+    configuration of a repository of the temporary directory, where they run `git ls-remote`:
+    another user of the machine may create a `.git` there, whose rewrites of URLs
+    (`url.<base>.insteadOf`) make git run a command, or check another repository.
+-   Refuse a `git://` URL whose port starts with a dash, even encoded, like
+    `git://host:-oProxyCommand=.../repository`: old versions of git, 1.8.4 for instance, pass it
+    to the command of `core.gitProxy`, which reads it as an option.
+
+### Bug fixes
+
+-   Fix a `TypeError` when hiding the credentials of a long text, the megabytes of a blob in the
+    logs for instance: with a logger, reading a file with a long password in a URL failed. It
+    no longer takes seconds either, without the JIT of PCRE.
+-   Fix `WorkingCopy::getUntrackedFiles()` returning the former name of a staged rename, when
+    it starts with `?? `.
+-   Fix a hash ending with a new line read as a hash: `Repository::getCommit()` returned a
+    commit holding it, instead of throwing a `ReferenceNotFoundException`, and `getTree()` and
+    `getBlob()` did not trigger the deprecation of a name that isn't a hash.
+-   Read the annotated tags of other tools like git does: with a header after the tagger, or
+    without the empty line before the message, `Tag::getMessage()` and the methods reading the
+    tagger threw a `RuntimeException`.
+
+### Deprecations
+
+-   `Parser\DiffParser` and `Parser\ParserBase`, internal in 2.0: parse a diff with
+    `Diff::parse()`.
+
 ## [v1.7.1] - 2026-11-07
 
 Changes since [v1.7.0](https://github.com/gitonomy/gitlib/releases/tag/v1.7.0).
@@ -165,6 +198,7 @@ _Earlier versions are not documented here. See the
 [GitHub releases](https://github.com/gitonomy/gitlib/releases) and
 [commit history](https://github.com/gitonomy/gitlib/commits/v1.6.0) for prior changes._
 
+[v1.7.2]: https://github.com/gitonomy/gitlib/compare/v1.7.1...v1.7.2
 [v1.7.1]: https://github.com/gitonomy/gitlib/compare/v1.7.0...v1.7.1
 [v1.7.0]: https://github.com/gitonomy/gitlib/compare/v1.6.0...v1.7.0
 [v1.6.0]: https://github.com/gitonomy/gitlib/releases/tag/v1.6.0

composer.json

@@ -43,6 +43,7 @@
     },
     "require-dev": {
         "ext-fileinfo": "*",
+        "ext-zlib": "*",
         "phpspec/prophecy-phpunit": "^2.1",
         "phpunit/phpunit": "^7.5.20 || ^8.5.20 || ^9.5.9",
         "psr/log": "^1.0",

src/Gitonomy/Git/Commit.php

@@ -51,7 +51,7 @@ public function __construct(Repository $repository, $hash, array $data = [])
             trigger_deprecation('gitonomy/gitlib', '1.7', 'Passing data to "%s()" is deprecated, the constructor is internal in 2.0, and takes a "CommitData" instead of an array: get the commit from "%s::getCommit()".', __METHOD__, Repository::class);
         }
 
-        if (!preg_match('/^[a-f0-9]{40}$/', $hash)) {
+        if (!preg_match('/^[a-f0-9]{40}$/D', $hash)) {
             throw new ReferenceNotFoundException($hash);
         }
 

src/Gitonomy/Git/Driver/Cli/CliDriver.php

@@ -29,6 +29,12 @@
  */
 final class CliDriver
 {
+    /**
+     * A path under a file is never a directory, whatever another user of the
+     * machine creates: git finds no repository there.
+     */
+    private const NO_GIT_DIR = __FILE__.'/none';
+
     /**
      * @param string                                  $command              The git binary
      * @param array<string, string|\Stringable|false> $environmentVariables Set on every git process, false removing an inherited one
@@ -193,30 +199,42 @@ public function cloneRepository(#[\SensitiveParameter] string $url, string $path
      */
     public function createProcess(string $command, array $args): Process
     {
-        $process = new Process(array_merge([$this->command, $command], $args));
-        $process->setEnv($this->environmentVariables);
-        $process->setTimeout($this->processTimeout);
-        $process->setIdleTimeout($this->processTimeout);
-
-        return $process;
+        return $this->createGitProcess(array_merge([$command], $args));
     }
 
     /**
      * Like in 2.0, out of the working directory of PHP, which may be a
      * repository, like the checkout of an application: ls-remote would read
      * its configuration, and a relative path as the name of one of its remotes.
      *
+     * --git-dir keeps git from looking for a repository, and reading its
+     * configuration: another user of the machine may create a ".git" in the
+     * temporary directory, whose rewrites of URLs run a command.
+     *
      * @param string[] $options
      * @param string[] $patterns
      */
     private function createLsRemoteProcess(array $options, string $url, array $patterns): Process
     {
-        $process = $this->createProcess('ls-remote', array_merge($options, [self::absoluteUrl($url)], $patterns));
+        $process = $this->createGitProcess(array_merge(['--git-dir', self::NO_GIT_DIR, 'ls-remote'], $options, [self::absoluteUrl($url)], $patterns));
         $process->setWorkingDirectory(sys_get_temp_dir());
 
         return $process;
     }
 
+    /**
+     * @param string[] $args The options of git, its command, and the arguments of the command
+     */
+    private function createGitProcess(array $args): Process
+    {
+        $process = new Process(array_merge([$this->command], $args));
+        $process->setEnv($this->environmentVariables);
+        $process->setTimeout($this->processTimeout);
+        $process->setIdleTimeout($this->processTimeout);
+
+        return $process;
+    }
+
     /**
      * A relative path, relative to the working directory of PHP.
      */

src/Gitonomy/Git/Parser/DiffParser.php

@@ -19,6 +19,8 @@
 
 /**
  * @final since gitonomy/gitlib 1.7
+ *
+ * @deprecated since gitonomy/gitlib 1.7.2, gitlib 2.0 makes it internal: parse a diff with Diff::parse()
  */
 class DiffParser extends ParserBase
 {

src/Gitonomy/Git/Parser/ParserBase.php

@@ -14,6 +14,9 @@
 
 use Gitonomy\Git\Exception\RuntimeException;
 
+/**
+ * @deprecated since gitonomy/gitlib 1.7.2, gitlib 2.0 makes it internal, like the parsers extending it
+ */
 abstract class ParserBase
 {
     protected $cursor;

src/Gitonomy/Git/Parser/TagParser.php

@@ -47,7 +47,14 @@ protected function doParse()
         $this->taggerDate = $this->parseDate($taggerDate);
 
         $this->consumeNewLine();
-        $this->consumeNewLine();
+
+        // Like git, the message starts after the first empty line: the tags
+        // of other tools may have more headers, or no message at all, not
+        // even the empty line.
+        while (!$this->isFinished() && !$this->expects("\n")) {
+            $this->consumeTo("\n");
+            $this->consumeNewLine();
+        }
 
         try {
             $this->message = $this->consumeTo('-----BEGIN PGP SIGNATURE-----');

src/Gitonomy/Git/Repository.php

@@ -488,7 +488,7 @@ public function getCommit($hash)
      */
     public function getTree($hash)
     {
-        if (!preg_match('/^[0-9a-f]{4,40}$/', $hash)) {
+        if (!preg_match('/^[0-9a-f]{4,40}$/D', $hash)) {
             trigger_deprecation('gitonomy/gitlib', '1.7', 'Passing "%s", which is not a hash, to "%s()" is deprecated, it throws in 2.0: use "getRevision($name)->getCommit()->getTree()" instead.', $hash, __METHOD__);
         }
 
@@ -510,7 +510,7 @@ public function getTree($hash)
      */
     public function getBlob($hash)
     {
-        if (!preg_match('/^[0-9a-f]{4,40}$/', $hash)) {
+        if (!preg_match('/^[0-9a-f]{4,40}$/D', $hash)) {
             trigger_deprecation('gitonomy/gitlib', '1.7', 'Passing "%s", which is not a hash, to "%s()" is deprecated, it throws in 2.0: use "Tree::resolvePath()" instead.', $hash, __METHOD__);
         }
 

src/Gitonomy/Git/Util/Credentials.php

@@ -25,15 +25,44 @@
  */
 final class Credentials
 {
+    /**
+     * What ends the "user:password@host" of a URL: a slash or a whitespace.
+     */
+    private const DELIMITERS = "/ \t\n\r\v\f";
+
+    private const LETTERS = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
+
+    private const SCHEME_CHARACTERS = self::LETTERS.'0123456789+.-';
+
     /**
      * Replaces the user information of the URLs of $text, a token or a user
      * and a password, by "***".
+     *
+     * Without a regular expression over $text, which may be the megabytes of
+     * a blob in the logs: it would reach the limits of PCRE, or take seconds.
      */
     public static function redact(#[\SensitiveParameter] string $text): string
     {
-        // Up to the last "@" before the host, as a password may hold one, or a
-        // "#" or a "?", which git accepts unencoded there.
-        return preg_replace('#\b([a-z][a-z0-9+.-]*+://)(?:[^/\s@]*+@)++#i', '$1***@', $text);
+        $redacted = '';
+        $offset = 0;
+        // Where a scheme may start: after the last "@" read.
+        $afterAt = 0;
+        while (false !== $at = strpos($text, '@', $offset)) {
+            // The characters around the "@", from $offset at most: one pass over $text.
+            $start = $at - strcspn(strrev(substr($text, $offset, $at - $offset)), self::DELIMITERS);
+            $end = $at + strcspn($text, self::DELIMITERS, $at);
+            // Up to the last "@" before the host, as a password may hold one,
+            // or a "#" or a "?", which git accepts unencoded there.
+            $host = $end - strcspn(strrev(substr($text, $at, $end - $at)), '@');
+
+            $redacted .= substr($text, $offset, $start - $offset);
+            $redacted .= self::isAfterScheme($text, $afterAt, $start) ? '***@' : substr($text, $start, $host - $start);
+            $redacted .= substr($text, $host, $end - $host);
+            $offset = $end;
+            $afterAt = $host;
+        }
+
+        return $redacted.substr($text, $offset);
     }
 
     /**
@@ -51,4 +80,28 @@ public static function runProcess(Process $process): void
             throw $e;
         }
     }
+
+    /**
+     * @param int $afterAt Where the scheme may start in $text
+     * @param int $start   Where the "user:password@host" starts
+     */
+    private static function isAfterScheme(#[\SensitiveParameter] string $text, int $afterAt, int $start): bool
+    {
+        if ($start - 3 <= $afterAt || '://' !== substr($text, $start - 3, 3)) {
+            return false;
+        }
+
+        $end = $start - 3;
+        $length = strspn(strrev(substr($text, $afterAt, $end - $afterAt)), self::SCHEME_CHARACTERS);
+
+        // A scheme starts with a letter, which starts a word: the one of
+        // "git+ssh" or its "ssh", but not the "https" of "_https".
+        for ($i = $end - $length; $i < $end; ++$i) {
+            if (str_contains(self::LETTERS, $text[$i]) && (0 === $i || !str_contains(self::LETTERS.'0123456789_', $text[$i - 1]))) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }

src/Gitonomy/Git/Util/RemoteUrl.php

@@ -46,10 +46,11 @@ public static function unsafeReason(#[\SensitiveParameter] string $url): ?string
         // Before 2.14.1 (CVE-2017-1000117), git passes the "user@host" of an ssh
         // URL to ssh, without its brackets: ssh reads "ssh://-oProxyCommand=..."
         // as an option, and runs the command. The command of core.gitProxy gets
-        // the host of a git URL the same way. A valid host never starts with a dash.
+        // the host of a git URL the same way, and whatever follows its colon as
+        // the port. A valid host or port never starts with a dash.
         $authority = self::authority($url);
-        if (null !== $authority && 1 === preg_match('/(?:^|@)\[?-/', $authority)) {
-            return sprintf('"%s" has a host or a user starting with a dash, which ssh, or the command of core.gitProxy, would read as an option.', Credentials::redact($url));
+        if (null !== $authority && 1 === preg_match('/(?:^|[@:])\[?-/', $authority)) {
+            return sprintf('"%s" has a host, a user or a port starting with a dash, which ssh, or the command of core.gitProxy, would read as an option.', Credentials::redact($url));
         }
 
         return null;

src/Gitonomy/Git/WorkingCopy.php

@@ -43,14 +43,18 @@ public function __construct(Repository $repository)
     public function getUntrackedFiles()
     {
         $lines = explode("\0", $this->run('status', ['--porcelain', '--untracked-files=all', '-z']));
-        $lines = array_filter($lines, function ($l) {
-            return substr($l, 0, 3) === '?? ';
-        });
-        $lines = array_map(function ($l) {
-            return substr($l, 3);
-        }, $lines);
-
-        return $lines;
+        $files = [];
+        for ($i = 0; $i < count($lines); ++$i) {
+            // "XY <path>": the path a renamed or copied file had before comes
+            // next, alone, and may start with "?? " itself.
+            if (false !== strpbrk(substr($lines[$i], 0, 2), 'CR')) {
+                ++$i;
+            } elseif (substr($lines[$i], 0, 3) === '?? ') {
+                $files[$i] = substr($lines[$i], 3);
+            }
+        }
+
+        return $files;
     }
 
     /**

tests/Gitonomy/Git/Tests/CommitTest.php

@@ -59,6 +59,13 @@ public function testInvalideHashThrowException($repository)
         new Commit($repository, 'that-hash-doest-not-exists');
     }
 
+    public function testAHashEndingWithANewLineIsRefused()
+    {
+        $this->expectException(ReferenceNotFoundException::class);
+
+        self::createFoobarRepository()->getCommit(self::LONGFILE_COMMIT."\n");
+    }
+
     /**
      * @dataProvider provideFoobar
      */

tests/Gitonomy/Git/Tests/ReferenceTest.php

@@ -111,6 +111,38 @@ public function testAnnotatedTag($repository)
         $this->assertEquals('fbde681b329a39e08b63dc54b341a3274c0380c0', $tag->getCommit()->getHash(), 'Tag commit is correct');
     }
 
+    /**
+     * Like git, which reads the tags other tools write: with a header after
+     * the tagger, or without the empty line before the message.
+     *
+     * @dataProvider provideTagsOfOtherTools
+     */
+    public function testATagOfAnotherToolIsReadLikeGit(string $end, string $message)
+    {
+        $repository = self::createFoobarRepository();
+        // A loose object, without the checks recent versions of git apply to it.
+        $object = sprintf("object %s\ntype commit\ntag other\ntagger A <[email protected]> 1700000000 +0000\n%s", self::LONGFILE_COMMIT, $end);
+        $object = 'tag '.strlen($object)."\0".$object;
+        $hash = sha1($object);
+        if (!is_dir($dir = $repository->getGitDir().'/objects/'.substr($hash, 0, 2))) {
+            mkdir($dir);
+        }
+        file_put_contents($dir.'/'.substr($hash, 2), gzcompress($object));
+        $repository->run('update-ref', ['refs/tags/other', $hash]);
+
+        $tag = $repository->getReferences()->getTag('other');
+
+        $this->assertSame('A', $tag->getTaggerName());
+        $this->assertSame($message, $tag->getMessage());
+        $this->assertSame(self::LONGFILE_COMMIT, $tag->getCommit()->getHash());
+    }
+
+    public static function provideTagsOfOtherTools()
+    {
+        yield 'a header after the tagger' => ["encoding ISO-8859-1\n\nMessage\n", "Message\n"];
+        yield 'no message, without the empty line' => ['', ''];
+    }
+
     /**
      * @dataProvider provideFoobar
      */

tests/Gitonomy/Git/Tests/RepositoryTest.php

@@ -405,6 +405,23 @@ public function testIsValidRepositoryReadsARelativePathAsAPath()
         }
     }
 
+    /**
+     * "$" matched before the new line.
+     *
+     * @group legacy
+     */
+    public function testAHashEndingWithANewLineIsNotAHash()
+    {
+        $repository = self::createFoobarRepository();
+        $hash = $repository->getCommit(self::LONGFILE_COMMIT)->getTreeHash();
+
+        $this->expectDeprecation(sprintf('Since gitonomy/gitlib 1.7: Passing "%s", which is not a hash, to "Gitonomy\\Git\\Repository::getTree()" is deprecated, it throws in 2.0: use "getRevision($name)->getCommit()->getTree()" instead.', $hash."\n"));
+        $this->expectDeprecation(sprintf('Since gitonomy/gitlib 1.7: Passing "%s", which is not a hash, to "Gitonomy\\Git\\Repository::getBlob()" is deprecated, it throws in 2.0: use "Tree::resolvePath()" instead.', $hash."\n"));
+
+        $repository->getTree($hash."\n");
+        $repository->getBlob($hash."\n");
+    }
+
     /**
      * @group legacy
      */