Merge branch 'metadata_improvements' into 'master'

Draft: metadata: Save context managers for opening files

See merge request fdroid/fdroidserver!1357

.gitlab-ci.yml

@@ -285,9 +285,7 @@ black:
     - black --check --diff --color $CI_PROJECT_DIR
 
 fedora_latest:
-  image: fedora:latest
-  only:
-    - master@fdroid/fdroidserver
+  image: fedora:39  # support ends on 2024-11-12
   script:
     # tricks to hopefully make runs more reliable
     - echo "timeout=600" >> /etc/dnf/dnf.conf

fdroidserver/common.py

@@ -2354,6 +2354,8 @@ def prepare_source(vcs, app, build, build_dir, srclib_dir, extlib_dir, onserver=
                 gradlefile = build_gradle
             elif os.path.exists(build_gradle_kts):
                 gradlefile = build_gradle_kts
+            else:
+                raise BuildException("No gradle file found")
             regsub_file(r'compileSdkVersion[ =]+[0-9]+',
                         r'compileSdkVersion %s' % n,
                         gradlefile)

fdroidserver/deploy.py

@@ -837,7 +837,15 @@ def push_binary_transparency(git_repo_path, git_remote):
                 origin.set_url(git_remote)
         else:
             origin = gitrepo.create_remote('origin', git_remote)
-        origin.push(GIT_BRANCH)
+        for _i in range(3):
+            try:
+                origin.push(GIT_BRANCH)
+            except git.GitCommandError as e:
+                logging.error(e)
+                continue
+            break
+        else:
+            raise FDroidException(_("Pushing to remote server failed!"))
 
 
 def main():

fdroidserver/lint.py

@@ -722,7 +722,13 @@ def check_updates_ucm_http_aum_pattern(app):  # noqa: D403
 
 
 def check_certificate_pinned_binaries(app):
-    if len(app.get('AllowedAPKSigningKeys')) > 0:
+    keys = app.get('AllowedAPKSigningKeys')
+    known_keys = common.config.get('apk_signing_key_block_list', [])
+    if keys:
+        if known_keys:
+            for key in keys:
+                if key in known_keys:
+                    yield _('Known debug key is used in AllowedAPKSigningKeys: ') + key
         return
     if app.get('Binaries') is not None:
         yield _(

fdroidserver/metadata.py

@@ -476,28 +476,26 @@ def parse_yaml_srclib(metadatapath):
         )
         return thisinfo
 
-    with metadatapath.open("r", encoding="utf-8") as f:
-        try:
-            yaml = ruamel.yaml.YAML(typ='safe')
-            data = yaml.load(f)
-            if type(data) is not dict:
-                if platform.system() == 'Windows':
-                    # Handle symlink on Windows
-                    symlink = metadatapath.parent / metadatapath.read_text(encoding='utf-8')
-                    if symlink.is_file():
-                        with symlink.open("r", encoding="utf-8") as s:
-                            data = yaml.load(s)
-            if type(data) is not dict:
-                raise ruamel.yaml.YAMLError(
-                    _('{file} is blank or corrupt!').format(file=metadatapath)
-                )
-        except ruamel.yaml.YAMLError as e:
-            _warn_or_exception(_("Invalid srclib metadata: could not "
-                                 "parse '{file}'")
-                               .format(file=metadatapath) + '\n'
-                               + common.run_yamllint(metadatapath, indent=4),
-                               cause=e)
-            return thisinfo
+    try:
+        yaml = ruamel.yaml.YAML(typ='safe')
+        data = yaml.load(metadatapath)
+        if type(data) is not dict:
+            if platform.system() == 'Windows':
+                # Handle symlink on Windows
+                symlink = metadatapath.parent / metadatapath.read_text(encoding='utf-8')
+                if symlink.is_file():
+                    data = yaml.load(symlink)
+        if type(data) is not dict:
+            raise ruamel.yaml.YAMLError(
+                _('{file} is blank or corrupt!').format(file=metadatapath)
+            )
+    except ruamel.yaml.YAMLError as e:
+        _warn_or_exception(_("Invalid srclib metadata: could not "
+                             "parse '{file}'")
+                           .format(file=metadatapath) + '\n'
+                           + common.run_yamllint(metadatapath, indent=4),
+                           cause=e)
+        return thisinfo
 
     for key in data:
         if key not in thisinfo:
@@ -579,14 +577,11 @@ def read_metadata(appids={}, sort_by_time=False):
             Path('.').glob('.fdroid.yml')
         )
 
+    # Most things want the index alpha sorted for stability
+    metadatafiles.sort()
     if sort_by_time:
-        entries = ((path.stat().st_mtime, path) for path in metadatafiles)
-        metadatafiles = []
-        for _ignored, path in sorted(entries, reverse=True):
-            metadatafiles.append(path)
-    else:
-        # most things want the index alpha sorted for stability
-        metadatafiles = sorted(metadatafiles)
+        # Sort on mtime as primary, alpha as secondary (sort stability)
+        metadatafiles.sort(key=lambda path: path.stat().st_mtime, reverse=True)
 
     for metadatapath in metadatafiles:
         appid = metadatapath.stem
@@ -650,8 +645,7 @@ def parse_metadata(metadatapath):
     app = App()
     app.metadatapath = metadatapath.as_posix()
     if metadatapath.suffix == '.yml':
-        with metadatapath.open('r', encoding='utf-8') as mf:
-            app.update(parse_yaml_metadata(mf))
+        app.update(parse_yaml_metadata(metadatapath))
     else:
         _warn_or_exception(
             _('Unknown metadata format: {path} (use: *.yml)').format(path=metadatapath)
@@ -1268,16 +1262,15 @@ def write_yaml(mf, app):
     yaml_app = _app_to_yaml(app)
     yaml = ruamel.yaml.YAML()
     yaml.indent(mapping=2, sequence=4, offset=2)
-    yaml.dump(yaml_app, stream=mf)
+    yaml.dump(yaml_app, mf)
 
 
 def write_metadata(metadatapath, app):
     metadatapath = Path(metadatapath)
     if metadatapath.suffix == '.yml':
-        with metadatapath.open('w') as mf:
-            return write_yaml(mf, app)
-
-    _warn_or_exception(_('Unknown metadata format: %s') % metadatapath)
+        write_yaml(metadatapath, app)
+    else:
+        _warn_or_exception(_('Unknown metadata format: %s') % metadatapath)
 
 
 def add_metadata_arguments(parser):

fdroidserver/mirror.py

@@ -268,7 +268,8 @@ def main():
                 urls[icondir].append(url)
 
         for icondir in icondirs:
-            _run_wget(os.path.join(basedir, section, icondir), urls[icondir])
+            if icondir in urls:
+                _run_wget(os.path.join(basedir, section, icondir), urls[icondir])
 
 
 if __name__ == "__main__":

fdroidserver/rewritemeta.py

@@ -33,14 +33,15 @@ options = None
 
 
 def proper_format(app):
-    s = io.StringIO()
     # TODO: currently reading entire file again, should reuse first
     # read in metadata.py
     cur_content = Path(app.metadatapath).read_text(encoding='utf-8')
+
+    content = ''
     if Path(app.metadatapath).suffix == '.yml':
-        metadata.write_yaml(s, app)
-    content = s.getvalue()
-    s.close()
+        with io.StringIO() as s:
+            metadata.write_yaml(s, app)
+            content = s.getvalue()
     return content == cur_content
 
 

tests/index.TestCase

@@ -701,8 +701,14 @@ class IndexTest(unittest.TestCase):
         app = apps[appid]
         metadata = index.package_metadata(app, 'repo')
         # files
-        self.assertEqual(36027, metadata['featureGraphic']['en-US']['size'])
-        self.assertEqual(1413, metadata['icon']['en-US']['size'])
+        self.assertEqual(
+            os.path.getsize(f'repo/{appid}/en-US/featureGraphic.png'),
+            metadata['featureGraphic']['en-US']['size'],
+        )
+        self.assertEqual(
+            os.path.getsize(f'repo/{appid}/en-US/icon.png'),
+            metadata['icon']['en-US']['size'],
+        )
         # localized strings
         self.assertEqual({'en-US': 'title'}, metadata['name'])
         self.assertEqual({'en-US': 'video'}, metadata['video'])

tests/lint.TestCase

@@ -438,6 +438,45 @@ class LintTest(unittest.TestCase):
         with self.assertRaises(TypeError):
             fdroidserver.lint.lint_config('mirrors.yml')
 
+    def test_check_certificate_pinned_binaries_empty(self):
+        fdroidserver.common.config = {}
+        app = fdroidserver.metadata.App()
+        app.AllowedAPKSigningKeys = [
+            'a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc'
+        ]
+        self.assertEqual(
+            [],
+            list(fdroidserver.lint.check_certificate_pinned_binaries(app)),
+            "when the config is empty, any signing key should be allowed",
+        )
+
+    def test_lint_known_debug_keys_no_match(self):
+        fdroidserver.common.config = {
+            "apk_signing_key_block_list": "a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc"
+        }
+        app = fdroidserver.metadata.App()
+        app.AllowedAPKSigningKeys = [
+            '2fd4fd5f54babba4bcb21237809bb653361d0d2583c80964ec89b28a26e9539e'
+        ]
+        self.assertEqual(
+            [],
+            list(fdroidserver.lint.check_certificate_pinned_binaries(app)),
+            "A signing key that does not match one in the config should be allowed",
+        )
+
+    def test_lint_known_debug_keys(self):
+        fdroidserver.common.config = {
+            'apk_signing_key_block_list': 'a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc'
+        }
+        app = fdroidserver.metadata.App()
+        app.AllowedAPKSigningKeys = [
+            'a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc'
+        ]
+        for warn in fdroidserver.lint.check_certificate_pinned_binaries(app):
+            anywarns = True
+            logging.debug(warn)
+        self.assertTrue(anywarns)
+
 
 class LintAntiFeaturesTest(unittest.TestCase):
     def setUp(self):

tests/nightly.TestCase

@@ -70,7 +70,10 @@ class NightlyTest(unittest.TestCase):
 
     def tearDown(self):
         self.tempdir.cleanup()
-        os.rmdir(self.testroot)
+        try:
+            os.rmdir(self.testroot)
+        except OSError:  # other test modules might have left stuff around
+            pass
 
     def _copy_test_debug_keystore(self):
         self.dot_android.mkdir()

tests/testcommon.py

@@ -18,6 +18,9 @@
 import os
 import sys
 import tempfile
+import unittest
+
+from pathlib import Path
 
 
 class TmpCwd:
@@ -60,3 +63,12 @@ def mkdtemp():
         return tempfile.TemporaryDirectory()
     else:
         return tempfile.TemporaryDirectory(ignore_cleanup_errors=True)
+
+
+def mkdir_testfiles(localmodule, test):
+    """Keep the test files in a labeled test dir for easy reference"""
+    testroot = Path(localmodule) / '.testfiles'
+    testroot.mkdir(exist_ok=True)
+    testdir = testroot / unittest.TestCase.id(test)
+    testdir.mkdir(exist_ok=True)
+    return tempfile.mkdtemp(dir=testdir)