launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+494096@xxxxxxxxxxxxxxxxxx>]

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:fix-update-cve into launchpad:master.

Commit message:
Fix cveimport update metadata key
    
When updating a key value as `cve.metadata["affected"]` we were getting
`Forbidden` exception. Now we update the entire `cve.metadata`.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/494096

When giving permissions to modify a JSONB/dict field, you don't get permissions to modify it in place. You need to change the entire `cve.metadata = new_value` instead of doing `cve.metadata['affected'] = new_value`.

This error was hidden in the tests.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:fix-update-cve into launchpad:master.

diff --git a/lib/lp/bugs/model/cve.py b/lib/lp/bugs/model/cve.py
index 19f3ed2..77cef96 100644
--- a/lib/lp/bugs/model/cve.py
+++ b/lib/lp/bugs/model/cve.py
@@ -94,6 +94,7 @@ class Cve(StormBase, BugLinkTargetMixin):
         self.date_made_public = date_made_public
         self.discovered_by = discovered_by
         self._cvss = cvss
+        self.metadata = metadata
 
     @property
     def url(self):
@@ -213,6 +214,7 @@ class CveSet:
         date_made_public=None,
         discovered_by=None,
         cvss=None,
+        metadata=None,
     ):
         """See ICveSet."""
         cve = Cve(
@@ -222,6 +224,7 @@ class CveSet:
             date_made_public=date_made_public,
             discovered_by=discovered_by,
             cvss=cvss,
+            metadata=metadata,
         )
 
         IStore(Cve).add(cve)
diff --git a/lib/lp/bugs/scripts/cveimport.py b/lib/lp/bugs/scripts/cveimport.py
index a1c27d5..e019d07 100644
--- a/lib/lp/bugs/scripts/cveimport.py
+++ b/lib/lp/bugs/scripts/cveimport.py
@@ -723,10 +723,11 @@ class CVEUpdater(LaunchpadCronScript):
             modified = True
 
         # handle affected
-        metadata = cve.metadata or {}
-        if metadata.get("affected", {}) != affected:
-            metadata["affected"] = affected
-            cve.metadata = metadata
+        if (
+            not isinstance(cve.metadata, dict)
+            or cve.metadata.get("affected", {}) != affected
+        ):
+            cve.metadata = {"affected": affected}
             modified = True
 
         if modified:
diff --git a/lib/lp/bugs/scripts/tests/test_cveimport.py b/lib/lp/bugs/scripts/tests/test_cveimport.py
index 63fd5b0..0ca501d 100644
--- a/lib/lp/bugs/scripts/tests/test_cveimport.py
+++ b/lib/lp/bugs/scripts/tests/test_cveimport.py
@@ -263,11 +263,24 @@ class TestCVEUpdater(TestCase):
         """Test updating an existing CVE with new data."""
         # First create a CVE
         original_desc = "Original description"
+        original_metadata = {
+            "affected": [
+                {
+                    "vendor": "original vendor",
+                    "product": "original product",
+                }
+            ],
+        }
         cveset = getUtility(ICveSet)
 
         # Create initial CVE using a properly initialized updater
         updater = self.make_updater()
-        cveset.new("2024-0004", original_desc, CveStatus.ENTRY)
+        cveset.new(
+            "2024-0004",
+            original_desc,
+            CveStatus.ENTRY,
+            metadata=original_metadata,
+        )
         updater.txn.commit()
 
         # Create updated data
diff --git a/lib/lp/bugs/tests/test_cve.py b/lib/lp/bugs/tests/test_cve.py
index d2eeb3b..3abf98d 100644
--- a/lib/lp/bugs/tests/test_cve.py
+++ b/lib/lp/bugs/tests/test_cve.py
@@ -520,12 +520,14 @@ class TestCve(TestCaseWithFactory):
                 date_made_public=None,
                 discovered_by=None,
                 cvss={},
+                metadata=None,
             ),
         )
 
     def test_cveset_new_method_parameters(self):
         today = datetime.now(tz=timezone.utc)
         cvss = {"nvd": ["CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"]}
+        metadata = {"example key": "example value"}
         cve = getUtility(ICveSet).new(
             sequence="2099-1234",
             description="A critical vulnerability",
@@ -533,6 +535,7 @@ class TestCve(TestCaseWithFactory):
             date_made_public=today,
             discovered_by="A person",
             cvss=cvss,
+            metadata=metadata,
         )
         self.assertThat(
             cve,
@@ -543,6 +546,7 @@ class TestCve(TestCaseWithFactory):
                 date_made_public=today,
                 discovered_by="A person",
                 cvss=cvss,
+                metadata=metadata,
             ),
         )