Fix bugs and hardening issues found by the code audit (#239)

- ReferenceBag::getFirstBranch() mixed up two different arrays
  (reset($this->branches) then current($this->references)) and could
  return a non-branch reference, or even throw, when no branch exists.
  It now correctly returns the first branch or false.

- Tag::getBodyMessage() used array_shift()+array_pop() instead of the
  two array_shift() used by Commit::getBodyMessage(), leaving a stray
  leading newline (or truncating the last line) for annotated tags
  using the standard "subject\n\nbody" convention. The body extraction
  now handles both the blank-line-separated and non-separated forms.

- Repository::run() can return null instead of throwing when
  debug=false and the git command fails, but several call sites fed
  that null straight into trim()/explode()/Diff::parse() or a
  non-nullable return type. Depending on the call site this silently
  produced wrong data (Tag::isAnnotated() reporting true for a
  lightweight tag, PushReference::isForce() defaulting to a coin
  flip) or crashed with a TypeError (Blob::getContent()). All call
  sites in Commit, Tag, Blob, Reference, PushReference, Repository and
  WorkingCopy now check for null and fail predictably.

- Repository::shell() interpolated the repository path into the shell
  command without escaping it (only the env vars were escaped);
  it is now passed through escapeshellarg().

- Hooks::set() created hook scripts as 0777 (world-writable); they are
  now created 0700, readable/writable/executable by the owner only.

- Replaced the untyped array<string, mixed> "data bag" backing Commit
  and Reference\Tag with real typed properties, validated at the one
  point external data enters (setData()/the parser). This removes the
  Commit.php and Reference\Tag.php entries from the PHPStan baseline
  (89 -> 51 ignored errors) without resorting to assert()/@var casts.

- Minor: fixed the isSuccessFul() casing typo in Admin.php, and
  dropped composer.json's leftover minimum-stability: dev (dependency
  resolution is unaffected, verified with composer update --dry-run).

Added regression tests for the two bugs, the debug=false fixes, and
the new hook permissions.

Author: lyrixx

composer.json

@@ -49,6 +49,5 @@
         "preferred-install": "dist",
         "sort-packages": true
     },
-    "minimum-stability": "dev",
     "prefer-stable": true
 }

phpstan-baseline.neon

@@ -78,162 +78,6 @@ parameters:
 			count: 1
 			path: src/Gitonomy/Git/Blame/Line.php
 
-		-
-			message: '#^Method Gitonomy\\Git\\Blob\:\:getContent\(\) should return string but returns string\|null\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Blob.php
-
-		-
-			message: '#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\.$#'
-			identifier: foreach.nonIterable
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:__construct\(\) has parameter \$data with no value type specified in iterable type array\.$#'
-			identifier: missingType.iterableValue
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getAuthorDate\(\) should return DateTime but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getAuthorEmail\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getAuthorName\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getBodyMessage\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getCommitterDate\(\) should return DateTime but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getCommitterEmail\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getCommitterName\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getMessage\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getParentHashes\(\) should return array\<string\> but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getShortHash\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getShortMessage\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getSubjectMessage\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getTree\(\) should return Gitonomy\\Git\\Tree but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:getTreeHash\(\) should return string but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Commit\:\:setData\(\) has parameter \$data with no value type specified in iterable type array\.$#'
-			identifier: missingType.iterableValue
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$hash of method Gitonomy\\Git\\Repository\:\:getCommit\(\) expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$hash of method Gitonomy\\Git\\Repository\:\:getTree\(\) expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$haystack of static method Gitonomy\\Git\\Util\\StringHelper\:\:strpos\(\) expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$rawDiff of static method Gitonomy\\Git\\Diff\\Diff\:\:parse\(\) expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$string of function trim expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 2
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$string of static method Gitonomy\\Git\\Util\\StringHelper\:\:strlen\(\) expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#1 \$string of static method Gitonomy\\Git\\Util\\StringHelper\:\:substr\(\) expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Commit.php
-
-		-
-			message: '#^Parameter \#2 \$string of function explode expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 2
-			path: src/Gitonomy/Git/Commit.php
-
 		-
 			message: '#^Method Gitonomy\\Git\\Diff\\Diff\:\:fromArray\(\) has parameter \$array with no value type specified in iterable type array\.$#'
 			identifier: missingType.iterableValue
@@ -336,72 +180,12 @@ parameters:
 			count: 1
 			path: src/Gitonomy/Git/Parser/ParserBase.php
 
-		-
-			message: '#^Parameter \#1 \$string of function trim expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/PushReference.php
-
-		-
-			message: '#^Parameter \#1 \$string of function trim expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Reference.php
-
 		-
 			message: '#^Method Gitonomy\\Git\\Reference\\Branch\:\:isLocal\(\) should return bool but returns bool\|null\.$#'
 			identifier: return.type
 			count: 1
 			path: src/Gitonomy/Git/Reference/Branch.php
 
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getBodyMessage\(\) should return string\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getGPGSignature\(\) should return string\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getMessage\(\) should return string\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getSubjectMessage\(\) should return string\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getTaggerDate\(\) should return DateTime\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getTaggerEmail\(\) should return string\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Method Gitonomy\\Git\\Reference\\Tag\:\:getTaggerName\(\) should return string\|false but returns mixed\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Gitonomy/Git/Reference/Tag.php
-
-		-
-			message: '#^Parameter \#2 \$string of function explode expects string, mixed given\.$#'
-			identifier: argument.type
-			count: 2
-			path: src/Gitonomy/Git/Reference/Tag.php
-
 		-
 			message: '#^Class Gitonomy\\Git\\ReferenceBag implements generic interface IteratorAggregate but does not specify its types\: TKey, TValue$#'
 			identifier: missingType.generics
@@ -468,12 +252,6 @@ parameters:
 			count: 1
 			path: src/Gitonomy/Git/Repository.php
 
-		-
-			message: '#^Parameter \#1 \$rawDiff of static method Gitonomy\\Git\\Diff\\Diff\:\:parse\(\) expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/Repository.php
-
 		-
 			message: '#^Parameter \#1 \$string of function trim expects string, string\|false given\.$#'
 			identifier: argument.type
@@ -521,15 +299,3 @@ parameters:
 			identifier: missingType.iterableValue
 			count: 1
 			path: src/Gitonomy/Git/WorkingCopy.php
-
-		-
-			message: '#^Parameter \#1 \$rawDiff of static method Gitonomy\\Git\\Diff\\Diff\:\:parse\(\) expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 2
-			path: src/Gitonomy/Git/WorkingCopy.php
-
-		-
-			message: '#^Parameter \#2 \$string of function explode expects string, string\|null given\.$#'
-			identifier: argument.type
-			count: 1
-			path: src/Gitonomy/Git/WorkingCopy.php

src/Gitonomy/Git/Admin.php

@@ -37,7 +37,7 @@ public static function init(string $path, bool $bare = true, array $options = []
 
         $process->run();
 
-        if (!$process->isSuccessFul()) {
+        if (!$process->isSuccessful()) {
             throw new RuntimeException(\sprintf("Error on repository initialization, command wasn't successful (%s). Error output:\n%s", $process->getCommandLine(), $process->getErrorOutput()));
         }
 
@@ -62,7 +62,7 @@ public static function isValidRepository(string $url, array $options = []): bool
 
         $process->run();
 
-        return $process->isSuccessFul();
+        return $process->isSuccessful();
     }
 
     /**
@@ -86,7 +86,7 @@ public static function isValidRepositoryAndBranch(string $url, string $branchNam
         $process->run();
         $processOutput = $process->getOutput();
 
-        return $process->isSuccessFul() && str_contains($processOutput, $branchName);
+        return $process->isSuccessful() && str_contains($processOutput, $branchName);
     }
 
     /**
@@ -149,7 +149,7 @@ public static function cloneRepository(string $path, string $url, array $args =
 
         $process->run();
 
-        if (!$process->isSuccessFul()) {
+        if (!$process->isSuccessful()) {
             throw new RuntimeException(\sprintf('Error while initializing repository: %s', $process->getErrorOutput()));
         }
 

src/Gitonomy/Git/Blob.php

@@ -12,6 +12,8 @@
 
 namespace Gitonomy\Git;
 
+use Gitonomy\Git\Exception\ReferenceNotFoundException;
+
 /**
  * Representation of a Blob commit.
  *
@@ -53,7 +55,12 @@ public function getHash(): string
     public function getContent(): string
     {
         if (null === $this->content) {
-            $this->content = $this->repository->run('cat-file', ['-p', $this->hash]);
+            $content = $this->repository->run('cat-file', ['-p', $this->hash]);
+            if (null === $content) {
+                throw new ReferenceNotFoundException($this->hash);
+            }
+
+            $this->content = $content;
         }
 
         return $this->content;