hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix(files): refuse live-DB downloads and share one sidecar-aware liveness check

Browse Source 
FileResponse opens and closes the file in the dashboard process, so
downloading a live state.db (or its -shm/-wal) via /api/fs/download or
the managed-file read/download/media routes still cancelled the
connection's POSIX locks. Both now return 409 via is_live_database_file;
the registry lock is not held across the streamed response.

The main-or-WAL-sidecar rule now lives in one _live_main_key helper used
by offline_file_access, has_live_connection and read_header_bytes_preopen,
and the sidecar refusal names the main database the connection is open on.

@file previews hold _live_lock only for the raw read; token counting and
formatting run after release. The Linux lock test gains requires_wal
(Hermes uses DELETE mode on WAL-reset-vulnerable SQLite), covers the
download refusal, and drops an ambiguous conditional assert.

Co-authored-by: Benjamin PERRY <benjaminperry6@yahoo.fr>
This commit is contained in:
kshitijk4poor
2026-09-27 15:46:36 +05:30
committed by kshitij
parent 4901f1a6b1
commit 48196b0ceb
4 changed files with 72 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
agent/context_references.py
View File

@@ -17,6 +17,7 @@ from typing import Awaitable, Callable
from agent.model_metadata import CHARS_PER_TOKEN, estimate_tokens_rough
from hermes_cli._subprocess_compat import IS_WINDOWS, harden_git_argv, noninteractive_git_env, windows_hide_flags
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
from hermes_cli.sizefmt import format_bytes
# ── Plugin context-reference provider API ────────────────────────────────────
@@ -296,21 +297,23 @@ def _expand_path_reference(ref: ContextReference, cwd: Path, *, allowed_root: Pa
if is_folder:
listing = _build_folder_listing(path, cwd, display_base=allowed_root)
return None, f"📁 {ref.raw} ({estimate_tokens_rough(listing)} tokens)\n{listing}"
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
try:
# Keep admission through every sniff, line count and text read: a connection
# can start between a check and a later open otherwise.
# Keep admission through every sniff, stat and text read (a connection can start
# between a check and a later open otherwise), but release it before token
# counting/formatting: the registry lock blocks every tracked connect/close.
with offline_file_access(path, what="preview context reference"):
return _expand_file_reference(ref, path, max_inline_tokens)
raw = _read_file_reference(ref, path, max_inline_tokens)
except LiveConnectionError:
return None, _on_disk_reference_block(
ref, path, descriptor="live SQLite database file",
reason="not previewed: raw access would cancel SQLite's POSIX locks.",
guidance="Do not open this file directly while its database connection is live.",
)
return raw if isinstance(raw, tuple) else _format_file_reference(ref, path, raw, max_inline_tokens)
def _expand_file_reference(ref: ContextReference, path: Path, max_inline_tokens: int | None) -> Expansion:
def _read_file_reference(ref: ContextReference, path: Path, max_inline_tokens: int | None) -> str | Expansion:
"""Raw file I/O for an @file ref: the text to inline, or an early refusal block."""
if _is_binary_file(path):
# A bare "not supported" warning was a dead end (the model gave up); the file IS
# on disk where the agent's tools run, so hand it an actionable block instead.
@@ -363,6 +366,10 @@ def _expand_file_reference(ref: ContextReference, path: Path, max_inline_tokens:
if max_inline_tokens is not None and size > max_inline_tokens * CHARS_PER_TOKEN:
return None, _oversized_text_reference_block(ref, path, size // CHARS_PER_TOKEN)
text = path.read_text(encoding="utf-8-sig")
return text
def _format_file_reference(ref: ContextReference, path: Path, text: str, max_inline_tokens: int | None) -> Expansion:
lang = _FENCE_LANGUAGES.get(path.suffix.lower(), "")
text_tokens = estimate_tokens_rough(text)
# Check BEFORE building the fenced block: an oversized file is not going to be
@@ -672,13 +679,12 @@ def _file_metadata(path: Path) -> str:
size = path.stat().st_size
except OSError:
return "unknown size"
# A listing line is a summary, not content: past the cap, byte size conveys the
# same "how big is this" without a full scan per entry.
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
try:
# A directory preview inspects each entry separately; the registry lock
# must cover both its binary sniff and optional line-count read.
with offline_file_access(path, what="inspect folder entry"):
# A listing line is a summary, not content: past the cap, byte size conveys
# the same "how big is this" without a full scan per entry.
if _is_binary_file(path) or size > _LINE_COUNT_MAX_BYTES:
return f"{size} bytes"
with path.open("rb") as fh:

41
hermes_cli/sqlite_safe_read.py
View File

@@ -76,10 +76,31 @@ def untrack_connection(path: Path | str) -> None:
_track_key(_key(path), -1)
def _live_main_key(key: str) -> Optional[str]:
"""The tracked main-database key that makes *key* live, or ``None`` (caller holds ``_live_lock``).
SQLite locks the main file and its WAL sidecars; a raw ``close()`` of any of those
inodes cancels this process's POSIX locks, but the registry is keyed by the main path."""
if key in _live_connections:
return key
for suffix in ("-wal", "-shm"):
if key.endswith(suffix) and key[:-len(suffix)] in _live_connections:
return key[:-len(suffix)]
return None
def has_live_connection(path: Path | str) -> bool:
"""Whether this process currently holds any connection to *path*."""
"""Whether this process holds a connection to *path* (or to the database it is a sidecar of)."""
with _live_lock:
return _key(path) in _live_connections
return _live_main_key(_key(path)) is not None
def is_live_database_file(path: Path | str) -> bool:
"""Whether a raw open/close of *path* would cancel a live connection's POSIX locks.
Point-in-time answer for callers that cannot hold ``_live_lock`` across their I/O
(e.g. a streamed download: the lock must never be held across an await/yield)."""
return has_live_connection(path)
class _TrackingMixin:
@@ -211,7 +232,7 @@ def read_header_bytes_preopen(path: Path | str, *, length: int = 100, force: boo
overwritten?). Check and open/read/close run together under ``_live_lock`` so a connection
cannot be opened between deciding "nothing is live" and closing this descriptor."""
with _live_lock:
if not force and _key(path) in _live_connections:
if not force and _live_main_key(_key(path)) is not None:
logger.debug(
"refusing byte-level read of %s: a live connection exists in "
"this process and close() would cancel its POSIX locks",
@@ -230,17 +251,11 @@ def offline_file_access(path: Path | str, *, what: str = "read"):
:func:`has_live_connection` and *then* doing raw I/O is a check/use race (a connection opened
in between loses its POSIX locks to the raw ``close()``). Held only for the raw I/O."""
with _live_lock:
key = _key(path)
# SQLite locks the main file and its WAL shared-memory sidecar. A raw
# close of either inode cancels this process's POSIX locks, while the
# connection registry is keyed by the main database path.
live = key in _live_connections or any(
key.endswith(suffix) and key[:-len(suffix)] in _live_connections
for suffix in ("-wal", "-shm")
)
if live:
main = _live_main_key(_key(path))
if main is not None:
subject = "it" if main == _key(path) else f"its main database {main}"
raise LiveConnectionError(
f"Refusing to {what} {path}: a connection to it is still open "
f"Refusing to {what} {path}: a connection to {subject} is still open "
"in this process, and raw file access would cancel that "
"connection's POSIX advisory locks. Close all database "
"handles (stop the gateway/dashboard) and retry.")

20
hermes_cli/web_routers/files.py
View File

@@ -25,6 +25,7 @@ from fastapi import APIRouter, File, Form, HTTPException, Request, UploadFile
from fastapi.responses import FileResponse
from hermes_cli._subprocess_compat import windows_hide_flags
from hermes_cli.sqlite_safe_read import LiveConnectionError, is_live_database_file, offline_file_access
from hermes_cli.web_deps import late
from hermes_cli.web_server_files import (
_fs_path, _managed_file_entry, _managed_response_meta, _resolve_managed_path,
@@ -174,9 +175,22 @@ def _fs_regular_file(path: Path) -> tuple[Path, os.stat_result]:
return target, st
def _refuse_live_database(target: Path) -> None:
"""409 before streaming ``target`` while this process has a live SQLite connection to it.
``FileResponse`` opens and closes the file in-process, and that raw close cancels the
connection's POSIX locks. The registry lock can't be held across a streamed response
(never across an await/yield), so this is a point-in-time admission check."""
if is_live_database_file(target):
raise HTTPException(
status_code=409,
detail=f"Refusing to download {target}: a SQLite connection to its database is "
"open in this process, and raw file access would cancel its POSIX advisory locks.",
)
def _fs_read_bytes(target: Path, limit: Optional[int] = None) -> bytes:
"""Read (a prefix of) ``target``; 403/400 on failure."""
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
"""Read (a prefix of) ``target``; 403/400 on failure, 409 while a SQLite connection to it is live."""
try:
# Keep admission through close; a raw close cancels this process's SQLite locks.
with offline_file_access(target, what="preview file"):
@@ -404,6 +418,7 @@ def _managed_readable_file(request: Request, path: str) -> tuple[Any, Path, str,
raise HTTPException(status_code=400, detail="Path is not a file")
if _is_sensitive_path(target):
raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed")
_refuse_live_database(target)
mime_type = mimetypes.guess_type(target.name)[0] or "application/octet-stream"
return policy, target, display_path, _MANAGED_FILE_MAX_BYTES, mime_type
@@ -740,6 +755,7 @@ async def fs_download(
path: str, profile: Optional[str] = None, session_id: Optional[str] = None,
):
target, _st = _fs_regular_file(await _fs_download_path(path, profile, session_id))
_refuse_live_database(target)
return FileResponse(
path=str(target),
media_type=_fs_mime_type(target),

15
tests/agent/test_live_db_preview_locks.py
View File

@@ -11,10 +11,11 @@ from fastapi import HTTPException
from agent.context_references import _expand_path_reference, parse_context_references
from hermes_state import SessionDB
from hermes_cli.web_routers.files import fs_read_text
from hermes_cli.web_routers.files import fs_download, fs_read_text
@pytest.mark.platforms("linux")
@pytest.mark.requires_wal
@pytest.mark.parametrize("route,target_kind", [
("file", "main"), ("folder", "directory"), ("desktop", "main"), ("desktop", "shm"),
])
@@ -45,7 +46,7 @@ def test_preview_preserves_live_database_locks(tmp_path, route, target_kind):
def posix_locks(file):
inode = file.stat().st_ino
return sorted(line.split(": ", 1)[1] for line in Path("/proc/locks").read_text(encoding="utf-8").splitlines()
if f":{inode} " in line and f"POSIX ADVISORY" in line
if f":{inode} " in line and "POSIX ADVISORY" in line
and f" {os.getpid()} " in line)
def rival_locked():
@@ -63,13 +64,20 @@ def test_preview_preserves_live_database_locks(tmp_path, route, target_kind):
with pytest.raises(HTTPException) as refused:
asyncio.run(fs_read_text(str(target)))
assert refused.value.status_code == 409
if target_kind == "shm":
assert "main database" in refused.value.detail
# FileResponse opens/closes in-process too, so a download must be refused as well.
with pytest.raises(HTTPException) as refused:
asyncio.run(fs_download(str(target)))
assert refused.value.status_code == 409
assert asyncio.run(fs_read_text(str(text)))["text"] == "ordinary readable text"
else:
ref = parse_context_references(f"@{route}:{target}")[0]
warning, block = _expand_path_reference(ref, tmp_path.parent)
assert warning is None
assert block is not None
assert "not previewed" in block if route == "file" else "state.db" in block
if route == "file":
assert "not previewed" in block
ordinary = parse_context_references(f"@file:{text}")[0]
warning, block = _expand_path_reference(ordinary, tmp_path.parent)
assert warning is None and block is not None and "ordinary readable text" in block
@@ -107,3 +115,4 @@ def test_closed_database_can_still_be_previewed(tmp_path):
assert warning is None and block is not None and "binary file" in block
preview = asyncio.run(fs_read_text(str(path)))
assert preview["binary"] is True and preview["byteSize"] == path.stat().st_size
assert asyncio.run(fs_download(str(path))).path == str(path)