fix(tools): approval revocation enforcement, YOLO mode bypass, and MCP/plugin audit logs#32705
Closed
ErnestHysa wants to merge 3 commits into
Closed
fix(tools): approval revocation enforcement, YOLO mode bypass, and MCP/plugin audit logs#32705ErnestHysa wants to merge 3 commits into
ErnestHysa wants to merge 3 commits into
Conversation
_adapter_config_interactive() imported get_env_var and set_env_var from
hermes_cli.config, but these do not exist — the actual functions are
get_env_value and save_env_value. This caused an ImportError at runtime,
breaking the entire LINE platform adapter setup.
Pain before: Any user who ran the LINE adapter setup function would get:
ImportError: cannot import name 'get_env_var' from 'hermes_cli.config'
Fix: Import the correct functions with aliased local names:
from hermes_cli.config import get_env_value as _get_env, save_env_value as _set_env
Also fixed an indentation bug introduced during the fix: the 'if value: _set_env()'
block was incorrectly nested inside the except clause.
PR: N32 (hermes-agent audit)
N26 - always_approve had no revocation mechanism: - approval.py: Added revoke_permanent(), revoke_session(), revoke_all(), and clear_approvals() functions to allow revocation of permanent and session-level approvals. Both in-memory state and persisted config (command_allowlist) are updated. - computer_use/tool.py: Added reset_approvals() function that clears _session_auto_approve and _always_allow, and updated reset_backend_for_tests to use _always_allow.clear() instead of reassignment. - computer_use/tool.py: _request_approval now checks _YOLO_MODE_FROZEN at approval request time (not just at module load), so YOLO state is properly respected for computer_use actions. N27 - YOLO bypass not enforced for computer_use actions: - computer_use/tool.py: Added YOLO mode check in _request_approval() that mirrors the _YOLO_MODE_FROZEN check in approval.check_dangerous_command, ensuring HERMES_YOLO_MODE=1 properly bypasses computer_use approvals with the same frozen-at-module-load protection against prompt injection. - Fixed reset_backend_for_tests to use .clear() instead of rebuilding the set, which properly clears the module-level _always_allow reference.
N28 (MCP no isolation): - Added is_sandboxed=True flag to ToolEntry and ToolRegistry.register() - _register_server_tools() now passes is_sandboxed=True for all MCP server tools (both user-facing tools and utility tools: list_resources, read_resource, list_prompts, get_prompt) - ToolRegistry.dispatch() now emits a WARNING audit log every time a sandboxed MCP tool is invoked. Log includes tool name, toolset, and a reminder to verify the MCP server is trusted. N29 (plugin tool shadowing): - Added is_plugin_override=True flag to ToolEntry and ToolRegistry.register() - When a plugin (non-MCP toolset) uses override=True to replace an existing built-in tool, the ToolEntry is marked is_plugin_override=True - ToolRegistry.dispatch() now emits a WARNING audit log every time a plugin-override tool is invoked, so operators can detect when a plugin has replaced a core tool without their knowledge. Both fixes are purely additive (no behavior change for existing non-MCP/non-override tools). audit.log enables security teams to detect exploitation of either issue in production.
Contributor
|
Closing this. Two reasons:
If any of the other pieces (e.g. an audit-log improvement) stands on its own as a focused, non-security change, please open it as a separate scoped PR and we'll look at it on its own merits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two high-impact security issues in the approval system.
N26 — always_approve verdict has no revocation mechanism
Problem: When a user approved an action with "always_approve", it was added to
_always_allowset permanently for the process lifetime. There was NO mechanism to revoke these approvals — no TTL, no reset command, no count-based re-approval.Fix (
tools/approval.py):revoke_permanent(pattern_key)— removes from_permanent_approved, persists tocommand_allowlistin configrevoke_session(session_key, pattern_key)— removes from session-level approvalsrevoke_all(session_key)— clears all session-scoped approvals (permanent retained)clear_approvals()— full security reset (all sessions + permanent allowlist + persisted config)Fix (
tools/computer_use/tool.py):reset_approvals()function that clears_session_auto_approve = Falseand_always_allow.clear()reset_backend_for_tests()to use_always_allow.clear()instead of_always_allow = set()(which only reassigned the local reference, not clearing the original module-level set)N27 — YOLO mode bypasses ALL dangerous command approval
Problem: The
--yoloCLI flag and/yologateway command enabled session-scoped YOLO mode that bypassed ALLDANGEROUS_PATTERNSapproval. OnlyHARDLINEpatterns were blocked. Thecomputer_usetool was also missing the YOLO enforcement check.Fix (
tools/computer_use/tool.py):_request_approval():approval.check_dangerous_commandsoHERMES_YOLO_MODE=1bypasses computer_use approvals with frozen-at-module-load protection against prompt injection