-
Notifications
You must be signed in to change notification settings - Fork 97
fix(sec-core): compose transform_llm_output hooks so skill-ledger warning is not dropped #2621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,46 @@ | |
| # If a single hook invocation exceeds this threshold (seconds), emit a warning. | ||
| _SLOW_HOOK_THRESHOLD = 2.0 | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # transform_llm_output composition | ||
| # | ||
| # Hermes applies transform_llm_output hooks with "first hook returning a | ||
| # non-empty string wins" semantics (see Hermes agent/turn_finalizer.py). | ||
| # Multiple agent-sec-core capabilities (pii-scan, prompt-scan, skill-ledger) | ||
| # each register a transform_llm_output hook; under "first-wins", a later | ||
| # capability's warning is silently dropped whenever an earlier capability also | ||
| # fires in the same turn — a security warning (e.g. "status=tampered") becomes | ||
| # invisible to the user. To avoid this, collect all capability transform | ||
| # callbacks here and register a single composed hook that chains them, so every | ||
| # capability's warning is prepended to the response. | ||
| # --------------------------------------------------------------------------- | ||
| _TRANSFORM_HOOKS: list[tuple[str, Any]] = [] | ||
|
|
||
|
|
||
| def _collect_transform_hook(capability_id: str, callback: Any) -> None: | ||
| """Called by AgentSecCoreCapability.register to collect (not register) | ||
| a capability's transform_llm_output callback for composition.""" | ||
| _TRANSFORM_HOOKS.append((capability_id, callback)) | ||
|
|
||
|
|
||
| def _compose_transform_chain(callbacks: list[tuple[str, Any]]): | ||
| """Return a single transform_llm_output callback that chains ``callbacks``. | ||
|
|
||
| Each callback receives the accumulated text as ``response_text`` and | ||
| prepends its own warnings, so the composed result carries every | ||
| capability's warning instead of only the first one to fire. | ||
| """ | ||
|
|
||
| def _chain(response_text: str = "", **kwargs: Any) -> str: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Hermes invokes AGENTS.md reference: src/agent-sec-core/AGENTS.md:L397-L405 Useful? React with 👍 / 👎. |
||
| current = response_text | ||
| for _capability_id, callback in callbacks: | ||
| result = callback(response_text=current, **kwargs) | ||
| if isinstance(result, str) and result: | ||
|
Comment on lines
+60
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] transform_llm_output 组合链未显式处理非字符串返回值 _chain 中仅在 isinstance(result, str) 且非空时更新 current,其余类型直接忽略。若未来某 capability transform 返回结构化对象或 None 表示特殊语义,现有实现会静默吞掉这类约定。建议在此明确约定:transform_llm_output 必须返回字符串,非字符串视为错误并记录日志,避免后续扩展产生难以发现的行为偏差。 🤖 Generated by Qoder • Fix in Qoder |
||
| current = result | ||
| return current | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When none of the collected capabilities emits a warning/redaction, Useful? React with 👍 / 👎. |
||
|
|
||
| return _chain | ||
|
|
||
|
|
||
| def load_config(plugin_dir: Path) -> dict[str, Any]: | ||
| """Load config.toml from the plugin directory. | ||
|
|
@@ -70,6 +110,8 @@ def register_capabilities( | |
| ctx, capabilities: list[AgentSecCoreCapability], config: dict | ||
| ) -> None: | ||
| """Register all enabled capabilities with the Hermes plugin context.""" | ||
| global _TRANSFORM_HOOKS | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] transform_llm_output 组合链对 _TRANSFORM_HOOKS 全局列表缺少并发安全说明 register_capabilities 通过全局 _TRANSFORM_HOOKS 收集能力回调,并在末尾清空。若 Hermes 插件未来支持并发 register_capabilities 调用或热重载能力,这种共享全局列表可能产生交叉污染。建议在此处增加显式约束:注册流程仅在初始化阶段串行执行,不支持并发/重入;如未来需要并发,需改为局部容器。 🤖 Generated by Qoder • Fix in Qoder |
||
| _TRANSFORM_HOOKS = [] | ||
| if "capabilities" not in config: | ||
| logger.error( | ||
| f"[agent-sec-core] config missing [capabilities] section, no capabilities registered" | ||
|
|
@@ -103,3 +145,15 @@ def register_capabilities( | |
| logger.info(f"[agent-sec-core] {cap.id} registered successfully") | ||
| except Exception as e: | ||
| logger.error(f"[agent-sec-core] {cap.id} registration failed: {e}") | ||
|
|
||
| # Register a single composed transform_llm_output hook that chains every | ||
| # capability's transform callback, so all security warnings (prompt-scan, | ||
| # pii-scan, skill-ledger) are preserved instead of one silently dropping | ||
| # another under Hermes' "first non-empty wins" semantics. | ||
| if _TRANSFORM_HOOKS: | ||
| composed = _compose_transform_chain(_TRANSFORM_HOOKS) | ||
| ctx.register_hook( | ||
| "transform_llm_output", | ||
| safe_hook_wrapper(composed, "transform-compose"), | ||
| ) | ||
| _TRANSFORM_HOOKS = [] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] transform_llm_output 组合链缺少顺序与幂等性约束说明
当前 _compose_transform_chain(callbacks) 直接依赖 callbacks 列表顺序,且在 register_capabilities 末尾用 "transform-compose" 统一包装。建议这里用注释明确:1)能力注册顺序即链执行顺序,后注册能力只能追加告警;2)transform-compose 自身不应在能力列表中重复注册,避免链上多次包装。
🤖 Generated by Qoder • Fix in Qoder