From b6c66877f5e40ad58428e7b65f3bcb5d7f18465b Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Tue, 23 Jun 2026 12:27:23 -0400 Subject: [PATCH] fix(ws1): harden HTTP API + declare fastapi/uvicorn deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security follow-ups from the per-PR review (non-blocking MEDIUMs): - Eager _get_token() at make_app build time so a missing AGENT_TEAM_API_TOKEN fails fast instead of serving requests first (matches the docstring contract). - Disable /docs, /redoc, /openapi.json (no auth dependency in FastAPI) — the API is VPN-only/127.0.0.1 and should not expose its schema unauthenticated. - Scrub raw exception text and subprocess stderr from 500 response bodies; log server-side instead (avoid internal-path/state disclosure). - Bound /orchestrator/invoke concurrency with a semaphore (429 over the cap) so an authenticated caller cannot exhaust the box via many 600s subprocesses. Also pin fastapi/uvicorn in requirements.txt (WS1 dep). With fastapi now installed in CI, the previously skip-guarded TestClient tests run for real; the importorskip guard stays as a no-op safety net. Tests: 23 pass (adds docs-disabled + concurrency-429 cases). --- agent-team/agent_team/api.py | 48 +++++++++++++++- .../tests/test_ws1_invoker_multi_api.py | 57 +++++++++++++++---- requirements.txt | 3 + 3 files changed, 96 insertions(+), 12 deletions(-) diff --git a/agent-team/agent_team/api.py b/agent-team/agent_team/api.py index 730d0c8..8691d0c 100644 --- a/agent-team/agent_team/api.py +++ b/agent-team/agent_team/api.py @@ -43,6 +43,7 @@ import hmac import os import subprocess import sys +import threading from pathlib import Path from typing import Any @@ -58,6 +59,14 @@ AGENT_TEAM_API_TOKEN_ENV = "AGENT_TEAM_API_TOKEN" _DEFAULT_HOST = "127.0.0.1" _DEFAULT_PORT = 8765 +# Bound how many `/orchestrator/invoke` subprocesses can run at once. Each spawns +# a 600s-timeout child; without a cap an authenticated caller could exhaust CPU / +# memory / file descriptors by firing many concurrent invocations (DoS). The +# endpoint runs in Starlette's threadpool, so a threading semaphore is the right +# primitive; over-limit requests get 429 rather than queueing unboundedly. +_MAX_CONCURRENT_INVOKES = 2 +_invoke_semaphore = threading.BoundedSemaphore(_MAX_CONCURRENT_INVOKES) + # --------------------------------------------------------------------------- # Pydantic request/response models (module-level so FastAPI resolves them). @@ -169,10 +178,23 @@ def make_app( "pip install fastapi uvicorn" ) from exc + # Fail fast at app-build time if the bearer token is unconfigured, so a + # misconfigured deploy never reaches a serving state (matches the module + # docstring's "refuses to start without a secret" contract). The per-request + # check still calls _get_token() so a token rotation/unset after boot is + # also caught. + _get_token() + + # VPN-only internal API: disable the interactive docs and the OpenAPI schema + # so the endpoint surface is not exposed unauthenticated (these routes carry + # no auth dependency by design in FastAPI). app = FastAPI( title="Sea Haven agent-team API", description="Internal HTTP API for the agent-team orchestrator (VPN-only, 127.0.0.1).", version="0.1.0", + docs_url=None, + redoc_url=None, + openapi_url=None, ) _bearer = HTTPBearer(auto_error=False) @@ -232,7 +254,13 @@ def make_app( except HTTPException: raise except Exception as exc: - raise HTTPException(status_code=500, detail=str(exc)) from exc + # Do not leak the raw exception text (may carry internal paths / + # state) into the response body; log it server-side and return a + # generic message. + print(f"[api] get_task error for {thread_id!r}: {exc!r}", file=sys.stderr) + raise HTTPException( + status_code=500, detail="internal error retrieving task state" + ) from exc return TaskStateResponse(thread_id=thread_id, state=state_dict) @app.post( @@ -255,6 +283,13 @@ def make_app( status_code=503, detail=f"orchestrator run.py not found at {run_py}", ) + # Bound concurrent subprocess fan-out (DoS guard). Non-blocking acquire: + # over the cap we reject with 429 rather than pile up 600s children. + if not _invoke_semaphore.acquire(blocking=False): + raise HTTPException( + status_code=429, + detail="too many concurrent orchestrator invocations; retry later", + ) try: completed = subprocess.run( # noqa: S603 - args are not shell-interpolated [sys.executable, str(run_py), request.prompt], @@ -268,10 +303,19 @@ def make_app( status_code=504, detail="orchestrator invocation timed out" ) from None except subprocess.CalledProcessError as exc: + # Log stderr server-side; do not return it in the response body + # (may carry internal paths / orchestrator internals). + print( + f"[api] orchestrator_invoke failed (exit {exc.returncode}): " + f"{(exc.stderr or '')[:1000]}", + file=sys.stderr, + ) raise HTTPException( status_code=500, - detail=f"orchestrator call failed (exit {exc.returncode}): {exc.stderr[:500]}", + detail=f"orchestrator call failed (exit {exc.returncode})", ) from exc + finally: + _invoke_semaphore.release() return OrchestratorInvokeResponse(text=completed.stdout) return app diff --git a/agent-team/tests/test_ws1_invoker_multi_api.py b/agent-team/tests/test_ws1_invoker_multi_api.py index 775c869..40e771e 100644 --- a/agent-team/tests/test_ws1_invoker_multi_api.py +++ b/agent-team/tests/test_ws1_invoker_multi_api.py @@ -378,19 +378,56 @@ def test_api_orchestrator_invoke_calls_subprocess( @requires_fastapi -def test_api_no_token_env_raises_on_request(monkeypatch: Any) -> None: +def test_api_no_token_env_raises_at_build(monkeypatch: Any) -> None: + from agent_team.api import make_app + + monkeypatch.delenv("AGENT_TEAM_API_TOKEN", raising=False) + # Fail-fast: an unconfigured token must raise at app-build time, not serve a + # request first (the eager _get_token() in make_app). + with pytest.raises(RuntimeError, match="AGENT_TEAM_API_TOKEN"): + make_app(coordinator=_make_stub_coordinator()) + + +@requires_fastapi +def test_api_docs_and_openapi_disabled(api_token: str) -> None: from fastapi.testclient import TestClient from agent_team.api import make_app - monkeypatch.delenv("AGENT_TEAM_API_TOKEN", raising=False) app = make_app(coordinator=_make_stub_coordinator()) client = TestClient(app, raise_server_exceptions=False) - # A missing env var raises RuntimeError inside the dependency — should 500 - # or 401; either way the request must not succeed. - resp = client.post( - "/tasks", - json={"task": "hi"}, - headers={"Authorization": "Bearer anything"}, - ) - assert resp.status_code in (401, 500) + # The unauthenticated docs/schema routes must be disabled (VPN-only API). + for path in ("/docs", "/redoc", "/openapi.json"): + assert client.get(path).status_code == 404, path + + +@requires_fastapi +def test_api_invoke_concurrency_cap_returns_429(api_token: str, tmp_path: Any) -> None: + import agent_team.api as api_mod + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + fake_run = tmp_path / "run.py" + fake_run.write_text("import sys; print(sys.argv[1])") + app = make_app(coordinator=_make_stub_coordinator()) + + # Exhaust the bounded semaphore so the request sees no free slot → 429. + acquired = [ + api_mod._invoke_semaphore.acquire(blocking=False) + for _ in range(api_mod._MAX_CONCURRENT_INVOKES) + ] + try: + assert all(acquired) + with patch("agent_team.api._resolve_run_py", return_value=fake_run): + client = TestClient(app, raise_server_exceptions=False) + resp = client.post( + "/orchestrator/invoke", + json={"prompt": "hello"}, + headers={"Authorization": f"Bearer {api_token}"}, + ) + assert resp.status_code == 429 + finally: + for ok in acquired: + if ok: + api_mod._invoke_semaphore.release() diff --git a/requirements.txt b/requirements.txt index 3f44e71..3e0a84d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,6 @@ langchain-google-genai==4.2.5 langchain-community==0.4.2 composio-langgraph==0.15.0 python-dotenv==1.2.2 +# WS1 agent-team HTTP API (agent_team/api.py): FastAPI app + uvicorn ASGI server. +fastapi==0.136.1 +uvicorn==0.46.0