launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32436
[Merge] ~enriqueesanchz/launchpad:add-vulnerability_patches into launchpad:master
Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-vulnerability_patches into launchpad:master.
Commit message:
Add bugattachment.vulnerability_patches support
Use one row per package and vulnerability_patches JSON column instead of
a bugattachment row for each patch. Now we mantain patches order.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/485293
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-vulnerability_patches into launchpad:master.
diff --git a/lib/lp/bugs/interfaces/bugattachment.py b/lib/lp/bugs/interfaces/bugattachment.py
index 4edb5de..8410bd7 100644
--- a/lib/lp/bugs/interfaces/bugattachment.py
+++ b/lib/lp/bugs/interfaces/bugattachment.py
@@ -22,7 +22,7 @@ from lazr.restful.declarations import (
)
from lazr.restful.fields import Reference
from zope.interface import Interface
-from zope.schema import URI, Bool, Bytes, Choice, Int, TextLine
+from zope.schema import URI, Bool, Bytes, Choice, Int, List, TextLine
from lp import _
from lp.bugs.interfaces.hasbug import IHasBug
@@ -118,6 +118,9 @@ class IBugAttachmentView(IHasBug):
),
readonly=True,
)
+ vulnerability_patches = List(
+ title=_("Vulnerability patches"), readonly=True
+ )
def getFileByName(filename):
"""Return the `ILibraryFileAlias` for the given file name.
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index 989abe8..2ebaad5 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -27,6 +27,7 @@ from email.utils import make_msgid
from functools import wraps
from io import BytesIO
from itertools import chain
+from typing import List
from lazr.lifecycle.event import ObjectCreatedEvent
from lazr.lifecycle.snapshot import Snapshot
@@ -1634,6 +1635,7 @@ class Bug(StormBase, InformationTypeMixin):
content_type=None,
description=None,
from_api=False,
+ vulnerability_patches: List[dict] = None,
):
"""See `IBug`."""
# XXX: StevenK 2013-02-06 bug=1116954: We should not need to refetch
@@ -1673,6 +1675,7 @@ class Bug(StormBase, InformationTypeMixin):
comment=comment,
is_patch=is_patch,
description=description,
+ vulnerability_patches=vulnerability_patches,
)
def linkAttachment(
@@ -1684,6 +1687,7 @@ class Bug(StormBase, InformationTypeMixin):
is_patch=False,
description=None,
send_notifications=True,
+ vulnerability_patches: List[dict] = None,
):
"""See `IBug`.
@@ -1717,6 +1721,7 @@ class Bug(StormBase, InformationTypeMixin):
title=description,
message=message,
send_notifications=send_notifications,
+ vulnerability_patches=vulnerability_patches,
)
def linkBranch(self, branch, registrant):
diff --git a/lib/lp/bugs/model/bugattachment.py b/lib/lp/bugs/model/bugattachment.py
index 064482a..896a956 100644
--- a/lib/lp/bugs/model/bugattachment.py
+++ b/lib/lp/bugs/model/bugattachment.py
@@ -3,7 +3,10 @@
__all__ = ["BugAttachment", "BugAttachmentSet"]
+from typing import List
+
from lazr.lifecycle.event import ObjectCreatedEvent, ObjectDeletedEvent
+from storm.databases.postgres import JSON
from storm.locals import Int, Reference, Store, Unicode
from zope.event import notify
from zope.interface import implementer
@@ -42,6 +45,7 @@ class BugAttachment(StormBase):
url = Unicode(allow_none=True)
_message_id = Int(name="message", allow_none=False)
_message = Reference(_message_id, "Message.id")
+ vulnerability_patches = JSON(allow_none=True)
def __init__(
self,
@@ -51,6 +55,7 @@ class BugAttachment(StormBase):
url,
message,
type=IBugAttachment["type"].default,
+ vulnerability_patches: List[dict] = None,
):
super().__init__()
self.bug = bug
@@ -59,6 +64,7 @@ class BugAttachment(StormBase):
self.url = url
self._message = message
self.type = type
+ self.vulnerability_patches = vulnerability_patches
@property
def title(self) -> str:
@@ -139,13 +145,20 @@ class BugAttachmentSet:
message,
attach_type=None,
send_notifications=False,
+ vulnerability_patches: List[dict] = None,
):
"""See `IBugAttachmentSet`."""
- if not filealias and not url:
- raise ValueError("Either filealias or url must be provided")
-
- if filealias and url:
- raise ValueError("Only one of filealias or url may be provided")
+ if not filealias and not url and not vulnerability_patches:
+ raise ValueError(
+ "Either filealias, url or vulnerability_patches "
+ "must be provided"
+ )
+
+ if filealias and url and vulnerability_patches:
+ raise ValueError(
+ "Only one of filealias, url or vulnerability_patches may be "
+ "provided"
+ )
if attach_type is None:
# XXX kiko 2005-08-03 bug=1659: this should use DEFAULT.
@@ -157,6 +170,7 @@ class BugAttachmentSet:
type=attach_type,
title=title,
message=message,
+ vulnerability_patches=vulnerability_patches,
)
# canonial_url(attachment) (called by notification subscribers
# to generate the download URL of the attachments) blows up if
diff --git a/lib/lp/bugs/scripts/tests/test_uct.py b/lib/lp/bugs/scripts/tests/test_uct.py
index d2242d5..2fe5eee 100644
--- a/lib/lp/bugs/scripts/tests/test_uct.py
+++ b/lib/lp/bugs/scripts/tests/test_uct.py
@@ -722,8 +722,8 @@ class TestUCTImporterExporter(TestCaseWithFactory):
),
package_name=self.esm_package.sourcepackagename,
importance=None,
- status=BugTaskStatus.WONTFIX,
- status_explanation="ignored",
+ status=BugTaskStatus.FIXRELEASED,
+ status_explanation="fix released",
),
CVE.SeriesPackage(
target=SourcePackage(
@@ -748,8 +748,8 @@ class TestUCTImporterExporter(TestCaseWithFactory):
target=self.product_2,
package_name=self.esm_package.sourcepackagename,
importance=BugTaskImportance.LOW,
- status=BugTaskStatus.WONTFIX,
- status_explanation="ignored",
+ status=BugTaskStatus.FIXRELEASED,
+ status_explanation="fix released",
),
],
importance=BugTaskImportance.MEDIUM,
@@ -784,6 +784,12 @@ class TestUCTImporterExporter(TestCaseWithFactory):
url="https://github.com/389ds/389-ds-base/"; "commit/456",
notes=None,
),
+ CVE.PatchURL(
+ package_name=self.esm_package.sourcepackagename,
+ type="upstream",
+ url="https://github.com/vim/vim/"; "commit/188",
+ notes=None,
+ ),
],
global_tags={"cisa-kev"},
)
@@ -871,18 +877,53 @@ class TestUCTImporterExporter(TestCaseWithFactory):
self.assertEqual(cve.assignee, t.assignee)
def checkBugAttachments(self, bug: Bug, cve: CVE):
- attachments_by_url = {a.url: a for a in bug.attachments if a.url}
+ # attachment.title is the package name
+ attachments_by_pkg = {att.title: att for att in bug.attachments}
+ patch_url_by_pkg = defaultdict(list)
for patch_url in cve.patch_urls:
- self.assertIn(patch_url.url, attachments_by_url)
- attachment = attachments_by_url[patch_url.url]
- expected_title = "{}/{}".format(
- patch_url.package_name.name, patch_url.type
+ patch_url_by_pkg[patch_url.package_name.name].append(patch_url)
+
+ self.assertEqual(
+ len(attachments_by_pkg),
+ len(patch_url_by_pkg),
+ "Mismatch in attachment count and patch URL count",
+ )
+
+ for pkg, patch_urls in patch_url_by_pkg.items():
+ attachment = attachments_by_pkg.get(pkg)
+ self.assertIsNotNone(
+ attachment, f"Attachment for package '{pkg}' not found"
)
- if patch_url.notes:
- expected_title = "{}/{}".format(
- expected_title, patch_url.notes
+
+ self.assertEqual(pkg, attachment.title)
+
+ vulnerability_patches = attachment.vulnerability_patches
+ self.assertEqual(
+ len(patch_urls),
+ len(vulnerability_patches),
+ "Number of patches and vulnerabilities don't match for "
+ f"package '{pkg}'",
+ )
+
+ # Check content and its order
+ for patch_url, vulnerability_patch in zip(
+ patch_urls, vulnerability_patches
+ ):
+ self.assertEqual(
+ patch_url.type,
+ vulnerability_patch["name"],
+ f"Type mismatch for patch in package '{pkg}'",
+ )
+ self.assertEqual(
+ patch_url.url,
+ vulnerability_patch["value"],
+ f"URL mismatch for patch in package '{pkg}'",
+ )
+ self.assertEqual(
+ patch_url.notes,
+ vulnerability_patch["comment"],
+ f"Notes mismatch for patch in package '{pkg}'",
)
- self.assertEqual(expected_title, attachment.title)
def checkVulnerabilities(self, bug: Bug, cve: CVE):
vulnerabilities = bug.vulnerabilities
@@ -1312,6 +1353,14 @@ class TestUCTImporterExporter(TestCaseWithFactory):
notes=None,
)
)
+ cve.patch_urls.append(
+ CVE.PatchURL(
+ package_name=cve.distro_packages[1].package_name,
+ type="upstream",
+ url="https://github.com/012";,
+ notes=None,
+ )
+ )
self.importer.update_bug(bug, cve, self.lp_cve)
self.checkBug(bug, cve)
diff --git a/lib/lp/bugs/scripts/uct/uctexport.py b/lib/lp/bugs/scripts/uct/uctexport.py
index 937860f..8fc72aa 100644
--- a/lib/lp/bugs/scripts/uct/uctexport.py
+++ b/lib/lp/bugs/scripts/uct/uctexport.py
@@ -2,7 +2,6 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
import logging
-import re
from collections import defaultdict
from pathlib import Path
from typing import Dict, List, NamedTuple, Optional
@@ -42,13 +41,6 @@ class UCTExporter:
description: str
references: List[str]
- # Example:
- # linux/upstream
- # linux/upstream/5.19.1
- BUG_ATTACHMENT_TITLE_RE = re.compile(
- "^(?P<package_name>.+?)/(?P<patch_type>.+?)(/(?P<notes>.+?))?$"
- )
-
def export_bug_to_uct_file(
self, bug_id: int, output_dir: Path
) -> Optional[Path]:
@@ -203,24 +195,20 @@ class UCTExporter:
patch_urls = []
for attachment in bug.attachments:
if (
- not attachment.url
+ not attachment.vulnerability_patches
or not attachment.type == BugAttachmentType.PATCH
):
continue
- title_match = self.BUG_ATTACHMENT_TITLE_RE.match(attachment.title)
- if not title_match:
- continue
- spn = packages_by_name.get(title_match.groupdict()["package_name"])
- if not spn:
- continue
- patch_urls.append(
- CVE.PatchURL(
- package_name=spn,
- type=title_match.groupdict()["patch_type"],
- url=attachment.url,
- notes=title_match.groupdict().get("notes"),
+
+ for patch in attachment.vulnerability_patches:
+ patch_urls.append(
+ CVE.PatchURL(
+ package_name=packages_by_name.get(attachment.title),
+ type=patch["name"],
+ url=patch["value"],
+ notes=patch["comment"],
+ )
)
- )
return CVE(
sequence=f"CVE-{lp_cve.sequence}",
diff --git a/lib/lp/bugs/scripts/uct/uctimport.py b/lib/lp/bugs/scripts/uct/uctimport.py
index e9f1dde..4a04510 100644
--- a/lib/lp/bugs/scripts/uct/uctimport.py
+++ b/lib/lp/bugs/scripts/uct/uctimport.py
@@ -25,6 +25,7 @@ Three types of bug tags are created:
status of the package in upstream.
"""
import logging
+from collections import defaultdict
from datetime import timezone
from itertools import chain
from pathlib import Path
@@ -446,26 +447,41 @@ class UCTImporter:
bug_watch_set.fromText(external_bug_url, bug, self.bug_importer)
def _update_patches(self, bug: BugModel, patch_urls: List[CVE.PatchURL]):
- attachments_by_url = {a.url: a for a in bug.attachments if a.url}
+ if not patch_urls:
+ return
+
+ attachments_by_pkg = {att.title: att for att in bug.attachments}
+
+ vulnerability_patches_by_pkg = defaultdict(list)
for patch_url in patch_urls:
- title = f"{patch_url.package_name.name}/{patch_url.type}"
- if patch_url.notes:
- title = f"{title}/{patch_url.notes}"
- if patch_url in attachments_by_url:
- attachment = removeSecurityProxy(
- attachments_by_url[patch_url.url]
- )
- attachment.title = title
+ vulnerability_patches_by_pkg[patch_url.package_name.name].append(
+ {
+ "name": patch_url.type,
+ "value": patch_url.url,
+ "comment": patch_url.notes,
+ }
+ )
+
+ for (
+ name,
+ vulnerability_patches,
+ ) in vulnerability_patches_by_pkg.items():
+ attachment = attachments_by_pkg.get(name)
+
+ if attachment:
+ attachment = removeSecurityProxy(attachment)
attachment.type = BugAttachmentType.PATCH
+ attachment.vulnerability_patches = vulnerability_patches
else:
bug.addAttachment(
owner=bug.owner,
data=None,
comment=None,
filename=None,
- url=patch_url.url,
+ url="REMOVE",
is_patch=True,
- description=title,
+ description=name,
+ vulnerability_patches=vulnerability_patches,
)
def _make_bug_description(self, cve: CVE) -> str:
