# NousResearch/hermes-agent · Cron absolute skill path security check correction ## Open now - Nothing open on GitHub. A bug caused the cron scheduler to silently produce no Discord delivery when absolute skill paths were rejected by a path-security check in skill_view. Multiple correction attempts address normalizing or permitting absolute paths that resolve within trusted directories. ## TL;DR - 5 member(s): 0 open issue(s), 1 closed issue(s), 0 open PR(s), 1 merged PR(s), 3 PR(s) closed without merging. - A fix was merged to main in PR #60109. Verify it addresses your case. - **Recommended:** read PR #60109 (and any competing PRs below) before filing. Confirm the merged change covers your symptom. ## Maintainer resolution verdict - Internal-tracker verdict over this cluster's issue members: `resolved` · deciding PR(s): `pr-60109`. This is distinct from the fix status above. Weigh it yourself. ## How this cluster was grouped - Link evidence (`STRUCTURAL_SPANNING`): every member is connected by structural evidence: shared changed files, or a closing/superseded reference. - Pairwise link density `0.40`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked. ## Closed / duplicate issues - #59824: \[Bug\] Cron scheduler silently fails (no Discord delivery) — absolute skill paths rejected by skill_view path-security check ## Prior & competing PRs - **#60109: fix(cron): normalize absolute skill paths before skill_view (#59824)**. merged to main · fix tier `verified` - maintainer direction: category `merged`; implication `superseded` - files: `agent/skill_commands.py`, `agent/skill_utils.py`, `cron/scheduler.py`, `tests/agent/test_skill_utils.py`, `tests/cron/test_scheduler.py` - fix hunks: - `agent/skill_commands.py` ```diff @@ -143,37 +143,9 @@ def _load_skill_payload(skill_identifier: str, task_id: str | None = None) -> tu try: from tools.skills_tool import SKILLS_DIR, skill_view - from agent.skill_utils import get_external_skills_dirs - - identifier_path = Path(raw_identifier).expanduser() - if identifier_path.is_absolute(): - normalized = None - trusted_roots = [SKILLS_DIR] - try: - trusted_roots.extend(get_external_skills_dirs()) - except Exception: - pass - - # Prefer the lexical path under a trusted skill root before - # resolving symlinks. Slash-command discovery can legitimately - # find a skill via ~/.hermes/skills/ where is a - # symlink to a checked-out skill elsewhere. Resolving first turns - # that trusted visible path into an arbitrary absolute path that - # skill_view() refuses to load. - for root in trusted_roots: - try: - normalized = str(identifier_path.relative_to(root)) - break - except ValueError: - continue + from agent.skill_utils import normalize_skill_lookup_name - if normalized is None: - try: - normalized = str(identifier_path.resolve().relative_to(SKILLS_DIR.resolve())) - except Exception: - normalized = raw_identifier - else: - normalized = raw_identifier.lstrip("/") + normalized = normalize_skill_lookup_name(raw_identifier) loaded_skill = json.loads( skill_view(normalized, task_id=task_id, preprocess=False) ``` - `agent/skill_utils.py` ```diff @@ -507,6 +507,63 @@ def get_all_skills_dirs() -> List[Path]: return dirs +def normalize_skill_lookup_name(identifier: str) -> str: + """Normalize a skill identifier to a ``skill_view()``-safe relative path. + + Slash commands and cron jobs may store absolute paths to skills that live + under ``~/.hermes/skills/`` (including via symlinks) or configured + ``skills.external_dirs``. ``skill_view()`` rejects absolute names for + security, so callers must translate trusted absolute paths to their + relative form first. + """ + raw_identifier = (identifier or "").strip() + if not raw_identifier: + return raw_identifier + + identifier_path = Path(raw_identifier).expanduser() + if not identifier_path.is_absolute(): + return raw_identifier.lstrip("/") + + # Look the primary skills root up on tools.skills_tool at CALL time + # (not via get_skills_dir()): callers and tests patch + # ``tools.skills_tool.SKILLS_DIR`` and skill_view() itself resolves + # against that module attribute, so normalization must agree with the + # exact root skill_view() will enforce. Import deferred to avoid a + # module cycle (tools.skills_tool imports agent.skill_utils). + try: + from tools import skills_tool as _skills_tool + primary_root = Path(_skills_tool.SKILLS_DIR) + except Exception: + primary_root = get_skills_dir() + + trusted_roots = [primary_root] + try: + trusted_roots.extend(get_external_skills_dirs()) + except Exception: + pass + + # Prefer the lexical path under a trusted skill root before resolving + # symlinks. Slash-command discovery can legitimately find a skill via + # ~/.hermes/skills/ where is a symlink to a checked-out + # skill elsewhere. Resolving first turns that trusted visible path into + # an arbitrary absolute path that skill_view() refuses to load. + for root in trusted_roots: + try: + return str(identifier_path.relative_to(root)) + except ValueError: + continue + + try: + return str(identifier_path.resolve().relative_to(primary_root.resolve())) + except Exception: + logger.debug( + "Skill identifier %r is an absolute path outside trusted skills " + "roots — passing through unchanged (skill_view will reject it)", + raw_identifier, + ) + return raw_identifier + + def _resolve_for_skill_ownership(path) -> Path: path_obj = path if isinstance(path, Path) else Path(str(path)) try: ``` - `cron/scheduler.py` ```diff @@ -2195,6 +2195,7 @@ def _build_job_prompt(job: dict, prerun_script: Optional[tuple] = None) -> str: from tools.skills_tool import skill_view from tools.skill_usage import bump_use from agent.skill_bundles import build_bundle_invocation_message, resolve_bundle_command_key + from agent.skill_utils import normalize_skill_lookup_name parts = [] skipped: list[str] = [] ``` ```diff @@ -2225,7 +2226,7 @@ def _build_job_prompt(job: dict, prerun_script: Optional[tuple] = None) -> str: continue try: - loaded = json.loads(skill_view(skill_name)) + loaded = json.loads(skill_view(normalize_skill_lookup_name(skill_name))) except (json.JSONDecodeError, TypeError): logger.warning("Cron job '%s': skill '%s' returned invalid JSON, skipping", job.get("name", job.get("id")), skill_name) skipped.append(skill_name) ``` - `tests/agent/test_skill_utils.py` ```diff @@ -333,3 +333,45 @@ def test_macos_skill_on_real_macos_unaffected(self): "agent.skill_utils.is_termux", return_value=False ): assert skill_matches_platform(fm) is True + + +class TestNormalizeSkillLookupName: + def test_relative_path_unchanged(self, tmp_path, monkeypatch): + from agent.skill_utils import normalize_skill_lookup_name + + # Relative identifiers early-return before any root lookup. + assert normalize_skill_lookup_name("foo/bar") == "foo/bar" + + def test_absolute_under_skills_dir_becomes_relative(self, tmp_path, monkeypatch): + from agent.skill_utils import normalize_skill_lookup_name + + skills_dir = tmp_path / "skills" + skill_dir = skills_dir / "category" / "my-skill" + skill_dir.mkdir(parents=True) + # Patch the root skill_view() itself enforces — normalization reads + # tools.skills_tool.SKILLS_DIR at call time so the two stay in sync. + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", skills_dir) + assert normalize_skill_lookup_name(str(skill_dir)) == "category/my-skill" + + def test_absolute_via_symlink_uses_lexical_relative_path(self, tmp_path, monkeypatch): + from agent.skill_utils import normalize_skill_lookup_name + + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + external = tmp_path / "external" / "my-skill" + external.mkdir(parents=True) + link = skills_dir / "my-skill" + try: + link.symlink_to(external) + except OSError: + pytest.skip("Symlinks not supported") + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", skills_dir) + assert normalize_skill_lookup_name(str(link)) == "my-skill" + + def test_untrusted_absolute_returned_unchanged(self, tmp_path, monkeypatch): + from agent.skill_utils import normalize_skill_lookup_name + + monkeypatch.setattr("tools.skills_tool.SKILLS_DIR", tmp_path / "skills") + monkeypatch.setattr("agent.skill_utils.get_skills_dir", lambda: tmp_path / "skills") + outside = str(tmp_path / "outside" / "skill") + assert normalize_skill_lookup_name(outside) == outside ``` - `tests/cron/test_scheduler.py` ```diff @@ -2885,6 +2885,31 @@ def _mixed_skill_view(name: str) -> str: assert "go" in result +class TestBuildJobPromptAbsoluteSkillPath: + """Cron jobs may store absolute skill paths; normalize before skill_view.""" + + def test_absolute_skill_path_normalized_before_skill_view(self, tmp_path): + skills_dir = tmp_path / "skills" + skill_dir = skills_dir / "alpha-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# Alpha\nDo alpha.") + absolute_path = str(skill_dir) + seen_names: list[str] = [] + + def _skill_view(name: str) -> str: + seen_names.append(name) + if name == "alpha-skill": + return json.dumps({"success": True, "content": "# Alpha\nDo alpha."}) + return json.dumps({"success": False, "error": f"Skill '{name}' not found."}) + + with patch("tools.skills_tool.SKILLS_DIR", skills_dir), \ + patch("tools.skills_tool.skill_view", side_effect=_skill_view): + result = _build_job_prompt({"skills": [absolute_path], "prompt": "go"}) + + assert seen_names == ["alpha-skill"] + assert "Do alpha." in result + + class TestBuildJobPromptBumpUse: """Verify that cron jobs bump skill usage counters so the curator sees them as active.""" ``` - **#59829: fix(cron): normalize absolute skill paths before skill_view (#59824)**. closed without merge - maintainer direction: category `merged_via`; implication `superseded`; refs `#60109` - **#59966: fix(skills-tool): accept absolute skill paths under trusted HERMES skills roots**. closed without merge - maintainer direction: category `silent_close`; implication `active` - **#60195: fix(skills): allow absolute skill paths that resolve within trusted directories (#59824)**. closed without merge - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `62972060caaa9f7f3fc5a58688757fdd59b9a117`, `#59824` ## Fix status - Merged to main in PR #60109 (cited above). No release data is tracked. Verify downstream. - Fix last verified (merged) `2026-07-07`.