From 2484409b7adfa8837eab847e8378478617297b67 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 16 Apr 2026 18:09:16 -0700 Subject: [PATCH] fix: HERMES_WEBUI_DEFAULT_WORKSPACE wins over settings.json; trust DEFAULT_WORKSPACE subtree (#610) Squash-merges PR #610. Fixes Docker workspace env var override and trust validation (issue #609). 1367 tests passing, QA harness green. Reviewed by independent agent (see PR comments). --- CHANGELOG.md | 6 ++ api/config.py | 11 ++- api/workspace.py | 17 +++- static/index.html | 2 +- tests/test_default_workspace_fallback.py | 45 ++++++++++ tests/test_issue609.py | 107 +++++++++++++++++++++++ 6 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 tests/test_issue609.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e94b5e..461f8e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Hermes Web UI -- Changelog +## [v0.50.71] — 2026-04-16 + +### Fixed +- **Docker: `HERMES_WEBUI_DEFAULT_WORKSPACE` was silently overridden by `settings.json`** — the startup block in `api/config.py` unconditionally restored the persisted `default_workspace`, so any container that had previously written `settings.json` would shadow the env var on the next start. The env var now wins when explicitly set, matching the documented priority order. (Closes #609, PR #610) +- **Docker: workspace trust validation rejected subdirectories of `DEFAULT_WORKSPACE`** — `resolve_trusted_workspace()` only trusted paths under `Path.home()` or in the saved list; subpaths of a Docker volume mount like `/data/workspace/myproject` failed with "outside the user home directory". Added a third trust condition for paths under the boot-time `DEFAULT_WORKSPACE`, which was already validated at startup. (Closes #609, PR #610) + ## [v0.50.70] — 2026-04-16 ### Changed diff --git a/api/config.py b/api/config.py index c4668da..c82fcf7 100644 --- a/api/config.py +++ b/api/config.py @@ -1311,13 +1311,18 @@ def save_settings(settings: dict) -> dict: # Apply saved settings on startup (override env-derived defaults) +# Exception: if HERMES_WEBUI_DEFAULT_WORKSPACE is explicitly set in the +# environment, it wins over whatever settings.json has stored. Persisted +# config must never shadow an explicit env-var override (Docker deployments +# rely on this — otherwise deleting settings.json is the only escape). _startup_settings = load_settings() if SETTINGS_FILE.exists(): if _startup_settings.get("default_model"): DEFAULT_MODEL = _startup_settings["default_model"] - DEFAULT_WORKSPACE = resolve_default_workspace( - _startup_settings.get("default_workspace") - ) + if not os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE"): + DEFAULT_WORKSPACE = resolve_default_workspace( + _startup_settings.get("default_workspace") + ) if _startup_settings.get("default_workspace") != str(DEFAULT_WORKSPACE): _startup_settings["default_workspace"] = str(DEFAULT_WORKSPACE) try: diff --git a/api/workspace.py b/api/workspace.py index 070b0c8..dddae29 100644 --- a/api/workspace.py +++ b/api/workspace.py @@ -283,9 +283,22 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path: except Exception: pass + # (C) Trusted if it is equal to or under the boot-time DEFAULT_WORKSPACE. + # In Docker deployments HERMES_WEBUI_DEFAULT_WORKSPACE is often set to a + # volume mount outside the user's home (e.g. /data/workspace). That path + # was already validated at server startup, so any sub-path of it is safe + # without requiring the user to add it to the workspace list manually. + try: + boot_default = Path(_BOOT_DEFAULT_WORKSPACE).expanduser().resolve() + candidate.relative_to(boot_default) + return candidate + except ValueError: + pass + raise ValueError( - f"Path is outside the user home directory and not in the saved workspace " - f"list: {candidate}. Add it via Settings → Workspaces first." + f"Path is outside the user home directory, not in the saved workspace " + f"list, and not under the default workspace: {candidate}. " + f"Add it via Settings → Workspaces first." ) diff --git a/static/index.html b/static/index.html index e87bc6c..9916ce0 100644 --- a/static/index.html +++ b/static/index.html @@ -558,7 +558,7 @@
System
Instance version and access controls.
- v0.50.70 + v0.50.71
diff --git a/tests/test_default_workspace_fallback.py b/tests/test_default_workspace_fallback.py index 2bb3329..0934be0 100644 --- a/tests/test_default_workspace_fallback.py +++ b/tests/test_default_workspace_fallback.py @@ -101,3 +101,48 @@ def test_ensure_workspace_dir_returns_false_for_unwritable_path(monkeypatch, tmp assert result is False finally: parent.chmod(stat.S_IRWXU) + + +def test_env_var_wins_over_settings_json_on_startup(monkeypatch, tmp_path): + """HERMES_WEBUI_DEFAULT_WORKSPACE must not be overridden by settings.json at startup. + + Regression for GitHub issue #609: Docker deployments set the env var to a + volume mount, but settings.json from a previous container run used to + silently win, reverting the files panel to the old path. + """ + import json as _json + import os as _os + + env_ws = tmp_path / "env_workspace" + env_ws.mkdir() + settings_ws = tmp_path / "settings_workspace" + settings_ws.mkdir() + state_dir = tmp_path / "state" + state_dir.mkdir() + settings_file = state_dir / "settings.json" + settings_file.write_text( + _json.dumps({"default_workspace": str(settings_ws)}), encoding="utf-8" + ) + + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + monkeypatch.setattr(config, "SETTINGS_FILE", settings_file) + # Simulate DEFAULT_WORKSPACE already set correctly from env var at import time + monkeypatch.setattr(config, "DEFAULT_WORKSPACE", env_ws.resolve()) + monkeypatch.setenv("HERMES_WEBUI_DEFAULT_WORKSPACE", str(env_ws)) + + # Execute the patched startup block logic inline — env var present → skip override + current_ws = config.DEFAULT_WORKSPACE + startup_settings = config.load_settings() + if not _os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE"): + # This branch must be skipped because env var is set + current_ws = config.resolve_default_workspace( + startup_settings.get("default_workspace") + ) + + # env var was set → the if block was skipped → env path wins over settings.json + assert current_ws == env_ws.resolve(), ( + f"Expected {env_ws.resolve()}, got {current_ws}. " + "settings.json must not override HERMES_WEBUI_DEFAULT_WORKSPACE." + ) + diff --git a/tests/test_issue609.py b/tests/test_issue609.py new file mode 100644 index 0000000..5716e3f --- /dev/null +++ b/tests/test_issue609.py @@ -0,0 +1,107 @@ +""" +Tests for GitHub issue #609 — Docker workspace path trust and env-var priority. + +Two independent bugs were fixed: + + 1. HERMES_WEBUI_DEFAULT_WORKSPACE env var was silently overridden by + settings.json at server startup. The env var must always win. + + 2. resolve_trusted_workspace() rejected paths that are children of + DEFAULT_WORKSPACE (e.g. /data/workspace/project) when the default is a + Docker volume mount outside the user's home directory. Any path under + the boot-time default should be trusted automatically. +""" +from pathlib import Path + +import pytest + +from api.workspace import resolve_trusted_workspace + + +# ── Fix 2: trust paths under DEFAULT_WORKSPACE ─────────────────────────────── + +def test_subdir_of_boot_default_is_trusted(monkeypatch, tmp_path): + """A subdirectory of BOOT_DEFAULT_WORKSPACE must be trusted without being in + the saved workspace list and without being under the user's home directory. + + This is the core Docker case: DEFAULT_WORKSPACE=/data/workspace, and the + user tries to open /data/workspace/myproject — should NOT raise ValueError. + """ + import api.workspace as ws_mod + + boot_default = tmp_path / "data" / "workspace" + boot_default.mkdir(parents=True) + sub = boot_default / "myproject" + sub.mkdir() + + monkeypatch.setattr(ws_mod, "_BOOT_DEFAULT_WORKSPACE", str(boot_default)) + + # Should not raise — sub is under the boot default + result = resolve_trusted_workspace(str(sub)) + assert result == sub.resolve() + + +def test_boot_default_itself_is_trusted(monkeypatch, tmp_path): + """The DEFAULT_WORKSPACE path itself must also be trusted (not only subdirs).""" + import api.workspace as ws_mod + + boot_default = tmp_path / "data" / "workspace" + boot_default.mkdir(parents=True) + + monkeypatch.setattr(ws_mod, "_BOOT_DEFAULT_WORKSPACE", str(boot_default)) + + result = resolve_trusted_workspace(str(boot_default)) + assert result == boot_default.resolve() + + +def test_path_outside_boot_default_and_home_is_rejected(monkeypatch, tmp_path): + """A path that is not under home, not in the saved list, and not under + DEFAULT_WORKSPACE must still be rejected.""" + import api.workspace as ws_mod + + boot_default = tmp_path / "data" / "workspace" + boot_default.mkdir(parents=True) + outside = tmp_path / "other_mount" / "secret" + outside.mkdir(parents=True) + + monkeypatch.setattr(ws_mod, "_BOOT_DEFAULT_WORKSPACE", str(boot_default)) + + with pytest.raises(ValueError, match="outside the user home"): + resolve_trusted_workspace(str(outside)) + + +def test_none_path_returns_boot_default(monkeypatch, tmp_path): + """resolve_trusted_workspace(None) always returns the boot default unchanged.""" + import api.workspace as ws_mod + + boot_default = tmp_path / "data" / "workspace" + boot_default.mkdir(parents=True) + + monkeypatch.setattr(ws_mod, "_BOOT_DEFAULT_WORKSPACE", str(boot_default)) + + result = resolve_trusted_workspace(None) + assert result == boot_default.resolve() + + +def test_path_traversal_via_dotdot_does_not_escape_boot_default(monkeypatch, tmp_path): + """A path that uses `..` to escape DEFAULT_WORKSPACE must not be trusted by (C). + + `Path.resolve()` collapses `..` before the `relative_to(boot_default)` check + runs, so `/data/workspace/../etc` resolves to `/etc` and is rejected (it's + also caught earlier by the system-roots block, but this test pins the + behavior in case the order of conditions ever changes). + """ + import api.workspace as ws_mod + + boot_default = tmp_path / "data" / "workspace" + boot_default.mkdir(parents=True) + sibling = tmp_path / "data" / "private" + sibling.mkdir(parents=True) + + monkeypatch.setattr(ws_mod, "_BOOT_DEFAULT_WORKSPACE", str(boot_default)) + + # `boot_default/../private` resolves to `tmp_path/data/private`, which is + # NOT a child of boot_default and not under home — must reject. + escape = boot_default / ".." / "private" + with pytest.raises(ValueError, match="outside the user home"): + resolve_trusted_workspace(str(escape))