[CHORE]: VaultTokenBackend hardening follow-ups — HTTP client pooling, startup validation, path length cap

[rakdutta]

Summary

Three minor hardening items identified during the Round-4 review of PR #5599 (pluggable OAuth token storage). All are non-blocking — the primary implementation is correct and tested — but worth addressing as a follow-up for production readiness.

---

S9 — _vault_request opens a fresh httpx.AsyncClient on every retry attempt

File: mcpgateway/services/token_backends/vault_backend.py:201–203

VaultTokenBackend._vault_request() wraps every retry iteration in async with httpx.AsyncClient(…) as client, creating a new TLS connection per attempt. Under a degraded Vault this holds the caller's checked-out DB session open for up to ~25 s (3 attempts × 10 s timeout + back-off) and bypasses the gateway-wide connection pool.

# Current — new client per attempt
for attempt in range(3):
    async with httpx.AsyncClient(verify=self.tls_verify, timeout=10.0) as client:
        ...

Fix: Replace with the shared pooled singleton from mcpgateway/services/http_client_service.py:196 (get_http_client()) or use ResilientHttpClient from mcpgateway/utils/retry_manager.py:183 which already provides connection pooling with retry logic.

---

S14 — No startup validation when OAUTH_TOKEN_BACKEND=vault

File: mcpgateway/main.py (lifespan hook)

When OAUTH_TOKEN_BACKEND=vault, a missing VAULT_TOKEN or unreachable VAULT_ADDR is only detected on the first real OAuth request (500 error). The gateway starts cleanly with no warning, so operators have no signal that the Vault backend is misconfigured.

The ValueError guard in vault_backend.py:96–97 fires at instantiation time, not startup.

Fix: Add a _validate_vault_configuration() helper modelled on the existing validate_security_configuration() and _validate_uaid_security() functions, and call it from lifespan():

async def _validate_vault_configuration() -> None:
    if settings.oauth_token_backend != "vault":
        return
    if not settings.vault_token:
        logger.error(
            "OAUTH_TOKEN_BACKEND=vault but VAULT_TOKEN is not set — OAuth flows will fail at runtime."
        )
    if settings.vault_addr == "http://127.0.0.1:8200":
        logger.warning("VAULT_ADDR is the default localhost address — confirm this is intentional.")
    # Optional lightweight probe
    try:
        client = await get_http_client()
        resp = await client.get(f"{settings.vault_addr}/v1/sys/health", timeout=5.0)
        if resp.status_code not in (200, 429, 472, 473):
            logger.warning("Vault health check returned unexpected status %d", resp.status_code)
    except Exception as e:
        logger.warning("Vault unreachable at startup (%s): %s", settings.vault_addr, e)

Vault config variables to validate: VAULT_TOKEN (required), VAULT_ADDR (warn if localhost default), VAULT_KV_MOUNT (document default secret).

---

S15 — No length cap on team_id in Vault path construction helpers

File: mcpgateway/services/token_backends/vault_backend.py:128–178
(_construct_vault_path, _construct_metadata_path, _construct_credentials_path)

All three path helpers URL-encode team_id (correctly preventing path-separator injection) but accept it without a length cap. Today team_id is always a server-generated UUID (36 chars), so there is no practical risk. If team slugs or display names are ever allowed, an unbounded string could create excessively long Vault paths or trigger silent truncation.

# Current — no length guard (vault_backend.py:177)
team_segment = quote(team_id, safe="") if team_id else "shared"

Fix: Add a class-level constant and guard in all three helpers:

# vault_backend.py — class level
_MAX_TEAM_ID_LEN: int = 128  # UUIDs are 36 chars; 128 accommodates future slugs

# In each path helper, before quote():
if team_id and len(team_id) > self._MAX_TEAM_ID_LEN:
    raise ValueError(
        f"team_id exceeds maximum allowed length ({self._MAX_TEAM_ID_LEN}): got {len(team_id)} chars"
    )

---

Acceptance Criteria

• S9: _vault_request uses a shared/pooled HTTP client; no new TLS connection per retry; existing retry behaviour (3 attempts, exponential back-off, VaultConnectionError on failure) preserved.
• S14: Starting the gateway with OAUTH_TOKEN_BACKEND=vault and no VAULT_TOKEN emits an ERROR log at startup. Unreachable Vault emits a WARNING. Normal startup is unaffected.
• S15: All three path helpers raise ValueError when team_id exceeds the cap. Normal UUID-length inputs are unaffected. Unit tests cover both pass and fail cases.
• All existing tests continue to pass.

---

Related

• PR [FEATURE] Pluggable OAuth Token Storage with HashiCorp Vault Backend #5599 — pluggable OAuth token storage (Vault backend)
• mcpgateway/services/token_backends/vault_backend.py
• mcpgateway/services/http_client_service.py — get_http_client()
• mcpgateway/utils/retry_manager.py — ResilientHttpClient
• mcpgateway/main.py — lifespan / startup validation helpers