diff --git a/api/extensions.py b/api/extensions.py index 91f4db79..4f5c9268 100644 --- a/api/extensions.py +++ b/api/extensions.py @@ -1854,7 +1854,7 @@ def install_extension(id: object, download_url: object, sha256: object) -> Dict[ ext_dir_resolved = ext_dir.resolve() for member_name in file_members: decoded = _fully_unquote_path(_stripped(member_name)) - if not decoded or not _is_safe_relative_path(decoded): + if not decoded or not _is_safe_archive_member(decoded): raise ExtensionInstallError("Unsafe archive member") resolved = (ext_dir / decoded).resolve() try: @@ -1956,7 +1956,7 @@ def uninstall_extension(id: object) -> Dict[str, Any]: raise ExtensionInstallError("Extension not installed", 404) ext_dir = root / ext_id for rel_path in entry.get("files", []): - if not _is_safe_relative_path(rel_path): + if not _is_safe_archive_member(rel_path): continue target = (ext_dir / rel_path).resolve() try: @@ -2068,17 +2068,42 @@ def inject_extension_tags(index_html: str) -> str: def _is_safe_relative_path(rel: str) -> bool: + # Strict: reject empty, traversal, AND any dot-prefixed segment. This is shared + # by static serving, asset URLs and manifest paths, where a hidden file must + # never become reachable. Archive members use _is_safe_archive_member() below. if not rel or "\x00" in rel or "\\" in rel: return False for segment in rel.split("/"): - # Reject empty and traversal segments only. Hidden leaf files such as - # ".gitkeep" or ".env.example" are legitimate and not a traversal risk - # (containment is also enforced by the resolved.relative_to(root) check). - if not segment or segment in (".", ".."): + if not segment or segment in (".", "..") or segment.startswith("."): return False return True +# Benign hidden LEAF files that extension archives legitimately ship. A dot-prefixed +# name is permitted only as the final path segment and only if it is one of these; +# dot-directories (e.g. ".git/") and every other dotfile (".env", ".secret") stay rejected. +_ALLOWED_ARCHIVE_DOTFILES = frozenset({".gitkeep", ".gitignore", ".gitattributes", ".env.example"}) + + +def _is_safe_archive_member(rel: str) -> bool: + """Path validator for archive install/uninstall ONLY. Same containment as + _is_safe_relative_path(), but permits a narrow allowlist of benign hidden leaf + files so extensions can ship ".gitkeep"/".env.example" without loosening the + shared validator that also gates static serving.""" + if not rel or "\x00" in rel or "\\" in rel: + return False + segments = rel.split("/") + for segment in segments[:-1]: + if not segment or segment in (".", "..") or segment.startswith("."): + return False + leaf = segments[-1] + if not leaf or leaf in (".", ".."): + return False + if leaf.startswith(".") and leaf not in _ALLOWED_ARCHIVE_DOTFILES: + return False + return True + + def _not_found(handler) -> bool: j(handler, {"error": "not found"}, status=404) return True diff --git a/tests/test_issue6619_dotfile_archive_validator.py b/tests/test_issue6619_dotfile_archive_validator.py new file mode 100644 index 00000000..0b1fa896 --- /dev/null +++ b/tests/test_issue6619_dotfile_archive_validator.py @@ -0,0 +1,58 @@ +"""Regression coverage for #6619 / #6620 — extension archive dotfile handling. + +`_is_safe_relative_path()` is shared by static serving, asset URLs and manifest +paths, and MUST stay strict (no dot-prefixed segment ever reachable) — loosening +it once let `/extensions/.env`, `/extensions/.git/config`, etc. return HTTP 200. +Archive members instead go through `_is_safe_archive_member()`, which permits a +narrow allowlist of benign hidden LEAF files (`.gitkeep`, `.env.example`, …) so +extensions can ship them, without touching the shared validator. +""" +import pytest + +from api.extensions import _is_safe_relative_path, _is_safe_archive_member + + +class TestSharedValidatorStaysStrict: + """Static serving / assets / manifest must never reach a hidden file.""" + + @pytest.mark.parametrize("rel", [ + ".env", ".secret", ".git", ".git/config", ".git/hooks/pre-commit", + ".gitkeep", ".gitignore", ".env.example", + "screenshots/.gitkeep", "sub/.env.example", "a/.git/config", + ]) + def test_rejects_every_dotfile(self, rel): + assert _is_safe_relative_path(rel) is False + + @pytest.mark.parametrize("rel", ["index.html", "assets/style.css", "a/b/c.js"]) + def test_allows_normal_paths(self, rel): + assert _is_safe_relative_path(rel) is True + + @pytest.mark.parametrize("rel", ["../evil", "a/../../etc", "a/./b", "..", ".", "", "a\x00b", "a\\b"]) + def test_still_blocks_traversal_and_junk(self, rel): + assert _is_safe_relative_path(rel) is False + + +class TestArchiveMemberAllowlist: + """Install/uninstall: benign hidden leaf files allowed, nothing else loosened.""" + + @pytest.mark.parametrize("rel", [ + ".gitkeep", ".gitignore", ".gitattributes", ".env.example", + "screenshots/.gitkeep", "a/b/.env.example", + "profile-avatars/screenshots/.gitkeep", + ]) + def test_allows_benign_leaf_dotfiles(self, rel): + assert _is_safe_archive_member(rel) is True + + @pytest.mark.parametrize("rel", ["index.html", "assets/x.css", "a/b/c.js"]) + def test_allows_normal_paths(self, rel): + assert _is_safe_archive_member(rel) is True + + @pytest.mark.parametrize("rel", [ + ".env", ".secret", ".htaccess", ".npmrc", # non-allowlisted leaf dotfiles + ".git/config", ".git/hooks/pre-commit", "a/.git/config", ".ssh/id_rsa", # dot-directories + ".gitignore/evil", # allowlisted name used as a directory + "../evil", "a/../../etc", "a/./b", "..", ".", # traversal + "", "a\x00b", "a\\b", # junk / separators + ]) + def test_rejects_everything_else(self, rel): + assert _is_safe_archive_member(rel) is False