From 43ec69cef3aae483fe7b132c204cb871bfc895d2 Mon Sep 17 00:00:00 2001 From: srojk34 <286497132+srojk34@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:21:31 +0300 Subject: [PATCH 1/2] security(dashboard): widen managed-files sensitive-filename guard past .env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _is_sensitive_filename() only blocked .env / .env., but the dashboard Files tab's managed root is operator-configurable and, per the docker-mount scenario #57505 was filed against, can point directly at HERMES_HOME — where the canonical credential stores enforced elsewhere in the codebase (gateway.platforms.base._ROOT_CREDENTIAL_FILES, agent.file_safety.get_read_block_error) all live: auth.json, OAuth token stores, webhook HMAC secrets, the Bitwarden disk cache. None of those basenames were blocked, so the Files tab could still list, read, and download them. .envrc (direnv) also slipped past the old check since it doesn't equal ".env" or start with ".env.". Widen the basename set to mirror both existing guards so the dashboard doesn't lag behind them. --- hermes_cli/web_server.py | 38 ++++++++++++++++--- tests/hermes_cli/test_web_server_files.py | 46 +++++++++++++++++++++++ 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index c3af49f2531..73cbf8f1b02 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1194,15 +1194,41 @@ _FS_READDIR_HIDDEN = { # Filenames that must never be listed, read, or downloaded through the # managed-files API. These typically contain credentials (API keys, tokens) # and exposing them through the dashboard file browser is a security leak — -# see issue #57505. -def _is_sensitive_filename(name: str) -> bool: - """Return True for ``.env`` and any ``.env.`` variant. +# see issue #57505. The set mirrors the two canonical credential guards +# elsewhere in the codebase (agent.file_safety.get_read_block_error and +# gateway.platforms.base._ROOT_CREDENTIAL_FILES) so the dashboard Files tab +# doesn't lag behind them — an operator can point the managed root at +# HERMES_HOME itself, at which point every one of these basenames is a live +# secret store sitting in the browsable tree. +_SENSITIVE_MANAGED_FILE_BASENAMES = frozenset({ + "auth.json", + "auth.lock", + "credentials", + "config.yaml", + ".anthropic_oauth.json", + "google_token.json", + "google_oauth_pending.json", + "google_oauth.json", + "webhook_subscriptions.json", + "bws_cache.json", +}) - Case-insensitive so ``.ENV`` / ``.Env.local`` on case-insensitive - filesystems (macOS/Windows mounts) can't slip past the guard. + +def _is_sensitive_filename(name: str) -> bool: + """Return True for a filename the managed-files API must never expose. + + Covers ``.env`` / ``.env.`` / ``.envrc`` variants plus the + canonical Hermes credential-store basenames (see + ``_SENSITIVE_MANAGED_FILE_BASENAMES`` above). + + Case-insensitive so ``.ENV`` / ``.Env.local`` / ``Auth.JSON`` on + case-insensitive filesystems (macOS/Windows mounts) can't slip past + the guard. """ lowered = name.lower() - return lowered == ".env" or lowered.startswith(".env.") + if lowered == ".env" or lowered.startswith(".env.") or lowered == ".envrc": + return True + return lowered in _SENSITIVE_MANAGED_FILE_BASENAMES _FS_DATA_URL_MAX_BYTES = 16 * 1024 * 1024 _FS_TEXT_SOURCE_MAX_BYTES = 64 * 1024 * 1024 _FS_TEXT_PREVIEW_MAX_BYTES = 512 * 1024 diff --git a/tests/hermes_cli/test_web_server_files.py b/tests/hermes_cli/test_web_server_files.py index 63a8b39ff07..f5bcf1d1198 100644 --- a/tests/hermes_cli/test_web_server_files.py +++ b/tests/hermes_cli/test_web_server_files.py @@ -560,3 +560,49 @@ def test_sensitive_env_case_insensitive_blocked(forced_files_client): p.write_text("SECRET=abc123") assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403 assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403 + + +def test_envrc_blocked(forced_files_client): + """Regression: .envrc (direnv) is a distinct basename from .env. and + was not caught by the old ``== ".env" or startswith(".env.")`` check.""" + client, root = forced_files_client + + root.mkdir(parents=True, exist_ok=True) + p = root / ".envrc" + p.write_text("export SECRET_KEY=abc123") + + listing = client.get("/api/files", params={"path": str(root)}) + assert ".envrc" not in [e["name"] for e in listing.json()["entries"]] + assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403 + assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403 + + +def test_other_credential_store_basenames_blocked(forced_files_client): + """Regression: the managed-files guard must cover the same credential + basenames as gateway.platforms.base._ROOT_CREDENTIAL_FILES and + agent.file_safety.get_read_block_error, not just .env — an operator can + point the managed root at HERMES_HOME itself (#57505), which contains + all of these live secret stores.""" + client, root = forced_files_client + root.mkdir(parents=True, exist_ok=True) + + for name in ( + "auth.json", + "auth.lock", + "credentials", + "config.yaml", + ".anthropic_oauth.json", + "google_token.json", + "google_oauth_pending.json", + "google_oauth.json", + "webhook_subscriptions.json", + "bws_cache.json", + ): + p = root / name + p.write_text("SECRET=abc123") + assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403, name + assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403, name + + listing = client.get("/api/files", params={"path": str(root)}) + names = [e["name"] for e in listing.json()["entries"]] + assert names == [] From 8b24376d63c9aee031b3cf533884b749a645972a Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:36:10 +0530 Subject: [PATCH 2/2] fix(dashboard): close credential-dir-tree gap + .git-credentials in managed-files guard Follow-up to @srojk34's basename-denylist widening. Two gaps the basename-only guard left, both covered by the two canonical guards it mirrors: - Directory-tree stores mcp-tokens/ (live MCP OAuth tokens) and pairing/ are denied as whole trees by gateway.platforms.base._ROOT_CREDENTIAL_DIRS and agent.file_safety, but the dashboard files API descends into subdirs, so mcp-tokens/.json (non-canonical basename) stayed listable/readable/downloadable. Add _is_sensitive_path(), a path-aware check that blocks any path with a credential-directory component, and route all three call sites (list/read/download) through it. - Add .git-credentials to the basename set (agent.file_safety blocks it too). - Correct the docstring: it now says it mirrors the credential-FILE basenames of the canonical guards, with the directory trees handled by the new path-aware helper (the prior wording overstated parity). Scope stays on the read/list/download exfil surface (#57505); the write endpoints (upload/mkdir/delete) are a separate threat and out of scope. Tests: dir-tree descent blocked (mcp-tokens/pairing per-server files), .git-credentials blocked, plus a positive control that a benign subdir file stays browsable. Mutation-checked (neuter _is_sensitive_path -> new tests fail). 39 web_server_files + fs tests pass, ruff clean. --- hermes_cli/web_server.py | 57 ++++++++++++++++-- tests/hermes_cli/test_web_server_files.py | 72 +++++++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 73cbf8f1b02..401dfa6506a 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1194,8 +1194,9 @@ _FS_READDIR_HIDDEN = { # Filenames that must never be listed, read, or downloaded through the # managed-files API. These typically contain credentials (API keys, tokens) # and exposing them through the dashboard file browser is a security leak — -# see issue #57505. The set mirrors the two canonical credential guards -# elsewhere in the codebase (agent.file_safety.get_read_block_error and +# see issue #57505. The set mirrors the credential-file basenames of the two +# canonical credential guards elsewhere in the codebase +# (agent.file_safety.get_read_block_error and # gateway.platforms.base._ROOT_CREDENTIAL_FILES) so the dashboard Files tab # doesn't lag behind them — an operator can point the managed root at # HERMES_HOME itself, at which point every one of these basenames is a live @@ -1211,11 +1212,27 @@ _SENSITIVE_MANAGED_FILE_BASENAMES = frozenset({ "google_oauth.json", "webhook_subscriptions.json", "bws_cache.json", + # git's credential-store helper cache (agent.file_safety blocks this too). + ".git-credentials", +}) + +# Directory names whose entire subtree is credential material. Both canonical +# guards deny these as directory trees, not basenames: +# * gateway.platforms.base._ROOT_CREDENTIAL_DIRS = {"pairing", "mcp-tokens"} +# * agent.file_safety.get_read_block_error (mcp-tokens/ prefix match) +# The managed-files API lets the browser descend into subdirs, so a +# basename-only guard would still expose e.g. ``mcp-tokens/.json`` +# (live MCP OAuth tokens) and ``pairing/``. We match on ANY path component +# so these trees are blocked wherever they appear under the browsable root, +# without needing to resolve them relative to HERMES_HOME. +_SENSITIVE_MANAGED_DIR_NAMES = frozenset({ + "mcp-tokens", + "pairing", }) def _is_sensitive_filename(name: str) -> bool: - """Return True for a filename the managed-files API must never expose. + """Return True for a basename the managed-files API must never expose. Covers ``.env`` / ``.env.`` / ``.envrc`` variants plus the canonical Hermes credential-store basenames (see @@ -1224,11 +1241,39 @@ def _is_sensitive_filename(name: str) -> bool: Case-insensitive so ``.ENV`` / ``.Env.local`` / ``Auth.JSON`` on case-insensitive filesystems (macOS/Windows mounts) can't slip past the guard. + + Basename-only: for the directory-tree credential stores + (``mcp-tokens/``, ``pairing/``) that the canonical guards also deny, + use :func:`_is_sensitive_path`, which the API call sites route through. """ lowered = name.lower() if lowered == ".env" or lowered.startswith(".env.") or lowered == ".envrc": return True return lowered in _SENSITIVE_MANAGED_FILE_BASENAMES + + +def _is_sensitive_path(path: Path) -> bool: + """Return True for any path the managed-files API must never expose. + + Combines the basename denylist (:func:`_is_sensitive_filename`) with a + credential-directory-tree check: a path is sensitive if its own basename + is sensitive OR any of its path components is a credential directory + (``mcp-tokens`` / ``pairing``). The component match is case-insensitive + and needs no HERMES_HOME resolution, so it blocks these trees wherever + they sit under the operator-configured managed root — closing the gap + the canonical guards cover as directory trees but a basename-only check + would miss. + + Read-side only: this guards list/read/download (the #57505 exfil surface). + The write endpoints (upload/mkdir/delete) are a separate threat class + handled by the write-path checks; extending this guard to them is out of + scope for this fix. + """ + if _is_sensitive_filename(path.name): + return True + return any(part.lower() in _SENSITIVE_MANAGED_DIR_NAMES for part in path.parts) + + _FS_DATA_URL_MAX_BYTES = 16 * 1024 * 1024 _FS_TEXT_SOURCE_MAX_BYTES = 64 * 1024 * 1024 _FS_TEXT_PREVIEW_MAX_BYTES = 512 * 1024 @@ -1662,7 +1707,7 @@ async def list_managed_files(request: Request, path: Optional[str] = None): entries = [ _managed_file_entry(policy, child) for child in target.iterdir() - if not _is_sensitive_filename(child.name) + if not _is_sensitive_path(child) ] except PermissionError: raise HTTPException(status_code=403, detail="Directory is not readable") @@ -1689,7 +1734,7 @@ async def read_managed_file(request: Request, path: str): raise HTTPException(status_code=404, detail="File not found") if not target.is_file(): raise HTTPException(status_code=400, detail="Path is not a file") - if _is_sensitive_filename(target.name): + if _is_sensitive_path(target): raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed") try: @@ -1733,7 +1778,7 @@ async def download_managed_file(request: Request, path: str): raise HTTPException(status_code=404, detail="File not found") if not target.is_file(): raise HTTPException(status_code=400, detail="Path is not a file") - if _is_sensitive_filename(target.name): + if _is_sensitive_path(target): raise HTTPException(status_code=403, detail="Access to sensitive files is not allowed") try: diff --git a/tests/hermes_cli/test_web_server_files.py b/tests/hermes_cli/test_web_server_files.py index f5bcf1d1198..d1e46e0f82a 100644 --- a/tests/hermes_cli/test_web_server_files.py +++ b/tests/hermes_cli/test_web_server_files.py @@ -606,3 +606,75 @@ def test_other_credential_store_basenames_blocked(forced_files_client): listing = client.get("/api/files", params={"path": str(root)}) names = [e["name"] for e in listing.json()["entries"]] assert names == [] + + +def test_git_credentials_blocked(forced_files_client): + """Regression: .git-credentials (git's credential-store helper cache) is + blocked by agent.file_safety; the dashboard guard must cover it too.""" + client, root = forced_files_client + + root.mkdir(parents=True, exist_ok=True) + p = root / ".git-credentials" + p.write_text("https://user:token@github.com\n") + + listing = client.get("/api/files", params={"path": str(root)}) + assert ".git-credentials" not in [e["name"] for e in listing.json()["entries"]] + assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403 + assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403 + + +def test_credential_dir_trees_blocked_on_subdir_descent(forced_files_client): + """Regression: mcp-tokens/ (live MCP OAuth tokens) and pairing/ are denied + as whole directory trees by both canonical guards + (gateway.platforms.base._ROOT_CREDENTIAL_DIRS and + agent.file_safety). A basename-only check would still expose their + per-server files (e.g. ``mcp-tokens/github.json``) once the browser + descends into the subdir. The managed-files guard must block any path with + a credential-directory component, not just leaf basenames.""" + client, root = forced_files_client + root.mkdir(parents=True, exist_ok=True) + + # A per-server MCP token file with a NON-canonical basename that the + # basename denylist alone would not catch. + mcp_dir = root / "mcp-tokens" + mcp_dir.mkdir(parents=True, exist_ok=True) + mcp_file = mcp_dir / "github.json" + mcp_file.write_text('{"access_token": "SECRET"}\n') + + pairing_dir = root / "pairing" + pairing_dir.mkdir(parents=True, exist_ok=True) + pairing_file = pairing_dir / "device-abc" + pairing_file.write_text("PAIRING-SECRET\n") + + # The token dirs themselves must not appear in the root listing. + root_names = [e["name"] for e in client.get( + "/api/files", params={"path": str(root)}).json()["entries"]] + assert "mcp-tokens" not in root_names + assert "pairing" not in root_names + + # Read/download of the per-server files must be denied even though their + # basenames aren't in _SENSITIVE_MANAGED_FILE_BASENAMES. + for p in (mcp_file, pairing_file): + assert client.get("/api/files/read", params={"path": str(p)}).status_code == 403, str(p) + assert client.get("/api/files/download", params={"path": str(p)}).status_code == 403, str(p) + + # Listing the credential dir itself yields nothing exploitable: every child + # is filtered because the parent component is a credential dir. + mcp_listing = client.get("/api/files", params={"path": str(mcp_dir)}) + assert [e["name"] for e in mcp_listing.json()["entries"]] == [] + + +def test_benign_subdir_file_still_browsable(forced_files_client): + """Positive control: the directory-component guard must NOT over-block a + benign subdir. A normal file under a normal subdir stays listable/readable.""" + client, root = forced_files_client + root.mkdir(parents=True, exist_ok=True) + + sub = root / "notes" + sub.mkdir(parents=True, exist_ok=True) + p = sub / "todo.txt" + p.write_text("buy milk\n") + + listing = client.get("/api/files", params={"path": str(sub)}) + assert "todo.txt" in [e["name"] for e in listing.json()["entries"]] + assert client.get("/api/files/read", params={"path": str(p)}).status_code == 200