mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
fix(curator): defense-in-depth gates against bundled/hub skills
Previous invariants only gated the primary entry points
(apply_automatic_transitions, archive_skill, CLI pin). Several paths
were unprotected:
- bump_view / bump_use / bump_patch / set_state / set_pinned wrote
usage records unconditionally, which is confusing noise in
.usage.json even though the review list filtered them out
- restore_skill did not check whether a bundled skill now shadows
the archived name
- CLI unpin was asymmetric with CLI pin — it had no gate
Fixes:
- _mutate() (the shared counter / state writer) now drops silently
when the skill is not agent-created. .usage.json never gains a
record for a bundled or hub-installed skill.
- restore_skill() refuses to restore under a name that is now
bundled or hub-installed (would shadow upstream).
- CLI unpin gate matches CLI pin.
New tests:
- 5 provenance-guard tests on skill_usage (one per mutator)
- 1 end-to-end test that hammers every mutator at a bundled skill
and a hub skill, asserts both are untouched on disk, and asserts
the sidecar stays clean
- 2 CLI tests proving pin/unpin refuse bundled skills symmetrically
64/64 tests passing (29 skill_usage + 27 curator + 8 new guards).
This commit is contained in:
@@ -151,6 +151,12 @@ def _cmd_pin(args) -> int:
|
||||
|
||||
def _cmd_unpin(args) -> int:
|
||||
from tools import skill_usage
|
||||
if not skill_usage.is_agent_created(args.skill):
|
||||
print(
|
||||
f"curator: '{args.skill}' is bundled or hub-installed — "
|
||||
"there's nothing to unpin (curator only tracks agent-created skills)"
|
||||
)
|
||||
return 1
|
||||
skill_usage.set_pinned(args.skill, False)
|
||||
print(f"curator: unpinned '{args.skill}'")
|
||||
return 0
|
||||
|
||||
@@ -393,3 +393,39 @@ def test_curator_does_not_instruct_model_to_pin():
|
||||
assert not any(l.strip().startswith("pin ") for l in lines), (
|
||||
f"Found a pin action line in:\n{decision_block}"
|
||||
)
|
||||
|
||||
|
||||
|
||||
def test_cli_unpin_refuses_bundled_skill(curator_env, capsys):
|
||||
"""hermes curator unpin must refuse bundled/hub skills too (matches pin)."""
|
||||
from hermes_cli import curator as cli
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
_write_skill(skills_dir, "ship-skill")
|
||||
(skills_dir / ".bundled_manifest").write_text(
|
||||
"ship-skill:abc\n", encoding="utf-8",
|
||||
)
|
||||
|
||||
class _A:
|
||||
skill = "ship-skill"
|
||||
|
||||
rc = cli._cmd_unpin(_A())
|
||||
captured = capsys.readouterr()
|
||||
assert rc == 1
|
||||
assert "bundled" in captured.out.lower() or "hub" in captured.out.lower()
|
||||
|
||||
|
||||
def test_cli_pin_refuses_bundled_skill(curator_env, capsys):
|
||||
from hermes_cli import curator as cli
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
_write_skill(skills_dir, "ship-skill")
|
||||
(skills_dir / ".bundled_manifest").write_text(
|
||||
"ship-skill:abc\n", encoding="utf-8",
|
||||
)
|
||||
|
||||
class _A:
|
||||
skill = "ship-skill"
|
||||
|
||||
rc = cli._cmd_pin(_A())
|
||||
captured = capsys.readouterr()
|
||||
assert rc == 1
|
||||
assert "bundled" in captured.out.lower() or "hub" in captured.out.lower()
|
||||
|
||||
@@ -364,3 +364,124 @@ def test_agent_created_report_excludes_bundled_and_hub(skills_home):
|
||||
assert "mine" in names
|
||||
assert "bundled" not in names
|
||||
assert "hubbed" not in names
|
||||
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Provenance guard — telemetry must not leak records for bundled/hub skills
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_bump_view_no_op_for_bundled_skill(skills_home):
|
||||
"""Telemetry bumps on bundled skills are dropped — the sidecar must stay
|
||||
focused on agent-created skills only."""
|
||||
from tools.skill_usage import bump_view, load_usage
|
||||
skills_dir = skills_home / "skills"
|
||||
(skills_dir / ".bundled_manifest").write_text(
|
||||
"ship-bundled:abc\n", encoding="utf-8",
|
||||
)
|
||||
|
||||
bump_view("ship-bundled")
|
||||
assert "ship-bundled" not in load_usage(), (
|
||||
"bundled skill leaked into .usage.json"
|
||||
)
|
||||
|
||||
|
||||
def test_bump_patch_no_op_for_hub_skill(skills_home):
|
||||
from tools.skill_usage import bump_patch, load_usage
|
||||
skills_dir = skills_home / "skills"
|
||||
hub = skills_dir / ".hub"
|
||||
hub.mkdir()
|
||||
(hub / "lock.json").write_text(
|
||||
json.dumps({"installed": {"from-hub": {}}}), encoding="utf-8",
|
||||
)
|
||||
|
||||
bump_patch("from-hub")
|
||||
assert "from-hub" not in load_usage()
|
||||
|
||||
|
||||
def test_bump_use_no_op_for_hub_skill(skills_home):
|
||||
from tools.skill_usage import bump_use, load_usage
|
||||
skills_dir = skills_home / "skills"
|
||||
hub = skills_dir / ".hub"
|
||||
hub.mkdir()
|
||||
(hub / "lock.json").write_text(
|
||||
json.dumps({"installed": {"from-hub": {}}}), encoding="utf-8",
|
||||
)
|
||||
|
||||
bump_use("from-hub")
|
||||
assert "from-hub" not in load_usage()
|
||||
|
||||
|
||||
def test_set_state_no_op_for_bundled_skill(skills_home):
|
||||
"""State transitions on bundled skills must not land in the sidecar."""
|
||||
from tools.skill_usage import set_state, load_usage, STATE_ARCHIVED
|
||||
skills_dir = skills_home / "skills"
|
||||
(skills_dir / ".bundled_manifest").write_text(
|
||||
"locked:abc\n", encoding="utf-8",
|
||||
)
|
||||
set_state("locked", STATE_ARCHIVED)
|
||||
assert "locked" not in load_usage()
|
||||
|
||||
|
||||
def test_restore_refuses_to_shadow_bundled_skill(skills_home):
|
||||
"""If a bundled skill now occupies the name, refuse to restore."""
|
||||
from tools.skill_usage import archive_skill, restore_skill
|
||||
skills_dir = skills_home / "skills"
|
||||
_write_skill(skills_dir, "shared-name")
|
||||
archive_skill("shared-name")
|
||||
|
||||
# Now a bundled skill appears with the same name
|
||||
(skills_dir / ".bundled_manifest").write_text(
|
||||
"shared-name:abc\n", encoding="utf-8",
|
||||
)
|
||||
_write_skill(skills_dir, "shared-name") # bundled install landed
|
||||
|
||||
ok, msg = restore_skill("shared-name")
|
||||
assert not ok
|
||||
assert "bundled" in msg.lower() or "shadow" in msg.lower()
|
||||
|
||||
|
||||
def test_end_to_end_no_code_path_mutates_bundled_skill(skills_home):
|
||||
"""The combined guarantee: no curator code path can archive, mark stale,
|
||||
set-state, or persist telemetry for a bundled or hub-installed skill."""
|
||||
from tools.skill_usage import (
|
||||
bump_view, bump_use, bump_patch, set_state, set_pinned,
|
||||
archive_skill, load_usage, STATE_STALE, STATE_ARCHIVED,
|
||||
)
|
||||
skills_dir = skills_home / "skills"
|
||||
_write_skill(skills_dir, "bundled-one")
|
||||
_write_skill(skills_dir, "hub-one")
|
||||
_write_skill(skills_dir, "mine")
|
||||
|
||||
(skills_dir / ".bundled_manifest").write_text(
|
||||
"bundled-one:abc\n", encoding="utf-8",
|
||||
)
|
||||
hub = skills_dir / ".hub"
|
||||
hub.mkdir()
|
||||
(hub / "lock.json").write_text(
|
||||
json.dumps({"installed": {"hub-one": {}}}), encoding="utf-8",
|
||||
)
|
||||
|
||||
# Hammer every mutator at the bundled/hub names
|
||||
for name in ("bundled-one", "hub-one"):
|
||||
bump_view(name)
|
||||
bump_use(name)
|
||||
bump_patch(name)
|
||||
set_state(name, STATE_STALE)
|
||||
set_state(name, STATE_ARCHIVED)
|
||||
set_pinned(name, True)
|
||||
ok, _msg = archive_skill(name)
|
||||
assert not ok, f"archive_skill(\"{name}\") should refuse"
|
||||
|
||||
# Sidecar must be clean of all three
|
||||
data = load_usage()
|
||||
assert "bundled-one" not in data
|
||||
assert "hub-one" not in data
|
||||
|
||||
# Directories must still be in place on disk
|
||||
assert (skills_dir / "bundled-one" / "SKILL.md").exists()
|
||||
assert (skills_dir / "hub-one" / "SKILL.md").exists()
|
||||
|
||||
# The agent-created skill can still be mutated normally
|
||||
bump_view("mine")
|
||||
assert load_usage()["mine"]["view_count"] == 1
|
||||
|
||||
@@ -239,10 +239,18 @@ def get_record(skill_name: str) -> Dict[str, Any]:
|
||||
|
||||
|
||||
def _mutate(skill_name: str, mutator) -> None:
|
||||
"""Load, apply *mutator(record)* in place, save. Best-effort."""
|
||||
"""Load, apply *mutator(record)* in place, save. Best-effort.
|
||||
|
||||
Bundled and hub-installed skills are NEVER recorded in the sidecar.
|
||||
This keeps .usage.json focused on agent-created skills (the only ones
|
||||
the curator considers) and prevents stale counters from hanging around
|
||||
for upstream-managed skills.
|
||||
"""
|
||||
if not skill_name:
|
||||
return
|
||||
try:
|
||||
if not is_agent_created(skill_name):
|
||||
return
|
||||
data = load_usage()
|
||||
rec = data.get(skill_name)
|
||||
if not isinstance(rec, dict):
|
||||
@@ -361,7 +369,18 @@ def archive_skill(skill_name: str) -> Tuple[bool, str]:
|
||||
|
||||
def restore_skill(skill_name: str) -> Tuple[bool, str]:
|
||||
"""Move an archived skill back to ~/.hermes/skills/. Restores to the flat
|
||||
top-level layout; original category nesting is NOT reconstructed."""
|
||||
top-level layout; original category nesting is NOT reconstructed.
|
||||
|
||||
Refuses to restore under a name that now collides with a bundled or
|
||||
hub-installed skill — that would shadow the upstream version.
|
||||
"""
|
||||
# If a bundled or hub skill has since been installed under the same
|
||||
# name, refuse to restore rather than shadow it.
|
||||
if not is_agent_created(skill_name):
|
||||
return False, (
|
||||
f"skill '{skill_name}' is now bundled or hub-installed; "
|
||||
"restore would shadow the upstream version"
|
||||
)
|
||||
archive_root = _archive_dir()
|
||||
if not archive_root.exists():
|
||||
return False, "no archive directory"
|
||||
|
||||
Reference in New Issue
Block a user