Feature: optional HOL Guard SecurityScanner backend for skill publish scans

[kantorcodes]

Proposal

Add an optional HOL Guard-backed implementation of SkillHub's existing SecurityScanner interface for pre-trust skill/package scanning.

HOL Guard: https://github.com/hashgraph-online/hol-guard
Distribution/Agent Skill repo: https://github.com/hashgraph-online/hol-guard-plugin

Why this fits the current architecture

SkillHub already has the provider-neutral domain contract:

• SecurityScanner.scan(SecurityScanRequest)
• isHealthy()
• getScannerType()

and currently maps the built-in scanner through SkillScannerAdapter. The published scanner docs also describe multiple analysis engines and a configurable scanner service URL.

A narrow integration could preserve that contract and add a HolGuardSecurityScannerAdapter that invokes HOL Guard locally against the unpacked skill/package path, then maps structured findings/severity into SkillHub's existing SecurityScanResponse / SecurityFinding model.

Intended boundary

• local scanning; Guard Cloud not required
• do not execute target install/lifecycle scripts merely to scan them
• preserve SkillHub's existing review/publish semantics
• fail explicitly if the configured scanner is unavailable rather than treating that as a clean result
• map prompt-injection, unsafe-command, secret/exfiltration, plugin/MCP, and supply-chain findings into the existing response model
• add malicious and benign SKILL.md/plugin fixtures plus adapter health/error tests

This would be an additional backend, not a request to replace SkillHub's built-in scanner or its existing engines.

If this direction fits the project, we can contribute the smallest adapter/config/docs/tests PR against the existing SecurityScanner abstraction.

[github-actions]

Issue Triage

• Route: triage/needs-info
• Priority: priority/p0 (5.0/5)
• Effort: effort/m (3/5)
• Confidence: 2/5
• Risk: high
• Analysis Mode: rules-only

Why

• Touches core SkillHub workflows (publish, review, scanner).
• Contains urgency signals (security).
• Open for 15 days; the 14-day closure SLA is breached, so priority is forced to P0.
• Text suggests a bounded change (docs).
• Thread already has follow-up discussion, so backlog pressure is rising.

High-Risk Signals

• Mentions sensitive areas (security).

Next Action

• Wait for more detail, then rerun triage after the author edits the issue or comments /retriage. Missing: .

---

问题分流结果

• 路由: triage/needs-info
• 优先级: priority/p0 (5.0/5)
• 修复投入: effort/m (3/5)
• 信息完整度: 2/5
• 风险: 高
• 分析模式: rules-only

原因

• 涉及 SkillHub 核心流程（publish, review, scanner）
• 包含紧急信号（security）
• 已打开 15 天，超过 14 天闭环 SLA，优先级强制提升到 P0
• 文本显示改动范围较可控（docs）
• 已有后续讨论，积压压力在上升

高风险信号

• 提到敏感区域（security）

下一步

• 等待补充更多信息；作者更新 issue 或评论 /retriage 后重新分流

[kantorcodes]

Adding concrete implementation detail for triage.

Minimal code shape

I would keep the existing publish/review flow and SecurityScanner domain interface unchanged.

1. Provider selection in configuration

   • add a scanner provider setting under the existing skillhub.security.scanner namespace, defaulting to the current skill-scanner implementation so existing deployments do not change behavior;
   • wire exactly one SecurityScanner bean based on that provider.

2. HOL Guard adapter

   • add a small infra adapter implementing the existing SecurityScanner methods: scan(SecurityScanRequest), isHealthy(), and getScannerType();
   • input remains request.skillPackagePath() and request.skillVersionId();
   • return the existing SecurityScanResponse, SecurityVerdict, and SecurityFinding types, so downstream review/publish code does not need a new contract.

3. Execution boundary

   • preferred lowest-coupling option: run HOL Guard behind the same scanner-service/HTTP boundary SkillHub already uses, with a tiny sidecar translating plugin-scanner scan PATH --format json into the scanner response. That avoids adding arbitrary shell execution to SkillHub itself;
   • if maintainers prefer an in-process local adapter, use ProcessBuilder/argument arrays rather than shell interpolation, enforce a hard timeout/output cap, and inject the process runner so tests never need to execute a real target repository.

4. Failure semantics

   • scanner unavailable / timeout / malformed response => throw the same scanner failure path (SecurityScanException or equivalent) rather than mapping it to SAFE;
   • a scan with zero covered findings may map to the existing safe verdict, but the adapter should not claim a broader guarantee than the scanner result.

5. Finding mapping

   • map HOL Guard severity/category/file/line/title/description/remediation into the existing SecurityFinding fields;
   • keep SkillHub's existing CRITICAL/HIGH/MEDIUM verdict policy unless maintainers want provider-specific policy to stay outside the adapter.

Expected files

Likely scope would be limited to the existing scanner/config areas, e.g.:

• server/skillhub-infra/.../scanner/ for the adapter/client DTO mapping;
• server/skillhub-app/.../config/SkillScannerConfig.java or a sibling provider config for conditional bean selection;
• scanner adapter/config tests alongside the existing SkillScannerAdapterTest coverage;
• scanner docs/config example.

No changes should be required to the skill package format, upload API, review domain model, or publish workflow.

Test matrix

• benign SKILL.md / plugin fixture -> zero covered findings;
• prompt-injection fixture;
• hidden-Unicode fixture;
• unsafe command / exfiltration fixture;
• malicious plugin/MCP fixture;
• scanner unavailable;
• timeout;
• malformed/partial scanner response;
• path containing spaces/special characters to prove no shell interpolation.

If the project prefers the HTTP-sidecar shape versus a local process adapter, I can keep the first PR to that exact choice.

[kantorcodes]

/retriage

[XiaoSeS]

Maintainer design decision: keep this open as a deferred provider PoC, but do not add an in-process Java adapter that launches Python or pipx from the SkillHub Server.

The existing HTTP scanner boundary is the cleaner extension point. The preferred shape is an optional endpoint-compatible adapter image/service:

SkillHub Server -> existing /scan-upload and /health contract -> default scanner image or HOL Guard adapter image

The first deliverable should be a small PoC that:

• accepts the same stored skill bundle input used by the current scanner service;
• runs a pinned local HOL Guard/plugin-scanner version without Guard Cloud;
• maps its stable JSON output into the existing SecurityScanResponse contract;
• fails explicitly on unavailable, timeout, malformed, or partial output instead of returning SAFE;
• enforces timeout, output-size, concurrency, and process isolation in the adapter container;
• proves benign, prompt-injection, hidden-Unicode, unsafe-command/exfiltration, plugin/MCP, malformed-output, and unavailable cases;
• documents the exact supported HOL Guard version and Apache-2.0 redistribution obligations.

If the PoC can remain protocol-compatible, existing deployments can select it through the scanner service URL and the Server does not need Python dependencies or ProcessBuilder code. Core adoption should wait until the result schema, performance, and upgrade compatibility are demonstrated.

The Redis accumulation concern is provider-independent and is now tracked separately by #808. #801 prevents concurrent retry pile-ups, while #808 covers distinct tasks remaining pending indefinitely during a long outage.