Merge ~emitorino/review-tools:fix_usn_notfication_service_using_dpkg_list into review-tools:master

[Emilia Torino]

This is a MP to fix https://bugs.launchpad.net/review-tools/+bug/1930106 and https://bugs.launchpad.net/review-tools/+bug/1930105.

- store.py: consider faked-by-review-tools-dpkg part when checking if manifest_has_primed_staged_section
- available.py: considering binary names could contain arch specifier when parsing dpkg.list

[Alex Murray (alexmurray)]

LGTM! Thanks Emi

review: Approve

diff --git a/reviewtools/available.py b/reviewtools/available.py
index fa52cae..e43960b 100644
--- a/reviewtools/available.py
+++ b/reviewtools/available.py
@@ -416,9 +416,9 @@ def _update_seen(seen_fn, seen_db, pkg_db):
 # rocks
 def scan_store(secnot_db_fn, store_db_fn, seen_db_fn, pkgname, store_db_type="snap"):
     """For each entry in store db (either snap or rock), see if there are any
-       binary packages with security notices, if see report them if not in the
-       seen db. We perform these actions on each snap and rock and do not form
-       a queue to keep the implementation simple.
+    binary packages with security notices, if see report them if not in the
+    seen db. We perform these actions on each snap and rock and do not form
+    a queue to keep the implementation simple.
     """
     secnot_db = read_usn_db(secnot_db_fn)
     store_db = read_file_as_json_dict(store_db_fn)
@@ -483,8 +483,16 @@ def scan_snap(secnot_db_fn, snap_fn, with_cves=False):
                 if not line.startswith("ii "):
                     continue
                 tmp = line.split()
+                pkg_name = tmp[1]
+                # Since dpkg 1.16.2, the binary name could include and arch qualifier: e.g liblz4-1:amd64
+                # For now we assume that if the colon is present, we are in a situation where the binary name is
+                # including the arch. We could assert valid architectures as well but that means this code should be
+                # updated as new arch are supported or even existing ones are removed.
+                arch_qualifier_parts = pkg_name.split(":")
+                if len(arch_qualifier_parts) == 2:
+                    pkg_name = arch_qualifier_parts[0]
                 man["parts"][fake_key]["stage-packages"].append(
-                    "%s=%s" % (tmp[1], tmp[2])
+                    "%s=%s" % (pkg_name, tmp[2])
                 )
 
     secnot_db = read_usn_db(secnot_db_fn)
@@ -526,7 +534,7 @@ def scan_rock(secnot_db_fn, rock_fn, with_cves=False):
 
 def scan_shared_publishers(store_fn):
     """Check store db for any snaps with a shared email that don't also have a
-       mapping.
+    mapping.
     """
     store_db = read_file_as_json_dict(store_fn)
     report = get_shared_snap_without_override(store_db)
diff --git a/reviewtools/store.py b/reviewtools/store.py
index 7cda73a..3e27b4e 100644
--- a/reviewtools/store.py
+++ b/reviewtools/store.py
@@ -325,7 +325,7 @@ def get_pkg_revisions(item, secnot_db, errors, pkg_type="snap"):
 
 def get_shared_snap_without_override(store_db):
     """Report snaps that use a shared email but don't have an entry for
-       additional addresses.
+    additional addresses.
     """
     missing = {}
     for item in store_db:
@@ -351,7 +351,7 @@ def get_shared_snap_without_override(store_db):
 # TODO: support for build-packages is not fully implemented yet.
 def get_staged_and_build_packages_from_manifest(m):
     """Obtain list of packages in primed-stage-packages if section is present.
-       If not, obtain it from stage-packages for various parts instead
+    If not, obtain it from stage-packages for various parts instead
     """
     if "parts" not in m:
         debug("Could not find 'parts' in manifest")
@@ -366,14 +366,20 @@ def get_staged_and_build_packages_from_manifest(m):
     manifest_has_primed_staged_section = False
 
     if "primed-stage-packages" in m and m["primed-stage-packages"] is not None:
+        # An empty primed-stage-packages section (i.e. primed-stage-packages: [])
+        # is consider valid as well.
         manifest_has_primed_staged_section = True
         # Note, prime-stage-packages is grouped with stage-packages
         get_packages_from_manifest_section(d, m["primed-stage-packages"], "staged")
 
     for part in m["parts"]:
-        # stage-packages part is only analyzed if primed-stage-packages is not
-        # present.
-        if not manifest_has_primed_staged_section:
+        # stage-packages in each part is only analyzed if primed-stage-packages
+        # is not present. The only exception is if the snap includes the
+        # dpkg.list file (e.g. core snaps) since primed-stage-packages is always empty
+        if (
+            not manifest_has_primed_staged_section
+            or part == "faked-by-review-tools-dpkg"
+        ):
             if (
                 "stage-packages" in m["parts"][part]
                 and m["parts"][part]["stage-packages"] is not None
@@ -401,8 +407,7 @@ def get_staged_and_build_packages_from_manifest(m):
 
 
 def get_staged_packages_from_rock_manifest(m):
-    """Obtain list of packages in stage-packages if section is present.
-    """
+    """Obtain list of packages in stage-packages if section is present."""
     if "stage-packages" not in m:
         debug("Could not find 'stage-packages' in manifest")
         return None
@@ -426,7 +431,7 @@ def get_staged_packages_from_rock_manifest(m):
 # key.
 def get_packages_from_manifest_section(d, manifest_section, package_type):
     """Obtain packages from a given manifest section (primed-stage-packages
-       or stage-packages along with any build-packages for a given part)
+    or stage-packages along with any build-packages for a given part)
     """
     for entry in manifest_section:
         if "=" not in entry:
@@ -451,7 +456,7 @@ def get_packages_from_manifest_section(d, manifest_section, package_type):
 # snaps, git trees)
 def get_packages_from_rock_manifest_section(d, manifest_section, package_type):
     """Obtain packages from a given manifest section (rock manifest v1 only
-       has staged-packages)
+    has staged-packages)
     """
     for entry in manifest_section:
         # rock manifest v1 stage-packages section is a list of
@@ -486,8 +491,8 @@ def get_packages_from_rock_manifest_section(d, manifest_section, package_type):
 
 def normalize_and_verify_snap_manifest(m):
     """Normalize manifest (ie, assign empty types if None for SafeLoader
-       defaults) and verify snap manifest is well-formed and has everything we
-       expect"""
+    defaults) and verify snap manifest is well-formed and has everything we
+    expect"""
     # normalize toplevel keys
     assign_type_to_dict_values(m, SnapReview.snap_manifest_required)
     assign_type_to_dict_values(m, SnapReview.snap_manifest_optional)
@@ -507,9 +512,9 @@ def normalize_and_verify_snap_manifest(m):
 
 def normalize_and_verify_rock_manifest(m):
     """Normalize manifest (ie, assign empty types if None for SafeLoader
-       defaults) and verify rock manifest is well-formed and has everything we
-       expect in this initial implementation.
-       TODO: Update once rock manifest is properly implemented"""
+    defaults) and verify rock manifest is well-formed and has everything we
+    expect in this initial implementation.
+    TODO: Update once rock manifest is properly implemented"""
     # normalize toplevel keys
     assign_type_to_dict_values(m, RockReview.rock_manifest_required)
     assign_type_to_dict_values(m, RockReview.rock_manifest_optional)
diff --git a/reviewtools/tests/test_available.py b/reviewtools/tests/test_available.py
index 08e73de..a9ce71e 100644
--- a/reviewtools/tests/test_available.py
+++ b/reviewtools/tests/test_available.py
@@ -568,7 +568,7 @@ each snap revision
 
     def test_check__secnot_report_for_kernel_stage_and_build_pkg_new_secnot(self):
         """Test _secnot_report_for_pkg() - new secnot for build and
-            staged pkg"""
+        staged pkg"""
         errors = {}
         self.pkg_db = store.get_pkg_revisions(
             self.kernel_store_db[0], self.secnot_kernel_and_build_pkgs_db, errors
@@ -912,6 +912,10 @@ Revision r12 (i386; channels: candidate, beta)
         snap_fn = "./tests/test-core_16-2.37.2_amd64.snap"
         res = available.scan_snap(self.secnot_core_with_dpkg_list_fn, snap_fn)
         self.assertTrue(len(res) > 0)
+        # This asserts a binary was obtained from the URLs since its not listed in the USN binaries keys
+        self.assertIn("libc-bin", res)
+        # This asserts the dpkg-query file was properly parsed and arch qualifiers were ignored LP: #1930105
+        self.assertIn("libc6", res)
         self.assertIn("3323-1", res)
 
     def test_check_scan_snap_dpkg_list_app(self):
@@ -1063,7 +1067,7 @@ Revision r12 (i386; channels: candidate, beta)
 
     def test_check_scan_store_with_pkgname_bad_publisher(self):
         """Test scan_store() - with pkgname and bad publisher - snaps and
-           rocks
+        rocks
         """
         store_dbs = {
             "snap": ["./tests/test-store-unittest-bad-1.db", "1ad"],
@@ -1128,7 +1132,7 @@ Revision r12 (i386; channels: candidate, beta)
         self,
     ):
         """Test scan_store() - kernel snap and build pkg update but invalid
-           snapcraft version"""
+        snapcraft version"""
         store_fn = "./tests/test-store-kernel-invalid-snapcraft-version.db"
         (sent, errors) = available.scan_store(
             self.secnot_build_pkgs_only_fn, store_fn, None, None
diff --git a/tests/test-core-with-primed-staged_16-2.37.2_amd64.snap b/tests/test-core-with-primed-staged_16-2.37.2_amd64.snap
new file mode 100644
index 0000000..dc17a96
Binary files /dev/null and b/tests/test-core-with-primed-staged_16-2.37.2_amd64.snap differ
diff --git a/tests/test-updates-available.sh b/tests/test-updates-available.sh
index fc90e53..05fea65 100755
--- a/tests/test-updates-available.sh
+++ b/tests/test-updates-available.sh
@@ -242,6 +242,15 @@ echo "Running: snap-updates-available --with-cves --usn-db='./tests/test-usn-cor
 PYTHONPATH=./ ./bin/snap-updates-available --with-cves --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-core_16-2.37.2_amd64.snap' 2>&1 | tee -a "$tmp"
 echo "" | tee -a "$tmp"
 
+# testing core snap with manifest including primed-stage-packages: [] LP: #1930106
+echo "Running: snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-core-with-primed-staged_16-2.37.2_amd64.snap'" | tee -a "$tmp"
+PYTHONPATH=./ ./bin/snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./test-core-with-primed-staged_16-2.37.2_amd64.snap' 2>&1 | tee -a "$tmp"
+echo "" | tee -a "$tmp"
+
+echo "Running: snap-updates-available --with-cves --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-core-with-primed-staged_16-2.37.2_amd64.snap'" | tee -a "$tmp"
+PYTHONPATH=./ ./bin/snap-updates-available --with-cves --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-core-with-primed-staged_16-2.37.2_amd64.snap' 2>&1 | tee -a "$tmp"
+echo "" | tee -a "$tmp"
+
 echo "Running: snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-dpkg-list-app_1.0_amd64.snap'" | tee -a "$tmp"
 PYTHONPATH=./ ./bin/snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-dpkg-list-app_1.0_amd64.snap' 2>&1 | tee -a "$tmp"
 echo "" | tee -a "$tmp"
diff --git a/tests/test-updates-available.sh.expected b/tests/test-updates-available.sh.expected
index 9558d25..20216b9 100644
--- a/tests/test-updates-available.sh.expected
+++ b/tests/test-updates-available.sh.expected
@@ -1051,6 +1051,10 @@ Running: snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.d
     "3323-1",
     "3534-1"
   ],
+  "libc6": [
+    "3323-1",
+    "3534-1"
+  ],
   "multiarch-support": [
     "3323-1",
     "3534-1"
@@ -1073,6 +1077,69 @@ Running: snap-updates-available --with-cves --usn-db='./tests/test-usn-core-with
       "CVE-2018-1000001"
     ]
   },
+  "libc6": {
+    "3323-1": [
+      "CVE-2017-1000366"
+    ],
+    "3534-1": [
+      "CVE-2017-1000408",
+      "CVE-2017-1000409",
+      "CVE-2017-15670",
+      "CVE-2017-15804",
+      "CVE-2017-16997",
+      "CVE-2017-17426",
+      "CVE-2018-1000001"
+    ]
+  },
+  "multiarch-support": {
+    "3323-1": [
+      "CVE-2017-1000366"
+    ],
+    "3534-1": [
+      "CVE-2017-1000408",
+      "CVE-2017-1000409",
+      "CVE-2017-15670",
+      "CVE-2017-15804",
+      "CVE-2017-16997",
+      "CVE-2017-17426",
+      "CVE-2018-1000001"
+    ]
+  }
+}
+
+Running: snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-core-with-primed-staged_16-2.37.2_amd64.snap'
+ERROR: Could not find './test-core-with-primed-staged_16-2.37.2_amd64.snap'
+
+Running: snap-updates-available --with-cves --usn-db='./tests/test-usn-core-with-dpkg-list.db' --snap='./tests/test-core-with-primed-staged_16-2.37.2_amd64.snap'
+{
+  "libc-bin": {
+    "3323-1": [
+      "CVE-2017-1000366"
+    ],
+    "3534-1": [
+      "CVE-2017-1000408",
+      "CVE-2017-1000409",
+      "CVE-2017-15670",
+      "CVE-2017-15804",
+      "CVE-2017-16997",
+      "CVE-2017-17426",
+      "CVE-2018-1000001"
+    ]
+  },
+  "libc6": {
+    "3323-1": [
+      "CVE-2017-1000366"
+    ],
+    "3534-1": [
+      "CVE-2017-1000408",
+      "CVE-2017-1000409",
+      "CVE-2017-15670",
+      "CVE-2017-15804",
+      "CVE-2017-16997",
+      "CVE-2017-17426",
+      "CVE-2018-1000001"
+    ]
+  },
   "multiarch-support": {
     "3323-1": [
       "CVE-2017-1000366"
@@ -1095,6 +1162,10 @@ Running: snap-updates-available --usn-db='./tests/test-usn-core-with-dpkg-list.d
     "3323-1",
     "3534-1"
   ],
+  "libc6": [
+    "3323-1",
+    "3534-1"
+  ],
   "multiarch-support": [
     "3323-1",
     "3534-1"
@@ -1117,6 +1188,20 @@ Running: snap-updates-available --with-cves --usn-db='./tests/test-usn-core-with
       "CVE-2018-1000001"
     ]
   },
+  "libc6": {
+    "3323-1": [
+      "CVE-2017-1000366"
+    ],
+    "3534-1": [
+      "CVE-2017-1000408",
+      "CVE-2017-1000409",
+      "CVE-2017-15670",
+      "CVE-2017-15804",
+      "CVE-2017-16997",
+      "CVE-2017-17426",
+      "CVE-2018-1000001"
+    ]
+  },
   "multiarch-support": {
     "3323-1": [
       "CVE-2017-1000366"
@@ -1813,6 +1898,10 @@ Running: snap-check-notices --no-fetch ./tests/test-core_16-2.37.2_amd64.snap
         "3323-1",
         "3534-1"
       ],
+      "libc6": [
+        "3323-1",
+        "3534-1"
+      ],
       "multiarch-support": [
         "3323-1",
         "3534-1"
@@ -1839,6 +1928,20 @@ Running: snap-check-notices --no-fetch --with-cves ./tests/test-core_16-2.37.2_a
           "CVE-2018-1000001"
         ]
       },
+      "libc6": {
+        "3323-1": [
+          "CVE-2017-1000366"
+        ],
+        "3534-1": [
+          "CVE-2017-1000408",
+          "CVE-2017-1000409",
+          "CVE-2017-15670",
+          "CVE-2017-15804",
+          "CVE-2017-16997",
+          "CVE-2017-17426",
+          "CVE-2018-1000001"
+        ]
+      },
       "multiarch-support": {
         "3323-1": [
           "CVE-2017-1000366"
diff --git a/tests/test.sh.expected b/tests/test.sh.expected
index 499cd67..e79db3e 100644
--- a/tests/test.sh.expected
+++ b/tests/test.sh.expected
@@ -26011,6 +26011,269 @@ test-content_0.1_all.snap: pass
   }
 }
 
+=  test-core-with-primed-staged_16-2.37.2_amd64.snap =
+Errors
+------
+ - lint-snap-v2:snap_type_redflag
+	(NEEDS REVIEW) type 'os' not allowed
+test-core-with-primed-staged_16-2.37.2_amd64.snap: FAIL
+
+= --sdk test-core-with-primed-staged_16-2.37.2_amd64.snap =
+= snap.v2_declaration =
+{
+  "error": {},
+  "info": {},
+  "warn": {}
+}
+= snap.v2_functional =
+{
+  "error": {},
+  "info": {},
+  "warn": {}
+}
+= snap.v2_lint =
+{
+  "error": {
+    "lint-snap-v2:snap_type_redflag": {
+      "manual_review": true,
+      "text": "(NEEDS REVIEW) type 'os' not allowed"
+    }
+  },
+  "info": {
+    "lint-snap-v2:apps_present": {
+      "manual_review": false,
+      "text": "OK (optional apps field not specified)"
+    },
+    "lint-snap-v2:architecture_specified_needed:amd64": {
+      "manual_review": false,
+      "text": "Could not find compiled binaries for architecture 'amd64'"
+    },
+    "lint-snap-v2:architecture_valid": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:assumes_valid": {
+      "manual_review": false,
+      "text": "OK (assumes not specified)"
+    },
+    "lint-snap-v2:confinement_valid": {
+      "manual_review": false,
+      "text": "'confinement' should not be used with 'type: os'"
+    },
+    "lint-snap-v2:description": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:description_present": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:grade_valid": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:hook_executable:configure": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:hooks_present": {
+      "manual_review": false,
+      "text": "OK (optional hooks field not specified)"
+    },
+    "lint-snap-v2:iffy_files": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:name_valid": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:snap_manifest": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:snap_type_valid": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:summary": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:summary_present": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:title_present": {
+      "manual_review": false,
+      "text": "OK (optional title field not specified)"
+    },
+    "lint-snap-v2:unknown_field": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:unknown_hook": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:valid_unicode": {
+      "manual_review": false,
+      "text": "ok"
+    },
+    "lint-snap-v2:vcs_files": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "lint-snap-v2:version_valid": {
+      "manual_review": false,
+      "text": "OK"
+    }
+  },
+  "warn": {}
+}
+= snap.v2_security =
+{
+  "error": {},
+  "info": {
+    "security-snap-v2:squashfs_files": {
+      "manual_review": false,
+      "text": "OK"
+    },
+    "security-snap-v2:squashfs_repack_checksum": {
+      "manual_review": false,
+      "text": "OK"
+    }
+  },
+  "warn": {}
+}
+
+= --json test-core-with-primed-staged_16-2.37.2_amd64.snap =
+{
+  "snap.v2_declaration": {
+    "error": {},
+    "info": {},
+    "warn": {}
+  },
+  "snap.v2_functional": {
+    "error": {},
+    "info": {},
+    "warn": {}
+  },
+  "snap.v2_lint": {
+    "error": {
+      "lint-snap-v2:snap_type_redflag": {
+        "manual_review": true,
+        "text": "(NEEDS REVIEW) type 'os' not allowed"
+      }
+    },
+    "info": {
+      "lint-snap-v2:apps_present": {
+        "manual_review": false,
+        "text": "OK (optional apps field not specified)"
+      },
+      "lint-snap-v2:architecture_specified_needed:amd64": {
+        "manual_review": false,
+        "text": "Could not find compiled binaries for architecture 'amd64'"
+      },
+      "lint-snap-v2:architecture_valid": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:assumes_valid": {
+        "manual_review": false,
+        "text": "OK (assumes not specified)"
+      },
+      "lint-snap-v2:confinement_valid": {
+        "manual_review": false,
+        "text": "'confinement' should not be used with 'type: os'"
+      },
+      "lint-snap-v2:description": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:description_present": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:grade_valid": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:hook_executable:configure": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:hooks_present": {
+        "manual_review": false,
+        "text": "OK (optional hooks field not specified)"
+      },
+      "lint-snap-v2:iffy_files": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:name_valid": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:snap_manifest": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:snap_type_valid": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:summary": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:summary_present": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:title_present": {
+        "manual_review": false,
+        "text": "OK (optional title field not specified)"
+      },
+      "lint-snap-v2:unknown_field": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:unknown_hook": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:valid_unicode": {
+        "manual_review": false,
+        "text": "ok"
+      },
+      "lint-snap-v2:vcs_files": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "lint-snap-v2:version_valid": {
+        "manual_review": false,
+        "text": "OK"
+      }
+    },
+    "warn": {}
+  },
+  "snap.v2_security": {
+    "error": {},
+    "info": {
+      "security-snap-v2:squashfs_files": {
+        "manual_review": false,
+        "text": "OK"
+      },
+      "security-snap-v2:squashfs_repack_checksum": {
+        "manual_review": false,
+        "text": "OK"
+      }
+    },
+    "warn": {}
+  }
+}
+
 =  test-core_16-2.37.2_amd64.snap =
 Errors
 ------