Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change audit.ignore behavior before 2.6.0 #11605

Merged
merged 7 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 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"]
}
}
}
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 let the audit command pass.",
"additionalProperties": {
"type": ["string", "string"]
}
},
{
"type": "array",
"description": "A set of advisory ids, remote ids or CVE ids that are reported but let the audit command pass.",
"items": {
"type": "string"
}
}
]
}
}
},
Expand Down
171 changes: 123 additions & 48 deletions src/Composer/Advisory/Auditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,34 +44,59 @@ 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';
$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($advisories) > 0 || count($ignoredAdvisories) > 0) {
[$affectedPackagesCount, $totalAdvisoryCount] = $this->countAdvisories($advisories);
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, $advisories, $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);
}
}

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>');
Expand All @@ -80,37 +105,78 @@ 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>>
* @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 ($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)) {
return false;
$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 = 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 ['advisories' => $advisories, 'ignoredAdvisories' => $ignored];
}

/**
Expand Down Expand Up @@ -146,8 +212,6 @@ private function outputAdvisories(IOInterface $io, array $advisories, string $fo

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:
Expand All @@ -162,24 +226,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 +276,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 +302,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 {
mxr576 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @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}>
Seldaek marked this conversation as resolved.
Show resolved Hide resolved
* @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'];
$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);
Expand Down