fix(security): prevent path traversal in skill_view#242
Closed
Bartok9 wants to merge 1 commit into
Closed
Conversation
Fixes NousResearch#220 skill_view() accepted a file_path parameter without validating for path traversal. An LLM or prompt injection could read arbitrary files including ~/.hermes/.env (API keys) or ~/.ssh/id_rsa. Changes: - Add '..' component check matching skill_manager_tool.py pattern - Add resolve() containment check as defense in depth - Return structured JSON errors for blocked paths - Add comprehensive test coverage The fix uses two layers of protection: 1. Reject any path containing '..' in its components 2. Verify resolved path is within skill directory boundary This prevents both direct traversal (../../.env) and symlink-based escapes.
teknium1
added a commit
that referenced
this pull request
Mar 2, 2026
skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR #242 (@Bartok9). Vulnerability reported by @Farukest (#220, PR #221). Tests rewritten to properly mock SKILLS_DIR. Closes #220
This was referenced Mar 2, 2026
Contributor
|
Thanks @Bartok9! Your code fix was the most robust — the trailing The tests needed a small adjustment — they mocked Fixed in commit 1cb2311 with credit to you. |
angelburgosrosado
pushed a commit
to angelburgosrosado/hermes-agent
that referenced
this pull request
Apr 27, 2026
…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220
olympus-terminal
pushed a commit
to olympus-terminal/hermes-agent
that referenced
this pull request
May 16, 2026
…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220
Egavasyug
pushed a commit
to Egavasyug/hermes-agent
that referenced
this pull request
Jun 10, 2026
…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220
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
2 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
Fixes #220
skill_view()accepted afile_pathparameter without validating for path traversal. An LLM or prompt injection could read arbitrary files including~/.hermes/.env(API keys) or~/.ssh/id_rsa.Changes
..component check matching existingskill_manager_tool.pypatternresolve()containment check as defense in depthtests/tools/test_skills_tool_path_traversal.pySecurity
The fix uses two layers of protection:
..in its partsThis prevents both direct traversal (
../../.env) and symlink-based escapes.Testing
Related
Matches the existing validation pattern in
skill_manager_tool.pylines 177-178.