mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(skill_manager): allow SKILL.md in _validate_file_path without weakening traversal guard (#40568)
Salvaged from #40453; cleaned up, re-verified against main, tests added. Co-authored-by: l37525778-coder <l37525778-coder@users.noreply.github.com>
This commit is contained in:
parent
c0424b06af
commit
5a36f76a00
2 changed files with 28 additions and 1 deletions
|
|
@ -179,6 +179,24 @@ class TestValidateFilePath:
|
|||
assert "File must be under one of:" in err
|
||||
assert "'malicious.py'" in err
|
||||
|
||||
def test_skill_md_accepted_at_root(self):
|
||||
# SKILL.md is the canonical skill file and must be accepted even
|
||||
# though it does not live under an allowed subdirectory.
|
||||
assert _validate_file_path("SKILL.md") is None
|
||||
|
||||
def test_skill_md_accepted_name_prefixed(self):
|
||||
assert _validate_file_path("my-skill/SKILL.md") is None
|
||||
|
||||
def test_skill_md_traversal_still_rejected(self):
|
||||
# The SKILL.md exception must not weaken the traversal guard.
|
||||
err = _validate_file_path("../SKILL.md")
|
||||
assert err == "Path traversal ('..') is not allowed."
|
||||
|
||||
def test_other_root_md_still_rejected(self):
|
||||
# Only SKILL.md gets the root-level exception, not arbitrary files.
|
||||
err = _validate_file_path("README.md")
|
||||
assert "File must be under one of:" in err
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# CRUD operations
|
||||
|
|
|
|||
|
|
@ -410,10 +410,19 @@ def _validate_file_path(file_path: str) -> Optional[str]:
|
|||
|
||||
normalized = Path(file_path)
|
||||
|
||||
# Prevent path traversal
|
||||
# Prevent path traversal (checked before any allow-listing so the SKILL.md
|
||||
# exception below can never be reached by a traversal-laden path).
|
||||
if has_traversal_component(file_path):
|
||||
return "Path traversal ('..') is not allowed."
|
||||
|
||||
# SKILL.md is the canonical skill file and lives at the skill root, not
|
||||
# under an allowed subdirectory. Accept its two natural spellings —
|
||||
# 'SKILL.md' and '<skill-name>/SKILL.md' — so callers can target the main
|
||||
# file. The traversal guard above still applies, so this can't escape.
|
||||
if normalized.parts and normalized.name == "SKILL.md":
|
||||
if len(normalized.parts) == 1 or len(normalized.parts) == 2:
|
||||
return None
|
||||
|
||||
# Must be under an allowed subdirectory
|
||||
if not normalized.parts or normalized.parts[0] not in ALLOWED_SUBDIRS:
|
||||
allowed = ", ".join(sorted(ALLOWED_SUBDIRS))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue