feat(skills): inventory and audit effective skill roots - #64244
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a deterministic “effective skills” inventory and extends hermes skills audit to optionally scan all effectively active skills across local roots, external roots, and symlinks. It also adjusts CLI exit-code behavior and raises the default hermes security audit --fail-on threshold to high so non-critical findings can still gate automation when desired.
Changes:
- Add
hermes skills inventory(table +--json) and new inventory implementation (hermes_cli/skills_inventory.py). - Extend
hermes skills auditwith--all-activeto scan effectively active skills across external roots and symlinks. - Raise default OSV audit failure threshold to
highand add parser/tests to confirm defaults.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/hermes_cli/test_skills_subparser.py | Adds parser coverage for skills inventory and skills audit --all-active. |
| tests/hermes_cli/test_skills_hub.py | Adds behavioral tests for inventory classification and all-active audit output. |
| tests/hermes_cli/test_security_audit.py | Adds coverage for the new default --fail-on high behavior and help text. |
| hermes_cli/subcommands/skills.py | Adds skills inventory subcommand and skills audit --all-active flag. |
| hermes_cli/subcommands/security.py | Updates security audit --fail-on default to high and help text. |
| hermes_cli/skills_inventory.py | New module implementing inventory collection + rendering/JSON output. |
| hermes_cli/skills_hub.py | Wires inventory, adds --all-active audit mode, and returns exit codes from skills_command. |
| hermes_cli/security_audit.py | Updates runtime default fail_on fallback from critical to high. |
| hermes_cli/main.py | Exits with the non-zero return code from skills_command. |
| .gitignore | Ignores .hardening-task.txt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _iter_status_skill_files(status_root: Path): | ||
| if not status_root.is_dir(): | ||
| return | ||
| for skill_md in sorted(status_root.rglob("SKILL.md")): | ||
| yield skill_md |
| targets = [e for e in installed if e["name"] == name] | ||
| if not targets: | ||
| c.print(f"[bold red]Error:[/] '{name}' is not a hub-installed skill.\n") | ||
| return | ||
| return 0 |
| report = collect_skill_inventory() | ||
| active_entries = [e for e in report["entries"] if e["active"]] | ||
| if name: | ||
| active_entries = [e for e in active_entries if e["name"] == name] | ||
| if not active_entries: | ||
| c.print(f"[bold red]Error:[/] '{name}' is not an active skill.\n") | ||
| return 1 | ||
|
|
||
| skipped = report["counts"]["total"] - len(active_entries) | ||
| c.print(f"\n[bold]Auditing {len(active_entries)} active skill(s)...[/]\n") |
alt-glitch
added
type/feature
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing an actual gap: current main audits only HubLockFile entries (hermes_cli/skills_hub.py:1074-1103) even though the runtime indexes configured external roots (agent/prompt_builder.py:1580-1613).
Problems
hermes_cli/skills_hub.py:1147-1148prints an error for a requested missing hub skill but returns0. The newcmd_skills()propagation athermes_cli/main.py:12779-12781means scripts still see success. Return a nonzero status and cover it.hermes_cli/skills_hub.py:1122supplieslocal/externalas the scanner source.tools/skills_guard.py:651-657,1102-1128derives trust from this value, so these labels becomecommunity; the existing hub path preserves the installed identifier athermes_cli/skills_hub.py:1161. Preserve available provenance or represent it as unknown rather than changing trust classification.- The new default
highathermes_cli/subcommands/security.py:42conflicts withwebsite/docs/reference/cli-commands.md:485, which documentscritical. The new skills commands also need documentation.
Suggested changes
- Add an exit-code regression test, preserve scan provenance, and update the relevant CLI/skills documentation.
Automated hermes-sweeper review.
| if not targets: | ||
| c.print(f"[bold red]Error:[/] '{name}' is not a hub-installed skill.\n") | ||
| return | ||
| return 0 |
There was a problem hiding this comment.
This is an explicit user error, but it returns 0. Since cmd_skills() now forwards nonzero results to the process exit status, automation invoking hermes skills audit missing-name will incorrectly report success; return 1 and add a regression test.
| if entry.get("symlinked"): | ||
| c.print(f"[dim] real path: {entry['real_path']}[/]") | ||
| try: | ||
| result = scan_skill(skill_path, source=entry.get("source", "active")) |
There was a problem hiding this comment.
scan_skill() derives trust from source; local and external resolve to community. That loses the HubLockFile identifier provenance used by the existing audit path and can misreport trusted or official installed skills. Preserve known provenance or explicitly model unknown provenance before scanning.
| audit_parser.add_argument( | ||
| "--fail-on", | ||
| default="critical", | ||
| default="high", |
There was a problem hiding this comment.
Please update website/docs/reference/cli-commands.md, which still documents critical as the default for hermes security audit --fail-on.
teknium1
added
sweeper:risk-compatibility
Summary
hermes skills inventorywith table and machine-readable JSON outputhermes skills audit --all-activeacross effective external roots while deduplicating symlink targetsWhy
Auditing only the stock profile directory misses effectively active skills supplied through external roots and symlinks. Operators need one deterministic inventory of the actual skill surface and an audit mode that scans each active target exactly once.
Verification
/Users/mudrii/.hermes/hermes-agent/venv/bin/python -m pytest -q tests/hermes_cli/test_security_audit.py tests/hermes_cli/test_skills_hub.py tests/hermes_cli/test_skills_subparser.pyruff checkpassedgit diff --checkpassed