From 451fe25f76e007e387989ae3e4e1ba93d24a7814 Mon Sep 17 00:00:00 2001 From: Sarvesh Date: Wed, 3 Jun 2026 11:03:24 +0530 Subject: [PATCH] fix(docker): chown gateway after UID remap --- Dockerfile | 5 +- docker/stage2-hook.sh | 13 +++- .../test_dockerfile_node_modules_perms.py | 10 ++- .../test_stage2_hook_install_dir_chown.py | 68 +++++++++++++++++++ 4 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 tests/tools/test_stage2_hook_install_dir_chown.py diff --git a/Dockerfile b/Dockerfile index eeaf20ddafb9a..afbd3e73de78a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -178,13 +178,16 @@ RUN cd web && npm run build && \ # hermes_cli/main.py succeeds (see #18800). /opt/hermes/web is build-time # only (HERMES_WEB_DIST points at hermes_cli/web_dist) and is intentionally # not chowned here. +# /opt/hermes/gateway is runtime-writable as Python may create __pycache__ and +# gateway state artifacts after services drop privileges, especially when the +# hermes UID is remapped at boot (#27221). # The .venv MUST remain hermes-writable so lazy_deps.py can install # remaining optional platform packages and future pin bumps at first use. # Without this, `uv pip install` fails with EACCES and adapters silently # fail to load. See tools/lazy_deps.py. USER root RUN chmod -R a+rX /opt/hermes && \ - chown -R hermes:hermes /opt/hermes/.venv /opt/hermes/ui-tui /opt/hermes/node_modules + chown -R hermes:hermes /opt/hermes/.venv /opt/hermes/ui-tui /opt/hermes/gateway /opt/hermes/node_modules # 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 56925198ebfb0..3f937be4ff421 100755 --- a/docker/stage2-hook.sh +++ b/docker/stage2-hook.sh @@ -52,16 +52,20 @@ validate_uid_gid() { # HERMES_GID still win when both are set. See #15290, salvages #25872. HERMES_UID="${HERMES_UID:-${PUID:-}}" HERMES_GID="${HERMES_GID:-${PGID:-}}" +uid_gid_remapped=false if [ -n "${HERMES_UID:-}" ] && validate_uid_gid "$HERMES_UID" && [ "$HERMES_UID" != "$(id -u hermes)" ]; then echo "[stage2] Changing hermes UID to $HERMES_UID" usermod -u "$HERMES_UID" hermes + uid_gid_remapped=true fi if [ -n "${HERMES_GID:-}" ] && validate_uid_gid "$HERMES_GID" && [ "$HERMES_GID" != "$(id -g hermes)" ]; then echo "[stage2] Changing hermes GID to $HERMES_GID" # -o allows non-unique GID (e.g. macOS GID 20 "staff" may already # exist as "dialout" in the Debian-based container image). - groupmod -o -g "$HERMES_GID" hermes 2>/dev/null || true + if groupmod -o -g "$HERMES_GID" hermes 2>/dev/null; then + uid_gid_remapped=true + fi fi # --- Docker socket group membership (docker-in-docker / DooD) --- @@ -134,7 +138,7 @@ needs_chown=false if [ "$(stat -c %u "$HERMES_HOME" 2>/dev/null)" != "$actual_hermes_uid" ]; then needs_chown=true fi -if [ "$needs_chown" = true ]; then +if [ "$needs_chown" = true ] || [ "$uid_gid_remapped" = true ]; then echo "[stage2] Fixing ownership of $HERMES_HOME (targeted) to hermes ($actual_hermes_uid)" # In rootless Podman the container's "root" is mapped to an # unprivileged host UID — chown will fail. That's fine: the volume @@ -162,6 +166,10 @@ if [ "$needs_chown" = true ]; then # the source mtime is newer than dist/ or when HERMES_TUI_FORCE_BUILD # is set) and writes to ui-tui/dist/. Without this chown the new # hermes UID can't write the build output (#28851). + # - gateway: Python writes __pycache__ and runtime artifacts beneath the + # gateway package on first use. After UID remap, those source-owned + # paths still belong to numeric UID 10000 unless we repair them here + # (#27221). # - node_modules: root-level dependencies (puppeteer, web tooling) # that runtime code may walk/update. # The set mirrors the build-time `chown -R hermes:hermes` line in the @@ -171,6 +179,7 @@ if [ "$needs_chown" = true ]; then chown -R hermes:hermes \ "$INSTALL_DIR/.venv" \ "$INSTALL_DIR/ui-tui" \ + "$INSTALL_DIR/gateway" \ "$INSTALL_DIR/node_modules" \ 2>/dev/null || \ echo "[stage2] Warning: chown of build trees failed (rootless container?) — continuing" diff --git a/tests/tools/test_dockerfile_node_modules_perms.py b/tests/tools/test_dockerfile_node_modules_perms.py index 56243248abe08..671a8d843a0a1 100644 --- a/tests/tools/test_dockerfile_node_modules_perms.py +++ b/tests/tools/test_dockerfile_node_modules_perms.py @@ -29,11 +29,15 @@ def test_dockerfile_chowns_runtime_node_modules_to_hermes_user() -> None: chown_block = "\n".join(chown_lines) - # both runtime-mutable trees must be passed to the chown command. + # Runtime-mutable trees must be passed to the chown command. # /opt/hermes/web is intentionally excluded: it is build-time only, # because HERMES_WEB_DIST points at hermes_cli/web_dist for runtime. - for required_path in ("/opt/hermes/ui-tui", "/opt/hermes/node_modules"): + for required_path in ( + "/opt/hermes/ui-tui", + "/opt/hermes/node_modules", + "/opt/hermes/gateway", + ): assert required_path in chown_block, ( f"{required_path} must be passed to a chown -R hermes:hermes " - f"command in the Dockerfile (see #18800)" + f"command in the Dockerfile (see #18800, #27221)" ) diff --git a/tests/tools/test_stage2_hook_install_dir_chown.py b/tests/tools/test_stage2_hook_install_dir_chown.py new file mode 100644 index 0000000000000..1fd05382ea60d --- /dev/null +++ b/tests/tools/test_stage2_hook_install_dir_chown.py @@ -0,0 +1,68 @@ +"""Contract tests for install-dir ownership repair in docker/stage2-hook.sh. + +When HERMES_UID is remapped at container boot, ``usermod -u`` only rewrites +files under the hermes user's home directory. Runtime-writable trees under +``/opt/hermes`` must be explicitly chowned to the new UID before services drop +privileges. +""" +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 _install_dir_chown_block(text: str) -> str: + match = re.search( + r"(chown -R hermes:hermes \\\n" + r"(?:\s+\"\$INSTALL_DIR/[^\"]+\" \\\n)+" + r"\s+2>/dev/null \|\| \\\n" + r"\s+echo \"\[stage2\] Warning: chown of build trees failed.*?\")", + text, + flags=re.DOTALL, + ) + assert match, "stage2-hook.sh must repair ownership of runtime-writable install trees" + return match.group(1) + + +def test_uid_remap_chowns_runtime_writable_gateway_tree(stage2_text: str) -> None: + block = _install_dir_chown_block(stage2_text) + + assert '"$INSTALL_DIR/gateway"' in block, ( + "UID remap must chown $INSTALL_DIR/gateway so the gateway runtime can " + "write Python cache/runtime artifacts after services drop to hermes (#27221)" + ) + + +def test_install_dir_chown_runs_when_identity_is_remapped(stage2_text: str) -> None: + assert "uid_gid_remapped=false" in stage2_text + assert "uid_gid_remapped=true" in stage2_text + assert ( + 'if [ "$needs_chown" = true ] || [ "$uid_gid_remapped" = true ]; then' + in stage2_text + ), ( + "runtime-writable install trees must be chowned when HERMES_UID/GID is " + "remapped, even if usermod already updated $HERMES_HOME ownership (#27221)" + ) + + +def test_uid_remap_keeps_existing_runtime_writable_install_trees(stage2_text: str) -> None: + block = _install_dir_chown_block(stage2_text) + + for required in ( + '"$INSTALL_DIR/.venv"', + '"$INSTALL_DIR/ui-tui"', + '"$INSTALL_DIR/node_modules"', + ): + assert required in block \ No newline at end of file