diff --git a/contributors/emails/bradmarshall987@users.noreply.github.com b/contributors/emails/bradmarshall987@users.noreply.github.com new file mode 100644 index 0000000000..aa50e7ccff --- /dev/null +++ b/contributors/emails/bradmarshall987@users.noreply.github.com @@ -0,0 +1 @@ +bradmarshall987 diff --git a/hermes_constants.py b/hermes_constants.py index 9b6c08a156..e26075ba34 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -350,6 +350,11 @@ _HERMES_NODE_TARGET_MAJOR = int(os.environ.get("HERMES_NODE_TARGET_MAJOR", "22") _managed_node_heal_attempted = False _NODE_BOOTSTRAP_SCRIPT = Path(__file__).resolve().parent / "scripts" / "lib" / "node-bootstrap.sh" +# Install tree root (this file lives at /hermes_constants.py). +# Used by secure_parent_dir() to skip chmod on the install dir — chmodding it +# 0700 breaks hermes-user traversal in Docker (UID 10000). See #25821, #93050. +_INSTALL_ROOT = Path(__file__).resolve().parent + def node_tool_runnable(path: str | None) -> bool: """Return True only when *path* is a Node/npm/npx binary that actually runs. @@ -1023,14 +1028,10 @@ def secure_parent_dir(path: Path) -> None: # Refuse root and its direct children (/usr, /home, /var, /tmp, …). if parent == Path("/") or len(parent.parts) < 3: return - # Refuse /opt/hermes. The install dir lives on the image layer; - # chmodding it to 0700 breaks hermes-user traversal and produces - # spurious "Permission denied" on every new exec until manual - # `chmod 0755 /opt/hermes`. Reproducer: any auth write to a file - # directly under /opt/hermes (e.g. /opt/hermes/auth.json when - # HERMES_HOME resolves there) triggers the 0700 chmod and locks - # out UID 10000. See issue #25821 follow-up. - if str(parent) == "/opt/hermes": + # Refuse the install tree root. chmodding it 0700 breaks hermes-user + # traversal in Docker (UID 10000) and any other install where the + # runtime user doesn't own the install dir. See #25821, #93050. + if parent == _INSTALL_ROOT or _INSTALL_ROOT in parent.parents: return try: os.chmod(parent, 0o700) diff --git a/tests/test_hermes_constants.py b/tests/test_hermes_constants.py index 9df43e08ff..0aef996614 100644 --- a/tests/test_hermes_constants.py +++ b/tests/test_hermes_constants.py @@ -560,8 +560,29 @@ class TestSecureParentDir: secure_parent_dir(Path("/foo")) assert called_with == [] + def test_install_tree_skipped(self, monkeypatch): + """Parent dir equal to (or inside) the install tree must NOT be chmod'd. + Regression test for #93050: secure_parent_dir() chmod'd /opt/hermes to + 0700 because it has 3 path parts and passed the ``< 3`` guard, locking + out UID 10000 (hermes user) from traversing the install dir. + """ + install_root = Path(hermes_constants.__file__).resolve().parent + # Directly under the install root (e.g. /opt/hermes/auth.json) + target = install_root / "auth.json" + called_with = [] + monkeypatch.setattr(os, "chmod", lambda p, m: called_with.append((str(p), m))) + secure_parent_dir(target) + assert called_with == [], "must not chmod the install root" + + # Inside a subdirectory of the install root + sub = install_root / "subdir" + target2 = sub / "auth.json" + called_with2 = [] + monkeypatch.setattr(os, "chmod", lambda p, m: called_with2.append((str(p), m))) + secure_parent_dir(target2) + assert called_with2 == [], "must not chmod dirs inside the install tree" @pytest.mark.require_symlinks def test_symlink_resolved(self, tmp_path, monkeypatch):