diff --git a/CHANGELOG.md b/CHANGELOG.md index 249686f2..7c39945e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Extensions that ship a `.gitkeep` (or `.gitignore`/`.gitattributes`/`.env.example`) install again instead of failing with "Unsafe archive member".** The archive validator rejected *any* dot-prefixed path segment, which also blocked the benign placeholder files extensions legitimately carry — so an extension with an empty `assets/` directory couldn't be installed at all. Archive install/uninstall now use a dedicated validator that permits a short allowlist of hidden files **as the final path segment only**; dot-directories (`.git/`, `.ssh/`) and every other dotfile (`.env`, secrets) stay rejected, as do traversal, absolute paths, encoded separators, NUL/backslash, and an allowlisted name used as a directory. The shared strict validator is unchanged and still gates static serving, asset URLs, and manifest paths, so an installed hidden file lands on disk but can never be fetched over HTTP or injected into a page (`.env.example` is installable but not servable; `.env` remains rejected outright). Thanks @asorourx. (#6620, closes #6619) + - **Passkey sign-in works behind a reverse proxy that rewrites `Host`.** WebAuthn scopes a credential to an "RPID" that must match the origin the browser is actually on, but the RPID was derived from the `Host` header — which a proxy fronting a separate SPA rewrites to its own upstream address, so registration and login failed with an RP ID mismatch. The RPID now comes from the request `Origin` (accepted only as a well-formed `http(s)` origin with a hostname, rebuilt from its parsed parts, otherwise falling back to `Host`). This is not a trust decision: the authenticator still only releases a credential whose RPID is a registrable suffix of the real page origin, `clientDataJSON.origin` must equal the origin stored with the challenge, the authenticator's `rpIdHash` must match that stored RPID, and the assertion signature is still verified against the stored credential public key. Thanks @cabluvsmkm2009-source. (#6772) - **Trusted-header auth can be told to accept pipe-separated groups from the identity proxy.** An Authentik outpost (depending on its proxy provider / property mapping) can emit a group header like `admins|developpeur`; the parser only split on commas and newlines, so the whole string was read as one group name, matched nothing in the configured group→profile map, and the session silently fell back to the unbound `default` profile despite a legitimate mapped membership. Setting `HERMES_WEBUI_TRUSTED_GROUPS_PIPE_SEPARATOR=1` now also treats `|` as a separator (comma and newline stay the default). It is opt-in on purpose: a group *name* can legitimately contain a literal `|`, so splitting on it unconditionally could silently change an existing deployment's profile binding. Repeated or adjacent separators collapse instead of producing empty group names. Thanks @SamsGuamejy. (#7331) diff --git a/api/extensions.py b/api/extensions.py index ba7104e4..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,6 +2068,9 @@ 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("/"): @@ -2076,6 +2079,31 @@ def _is_safe_relative_path(rel: str) -> bool: 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