diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 4fca9906d17..c292ab9a05e 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -74,6 +74,82 @@ def _warn_config_parse_failure(config_path: Path, exc: Exception) -> None: _IS_WINDOWS = platform.system() == "Windows" _ENV_VAR_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + +# Env var names that influence how the next subprocess executes — +# never writable through ``save_env_value``. Anything that controls +# the loader, interpreter, shell, or replacement editor counts: +# +# * ``LD_PRELOAD`` / ``LD_LIBRARY_PATH`` / ``LD_AUDIT`` — Linux dynamic +# loader. ``DYLD_*`` — macOS equivalent. Planting a path here means +# the next ``subprocess.run([...])`` Hermes makes loads attacker code +# before main(). +# * ``PYTHONPATH`` / ``PYTHONHOME`` / ``PYTHONSTARTUP`` / +# ``PYTHONUSERBASE`` — Python interpreter init. Hermes itself starts +# from one of these on every restart. +# * ``NODE_OPTIONS`` / ``NODE_PATH`` — Node interpreter; affects npm, +# ``hermes update``, the TUI build. +# * ``PATH`` — too broad to allow. The dashboard never needs to rewrite +# the operator's PATH; if a tool can't be found, the fix is to add an +# absolute path in the integration config, not to mutate PATH globally. +# * ``GIT_SSH_COMMAND`` / ``GIT_EXEC_PATH`` — git rewrites that fire +# on every plugin install / ``hermes update``. +# * ``BROWSER`` / ``EDITOR`` / ``VISUAL`` / ``PAGER`` — commands the +# shell or CLI invokes implicitly. Wrong values here = RCE on next +# ``$EDITOR``. +# * ``SHELL`` — what subprocess uses with ``shell=True`` (we try to +# avoid that, but defense in depth). +# * ``HERMES_HOME`` / ``HERMES_PROFILE`` / ``HERMES_CONFIG`` / +# ``HERMES_ENV`` — Hermes runtime location flags. Writing these into +# ``.env`` would relocate state in ways the user did not request from +# the dashboard. ``config.yaml`` is the supported surface for these. +# +# IMPORTANT: ``HERMES_*`` overall is NOT blocked. Many legitimate +# integration credentials follow that prefix (HERMES_GEMINI_CLIENT_ID, +# HERMES_LANGFUSE_PUBLIC_KEY, HERMES_SPOTIFY_CLIENT_ID, ...). The +# denylist is name-by-name on purpose so the gate stays narrow and +# doesn't accidentally break provider setup wizards. +# +# This is enforced on *write* only — values already in ``.env`` (set +# by the operator out-of-band, or pre-existing) keep working. The +# point is that the dashboard's writable surface cannot escalate by +# planting them. +_ENV_VAR_NAME_DENYLIST: frozenset[str] = frozenset({ + # Loader / linker + "LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT", "LD_DEBUG", + "DYLD_INSERT_LIBRARIES", "DYLD_LIBRARY_PATH", "DYLD_FRAMEWORK_PATH", + "DYLD_FALLBACK_LIBRARY_PATH", "DYLD_FALLBACK_FRAMEWORK_PATH", + # Python + "PYTHONPATH", "PYTHONHOME", "PYTHONSTARTUP", "PYTHONUSERBASE", + "PYTHONEXECUTABLE", "PYTHONNOUSERSITE", + # Node + "NODE_OPTIONS", "NODE_PATH", + # General + "PATH", "SHELL", "BROWSER", "EDITOR", "VISUAL", "PAGER", + # Git + "GIT_SSH_COMMAND", "GIT_EXEC_PATH", "GIT_SHELL", + # Hermes runtime location — never via dashboard env writer. + # NOT a HERMES_* blanket: integration credentials (HERMES_GEMINI_*, + # HERMES_LANGFUSE_*, HERMES_SPOTIFY_*, ...) ARE allowed. + "HERMES_HOME", "HERMES_PROFILE", "HERMES_CONFIG", "HERMES_ENV", +}) + + +def _reject_denylisted_env_var(key: str) -> None: + """Raise if ``key`` is in :data:`_ENV_VAR_NAME_DENYLIST`. + + Centralised so both the regular and "secure" env writers share the + same gate, and so the message is consistent for callers. + """ + if key in _ENV_VAR_NAME_DENYLIST: + raise ValueError( + f"Environment variable {key!r} is on the writer denylist. " + "Names that influence subprocess execution (LD_PRELOAD, " + "PYTHONPATH, PATH, EDITOR, ...) or Hermes runtime location " + "(HERMES_HOME, HERMES_PROFILE, ...) cannot be persisted via " + "the env writer. If you really need this, edit " + "~/.hermes/.env directly." + ) + _LAST_EXPANDED_CONFIG_BY_PATH: Dict[str, Any] = {} # (path, mtime_ns, size) -> cached expanded config dict. # load_config() returns a deepcopy of the cached value when the file @@ -4874,6 +4950,7 @@ def save_env_value(key: str, value: str): return if not _ENV_VAR_NAME_RE.match(key): raise ValueError(f"Invalid environment variable name: {key!r}") + _reject_denylisted_env_var(key) value = value.replace("\n", "").replace("\r", "") # API keys / tokens must be ASCII — strip non-ASCII with a warning. value = _check_non_ascii_credential(key, value) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index f7a73f17e48..d8d7996b868 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1223,6 +1223,12 @@ async def set_env_var(body: EnvVarUpdate): try: save_env_value(body.key, body.value) return {"ok": True, "key": body.key} + except ValueError as exc: + # save_env_value raises ValueError for invalid names and for keys + # on the denylist (LD_PRELOAD, PATH, PYTHONPATH, …). Surface the + # message to the SPA so the user understands why the write was + # refused instead of seeing an opaque 500. + raise HTTPException(status_code=400, detail=str(exc)) from exc except Exception: _log.exception("PUT /api/env failed") raise HTTPException(status_code=500, detail="Internal server error") @@ -4543,6 +4549,17 @@ async def serve_plugin_asset(plugin_name: str, file_path: str): Only serves files from the plugin's ``dashboard/`` subdirectory. Path traversal is blocked by checking ``resolve().is_relative_to()``. + + Restricted to a browser-fetchable suffix allowlist (JS/CSS/JSON/HTML/ + SVG/PNG/JPG/WOFF). The dashboard loads plugin JS via ``