v1.7.12: security hardening — CORS fix, security headers, fail-closed rate limiter, OpenAPI docs disabled by default, config auth privacy, webhook validation
This commit is contained in:
@@ -76,6 +76,9 @@ class Settings(BaseSettings):
|
||||
RATE_LIMIT_REQUESTS: int = 120
|
||||
RATE_LIMIT_WINDOW_SECONDS: int = 60
|
||||
|
||||
# Security / docs exposure
|
||||
DOCS_ENABLED: bool = False
|
||||
|
||||
|
||||
_settings = Settings()
|
||||
|
||||
@@ -127,3 +130,5 @@ WEBHOOK_CLIENT_SECRET = _settings.WEBHOOK_CLIENT_SECRET
|
||||
RATE_LIMIT_ENABLED = _settings.RATE_LIMIT_ENABLED
|
||||
RATE_LIMIT_REQUESTS = _settings.RATE_LIMIT_REQUESTS
|
||||
RATE_LIMIT_WINDOW_SECONDS = _settings.RATE_LIMIT_WINDOW_SECONDS
|
||||
|
||||
DOCS_ENABLED = _settings.DOCS_ENABLED
|
||||
|
||||
@@ -7,7 +7,14 @@ from pathlib import Path
|
||||
|
||||
import structlog
|
||||
from audit_trail import log_action
|
||||
from config import AI_FEATURES_ENABLED, AUTH_ENABLED, CORS_ORIGINS, ENABLE_PERIODIC_FETCH, FETCH_INTERVAL_MINUTES
|
||||
from config import (
|
||||
AI_FEATURES_ENABLED,
|
||||
AUTH_ENABLED,
|
||||
CORS_ORIGINS,
|
||||
DOCS_ENABLED,
|
||||
ENABLE_PERIODIC_FETCH,
|
||||
FETCH_INTERVAL_MINUTES,
|
||||
)
|
||||
from database import setup_indexes
|
||||
from fastapi import FastAPI, HTTPException, Request
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
@@ -51,20 +58,28 @@ def configure_logging():
|
||||
configure_logging()
|
||||
logger = structlog.get_logger("aoc.fetcher")
|
||||
|
||||
app = FastAPI()
|
||||
# Disable OpenAPI docs in production by default
|
||||
app = FastAPI(
|
||||
docs_url="/docs" if DOCS_ENABLED else None,
|
||||
redoc_url="/redoc" if DOCS_ENABLED else None,
|
||||
openapi_url="/openapi.json" if DOCS_ENABLED else None,
|
||||
)
|
||||
|
||||
# CORS: warn if wildcard is used with auth enabled, but do not break deployments
|
||||
# CORS: when auth is enabled, never allow credentials with wildcard origins
|
||||
_effective_cors = CORS_ORIGINS
|
||||
_cors_credentials = True
|
||||
if AUTH_ENABLED and "*" in _effective_cors:
|
||||
logger.warning(
|
||||
"CORS wildcard (*) is insecure when AUTH_ENABLED=true. Set CORS_ORIGINS to your actual origin(s) in production."
|
||||
"CORS wildcard (*) is insecure with AUTH_ENABLED=true and allow_credentials. "
|
||||
"Disabling credentials. Set CORS_ORIGINS to your actual origin(s)."
|
||||
)
|
||||
_cors_credentials = False
|
||||
|
||||
app.add_middleware(CorrelationIdMiddleware)
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=_effective_cors,
|
||||
allow_credentials=True,
|
||||
allow_credentials=_cors_credentials,
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
)
|
||||
@@ -81,7 +96,7 @@ async def prometheus_middleware(request: Request, call_next):
|
||||
|
||||
|
||||
@app.middleware("http")
|
||||
async def cache_control_middleware(request: Request, call_next):
|
||||
async def security_headers_middleware(request: Request, call_next):
|
||||
response = await call_next(request)
|
||||
# Prevent caching of HTML and API responses by default
|
||||
if request.url.path.startswith("/api/") or request.url.path in ("/", "/index.html"):
|
||||
@@ -100,6 +115,13 @@ async def cache_control_middleware(request: Request, call_next):
|
||||
"img-src 'self' data:; "
|
||||
"font-src 'self' data:;"
|
||||
)
|
||||
# Additional security headers
|
||||
response.headers["X-Content-Type-Options"] = "nosniff"
|
||||
response.headers["X-Frame-Options"] = "DENY"
|
||||
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
|
||||
response.headers["Permissions-Policy"] = (
|
||||
"accelerometer=(), camera=(), geolocation=(), gyroscope=(), magnetometer=(), microphone=(), payment=(), usb=()"
|
||||
)
|
||||
return response
|
||||
|
||||
|
||||
|
||||
@@ -79,4 +79,5 @@ async def check_rate_limit(request: Request):
|
||||
except RateLimitExceeded:
|
||||
raise
|
||||
except Exception as exc:
|
||||
logger.warning("Rate limiter Redis error; allowing request", error=str(exc))
|
||||
logger.warning("Rate limiter Redis error; failing closed", error=str(exc))
|
||||
raise RateLimitExceeded(retry_after=60) from None
|
||||
|
||||
@@ -18,8 +18,8 @@ def auth_config():
|
||||
logger.debug("Auth config requested", auth_enabled=AUTH_ENABLED)
|
||||
return {
|
||||
"auth_enabled": AUTH_ENABLED,
|
||||
"tenant_id": AUTH_TENANT_ID,
|
||||
"client_id": AUTH_CLIENT_ID,
|
||||
"tenant_id": AUTH_TENANT_ID if AUTH_ENABLED else "",
|
||||
"client_id": AUTH_CLIENT_ID if AUTH_ENABLED else "",
|
||||
"scope": AUTH_SCOPE,
|
||||
"redirect_uri": None, # frontend uses window.location.origin by default
|
||||
}
|
||||
|
||||
@@ -17,7 +17,15 @@ async def graph_webhook(request: Request):
|
||||
if validation_token:
|
||||
# Microsoft sends validationToken as a query param during subscription creation.
|
||||
# Echo it back as plain text to prove endpoint ownership.
|
||||
return Response(content=validation_token, media_type="text/plain")
|
||||
# Validate to prevent content injection if endpoint is hit directly.
|
||||
if len(validation_token) > 1024 or not validation_token.isascii():
|
||||
logger.warning("Invalid validationToken rejected", length=len(validation_token))
|
||||
return Response(status_code=400)
|
||||
return Response(
|
||||
content=validation_token,
|
||||
media_type="text/plain",
|
||||
headers={"X-Content-Type-Options": "nosniff"},
|
||||
)
|
||||
|
||||
try:
|
||||
body = await request.json()
|
||||
|
||||
@@ -51,18 +51,32 @@ def client(mock_events_collection, mock_watermarks_collection, monkeypatch):
|
||||
|
||||
# Mock Redis so tests don't require a running Redis server
|
||||
class FakeRedis:
|
||||
_store = {}
|
||||
|
||||
async def get(self, key):
|
||||
return None
|
||||
return self._store.get(key)
|
||||
|
||||
async def setex(self, key, ttl, value):
|
||||
self._store[key] = value
|
||||
|
||||
async def incr(self, key):
|
||||
self._store[key] = self._store.get(key, 0) + 1
|
||||
return self._store[key]
|
||||
|
||||
async def expire(self, key, ttl):
|
||||
pass
|
||||
|
||||
async def fake_get_arq_pool():
|
||||
return FakeRedis()
|
||||
|
||||
async def fake_get_redis():
|
||||
return FakeRedis()
|
||||
|
||||
monkeypatch.setattr("redis_client.get_arq_pool", fake_get_arq_pool)
|
||||
monkeypatch.setattr("redis_client.get_redis", fake_get_redis)
|
||||
monkeypatch.setattr("routes.ask.get_arq_pool", fake_get_arq_pool)
|
||||
monkeypatch.setattr("routes.jobs.get_redis", fake_get_arq_pool)
|
||||
monkeypatch.setattr("routes.jobs.get_redis", fake_get_redis)
|
||||
monkeypatch.setattr("rate_limiter.get_redis", fake_get_redis)
|
||||
|
||||
from main import app
|
||||
|
||||
|
||||
Reference in New Issue
Block a user