[rennf93]

Closes #1360

Up to date version of #1361, which closed when #1492 merged (its body says "resolve #1361", a closing keyword). Same branch, merged with main at 693777b, @burtenshaw's lifespan commits (67e82bd, ea5bbc9) included.

Summary

Opt-in fastapi-guard middleware wired into both app factories (create_fastapi_app and the gradio web-interface path), covering HTTP requests and WebSocket handshakes. Default off: with OPENENV_GUARD_ENABLED unset, attach_guard() is a no-op and env servers run exactly as before. Enabling without the extra installed raises a clear ImportError with the install command.

Passive mode and Redis outages (the open thread on #1361)

Decision: passive mode lets infrastructure failures through too, not only policy violations, matching upstream's "passive mode never blocks". Passive mode now sets fastapi-guard's redis_fail_open, and the WebSocket startup gate follows the same passive branch as every other refusal.

1. Passive mode, Redis unreachable at startup or after it: HTTP requests and WebSocket handshakes are logged and let through. Before this commit, HTTP answered 503 at startup and 500 after it, and a WebSocket was closed with 1013 at startup.
2. Active mode is unchanged: WebSocket handshakes are closed with 1013 (HTTP 403 on the upgrade), HTTP answers 503 with Retry-After while the guard can't initialize and 500 after a successful start. That 500 is fastapi-guard's fail_secure answer on 8.x; I'm changing it to 503 with Retry-After in fastapi-guard, OpenEnv needs no change for it.
3. Non-Redis detection errors keep their current behavior in both modes: no blanket close on 1013.
4. The module docstring states both modes.

Type of Change

• New feature
• Bug fix
• Breaking change
• Documentation
• New environment
• Refactoring

Alignment Checklist

• I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
• I have checked .claude/docs/INVARIANTS.md and no invariants are violated (no API signature or wire-type change, env-var only config, nothing secret logged)
• I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

• Not required (additive, default-off feature)
• RFC exists: #___
• RFC needed (will create before merge)

What you get when enabled

pip install "openenv[security]" plus OPENENV_GUARD_ENABLED=1:

• rate limiting with auto-ban: OPENENV_GUARD_RATE_LIMIT / _RATE_LIMIT_WINDOW (defaults 100 req / 60s), _AUTO_BAN_THRESHOLD / _AUTO_BAN_DURATION (defaults 10 / 300s); repeated 429s feed the ban (OPENENV_GUARD_RATE_LIMIT_AUTO_BAN=0 keeps 429-only)
• IP block/allow lists, CIDR supported: OPENENV_GUARD_BLOCKED_IPS / _ALLOWED_IPS
• user-agent blocking: OPENENV_GUARD_BLOCKED_USER_AGENTS
• penetration-attempt detection on path, query and headers; request bodies are model input, so body scanning is opt-in (OPENENV_GUARD_SCAN_BODY)
• WebSocket handshakes on /ws, /mcp, /harness and /ws/ui go through fastapi-guard's guard_websocket: ban check, IP and country lists, a rate-limit bucket separate from HTTP, and penetration detection on the handshake. A refused handshake is closed before accept(), so the client gets HTTP 403 on the upgrade and no socket. Messages on an accepted socket are not screened, and a ban issued mid-session applies from the next handshake. With Redis configured, the guard initializes at startup (the app lifespan runs inside fastapi-guard's guard_lifespan), and while Redis is unreachable handshakes are closed with 1013 in active mode
• passive/log-only mode (OPENENV_GUARD_PASSIVE_MODE), see above; optional security headers (OPENENV_GUARD_SECURITY_HEADERS), enforce-HTTPS (OPENENV_GUARD_ENFORCE_HTTPS), custom log file/format, and blocked/allowed countries plus cloud-provider blocking (countries require IPINFO_TOKEN)
• trusted proxies for client IP resolution: OPENENV_GUARD_TRUSTED_PROXIES / _TRUSTED_PROXY_DEPTH. No proxy is trusted by default, so X-Forwarded-For is ignored and the client is the TCP peer; behind a proxy, list it or every client shares the proxy's IP. X-Forwarded-Proto is only honored from a trusted proxy, so enforce-HTTPS behind a TLS-terminating proxy needs it listed too
• Redis for limits and bans shared across workers: OPENENV_GUARD_REDIS_URL, in-memory otherwise. The library's localhost default is disabled; an opt-in feature shouldn't require a running Redis
• /health, /schema, /metadata and the docs paths are excluded from detection checks so orchestrators can keep probing, but IP lists and rate limiting still apply on them (fastapi-guard's enforced_on_excluded_paths)

Test Plan

• pytest tests/core/test_guard_security.py: 23 passed on fastapi-guard 8.0.3 / guard-core 4.3.2 (the versions in uv.lock) and on 8.1.2 / 4.4.0 (latest on PyPI). The new test_passive_mode_admits_while_redis_unreachable fails on the previous head (the handshake is closed, WebSocketDisconnect).

• Live check after startup, uvicorn 0.54.0, websockets client 15.0.1, real Redis behind a TCP proxy that I close (outage) and reopen (recovery), /health and a /ws reset at each step:

  mode	Redis up	Redis down	Redis back
  active	200, observation	500, refused (403)	200, observation
  passive, previous head	200, observation	500, observation	200, observation
  passive, this PR	200, observation	200, observation	200, observation

  Same rows on 8.0.3 / 4.3.2 and on 8.1.2 / 4.4.0.

• usort, ruff format and ruff check clean on the changed files. On the whole tree, ruff format flags src/openenv/core/harness/README.md and usort flags tests/envs/test_grid_world.py and tests/envs/test_julia_env.py, all from main; this PR doesn't touch them.

• I didn't rerun the full suite locally for this round: the merge only brings main, and the new commit touches security.py and its tests. fix(qed): keep spawned verifier imports lightweight #1491 and test(qed): cover worker import isolation and busy-worker recovery #1492, which fix the test (3.11) qed timeout that was red on Add optional fastapi-guard security middleware for env servers (default off) #1361, are in the merge.

Claude Code Review

N/A

[rennf93]

Hi @burtenshaw, whenever you get a minute: this is the up to date version of #1361 you asked for, now merged with current main. The workflows need a maintainer approval to run, so CI hasn't had a go yet. Happy to adjust anything once it does.

[burtenshaw]

Reviewed 9e32d828fd4898b7be5e6937dc3c81621c14684f. No verified blocking code findings. Merge needs a maintainer decision on the RFC requirement and configuration scope.

Earlier findings from #1361 at this head:

• Duplicate middleware: resolved. Each factory gets one HTTP middleware and one WebSocket adapter; calling attach_guard again adds neither (security.py:222, security.py:234).
• Missing Redis dependency: refuted. Redis is a base dependency of guard-core; the extra is named security (pyproject.toml:66).
• Missing GeoIP handler: refuted. SecurityConfig constructs IPInfoManager from country rules and IPINFO_TOKEN; no external GeoIP service availability claim was tested (security.py:145).
• Forwarded HTTPS scheme: resolved. A listed proxy's forwarded HTTPS scheme is honored; an unlisted proxy's is ignored (security.py:134).
• Enable flag parsing and optional-test handling: resolved. on enables protection, invalid values raise, and dependency-independent tests run without the extra (security.py:61, test_guard_security.py:31).
• WebSocket bypass and WebSocket-first Redis initialization: resolved. Blocked peers are refused on registered /ws, /mcp, /harness, and /ws/ui routes. Active cold Redis failure refuses the first handshake; recovery creates Redis rate-limit keys (security.py:184, security.py:238).
• Passive policy refusals: resolved. Blocked-peer handshakes are logged and admitted (security.py:195).
• Lifespan state loss and missing context exit: resolved by the retained 67e82bd8 and ea5bbc93 changes. Startup, yielded state, same-task exit, shutdown, and failed-startup unwinding pass. Upstream resource release remains a separate limitation below (security.py:240).
• Passive Redis inconsistency: the implementation addresses the last open thread, discussion_r4379761663. The GitHub thread remains unresolved. Passive HTTP and WebSocket admission now agree during cold and warm outages (security.py:117, security.py:188). This matches upstream's documented passive-mode policy.

With OPENENV_GUARD_ENABLED unset, both factories make no guard, guard_core, or fastapi_guard import attempts. attach_guard preserves middleware, app state, and lifespan identity. HTTP, environment WebSockets, /web/, and /ws/ui remain usable. Enabling with the import unavailable raises the documented pip install "openenv[security]" ImportError in both factories (security.py:220, security.py:227).

Real Redis 7.2.5 and Uvicorn probes produced these results on fastapi-guard/guard-core 8.0.3/4.3.2 and 8.1.2/4.4.0:

• Active: cold outage gives HTTP 503 with Retry-After: 5 and a refused WebSocket upgrade, HTTP 403; an outage after initialization gives HTTP 500 and a refused upgrade. Redis recovery restores HTTP 200 and a WebSocket reset observation. The cold ASGI close code is 1013.
• Passive: cold and post-initialization outages give HTTP 200 and a WebSocket reset observation. Admission after a warm-start outage recovers with Redis.

Validation: all 23 PR tests pass on 8.0.3/4.1.0, 8.0.3/4.3.2, and 8.1.2/4.4.0. PYTHONPATH=src:envs uv run pytest tests/core/ -q gives 942 passed, 9 skipped. Changed Python files pass usort, ruff format, and ruff check; whole-tree ruff check passes. The full lint hook flags unchanged files outside this PR. No added debug artifacts were found. The observed CI snapshot has 13 successful checks and no pending or failed checks.

Suggestions and remaining limitations:

• security.py:242: upstream Redis cleanup is still incomplete. Running lifespan_redis_cleanup.py integration and upstream (below) against real Redis gives client counts 1 before entry, 2 inside, 2 after exit, and 1 after explicit RedisManager.close(), on both dependency pairs. OpenEnv invokes the public exit hook, but upstream guard_lifespan has no cleanup. Track and fix ownership/teardown upstream rather than closing a process-wide singleton from one app.
• security.py:207: add passive cold-start recovery coverage and an upstream retry fix. Run PYTHONPATH=src:envs uv run python live_redis_probe.py passive (below): after starting with Redis down and then restoring it, the handler remains absent, Redis keys stay empty, and later handshakes report temporary unavailability while being admitted. Cached fail-open initialization skips reconnecting on both dependency pairs. This limits passive-mode observation; it does not violate the chosen admission policy.
• web_interface.py:647: correct the mount-order comment. create_fastapi_app runs before the Gradio mount at line 638. App-level middleware still covers the mount.

Design questions for maintainers, not changes requested from you:

• .claude/skills/rfc-check/SKILL.md lists "New APIs in src/openenv/core/" and "New abstractions or design patterns" as needing an RFC. This PR adds attach_guard, an ASGI adapter, lifespan composition and 24 OPENENV_GUARD_* settings to core. Default-off operation keeps existing servers unchanged, and the existing Gym signatures, wire types and RFC 001/002/003/005 boundaries stay intact. Maintainers need to decide whether this needs an RFC or a waiver.
• CLAUDE.md asks core to avoid configuration options added "just in case". The GeoIP/cloud, HTTPS/security-header, logging-format, forwarded-scheme override and Redis-prefix settings go beyond the deployment needs in #1360. Maintainers may ask to trim the first version to what #1360 needs.

We will follow up here once those are decided.

lifespan_redis_cleanup.py

import asyncio
import json
import os
import subprocess
import sys
from importlib.metadata import version

import redis
import redis.asyncio as aioredis
from fastapi import FastAPI

from live_redis_probe import REDIS_SERVER, free_port, ping, wait_until


async def audit(port):
    for name in list(os.environ):
        if name.startswith("OPENENV_GUARD_"):
            del os.environ[name]
    os.environ.update(OPENENV_GUARD_ENABLED="1", OPENENV_GUARD_REDIS_URL=f"redis://127.0.0.1:{port}")
    from guard.lifespan import guard_lifespan
    from guard_core.handlers.redis_handler import RedisManager
    from openenv.core.env_server.security import attach_guard

    app = FastAPI()
    attach_guard(app)
    observer = aioredis.Redis(host="127.0.0.1", port=port)
    before = len(await observer.client_list())
    context = app.router.lifespan_context if sys.argv[1] == "integration" else guard_lifespan
    async with context(app):
        during = len(await observer.client_list())
    after = len(await observer.client_list())
    await RedisManager._instance.close()
    closed = len(await observer.client_list())
    await observer.aclose()
    print(json.dumps({
        "context": sys.argv[1], "fastapi-guard": version("fastapi-guard"),
        "guard-core": version("guard-core"), "clients_before": before,
        "clients_during": during, "clients_after": after,
        "clients_after_explicit_close": closed,
    }))
    assert (before, during, after, closed) == (1, 2, 2, 1)


port = free_port()
process = subprocess.Popen([
    str(REDIS_SERVER), "--bind", "127.0.0.1", "--port", str(port),
    "--save", "", "--appendonly", "no",
], stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT)
probe = redis.Redis(host="127.0.0.1", port=port)
try:
    wait_until(lambda: ping(probe))
    probe.close()
    asyncio.run(audit(port))
finally:
    process.terminate()
    process.wait(timeout=5)

live_redis_probe.py

import json
import logging
import os
from pathlib import Path
import runpy
import socket
import subprocess
import sys
import threading
import time
from importlib.metadata import version

import httpx
import redis
import uvicorn
from websockets.exceptions import InvalidStatus
from websockets.sync.client import connect

ROOT = Path(__file__).resolve().parent  # directory holding a redis-7.2.5 source build
REDIS_SERVER = ROOT / "redis-7.2.5/src/redis-server"


def free_port():
    with socket.socket() as sock:
        sock.bind(("127.0.0.1", 0))
        return sock.getsockname()[1]


def wait_until(predicate, timeout=15):
    deadline = time.monotonic() + timeout
    while time.monotonic() < deadline:
        if predicate():
            return
        time.sleep(0.05)
    raise TimeoutError("probe condition timed out")


def ping(client):
    try:
        return client.ping()
    except redis.ConnectionError:
        return False


def main():
    mode = sys.argv[1]
    passive = mode == "passive"
    initially_up = len(sys.argv) > 2 and sys.argv[2] == "warm"
    redis_port, app_port = free_port(), free_port()
    for name in list(os.environ):
        if name.startswith("OPENENV_GUARD_"):
            del os.environ[name]
    os.environ.update(
        OPENENV_GUARD_ENABLED="1",
        OPENENV_GUARD_PASSIVE_MODE="1" if passive else "0",
        OPENENV_GUARD_REDIS_URL=f"redis://127.0.0.1:{redis_port}/0",
        OPENENV_GUARD_RATE_LIMIT="1000",
    )
    namespace = runpy.run_path("tests/core/test_guard_security.py")
    app = namespace["create_fastapi_app"](
        namespace["GuardEnvironment"], namespace["GuardAction"], namespace["GuardObservation"]
    )
    server = uvicorn.Server(uvicorn.Config(
        app, host="127.0.0.1", port=app_port, log_level="warning", proxy_headers=False,
    ))
    worker = threading.Thread(target=server.run, daemon=True)
    redis_process = None
    redis_log = (ROOT / f"redis-{mode}.log").open("w")
    observer = redis.Redis(host="127.0.0.1", port=redis_port, socket_timeout=2)

    def start_redis():
        nonlocal redis_process
        redis_process = subprocess.Popen([
            str(REDIS_SERVER), "--bind", "127.0.0.1", "--port", str(redis_port),
            "--save", "", "--appendonly", "no",
        ], stdout=redis_log, stderr=subprocess.STDOUT)
        wait_until(lambda: ping(observer))

    def stop_redis():
        nonlocal redis_process
        if redis_process is not None:
            redis_process.terminate()
            redis_process.wait(timeout=5)
            redis_process = None

    def ws_reset():
        try:
            with connect(f"ws://127.0.0.1:{app_port}/ws", proxy=None, open_timeout=8, close_timeout=2) as ws:
                ws.send(json.dumps({"type": "reset", "data": {}}))
                message = json.loads(ws.recv(timeout=8))
                assert message["type"] == "observation", message
                return "observation"
        except InvalidStatus as exc:
            return exc.response.status_code

    def stage(label, expected_http, expected_ws):
        # The first application request in the cold stage is a WebSocket.
        ws = ws_reset()
        with httpx.Client(trust_env=False, timeout=8) as client:
            response = client.get(f"http://127.0.0.1:{app_port}/health")
        row = {"mode": mode, "stage": label, "http": response.status_code, "websocket": ws}
        from guard.lifespan import _find_security_redis_handler
        row["redis_handler_found"] = _find_security_redis_handler(app) is not None
        if response.headers.get("retry-after"):
            row["retry_after"] = response.headers["retry-after"]
        print(json.dumps(row), flush=True)
        assert response.status_code == expected_http, row
        assert ws == expected_ws, row

    try:
        if initially_up:
            start_redis()
        worker.start()
        wait_until(lambda: server.started or not worker.is_alive())
        assert worker.is_alive(), "uvicorn did not start"
        print(json.dumps({"fastapi-guard": version("fastapi-guard"), "guard-core": version("guard-core")}), flush=True)
        if not initially_up:
            stage("cold Redis down", 200 if passive else 503, "observation" if passive else 403)
            start_redis()
        stage("Redis available", 200, "observation")
        keys = sorted(k.decode() for k in observer.scan_iter("openenv_guard:*"))
        if initially_up or not passive:
            assert len(keys) >= 2, keys
        print(json.dumps({"mode": mode, "redis_keys": keys}), flush=True)
        stop_redis()
        stage("Redis outage after initialization", 200 if passive else 500, "observation" if passive else 403)
        start_redis()
        stage("Redis recovered", 200, "observation")
    finally:
        server.should_exit = True
        worker.join(timeout=15)
        assert not worker.is_alive(), "uvicorn did not stop"
        stop_redis()
        observer.close()
        redis_log.close()


if __name__ == "__main__":
    main()

[rennf93]

Thanks for going through all of it, and for the probes.

I fixed the mount-order comment in c7023c0. create_fastapi_app runs first, and the guard is app-level middleware, so it covers /web anyway.

The two Redis ones are on the fastapi-guard side, so I'm fixing them there and not in OpenEnv. guard_lifespan not closing its Redis client is already fixed for the next fastapi-guard release: through its own lifespan the client count now goes 1 before, 2 inside, 1 after exit. The cold start one I reproduced in fastapi-guard alone, and it comes from redis_fail_open=True, which passive mode sets here (security.py:117). When the first Redis connect fails, guard-core takes the no-Redis path and never retries, so bans and rate limits stay in-process until a restart. The fix goes into the same release... I'll link both here when they're out, and add the cold-start recovery test on this side once it ships.

On the RFC and the settings, it's your call. If you want an RFC I'll write it. If you'd rather start smaller, I can trim this first version down to what #1360 needs and move the rest (GeoIP/cloud, security headers, log format, the forwarded-scheme override, Redis prefix) to a follow-up.