Merge pull request #5263 from nesquena/rebase-5179
Release v0.51.762 — file-manager active-profile enforcement (#5179, @Hinotoi-agent)
This commit is contained in:
3 files changed
+127
-8
No files matched your search
@@ -35,6 +35,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **The file manager now enforces the active profile when resolving a session's workspace.** When multiple Hermes profiles share a box, a session-scoped file operation looked up the session by id without checking which profile owns it, so a session belonging to a *different* profile could expose its workspace path to the currently active profile. The session-for-file-ops resolver now rejects a session whose owning profile doesn't match the request's active profile (raising the same not-found that the file routes already translate to a 404), failing closed. Same-profile access, single-profile/no-auth deployments, and the external-session (state.db) fallback are all unchanged — only genuine cross-profile lookups are now denied. Thanks @Hinotoi-agent. (#5179)
|
||||
|
||||
- **A terminal streaming failure no longer wipes the transcript you can already see.** When the live SSE connection dropped for good mid-response and the server's settled snapshot came back *shorter* than what the browser had already rendered, the chat pane replaced the visible transcript with the shorter snapshot — momentarily losing rows the reader was looking at. The recovery path now preserves the visible transcript when the server snapshot is a verified strict prefix of it and the visible tail is the "Connection interrupted" marker, and otherwise still takes the authoritative server messages (normal settlement is unchanged and stays server-authoritative). The interrupted-connection marker is also de-duplicated so a recovery can't stack multiple copies. Thanks @rodboev. (#5240, fixes #5224)
|
||||
|
||||
- **Forking/truncating a conversation mid-history no longer drops a kept turn or its tool rows.** When the truncate logic matched rows that lacked a stable id/timestamp (common for provider rows, especially after a recovery that strips timestamps), a duplicate row adjacent to the cut could bind the wrong copy — dropping the kept user turn, or, in the ambiguous case, the kept assistant turn's trailing tool-result rows. The matcher now compares a richer row signature (role + content + tool_call_id / tool_use_id / tool_name / tool_calls) and **fails closed on ambiguity** (cutting at the earliest ambiguous candidate so the kept turn and its tool tail survive) instead of greedily binding the first match. Thanks @rodboev. (#5212, #5134)
|
||||
|
||||
+21
-5
@@ -3392,21 +3392,37 @@ class _ExternalSessionView:
|
||||
|
||||
|
||||
def get_session_for_file_ops(sid: str):
|
||||
"""Return a session-like object for file-manager handlers.
|
||||
"""Return a profile-authorized session-like object for file-manager handlers.
|
||||
|
||||
Tries ``get_session`` first (preserves all existing behavior for WebUI
|
||||
sessions). If that raises ``KeyError``, checks state.db; when the session
|
||||
exists there, returns an ``_ExternalSessionView`` whose ``workspace`` is
|
||||
the active WebUI workspace. If neither has the session, re-raises
|
||||
sessions) and only returns that session when its stored profile belongs to
|
||||
the active request profile. If that lookup fails, checks state.db; when the
|
||||
session exists there, returns an ``_ExternalSessionView`` whose ``workspace``
|
||||
is the active WebUI workspace. If neither has the session, re-raises
|
||||
``KeyError`` so callers continue to return their existing 404.
|
||||
"""
|
||||
try:
|
||||
return get_session(sid, metadata_only=True)
|
||||
session = get_session(sid, metadata_only=True)
|
||||
except KeyError:
|
||||
if state_db_has_session(sid):
|
||||
return _ExternalSessionView(str(sid), str(get_last_workspace()))
|
||||
raise
|
||||
|
||||
from api.profiles import _profiles_match, get_active_profile_name
|
||||
|
||||
session_profile = getattr(session, 'profile', None)
|
||||
active_profile = get_active_profile_name()
|
||||
if not _profiles_match(session_profile, active_profile):
|
||||
logger.debug(
|
||||
"Rejected file-manager session for foreign profile: "
|
||||
"session_id=%s session_profile=%r active_profile=%r",
|
||||
sid,
|
||||
session_profile,
|
||||
active_profile,
|
||||
)
|
||||
raise KeyError(sid)
|
||||
return session
|
||||
|
||||
|
||||
def _active_state_db_path() -> Path:
|
||||
"""Return state.db for the active Hermes profile, degrading to HERMES_HOME."""
|
||||
|
||||
@@ -11,9 +11,13 @@ Covers:
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import logging
|
||||
import re
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from urllib.parse import urlparse
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -107,22 +111,119 @@ def models_module():
|
||||
|
||||
def test_get_session_for_file_ops_webui_passthrough(models_module, monkeypatch):
|
||||
"""(a) WebUI session — delegates to get_session, no state.db consulted."""
|
||||
sentinel = object()
|
||||
called = {"get_session": 0, "state_db": 0}
|
||||
profiles_module = pytest.importorskip("api.profiles")
|
||||
sentinel = SimpleNamespace(profile=None)
|
||||
called = {"get_session": 0, "profile_match": 0, "state_db": 0}
|
||||
|
||||
def fake_get_session(sid, metadata_only=False):
|
||||
called["get_session"] += 1
|
||||
return sentinel
|
||||
|
||||
def fake_profiles_match(session_profile, active_profile):
|
||||
called["profile_match"] += 1
|
||||
assert session_profile is None
|
||||
assert active_profile == "default"
|
||||
return True
|
||||
|
||||
def fake_has(_sid):
|
||||
called["state_db"] += 1
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(models_module, "get_session", fake_get_session)
|
||||
monkeypatch.setattr(models_module, "state_db_has_session", fake_has)
|
||||
monkeypatch.setattr(profiles_module, "_profiles_match", fake_profiles_match)
|
||||
monkeypatch.setattr(profiles_module, "get_active_profile_name", lambda: "default")
|
||||
result = models_module.get_session_for_file_ops("webui-sid")
|
||||
assert result is sentinel
|
||||
assert called == {"get_session": 1, "state_db": 0}
|
||||
assert called == {"get_session": 1, "profile_match": 1, "state_db": 0}
|
||||
|
||||
|
||||
def test_get_session_for_file_ops_rejects_foreign_profile(
|
||||
models_module, monkeypatch, tmp_path, caplog
|
||||
):
|
||||
"""WebUI sessions must belong to the active profile before file access."""
|
||||
profiles_module = pytest.importorskip("api.profiles")
|
||||
foreign_session = SimpleNamespace(profile="research", workspace=str(tmp_path))
|
||||
called = {"get_session": 0, "profile_match": 0, "state_db": 0}
|
||||
|
||||
def fake_get_session(sid, metadata_only=False):
|
||||
called["get_session"] += 1
|
||||
return foreign_session
|
||||
|
||||
def fake_profiles_match(session_profile, active_profile):
|
||||
called["profile_match"] += 1
|
||||
assert session_profile == "research"
|
||||
assert active_profile == "default"
|
||||
return False
|
||||
|
||||
def fake_has(_sid):
|
||||
called["state_db"] += 1
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(models_module, "get_session", fake_get_session)
|
||||
monkeypatch.setattr(models_module, "state_db_has_session", fake_has)
|
||||
monkeypatch.setattr(profiles_module, "_profiles_match", fake_profiles_match)
|
||||
monkeypatch.setattr(profiles_module, "get_active_profile_name", lambda: "default")
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger=models_module.logger.name):
|
||||
with pytest.raises(KeyError):
|
||||
models_module.get_session_for_file_ops("foreign-webui-sid")
|
||||
# A found-but-foreign WebUI sidecar is an authorization failure, not a
|
||||
# missing-session condition that can fall through to the state.db fallback.
|
||||
assert called == {"get_session": 1, "profile_match": 1, "state_db": 0}
|
||||
assert "Rejected file-manager session for foreign profile" in caplog.text
|
||||
assert "foreign-webui-sid" in caplog.text
|
||||
assert "session_profile='research'" in caplog.text
|
||||
assert "active_profile='default'" in caplog.text
|
||||
|
||||
|
||||
def test_file_read_rejects_foreign_profile_session(
|
||||
models_module, monkeypatch, tmp_path
|
||||
):
|
||||
"""A default-profile file route cannot read a named-profile workspace."""
|
||||
profiles_module = pytest.importorskip("api.profiles")
|
||||
routes_module = pytest.importorskip("api.routes")
|
||||
workspace = tmp_path / "named-workspace"
|
||||
workspace.mkdir()
|
||||
(workspace / "marker.txt").write_text("foreign profile marker")
|
||||
session = models_module.Session(
|
||||
session_id="foreign-profile-file-read",
|
||||
workspace=str(workspace),
|
||||
profile="research",
|
||||
)
|
||||
models_module.SESSIONS[session.session_id] = session
|
||||
|
||||
class Handler:
|
||||
command = "GET"
|
||||
headers = {}
|
||||
|
||||
def __init__(self):
|
||||
self.status = None
|
||||
self.headers_sent = []
|
||||
self.wfile = io.BytesIO()
|
||||
|
||||
def send_response(self, code):
|
||||
self.status = code
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.headers_sent.append((key, value))
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(profiles_module, "get_active_profile_name", lambda: "default")
|
||||
try:
|
||||
handler = Handler()
|
||||
routes_module._handle_file_read(
|
||||
handler,
|
||||
urlparse(
|
||||
"/api/file?session_id=foreign-profile-file-read&path=marker.txt"
|
||||
),
|
||||
)
|
||||
assert handler.status == 404
|
||||
assert b"foreign profile marker" not in handler.wfile.getvalue()
|
||||
finally:
|
||||
models_module.SESSIONS.pop(session.session_id, None)
|
||||
|
||||
|
||||
def test_get_session_for_file_ops_state_db_fallback(
|
||||
|
||||
Reference in new issue
Block a user