From 4440d77bf32d6267775be5eba2189e1ebde0b5b5 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Thu, 18 Jun 2026 14:14:41 +1000 Subject: [PATCH] fix(update): scope install-method stamp to the code tree, not $HERMES_HOME (#48188) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The install method (docker/git/pip/...) describes the *running binary*, but detect_install_method() read it from $HERMES_HOME/.install_method — a shared DATA directory. The Docker docs deliberately bind-mount $HERMES_HOME (~/.hermes:/opt/data) so config/sessions/memory persist and can be shared with a host-side Desktop/CLI install. When a containerized gateway and a host install share one $HERMES_HOME, the home-scoped stamp is a single slot describing two installs: the published image stamps 'docker' on every boot, the host install then reads 'docker' and the in-app updater refuses to run 'hermes update' ("doesn't apply inside the Docker container"). Reinstalling the Desktop app from the DMG doesn't help because the contaminated stamp is re-read every time. Fix (option 1 — code-scoped stamp): - detect_install_method() reads /.install_method first (next to the running code, immune to the shared data dir). It falls back to the legacy $HERMES_HOME stamp for back-compat, but IGNORES a 'docker' home stamp when not actually containerized — so already-poisoned shared homes self-heal. - stamp_install_method() writes the code-scoped stamp. - install.sh stamps $INSTALL_DIR instead of $HERMES_HOME. - Dockerfile bakes 'docker' into /opt/hermes/.install_method at build time (inside the immutable block); stage2-hook.sh no longer writes the home stamp and proactively removes a stale 'docker' one to heal existing shared homes. Genuine containers still resolve to 'docker' (baked stamp, or legacy home stamp honored when containerized). Unstamped installs in generic containers still fall through to git/pip (preserves the #34397 fix). --- Dockerfile | 7 + docker/stage2-hook.sh | 26 +++- hermes_cli/config.py | 122 ++++++++++++++---- scripts/install.sh | 14 +- .../hermes_cli/test_pip_install_detection.py | 91 +++++++++++++ tests/test_install_sh_install_method_stamp.py | 40 ++++++ .../test_dockerfile_immutable_install.py | 25 ++++ .../test_stage2_hook_install_method_stamp.py | 61 +++++++++ 8 files changed, 352 insertions(+), 34 deletions(-) create mode 100644 tests/test_install_sh_install_method_stamp.py create mode 100644 tests/tools/test_stage2_hook_install_method_stamp.py diff --git a/Dockerfile b/Dockerfile index dba5af1d56e..b4ebd093697 100644 --- a/Dockerfile +++ b/Dockerfile @@ -206,9 +206,16 @@ USER root RUN mkdir -p /opt/hermes/bin && \ cp /opt/hermes/docker/hermes-exec-shim.sh /opt/hermes/bin/hermes && \ chmod 0755 /opt/hermes/bin/hermes && \ + printf 'docker\n' > /opt/hermes/.install_method && \ chown -R root:root /opt/hermes && \ chmod -R a+rX /opt/hermes && \ chmod -R a-w /opt/hermes +# The ``.install_method`` stamp is baked next to the running code (the install +# tree), NOT into $HERMES_HOME. $HERMES_HOME (/opt/data) is a shared data +# volume that is commonly bind-mounted from the host and even shared with a +# host-side Desktop/CLI install; stamping it at boot used to clobber that +# host install's marker and wrongly block its ``hermes update``. A code-scoped +# stamp is read first by detect_install_method() and is immune to the share. # Start as root so the s6-overlay stage2 hook can usermod/groupmod and chown # the data volume. Each supervised service then drops to the hermes user via # `s6-setuidgid hermes` in its run script. If HERMES_UID is unset, services diff --git a/docker/stage2-hook.sh b/docker/stage2-hook.sh index 89fc914b337..54b3bb0ac96 100755 --- a/docker/stage2-hook.sh +++ b/docker/stage2-hook.sh @@ -291,13 +291,25 @@ as_hermes mkdir -p \ "$HERMES_HOME/pairing" \ "$HERMES_HOME/platforms/pairing" -# --- Install-method stamp (read by detect_install_method() in hermes status) --- -# Preserved from the tini-era entrypoint (PR #27843). Must be written as -# the hermes user so ownership matches the file's documented owner. -# tee is invoked directly via s6-setuidgid (no `sh -c` wrapper) for the -# same shell-metacharacter safety described above. -printf 'docker\n' | as_hermes tee "$HERMES_HOME/.install_method" >/dev/null \ - || true +# --- Install-method stamp --- +# The 'docker' stamp is baked into the immutable install tree at +# /opt/hermes/.install_method (see Dockerfile), NOT written here into +# $HERMES_HOME. detect_install_method() reads the code-scoped stamp first. +# +# Why we no longer stamp $HERMES_HOME: it is a shared DATA volume, commonly +# bind-mounted from the host (~/.hermes:/opt/data) and sometimes shared with a +# host-side Desktop/CLI install. Stamping 'docker' here clobbered that host +# install's marker, so its in-app updater read 'docker' and refused to run +# 'hermes update'. To heal homes already poisoned by older images, remove a +# stale 'docker' stamp from $HERMES_HOME if one is present (the host install's +# own installer re-creates its code-scoped stamp; a genuine container relies on +# the baked /opt/hermes stamp, so deleting the data-dir copy is safe). +if [ -f "$HERMES_HOME/.install_method" ]; then + stamped="$(tr -d '[:space:]' < "$HERMES_HOME/.install_method" 2>/dev/null || true)" + if [ "$stamped" = "docker" ]; then + rm -f "$HERMES_HOME/.install_method" 2>/dev/null || true + fi +fi # --- Seed config files (only on first boot) --- seed_one() { diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 356839f9903..30d4b07384d 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -350,52 +350,124 @@ def get_managed_update_command() -> Optional[str]: return None +def _install_method_project_root(project_root: Optional[Path] = None) -> Path: + """Resolve the directory that holds the *running code* (the install tree). + + This is the parent of ``hermes_cli/`` — i.e. the git checkout for source + installs, ``/opt/hermes`` inside the published image, the venv's + site-packages root for pip installs. It is a property of the running + interpreter, NOT of ``$HERMES_HOME``, which is why a code-scoped stamp + here is immune to two installs sharing one data directory. + """ + if project_root is not None: + return project_root + return Path(__file__).parent.parent.resolve() + + def detect_install_method(project_root: Optional[Path] = None) -> str: """Detect how Hermes was installed: 'docker', 'nixos', 'homebrew', 'git', or 'pip'. Resolution order: - 1. Stamped ``~/.hermes/.install_method`` file (written by installers) - 2. HERMES_MANAGED env / .managed marker (NixOS, Homebrew) - 3. .git directory presence -> 'git' - 4. Fallback -> 'pip' + 1. Code-scoped stamp ``/.install_method`` (next to the + running code) — the authoritative marker. + 2. Legacy home-scoped stamp ``$HERMES_HOME/.install_method`` — read for + backward compatibility, but a ``docker`` value is IGNORED when we are + not actually running inside a container (see below). + 3. HERMES_MANAGED env / .managed marker (NixOS, Homebrew) + 4. .git directory presence -> 'git' + 5. Fallback -> 'pip' + + Why the stamp is code-scoped, not home-scoped (issue: shared ``~/.hermes``) + -------------------------------------------------------------------------- + The install method describes *the binary that is running*, but + ``$HERMES_HOME`` is a shared DATA directory — the Docker docs deliberately + bind-mount it (``~/.hermes:/opt/data``) so config/sessions/memory persist + and can be shared with a host-side Desktop/CLI install. When a + containerised gateway and a host install share one ``$HERMES_HOME``, a + home-scoped stamp is a single slot describing two different installs: + the container stamps ``docker`` on every boot, the host install then reads + ``docker`` and ``hermes update`` refuses to run ("doesn't apply inside the + Docker container") even though the host binary is a perfectly updatable + git/pip install. Scoping the stamp to the install tree gives each install + its own truthful marker. + + Self-healing for already-poisoned homes: a legacy ``docker`` value in the + home-scoped stamp is only honoured when we are genuinely in a container. + On a host install that read a contaminating ``docker`` stamp, we fall + through to managed/.git/pip detection instead — so existing shared-home + setups recover without the user touching anything. Note: running inside a container is NOT treated as "docker" on its own. - The two supported install paths both self-identify via the - ``.install_method`` stamp (caught by step 1), so neither relies on - container detection here: + The supported installs self-identify via the code-scoped stamp: - the curl installer (scripts/install.sh, the README/website install - command) git-clones the repo and stamps ``git``; - - the published ``nousresearch/hermes-agent`` image stamps ``docker`` - at boot via ``docker/stage2-hook.sh``. - An unsupported manual install dropped into a container (no stamp) was - wrongly classified as the published image by bare container detection, - so ``hermes update`` bailed with "doesn't apply inside the Docker - container". Without that fallback such installs fall through to the - ``.git``/pip checks and behave like any off-path install. See issue #34397. + command) git-clones the repo and stamps ``git`` next to the code; + - the published ``nousresearch/hermes-agent`` image bakes a ``docker`` + stamp into ``/opt/hermes`` at build time. + An unsupported manual install dropped into a container (no stamp) falls + through to the ``.git``/pip checks and behaves like any off-path install. + See issue #34397. """ - stamp = get_hermes_home() / ".install_method" + root = _install_method_project_root(project_root) + + # 1. Code-scoped stamp — authoritative, immune to shared $HERMES_HOME. try: - method = stamp.read_text(encoding="utf-8").strip().lower() + method = (root / ".install_method").read_text(encoding="utf-8").strip().lower() if method: return method except OSError: pass + + # 2. Legacy home-scoped stamp — back-compat. Ignore a ``docker`` value + # when we are not actually containerised: that is the signature of a + # host install whose shared $HERMES_HOME was stamped by a co-located + # container, and honouring it wrongly blocks ``hermes update``. + try: + method = ( + (get_hermes_home() / ".install_method") + .read_text(encoding="utf-8") + .strip() + .lower() + ) + if method and not (method == "docker" and not _running_in_container()): + return method + except OSError: + pass + managed = get_managed_system() if managed: return managed.lower().replace(" ", "-") - if project_root is None: - project_root = Path(__file__).parent.parent.resolve() - if (project_root / ".git").is_dir(): + if (root / ".git").is_dir(): return "git" return "pip" -def stamp_install_method(method: str) -> None: - """Write the install method to ~/.hermes/.install_method.""" - stamp = get_hermes_home() / ".install_method" +def _running_in_container() -> bool: + """Thin wrapper around ``hermes_constants.is_container`` (import-safe).""" try: - stamp.parent.mkdir(parents=True, exist_ok=True) - stamp.write_text(method + "\n", encoding="utf-8") + from hermes_constants import is_container + + return is_container() + except Exception: + return False + + +def stamp_install_method(method: str, project_root: Optional[Path] = None) -> None: + """Write the install method next to the running code (code-scoped stamp). + + The stamp lives in the install tree (``/.install_method``), + not in ``$HERMES_HOME``, so that two installs sharing one data directory + do not overwrite each other's marker. See ``detect_install_method`` for + the full rationale. + + Best-effort: if the install tree is read-only (e.g. the immutable + ``/opt/hermes`` in the published image, which instead bakes the stamp at + build time) the write silently no-ops and detection falls back to its + other signals. + """ + root = _install_method_project_root(project_root) + try: + root.mkdir(parents=True, exist_ok=True) + (root / ".install_method").write_text(method + "\n", encoding="utf-8") except OSError: pass diff --git a/scripts/install.sh b/scripts/install.sh index 2ae81d4bc21..a969f31facd 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -2732,7 +2732,12 @@ run_stage_body() { detect_os resolve_install_layout print_success - echo "git" > "$HERMES_HOME/.install_method" + # Code-scoped stamp: write next to the install tree, not into + # $HERMES_HOME. $HERMES_HOME is a shared data dir (it can be + # bind-mounted into a Docker gateway too), so a stamp there gets + # clobbered by the container's 'docker' stamp and wrongly blocks + # 'hermes update' on this host install. See detect_install_method(). + echo "git" > "$INSTALL_DIR/.install_method" ;; *) log_error "Unknown stage: $stage" @@ -2811,7 +2816,12 @@ main() { print_success - echo "git" > "$HERMES_HOME/.install_method" + # Code-scoped stamp: write next to the install tree, not into $HERMES_HOME. + # $HERMES_HOME is a shared data dir (it can be bind-mounted into a Docker + # gateway too), so a stamp there gets clobbered by the container's 'docker' + # stamp and wrongly blocks 'hermes update' on this host install. + # See detect_install_method(). + echo "git" > "$INSTALL_DIR/.install_method" } if [ "$MANIFEST_MODE" = true ]; then diff --git a/tests/hermes_cli/test_pip_install_detection.py b/tests/hermes_cli/test_pip_install_detection.py index eb06e35f2bf..673ea568759 100644 --- a/tests/hermes_cli/test_pip_install_detection.py +++ b/tests/hermes_cli/test_pip_install_detection.py @@ -48,6 +48,97 @@ def test_stamp_file_takes_precedence(tmp_path): assert detect_install_method(project_root=tmp_path) == "docker" +def test_code_scoped_stamp_wins_over_home_stamp(tmp_path): + """The stamp next to the running code is authoritative over $HERMES_HOME. + + Models a host git install whose $HERMES_HOME is shared with (and stamped + 'docker' by) a co-located container. The code-scoped stamp must win so the + host install is correctly identified as 'git' and 'hermes update' works. + """ + code = tmp_path / "code" + home = tmp_path / "home" + code.mkdir() + home.mkdir() + (code / ".install_method").write_text("git\n") + (home / ".install_method").write_text("docker\n") # container contamination + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=home): + from hermes_cli.config import detect_install_method + assert detect_install_method(project_root=code) == "git" + + +def test_home_docker_stamp_ignored_when_not_containerized(tmp_path): + """A 'docker' home stamp is ignored on a host (non-container) install. + + Self-heal path for homes already poisoned by an older image that wrote + 'docker' into the shared $HERMES_HOME. With no code-scoped stamp, a host + git checkout must fall through to '.git' detection rather than honour the + contaminating 'docker' value and refuse to update. + """ + code = tmp_path / "code" + home = tmp_path / "home" + code.mkdir() + home.mkdir() + (code / ".git").mkdir() + (home / ".install_method").write_text("docker\n") + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=home), \ + patch("hermes_cli.config._running_in_container", return_value=False): + from hermes_cli.config import detect_install_method + assert detect_install_method(project_root=code) == "git" + + +def test_home_docker_stamp_honored_inside_container(tmp_path): + """A 'docker' home stamp is still honoured when genuinely containerized. + + Back-compat: an older published image that only ever wrote the home-scoped + stamp (no baked code stamp) must still resolve to 'docker' so the update + path keeps directing the user to ``docker pull``. + """ + code = tmp_path / "code" + home = tmp_path / "home" + code.mkdir() + home.mkdir() + (home / ".install_method").write_text("docker\n") + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=home), \ + patch("hermes_cli.config._running_in_container", return_value=True): + from hermes_cli.config import detect_install_method + assert detect_install_method(project_root=code) == "docker" + + +def test_home_non_docker_stamp_still_honored_for_backcompat(tmp_path): + """Legacy non-'docker' home stamps (e.g. 'git') are still respected. + + Only the 'docker' value carries the cross-contamination risk, so a host + install that historically stamped 'git'/'pip' into $HERMES_HOME keeps + resolving from there when no code-scoped stamp exists yet. + """ + code = tmp_path / "code" + home = tmp_path / "home" + code.mkdir() + home.mkdir() + (home / ".install_method").write_text("git\n") + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=home), \ + patch("hermes_cli.config._running_in_container", return_value=False): + from hermes_cli.config import detect_install_method + assert detect_install_method(project_root=code) == "git" + + +def test_stamp_install_method_writes_code_scoped(tmp_path): + """stamp_install_method writes next to the code, not into $HERMES_HOME.""" + code = tmp_path / "code" + home = tmp_path / "home" + code.mkdir() + home.mkdir() + with patch("hermes_cli.config.get_hermes_home", return_value=home): + from hermes_cli.config import stamp_install_method + stamp_install_method("pip", project_root=code) + assert (code / ".install_method").read_text().strip() == "pip" + assert not (home / ".install_method").exists() + + def test_container_without_stamp_is_not_docker(tmp_path): """An unstamped install in a generic container must NOT be flagged as docker. diff --git a/tests/test_install_sh_install_method_stamp.py b/tests/test_install_sh_install_method_stamp.py new file mode 100644 index 00000000000..b2a1b7da8cf --- /dev/null +++ b/tests/test_install_sh_install_method_stamp.py @@ -0,0 +1,40 @@ +"""Contract test: install.sh stamps the install method next to the code tree +($INSTALL_DIR), not into the shared $HERMES_HOME. + +Background (shared-$HERMES_HOME bug) +------------------------------------ +$HERMES_HOME is a data directory users frequently bind-mount into a Docker +gateway as well (``~/.hermes:/opt/data``). The published image stamps 'docker' +there on boot, so if install.sh had written its 'git' marker into the same +$HERMES_HOME the two installs would fight over one slot — and the container, +booting last, would win and wrongly make the host install look like 'docker' +(blocking ``hermes update``). + +The fix: detect_install_method() reads a CODE-scoped stamp first, and the +installer writes ``git`` into $INSTALL_DIR (the git checkout, e.g. +``~/.hermes/hermes-agent``), which is unique to this install and immune to the +shared data dir. +""" +from __future__ import annotations + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +INSTALL_SH = REPO_ROOT / "scripts" / "install.sh" + + +def test_install_sh_stamps_code_tree_not_home() -> None: + text = INSTALL_SH.read_text() + + # Stamps the code tree. + assert text.count('echo "git" > "$INSTALL_DIR/.install_method"') >= 1, ( + "install.sh must stamp $INSTALL_DIR/.install_method (code-scoped)" + ) + + # Never stamps the shared data dir. + assert not re.search(r'>\s*"\$HERMES_HOME/\.install_method"', text), ( + "install.sh must not stamp $HERMES_HOME/.install_method — that data " + "dir may be shared with a Docker gateway whose 'docker' stamp would " + "clobber it and block host-side `hermes update`" + ) diff --git a/tests/tools/test_dockerfile_immutable_install.py b/tests/tools/test_dockerfile_immutable_install.py index a1ae5377b68..fc7804f5d63 100644 --- a/tests/tools/test_dockerfile_immutable_install.py +++ b/tests/tools/test_dockerfile_immutable_install.py @@ -59,3 +59,28 @@ def test_dockerfile_does_not_chown_install_trees_to_hermes() -> None: "runtime install trees under /opt/hermes must stay immutable; " f"found forbidden pattern {pattern!r}" ) + + +def test_dockerfile_bakes_code_scoped_install_method_stamp() -> None: + """The 'docker' install-method stamp is baked next to the code. + + detect_install_method() reads the code-scoped stamp + (/opt/hermes/.install_method) first; baking it at build time keeps the + published image self-identifying as 'docker' WITHOUT writing into the + shared $HERMES_HOME data volume (which a host install may also use). + It must live inside the immutable block so the runtime user can't alter it. + """ + text = _dockerfile_text() + assert "printf 'docker\\n' > /opt/hermes/.install_method" in text + + immutable_block = re.search( + r"RUN mkdir -p /opt/hermes/bin && \\\n" + r"(?:.*\\\n)+?" + r"\s+chmod -R a-w /opt/hermes", + text, + ) + assert immutable_block, "immutable block must exist" + assert ".install_method" in immutable_block.group(0), ( + "the code-scoped install-method stamp must be baked inside the " + "immutable /opt/hermes block" + ) diff --git a/tests/tools/test_stage2_hook_install_method_stamp.py b/tests/tools/test_stage2_hook_install_method_stamp.py new file mode 100644 index 00000000000..60835f3bad5 --- /dev/null +++ b/tests/tools/test_stage2_hook_install_method_stamp.py @@ -0,0 +1,61 @@ +"""Contract test: the s6-overlay stage2 hook must NOT stamp the install method +into the shared $HERMES_HOME, and must heal a stale 'docker' stamp left there +by older images. + +Background (shared-$HERMES_HOME bug) +------------------------------------ +$HERMES_HOME (/opt/data) is a DATA volume that users commonly bind-mount from +the host (``~/.hermes:/opt/data``) and sometimes share with a host-side +Desktop/CLI install. Older images wrote ``printf 'docker' > $HERMES_HOME/.install_method`` +at boot, which clobbered the host install's own marker — so the host's in-app +updater read 'docker' and refused to run ``hermes update`` ("doesn't apply +inside the Docker container"). + +The fix scopes the stamp to the install tree (baked at +``/opt/hermes/.install_method`` in the Dockerfile, read first by +``detect_install_method``). stage2 must therefore: + + * NOT write the 'docker' stamp into $HERMES_HOME any more, and + * proactively remove a stale 'docker' stamp from $HERMES_HOME so homes + already poisoned by an older image self-heal on the next boot. +""" +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +STAGE2_HOOK = REPO_ROOT / "docker" / "stage2-hook.sh" + + +@pytest.fixture(scope="module") +def stage2_text() -> str: + if not STAGE2_HOOK.exists(): + pytest.skip("docker/stage2-hook.sh not present in this checkout") + return STAGE2_HOOK.read_text() + + +def test_stage2_does_not_write_install_method_into_home(stage2_text: str) -> None: + # No write/tee of the home-scoped install-method stamp anywhere. + assert not re.search( + r"(tee|>)\s*\"?\$HERMES_HOME/\.install_method", stage2_text + ), ( + "stage2 must not stamp $HERMES_HOME/.install_method — that data dir " + "may be shared with a host install whose marker would be clobbered" + ) + + +def test_stage2_heals_stale_docker_home_stamp(stage2_text: str) -> None: + # It must remove a stale 'docker' stamp from $HERMES_HOME so already + # poisoned shared homes recover. + assert 'rm -f "$HERMES_HOME/.install_method"' in stage2_text, ( + "stage2 must remove a stale 'docker' stamp from $HERMES_HOME to heal " + "homes poisoned by older images" + ) + # The removal must be guarded on the value being 'docker' so we never + # delete a legitimately-different stamp a user/host install put there. + assert re.search(r'\[\s*"\$stamped"\s*=\s*"docker"\s*\]', stage2_text), ( + "the stale-stamp removal must be guarded on the value == 'docker'" + )