Merge ~pfsmorigo/ubuntu-cve-tracker:pfsmorigo/reason_in_notes into ubuntu-cve-tracker:master

Proposed by Paulo Flabiano Smorigo
Status: Merged
Merged at revision: 1a98a3ca6297b5152a92307a832b3c6f1a43af73
Proposed branch: ~pfsmorigo/ubuntu-cve-tracker:pfsmorigo/reason_in_notes
Merge into: ubuntu-cve-tracker:master
Diff against target: 14 lines (+4/-0)
1 file modified
scripts/publish-cves-to-website-api.py (+4/-0)
Reviewer Review Type Date Requested Status
Ubuntu Security Team Pending
Review via email: mp+448821@code.launchpad.net

Description of the change

Add the priority reason as the top comment in the notes session like this:

https://ubuntu.com/security/CVE-2018-7409

This is a temporary solution until the new field is added by the web team.

To post a comment you must log in.
Revision history for this message
Emilia Torino (emitorino) wrote :

Leaving one question just in case, otherwise LGTM!

Revision history for this message
Steve Beattie (sbeattie) wrote :

On Wed, Aug 09, 2023 at 06:39:15PM -0000, Paulo Flabiano Smorigo wrote:
> Paulo Flabiano Smorigo has proposed merging ~pfsmorigo/ubuntu-cve-tracker:pfsmorigo/reason_in_notes into ubuntu-cve-tracker:master.
>
> Requested reviews:
> Ubuntu Security Team (ubuntu-security)
>
> For more details, see:
> https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821
>
> Add the priority reason as the top comment in the notes session like this:
>
> https://ubuntu.com/security/CVE-2018-7409
>
> This is a temporary solution until the new field is added by the web team.

You could conceivably make the author of the priority reason be the
ubuntu-security user on launchpad, but I'm not sure it's worth the
bother for a temporary fix.

I would like to see the attached patch containing testcases for putting
the priority reason in the notes field, however.

Thanks.

--
Steve Beattie
<email address hidden>

0From 1849959a109b7b9d57af562932f0fe32f7d00e0d Mon Sep 17 00:00:00 20010From 1849959a109b7b9d57af562932f0fe32f7d00e0d Mon Sep 17 00:00:00 2001
1From: Steve Beattie <steve.beattie@canonical.com>1From: Steve Beattie <steve.beattie@canonical.com>
2Date: Thu, 10 Aug 2023 15:34:15 -07002Date: Thu, 10 Aug 2023 15:34:15 -0700
3Subject: [PATCH] publish-cves-to-website-api.py: add priority reason notes3Subject: [PATCH] publish-cves-to-website-api.py: add priority reason notes
4tests4tests
55
6Add a couple of tests to validate putting the priority reason in the6Add a couple of tests to validate putting the priority reason in the
7notes field as a temporary fix. These can also be used as a basis7notes field as a temporary fix. These can also be used as a basis
8for tests when the web team adds a separate priority reason field.8for tests when the web team adds a separate priority reason field.
99
10MR: https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/44882110MR: https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821
11Signed-off-by: Steve Beattie <steve.beattie@canonical.com>11Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
12--
13test/website_api/use_priority_reason | 26 ++++++++12test/website_api/use_priority_reason | 26 ++++++++
14test/website_api/use_priority_reason.json | 56 ++++++++++++++++13test/website_api/use_priority_reason.json | 56 ++++++++++++++++
15.../use_priority_reason_plus_notes | 29 +++++++++14.../use_priority_reason_plus_notes | 29 +++++++++
16.../use_priority_reason_plus_notes.json | 64 +++++++++++++++++++15.../use_priority_reason_plus_notes.json | 64 +++++++++++++++++++
174 files changed, 175 insertions(+)164 files changed, 175 insertions(+)
18create mode 100644 test/website_api/use_priority_reason17create mode 100644 test/website_api/use_priority_reason
19create mode 100644 test/website_api/use_priority_reason.json18create mode 100644 test/website_api/use_priority_reason.json
20create mode 100644 test/website_api/use_priority_reason_plus_notes19create mode 100644 test/website_api/use_priority_reason_plus_notes
21create mode 100644 test/website_api/use_priority_reason_plus_notes.json20create mode 100644 test/website_api/use_priority_reason_plus_notes.json
2221
diff --git a/test/website_api/use_priority_reason b/test/website_api/use_priority_reason
23new file mode 10064422new file mode 100644
index 00000000000..1d542f4e061
--- /dev/null
+++ b/test/website_api/use_priority_reason
@@ -0,0 +1,26 @@
1PublicDateAtUSN: 2020-08-04 17:00:00 UTC
2Candidate: CVE-2020-1234
3CRD: 2020-08-04 17:00:00 UTC
4PublicDate: 2020-08-04 17:00:00 UTC
5References:
6 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234
7Description:
8 Publish CVEs to Website API tests
9Ubuntu-Description:
10Notes:
11Mitigation:
12Bugs:
13Priority: low
14 This is the reason the priority of this CVE is low. This cve is
15 priority low for a very good reason.
16Discovered-by:
17Assigned-to:
18CVSS:
19
20
21Patches_package:
22upstream_package: needs-triage
23trusty_package: released (1.2.3)
24trusty/esm_package: not-affected (1.2.3)
25jammy_package: released (4.5.6)
26esm-apps/jammy_package: not-affected (4.5.6)
diff --git a/test/website_api/use_priority_reason.json b/test/website_api/use_priority_reason.json
0new file mode 10064427new file mode 100644
index 00000000000..f0fdf4b9f76
--- /dev/null
+++ b/test/website_api/use_priority_reason.json
@@ -0,0 +1,56 @@
1[
2 {
3 "id": "CVE-2020-1234",
4 "description": "\nPublish CVEs to Website API tests",
5 "ubuntu_description": "",
6 "mitigation": "",
7 "notes": [
8 {
9 "author": "",
10 "note": "Priority reason:\nThis is the reason the priority of this CVE is low. This cve is priority low for a very good reason."
11 }
12 ],
13 "priority": "low",
14 "cvss3": null,
15 "references": [
16 "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234"
17 ],
18 "bugs": [
19 ""
20 ],
21 "packages": [
22 {
23 "name": "package",
24 "source": "https://launchpad.net/ubuntu/+source/package",
25 "ubuntu": "https://packages.ubuntu.com/search?suite=all&section=all&arch=any&searchon=sourcenames&keywords=package",
26 "debian": "https://tracker.debian.org/pkg/package",
27 "statuses": [
28 {
29 "release_codename": "trusty",
30 "status": "released",
31 "description": "1.2.3",
32 "pocket": "security"
33 },
34 {
35 "release_codename": "jammy",
36 "status": "released",
37 "description": "4.5.6",
38 "pocket": "security"
39 },
40 {
41 "release_codename": "upstream",
42 "status": "needs-triage",
43 "description": "",
44 "pocket": "security"
45 }
46 ]
47 }
48 ],
49 "status": "active",
50 "tags": {},
51 "patches": {
52 "package": []
53 },
54 "published": "2020-08-04 17:00:00 UTC"
55 }
56]
diff --git a/test/website_api/use_priority_reason_plus_notes b/test/website_api/use_priority_reason_plus_notes
0new file mode 10064457new file mode 100644
index 00000000000..41e6cc83b7c
--- /dev/null
+++ b/test/website_api/use_priority_reason_plus_notes
@@ -0,0 +1,29 @@
1PublicDateAtUSN: 2020-08-04 17:00:00 UTC
2Candidate: CVE-2020-1234
3CRD: 2020-08-04 17:00:00 UTC
4PublicDate: 2020-08-04 17:00:00 UTC
5References:
6 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234
7Description:
8 Publish CVEs to Website API tests
9Ubuntu-Description:
10Notes:
11 pfsmorigo> this is a note
12 sbeattie> this is another note. It contains for too many words for a
13 note. Why are there so many words in this note?
14Mitigation:
15Bugs:
16Priority: low
17 This is the reason the priority of this CVE is low. This cve is
18 priority low for a very good reason.
19Discovered-by:
20Assigned-to:
21CVSS:
22
23
24Patches_package:
25upstream_package: needs-triage
26trusty_package: released (1.2.3)
27trusty/esm_package: not-affected (1.2.3)
28jammy_package: released (4.5.6)
29esm-apps/jammy_package: not-affected (4.5.6)
diff --git a/test/website_api/use_priority_reason_plus_notes.json b/test/website_api/use_priority_reason_plus_notes.json
0new file mode 10064430new file mode 100644
index 00000000000..6e4a5d06d46
--- /dev/null
+++ b/test/website_api/use_priority_reason_plus_notes.json
@@ -0,0 +1,64 @@
0- 1[
2 {
3 "id": "CVE-2020-1234",
4 "description": "\nPublish CVEs to Website API tests",
5 "ubuntu_description": "",
6 "mitigation": "",
7 "notes": [
8 {
9 "author": "",
10 "note": "Priority reason:\nThis is the reason the priority of this CVE is low. This cve is priority low for a very good reason."
11 },
12 {
13 "author": "pfsmorigo",
14 "note": "this is a note"
15 },
16 {
17 "author": "sbeattie",
18 "note": "this is another note. It contains for too many words for a\nnote. Why are there so many words in this note?"
19 }
20 ],
21 "priority": "low",
22 "cvss3": null,
23 "references": [
24 "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1234"
25 ],
26 "bugs": [
27 ""
28 ],
29 "packages": [
30 {
31 "name": "package",
32 "source": "https://launchpad.net/ubuntu/+source/package",
33 "ubuntu": "https://packages.ubuntu.com/search?suite=all&section=all&arch=any&searchon=sourcenames&keywords=package",
34 "debian": "https://tracker.debian.org/pkg/package",
35 "statuses": [
36 {
37 "release_codename": "trusty",
38 "status": "released",
39 "description": "1.2.3",
40 "pocket": "security"
41 },
42 {
43 "release_codename": "jammy",
44 "status": "released",
45 "description": "4.5.6",
46 "pocket": "security"
47 },
48 {
49 "release_codename": "upstream",
50 "status": "needs-triage",
51 "description": "",
52 "pocket": "security"
53 }
54 ]
55 }
56 ],
57 "status": "active",
58 "tags": {},
59 "patches": {
60 "package": []
61 },
62 "published": "2020-08-04 17:00:00 UTC"
63 }
64]
12.40.1652.40.1
Revision history for this message
Steve Beattie (sbeattie) wrote :

On Thu, Aug 10, 2023 at 08:35:58PM -0000, Emilia Torino wrote:
> Leaving one question just in case, otherwise LGTM!
>
> Diff comments:
>
> > diff --git a/scripts/publish-cves-to-website-api.py b/scripts/publish-cves-to-website-api.py
> > index d6af0e7..d5a0042 100755
> > --- a/scripts/publish-cves-to-website-api.py
> > +++ b/scripts/publish-cves-to-website-api.py
> > @@ -163,6 +163,10 @@ def post_single_cve(cve_filename):
> >
> > notes = []
> >
> > + # TODO Remove this when we have the proper field ir ready
>
> s/ir/is? Seems a typo :), also if thats the case maybe "we have" should be removed? So either remove everything after field, or remove "we have" I guess
>
> > + if cve_data["Priority"][1]:
>
> Should we check cve_data["Priority"] length before accessing the reason at index 1?

In hindsight, it would have been useful to have hidden the accessors behind an
api in cve_lib, something like

  cve_lib.cve_get_priority(cve)
  cve_lib.cve_get_priority_reason(cve)

so that we can move to a future where the internal structures of cves
are hidden in a class. That said, I don't know how to make the api
export the case where we have a different priority for a specific
package or release.

> > + notes.append({"author": "", "note": "Priority reason:\n" + cve_data["Priority"][1]})
> > +
> > for [author, note] in cve_data["Notes"]:
> > notes.append({"author": author, "note": note})
> >
>
>
> --
> https://code.launchpad.net/~pfsmorigo/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448821
> You are subscribed to branch ubuntu-cve-tracker:master.
>

--
Steve Beattie
<email address hidden>

Revision history for this message
Steve Beattie (sbeattie) wrote :

Wow, emailing a patch attachment really confused the merge request interface.

Anyway, my attempts to create testcases resulted in the following merge request:
https://code.launchpad.net/~sbeattie/ubuntu-cve-tracker/+git/ubuntu-cve-tracker/+merge/448958

Revision history for this message
Paulo Flabiano Smorigo (pfsmorigo) wrote :

> Leaving one question just in case, otherwise LGTM!

Fixed the mistypo and added a check as you suggested. I added it anyway but, to be honest, the check is not really necessary since the element is always a tuplet with two elements:

>>> cveinfo["CVE-2019-11840"]["Priority"]
['medium', '']
>>> cveinfo["CVE-2019-11840"]["Priority_snapd"]
['negligible', '']
>>> cveinfo["CVE-2007-6752"]["Priority"]
['low', '']

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/scripts/publish-cves-to-website-api.py b/scripts/publish-cves-to-website-api.py
index d6af0e7..4bb95c7 100755
--- a/scripts/publish-cves-to-website-api.py
+++ b/scripts/publish-cves-to-website-api.py
@@ -163,6 +163,10 @@ def post_single_cve(cve_filename):
163163
164 notes = []164 notes = []
165165
166 # TODO Remove this when we have the proper field is ready
167 if len(cve_data["Priority"]) > 1 and cve_data["Priority"][1]:
168 notes.append({"author": "", "note": "Priority reason:\n" + cve_data["Priority"][1]})
169
166 for [author, note] in cve_data["Notes"]:170 for [author, note] in cve_data["Notes"]:
167 notes.append({"author": author, "note": note})171 notes.append({"author": author, "note": note})
168172

Subscribers

People subscribed via source and target branches