mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-11 06:28:58 +00:00
* fix(mcp): reject credentials that cannot travel as HTTP header values
A request-scoped secret or user_auth credential with a trailing newline
(the usual result of reading a token from a file, or a CRLF env-file),
CR/LF, surrounding whitespace, or characters outside Latin-1 sailed
through the credential interceptors into the HTTP client, where httpx/h11
reject it with an exception that echoes the full value:
LocalProtocolError: Illegal header value b'Bearer sk-...\n'
ToolErrorHandlingMiddleware copies that message into a model-visible
ToolMessage, so the secret landed in the prompt, the checkpoint, and
traces - everywhere headers_from_context promises it never goes.
Add illegal_header_value_reason to mcp/headers.py, mirroring the
transport's own rules (Latin-1 encodable; h11's field_vchar is [^\x00\s]
with SP/HTAB legal only between visible characters), and fail closed in
both interceptors before the value can reach the client. The denial names
only the secret key (plus the reason) and never repeats the value.
Illegal values are denied regardless of on_missing: the key is present,
so a passthrough fallback would silently run the call under the shared
discovery credential - the exact authority confusion the deny default
exists to prevent.
Values the transport accepts are not rejected: embedded SP/HTAB
('Bearer <token>'), Latin-1 high bytes, and DEL all still pass, pinned
by tests against h11's observed behaviour.
* fix(mcp): tighten header value validation to httpx's ASCII boundary
The validator mirrored h11's Latin-1 boundary, but the transport rejects
more than h11 does: build_server_params hands dict[str, str] headers
through the MCP SDK's create_mcp_http_client into httpx.AsyncClient, and
httpx (pinned 0.28.1) encodes str header values as ASCII - so a Latin-1
high byte like 'Bearer caf\xe9' passed validation here only to raise
UnicodeEncodeError inside httpx before h11 ever ran, with the exception
message repeating the offending value.
Validate str values against ASCII instead, flip the tests that pinned
Latin-1 high bytes as transportable, and pin the boundary against the
real client: create_mcp_http_client must reject what the validator
flags and construct cleanly for what it accepts (embedded SP/HTAB and
DEL still pass).
Addresses review feedback on the ASCII vs Latin-1 boundary.
* fix(mcp): validate OAuth and static header values at the same boundary
The validator added for headers_from_context and user_auth left two paths
uncovered. A token endpoint returning an access_token or token_type with a
newline reached httpx/h11, which raise with the full token in the message, and
ToolErrorHandlingMiddleware copies that message into a model-visible
ToolMessage -- the leak this PR set out to close. The operator's static headers
had the same hole.
OAuthTokenManager.get_authorization_header now renders the Authorization value
through one checked helper, so the tool interceptor, the initial discovery
headers and the durable task path are all covered by a single guard. The
rendered value is what gets checked rather than the two fields separately,
because that is what the transport sees: an access_token with leading
whitespace is legal once it follows "Bearer ".
build_server_params applies the same check to statically configured headers.
build_servers_config already isolates a per-server failure, so a bad value
drops that one server and logs the reason instead of the value.
* docs(mcp): correct which transport echoes the full header value
The rationale claimed httpx and h11 both render the full value into their
exception message. Only h11 does, on the line break and surrounding whitespace
cases. httpx's ASCII failure is a UnicodeEncodeError naming the offending
character and its position, not the credential, so at most one character
escapes there; refusing the value up front buys an actionable error rather than
an encode failure raised from inside the client.
Corrected in headers.py and in every copy of the claim: context_headers.py,
user_scoped_auth.py, oauth.py, client.py, mcp/AGENTS.md, docs/MCP_SERVER.md,
the frontend mcp.mdx, and the test comments carrying the same wording. No
behavior change.
---------
Co-authored-by: Terminator666666 <Terminator666666@users.noreply.github.com>
104 lines
4.6 KiB
Python
104 lines
4.6 KiB
Python
"""Case-insensitive header writes for MCP tool-call interceptors.
|
|
|
|
HTTP field names are case-insensitive (RFC 9110 §5.1), but every dictionary on
|
|
the path from config to the wire is case-*sensitive*: ``build_server_params``
|
|
copies the operator's static ``headers`` spelling verbatim, and
|
|
``langchain_mcp_adapters`` merges interceptor overrides with a plain
|
|
``{**connection_headers, **override_headers}`` splat. So a static
|
|
``authorization`` and an interceptor-written ``Authorization`` do not collide —
|
|
both survive, httpx puts both on the wire, and a server reading the field with a
|
|
single-value accessor sees the *first* one, which is the static entry the
|
|
override was supposed to replace.
|
|
|
|
The credential interceptors therefore write header names through
|
|
:func:`apply_header_overrides`, which drops any key differing only in case and
|
|
emits the spelling the connection already uses, so the adapter's merge replaces
|
|
the static entry instead of duplicating it.
|
|
|
|
Every path that writes a value into an MCP request header checks it with
|
|
:func:`illegal_header_value_reason` first — both credential interceptors, the
|
|
OAuth token manager, and ``build_server_params`` for the operator's static
|
|
headers.
|
|
|
|
The two halves of that boundary fail differently, and only one of them leaks.
|
|
h11 rejects line breaks and surrounding whitespace with the *full value* in its
|
|
message, ``ToolErrorHandlingMiddleware`` copies that into a model-visible
|
|
ToolMessage, and the credential lands in the prompt, the checkpoint, and
|
|
traces. That is the leak this check exists to stop. httpx encodes ``str``
|
|
values as ASCII and raises ``UnicodeEncodeError``, whose message names only the
|
|
offending character and its position, so at most one character escapes;
|
|
refusing that value up front buys an actionable error rather than an encode
|
|
failure raised from inside the client.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from collections.abc import Iterable, Mapping
|
|
|
|
# What h11 refuses inside a field value (its field_vchar is ``[^\x00\s]``):
|
|
# NUL and the vertical-whitespace characters. SP/HTAB are legal separators
|
|
# *between* visible characters but not at either end.
|
|
_FORBIDDEN_HEADER_VALUE_CHARS = re.compile(r"[\x00\n\x0b\x0c\r]")
|
|
|
|
|
|
def illegal_header_value_reason(value: str) -> str | None:
|
|
"""Explain why *value* cannot be sent as an HTTP header value, or ``None``.
|
|
|
|
Mirrors what the transport enforces — the MCP clients hand ``dict[str,
|
|
str]`` headers to ``httpx``, which encodes ``str`` values as ASCII (raising
|
|
``UnicodeEncodeError`` before h11 ever sees the value), and h11 rejects
|
|
NUL/vertical whitespace and leading or trailing SP/HTAB — without
|
|
repeating the value, so callers can fail closed with a message that names
|
|
the credential instead of leaking it.
|
|
"""
|
|
try:
|
|
value.encode("ascii")
|
|
except UnicodeEncodeError:
|
|
return "contains characters outside ASCII"
|
|
if _FORBIDDEN_HEADER_VALUE_CHARS.search(value):
|
|
return "contains a line break or another forbidden control character"
|
|
if value != value.strip(" \t"):
|
|
return "has leading or trailing whitespace"
|
|
return None
|
|
|
|
|
|
def header_spellings(names: Iterable[str] | None) -> dict[str, str]:
|
|
"""Index header names by their lowercased form.
|
|
|
|
Used to pin the spelling an interceptor should emit: the connection's own
|
|
static ``headers`` keys, which the adapter merges the override into. A
|
|
server that declares none passes ``None`` here rather than being special-
|
|
cased at each call site.
|
|
"""
|
|
return {name.lower(): name for name in (names or ())}
|
|
|
|
|
|
def apply_header_overrides(
|
|
base: Mapping[str, str] | None,
|
|
overrides: Mapping[str, str],
|
|
*,
|
|
spellings: Mapping[str, str] | None = None,
|
|
) -> dict[str, str]:
|
|
"""Return ``base`` with ``overrides`` applied, case-insensitively.
|
|
|
|
``spellings`` maps a lowercased header name to the spelling to emit and
|
|
takes priority over ``base``'s own keys, so an override lands on the static
|
|
connection header it is meant to replace even when an earlier interceptor
|
|
already wrote a differently-cased variant. Any key of ``base`` that differs
|
|
from the emitted name only in case is removed, so the result never carries
|
|
one header under two spellings.
|
|
"""
|
|
merged = dict(base or {})
|
|
lookup = dict(spellings or {})
|
|
for key in merged:
|
|
lookup.setdefault(key.lower(), key)
|
|
|
|
for name, value in overrides.items():
|
|
lowered = name.lower()
|
|
canonical = lookup.get(lowered, name)
|
|
for existing in [key for key in merged if key != canonical and key.lower() == lowered]:
|
|
del merged[existing]
|
|
merged[canonical] = value
|
|
return merged
|