mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-15 00:19:14 +00:00
* refactor(gateway): issue request trace ids unconditionally The request trace id was gated behind logging.enhance.enabled at every entry point, so downstream code had to keep asking whether one existed: a header-provenance flag in its own ContextVar, a precedence resolver, and three-level carrier fallbacks at each consumer. Bind one unconditionally instead. TraceMiddleware covers Gateway HTTP; ensure_trace_context covers the entry points that never touch ASGI -- scheduled occurrences, MCP task notification runs, IM channel messages, and the embedded client -- each scoped to one unit of work so a long-lived worker task cannot leak one occurrence's id into the next. The ContextVar becomes the only source; the response header, runtime context, run metadata and log records are derived outputs. Consumers now use ensure_trace_id() or resolve_trace_id(*carriers) and drop their presence guards. Removed: resolve_deerflow_trace_id, the header-provenance flag and its three helpers, set/reset_current_trace_id, is_trace_correlation_enabled and its gateway alias. BREAKING CHANGE: every Gateway HTTP response now carries X-Trace-Id and it cannot be turned off; logging.enhance.enabled controls log output only. Installations on the default enabled: false will start seeing the header. No config keys were added or removed. * fix(gateway): stop persisting a caller-supplied trace id on the run record body.metadata forks two ways: through build_run_config into the live run config, which the run worker restamps, and through create_or_reject into the run record that the runs API echoes verbatim. Only the first was covered, so a client sending metadata.deerflow_trace_id made the most durable and most visible surface of a run disagree with the X-Trace-Id and the log lines the same request produced -- a correlation id that does not match the logs is worse than none. Stamp the server-issued id once at the trust boundary so both forks receive it, preserving the caller's own metadata keys. Close the same gap on config.context, which reaches the runtime context by a separate path: _build_runtime_context no longer merges server-owned keys from the caller, and _install_runtime_context assigns rather than setdefaults. A thread's metadata is no longer seeded with the run-scoped id of whichever run created it -- one thread spans many runs and as many trace ids. Found by driving a real run through the Gateway and reading the run back from the runs API; every unit test built its metadata by hand and so could not see it. * fix(gateway): expose X-Trace-Id to split-origin browser clients X-Trace-Id is not on the CORS safelist, so a browser client served from a separate origin could not read it -- and those are exactly the clients that cannot read the Gateway's logs either, leaving them with nothing to quote in a bug report. Same-origin nginx deployments were unaffected, which is why this stayed hidden. Add it to CORS_EXPOSED_HEADERS beside Content-Location, referencing TRACE_ID_HEADER rather than repeating the literal. * fix(gateway): keep X-Trace-Id on unhandled-exception 500s Starlette's ServerErrorMiddleware sits outside every user middleware and emits unhandled-exception 500s through the raw send, so those responses never pass TraceMiddleware's header-writing wrapper. The 500 for a server bug is exactly the response a user most needs to correlate with a log line, and it was the one response that shipped without the id. TraceMiddleware now tracks whether http.response.start has been sent. On an exception with no response started it emits its own plain 500 carrying the header, then re-raises: the outer ServerErrorMiddleware sees the response already started and only re-raises too, so the server's exception logging is untouched. An exception mid-stream keeps propagating unchanged — a second response start cannot be sent, and the already-written header stands. The trace id is printable ASCII by construction (normalize_trace_id / generate_trace_id), which is what makes the raw latin-1 header encoding safe. * fix(gateway): strip the forged trace id from the persisted request echo The run-record fix stopped a forged metadata.deerflow_trace_id on the authoritative metadata surface, but the raw request echo still carried one: create_or_reject persists body.config verbatim as runs.kwargs_json, which the runs API serves back. A client posting config.context.deerflow_trace_id therefore still got its forged value stored and echoed on one API surface while the header, logs, run metadata, and checkpoint all carried the real id — the id is ignored as input there, so echoing it back only manufactures disagreement. Two changes close it. redact_config_secrets — already the shared scrub for that echo, applied at admission and again at serve time, so historical records are covered too — now also drops deerflow_trace_id from config.metadata and config.context. And build_run_config now merges run metadata onto a copy of the caller's config["metadata"] instead of updating it in place: the nested values of the request config are reference copies, so the in-place merge was writing the server-stamped key through into body.config, contaminating the "what the client sent" record before it was persisted (and incidentally masking the forged-value echo on the metadata container). The regression test posts a forged id through body.metadata, config.metadata, and config.context at once and reads the kwargs echo back off the run record, failing if either leak returns. * docs(harness): record the trace-echo scrub, 500 fallback, and accepted retry divergence The trace section of the harness AGENTS.md now covers the two fixes that close the derived-output rule (the kwargs-echo scrub in redact_config_secrets plus build_run_config's copy merge, and TraceMiddleware's own 500 for unhandled exceptions), and CHANGELOG gains their Fixed entries. It also writes down the one accepted divergence: a crash-recovered scheduled launch reuses the durable run through its idempotency key, and start_run returns early on idempotency_reused without restamping — so the run record keeps the first attempt's deerflow_trace_id while the retry's own log lines carry the freshly minted id of its ensure_trace_context binding. The divergence is confined to the crash-recovery window and is accepted rather than fixed: restamping on reuse would rewrite a persisted record for a run that already exists, which is worse than two ids that each correlate their own attempt's logs. Written down so the next reader of the scheduler recovery path does not diagnose it as a bug. * docs(config): align the logging.enhance schema note with the unconditional trace id The config-module AGENTS.md still described logging.enhance as the gate for the Gateway X-Trace-Id header and Langfuse deerflow_trace_id. That model is gone: ids are issued unconditionally and this block decides log output only. Left as-is, the stale wording invites an agent to "restore" a header gate it believes was lost. Reworded to match the sibling AGENTS.md files and config.example.yaml, with a pointer to the Request Trace Context section that owns the full model. * docs(changelog): link the trace entries to #5119 The five new entries pointed at the ([#XXXX]) placeholder with no reference definition, rendering as literal text instead of a link — and RELEASING.md step 2 relies on those references when the section becomes release notes. All five now point at #5119, with the definition appended to the reference block. * refactor(harness): rename _stream_without_trace_context to _stream_turn The name asserted the opposite of what the method now does. It was accurate while logging.enhance.enabled could route stream() around the trace scope; with the gate gone it is the only stream implementation left, and it binds the id itself via ensure_trace_id(). Private, so the rename touches only the definition and the one stream() call site. * docs(harness): fit the trace-context guidance inside the AGENTS.md chain budget The expanded Request Trace Context section pushed the effective AGENTS.md chain for agents/middlewares to 99,815 bytes, past the 98,304 hard limit scripts/check_agent_guidance.py enforces in CI (AG002). Compressed the section from 7,359 to 4592 bytes with no facts removed: the entry-point table, the derived-output rule and its enforcement points, the accepted scheduled-retry divergence, the two resolution helpers, the stream() binding rationale, the log-output-only gate, the CORS listing, the 500 fallback, and the test map all remain. Sized against the merge, not just the branch: current main grew the same chain by ~724 bytes, so the check was verified on the merged tree as well (97,772 bytes; branch tree 97,048). * fix(gateway): declare content-length on the fallback 500 The pre-response 500 declared content-type but no content-length, leaving the framing to the ASGI server: chunked on HTTP/1.1, close-delimited on HTTP/1.0 — the one wire difference from the ServerErrorMiddleware response it replaces, which sends content-length: 21. The explicit header keeps the fallback byte-identical to what clients saw before. * docs(readme): drop the trace-correlation condition from the translations The zh/ja/fr/ru Langfuse sections still said metadata.deerflow_trace_id matches X-Trace-Id "when request trace correlation is enabled". The id now always matches and that condition no longer exists, so each bullet states the unconditional match and that logging.enhance.enabled only controls whether the id is printed into logs — the one piece of the feature a user can still configure. * test(gateway): pin TraceMiddleware wiring through create_app() Every X-Trace-Id test exercised a hand-built four-route app, so the real stack's add_middleware(TraceMiddleware) line was pinned by nothing: deleting it — or short-circuiting above it — passed CI while silently dropping both the response header and the ambient id the run-record stamp and enhanced log records derive from. One case now drives /health through create_app() and asserts the inbound id round-trips; mutation-checked by removing the wiring line, which fails exactly this test. * docs(gateway): note the fallback 500 is CORS-opaque The pre-response 500 is emitted outside CORSMiddleware — the exception has already unwound past it — so it carries no Access-Control-Allow-Origin and a split-origin browser client cannot read the id on this one response, unchanged from the ServerErrorMiddleware 500 it replaces. Documented on the class and in the CHANGELOG entry rather than fixed: replicating the origin allowlist outside CORSMiddleware would let the two policies drift. * fix(harness): keep abandoned-stream cleanup inside the trace binding stream() binds the turn's id around each next(inner) and resets it before yielding, but the finally's inner.close() ran after that binding was gone. Abandoning the stream therefore drove the inner LangGraph generator's GeneratorExit/finally path with no trace id — or an unrelated ambient one from whichever context ran the close — so cancellation and finalization logs and callbacks did not correlate with the turn they belong to. inner.close() is now wrapped in a local bind/reset of the same turn id. The token is set and reset in the same frame, never across a yield, so the per-step cross-context safety is preserved even when GC closes the generator from another Context — pinned by the existing copy_context close test, which now exercises this path. The regression test records the id from the inner generator's finally and fails without the binding. * test(harness): teach the worker-trace fake about RunManager.cleanup Upstream #5112 (bound gateway memory after terminal runs) added a run_manager.cleanup(run_id) call to run_agent's finalization, so the merge-commit CI run failed all five worker-trace-binding tests with AttributeError on this PR's _FakeRunManager. The fake gains the same no-op shape as its other methods. * docs(gateway): bring the gateway AGENTS.md back under its soft budget Upstream #5092 grew backend/app/gateway/AGENTS.md to 40,966 bytes, 6 over the 40,960 soft budget that test_agent_guidance_check.py::test_repository_guidance_stays_below_soft_budgets_and_avoids_doc_indexes enforces — its Unit Tests run on main was cancelled by push concurrency, so main is currently red on that test and every PR merge-run inherits the failure. Two whitespace/wording trims in the row #5092 touched (a doubled space, and "its configured `context_window`" → "its `context_window`") bring the file to 40,953 with no content change. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
288 lines
12 KiB
Python
288 lines
12 KiB
Python
"""CSRF protection middleware for FastAPI.
|
|
|
|
Per RFC-001:
|
|
State-changing operations require CSRF protection.
|
|
"""
|
|
|
|
import os
|
|
import secrets
|
|
from collections.abc import Awaitable, Callable
|
|
from urllib.parse import urlsplit
|
|
|
|
from fastapi import Request, Response
|
|
from starlette.middleware.base import BaseHTTPMiddleware
|
|
from starlette.responses import JSONResponse
|
|
from starlette.types import ASGIApp
|
|
|
|
from app.gateway.auth.config import get_auth_config
|
|
from app.gateway.auth.session_cookie_state import SESSION_COOKIE_ISSUED_STATE_ATTR, SESSION_COOKIE_MAX_AGE_STATE_ATTR, SESSION_COOKIE_SECURE_STATE_ATTR, SKIP_AUTH_CSRF_COOKIE_STATE_ATTR
|
|
from app.gateway.auth_disabled import is_auth_disabled
|
|
from app.gateway.request_path import get_request_route_path
|
|
from deerflow.trace_context import TRACE_ID_HEADER
|
|
|
|
CSRF_COOKIE_NAME = "csrf_token"
|
|
CSRF_HEADER_NAME = "X-CSRF-Token"
|
|
CSRF_TOKEN_LENGTH = 64 # bytes
|
|
_CSRF_STATE_CHANGING_METHODS: frozenset[str] = frozenset({"POST", "PUT", "DELETE", "PATCH"})
|
|
_CSRF_EXEMPT_EXACT_PATHS: frozenset[str] = frozenset({"/api/v1/auth/me"})
|
|
|
|
|
|
def is_secure_request(request: Request) -> bool:
|
|
"""Detect whether the original client request was made over HTTPS."""
|
|
return _request_scheme(request) == "https"
|
|
|
|
|
|
def generate_csrf_token() -> str:
|
|
"""Generate a secure random CSRF token."""
|
|
return secrets.token_urlsafe(CSRF_TOKEN_LENGTH)
|
|
|
|
|
|
def should_check_csrf(request: Request) -> bool:
|
|
"""Determine if a request needs CSRF validation.
|
|
|
|
CSRF is checked for state-changing methods (POST, PUT, DELETE, PATCH).
|
|
GET, HEAD, OPTIONS, and TRACE are exempt per RFC 7231.
|
|
"""
|
|
if request.method not in _CSRF_STATE_CHANGING_METHODS:
|
|
return False
|
|
|
|
if is_auth_disabled():
|
|
return False
|
|
|
|
route_path = get_request_route_path(request)
|
|
path = route_path.rstrip("/")
|
|
# Exempt host-owned endpoints that implement their own request posture.
|
|
if path in _CSRF_EXEMPT_EXACT_PATHS:
|
|
return False
|
|
# Inbound webhooks authenticate themselves via provider-specific signatures
|
|
# (e.g. GitHub's X-Hub-Signature-256), not the CSRF double-submit cookie.
|
|
if route_path.startswith("/api/webhooks/"):
|
|
return False
|
|
return True
|
|
|
|
|
|
_AUTH_EXEMPT_PATHS: frozenset[str] = frozenset(
|
|
{
|
|
"/api/v1/auth/login/local",
|
|
"/api/v1/auth/logout",
|
|
"/api/v1/auth/register",
|
|
"/api/v1/auth/initialize",
|
|
}
|
|
)
|
|
|
|
|
|
def is_auth_endpoint(request: Request) -> bool:
|
|
"""Check if the request is to an auth endpoint.
|
|
|
|
Auth endpoints don't need CSRF validation on first call (no token).
|
|
"""
|
|
return get_request_route_path(request).rstrip("/") in _AUTH_EXEMPT_PATHS
|
|
|
|
|
|
def _host_with_optional_port(hostname: str, port: int | None, scheme: str) -> str:
|
|
"""Return normalized host[:port], omitting default ports."""
|
|
host = hostname.lower()
|
|
if ":" in host and not host.startswith("["):
|
|
host = f"[{host}]"
|
|
|
|
if port is None or (scheme == "http" and port == 80) or (scheme == "https" and port == 443):
|
|
return host
|
|
return f"{host}:{port}"
|
|
|
|
|
|
def _normalize_origin(origin: str) -> str | None:
|
|
"""Return a normalized scheme://host[:port] origin, or None for invalid input."""
|
|
try:
|
|
parsed = urlsplit(origin.strip())
|
|
port = parsed.port
|
|
except ValueError:
|
|
return None
|
|
|
|
scheme = parsed.scheme.lower()
|
|
if scheme not in {"http", "https"} or not parsed.hostname:
|
|
return None
|
|
|
|
# Browser Origin is only scheme/host/port. Reject URL-shaped or credentialed values.
|
|
if parsed.username or parsed.password or parsed.path or parsed.query or parsed.fragment:
|
|
return None
|
|
|
|
return f"{scheme}://{_host_with_optional_port(parsed.hostname, port, scheme)}"
|
|
|
|
|
|
def _configured_cors_origins() -> set[str]:
|
|
"""Return explicit configured browser origins that may call auth routes."""
|
|
origins = set()
|
|
for raw_origin in os.environ.get("GATEWAY_CORS_ORIGINS", "").split(","):
|
|
origin = raw_origin.strip()
|
|
if not origin or origin == "*":
|
|
continue
|
|
normalized = _normalize_origin(origin)
|
|
if normalized:
|
|
origins.add(normalized)
|
|
return origins
|
|
|
|
|
|
def get_configured_cors_origins() -> set[str]:
|
|
"""Return normalized explicit browser origins from GATEWAY_CORS_ORIGINS."""
|
|
return _configured_cors_origins()
|
|
|
|
|
|
# Response headers a split-origin browser client must be able to read. Only the
|
|
# CORS-safelisted set is visible to JS by default, and the created run's id
|
|
# travels in `Content-Location` — the LangGraph SDK resolves run metadata from
|
|
# it, so withholding it leaves such a client unable to learn its own run id.
|
|
# `X-Trace-Id` is listed for the same reason: TraceMiddleware puts it on every
|
|
# response as the correlation id to quote in a bug report, and unexposed it is
|
|
# readable on same-origin nginx deployments but invisible to exactly the
|
|
# split-origin clients that cannot see the Gateway's logs either.
|
|
CORS_EXPOSED_HEADERS: tuple[str, ...] = ("Content-Location", TRACE_ID_HEADER)
|
|
|
|
|
|
def _first_header_value(value: str | None) -> str | None:
|
|
"""Return the first value from a comma-separated proxy header."""
|
|
if not value:
|
|
return None
|
|
first = value.split(",", 1)[0].strip()
|
|
return first or None
|
|
|
|
|
|
def _forwarded_param(request: Request, name: str) -> str | None:
|
|
"""Extract a parameter from the first RFC 7239 Forwarded header entry."""
|
|
forwarded = _first_header_value(request.headers.get("forwarded"))
|
|
if not forwarded:
|
|
return None
|
|
|
|
for part in forwarded.split(";"):
|
|
key, sep, value = part.strip().partition("=")
|
|
if sep and key.lower() == name:
|
|
return value.strip().strip('"') or None
|
|
return None
|
|
|
|
|
|
def _request_scheme(request: Request) -> str:
|
|
"""Resolve the original request scheme from trusted proxy headers."""
|
|
scheme = _forwarded_param(request, "proto") or _first_header_value(request.headers.get("x-forwarded-proto")) or request.url.scheme
|
|
return scheme.lower()
|
|
|
|
|
|
def _request_origin(request: Request) -> str | None:
|
|
"""Build the origin for the URL the browser is targeting."""
|
|
scheme = _request_scheme(request)
|
|
host = _forwarded_param(request, "host") or _first_header_value(request.headers.get("x-forwarded-host")) or request.headers.get("host") or request.url.netloc
|
|
|
|
forwarded_port = _first_header_value(request.headers.get("x-forwarded-port"))
|
|
if forwarded_port and ":" not in host.rsplit("]", 1)[-1]:
|
|
host = f"{host}:{forwarded_port}"
|
|
|
|
return _normalize_origin(f"{scheme}://{host}")
|
|
|
|
|
|
def is_allowed_auth_origin(request: Request) -> bool:
|
|
"""Allow auth POSTs only from the same origin or explicit configured origins.
|
|
|
|
Login/register/initialize are exempt from the double-submit token because
|
|
first-time browser clients do not have a CSRF token yet. They still create
|
|
a session cookie, so browser requests with a hostile Origin header must be
|
|
rejected to prevent login CSRF / session fixation. Requests without Origin
|
|
are allowed for non-browser clients such as curl and mobile integrations.
|
|
"""
|
|
origin = request.headers.get("origin")
|
|
if not origin:
|
|
return True
|
|
|
|
normalized_origin = _normalize_origin(origin)
|
|
if normalized_origin is None:
|
|
return False
|
|
|
|
request_origin = _request_origin(request)
|
|
return normalized_origin in _configured_cors_origins() or (request_origin is not None and normalized_origin == request_origin)
|
|
|
|
|
|
def auth_csrf_cookie_settings(request: Request) -> tuple[bool, int | None]:
|
|
"""Return ``(secure, max_age)`` for auth-created CSRF cookies."""
|
|
session_cookie_issued = getattr(request.state, SESSION_COOKIE_ISSUED_STATE_ATTR, False)
|
|
if session_cookie_issued:
|
|
return (
|
|
bool(getattr(request.state, SESSION_COOKIE_SECURE_STATE_ATTR, is_secure_request(request))),
|
|
getattr(request.state, SESSION_COOKIE_MAX_AGE_STATE_ATTR, None),
|
|
)
|
|
|
|
secure = is_secure_request(request)
|
|
max_age = get_auth_config().token_expiry_days * 24 * 3600 if secure else None
|
|
return secure, max_age
|
|
|
|
|
|
class CSRFMiddleware(BaseHTTPMiddleware):
|
|
"""Middleware that implements CSRF protection using Double Submit Cookie pattern."""
|
|
|
|
def __init__(self, app: ASGIApp) -> None:
|
|
super().__init__(app)
|
|
|
|
async def dispatch(self, request: Request, call_next: Callable[[Request], Awaitable[Response]]) -> Response:
|
|
_is_auth = is_auth_endpoint(request)
|
|
|
|
if should_check_csrf(request) and _is_auth and not is_allowed_auth_origin(request):
|
|
return JSONResponse(
|
|
status_code=403,
|
|
content={"detail": "Cross-site auth request denied."},
|
|
)
|
|
|
|
if should_check_csrf(request) and not _is_auth and request.headers.get("authorization") is None:
|
|
# Bearer-authenticated requests (PAT, #4849) are exempt from the
|
|
# cookie double-submit check only — the cross-site origin check on
|
|
# auth endpoints above still runs for every request. Safety rests
|
|
# on AuthMiddleware's strict Bearer precedence: an invalid Bearer
|
|
# header is a 401 there, so a cross-site attacker cannot ride a
|
|
# victim's cookie by padding a garbage Authorization header, and a
|
|
# cross-site request carrying a custom Authorization header at all
|
|
# requires a CORS preflight the attacker cannot obtain.
|
|
cookie_token = request.cookies.get(CSRF_COOKIE_NAME)
|
|
header_token = request.headers.get(CSRF_HEADER_NAME)
|
|
|
|
if not cookie_token or not header_token:
|
|
return JSONResponse(
|
|
status_code=403,
|
|
content={"detail": "CSRF token missing. Include X-CSRF-Token header."},
|
|
)
|
|
|
|
if not secrets.compare_digest(cookie_token, header_token):
|
|
return JSONResponse(
|
|
status_code=403,
|
|
content={"detail": "CSRF token mismatch."},
|
|
)
|
|
|
|
response = await call_next(request)
|
|
|
|
# For auth endpoints that set up session, also set CSRF cookie.
|
|
# Session-creating handlers may stamp the final access-token max_age on
|
|
# request.state; mirroring it here keeps the double-submit cookie pair
|
|
# from diverging across HTTPS, localhost, and sandbox deployments.
|
|
if _is_auth and request.method == "POST" and not getattr(request.state, SKIP_AUTH_CSRF_COOKIE_STATE_ATTR, False):
|
|
# Generate a new CSRF token for the session
|
|
csrf_token = generate_csrf_token()
|
|
secure, max_age = auth_csrf_cookie_settings(request)
|
|
response.set_cookie(
|
|
key=CSRF_COOKIE_NAME,
|
|
value=csrf_token,
|
|
httponly=False, # Must be JS-readable for Double Submit Cookie pattern
|
|
secure=secure,
|
|
samesite="strict",
|
|
# Match the access_token cookie's lifetime (auth.py::_set_session_cookie)
|
|
# so the double-submit pair never diverges. A session-only csrf_token is
|
|
# evicted when iOS Safari terminates a home-screen PWA while the persistent
|
|
# access_token survives — leaving the user "logged in" but unable to make
|
|
# any state-changing request (403 "CSRF token missing").
|
|
max_age=max_age,
|
|
)
|
|
|
|
return response
|
|
|
|
|
|
def get_csrf_token(request: Request) -> str | None:
|
|
"""Get the CSRF token from the current request's cookies.
|
|
|
|
This is useful for server-side rendering where you need to embed
|
|
token in forms or headers.
|
|
"""
|
|
return request.cookies.get(CSRF_COOKIE_NAME)
|