From 031ae1103e23731ccd0cb2d82ab140917122b110 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Feb 2025 19:48:22 +0100 Subject: [PATCH 1/8] function-local imports to limit deps for publish/signindex/gpgsign This eliminates the need to have these installed on the signing server: * python3-qrcode * python3-requests The signing server currently uses a git clone to run _fdroidserver_ and `apt-get install` for dependencies. This leaves "qrcode" in "install_requires" since moving it to "extras_require" would break `fdroid update` and `fdroid nightly` for anything that does `pip install fdroidserver`: https://gitlab.com/eighthave/fdroidserver/-/jobs/9386520037 --- fdroidserver/index.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/fdroidserver/index.py b/fdroidserver/index.py index 8ce2f8e8..0beba4ef 100644 --- a/fdroidserver/index.py +++ b/fdroidserver/index.py @@ -20,6 +20,16 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +"""Process the index files. + +This module is loaded by all fdroid subcommands since it is loaded in +fdroidserver/__init__.py. Any narrowly used dependencies should be +imported where they are used to limit dependencies for subcommands +like publish/signindex/gpgsign. This eliminates the need to have +these installed on the signing server. + +""" + import collections import hashlib import json @@ -32,7 +42,6 @@ import tempfile import urllib.parse import zipfile import calendar -import qrcode from binascii import hexlify, unhexlify from datetime import datetime, timezone from pathlib import Path @@ -41,7 +50,6 @@ from xml.dom.minidom import Document from . import _ from . import common from . import metadata -from . import net from . import signindex from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME, CONFIG_CONFIG_NAME, MIRRORS_CONFIG_NAME, RELEASECHANNELS_CONFIG_NAME, DEFAULT_LOCALE, FDroidPopen, FDroidPopenBytes, load_stats_fdroid_signing_key_fingerprints from fdroidserver._yaml import yaml @@ -160,6 +168,7 @@ def make_website(apps, repodir, repodict): html_file = os.path.join(repodir, html_name) if _should_file_be_generated(html_file, autogenerate_comment): + import qrcode qrcode.make(link_fingerprinted).save(os.path.join(repodir, "index.png")) with open(html_file, 'w') as f: name = repodict["name"] @@ -1378,7 +1387,15 @@ def make_v0(apps, apks, repodir, repodict, requestsdict, fdroid_signing_key_fing % repo_icon) os.makedirs(os.path.dirname(iconfilename), exist_ok=True) try: + import qrcode + qrcode.make(common.config['repo_url']).save(iconfilename) + except ModuleNotFoundError as e: + raise ModuleNotFoundError( + _( + 'The "qrcode" Python package is not installed (e.g. apt-get install python3-qrcode)!' + ) + ) from e except Exception: exampleicon = os.path.join(common.get_examples_dir(), common.default_config['repo_icon']) @@ -1624,6 +1641,8 @@ def download_repo_index_v1(url_str, etag=None, verify_fingerprint=True, timeout= - The new eTag as returned by the HTTP request """ + from . import net + url = urllib.parse.urlsplit(url_str) fingerprint = None @@ -1675,6 +1694,8 @@ def download_repo_index_v2(url_str, etag=None, verify_fingerprint=True, timeout= - The new eTag as returned by the HTTP request """ + from . import net + etag # etag is unused but needs to be there to keep the same API as the earlier functions. url = urllib.parse.urlsplit(url_str) From 081e02c10947fda58f59ca780178453398c41da7 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 27 Feb 2025 15:48:58 +0100 Subject: [PATCH 2/8] expand {env: foo} in any place a string can be `keypass: {env: keypass}` has been in use in production repos for years. That is not anything new. It makes it possible to maintain _config.yml_ publicly even when it needs secrets. This change makes sure it is possible to use {env: foo} syntax anywhere where a string value is valid. The "list of dicts" values can be str, list of str or list of dicts with str. Before the {env: keypass} syntax, the actual password was just inline in the config file. Before this commit, it was only possible to use {env: key} syntax in simple, string-only configs, e.g. from examples/config.yml: --- fdroidserver/common.py | 39 +++++++++++++++++---- tests/test_common.py | 77 ++++++++++++++++++++++++++++++++++++++++++ tests/test_lint.py | 7 +++- 3 files changed, 116 insertions(+), 7 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 80806c6e..06ac9cf9 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -642,16 +642,43 @@ def read_config(): return config +def expand_env_dict(s): + """Expand env var dict to a string value. + + {env: varName} syntax can be used to replace any string value in the + config with the value of an environment variable "varName". This + allows for secrets management when commiting the config file to a + public git repo. + + """ + if not s or type(s) not in (str, dict): + return + if isinstance(s, dict): + if 'env' not in s or len(s) > 1: + raise TypeError(_('Only accepts a single key "env"')) + var = s['env'] + s = os.getenv(var) + if not s: + logging.error( + _('Environment variable {{env: {var}}} is not set!').format(var=var) + ) + return + return os.path.expanduser(s) + + def parse_mirrors_config(mirrors): """Mirrors can be specified as a string, list of strings, or dictionary map.""" if isinstance(mirrors, str): - return [{"url": mirrors}] - elif all(isinstance(item, str) for item in mirrors): - return [{'url': i} for i in mirrors] - elif all(isinstance(item, dict) for item in mirrors): + return [{"url": expand_env_dict(mirrors)}] + if isinstance(mirrors, dict): + return [{"url": expand_env_dict(mirrors)}] + if all(isinstance(item, str) for item in mirrors): + return [{'url': expand_env_dict(i)} for i in mirrors] + if all(isinstance(item, dict) for item in mirrors): + for item in mirrors: + item['url'] = expand_env_dict(item['url']) return mirrors - else: - raise TypeError(_('only accepts strings, lists, and tuples')) + raise TypeError(_('only accepts strings, lists, and tuples')) def get_mirrors(url, filename=None): diff --git a/tests/test_common.py b/tests/test_common.py index 3513bf53..bbdaa016 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -2122,6 +2122,27 @@ class CommonTest(unittest.TestCase): ) fdroidserver.common.read_config() + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_config_with_env_string(self): + """Test whether env works in keys with string values.""" + os.chdir(self.testdir) + testvalue = 'this is just a test' + Path('config.yml').write_text('keypass: {env: foo}') + os.environ['foo'] = testvalue + self.assertEqual(testvalue, fdroidserver.common.get_config()['keypass']) + + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_config_with_env_path(self): + """Test whether env works in keys with path values.""" + os.chdir(self.testdir) + path = 'user@server:/path/to/bar/' + os.environ['foo'] = path + Path('config.yml').write_text('serverwebroot: {env: foo}') + self.assertEqual( + [{'url': path}], + fdroidserver.common.get_config()['serverwebroot'], + ) + def test_setup_status_output(self): os.chdir(self.tmpdir) start_timestamp = time.gmtime() @@ -2847,6 +2868,41 @@ class CommonTest(unittest.TestCase): fdroidserver.common.read_config()['serverwebroot'], ) + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_config_serverwebroot_list_of_dicts_env(self): + os.chdir(self.testdir) + url = 'foo@example.com:/var/www/' + os.environ['serverwebroot'] = url + fdroidserver.common.write_config_file( + textwrap.dedent( + """\ + serverwebroot: + - url: {env: serverwebroot} + index_only: true + """ + ) + ) + self.assertEqual( + [{'url': url, 'index_only': True}], + fdroidserver.common.read_config()['serverwebroot'], + ) + + def test_expand_env_dict_fake_str(self): + testvalue = '"{env: foo}"' + self.assertEqual(testvalue, fdroidserver.common.expand_env_dict(testvalue)) + + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_expand_env_dict_good(self): + name = 'foo' + value = 'bar' + os.environ[name] = value + self.assertEqual(value, fdroidserver.common.expand_env_dict({'env': name})) + + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_expand_env_dict_bad_dict(self): + with self.assertRaises(TypeError): + fdroidserver.common.expand_env_dict({'env': 'foo', 'foo': 'bar'}) + def test_parse_mirrors_config_str(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""'%s'""" % s) @@ -2868,6 +2924,27 @@ class CommonTest(unittest.TestCase): [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) ) + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH'), 'foo': 'bar'}, clear=True) + def test_parse_mirrors_config_env_str(self): + mirrors = yaml.load('{env: foo}') + self.assertEqual( + [{'url': 'bar'}], fdroidserver.common.parse_mirrors_config(mirrors) + ) + + def test_parse_mirrors_config_env_list(self): + s = 'foo@example.com:/var/www' + mirrors = yaml.load("""- '%s'""" % s) + self.assertEqual( + [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + ) + + def test_parse_mirrors_config_env_dict(self): + s = 'foo@example.com:/var/www' + mirrors = yaml.load("""- url: '%s'""" % s) + self.assertEqual( + [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + ) + def test_KnownApks_recordapk(self): """Test that added dates are being fetched from the index. diff --git a/tests/test_lint.py b/tests/test_lint.py index c9e7b3f4..95752cb9 100755 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -550,7 +550,12 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text('sdk_path: /opt/android-sdk\n') self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) - def test_config_yml_str_dict(self): + def test_config_yml_str_list_of_dicts_env(self): + """serverwebroot can be str, list of str, or list of dicts.""" + self.config_yml.write_text('serverwebroot: {env: ANDROID_HOME}\n') + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_str_env(self): self.config_yml.write_text('sdk_path: {env: ANDROID_HOME}\n') self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) From 8cf1297e2c939729a01f463cbda8772b9593e684 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 6 Mar 2025 13:15:21 +0100 Subject: [PATCH 3/8] clarify config data types and structures --- fdroidserver/common.py | 66 +++++++++++++++++++++++++++++++----------- fdroidserver/lint.py | 2 +- fdroidserver/net.py | 2 +- tests/test_common.py | 24 +++++++-------- tests/test_lint.py | 53 +++++++++++++++++++++++++++++++++ 5 files changed, 116 insertions(+), 31 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 06ac9cf9..b2386396 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -25,8 +25,32 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -# common.py is imported by all modules, so do not import third-party -# libraries here as they will become a requirement for all commands. + +"""Collection of functions shared by subcommands. + +This is basically the "shared library" for all the fdroid subcommands. +The contains core functionality and a number of utility functions. +This is imported by all modules, so do not import third-party +libraries here as they will become a requirement for all commands. + +Config +------ + +Parsing and using the configuration settings from config.yml is +handled here. The data format is YAML 1.2. The config has its own +supported data types: + +* Boolean (e.g. deploy_process_logs:) +* Integer (e.g. archive_older:, repo_maxage:) +* String-only (e.g. repo_name:, sdk_path:) +* Multi-String (string, list of strings, or list of dicts with + strings, e.g. serverwebroot:, mirrors:) + +String-only fields can also use a special value {env: varname}, which +is a dict with a single key 'env' and a value that is the name of the +environment variable to include. + +""" import copy import difflib @@ -586,12 +610,15 @@ def read_config(): fill_config_defaults(config) if 'serverwebroot' in config: - roots = parse_mirrors_config(config['serverwebroot']) + roots = parse_list_of_dicts(config['serverwebroot']) rootlist = [] for d in roots: # since this is used with rsync, where trailing slashes have # meaning, ensure there is always a trailing slash - rootstr = d['url'] + rootstr = d.get('url') + if not rootstr: + logging.error('serverwebroot: has blank value!') + continue if rootstr[-1] != '/': rootstr += '/' d['url'] = rootstr.replace('//', '/') @@ -599,7 +626,7 @@ def read_config(): config['serverwebroot'] = rootlist if 'servergitmirrors' in config: - config['servergitmirrors'] = parse_mirrors_config(config['servergitmirrors']) + config['servergitmirrors'] = parse_list_of_dicts(config['servergitmirrors']) limit = config['git_mirror_size_limit'] config['git_mirror_size_limit'] = parse_human_readable_size(limit) @@ -666,18 +693,23 @@ def expand_env_dict(s): return os.path.expanduser(s) -def parse_mirrors_config(mirrors): - """Mirrors can be specified as a string, list of strings, or dictionary map.""" - if isinstance(mirrors, str): - return [{"url": expand_env_dict(mirrors)}] - if isinstance(mirrors, dict): - return [{"url": expand_env_dict(mirrors)}] - if all(isinstance(item, str) for item in mirrors): - return [{'url': expand_env_dict(i)} for i in mirrors] - if all(isinstance(item, dict) for item in mirrors): - for item in mirrors: +def parse_list_of_dicts(l_of_d): + """Parse config data structure that is a list of dicts of strings. + + The value can be specified as a string, list of strings, or list of dictionary maps + where the values are strings. + + """ + if isinstance(l_of_d, str): + return [{"url": expand_env_dict(l_of_d)}] + if isinstance(l_of_d, dict): + return [{"url": expand_env_dict(l_of_d)}] + if all(isinstance(item, str) for item in l_of_d): + return [{'url': expand_env_dict(i)} for i in l_of_d] + if all(isinstance(item, dict) for item in l_of_d): + for item in l_of_d: item['url'] = expand_env_dict(item['url']) - return mirrors + return l_of_d raise TypeError(_('only accepts strings, lists, and tuples')) @@ -690,7 +722,7 @@ def get_mirrors(url, filename=None): if url.netloc == 'f-droid.org': mirrors = FDROIDORG_MIRRORS else: - mirrors = parse_mirrors_config(url.geturl()) + mirrors = parse_list_of_dicts(url.geturl()) if filename: return append_filename_to_mirrors(filename, mirrors) diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index 4e62a404..ce541c4d 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -899,7 +899,7 @@ def lint_config(arg): show_error = False if t is str: - if type(data[key]) not in (str, dict): + if type(data[key]) not in (str, list, dict): passed = False show_error = True elif type(data[key]) != t: diff --git a/fdroidserver/net.py b/fdroidserver/net.py index 5c6e0144..1ec7d096 100644 --- a/fdroidserver/net.py +++ b/fdroidserver/net.py @@ -92,7 +92,7 @@ def download_using_mirrors(mirrors, local_filename=None): logic will try it twice: first without SNI, then again with SNI. """ - mirrors = common.parse_mirrors_config(mirrors) + mirrors = common.parse_list_of_dicts(mirrors) mirror_configs_to_try = [] for mirror in mirrors: mirror_configs_to_try.append(mirror) diff --git a/tests/test_common.py b/tests/test_common.py index bbdaa016..c6f90890 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -2903,46 +2903,46 @@ class CommonTest(unittest.TestCase): with self.assertRaises(TypeError): fdroidserver.common.expand_env_dict({'env': 'foo', 'foo': 'bar'}) - def test_parse_mirrors_config_str(self): + def test_parse_list_of_dicts_str(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""'%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_list(self): + def test_parse_list_of_dicts_list(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_dict(self): + def test_parse_list_of_dicts_dict(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- url: '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH'), 'foo': 'bar'}, clear=True) - def test_parse_mirrors_config_env_str(self): + def test_parse_list_of_dicts_env_str(self): mirrors = yaml.load('{env: foo}') self.assertEqual( - [{'url': 'bar'}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': 'bar'}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_env_list(self): + def test_parse_list_of_dicts_env_list(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) - def test_parse_mirrors_config_env_dict(self): + def test_parse_list_of_dicts_env_dict(self): s = 'foo@example.com:/var/www' mirrors = yaml.load("""- url: '%s'""" % s) self.assertEqual( - [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors) + [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors) ) def test_KnownApks_recordapk(self): diff --git a/tests/test_lint.py b/tests/test_lint.py index 95752cb9..5a6a1001 100755 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -4,6 +4,7 @@ import logging import os import shutil import tempfile +import textwrap import unittest from pathlib import Path @@ -534,6 +535,13 @@ class LintAntiFeaturesTest(unittest.TestCase): class ConfigYmlTest(LintTest): + """Test data formats used in config.yml. + + lint.py uses print() and not logging so hacks are used to control + the output when running in the test runner. + + """ + def setUp(self): super().setUp() self.config_yml = Path(self.testdir) / fdroidserver.common.CONFIG_FILE @@ -550,6 +558,22 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text('sdk_path: /opt/android-sdk\n') self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + def test_config_yml_str_list(self): + self.config_yml.write_text('serverwebroot: [server1, server2]\n') + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_str_list_of_dicts(self): + self.config_yml.write_text( + textwrap.dedent( + """\ + serverwebroot: + - url: 'me@b.az:/srv/fdroid' + index_only: true + """ + ) + ) + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + def test_config_yml_str_list_of_dicts_env(self): """serverwebroot can be str, list of str, or list of dicts.""" self.config_yml.write_text('serverwebroot: {env: ANDROID_HOME}\n') @@ -595,3 +619,32 @@ class ConfigYmlTest(LintTest): fdroidserver.lint.lint_config(self.config_yml), f'{key} should fail on value of "{value}"', ) + + def test_config_yml_keyaliases(self): + self.config_yml.write_text( + textwrap.dedent( + """\ + keyaliases: + com.example: myalias + com.foo: '@com.example' + """ + ) + ) + self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_keyaliases_bad_str(self): + """The keyaliases: value is a dict not a str.""" + self.config_yml.write_text("keyaliases: '@com.example'\n") + self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) + + def test_config_yml_keyaliases_bad_list(self): + """The keyaliases: value is a dict not a list.""" + self.config_yml.write_text( + textwrap.dedent( + """\ + keyaliases: + - com.example: myalias + """ + ) + ) + self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) From f269232b9661dadfc19f115c75e3b9aa281dc13d Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 27 Feb 2025 16:11:12 +0100 Subject: [PATCH 4/8] hide error messages in tests that are meant to fail --- tests/test_lint.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_lint.py b/tests/test_lint.py index 5a6a1001..f65f3297 100755 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -7,6 +7,7 @@ import tempfile import textwrap import unittest from pathlib import Path +from unittest import mock from .shared_test_code import mkdtemp @@ -550,6 +551,7 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text('repo_maxage: 1\n') self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_int_bad(self): self.config_yml.write_text('repo_maxage: "1"\n') self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) @@ -583,6 +585,7 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text('sdk_path: {env: ANDROID_HOME}\n') self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_str_bad(self): self.config_yml.write_text('sdk_path: 1.0\n') self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) @@ -591,6 +594,7 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text("deploy_process_logs: true\n") self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_bool_bad(self): self.config_yml.write_text('deploy_process_logs: 2342fe23\n') self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) @@ -599,14 +603,17 @@ class ConfigYmlTest(LintTest): self.config_yml.write_text("keyaliases: {com.example: '@com.foo'}\n") self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_dict_bad(self): self.config_yml.write_text('keyaliases: 2342fe23\n') self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_bad_key_name(self): self.config_yml.write_text('keyalias: 2342fe23\n') self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_bad_value_for_all_keys(self): """Check all config keys with a bad value.""" for key in fdroidserver.lint.check_config_keys: @@ -632,11 +639,13 @@ class ConfigYmlTest(LintTest): ) self.assertTrue(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_keyaliases_bad_str(self): """The keyaliases: value is a dict not a str.""" self.config_yml.write_text("keyaliases: '@com.example'\n") self.assertFalse(fdroidserver.lint.lint_config(self.config_yml)) + @mock.patch('builtins.print', mock.Mock()) # hide error message def test_config_yml_keyaliases_bad_list(self): """The keyaliases: value is a dict not a list.""" self.config_yml.write_text( From 36007d50e5caca7a58a915354b7d5ee3f53778bd Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 6 Mar 2025 11:13:37 +0100 Subject: [PATCH 5/8] AbstractBaseTest class for sharing setUp and tearDown in tests --- tests/test_common.py | 10 +++++++--- tests/test_lint.py | 8 ++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/test_common.py b/tests/test_common.py index c6f90890..cbbba221 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -46,8 +46,8 @@ def _mock_common_module_options_instance(): fdroidserver.common.options.verbose = False -class CommonTest(unittest.TestCase): - '''fdroidserver/common.py''' +class SetUpTearDownMixin: + """A mixin with no tests in it for shared setUp and tearDown.""" def setUp(self): logging.basicConfig(level=logging.DEBUG) @@ -77,6 +77,10 @@ class CommonTest(unittest.TestCase): if os.path.exists(self.tmpdir): shutil.rmtree(self.tmpdir) + +class CommonTest(SetUpTearDownMixin, unittest.TestCase): + '''fdroidserver/common.py''' + def test_yaml_1_2(self): """Return a ruamel.yaml instance that supports YAML 1.2 @@ -3307,7 +3311,7 @@ class SignerExtractionTest(unittest.TestCase): ) -class IgnoreApksignerV33Test(CommonTest): +class IgnoreApksignerV33Test(SetUpTearDownMixin, unittest.TestCase): """apksigner v33 should be entirely ignored https://gitlab.com/fdroid/fdroidserver/-/issues/1253 diff --git a/tests/test_lint.py b/tests/test_lint.py index f65f3297..f0bf6b4d 100755 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -19,8 +19,8 @@ from fdroidserver._yaml import config_dump basedir = Path(__file__).parent -class LintTest(unittest.TestCase): - '''fdroidserver/lint.py''' +class SetUpTearDownMixin: + """A base class with no test in it for shared setUp and tearDown.""" def setUp(self): os.chdir(basedir) @@ -33,6 +33,10 @@ class LintTest(unittest.TestCase): def tearDown(self): self._td.cleanup() + +class LintTest(SetUpTearDownMixin, unittest.TestCase): + '''fdroidserver/lint.py''' + def test_check_for_unsupported_metadata_files(self): self.assertTrue(fdroidserver.lint.check_for_unsupported_metadata_files()) From 858068c64b279ef17113ea2b59ad852e38e7d624 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 5 Mar 2025 12:22:22 +0100 Subject: [PATCH 6/8] only show "unsafe perms on config.yml" when secrets are present This should make for fewer false positives. --- fdroidserver/common.py | 21 +++++++++-------- tests/test_common.py | 51 +++++++++++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index b2386396..2a63803f 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -598,15 +598,6 @@ def read_config(): 'sun.security.pkcs11.SunPKCS11', '-providerArg', 'opensc-fdroid.cfg'] - if any(k in config for k in ["keystore", "keystorepass", "keypass"]): - st = os.stat(CONFIG_FILE) - if st.st_mode & stat.S_IRWXG or st.st_mode & stat.S_IRWXO: - logging.warning( - _("unsafe permissions on '{config_file}' (should be 0600)!").format( - config_file=CONFIG_FILE - ) - ) - fill_config_defaults(config) if 'serverwebroot' in config: @@ -666,6 +657,18 @@ def read_config(): for configname in confignames_to_delete: del config[configname] + if any( + k in config and config.get(k) + for k in ["awssecretkey", "keystorepass", "keypass"] + ): + st = os.stat(CONFIG_FILE) + if st.st_mode & stat.S_IRWXG or st.st_mode & stat.S_IRWXO: + logging.warning( + _("unsafe permissions on '{config_file}' (should be 0600)!").format( + config_file=CONFIG_FILE + ) + ) + return config diff --git a/tests/test_common.py b/tests/test_common.py index cbbba221..293d69ec 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -1964,16 +1964,6 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): with self.assertRaises(ruamel.yaml.scanner.ScannerError): fdroidserver.common.read_config() - def test_config_perm_warning(self): - """Exercise the code path that issues a warning about unsafe permissions.""" - os.chdir(self.tmpdir) - fdroidserver.common.write_config_file('keystore: foo.jks') - self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE)) - os.chmod(fdroidserver.common.CONFIG_FILE, 0o666) # nosec B103 - fdroidserver.common.read_config() - os.remove(fdroidserver.common.CONFIG_FILE) - fdroidserver.common.config = None - def test_config_repo_url(self): """repo_url ends in /repo, archive_url ends in /archive.""" os.chdir(self.tmpdir) @@ -3444,3 +3434,44 @@ class ConfigOptionsScopeTest(unittest.TestCase): 'config' not in vars() and 'config' not in globals(), "The config should not be set in the global context, only module-level.", ) + + +class UnsafePermissionsTest(SetUpTearDownMixin, unittest.TestCase): + def setUp(self): + config = dict() + fdroidserver.common.find_apksigner(config) + if not config.get('apksigner'): + self.skipTest('SKIPPING, apksigner not installed!') + + super().setUp() + os.chdir(self.testdir) + fdroidserver.common.write_config_file('keypass: {env: keypass}') + os.chmod(fdroidserver.common.CONFIG_FILE, 0o666) # nosec B103 + + def test_config_perm_no_warning(self): + fdroidserver.common.write_config_file('keystore: foo.jks') + with self.assertNoLogs(level=logging.WARNING): + fdroidserver.common.read_config() + + def test_config_perm_keypass_warning(self): + fdroidserver.common.write_config_file('keypass: supersecret') + with self.assertLogs(level=logging.WARNING) as lw: + fdroidserver.common.read_config() + self.assertTrue('unsafe' in lw.output[0]) + + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_config_perm_env_warning(self): + os.environ['keypass'] = 'supersecret' + fdroidserver.common.write_config_file('keypass: {env: keypass}') + with self.assertLogs(level=logging.WARNING) as lw: + fdroidserver.common.read_config() + self.assertTrue('unsafe' in lw.output[0]) + self.assertEqual(1, len(lw.output)) + + @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True) + def test_config_perm_unset_env_no_warning(self): + fdroidserver.common.write_config_file('keypass: {env: keypass}') + with self.assertLogs(level=logging.WARNING) as lw: + fdroidserver.common.read_config() + self.assertTrue('unsafe' not in lw.output[0]) + self.assertEqual(1, len(lw.output)) From 9d147c6b6b4ca019329db656100b902c1f3c228b Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 6 Mar 2025 13:03:19 +0100 Subject: [PATCH 7/8] test_common: remove self.tmpdir and use standard self.testdir pat --- tests/test_common.py | 147 ++++++++++++++++++++----------------------- 1 file changed, 69 insertions(+), 78 deletions(-) diff --git a/tests/test_common.py b/tests/test_common.py index 293d69ec..51d9a02a 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -4,7 +4,6 @@ import difflib import git import glob import importlib -import inspect import json import logging import os @@ -29,7 +28,7 @@ import fdroidserver import fdroidserver.signindex import fdroidserver.common import fdroidserver.metadata -from .shared_test_code import TmpCwd, mkdtemp +from .shared_test_code import TmpCwd, mkdtemp, mkdir_testfiles from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME from fdroidserver._yaml import yaml, yaml_dumper, config_dump from fdroidserver.exception import FDroidException, VCSException,\ @@ -53,9 +52,6 @@ class SetUpTearDownMixin: logging.basicConfig(level=logging.DEBUG) logger = logging.getLogger('androguard.axml') logger.setLevel(logging.INFO) # tame the axml debug messages - self.tmpdir = os.path.abspath(os.path.join(basedir, '..', '.testfiles')) - if not os.path.exists(self.tmpdir): - os.makedirs(self.tmpdir) os.chdir(basedir) self.verbose = '-v' in sys.argv or '--verbose' in sys.argv @@ -66,16 +62,14 @@ class SetUpTearDownMixin: fdroidserver.common.options = None fdroidserver.metadata.srclibs = None - self._td = mkdtemp() - self.testdir = self._td.name + self.testdir = mkdir_testfiles(basedir, self) def tearDown(self): fdroidserver.common.config = None fdroidserver.common.options = None os.chdir(basedir) - self._td.cleanup() - if os.path.exists(self.tmpdir): - shutil.rmtree(self.tmpdir) + if os.path.exists(self.testdir): + shutil.rmtree(self.testdir) class CommonTest(SetUpTearDownMixin, unittest.TestCase): @@ -178,7 +172,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): print('no build-tools found: ' + build_tools) def test_find_java_root_path(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) all_pathlists = [ ( @@ -310,11 +304,11 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): shutil.copytree( os.path.join(basedir, 'source-files'), - os.path.join(self.tmpdir, 'source-files'), + os.path.join(self.testdir, 'source-files'), ) fdroidclient_testdir = os.path.join( - self.tmpdir, 'source-files', 'fdroid', 'fdroidclient' + self.testdir, 'source-files', 'fdroid', 'fdroidclient' ) config = dict() @@ -425,7 +419,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_prepare_sources_refresh(self): _mock_common_module_options_instance() packageName = 'org.fdroid.ci.test.app' - os.chdir(self.tmpdir) + os.chdir(self.testdir) os.mkdir('build') os.mkdir('metadata') @@ -443,7 +437,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): with open(os.path.join('metadata', packageName + '.yml'), 'w') as fp: yaml_dumper.dump(metadata, fp) - gitrepo = os.path.join(self.tmpdir, 'build', packageName) + gitrepo = os.path.join(self.testdir, 'build', packageName) vcs0 = fdroidserver.common.getvcs('git', git_url, gitrepo) vcs0.gotorevision('0.3', refresh=True) vcs1 = fdroidserver.common.getvcs('git', git_url, gitrepo) @@ -512,18 +506,15 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): fdroidserver.signindex.config = config sourcedir = os.path.join(basedir, 'signindex') - with tempfile.TemporaryDirectory( - prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir - ) as testsdir: - for f in ('testy.jar', 'guardianproject.jar'): - sourcefile = os.path.join(sourcedir, f) - testfile = os.path.join(testsdir, f) - shutil.copy(sourcefile, testsdir) - fdroidserver.signindex.sign_jar(testfile, use_old_algs=True) - # these should be resigned, and therefore different - self.assertNotEqual( - open(sourcefile, 'rb').read(), open(testfile, 'rb').read() - ) + for f in ('testy.jar', 'guardianproject.jar'): + sourcefile = os.path.join(sourcedir, f) + testfile = os.path.join(self.testdir, f) + shutil.copy(sourcefile, self.testdir) + fdroidserver.signindex.sign_jar(testfile, use_old_algs=True) + # these should be resigned, and therefore different + self.assertNotEqual( + open(sourcefile, 'rb').read(), open(testfile, 'rb').read() + ) def test_verify_apk_signature(self): _mock_common_module_options_instance() @@ -622,7 +613,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): shutil.copy(sourceapk, copyapk) self.assertTrue(fdroidserver.common.verify_apk_signature(copyapk)) self.assertIsNone( - fdroidserver.common.verify_apks(sourceapk, copyapk, self.tmpdir) + fdroidserver.common.verify_apks(sourceapk, copyapk, self.testdir) ) unsignedapk = os.path.join(self.testdir, 'urzip-unsigned.apk') @@ -632,7 +623,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): if not info.filename.startswith('META-INF/'): testapk.writestr(info, apk.read(info.filename)) self.assertIsNone( - fdroidserver.common.verify_apks(sourceapk, unsignedapk, self.tmpdir) + fdroidserver.common.verify_apks(sourceapk, unsignedapk, self.testdir) ) twosigapk = os.path.join(self.testdir, 'urzip-twosig.apk') @@ -645,7 +636,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): testapk.writestr(info.filename, otherapk.read(info.filename)) otherapk.close() self.assertFalse(fdroidserver.common.verify_apk_signature(twosigapk)) - self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, twosigapk, self.tmpdir)) + self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, twosigapk, self.testdir)) def test_get_certificate_with_chain_sandisk(self): """Test that APK signatures with a cert chain are parsed like apksigner. @@ -825,14 +816,14 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_find_apksigner_config_overrides(self): """apksigner should come from config before any auto-detection""" - os.chdir(self.tmpdir) - android_home = os.path.join(self.tmpdir, 'ANDROID_HOME') + os.chdir(self.testdir) + android_home = os.path.join(self.testdir, 'ANDROID_HOME') do_not_use = os.path.join(android_home, 'build-tools', '30.0.3', 'apksigner') os.makedirs(os.path.dirname(do_not_use)) with open(do_not_use, 'w') as fp: fp.write('#!/bin/sh\ndate\n') os.chmod(do_not_use, 0o0755) # nosec B103 - apksigner = os.path.join(self.tmpdir, 'apksigner') + apksigner = os.path.join(self.testdir, 'apksigner') config = {'apksigner': apksigner} with mock.patch.dict(os.environ, clear=True): os.environ['ANDROID_HOME'] = android_home @@ -842,13 +833,13 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_find_apksigner_prefer_path(self): """apksigner should come from PATH before ANDROID_HOME""" - os.chdir(self.tmpdir) - apksigner = os.path.join(self.tmpdir, 'apksigner') + os.chdir(self.testdir) + apksigner = os.path.join(self.testdir, 'apksigner') with open(apksigner, 'w') as fp: fp.write('#!/bin/sh\ndate\n') os.chmod(apksigner, 0o0755) # nosec B103 - android_home = os.path.join(self.tmpdir, 'ANDROID_HOME') + android_home = os.path.join(self.testdir, 'ANDROID_HOME') do_not_use = os.path.join(android_home, 'build-tools', '30.0.3', 'apksigner') os.makedirs(os.path.dirname(do_not_use)) with open(do_not_use, 'w') as fp: @@ -864,8 +855,8 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_find_apksigner_prefer_newest(self): """apksigner should be the newest available in ANDROID_HOME""" - os.chdir(self.tmpdir) - android_home = os.path.join(self.tmpdir, 'ANDROID_HOME') + os.chdir(self.testdir) + android_home = os.path.join(self.testdir, 'ANDROID_HOME') apksigner = os.path.join(android_home, 'build-tools', '30.0.3', 'apksigner') os.makedirs(os.path.dirname(apksigner)) @@ -887,7 +878,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_find_apksigner_system_package_android_home(self): """Test that apksigner v30 or newer is found""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) android_home = os.getenv('ANDROID_HOME') if not android_home or not os.path.isdir(android_home): self.skipTest('SKIPPING since ANDROID_HOME (%s) is not a dir!' % android_home) @@ -1049,7 +1040,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): fdroidserver.common.config = config fdroidserver.signindex.config = config - os.chdir(self.tmpdir) + os.chdir(self.testdir) os.mkdir('unsigned') os.mkdir('repo') @@ -1129,8 +1120,8 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): """get_apk_id should never return None on error, only raise exceptions""" with self.assertRaises(KeyError): fdroidserver.common.get_apk_id('Norway_bouvet_europe_2.obf.zip') - shutil.copy('Norway_bouvet_europe_2.obf.zip', self.tmpdir) - os.chdir(self.tmpdir) + shutil.copy('Norway_bouvet_europe_2.obf.zip', self.testdir) + os.chdir(self.testdir) with ZipFile('Norway_bouvet_europe_2.obf.zip', 'a') as zipfp: zipfp.writestr('AndroidManifest.xml', 'not a manifest') with self.assertRaises(KeyError): @@ -1147,7 +1138,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): ) def test_get_apk_id_bad_zip(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) badzip = 'badzip.apk' with open(badzip, 'w') as fp: fp.write('not a ZIP') @@ -1563,9 +1554,9 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_remove_signing_keys(self): shutil.copytree( os.path.join(basedir, 'source-files'), - os.path.join(self.tmpdir, 'source-files'), + os.path.join(self.testdir, 'source-files'), ) - os.chdir(self.tmpdir) + os.chdir(self.testdir) with_signingConfigs = [ 'source-files/com.seafile.seadroid2/app/build.gradle', 'source-files/eu.siacs.conversations/build.gradle', @@ -1734,11 +1725,11 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): self.assertEqual(f.read(), mocklogcontent) def test_deploy_status_json(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) fakesubcommand = 'fakesubcommand' fake_timestamp = 1234567890 fakeserver = 'example.com:/var/www/fbot/' - expected_dir = os.path.join(self.tmpdir, fakeserver.replace(':', ''), 'repo', 'status') + expected_dir = os.path.join(self.testdir, fakeserver.replace(':', ''), 'repo', 'status') fdroidserver.common.options = mock.Mock() fdroidserver.common.config = {} @@ -1746,7 +1737,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): fdroidserver.common.config['identity_file'] = 'ssh/id_rsa' def assert_subprocess_call(cmd): - dest_path = os.path.join(self.tmpdir, cmd[-1].replace(':', '')) + dest_path = os.path.join(self.testdir, cmd[-1].replace(':', '')) if not os.path.exists(dest_path): os.makedirs(dest_path) return subprocess.run(cmd[:-1] + [dest_path]).returncode @@ -1902,14 +1893,14 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_with_no_config(self): """It should set defaults if no config file is found""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) self.assertFalse(os.path.exists(fdroidserver.common.CONFIG_FILE)) config = fdroidserver.common.read_config() self.assertIsNotNone(config.get('char_limits')) def test_with_zero_size_config(self): """It should set defaults if config file has nothing in it""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('') self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE)) config = fdroidserver.common.read_config() @@ -1917,7 +1908,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_with_config_yml(self): """Make sure it is possible to use config.yml alone.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('apksigner: yml') self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE)) config = fdroidserver.common.read_config() @@ -1925,7 +1916,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_with_config_yml_utf8(self): """Make sure it is possible to use config.yml in UTF-8 encoding.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) teststr = '/πÇÇ现代通用字-български-عربي1/ö/yml' fdroidserver.common.write_config_file('apksigner: ' + teststr) self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE)) @@ -1934,7 +1925,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_with_config_yml_utf8_as_ascii(self): """Make sure it is possible to use config.yml Unicode encoded as ASCII.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) teststr = '/πÇÇ现代通用字-български-عربي1/ö/yml' with open(fdroidserver.common.CONFIG_FILE, 'w', encoding='utf-8') as fp: config_dump({'apksigner': teststr}, fp) @@ -1944,7 +1935,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_with_config_yml_with_env_var(self): """Make sure it is possible to use config.yml alone.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) with mock.patch.dict(os.environ): os.environ['SECRET'] = 'mysecretpassword' # nosec B105 fdroidserver.common.write_config_file("""keypass: {'env': 'SECRET'}\n""") @@ -1953,20 +1944,20 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): self.assertEqual(os.getenv('SECRET', 'fail'), config.get('keypass')) def test_with_config_yml_is_dict(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) Path(fdroidserver.common.CONFIG_FILE).write_text('apksigner = /bin/apksigner') with self.assertRaises(TypeError): fdroidserver.common.read_config() def test_with_config_yml_is_not_mixed_type(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) Path(fdroidserver.common.CONFIG_FILE).write_text('k: v\napksigner = /bin/apk') with self.assertRaises(ruamel.yaml.scanner.ScannerError): fdroidserver.common.read_config() def test_config_repo_url(self): """repo_url ends in /repo, archive_url ends in /archive.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file( textwrap.dedent( """\ @@ -1985,34 +1976,34 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): def test_config_repo_url_extra_slash(self): """repo_url ends in /repo, archive_url ends in /archive.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('repo_url: https://MyFirstFDroidRepo.org/fdroid/repo/') with self.assertRaises(FDroidException): fdroidserver.common.read_config() def test_config_repo_url_not_repo(self): """repo_url ends in /repo, archive_url ends in /archive.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('repo_url: https://MyFirstFDroidRepo.org/fdroid/foo') with self.assertRaises(FDroidException): fdroidserver.common.read_config() def test_config_archive_url_extra_slash(self): """repo_url ends in /repo, archive_url ends in /archive.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('archive_url: https://MyFirstFDroidRepo.org/fdroid/archive/') with self.assertRaises(FDroidException): fdroidserver.common.read_config() def test_config_archive_url_not_repo(self): """repo_url ends in /repo, archive_url ends in /archive.""" - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('archive_url: https://MyFirstFDroidRepo.org/fdroid/foo') with self.assertRaises(FDroidException): fdroidserver.common.read_config() def test_write_to_config_yml(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file('apksigner: yml') os.chmod(fdroidserver.common.CONFIG_FILE, 0o0600) self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE)) @@ -2025,7 +2016,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): self.assertEqual('mysecretpassword', config['keypass']) def test_config_dict_with_int_keys(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file( textwrap.dedent( """ @@ -2138,7 +2129,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): ) def test_setup_status_output(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) start_timestamp = time.gmtime() subcommand = 'test' @@ -2154,9 +2145,9 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): self.assertEqual(subcommand, data['subcommand']) def test_setup_status_output_in_git_repo(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) logging.getLogger('git.cmd').setLevel(logging.INFO) - git_repo = git.Repo.init(self.tmpdir) + git_repo = git.Repo.init(self.testdir) file_in_git = 'README.md' with open(file_in_git, 'w') as fp: fp.write('this is just a test') @@ -2410,40 +2401,40 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): @unittest.skip("This test downloads and unzips a 1GB file.") def test_install_ndk(self): """NDK r10e is a special case since its missing source.properties""" - config = {'sdk_path': self.tmpdir} + config = {'sdk_path': self.testdir} fdroidserver.common.config = config fdroidserver.common._install_ndk('r10e') - r10e = os.path.join(self.tmpdir, 'ndk', 'r10e') + r10e = os.path.join(self.testdir, 'ndk', 'r10e') self.assertEqual('r10e', fdroidserver.common.get_ndk_version(r10e)) fdroidserver.common.fill_config_defaults(config) self.assertEqual({'r10e': r10e}, config['ndk_paths']) def test_fill_config_defaults(self): """Test the auto-detection of NDKs installed in standard paths""" - ndk_bundle = os.path.join(self.tmpdir, 'ndk-bundle') + ndk_bundle = os.path.join(self.testdir, 'ndk-bundle') os.makedirs(ndk_bundle) with open(os.path.join(ndk_bundle, 'source.properties'), 'w') as fp: fp.write('Pkg.Desc = Android NDK\nPkg.Revision = 17.2.4988734\n') - config = {'sdk_path': self.tmpdir} + config = {'sdk_path': self.testdir} fdroidserver.common.fill_config_defaults(config) self.assertEqual({'17.2.4988734': ndk_bundle}, config['ndk_paths']) - r21e = os.path.join(self.tmpdir, 'ndk', '21.4.7075529') + r21e = os.path.join(self.testdir, 'ndk', '21.4.7075529') os.makedirs(r21e) with open(os.path.join(r21e, 'source.properties'), 'w') as fp: fp.write('Pkg.Desc = Android NDK\nPkg.Revision = 21.4.7075529\n') - config = {'sdk_path': self.tmpdir} + config = {'sdk_path': self.testdir} fdroidserver.common.fill_config_defaults(config) self.assertEqual( {'17.2.4988734': ndk_bundle, '21.4.7075529': r21e}, config['ndk_paths'], ) - r10e = os.path.join(self.tmpdir, 'ndk', 'r10e') + r10e = os.path.join(self.testdir, 'ndk', 'r10e') os.makedirs(r10e) with open(os.path.join(r10e, 'RELEASE.TXT'), 'w') as fp: fp.write('r10e-rc4 (64-bit)\n') - config = {'sdk_path': self.tmpdir} + config = {'sdk_path': self.testdir} fdroidserver.common.fill_config_defaults(config) self.assertEqual( {'r10e': r10e, '17.2.4988734': ndk_bundle, '21.4.7075529': r21e}, @@ -2453,7 +2444,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): @unittest.skipIf(not os.path.isdir('/usr/lib/jvm/default-java'), 'uses Debian path') def test_fill_config_defaults_java(self): """Test the auto-detection of Java installed in standard paths""" - config = {'sdk_path': self.tmpdir} + config = {'sdk_path': self.testdir} fdroidserver.common.fill_config_defaults(config) java_paths = [] # use presence of javac to make sure its JDK not just JRE @@ -2640,7 +2631,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): self.assertFalse(is_repo_file(d), d + ' not repo file') def test_get_apksigner_smartcardoptions(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) with open(fdroidserver.common.CONFIG_FILE, 'w', encoding='utf-8') as fp: d = { 'smartcardoptions': '-storetype PKCS11' @@ -2668,7 +2659,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): ) def test_get_smartcardoptions_list(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file( textwrap.dedent( """ @@ -2702,7 +2693,7 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase): ) def test_get_smartcardoptions_spaces(self): - os.chdir(self.tmpdir) + os.chdir(self.testdir) fdroidserver.common.write_config_file( textwrap.dedent( """ From d06e33697a999fce3a4f0099a2a3a69c07c8d31e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 12 Mar 2025 12:13:28 +0100 Subject: [PATCH 8/8] lint: missing valid config key: apk_signing_key_block_list --- fdroidserver/lint.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index ce541c4d..6a4299bd 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -236,6 +236,7 @@ bool_keys = ( check_config_keys = ( 'ant', + 'apk_signing_key_block_list', 'archive', 'archive_description', 'archive_icon',