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

Add audit.abandoned warnings for abandoned packages #11639

Merged
merged 1 commit into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 deletions doc/06-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ or
}
```

### abandoned

Defaults to `report` in Composer 2.6, and defaults to `fail` from Composer 2.7 on. Defines whether the audit command reports abandoned packages or not, this has three possible values:

- `ignore` means the audit command does not consider abandoned packages at all.
- `report` means abandoned packages are reported as an error but do not cause the command to exit with a non-zero code.
- `fail` means abandoned packages will cause audits to fail with a non-zero code.

## use-parent-dir

When running Composer in a directory where there is no composer.json, if there
Expand Down
4 changes: 4 additions & 0 deletions res/composer-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@
}
}
]
},
"abandoned": {
"enum": ["ignore", "report", "fail"],
"description": "Whether abandoned packages should be ignored, reported as problems or cause an audit failure."
}
}
},
Expand Down
101 changes: 96 additions & 5 deletions src/Composer/Advisory/Auditor.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
use Composer\IO\ConsoleIO;
use Composer\IO\IOInterface;
use Composer\Json\JsonFile;
use Composer\Package\CompletePackageInterface;
use Composer\Package\PackageInterface;
use Composer\Repository\RepositorySet;
use Composer\Util\PackageInfo;
use InvalidArgumentException;
use Symfony\Component\Console\Formatter\OutputFormatter;

Expand All @@ -40,17 +42,26 @@ class Auditor
self::FORMAT_SUMMARY,
];

public const ABANDONED_IGNORE = 'ignore';
public const ABANDONED_REPORT = 'report';
public const ABANDONED_FAIL = 'fail';

/**
* @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[] $ignoreList List of advisory IDs, remote IDs or CVE IDs that reported but not listed as vulnerabilities.
* @param self::ABANDONED_* $abandoned
*
* @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 $ignoreList = []): int
public function audit(IOInterface $io, RepositorySet $repoSet, array $packages, string $format, bool $warningOnly = true, array $ignoreList = [], string $abandoned = self::ABANDONED_REPORT): int
{
if ($abandoned === 'default' && $format !== self::FORMAT_SUMMARY) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default is not one of the values supported in the method signature for static analysis.

Copy link
Contributor

@curry684 curry684 Sep 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would define an extra constant ABANDONED_DEFAULT = 'default' for now and switch it to ABANDONED_DEFAULT = self::ABANDONED_FAIL in 2.7. Also this section needs a todo to remove the warning in 2.7

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is default as a value will be gone in 2.7 so i rather keep it low profile. IMO it's fine as is

$io->writeError('<warning>The new audit.abandoned setting (currently defaulting to "report" will default to "fail" in Composer 2.7, make sure to set it to "report" or "ignore" explicitly by then if you do not want this.</warning>');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this warning really always be output or only if an actual warning is output? Seems a bit much to force everyone to set this to a value to make the warning go away?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well if you run audit in CI you can also just ignore this warning.. my thinking was more that if it's hidden then you won't notice it unless you have abandoned packages. Then people will come complaining when 2.7 hits and they have an abandoned package in six months..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really an issue? I highly doubt audit is frequently used outside CI context, so it's not really a 'daily annoyance' or anything, and those that do consciously run audit will be the kind of user happy with the verbose explanation.

}

$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
Expand All @@ -59,15 +70,27 @@ public function audit(IOInterface $io, RepositorySet $repoSet, array $packages,
}
['advisories' => $advisories, 'ignoredAdvisories' => $ignoredAdvisories] = $this->processAdvisories($allAdvisories, $ignoreList);

$abandonedCount = 0;
$affectedPackagesCount = 0;
if ($abandoned === self::ABANDONED_IGNORE) {
$abandonedPackages = [];
} else {
$abandonedPackages = $this->filterAbandonedPackages($packages);
if ($abandoned === self::ABANDONED_FAIL) {
$abandonedCount = count($abandonedPackages);
}
}

if (self::FORMAT_JSON === $format) {
$json = ['advisories' => $advisories];
if ($ignoredAdvisories !== []) {
$json['ignored-advisories'] = $ignoredAdvisories;
}
$json['abandoned'] = $abandonedPackages;

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

return count($advisories);
return count($advisories) + $abandonedCount;
}

$errorOrWarn = $warningOnly ? 'warning' : 'error';
Expand All @@ -91,13 +114,26 @@ public function audit(IOInterface $io, RepositorySet $repoSet, array $packages,
if ($format === self::FORMAT_SUMMARY) {
$io->writeError('Run "composer audit" for a full list of advisories.');
}
} else {
$io->writeError('<info>No security vulnerability advisories found.</info>');
}

return $affectedPackagesCount;
if (count($abandonedPackages) > 0 && $format !== self::FORMAT_SUMMARY) {
$this->outputAbandonedPackages($io, $abandonedPackages, $format);
}

$io->writeError('<info>No security vulnerability advisories found</info>');
return $affectedPackagesCount + $abandonedCount;
}

return 0;
/**
* @param array<PackageInterface> $packages
* @return array<PackageInterface>
*/
private function filterAbandonedPackages(array $packages): array
{
return array_filter($packages, function (PackageInterface $pkg) {
return $pkg instanceof CompletePackageInterface && $pkg->isAbandoned();
});
}

/**
Expand Down Expand Up @@ -268,6 +304,61 @@ private function outputAdvisoriesPlain(IOInterface $io, array $advisories): void
$io->writeError($error);
}

/**
* @param array<PackageInterface> $packages
* @param self::FORMAT_PLAIN|self::FORMAT_TABLE $format
*/
private function outputAbandonedPackages(IOInterface $io, array $packages, string $format): void
{
$io->writeError(sprintf('<error>Found %d abandoned package%s:</error>', count($packages), count($packages) > 1 ? 's' : ''));

if ($format === self::FORMAT_PLAIN) {
foreach ($packages as $pkg) {
if (!$pkg instanceof CompletePackageInterface) {
continue;
}

$replacement = $pkg->getReplacementPackage() !== null
? 'Use '.$pkg->getReplacementPackage().' instead'
: 'No replacement was suggested';
$io->writeError(sprintf(
'%s is abandoned. %s.',
$this->getPackageNameWithLink($pkg),
$replacement
));
}

return;
}

if (!($io instanceof ConsoleIO)) {
throw new InvalidArgumentException('Cannot use table format with ' . get_class($io));
}

$table = $io->getTable()
->setHeaders(['Abandoned Package', 'Suggested Replacement'])
->setColumnWidth(1, 80)
->setColumnMaxWidth(1, 80);

foreach ($packages as $pkg) {
if (!$pkg instanceof CompletePackageInterface) {
continue;
}

$replacement = $pkg->getReplacementPackage() !== null ? $pkg->getReplacementPackage() : 'none';
$table->addRow([$this->getPackageNameWithLink($pkg), $replacement]);
}

$table->render();
}

private function getPackageNameWithLink(PackageInterface $package): string
{
$packageUrl = PackageInfo::getViewSourceOrHomepageUrl($package);

return $packageUrl !== null ? '<href=' . OutputFormatter::escape($packageUrl) . '>' . $package->getPrettyName() . '</>' : $package->getPrettyName();
}

private function getCVE(SecurityAdvisory $advisory): string
{
if ($advisory->cve === null) {
Expand Down
4 changes: 3 additions & 1 deletion src/Composer/Command/AuditCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ 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')['ignore'] ?? []));
$auditConfig = $composer->getConfig()->get('audit');

return min(255, $auditor->audit($this->getIO(), $repoSet, $packages, $this->getAuditFormat($input, 'format'), false, $auditConfig['ignore'] ?? [], $auditConfig['abandoned'] ?? Auditor::ABANDONED_REPORT));
}

/**
Expand Down
9 changes: 9 additions & 0 deletions src/Composer/Command/ConfigCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

namespace Composer\Command;

use Composer\Advisory\Auditor;
use Composer\Pcre\Preg;
use Composer\Util\Filesystem;
use Composer\Util\Platform;
Expand Down Expand Up @@ -512,6 +513,14 @@ static function ($val) {
return $val !== 'false' && (bool) $val;
},
],
'audit.abandoned' => [
static function ($val): bool {
return in_array($val, [Auditor::ABANDONED_IGNORE, Auditor::ABANDONED_REPORT, Auditor::ABANDONED_FAIL], true);
},
static function ($val) {
return $val;
},
],
];
$multiConfigValues = [
'github-protocols' => [
Expand Down
5 changes: 3 additions & 2 deletions src/Composer/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

namespace Composer;

use Composer\Advisory\Auditor;
use Composer\Config\ConfigSourceInterface;
use Composer\Downloader\TransportException;
use Composer\IO\IOInterface;
Expand All @@ -37,7 +38,7 @@ class Config
'allow-plugins' => [],
'use-parent-dir' => 'prompt',
'preferred-install' => 'dist',
'audit' => ['ignore' => []],
'audit' => ['ignore' => [], 'abandoned' => 'default'], // TODO in 2.7 switch to ABANDONED_FAIL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencing Auditor::ABANDONED_DEFAULT here automatically future-proofs the code.

'notify-on-install' => true,
'github-protocols' => ['https', 'ssh', 'git'],
'gitlab-protocol' => null,
Expand Down Expand Up @@ -210,7 +211,7 @@ public function merge(array $config, string $source = self::SOURCE_UNKNOWN): voi
}
} elseif ('audit' === $key) {
$currentIgnores = $this->config['audit']['ignore'];
$this->config[$key] = $val;
$this->config[$key] = array_merge($this->config['audit'], $val);
$this->setSourceOfConfigValue($val, $key, $source);
$this->config['audit']['ignore'] = array_merge($currentIgnores, $val['ignore'] ?? []);
} else {
Expand Down
4 changes: 3 additions & 1 deletion src/Composer/Installer.php
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,9 @@ public function run(): int
$repoSet->addRepository($repo);
}

return $auditor->audit($this->io, $repoSet, $packages, $this->auditFormat, true, $this->config->get('audit')['ignore'] ?? []) > 0 && $this->errorOnAudit ? self::ERROR_AUDIT_FAILED : 0;
$auditConfig = $this->config->get('audit');

return $auditor->audit($this->io, $repoSet, $packages, $this->auditFormat, true, $auditConfig['ignore'] ?? [], $auditConfig['abandoned'] ?? Auditor::ABANDONED_REPORT) > 0 && $this->errorOnAudit ? self::ERROR_AUDIT_FAILED : 0;
} catch (TransportException $e) {
$this->io->error('Failed to audit '.$target.' packages.');
if ($this->io->isVerbose()) {
Expand Down