fix(docker): stage2 API_SERVER_KEY bootstrap no longer depends on .env existing (OOF-285) (#88926)

* fix(docker): stage2 API_SERVER_KEY bootstrap no longer depends on .env existing (OOF-285)

Fleet sweep found 144/351 started hosted instances (41%) on v2026.8.13+
with no API_SERVER_KEY: the loopback gateway api_server (which serves
/api/cron/fire on :8642) never started, so every scheduled cron fire was
silently lost until the NAS retry budget exhausted.

Root cause chain:
- .dockerignore excludes .env.example (image-size optimization), so
  /opt/hermes/.env.example does not exist in shipped images
- stage2's first-boot seed `seed_one ".env" ".env.example"` is a silent
  no-op when the source is missing -> fresh volumes never get a .env
- the API_SERVER_KEY generation added in #84339 was gated on
  `[ -f "$HERMES_HOME/.env" ]` -> never ran on those instances

Fixes:
- stage2-hook.sh: keygen now creates an owner-only .env when missing
  instead of requiring it to exist; still append-only w.r.t. operator
  keys, still refuses symlinked paths
- .dockerignore: re-include .env.example (negation after the .env.*
  exclusion) so the first-boot template seed works again
- tests: new tests/tools/test_stage2_hook_api_server_keygen.py covers
  create-when-missing, append-without-clobber, operator-key preservation,
  symlink refusal, and a .dockerignore contract test for .env.example

* fix(docker): container-provided API_SERVER_KEY wins over stage2 keygen (review)

The bootstrap generated a key whenever .env lacked one, without checking
the inherited container environment. That broke the documented
`docker run -e API_SERVER_KEY=...` flow: Hermes loads $HERMES_HOME/.env
with override=True (hermes_cli/env_loader.py), so the generated key
silently shadowed the operator's env key and 401'd existing clients.

- stage2-hook.sh: skip generation when API_SERVER_KEY is present in the
  container environment; if BOTH the env and .env carry keys, warn that
  the .env value wins at runtime and touch nothing
- tests: regression tests for the env-provided path (skip + no .env
  write; env+file conflict warns without clobbering); sandbox runner now
  pins/unsets API_SERVER_KEY explicitly so results don't depend on the
  host environment

* fix(docker): drop stale empty API_SERVER_KEY= line when container env provides the key

A leftover empty 'API_SERVER_KEY=' assignment in .env clobbers a
container-provided key at runtime (.env loads with override=True and
python-dotenv sets the empty string), so the api_server startup guard
fails and every scheduled cron fire is silently lost — the exact
symptom class this PR fixes, reintroduced in the env-key branch.

Remove the stale empty line (behind the existing symlink guard) before
skipping generation, so the operator's env key actually wins. Addresses
the IMPORTANT finding both reviewers converged on.

Test: env-key + stale-empty-line combination now covered; strict
removal assertion gated on GNU sed (BSD sed on macOS dev hosts skips
the -i invocation, same caveat as the append test).

* fix(docker): warn at boot when a container-provided API_SERVER_KEY is too weak to start the api_server

The startup guard refuses keys under 16 chars. Now that a
container-provided key suppresses stage2 generation, a weak
`docker run -e API_SERVER_KEY=...` value means the api_server stays
down (cron fires unavailable) instead of clients getting 401s against
a generated key. Say so in the boot log, where the operator will look.

* fix(docker): create .env under umask 077 instead of touch+chmod

touch created the file with the inherited umask (typically 0644), then
a silenced chmod tightened it to 0600 — a brief group/world-readable
window, and no warning if the chmod failed. Creating under umask 077
makes the file owner-only from the first instant with no dependence on
a second command succeeding. Covered by the existing 0600 mode
assertion in test_keygen_creates_env_when_missing.

* fix(docker): guard the API_SERVER_KEY append so a read-only .env degrades to a warning, not a failed boot

stage2 runs under set -eu; the unguarded printf append meant a keyless
.env on a read-only volume (or full disk) aborted the whole cont-init
phase and the container boot. Guard it and emit the same loud warning
the create-failure path uses.

Test harness now runs the extracted block under set -eu to match
production (it ran set -u only, so it could not see this defect class);
new read-only regression test verified RED against the unguarded
append via mutation.

* fix(docker): only warn about a weak container API_SERVER_KEY when it is actually the effective key

The <16-chars warning fired before the .env inspection, so a weak
container key alongside a strong .env key produced a false boot-log
claim that the api_server 'will refuse to start' — immediately followed
by the both-keys warning saying the .env value wins, and the server in
fact starts. Move the check into the branch where the env key really is
the effective key on this boot (round-2 review finding, verified by
execution against python-dotenv last-wins semantics).

---------

Co-authored-by: Ben Barclay <ben@nousresearch.com>
This commit is contained in:
shannonsands
2026-08-21 14:56:28 +10:00
committed by GitHub
parent 1acbeed146
commit 7a17a1b8a6
3 changed files with 359 additions and 10 deletions

View File

@@ -40,6 +40,10 @@ ui-tui/packages/hermes-ink/dist/
# Environment files
.env
.env.*
# ...but keep the template: docker/stage2-hook.sh seeds $HERMES_HOME/.env from
# /opt/hermes/.env.example on first boot (OOF-285 — excluding it silently broke
# first-boot .env seeding and the API_SERVER_KEY generation that depends on it).
!.env.example
# IDE
.vscode/
@@ -98,7 +102,6 @@ plans/
.plans/
# Repo-level dotfiles that are git-only or dev-tooling config
.env.example
.envrc
.gitattributes
.hadolint.yaml

View File

@@ -442,19 +442,75 @@ seed_one "SOUL.md" "docker/SOUL.md"
# volume), never overwrite an operator-provided value. Loopback-only: the
# default bind host is 127.0.0.1 and the Fly service only exposes the
# dashboard's port, so this listener is never publicly reachable.
if [ -f "$HERMES_HOME/.env" ] && ! grep -q '^API_SERVER_KEY=..*' "$HERMES_HOME/.env" 2>/dev/null; then
#
# CREATE .env when it is missing rather than requiring it to exist (OOF-285):
# the first-boot seed above depends on /opt/hermes/.env.example being present
# in the image, and when it isn't (the .dockerignore excluded it for a long
# stretch of releases) seed_one is a silent no-op, no .env ever exists, this
# keygen never ran, the api_server never started, and every scheduled cron
# fire on the instance was silently lost. The key must not depend on the
# example-file seed having worked.
#
# OPERATOR-PROVIDED KEYS WIN: if the container environment already carries
# API_SERVER_KEY (documented `docker run -e API_SERVER_KEY=...` flow), do
# not generate one. Hermes loads $HERMES_HOME/.env with override=True, so
# a generated key written here would silently SHADOW the operator's env
# key and 401 every client still using the supplied credential.
if [ -n "${API_SERVER_KEY:-}" ]; then
if [ -f "$HERMES_HOME/.env" ] && grep -q '^API_SERVER_KEY=..*' "$HERMES_HOME/.env" 2>/dev/null; then
echo "[stage2] Warning: API_SERVER_KEY is set in both the container environment and $HERMES_HOME/.env — the .env value wins at runtime (loaded with override=True)"
else
# The env key is the effective key on this boot (no .env key wins
# over it). The api_server startup guard refuses keys shorter than
# 16 chars; since this branch skips generation, a weak operator key
# means the server stays DOWN (cron fires lost), not just 401s.
# Warn where the operator will look — the boot log. Checked only in
# this branch: when a strong .env key wins at runtime, the warning
# would be false (the server does start).
if [ "${#API_SERVER_KEY}" -lt 16 ]; then
echo "[stage2] Warning: container-provided API_SERVER_KEY is shorter than 16 characters — the gateway api_server will refuse to start (cron fires unavailable). Generate a strong secret, e.g. \`openssl rand -hex 32\`."
fi
# A stale empty `API_SERVER_KEY=` line (left by an old seed) would
# clobber the container-provided key at runtime: .env is loaded with
# override=True and python-dotenv sets the empty string, which fails
# the api_server's startup guard — the exact silent-cron-loss symptom
# this hook exists to prevent. Drop it so the operator key wins.
if [ -f "$HERMES_HOME/.env" ] && ! refuse_symlinked_path "clean" "$HERMES_HOME/.env"; then
sed -i '/^API_SERVER_KEY=$/d' "$HERMES_HOME/.env" 2>/dev/null || true
fi
echo "[stage2] API_SERVER_KEY provided via container environment — skipping generation"
fi
elif ! grep -q '^API_SERVER_KEY=..*' "$HERMES_HOME/.env" 2>/dev/null; then
if refuse_symlinked_path "append" "$HERMES_HOME/.env"; then
:
else
_gen_key=$(head -c 32 /dev/urandom | od -An -tx1 | tr -d ' \n')
if [ -n "$_gen_key" ]; then
# Drop an empty assignment line if the seed left one behind, then
# append the generated key.
sed -i '/^API_SERVER_KEY=$/d' "$HERMES_HOME/.env" 2>/dev/null || true
printf 'API_SERVER_KEY=%s\n' "$_gen_key" >> "$HERMES_HOME/.env"
echo "[stage2] Generated API_SERVER_KEY for the loopback gateway api_server"
if [ ! -f "$HERMES_HOME/.env" ]; then
# Create an empty, owner-only .env so the append below (and any
# later runtime save_env_value writes) have a durable target.
# Created under a restrictive umask so the file is 0600 from the
# first instant — no touch→chmod window, and no dependence on a
# silenced chmod succeeding. The chown/chmod block below still
# re-tightens perms every boot.
(umask 077 && as_hermes touch "$HERMES_HOME/.env") 2>/dev/null || true
fi
if [ -f "$HERMES_HOME/.env" ]; then
_gen_key=$(head -c 32 /dev/urandom | od -An -tx1 | tr -d ' \n')
if [ -n "$_gen_key" ]; then
# Drop an empty assignment line if the seed left one behind,
# then append the generated key. The append is guarded: on a
# read-only volume / full disk it must degrade to the warning
# below, not abort the whole stage2 hook under `set -e`.
sed -i '/^API_SERVER_KEY=$/d' "$HERMES_HOME/.env" 2>/dev/null || true
if printf 'API_SERVER_KEY=%s\n' "$_gen_key" >> "$HERMES_HOME/.env" 2>/dev/null; then
echo "[stage2] Generated API_SERVER_KEY for the loopback gateway api_server"
else
echo "[stage2] Warning: could not write API_SERVER_KEY to $HERMES_HOME/.env (read-only volume?) — gateway api_server (cron fires) will be unavailable"
fi
fi
unset _gen_key
else
echo "[stage2] Warning: could not create $HERMES_HOME/.env — gateway api_server (cron fires) will be unavailable"
fi
unset _gen_key
fi
fi

View File

@@ -0,0 +1,290 @@
"""Regression tests for the stage2 API_SERVER_KEY bootstrap (OOF-285).
The gateway's loopback api_server refuses to start without a strong
API_SERVER_KEY, and hosted cron fires are forwarded through it — so the
stage2 keygen must succeed even when no ``.env`` exists yet. Historically it
was gated on ``[ -f "$HERMES_HOME/.env" ]`` while the first-boot seed that
was supposed to create ``.env`` silently no-oped (``.env.example`` was
excluded from the image by ``.dockerignore``), leaving 40%+ of the hosted
fleet with no api_server and every scheduled cron fire silently lost.
"""
from __future__ import annotations
import re
import shutil
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parents[2]
STAGE2_HOOK = REPO_ROOT / "docker" / "stage2-hook.sh"
DOCKERIGNORE = REPO_ROOT / ".dockerignore"
KEY_LINE_RE = re.compile(r"^API_SERVER_KEY=[0-9a-f]{64}$", re.MULTILINE)
@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 _keygen_block(text: str) -> str:
start = text.index("# --- Ensure a gateway api_server key exists")
end = text.index("# .env holds API keys and secrets", start)
return text[start:end]
def _path_guard_functions(text: str) -> str:
start = text.index("path_has_symlink_component() {")
end = text.index("\n\nchown_hermes_tree() {", start)
return text[start:end]
def _run_keygen(
stage2_text: str,
home: Path,
env_key: str | None = None,
) -> subprocess.CompletedProcess[str]:
if shutil.which("sh") is None:
pytest.skip("sh not available")
if env_key is None:
env_setup = "unset API_SERVER_KEY\n"
else:
env_setup = f'API_SERVER_KEY="{env_key}"\n'
script = (
# Production runs the hook under `set -eu` (docker/stage2-hook.sh
# shebang block) — the sandbox must match, or the tests cannot see
# unguarded-command defects that would abort a real container boot.
"set -eu\n"
f"{env_setup}"
f'HERMES_HOME="{home}"\n'
# In tests we run unprivileged; as_hermes is a passthrough then.
'as_hermes() { "$@"; }\n'
f"{_path_guard_functions(stage2_text)}\n"
f"{_keygen_block(stage2_text)}\n"
)
return subprocess.run(
["sh", "-c", script],
capture_output=True,
text=True,
timeout=30,
)
def test_keygen_creates_env_when_missing(stage2_text: str, tmp_path: Path) -> None:
"""No .env at all (failed/absent first-boot seed) must still yield a key."""
home = tmp_path / "home"
home.mkdir()
result = _run_keygen(stage2_text, home)
assert result.returncode == 0, result.stderr
env_path = home / ".env"
assert env_path.is_file(), "keygen must create .env when it is missing"
assert KEY_LINE_RE.search(env_path.read_text()), "generated key missing/malformed"
mode = env_path.stat().st_mode & 0o777
assert mode == 0o600, f".env must be owner-only, got {oct(mode)}"
def test_keygen_appends_to_existing_env_without_key(
stage2_text: str, tmp_path: Path
) -> None:
home = tmp_path / "home"
home.mkdir()
(home / ".env").write_text("OTHER=1\nAPI_SERVER_KEY=\n")
result = _run_keygen(stage2_text, home)
assert result.returncode == 0, result.stderr
content = (home / ".env").read_text()
assert "OTHER=1" in content
assert KEY_LINE_RE.search(content)
# Exactly one real key assignment. (The stale empty `API_SERVER_KEY=` line
# is dropped by GNU sed in the production container; BSD sed on macOS dev
# hosts silently skips the -i invocation, so don't assert its removal.)
assert len(re.findall(r"^API_SERVER_KEY=..+$", content, re.MULTILINE)) == 1
def test_keygen_never_overwrites_operator_key(
stage2_text: str, tmp_path: Path
) -> None:
home = tmp_path / "home"
home.mkdir()
(home / ".env").write_text("API_SERVER_KEY=operator-provided-key-123\n")
result = _run_keygen(stage2_text, home)
assert result.returncode == 0, result.stderr
content = (home / ".env").read_text()
assert content == "API_SERVER_KEY=operator-provided-key-123\n"
def test_keygen_refuses_symlinked_env(stage2_text: str, tmp_path: Path) -> None:
home = tmp_path / "home"
home.mkdir()
outside = tmp_path / "outside.env"
outside.write_text("HIJACK=1\n")
(home / ".env").symlink_to(outside)
result = _run_keygen(stage2_text, home)
assert result.returncode == 0, result.stderr
assert "refusing append" in (result.stdout + result.stderr)
assert outside.read_text() == "HIJACK=1\n", "must not write through symlink"
def test_keygen_skips_when_container_env_provides_key(
stage2_text: str, tmp_path: Path
) -> None:
"""`docker run -e API_SERVER_KEY=...` must win: no generated key.
Hermes loads $HERMES_HOME/.env with override=True, so a key generated
into .env would silently shadow the operator's env-provided credential
and 401 every client still using it.
"""
home = tmp_path / "home"
home.mkdir()
result = _run_keygen(stage2_text, home, env_key="operator-env-key-0123456789")
assert result.returncode == 0, result.stderr
assert "skipping generation" in (result.stdout + result.stderr)
env_path = home / ".env"
if env_path.exists():
assert "API_SERVER_KEY=" not in env_path.read_text()
def test_keygen_env_key_with_existing_env_file_key_warns_not_clobbers(
stage2_text: str, tmp_path: Path
) -> None:
"""Both container env AND .env carry keys: warn, touch nothing."""
home = tmp_path / "home"
home.mkdir()
(home / ".env").write_text("API_SERVER_KEY=file-key-abcdef0123456789\n")
result = _run_keygen(stage2_text, home, env_key="env-key-9876543210fedcba")
assert result.returncode == 0, result.stderr
assert "the .env value wins" in (result.stdout + result.stderr)
content = (home / ".env").read_text()
assert content == "API_SERVER_KEY=file-key-abcdef0123456789\n"
def test_keygen_env_key_drops_stale_empty_assignment(
stage2_text: str, tmp_path: Path
) -> None:
"""Container env key + stale empty `API_SERVER_KEY=` line in .env.
The empty assignment must be removed: .env is loaded with override=True,
so python-dotenv would set API_SERVER_KEY to the empty string, clobbering
the operator's container-provided key and failing the api_server startup
guard — silently losing every cron fire.
"""
home = tmp_path / "home"
home.mkdir()
(home / ".env").write_text("OTHER=1\nAPI_SERVER_KEY=\n")
result = _run_keygen(stage2_text, home, env_key="operator-env-key-0123456789")
assert result.returncode == 0, result.stderr
assert "skipping generation" in (result.stdout + result.stderr)
content = (home / ".env").read_text()
assert "OTHER=1" in content
# No generated (non-empty) key may appear — the operator's env key wins.
assert not re.search(r"^API_SERVER_KEY=..+$", content, re.MULTILINE)
# The stale empty line must be gone so override=True cannot clobber the
# container key. (GNU sed only: BSD sed on macOS dev hosts silently skips
# the -i invocation, same caveat as the append test above.)
if _sed_is_gnu():
assert "API_SERVER_KEY=" not in content
def _sed_is_gnu() -> bool:
try:
probe = subprocess.run(
["sed", "--version"], capture_output=True, text=True, timeout=10
)
except (OSError, subprocess.TimeoutExpired):
return False
return probe.returncode == 0 and "GNU sed" in probe.stdout
def test_keygen_readonly_env_degrades_to_warning_not_boot_abort(
stage2_text: str, tmp_path: Path
) -> None:
"""A keyless .env on a read-only volume must not abort stage2.
The hook runs under `set -eu`; an unguarded failing append would kill the
whole cont-init phase and the container boot. It must instead warn that
the api_server will be unavailable and exit 0.
"""
import os
if os.geteuid() == 0:
pytest.skip("running as root — file write perms are not enforced")
home = tmp_path / "home"
home.mkdir()
env_path = home / ".env"
env_path.write_text("OTHER=1\n")
env_path.chmod(0o444)
try:
result = _run_keygen(stage2_text, home)
assert result.returncode == 0, (
f"stage2 keygen must not abort the boot on a read-only .env:\n"
f"{result.stderr}"
)
assert "could not write API_SERVER_KEY" in (result.stdout + result.stderr)
assert env_path.read_text() == "OTHER=1\n"
finally:
env_path.chmod(0o644)
def test_keygen_warns_on_weak_container_env_key(
stage2_text: str, tmp_path: Path
) -> None:
"""A container key shorter than 16 chars now means api_server DOWN, not
401s — the hook must say so in the boot log where the operator looks."""
home = tmp_path / "home"
home.mkdir()
result = _run_keygen(stage2_text, home, env_key="short-key")
assert result.returncode == 0, result.stderr
out = result.stdout + result.stderr
assert "shorter than 16 characters" in out
assert "skipping generation" in out
def test_keygen_weak_env_key_warning_suppressed_when_env_file_key_wins(
stage2_text: str, tmp_path: Path
) -> None:
"""Weak container key + strong .env key: the .env key wins at runtime
(override=True), the server DOES start — the 'will refuse to start'
warning would be false and must not fire. The both-keys warning must."""
home = tmp_path / "home"
home.mkdir()
(home / ".env").write_text("API_SERVER_KEY=strong-file-key-abcdef0123456789\n")
result = _run_keygen(stage2_text, home, env_key="short-key")
assert result.returncode == 0, result.stderr
out = result.stdout + result.stderr
assert "shorter than 16 characters" not in out
assert "the .env value wins" in out
def test_dockerignore_keeps_env_example_template() -> None:
"""The first-boot seed copies /opt/hermes/.env.example -> $HERMES_HOME/.env.
``.env.*`` in .dockerignore matches the template, so an explicit
``!.env.example`` re-include must appear AFTER it (last match wins), and
no later rule may exclude it again (OOF-285).
"""
if not DOCKERIGNORE.exists():
pytest.skip(".dockerignore not present in this checkout")
lines = [ln.strip() for ln in DOCKERIGNORE.read_text().splitlines()]
rules = [ln for ln in lines if ln and not ln.startswith("#")]
verdict = "excluded" # default: not matched -> included
for rule in rules:
negate = rule.startswith("!")
pattern = rule.lstrip("!")
if pattern in (".env.example",) or _dockerignore_match(pattern):
verdict = "included" if negate else "excluded"
assert verdict == "included", (
".env.example must survive .dockerignore — docker/stage2-hook.sh "
"seeds $HERMES_HOME/.env from it on first boot"
)
def _dockerignore_match(pattern: str) -> bool:
"""Minimal dockerignore glob match of ``pattern`` against '.env.example'."""
import fnmatch
if pattern.endswith("/"):
return False
return fnmatch.fnmatch(".env.example", pattern)