fix(ws1): harden HTTP API + declare fastapi/uvicorn deps
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).
This commit is contained in:
parent
366d07a84e
commit
b6c66877f5
3 changed files with 96 additions and 12 deletions
|
|
@ -43,6 +43,7 @@ import hmac
|
||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
|
import threading
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
|
|
@ -58,6 +59,14 @@ AGENT_TEAM_API_TOKEN_ENV = "AGENT_TEAM_API_TOKEN"
|
||||||
_DEFAULT_HOST = "127.0.0.1"
|
_DEFAULT_HOST = "127.0.0.1"
|
||||||
_DEFAULT_PORT = 8765
|
_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).
|
# Pydantic request/response models (module-level so FastAPI resolves them).
|
||||||
|
|
@ -169,10 +178,23 @@ def make_app(
|
||||||
"pip install fastapi uvicorn"
|
"pip install fastapi uvicorn"
|
||||||
) from exc
|
) 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(
|
app = FastAPI(
|
||||||
title="Sea Haven agent-team API",
|
title="Sea Haven agent-team API",
|
||||||
description="Internal HTTP API for the agent-team orchestrator (VPN-only, 127.0.0.1).",
|
description="Internal HTTP API for the agent-team orchestrator (VPN-only, 127.0.0.1).",
|
||||||
version="0.1.0",
|
version="0.1.0",
|
||||||
|
docs_url=None,
|
||||||
|
redoc_url=None,
|
||||||
|
openapi_url=None,
|
||||||
)
|
)
|
||||||
|
|
||||||
_bearer = HTTPBearer(auto_error=False)
|
_bearer = HTTPBearer(auto_error=False)
|
||||||
|
|
@ -232,7 +254,13 @@ def make_app(
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
except Exception as exc:
|
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)
|
return TaskStateResponse(thread_id=thread_id, state=state_dict)
|
||||||
|
|
||||||
@app.post(
|
@app.post(
|
||||||
|
|
@ -255,6 +283,13 @@ def make_app(
|
||||||
status_code=503,
|
status_code=503,
|
||||||
detail=f"orchestrator run.py not found at {run_py}",
|
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:
|
try:
|
||||||
completed = subprocess.run( # noqa: S603 - args are not shell-interpolated
|
completed = subprocess.run( # noqa: S603 - args are not shell-interpolated
|
||||||
[sys.executable, str(run_py), request.prompt],
|
[sys.executable, str(run_py), request.prompt],
|
||||||
|
|
@ -268,10 +303,19 @@ def make_app(
|
||||||
status_code=504, detail="orchestrator invocation timed out"
|
status_code=504, detail="orchestrator invocation timed out"
|
||||||
) from None
|
) from None
|
||||||
except subprocess.CalledProcessError as exc:
|
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(
|
raise HTTPException(
|
||||||
status_code=500,
|
status_code=500,
|
||||||
detail=f"orchestrator call failed (exit {exc.returncode}): {exc.stderr[:500]}",
|
detail=f"orchestrator call failed (exit {exc.returncode})",
|
||||||
) from exc
|
) from exc
|
||||||
|
finally:
|
||||||
|
_invoke_semaphore.release()
|
||||||
return OrchestratorInvokeResponse(text=completed.stdout)
|
return OrchestratorInvokeResponse(text=completed.stdout)
|
||||||
|
|
||||||
return app
|
return app
|
||||||
|
|
|
||||||
|
|
@ -378,19 +378,56 @@ def test_api_orchestrator_invoke_calls_subprocess(
|
||||||
|
|
||||||
|
|
||||||
@requires_fastapi
|
@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 fastapi.testclient import TestClient
|
||||||
|
|
||||||
from agent_team.api import make_app
|
from agent_team.api import make_app
|
||||||
|
|
||||||
monkeypatch.delenv("AGENT_TEAM_API_TOKEN", raising=False)
|
|
||||||
app = make_app(coordinator=_make_stub_coordinator())
|
app = make_app(coordinator=_make_stub_coordinator())
|
||||||
client = TestClient(app, raise_server_exceptions=False)
|
client = TestClient(app, raise_server_exceptions=False)
|
||||||
# A missing env var raises RuntimeError inside the dependency — should 500
|
# The unauthenticated docs/schema routes must be disabled (VPN-only API).
|
||||||
# or 401; either way the request must not succeed.
|
for path in ("/docs", "/redoc", "/openapi.json"):
|
||||||
resp = client.post(
|
assert client.get(path).status_code == 404, path
|
||||||
"/tasks",
|
|
||||||
json={"task": "hi"},
|
|
||||||
headers={"Authorization": "Bearer anything"},
|
@requires_fastapi
|
||||||
)
|
def test_api_invoke_concurrency_cap_returns_429(api_token: str, tmp_path: Any) -> None:
|
||||||
assert resp.status_code in (401, 500)
|
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()
|
||||||
|
|
|
||||||
|
|
@ -7,3 +7,6 @@ langchain-google-genai==4.2.5
|
||||||
langchain-community==0.4.2
|
langchain-community==0.4.2
|
||||||
composio-langgraph==0.15.0
|
composio-langgraph==0.15.0
|
||||||
python-dotenv==1.2.2
|
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
|
||||||
|
|
|
||||||
Reference in a new issue