Fix move_ent with signed binpkg

The gpkg file that cannot be updated will be removed.

[sam: We did discuss stripping the signatures to re-use them but that
feels messier and likely unexpected by the user. We also don't distinguish
between locally-made vs fetched binpkgs (where perhaps for local ones, we could
just re-sign if signed).

It was also raised that for fetched binpkgs, we could not worry once it's
been verified the first time, but that's again complicated.

Simply dropping these binpkgs seems like the best solution and to allow
the binhost to re-sign it on updating instead.]

Bug: https://bugs.gentoo.org/919419
Signed-off-by: Sheng Yu <syu.os@protonmail.com>
Closes: https://github.com/gentoo/portage/pull/1043
Signed-off-by: Sam James <sam@gentoo.org>

7 files changed, 436 insertions, 5 deletions

diff --git a/lib/portage/dbapi/bintree.py b/lib/portage/dbapi/bintree.py
index 6446fde95..a6e1f9773 100644
--- a/lib/portage/dbapi/bintree.py
+++ b/lib/portage/dbapi/bintree.py
@@ -291,6 +291,24 @@ class bindbapi(fakedbapi):
         elif binpkg_format == "gpkg":
             mybinpkg = portage.gpkg.gpkg(self.settings, cpv_str, binpkg_path)
             mydata = mybinpkg.get_metadata()
+            if mybinpkg.signature_exist:
+                writemsg(
+                    colorize(
+                        "WARN",
+                        f"Binpkg update ignored for signed package: {binpkg_path}, "
+                        "the file will be removed.",
+                    )
+                )
+                try:
+                    os.remove(binpkg_path)
+                except OSError as err:
+                    writemsg(
+                        colorize(
+                            "WARN",
+                            f"Failed to remove moved signed package: {binpkg_path} {str(err)}",
+                        )
+                    )
+                return
             encoding_key = False
         else:
             raise InvalidBinaryPackageFormat(
@@ -687,7 +705,6 @@ class binarytree:
                 )
                 continue
 
-            moves += 1
             binpkg_format = get_binpkg_format(binpkg_path)
             if binpkg_format == "xpak":
                 mytbz2 = portage.xpak.tbz2(binpkg_path)
@@ -708,6 +725,8 @@ class binarytree:
             else:
                 continue
 
+            moves += 1
+
             updated_items = update_dbentries([mylist], mydata, parent=mycpv)
             mydata.update(updated_items)
             if decode_metadata_name:
diff --git a/lib/portage/exception.py b/lib/portage/exception.py
index 505e920de..153a9f9a5 100644
--- a/lib/portage/exception.py
+++ b/lib/portage/exception.py
@@ -197,6 +197,10 @@ class CompressorOperationFailed(PortagePackageException):
     """An error occurred during external operation"""
 
 
+class SignedPackage(PortagePackageException):
+    """Unable to update a signed package"""
+
+
 class InvalidAtom(PortagePackageException):
     """Malformed atom spec"""
 
diff --git a/lib/portage/gpkg.py b/lib/portage/gpkg.py
index c56076ab9..2e1130857 100644
--- a/lib/portage/gpkg.py
+++ b/lib/portage/gpkg.py
@@ -34,6 +34,7 @@ from portage.exception import (
     DigestException,
     MissingSignature,
     InvalidSignature,
+    SignedPackage,
 )
 from portage.output import colorize, EOutput
 from portage.util._urlopen import urlopen
@@ -991,7 +992,7 @@ class gpkg:
                     finally:
                         image_tar.kill()
 
-    def update_metadata(self, metadata, new_basename=None):
+    def update_metadata(self, metadata, new_basename=None, force=False):
         """
         Update metadata in the gpkg file.
         """
@@ -999,6 +1000,9 @@ class gpkg:
         self.checksums = []
         old_basename = self.prefix
 
+        if self.signature_exist and not force:
+            raise SignedPackage("Cannot update a signed gpkg file")
+
         if new_basename is None:
             new_basename = old_basename
         else:
diff --git a/lib/portage/tests/gpkg/test_gpkg_metadata_update.py b/lib/portage/tests/gpkg/test_gpkg_metadata_update.py
index d2da630f3..51ad8b404 100644
--- a/lib/portage/tests/gpkg/test_gpkg_metadata_update.py
+++ b/lib/portage/tests/gpkg/test_gpkg_metadata_update.py
@@ -16,7 +16,7 @@ class test_gpkg_metadata_case(TestCase):
     def test_gpkg_update_metadata(self):
         playground = ResolverPlayground(
             user_config={
-                "make.conf": ('BINPKG_COMPRESS="gzip"',),
+                "make.conf": ('BINPKG_COMPRESS="gzip"', 'FEATURES="-binpkg-signing"'),
             }
         )
         tmpdir = tempfile.mkdtemp()
diff --git a/lib/portage/tests/update/test_move_ent.py b/lib/portage/tests/update/test_move_ent.py
index 22d0c8feb..436b846cf 100644
--- a/lib/portage/tests/update/test_move_ent.py
+++ b/lib/portage/tests/update/test_move_ent.py
@@ -122,3 +122,111 @@ class MoveEntTestCase(TestCase):
 
                 finally:
                     playground.cleanup()
+
+    def testMoveEntWithSignature(self):
+        ebuilds = {
+            "dev-libs/A-2::dont_apply_updates": {
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+        }
+
+        installed = {
+            "dev-libs/A-1::test_repo": {
+                "EAPI": "4",
+            },
+            "dev-libs/A-2::dont_apply_updates": {
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+        }
+
+        binpkgs = {
+            "dev-libs/A-1::test_repo": {
+                "EAPI": "4",
+            },
+            "dev-libs/A-2::dont_apply_updates": {
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+        }
+
+        updates = textwrap.dedent(
+            """
+			move dev-libs/A dev-libs/A-moved
+		"""
+        )
+
+        for binpkg_format in ("gpkg",):
+            with self.subTest(binpkg_format=binpkg_format):
+                print(colorize("HILITE", binpkg_format), end=" ... ")
+                sys.stdout.flush()
+                playground = ResolverPlayground(
+                    binpkgs=binpkgs,
+                    ebuilds=ebuilds,
+                    installed=installed,
+                    user_config={
+                        "make.conf": (f'BINPKG_FORMAT="{binpkg_format}"',),
+                    },
+                )
+
+                settings = playground.settings
+                trees = playground.trees
+                eroot = settings["EROOT"]
+                test_repo_location = settings.repositories["test_repo"].location
+                portdb = trees[eroot]["porttree"].dbapi
+                vardb = trees[eroot]["vartree"].dbapi
+                bindb = trees[eroot]["bintree"].dbapi
+
+                updates_dir = os.path.join(test_repo_location, "profiles", "updates")
+
+                try:
+                    ensure_dirs(updates_dir)
+                    with open(os.path.join(updates_dir, "1Q-2010"), "w") as f:
+                        f.write(updates)
+
+                    # Create an empty updates directory, so that this
+                    # repo doesn't inherit updates from the main repo.
+                    ensure_dirs(
+                        os.path.join(
+                            portdb.getRepositoryPath("dont_apply_updates"),
+                            "profiles",
+                            "updates",
+                        )
+                    )
+
+                    global_noiselimit = portage.util.noiselimit
+                    portage.util.noiselimit = -2
+                    try:
+                        _do_global_updates(trees, {})
+                    finally:
+                        portage.util.noiselimit = global_noiselimit
+
+                    # Workaround for cache validation not working
+                    # correctly when filesystem has timestamp precision
+                    # of 1 second.
+                    vardb._clear_cache()
+
+                    # A -> A-moved
+                    self.assertRaises(KeyError, vardb.aux_get, "dev-libs/A-1", ["EAPI"])
+                    vardb.aux_get("dev-libs/A-moved-1", ["EAPI"])
+                    # The original package should still exist because a binary
+                    # package move is a copy on write operation.
+                    bindb.aux_get("dev-libs/A-1", ["EAPI"])
+                    print(bindb.aux_get("dev-libs/A-1", "PF"))
+                    self.assertRaises(
+                        KeyError, bindb.aux_get, "dev-libs/A-moved-1", ["EAPI"]
+                    )
+
+                    # dont_apply_updates
+                    self.assertRaises(
+                        KeyError, vardb.aux_get, "dev-libs/A-moved-2", ["EAPI"]
+                    )
+                    vardb.aux_get("dev-libs/A-2", ["EAPI"])
+                    self.assertRaises(
+                        KeyError, bindb.aux_get, "dev-libs/A-moved-2", ["EAPI"]
+                    )
+                    bindb.aux_get("dev-libs/A-2", ["EAPI"])
+
+                finally:
+                    playground.cleanup()
diff --git a/lib/portage/tests/update/test_move_slot_ent.py b/lib/portage/tests/update/test_move_slot_ent.py
index 88d9802cf..caefdb4c9 100644
--- a/lib/portage/tests/update/test_move_slot_ent.py
+++ b/lib/portage/tests/update/test_move_slot_ent.py
@@ -86,7 +86,10 @@ class MoveSlotEntTestCase(TestCase):
                     ebuilds=ebuilds,
                     installed=installed,
                     user_config={
-                        "make.conf": (f'BINPKG_FORMAT="{binpkg_format}"',),
+                        "make.conf": (
+                            f'BINPKG_FORMAT="{binpkg_format}"',
+                            'FEATURES="-binpkg-signing"',
+                        ),
                     },
                 )
 
@@ -154,3 +157,146 @@ class MoveSlotEntTestCase(TestCase):
 
                 finally:
                     playground.cleanup()
+
+    def testMoveSlotEntWithSignature(self):
+        ebuilds = {
+            "dev-libs/A-2::dont_apply_updates": {
+                "EAPI": "5",
+                "SLOT": "0/2.30",
+            },
+            "dev-libs/B-2::dont_apply_updates": {
+                "SLOT": "0",
+            },
+            "dev-libs/C-2.1::dont_apply_updates": {
+                "EAPI": "5",
+                "SLOT": "0/2.1",
+            },
+        }
+
+        installed = {
+            "dev-libs/A-1::test_repo": {
+                "EAPI": "5",
+                "SLOT": "0/2.30",
+            },
+            "dev-libs/B-1::test_repo": {
+                "SLOT": "0",
+            },
+            "dev-libs/C-1::test_repo": {
+                "EAPI": "5",
+                "SLOT": "0/1",
+            },
+        }
+
+        binpkgs = {
+            "dev-libs/A-1::test_repo": {
+                "EAPI": "5",
+                "SLOT": "0/2.30",
+            },
+            "dev-libs/A-2::dont_apply_updates": {
+                "EAPI": "5",
+                "SLOT": "0/2.30",
+            },
+            "dev-libs/B-1::test_repo": {
+                "SLOT": "0",
+            },
+            "dev-libs/B-2::dont_apply_updates": {
+                "SLOT": "0",
+            },
+            "dev-libs/C-1::test_repo": {
+                "EAPI": "5",
+                "SLOT": "0/1",
+            },
+            "dev-libs/C-2.1::dont_apply_updates": {
+                "EAPI": "5",
+                "SLOT": "0/2.1",
+            },
+        }
+
+        updates = textwrap.dedent(
+            """
+			slotmove dev-libs/A 0 2
+			slotmove dev-libs/B 0 1
+			slotmove dev-libs/C 0 1
+		"""
+        )
+
+        for binpkg_format in ("gpkg",):
+            with self.subTest(binpkg_format=binpkg_format):
+                print(colorize("HILITE", binpkg_format), end=" ... ")
+                sys.stdout.flush()
+                playground = ResolverPlayground(
+                    binpkgs=binpkgs,
+                    ebuilds=ebuilds,
+                    installed=installed,
+                    user_config={
+                        "make.conf": (
+                            f'BINPKG_FORMAT="{binpkg_format}"',
+                            'FEATURES="binpkg-signing"',
+                        ),
+                    },
+                )
+
+                settings = playground.settings
+                trees = playground.trees
+                eroot = settings["EROOT"]
+                test_repo_location = settings.repositories["test_repo"].location
+                portdb = trees[eroot]["porttree"].dbapi
+                vardb = trees[eroot]["vartree"].dbapi
+                bindb = trees[eroot]["bintree"].dbapi
+
+                updates_dir = os.path.join(test_repo_location, "profiles", "updates")
+
+                try:
+                    ensure_dirs(updates_dir)
+                    with open(os.path.join(updates_dir, "1Q-2010"), "w") as f:
+                        f.write(updates)
+
+                    # Create an empty updates directory, so that this
+                    # repo doesn't inherit updates from the main repo.
+                    ensure_dirs(
+                        os.path.join(
+                            portdb.getRepositoryPath("dont_apply_updates"),
+                            "profiles",
+                            "updates",
+                        )
+                    )
+
+                    global_noiselimit = portage.util.noiselimit
+                    portage.util.noiselimit = -2
+                    try:
+                        _do_global_updates(trees, {})
+                    finally:
+                        portage.util.noiselimit = global_noiselimit
+
+                    # Workaround for cache validation not working
+                    # correctly when filesystem has timestamp precision
+                    # of 1 second.
+                    vardb._clear_cache()
+
+                    # 0/2.30 -> 2/2.30
+                    self.assertEqual(
+                        "2/2.30", vardb.aux_get("dev-libs/A-1", ["SLOT"])[0]
+                    )
+                    self.assertEqual(
+                        "0/2.30", bindb.aux_get("dev-libs/A-1", ["SLOT"])[0]
+                    )
+
+                    # 0 -> 1
+                    self.assertEqual("1", vardb.aux_get("dev-libs/B-1", ["SLOT"])[0])
+                    self.assertEqual("0", bindb.aux_get("dev-libs/B-1", ["SLOT"])[0])
+
+                    # 0/1 -> 1 (equivalent to 1/1)
+                    self.assertEqual("1", vardb.aux_get("dev-libs/C-1", ["SLOT"])[0])
+                    self.assertEqual("0/1", bindb.aux_get("dev-libs/C-1", ["SLOT"])[0])
+
+                    # dont_apply_updates
+                    self.assertEqual(
+                        "0/2.30", bindb.aux_get("dev-libs/A-2", ["SLOT"])[0]
+                    )
+                    self.assertEqual("0", bindb.aux_get("dev-libs/B-2", ["SLOT"])[0])
+                    self.assertEqual(
+                        "0/2.1", bindb.aux_get("dev-libs/C-2.1", ["SLOT"])[0]
+                    )
+
+                finally:
+                    playground.cleanup()
diff --git a/lib/portage/tests/update/test_update_dbentry.py b/lib/portage/tests/update/test_update_dbentry.py
index a473cd937..4e6554496 100644
--- a/lib/portage/tests/update/test_update_dbentry.py
+++ b/lib/portage/tests/update/test_update_dbentry.py
@@ -235,7 +235,10 @@ class UpdateDbentryTestCase(TestCase):
                     installed=installed,
                     world=world,
                     user_config={
-                        "make.conf": (f'BINPKG_FORMAT="{binpkg_format}"',),
+                        "make.conf": (
+                            f'BINPKG_FORMAT="{binpkg_format}"',
+                            'FEATURES="-binpkg-signing"',
+                        ),
                     },
                 )
 
@@ -307,3 +310,150 @@ class UpdateDbentryTestCase(TestCase):
 
                 finally:
                     playground.cleanup()
+
+    def testUpdateDbentryDbapiTestCaseWithSignature(self):
+        ebuilds = {
+            "dev-libs/A-2::dont_apply_updates": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+            "dev-libs/B-2::dont_apply_updates": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+        }
+
+        installed = {
+            "dev-libs/A-1::test_repo": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+            },
+            "dev-libs/A-2::dont_apply_updates": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+            "dev-libs/B-1::test_repo": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+            },
+            "dev-libs/M-1::test_repo": {
+                "EAPI": "4",
+            },
+            "dev-libs/N-1::test_repo": {
+                "EAPI": "4",
+            },
+            "dev-libs/N-2::test_repo": {
+                "EAPI": "4",
+            },
+        }
+
+        binpkgs = {
+            "dev-libs/A-1::test_repo": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+            },
+            "dev-libs/A-2::dont_apply_updates": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+                "SLOT": "2",
+            },
+            "dev-libs/B-1::test_repo": {
+                "RDEPEND": "dev-libs/M dev-libs/N dev-libs/P",
+                "EAPI": "4",
+            },
+        }
+
+        world = ["dev-libs/M", "dev-libs/N"]
+
+        updates = textwrap.dedent(
+            """
+			move dev-libs/M dev-libs/M-moved
+		"""
+        )
+
+        for binpkg_format in ("gpkg",):
+            with self.subTest(binpkg_format=binpkg_format):
+                print(colorize("HILITE", binpkg_format), end=" ... ")
+                sys.stdout.flush()
+                playground = ResolverPlayground(
+                    binpkgs=binpkgs,
+                    ebuilds=ebuilds,
+                    installed=installed,
+                    world=world,
+                    user_config={
+                        "make.conf": (f'BINPKG_FORMAT="{binpkg_format}"',),
+                    },
+                )
+
+                settings = playground.settings
+                trees = playground.trees
+                eroot = settings["EROOT"]
+                test_repo_location = settings.repositories["test_repo"].location
+                portdb = trees[eroot]["porttree"].dbapi
+                vardb = trees[eroot]["vartree"].dbapi
+                bindb = trees[eroot]["bintree"].dbapi
+                setconfig = trees[eroot]["root_config"].setconfig
+                selected_set = setconfig.getSets()["selected"]
+
+                updates_dir = os.path.join(test_repo_location, "profiles", "updates")
+
+                try:
+                    ensure_dirs(updates_dir)
+                    with open(os.path.join(updates_dir, "1Q-2010"), "w") as f:
+                        f.write(updates)
+
+                    # Create an empty updates directory, so that this
+                    # repo doesn't inherit updates from the main repo.
+                    ensure_dirs(
+                        os.path.join(
+                            portdb.getRepositoryPath("dont_apply_updates"),
+                            "profiles",
+                            "updates",
+                        )
+                    )
+
+                    global_noiselimit = portage.util.noiselimit
+                    portage.util.noiselimit = -2
+                    try:
+                        _do_global_updates(trees, {})
+                    finally:
+                        portage.util.noiselimit = global_noiselimit
+
+                    # Workaround for cache validation not working
+                    # correctly when filesystem has timestamp precision
+                    # of 1 second.
+                    vardb._clear_cache()
+
+                    # M -> M-moved
+                    old_pattern = re.compile(r"\bdev-libs/M(\s|$)")
+                    rdepend = vardb.aux_get("dev-libs/A-1", ["RDEPEND"])[0]
+                    self.assertTrue(old_pattern.search(rdepend) is None)
+                    self.assertTrue("dev-libs/M-moved" in rdepend)
+                    rdepend = bindb.aux_get("dev-libs/A-1", ["RDEPEND"])[0]
+                    print(old_pattern.search(rdepend) is None)
+                    self.assertFalse(old_pattern.search(rdepend) is None)
+                    self.assertFalse("dev-libs/M-moved" in rdepend)
+                    rdepend = vardb.aux_get("dev-libs/B-1", ["RDEPEND"])[0]
+                    self.assertTrue(old_pattern.search(rdepend) is None)
+                    self.assertTrue("dev-libs/M-moved" in rdepend)
+                    rdepend = vardb.aux_get("dev-libs/B-1", ["RDEPEND"])[0]
+                    self.assertTrue(old_pattern.search(rdepend) is None)
+                    self.assertTrue("dev-libs/M-moved" in rdepend)
+
+                    # dont_apply_updates
+                    rdepend = vardb.aux_get("dev-libs/A-2", ["RDEPEND"])[0]
+                    self.assertTrue("dev-libs/M" in rdepend)
+                    self.assertTrue("dev-libs/M-moved" not in rdepend)
+                    rdepend = bindb.aux_get("dev-libs/A-2", ["RDEPEND"])[0]
+                    self.assertTrue("dev-libs/M" in rdepend)
+                    self.assertTrue("dev-libs/M-moved" not in rdepend)
+
+                    selected_set.load()
+                    self.assertTrue("dev-libs/M" not in selected_set)
+                    self.assertTrue("dev-libs/M-moved" in selected_set)
+
+                finally:
+                    playground.cleanup()