mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-20 14:40:35 +00:00
fix(profiles): use profile-scoped resolver for /api/profile/active default_workspace (Codex gate #5169)
Codex flagged that get_last_workspace() falls back to the GLOBAL last_workspace.txt before the profile config.yaml — so a named profile without its own last_workspace.txt would leak the global path, re-introducing the #5169 bug. Add get_profile_default_workspace() (profile last_workspace.txt -> config.yaml -> terminal.cwd -> default, NO global fallback) and call it from /api/profile/active. Adds a regression test: global last_workspace.txt present + named profile with only config.yaml workspace must return the profile path, never the global. Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
+8
-7
@@ -7533,6 +7533,7 @@ from api.workspace import (
|
||||
load_workspaces,
|
||||
save_workspaces,
|
||||
get_last_workspace,
|
||||
get_profile_default_workspace,
|
||||
set_last_workspace,
|
||||
git_info_for_workspace,
|
||||
authorize_escape_target,
|
||||
@@ -11264,14 +11265,14 @@ def handle_get(handler, parsed) -> bool:
|
||||
active_profile_name = get_active_profile_name()
|
||||
# Resolve the ACTIVE PROFILE's configured workspace so a cold boot with a
|
||||
# profile cookie shows the right composer workspace chip on a blank
|
||||
# new-chat page (#5169). get_last_workspace() is already profile-scoped
|
||||
# (server.py sets the per-request profile from the hermes_profile cookie
|
||||
# before this handler runs) and resolves in the same priority order as
|
||||
# POST /api/profile/switch. Fail open: a resolution error must never 500
|
||||
# this endpoint (boot depends on it) — the chip just falls back to the
|
||||
# global default as before.
|
||||
# new-chat page (#5169). Use get_profile_default_workspace() (NOT
|
||||
# get_last_workspace) so a named profile without its own last_workspace.txt
|
||||
# resolves to its config.yaml workspace/terminal.cwd rather than leaking the
|
||||
# GLOBAL last-workspace file (the #5169 regression Codex flagged). It is
|
||||
# profile-scoped via the per-request hermes_profile cookie set in server.py.
|
||||
# Fail open: a resolution error must never 500 this boot-critical endpoint.
|
||||
try:
|
||||
_profile_default_workspace = get_last_workspace()
|
||||
_profile_default_workspace = get_profile_default_workspace()
|
||||
except Exception:
|
||||
logger.debug("Failed to resolve profile default workspace for /api/profile/active", exc_info=True)
|
||||
_profile_default_workspace = None
|
||||
|
||||
@@ -373,6 +373,45 @@ def save_workspaces(workspaces: list) -> None:
|
||||
ws_file.write_text(json.dumps(workspaces, ensure_ascii=False, indent=2), encoding='utf-8')
|
||||
|
||||
|
||||
def get_profile_default_workspace() -> str:
|
||||
"""Resolve the ACTIVE PROFILE's default workspace, never the global file.
|
||||
|
||||
Like get_last_workspace() but WITHOUT the global ``_GLOBAL_LW_FILE``
|
||||
fallback: for a named profile that has not yet written its own
|
||||
profile-scoped ``last_workspace.txt``, that global fallback would leak the
|
||||
*global* last-workspace instead of the profile's configured workspace —
|
||||
which is exactly the #5169 bug (the composer chip on a blank new-chat page
|
||||
showing the wrong/global workspace for a named profile). Used by
|
||||
``GET /api/profile/active`` so a cold boot under a profile cookie reflects
|
||||
the profile's own configured working directory.
|
||||
|
||||
Priority: profile-scoped ``last_workspace.txt`` -> profile ``config.yaml``
|
||||
``workspace``/``default_workspace`` -> ``terminal.cwd`` -> process default.
|
||||
"""
|
||||
remote_cwd = _remote_terminal_cwd()
|
||||
|
||||
def _valid(raw: str) -> str | None:
|
||||
if not raw:
|
||||
return None
|
||||
if remote_cwd:
|
||||
if _remote_terminal_workspace_candidate(raw) is not None:
|
||||
return raw
|
||||
return None
|
||||
if Path(raw).is_dir():
|
||||
return raw
|
||||
return None
|
||||
|
||||
lw_file = _last_workspace_file()
|
||||
if lw_file.exists():
|
||||
try:
|
||||
p = _valid(lw_file.read_text(encoding='utf-8').strip())
|
||||
if p:
|
||||
return p
|
||||
except Exception:
|
||||
logger.debug("Failed to read profile last workspace from %s", lw_file)
|
||||
return _profile_default_workspace()
|
||||
|
||||
|
||||
def get_last_workspace() -> str:
|
||||
remote_cwd = _remote_terminal_cwd()
|
||||
|
||||
|
||||
@@ -57,7 +57,7 @@ def test_profile_active_includes_default_workspace_from_resolver(monkeypatch):
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: "/home/u/.hermes/profiles/work")
|
||||
monkeypatch.setattr(profiles, "_is_root_profile", lambda name: name in ("default", ""))
|
||||
# The resolver is the single source of truth for the workspace value.
|
||||
monkeypatch.setattr(routes, "get_last_workspace", lambda: "/srv/projects/work")
|
||||
monkeypatch.setattr(routes, "get_profile_default_workspace", lambda: "/srv/projects/work")
|
||||
|
||||
routes.handle_get(SimpleNamespace(), urlparse("/api/profile/active"))
|
||||
|
||||
@@ -67,7 +67,7 @@ def test_profile_active_includes_default_workspace_from_resolver(monkeypatch):
|
||||
assert payload["is_default"] is False
|
||||
assert payload["default_workspace"] == "/srv/projects/work", (
|
||||
"GET /api/profile/active must surface the profile-scoped workspace from "
|
||||
"get_last_workspace() so the blank new-chat composer chip shows it (#5169)"
|
||||
"get_profile_default_workspace() so the blank new-chat composer chip shows it (#5169)"
|
||||
)
|
||||
|
||||
|
||||
@@ -159,3 +159,56 @@ def test_profile_active_default_workspace_falls_back_to_config_workspace(monkeyp
|
||||
"default_workspace must fall back to config.yaml `workspace` when no "
|
||||
"last_workspace.txt exists (mirrors POST /api/profile/switch priority, #5169)"
|
||||
)
|
||||
|
||||
|
||||
def test_profile_active_default_workspace_ignores_global_last_workspace(monkeypatch, tmp_path):
|
||||
"""The #5169 regression Codex flagged: a GLOBAL last_workspace.txt must NOT
|
||||
leak into a named profile's default_workspace.
|
||||
|
||||
get_last_workspace() falls back to the global _GLOBAL_LW_FILE before reaching
|
||||
the profile config.yaml — so a named profile with NO own last_workspace.txt
|
||||
but WITH a config.yaml `workspace` would have returned the global path,
|
||||
re-introducing exactly the wrong-workspace bug. /api/profile/active now uses
|
||||
get_profile_default_workspace(), which skips the global file: profile-scoped
|
||||
last_workspace.txt -> config.yaml -> terminal.cwd -> default.
|
||||
"""
|
||||
base_home = tmp_path / ".hermes"
|
||||
profile_home = base_home / "profiles" / "work"
|
||||
(profile_home / "webui_state").mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# A GLOBAL last_workspace.txt pointing somewhere the named profile must NOT use.
|
||||
global_ws = tmp_path / "global-workspace"
|
||||
global_ws.mkdir(parents=True, exist_ok=True)
|
||||
global_lw = tmp_path / "global-last_workspace.txt"
|
||||
global_lw.write_text(str(global_ws.resolve()), encoding="utf-8")
|
||||
|
||||
# The named profile's OWN configured workspace (config.yaml), which must win.
|
||||
cfg_workspace = tmp_path / "work-config-workspace"
|
||||
cfg_workspace.mkdir(parents=True, exist_ok=True)
|
||||
resolved_cfg_ws = str(cfg_workspace.resolve())
|
||||
|
||||
captured = _capture_j(monkeypatch)
|
||||
monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base_home)
|
||||
monkeypatch.delenv("HERMES_WEBUI_ISOLATED_PROFILE", raising=False)
|
||||
monkeypatch.setattr(profiles, "_INITIAL_ISOLATED_PROFILE_OPT_IN", "")
|
||||
monkeypatch.setattr(workspace, "_remote_terminal_cwd", lambda: None)
|
||||
# The global last-workspace file DOES exist (and is valid) — the named-profile
|
||||
# resolver must still ignore it.
|
||||
monkeypatch.setattr(workspace, "_GLOBAL_LW_FILE", global_lw)
|
||||
# No profile-scoped last_workspace.txt on disk -> resolver consults config.yaml.
|
||||
monkeypatch.setattr(config_mod, "get_config", lambda: {"workspace": resolved_cfg_ws})
|
||||
|
||||
profiles.set_request_profile("work")
|
||||
try:
|
||||
routes.handle_get(SimpleNamespace(), urlparse("/api/profile/active"))
|
||||
finally:
|
||||
profiles.clear_request_profile()
|
||||
|
||||
payload = captured["payload"]
|
||||
assert payload["default_workspace"] == resolved_cfg_ws, (
|
||||
"default_workspace must resolve from the named profile's config.yaml workspace, "
|
||||
"NOT the global last_workspace.txt — the #5169 regression guard"
|
||||
)
|
||||
assert payload["default_workspace"] != str(global_ws.resolve()), (
|
||||
"the global last-workspace path must never leak into a named profile's default_workspace"
|
||||
)
|
||||
|
||||
@@ -146,7 +146,7 @@ class TestProfileActiveDefaultWorkspaceApi:
|
||||
monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "work")
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: "/home/.hermes/profiles/work")
|
||||
monkeypatch.setattr(profiles, "_is_root_profile", lambda _n: False)
|
||||
monkeypatch.setattr(routes, "get_last_workspace", lambda: expected_ws)
|
||||
monkeypatch.setattr(routes, "get_profile_default_workspace", lambda: expected_ws)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"j",
|
||||
|
||||
Reference in New Issue
Block a user