diff mbox series

[v2] fetch2/local: verify checksums for file:// urls

Message ID 20260511-b4-yocto-9993-v2-1-9d5a1c1451ba@gmail.com
State New
Headers show
Series [v2] fetch2/local: verify checksums for file:// urls | expand

Commit Message

Jhonata Poma-Hansen May 11, 2026, 2:58 p.m. UTC
UNINATIVE_URL ships as an https:// url in metadata and verify_checksum
catches a corrupt download. Replace it with a local file:// (the
supported form for offline or pinned uninative tarballs) and the
checksum supplied via UNINATIVE_CHECKSUM was silently ignored:
Local.urldata_init unconditionally cleared ud.needdonestamp,
short-circuiting the donestamp + verify_checksum cycle for every
file:// url. Recipes that supply ;sha256sum= on a file:// url hit the
same hole.

Keep needdonestamp at the FetchData default (True) when the url carries
any of the known checksum parameters (bare ;<algo>sum= or named
;name=foo;foo.<algo>sum= form), so the standard verify_checksum flow
runs and a mismatch raises ChecksumError just like it does for
http(s)://. When no checksum is supplied (the common file:// recipe
patch case) needdonestamp stays False and behavior is unchanged.

A checksum mismatch on a file:// url must not mutate the user's source
tree the way it does for downloaded artifacts under DL_DIR. Move
rename_bad_checksum onto FetchMethod and override it as a no-op in the
Local fetcher so a ChecksumError surfaces without an on-disk
<localpath>_bad-checksum_<sha> sibling.

Link: https://bugzilla.yoctoproject.org/show_bug.cgi?id=9993
[YOCTO #9993]

Signed-off-by: Jhonata Poma-Hansen <jhonata.poma@gmail.com>
---
Changes in v2:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v1: https://lore.kernel.org/r/20260430-b4-yocto-9993-v1-1-a54d842cd0fd@gmail.com
---
 lib/bb/fetch2/__init__.py | 36 ++++++++++++++++++------------------
 lib/bb/fetch2/local.py    | 18 ++++++++++++++++--
 lib/bb/tests/fetch.py     | 30 ++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 20 deletions(-)


---
base-commit: 5d722b5d65e4eef7befe6376983385421e993f86
change-id: 20260429-b4-yocto-9993-cadd9cbed7ce

Best regards,

Comments

Jhonata Poma-Hansen May 11, 2026, 3:01 p.m. UTC | #1
Apologies, the "Changes in v2:" section in v2 ships with two literal
b4 placeholder lines starting with "EDITME: ..." that I forgot to
fill in before sending. Please disregard those two lines. The actual
changes from v1, addressing Paul Barker's review
(lore.kernel.org/r/162413fca4aa8fa53472aa6ba1e12b56c1a5c5c9.camel@pbarker.dev):

- Commit body trimmed; UNINATIVE_URL trigger now leads paragraph 1
  (Paul flagged this was missing from v1).
- Dropped the long historical commit references (b8b14d975a25,
  6424f4b7e9c1, 4b8de2e7d126) from the commit body.
- rename_bad_checksum moved from a module-level helper in
  bb/fetch2/__init__.py onto the FetchMethod class; Local overrides
  it as a no-op so a ChecksumError surfaces without renaming files
  under the user's source tree. Four callsites updated to
  ud.method.rename_bad_checksum(...). No "ud.type == 'file'" special
  case in __init__.py.
- Inline comment in local.py trimmed from ~10 lines to 3.
- Tests reduced to two keepers in FetcherLocalTest:
  - test_local_checksum_match: simpler comment; same assertion.
  - test_local_checksum_mismatch: dropped the os.listdir loop,
    replaced with a direct assertFalse on the specific
    bad-checksum sibling path.
  Dropped: test_local_no_checksum_no_donestamp,
  test_local_checksum_mismatch_md5, test_local_checksum_named,
  test_local_checksum_directory_ignored (the directory-with-bad-
  checksum case Paul noted should raise; left out of this patch
  rather than widening scope).

Thanks,
Jhonata
diff mbox series

Patch

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 52d5556d..5f4db608 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -738,7 +738,7 @@  def verify_donestamp(ud, d, origud=None):
         logger.warning("Checksum mismatch for local file %s\n"
                        "Cleaning and trying again." % ud.localpath)
         if os.path.exists(ud.localpath):
-            rename_bad_checksum(ud, e.checksum)
+            ud.method.rename_bad_checksum(ud, e.checksum)
         bb.utils.remove(ud.donestamp)
     return False
 
@@ -771,7 +771,7 @@  def update_stamp(ud, d):
             logger.warning("Checksum mismatch for local file %s\n"
                            "Cleaning and trying again." % ud.localpath)
             if os.path.exists(ud.localpath):
-                rename_bad_checksum(ud, e.checksum)
+                ud.method.rename_bad_checksum(ud, e.checksum)
             bb.utils.remove(ud.donestamp)
             raise
 
@@ -1067,20 +1067,6 @@  def build_mirroruris(origud, mirrors, ld):
 
     return uris, uds
 
-def rename_bad_checksum(ud, suffix):
-    """
-    Renames files to have suffix from parameter
-    """
-
-    if ud.localpath is None:
-        return
-
-    new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix)
-    bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath))
-    if not bb.utils.movefile(ud.localpath, new_localpath):
-        bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath))
-
-
 def try_mirror_url(fetch, origud, ud, ld, check = False):
     # Return of None or a value means we're finished
     # False means try another url
@@ -1156,7 +1142,7 @@  def try_mirror_url(fetch, origud, ud, ld, check = False):
             logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url))
             logger.warning(str(e))
             if os.path.exists(ud.localpath):
-                rename_bad_checksum(ud, e.checksum)
+                ud.method.rename_bad_checksum(ud, e.checksum)
         elif isinstance(e, NoChecksumError):
             raise
         else:
@@ -1472,6 +1458,20 @@  class FetchMethod(object):
         """
         return True
 
+    def rename_bad_checksum(self, ud, suffix):
+        """
+        Rename ud.localpath with the given suffix on checksum mismatch.
+        Local overrides this to a no-op since its localpath points at the
+        user's source tree.
+        """
+        if ud.localpath is None:
+            return
+
+        new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix)
+        bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath))
+        if not bb.utils.movefile(ud.localpath, new_localpath):
+            bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath))
+
     def verify_donestamp(self, ud, d):
         """
         Verify the donestamp file
@@ -1937,7 +1937,7 @@  class Fetch(object):
                             logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u)
                             logger.debug(str(e))
                             if os.path.exists(ud.localpath):
-                                rename_bad_checksum(ud, e.checksum)
+                                ud.method.rename_bad_checksum(ud, e.checksum)
                         elif isinstance(e, NoChecksumError):
                             raise
                         else:
diff --git a/lib/bb/fetch2/local.py b/lib/bb/fetch2/local.py
index fda56a56..f9808e9b 100644
--- a/lib/bb/fetch2/local.py
+++ b/lib/bb/fetch2/local.py
@@ -17,7 +17,7 @@  import os
 import urllib.request, urllib.parse, urllib.error
 import bb
 import bb.utils
-from   bb.fetch2 import FetchMethod, FetchError, ParameterError
+from   bb.fetch2 import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError
 from   bb.fetch2 import logger
 
 class Local(FetchMethod):
@@ -31,11 +31,25 @@  class Local(FetchMethod):
         # We don't set localfile as for this fetcher the file is already local!
         ud.basename = os.path.basename(ud.path)
         ud.basepath = ud.path
-        ud.needdonestamp = False
+        # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe
+        # ;sha256sum=) by leaving needdonestamp at FetchData's default so
+        # verify_checksum runs.
+        name = ud.parm.get("name")
+        ud.needdonestamp = any(
+            (name and "%s.%ssum" % (name, a) in ud.parm)
+            or "%ssum" % a in ud.parm
+            for a in CHECKSUM_LIST
+        )
         if "*" in ud.path:
             raise bb.fetch2.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url)
         return
 
+    def rename_bad_checksum(self, ud, suffix):
+        # file:// urls point at the user's source tree (recipe-shipped files
+        # under FILESPATH or an absolute path the user passed in); skip the
+        # rename so a ChecksumError surfaces without mutating their files.
+        return
+
     def localpath(self, urldata, d):
         """
         Return the local filename of a given url assuming a successful fetch.
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 077f741e..93e889fd 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -813,6 +813,36 @@  class FetcherLocalTest(FetcherTest):
         tree = self.fetchUnpack(['file://archive.tar.bz2;subdir=bar;striplevel=1'])
         self.assertEqual(tree, ['bar/c', 'bar/d', 'bar/subdir/e'])
 
+    def test_local_checksum_match(self):
+        # Correct sha256 must run verify_checksum to completion; donestamp
+        # lands in DL_DIR.
+        import hashlib
+        content = b"file:// checksum match test\n"
+        with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f:
+            f.write(content)
+        good = hashlib.sha256(content).hexdigest()
+        fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d)
+        ud = fetcher.ud[fetcher.urls[0]]
+        self.assertTrue(ud.needdonestamp)
+        fetcher.download()
+        self.assertTrue(os.path.exists(ud.donestamp))
+
+    def test_local_checksum_mismatch(self):
+        # Bad sha256 raises ChecksumError without renaming the user's
+        # source file to the <localpath>_bad-checksum_<sha> sibling.
+        content = b"file:// checksum mismatch test\n"
+        srcpath = os.path.join(self.localsrcdir, 'sumfile')
+        with open(srcpath, 'wb') as f:
+            f.write(content)
+        bad = "0" * 64
+        fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d)
+        with self.assertRaises(bb.fetch2.FetchError):
+            fetcher.download()
+        self.assertTrue(os.path.exists(srcpath))
+        with open(srcpath, 'rb') as f:
+            self.assertEqual(f.read(), content)
+        self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad))
+
     def dummyGitTest(self, suffix):
         # Create dummy local Git repo
         src_dir = tempfile.mkdtemp(dir=self.tempdir,