Skip to content

fix(dashboard): block managed credential directories - #57865

Closed
necoweb3 wants to merge 2 commits into
NousResearch:mainfrom
necoweb3:fix/dashboard-managed-files-sensitive-dirs
Closed

fix(dashboard): block managed credential directories#57865
necoweb3 wants to merge 2 commits into
NousResearch:mainfrom
necoweb3:fix/dashboard-managed-files-sensitive-dirs

Conversation

@necoweb3

@necoweb3 necoweb3 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

The dashboard managed-files API only filtered sensitive files by basename. That blocked .env and .env.*, but directory-scoped credential stores still remained browsable when the managed files root pointed at HERMES_HOME or a hosted /opt/data layout.

Affected examples:

  • mcp-tokens/<server>.json
  • mcp-tokens/<server>.client.json
  • pairing/*

Those directories can contain live OAuth/MCP bearer tokens or pairing secrets. If the dashboard file browser is configured with a managed root that includes Hermes state, /api/files, /api/files/read, and /api/files/download could still list/read/download files under those sensitive directories.

Changes

  • Add a path-aware managed-files sensitivity check.
  • Keep the existing .env basename behavior unchanged.
  • Hide sensitive credential directories from /api/files listings.
  • Block direct /api/files/read and /api/files/download access to files under those directories.
  • Add regression coverage for mcp-tokens/ and pairing/.

Related

This is complementary to #57833. That PR widens the basename denylist for files like auth.json, config.yaml, and .envrc. This PR covers the directory-scoped credential stores that cannot be represented by basename-only filtering.

Tests

python -m pytest tests/hermes_cli/test_web_server_files.py -k "sensitive" -q --timeout-method=thread
6 passed, 16 deselected

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #57833 (widens the /api/files/* sensitive-basename denylist) and #57869 (guards the /api/fs/* preview API). This PR handles the directory-scoped credential stores (mcp-tokens/, pairing/) that basename-only filtering can't represent -- complementary, not a duplicate. The three together give full /api/files/* + /api/fs/* credential coverage.

@prof-ramos

Copy link
Copy Markdown

Review — PR #57865 (fix(dashboard): block managed credential directories)

Veredito: Código correto, bem testado. Aprovo.

O que faz

Expande o filtro de segurança do dashboard file browser: antes só bloqueava arquivos .env pelo nome. Agora também bloqueia diretórios gerenciados inteiros (mcp-tokens/, pairing/) que contêm credenciais.

Pontos fortes

  • 3 endpoints cobertos: list, read, download — sem brecha
  • _is_sensitive_filename preservado como fallback dentro de _is_sensitive_managed_path
  • Teste de regressão com mcp-tokens/server.json e pairing/device.json

Observação menor (não blocker)

_is_sensitive_managed_path usa part.lower() só para arquivos via _is_sensitive_filename herdado, mas a checagem de diretórios contra _SENSITIVE_MANAGED_DIRNAMES é case-sensitive (part sem lower). Em ext4/Btrfs isso é seguro (o Hermes cria em lowercase), mas valeria normalizar com .lower() como defesa em profundidade.

@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have needs-repro Bug needs reproduction steps and removed area/auth Authentication, OAuth, credential pools P1 High — major feature broken, no workaround sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Priority corrected: P1 → P3 (triage calibration).

Assessed against the repo SECURITY.md: this is §3.2 out-of-scope hardening, not a
§3.1 boundary crossing. Blocks managed credential directories — a consequence of a chosen managed-root posture, not a boundary crossing.

The dashboard is loopback-by-default and its caller is the operator/agent already
inside the trust envelope, so an in-process guard over operator-owned files does not
cross an OS-isolation boundary or expose an unauthorized external surface. Per the
"PoC or GTFO" policy, type/security alone does not earn P0/P1.

Keeping type/security (this is valid low-priority hardening) and adding needs-repro.
To re-escalate: attach a reproducible exploit reachable by an unauthorized external
party
and state the in-scope class, i.e. security-scope: in-scope §3.1 <class> with the
attacker path. Without that, priority stays P3.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused credential-directory coverage. This is already implemented on current main by a broader follow-up, so this PR is redundant.

  • Automated hermes-sweeper review verified 8b24376d63c9aee031b3cf533884b749a645972a is an ancestor of current main and is contained in v2026.7.7.
  • hermes_cli/web_server.py:1382-1401 blocks mcp-tokens and pairing case-insensitively as path components.
  • hermes_cli/web_server.py:1922, 1949, and 1993 apply that guard to listing, read, and download.
  • tests/hermes_cli/test_web_server_files.py:626-680 covers directory descent, list/read/download denial, and a benign-directory positive control.

@teknium1 teknium1 closed this Jul 15, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/dashboard Web dashboard / control panel UI (dashboard/, landing) needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants