From e269e41b122feab2d20c45f2599e89dc4d5f1e39 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 12 Mar 2024 08:42:42 +0100 Subject: [PATCH] publish: --error-on-failed to exit when signing/verifying fails Since we have limited visibility into @CiaranG's signing server, it is hard to make changes to the publishing process, especially ones that might break @CiaranG's automation. So `fdroid publish` mostly reports success by moving an APK from unsigned/ to repo/. In some cases, we want immediate failure, like in CI. So this adds `--error-on-failed` for that purpose. --- fdroidserver/publish.py | 26 +++++++++++++ tests/publish.TestCase | 85 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/fdroidserver/publish.py b/fdroidserver/publish.py index 07b9c7b7..b87578ad 100644 --- a/fdroidserver/publish.py +++ b/fdroidserver/publish.py @@ -18,6 +18,16 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +"""Sign APKs using keys or via reproducible builds signature copying. + +This command takes unsigned APKs and signs them. It looks for +unsigned APKs in the unsigned/ directory and puts successfully signed +APKs into the repo/ directory. The default is to run in a kind of +batch mode, where it will only quit on certain kinds of errors. It +mostly reports success by moving an APK from unsigned/ to repo/ + +""" + import sys import os import re @@ -266,6 +276,13 @@ def main(): usage="%(prog)s [options] " "[APPID[:VERCODE] [APPID[:VERCODE] ...]]" ) common.setup_global_opts(parser) + parser.add_argument( + "-e", + "--error-on-failed", + action="store_true", + default=False, + help=_("When signing or verifying fails, exit with an error code."), + ) parser.add_argument( "appid", nargs='*', @@ -322,6 +339,7 @@ def main(): ).format(len(allapps), len(allaliases)) ) + failed = 0 # Process any APKs or ZIPs that are waiting to be signed... for apkfile in sorted( glob.glob(os.path.join(unsigned_dir, '*.apk')) @@ -364,12 +382,14 @@ def main(): if not os.path.isfile(srcapk): logging.error("...reference binary missing - publish skipped: " "'{refpath}'".format(refpath=srcapk)) + failed += 1 else: # Compare our unsigned one with the downloaded one... compare_result = common.verify_apks(srcapk, apkfile, tmp_dir) if compare_result: logging.error("...verification failed - publish skipped : " "{result}".format(result=compare_result)) + failed += 1 else: # Success! So move the downloaded file to the repo, and remove # our built version. @@ -415,6 +435,7 @@ def main(): os.remove(devsignedtmp) logging.error('...verification failed - skipping: %s', devsigned) skipsigning = True + failed += 1 # Now we sign with the F-Droid key. if not skipsigning: @@ -444,6 +465,11 @@ def main(): status_update_json(generated_keys, signed_apks) logging.info('published list signing-key fingerprints') + if failed: + logging.error(_('%d APKs failed to be signed or verified!') % failed) + if options.error_on_failed: + sys.exit(failed) + if __name__ == "__main__": main() diff --git a/tests/publish.TestCase b/tests/publish.TestCase index 7542c5f7..80556744 100755 --- a/tests/publish.TestCase +++ b/tests/publish.TestCase @@ -324,6 +324,91 @@ class PublishTest(unittest.TestCase): self.assertFalse(os.path.exists(unsigned)) self.assertTrue(os.path.exists(signed)) + def test_exit_on_error(self): + """Exits properly on errors, with and without --error-on-failed. + + `fdroid publish` runs on the signing server and does large + batches. In that case, it shouldn't exit after a single + failure since it should try to complete the whole batch. For + CI and other use cases, there is --error-on-failed to force it + to exit after a failure. + + """ + + class Options: + error_on_failed = True + verbose = False + + os.chdir(self.testdir) + + config = common.read_config(Options) + if 'apksigner' not in config: + self.skipTest('SKIPPING test_error_on_failed, apksigner not installed!') + config['repo_keyalias'] = 'sova' + config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI=' + config['keypass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI=' + shutil.copy(os.path.join(self.basedir, 'keystore.jks'), self.testdir) + config['keystore'] = 'keystore.jks' + config[ + 'keydname' + ] = 'CN=Birdman, OU=Cell, O=Alcatraz, L=Alcatraz, S=California, C=US' + publish.config = config + common.config = config + + app = metadata.App() + app.id = 'org.fdroid.ci' + versionCode = 1 + build = metadata.Build( + { + 'versionCode': versionCode, + 'versionName': '1.0', + } + ) + app.Builds = [build] + os.mkdir('metadata') + metadata.write_metadata(os.path.join('metadata', '%s.yml' % app.id), app) + + os.mkdir('unsigned') + testapk = os.path.join(self.basedir, 'no_targetsdk_minsdk1_unsigned.apk') + unsigned = os.path.join('unsigned', common.get_release_filename(app, build)) + signed = os.path.join('repo', common.get_release_filename(app, build)) + shutil.copy(testapk, unsigned) + + # sign the unsigned APK + self.assertTrue(os.path.exists(unsigned)) + self.assertFalse(os.path.exists(signed)) + with mock.patch( + 'sys.argv', ['fdroid publish', '%s:%d' % (app.id, versionCode)] + ): + publish.main() + self.assertFalse(os.path.exists(unsigned)) + self.assertTrue(os.path.exists(signed)) + + with mock.patch('sys.argv', ['fdroid signatures', signed]): + signatures.main() + mf = os.path.join('metadata', 'org.fdroid.ci', 'signatures', '1', 'MANIFEST.MF') + self.assertTrue(os.path.exists(mf)) + os.remove(signed) + + with open(mf, 'a') as fp: + fp.write('appended to break signature') + + # implant the signature into the unsigned APK + shutil.copy(testapk, unsigned) + self.assertTrue(os.path.exists(unsigned)) + self.assertFalse(os.path.exists(signed)) + apk_id = '%s:%d' % (app.id, versionCode) + + # by default, it should complete without exiting + with mock.patch('sys.argv', ['fdroid publish', apk_id]): + publish.main() + + # --error-on-failed should make it exit + with mock.patch('sys.argv', ['fdroid publish', '--error-on-failed', apk_id]): + with self.assertRaises(SystemExit) as e: + publish.main() + self.assertEqual(e.exception.code, 1) + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))