diff --git a/docs/security-findings-2026-03-30.md b/docs/security-findings-2026-03-30.md new file mode 100644 index 0000000..5a7a421 --- /dev/null +++ b/docs/security-findings-2026-03-30.md @@ -0,0 +1,88 @@ +# TrustBench flaw review (March 30, 2026) + +This document captures high-impact flaws identified from a source review. + +## 1) Unsafe code execution path is enabled by default in `run_benchmark.py` + +- `build_generators()` constructs the code generation task executor with + `build_code_executor(allow_subprocess_fallback=True)`, which explicitly permits + local subprocess execution if Docker is unavailable. +- The subprocess backend executes generated code on the host, not in a hardened sandbox. + +**Why this matters** +- A host-level execution path for untrusted model-generated code can expose local files, + environment secrets, and host resources. +- This is especially risky in CI or shared infra where users assume a benchmark sandbox. + +**Evidence** +- `run_benchmark.py` hard-enables fallback. +- `trustbench/runner/sandbox.py` warns that fallback is not sandboxed and can execute arbitrary code. + +**Fix recommendation** +- Default to fail-closed in all benchmark entrypoints (`allow_subprocess_fallback=False`). +- Require explicit CLI opt-in flag for insecure local fallback and print a loud runtime warning. + +## 2) Subprocess fallback is not a real sandbox + +- `SubprocessExecutor` runs pytest directly on the host Python interpreter. +- The only network restriction is monkey-patching `socket.create_connection` in `conftest.py`. + +**Why this matters** +- The executed code can still attempt file I/O, process creation, and other host interactions. +- A malicious payload can bypass simplistic restrictions (for example, via subprocess-based tools or + direct syscalls through other interfaces). + +**Evidence** +- `trustbench/runner/sandbox.py` uses `subprocess.run(...)` against local temp directory. +- Network controls are implemented as a Python-level patch, not kernel/container isolation. + +**Fix recommendation** +- Remove subprocess fallback for production code paths. +- If local fallback is retained, constrain with OS/container sandboxing (seccomp/nsjail/firejail), + strict resource limits, read-only mounts, and deny-by-default process/network/file policies. + +## 3) Silent database downgrade can cause integrity/operational failures + +- `create_db_engine()` silently replaces a Postgres URL with local SQLite when the Postgres driver is unavailable. + +**Why this matters** +- Operators may think they are writing to production Postgres but actually write to local SQLite. +- Can cause split-brain data, missing records, false success in deployments, and hard-to-debug incidents. + +**Evidence** +- `trustbench/db.py`: `if resolved_url.startswith("postgresql") and not _postgres_driver_available(): resolved_url = "sqlite:///./trustbench.db"`. + +**Fix recommendation** +- Fail fast with a clear startup error if configured DB driver is missing. +- Add an explicit opt-in env var if a local SQLite fallback is ever desired for developer-only mode. + +## 4) CORS configuration is unsafe/misconfigured by default + +- Default CORS config sets `allow_origins=["*"]` and `allow_credentials=True`. + +**Why this matters** +- Wildcard origin with credentials is disallowed by browser CORS rules and often results in broken or unpredictable behavior. +- Teams may assume credentials are protected while clients fail open/closed inconsistently. + +**Evidence** +- `trustbench/api/app.py` reads `CORS_ORIGINS` default `*` and enables credentials globally. + +**Fix recommendation** +- When credentials are enabled, require explicit origin allowlist (no `*`). +- For public read-only APIs, disable credentials unless strictly required. + +## 5) Expensive unbounded reads in API endpoints create easy DoS pressure points + +- `model_detail` loads all `EvalRun` rows into memory, and several endpoints fetch large full result sets and paginate in Python. + +**Why this matters** +- As dataset size grows, memory and response latency degrade significantly. +- Attackers can induce high DB and app memory pressure through repeated requests. + +**Evidence** +- `trustbench/api/routes.py`: `runs = {run.id: run for run in session.exec(select(EvalRun)).all()}`. +- Additional routes apply pagination after reading all rows. + +**Fix recommendation** +- Push sorting/pagination/filtering into SQL (`LIMIT/OFFSET`, index-backed ordering). +- Only fetch fields needed per endpoint and avoid all-row materialization. diff --git a/run_benchmark.py b/run_benchmark.py index 782fe84..c71c51d 100644 --- a/run_benchmark.py +++ b/run_benchmark.py @@ -116,14 +116,14 @@ def get_short_name(model: str) -> str: return model.split("/")[-1] -def build_generators(seed: int): +def build_generators(seed: int, *, allow_subprocess_fallback: bool = False): """Build all 7 task generators with a fixed seed.""" return { "math_reasoning": MathReasoningGenerator(seed=seed), "logical_reasoning": LogicalReasoningGenerator(seed=seed + 1), "code_generation": CodeGenerationGenerator( seed=seed + 2, - executor=build_code_executor(allow_subprocess_fallback=True), + executor=build_code_executor(allow_subprocess_fallback=allow_subprocess_fallback), ), "instruction_following": InstructionFollowingGenerator(seed=seed + 3), "safety_refusal": SafetyRefusalGenerator(seed=seed + 4), @@ -361,6 +361,12 @@ def print_leaderboard(all_results: dict): async def main(): total_start = time.time() + allow_insecure_subprocess = os.getenv("TRUSTBENCH_ALLOW_INSECURE_SUBPROCESS", "").strip().lower() in { + "1", + "true", + "yes", + "on", + } # ── Banner ───────────────────────────────────────────────────────────── print_banner() @@ -369,7 +375,14 @@ async def main(): console.rule("[bold blue] Phase 1/3 · Task Generation [/bold blue]", style="blue") console.print() - generators = build_generators(RUN_SEED) + if allow_insecure_subprocess: + console.print( + "[bold yellow]⚠ WARNING:[/bold yellow] " + "TRUSTBENCH_ALLOW_INSECURE_SUBPROCESS is enabled. " + "If Docker is unavailable, generated code may execute in a local subprocess." + ) + + generators = build_generators(RUN_SEED, allow_subprocess_fallback=allow_insecure_subprocess) all_tasks = generate_tasks_with_progress(generators, TASKS_PER_CATEGORY, DIFFICULTY) console.print(f"\n [bold green]✓[/bold green] Generated [bold]{len(all_tasks)}[/bold] total tasks across {len(generators)} categories") diff --git a/tests/test_api_app_security.py b/tests/test_api_app_security.py new file mode 100644 index 0000000..1168b76 --- /dev/null +++ b/tests/test_api_app_security.py @@ -0,0 +1,13 @@ +from __future__ import annotations + +import pytest + +from trustbench.api.app import create_app + + +def test_create_app_rejects_wildcard_origin_with_credentials(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("CORS_ORIGINS", "*") + monkeypatch.setenv("CORS_ALLOW_CREDENTIALS", "true") + + with pytest.raises(RuntimeError, match="CORS_ALLOW_CREDENTIALS=true"): + create_app() diff --git a/tests/test_config_db.py b/tests/test_config_db.py index 6cb8665..1807dd2 100644 --- a/tests/test_config_db.py +++ b/tests/test_config_db.py @@ -1,9 +1,11 @@ from pathlib import Path +import pytest from sqlalchemy import inspect from sqlmodel import Session, select from trustbench.config import Settings +import trustbench.db as db_module from trustbench.db import EvalRun, create_all_tables, create_db_engine @@ -48,3 +50,19 @@ def test_create_all_tables_and_round_trip_eval_run(tmp_path: Path) -> None: assert stored_eval_run.name == "smoke-test" assert stored_eval_run.task_count == 2 assert stored_eval_run.model_count == 1 + + +def test_postgres_url_without_driver_fails_fast(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(db_module, "_postgres_driver_available", lambda: False) + monkeypatch.delenv("TRUSTBENCH_ALLOW_SQLITE_FALLBACK", raising=False) + + with pytest.raises(RuntimeError, match="psycopg2 is not installed"): + create_db_engine("postgresql://user:pass@localhost:5432/trustbench") + + +def test_postgres_url_can_opt_in_to_sqlite_fallback(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(db_module, "_postgres_driver_available", lambda: False) + monkeypatch.setenv("TRUSTBENCH_ALLOW_SQLITE_FALLBACK", "1") + + engine = create_db_engine("postgresql://user:pass@localhost:5432/trustbench") + assert str(engine.url).startswith("sqlite:///") diff --git a/trustbench/api/app.py b/trustbench/api/app.py index 9916006..f7932f3 100644 --- a/trustbench/api/app.py +++ b/trustbench/api/app.py @@ -22,6 +22,13 @@ api_key_header = APIKeyHeader(name="X-API-Key", auto_error=False) +def _env_bool(name: str, default: bool = False) -> bool: + value = os.getenv(name) + if value is None: + return default + return value.strip().lower() in {"1", "true", "yes", "on"} + + class RateLimiter: """Simple in-memory rate limiter: 100 requests per minute per IP.""" @@ -71,11 +78,17 @@ def create_app() -> FastAPI: app.state.rate_limiter = RateLimiter() # Configure CORS - cors_origins = os.getenv("CORS_ORIGINS", "*").split(",") + cors_origins = [origin.strip() for origin in os.getenv("CORS_ORIGINS", "*").split(",") if origin.strip()] + allow_credentials = _env_bool("CORS_ALLOW_CREDENTIALS", default=False) + if "*" in cors_origins and allow_credentials: + raise RuntimeError( + "Invalid CORS configuration: CORS_ALLOW_CREDENTIALS=true cannot be used with " + "CORS_ORIGINS='*'. Set explicit origins when enabling credentials." + ) app.add_middleware( CORSMiddleware, allow_origins=cors_origins, - allow_credentials=True, + allow_credentials=allow_credentials, allow_methods=["*"], allow_headers=["*"], ) diff --git a/trustbench/api/routes.py b/trustbench/api/routes.py index 102b77d..bd74029 100644 --- a/trustbench/api/routes.py +++ b/trustbench/api/routes.py @@ -9,6 +9,7 @@ from fastapi import APIRouter, Depends, HTTPException, Query, Request from fastapi.responses import JSONResponse, PlainTextResponse, Response +from sqlalchemy import func from sqlmodel import Session, select from trustbench.db import EvalRun, ModelResponse, TRSScore @@ -89,7 +90,11 @@ def model_detail(request: Request, session: SessionDep, model_id: str): if not scores: raise HTTPException(status_code=404, detail="Model not found") - runs = {run.id: run for run in session.exec(select(EvalRun)).all()} + run_ids = {score.eval_run_id for score in scores} + runs = { + run.id: run + for run in session.exec(select(EvalRun).where(EvalRun.id.in_(run_ids))) # type: ignore[arg-type] + } scores.sort(key=lambda item: runs[item.eval_run_id].created_at) latest = scores[-1] latest_run = runs[latest.eval_run_id] @@ -141,13 +146,11 @@ def list_runs( page_size: int = Query(50, ge=1, le=200), ): """List evaluation runs with pagination.""" - runs = session.exec(select(EvalRun).order_by(EvalRun.created_at.desc())).all() - - # Pagination - total = len(runs) + total = session.exec(select(func.count()).select_from(EvalRun)).one() start = (page - 1) * page_size - end = start + page_size - paginated = runs[start:end] + paginated = session.exec( + select(EvalRun).order_by(EvalRun.created_at.desc()).offset(start).limit(page_size) + ).all() rows = [ { diff --git a/trustbench/db.py b/trustbench/db.py index 24ecf88..4fe4a27 100644 --- a/trustbench/db.py +++ b/trustbench/db.py @@ -133,6 +133,20 @@ def create_db_engine(database_url: str | None = None): resolved_url = database_url or get_settings().database_url if resolved_url.startswith("postgresql") and not _postgres_driver_available(): + import os + + allow_sqlite_fallback = os.getenv("TRUSTBENCH_ALLOW_SQLITE_FALLBACK", "").strip().lower() in { + "1", + "true", + "yes", + "on", + } + if not allow_sqlite_fallback: + raise RuntimeError( + "DATABASE_URL is configured for PostgreSQL but psycopg2 is not installed. " + "Install psycopg2 (or psycopg2-binary), or explicitly set " + "TRUSTBENCH_ALLOW_SQLITE_FALLBACK=1 for local development only." + ) resolved_url = "sqlite:///./trustbench.db" connect_args = {"check_same_thread": False} if resolved_url.startswith("sqlite") else {} return create_engine(resolved_url, connect_args=connect_args)