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'" + )