Skip to content

Commit

Permalink
Still report ignored security advisories
Browse files Browse the repository at this point in the history
  • Loading branch information
mxr576 committed Aug 31, 2023
1 parent 52f52dd commit 89eb824
Show file tree
Hide file tree
Showing 10 changed files with 332 additions and 86 deletions.
22 changes: 19 additions & 3 deletions doc/06-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 reported but they do not make the audit command fail.

```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"]
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion phpstan/baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ parameters:
ignoreErrors:
-
message: "#^Parameter \\#2 \\$advisories of method Composer\\\\Advisory\\\\Auditor\\:\\:outputAdvisories\\(\\) expects array\\<string, array\\<Composer\\\\Advisory\\\\SecurityAdvisory\\>\\>, array\\<string, array\\<Composer\\\\Advisory\\\\PartialSecurityAdvisory\\>\\> given\\.$#"
count: 1
count: 3
path: ../src/Composer/Advisory/Auditor.php

-
Expand Down
23 changes: 17 additions & 6 deletions res/composer-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,23 @@
"type": "object",
"description": "Security audit configuration options",
"properties": {
"ignored": {
"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.",
"items": {
"type": "string"
}
"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 they do not make the audit command fail.",
"additionalProperties": {
"type": ["string", "string"]
}
},
{
"type": "array",
"description": "A set of advisory ids, remote ids or CVE ids that reported but they do not make the audit command fail.",
"items": {
"type": "string"
}
}
]
}
}
},
Expand Down
160 changes: 114 additions & 46 deletions src/Composer/Advisory/Auditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,34 +44,50 @@ 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);
}
['filtered' => $filteredAdvisories, 'ignored' => $ignoredAdvisories] = $this->processAdvisories($repoSet->getMatchingSecurityAdvisories($packages, $format === self::FORMAT_SUMMARY), $ignoreList);

if (self::FORMAT_JSON === $format) {
$io->write(JsonFile::encode(['advisories' => $advisories]));
$json = ['advisories' => $filteredAdvisories];
if ($ignoredAdvisories !== []) {
$json['ignored-advisories'] = $ignoredAdvisories;
}

$io->write(JsonFile::encode($json));

return count($advisories);
return count($filteredAdvisories);
}

$errorOrWarn = $warningOnly ? 'warning' : 'error';
if (count($advisories) > 0) {
[$affectedPackages, $totalAdvisories] = $this->countAdvisories($advisories);
$plurality = $totalAdvisories === 1 ? 'y' : 'ies';
$pkgPlurality = $affectedPackages === 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);

return $affectedPackages;
if (count($filteredAdvisories) > 0 || count($ignoredAdvisories) > 0) {
[$affectedPackagesCount, $totalAdvisoryCount] = $this->countAdvisories($filteredAdvisories);
if ($affectedPackagesCount > 0) {
$plurality = $totalAdvisoryCount === 1 ? 'y' : 'ies';
$pkgPlurality = $affectedPackagesCount === 1 ? '' : 's';
$punctuation = $format === 'summary' ? '.' : ':';
$io->writeError("<$errorOrWarn>Found $totalAdvisoryCount security vulnerability advisor{$plurality} affecting $affectedPackagesCount package{$pkgPlurality}{$punctuation}</$errorOrWarn>");
$this->outputAdvisories($io, $filteredAdvisories, $format);
}

if ($ignoredAdvisories !== []) {
[$ignoredAffectedPackagesCount, $ignoredTotalAdvisoryCount] = $this->countAdvisories($ignoredAdvisories);
if ($ignoredAffectedPackagesCount > 0) {
$plurality = $ignoredTotalAdvisoryCount === 1 ? 'y' : 'ies';
$pkgPlurality = $ignoredAffectedPackagesCount === 1 ? '' : 's';
$punctuation = $format === 'summary' ? '.' : ':';
$io->writeError("<info>$ignoredTotalAdvisoryCount ignored security vulnerability advisor{$plurality} affecting $ignoredAffectedPackagesCount package{$pkgPlurality}{$punctuation}</info>");
$this->outputAdvisories($io, $ignoredAdvisories, $format);
}

}

return $affectedPackagesCount;
}

$io->writeError('<info>No security vulnerability advisories found</info>');
Expand All @@ -81,36 +97,77 @@ public function audit(IOInterface $io, RepositorySet $repoSet, array $packages,

/**
* @phpstan-param array<string, array<PartialSecurityAdvisory|SecurityAdvisory>> $advisories
* @param array<string> $ignoredIds
* @phpstan-return array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>
* @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{filtered: array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>, ignored: array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>}
*/
private function filterIgnoredAdvisories(array $advisories, array $ignoredIds): array
private function processAdvisories(array $advisories, array $ignoreList): array
{
if ($ignoreList === []) {
return ['filtered' => $advisories, 'ignored' => []];
}

if ((\count($ignoreList) > 0) && !\array_is_list($ignoreList)) {
$ignoredIds = array_keys($ignoreList);
}
else {
$ignoredIds = $ignoreList;
}

$filtered = [];
$ignored = [];
$ignoreReason = NULL;

foreach ($advisories as $package => $pkgAdvisories) {
$advisories[$package] = array_filter($pkgAdvisories, static function (PartialSecurityAdvisory $advisory) use ($ignoredIds) {
foreach ($pkgAdvisories as $advisory) {
$isFiltered = true;

if (in_array($advisory->advisoryId, $ignoredIds, true)) {
return false;
$isFiltered = false;
$ignoreReason = $ignoreList[$advisory->advisoryId] ?? NULL;
}

if ($advisory instanceof SecurityAdvisory) {
if (in_array($advisory->cve, $ignoredIds, true)) {
return false;
$isFiltered = false;
$ignoreReason = $ignoreList[$advisory->cve] ?? NULL;
}

foreach ($advisory->sources as $source) {
if (in_array($source['remoteId'], $ignoredIds, true)) {
return false;
$isFiltered = false;
$ignoreReason = $ignoreList[$source['remoteId']] ?? NULL;
break;
}
}
}

return true;
});
if (\count($advisories[$package]) === 0) {
unset($advisories[$package]);
if ($isFiltered) {
$filtered[$package][] = $advisory;
}
else {
// Partial security advisories only used in summary mode
// and that case we do not need to cast the object.
assert($advisory instanceof SecurityAdvisory);
$advisory = new IgnoredSecurityAdvisory(
$advisory->packageName,
$advisory->advisoryId,
$advisory->affectedVersions,
$advisory->title,
$advisory->sources,
$advisory->reportedAt,
$advisory->cve,
$advisory->link
);
if ($ignoreReason !== NULL) {
$advisory->specifyIgnoreReason($ignoreReason);
}

$ignored[$package][] = $advisory;
}
}
}

return $advisories;
return ['filtered' => $filtered, 'ignored' => $ignored];
}

/**
Expand Down Expand Up @@ -162,24 +219,31 @@ private function outputAdvisoriesTable(ConsoleIO $io, array $advisories): void
{
foreach ($advisories as $packageAdvisories) {
foreach ($packageAdvisories as $advisory) {
$headers = [
'Package',
'CVE',
'Title',
'URL',
'Affected versions',
'Reported at',
];
$row = [
$advisory->packageName,
$this->getCVE($advisory),
$advisory->title,
$this->getURL($advisory),
$advisory->affectedVersions->getPrettyString(),
$advisory->reportedAt->format(DATE_ATOM),
];
$showIgnoreReason = $advisory instanceof IgnoredSecurityAdvisory && $advisory->ignoreReason !== NULL;
if ($showIgnoreReason) {
$headers[] = 'Ignore reason';
$row[] = $advisory->ignoreReason;
}
$io->getTable()
->setHorizontal()
->setHeaders([
'Package',
'CVE',
'Title',
'URL',
'Affected versions',
'Reported at',
])
->addRow([
$advisory->packageName,
$this->getCVE($advisory),
$advisory->title,
$this->getURL($advisory),
$advisory->affectedVersions->getPrettyString(),
$advisory->reportedAt->format(DATE_ATOM),
])
->setHeaders($headers)
->addRow($row)
->setColumnWidth(1, 80)
->setColumnMaxWidth(1, 80)
->render();
Expand All @@ -205,6 +269,9 @@ private function outputAdvisoriesPlain(IOInterface $io, array $advisories): void
$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 && $advisory->ignoreReason !== NULL) {
$error[] = "Ignore reason: ".$advisory->ignoreReason;
}
$firstAdvisory = false;
}
}
Expand All @@ -228,4 +295,5 @@ private function getURL(SecurityAdvisory $advisory): string

return '<href='.OutputFormatter::escape($advisory->link).'>'.OutputFormatter::escape($advisory->link).'</>';
}

}
41 changes: 41 additions & 0 deletions src/Composer/Advisory/IgnoredSecurityAdvisory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?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;


class IgnoredSecurityAdvisory extends SecurityAdvisory {

/**
* @var string|null
*/
public $ignoreReason;

public function specifyIgnoreReason(string $ignoreReason): void {
$this->ignoreReason = $ignoreReason;
}

/**
* @return mixed
*/
#[\ReturnTypeWillChange]
public function jsonSerialize()
{
$data = parent::jsonSerialize();
if ($this->ignoreReason === NULL) {
unset($data['ignoreReason']);
}

return $data;
}

}
2 changes: 1 addition & 1 deletion src/Composer/Advisory/SecurityAdvisory.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ 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;
Expand Down
2 changes: 1 addition & 1 deletion src/Composer/Command/AuditCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
$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'] ?? []));
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/Composer/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -209,10 +209,10 @@ public function merge(array $config, string $source = self::SOURCE_UNKNOWN): voi
$this->setSourceOfConfigValue($val, $key, $source);
}
} elseif ('audit' === $key) {
$currentIgnores = $this->config['audit']['ignored'];
$currentAuditPass = $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($currentAuditPass, $val['ignore'] ?? []);
} else {
$this->config[$key] = $val;
$this->setSourceOfConfigValue($val, $key, $source);
Expand Down
2 changes: 1 addition & 1 deletion src/Composer/Installer.php
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public function run(): int
$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()) {
Expand Down

0 comments on commit 89eb824

Please sign in to comment.