Merge branch 'test_sdk_exists' into 'master'

test_sdk_exists to be based on apksigner, that's the requirement

See merge request fdroid/fdroidserver!1391

.safety-policy.yml

@@ -5,3 +5,6 @@ security:
     52495:
       reason: setuptools comes from Debian
       expires: '2025-01-31'
+    60350:
+      reason: GitPython comes from Debian https://security-tracker.debian.org/tracker/CVE-2023-40267
+      expires: '2025-01-31'

fdroidserver/common.py

@@ -728,13 +728,16 @@ def test_aapt_version(aapt):
 
 def test_sdk_exists(thisconfig):
     if 'sdk_path' not in thisconfig:
-        # TODO convert this to apksigner once it is required
-        if 'aapt' in thisconfig and os.path.isfile(thisconfig['aapt']):
-            test_aapt_version(thisconfig['aapt'])
-            return True
-        else:
-            logging.error(_("'sdk_path' not set in config.yml!"))
-            return False
+        # check the 'apksigner' value in the config to see if its new enough
+        f = thisconfig.get('apksigner', '')
+        if os.path.isfile(f):
+            sdk_path = os.path.dirname(os.path.dirname(os.path.dirname(f)))
+            tmpconfig = {'sdk_path': sdk_path}
+            find_apksigner(tmpconfig)
+            if os.path.exists(tmpconfig.get('apksigner', '')):
+                return True
+        logging.error(_("'sdk_path' not set in config.yml!"))
+        return False
     if thisconfig['sdk_path'] == default_config['sdk_path']:
         logging.error(_('No Android SDK found!'))
         logging.error(_('You can use ANDROID_HOME to set the path to your SDK, i.e.:'))
@@ -748,6 +751,9 @@ def test_sdk_exists(thisconfig):
         logging.critical(_("Android SDK path '{path}' is not a directory!")
                          .format(path=thisconfig['sdk_path']))
         return False
+    find_apksigner(thisconfig)
+    if not os.path.exists(thisconfig.get('apksigner', '')):
+        return False
     return True
 
 

tests/common.TestCase

@@ -2051,6 +2051,75 @@ class CommonTest(unittest.TestCase):
         config = fdroidserver.common.read_config(fdroidserver.common.options)
         self.assertEqual('/usr/lib/jvm/java-8-openjdk', config.get('java_paths', {}).get('8'))
 
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_test_sdk_exists_fails_on_bad_sdk_path(self):
+        config = {'sdk_path': 'nothinghere'}
+        self.assertFalse(fdroidserver.common.test_sdk_exists(config))
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_test_sdk_exists_fails_on_empty(self):
+        self.assertFalse(fdroidserver.common.test_sdk_exists(dict()))
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_test_sdk_exists_fails_on_non_existent(self):
+        config = {'sdk_path': os.path.join(self.testdir, 'non_existent')}
+        self.assertFalse(fdroidserver.common.test_sdk_exists(config))
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_test_sdk_exists_fails_on_file(self):
+        f = os.path.join(self.testdir, 'testfile')
+        open(f, 'w').close()
+        config = {'sdk_path': f}
+        self.assertFalse(fdroidserver.common.test_sdk_exists(config))
+
+    @mock.patch.dict(os.environ, {'PATH': '/nonexistent'}, clear=True)
+    def test_test_sdk_exists_valid_apksigner_in_config(self):
+        apksigner = os.path.join(
+            self.testdir,
+            'build-tools',
+            fdroidserver.common.MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION,
+            'apksigner',
+        )
+        os.makedirs(os.path.dirname(apksigner))
+        with open(apksigner, 'w') as fp:
+            fp.write('#!/bin/sh\ndate\n')
+        os.chmod(apksigner, 0o0755)
+        config = {'apksigner': apksigner}
+        self.assertTrue(fdroidserver.common.test_sdk_exists(config))
+
+    @mock.patch.dict(os.environ, {'PATH': '/nonexistent'}, clear=True)
+    def test_test_sdk_exists_old_apksigner_in_config(self):
+        apksigner = os.path.join(self.testdir, 'build-tools', '28.0.0', 'apksigner')
+        os.makedirs(os.path.dirname(apksigner))
+        with open(apksigner, 'w') as fp:
+            fp.write('#!/bin/sh\ndate\n')
+        os.chmod(apksigner, 0o0755)
+        config = {'apksigner': apksigner}
+        self.assertFalse(fdroidserver.common.test_sdk_exists(config))
+
+    @mock.patch.dict(os.environ, {'PATH': '/nonexistent'}, clear=True)
+    def test_test_sdk_exists_with_valid_apksigner(self):
+        apksigner = (
+            Path(self.testdir)
+            / 'build-tools'
+            / fdroidserver.common.MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION
+            / 'apksigner'
+        )
+        apksigner.parent.mkdir(parents=True)
+        apksigner.write_text('#!/bin/sh\ndate\n')
+        apksigner.chmod(0o0755)
+        config = {'sdk_path': self.testdir}
+        self.assertTrue(fdroidserver.common.test_sdk_exists(config))
+
+    @mock.patch.dict(os.environ, {'PATH': '/nonexistent'}, clear=True)
+    def test_test_sdk_exists_with_old_apksigner(self):
+        apksigner = Path(self.testdir) / 'build-tools' / '17.0.0' / 'apksigner'
+        apksigner.parent.mkdir(parents=True)
+        apksigner.write_text('#!/bin/sh\ndate\n')
+        apksigner.chmod(0o0755)
+        config = {'sdk_path': self.testdir}
+        self.assertFalse(fdroidserver.common.test_sdk_exists(config))
+
     def test_loading_config_buildserver_yml(self):
         """Smoke check to make sure this file is properly parsed"""
         os.chdir(self.tmpdir)