Skip to content

Commit 36d3c7f

Browse files
yasinBursaliclaude
andcommitted
fix(host-agent): use new hooks schema + sandboxed env for install hooks
_handle_install's inner _run_install worker used the legacy _resolve_setup_hook() which only read the old service.setup_hook field, silently skipping any extension that defines its setup via the new hooks.post_install schema. It also invoked subprocess.run without an env= kwarg, inheriting the full host-agent environment (AGENT_API_KEY, DREAM_AGENT_KEY, DASHBOARD_API_KEY and other secrets) into extension setup scripts — a credential exfiltration vector. Switch to _resolve_hook(ext_dir, "post_install") to honour both schemas, and build the same minimal env allowlist that _execute_hook already uses. Delete the now-unreferenced _resolve_setup_hook function and add a source-level regression test that fails if the kwarg, allowlist, or resolver call is ever reverted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c0600ca commit 36d3c7f

File tree

2 files changed

+65
-36
lines changed

2 files changed

+65
-36
lines changed

dream-server/bin/dream-host-agent.py

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -185,40 +185,6 @@ def _precreate_data_dirs(service_id: str):
185185
logger.warning("Failed to pre-create %s: %s", dir_path, e)
186186

187187

188-
def _resolve_setup_hook(ext_dir: Path) -> Path | None:
189-
"""Read manifest to find setup_hook path. Returns None if no hook defined."""
190-
manifest_path = None
191-
for name in ("manifest.yaml", "manifest.yml"):
192-
candidate = ext_dir / name
193-
if candidate.exists():
194-
manifest_path = candidate
195-
break
196-
if manifest_path is None:
197-
return None
198-
try:
199-
import yaml
200-
manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8"))
201-
except (ImportError, OSError):
202-
return None
203-
if not isinstance(manifest, dict):
204-
return None
205-
service_def = manifest.get("service", {})
206-
if not isinstance(service_def, dict):
207-
return None
208-
setup_hook = service_def.get("setup_hook", "")
209-
if not isinstance(setup_hook, str) or not setup_hook:
210-
return None
211-
hook_path = (ext_dir / setup_hook).resolve()
212-
try:
213-
hook_path.relative_to(ext_dir.resolve())
214-
except ValueError:
215-
logger.warning("Path traversal attempt in setup_hook for %s: %s", ext_dir.name, setup_hook)
216-
return None
217-
if not hook_path.is_file():
218-
return None
219-
return hook_path
220-
221-
222188
def docker_compose_action(service_id: str, action: str) -> tuple:
223189
flags = resolve_compose_flags()
224190
if action == "start":
@@ -931,11 +897,27 @@ def _run_install():
931897
if run_setup_hook:
932898
_write_progress(service_id, "setup_hook", "Running setup...")
933899
ext_dir = USER_EXTENSIONS_DIR / service_id
934-
hook_path = _resolve_setup_hook(ext_dir)
900+
hook_path = _resolve_hook(ext_dir, "post_install")
935901
if hook_path:
902+
# Minimal allowlist env — mirror _execute_hook (L856-866)
903+
# to prevent leaking host-agent secrets to extension scripts.
904+
manifest = _read_manifest(ext_dir)
905+
service_def = manifest.get("service", {}) if manifest else {}
906+
if not isinstance(service_def, dict):
907+
service_def = {}
908+
hook_env = {
909+
"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
910+
"HOME": os.environ.get("HOME", ""),
911+
"SERVICE_ID": service_id,
912+
"SERVICE_PORT": str(service_def.get("port", 0)),
913+
"SERVICE_DATA_DIR": str(DATA_DIR / service_id),
914+
"DREAM_VERSION": DREAM_VERSION,
915+
"GPU_BACKEND": GPU_BACKEND,
916+
"HOOK_NAME": "post_install",
917+
}
936918
result = subprocess.run(
937919
["bash", str(hook_path), str(INSTALL_DIR), GPU_BACKEND],
938-
cwd=str(ext_dir),
920+
cwd=str(ext_dir), env=hook_env,
939921
capture_output=True, text=True,
940922
timeout=SUBPROCESS_TIMEOUT_START,
941923
)

dream-server/extensions/services/dashboard-api/tests/test_host_agent.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,50 @@ def test_missing_cache_file_is_noop(self, tmp_path, monkeypatch):
180180
monkeypatch.setattr(_mod, "INSTALL_DIR", install_dir)
181181

182182
invalidate_compose_cache() # must not raise
183+
184+
185+
# --- Install setup-hook env allowlist (regression) ---
186+
#
187+
# Locks in the fix that strips host-agent secrets from the env passed to
188+
# extension setup hooks during _handle_install. A source-level check is used
189+
# because the subprocess.run call lives inside a nested closure started on a
190+
# daemon thread, which makes dynamic mocking fragile.
191+
192+
193+
class TestInstallHookEnvAllowlist:
194+
195+
def _install_source(self):
196+
import inspect
197+
return inspect.getsource(_mod.AgentHandler._handle_install)
198+
199+
def test_setup_hook_subprocess_run_passes_env_kwarg(self):
200+
src = self._install_source()
201+
assert "env=hook_env" in src, (
202+
"setup_hook subprocess.run must pass env=hook_env "
203+
"(regression: do not fall back to inheriting os.environ)"
204+
)
205+
206+
def test_setup_hook_env_excludes_host_agent_secrets(self):
207+
src = self._install_source()
208+
for secret in ("AGENT_API_KEY", "DREAM_AGENT_KEY", "DASHBOARD_API_KEY"):
209+
assert secret not in src, (
210+
f"_handle_install must not reference {secret}; "
211+
"extension setup hooks must not receive host-agent secrets"
212+
)
213+
214+
def test_setup_hook_env_contains_allowlist_keys(self):
215+
src = self._install_source()
216+
for key in (
217+
"PATH", "HOME", "SERVICE_ID", "SERVICE_PORT",
218+
"SERVICE_DATA_DIR", "DREAM_VERSION", "GPU_BACKEND", "HOOK_NAME",
219+
):
220+
assert f'"{key}"' in src, (
221+
f"setup_hook env allowlist missing required key {key}"
222+
)
223+
224+
def test_setup_hook_uses_resolve_hook_with_post_install(self):
225+
src = self._install_source()
226+
assert '_resolve_hook(ext_dir, "post_install")' in src, (
227+
"setup_hook must use _resolve_hook(..., 'post_install'); "
228+
"the legacy _resolve_setup_hook has been removed"
229+
)

0 commit comments

Comments
 (0)