From aa3bba5bd29245ab0893879e7cd4b07208c810ff Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 15 Dec 2023 13:20:30 +0100 Subject: [PATCH 1/3] Fix edge cases in validation of docs_dir/site_dir Fix bypassing it when: * a non-absolute config_file_path is passed on the command line * the directory is not an *exact* parent of the other one, but is instead a "grandparent". Also validate it in `mkdocs serve` because why reserve the check only to `mkdocs build`. --- mkdocs/commands/serve.py | 7 ++----- mkdocs/config/base.py | 5 ++++- mkdocs/config/config_options.py | 16 ++++++++++++---- .../tests/config/config_options_legacy_tests.py | 2 +- mkdocs/tests/config/config_options_tests.py | 2 +- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/mkdocs/commands/serve.py b/mkdocs/commands/serve.py index 889d8066..0069d58c 100644 --- a/mkdocs/commands/serve.py +++ b/mkdocs/commands/serve.py @@ -40,11 +40,8 @@ def serve( site_dir = tempfile.mkdtemp(prefix='mkdocs_') def get_config(): - config = load_config( - config_file=config_file, - site_dir=site_dir, - **kwargs, - ) + config = load_config(config_file=config_file, **kwargs) + config.site_dir = site_dir # Overwrite it afterwards, so normal validation still kicks in. config.watch.extend(watch) return config diff --git a/mkdocs/config/base.py b/mkdocs/config/base.py index 40f5aacf..834ee6c7 100644 --- a/mkdocs/config/base.py +++ b/mkdocs/config/base.py @@ -168,7 +168,10 @@ def __init__(self, config_file_path: str | bytes | None = None): config_file_path = config_file_path.decode(encoding=sys.getfilesystemencoding()) except UnicodeDecodeError: raise ValidationError("config_file_path is not a Unicode string.") - self.config_file_path = config_file_path or '' + if config_file_path: + self.config_file_path = os.path.abspath(config_file_path) + else: + self.config_file_path = '' def set_defaults(self) -> None: """ diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 3025045f..888c9e4a 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -4,6 +4,7 @@ import ipaddress import logging import os +import pathlib import string import sys import traceback @@ -737,8 +738,8 @@ def post_validation(self, config: Config, key_name: str): if not config.config_file_path: return - # Validate that the dir is not the parent dir of the config file. - if os.path.dirname(config.config_file_path) == config[key_name]: + # Validate that the docs dir does not contain the config file. + if _relative_path(config.config_file_path, to=config[key_name]): raise ValidationError( f"The '{key_name}' should not be the parent directory of the" f" config file. Use a child directory instead so that the" @@ -796,14 +797,14 @@ def post_validation(self, config: Config, key_name: str): # Validate that the docs_dir and site_dir don't contain the # other as this will lead to copying back and forth on each # and eventually make a deep nested mess. - if (docs_dir + os.sep).startswith(site_dir.rstrip(os.sep) + os.sep): + if _relative_path(docs_dir, to=site_dir): raise ValidationError( f"The 'docs_dir' should not be within the 'site_dir' as this " f"can mean the source files are overwritten by the output or " f"it will be deleted if --clean is passed to mkdocs build. " f"(site_dir: '{site_dir}', docs_dir: '{docs_dir}')" ) - elif (site_dir + os.sep).startswith(docs_dir.rstrip(os.sep) + os.sep): + if _relative_path(site_dir, to=docs_dir): raise ValidationError( f"The 'site_dir' should not be within the 'docs_dir' as this " f"leads to the build directory being copied into itself and " @@ -1227,3 +1228,10 @@ def run_validation(self, value: object) -> pathspec.gitignore.GitIgnoreSpec: return pathspec.gitignore.GitIgnoreSpec.from_lines(lines=value.splitlines()) except ValueError as e: raise ValidationError(str(e)) + + +def _relative_path(main: str | os.PathLike, to: str | os.PathLike) -> pathlib.Path | None: + try: + return pathlib.Path(main).relative_to(to) + except ValueError: + return None diff --git a/mkdocs/tests/config/config_options_legacy_tests.py b/mkdocs/tests/config/config_options_legacy_tests.py index 8d6da457..03263601 100644 --- a/mkdocs/tests/config/config_options_legacy_tests.py +++ b/mkdocs/tests/config/config_options_legacy_tests.py @@ -1275,7 +1275,7 @@ class Schema: config_path = "foo/mkdocs.yaml" self.get_config(Schema, {"sub": {"opt": "bar"}}, config_file_path=config_path) - self.assertEqual(passed_config_path, config_path) + self.assertEqual(passed_config_path, os.path.abspath(config_path)) class ConfigItemsTest(TestCase): diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index 119cd726..45f52679 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1571,7 +1571,7 @@ class Schema(Config): config_path = "foo/mkdocs.yaml" self.get_config(Schema, {"sub": [{"opt": "bar"}]}, config_file_path=config_path) - self.assertEqual(passed_config_path, config_path) + self.assertEqual(passed_config_path, os.path.abspath(config_path)) class NestedSubConfigTest(TestCase): From bf6de0d7acd043e8684001ef216c0e584995308c Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 15 Dec 2023 12:51:06 +0100 Subject: [PATCH 2/3] Allow the config file to be inside docs_dir, if it's excluded --- mkdocs/config/config_options.py | 17 ++++--- .../config/config_options_legacy_tests.py | 13 +++--- mkdocs/tests/config/config_options_tests.py | 44 ++++++++++++++++--- 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 888c9e4a..6a24f956 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -739,12 +739,17 @@ def post_validation(self, config: Config, key_name: str): return # Validate that the docs dir does not contain the config file. - if _relative_path(config.config_file_path, to=config[key_name]): - raise ValidationError( - f"The '{key_name}' should not be the parent directory of the" - f" config file. Use a child directory instead so that the" - f" '{key_name}' is a sibling of the config file." - ) + config_rel_path = _relative_path(config.config_file_path, to=config[key_name]) + if config_rel_path is None: + return # Good, it's not inside the docs_dir. + exclude_docs: pathspec.gitignore.GitIgnoreSpec | None = config.get('exclude_docs') + if exclude_docs and exclude_docs.match_file(config_rel_path): + return # Good, the appropriate config is present. + raise ValidationError( + f"The '{config_rel_path.name}' config file should not be inside the {key_name}.\n" + f"To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration:\n\n" + f"exclude_docs: |\n /{config_rel_path.as_posix()}" + ) class File(FilesystemObject): diff --git a/mkdocs/tests/config/config_options_legacy_tests.py b/mkdocs/tests/config/config_options_legacy_tests.py index 03263601..9fef73e1 100644 --- a/mkdocs/tests/config/config_options_legacy_tests.py +++ b/mkdocs/tests/config/config_options_legacy_tests.py @@ -768,17 +768,20 @@ class Schema: ) self.assertEqual(conf['dir'], os.path.join(base_path, 'foo')) - def test_site_dir_is_config_dir_fails(self): + def test_docs_dir_is_config_dir_fails(self): class Schema: - dir = c.DocsDir() + docs_dir = c.DocsDir() with self.expect_error( - dir="The 'dir' should not be the parent directory of the config file. " - "Use a child directory instead so that the 'dir' is a sibling of the config file." + docs_dir="""The 'mkdocs.yml' config file should not be inside the docs_dir. +To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /mkdocs.yml""" ): self.get_config( Schema, - {'dir': '.'}, + {'docs_dir': '.'}, config_file_path=os.path.join(os.path.abspath('.'), 'mkdocs.yml'), ) diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index 45f52679..1096ac57 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -972,20 +972,54 @@ class Schema(Config): ) self.assertEqual(conf.dir, os.path.join(base_path, 'foo')) - def test_site_dir_is_config_dir_fails(self) -> None: + def test_docs_dir_is_config_dir_fails(self) -> None: class Schema(Config): - dir = c.DocsDir() + docs_dir = c.DocsDir() with self.expect_error( - dir="The 'dir' should not be the parent directory of the config file. " - "Use a child directory instead so that the 'dir' is a sibling of the config file." + docs_dir="""The 'mkdocs.yml' config file should not be inside the docs_dir. +To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /mkdocs.yml""" ): self.get_config( Schema, - {'dir': '.'}, + {'docs_dir': '.'}, config_file_path=os.path.join(os.path.abspath('.'), 'mkdocs.yml'), ) + def test_docs_dir_is_config_dir_nested_fails(self) -> None: + class Schema(Config): + docs_dir = c.DocsDir() + + with self.expect_error( + docs_dir="""The 'mkdocos.yaml' config file should not be inside the docs_dir. +To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /config/mkdocos.yaml""" + ): + self.get_config( + Schema, + {'docs_dir': '..'}, + config_file_path=os.path.join( + os.path.abspath('.'), 'docs', 'config', 'mkdocos.yaml' + ), + ) + + def test_docs_dir_is_config_dir_succeeds(self) -> None: + class Schema(Config): + docs_dir = c.DocsDir() + exclude_docs = c.Optional(c.PathSpec()) + + conf = self.get_config( + Schema, + {'docs_dir': '.', 'exclude_docs': '/mkdocs.y*l'}, + config_file_path=os.path.join(os.path.abspath('.'), 'mkdocs.yml'), + ) + self.assertEqual(conf.docs_dir, os.path.abspath('.')) + class ListOfPathsTest(TestCase): def test_valid_path(self) -> None: From 68045323743b48ec5d4110ced43cf3320ae31dc5 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 15 Dec 2023 13:13:13 +0100 Subject: [PATCH 3/3] Allow the site_dir to be inside docs_dir, if it's excluded --- mkdocs/config/config_options.py | 20 +++++++------- .../config/config_options_legacy_tests.py | 13 +++++---- mkdocs/tests/config/config_options_tests.py | 27 +++++++++++++++---- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 6a24f956..02958a56 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -804,17 +804,19 @@ def post_validation(self, config: Config, key_name: str): # and eventually make a deep nested mess. if _relative_path(docs_dir, to=site_dir): raise ValidationError( - f"The 'docs_dir' should not be within the 'site_dir' as this " - f"can mean the source files are overwritten by the output or " - f"it will be deleted if --clean is passed to mkdocs build. " - f"(site_dir: '{site_dir}', docs_dir: '{docs_dir}')" + "The docs_dir should not be inside the site_dir as this " + "can mean the source files are overwritten by the output or " + "deleted entirely." ) - if _relative_path(site_dir, to=docs_dir): + if site_rel_path := _relative_path(site_dir, to=docs_dir): + exclude_docs: pathspec.gitignore.GitIgnoreSpec | None = config.get('exclude_docs') + if exclude_docs and exclude_docs.match_file(site_rel_path): + return # Good, the appropriate config is present. raise ValidationError( - f"The 'site_dir' should not be within the 'docs_dir' as this " - f"leads to the build directory being copied into itself and " - f"duplicate nested files in the 'site_dir'. " - f"(site_dir: '{site_dir}', docs_dir: '{docs_dir}')" + f"The site_dir should not be inside the docs_dir as this " + f"leads to the build directory being copied into itself.\n" + f"To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration:\n\n" + f"exclude_docs: |\n /{site_rel_path.as_posix()}" ) diff --git a/mkdocs/tests/config/config_options_legacy_tests.py b/mkdocs/tests/config/config_options_legacy_tests.py index 9fef73e1..d443df90 100644 --- a/mkdocs/tests/config/config_options_legacy_tests.py +++ b/mkdocs/tests/config/config_options_legacy_tests.py @@ -863,7 +863,7 @@ class Schema: site_dir = c.SiteDir() docs_dir = c.Dir() - def test_doc_dir_in_site_dir(self): + def test_doc_dir_in_site_dir_fails(self): j = os.path.join # The parent dir is not the same on every system, so use the actual dir name parent_dir = mkdocs.__file__.split(os.sep)[-3] @@ -881,24 +881,27 @@ def test_doc_dir_in_site_dir(self): for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'docs_dir' should not be within the 'site_dir'.*") + site_dir="The docs_dir should not be inside the site_dir as this can mean the source files are overwritten by the output or deleted entirely." ): self.get_config(self.Schema, test_config) - def test_site_dir_in_docs_dir(self): + def test_site_dir_in_docs_dir_fails(self): j = os.path.join test_configs = ( {'docs_dir': 'docs', 'site_dir': j('docs', 'site')}, {'docs_dir': '.', 'site_dir': 'site'}, {'docs_dir': '', 'site_dir': 'site'}, - {'docs_dir': '/', 'site_dir': 'site'}, ) for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'site_dir' should not be within the 'docs_dir'.*") + site_dir="""The site_dir should not be inside the docs_dir as this leads to the build directory being copied into itself. +To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /site""" ): self.get_config(self.Schema, test_config) diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index 1096ac57..be8c4047 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1106,8 +1106,9 @@ class SiteDirTest(TestCase): class Schema(Config): site_dir = c.SiteDir() docs_dir = c.Dir() + exclude_docs = c.Optional(c.PathSpec()) - def test_doc_dir_in_site_dir(self) -> None: + def test_doc_dir_in_site_dir_fails(self) -> None: j = os.path.join # The parent dir is not the same on every system, so use the actual dir name parent_dir = mkdocs.__file__.split(os.sep)[-3] @@ -1125,27 +1126,43 @@ def test_doc_dir_in_site_dir(self) -> None: for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'docs_dir' should not be within the 'site_dir'.*") + site_dir="The docs_dir should not be inside the site_dir as this can mean the source files are overwritten by the output or deleted entirely." ): self.get_config(self.Schema, test_config) - def test_site_dir_in_docs_dir(self) -> None: + def test_site_dir_in_docs_dir_fails(self) -> None: j = os.path.join test_configs = ( {'docs_dir': 'docs', 'site_dir': j('docs', 'site')}, {'docs_dir': '.', 'site_dir': 'site'}, {'docs_dir': '', 'site_dir': 'site'}, - {'docs_dir': '/', 'site_dir': 'site'}, ) for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'site_dir' should not be within the 'docs_dir'.*") + site_dir="""The site_dir should not be inside the docs_dir as this leads to the build directory being copied into itself. +To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /site""" ): self.get_config(self.Schema, test_config) + def test_site_dir_in_docs_dir_succeeds(self) -> None: + j = os.path.join + + test_configs = ( + {'docs_dir': 'docs', 'site_dir': j('docs', 'site'), 'exclude_docs': 'site'}, + {'docs_dir': '.', 'site_dir': 'site', 'exclude_docs': '/site'}, + {'docs_dir': '', 'site_dir': 'site', 'exclude_docs': '/si*e'}, + ) + + for test_config in test_configs: + with self.subTest(test_config): + self.get_config(self.Schema, test_config) + def test_common_prefix(self) -> None: """Legitimate settings with common prefixes should not fail validation.""" test_configs = (