Issues/451 Add advisory information to a commit messages #459

slash3b · 2021-11-22T21:45:25Z

This patch adds advisory information when advisory is added/updated

Fixes #451

Example of how it will look like.
composer.json has conflict "snipe/snipe-it": "<5.3.11|>=6.0.0-RC-1,<=6.0.0-RC-5",
after php build-conflicts.php it will become "snipe/snipe-it": "<5.4.3|>=6.0.0-RC-1,<=6.0.0-RC-5",

Commit message will be the following:

slash3b@Nostromo ~/P/p/SecurityAdvisoriesBuilder (issues/451)> php build-conflicts.php

"Committing generated "composer.json" file as per "2022-05-18T19:29:04+00:00"
Original commit: "https://github.com/FriendsOfPHP/security-advisories/commit/41f90c0690ef7ab1de89a12fada124e647ca2a87"

 Security advisories updated:
        Package name   | snipe/snipe-it
        Summary        | Cross-site Scripting in snipe-it
        URI            | https://github.com/advisories/GHSA-p885-prv3-m4xv
        Constraints    | <5.4.3

        Package name   | snipe/snipe-it
        Summary        | Old sessions not blocked by login enable function in Snipe-IT
        URI            | https://github.com/advisories/GHSA-636j-7x7r-gvw2
        Constraints    | <5.4.2

Ocramius · 2021-11-23T04:41:19Z

build-conflicts.php

+
+    $addedAdvisories = [];
+    foreach ($getAdvisories() as $advisory) {
+        if (in_array($advisory->package->packageName, $oldConflictPackages, true)) {


Well, this would only list new packages, but not new versions of those packages 🤔

A bit of a more elaborate intersection mechanism is needed here, perhaps reading the current composer.json into a set of exclusion ranges (we already have value objects for that)

oh, okay, thank you. I'll try to tackle it this weekend 🤔

No need to push it, don't worry! We can also try discussing this 1-on-1 in the coming weeks, if you want.

...and I got overwhelmed with stuff. Sorry for late response. Yes, 1-on-1 would be great as I'm not sure I understand what is needed to be done.
For example we have an old set of ranges "amphp/http-client": "<1|>=4,<4.4"
and new set of ranges that looks like so "amphp/http-client": ">=4,<4.4|>=4.6",
where >=4.6 is new and <1 was deleted/retracted.

I should mention both <1 and >=4.6 in a commit message, right ? Basically everything that is not "overlapping" between the two 🤔

@azjezz thank you for suggestions! It was a bit hard to wrap my head around some PSL stuff.

been staring at this whole evening. An elaborate intersection mechanism is, of course, possible, but looks overly complex(?) 🤔
If that is for human to read, could we just do something dumb like this:

get old and fresh json files with conflicts

if package is completely new, mark it as such

if package exists in both files, try to compare string values.

if strings are different, then add it, for example, like so

"amphp/http-client" updated from "<1|>=4,<4.4" to ">=4,<4.4|>=4.6"

And finally in commit message, we'd have it

Newly added: Package: "amphp/http" "<1.0.1" Summary: "Potential SQL injection vector" URI: "https://github.com/...." Updated: "amphp/http-client" updated from "<1|>=4,<4.4" to ">=4,<4.4|>=4.6" Summary: "Potential SQL injection vector" URI: "https://github.com/...." Deleted (completely): "foo/bar" ">1.2" Summary: "Potential SQL injection vector" URI: "https://github.com/...."

Just thinking out loud 🙈

An elaborate intersection mechanism is, of course, possible, but looks overly complex(?) thinking

We already have that.

read current range

read advisory

compare advisory with current range: if it is already fully included, then consider it "done"

otherwise, add it to the bucket of "advisories that will change composer.json"

Discussed briefly on twitter:

read current composer.json into a list<Component>, where each Component has its associated dependency exclusion range parsed (we already have most of that written)

read the individual advisories from their respective sources - keep a link to the source in each of the read advisories

compare the list<Component> to the list<Advisory>, and determine which Advisory instances are NOT fully contained in list<Component>

use the resulting data to produce the commit message

azjezz

I added some comments that should help you with using PSL instead of stdlib, let me know if you need help with anything else.

azjezz · 2021-12-01T22:01:20Z

build-conflicts.php

@@ -34,6 +34,11 @@
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromGithubApi;
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromMultipleSources;

+use function array_keys;


Suggested change

use function array_keys;

use Psl\Vec;

$keys = Vec\keys($array);

azjezz · 2021-12-01T22:01:40Z

build-conflicts.php

@@ -34,6 +34,11 @@
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromGithubApi;
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromMultipleSources;

+use function array_keys;
+use function count;


Suggested change

use function count;

use Psl\Iter;

$count = Iter\count($iterable);

azjezz · 2021-12-01T22:02:00Z

build-conflicts.php

@@ -34,6 +34,11 @@
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromGithubApi;
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromMultipleSources;

+use function array_keys;
+use function count;
+use function file_get_contents;


Suggested change

use function file_get_contents;

use Psl\Filesystem;

$content = Filesystem\read_file($filename);

azjezz · 2021-12-01T22:02:23Z

build-conflicts.php

+use function array_keys;
+use function count;
+use function file_get_contents;
+use function in_array;


Suggested change

use function in_array;

use Psl\Iter;

$contains = Iter\contains($array, $value);
$contains_key = Iter\contains_key($array, $key);

azjezz · 2021-12-01T22:03:51Z

build-conflicts.php

+use function count;
+use function file_get_contents;
+use function in_array;
+use function json_decode;


Suggested change

use function json_decode;

use Psl\Json;

$result = Json\decode($json);

or better:

use Psl\Json; use Psl\Type; $result = Json\typed($json, Type\shape([ 'foo' => Type\string(), 'bar' => Type\vec(Type\int()), // vec<T> is list<T> 'baz' => Type\dict(Type\int(), Type\string()), // dict<Tk, Tv> is array<Tk, Tv> ]);

…izations

…ersions represented as strings

slash3b · 2022-05-18T16:00:38Z

build-conflicts.php

@@ -34,6 +34,11 @@
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromGithubApi;
 use Roave\SecurityAdvisories\AdvisorySources\GetAdvisoriesFromMultipleSources;

+use function array_keys;
+use function count;
+use function file_get_contents;


slash3b · 2022-05-18T16:51:48Z

src/Roave/SecurityAdvisories/VersionConstraint.php

@@ -33,7 +35,7 @@ private function __construct()
     */
    public static function fromString(string $versionConstraint): self
    {
-        $constraintString = $versionConstraint;
+        $constraintString = Str\replace($versionConstraint, ' ', '');


this one handles spaces in boundaries, coming from github advisories, not super critical but annoying

slash3b · 2022-05-18T17:31:35Z

src/Roave/SecurityAdvisories/VersionConstraint.php

@@ -142,8 +144,12 @@ public function mergeWith(self $other): self
        ));
    }

-    private function contains(self $other): bool
+    public function contains(self $other): bool


not sure how to do it any other was 🤔

slash3b · 2022-05-18T19:10:19Z

test/RoaveTest/SecurityAdvisories/Helper/ConstraintsMapTest.php

+    }
+
+    /**
+     * @return array<string, mixed>


need a little push, how do I describe type for this ?

ERROR: MixedInferredReturnType - test/RoaveTest/SecurityAdvisories/Helper/ConstraintsMapTest.php:69:16 - Providers must return iterable<array-key, array<array-key, mixed>>, possibly different array<string, mixed> provided (see https://psalm.dev/047) * @return array<string, mixed> ERROR: MixedInferredReturnType - test/RoaveTest/SecurityAdvisories/Helper/ConstraintsMapTest.php:148:16 - Providers must return iterable<array-key, array<array-key, mixed>>, possibly different array<string, mixed> provided (see https://psalm.dev/047) * @return array<string, mixed>

…+00:00" Original commit: "FriendsOfPHP/security-advisories@e14352c"

slash3b force-pushed the issues/451 branch from 2822e5f to 698b3b9 Compare November 22, 2021 21:46

Ocramius requested changes Nov 23, 2021

View reviewed changes

Ocramius added the enhancement label Nov 23, 2021

azjezz reviewed Dec 1, 2021

View reviewed changes

Ocramius mentioned this pull request Apr 13, 2022

Fix snipe/snipe-it major version constraints Roave/SecurityAdvisories#93

Closed

slash3b added 22 commits May 11, 2022 21:37

dirty crude implementation

2876480

stuck with PSL, Psalm and.. everything

b4069af

friends of php test is fixed

c21c6d7

components fixed

6e7fcda

advisory tests fixes

03034c0

multiple sources fix

4271453

fuck yes! unit tests are no longer broken

f35e700

some polishing ftw

a89685b

phpcs automatic fixes

3f0c1cb

a bit stuck

fcb9dda

deleted bad comment

4cd6eae

crude PoC

08a02eb

constraints map implemented (?) but requires test

9431fb9

test for constraint map

e3c11e4

code sniffer fixes

0552bf2

code sniffer satisfied for now

0339ba2

build-conflicts changes

c528ad8

build-conflicts cs fix

b89a3f4

tests are good to go now. Had to add "source" to all Advisory initial…

03d5e8d

…izations

no need for asserts for now

bb88c3d

build-conflict psalm resolved for now (?)

72e7e73

psalm fixes

b8e40fb

slash3b force-pushed the issues/451 branch from 698b3b9 to b8e40fb Compare May 17, 2022 21:06

how about now ?

18bd74a

slash3b changed the title ~~Issues/451 Add advisory information to a commit messages~~ [WIP] Issues/451 Add advisory information to a commit messages May 18, 2022

slash3b added 2 commits May 18, 2022 17:07

remove whitespace coming from github

9be9605

for contains in VersionConstraint.php lets try to compare complex v…

3638683

…ersions represented as strings

slash3b commented May 18, 2022

View reviewed changes

slash3b marked this pull request as ready for review May 18, 2022 19:34

slash3b changed the title ~~[WIP] Issues/451 Add advisory information to a commit messages~~ Issues/451 Add advisory information to a commit messages May 24, 2022

Ocramius referenced this pull request in Roave/SecurityAdvisories Feb 2, 2024

Committing generated "composer.json" file as per "2024-02-01T20:04:27…

b44b25c

…+00:00" Original commit: "FriendsOfPHP/security-advisories@e14352c"

Ocramius mentioned this pull request Feb 8, 2024

Unrelated conflict changes (typo3/cms-* vs. mediawiki/semantic-media-wiki) Roave/SecurityAdvisories#127

Closed

Ocramius mentioned this pull request Feb 23, 2024

Something weird is happening in regards of version constraints Roave/SecurityAdvisories#128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issues/451 Add advisory information to a commit messages #459

Issues/451 Add advisory information to a commit messages #459

slash3b commented Nov 22, 2021 •

edited

Loading

Ocramius Nov 23, 2021

slash3b Nov 23, 2021

Ocramius Nov 23, 2021

slash3b Dec 8, 2021

slash3b Dec 14, 2021

Ocramius Dec 27, 2021

Ocramius Jan 4, 2022

azjezz left a comment

azjezz Dec 1, 2021

azjezz Dec 1, 2021

azjezz Dec 1, 2021

slash3b May 18, 2022

azjezz Dec 1, 2021

azjezz Dec 1, 2021

slash3b May 18, 2022

slash3b May 18, 2022

slash3b May 18, 2022

slash3b May 18, 2022

Issues/451 Add advisory information to a commit messages #459

Are you sure you want to change the base?

Issues/451 Add advisory information to a commit messages #459

Conversation

slash3b commented Nov 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azjezz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slash3b commented Nov 22, 2021 •

edited

Loading