From 556e0f4b4326cf7e04e7095e7946583bce8b4296 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 17 Mar 2026 02:34:25 -0700 Subject: [PATCH] fix(docker): add explicit env allowlist for container credentials (#1436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docker terminal sessions are secret-dark by default. This adds terminal.docker_forward_env as an explicit allowlist for env vars that may be forwarded into Docker containers. Values resolve from the current shell first, then fall back to ~/.hermes/.env. Only variables the user explicitly lists are forwarded — nothing is auto-exposed. Cherry-picked from PR #1449 by @teknium1, conflict-resolved onto current main. Fixes #1436 Supersedes #1439 --- cli-config.yaml.example | 6 ++ cli.py | 2 + gateway/run.py | 1 + hermes_cli/config.py | 1 + tests/tools/test_docker_environment.py | 62 +++++++++++++++++++ tests/tools/test_parse_env_var.py | 22 +++++++ tools/environments/docker.py | 47 +++++++++++++- tools/terminal_tool.py | 3 + .../docs/reference/environment-variables.md | 1 + website/docs/user-guide/configuration.md | 20 ++++++ website/docs/user-guide/features/tools.md | 2 + website/docs/user-guide/security.md | 5 ++ 12 files changed, 170 insertions(+), 2 deletions(-) diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 4623ccfb..4e05b397 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -123,6 +123,12 @@ terminal: # lifetime_seconds: 300 # docker_image: "nikolaik/python-nodejs:python3.11-nodejs20" # docker_mount_cwd_to_workspace: true # Explicit opt-in: mount your launch cwd into /workspace +# # Optional: explicitly forward selected env vars into Docker. +# # These values come from your current shell first, then ~/.hermes/.env. +# # Warning: anything forwarded here is visible to commands run in the container. +# docker_forward_env: +# - "GITHUB_TOKEN" +# - "NPM_TOKEN" # ----------------------------------------------------------------------------- # OPTION 4: Singularity/Apptainer container diff --git a/cli.py b/cli.py index ad30e297..fb447908 100755 --- a/cli.py +++ b/cli.py @@ -161,6 +161,7 @@ def load_cli_config() -> Dict[str, Any]: "timeout": 60, "lifetime_seconds": 300, "docker_image": "python:3.11", + "docker_forward_env": [], "singularity_image": "docker://python:3.11", "modal_image": "python:3.11", "daytona_image": "nikolaik/python-nodejs:python3.11-nodejs20", @@ -325,6 +326,7 @@ def load_cli_config() -> Dict[str, Any]: "timeout": "TERMINAL_TIMEOUT", "lifetime_seconds": "TERMINAL_LIFETIME_SECONDS", "docker_image": "TERMINAL_DOCKER_IMAGE", + "docker_forward_env": "TERMINAL_DOCKER_FORWARD_ENV", "singularity_image": "TERMINAL_SINGULARITY_IMAGE", "modal_image": "TERMINAL_MODAL_IMAGE", "daytona_image": "TERMINAL_DAYTONA_IMAGE", diff --git a/gateway/run.py b/gateway/run.py index f4c23c12..f1e1be68 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -107,6 +107,7 @@ if _config_path.exists(): "timeout": "TERMINAL_TIMEOUT", "lifetime_seconds": "TERMINAL_LIFETIME_SECONDS", "docker_image": "TERMINAL_DOCKER_IMAGE", + "docker_forward_env": "TERMINAL_DOCKER_FORWARD_ENV", "singularity_image": "TERMINAL_SINGULARITY_IMAGE", "modal_image": "TERMINAL_MODAL_IMAGE", "daytona_image": "TERMINAL_DAYTONA_IMAGE", diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 65096209..85054350 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -118,6 +118,7 @@ DEFAULT_CONFIG = { "cwd": ".", # Use current directory "timeout": 180, "docker_image": "nikolaik/python-nodejs:python3.11-nodejs20", + "docker_forward_env": [], "singularity_image": "docker://nikolaik/python-nodejs:python3.11-nodejs20", "modal_image": "nikolaik/python-nodejs:python3.11-nodejs20", "daytona_image": "nikolaik/python-nodejs:python3.11-nodejs20", diff --git a/tests/tools/test_docker_environment.py b/tests/tools/test_docker_environment.py index 03b32d20..68b040e4 100644 --- a/tests/tools/test_docker_environment.py +++ b/tests/tools/test_docker_environment.py @@ -1,4 +1,5 @@ import logging +from io import StringIO import subprocess import sys import types @@ -211,3 +212,64 @@ def test_auto_mount_replaces_persistent_workspace_bind(monkeypatch, tmp_path): assert f"{project_dir}:/workspace" in run_args_str assert "/sandboxes/docker/test-persistent-auto-mount/workspace:/workspace" not in run_args_str + +class _FakePopen: + def __init__(self, cmd, **kwargs): + self.cmd = cmd + self.kwargs = kwargs + self.stdout = StringIO("") + self.stdin = None + self.returncode = 0 + + def poll(self): + return self.returncode + + +def _make_execute_only_env(forward_env=None): + env = docker_env.DockerEnvironment.__new__(docker_env.DockerEnvironment) + env.cwd = "/root" + env.timeout = 60 + env._forward_env = forward_env or [] + env._prepare_command = lambda command: (command, None) + env._timeout_result = lambda timeout: {"output": f"timed out after {timeout}", "returncode": 124} + env._inner = type("Inner", (), { + "container_id": "test-container", + "config": type("Cfg", (), {"executable": "/usr/bin/docker", "env": {}})(), + })() + return env + + +def test_execute_uses_hermes_dotenv_for_allowlisted_env(monkeypatch): + env = _make_execute_only_env(["GITHUB_TOKEN"]) + popen_calls = [] + + def _fake_popen(cmd, **kwargs): + popen_calls.append(cmd) + return _FakePopen(cmd, **kwargs) + + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + monkeypatch.setattr(docker_env, "_load_hermes_env_vars", lambda: {"GITHUB_TOKEN": "value_from_dotenv"}) + monkeypatch.setattr(docker_env.subprocess, "Popen", _fake_popen) + + result = env.execute("echo hi") + + assert result["returncode"] == 0 + assert "GITHUB_TOKEN=value_from_dotenv" in popen_calls[0] + + +def test_execute_prefers_shell_env_over_hermes_dotenv(monkeypatch): + env = _make_execute_only_env(["GITHUB_TOKEN"]) + popen_calls = [] + + def _fake_popen(cmd, **kwargs): + popen_calls.append(cmd) + return _FakePopen(cmd, **kwargs) + + monkeypatch.setenv("GITHUB_TOKEN", "value_from_shell") + monkeypatch.setattr(docker_env, "_load_hermes_env_vars", lambda: {"GITHUB_TOKEN": "value_from_dotenv"}) + monkeypatch.setattr(docker_env.subprocess, "Popen", _fake_popen) + + env.execute("echo hi") + + assert "GITHUB_TOKEN=value_from_shell" in popen_calls[0] + assert "GITHUB_TOKEN=value_from_dotenv" not in popen_calls[0] diff --git a/tests/tools/test_parse_env_var.py b/tests/tools/test_parse_env_var.py index 48c282bc..cffee7c9 100644 --- a/tests/tools/test_parse_env_var.py +++ b/tests/tools/test_parse_env_var.py @@ -30,6 +30,28 @@ class TestParseEnvVar: result = _parse_env_var("TERMINAL_DOCKER_VOLUMES", "[]", json.loads, "valid JSON") assert result == ["/host:/container"] + def test_get_env_config_parses_docker_forward_env_json(self): + with patch.dict("os.environ", { + "TERMINAL_ENV": "docker", + "TERMINAL_DOCKER_FORWARD_ENV": '["GITHUB_TOKEN", "NPM_TOKEN"]', + }, clear=False): + config = _tt_mod._get_env_config() + assert config["docker_forward_env"] == ["GITHUB_TOKEN", "NPM_TOKEN"] + + def test_create_environment_passes_docker_forward_env(self): + fake_env = object() + with patch.object(_tt_mod, "_DockerEnvironment", return_value=fake_env) as mock_docker: + result = _tt_mod._create_environment( + "docker", + image="python:3.11", + cwd="/root", + timeout=180, + container_config={"docker_forward_env": ["GITHUB_TOKEN"]}, + ) + + assert result is fake_env + assert mock_docker.call_args.kwargs["forward_env"] == ["GITHUB_TOKEN"] + def test_falls_back_to_default(self): with patch.dict("os.environ", {}, clear=False): # Remove the var if it exists, rely on default diff --git a/tools/environments/docker.py b/tools/environments/docker.py index ec6d8b30..90e59849 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -7,6 +7,7 @@ persistence via bind mounts. import logging import os +import re import shutil import subprocess import sys @@ -30,6 +31,42 @@ _DOCKER_SEARCH_PATHS = [ ] _docker_executable: Optional[str] = None # resolved once, cached +_ENV_VAR_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + + +def _normalize_forward_env_names(forward_env: list[str] | None) -> list[str]: + """Return a deduplicated list of valid environment variable names.""" + normalized: list[str] = [] + seen: set[str] = set() + + for item in forward_env or []: + if not isinstance(item, str): + logger.warning("Ignoring non-string docker_forward_env entry: %r", item) + continue + + key = item.strip() + if not key: + continue + if not _ENV_VAR_NAME_RE.match(key): + logger.warning("Ignoring invalid docker_forward_env entry: %r", item) + continue + if key in seen: + continue + + seen.add(key) + normalized.append(key) + + return normalized + + +def _load_hermes_env_vars() -> dict[str, str]: + """Load ~/.hermes/.env values without failing Docker command execution.""" + try: + from hermes_cli.config import load_env + + return load_env() or {} + except Exception: + return {} def find_docker() -> Optional[str]: @@ -171,6 +208,7 @@ class DockerEnvironment(BaseEnvironment): persistent_filesystem: bool = False, task_id: str = "default", volumes: list = None, + forward_env: list[str] | None = None, network: bool = True, host_cwd: str = None, auto_mount_cwd: bool = False, @@ -181,6 +219,7 @@ class DockerEnvironment(BaseEnvironment): self._base_image = image self._persistent = persistent_filesystem self._task_id = task_id + self._forward_env = _normalize_forward_env_names(forward_env) self._container_id: Optional[str] = None logger.info(f"DockerEnvironment volumes: {volumes}") # Ensure volumes is a list (config.yaml could be malformed) @@ -355,8 +394,12 @@ class DockerEnvironment(BaseEnvironment): if effective_stdin is not None: cmd.append("-i") cmd.extend(["-w", work_dir]) - for key in self._inner.config.forward_env: - if (value := os.getenv(key)) is not None: + hermes_env = _load_hermes_env_vars() if self._forward_env else {} + for key in self._forward_env: + value = os.getenv(key) + if value is None: + value = hermes_env.get(key) + if value is not None: cmd.extend(["-e", f"{key}={value}"]) for key, value in self._inner.config.env.items(): cmd.extend(["-e", f"{key}={value}"]) diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 49a82e24..3cc541b5 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -505,6 +505,7 @@ def _get_env_config() -> Dict[str, Any]: return { "env_type": env_type, "docker_image": os.getenv("TERMINAL_DOCKER_IMAGE", default_image), + "docker_forward_env": _parse_env_var("TERMINAL_DOCKER_FORWARD_ENV", "[]", json.loads, "valid JSON"), "singularity_image": os.getenv("TERMINAL_SINGULARITY_IMAGE", f"docker://{default_image}"), "modal_image": os.getenv("TERMINAL_MODAL_IMAGE", default_image), "daytona_image": os.getenv("TERMINAL_DAYTONA_IMAGE", default_image), @@ -562,6 +563,7 @@ def _create_environment(env_type: str, image: str, cwd: str, timeout: int, disk = cc.get("container_disk", 51200) persistent = cc.get("container_persistent", True) volumes = cc.get("docker_volumes", []) + docker_forward_env = cc.get("docker_forward_env", []) if env_type == "local": lc = local_config or {} @@ -576,6 +578,7 @@ def _create_environment(env_type: str, image: str, cwd: str, timeout: int, volumes=volumes, host_cwd=host_cwd, auto_mount_cwd=cc.get("docker_mount_cwd_to_workspace", False), + forward_env=docker_forward_env, ) elif env_type == "singularity": diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index d10d66c1..07742c3d 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -80,6 +80,7 @@ For native Anthropic auth, Hermes prefers Claude Code's own credential files whe |----------|-------------| | `TERMINAL_ENV` | Backend: `local`, `docker`, `ssh`, `singularity`, `modal`, `daytona` | | `TERMINAL_DOCKER_IMAGE` | Docker image (default: `python:3.11`) | +| `TERMINAL_DOCKER_FORWARD_ENV` | JSON array of env var names to explicitly forward into Docker terminal sessions | | `TERMINAL_DOCKER_VOLUMES` | Additional Docker volume mounts (comma-separated `host:container` pairs) | | `TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE` | Advanced opt-in: mount the launch cwd into Docker `/workspace` (`true`/`false`, default: `false`) | | `TERMINAL_SINGULARITY_IMAGE` | Singularity image or `.sif` path | diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index abaabbad..f9a2198f 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -488,6 +488,8 @@ terminal: # Docker-specific settings docker_image: "nikolaik/python-nodejs:python3.11-nodejs20" docker_mount_cwd_to_workspace: false # SECURITY: off by default. Opt in to mount the launch cwd into /workspace. + docker_forward_env: # Optional explicit allowlist for env passthrough + - "GITHUB_TOKEN" docker_volumes: # Additional explicit host mounts - "/home/user/projects:/workspace/projects" - "/home/user/data:/data:ro" # :ro for read-only @@ -555,6 +557,24 @@ This is useful for: Can also be set via environment variable: `TERMINAL_DOCKER_VOLUMES='["/host:/container"]'` (JSON array). +### Docker Credential Forwarding + +By default, Docker terminal sessions do not inherit arbitrary host credentials. If you need a specific token inside the container, add it to `terminal.docker_forward_env`. + +```yaml +terminal: + backend: docker + docker_forward_env: + - "GITHUB_TOKEN" + - "NPM_TOKEN" +``` + +Hermes resolves each listed variable from your current shell first, then falls back to `~/.hermes/.env` if it was saved with `hermes config set`. + +:::warning +Anything listed in `docker_forward_env` becomes visible to commands run inside the container. Only forward credentials you are comfortable exposing to the terminal session. +::: + ### Optional: Mount the Launch Directory into `/workspace` Docker sandboxes stay isolated by default. Hermes does **not** pass your current host working directory into the container unless you explicitly opt in. diff --git a/website/docs/user-guide/features/tools.md b/website/docs/user-guide/features/tools.md index faf1023e..981d2caf 100644 --- a/website/docs/user-guide/features/tools.md +++ b/website/docs/user-guide/features/tools.md @@ -135,6 +135,8 @@ All container backends run with security hardening: - Full namespace isolation - Persistent workspace via volumes, not writable root layer +Docker can optionally receive an explicit env allowlist via `terminal.docker_forward_env`, but forwarded variables are visible to commands inside the container and should be treated as exposed to that session. + ## Background Process Management Start background processes and manage them: diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index 9fcf527f..d31cc175 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -212,6 +212,7 @@ Container resources are configurable in `~/.hermes/config.yaml`: terminal: backend: docker docker_image: "nikolaik/python-nodejs:python3.11-nodejs20" + docker_forward_env: [] # Explicit allowlist only; empty keeps secrets out of the container container_cpu: 1 # CPU cores container_memory: 5120 # MB (default 5GB) container_disk: 51200 # MB (default 50GB, requires overlay2 on XFS) @@ -227,6 +228,10 @@ terminal: For production gateway deployments, use `docker`, `modal`, or `daytona` backend to isolate agent commands from your host system. This eliminates the need for dangerous command approval entirely. ::: +:::warning +If you add names to `terminal.docker_forward_env`, those variables are intentionally injected into the container for terminal commands. This is useful for task-specific credentials like `GITHUB_TOKEN`, but it also means code running in the container can read and exfiltrate them. +::: + ## Terminal Backend Security Comparison | Backend | Isolation | Dangerous Cmd Check | Best For |