Still report ignored security advisories

composer · Aug 31, 2023 · 084575a · 084575a
1 parent 52f52dd
commit 084575a
Show file tree

Hide file tree

Showing 8 changed files with 233 additions and 51 deletions.
diff --git a/doc/06-config.md b/doc/06-config.md
@@ -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": {
+            "suppressed": ["CVE-1234", "GHSA-xx", "PKSA-yy"]
         }
     }
 }

diff --git a/phpstan/baseline.neon b/phpstan/baseline.neon
@@ -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
 
 		-

diff --git a/res/composer-schema.json b/res/composer-schema.json
@@ -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 their explanations (keys) for why they're being suppressed. 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"
+                                    }
+                                }
+                            ]
                         }
                     }
                 },

diff --git a/src/Composer/Advisory/Auditor.php b/src/Composer/Advisory/Auditor.php
@@ -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[] $ignoredIds 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
     {
-        $advisories = $repoSet->getMatchingSecurityAdvisories($packages, $format === self::FORMAT_SUMMARY);
+        $allAdvisories = $advisories = $repoSet->getMatchingSecurityAdvisories($packages, $format === self::FORMAT_SUMMARY);
 
         if (\count($ignoredIds) > 0) {
-            $advisories = $this->filterIgnoredAdvisories($advisories, $ignoredIds);
+          if (!\array_is_list($ignoredIds)) {
+              $ignoredIds = array_keys($ignoredIds);
+          }
+          $advisories = $this->filterIgnoredAdvisories($advisories, $ignoredIds);
         }
 
         if (self::FORMAT_JSON === $format) {
-            $io->write(JsonFile::encode(['advisories' => $advisories]));
+            $io->write(JsonFile::encode(['advisories' => $allAdvisories]));
 
             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($allAdvisories) > 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);
+            }
+
+            $ignoredAdvisories = $this->diffAdvisories($allAdvisories, $advisories);
+            [$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>');
@@ -228,4 +244,33 @@ private function getURL(SecurityAdvisory $advisory): string
 
         return '<href='.OutputFormatter::escape($advisory->link).'>'.OutputFormatter::escape($advisory->link).'</>';
     }
+
+    /**
+     * @phpstan-param array<string, array<\Composer\Advisory\PartialSecurityAdvisory>> $a
+     * @phpstan-param array<string, array<\Composer\Advisory\PartialSecurityAdvisory>> $b     *
+     * @return array<string, array<\Composer\Advisory\PartialSecurityAdvisory>>
+     */
+    private function diffAdvisories(array $a, array $b): array {
+        $result = $a;
+        foreach ($a as $package => $advisories) {
+            if (!array_key_exists($package, $b)) {
+                continue;
+            }
+            $packageAdvisoryIdsInB = array_map(static fn(PartialSecurityAdvisory $advisory): string => $advisory->advisoryId, $b[$package]);
+            $advisoryListChanged = FALSE;
+            foreach ($advisories as $index => $advisory) {
+                if (in_array($advisory->advisoryId, $packageAdvisoryIdsInB, TRUE)) {
+                    unset($result[$package][$index]);
+                    $advisoryListChanged = TRUE;
+                }
+            }
+            if ($advisoryListChanged) {
+                $result[$package] = array_values($result[$package]);
+            }
+        }
+
+        return array_filter($result);
+    }
+
+
 }
diff --git a/src/Composer/Command/AuditCommand.php b/src/Composer/Command/AuditCommand.php
@@ -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'] ?? []));
     }
 
     /**

diff --git a/src/Composer/Config.php b/src/Composer/Config.php
@@ -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 @@ 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']['suppressed'];
                     $this->config[$key] = $val;
                     $this->setSourceOfConfigValue($val, $key, $source);
-                    $this->config['audit']['ignored'] = array_merge($currentIgnores, $val['ignored'] ?? []);
+                    $this->config['audit']['pass'] = array_merge($currentAuditPass, $val['suppressed'] ?? []);
                 } else {
                     $this->config[$key] = $val;
                     $this->setSourceOfConfigValue($val, $key, $source);

diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php
@@ -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()) {

diff --git a/tests/Composer/Test/Advisory/AuditorTest.php b/tests/Composer/Test/Advisory/AuditorTest.php
@@ -72,33 +72,143 @@ public function testAudit(array $data, int $expected, string $message): void
         $this->assertSame($expected, $result, $message);
     }
 
-    public function testAuditIgnoredIDs(): void
-    {
-        $packages = [
-            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'),
+    public function ignoredIdsProvider(): \Generator {
+        yield 'ignore by CVE' => [
+            [
+                new Package('vendor1/package1', '3.0.0.0', '3.0.0'),
+            ],
+            ['CVE1'],
+            0,
+            [
+                ['text' => '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'],
+            ]
         ];
+        yield 'ignore by CVE with reasoning' => [
+            [
+                new Package('vendor1/package1', '3.0.0.0', '3.0.0'),
+            ],
+            ['CVE1' => 'A good reason'],
+            0,
+            [
+                ['text' => '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'],
+            ]
+        ];
+        yield 'ignore by advisory id' => [
+            [
+                new Package('vendor1/package2', '3.0.0.0', '3.0.0'),
+            ],
+            ['ID2'],
+            0,
+            [
+                ['text' => '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' => '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 1 security vulnerability advisory affecting 1 package:'],
+                ['text' => 'Package: vendor3/package1'],
+                ['text' => 'CVE: CVE5'],
+                ['text' => 'Title: advisory7'],
+                ['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'],
+                ['text' => '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' => '--------'],
+                ['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' => '--------'],
+                ['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'],
+            ]
+        ];
+    }
 
-        $ignoredIds = ['CVE1', 'ID2', 'RemoteIDx'];
-
+    /**
+     * @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([
-            ['text' => 'Found 1 security vulnerability advisory affecting 1 package:'],
-            ['text' => 'Package: vendor3/package1'],
-            ['text' => 'CVE: CVE5'],
-            ['text' => 'Title: advisory7'],
-            ['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);
+        $io->expects($expectedOutput, true);
+        $this->assertSame($exitCode, $result);
     }
 
     private function getRepoSet(): RepositorySet