Include advisory source in new security advisory commits

[Ocramius]

One regular source of support questions is "why is some/library:1.2.3 included in the roave/security-advisories conflicts section?"

This is becoming regular and quite frustrating:

• Why was ignition 1.x removed from the conflict exception? SecurityAdvisories#86
• Conflict with silverstripe/admin SecurityAdvisories#85
• Typo in package name "symfont/process" SecurityAdvisories#84

We probably do want to start committing the source of an advisory.

Specifically, we need to add a Source (value object with a URI in it, basically) to Advisory:

SecurityAdvisoriesBuilder/src/Roave/SecurityAdvisories/Advisory.php

Lines 33 to 46 in 0246933

	/** @psalm-immutable */
	final class Advisory
	{
	public PackageName $package;
	/** @var list<VersionConstraint> */
	private array $branchConstraints;
	/** @param list<VersionConstraint> $branchConstraints */
	private function __construct(PackageName $package, array $branchConstraints)
	{
	$this->package = $package;
	$this->branchConstraints = $this->sortVersionConstraints($branchConstraints);
	}

After doing that comes the tricky part: we need to identify which advisories were not considered as part of the pre-existing composer.json.

For that, we need to:

1. read the pre-existing composer.json into an usable in-memory data structure
2. compare each of the Advisory instances against it
3. isolate those Advisory instances that would lead to a change of the excluded (data structure above?)
4. add these Advisory instances to a list that is then used to determine the commit message to be generated
5. generate the new commit message, which is currently hardcoded:
   

   SecurityAdvisoriesBuilder/build-conflicts.php

   Lines 247 to 275 in 14a83da

   	$commitComposerJson = static function (string $composerJsonPath) use ($execute): void {
   	$parseHead =
   	/** @psalm-return non-empty-list<string> */
   	static function () use ($execute): array {
   	return $execute('git rev-parse HEAD');
   	};
   	$originalHash = runInPath(
   	$parseHead,
   	dirname($composerJsonPath) . '/../security-advisories'
   	);
   	runInPath(
   	static function () use ($composerJsonPath, $originalHash, $execute): void {
   	$execute('git add ' . escapeshellarg(realpath($composerJsonPath)));
   	$message = sprintf(
   	'Committing generated "composer.json" file as per "%s"',
   	(new DateTime('now', new DateTimeZone('UTC')))->format(DateTime::W3C)
   	);
   	$message .= "\n" . sprintf(
   	'Original commit: "%s"',
   	'https://github.com/FriendsOfPHP/security-advisories/commit/' . $originalHash[0]
   	);
   	$execute('git diff-index --quiet HEAD || git commit -m ' . escapeshellarg($message));
   	},
   	dirname($composerJsonPath)
   	);
   	};

[slash3b]

Locally, I've just deleted some of packages from the "old" version and do comparison of newly generated composer.json to the old one. So this way we will have a a list of packages that are not in the old composer.json version.

Please check example below. Is this what we need ?

/usr/bin/php /home/slash3b/Projects/php/SecurityAdvisoriesBuilder/build-conflicts.php
Committing generated "composer.json" file as per "2021-11-20T22:33:36+00:00"
Original commit: "https://github.com/FriendsOfPHP/security-advisories/commit/486a92e290b38e5f01f0075c3919a5d251960671"

Added advisories:
   Package name: "alterphp/easyadmin-extension-bundle"
   Summary: "Action case insensitivity"
   URI: "https://github.com/alterphp/EasyAdminExtensionBundle/releases/tag/v1.3.1"

   Package name: "3f/pygmentize"
   Summary: "Remote Code Execution"
   URI: "https://github.com/dedalozzo/pygmentize/issues/1"

   Package name: "adodb/adodb-php"
   Summary: "XSS vulnerability in old test script"
   URI: "https://github.com/ADOdb/ADOdb/issues/274"

   Package name: "adodb/adodb-php"
   Summary: "Potential SQL injection vector"
   URI: "https://github.com/ADOdb/ADOdb/pull/401"

   Package name: "akaunting/akaunting"
   Summary: "Malicious password-reset in Akaunting"
   URI: "https://github.com/advisories/GHSA-246r-r2wf-frhx"

[Ocramius]

That seems exactly like what we're trying to achieve here, yes 👍