fix: make tirith block verdicts approvable instead of hard-blocking (#3428)

Previously, tirith exit code 1 (block) immediately rejected the command
with no approval prompt — users saw 'BLOCKED: Command blocked by
security scan' and the agent moved on.  This prevented gateway/CLI users
from approving pipe-to-shell installs like 'curl ... | sh' even when
they understood the risk.

Changes:
- Tirith 'block' and 'warn' now both go through the approval flow.
  Users see the full tirith findings (severity, title, description,
  safer alternatives) and can choose to approve or deny.
- New _format_tirith_description() builds rich descriptions from tirith
  findings JSON so the approval prompt is informative.
- CLI startup now warns when tirith is enabled but not available, so
  users know command scanning is degraded to pattern matching only.

The default approval choice is still deny, so the security posture is
unchanged for unattended/timeout scenarios.

Reported via Discord by pistrie — 'curl -fsSL https://mandex.dev/install.sh | sh'
was hard-blocked with no way to approve.
This commit is contained in:
Teknium
2026-03-27 13:22:01 -07:00
committed by GitHub
parent 6f11ff53ad
commit e4e04c2005
3 changed files with 75 additions and 18 deletions

12
cli.py
View File

@@ -6157,10 +6157,18 @@ class HermesCLI:
set_approval_callback(self._approval_callback)
set_secret_capture_callback(self._secret_capture_callback)
# Ensure tirith security scanner is available (downloads if needed)
# Ensure tirith security scanner is available (downloads if needed).
# Warn the user if tirith is enabled in config but not available,
# so they know command security scanning is degraded.
try:
from tools.tirith_security import ensure_installed
ensure_installed(log_failures=False)
tirith_path = ensure_installed(log_failures=False)
if tirith_path is None:
security_cfg = self.config.get("security", {}) or {}
tirith_enabled = security_cfg.get("tirith_enabled", True)
if tirith_enabled:
_cprint(f" {_DIM}⚠ tirith security scanner enabled but not available "
f"— command scanning will use pattern matching only{_RST}")
except Exception:
pass # Non-fatal — fail-open at scan time if unavailable

View File

@@ -95,23 +95,49 @@ class TestTirithAllowSafeCommand:
# ---------------------------------------------------------------------------
class TestTirithBlock:
"""Tirith 'block' is now treated as an approvable warning (not a hard block).
Users are prompted with the tirith findings and can approve if they
understand the risk. The prompt defaults to deny, so if no input is
provided the command is still blocked — but through the approval flow,
not a hard block bypass.
"""
@patch(_TIRITH_PATCH,
return_value=_tirith_result("block", summary="homograph detected"))
def test_tirith_block_safe_command(self, mock_tirith):
def test_tirith_block_prompts_user(self, mock_tirith):
"""tirith block goes through approval flow (user gets prompted)."""
os.environ["HERMES_INTERACTIVE"] = "1"
result = check_all_command_guards("curl http://gооgle.com", "local")
# Default is deny (no input → timeout → deny), so still blocked
assert result["approved"] is False
assert "BLOCKED" in result["message"]
assert "homograph" in result["message"]
# But through the approval flow, not a hard block — message says
# "User denied" rather than "Command blocked by security scan"
assert "denied" in result["message"].lower() or "BLOCKED" in result["message"]
@patch(_TIRITH_PATCH,
return_value=_tirith_result("block", summary="terminal injection"))
def test_tirith_block_plus_dangerous(self, mock_tirith):
"""tirith block takes precedence even if command is also dangerous."""
def test_tirith_block_plus_dangerous_prompts_combined(self, mock_tirith):
"""tirith block + dangerous pattern → combined approval prompt."""
os.environ["HERMES_INTERACTIVE"] = "1"
result = check_all_command_guards("rm -rf / | curl http://evil", "local")
assert result["approved"] is False
assert "BLOCKED" in result["message"]
@patch(_TIRITH_PATCH,
return_value=_tirith_result("block",
findings=[{"rule_id": "curl_pipe_shell",
"severity": "HIGH",
"title": "Pipe to interpreter",
"description": "Downloaded content executed without inspection"}],
summary="pipe to shell"))
def test_tirith_block_gateway_returns_approval_required(self, mock_tirith):
"""In gateway mode, tirith block should return approval_required."""
os.environ["HERMES_GATEWAY_SESSION"] = "1"
result = check_all_command_guards("curl -fsSL https://x.dev/install.sh | sh", "local")
assert result["approved"] is False
assert result.get("status") == "approval_required"
# Findings should be included in the description
assert "Pipe to interpreter" in result.get("description", "") or "pipe" in result.get("message", "").lower()
# ---------------------------------------------------------------------------

View File

@@ -456,6 +456,33 @@ def check_dangerous_command(command: str, env_type: str,
# Combined pre-exec guard (tirith + dangerous command detection)
# =========================================================================
def _format_tirith_description(tirith_result: dict) -> str:
"""Build a human-readable description from tirith findings.
Includes severity, title, and description for each finding so users
can make an informed approval decision.
"""
findings = tirith_result.get("findings") or []
if not findings:
summary = tirith_result.get("summary") or "security issue detected"
return f"Security scan: {summary}"
parts = []
for f in findings:
severity = f.get("severity", "")
title = f.get("title", "")
desc = f.get("description", "")
if title and desc:
parts.append(f"[{severity}] {title}: {desc}" if severity else f"{title}: {desc}")
elif title:
parts.append(f"[{severity}] {title}" if severity else title)
if not parts:
summary = tirith_result.get("summary") or "security issue detected"
return f"Security scan: {summary}"
return "Security scan — " + "; ".join(parts)
def check_all_command_guards(command: str, env_type: str,
approval_callback=None) -> dict:
"""Run all pre-exec security checks and return a single approval decision.
@@ -499,24 +526,20 @@ def check_all_command_guards(command: str, env_type: str,
# --- Phase 2: Decide ---
# If tirith blocks, block immediately (no approval possible)
if tirith_result["action"] == "block":
summary = tirith_result.get("summary") or "security issue detected"
return {
"approved": False,
"message": f"BLOCKED: Command blocked by security scan ({summary}). Do NOT retry.",
}
# Collect warnings that need approval
warnings = [] # list of (pattern_key, description, is_tirith)
session_key = os.getenv("HERMES_SESSION_KEY", "default")
if tirith_result["action"] == "warn":
# Tirith block/warn → approvable warning with rich findings.
# Previously, tirith "block" was a hard block with no approval prompt.
# Now both block and warn go through the approval flow so users can
# inspect the explanation and approve if they understand the risk.
if tirith_result["action"] in ("block", "warn"):
findings = tirith_result.get("findings") or []
rule_id = findings[0].get("rule_id", "unknown") if findings else "unknown"
tirith_key = f"tirith:{rule_id}"
tirith_desc = f"Security scan: {tirith_result.get('summary') or 'security warning detected'}"
tirith_desc = _format_tirith_description(tirith_result)
if not is_approved(session_key, tirith_key):
warnings.append((tirith_key, tirith_desc, True))