From a70204a8b8f3f533d93715932537ef280fbd15ce Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Mon, 20 Jul 2026 13:25:49 -0300 Subject: [PATCH 1/3] [fix] Validated extra file paths #400 Extra file paths were written into generated archives and later applied by device agents. The backend now rejects malformed paths during validation so bogus archive entries do not reach downstream tools. Closes #400 --- netjsonconfig/backends/base/backend.py | 52 ++++++++++++++++++++++++++ tests/test_base.py | 45 ++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/netjsonconfig/backends/base/backend.py b/netjsonconfig/backends/base/backend.py index de80a991c..07ccda004 100644 --- a/netjsonconfig/backends/base/backend.py +++ b/netjsonconfig/backends/base/backend.py @@ -16,6 +16,10 @@ format_checker = Draft4Validator.FORMAT_CHECKER _host_name_re = re.compile(r"^[A-Za-z0-9][A-Za-z0-9\.\-]{1,255}$") +# Public file paths allow only letters, numbers, dots, underscores, dashes +# and slashes; internal placeholders are handled separately by +# _validate_file_path_chars(). +_file_path_re = re.compile(r"\A/?[A-Za-z0-9._/-]+\Z") class BaseBackend(object): @@ -158,9 +162,57 @@ def validate(self): Draft4Validator(self.schema, format_checker=format_checker).validate( self.config ) + self._validate_file_paths() except JsonSchemaError as e: raise ValidationError(e) + def _validate_file_paths(self): + """ + Validates paths used by extra configuration files. + + These paths are later written to configuration archives and applied on + devices, so accepting malformed or surprising paths can lead to unsafe + or broken behavior outside netjsonconfig. + """ + for index, file_item in enumerate(self.config.get("files", [])): + path = file_item["path"] + components = path.lstrip("/").split("/") + valid_path = self._validate_file_path_chars(path) + valid_components = all( + component and component not in {".", ".."} for component in components + ) + if valid_path and valid_components: + continue + raise JsonSchemaError( + 'Invalid file path "{0}". Allowed characters are letters, numbers, ' + '".", "_", "-", and "/".'.format(path), + path=("files", index, "path"), + ) + + def _validate_file_path_chars(self, path): + """ + Checks the visible characters of a file path. + + Template placeholders are internal values, so this method accepts them + only when they are properly wrapped in ``{{`` and ``}}`` while keeping + the public path character set strict and easy to explain. + """ + index = 0 + clean_path = "" + while index < len(path): + if path[index : index + 2] == "{{": + end_index = path.find("}}", index + 2) + if end_index == -1: + return False + clean_path += "x" + index = end_index + 2 + continue + if path[index] in "{}": + return False + clean_path += path[index] + index += 1 + return bool(_file_path_re.match(clean_path)) + def render(self, files=True): """ Converts the configuration dictionary into the corresponding configuration format diff --git a/tests/test_base.py b/tests/test_base.py index 24e63dde8..f0bd0428d 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -4,6 +4,12 @@ from netjsonconfig.backends.base.backend import BaseBackend from netjsonconfig.backends.base.parser import BaseParser from netjsonconfig.backends.base.renderer import BaseRenderer +from netjsonconfig.exceptions import ValidationError +from netjsonconfig.schema import schema + + +class BaseBackendWithSchema(BaseBackend): + schema = schema class TestBase(unittest.TestCase): @@ -32,3 +38,42 @@ def test_parse_tar_not_implemented(self): def test_base_backend_parse_not_implemented(self): with self.assertRaises(NotImplementedError): BaseBackend(native="") + + def test_validate_file_paths(self): + b = BaseBackendWithSchema( + { + "files": [ + {"path": "/etc/config/network", "mode": "0644", "contents": ""}, + {"path": "etc/openvpn/client.conf", "mode": "0644", "contents": ""}, + {"path": "{{cert_path_abc123}}", "mode": "0644", "contents": ""}, + { + "path": "/etc/{{cert_path_abc123}}", + "mode": "0644", + "contents": "", + }, + ] + } + ) + b.validate() + + def test_validate_file_paths_rejects_invalid_paths(self): + invalid_paths = [ + "tmp/x; reboot", + "../../../etc/passwd", + "tmp/a b", + "tmp/file\n", + "tmp/$(reboot)", + "tmp//file", + "tmp/./file", + "tmp/{file}", + "tmp/{{file}", + "tmp/file}}", + ] + for path in invalid_paths: + with self.subTest(path=path): + b = BaseBackendWithSchema( + {"files": [{"path": path, "mode": "0644", "contents": ""}]} + ) + with self.assertRaises(ValidationError) as context: + b.validate() + self.assertIn("Invalid file path", context.exception.message) From 7bbb28d7d2ca37091203c6f6fbe3012d0d57d261 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Mon, 20 Jul 2026 13:44:50 -0300 Subject: [PATCH 2/3] [chores] Follow up with @coderabbitai --- netjsonconfig/backends/base/backend.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/netjsonconfig/backends/base/backend.py b/netjsonconfig/backends/base/backend.py index 07ccda004..48e207a01 100644 --- a/netjsonconfig/backends/base/backend.py +++ b/netjsonconfig/backends/base/backend.py @@ -185,7 +185,8 @@ def _validate_file_paths(self): continue raise JsonSchemaError( 'Invalid file path "{0}". Allowed characters are letters, numbers, ' - '".", "_", "-", and "/".'.format(path), + '".", "_", "-", and "/". Empty path parts and "." or ".." ' + "components are not allowed.".format(path), path=("files", index, "path"), ) @@ -200,7 +201,7 @@ def _validate_file_path_chars(self, path): index = 0 clean_path = "" while index < len(path): - if path[index : index + 2] == "{{": + if path.startswith("{{", index): end_index = path.find("}}", index + 2) if end_index == -1: return False From 0503795558285b22cf5967a087747894c6cc838a Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Mon, 20 Jul 2026 20:33:10 -0300 Subject: [PATCH 3/3] [chores] Follow up with @coderabbitai --- docs/source/backends/openwrt.rst | 3 +++ netjsonconfig/backends/base/backend.py | 18 +++++++++++++----- tests/test_base.py | 2 ++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/docs/source/backends/openwrt.rst b/docs/source/backends/openwrt.rst index 2ba56c85a..656ecf0a5 100644 --- a/docs/source/backends/openwrt.rst +++ b/docs/source/backends/openwrt.rst @@ -2846,6 +2846,9 @@ key name type required function The ``files`` key of the *configuration dictionary* is a custom NetJSON extension not present in the original NetJSON RFC. +File paths may contain only letters, numbers, dots, underscores, dashes +and slashes. Empty path parts, ``.`` and ``..`` are not allowed. + .. warning:: The files are included in the output of the ``render`` method unless diff --git a/netjsonconfig/backends/base/backend.py b/netjsonconfig/backends/base/backend.py index 48e207a01..910011a55 100644 --- a/netjsonconfig/backends/base/backend.py +++ b/netjsonconfig/backends/base/backend.py @@ -16,9 +16,8 @@ format_checker = Draft4Validator.FORMAT_CHECKER _host_name_re = re.compile(r"^[A-Za-z0-9][A-Za-z0-9\.\-]{1,255}$") -# Public file paths allow only letters, numbers, dots, underscores, dashes -# and slashes; internal placeholders are handled separately by -# _validate_file_path_chars(). +# Literal path text is intentionally limited to common path characters. +# Template variables are checked separately. _file_path_re = re.compile(r"\A/?[A-Za-z0-9._/-]+\Z") @@ -176,7 +175,11 @@ def _validate_file_paths(self): """ for index, file_item in enumerate(self.config.get("files", [])): path = file_item["path"] - components = path.lstrip("/").split("/") + # Keep one leading slash valid, but do not hide extra slashes. + # They are empty path components and should fail validation. + components = ( + path[1:].split("/") if path.startswith("/") else path.split("/") + ) valid_path = self._validate_file_path_chars(path) valid_components = all( component and component not in {".", ".."} for component in components @@ -194,7 +197,7 @@ def _validate_file_path_chars(self, path): """ Checks the visible characters of a file path. - Template placeholders are internal values, so this method accepts them + Template variables are internal values, so this method accepts them only when they are properly wrapped in ``{{`` and ``}}`` while keeping the public path character set strict and easy to explain. """ @@ -202,6 +205,8 @@ def _validate_file_path_chars(self, path): clean_path = "" while index < len(path): if path.startswith("{{", index): + # Variable names are not part of the path syntax. Only the + # delimiters matter here; callers decide what names are valid. end_index = path.find("}}", index + 2) if end_index == -1: return False @@ -270,6 +275,9 @@ def generate(self): :returns: in-memory tar.gz archive, instance of ``BytesIO`` """ + # Do not validate here. Old saved configs should still be downloadable + # after stricter validation is introduced; new data should be rejected + # when validate() is called. tar_bytes = BytesIO() tar = tarfile.open(fileobj=tar_bytes, mode="w") self._generate_contents(tar) diff --git a/tests/test_base.py b/tests/test_base.py index f0bd0428d..8ff498611 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -63,6 +63,8 @@ def test_validate_file_paths_rejects_invalid_paths(self): "tmp/a b", "tmp/file\n", "tmp/$(reboot)", + "//etc/passwd", + "///etc/passwd", "tmp//file", "tmp/./file", "tmp/{file}",