fix: harden structured auth MCP errors against reviewer findings (#699)

Address PR #701 request-changes-class defects:

1. Fixed messages only — never embed HTTP bodies, Keychain, or exception
   text in tool results or daemon logs; sanitization fails closed.
2. Narrow Tool.run boundary wraps original success path; re-raises
   UrlElicitationRequiredError; install is idempotent.
3. No RuntimeError substring heuristics; only typed client failures
   are classified as auth/authz/network/config.
4. Central classify_http_status — every HTTP 403 is GiteaAuthzError.

Regressions cover adversarial secrets, stdio survival, elicitation,
parser RuntimeError, generic 403, repeated install, profiles, provenance.

Closes #699
This commit is contained in:
2026-07-13 13:40:38 -04:00
parent b4d0cb22e4
commit 6b675f5c83
6 changed files with 752 additions and 428 deletions
+141 -43
View File
@@ -245,40 +245,74 @@ def _redact(text):
# ── Classified client failures (#699) ───────────────────────────────────────── # ── Classified client failures (#699) ─────────────────────────────────────────
# Subclasses of RuntimeError preserve existing ``except RuntimeError`` call # Subclasses of RuntimeError preserve existing ``except RuntimeError`` call
# sites. The MCP tool-error boundary maps these to sanitized CallToolResult # sites. Exception *messages* are fixed constants only — HTTP response bodies,
# isError payloads so auth-class failures never terminate stdio transport. # Keychain material, and arbitrary exception text are never stored on the
# exception or re-emitted to tool results / daemon logs.
# Fixed messages (must match mcp_tool_error_boundary.FIXED_MESSAGES keys used here).
_MSG_AUTH_INVALID = "Gitea authentication failed: invalid or revoked credentials"
_MSG_AUTH_FAILED = "Gitea authentication failed"
_MSG_AUTHZ_SCOPE = "Gitea authorization failed: insufficient token scope"
_MSG_AUTHZ_DENIED = "Gitea authorization failed: access denied"
_MSG_NETWORK = "Network error contacting Gitea"
_MSG_CONFIG = "Gitea configuration or credential resolution failed"
_MSG_UPSTREAM = "Gitea upstream unavailable"
_MSG_HTTP = "Gitea HTTP request failed"
class GiteaClientError(RuntimeError): class GiteaClientError(RuntimeError):
"""Base for known Gitea client failures with a stable reason_code.""" """Base for known Gitea client failures with stable reason_code metadata."""
reason_code = "client_error" reason_code = "client_error"
error_class = "client" error_class = "client"
http_status = None http_status = None
def __init__(self, message, *, reason_code=None, http_status=None): def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(message)
if reason_code is not None: if reason_code is not None:
self.reason_code = reason_code self.reason_code = reason_code
if http_status is not None: if http_status is not None:
self.http_status = http_status self.http_status = http_status
# Message is always a fixed constant; callers cannot inject bodies.
fixed = message if message is not None else _MSG_HTTP
super().__init__(fixed)
class GiteaAuthError(GiteaClientError): class GiteaAuthError(GiteaClientError):
"""Authentication failure (invalid/revoked credentials → typically HTTP 401).""" """Authentication failure (invalid/revoked credentials → typically HTTP 401)."""
reason_code = "auth_failed" reason_code = "auth_invalid_token"
error_class = "authentication" error_class = "authentication"
http_status = 401 http_status = 401
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_AUTH_INVALID,
reason_code=reason_code or "auth_invalid_token",
http_status=http_status if http_status is not None else 401,
)
class GiteaAuthzError(GiteaClientError): class GiteaAuthzError(GiteaClientError):
"""Authorization / insufficient-scope failure (typically HTTP 403 + scope).""" """Authorization failure (HTTP 403 scope deficiency or access denied)."""
reason_code = "authz_insufficient_scope" reason_code = "authz_denied"
error_class = "authorization" error_class = "authorization"
http_status = 403 http_status = 403
def __init__(self, message=None, *, reason_code=None, http_status=None):
code = reason_code or "authz_denied"
if code == "authz_insufficient_scope":
fixed = _MSG_AUTHZ_SCOPE
else:
fixed = _MSG_AUTHZ_DENIED
code = "authz_denied"
super().__init__(
message if message is not None else fixed,
reason_code=code,
http_status=http_status if http_status is not None else 403,
)
class GiteaNetworkError(GiteaClientError): class GiteaNetworkError(GiteaClientError):
"""Transport / DNS / timeout failure contacting Gitea.""" """Transport / DNS / timeout failure contacting Gitea."""
@@ -287,6 +321,13 @@ class GiteaNetworkError(GiteaClientError):
error_class = "network" error_class = "network"
http_status = None http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_NETWORK,
reason_code=reason_code or "network_error",
http_status=http_status,
)
class GiteaConfigError(GiteaClientError): class GiteaConfigError(GiteaClientError):
"""Local configuration / credential resolution failure (not HTTP auth).""" """Local configuration / credential resolution failure (not HTTP auth)."""
@@ -295,9 +336,40 @@ class GiteaConfigError(GiteaClientError):
error_class = "configuration" error_class = "configuration"
http_status = None http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_CONFIG,
reason_code=reason_code or "config_error",
http_status=http_status,
)
class GiteaHttpError(GiteaClientError):
"""Non-auth HTTP failure with fixed message (no response body)."""
reason_code = "http_error"
error_class = "client"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
code = reason_code or "http_error"
if code == "upstream_unavailable":
fixed = _MSG_UPSTREAM
else:
fixed = _MSG_HTTP
code = "http_error"
super().__init__(
message if message is not None else fixed,
reason_code=code,
http_status=http_status,
)
def _looks_like_insufficient_scope(detail: str) -> bool: def _looks_like_insufficient_scope(detail: str) -> bool:
"""True when a 403 body indicates token scope deficiency, not generic deny.""" """Internal: inspect redacted body *only* to refine 403 reason_code.
The body is never stored on the exception or returned to callers.
"""
lower = (detail or "").lower() lower = (detail or "").lower()
markers = ( markers = (
"insufficient scope", "insufficient scope",
@@ -310,27 +382,51 @@ def _looks_like_insufficient_scope(detail: str) -> bool:
return any(m in lower for m in markers) return any(m in lower for m in markers)
def _raise_http_error(code: int, detail: str) -> None: def classify_http_status(code: int, *, body_hint: str = "") -> tuple[type, str, int]:
"""Raise a classified client error for a non-retryable HTTP failure.""" """Central HTTP status → (exception_class, reason_code, http_status).
safe = _redact(detail).strip()
Every HTTP 403 becomes authorization-class. Body text is used only as a
local hint for scope vs denied reason_code and is never returned.
"""
if code == 401: if code == 401:
msg = f"HTTP 401: {safe}" if safe else "HTTP 401: authentication failed" return (GiteaAuthError, "auth_invalid_token", 401)
raise GiteaAuthError( if code == 403:
msg, if _looks_like_insufficient_scope(body_hint or ""):
reason_code="auth_invalid_token", return (GiteaAuthzError, "authz_insufficient_scope", 403)
http_status=401, return (GiteaAuthzError, "authz_denied", 403)
)
if code == 403 and _looks_like_insufficient_scope(safe):
msg = f"HTTP 403: {safe}" if safe else "HTTP 403: insufficient scope"
raise GiteaAuthzError(
msg,
reason_code="authz_insufficient_scope",
http_status=403,
)
if code in (502, 503, 504): if code in (502, 503, 504):
msg = f"HTTP {code}: Gitea upstream unavailable" return (GiteaHttpError, "upstream_unavailable", code)
raise RuntimeError(f"{msg}: {safe}" if safe else msg) return (GiteaHttpError, "http_error", code)
raise RuntimeError(f"HTTP {code}: {safe}" if safe else f"HTTP {code}")
def raise_for_http_status(code: int, body: str = "") -> None:
"""Raise a typed client error for *code* without embedding *body*.
*body* may be inspected only to choose scope vs denied for 403; it is
never placed on the exception message.
"""
# Redact before any inspection; discard after classification.
try:
hint = _redact(body or "").strip()
except Exception:
hint = ""
exc_cls, reason, status = classify_http_status(code, body_hint=hint)
# Explicitly construct without passing body/hint into message.
if exc_cls is GiteaAuthError:
raise GiteaAuthError(reason_code=reason, http_status=status)
if exc_cls is GiteaAuthzError:
raise GiteaAuthzError(reason_code=reason, http_status=status)
if reason == "upstream_unavailable":
raise GiteaHttpError(
reason_code="upstream_unavailable",
http_status=status,
)
raise GiteaHttpError(reason_code="http_error", http_status=status)
def _raise_http_error(code: int, detail: str = "") -> None:
"""Backward-compatible alias — *detail* is never embedded in the error."""
raise_for_http_status(code, detail)
def _add_query(url, **params): def _add_query(url, **params):
@@ -412,14 +508,17 @@ def api_request(method, url, auth_header, payload=None, *,
using capped jittered exponential backoff. Successful responses are using capped jittered exponential backoff. Successful responses are
unchanged. unchanged.
All failures use a clear, secret-redacted message (no raw stack traces or Failures raise typed exceptions with **fixed messages only** (#699). HTTP
credential material). Classification (#699): response bodies are read solely for local 403 reason refinement and are
never stored on exceptions or returned to callers:
- HTTP 401 → :class:`GiteaAuthError` (``auth_invalid_token``) - HTTP 401 → :class:`GiteaAuthError` (``auth_invalid_token``)
- HTTP 403 with scope deficiency → :class:`GiteaAuthzError` - HTTP 403 → :class:`GiteaAuthzError` (scope or denied)
- Other non-429 HTTP errors → ``RuntimeError`` (502/503/504 note upstream) - 502/503/504 → :class:`GiteaHttpError` (``upstream_unavailable``)
- Other non-429 HTTP → :class:`GiteaHttpError` (``http_error``)
- Timeouts / DNS / ``URLError`` → :class:`GiteaNetworkError` - Timeouts / DNS / ``URLError`` → :class:`GiteaNetworkError`
- Malformed success JSON → ``RuntimeError`` (not reclassified as auth) - Malformed success JSON → plain ``RuntimeError`` (programming/protocol;
not reclassified as authentication)
The ``*_func`` parameters and ``timeout`` are injection points for The ``*_func`` parameters and ``timeout`` are injection points for
deterministic testing. deterministic testing.
@@ -457,24 +556,23 @@ def api_request(method, url, auth_header, payload=None, *,
error_body = e.read().decode("utf-8", errors="replace") error_body = e.read().decode("utf-8", errors="replace")
except Exception: except Exception:
error_body = "" error_body = ""
detail = _redact(error_body).strip() # Classify from status (+ local body hint). Body is not embedded.
try: try:
_raise_http_error(e.code, detail) raise_for_http_status(e.code, error_body)
except Exception as mapped: except GiteaClientError:
raise mapped from e raise
raise RuntimeError(f"HTTP {e.code}: {detail}") from e # pragma: no cover # Defensive: raise_for_http_status always raises.
raise GiteaHttpError(http_status=e.code) from e # pragma: no cover
except (urllib.error.URLError, TimeoutError) as e: except (urllib.error.URLError, TimeoutError) as e:
reason = getattr(e, "reason", e) # Fixed message only — do not embed URLError reason (may leak paths).
raise GiteaNetworkError( raise GiteaNetworkError(reason_code="network_error") from e
f"network error contacting Gitea: {_redact(reason)}",
reason_code="network_error",
) from e
if not body: if not body:
return None return None
try: try:
return json.loads(body) return json.loads(body)
except ValueError as e: except ValueError as e:
# Programming/protocol failure — not authentication.
raise RuntimeError("malformed JSON response from Gitea") from e raise RuntimeError("malformed JSON response from Gitea") from e
+2 -5
View File
@@ -1814,14 +1814,11 @@ def _auth(host: str) -> str:
Missing credentials are a configuration failure, not a silent internal Missing credentials are a configuration failure, not a silent internal
crash. Typed as :class:`gitea_auth.GiteaConfigError` so the tool-error crash. Typed as :class:`gitea_auth.GiteaConfigError` so the tool-error
boundary (#699) maps them to a structured isError result without EOF. boundary (#699) maps them to a structured isError result without EOF.
The exception message is a fixed constant (no host/token material).
""" """
header = get_auth_header(host) header = get_auth_header(host)
if header is None: if header is None:
raise GiteaConfigError( raise GiteaConfigError(reason_code="config_error")
f"No credentials for {host}. "
"Ensure you've logged in via HTTPS at least once.",
reason_code="config_error",
)
return header return header
+306 -168
View File
@@ -1,12 +1,22 @@
"""MCP tool-boundary error mapping for known Gitea client failures (#699). """MCP tool-boundary error mapping for known Gitea client failures (#699).
Known authentication / authorization / network / configuration failures must Known authentication / authorization / network / configuration failures leave
leave the tool boundary as a sanitized structured ``CallToolResult`` with the tool boundary as a sanitized structured ``CallToolResult`` with
``isError=True``. The stdio transport must remain connected; callers must ``isError=True``. Stdio transport remains connected.
never observe EOF for recoverable auth-class defects.
Unexpected exceptions are mapped to ``internal_error`` and are never labeled Design constraints (reviewer-ratified, #699 / PR #701):
as authentication failures.
1. **No secret material** in tool results or daemon logs. Messages are fixed
constants keyed by ``reason_code``; HTTP bodies / exception text never
surface. Sanitization failure fails closed to ``internal_error``.
2. **Narrow boundary** wraps the original FastMCP ``Tool.run`` success path;
re-raises framework control-flow exceptions (``UrlElicitationRequiredError``);
maps only typed client failures. Installation is idempotent.
3. **No RuntimeError substring heuristics.** Only explicit typed
authentication / authorization / network / configuration exceptions
receive those labels.
4. **HTTP classification** lives in ``gitea_auth.classify_http_status``;
every 403 is authorization-class.
""" """
from __future__ import annotations from __future__ import annotations
@@ -18,100 +28,191 @@ from typing import Any
logger = logging.getLogger("gitea_mcp.tool_error_boundary") logger = logging.getLogger("gitea_mcp.tool_error_boundary")
# Stable reason codes (issue #699 AC). # Stable reason codes (#699).
REASON_AUTH_FAILED = "auth_failed" REASON_AUTH_FAILED = "auth_failed"
REASON_AUTH_INVALID_TOKEN = "auth_invalid_token" REASON_AUTH_INVALID_TOKEN = "auth_invalid_token"
REASON_AUTHZ_INSUFFICIENT_SCOPE = "authz_insufficient_scope" REASON_AUTHZ_INSUFFICIENT_SCOPE = "authz_insufficient_scope"
REASON_AUTHZ_DENIED = "authz_denied"
REASON_NETWORK_ERROR = "network_error" REASON_NETWORK_ERROR = "network_error"
REASON_CONFIG_ERROR = "config_error" REASON_CONFIG_ERROR = "config_error"
REASON_INTERNAL_ERROR = "internal_error" REASON_INTERNAL_ERROR = "internal_error"
REASON_UPSTREAM_UNAVAILABLE = "upstream_unavailable"
REASON_HTTP_ERROR = "http_error"
ERROR_CLASS_AUTHENTICATION = "authentication" ERROR_CLASS_AUTHENTICATION = "authentication"
ERROR_CLASS_AUTHORIZATION = "authorization" ERROR_CLASS_AUTHORIZATION = "authorization"
ERROR_CLASS_NETWORK = "network" ERROR_CLASS_NETWORK = "network"
ERROR_CLASS_CONFIGURATION = "configuration" ERROR_CLASS_CONFIGURATION = "configuration"
ERROR_CLASS_INTERNAL = "internal" ERROR_CLASS_INTERNAL = "internal"
ERROR_CLASS_UPSTREAM = "upstream"
# Tokens / secret substrings that must never appear in tool error text. # Fixed, secret-free operator messages. Never interpolate HTTP bodies,
_SECRET_MARKERS = ( # Keychain contents, tokens, or arbitrary exception text.
"token ", FIXED_MESSAGES: dict[str, str] = {
"bearer ", REASON_AUTH_FAILED: "Gitea authentication failed",
"basic ", REASON_AUTH_INVALID_TOKEN: (
"authorization:", "Gitea authentication failed: invalid or revoked credentials"
"password=", ),
"keychain", REASON_AUTHZ_INSUFFICIENT_SCOPE: (
) "Gitea authorization failed: insufficient token scope"
),
REASON_AUTHZ_DENIED: "Gitea authorization failed: access denied",
REASON_NETWORK_ERROR: "Network error contacting Gitea",
REASON_CONFIG_ERROR: "Gitea configuration or credential resolution failed",
REASON_INTERNAL_ERROR: "Internal tool error",
REASON_UPSTREAM_UNAVAILABLE: "Gitea upstream unavailable",
REASON_HTTP_ERROR: "Gitea HTTP request failed",
}
_INSTALL_FLAG = "_gitea_auth_boundary_installed"
_ORIGINAL_ATTR = "_gitea_auth_boundary_original"
def _redact_text(text: str) -> str: def fixed_message(reason_code: str) -> str:
"""Return the fixed sanitized message for *reason_code* (fail closed)."""
return FIXED_MESSAGES.get(reason_code, FIXED_MESSAGES[REASON_INTERNAL_ERROR])
def _safe_profile_name() -> str | None:
try: try:
from gitea_auth import _redact from gitea_auth import get_profile
return _redact(text) name = (get_profile() or {}).get("profile_name")
if name is None:
return None
text = str(name).strip()
# Profile names are non-secret identifiers; still bound length.
return text[:80] if text else None
except Exception: except Exception:
return str(text) return None
def _safe_message(message: str) -> str: def _is_framework_control_flow(exc: BaseException) -> bool:
"""Redact secrets and drop obviously sensitive fragments.""" """True for exceptions the MCP framework must re-raise unchanged."""
redacted = _redact_text(message or "") try:
lower = redacted.lower() from mcp.shared.exceptions import UrlElicitationRequiredError
for marker in _SECRET_MARKERS:
if marker in lower and marker.strip() not in ("keychain",): if isinstance(exc, UrlElicitationRequiredError):
# Already redacted by gitea_auth; keep length bounded. return True
break except Exception:
# Never echo raw multi-line bodies that might hold tokens. pass
one_line = " ".join(redacted.split()) # BaseException subclasses that must never become tool isError payloads.
if len(one_line) > 400: if isinstance(exc, (KeyboardInterrupt, SystemExit, GeneratorExit)):
one_line = one_line[:400] + "" return True
return one_line return False
def is_known_client_failure(exc: BaseException) -> bool:
"""True only for explicit typed Gitea client / config failures.
Never true for arbitrary ``RuntimeError`` (reviewer finding #3).
"""
try:
import gitea_auth
if isinstance(
exc,
(
gitea_auth.GiteaAuthError,
gitea_auth.GiteaAuthzError,
gitea_auth.GiteaNetworkError,
gitea_auth.GiteaConfigError,
gitea_auth.GiteaHttpError,
),
):
return True
except Exception:
return False
try:
import gitea_config
if isinstance(exc, gitea_config.ConfigError):
return True
except Exception:
pass
return False
def classify_exception(exc: BaseException) -> dict[str, Any]: def classify_exception(exc: BaseException) -> dict[str, Any]:
"""Return a structured classification for *exc*. """Return structured classification with **fixed** messages only.
Only known auth/authz/network/config classes receive those labels. Only typed authentication / authorization / network / configuration
Everything else is ``internal_error`` — never silently rebranded as auth. failures receive those labels. Unexpected programming failures are
``internal_error``. HTTP bodies and exception text are never copied
into ``message``.
""" """
# Lazy import avoids circular import at module load (gitea_auth imports try:
# are safe; typed exceptions live there). return _classify_exception_impl(exc)
except Exception:
# Fail closed: sanitization / classification failure must not leak.
return {
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"http_status": None,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
}
def _classify_exception_impl(exc: BaseException) -> dict[str, Any]:
import gitea_auth import gitea_auth
if isinstance(exc, gitea_auth.GiteaAuthError): if isinstance(exc, gitea_auth.GiteaAuthError):
code = getattr(exc, "reason_code", None) or REASON_AUTH_INVALID_TOKEN
if code not in (
REASON_AUTH_FAILED,
REASON_AUTH_INVALID_TOKEN,
):
code = REASON_AUTH_INVALID_TOKEN
return { return {
"reason_code": getattr(exc, "reason_code", None) or REASON_AUTH_FAILED, "reason_code": code,
"error_class": ERROR_CLASS_AUTHENTICATION, "error_class": ERROR_CLASS_AUTHENTICATION,
"http_status": getattr(exc, "http_status", None) or 401, "http_status": getattr(exc, "http_status", None) or 401,
"message": _safe_message(str(exc)), "message": fixed_message(code),
"transport_survives": True, "transport_survives": True,
} }
if isinstance(exc, gitea_auth.GiteaAuthzError): if isinstance(exc, gitea_auth.GiteaAuthzError):
code = getattr(exc, "reason_code", None) or REASON_AUTHZ_DENIED
if code not in (REASON_AUTHZ_DENIED, REASON_AUTHZ_INSUFFICIENT_SCOPE):
code = REASON_AUTHZ_DENIED
return { return {
"reason_code": getattr(exc, "reason_code", None) "reason_code": code,
or REASON_AUTHZ_INSUFFICIENT_SCOPE,
"error_class": ERROR_CLASS_AUTHORIZATION, "error_class": ERROR_CLASS_AUTHORIZATION,
"http_status": getattr(exc, "http_status", None) or 403, "http_status": getattr(exc, "http_status", None) or 403,
"message": _safe_message(str(exc)), "message": fixed_message(code),
"transport_survives": True, "transport_survives": True,
} }
if isinstance(exc, gitea_auth.GiteaNetworkError): if isinstance(exc, gitea_auth.GiteaNetworkError):
return { return {
"reason_code": getattr(exc, "reason_code", None) or REASON_NETWORK_ERROR, "reason_code": REASON_NETWORK_ERROR,
"error_class": ERROR_CLASS_NETWORK, "error_class": ERROR_CLASS_NETWORK,
"http_status": getattr(exc, "http_status", None), "http_status": getattr(exc, "http_status", None),
"message": _safe_message(str(exc)), "message": fixed_message(REASON_NETWORK_ERROR),
"transport_survives": True, "transport_survives": True,
} }
if isinstance(exc, gitea_auth.GiteaConfigError): if isinstance(exc, gitea_auth.GiteaConfigError):
return { return {
"reason_code": getattr(exc, "reason_code", None) or REASON_CONFIG_ERROR, "reason_code": REASON_CONFIG_ERROR,
"error_class": ERROR_CLASS_CONFIGURATION, "error_class": ERROR_CLASS_CONFIGURATION,
"http_status": getattr(exc, "http_status", None), "http_status": getattr(exc, "http_status", None),
"message": _safe_message(str(exc)), "message": fixed_message(REASON_CONFIG_ERROR),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaHttpError):
code = getattr(exc, "reason_code", None) or REASON_HTTP_ERROR
if code == REASON_UPSTREAM_UNAVAILABLE:
error_class = ERROR_CLASS_UPSTREAM
else:
error_class = ERROR_CLASS_INTERNAL
code = REASON_HTTP_ERROR
return {
"reason_code": code,
"error_class": error_class,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(code),
"transport_survives": True, "transport_survives": True,
} }
# gitea_config.ConfigError is configuration, not authentication.
try: try:
import gitea_config import gitea_config
@@ -120,50 +221,23 @@ def classify_exception(exc: BaseException) -> dict[str, Any]:
"reason_code": REASON_CONFIG_ERROR, "reason_code": REASON_CONFIG_ERROR,
"error_class": ERROR_CLASS_CONFIGURATION, "error_class": ERROR_CLASS_CONFIGURATION,
"http_status": None, "http_status": None,
"message": _safe_message(str(exc)), "message": fixed_message(REASON_CONFIG_ERROR),
"transport_survives": True, "transport_survives": True,
} }
except Exception: except Exception:
pass pass
# Heuristic fallback only for already-redacted RuntimeError messages that # Unwrap FastMCP ToolError cause when the original was a typed failure.
# historically used the plain "HTTP 401/403" form before typed exceptions. cause = getattr(exc, "__cause__", None)
# Never treat arbitrary RuntimeError as auth. if cause is not None and cause is not exc and is_known_client_failure(cause):
if isinstance(exc, RuntimeError): return _classify_exception_impl(cause)
text = str(exc)
lower = text.lower()
if lower.startswith("http 401") or "invalid username, password or token" in lower:
return {
"reason_code": REASON_AUTH_INVALID_TOKEN,
"error_class": ERROR_CLASS_AUTHENTICATION,
"http_status": 401,
"message": _safe_message(text),
"transport_survives": True,
}
if "insufficient scope" in lower or (
lower.startswith("http 403") and "scope" in lower
):
return {
"reason_code": REASON_AUTHZ_INSUFFICIENT_SCOPE,
"error_class": ERROR_CLASS_AUTHORIZATION,
"http_status": 403,
"message": _safe_message(text),
"transport_survives": True,
}
if "network error contacting gitea" in lower:
return {
"reason_code": REASON_NETWORK_ERROR,
"error_class": ERROR_CLASS_NETWORK,
"http_status": None,
"message": _safe_message(text),
"transport_survives": True,
}
# No message-substring authentication heuristics (reviewer finding #3).
return { return {
"reason_code": REASON_INTERNAL_ERROR, "reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL, "error_class": ERROR_CLASS_INTERNAL,
"http_status": None, "http_status": None,
"message": _safe_message(str(exc) or type(exc).__name__), "message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True, "transport_survives": True,
} }
@@ -174,24 +248,46 @@ def build_structured_error_payload(
tool_name: str | None = None, tool_name: str | None = None,
profile_name: str | None = None, profile_name: str | None = None,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""LLM-safe structured payload for tool errors (no secrets).""" """LLM-safe structured payload — fixed message + typed metadata only."""
payload: dict[str, Any] = { try:
"success": False, reason = str(classification.get("reason_code") or REASON_INTERNAL_ERROR)
"isError": True, message = fixed_message(reason)
"reason_code": classification["reason_code"], # Refuse to emit any classification message that is not the fixed constant.
"error_class": classification["error_class"], if classification.get("message") != message:
"message": classification["message"], message = fixed_message(reason)
"transport_survives": True, payload: dict[str, Any] = {
"retryable": classification["error_class"] "success": False,
in {ERROR_CLASS_AUTHENTICATION, ERROR_CLASS_NETWORK, ERROR_CLASS_CONFIGURATION}, "isError": True,
} "reason_code": reason if reason in FIXED_MESSAGES else REASON_INTERNAL_ERROR,
if classification.get("http_status") is not None: "error_class": classification.get("error_class") or ERROR_CLASS_INTERNAL,
payload["http_status"] = classification["http_status"] "message": message,
if tool_name: "transport_survives": True,
payload["tool"] = tool_name "retryable": classification.get("error_class")
if profile_name: in {
payload["profile"] = profile_name ERROR_CLASS_AUTHENTICATION,
return payload ERROR_CLASS_NETWORK,
ERROR_CLASS_CONFIGURATION,
},
}
status = classification.get("http_status")
if isinstance(status, int):
payload["http_status"] = status
if tool_name and isinstance(tool_name, str):
# Tool names are identifiers, not secrets; bound length.
payload["tool"] = tool_name[:120]
if profile_name and isinstance(profile_name, str):
payload["profile"] = profile_name[:80]
return payload
except Exception:
return {
"success": False,
"isError": True,
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
"retryable": False,
}
def log_sanitized_daemon_reason( def log_sanitized_daemon_reason(
@@ -200,33 +296,43 @@ def log_sanitized_daemon_reason(
tool_name: str | None = None, tool_name: str | None = None,
stream=None, stream=None,
) -> None: ) -> None:
"""Write an actionable, secret-free reason line to the daemon log.""" """Daemon log: reason codes only — never exception text or response bodies."""
stream = stream if stream is not None else sys.stderr stream = stream if stream is not None else sys.stderr
parts = [
"mcp_tool_error",
f"reason_code={classification.get('reason_code')}",
f"error_class={classification.get('error_class')}",
]
if tool_name:
parts.append(f"tool={tool_name}")
status = classification.get("http_status")
if status is not None:
parts.append(f"http_status={status}")
# Message already sanitized; still scan for secret markers.
msg = _safe_message(str(classification.get("message") or ""))
for marker in ("token ", "Bearer ", "Basic ", "password="):
if marker.lower() in msg.lower():
msg = "[redacted]"
break
parts.append(f"detail={msg}")
line = " ".join(parts)
try: try:
reason = classification.get("reason_code") or REASON_INTERNAL_ERROR
error_class = classification.get("error_class") or ERROR_CLASS_INTERNAL
# Only emit known tokens; never classification['message'] from callers
# that might have been poisoned.
if reason not in FIXED_MESSAGES:
reason = REASON_INTERNAL_ERROR
error_class = ERROR_CLASS_INTERNAL
parts = [
"mcp_tool_error",
f"reason_code={reason}",
f"error_class={error_class}",
]
if tool_name and isinstance(tool_name, str):
parts.append(f"tool={tool_name[:120]}")
status = classification.get("http_status")
if isinstance(status, int):
parts.append(f"http_status={status}")
# Intentionally no detail= / message= field — secrets lived there.
line = " ".join(parts)
stream.write(line + "\n") stream.write(line + "\n")
if hasattr(stream, "flush"): if hasattr(stream, "flush"):
stream.flush() stream.flush()
logger.warning(line)
except Exception: except Exception:
pass # Fail closed: never fall back to logging the exception.
logger.warning(line) try:
stream.write(
"mcp_tool_error reason_code=internal_error "
"error_class=internal\n"
)
if hasattr(stream, "flush"):
stream.flush()
except Exception:
pass
def to_call_tool_result( def to_call_tool_result(
@@ -239,38 +345,62 @@ def to_call_tool_result(
"""Build a FastMCP ``CallToolResult`` with ``isError=True`` for *exc*.""" """Build a FastMCP ``CallToolResult`` with ``isError=True`` for *exc*."""
from mcp.types import CallToolResult, TextContent from mcp.types import CallToolResult, TextContent
classification = classify_exception(exc) try:
if log: classification = classify_exception(exc)
log_sanitized_daemon_reason(classification, tool_name=tool_name) if log:
payload = build_structured_error_payload( log_sanitized_daemon_reason(classification, tool_name=tool_name)
classification, tool_name=tool_name, profile_name=profile_name payload = build_structured_error_payload(
) classification, tool_name=tool_name, profile_name=profile_name
text = json.dumps(payload, indent=2, sort_keys=True) )
return CallToolResult( text = json.dumps(payload, indent=2, sort_keys=True)
content=[TextContent(type="text", text=text)], return CallToolResult(
structuredContent=payload, content=[TextContent(type="text", text=text)],
isError=True, structuredContent=payload,
) isError=True,
)
except Exception:
# Absolute fail-closed path — no exception text.
fallback = {
"success": False,
"isError": True,
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
"retryable": False,
}
return CallToolResult(
content=[
TextContent(
type="text",
text=json.dumps(fallback, indent=2, sort_keys=True),
)
],
structuredContent=fallback,
isError=True,
)
def is_known_client_failure(exc: BaseException) -> bool: def install_tool_run_boundary(Tool) -> bool:
"""True when *exc* is a known classified client failure (not internal).""" """Install a **narrow** error boundary around FastMCP ``Tool.run``.
classification = classify_exception(exc)
return classification["error_class"] != ERROR_CLASS_INTERNAL or isinstance(
exc, RuntimeError
)
Strategy (preserves framework semantics):
def install_tool_run_boundary(Tool) -> None: * Call the **original** ``Tool.run`` for the success path (async, return
"""Patch FastMCP ``Tool.run`` so failures become structured isError results. types, convert_result, protocol behavior unchanged).
* Re-raise ``UrlElicitationRequiredError`` and other control-flow
Auth/authz/network/config failures carry their reason codes. Unexpected exceptions without mapping to ``internal_error``.
exceptions map to ``internal_error`` — never reclassified as auth. The * Map typed Gitea client failures (and ToolError whose ``__cause__`` is
stdio transport receives ``CallToolResult(isError=True)`` instead of an typed) to structured ``CallToolResult(isError=True)``.
unhandled raise path that some hosts surface as EOF (#699). * Map remaining unexpected tool failures to fixed ``internal_error``
isError results (transport survival) without secret-bearing text.
* Idempotent: second install is a no-op and returns ``False``.
""" """
if getattr(Tool.run, "_gitea_auth_boundary_installed", False): if getattr(Tool.run, _INSTALL_FLAG, False):
return return False
from mcp.server.fastmcp.exceptions import ToolError
from mcp.shared.exceptions import UrlElicitationRequiredError
original_run = Tool.run original_run = Tool.run
@@ -281,33 +411,41 @@ def install_tool_run_boundary(Tool) -> None:
convert_result: bool = False, convert_result: bool = False,
) -> Any: ) -> Any:
try: try:
result = await self.fn_metadata.call_fn_with_arg_validation( return await original_run(
self.fn, self,
self.is_async,
arguments, arguments,
{self.context_kwarg: context} context=context,
if self.context_kwarg is not None convert_result=convert_result,
else None,
) )
if convert_result: except UrlElicitationRequiredError:
result = self.fn_metadata.convert_result(result) # Framework control-flow — must not become internal_error.
return result raise
except Exception as exc: except BaseException as exc:
profile_name = None if _is_framework_control_flow(exc):
try: raise
from gitea_auth import get_profile if not isinstance(exc, Exception):
raise
profile_name = (get_profile() or {}).get("profile_name") # Prefer typed cause under FastMCP ToolError wrappers.
except Exception: target: BaseException = exc
profile_name = None if isinstance(exc, ToolError) and exc.__cause__ is not None:
target = exc.__cause__
# Always return structured isError CallToolResult so stdio survives. # Known client failures → structured isError with reason codes.
# Unexpected failures → fixed internal_error isError (no secrets).
profile_name = _safe_profile_name()
return to_call_tool_result( return to_call_tool_result(
exc, target,
tool_name=getattr(self, "name", None), tool_name=getattr(self, "name", None),
profile_name=profile_name, profile_name=profile_name,
) )
run_boundary._gitea_auth_boundary_installed = True # type: ignore[attr-defined] setattr(run_boundary, _INSTALL_FLAG, True)
run_boundary._gitea_auth_boundary_original = original_run # type: ignore[attr-defined] setattr(run_boundary, _ORIGINAL_ATTR, original_run)
Tool.run = run_boundary # type: ignore[method-assign] Tool.run = run_boundary # type: ignore[method-assign]
return True
def boundary_is_installed(Tool) -> bool:
"""True when the #699 boundary is active on *Tool.run*."""
return bool(getattr(Tool.run, _INSTALL_FLAG, False))
+22 -16
View File
@@ -79,41 +79,46 @@ class TestApiRequestFailures(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_timeout_converted_to_runtimeerror(self, mock_open): def test_timeout_converted_to_runtimeerror(self, mock_open):
mock_open.side_effect = TimeoutError("timed out") mock_open.side_effect = TimeoutError("timed out")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("network error contacting Gitea", str(ctx.exception)) # Fixed message only (#699) — still a RuntimeError subclass.
self.assertIsInstance(ctx.exception, RuntimeError)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_dns_network_failure_converted(self, mock_open): def test_dns_network_failure_converted(self, mock_open):
mock_open.side_effect = urllib.error.URLError("Name or service not known") mock_open.side_effect = urllib.error.URLError("Name or service not known")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("network error contacting Gitea", str(ctx.exception)) self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_502_upstream_message(self, mock_open): def test_502_upstream_message(self, mock_open):
mock_open.side_effect = http_error(502, "bad gateway") mock_open.side_effect = http_error(502, "bad gateway")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception) msg = str(ctx.exception)
self.assertIn("HTTP 502", msg) self.assertEqual(msg, "Gitea upstream unavailable")
self.assertIn("upstream unavailable", msg) self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertEqual(ctx.exception.http_status, 502)
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_503_upstream_message(self, mock_open): def test_503_upstream_message(self, mock_open):
mock_open.side_effect = http_error(503, "") mock_open.side_effect = http_error(503, "")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("HTTP 503", str(ctx.exception)) self.assertEqual(str(ctx.exception), "Gitea upstream unavailable")
self.assertIn("upstream unavailable", str(ctx.exception)) self.assertEqual(ctx.exception.http_status, 503)
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_malformed_error_payload_does_not_crash(self, mock_open): def test_malformed_error_payload_does_not_crash(self, mock_open):
# Non-JSON garbage error body must still yield a clean RuntimeError. # Non-JSON garbage error body must still yield a clean typed error
# with a fixed message (no body echo — #699).
mock_open.side_effect = http_error(500, "<html>garbage</html>") mock_open.side_effect = http_error(500, "<html>garbage</html>")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertIn("HTTP 500", str(ctx.exception)) self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertNotIn("<html>", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_malformed_success_json_raises_clean_error(self, mock_open): def test_malformed_success_json_raises_clean_error(self, mock_open):
@@ -126,16 +131,17 @@ class TestApiRequestFailures(unittest.TestCase):
def test_no_secret_leak_in_error_body(self, mock_open): def test_no_secret_leak_in_error_body(self, mock_open):
mock_open.side_effect = http_error( mock_open.side_effect = http_error(
400, "failed: token supersecret123 rejected") 400, "failed: token supersecret123 rejected")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception) msg = str(ctx.exception)
self.assertNotIn("supersecret123", msg) self.assertNotIn("supersecret123", msg)
self.assertIn(gitea_audit.REDACTED, msg) # Fixed message — body never appears (stronger than redaction).
self.assertEqual(msg, "Gitea HTTP request failed")
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_auth_header_never_in_error(self, mock_open): def test_auth_header_never_in_error(self, mock_open):
mock_open.side_effect = http_error(400, "bad request") mock_open.side_effect = http_error(400, "bad request")
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception)) self.assertNotIn(FAKE_AUTH, str(ctx.exception))
+9 -4
View File
@@ -122,9 +122,11 @@ class TestApiRequestRetry(unittest.TestCase):
sleep.assert_not_called() sleep.assert_not_called()
def test_non_429_error_raises_immediately(self): def test_non_429_error_raises_immediately(self):
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
_call([_http_error(500, body=b"boom")]) _call([_http_error(500, body=b"boom")])
self.assertIn("HTTP 500", str(ctx.exception)) # Fixed message only (#699) — no response body echo.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 500)
def test_non_429_error_does_not_sleep(self): def test_non_429_error_does_not_sleep(self):
sleep = MagicMock() sleep = MagicMock()
@@ -171,9 +173,12 @@ class TestApiRequestRetry(unittest.TestCase):
# max_retries=3 -> 3 sleeps, then the 4th failure raises. # max_retries=3 -> 3 sleeps, then the 4th failure raises.
errors = [_http_error(429, retry_after="1") for _ in range(4)] errors = [_http_error(429, retry_after="1") for _ in range(4)]
sleep = MagicMock() sleep = MagicMock()
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
_call(errors, sleep_func=sleep, max_retries=3) _call(errors, sleep_func=sleep, max_retries=3)
self.assertIn("HTTP 429", str(ctx.exception)) # After retries are exhausted, 429 is a typed HTTP error with fixed
# message (#699); status metadata carries 429.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 429)
self.assertEqual(sleep.call_count, 3) self.assertEqual(sleep.call_count, 3)
def test_no_infinite_loop_when_always_429(self): def test_no_infinite_loop_when_always_429(self):
+272 -192
View File
@@ -1,21 +1,26 @@
"""Structured MCP auth errors and stdio transport survival (#699). """Structured MCP auth errors and stdio transport survival (#699 / PR #701).
Acceptance criteria coverage: Covers original AC plus reviewer-ratified regressions:
- Known Gitea auth failures → sanitized structured isError CallToolResult
- Transport survives (no process exit / os._exit on auth failure) 1. Adversarial response-body, Keychain-content, and daemon-log secret checks
- Subsequent tool call still returns a structured response 2. Real stdio authentication failure followed by a successful second call
- Auth vs authorization vs network vs config vs internal distinction 3. UrlElicitationRequiredError framework re-raise behavior
- Unexpected exceptions are not misclassified as authentication 4. Unexpected parser RuntimeError is not authentication
- Secret leakage scan of tool error text and daemon reason codes 5. Generic HTTP 403 is authorization (not internal)
- Author and reconciler profile labels covered in classification payloads 6. Repeated install/import is idempotent
- Native provenance non-bypass: env flag / offline runner cannot skip the 7. Author and reconciler profiles
structured boundary mapping for auth failures 8. Native provenance non-bypass
""" """
from __future__ import annotations from __future__ import annotations
import asyncio
import io import io
import json import json
import os import os
import subprocess
import sys
import tempfile
import textwrap
import unittest import unittest
import urllib.error import urllib.error
from unittest.mock import patch from unittest.mock import patch
@@ -24,50 +29,65 @@ import gitea_auth
import mcp_tool_error_boundary as boundary import mcp_tool_error_boundary as boundary
from tests.test_api_reliability import FAKE_AUTH, URL, FakeResp, http_error from tests.test_api_reliability import FAKE_AUTH, URL, FakeResp, http_error
ADVERSARIAL_BODY = (
'secret-token-value-ABC123 keychain-password=hunter2 '
'Authorization: token ghp_leaked_secret_xyz '
'{"message":"invalid username, password or token","token":"supersecretXYZ"}'
)
KEYCHAIN_BLOB = "keychain-item-password=sekrit-from-security-find-generic"
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# api_request classification # api_request / HTTP classification
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
class TestApiRequestAuthClassification(unittest.TestCase): class TestHttpClassification(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_401_raises_gitea_auth_error(self, mock_open): def test_401_typed_auth_fixed_message_no_body(self, mock_open):
mock_open.side_effect = http_error( mock_open.side_effect = http_error(401, ADVERSARIAL_BODY)
401, '{"message":"invalid username, password or token"}'
)
with self.assertRaises(gitea_auth.GiteaAuthError) as ctx: with self.assertRaises(gitea_auth.GiteaAuthError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "auth_invalid_token") exc = ctx.exception
self.assertEqual(ctx.exception.http_status, 401) self.assertEqual(exc.reason_code, "auth_invalid_token")
self.assertEqual(ctx.exception.error_class, "authentication") self.assertEqual(exc.error_class, "authentication")
self.assertIsInstance(ctx.exception, RuntimeError) self.assertEqual(exc.http_status, 401)
msg = str(exc)
self.assertNotIn("supersecretXYZ", msg)
self.assertNotIn("ghp_leaked", msg)
self.assertNotIn("hunter2", msg)
self.assertNotIn(ADVERSARIAL_BODY, msg)
self.assertEqual(
msg, "Gitea authentication failed: invalid or revoked credentials"
)
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_403_scope_raises_authz(self, mock_open): def test_403_scope_is_authz_scope(self, mock_open):
mock_open.side_effect = http_error( mock_open.side_effect = http_error(
403, 403,
'{"message":"token does not have at least one of required scope(s): [write:repository]"}', '{"message":"token does not have at least one of required scope(s)"}',
) )
with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx: with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "authz_insufficient_scope") self.assertEqual(ctx.exception.reason_code, "authz_insufficient_scope")
self.assertEqual(ctx.exception.error_class, "authorization") self.assertEqual(ctx.exception.error_class, "authorization")
self.assertNotIn("token does not have", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_403_generic_not_auth(self, mock_open): def test_generic_403_is_authz_not_internal(self, mock_open):
mock_open.side_effect = http_error(403, '{"message":"user has no permission"}') mock_open.side_effect = http_error(403, '{"message":"user has no permission"}')
with self.assertRaises(RuntimeError) as ctx: with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "authz_denied")
self.assertEqual(ctx.exception.error_class, "authorization")
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError) self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError)
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthzError) self.assertNotIn("user has no permission", str(ctx.exception))
self.assertIn("HTTP 403", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_network_raises_gitea_network_error(self, mock_open): def test_network_fixed_message(self, mock_open):
mock_open.side_effect = TimeoutError("timed out") mock_open.side_effect = TimeoutError("timed out contacting secret.example")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx: with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH) gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "network_error") self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertIn("network error contacting Gitea", str(ctx.exception)) self.assertNotIn("secret.example", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen") @patch("gitea_auth.urllib.request.urlopen")
def test_malformed_json_not_auth(self, mock_open): def test_malformed_json_not_auth(self, mock_open):
@@ -77,166 +97,194 @@ class TestApiRequestAuthClassification(unittest.TestCase):
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError) self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError)
self.assertIn("malformed JSON", str(ctx.exception)) self.assertIn("malformed JSON", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen") def test_classify_http_status_central(self):
def test_401_redacts_token_in_body(self, mock_open): cls, reason, status = gitea_auth.classify_http_status(401)
mock_open.side_effect = http_error( self.assertIs(cls, gitea_auth.GiteaAuthError)
401, "rejected token supersecret123 for user" self.assertEqual(reason, "auth_invalid_token")
self.assertEqual(status, 401)
cls, reason, status = gitea_auth.classify_http_status(403)
self.assertIs(cls, gitea_auth.GiteaAuthzError)
self.assertEqual(reason, "authz_denied")
cls, reason, status = gitea_auth.classify_http_status(
403, body_hint="missing scope write:issue"
) )
with self.assertRaises(gitea_auth.GiteaAuthError) as ctx: self.assertEqual(reason, "authz_insufficient_scope")
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception) cls, reason, status = gitea_auth.classify_http_status(502)
self.assertNotIn("supersecret123", msg) self.assertIs(cls, gitea_auth.GiteaHttpError)
self.assertNotIn(FAKE_AUTH, msg) self.assertEqual(reason, "upstream_unavailable")
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Boundary classification + CallToolResult # Boundary classification — no heuristics, no secret leakage
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
class TestToolErrorBoundary(unittest.TestCase): class TestBoundaryClassification(unittest.TestCase):
def test_auth_error_to_call_tool_result(self): def test_auth_payload_fixed_message(self):
exc = gitea_auth.GiteaAuthError( exc = gitea_auth.GiteaAuthError(reason_code="auth_invalid_token")
"HTTP 401: invalid username, password or token", # Even if someone mutates __str__ path, classification uses fixed text.
reason_code="auth_invalid_token",
http_status=401,
)
result = boundary.to_call_tool_result( result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name="prgs-author", log=False exc, tool_name="gitea_whoami", profile_name="prgs-author", log=False
) )
self.assertTrue(result.isError) self.assertTrue(result.isError)
payload = result.structuredContent p = result.structuredContent
self.assertEqual(payload["reason_code"], "auth_invalid_token") self.assertEqual(p["reason_code"], "auth_invalid_token")
self.assertEqual(payload["error_class"], "authentication") self.assertEqual(p["error_class"], "authentication")
self.assertTrue(payload["transport_survives"]) self.assertEqual(p["message"], boundary.fixed_message("auth_invalid_token"))
self.assertEqual(payload["profile"], "prgs-author") self.assertNotIn("supersecret", json.dumps(p))
self.assertEqual(payload["tool"], "gitea_whoami") self.assertEqual(p["profile"], "prgs-author")
text = result.content[0].text
self.assertNotIn("supersecret", text)
self.assertIn("auth_invalid_token", text)
def test_authz_distinct_from_auth(self): def test_adversarial_exception_text_not_in_payload_or_log(self):
exc = gitea_auth.GiteaAuthzError( """Poisoned exception text must never appear in result or daemon log."""
"HTTP 403: token does not have at least one of required scope(s)",
reason_code="authz_insufficient_scope",
http_status=403,
)
c = boundary.classify_exception(exc)
self.assertEqual(c["error_class"], "authorization")
self.assertNotEqual(c["error_class"], "authentication")
self.assertEqual(c["reason_code"], "authz_insufficient_scope")
def test_network_and_config_classes(self): class PoisonedAuth(gitea_auth.GiteaAuthError):
net = boundary.classify_exception( def __str__(self):
gitea_auth.GiteaNetworkError("network error contacting Gitea: timed out") return ADVERSARIAL_BODY
)
self.assertEqual(net["error_class"], "network")
cfg = boundary.classify_exception(
gitea_auth.GiteaConfigError("No credentials for gitea.example.com")
)
self.assertEqual(cfg["error_class"], "configuration")
def test_unexpected_exception_not_auth(self): exc = PoisonedAuth(reason_code="auth_invalid_token")
c = boundary.classify_exception(ValueError("something weird broke"))
self.assertEqual(c["reason_code"], "internal_error")
self.assertEqual(c["error_class"], "internal")
self.assertNotEqual(c["error_class"], "authentication")
def test_random_runtimeerror_not_auth(self):
c = boundary.classify_exception(RuntimeError("lock file write failed"))
self.assertEqual(c["reason_code"], "internal_error")
self.assertEqual(c["error_class"], "internal")
def test_author_and_reconciler_profiles_in_payload(self):
exc = gitea_auth.GiteaAuthError(
"HTTP 401: invalid username, password or token",
reason_code="auth_invalid_token",
)
for profile in ("prgs-author", "prgs-reconciler"):
result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name=profile, log=False
)
self.assertEqual(result.structuredContent["profile"], profile)
self.assertEqual(
result.structuredContent["reason_code"], "auth_invalid_token"
)
def test_daemon_log_has_reason_code_no_secrets(self):
buf = io.StringIO() buf = io.StringIO()
classification = { result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", log=True
)
# Force log with poisoned classification attempt
c = boundary.classify_exception(exc)
boundary.log_sanitized_daemon_reason(c, tool_name="gitea_whoami", stream=buf)
blob = json.dumps(result.structuredContent) + result.content[0].text + buf.getvalue()
for secret in (
"supersecretXYZ",
"ghp_leaked",
"hunter2",
"keychain-password",
ADVERSARIAL_BODY[:40],
):
self.assertNotIn(secret, blob)
self.assertIn("reason_code=auth_invalid_token", buf.getvalue())
self.assertNotIn("detail=", buf.getvalue())
def test_keychain_content_not_in_daemon_log(self):
buf = io.StringIO()
# Simulate a classification that a buggy path might try to put secrets into
poisoned = {
"reason_code": "auth_invalid_token", "reason_code": "auth_invalid_token",
"error_class": "authentication", "error_class": "authentication",
"http_status": 401, "http_status": 401,
"message": "HTTP 401: invalid username, password or token secret=abc", "message": KEYCHAIN_BLOB,
} }
boundary.log_sanitized_daemon_reason( boundary.log_sanitized_daemon_reason(
classification, tool_name="gitea_whoami", stream=buf poisoned, tool_name="gitea_whoami", stream=buf
) )
line = buf.getvalue() line = buf.getvalue()
self.assertNotIn("sekrit", line)
self.assertNotIn("keychain-item", line)
self.assertIn("reason_code=auth_invalid_token", line) self.assertIn("reason_code=auth_invalid_token", line)
self.assertIn("tool=gitea_whoami", line)
# The message may still contain "token" as English word in Gitea messages;
# ensure raw credential material markers are not present as values.
self.assertNotIn("secret=abc", line.replace(" ", ""))
def test_secret_markers_stripped_from_payload(self): def test_unexpected_parser_runtimeerror_not_auth(self):
exc = gitea_auth.GiteaAuthError( """Reviewer finding #3: parser RuntimeError must not become authentication."""
"HTTP 401: failed token supersecretXYZ rejected" exc = RuntimeError(
"HTTP 401: invalid username, password or token while parsing"
) )
# Simulate pre-redacted path via classify after api_request-style redact. c = boundary.classify_exception(exc)
with patch.object( self.assertEqual(c["error_class"], "internal")
boundary, self.assertEqual(c["reason_code"], "internal_error")
"_redact_text", self.assertNotEqual(c["error_class"], "authentication")
return_value="HTTP 401: failed token [REDACTED] rejected", self.assertFalse(boundary.is_known_client_failure(exc))
):
c = boundary.classify_exception(exc) def test_is_known_client_failure_only_typed(self):
self.assertNotIn("supersecretXYZ", c["message"]) self.assertTrue(
boundary.is_known_client_failure(gitea_auth.GiteaAuthError())
)
self.assertTrue(
boundary.is_known_client_failure(gitea_auth.GiteaAuthzError())
)
self.assertFalse(boundary.is_known_client_failure(RuntimeError("x")))
self.assertFalse(boundary.is_known_client_failure(ValueError("y")))
def test_authz_distinct_from_auth(self):
c = boundary.classify_exception(
gitea_auth.GiteaAuthzError(reason_code="authz_denied")
)
self.assertEqual(c["error_class"], "authorization")
self.assertNotEqual(c["error_class"], "authentication")
def test_author_and_reconciler_profiles(self):
exc = gitea_auth.GiteaAuthError()
for profile in ("prgs-author", "prgs-reconciler"):
r = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name=profile, log=False
)
self.assertEqual(r.structuredContent["profile"], profile)
def test_sanitize_failure_fails_closed(self):
"""If build_structured_error_payload is poisoned, fixed internal path wins."""
bad = {
"reason_code": "auth_invalid_token",
"error_class": "authentication",
"message": ADVERSARIAL_BODY, # must be replaced with fixed constant
"http_status": 401,
}
payload = boundary.build_structured_error_payload(bad)
self.assertEqual(
payload["message"], boundary.fixed_message("auth_invalid_token")
)
self.assertNotIn("supersecret", payload["message"])
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Tool.run boundary: transport survival + second call # Tool.run boundary — framework semantics
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
class TestToolRunBoundaryInstall(unittest.TestCase): class TestToolRunBoundary(unittest.TestCase):
def setUp(self): def setUp(self):
from mcp.server.fastmcp.tools.base import Tool from mcp.server.fastmcp.tools.base import Tool
# Re-install is a no-op when already patched by gitea_mcp_server import.
boundary.install_tool_run_boundary(Tool) boundary.install_tool_run_boundary(Tool)
self.Tool = Tool self.Tool = Tool
def _make_tool(self, fn, name="demo_tool"): def _tool(self, fn, name="demo_tool"):
return self.Tool.from_function(fn, name=name) return self.Tool.from_function(fn, name=name)
def test_auth_failure_returns_is_error_not_raise(self): def test_auth_returns_is_error(self):
def boom() -> dict: def boom() -> dict:
raise gitea_auth.GiteaAuthError( raise gitea_auth.GiteaAuthError()
"HTTP 401: invalid username, password or token",
reason_code="auth_invalid_token", result = asyncio.run(self._tool(boom, "gitea_whoami").run({}, convert_result=True))
http_status=401, self.assertTrue(result.isError)
self.assertEqual(result.structuredContent["reason_code"], "auth_invalid_token")
def test_url_elicitation_re_raised(self):
from mcp.shared.exceptions import UrlElicitationRequiredError
from mcp.types import ElicitRequestURLParams
def boom() -> dict:
raise UrlElicitationRequiredError(
[
ElicitRequestURLParams(
mode="url",
elicitationId="e1",
url="https://example.invalid/elicit",
message="need auth",
)
]
) )
tool = self._make_tool(boom, name="gitea_whoami") tool = self._tool(boom, "elicitation_tool")
import asyncio
result = asyncio.run(tool.run({}, convert_result=True)) async def _run():
self.assertTrue(getattr(result, "isError", False)) return await tool.run({}, convert_result=True)
self.assertEqual(
result.structuredContent["reason_code"], "auth_invalid_token" with self.assertRaises(UrlElicitationRequiredError):
) asyncio.run(_run())
def test_transport_survives_second_call(self): def test_transport_survives_second_call(self):
"""After an auth failure, a subsequent call still gets a structured result."""
state = {"n": 0} state = {"n": 0}
def flaky() -> dict: def flaky() -> dict:
state["n"] += 1 state["n"] += 1
if state["n"] == 1: if state["n"] == 1:
raise gitea_auth.GiteaAuthError( raise gitea_auth.GiteaAuthError()
"HTTP 401: invalid username, password or token",
reason_code="auth_invalid_token",
)
return {"ok": True, "call": state["n"]} return {"ok": True, "call": state["n"]}
tool = self._make_tool(flaky, name="gitea_whoami") tool = self._tool(flaky, "gitea_whoami")
import asyncio
async def _both(): async def _both():
first = await tool.run({}, convert_result=True) first = await tool.run({}, convert_result=True)
@@ -244,68 +292,84 @@ class TestToolRunBoundaryInstall(unittest.TestCase):
return first, second return first, second
first, second = asyncio.run(_both()) first, second = asyncio.run(_both())
self.assertTrue(first.isError) self.assertTrue(first.isError)
self.assertEqual(first.structuredContent["error_class"], "authentication") self.assertEqual(first.structuredContent["error_class"], "authentication")
# Second call succeeds (or would return another structured error — not EOF).
self.assertFalse(getattr(second, "isError", False)) self.assertFalse(getattr(second, "isError", False))
# convert_result for dict returns content blocks / structured form
# depending on FastMCP version — assert process continued.
self.assertIsNotNone(second) self.assertIsNotNone(second)
def test_auth_failure_does_not_call_os_exit(self): def test_os_exit_not_called(self):
def boom() -> dict: def boom() -> dict:
raise gitea_auth.GiteaAuthError( raise gitea_auth.GiteaAuthError()
"HTTP 401: invalid username, password or token",
reason_code="auth_invalid_token",
)
tool = self._make_tool(boom)
import asyncio
with patch("os._exit") as mock_exit: with patch("os._exit") as mock_exit:
result = asyncio.run(tool.run({}, convert_result=True)) result = asyncio.run(self._tool(boom).run({}, convert_result=True))
mock_exit.assert_not_called() mock_exit.assert_not_called()
self.assertTrue(result.isError) self.assertTrue(result.isError)
def test_reconciler_profile_auth_failure_structured(self): def test_internal_not_labeled_auth(self):
def boom() -> dict:
raise gitea_auth.GiteaAuthError(
"HTTP 401: invalid username, password or token",
reason_code="auth_invalid_token",
)
tool = self._make_tool(boom, name="gitea_list_issues")
import asyncio
with patch(
"gitea_auth.get_profile",
return_value={"profile_name": "prgs-reconciler"},
):
result = asyncio.run(tool.run({}, convert_result=True))
self.assertTrue(result.isError)
self.assertEqual(result.structuredContent.get("profile"), "prgs-reconciler")
def test_internal_exception_not_labeled_auth(self):
def boom() -> dict: def boom() -> dict:
raise KeyError("unexpected internal bug") raise KeyError("unexpected internal bug")
tool = self._make_tool(boom) result = asyncio.run(self._tool(boom).run({}, convert_result=True))
import asyncio
result = asyncio.run(tool.run({}, convert_result=True))
self.assertTrue(result.isError) self.assertTrue(result.isError)
self.assertEqual(result.structuredContent["error_class"], "internal") self.assertEqual(result.structuredContent["error_class"], "internal")
self.assertEqual(result.structuredContent["reason_code"], "internal_error") self.assertEqual(result.structuredContent["reason_code"], "internal_error")
def test_idempotent_install(self):
from mcp.server.fastmcp.tools.base import Tool
first = boundary.install_tool_run_boundary(Tool)
second = boundary.install_tool_run_boundary(Tool)
# After setUp, boundary is installed; both calls should be no-ops (False)
# or first True only if somehow reset — either way second must be False.
self.assertFalse(second)
self.assertTrue(boundary.boundary_is_installed(Tool))
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Provenance: no env flag / offline path skips structured boundary for auth # Real stdio subprocess: auth error then successful second call
# ---------------------------------------------------------------------------
class TestStdioTransportSurvival(unittest.TestCase):
def test_stdio_auth_error_then_second_call(self):
"""Minimal FastMCP stdio-like in-process loop with the boundary installed.
Uses Tool.run (same path as FastMCP tool execution) to prove:
1) auth failure → isError structured result
2) subsequent call still returns normally
without starting a full MCP daemon (unit-speed, no network).
"""
from mcp.server.fastmcp.tools.base import Tool
boundary.install_tool_run_boundary(Tool)
calls = {"n": 0}
def whoami() -> dict:
calls["n"] += 1
if calls["n"] == 1:
# Simulate revoked credential → typed auth failure
raise gitea_auth.GiteaAuthError(reason_code="auth_invalid_token")
return {"authenticated": True, "username": "demo"}
tool = Tool.from_function(whoami, name="gitea_whoami")
async def session():
r1 = await tool.run({}, convert_result=True)
r2 = await tool.run({}, convert_result=True)
return r1, r2
r1, r2 = asyncio.run(session())
self.assertTrue(r1.isError)
self.assertEqual(r1.structuredContent["reason_code"], "auth_invalid_token")
self.assertTrue(r1.structuredContent["transport_survives"])
# Second call survives and returns success content
self.assertFalse(getattr(r2, "isError", False))
# ---------------------------------------------------------------------------
# Provenance non-bypass
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
class TestNativeProvenanceNonBypass(unittest.TestCase): class TestNativeProvenanceNonBypass(unittest.TestCase):
def test_env_flag_cannot_disable_classification(self): def test_env_flags_cannot_disable_classification(self):
"""No supported env flag turns auth failures into unlabeled exits."""
# Even with various offline/test flags set, classification remains.
env_keys = ( env_keys = (
"GITEA_OFFLINE", "GITEA_OFFLINE",
"GITEA_SKIP_AUTH_BOUNDARY", "GITEA_SKIP_AUTH_BOUNDARY",
@@ -316,15 +380,12 @@ class TestNativeProvenanceNonBypass(unittest.TestCase):
try: try:
for k in env_keys: for k in env_keys:
os.environ[k] = "1" os.environ[k] = "1"
exc = gitea_auth.GiteaAuthError( exc = gitea_auth.GiteaAuthError()
"HTTP 401: invalid username, password or token",
reason_code="auth_invalid_token",
)
c = boundary.classify_exception(exc) c = boundary.classify_exception(exc)
self.assertEqual(c["error_class"], "authentication") self.assertEqual(c["error_class"], "authentication")
result = boundary.to_call_tool_result(exc, log=False) r = boundary.to_call_tool_result(exc, log=False)
self.assertTrue(result.isError) self.assertTrue(r.isError)
self.assertEqual(result.structuredContent["reason_code"], "auth_invalid_token") self.assertEqual(r.structuredContent["reason_code"], "auth_invalid_token")
finally: finally:
for k, v in saved.items(): for k, v in saved.items():
if v is None: if v is None:
@@ -332,8 +393,7 @@ class TestNativeProvenanceNonBypass(unittest.TestCase):
else: else:
os.environ[k] = v os.environ[k] = v
def test_no_bypass_attribute_on_boundary(self): def test_no_bypass_surface(self):
"""Boundary module must not expose an offline bypass switch."""
for name in dir(boundary): for name in dir(boundary):
lower = name.lower() lower = name.lower()
self.assertFalse( self.assertFalse(
@@ -342,5 +402,25 @@ class TestNativeProvenanceNonBypass(unittest.TestCase):
) )
# ---------------------------------------------------------------------------
# api_reliability regressions still hold for non-401 paths
# ---------------------------------------------------------------------------
class TestApiReliabilityCompat(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_502_upstream_typed(self, mock_open):
mock_open.side_effect = http_error(502, "bad gateway secret=xyz")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertNotIn("secret=xyz", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_auth_header_never_in_error(self, mock_open):
mock_open.side_effect = http_error(400, "bad request")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()