Harden hosted Docker install tree against self-modification (#47490)
* Harden hosted Docker install tree * Document hosted Docker immutable install tree
This commit is contained in:
@@ -0,0 +1,61 @@
|
||||
"""Contract tests for the Docker image's immutable /opt/hermes install tree."""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
DOCKERFILE = REPO_ROOT / "Dockerfile"
|
||||
|
||||
|
||||
def _dockerfile_text() -> str:
|
||||
return DOCKERFILE.read_text()
|
||||
|
||||
|
||||
def test_dockerfile_makes_opt_hermes_root_owned_and_non_writable() -> None:
|
||||
text = _dockerfile_text()
|
||||
|
||||
assert "COPY --chown=hermes:hermes . ." not in text
|
||||
assert "COPY . ." in text
|
||||
assert "chown -R root:root /opt/hermes" in text
|
||||
assert "chmod -R a+rX /opt/hermes" in text
|
||||
assert "chmod -R a-w /opt/hermes" 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, "Dockerfile must lock /opt/hermes after installing code/deps"
|
||||
|
||||
|
||||
def test_dockerfile_keeps_mutable_state_under_opt_data() -> None:
|
||||
text = _dockerfile_text()
|
||||
|
||||
assert "ENV HERMES_HOME=/opt/data" in text
|
||||
assert "ENV HERMES_WRITE_SAFE_ROOT=/opt/data" in text
|
||||
assert 'VOLUME [ "/opt/data" ]' in text
|
||||
|
||||
|
||||
def test_dockerfile_disables_runtime_install_mutations() -> None:
|
||||
text = _dockerfile_text()
|
||||
|
||||
assert "ENV PYTHONDONTWRITEBYTECODE=1" in text
|
||||
assert "ENV HERMES_DISABLE_LAZY_INSTALLS=1" in text
|
||||
assert "HERMES_TUI_DIR=/opt/hermes/ui-tui" in text
|
||||
|
||||
|
||||
def test_dockerfile_does_not_chown_install_trees_to_hermes() -> None:
|
||||
text = _dockerfile_text()
|
||||
forbidden_patterns = (
|
||||
r"chown\s+-R\s+hermes:hermes\s+/opt/hermes/\.venv",
|
||||
r"chown\s+-R\s+hermes:hermes\s+/opt/hermes/ui-tui",
|
||||
r"chown\s+-R\s+hermes:hermes\s+/opt/hermes/gateway",
|
||||
r"chown\s+-R\s+hermes:hermes\s+/opt/hermes/node_modules",
|
||||
)
|
||||
for pattern in forbidden_patterns:
|
||||
assert not re.search(pattern, text), (
|
||||
"runtime install trees under /opt/hermes must stay immutable; "
|
||||
f"found forbidden pattern {pattern!r}"
|
||||
)
|
||||
@@ -1,11 +1,9 @@
|
||||
"""contract test: dockerfile chowns runtime node_modules trees to hermes
|
||||
"""Contract test: Docker TUI must not require writable node_modules.
|
||||
|
||||
regression guard for #18800. the container drops privileges to the hermes
|
||||
user (uid 10000) in entrypoint.sh, then the TUI launcher's
|
||||
_tui_need_npm_install() trips on every startup (see the
|
||||
npm_config_install_links=false comment in the Dockerfile) and runs
|
||||
`npm install` in /opt/hermes/ui-tui. that install fails with EACCES unless
|
||||
the runtime node_modules trees are owned by hermes.
|
||||
Older images made /opt/hermes/ui-tui and /opt/hermes/node_modules writable so a
|
||||
runtime npm install could repair stale dependencies. The hosted install tree is
|
||||
now immutable, so the Docker image must take the prebuilt TUI bundle path
|
||||
instead of writing to node_modules at runtime.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -15,29 +13,10 @@ REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
DOCKERFILE = REPO_ROOT / "Dockerfile"
|
||||
|
||||
|
||||
def test_dockerfile_chowns_runtime_node_modules_to_hermes_user() -> None:
|
||||
def test_dockerfile_uses_prebuilt_tui_instead_of_writable_node_modules() -> None:
|
||||
text = DOCKERFILE.read_text()
|
||||
|
||||
chown_lines = [
|
||||
line for line in text.splitlines()
|
||||
if "chown" in line and "hermes:hermes" in line
|
||||
]
|
||||
assert chown_lines, (
|
||||
"Dockerfile must contain a chown -R hermes:hermes for the runtime "
|
||||
"node_modules trees; see #18800"
|
||||
)
|
||||
|
||||
chown_block = "\n".join(chown_lines)
|
||||
|
||||
# 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",
|
||||
"/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, #27221)"
|
||||
)
|
||||
assert "ENV HERMES_TUI_DIR=/opt/hermes/ui-tui" in text
|
||||
assert "cd ../ui-tui && npm run build" in text
|
||||
assert "chown -R hermes:hermes /opt/hermes/ui-tui" not in text
|
||||
assert "chown -R hermes:hermes /opt/hermes/node_modules" not in text
|
||||
|
||||
@@ -1,122 +0,0 @@
|
||||
"""Contract test: the s6-overlay stage2 hook re-chowns the build trees under
|
||||
$INSTALL_DIR (/opt/hermes/.venv, ui-tui, node_modules) to the runtime hermes
|
||||
UID whenever they are not already hermes-owned — INDEPENDENTLY of whether
|
||||
$HERMES_HOME ownership already matches.
|
||||
|
||||
Regression guard for the HERMES_UID/PUID remap path broken by #35027.
|
||||
|
||||
`usermod -u <new> hermes` re-chowns the hermes home dir ($HERMES_HOME ==
|
||||
/opt/data) to the new UID as a side effect. #35027 gated the build-tree chown
|
||||
behind `stat $HERMES_HOME != hermes_uid`, so after any remap that stat is
|
||||
already satisfied and the build-tree chown was silently skipped — leaving
|
||||
.venv owned by the build-time UID (10000) and breaking:
|
||||
- lazy_deps.py `uv pip install` of platform extras (#15012, #21100)
|
||||
- the TUI esbuild rebuild into ui-tui/dist (#28851)
|
||||
|
||||
The fix probes the build trees directly (stat .venv) rather than $HERMES_HOME.
|
||||
|
||||
The extraction + stubbed-shell-run approach mirrors
|
||||
tests/tools/test_stage2_hook_toplevel_chown.py.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
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 _build_tree_block(text: str) -> str:
|
||||
"""Extract the build-tree chown block: from the `venv_owner=` probe
|
||||
through the closing `fi` of the chown."""
|
||||
m = re.search(
|
||||
r"(venv_owner=\$\(stat[^\n]*\n(?:.*\n)*?fi)",
|
||||
text,
|
||||
)
|
||||
assert m, "stage2-hook.sh must contain the venv_owner-gated build-tree chown block"
|
||||
return m.group(1)
|
||||
|
||||
|
||||
def test_build_tree_chown_not_gated_on_hermes_home(stage2_text: str) -> None:
|
||||
"""The build-tree chown must NOT live inside the `if [ "$needs_chown" = true ]`
|
||||
block keyed on $HERMES_HOME ownership — that is exactly the #35027 bug."""
|
||||
block = _build_tree_block(stage2_text)
|
||||
# The block probes the venv owner, not $HERMES_HOME.
|
||||
assert "venv_owner" in block
|
||||
assert "$INSTALL_DIR/.venv" in block
|
||||
# All three build trees are covered.
|
||||
for tree in ("$INSTALL_DIR/.venv", "$INSTALL_DIR/ui-tui", "$INSTALL_DIR/node_modules"):
|
||||
assert tree in block, f"build-tree chown must cover {tree}"
|
||||
|
||||
|
||||
def _run_build_tree_block(
|
||||
text: str, *, venv_owner: int, hermes_uid: int
|
||||
) -> bool:
|
||||
"""Run the extracted build-tree block with `stat`, `id`, and `chown`
|
||||
stubbed. Returns True iff the block attempted the recursive chown."""
|
||||
bash = shutil.which("bash")
|
||||
if bash is None:
|
||||
pytest.skip("bash not available")
|
||||
block = _build_tree_block(text)
|
||||
|
||||
with tempfile.TemporaryDirectory() as d:
|
||||
dpath = Path(d)
|
||||
log = dpath / "chown.log"
|
||||
# Stubs:
|
||||
# stat -c %u <path> -> echo the simulated venv owner
|
||||
# id -u hermes -> handled via actual_hermes_uid var below
|
||||
# chown ... -> record that it fired
|
||||
script = (
|
||||
"set -eu\n"
|
||||
f'INSTALL_DIR="/opt/hermes"\n'
|
||||
f'actual_hermes_uid={hermes_uid}\n'
|
||||
f'stat() {{ echo {venv_owner}; }}\n'
|
||||
f'chown() {{ echo fired >> "{log}"; }}\n'
|
||||
+ block
|
||||
)
|
||||
script_path = dpath / "harness.sh"
|
||||
script_path.write_text(script)
|
||||
proc = subprocess.run([bash, str(script_path)], capture_output=True, text=True)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
return log.exists() and "fired" in log.read_text()
|
||||
|
||||
|
||||
def test_chown_fires_when_venv_owner_differs(stage2_text: str) -> None:
|
||||
"""The #35027 regression scenario: after a remap $HERMES_HOME already
|
||||
matches the new UID, but the venv is still owned by the build-time UID
|
||||
(10000). The build-tree chown MUST still fire."""
|
||||
fired = _run_build_tree_block(stage2_text, venv_owner=10000, hermes_uid=4242)
|
||||
assert fired, (
|
||||
"build-tree chown must fire when the venv is not owned by the runtime "
|
||||
"hermes UID, regardless of $HERMES_HOME ownership (#35027 regression)"
|
||||
)
|
||||
|
||||
|
||||
def test_chown_skipped_when_venv_already_owned(stage2_text: str) -> None:
|
||||
"""Idempotency: once the venv is hermes-owned, the recursive chown is
|
||||
skipped on subsequent boots."""
|
||||
fired = _run_build_tree_block(stage2_text, venv_owner=4242, hermes_uid=4242)
|
||||
assert not fired, (
|
||||
"build-tree chown must be skipped when the venv already matches the "
|
||||
"runtime hermes UID (avoid expensive recursive chown on every restart)"
|
||||
)
|
||||
|
||||
|
||||
def test_chown_skipped_for_default_uid(stage2_text: str) -> None:
|
||||
"""No remap: venv owned by the default build UID (10000) and hermes is
|
||||
still 10000 — nothing to do."""
|
||||
fired = _run_build_tree_block(stage2_text, venv_owner=10000, hermes_uid=10000)
|
||||
assert not fired
|
||||
@@ -0,0 +1,48 @@
|
||||
"""Contract tests for the Docker stage2 immutable install-tree policy.
|
||||
|
||||
Hosted/container Hermes keeps user-writable state under HERMES_HOME
|
||||
(/opt/data). The installed source, venv, TUI bundle, and node_modules under
|
||||
/opt/hermes must remain root-owned/non-writable by the runtime hermes user so
|
||||
an agent session cannot self-modify the installation and brick the gateway.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
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_chown_install_tree_to_hermes(stage2_text: str) -> None:
|
||||
assert "Fixing ownership of build trees under $INSTALL_DIR" not in stage2_text
|
||||
assert 'chown -R hermes:hermes \\\n "$INSTALL_DIR/.venv"' not in stage2_text
|
||||
|
||||
assert "venv_owner=$(stat -c %u \"$INSTALL_DIR/.venv\"" not in stage2_text
|
||||
assert "chown of build trees failed" not in stage2_text
|
||||
for install_tree in (
|
||||
'"$INSTALL_DIR/.venv" \\',
|
||||
'"$INSTALL_DIR/ui-tui" \\',
|
||||
'"$INSTALL_DIR/gateway" \\',
|
||||
'"$INSTALL_DIR/node_modules" \\',
|
||||
):
|
||||
assert install_tree not in stage2_text, (
|
||||
f"stage2 must not chown {install_tree} back to hermes; "
|
||||
"the Dockerfile keeps /opt/hermes immutable and writable state "
|
||||
"belongs under HERMES_HOME"
|
||||
)
|
||||
|
||||
|
||||
def test_stage2_documents_immutable_install_contract(stage2_text: str) -> None:
|
||||
assert "Immutable install tree" in stage2_text
|
||||
assert "PYTHONDONTWRITEBYTECODE" in stage2_text
|
||||
assert "HERMES_DISABLE_LAZY_INSTALLS=1" in stage2_text
|
||||
assert "/opt/hermes" in stage2_text
|
||||
@@ -1,57 +0,0 @@
|
||||
"""Contract test: stage2-hook repairs ownership of the gateway install tree.
|
||||
|
||||
When HERMES_UID is remapped at container boot, ``usermod -u`` only rewrites
|
||||
files under the hermes user's home directory ($HERMES_HOME == /opt/data).
|
||||
Runtime-writable trees under ``/opt/hermes`` must be explicitly chowned to the
|
||||
new UID before services drop privileges. ``/opt/hermes/gateway`` is one such
|
||||
tree: Python writes ``__pycache__`` beneath the package on first import, which
|
||||
fails with EACCES if the tree still belongs to the build-time UID (10000) after
|
||||
a remap (#27221).
|
||||
"""
|
||||
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, (
|
||||
"the build-tree ownership repair must chown $INSTALL_DIR/gateway so the "
|
||||
"gateway runtime can write Python cache artifacts after a UID remap (#27221)"
|
||||
)
|
||||
|
||||
|
||||
def test_install_dir_chown_keeps_existing_runtime_writable_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
|
||||
Reference in New Issue
Block a user