fix: fail closed on an unwritable audit log, bound the rate limiter and harden startup

The audit logger is opened at startup (exit 2 on failure) and obtained before
every call, which is refused with audit_unavailable otherwise. Audit records only
take validated path and sudo values and use the request id as correlation id. The
rate limiter evicts idle keys, caps tracked keys, audits the first excess per key
and window and uses the settings passed to the app. FORWARDED_ALLOW_IPS is now a
setting, and startup warns about a missing host allow-list and unavailable OAuth.

src/aegis_gitea_mcp/config.py

@@ -128,6 +128,7 @@ class Settings(BaseSettings):
     mcp_bearer_token: SecretStr | None = Field(default=None)
     public_base_url: str | None = Field(default=None)
     allowed_hosts: Annotated[list[str], NoDecode] = Field(default_factory=list)
+    forwarded_allow_ips: str = Field(default="127.0.0.1")
 
     # Per-user OAuth (HTTP only)
     oauth_enabled: bool = Field(default=False)
@@ -199,6 +200,28 @@ class Settings(BaseSettings):
             raise ValueError("PUBLIC_BASE_URL must use https unless the host is loopback")
         return normalized
 
+    @field_validator("forwarded_allow_ips", mode="before")
+    @classmethod
+    def _validate_forwarded_allow_ips(cls, value: Any) -> Any:
+        """Accept ``*`` or a comma list of IP addresses and networks (blank means the default)."""
+        if not isinstance(value, str):
+            return value
+        items = [item.strip() for item in value.split(",") if item.strip()]
+        if not items:
+            return "127.0.0.1"
+        if "*" in items and len(items) > 1:
+            raise ValueError("FORWARDED_ALLOW_IPS: '*' cannot be combined with other entries")
+        for item in items:
+            if item == "*":
+                continue
+            try:
+                ipaddress.ip_network(item, strict=False)
+            except ValueError:
+                raise ValueError(
+                    "FORWARDED_ALLOW_IPS must be '*' or a comma list of IP addresses or networks"
+                ) from None
+        return ",".join(items)
+
     @field_validator("mcp_path")
     @classmethod
     def _validate_mcp_path(cls, value: str) -> str:

src/aegis_gitea_mcp/rate_limit.py

@@ -1,14 +1,29 @@
-"""In-memory request rate limiting for MCP endpoints."""
+"""In-memory request rate limiting for the HTTP MCP endpoint.
+
+The limiter runs before authentication, so every key it sees is attacker
+controlled. Memory is therefore bounded: a key whose window has emptied is
+dropped, and each limiter holds at most ``max_keys`` keys (least recently used
+keys are evicted first). Per-IP limiting always applies; the per-token bucket is
+keyed by a hash of the Authorization header and is only a second line.
+"""
 
 from __future__ import annotations
 
 import hashlib
+import logging
 import time
-from collections import defaultdict, deque
+from collections import OrderedDict, deque
 from dataclasses import dataclass
 
 from aegis_gitea_mcp.audit import get_audit_logger
-from aegis_gitea_mcp.config import get_settings
+from aegis_gitea_mcp.config import Settings
+
+logger = logging.getLogger(__name__)
+
+#: Maximum number of distinct client IPs tracked at once.
+MAX_IP_KEYS = 100_000
+#: Maximum number of distinct Authorization hashes tracked at once.
+MAX_TOKEN_KEYS = 10_000
 
 
 @dataclass(frozen=True)
@@ -20,25 +35,52 @@ class RateLimitDecision:
 
 
 class SlidingWindowLimiter:
-    """Sliding-window limiter keyed by arbitrary identifiers."""
+    """Sliding-window limiter keyed by arbitrary identifiers, with bounded memory."""
 
-    def __init__(self, max_requests: int, window_seconds: int) -> None:
-        """Initialize a fixed-window limiter.
+    def __init__(self, max_requests: int, window_seconds: int, max_keys: int = MAX_IP_KEYS) -> None:
+        """Initialize a sliding-window limiter.
 
         Args:
             max_requests: Maximum allowed requests within window.
             window_seconds: Rolling time window length.
+            max_keys: Maximum number of tracked keys; the least recently used key is
+                evicted beyond it.
         """
         self.max_requests = max_requests
         self.window_seconds = window_seconds
-        self._events: dict[str, deque[float]] = defaultdict(deque)
+        self.max_keys = max_keys
+        self._events: OrderedDict[str, deque[float]] = OrderedDict()
+        self._reported: dict[str, float] = {}
+        self._next_sweep = 0.0
+
+    def __len__(self) -> int:
+        return len(self._events)
+
+    def _sweep(self, now: float) -> None:
+        """Drop keys with no event inside the window (at most once per window)."""
+        if now < self._next_sweep:
+            return
+        self._next_sweep = now + self.window_seconds
+        boundary = now - self.window_seconds
+        for key in [k for k, events in self._events.items() if not events or events[-1] < boundary]:
+            del self._events[key]
+        for key in [k for k in self._reported if k not in self._events]:
+            del self._reported[key]
 
     def allow(self, key: str) -> bool:
         """Check and record request for the provided key."""
         now = time.time()
         boundary = now - self.window_seconds
+        self._sweep(now)
 
-        events = self._events[key]
+        events = self._events.get(key)
+        if events is None:
+            events = self._events[key] = deque()
+            while len(self._events) > self.max_keys:
+                evicted, _ = self._events.popitem(last=False)
+                self._reported.pop(evicted, None)
+        else:
+            self._events.move_to_end(key)
         while events and events[0] < boundary:
             events.popleft()
 
@@ -48,63 +90,76 @@ class SlidingWindowLimiter:
         events.append(now)
         return True
 
+    def first_excess(self, key: str) -> bool:
+        """Return True once per window for a key that is over its limit.
+
+        Callers use it to audit only the first rejected request of a burst instead
+        of every one (which would let an attacker fill the audit log).
+        """
+        now = time.time()
+        reported_at = self._reported.get(key)
+        if reported_at is not None and now - reported_at < self.window_seconds:
+            return False
+        self._reported[key] = now
+        return True
+
 
 class RequestRateLimiter:
-    """Combined per-IP and per-token request limiter."""
+    """Combined per-IP and per-token request limiter.
 
-    def __init__(self) -> None:
-        """Initialize with current settings."""
-        settings = get_settings()
-        self._audit = get_audit_logger()
-        self._ip_limiter = SlidingWindowLimiter(settings.rate_limit_per_minute, 60)
-        self._token_limiter = SlidingWindowLimiter(settings.rate_limit_per_minute, 60)
+    Args:
+        settings: Application settings (``RATE_LIMIT_PER_MINUTE``).
+    """
+
+    def __init__(self, settings: Settings) -> None:
+        limit = settings.rate_limit_per_minute
+        self._ip_limiter = SlidingWindowLimiter(limit, 60, MAX_IP_KEYS)
+        self._token_limiter = SlidingWindowLimiter(limit, 60, MAX_TOKEN_KEYS)
+
+    @staticmethod
+    def _audit_excess(event_type: str, description: str, severity: str, client_ip: str) -> None:
+        """Record a limit hit; an unavailable audit log must not break request handling."""
+        try:
+            get_audit_logger().log_security_event(
+                event_type=event_type,
+                description=description,
+                severity=severity,
+                metadata={"client_ip": client_ip},
+            )
+        except Exception as exc:
+            logger.error("audit log write failed: %s", type(exc).__name__)
 
     def check(self, client_ip: str, token: str | None) -> RateLimitDecision:
         """Evaluate request against IP and token limits.
 
         Args:
             client_ip: Request source IP.
-            token: Optional authenticated API token.
+            token: Optional Authorization header value.
 
         Returns:
             Rate limit decision.
         """
         if not self._ip_limiter.allow(client_ip):
-            self._audit.log_security_event(
-                event_type="rate_limit_ip_exceeded",
-                description="Per-IP request rate limit exceeded",
-                severity="medium",
-                metadata={"client_ip": client_ip},
-            )
+            if self._ip_limiter.first_excess(client_ip):
+                self._audit_excess(
+                    "rate_limit_ip_exceeded",
+                    "Per-IP request rate limit exceeded",
+                    "medium",
+                    client_ip,
+                )
             return RateLimitDecision(False, "Per-IP rate limit exceeded")
 
         if token:
             # Hash token before using it as a key to avoid storing secrets in memory maps.
             token_key = hashlib.sha256(token.encode("utf-8")).hexdigest()
             if not self._token_limiter.allow(token_key):
-                self._audit.log_security_event(
-                    event_type="rate_limit_token_exceeded",
-                    description="Per-token request rate limit exceeded",
-                    severity="high",
-                    metadata={"client_ip": client_ip},
-                )
+                if self._token_limiter.first_excess(token_key):
+                    self._audit_excess(
+                        "rate_limit_token_exceeded",
+                        "Per-token request rate limit exceeded",
+                        "high",
+                        client_ip,
+                    )
                 return RateLimitDecision(False, "Per-token rate limit exceeded")
 
         return RateLimitDecision(True, "within limits")
-
-
-_rate_limiter: RequestRateLimiter | None = None
-
-
-def get_rate_limiter() -> RequestRateLimiter:
-    """Get global request limiter."""
-    global _rate_limiter
-    if _rate_limiter is None:
-        _rate_limiter = RequestRateLimiter()
-    return _rate_limiter
-
-
-def reset_rate_limiter() -> None:
-    """Reset global limiter (primarily for tests)."""
-    global _rate_limiter
-    _rate_limiter = None

src/aegis_gitea_mcp/server.py

@@ -347,6 +347,7 @@ class _Record:
     """Everything one audit entry needs, filled in as the call progresses."""
 
     tool: str
+    request_id: str = ""
     domain: str | None = None
     method: str | None = None
     path: str | None = None
@@ -405,10 +406,24 @@ class _Dispatcher:
     ) -> types.CallToolResult:
         """Handle ``tools/call``; every call produces exactly one audit entry."""
         started = time.perf_counter()
-        set_request_id(uuid.uuid4().hex)
+        request_id = uuid.uuid4().hex
+        set_request_id(request_id)
         arguments = params.arguments or {}
-        record = _Record(tool=params.name[:_MAX_UNKNOWN_NAME_CHARS])
+        record = _Record(tool=params.name[:_MAX_UNKNOWN_NAME_CHARS], request_id=request_id)
         record.argument_names = sorted(_clip(key) for key in arguments)[:_MAX_AUDIT_ARGUMENT_NAMES]
+        # Fail closed: the audit logger is obtained BEFORE anything runs, so a call is
+        # never executed (and its side effects never happen) without a way to record it.
+        try:
+            audit = self._audit_override or get_audit_logger()
+        except Exception as exc:
+            logger.error("audit log unavailable, refusing the call: %s", type(exc).__name__)
+            return _result(
+                _error_payload(
+                    "audit_unavailable",
+                    "The audit log is not writable, so the call was not executed",
+                ),
+                is_error=True,
+            )
         try:
             entry = self._registry.get(params.name)
             if entry is None:
@@ -426,7 +441,7 @@ class _Dispatcher:
                     message += f" ({type(exc).__name__})"
                 return _result(_error_payload("internal_error", message), is_error=True)
         finally:
-            self._audit(record, (time.perf_counter() - started) * 1000)
+            self._audit(audit, record, (time.perf_counter() - started) * 1000)
 
     async def _dispatch(
         self, entry: _Entry, arguments: dict[str, Any], record: _Record
@@ -434,20 +449,19 @@ class _Dispatcher:
         operation = entry.operation
         if operation is not None:
             record.method, record.path = operation.method, operation.path
-        else:
-            method, path = arguments.get("method"), arguments.get("path")
-            if isinstance(method, str):
-                record.method = method.upper()[:10]
-            if isinstance(path, str):
-                record.path = path[:_MAX_AUDIT_PATH_CHARS]
-        sudo = arguments.get("sudo")
-        record.sudo = sudo[:NAME_MAX_LENGTH] if isinstance(sudo, str) else None
 
         invalid = validate_arguments(self._registry.validator(entry.tool.name), arguments)
         if invalid is None and operation is not None:
             invalid = _upload_cap_error(operation, arguments, self._settings.max_binary_bytes)
         if invalid is not None:
             return self._failed(record, "invalid_argument", {"error": invalid})
+        # Only validated values reach the audit record: a rejected call may carry
+        # anything (including a secret) in its raw arguments.
+        sudo = arguments.get("sudo")
+        record.sudo = sudo[:NAME_MAX_LENGTH] if isinstance(sudo, str) else None
+        if operation is None:
+            record.method = arguments["method"]
+            record.path = arguments["path"][:_MAX_AUDIT_PATH_CHARS]
         if operation is None and self._settings.read_only and arguments["method"] != "GET":
             message = "READ_ONLY mode is on: gitea_api_request only accepts GET"
             return self._failed(record, "read_only", _error_payload("read_only", message))
@@ -492,15 +506,23 @@ class _Dispatcher:
             record.outcome = "success"
         return _result(outcome.payload, is_error=outcome.is_error, text=outcome.to_text())
 
-    def _audit(self, record: _Record, duration_ms: float) -> None:
+    @staticmethod
+    def _audit(audit: AuditLogger, record: _Record, duration_ms: float) -> None:
+        """Write the audit entry of a finished call.
+
+        A write failure here is logged and swallowed on purpose: the tool already
+        ran, its side effect (possibly a Gitea mutation) cannot be undone, and the
+        caller still has to learn the result. The fail-closed check happens before
+        execution, in :meth:`call_tool`.
+        """
         params: dict[str, Any] = {"argument_names": record.argument_names}
         if record.http_status is not None:
             params["http_status"] = record.http_status
         try:
-            audit = self._audit_override or get_audit_logger()
             audit.log_tool_invocation(
                 record.tool,
                 params=params,
+                correlation_id=record.request_id,
                 result_status="error" if record.is_error else "success",
                 domain=record.domain,
                 method=record.method,

src/aegis_gitea_mcp/transports.py

@@ -31,9 +31,10 @@ from starlette.routing import Route
 from starlette.types import ASGIApp, Receive, Scope, Send
 
 from aegis_gitea_mcp import __version__
+from aegis_gitea_mcp.audit import get_audit_logger
 from aegis_gitea_mcp.auth import CompositeVerifier, IdentityResolver, StaticBearerVerifier
 from aegis_gitea_mcp.config import Settings
-from aegis_gitea_mcp.rate_limit import get_rate_limiter
+from aegis_gitea_mcp.rate_limit import RequestRateLimiter
 from aegis_gitea_mcp.server import (
     SERVER_NAME,
     AegisServer,
@@ -134,9 +135,10 @@ class GiteaProbe:
 class RateLimitMiddleware:
     """ASGI middleware applying the per-IP and per-token limits to the MCP path."""
 
-    def __init__(self, app: ASGIApp, mcp_path: str) -> None:
+    def __init__(self, app: ASGIApp, mcp_path: str, limiter: RequestRateLimiter) -> None:
         self.app = app
         self._mcp_path = mcp_path
+        self._limiter = limiter
 
     async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
         if scope["type"] != "http" or scope["path"].rstrip("/") != self._mcp_path:
@@ -149,7 +151,7 @@ class RateLimitMiddleware:
             if name == b"authorization":
                 authorization = value.decode("latin-1")
                 break
-        decision = get_rate_limiter().check(client_ip, authorization)
+        decision = self._limiter.check(client_ip, authorization)
         if decision.allowed:
             await self.app(scope, receive, send)
             return
@@ -307,7 +309,9 @@ def build_http_app(
             await gitea_probe.aclose()
 
     app.router.lifespan_context = lifespan
-    app.add_middleware(RateLimitMiddleware, mcp_path=settings.mcp_path)
+    app.add_middleware(
+        RateLimitMiddleware, mcp_path=settings.mcp_path, limiter=RequestRateLimiter(settings)
+    )
     return app
 
 
@@ -333,10 +337,56 @@ def http_startup_errors(settings: Settings, extras: HttpAuthExtras | None) -> li
     return errors
 
 
+def http_startup_warnings(settings: Settings, extras: HttpAuthExtras | None) -> list[str]:
+    """List non-fatal startup problems of the HTTP server (printed to stderr)."""
+    warnings: list[str] = []
+    if (
+        not settings.is_loopback_bind()
+        and not settings.public_base_url
+        and not settings.allowed_hosts
+    ):
+        warnings.append(
+            f"MCP_HOST={settings.mcp_host} is not loopback and neither PUBLIC_BASE_URL nor "
+            "ALLOWED_HOSTS is set: requests whose Host header is not a loopback name or the "
+            "bind address are rejected with 421. Set PUBLIC_BASE_URL (the public https URL) "
+            "or ALLOWED_HOSTS (comma separated host names clients use)"
+        )
+    if settings.oauth_enabled and extras is None and settings.mcp_bearer_token is not None:
+        warnings.append(
+            "OAUTH_ENABLED=true but per-user OAuth is not available in this build: only "
+            "MCP_BEARER_TOKEN is accepted and every call runs as the GITEA_TOKEN owner"
+        )
+    if settings.forwarded_allow_ips == "*":
+        warnings.append(
+            "FORWARDED_ALLOW_IPS=*: X-Forwarded-For is trusted from any peer, so clients can "
+            "spoof their IP and dodge the per-IP rate limit. List the proxy addresses instead"
+        )
+    return warnings
+
+
+def _audit_startup_error(settings: Settings) -> str | None:
+    """Open the audit log now; return an error message when it is not writable.
+
+    A server that cannot record its calls must not start: the dispatcher refuses
+    calls without an audit log, so every call would fail anyway.
+    """
+    try:
+        get_audit_logger()
+    except OSError as exc:
+        reason = exc.strerror or type(exc).__name__
+        return (
+            f"cannot open the audit log {settings.resolved_audit_log_path} for append "
+            f"({reason}). Fix the permissions or point AUDIT_LOG_PATH (or DATA_DIR) at a "
+            "writable location"
+        )
+    return None
+
+
 def run_http(settings: Settings) -> int:
     """Run the streamable HTTP server; return a process exit code.
 
-    Returns 2 without starting when a startup check fails.
+    Returns 2 without starting when a startup check fails (including an audit
+    log that cannot be opened for append).
     """
     extras = load_oauth_components(settings) if settings.oauth_enabled else None
     errors = http_startup_errors(settings, extras)
@@ -344,6 +394,11 @@ def run_http(settings: Settings) -> int:
         for message in errors:
             _fail(message)
         return EXIT_USAGE
+    audit_error = _audit_startup_error(settings)
+    if audit_error is not None:
+        return _fail(audit_error)
+    for message in http_startup_warnings(settings, extras):
+        print(f"aegis-gitea-mcp: warning: {message}", file=sys.stderr)
     try:
         server = build_server(settings, resolver=extras.resolver if extras else None)
     except ServerConfigError as exc:
@@ -363,7 +418,7 @@ def run_http(settings: Settings) -> int:
         port=settings.mcp_port,
         log_level=settings.log_level.lower(),
         proxy_headers=True,
-        forwarded_allow_ips=getattr(settings, "forwarded_allow_ips", None) or "127.0.0.1",
+        forwarded_allow_ips=settings.forwarded_allow_ips,
         server_header=False,
     )
     return 0
@@ -377,11 +432,14 @@ async def _serve_stdio(server: AegisServer) -> None:
 def run_stdio(settings: Settings) -> int:
     """Run the local stdio server (single identity: the owner of ``GITEA_TOKEN``).
 
-    Returns 2 without starting when ``GITEA_TOKEN`` is missing or the tool
-    filters are invalid. All logging goes to stderr; stdout carries JSON-RPC only.
+    Returns 2 without starting when ``GITEA_TOKEN`` is missing, the audit log is
+    not writable or the tool filters are invalid. All logging goes to stderr; stdout carries JSON-RPC only.
     """
     if settings.gitea_token is None:
         return _fail("stdio needs GITEA_TOKEN; create a Gitea access token and set it in .env")
+    audit_error = _audit_startup_error(settings)
+    if audit_error is not None:
+        return _fail(audit_error)
     try:
         server = build_server(settings)
     except ServerConfigError as exc: