fix(skills): trust-scope inline-shell auto-exec to non-community hub skills
Inline `!`cmd`` expansion in SKILL.md bodies ran for every skill whenever the global skills.inline_shell flag was on — a community hub skill and a builtin were expanded identically. The hub's install gate (INSTALL_POLICY) blocks a community install on the caution/dangerous verdicts the inline-shell DSL scans as, but --force or a pre-scanner install puts that skill on disk anyway, and the next view/slash/preload would auto-execute it on the host. Gate expansion at the single preprocess_skill_content choke point (all three surfaces route through it): community-trust hub lock entries never expand; trusted/builtin hub entries and anything without a hub lock entry (bundled- synced, user-created, project/external) keep the flag's contract. Lock read failures skip the gate, like every other provenance consumer. Fixes #63307
This commit is contained in:
@@ -95,6 +95,39 @@ def expand_inline_shell(content: str, skill_dir: Path | None, timeout: int) -> s
|
||||
return _INLINE_SHELL_RE.sub(_replace, content)
|
||||
|
||||
|
||||
def _is_community_hub_skill(skill_dir: Path | None) -> bool:
|
||||
"""Whether *skill_dir* is a hub-installed skill the scan gate classifies as community trust.
|
||||
|
||||
The hub's INSTALL_POLICY blocks a community install on a caution/dangerous verdict — but
|
||||
``--force`` (or a pre-scanner install) puts that skill on disk anyway, and the inline-shell
|
||||
DSL scans as high severity, so auto-executing it on view would re-arm exactly what the
|
||||
gate refused (#63307). Provenance is the hub lock entry (trusted/builtin entries expand;
|
||||
anything without one — bundled-synced, user-created, project/external — keeps the flag's
|
||||
contract). A lock read failure skips the gate, like every other provenance consumer.
|
||||
"""
|
||||
if skill_dir is None:
|
||||
return False
|
||||
try:
|
||||
from tools.skills_tool import _skills_dir
|
||||
from tools.skills_hub import HubLockFile
|
||||
installed = HubLockFile().load().get("installed") or {}
|
||||
for entry in installed.values():
|
||||
if not (isinstance(entry, dict) and entry.get("trust_level") == "community"):
|
||||
continue
|
||||
rel = str(entry.get("install_path") or "")
|
||||
if not rel:
|
||||
continue
|
||||
try:
|
||||
if skill_dir.resolve() == (_skills_dir() / rel).resolve():
|
||||
return True
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
return False
|
||||
except Exception:
|
||||
logger.debug("Could not read hub lock for inline-shell trust scoping", exc_info=True)
|
||||
return False
|
||||
|
||||
|
||||
def preprocess_skill_content(
|
||||
content: str,
|
||||
skill_dir: Path | None,
|
||||
@@ -107,6 +140,6 @@ def preprocess_skill_content(
|
||||
cfg = skills_cfg if isinstance(skills_cfg, dict) else load_skills_config()
|
||||
if cfg.get("template_vars", True):
|
||||
content = substitute_template_vars(content, skill_dir, session_id)
|
||||
if cfg.get("inline_shell", False):
|
||||
if cfg.get("inline_shell", False) and not _is_community_hub_skill(skill_dir):
|
||||
content = expand_inline_shell(content, skill_dir, int(cfg.get("inline_shell_timeout", 10) or 10))
|
||||
return content
|
||||
|
||||
@@ -1465,7 +1465,7 @@ DEFAULT_CONFIG = {
|
||||
# Substitute ${HERMES_SKILL_DIR} / ${HERMES_SESSION_ID} in SKILL.md content.
|
||||
"template_vars": True,
|
||||
# Pre-execute !`cmd` snippets in SKILL.md, inlining stdout (dates, git state...). Off:
|
||||
# skill-author content would run on the host unapproved — trusted sources only.
|
||||
# host-unapproved skill-author code; community hub installs never auto-execute (#63307).
|
||||
"inline_shell": False,
|
||||
"inline_shell_timeout": 10, # seconds per !`cmd` snippet
|
||||
# Security-scan skills the agent writes via skill_manage. Off: the agent can run the same
|
||||
|
||||
79
tests/agent/test_skill_inline_shell_trust.py
Normal file
79
tests/agent/test_skill_inline_shell_trust.py
Normal file
@@ -0,0 +1,79 @@
|
||||
"""Inline-shell expansion is trust-scoped (#63307): community hub installs never
|
||||
auto-execute `` !`cmd` `` snippets, mirroring the hub scan's INSTALL_POLICY trust
|
||||
gate — a ``--force`` or pre-scanner community install must not re-arm them."""
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent.skill_preprocessing import preprocess_skill_content
|
||||
|
||||
CFG = {"inline_shell": True, "inline_shell_timeout": 5}
|
||||
SNIPPET = "Dynamic: !`printf SENTINEL`"
|
||||
|
||||
|
||||
def _make_skill(skills_root, rel, body=SNIPPET):
|
||||
skill_dir = skills_root / rel
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
name = rel.rsplit("/", 1)[-1]
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
f"---\nname: {name}\ndescription: Description for {name}.\n---\n\n{body}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
return skill_dir
|
||||
|
||||
|
||||
def _record_hub_install(install_path: str, trust_level: str):
|
||||
"""Write a hub lock entry the way ``install_from_quarantine`` does."""
|
||||
from tools.skills_hub import HubLockFile
|
||||
|
||||
HubLockFile().record_install(
|
||||
name=install_path.rsplit("/", 1)[-1], source="github", identifier=f"o/r/{install_path}",
|
||||
trust_level=trust_level, scan_verdict="caution", skill_hash="sha256:0",
|
||||
install_path=install_path, files=["SKILL.md"])
|
||||
|
||||
|
||||
def test_community_hub_install_does_not_expand(tmp_path):
|
||||
"""A community-trust hub lock entry means the snippet must NOT run on view."""
|
||||
skill_dir = _make_skill(tmp_path, "mlops/hubskill")
|
||||
_record_hub_install("mlops/hubskill", "community")
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
rendered = preprocess_skill_content(SNIPPET, skill_dir, skills_cfg=dict(CFG))
|
||||
assert rendered == SNIPPET # raw DSL preserved — nothing executed
|
||||
|
||||
|
||||
def test_trusted_hub_install_still_expands(tmp_path):
|
||||
"""Trusted/builtin hub installs keep the operator's opt-in contract."""
|
||||
skill_dir = _make_skill(tmp_path, "mlops/hubskill")
|
||||
_record_hub_install("mlops/hubskill", "trusted")
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
rendered = preprocess_skill_content(SNIPPET, skill_dir, skills_cfg=dict(CFG))
|
||||
assert rendered == "Dynamic: SENTINEL"
|
||||
|
||||
|
||||
def test_local_skill_without_hub_entry_still_expands(tmp_path):
|
||||
"""Bundled-synced and user-created skills carry no lock entry and keep expanding."""
|
||||
skill_dir = _make_skill(tmp_path, "mlops/my-own")
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
rendered = preprocess_skill_content(SNIPPET, skill_dir, skills_cfg=dict(CFG))
|
||||
assert rendered == "Dynamic: SENTINEL"
|
||||
|
||||
|
||||
def test_slash_invocation_leaves_community_snippet_raw(tmp_path):
|
||||
"""The slash/bundle surface routes through the same gate: a community hub
|
||||
skill's message shows the raw snippet, never the executed output."""
|
||||
from agent.skill_commands import build_skill_invocation_message, scan_skill_commands
|
||||
|
||||
_make_skill(tmp_path, "dyn-comm")
|
||||
_record_hub_install("dyn-comm", "community")
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch("agent.skill_commands._load_skills_config",
|
||||
return_value={"template_vars": True, "inline_shell": True,
|
||||
"inline_shell_timeout": 5}),
|
||||
):
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/dyn-comm")
|
||||
|
||||
assert msg is not None
|
||||
# Executed output would be "Dynamic: SENTINEL"; the gate leaves the raw DSL.
|
||||
assert "Dynamic: SENTINEL" not in msg
|
||||
assert "!`printf SENTINEL`" in msg
|
||||
@@ -320,6 +320,8 @@ skills:
|
||||
|
||||
Snippets run with the skill directory as their working directory, and output is capped at 4000 characters. Failures (timeouts, non-zero exits) show up as a short `[inline-shell error: ...]` marker instead of breaking the whole skill.
|
||||
|
||||
Trust scoping: community hub installs never auto-execute snippets, even with the flag on — the hub's install scan blocks a community skill carrying this DSL on a caution/dangerous verdict, and a `--force` install (or one from before the scanner caught the pattern) must not re-arm what the gate refused. Bundled, trusted, and your own skills expand as before.
|
||||
|
||||
### Test It
|
||||
|
||||
Run the skill and verify the agent follows the instructions correctly:
|
||||
|
||||
Reference in New Issue
Block a user