diff --git a/CHANGELOG.md b/CHANGELOG.md index c82ccb46d5..d1f784dcd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -327,6 +327,15 @@ This release closes that milestone with **181 merged pull requests**. ### Fixed +- **gateway:** A non-ASCII CSRF token, GitHub webhook signature, internal auth + token, OIDC `state`, or provisioner `X-API-Key` is now rejected with the + usual 403/401 instead of a 500. `hmac.compare_digest` raises `TypeError` for + `str` operands with non-ASCII characters, and Starlette decodes header bytes + as latin-1, so a single `0xE9` byte crashed the comparison. The Gateway now + compares the UTF-8 bytes through one helper, + `app.gateway.utils.constant_time_equals`, and the standalone provisioner + encodes inline. No bypass was possible; the request was already failing, just + with the wrong status. ([#6076]) - **agents:** Context-compaction fraction triggers and fraction-based retention now use the active run model's context profile; a separate `summarization.model_name` remains generation-only. This prevents mismatched @@ -6293,5 +6302,6 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6066]: https://github.com/bytedance/deer-flow/pull/6066 [#6069]: https://github.com/bytedance/deer-flow/pull/6069 [#6070]: https://github.com/bytedance/deer-flow/pull/6070 +[#6076]: https://github.com/bytedance/deer-flow/pull/6076 [#6088]: https://github.com/bytedance/deer-flow/pull/6088 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index f9cd63e177..6d5085ac19 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -332,6 +332,12 @@ ### 修复 +- **Gateway：** 含非 ASCII 字符的 CSRF token、GitHub webhook 签名、内部认证 token、 + OIDC `state` 或 provisioner `X-API-Key` 现在按常规返回 403/401，而不是 500。 + `hmac.compare_digest` 遇到含非 ASCII 字符的 `str` 参数会抛出 `TypeError`，而 + Starlette 以 latin-1 解码请求头字节，一个 `0xE9` 字节就能让比较崩溃。Gateway 现在通过统一的 + `app.gateway.utils.constant_time_equals` 比较 UTF-8 字节，独立部署的 provisioner + 则在原处编码后比较。此前不存在绕过，请求本就会失败，只是状态码错误。([#6076]) - **智能体：** 上下文压缩的 fraction 触发器与 fraction 保留量现在使用当前运行 模型的上下文 profile；单独配置的 `summarization.model_name` 只负责生成摘要。 这避免运行模型与摘要模型的窗口不一致时压缩过晚或过早。中间件发布身份现在 @@ -5336,4 +5342,5 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#6066]: https://github.com/bytedance/deer-flow/pull/6066 [#6069]: https://github.com/bytedance/deer-flow/pull/6069 [#6070]: https://github.com/bytedance/deer-flow/pull/6070 +[#6076]: https://github.com/bytedance/deer-flow/pull/6076 [#6088]: https://github.com/bytedance/deer-flow/pull/6088 diff --git a/backend/app/gateway/csrf_middleware.py b/backend/app/gateway/csrf_middleware.py index 01ef02ebe4..fa548a710e 100644 --- a/backend/app/gateway/csrf_middleware.py +++ b/backend/app/gateway/csrf_middleware.py @@ -18,6 +18,7 @@ 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 app.gateway.utils import constant_time_equals from deerflow.trace_context import TRACE_ID_HEADER CSRF_COOKIE_NAME = "csrf_token" @@ -245,7 +246,7 @@ class CSRFMiddleware(BaseHTTPMiddleware): content={"detail": "CSRF token missing. Include X-CSRF-Token header."}, ) - if not secrets.compare_digest(cookie_token, header_token): + if not constant_time_equals(cookie_token, header_token): return JSONResponse( status_code=403, content={"detail": "CSRF token mismatch."}, diff --git a/backend/app/gateway/internal_auth.py b/backend/app/gateway/internal_auth.py index 85c2d0bf85..460c298d0b 100644 --- a/backend/app/gateway/internal_auth.py +++ b/backend/app/gateway/internal_auth.py @@ -7,6 +7,7 @@ import secrets from types import SimpleNamespace from typing import Any +from app.gateway.utils import constant_time_equals from deerflow.config.paths import make_safe_user_id from deerflow.runtime.user_context import DEFAULT_USER_ID @@ -36,7 +37,7 @@ def create_internal_auth_headers(*, owner_user_id: str | None = None) -> dict[st def is_valid_internal_auth_token(token: str | None) -> bool: """Return True when *token* matches this Gateway worker's internal token.""" - return bool(token) and secrets.compare_digest(token, _INTERNAL_AUTH_TOKEN) + return bool(token) and constant_time_equals(token, _INTERNAL_AUTH_TOKEN) def get_internal_user(owner_user_id: str | None = None): diff --git a/backend/app/gateway/langgraph_auth.py b/backend/app/gateway/langgraph_auth.py index 1064487079..0c035763f6 100644 --- a/backend/app/gateway/langgraph_auth.py +++ b/backend/app/gateway/langgraph_auth.py @@ -14,7 +14,6 @@ Two layers: 2. @auth.on — returns metadata filter so each user only sees own threads """ -import secrets from contextvars import ContextVar from uuid import uuid4 @@ -25,6 +24,7 @@ from app.gateway.auth.errors import TokenError from app.gateway.auth.jwt import decode_token from app.gateway.auth_disabled import AUTH_DISABLED_USER_ID, is_auth_disabled from app.gateway.deps import get_local_provider +from app.gateway.utils import constant_time_equals from deerflow.mcp_scope import ( THREAD_INCARNATION_CONTEXT_KEY, THREAD_INCARNATION_METADATA_GUARD_KEY, @@ -234,7 +234,7 @@ def _check_csrf(request) -> None: detail="CSRF token missing. Include X-CSRF-Token header.", ) - if not secrets.compare_digest(cookie_token, header_token): + if not constant_time_equals(cookie_token, header_token): raise Auth.exceptions.HTTPException( status_code=403, detail="CSRF token mismatch.", diff --git a/backend/app/gateway/routers/auth.py b/backend/app/gateway/routers/auth.py index 23be4a0168..c0da7df496 100644 --- a/backend/app/gateway/routers/auth.py +++ b/backend/app/gateway/routers/auth.py @@ -4,7 +4,6 @@ import asyncio import logging import os import re -import secrets import time import urllib.parse from ipaddress import ip_address, ip_network @@ -37,6 +36,7 @@ from app.gateway.auth.session_cookie_state import SKIP_AUTH_CSRF_COOKIE_STATE_AT from app.gateway.auth.user_provisioning import get_or_provision_oidc_user from app.gateway.csrf_middleware import CSRF_COOKIE_NAME, _request_origin, auth_csrf_cookie_settings, generate_csrf_token, is_secure_request from app.gateway.deps import get_current_user_from_request, get_local_provider +from app.gateway.utils import constant_time_equals from deerflow.config.auth_config import OIDCProviderConfig logger = logging.getLogger(__name__) @@ -1031,7 +1031,7 @@ async def oauth_callback( if not state_payload: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Missing or expired OIDC state cookie") - if not secrets.compare_digest(state_payload.state, state): + if not constant_time_equals(state_payload.state, state): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="OIDC state mismatch") # ── Resolve redirect URI ───────────────────────────────────────── diff --git a/backend/app/gateway/routers/github_webhooks.py b/backend/app/gateway/routers/github_webhooks.py index 1460351548..93e9a15c85 100644 --- a/backend/app/gateway/routers/github_webhooks.py +++ b/backend/app/gateway/routers/github_webhooks.py @@ -35,6 +35,7 @@ from typing import Any from fastapi import APIRouter, Header, HTTPException, Request from app.gateway.github.dispatcher import fanout_event +from app.gateway.utils import constant_time_equals logger = logging.getLogger(__name__) @@ -110,7 +111,7 @@ def _verify_signature(secret: str, body: bytes, signature_header: str | None) -> return False provided = signature_header.removeprefix("sha256=").strip() expected = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest() - return hmac.compare_digest(provided, expected) + return constant_time_equals(provided, expected) def _summarise_event(event: str, payload: dict[str, Any]) -> str: diff --git a/backend/app/gateway/utils.py b/backend/app/gateway/utils.py index 8368d84fc8..b42ed00f0a 100644 --- a/backend/app/gateway/utils.py +++ b/backend/app/gateway/utils.py @@ -1,6 +1,20 @@ """Shared utility helpers for the Gateway layer.""" +import hmac + def sanitize_log_param(value: str) -> str: """Strip control characters to prevent log injection.""" return value.replace("\n", "").replace("\r", "").replace("\x00", "") + + +def constant_time_equals(a: str, b: str) -> bool: + """Compare two secret strings in constant time, never raising on content. + + ``hmac.compare_digest`` raises ``TypeError`` for ``str`` operands holding + non-ASCII characters, and headers, cookies and query values are client + controlled, so a stray byte would turn a rejection into a 500. Comparing + the UTF-8 encodings keeps the same result for ASCII input; ``surrogatepass`` + lets even a lone surrogate encode instead of raising. + """ + return hmac.compare_digest(a.encode("utf-8", "surrogatepass"), b.encode("utf-8", "surrogatepass")) diff --git a/backend/tests/test_auth_middleware.py b/backend/tests/test_auth_middleware.py index 38251dc3c4..e1a8c371a6 100644 --- a/backend/tests/test_auth_middleware.py +++ b/backend/tests/test_auth_middleware.py @@ -447,6 +447,17 @@ def test_protected_post_with_internal_auth_header_passes(): assert res.status_code == 200 +def test_protected_post_with_non_ascii_internal_auth_header_returns_401(client): + from app.gateway.internal_auth import INTERNAL_AUTH_HEADER_NAME + + res = client.post( + "/api/threads/abc/runs/stream", + headers={INTERNAL_AUTH_HEADER_NAME: "forged-tok\xe9n".encode("latin-1")}, + ) + + assert res.status_code == 401 + + # ── Method matrix: PUT/DELETE/PATCH also protected ──────────────────────── diff --git a/backend/tests/test_csrf_middleware.py b/backend/tests/test_csrf_middleware.py index f9d4d48a00..0b4329491f 100644 --- a/backend/tests/test_csrf_middleware.py +++ b/backend/tests/test_csrf_middleware.py @@ -276,6 +276,24 @@ def test_non_auth_mutation_rejects_mismatched_double_submit_token(): assert response.json()["detail"] == "CSRF token mismatch." +def test_non_auth_mutation_rejects_non_ascii_double_submit_token(): + # Starlette decodes header bytes as latin-1, so a raw 0xE9 reaches the + # comparison as "é"; that must be a 403 mismatch, not a 500. + client = TestClient(_make_app(), base_url="https://deerflow.example") + client.cookies.set("csrf_token", "cookie-token") + + response = client.post( + "/api/threads/abc/runs/stream", + headers={ + "Origin": "https://deerflow.example", + "X-CSRF-Token": "cookie-tok\xe9n".encode("latin-1"), + }, + ) + + assert response.status_code == 403 + assert response.json()["detail"] == "CSRF token mismatch." + + def test_channel_posts_require_double_submit_csrf(): client = TestClient(_make_app(), base_url="https://deerflow.example") diff --git a/backend/tests/test_gateway_utils.py b/backend/tests/test_gateway_utils.py new file mode 100644 index 0000000000..6b76f77ae1 --- /dev/null +++ b/backend/tests/test_gateway_utils.py @@ -0,0 +1,21 @@ +"""Tests for shared Gateway utility helpers.""" + +import pytest + +from app.gateway.utils import constant_time_equals + + +@pytest.mark.parametrize( + ("a", "b", "expected"), + [ + ("token", "token", True), + ("token", "other", False), + ("tok\xe9n", "tok\xe9n", True), + ("token", "tok\xe9n", False), + ("tok\xe9n", "token", False), + ("tok\ud800n", "token", False), + ("", "", True), + ], +) +def test_constant_time_equals_never_raises_on_non_ascii(a, b, expected): + assert constant_time_equals(a, b) is expected diff --git a/backend/tests/test_github_webhooks.py b/backend/tests/test_github_webhooks.py index 49d6d2bd63..daec491dae 100644 --- a/backend/tests/test_github_webhooks.py +++ b/backend/tests/test_github_webhooks.py @@ -315,6 +315,21 @@ def test_signature_mismatch_returns_401(client: TestClient) -> None: assert response.status_code == 401 +def test_non_ascii_signature_returns_401(client: TestClient) -> None: + body = b'{"zen": "x"}' + response = client.post( + "/api/webhooks/github", + content=body, + headers={ + "X-GitHub-Event": "ping", + "X-GitHub-Delivery": DELIVERY_ID, + "X-Hub-Signature-256": ("sha256=" + "\xe9" * 64).encode("latin-1"), + }, + ) + + assert response.status_code == 401 + + def test_signature_verified_against_exact_bytes(client: TestClient) -> None: """Signature must be computed over the request body bytes, not re-serialised JSON. Whitespace and key ordering matter.""" diff --git a/backend/tests/test_langgraph_auth.py b/backend/tests/test_langgraph_auth.py index ae4e49ae0e..9c53adda0f 100644 --- a/backend/tests/test_langgraph_auth.py +++ b/backend/tests/test_langgraph_auth.py @@ -764,6 +764,22 @@ def test_csrf_post_mismatched_token(): assert "mismatch" in str(exc.value.detail) +def test_csrf_post_non_ascii_token_is_a_mismatch(): + """POST with a non-ASCII CSRF header → 403 mismatch, not TypeError.""" + with pytest.raises(Auth.exceptions.HTTPException) as exc: + asyncio.run( + authenticate( + _req( + method="POST", + cookies={"access_token": "some-jwt", "csrf_token": "real-token"}, + headers={"x-csrf-token": "real-tok\xe9n"}, + ) + ) + ) + assert exc.value.status_code == 403 + assert "mismatch" in str(exc.value.detail) + + def test_csrf_post_matching_token_proceeds_to_jwt(): """POST with matching CSRF tokens passes CSRF check, then fails on JWT.""" with pytest.raises(Auth.exceptions.HTTPException) as exc: diff --git a/backend/tests/test_oidc_auth.py b/backend/tests/test_oidc_auth.py index 08ccc74b61..1337977ff4 100644 --- a/backend/tests/test_oidc_auth.py +++ b/backend/tests/test_oidc_auth.py @@ -405,3 +405,33 @@ def test_oidc_redirect_uri_fallback_plain_host_when_no_proxy_headers(): result = _resolve_oidc_redirect_uri(req, "keycloak", cfg) assert result == "http://localhost:8001/api/v1/auth/callback/keycloak" + + +def test_oidc_callback_rejects_non_ascii_state_as_mismatch(monkeypatch): + """A percent-encoded non-ASCII ``state`` is a 403 mismatch, not a 500.""" + from types import SimpleNamespace + + from fastapi import FastAPI + from fastapi.testclient import TestClient + + import deerflow.config.app_config as app_config_module + from app.gateway.auth.config import AuthConfig, get_auth_config, set_auth_config + from app.gateway.auth.oidc_state import OIDCStatePayload, _sign_state_payload + from app.gateway.routers import auth as auth_router + + previous_auth_config = get_auth_config() + set_auth_config(AuthConfig(jwt_secret="test-secret-key-for-oidc-state-mismatch-min-32")) + try: + oidc_config = SimpleNamespace(enabled=True, providers={"keycloak": _provider_config()}) + monkeypatch.setattr(app_config_module, "get_app_config", lambda: SimpleNamespace(auth=SimpleNamespace(oidc=oidc_config))) + app = FastAPI() + app.include_router(auth_router.router) + client = TestClient(app) + client.cookies.set("df_oidc_state_keycloak", _sign_state_payload(OIDCStatePayload(provider="keycloak", state="expected-state"))) + + response = client.get("/api/v1/auth/callback/keycloak", params={"code": "code", "state": "expected-st\xe4te"}) + finally: + set_auth_config(previous_auth_config) + + assert response.status_code == 403 + assert response.json()["detail"] == "OIDC state mismatch" diff --git a/backend/tests/test_provisioner_request_threading.py b/backend/tests/test_provisioner_request_threading.py index cf42e25e1f..d3429fb96a 100644 --- a/backend/tests/test_provisioner_request_threading.py +++ b/backend/tests/test_provisioner_request_threading.py @@ -558,6 +558,10 @@ async def test_auth_middleware(monkeypatch: pytest.MonkeyPatch, provisioner_modu r = await client.get("/api/sandboxes", headers={"X-API-Key": "wrong-key"}) assert r.status_code == 401 + # /api/* with a non-ASCII key → 401, not a TypeError from compare_digest + r = await client.get("/api/sandboxes", headers={"X-API-Key": "test-secr\xe9t".encode("latin-1")}) + assert r.status_code == 401 + # /api/* with correct key → not 401 (auth passed; handler runs with the K8s mock) r = await client.get("/api/sandboxes", headers={"X-API-Key": "test-secret"}) assert r.status_code != 401 diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index ac9c65ac81..107b8e2723 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -450,8 +450,10 @@ app = FastAPI(title="DeerFlow Sandbox Provisioner", lifespan=lifespan) @app.middleware("http") async def verify_api_key(request: Request, call_next): if request.url.path.startswith("/api/"): - key = request.headers.get("X-API-Key", "") - if not PROVISIONER_API_KEY or not secrets.compare_digest(key, PROVISIONER_API_KEY): + # Compare bytes: compare_digest raises TypeError for non-ASCII str, + # and header values are client-controlled (decoded as latin-1). + key = request.headers.get("X-API-Key", "").encode("utf-8", "surrogatepass") + if not PROVISIONER_API_KEY or not secrets.compare_digest(key, PROVISIONER_API_KEY.encode("utf-8", "surrogatepass")): logger.warning("provisioner auth rejected: %s %s", request.method, request.url.path) return Response(status_code=401, content="Unauthorized") return await call_next(request)