security(dashboard): widen managed-files credential guard past .env + close dir-tree gap - #58222
Merged
kshitijk4poor merged 2 commits intoJul 4, 2026
Conversation
…t .env _is_sensitive_filename() only blocked .env / .env.<suffix>, but the dashboard Files tab's managed root is operator-configurable and, per the docker-mount scenario NousResearch#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.
kshitijk4poor
force-pushed
the
salvage/dashboard-credential-guard
branch
from
July 4, 2026 11:10
17be186 to
8d80fc2
Compare
alt-glitch
added
type/security
Collaborator
Related: salvage of #57833 (@srojk34) plus a follow-up that closes the directory-tree gap a basename-only guard left open (mcp-tokens/, pairing/). Part of the dashboard credential-guard cluster: #57833 (basename widen), #57865 (credential dirs), #57869 (/api/fs preview), #58034 (managed-file mutations), #58036 (git review diffs). This PR supersedes the read/list/download basename portion of #57833 and adds the path-aware directory check — a human should pick which of the overlapping reads-side PRs to merge. |
tonydwb
reviewed
Jul 4, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Summary
Security fix that widens the managed-files credential guard to cover more credential file basenames (auth.json, credentials, .git-credentials, etc.) and adds directory-level guards for mcp-tokens and pairing directories. Previously, the guard only covered .env variants.
Looks Good
- Security improvement: covers more credential file types
- Directory-level matching prevents exposure of nested credential stores
- Consistent with agent.file_safety and gateway.platforms.base guards
- Clear comment explaining the design
- No security concerns in the implementation
Reviewed by Hermes Agent
…anaged-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/<server>.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 (NousResearch#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.
kshitijk4poor
force-pushed
the
salvage/dashboard-credential-guard
branch
from
July 4, 2026 12:28
8d80fc2 to
8b24376
Compare
kshitijk4poor
enabled auto-merge
July 4, 2026 12:28
4 tasks
habarmc1223-sudo
pushed a commit
to habarmc1223-sudo/hermes-agent-fluxmem
that referenced
this pull request
Jul 8, 2026
…board-credential-guard security(dashboard): widen managed-files credential guard past .env + close dir-tree gap
santhreal
pushed a commit
to santhreal/hermes-agent
that referenced
this pull request
Jul 13, 2026
…board-credential-guard security(dashboard): widen managed-files credential guard past .env + close dir-tree gap
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…board-credential-guard security(dashboard): widen managed-files credential guard past .env + close dir-tree gap
leewenjie
pushed a commit
to leewenjie/hermes-agent
that referenced
this pull request
Aug 7, 2026
…board-credential-guard security(dashboard): widen managed-files credential guard past .env + close dir-tree gap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Salvage of #57833 (@srojk34) onto current
main, plus a follow-up that closes the two gaps a basename-only guard left open.The dashboard managed-files API (
/api/fileslist/read/download) exposed credential stores when the operator points the managed root atHERMES_HOME(thedocker run -v ~/.hermes:/opt/datascenario from #57505). The guard only blocked.env/.env.*.Changes
_is_sensitive_filename()past.envto.envrc+ the canonical credential basenames (auth.json,.anthropic_oauth.json,google_token.json,webhook_subscriptions.json,bws_cache.json, etc.), case-insensitive.mcp-tokens/(live MCP OAuth tokens) andpairing/are denied as whole trees by both canonical guards (gateway.platforms.base._ROOT_CREDENTIAL_DIRS,agent.file_safety), but the files API descends into subdirs, somcp-tokens/<server>.json(a non-canonical basename) stayed exposed. Added_is_sensitive_path()— a path-aware check that blocks any path with a credential-directory component — and routed all three call sites (list/read/download) through it..git-credentialsto the basename set (agent.file_safetyblocks it too).Scope stays on the read/list/download exfil surface (#57505). The write endpoints (upload/mkdir/delete) are a separate threat class and out of scope for this PR.
Validation
auth.json,.envrc, etc. via Files tabmcp-tokens/<server>.jsonon subdir descent.git-credentialsnotes/todo.txt)tests/hermes_cli/test_web_server_files.py— 26 passed (23 original + new dir-tree,.git-credentials, and benign-subdir positive-control tests).test_web_server_files.py+test_web_server_fs.py— 39 passed._is_sensitive_pathmakes the new regression tests fail (guard genuinely enforced), restoring makes them pass.ruff checkclean.Closes #57833. Credit to @srojk34 — their commit is cherry-picked with authorship preserved.