fix(approval,mcp): log silent exception handlers, narrow OAuth catches, close server on error

Three silent `except Exception` blocks in approval.py (lines 345, 387, 469) return
fallback values with zero logging — making it impossible to debug callback failures,
allowlist load errors, or config read issues.  Add logger.warning/error calls that
match the pattern already used by save_permanent_allowlist() and _smart_approve()
in the same file.

In mcp_oauth.py, narrow the overly-broad `except Exception` in get_tokens() and
get_client_info() to the specific exceptions Pydantic's model_validate() can raise
(ValueError, TypeError, KeyError), and include the exception message in the warning.
Also wrap the _wait_for_callback() polling loop in try/finally so the HTTPServer is
always closed — previously an asyncio.CancelledError or any exception in the loop
would leak the server socket.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
aaronagent
2026-04-10 11:42:40 +08:00
committed by Teknium
parent 738f0bac13
commit 94f5979cc2
2 changed files with 18 additions and 14 deletions

View File

@@ -342,7 +342,8 @@ def load_permanent_allowlist() -> set:
if patterns: if patterns:
load_permanent(patterns) load_permanent(patterns)
return patterns return patterns
except Exception: except Exception as e:
logger.warning("Failed to load permanent allowlist: %s", e)
return set() return set()
@@ -384,7 +385,8 @@ def prompt_dangerous_approval(command: str, description: str,
try: try:
return approval_callback(command, description, return approval_callback(command, description,
allow_permanent=allow_permanent) allow_permanent=allow_permanent)
except Exception: except Exception as e:
logger.error("Approval callback failed: %s", e, exc_info=True)
return "deny" return "deny"
os.environ["HERMES_SPINNER_PAUSE"] = "1" os.environ["HERMES_SPINNER_PAUSE"] = "1"
@@ -466,7 +468,8 @@ def _get_approval_config() -> dict:
from hermes_cli.config import load_config from hermes_cli.config import load_config
config = load_config() config = load_config()
return config.get("approvals", {}) or {} return config.get("approvals", {}) or {}
except Exception: except Exception as e:
logger.warning("Failed to load approval config: %s", e)
return {} return {}

View File

@@ -198,8 +198,8 @@ class HermesTokenStorage:
return None return None
try: try:
return OAuthToken.model_validate(data) return OAuthToken.model_validate(data)
except Exception: except (ValueError, TypeError, KeyError) as exc:
logger.warning("Corrupt tokens at %s -- ignoring", self._tokens_path()) logger.warning("Corrupt tokens at %s -- ignoring: %s", self._tokens_path(), exc)
return None return None
async def set_tokens(self, tokens: "OAuthToken") -> None: async def set_tokens(self, tokens: "OAuthToken") -> None:
@@ -214,8 +214,8 @@ class HermesTokenStorage:
return None return None
try: try:
return OAuthClientInformationFull.model_validate(data) return OAuthClientInformationFull.model_validate(data)
except Exception: except (ValueError, TypeError, KeyError) as exc:
logger.warning("Corrupt client info at %s -- ignoring", self._client_info_path()) logger.warning("Corrupt client info at %s -- ignoring: %s", self._client_info_path(), exc)
return None return None
async def set_client_info(self, client_info: "OAuthClientInformationFull") -> None: async def set_client_info(self, client_info: "OAuthClientInformationFull") -> None:
@@ -343,13 +343,14 @@ async def _wait_for_callback() -> tuple[str, str | None]:
timeout = 300.0 timeout = 300.0
poll_interval = 0.5 poll_interval = 0.5
elapsed = 0.0 elapsed = 0.0
while elapsed < timeout: try:
if result["auth_code"] is not None or result["error"] is not None: while elapsed < timeout:
break if result["auth_code"] is not None or result["error"] is not None:
await asyncio.sleep(poll_interval) break
elapsed += poll_interval await asyncio.sleep(poll_interval)
elapsed += poll_interval
server.server_close() finally:
server.server_close()
if result["error"]: if result["error"]:
raise RuntimeError(f"OAuth authorization failed: {result['error']}") raise RuntimeError(f"OAuth authorization failed: {result['error']}")