diff --git a/.hadolint.yaml b/.hadolint.yaml index 295211278a7..81e80c14b61 100644 --- a/.hadolint.yaml +++ b/.hadolint.yaml @@ -24,11 +24,10 @@ ignored: # expensive layer-cached step we want isolated, and merging them # would invalidate the cache for trivial changes. - DL3059 - # Last USER should not be root. The entrypoint is responsible for - # gosu-dropping to the hermes user; running as root is required so - # usermod/groupmod can remap UIDs per HERMES_UID at runtime. Phase 2 - # of the s6-overlay migration preserves this contract — /init runs - # as root, individual services drop via s6-setuidgid. + # Last USER should not be root. /init (s6-overlay) runs as root so the + # stage2 hook can usermod/groupmod and chown the data volume per + # HERMES_UID at runtime; each supervised service then drops to the + # hermes user via `s6-setuidgid`. - DL3002 # Require explicit base-image pins (SHA256) — we already do this. diff --git a/Dockerfile b/Dockerfile index 1238f5f7565..f13ab6bd6d7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,4 @@ FROM ghcr.io/astral-sh/uv:0.11.6-python3.13-trixie@sha256:b3c543b6c4f23a5f2df22866bd7857e5d304b67a564f4feab6ac22044dde719b AS uv_source -FROM tianon/gosu:1.19-trixie@sha256:3b176695959c71e123eb390d427efc665eeb561b1540e82679c15e992006b8b9 AS gosu_source FROM debian:13.4 # Disable Python stdout buffering to ensure logs are printed immediately @@ -38,7 +37,6 @@ RUN tar -C / -Jxpf /tmp/s6-overlay-noarch.tar.xz && \ # Non-root user for runtime; UID can be overridden via HERMES_UID at runtime RUN useradd -u 10000 -m -d /opt/data hermes -COPY --chmod=0755 --from=gosu_source /gosu /usr/local/bin/ COPY --chmod=0755 --from=uv_source /usr/local/bin/uv /usr/local/bin/uvx /usr/local/bin/ WORKDIR /opt/hermes @@ -121,8 +119,10 @@ RUN cd web && npm run build && \ USER root RUN chmod -R a+rX /opt/hermes && \ chown -R hermes:hermes /opt/hermes/.venv /opt/hermes/ui-tui /opt/hermes/node_modules -# Start as root so the entrypoint can usermod/groupmod + gosu. -# If HERMES_UID is unset, the entrypoint drops to the default hermes user (10000). +# 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 +# run as the default hermes user (UID 10000). # ---------- Link hermes-agent itself (editable) ---------- # Deps are already installed in the cached layer above; `--no-deps` makes @@ -138,8 +138,8 @@ RUN uv pip install --no-cache-dir --no-deps -e "." COPY docker/s6-rc.d/ /etc/s6-overlay/s6-rc.d/ # stage2-hook handles UID/GID remap, volume chown, config seeding, -# skills sync — all the work the old entrypoint.sh did between -# gosu-drop and `exec hermes`. Wired in as cont-init.d/01- so it +# skills sync — all the work the old entrypoint.sh did before +# `exec hermes`. Wired in as cont-init.d/01- so it # runs before user services start. # # 02-reconcile-profiles re-creates per-profile gateway s6 service diff --git a/docker-compose.yml b/docker-compose.yml index 8bdc96b7a97..e7cc0fb7dba 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,8 +6,9 @@ # # Set HERMES_UID / HERMES_GID to the host user that owns ~/.hermes so # files created inside the container stay readable/writable on the host. -# The entrypoint remaps the internal `hermes` user to these values via -# usermod/groupmod + gosu. +# The s6-overlay stage2 hook remaps the internal `hermes` user to these +# values via usermod/groupmod; each supervised service then drops to that +# user via `s6-setuidgid`. # # Security notes: # - The dashboard service binds to 127.0.0.1 by default. It stores API diff --git a/docker/main-wrapper.sh b/docker/main-wrapper.sh index 8a430ba6b06..0e25e5adf91 100755 --- a/docker/main-wrapper.sh +++ b/docker/main-wrapper.sh @@ -9,8 +9,8 @@ # first arg is an executable → exec it directly (sleep, bash, sh, …) # first arg is anything else → exec `hermes ` (subcommand passthrough) # -# We drop to the hermes user via `s6-setuidgid` — running as that -# user matches the pre-s6 contract (gosu drop). +# We drop to the hermes user via `s6-setuidgid` so the supervised +# workload runs unprivileged (UID 10000 by default). set -e cd /opt/data diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 9f457d741d0..61f46935bc5 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -658,7 +658,8 @@ DEFAULT_CONFIG = { # are owned by your host user instead of root, which avoids needing # `sudo chown` after container runs. Default off to preserve behavior # for images whose entrypoints expect to start as root (e.g. the - # bundled Hermes image, which drops to the `hermes` user via gosu). + # bundled Hermes image, which drops to the `hermes` user via + # s6-setuidgid inside each supervised service). # When on, SETUID/SETGID caps are omitted from the container since # no privilege drop is needed. "docker_run_as_host_user": False, diff --git a/tests/tools/test_docker_environment.py b/tests/tools/test_docker_environment.py index cd3b7aae6f6..439d59bd76c 100644 --- a/tests/tools/test_docker_environment.py +++ b/tests/tools/test_docker_environment.py @@ -385,18 +385,19 @@ def test_normalize_env_dict_rejects_complex_values(): assert result == {"GOOD": "string"} -def test_security_args_include_setuid_setgid_for_gosu_drop(monkeypatch): +def test_security_args_include_setuid_setgid_for_privdrop(monkeypatch): """The default (run_as_host_user=False) invocation must include SETUID and - SETGID caps so the image entrypoint can drop from root to the non-root - `hermes` user via gosu. + SETGID caps so the image's init can drop from root to a non-root user + (e.g. via ``s6-setuidgid`` in the bundled Hermes image, or ``gosu``/``su`` + in user-provided images). - Without these caps gosu exits with - ``error: failed switching to 'hermes': operation not permitted`` - and the container exits immediately (exit 1) before running any work. + Without these caps the privilege-drop helper fails with + ``operation not permitted`` and the container exits immediately (exit 1) + before running any work. - `no-new-privileges` is kept, so gosu still cannot escalate back to root - after the drop — the drop is a one-way transition performed before the - `no_new_privs` bit is enforced on the exec boundary. + ``no-new-privileges`` is kept, so the dropped process still cannot + escalate back to root after the drop — the drop is a one-way transition + performed before the ``no_new_privs`` bit is enforced on the exec boundary. """ monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") calls = _mock_subprocess_run(monkeypatch) @@ -412,8 +413,8 @@ def test_security_args_include_setuid_setgid_for_gosu_drop(monkeypatch): for i, flag in enumerate(run_args[:-1]) if flag == "--cap-add" } - assert "SETUID" in added, "SETUID cap missing — gosu drop in entrypoint will fail" - assert "SETGID" in added, "SETGID cap missing — gosu drop in entrypoint will fail" + assert "SETUID" in added, "SETUID cap missing — image privilege-drop will fail" + assert "SETGID" in added, "SETGID cap missing — image privilege-drop will fail" # ── run_as_host_user tests ──────────────────────────────────────── @@ -441,8 +442,9 @@ def test_run_as_host_user_passes_uid_gid(monkeypatch): def test_run_as_host_user_drops_setuid_setgid_caps(monkeypatch): - """When --user is passed, the container never needs gosu, so SETUID/SETGID - caps are omitted for a tighter security posture.""" + """When --user is passed, the container already starts unprivileged and + never needs a privilege drop, so SETUID/SETGID caps are omitted for a + tighter security posture.""" monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker") monkeypatch.setattr(docker_env.os, "getuid", lambda: 1000, raising=False) monkeypatch.setattr(docker_env.os, "getgid", lambda: 1000, raising=False) @@ -459,10 +461,10 @@ def test_run_as_host_user_drops_setuid_setgid_caps(monkeypatch): if flag == "--cap-add" } assert "SETUID" not in added, ( - "SETUID cap should be dropped when running as host user — no gosu drop is needed" + "SETUID cap should be dropped when running as host user — no privilege drop is needed" ) assert "SETGID" not in added, ( - "SETGID cap should be dropped when running as host user — no gosu drop is needed" + "SETGID cap should be dropped when running as host user — no privilege drop is needed" ) # Core non-privilege-drop caps must still be there (pip/npm/apt need them). assert "DAC_OVERRIDE" in added diff --git a/tools/environments/docker.py b/tools/environments/docker.py index 1cd72ce8552..ed53cd07c41 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -148,12 +148,14 @@ def find_docker() -> Optional[str]: # We drop all capabilities then add back the minimum needed: # DAC_OVERRIDE - root can write to bind-mounted dirs owned by host user # CHOWN/FOWNER - package managers (pip, npm, apt) need to set file ownership -# SETUID/SETGID - the image entrypoint drops from root to the 'hermes' -# user via `gosu`, which requires these caps. Combined with -# `no-new-privileges`, gosu still cannot escalate back to root after -# the drop, so the security posture is preserved. Omitted entirely -# when the container starts as a non-root user via --user, since -# no gosu drop is needed in that mode. +# SETUID/SETGID - the image's init drops from root to the 'hermes' +# user (via `s6-setuidgid` in the bundled image, or whatever +# privilege-drop helper a user image uses), which requires these +# caps. Combined with `no-new-privileges`, the dropped process +# still cannot escalate back to root, so the security posture is +# preserved. Omitted entirely when the container starts as a +# non-root user via --user, since no privilege drop is needed +# in that mode. # Block privilege escalation and limit PIDs. # /tmp is size-limited and nosuid but allows exec (needed by pip/npm builds). _BASE_SECURITY_ARGS = [ @@ -168,10 +170,11 @@ _BASE_SECURITY_ARGS = [ "--tmpfs", "/run:rw,noexec,nosuid,size=64m", ] -# Extra caps needed when the container starts as root and an entrypoint -# must drop privileges via gosu/su. Skipped when --user is passed because -# the container already starts unprivileged and never needs to switch. -_GOSU_CAP_ARGS = [ +# Extra caps needed when the container starts as root and an init/entrypoint +# must drop privileges (via `s6-setuidgid`, `gosu`, `su`, or similar). +# Skipped when --user is passed because the container already starts +# unprivileged and never needs to switch. +_PRIVDROP_CAP_ARGS = [ "--cap-add", "SETUID", "--cap-add", "SETGID", ] @@ -181,7 +184,7 @@ def _build_security_args(run_as_host_user: bool) -> list[str]: """Return the security/cap/tmpfs args tailored to the privilege mode.""" if run_as_host_user: return list(_BASE_SECURITY_ARGS) - return list(_BASE_SECURITY_ARGS) + list(_GOSU_CAP_ARGS) + return list(_BASE_SECURITY_ARGS) + list(_PRIVDROP_CAP_ARGS) def _resolve_host_user_spec() -> Optional[str]: @@ -473,7 +476,7 @@ class DockerEnvironment(BaseEnvironment): "image default user." ) # Fall back to the full cap set — without --user, an image's - # entrypoint may still need gosu/su to drop privileges. + # init may still need s6-setuidgid/gosu/su to drop privileges. security_args = _build_security_args(run_as_host_user and bool(user_args)) logger.info(f"Docker volume_args: {volume_args}") diff --git a/website/docs/user-guide/docker.md b/website/docs/user-guide/docker.md index 615bafc9a5a..41d3fad7aaa 100644 --- a/website/docs/user-guide/docker.md +++ b/website/docs/user-guide/docker.md @@ -475,7 +475,7 @@ Check logs: `docker logs hermes`. Common causes: ### "Permission denied" errors -The container's entrypoint drops privileges to the non-root `hermes` user (UID 10000) via `gosu`. If your host `~/.hermes/` is owned by a different UID, set `HERMES_UID`/`HERMES_GID` to match your host user, or ensure the data directory is writable: +The container's stage2 hook drops privileges to the non-root `hermes` user (UID 10000) via `s6-setuidgid` inside each supervised service. If your host `~/.hermes/` is owned by a different UID, set `HERMES_UID`/`HERMES_GID` to match your host user, or ensure the data directory is writable: ```sh chmod -R 755 ~/.hermes