mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
feat: add 'View full command' option to dangerous command approval (#887)
When a dangerous command is detected and the user is prompted for approval, long commands are truncated (80 chars in fallback, 70 chars in the TUI). Users had no way to see the full command before deciding. This adds a 'View full command' option across all approval interfaces: - CLI fallback (tools/approval.py): [v]iew option in the prompt menu. Shows the full command and re-prompts for approval decision. - CLI TUI (cli.py): 'Show full command' choice in the arrow-key selection panel. Expands the command display in-place and removes the view option after use. - CLI callbacks (callbacks.py): 'view' choice added to the list when the command exceeds 70 characters. - Gateway (gateway/run.py): 'full', 'show', 'view' responses reveal the complete command while keeping the approval pending. Includes 7 new tests covering view-then-approve, view-then-deny, short command fallthrough, and double-view behavior. Closes community feedback about the 80-char cap on dangerous commands.
This commit is contained in:
19
cli.py
19
cli.py
@@ -3824,7 +3824,17 @@ class HermesCLI:
|
|||||||
selected = state["selected"]
|
selected = state["selected"]
|
||||||
choices = state["choices"]
|
choices = state["choices"]
|
||||||
if 0 <= selected < len(choices):
|
if 0 <= selected < len(choices):
|
||||||
state["response_queue"].put(choices[selected])
|
chosen = choices[selected]
|
||||||
|
if chosen == "view":
|
||||||
|
# Toggle full command display without closing the prompt
|
||||||
|
state["show_full"] = True
|
||||||
|
# Remove the "view" option since it's been used
|
||||||
|
state["choices"] = [c for c in choices if c != "view"]
|
||||||
|
if state["selected"] >= len(state["choices"]):
|
||||||
|
state["selected"] = len(state["choices"]) - 1
|
||||||
|
event.app.invalidate()
|
||||||
|
return
|
||||||
|
state["response_queue"].put(chosen)
|
||||||
self._approval_state = None
|
self._approval_state = None
|
||||||
event.app.invalidate()
|
event.app.invalidate()
|
||||||
return
|
return
|
||||||
@@ -4372,13 +4382,18 @@ class HermesCLI:
|
|||||||
description = state["description"]
|
description = state["description"]
|
||||||
choices = state["choices"]
|
choices = state["choices"]
|
||||||
selected = state.get("selected", 0)
|
selected = state.get("selected", 0)
|
||||||
|
show_full = state.get("show_full", False)
|
||||||
|
|
||||||
cmd_display = command[:70] + '...' if len(command) > 70 else command
|
if show_full or len(command) <= 70:
|
||||||
|
cmd_display = command
|
||||||
|
else:
|
||||||
|
cmd_display = command[:70] + '...'
|
||||||
choice_labels = {
|
choice_labels = {
|
||||||
"once": "Allow once",
|
"once": "Allow once",
|
||||||
"session": "Allow for this session",
|
"session": "Allow for this session",
|
||||||
"always": "Add to permanent allowlist",
|
"always": "Add to permanent allowlist",
|
||||||
"deny": "Deny",
|
"deny": "Deny",
|
||||||
|
"view": "Show full command",
|
||||||
}
|
}
|
||||||
preview_lines = _wrap_panel_text(description, 60)
|
preview_lines = _wrap_panel_text(description, 60)
|
||||||
preview_lines.extend(_wrap_panel_text(cmd_display, 60))
|
preview_lines.extend(_wrap_panel_text(cmd_display, 60))
|
||||||
|
|||||||
@@ -988,6 +988,10 @@ class GatewayRunner:
|
|||||||
elif user_text in ("no", "n", "deny", "cancel", "nope"):
|
elif user_text in ("no", "n", "deny", "cancel", "nope"):
|
||||||
self._pending_approvals.pop(session_key_preview)
|
self._pending_approvals.pop(session_key_preview)
|
||||||
return "❌ Command denied."
|
return "❌ Command denied."
|
||||||
|
elif user_text in ("full", "show", "view", "show full", "view full"):
|
||||||
|
# Show full command without consuming the approval
|
||||||
|
cmd = self._pending_approvals[session_key_preview]["command"]
|
||||||
|
return f"Full command:\n\n```\n{cmd}\n```\n\nReply yes/no to approve or deny."
|
||||||
# If it's not clearly an approval/denial, fall through to normal processing
|
# If it's not clearly an approval/denial, fall through to normal processing
|
||||||
|
|
||||||
# Get or create session
|
# Get or create session
|
||||||
|
|||||||
@@ -105,10 +105,14 @@ def approval_callback(cli, command: str, description: str) -> str:
|
|||||||
"""Prompt for dangerous command approval through the TUI.
|
"""Prompt for dangerous command approval through the TUI.
|
||||||
|
|
||||||
Shows a selection UI with choices: once / session / always / deny.
|
Shows a selection UI with choices: once / session / always / deny.
|
||||||
|
When the command is longer than 70 characters, a "view" option is
|
||||||
|
included so the user can reveal the full text before deciding.
|
||||||
"""
|
"""
|
||||||
timeout = 60
|
timeout = 60
|
||||||
response_queue = queue.Queue()
|
response_queue = queue.Queue()
|
||||||
choices = ["once", "session", "always", "deny"]
|
choices = ["once", "session", "always", "deny"]
|
||||||
|
if len(command) > 70:
|
||||||
|
choices.append("view")
|
||||||
|
|
||||||
cli._approval_state = {
|
cli._approval_state = {
|
||||||
"command": command,
|
"command": command,
|
||||||
|
|||||||
@@ -1,5 +1,7 @@
|
|||||||
"""Tests for the dangerous command approval module."""
|
"""Tests for the dangerous command approval module."""
|
||||||
|
|
||||||
|
from unittest.mock import patch as mock_patch
|
||||||
|
|
||||||
from tools.approval import (
|
from tools.approval import (
|
||||||
approve_session,
|
approve_session,
|
||||||
clear_session,
|
clear_session,
|
||||||
@@ -7,6 +9,7 @@ from tools.approval import (
|
|||||||
has_pending,
|
has_pending,
|
||||||
is_approved,
|
is_approved,
|
||||||
pop_pending,
|
pop_pending,
|
||||||
|
prompt_dangerous_approval,
|
||||||
submit_pending,
|
submit_pending,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -338,3 +341,63 @@ class TestFindExecFullPathRm:
|
|||||||
assert dangerous is False
|
assert dangerous is False
|
||||||
assert key is None
|
assert key is None
|
||||||
|
|
||||||
|
|
||||||
|
class TestViewFullCommand:
|
||||||
|
"""Tests for the 'view full command' option in prompt_dangerous_approval."""
|
||||||
|
|
||||||
|
def test_view_then_once_fallback(self):
|
||||||
|
"""Pressing 'v' shows the full command, then 'o' approves once."""
|
||||||
|
long_cmd = "rm -rf " + "a" * 200
|
||||||
|
inputs = iter(["v", "o"])
|
||||||
|
with mock_patch("builtins.input", side_effect=inputs):
|
||||||
|
result = prompt_dangerous_approval(long_cmd, "recursive delete")
|
||||||
|
assert result == "once"
|
||||||
|
|
||||||
|
def test_view_then_deny_fallback(self):
|
||||||
|
"""Pressing 'v' shows the full command, then 'd' denies."""
|
||||||
|
long_cmd = "rm -rf " + "b" * 200
|
||||||
|
inputs = iter(["v", "d"])
|
||||||
|
with mock_patch("builtins.input", side_effect=inputs):
|
||||||
|
result = prompt_dangerous_approval(long_cmd, "recursive delete")
|
||||||
|
assert result == "deny"
|
||||||
|
|
||||||
|
def test_view_then_session_fallback(self):
|
||||||
|
"""Pressing 'v' shows the full command, then 's' approves for session."""
|
||||||
|
long_cmd = "rm -rf " + "c" * 200
|
||||||
|
inputs = iter(["v", "s"])
|
||||||
|
with mock_patch("builtins.input", side_effect=inputs):
|
||||||
|
result = prompt_dangerous_approval(long_cmd, "recursive delete")
|
||||||
|
assert result == "session"
|
||||||
|
|
||||||
|
def test_view_then_always_fallback(self):
|
||||||
|
"""Pressing 'v' shows the full command, then 'a' approves always."""
|
||||||
|
long_cmd = "rm -rf " + "d" * 200
|
||||||
|
inputs = iter(["v", "a"])
|
||||||
|
with mock_patch("builtins.input", side_effect=inputs):
|
||||||
|
result = prompt_dangerous_approval(long_cmd, "recursive delete")
|
||||||
|
assert result == "always"
|
||||||
|
|
||||||
|
def test_view_not_shown_for_short_command(self):
|
||||||
|
"""Short commands don't offer the view option; 'v' falls through to deny."""
|
||||||
|
short_cmd = "rm -rf /tmp"
|
||||||
|
with mock_patch("builtins.input", return_value="v"):
|
||||||
|
result = prompt_dangerous_approval(short_cmd, "recursive delete")
|
||||||
|
# 'v' is not a valid choice for short commands, should deny
|
||||||
|
assert result == "deny"
|
||||||
|
|
||||||
|
def test_once_without_view(self):
|
||||||
|
"""Directly pressing 'o' without viewing still works."""
|
||||||
|
long_cmd = "rm -rf " + "e" * 200
|
||||||
|
with mock_patch("builtins.input", return_value="o"):
|
||||||
|
result = prompt_dangerous_approval(long_cmd, "recursive delete")
|
||||||
|
assert result == "once"
|
||||||
|
|
||||||
|
def test_view_ignored_after_already_shown(self):
|
||||||
|
"""After viewing once, 'v' on a now-untruncated display falls through to deny."""
|
||||||
|
long_cmd = "rm -rf " + "f" * 200
|
||||||
|
inputs = iter(["v", "v"]) # second 'v' should not match since is_truncated is False
|
||||||
|
with mock_patch("builtins.input", side_effect=inputs):
|
||||||
|
result = prompt_dangerous_approval(long_cmd, "recursive delete")
|
||||||
|
# After first 'v', is_truncated becomes False, so second 'v' -> deny
|
||||||
|
assert result == "deny"
|
||||||
|
|
||||||
|
|||||||
@@ -184,43 +184,52 @@ def prompt_dangerous_approval(command: str, description: str,
|
|||||||
|
|
||||||
os.environ["HERMES_SPINNER_PAUSE"] = "1"
|
os.environ["HERMES_SPINNER_PAUSE"] = "1"
|
||||||
try:
|
try:
|
||||||
print()
|
is_truncated = len(command) > 80
|
||||||
print(f" ⚠️ DANGEROUS COMMAND: {description}")
|
while True:
|
||||||
print(f" {command[:80]}{'...' if len(command) > 80 else ''}")
|
print()
|
||||||
print()
|
print(f" ⚠️ DANGEROUS COMMAND: {description}")
|
||||||
print(f" [o]nce | [s]ession | [a]lways | [d]eny")
|
print(f" {command[:80]}{'...' if is_truncated else ''}")
|
||||||
print()
|
print()
|
||||||
sys.stdout.flush()
|
view_hint = " | [v]iew full" if is_truncated else ""
|
||||||
|
print(f" [o]nce | [s]ession | [a]lways | [d]eny{view_hint}")
|
||||||
|
print()
|
||||||
|
sys.stdout.flush()
|
||||||
|
|
||||||
result = {"choice": ""}
|
result = {"choice": ""}
|
||||||
|
|
||||||
def get_input():
|
def get_input():
|
||||||
try:
|
try:
|
||||||
result["choice"] = input(" Choice [o/s/a/D]: ").strip().lower()
|
result["choice"] = input(" Choice [o/s/a/D]: ").strip().lower()
|
||||||
except (EOFError, OSError):
|
except (EOFError, OSError):
|
||||||
result["choice"] = ""
|
result["choice"] = ""
|
||||||
|
|
||||||
thread = threading.Thread(target=get_input, daemon=True)
|
thread = threading.Thread(target=get_input, daemon=True)
|
||||||
thread.start()
|
thread.start()
|
||||||
thread.join(timeout=timeout_seconds)
|
thread.join(timeout=timeout_seconds)
|
||||||
|
|
||||||
if thread.is_alive():
|
if thread.is_alive():
|
||||||
print("\n ⏱ Timeout - denying command")
|
print("\n ⏱ Timeout - denying command")
|
||||||
return "deny"
|
return "deny"
|
||||||
|
|
||||||
choice = result["choice"]
|
choice = result["choice"]
|
||||||
if choice in ('o', 'once'):
|
if choice in ('v', 'view') and is_truncated:
|
||||||
print(" ✓ Allowed once")
|
print()
|
||||||
return "once"
|
print(" Full command:")
|
||||||
elif choice in ('s', 'session'):
|
print(f" {command}")
|
||||||
print(" ✓ Allowed for this session")
|
is_truncated = False # show full on next loop iteration too
|
||||||
return "session"
|
continue
|
||||||
elif choice in ('a', 'always'):
|
if choice in ('o', 'once'):
|
||||||
print(" ✓ Added to permanent allowlist")
|
print(" ✓ Allowed once")
|
||||||
return "always"
|
return "once"
|
||||||
else:
|
elif choice in ('s', 'session'):
|
||||||
print(" ✗ Denied")
|
print(" ✓ Allowed for this session")
|
||||||
return "deny"
|
return "session"
|
||||||
|
elif choice in ('a', 'always'):
|
||||||
|
print(" ✓ Added to permanent allowlist")
|
||||||
|
return "always"
|
||||||
|
else:
|
||||||
|
print(" ✗ Denied")
|
||||||
|
return "deny"
|
||||||
|
|
||||||
except (EOFError, KeyboardInterrupt):
|
except (EOFError, KeyboardInterrupt):
|
||||||
print("\n ✗ Cancelled")
|
print("\n ✗ Cancelled")
|
||||||
|
|||||||
Reference in New Issue
Block a user