-
Notifications
You must be signed in to change notification settings - Fork 2
fix: pentest batch 2 — #13 #17 #18 #6 #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
2868fb4
995748c
441483c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| import sqlalchemy as sa | ||
| from fastapi import FastAPI, Request | ||
| from fastapi.exceptions import HTTPException as StarletteHTTPException | ||
| from fastapi.exceptions import RequestValidationError | ||
| from fastapi.middleware.cors import CORSMiddleware | ||
| from fastapi.responses import FileResponse, HTMLResponse, JSONResponse, RedirectResponse | ||
| from fastapi.staticfiles import StaticFiles | ||
|
|
@@ -280,6 +281,18 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: | |
| settings.base_url, | ||
| ) | ||
|
|
||
| # Warn when serving behind a proxy without trusted proxy config (H1/L1). | ||
| # Without ProxyHeadersMiddleware, Starlette uses http:// in redirects | ||
| # and rate-limiting keys on the proxy IP instead of the real client. | ||
| if settings.base_url.startswith("https://") and not settings.trusted_proxies: | ||
| logger.warning( | ||
| "EDICTUM_BASE_URL is HTTPS but EDICTUM_TRUSTED_PROXIES is not set. " | ||
| "Trailing-slash redirects will use http:// (downgrade) and rate " | ||
| "limiting will key on the proxy IP, not the real client. " | ||
| "Set EDICTUM_TRUSTED_PROXIES to your reverse proxy addresses " | ||
| "(e.g. '*' for Railway/Render, or specific CIDRs).", | ||
| ) | ||
|
|
||
| # Validate signing key secret early — log clearly if misconfigured | ||
| try: | ||
| settings.get_signing_secret() | ||
|
|
@@ -392,8 +405,8 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: | |
| CORSMiddleware, | ||
| allow_origins=[o.strip() for o in _settings.cors_origins.split(",") if o.strip()], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| allow_methods=["GET", "POST", "PUT", "DELETE", "OPTIONS"], | ||
|
||
| allow_headers=["Content-Type", "X-Requested-With", "Authorization", "X-Edictum-Agent-Id"], | ||
| ) | ||
|
|
||
| # Security response headers (HSTS, CSP, X-Frame-Options, etc.) | ||
|
|
@@ -441,6 +454,23 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: | |
| app.include_router(ai_usage.router) | ||
|
|
||
|
|
||
| # --- Validation error handler: strip Pydantic internals ---------------------- | ||
| @app.exception_handler(RequestValidationError) | ||
| async def validation_error_handler( | ||
| _request: Request, exc: RequestValidationError | ||
| ) -> JSONResponse: | ||
| """Return 422 with sanitized error details. | ||
|
|
||
| Strips ``ctx`` and ``type`` fields from Pydantic errors to avoid | ||
| leaking framework internals (L4 finding). | ||
| """ | ||
| sanitized = [ | ||
| {"loc": e.get("loc", []), "msg": e.get("msg", "Validation error")} | ||
| for e in exc.errors() | ||
| ] | ||
| return JSONResponse(status_code=422, content={"detail": sanitized}) | ||
|
|
||
|
|
||
| # --- 404 handler: redirect non-API paths to dashboard ------------------------- | ||
| @app.exception_handler(404) | ||
| async def not_found_handler( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
|
|
||
| from fastapi import APIRouter, Depends, HTTPException | ||
|
|
||
| from edictum_server.auth.dependencies import ( | ||
|
|
@@ -13,6 +15,13 @@ | |
|
|
||
| router = APIRouter(prefix="/api/v1/bundles", tags=["evaluate"]) | ||
|
|
||
| # Maximum time allowed for a single evaluation (seconds). | ||
| _EVALUATE_TIMEOUT_SECONDS = 5.0 | ||
|
|
||
| # Cap concurrent evaluations so timed-out threads (which can't be killed) | ||
| # don't exhaust the default ThreadPoolExecutor. | ||
| _EVALUATE_SEMAPHORE = asyncio.Semaphore(4) | ||
|
|
||
|
|
||
| @router.post("/evaluate", response_model=EvaluateResponse) | ||
| async def evaluate( | ||
|
|
@@ -23,14 +32,34 @@ async def evaluate( | |
|
|
||
| This is a development-time endpoint for testing contracts in the dashboard. | ||
| It is never called by agents during production execution. | ||
| Evaluation runs in a thread with a timeout to prevent DoS via complex YAML. | ||
| Concurrent evaluations are capped to prevent thread pool exhaustion. | ||
| """ | ||
| try: | ||
| return evaluate_contracts( | ||
| yaml_content=body.yaml_content, | ||
| tool_name=body.tool_name, | ||
| tool_args=body.tool_args, | ||
| environment=body.environment, | ||
| principal_input=body.principal, | ||
| await asyncio.wait_for(_EVALUATE_SEMAPHORE.acquire(), timeout=1.0) | ||
| except TimeoutError as exc: | ||
| raise HTTPException( | ||
| status_code=429, | ||
| detail="Too many concurrent evaluations. Try again shortly.", | ||
| ) from exc | ||
| try: | ||
| return await asyncio.wait_for( | ||
| asyncio.to_thread( | ||
| evaluate_contracts, | ||
| yaml_content=body.yaml_content, | ||
| tool_name=body.tool_name, | ||
| tool_args=body.tool_args, | ||
| environment=body.environment, | ||
| principal_input=body.principal, | ||
| ), | ||
| timeout=_EVALUATE_TIMEOUT_SECONDS, | ||
| ) | ||
| except TimeoutError as exc: | ||
| raise HTTPException( | ||
| status_code=422, | ||
| detail="Evaluation timed out — contract bundle may be too complex.", | ||
| ) from exc | ||
| except ValueError as exc: | ||
| raise HTTPException(status_code=422, detail=str(exc)) from exc | ||
| finally: | ||
|
||
| _EVALUATE_SEMAPHORE.release() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restricting CORS methods to
GET/POST/PUT/DELETE/OPTIONSdropsPATCH, but the app exposes PATCH routes (for example assignment rules and agent registrations) and the dashboard client calls them (dashboard/src/lib/api/agents.ts), so those browser requests will fail preflight in any cross-origin deployment even though they previously worked with wildcard methods.Useful? React with 👍 / 👎.