From a97b3ca4dd30999091c8afc4e9ddd4d354ae09ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20P=C3=B6hn?= Date: Wed, 10 Jun 2020 18:15:38 +0200 Subject: [PATCH] implement plugin system review suggestsions --- fdroidserver/__main__.py | 36 ++++++++-------- tests/main.TestCase | 93 +++++++++++++++++++++++++++++++--------- 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/fdroidserver/__main__.py b/fdroidserver/__main__.py index 785179de..770d8c13 100755 --- a/fdroidserver/__main__.py +++ b/fdroidserver/__main__.py @@ -32,7 +32,7 @@ from argparse import ArgumentError from collections import OrderedDict -commands = OrderedDict([ +COMMANDS = OrderedDict([ ("build", _("Build a package from source")), ("init", _("Quickly start a new repository")), ("publish", _("Sign and place packages in the repo")), @@ -57,16 +57,16 @@ commands = OrderedDict([ ]) -def print_help(fdroid_modules=None): +def print_help(available_plugins=None): print(_("usage: ") + _("fdroid [] [-h|--help|--version|]")) print("") print(_("Valid commands are:")) - for cmd, summary in commands.items(): + for cmd, summary in COMMANDS.items(): print(" " + cmd + ' ' * (15 - len(cmd)) + summary) - if fdroid_modules: + if available_plugins: print(_('commands from plugin modules:')) - for command in sorted(fdroid_modules.keys()): - print(' {:15}{}'.format(command, fdroid_modules[command]['summary'])) + for command in sorted(available_plugins.keys()): + print(' {:15}{}'.format(command, available_plugins[command]['summary'])) print("") @@ -109,12 +109,12 @@ def preparse_plugin(module_name, module_dir): def find_plugins(): found_plugins = [{'name': x[1], 'dir': x[0].path} for x in pkgutil.iter_modules() if x[1].startswith('fdroid_')] - commands = {} + plugin_infos = {} for plugin_def in found_plugins: command_name = plugin_def['name'][7:] try: - commands[command_name] = preparse_plugin(plugin_def['name'], - plugin_def['dir']) + plugin_infos[command_name] = preparse_plugin(plugin_def['name'], + plugin_def['dir']) except Exception as e: # We need to keep module lookup fault tolerant because buggy # modules must not prevent fdroidserver from functioning @@ -122,20 +122,20 @@ def find_plugins(): # only raise exeption when a user specifies the broken # plugin in explicitly in command line raise e - return commands + return plugin_infos def main(): - fdroid_modules = find_plugins() + available_plugins = find_plugins() if len(sys.argv) <= 1: - print_help(fdroid_modules=fdroid_modules) + print_help(available_plugins=available_plugins) sys.exit(0) command = sys.argv[1] - if command not in commands and command not in fdroid_modules.keys(): + if command not in COMMANDS and command not in available_plugins.keys(): if command in ('-h', '--help'): - print_help(fdroid_modules=fdroid_modules) + print_help(available_plugins=available_plugins) sys.exit(0) elif command == '--version': output = _('no version info found!') @@ -162,11 +162,11 @@ def main(): else: from pkg_resources import get_distribution output = get_distribution('fdroidserver').version + '\n' - print(output), + print(output) sys.exit(0) else: print(_("Command '%s' not recognised.\n" % command)) - print_help(fdroid_modules=fdroid_modules) + print_help(available_plugins=available_plugins) sys.exit(1) verbose = any(s in sys.argv for s in ['-v', '--verbose']) @@ -196,10 +196,10 @@ def main(): sys.argv[0] += ' ' + command del sys.argv[1] - if command in commands.keys(): + if command in COMMANDS.keys(): mod = __import__('fdroidserver.' + command, None, None, [command]) else: - mod = __import__(fdroid_modules[command]['name'], None, None, [command]) + mod = __import__(available_plugins[command]['name'], None, None, [command]) system_langcode, system_encoding = locale.getdefaultlocale() if system_encoding is None or system_encoding.lower() not in ('utf-8', 'utf8'): diff --git a/tests/main.TestCase b/tests/main.TestCase index 35b80bc2..e05efd95 100755 --- a/tests/main.TestCase +++ b/tests/main.TestCase @@ -24,9 +24,9 @@ import fdroidserver.__main__ class MainTest(unittest.TestCase): '''this tests fdroid.py''' - def test_commands(self): + def test_COMMANDS_check(self): """make sure the built in sub-command defs didn't change unintentionally""" - self.assertListEqual([x for x in fdroidserver.__main__.commands.keys()], + self.assertListEqual([x for x in fdroidserver.__main__.COMMANDS.keys()], ['build', 'init', 'publish', @@ -55,6 +55,8 @@ class MainTest(unittest.TestCase): with mock.patch('fdroidserver.init.main', co): with mock.patch('sys.exit') as exit_mock: fdroidserver.__main__.main() + # note: this is sloppy, if `init` changes + # this might need changing too exit_mock.assert_called_once_with(0) co.assert_called_once_with() @@ -64,51 +66,102 @@ class MainTest(unittest.TestCase): with mock.patch('fdroidserver.server.main', co): with mock.patch('sys.exit') as exit_mock: fdroidserver.__main__.main() + # note: this is sloppy, if `deploy` changes + # this might need changing too exit_mock.assert_called_once_with(0) co.assert_called_once_with() def test_find_plugins(self): with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - with open('fdroid_testy.py', 'w') as f: + with open('fdroid_testy1.py', 'w') as f: f.write(textwrap.dedent("""\ fdroid_summary = "ttt" main = lambda: 'all good'""")) with TmpPyPath(tmpdir): plugins = fdroidserver.__main__.find_plugins() - self.assertIn('testy', plugins.keys()) - self.assertEqual(plugins['testy']['summary'], 'ttt') - self.assertEqual(__import__(plugins['testy']['name'], + self.assertIn('testy1', plugins.keys()) + self.assertEqual(plugins['testy1']['summary'], 'ttt') + self.assertEqual(__import__(plugins['testy1']['name'], None, None, - ['testy']) + ['testy1']) .main(), 'all good') def test_main_plugin_lambda(self): with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - with open('fdroid_testy.py', 'w') as f: + with open('fdroid_testy2.py', 'w') as f: f.write(textwrap.dedent("""\ fdroid_summary = "ttt" - main = lambda: pritn('all good')""")) + main = lambda: print('all good')""")) with TmpPyPath(tmpdir): - with mock.patch('sys.argv', ['', 'testy']): + with mock.patch('sys.argv', ['', 'testy2']): with mock.patch('sys.exit') as exit_mock: fdroidserver.__main__.main() exit_mock.assert_called_once_with(0) def test_main_plugin_def(self): with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - with open('fdroid_testy.py', 'w') as f: + with open('fdroid_testy3.py', 'w') as f: f.write(textwrap.dedent("""\ fdroid_summary = "ttt" def main(): - pritn('all good')""")) + print('all good')""")) with TmpPyPath(tmpdir): - with mock.patch('sys.argv', ['', 'testy']): + with mock.patch('sys.argv', ['', 'testy3']): with mock.patch('sys.exit') as exit_mock: fdroidserver.__main__.main() exit_mock.assert_called_once_with(0) + def test_main_broken_plugin(self): + """making sure broken plugins get their exceptions through""" + with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): + with open('fdroid_testy4.py', 'w') as f: + f.write(textwrap.dedent("""\ + fdroid_summary = "ttt" + def main(): + raise Exception("this plugin is broken")""")) + with TmpPyPath(tmpdir): + with mock.patch('sys.argv', ['', 'testy4']): + with self.assertRaisesRegex(Exception, "this plugin is broken"): + fdroidserver.__main__.main() + + def test_main_malicious_plugin(self): + """The purpose of this test is to make sure code in plugins + doesn't get executed unintentionally. + """ + with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): + with open('fdroid_testy5.py', 'w') as f: + f.write(textwrap.dedent("""\ + fdroid_summary = "ttt" + raise Exception("this plugin is malicious") + def main(): + print("evil things")""")) + with TmpPyPath(tmpdir): + with mock.patch('sys.argv', ['', 'lint']): + with mock.patch('sys.exit') as exit_mock: + fdroidserver.__main__.main() + # note: this is sloppy, if `lint` changes + # this might need changing too + exit_mock.assert_called_once_with(0) + + def test_main_prevent_plugin_override(self): + """making sure build-in subcommands cannot be overridden by plugins + """ + with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): + with open('fdroid_signatures.py', 'w') as f: + f.write(textwrap.dedent("""\ + fdroid_summary = "ttt" + def main(): + raise("plugin overrides don't get prevent!")""")) + with TmpPyPath(tmpdir): + with mock.patch('sys.argv', ['', 'signatures']): + with mock.patch('sys.exit') as exit_mock: + fdroidserver.__main__.main() + # note: this is sloppy, if `signatures` changes + # this might need changing too + self.assertEqual(exit_mock.call_count, 2) + def test_preparse_plugin_lookup_bad_name(self): self.assertRaises(ValueError, fdroidserver.__main__.preparse_plugin, @@ -121,7 +174,7 @@ class MainTest(unittest.TestCase): def test_preparse_plugin_lookup_summary_missing(self): with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - with open('fdroid_testy.py', 'w') as f: + with open('fdroid_testy6.py', 'w') as f: f.write(textwrap.dedent("""\ main = lambda: print('all good')""")) with TmpPyPath(tmpdir): @@ -134,7 +187,7 @@ class MainTest(unittest.TestCase): def test_preparse_plugin_lookup_module_file(self): with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - with open('fdroid_testy.py', 'w') as f: + with open('fdroid_testy7.py', 'w') as f: f.write(textwrap.dedent("""\ fdroid_summary = "ttt" main = lambda: pritn('all good')""")) @@ -143,24 +196,24 @@ class MainTest(unittest.TestCase): module_path = p[0][0].path module_name = p[0][1] d = fdroidserver.__main__.preparse_plugin(module_name, module_path) - self.assertDictEqual(d, {'name': 'fdroid_testy', + self.assertDictEqual(d, {'name': 'fdroid_testy7', 'summary': 'ttt'}) def test_preparse_plugin_lookup_module_dir(self): with tempfile.TemporaryDirectory() as tmpdir, TmpCwd(tmpdir): - os.mkdir(os.path.join(tmpdir, 'fdroid_testy')) - with open('fdroid_testy/__main__.py', 'w') as f: + os.mkdir(os.path.join(tmpdir, 'fdroid_testy8')) + with open('fdroid_testy8/__main__.py', 'w') as f: f.write(textwrap.dedent("""\ fdroid_summary = "ttt" main = lambda: print('all good')""")) - with open('fdroid_testy/__init__.py', 'w') as f: + with open('fdroid_testy8/__init__.py', 'w') as f: pass with TmpPyPath(tmpdir): p = [x for x in pkgutil.iter_modules() if x[1].startswith('fdroid_')] module_path = p[0][0].path module_name = p[0][1] d = fdroidserver.__main__.preparse_plugin(module_name, module_path) - self.assertDictEqual(d, {'name': 'fdroid_testy', + self.assertDictEqual(d, {'name': 'fdroid_testy8', 'summary': 'ttt'})