From 9e67e466f8d8e8884ee72132b82d021052066096 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 11 Nov 2023 16:36:18 +0100 Subject: [PATCH 1/4] Stop allowing arbitrary YAML tags for mkdocs_theme.yml --- mkdocs/tests/theme_tests.py | 4 ++-- mkdocs/tests/utils/utils_tests.py | 2 +- mkdocs/theme.py | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/mkdocs/tests/theme_tests.py b/mkdocs/tests/theme_tests.py index 4d123c68..de12ae77 100644 --- a/mkdocs/tests/theme_tests.py +++ b/mkdocs/tests/theme_tests.py @@ -76,7 +76,7 @@ class ThemeTests(unittest.TestCase): self.assertTrue('new' in theme) self.assertEqual(theme['new'], 42) - @mock.patch('mkdocs.utils.yaml_load', return_value={}) + @mock.patch('yaml.safe_load', return_value={}) def test_no_theme_config(self, m): theme = Theme(name='mkdocs') self.assertEqual(m.call_count, 1) @@ -89,7 +89,7 @@ class ThemeTests(unittest.TestCase): {'static_templates': ['parent.html']}, ] ) - with mock.patch('mkdocs.utils.yaml_load', m) as m: + with mock.patch('yaml.safe_load', m) as m: theme = Theme(name='mkdocs') self.assertEqual(m.call_count, 2) self.assertEqual( diff --git a/mkdocs/tests/utils/utils_tests.py b/mkdocs/tests/utils/utils_tests.py index a72044e2..e7f821dd 100644 --- a/mkdocs/tests/utils/utils_tests.py +++ b/mkdocs/tests/utils/utils_tests.py @@ -221,7 +221,7 @@ class UtilsTests(unittest.TestCase): self.assertEqual(utils.get_theme_dir(theme.name), os.path.abspath(path)) - def test_get_theme_dir_keyerror(self): + def test_get_theme_dir_error(self): with self.assertRaises(KeyError): utils.get_theme_dir('nonexistanttheme') diff --git a/mkdocs/theme.py b/mkdocs/theme.py index 77134d41..cc846695 100644 --- a/mkdocs/theme.py +++ b/mkdocs/theme.py @@ -6,6 +6,7 @@ import warnings from typing import Any, Collection, MutableMapping import jinja2 +import yaml from mkdocs import localization, utils from mkdocs.config.base import ValidationError @@ -123,7 +124,7 @@ class Theme(MutableMapping[str, Any]): try: file_path = os.path.join(theme_dir, 'mkdocs_theme.yml') with open(file_path, 'rb') as f: - theme_config = utils.yaml_load(f) + theme_config = yaml.safe_load(f) except OSError as e: log.debug(e) raise ValidationError( From 9ff9bb10bff997532aff1e8588734b924dd7e079 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 11 Nov 2023 16:37:25 +0100 Subject: [PATCH 2/4] Docs: warn about installing third-party plugins --- docs/dev-guide/plugins.md | 2 ++ docs/user-guide/choosing-your-theme.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/docs/dev-guide/plugins.md b/docs/dev-guide/plugins.md index 8d4a8c63..20218607 100644 --- a/docs/dev-guide/plugins.md +++ b/docs/dev-guide/plugins.md @@ -15,6 +15,8 @@ appropriate package name and install it using `pip`: pip install mkdocs-foo-plugin ``` +WARNING: Installing an MkDocs plugin means installing a Python package and executing any code that the author has put in there. So, exercise the usual caution; there's no attempt at sandboxing. + Once a plugin has been successfully installed, it is ready to use. It just needs to be [enabled](#using-plugins) in the configuration file. The [Catalog] repository has a large ranked list of plugins that you can install and use. diff --git a/docs/user-guide/choosing-your-theme.md b/docs/user-guide/choosing-your-theme.md index 8ababf5b..383ce193 100644 --- a/docs/user-guide/choosing-your-theme.md +++ b/docs/user-guide/choosing-your-theme.md @@ -196,6 +196,8 @@ theme supports the following options: A list of third party themes can be found at the [community wiki] page and [the ranked catalog][catalog]. If you have created your own, please add them there. +WARNING: Installing an MkDocs theme means installing a Python package and executing any code that the author has put in there. So, exercise the usual caution; there's no attempt at sandboxing. + [third party themes]: #third-party-themes [theme]: configuration.md#theme [Bootstrap]: https://getbootstrap.com/ From c3c980582f76f60135c9f3613eac7b180a102c3a Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 11 Nov 2023 16:50:29 +0100 Subject: [PATCH 3/4] Cache the result of the function `get_themes()` --- mkdocs/tests/utils/utils_tests.py | 161 +++++++++++++++--------------- mkdocs/theme.py | 1 + mkdocs/utils/__init__.py | 1 + 3 files changed, 85 insertions(+), 78 deletions(-) diff --git a/mkdocs/tests/utils/utils_tests.py b/mkdocs/tests/utils/utils_tests.py index e7f821dd..2a39fddd 100644 --- a/mkdocs/tests/utils/utils_tests.py +++ b/mkdocs/tests/utils/utils_tests.py @@ -203,84 +203,6 @@ class UtilsTests(unittest.TestCase): utils.insort(a, (1, 'a'), key=lambda v: v[0]) self.assertEqual(a, [(1, 'a'), (1, 'b'), (1, 'a'), (2, 'c')]) - def test_get_themes(self): - themes = utils.get_theme_names() - self.assertIn('mkdocs', themes) - self.assertIn('readthedocs', themes) - - @mock.patch('mkdocs.utils.entry_points', autospec=True) - def test_get_theme_dir(self, mock_iter): - path = 'some/path' - - theme = mock.Mock() - theme.name = 'mkdocs2' - theme.dist.name = 'mkdocs2' - theme.load().__file__ = os.path.join(path, '__init__.py') - - mock_iter.return_value = [theme] - - self.assertEqual(utils.get_theme_dir(theme.name), os.path.abspath(path)) - - def test_get_theme_dir_error(self): - with self.assertRaises(KeyError): - utils.get_theme_dir('nonexistanttheme') - - @mock.patch('mkdocs.utils.entry_points', autospec=True) - def test_get_theme_dir_importerror(self, mock_iter): - theme = mock.Mock() - theme.name = 'mkdocs2' - theme.dist.name = 'mkdocs2' - theme.load.side_effect = ImportError() - - mock_iter.return_value = [theme] - - with self.assertRaises(ImportError): - utils.get_theme_dir(theme.name) - - @mock.patch('mkdocs.utils.entry_points', autospec=True) - def test_get_themes_warning(self, mock_iter): - theme1 = mock.Mock() - theme1.name = 'mkdocs2' - theme1.dist.name = 'mkdocs2' - theme1.load().__file__ = "some/path1" - - theme2 = mock.Mock() - theme2.name = 'mkdocs2' - theme2.dist.name = 'mkdocs3' - theme2.load().__file__ = "some/path2" - - mock_iter.return_value = [theme1, theme2] - - with self.assertLogs('mkdocs') as cm: - theme_names = utils.get_theme_names() - self.assertEqual( - '\n'.join(cm.output), - "WARNING:mkdocs.utils:A theme named 'mkdocs2' is provided by the Python " - "packages 'mkdocs3' and 'mkdocs2'. The one in 'mkdocs3' will be used.", - ) - self.assertCountEqual(theme_names, ['mkdocs2']) - - @mock.patch('mkdocs.utils.entry_points', autospec=True) - def test_get_themes_error(self, mock_iter): - theme1 = mock.Mock() - theme1.name = 'mkdocs' - theme1.dist.name = 'mkdocs' - theme1.load().__file__ = "some/path1" - - theme2 = mock.Mock() - theme2.name = 'mkdocs' - theme2.dist.name = 'mkdocs2' - theme2.load().__file__ = "some/path2" - - mock_iter.return_value = [theme1, theme2] - - with self.assertRaisesRegex( - exceptions.ConfigurationError, - "The theme 'mkdocs' is a builtin theme but the package 'mkdocs2' " - "attempts to provide a theme with the same name.", - ): - utils.get_theme_names() - def test_nest_paths(self, j=posixpath.join): result = utils.nest_paths( [ @@ -528,6 +450,89 @@ class UtilsTests(unittest.TestCase): self.assertEqual(meta.get_data(doc), (doc, {})) +class ThemeUtilsTests(unittest.TestCase): + def setUp(self): + utils.get_themes.cache_clear() + + def test_get_themes(self): + themes = utils.get_theme_names() + self.assertIn('mkdocs', themes) + self.assertIn('readthedocs', themes) + + @mock.patch('mkdocs.utils.entry_points', autospec=True) + def test_get_theme_dir(self, mock_iter): + path = 'some/path' + + theme = mock.Mock() + theme.name = 'mkdocs2' + theme.dist.name = 'mkdocs2' + theme.load().__file__ = os.path.join(path, '__init__.py') + + mock_iter.return_value = [theme] + + self.assertEqual(utils.get_theme_dir(theme.name), os.path.abspath(path)) + + def test_get_theme_dir_error(self): + with self.assertRaises(KeyError): + utils.get_theme_dir('nonexistanttheme') + + @mock.patch('mkdocs.utils.entry_points', autospec=True) + def test_get_theme_dir_importerror(self, mock_iter): + theme = mock.Mock() + theme.name = 'mkdocs2' + theme.dist.name = 'mkdocs2' + theme.load.side_effect = ImportError() + + mock_iter.return_value = [theme] + + with self.assertRaises(ImportError): + utils.get_theme_dir(theme.name) + + @mock.patch('mkdocs.utils.entry_points', autospec=True) + def test_get_themes_warning(self, mock_iter): + theme1 = mock.Mock() + theme1.name = 'mkdocs2' + theme1.dist.name = 'mkdocs2' + theme1.load().__file__ = "some/path1" + + theme2 = mock.Mock() + theme2.name = 'mkdocs2' + theme2.dist.name = 'mkdocs3' + theme2.load().__file__ = "some/path2" + + mock_iter.return_value = [theme1, theme2] + + with self.assertLogs('mkdocs') as cm: + theme_names = utils.get_theme_names() + self.assertEqual( + '\n'.join(cm.output), + "WARNING:mkdocs.utils:A theme named 'mkdocs2' is provided by the Python " + "packages 'mkdocs3' and 'mkdocs2'. The one in 'mkdocs3' will be used.", + ) + self.assertCountEqual(theme_names, ['mkdocs2']) + + @mock.patch('mkdocs.utils.entry_points', autospec=True) + def test_get_themes_error(self, mock_iter): + theme1 = mock.Mock() + theme1.name = 'mkdocs' + theme1.dist.name = 'mkdocs' + theme1.load().__file__ = "some/path1" + + theme2 = mock.Mock() + theme2.name = 'mkdocs' + theme2.dist.name = 'mkdocs2' + theme2.load().__file__ = "some/path2" + + mock_iter.return_value = [theme1, theme2] + + with self.assertRaisesRegex( + exceptions.ConfigurationError, + "The theme 'mkdocs' is a builtin theme but the package 'mkdocs2' " + "attempts to provide a theme with the same name.", + ): + utils.get_theme_names() + + class LogCounterTests(unittest.TestCase): def setUp(self): self.log = logging.getLogger('dummy') diff --git a/mkdocs/theme.py b/mkdocs/theme.py index cc846695..7a6126d6 100644 --- a/mkdocs/theme.py +++ b/mkdocs/theme.py @@ -119,6 +119,7 @@ class Theme(MutableMapping[str, Any]): def _load_theme_config(self, name: str) -> None: """Recursively load theme and any parent themes.""" theme_dir = utils.get_theme_dir(name) + utils.get_themes.cache_clear() self.dirs.append(theme_dir) try: diff --git a/mkdocs/utils/__init__.py b/mkdocs/utils/__init__.py index b9c034d7..1560dfa1 100644 --- a/mkdocs/utils/__init__.py +++ b/mkdocs/utils/__init__.py @@ -258,6 +258,7 @@ def get_theme_dir(name: str) -> str: return os.path.dirname(os.path.abspath(theme.load().__file__)) +@functools.lru_cache(maxsize=None) def get_themes() -> dict[str, EntryPoint]: """Return a dict of all installed themes as {name: EntryPoint}.""" themes: dict[str, EntryPoint] = {} From 47e41a9a0ebc097fb45899ca57edc4d005b7b154 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Tue, 21 Nov 2023 21:08:28 +0100 Subject: [PATCH 4/4] Still use the fast version of SafeLoader --- mkdocs/tests/theme_tests.py | 4 ++-- mkdocs/theme.py | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/mkdocs/tests/theme_tests.py b/mkdocs/tests/theme_tests.py index de12ae77..2b1aea90 100644 --- a/mkdocs/tests/theme_tests.py +++ b/mkdocs/tests/theme_tests.py @@ -76,7 +76,7 @@ class ThemeTests(unittest.TestCase): self.assertTrue('new' in theme) self.assertEqual(theme['new'], 42) - @mock.patch('yaml.safe_load', return_value={}) + @mock.patch('yaml.load', return_value={}) def test_no_theme_config(self, m): theme = Theme(name='mkdocs') self.assertEqual(m.call_count, 1) @@ -89,7 +89,7 @@ class ThemeTests(unittest.TestCase): {'static_templates': ['parent.html']}, ] ) - with mock.patch('yaml.safe_load', m) as m: + with mock.patch('yaml.load', m) as m: theme = Theme(name='mkdocs') self.assertEqual(m.call_count, 2) self.assertEqual( diff --git a/mkdocs/theme.py b/mkdocs/theme.py index 7a6126d6..bf587201 100644 --- a/mkdocs/theme.py +++ b/mkdocs/theme.py @@ -8,6 +8,11 @@ from typing import Any, Collection, MutableMapping import jinja2 import yaml +try: + from yaml import CSafeLoader as SafeLoader +except ImportError: # pragma: no cover + from yaml import SafeLoader # type: ignore + from mkdocs import localization, utils from mkdocs.config.base import ValidationError from mkdocs.utils import templates @@ -125,7 +130,7 @@ class Theme(MutableMapping[str, Any]): try: file_path = os.path.join(theme_dir, 'mkdocs_theme.yml') with open(file_path, 'rb') as f: - theme_config = yaml.safe_load(f) + theme_config = yaml.load(f, SafeLoader) except OSError as e: log.debug(e) raise ValidationError(