1
0
Fork 0

Fix PHPStan issues and a couple minor bugs in audit functionality, refs #10798

pull/10893/head
Jordi Boggiano 2022-06-22 15:39:51 +02:00
parent d93239ddd9
commit 611b215896
No known key found for this signature in database
GPG Key ID: 7BBD42C429EC80BC
10 changed files with 53 additions and 35 deletions

View File

@ -9,7 +9,7 @@ use Composer\Package\PackageInterface;
use Composer\Repository\InstalledRepository; use Composer\Repository\InstalledRepository;
use Composer\Repository\RepositoryInterface; use Composer\Repository\RepositoryInterface;
use Composer\Util\Auditor; use Composer\Util\Auditor;
use Symfony\Component\Console\Input\InputOption; use Composer\Console\Input\InputOption;
class AuditCommand extends BaseCommand class AuditCommand extends BaseCommand
{ {
@ -47,7 +47,7 @@ EOT
} }
$auditor = new Auditor($httpDownloader); $auditor = new Auditor($httpDownloader);
return $auditor->audit($this->getIO(), $packages, $input->getOption('format'), false); return $auditor->audit($this->getIO(), $packages, $this->getAuditFormat($input, 'format'), false);
} }
/** /**

View File

@ -25,6 +25,7 @@ use Composer\IO\NullIO;
use Composer\Plugin\PreCommandRunEvent; use Composer\Plugin\PreCommandRunEvent;
use Composer\Package\Version\VersionParser; use Composer\Package\Version\VersionParser;
use Composer\Plugin\PluginEvents; use Composer\Plugin\PluginEvents;
use Composer\Util\Auditor;
use Composer\Util\Platform; use Composer\Util\Platform;
use Symfony\Component\Console\Completion\CompletionInput; use Symfony\Component\Console\Completion\CompletionInput;
use Symfony\Component\Console\Completion\CompletionSuggestions; use Symfony\Component\Console\Completion\CompletionSuggestions;
@ -405,4 +406,23 @@ abstract class BaseCommand extends Command
return $width; return $width;
} }
/**
* @internal
* @param 'format'|'audit-format' $optName
* @return Auditor::FORMAT_*
*/
protected function getAuditFormat(InputInterface $input, string $optName = 'audit-format'): string
{
if (!$input->hasOption($optName)) {
throw new \LogicException('This should not be called on a Command which has no '.$optName.' option defined.');
}
$val = $input->getOption($optName);
if (!in_array($val, Auditor::FORMATS, true)) {
throw new \InvalidArgumentException('--'.$optName.' must be one of '.implode(', ', Auditor::FORMATS).'.');
}
return $val;
}
} }

View File

@ -264,7 +264,7 @@ EOT
->setClassMapAuthoritative($config->get('classmap-authoritative')) ->setClassMapAuthoritative($config->get('classmap-authoritative'))
->setApcuAutoloader($config->get('apcu-autoloader')) ->setApcuAutoloader($config->get('apcu-autoloader'))
->setAudit(!$input->getOption('no-audit')) ->setAudit(!$input->getOption('no-audit'))
->setAuditFormat($input->getOption('audit-format')); ->setAuditFormat($this->getAuditFormat($input));
if (!$composer->getLocker()->isLocked()) { if (!$composer->getLocker()->isLocked()) {
$installer->setUpdate(true); $installer->setUpdate(true);

View File

@ -134,7 +134,7 @@ EOT
->setApcuAutoloader($apcu, $apcuPrefix) ->setApcuAutoloader($apcu, $apcuPrefix)
->setPlatformRequirementFilter($this->getPlatformRequirementFilter($input)) ->setPlatformRequirementFilter($this->getPlatformRequirementFilter($input))
->setAudit(!$input->getOption('no-audit')) ->setAudit(!$input->getOption('no-audit'))
->setAuditFormat($input->getOption('audit-format')) ->setAuditFormat($this->getAuditFormat($input))
; ;
if ($input->getOption('no-plugins')) { if ($input->getOption('no-plugins')) {

View File

@ -286,7 +286,7 @@ EOT
->setPlatformRequirementFilter($this->getPlatformRequirementFilter($input)) ->setPlatformRequirementFilter($this->getPlatformRequirementFilter($input))
->setDryRun($dryRun) ->setDryRun($dryRun)
->setAudit(!$input->getOption('no-audit')) ->setAudit(!$input->getOption('no-audit'))
->setAuditFormat($input->getOption('audit-format')) ->setAuditFormat($this->getAuditFormat($input))
; ;
// if no lock is present, we do not do a partial update as // if no lock is present, we do not do a partial update as

View File

@ -419,7 +419,7 @@ EOT
->setPreferStable($input->getOption('prefer-stable')) ->setPreferStable($input->getOption('prefer-stable'))
->setPreferLowest($input->getOption('prefer-lowest')) ->setPreferLowest($input->getOption('prefer-lowest'))
->setAudit(!$input->getOption('no-audit')) ->setAudit(!$input->getOption('no-audit'))
->setAuditFormat($input->getOption('audit-format')) ->setAuditFormat($this->getAuditFormat($input))
; ;
// if no lock is present, or the file is brand new, we do not do a // if no lock is present, or the file is brand new, we do not do a
@ -475,7 +475,7 @@ EOT
protected function interact(InputInterface $input, OutputInterface $output): void protected function interact(InputInterface $input, OutputInterface $output): void
{ {
} }
/** /**

View File

@ -233,7 +233,7 @@ EOT
->setPreferLowest($input->getOption('prefer-lowest')) ->setPreferLowest($input->getOption('prefer-lowest'))
->setTemporaryConstraints($temporaryConstraints) ->setTemporaryConstraints($temporaryConstraints)
->setAudit(!$input->getOption('no-audit')) ->setAudit(!$input->getOption('no-audit'))
->setAuditFormat($input->getOption('audit-format')) ->setAuditFormat($this->getAuditFormat($input))
; ;
if ($input->getOption('no-plugins')) { if ($input->getOption('no-plugins')) {

View File

@ -167,8 +167,8 @@ class Installer
protected $executeOperations = true; protected $executeOperations = true;
/** @var bool */ /** @var bool */
protected $audit = true; protected $audit = true;
/** @var string */ /** @var Auditor::FORMAT_* */
protected $auditFormat = Auditor::FORMAT_TABLE; protected $auditFormat = Auditor::FORMAT_SUMMARY;
/** @var bool */ /** @var bool */
protected $updateMirrors = false; protected $updateMirrors = false;
@ -1460,7 +1460,7 @@ class Installer
/** /**
* What format should be used for audit output? * What format should be used for audit output?
* *
* @param string $auditFormat * @param Auditor::FORMAT_* $auditFormat
* @return Installer * @return Installer
*/ */
public function setAuditFormat(string $auditFormat): self public function setAuditFormat(string $auditFormat): self

View File

@ -53,7 +53,8 @@ class Auditor
if (count($advisories) > 0) { if (count($advisories) > 0) {
$numAdvisories = $this->countAdvisories($advisories); $numAdvisories = $this->countAdvisories($advisories);
$plurality = $numAdvisories === 1 ? 'y' : 'ies'; $plurality = $numAdvisories === 1 ? 'y' : 'ies';
$io->writeError("<$errorOrWarn>Found $numAdvisories security vulnerability advisor$plurality:</$errorOrWarn>"); $punctuation = $format === 'summary' ? '.' : ':';
$io->writeError("<$errorOrWarn>Found $numAdvisories security vulnerability advisor{$plurality}{$punctuation}</$errorOrWarn>");
$this->outputAdvisories($io, $advisories, $format); $this->outputAdvisories($io, $advisories, $format);
return 1; return 1;
} }
@ -177,8 +178,9 @@ class Auditor
case self::FORMAT_SUMMARY: case self::FORMAT_SUMMARY:
// We've already output the number of advisories in audit() // We've already output the number of advisories in audit()
$io->writeError('Run composer audit for a full list of advisories.'); $io->writeError('Run composer audit for a full list of advisories.');
return;
default: default:
throw new InvalidArgumentException('Invalid format.'); throw new InvalidArgumentException('Invalid format "'.$format.'".');
} }
} }
@ -203,7 +205,7 @@ class Auditor
]) ])
->addRow([ ->addRow([
$package, $package,
$advisory['cve'] ?: 'NO CVE', $advisory['cve'] ?? 'NO CVE',
$advisory['title'], $advisory['title'],
$advisory['link'], $advisory['link'],
$advisory['affectedVersions'], $advisory['affectedVersions'],
@ -230,7 +232,7 @@ class Auditor
if (!$firstAdvisory) { if (!$firstAdvisory) {
$error[] = '--------'; $error[] = '--------';
} }
$cve = $advisory['cve'] ?: 'NO CVE'; $cve = $advisory['cve'] ?? 'NO CVE';
$error[] = "Package: $package"; $error[] = "Package: $package";
$error[] = "CVE: $cve"; $error[] = "CVE: $cve";
$error[] = "Title: {$advisory['title']}"; $error[] = "Title: {$advisory['title']}";

View File

@ -3,6 +3,8 @@
namespace Composer\Test\Util; namespace Composer\Test\Util;
use Composer\IO\IOInterface; use Composer\IO\IOInterface;
use Composer\IO\NullIO;
use Composer\Json\JsonFile;
use Composer\Package\Package; use Composer\Package\Package;
use Composer\Test\TestCase; use Composer\Test\TestCase;
use Composer\Util\Auditor; use Composer\Util\Auditor;
@ -13,17 +15,6 @@ use PHPUnit\Framework\MockObject\MockObject;
class AuditorTest extends TestCase class AuditorTest extends TestCase
{ {
/** @var IOInterface&\PHPUnit\Framework\MockObject\MockObject */
private $io;
protected function setUp(): void
{
$this->io = $this
->getMockBuilder(IOInterface::class)
->disableOriginalConstructor()
->getMock();
}
public function auditProvider() public function auditProvider()
{ {
return [ return [
@ -75,11 +66,14 @@ class AuditorTest extends TestCase
$this->expectException(InvalidArgumentException::class); $this->expectException(InvalidArgumentException::class);
} }
$auditor = new Auditor($this->getHttpDownloader()); $auditor = new Auditor($this->getHttpDownloader());
$result = $auditor->audit($this->io, $data['packages'], Auditor::FORMAT_PLAIN, $data['warningOnly']); $result = $auditor->audit(new NullIO(), $data['packages'], Auditor::FORMAT_PLAIN, $data['warningOnly']);
$this->assertSame($expected, $result, $message); $this->assertSame($expected, $result, $message);
} }
public function advisoriesProvider() /**
* @return mixed[]
*/
public function advisoriesProvider(): array
{ {
$advisories = static::getMockAdvisories(null); $advisories = static::getMockAdvisories(null);
return [ return [
@ -187,7 +181,7 @@ class AuditorTest extends TestCase
if (count($data['packages']) === 0 && $data['updatedSince'] === null) { if (count($data['packages']) === 0 && $data['updatedSince'] === null) {
$this->expectException(InvalidArgumentException::class); $this->expectException(InvalidArgumentException::class);
} }
$auditor = new Auditor($this->getHttpDownloader(), Auditor::FORMAT_PLAIN); $auditor = new Auditor($this->getHttpDownloader());
$result = $auditor->getAdvisories($data['packages'], $data['updatedSince'], $data['filterByVersion']); $result = $auditor->getAdvisories($data['packages'], $data['updatedSince'], $data['filterByVersion']);
$this->assertSame($expected, $result, $message); $this->assertSame($expected, $result, $message);
} }
@ -204,7 +198,7 @@ class AuditorTest extends TestCase
->getMock(); ->getMock();
$callback = function(string $url, array $options) { $callback = function(string $url, array $options) {
parse_str(parse_url($url, PHP_URL_QUERY) ?? '', $query); parse_str((string) parse_url($url, PHP_URL_QUERY), $query);
$updatedSince = null; $updatedSince = null;
if (isset($query['updatedSince'])) { if (isset($query['updatedSince'])) {
$updatedSince = $query['updatedSince']; $updatedSince = $query['updatedSince'];
@ -217,13 +211,13 @@ class AuditorTest extends TestCase
parse_str($options['http']['content'], $body); parse_str($options['http']['content'], $body);
$packages = $body['packages']; $packages = $body['packages'];
foreach ($advisories as $package => $data) { foreach ($advisories as $package => $data) {
if (!in_array($package, $packages)) { if (!in_array($package, $packages, true)) {
unset($advisories[$package]); unset($advisories[$package]);
} }
} }
} }
return new Response(['url' => 'https://packagist.org/api/security-advisories/'], 200, [], json_encode(['advisories' => $advisories])); return new Response(['url' => 'https://packagist.org/api/security-advisories/'], 200, [], JsonFile::encode(['advisories' => $advisories]));
}; };
$httpDownloader $httpDownloader
@ -233,7 +227,10 @@ class AuditorTest extends TestCase
return $httpDownloader; return $httpDownloader;
} }
public static function getMockAdvisories(?int $updatedSince) /**
* @return array<mixed>
*/
public static function getMockAdvisories(?int $updatedSince): array
{ {
$advisories = [ $advisories = [
'vendor1/package1' => [ 'vendor1/package1' => [
@ -326,8 +323,7 @@ class AuditorTest extends TestCase
], ],
]; ];
// Intentionally allow updatedSince === 0 to include these advisories if (0 === $updatedSince || null === $updatedSince) {
if (!$updatedSince) {
$advisories['vendor1/package1'][] = [ $advisories['vendor1/package1'][] = [
'advisoryId' => 'ID5', 'advisoryId' => 'ID5',
'packageName' => 'vendor1/package1', 'packageName' => 'vendor1/package1',