Add optional fastapi-guard security middleware for env servers (default off) by rennf93 · Pull Request #1361 · openenv/OpenEnv

rennf93 · 2026-10-09T09:01:38Z

Implementation for the discussion in #1360.

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, so misconfiguration is loud instead of silent.

Type of Change

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; happy to write an RFC first if you'd rather)
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, on whenever the middleware is on; 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. In passive mode a handshake that would be refused is logged and accepted. With Redis configured, the guard initializes at startup (fastapi-guard's make_lifespan wraps the app lifespan), and while Redis is unreachable handshakes are closed with 1013, the WebSocket side of the HTTP 503
passive/log-only mode (OPENENV_GUARD_PASSIVE_MODE), 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. I explicitly disabled the library's localhost default; 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. That's fastapi-guard's enforced_on_excluded_paths design and I think it's the right call: excluded shouldn't mean free pass.

Test Plan

uv run --extra security pytest tests/core/test_guard_security.py: 19 passed on fastapi-guard 8.0.3 / guard-core 4.3.2 and on 8.1.2 / 4.4.0 (default-off no-op, enable-flag parsing, middleware wiring, blocked IP 403, IP lists on excluded paths, rate-limit 429, zero-trust defaults, env parsing, the websocket handshake refused with 1008 on /ws and /mcp, rate-limited, untouched when the guard is off, passive mode logging and accepting, startup init, and 1013 for a WebSocket-first request while Redis is unreachable). The websocket, passive and Redis tests fail on the code before them. Rate-limit state in the library is process-wide, so each test drives its own TEST-NET client IP to stay hermetic.
Against a real Redis, a WebSocket as the first request writes its rate-limit key to Redis.
Live check with uvicorn 0.54.0 and the websockets client: with my IP in OPENENV_GUARD_BLOCKED_IPS, /ws and /mcp answer the upgrade with 403; with the guard on and the IP allowed, or the guard off, /ws accepts and a reset returns an observation.
CI's test command (pytest tests/ with its ignores and markers) on 28985ec; the commits after it change only security.py and its tests: 3620 passed, 155 skipped, 3 failed, 1 collection error, none in the guard code. The collection error is smolagents missing from my venv (CI installs it). The failures are a harness process test hitting PermissionError: [Errno 1] Operation not permitted at harness/process.py:188 in my sandbox (same with this PR stashed), test_harbor_capture_server.py:104 and test_prepare_hf_deployment.py, neither of which imports the guard.
usort + ruff format + ruff check clean on src/ and tests/.

Claude Code Review

N/A

Off by default (OPENENV_GUARD_ENABLED=1 to enable): IP block/allow lists, rate limiting with auto-ban, user-agent blocking, and penetration detection, wired into both create_fastapi_app and the gradio web-interface path. Ships as a 'security' extra; in-memory state by default, Redis via OPENENV_GUARD_REDIS_URL.

cursor

Stale Bugbot comment from a previous run.

…loud blocking, and log options Brings the OPENENV_GUARD_* surface to parity: passive/log-only mode, optional security headers, enforce-HTTPS, blocked/allowed countries and cloud-provider blocking, custom log file/format, and IPINFO_TOKEN geo lookups. Country rules require IPINFO_TOKEN (guard-core resolves a geo handler from it).

cursor

Stale Bugbot comment from a previous run.

…enforcement, strict booleans, rate-limit auto-ban knob - web_interface no longer attaches guard twice (create_fastapi_app already covers it) and attach_guard is idempotent via app.state - enforce_https now sets trust_x_forwarded_proto by default so TLS-terminating proxies are read correctly (explicit override via OPENENV_GUARD_TRUST_X_FORWARDED_PROTO) - invalid boolean env values now fail loudly instead of silently disabling security - new OPENENV_GUARD_RATE_LIMIT_AUTO_BAN knob

cursor

Stale Bugbot comment from a previous run.

…security-middleware

cursor

Stale Bugbot comment from a previous run.

- Trusted proxies default to empty: trusting RFC1918 ranges let any LAN peer spoof X-Forwarded-For past the blocklist and rate limits. Behind a reverse proxy, list it in OPENENV_GUARD_TRUSTED_PROXIES. - Penetration-detection body scanning is now opt-in via OPENENV_GUARD_SCAN_BODY: request bodies are model input (observations, prompts, completions), not commands, so signature scans false-positived on ordinary content and rejected legitimate requests with 400. URL, query, and header screening stay on. - Rate-limit auto-ban defaults on; OPENENV_GUARD_RATE_LIMIT_AUTO_BAN=0 keeps 429-only enforcement. _env_bool gained a keyword-only default.

burtenshaw

Independent review: basic HTTP/WebSocket blocklist and rate-limit tests pass using the installed dependency. Pushed simple enable-flag and optional-test fixes. Two WebSocket behavior gaps remain; both reproduced on fastapi-guard 8.0.3 and 8.1.2 with guard-core 4.4.0, using real FastAPI TestClient lifespan and guard implementations. No workflow approvals or merge actions taken.

…ket handshakes

cursor

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit ce6d931. Configure here.}

cursor · 2026-10-26T16:28:41Z

+                    getattr(websocket.state, "client_ip", None),
+                    exc.reason,
+                )
+        await self.app(scope, receive, send)


Passive mode admits Redis-down sockets

Medium Severity

After Redis has initialized, an outage surfaces as WebSocketException and passive_mode accepts the handshake, so WebSocket sessions skip distributed limits while HTTP still returns 503. A missing handler already maps to 1013 even in passive mode.

Additional Locations (1)

src/openenv/core/env_server/security.py#L180-L187

^{Reviewed by Cursor Bugbot for commit ce6d931. Configure here.}

Independently reproduced on real Redis 7.2.5 + Uvicorn + WebSocket clients, with fastapi-guard/guard-core 8.0.3/4.1.0 and 8.1.2/4.4.0. Passive mode: cold Redis-down startup refuses the upgrade (HTTP 403, ASGI 1013); after successful initialization, stopping Redis allows /ws reset to return an observation. HTTP /health returns 500 in this post-initialization outage, not 503 (503 is the initialization failure path). Active mode refuses the upgrade after the same outage; recovery succeeds.

Leaving this unresolved without tightening passive-mode policy. Upstream documents passive mode as never blocking, and uses a distinct redis_fail_open/fail_secure policy for infrastructure failures; this PR's module documentation simultaneously promises passive acceptance and Redis-down refusal. We need an explicit decision whether OpenEnv's passive mode also bypasses infrastructure failures or only policy violations, then align cold/warm startup, HTTP/WS behavior, documentation, and tests. A blanket close on every 1013 would also change non-Redis detection-error behavior. No policy change was made in this round.

Upstream references: https://github.com/Guard-Core/fastapi-guard/blob/master/guard/.agents/skills/fastapi-guard/SKILL.md and https://rennf93.github.io/fastapi-guard/7.8.2/tutorial/redis-integration/caching/

rennf93 · 2026-10-26T21:51:47Z

@burtenshaw I think this got auto-closed by #1492: its body says "These tests do not ... resolve #1361 policy gates", GitHub reads "resolve #1361" as a closing keyword, and this closed 2 seconds after #1492 merged. Can you reopen it? I can't from my side.

Once it's back:

The only red check on f16842e is test (3.11): test_base_preset_negative_reward_for_wrong got reward 0.0 instead of -0.5 because the math_verify request timed out after 5s, twice. The same test passed on 3.12 in the same run. I think fix(qed): keep spawned verifier imports lightweight #1491 and test(qed): cover worker import isolation and busy-worker recovery #1492 fix exactly that, so merging main should clear it.
Teardown (r4379761642): agreed, that's upstream. guard_lifespan has no shutdown code in 8.0.3 or 8.1.2, and make_lifespan drops the state the app lifespan yields. I'm fixing both in fastapi-guard.

cursor Bot reviewed Oct 9, 2026

View reviewed changes

Comment thread src/openenv/core/env_server/web_interface.py Resolved

Comment thread pyproject.toml Resolved

cursor Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread src/openenv/core/env_server/security.py Resolved

Comment thread src/openenv/core/env_server/security.py Resolved

cursor Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread src/openenv/core/env_server/security.py Resolved

Merge remote-tracking branch 'upstream/main' into feat/fastapi-guard-…

9278c59

…security-middleware

cursor Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread src/openenv/core/env_server/security.py Resolved

rennf93 and others added 3 commits October 11, 2026 07:10

Merge branch 'main' into feat/fastapi-guard-security-middleware

6d31cc0

Merge branch 'main' into feat/fastapi-guard-security-middleware

162db5c

burtenshaw mentioned this pull request Oct 26, 2026

Opt-in request security for env servers (rate limiting, IP lists, payload screening) #1360

Open

rennf93 and others added 2 commits October 26, 2026 15:53

feat: guard WebSocket handshakes when the guard is enabled

28985ec

fix: validate guard enable flag and skip optional security tests

bce011e

burtenshaw requested changes Oct 26, 2026

View reviewed changes

Comment thread src/openenv/core/env_server/security.py Resolved

Comment thread src/openenv/core/env_server/security.py Resolved

rennf93 and others added 3 commits October 26, 2026 16:28

fix: initialize the guard at startup and honor passive mode on WebSoc…

350cf94

…ket handshakes

fix(security): preserve existing lifespan state (openenv#1361)

67e82bd

Merge branch 'main' into feat/fastapi-guard-security-middleware

ce6d931

cursor Bot reviewed Oct 26, 2026

View reviewed changes

burtenshaw added 2 commits October 26, 2026 17:42

fix: compose guard lifespan with application state and cleanup

ea5bbc9

Merge remote-tracking branch 'origin/main' into HEAD

f16842e

burtenshaw mentioned this pull request Oct 26, 2026

test(qed): cover worker import isolation and busy-worker recovery #1492

Merged

burtenshaw closed this in #1492 Oct 26, 2026

rennf93 mentioned this pull request Oct 27, 2026

Add optional fastapi-guard security middleware for env servers (default off) #1506

Open

5 of 12 tasks

Conversation

rennf93 commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of Change

Alignment Checklist

RFC Status

What you get when enabled

Test Plan

Claude Code Review

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

burtenshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot Oct 26, 2026

Choose a reason for hiding this comment

Passive mode admits Redis-down sockets

Uh oh!

burtenshaw Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

rennf93 commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rennf93 commented Oct 9, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

rennf93 commented Oct 26, 2026 •

edited

Loading