public-mirrors
/
composer
mirror of https://github.com/composer/composer
1
0
Fork 0
Code

Change audit.ignore behavior before 2.6.0 (#11605) 

* Still report ignored security advisories

Co-authored-by: Jordi Boggiano <j.boggiano@seld.be>
pull/11616/head
Dezső BICZÓ 2023-09-01 10:04:31 +02:00 committed by GitHub
parent b6fe941911
commit 0ab4dfba7c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 359 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
doc/06-config.md
View File

@ -105,15 +105,31 @@ optionally be an object with package name patterns for keys for more granular in
Security audit configuration options
### ignored
### ignore
A set of advisory ids, remote ids or CVE ids that should be ignored and not reported as part of an audit.
A list of advisory ids, remote ids or CVE ids that are reported but let the audit command pass.
```json
{
"config": {
"audit": {
"ignored": ["CVE-1234", "GHSA-xx", "PKSA-yy"]
"ignore": {
"CVE-1234": "The affected component is not in use.",
"GHSA-xx": "The security fix was applied as a patch.",
"PKSA-yy": "Due to mitigations in place the update can be delayed."
}
}
}
}
```
or
```json
{
"config": {
"audit": {
"ignore": ["CVE-1234", "GHSA-xx", "PKSA-yy"]
}
}
}

9
phpstan/baseline.neon
View File

@ -5,6 +5,11 @@ parameters:
count: 1
path: ../src/Composer/Advisory/Auditor.php
-
message: "#^Variable \\$affectedPackagesCount might not be defined\\.$#"
count: 1
path: ../src/Composer/Advisory/Auditor.php
-
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
count: 10
@ -3575,7 +3580,7 @@ parameters:
-
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
count: 6
count: 5
path: ../src/Composer/Repository/Vcs/GitHubDriver.php
-
@ -3610,7 +3615,7 @@ parameters:
-
message: "#^Only booleans are allowed in a negated boolean, string given\\.$#"
count: 2
count: 1
path: ../src/Composer/Repository/Vcs/GitHubDriver.php
-

15
res/composer-schema.json
View File

@ -329,13 +329,24 @@
"type": "object",
"description": "Security audit configuration options",
"properties": {
"ignored": {
"ignore": {
"anyOf": [
{
"type": "object",
"description": "A list of advisory ids, remote ids or CVE ids (keys) and the explanations (values) for why they're being ignored. The listed items are reported but let the audit command pass.",
"additionalProperties": {
"type": ["string", "string"]
}
},
{
"type": "array",
"description": "A set of advisory ids, remote ids or CVE ids that should be ignored and not reported as part of an audit.",
"description": "A set of advisory ids, remote ids or CVE ids that are reported but let the audit command pass.",
"items": {
"type": "string"
}
}
]
}
}
},
"notify-on-install": {

132
src/Composer/Advisory/Auditor.php
View File

@ -44,34 +44,55 @@ class Auditor
* @param PackageInterface[] $packages
* @param self::FORMAT_* $format The format that will be used to output audit results.
* @param bool $warningOnly If true, outputs a warning. If false, outputs an error.
* @param string[] $ignoredIds Ignored advisory IDs, remote IDs or CVE IDs
* @param string[] $ignoreList List of advisory IDs, remote IDs or CVE IDs that reported but not listed as vulnerabilities.
*
* @return int Amount of packages with vulnerabilities found
* @throws InvalidArgumentException If no packages are passed in
*/
public function audit(IOInterface $io, RepositorySet $repoSet, array $packages, string $format, bool $warningOnly = true, array $ignoredIds = []): int
public function audit(IOInterface $io, RepositorySet $repoSet, array $packages, string $format, bool $warningOnly = true, array $ignoreList = []): int
{
$advisories = $repoSet->getMatchingSecurityAdvisories($packages, $format === self::FORMAT_SUMMARY);
if (\count($ignoredIds) > 0) {
$advisories = $this->filterIgnoredAdvisories($advisories, $ignoredIds);
$allAdvisories = $repoSet->getMatchingSecurityAdvisories($packages, $format === self::FORMAT_SUMMARY);
// we need the CVE & remote IDs set to filter ignores correctly so if we have any matches using the optimized codepath above
// and ignores are set then we need to query again the full data to make sure it can be filtered
if (count($allAdvisories) > 0 && $ignoreList !== [] && $format === self::FORMAT_SUMMARY) {
$allAdvisories = $repoSet->getMatchingSecurityAdvisories($packages, false);
}
['advisories' => $advisories, 'ignoredAdvisories' => $ignoredAdvisories] = $this->processAdvisories($allAdvisories, $ignoreList);
if (self::FORMAT_JSON === $format) {
$io->write(JsonFile::encode(['advisories' => $advisories]));
$json = ['advisories' => $advisories];
if ($ignoredAdvisories !== []) {
$json['ignored-advisories'] = $ignoredAdvisories;
}
$io->write(JsonFile::encode($json));
return count($advisories);
}
$errorOrWarn = $warningOnly ? 'warning' : 'error';
if (count($advisories) > 0) {
[$affectedPackages, $totalAdvisories] = $this->countAdvisories($advisories);
$plurality = $totalAdvisories === 1 ? 'y' : 'ies';
$pkgPlurality = $affectedPackages === 1 ? '' : 's';
if (count($advisories) > 0 || count($ignoredAdvisories) > 0) {
$passes = [
[$ignoredAdvisories, "<info>Found %d ignored security vulnerability advisor%s affecting %d package%s%s</info>"],
// this has to run last to allow $affectedPackagesCount in the return statement to be correct
[$advisories, "<$errorOrWarn>Found %d security vulnerability advisor%s affecting %d package%s%s</$errorOrWarn>"],
];
foreach ($passes as [$advisoriesToOutput, $message]) {
[$affectedPackagesCount, $totalAdvisoryCount] = $this->countAdvisories($advisoriesToOutput);
if ($affectedPackagesCount > 0) {
$plurality = $totalAdvisoryCount === 1 ? 'y' : 'ies';
$pkgPlurality = $affectedPackagesCount === 1 ? '' : 's';
$punctuation = $format === 'summary' ? '.' : ':';
$io->writeError("<$errorOrWarn>Found $totalAdvisories security vulnerability advisor{$plurality} affecting $affectedPackages package{$pkgPlurality}{$punctuation}</$errorOrWarn>");
$this->outputAdvisories($io, $advisories, $format);
$io->writeError(sprintf($message, $totalAdvisoryCount, $plurality, $affectedPackagesCount, $pkgPlurality, $punctuation));
$this->outputAdvisories($io, $advisoriesToOutput, $format);
}
}
return $affectedPackages;
if ($format === self::FORMAT_SUMMARY) {
$io->writeError('Run "composer audit" for a full list of advisories.');
}
return $affectedPackagesCount;
}
$io->writeError('<info>No security vulnerability advisories found</info>');
@ -80,37 +101,66 @@ class Auditor
}
/**
* @phpstan-param array<string, array<PartialSecurityAdvisory|SecurityAdvisory>> $advisories
* @param array<string> $ignoredIds
* @phpstan-return array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>
* @phpstan-param array<string, array<PartialSecurityAdvisory|SecurityAdvisory>> $allAdvisories
* @param array<string>|array<string,string> $ignoreList List of advisory IDs, remote IDs or CVE IDs that reported but not listed as vulnerabilities.
* @phpstan-return array{advisories: array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>, ignoredAdvisories: array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>}
*/
private function filterIgnoredAdvisories(array $advisories, array $ignoredIds): array
private function processAdvisories(array $allAdvisories, array $ignoreList): array
{
foreach ($advisories as $package => $pkgAdvisories) {
$advisories[$package] = array_filter($pkgAdvisories, static function (PartialSecurityAdvisory $advisory) use ($ignoredIds) {
if (in_array($advisory->advisoryId, $ignoredIds, true)) {
return false;
if ($ignoreList === []) {
return ['advisories' => $allAdvisories, 'ignoredAdvisories' => []];
}
if (\count($ignoreList) > 0 && !\array_is_list($ignoreList)) {
$ignoredIds = array_keys($ignoreList);
} else {
$ignoredIds = $ignoreList;
}
$advisories = [];
$ignored = [];
$ignoreReason = null;
foreach ($allAdvisories as $package => $pkgAdvisories) {
foreach ($pkgAdvisories as $advisory) {
$isActive = true;
if (in_array($advisory->advisoryId, $ignoredIds, true)) {
$isActive = false;
$ignoreReason = $ignoreList[$advisory->advisoryId] ?? null;
}
if ($advisory instanceof SecurityAdvisory) {
if (in_array($advisory->cve, $ignoredIds, true)) {
return false;
$isActive = false;
$ignoreReason = $ignoreList[$advisory->cve] ?? null;
}
foreach ($advisory->sources as $source) {
if (in_array($source['remoteId'], $ignoredIds, true)) {
return false;
$isActive = false;
$ignoreReason = $ignoreList[$source['remoteId']] ?? null;
break;
}
}
}
return true;
});
if (\count($advisories[$package]) === 0) {
unset($advisories[$package]);
if ($isActive) {
$advisories[$package][] = $advisory;
continue;
}
// Partial security advisories only used in summary mode
// and in that case we do not need to cast the object.
if ($advisory instanceof SecurityAdvisory) {
$advisory = $advisory->toIgnoredAdvisory($ignoreReason);
}
$ignored[$package][] = $advisory;
}
}
return $advisories;
return ['advisories' => $advisories, 'ignoredAdvisories' => $ignored];
}
/**
@ -146,8 +196,6 @@ class Auditor
return;
case self::FORMAT_SUMMARY:
// We've already output the number of advisories in audit()
$io->writeError('Run composer audit for a full list of advisories.');
return;
default:
@ -162,24 +210,30 @@ class Auditor
{
foreach ($advisories as $packageAdvisories) {
foreach ($packageAdvisories as $advisory) {
$io->getTable()
->setHorizontal()
->setHeaders([
$headers = [
'Package',
'CVE',
'Title',
'URL',
'Affected versions',
'Reported at',
])
->addRow([
];
$row = [
$advisory->packageName,
$this->getCVE($advisory),
$advisory->title,
$this->getURL($advisory),
$advisory->affectedVersions->getPrettyString(),
$advisory->reportedAt->format(DATE_ATOM),
])
];
if ($advisory instanceof IgnoredSecurityAdvisory) {
$headers[] = 'Ignore reason';
$row[] = $advisory->ignoreReason ?? 'None specified';
}
$io->getTable()
->setHorizontal()
->setHeaders($headers)
->addRow($row)
->setColumnWidth(1, 80)
->setColumnMaxWidth(1, 80)
->render();
@ -205,6 +259,9 @@ class Auditor
$error[] = "URL: ".$this->getURL($advisory);
$error[] = "Affected versions: ".OutputFormatter::escape($advisory->affectedVersions->getPrettyString());
$error[] = "Reported at: ".$advisory->reportedAt->format(DATE_ATOM);
if ($advisory instanceof IgnoredSecurityAdvisory) {
$error[] = "Ignore reason: ".($advisory->ignoreReason ?? 'None specified');
}
$firstAdvisory = false;
}
}
@ -228,4 +285,5 @@ class Auditor
return '<href='.OutputFormatter::escape($advisory->link).'>'.OutputFormatter::escape($advisory->link).'</>';
}
}

50
src/Composer/Advisory/IgnoredSecurityAdvisory.php Normal file
View File

@ -0,0 +1,50 @@
<?php declare(strict_types=1);
/*
* This file is part of Composer.
*
* (c) Nils Adermann <naderman@naderman.de>
* Jordi Boggiano <j.boggiano@seld.be>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Composer\Advisory;
use Composer\Semver\Constraint\ConstraintInterface;
use DateTimeImmutable;
class IgnoredSecurityAdvisory extends SecurityAdvisory
{
/**
* @var string|null
* @readonly
*/
public $ignoreReason;
/**
* @param non-empty-array<array{name: string, remoteId: string}> $sources
*/
public function __construct(string $packageName, string $advisoryId, ConstraintInterface $affectedVersions, string $title, array $sources, DateTimeImmutable $reportedAt, ?string $cve = null, ?string $link = null, ?string $ignoreReason = null)
{
parent::__construct($packageName, $advisoryId, $affectedVersions, $title, $sources, $reportedAt, $cve, $link);
$this->ignoreReason = $ignoreReason;
}
/**
* @return mixed
*/
#[\ReturnTypeWillChange]
public function jsonSerialize()
{
$data = parent::jsonSerialize();
if ($this->ignoreReason === NULL) {
unset($data['ignoreReason']);
}
return $data;
}
}

21
src/Composer/Advisory/SecurityAdvisory.php
View File

@ -42,14 +42,13 @@ class SecurityAdvisory extends PartialSecurityAdvisory
public $reportedAt;
/**
* @var array<array{name: string, remoteId: string}>
* @var non-empty-array<array{name: string, remoteId: string}>
* @readonly
*/
public $sources;
/**
* @param non-empty-array<array{name: string, remoteId: string}> $sources
* @readonly
*/
public function __construct(string $packageName, string $advisoryId, ConstraintInterface $affectedVersions, string $title, array $sources, DateTimeImmutable $reportedAt, ?string $cve = null, ?string $link = null)
{
@ -62,6 +61,24 @@ class SecurityAdvisory extends PartialSecurityAdvisory
$this->link = $link;
}
/**
* @internal
*/
public function toIgnoredAdvisory(?string $ignoreReason): IgnoredSecurityAdvisory
{
return new IgnoredSecurityAdvisory(
$this->packageName,
$this->advisoryId,
$this->affectedVersions,
$this->title,
$this->sources,
$this->reportedAt,
$this->cve,
$this->link,
$ignoreReason
);
}
/**
* @return mixed
*/

2
src/Composer/Command/AuditCommand.php
View File

@ -63,7 +63,7 @@ EOT
$repoSet->addRepository($repo);
}
return min(255, $auditor->audit($this->getIO(), $repoSet, $packages, $this->getAuditFormat($input, 'format'), false, $composer->getConfig()->get('audit')['ignored'] ?? []));
return min(255, $auditor->audit($this->getIO(), $repoSet, $packages, $this->getAuditFormat($input, 'format'), false, $composer->getConfig()->get('audit')['ignore'] ?? []));
}
/**

6
src/Composer/Config.php
View File

@ -37,7 +37,7 @@ class Config
'allow-plugins' => [],
'use-parent-dir' => 'prompt',
'preferred-install' => 'dist',
'audit' => ['ignored' => []],
'audit' => ['ignore' => []],
'notify-on-install' => true,
'github-protocols' => ['https', 'ssh', 'git'],
'gitlab-protocol' => null,
@ -209,10 +209,10 @@ class Config
$this->setSourceOfConfigValue($val, $key, $source);
}
} elseif ('audit' === $key) {
$currentIgnores = $this->config['audit']['ignored'];
$currentIgnores = $this->config['audit']['ignore'];
$this->config[$key] = $val;
$this->setSourceOfConfigValue($val, $key, $source);
$this->config['audit']['ignored'] = array_merge($currentIgnores, $val['ignored'] ?? []);
$this->config['audit']['ignore'] = array_merge($currentIgnores, $val['ignore'] ?? []);
} else {
$this->config[$key] = $val;
$this->setSourceOfConfigValue($val, $key, $source);

2
src/Composer/Installer.php
View File

@ -402,7 +402,7 @@ class Installer
$repoSet->addRepository($repo);
}
return $auditor->audit($this->io, $repoSet, $packages, $this->auditFormat, true, $this->config->get('audit')['ignored'] ?? []) > 0 ? self::ERROR_AUDIT_FAILED : 0;
return $auditor->audit($this->io, $repoSet, $packages, $this->auditFormat, true, $this->config->get('audit')['ignore'] ?? []) > 0 ? self::ERROR_AUDIT_FAILED : 0;
} catch (TransportException $e) {
$this->io->error('Failed to audit '.$target.' packages.');
if ($this->io->isVerbose()) {

149
tests/Composer/Test/Advisory/AuditorTest.php
View File

@ -14,7 +14,6 @@ namespace Composer\Test\Advisory;
use Composer\Advisory\PartialSecurityAdvisory;
use Composer\Advisory\SecurityAdvisory;
use Composer\IO\BufferIO;
use Composer\IO\NullIO;
use Composer\Package\Package;
use Composer\Package\Version\VersionParser;
@ -72,20 +71,123 @@ class AuditorTest extends TestCase
$this->assertSame($expected, $result, $message);
}
public function testAuditIgnoredIDs(): void
{
$packages = [
public function ignoredIdsProvider(): \Generator {
yield 'ignore by CVE' => [
[
new Package('vendor1/package1', '3.0.0.0', '3.0.0'),
new Package('vendor1/package2', '3.0.0.0', '3.0.0'),
new Package('vendorx/packagex', '3.0.0.0', '3.0.0'),
new Package('vendor3/package1', '3.0.0.0', '3.0.0'),
],
['CVE1'],
0,
[
['text' => 'Found 1 ignored security vulnerability advisory affecting 1 package:'],
['text' => 'Package: vendor1/package1'],
['text' => 'CVE: CVE1'],
['text' => 'Title: advisory1'],
['text' => 'URL: https://advisory.example.com/advisory1'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2022-05-25T13:21:00+00:00'],
]
];
$ignoredIds = ['CVE1', 'ID2', 'RemoteIDx'];
$auditor = new Auditor();
$result = $auditor->audit($io = $this->getIOMock(), $this->getRepoSet(), $packages, Auditor::FORMAT_PLAIN, false, $ignoredIds);
$io->expects([
yield 'ignore by CVE with reasoning' => [
[
new Package('vendor1/package1', '3.0.0.0', '3.0.0'),
],
['CVE1' => 'A good reason'],
0,
[
['text' => 'Found 1 ignored security vulnerability advisory affecting 1 package:'],
['text' => 'Package: vendor1/package1'],
['text' => 'CVE: CVE1'],
['text' => 'Title: advisory1'],
['text' => 'URL: https://advisory.example.com/advisory1'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2022-05-25T13:21:00+00:00'],
['text' => 'Ignore reason: A good reason'],
]
];
yield 'ignore by advisory id' => [
[
new Package('vendor1/package2', '3.0.0.0', '3.0.0'),
],
['ID2'],
0,
[
['text' => 'Found 1 ignored security vulnerability advisory affecting 1 package:'],
['text' => 'Package: vendor1/package2'],
['text' => 'CVE: '],
['text' => 'Title: advisory2'],
['text' => 'URL: https://advisory.example.com/advisory2'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2022-05-25T13:21:00+00:00'],
]
];
yield 'ignore by remote id' => [
[
new Package('vendorx/packagex', '3.0.0.0', '3.0.0'),
],
['RemoteIDx'],
0,
[
['text' => 'Found 1 ignored security vulnerability advisory affecting 1 package:'],
['text' => 'Package: vendorx/packagex'],
['text' => 'CVE: CVE5'],
['text' => 'Title: advisory17'],
['text' => 'URL: https://advisory.example.com/advisory17'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2015-05-25T13:21:00+00:00'],
]
];
yield '1 vulnerability, 0 ignored' => [
[
new Package('vendor1/package1', '3.0.0.0', '3.0.0'),
],
[],
1,
[
['text' => 'Found 1 security vulnerability advisory affecting 1 package:'],
['text' => 'Package: vendor1/package1'],
['text' => 'CVE: CVE1'],
['text' => 'Title: advisory1'],
['text' => 'URL: https://advisory.example.com/advisory1'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2022-05-25T13:21:00+00:00'],
]
];
yield '1 vulnerability, 3 ignored affecting 2 packages' => [
[
new Package('vendor3/package1', '3.0.0.0', '3.0.0'),
// RemoteIDx
new Package('vendorx/packagex', '3.0.0.0', '3.0.0'),
// ID3, ID6
new Package('vendor2/package1', '3.0.0.0', '3.0.0'),
],
['RemoteIDx', 'ID3', 'ID6'],
1,
[
['text' => 'Found 3 ignored security vulnerability advisories affecting 2 packages:'],
['text' => 'Package: vendor2/package1'],
['text' => 'CVE: CVE2'],
['text' => 'Title: advisory3'],
['text' => 'URL: https://advisory.example.com/advisory3'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2022-05-25T13:21:00+00:00'],
['text' => 'Ignore reason: None specified'],
['text' => '--------'],
['text' => 'Package: vendor2/package1'],
['text' => 'CVE: CVE4'],
['text' => 'Title: advisory6'],
['text' => 'URL: https://advisory.example.com/advisory6'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2015-05-25T13:21:00+00:00'],
['text' => 'Ignore reason: None specified'],
['text' => '--------'],
['text' => 'Package: vendorx/packagex'],
['text' => 'CVE: CVE5'],
['text' => 'Title: advisory17'],
['text' => 'URL: https://advisory.example.com/advisory17'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2015-05-25T13:21:00+00:00'],
['text' => 'Ignore reason: None specified'],
['text' => 'Found 1 security vulnerability advisory affecting 1 package:'],
['text' => 'Package: vendor3/package1'],
['text' => 'CVE: CVE5'],
@ -93,12 +195,23 @@ class AuditorTest extends TestCase
['text' => 'URL: https://advisory.example.com/advisory7'],
['text' => 'Affected versions: >=3,<3.4.3|>=1,<2.5.6'],
['text' => 'Reported at: 2015-05-25T13:21:00+00:00'],
], true);
$this->assertSame(1, $result);
]
];
}
// without ignored IDs, we should get all 4
$result = $auditor->audit($io, $this->getRepoSet(), $packages, Auditor::FORMAT_PLAIN, false);
$this->assertSame(4, $result);
/**
* @dataProvider ignoredIdsProvider
* @phpstan-param array<\Composer\Package\Package> $packages
* @phpstan-param array<string>|array<string,string> $ignoredIds
* @phpstan-param 0|positive-int $exitCode
* @phpstan-param list<array{text: string, verbosity?: \Composer\IO\IOInterface::*, regex?: true}|array{ask: string, reply: string}|array{auth: array{string, string, string|null}}> $expectedOutput
*/
public function testAuditWithIgnore($packages, $ignoredIds, $exitCode, $expectedOutput): void
{
$auditor = new Auditor();
$result = $auditor->audit($io = $this->getIOMock(), $this->getRepoSet(), $packages, Auditor::FORMAT_PLAIN, false, $ignoredIds);
$io->expects($expectedOutput, true);
$this->assertSame($exitCode, $result);
}
private function getRepoSet(): RepositorySet