fix(security): block agent-created skills when scan verdict requires confirmation

[dlkakbs]

What does this PR do?

The should_allow_install() policy table in skills_guard.py declares ask (returns None) for agent-created skills with a dangerous verdict, meaning the user must confirm before installation proceeds:

INSTALL_POLICY = {
    #                  safe      caution    dangerous
    "agent-created": ("allow",  "allow",   "ask"),   # dangerous → ask
}

The previous implementation in _security_scan_skill() silently logged a warning and returned None (allowed) when should_allow_install returned None, making the ask gate a complete no-op:

if allowed is None:
    # "ask" — allow but include the warning so the user sees the findings
    logger.warning("Agent-created skill has security findings: %s", reason)
    # Don't block — return None to allow, but log the warning
    return None  # ← silently allowed through

Security Impact

A prompt-injected agent (e.g. reading a malicious web page via web_extract or browser_navigate) could exploit this to write a persistent skill containing critical-severity patterns — secret exfiltration via curl, reading ~/.hermes/auth.json, etc. — and have it installed without any user confirmation. The skill would then execute in subsequent agent sessions.

Fix

When allowed is None, _security_scan_skill now returns the scan report as an error string, identical to the allowed is False path. The skill create/edit path already surfaces any non-None return as a failure to the caller, so no other changes are needed.

if allowed is None:
    report = format_scan_report(result)
    return (
        f"Security scan requires confirmation before installing this skill "
        f"({reason}):\n{report}\n"
        "To install anyway, delete the skill directory and recreate it after "
        "reviewing the findings above."
    )

Tests

Three new tests in TestSecurityScanAskPolicy:

• test_ask_verdict_is_blocked — allowed=None must return an error string, not None
• test_blocked_verdict_still_blocked — allowed=False path unaffected
• test_safe_verdict_passes — allowed=True still returns None (no error)

Related

• security: harden subprocess isolation, add exfiltration detection, fix YOLO bypass #4168 — broader subprocess isolation and exfiltration detection audit
• security: mark sticker descriptions as user-supplied content to prevent prompt injection #4268 — prompt injection via sticker descriptions (same attack surface category)

Type of Change

• 🔒 Security fix

Checklist

• Commit messages follow Conventional Commits
• Tests added for the fixed behavior
• No behavior change for safe or caution verdicts

[teknium1]

Thanks for the careful security analysis and the well-structured PR, @dlkakbs!

This is an automated hermes-sweeper review.

The bug you identified — allowed is None silently allowing dangerous agent-created skills through — was fixed on main independently before this PR could be merged:

• Commit ce089169d (Apr 23 2026) restored INSTALL_POLICY['agent-created'] = ('allow', 'allow', 'ask') in tools/skills_guard.py and updated _security_scan_skill() in tools/skill_manager_tool.py so the allowed is None branch now returns an error string (lines 87–93), identical in effect to what this PR proposes.
• The fix also adds an opt-in skills.guard_agent_created config flag (default false) so the scanner is only active when users explicitly enable it, reducing false-positive friction for prose-heavy skills.
• Behavioral test coverage for the dangerous-verdict-blocked case is present in TestSecurityScanGate.test_scan_blocks_dangerous_when_flag_on (tests/tools/test_skill_manager_tool.py line 526).

The security concern you raised is valid and was taken seriously — it directly informed the design of the opt-in gate. Closing as implemented on main.