mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(mcp): reject credentials that cannot travel as HTTP header values (#5066)
* 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>
This commit is contained in:
Terminator666666
GitHub
parent
317577e285
commit
c17aa8b98f
@ -409,6 +409,14 @@ The caller supplies the values on each run request:
|
||||
the discovery credential — which in a multi-tenant deployment would send one
|
||||
tenant's request under another tenant's authority. Set `"passthrough"` to opt
|
||||
out and forward the static headers instead.
|
||||
- A value that cannot be sent as an HTTP header — a stray newline picked up
|
||||
when reading a token from a file, leading/trailing whitespace, characters
|
||||
outside ASCII — is always denied, regardless of `on_missing`. The error
|
||||
names the offending key but never repeats the value; without this check a
|
||||
newline or stray whitespace would reach h11, whose rejection echoes the full
|
||||
credential into a model-visible tool error. The same check covers every other
|
||||
way a value reaches these headers: `user_auth`, the OAuth token returned by
|
||||
the token endpoint, and the static `headers` in the config file.
|
||||
- Precedence for a server declaring several sources: static `headers` <
|
||||
`oauth` < `user_auth` < `headers_from_context`. The value chosen for this one
|
||||
request is the most specific, so it wins.
|
||||
|
||||
@ -9,9 +9,9 @@
|
||||
- **Cache invalidation**: Detects extensions-config changes by comparing the resolved config path and a `(mtime, size, sha256)` content signature against the values recorded at initialization, not a strict mtime `>` comparison. This catches same-second edits, mtime that stays put or moves backward (`git checkout`, `cp -p` / backup restore, `tar` / `rsync`, object-store / network mounts), and a switch to a different config file with an equal-or-older mtime. The signature helper (`config/file_signature.py::get_config_signature`) is shared with `config/app_config.py::get_app_config()` for the sibling runtime-editable config file, rather than each maintaining its own copy. `ExtensionsConfig.resolve_config_path()` raises `FileNotFoundError` for an explicit `config_path`/`DEER_FLOW_EXTENSIONS_CONFIG_PATH` that points at a missing file — an operator-asserted path going missing is a real misconfiguration, so this is intentionally loud for callers that load the config for actual use (e.g. `from_file()` via `get_mcp_tools()`); only the fallback search mode returns `None`. The MCP cache's own path resolution (`mcp/cache.py::_resolve_config_path`) is narrower: it catches that specific `FileNotFoundError` locally and treats it the same as "unconfigured", so this staleness check degrades to "not stale" instead of propagating an exception when a previously-valid explicit/env-var config disappears mid-run
|
||||
- **Transports**: stdio (command-based), SSE, HTTP
|
||||
- **Per-server tool-name prefixing**: `mcpServers.<server>.tool_name_prefix` defaults to `true`, preserving the collision-safe `<server_name>_` prefix. Servers whose tools already carry a stable namespace may set it to `false`; discovery then calls `langchain_mcp_adapters.tools.load_mcp_tools` with that server's flag. Source routing and stdio session-pool wrapping are based on the producing server and transport, never on whether the visible tool name starts with the server prefix.
|
||||
- **OAuth (HTTP/SSE)**: Supports token endpoint flows (`client_credentials`, `refresh_token`) with automatic token refresh + Authorization header injection
|
||||
- **Per-user credentials (HTTP/SSE)**: `mcpServers.<server>.user_auth` maps DeerFlow user ids to credential header values (`$ENV_VAR` references supported). `mcp/user_scoped_auth.py::build_user_scoped_auth_interceptor` rewrites the configured header on every tool call from the authenticated runtime user (registered after OAuth in `mcp/interceptors.py`, so its per-user value wins the header for servers declaring both). Fail-closed: an unmapped user or an empty resolved credential raises a `ToolException` unless `on_missing: "passthrough"` is set. The server's static `headers` serve startup tool discovery only. Gateway GET masks `user_auth.users` values; PUT round-trips masked values by preserving stored credentials.
|
||||
- **Per-request credentials (HTTP/SSE)**: `mcpServers.<server>.headers_from_context` maps HTTP header names to keys of the run request's `config.context.secrets` carrier, for credentials the caller chooses per request (multi-tenant gateways, per-run API keys) rather than per configured user. `mcp/context_headers.py::build_context_headers_interceptor` resolves the mapping on every tool call and rewrites those headers. Registered **after** `user_auth` in `mcp/interceptors.py`, so for a server declaring several sources the per-request value wins the final header: precedence is static `headers` < `oauth` < `user_auth` < `headers_from_context`. Fail-closed: a mapped key absent from the request secrets (or resolved empty) raises a `ToolException` naming only the missing key, unless `on_missing: "passthrough"` is set — a silent fallback would send one tenant's call under the discovery credential's authority. `sse`/`http` only; a stdio server warns and is skipped, as with `user_auth`. The block stores names, never a credential, so the Gateway returns it unmasked and a `PUT` replaces the declared mapping verbatim; only `extra="allow"` keys inside it get sensitive-key masking, and those are restored from the stored block on a round-trip like every other masked extra. Durable `task_toolsets` calls split: submit is awaited inside the Agent run and carries the mapped headers (`McpTaskToolCaller.call_tool(request_scoped_headers=True)`, set only by `OrdinaryMcpTaskDriver.submit`), while status and cancel run after that run ended and keep the server-level credentials — so `on_missing: "deny"` covers submit but not those polls, which is what the startup warning is about.
|
||||
- **OAuth (HTTP/SSE)**: Supports token endpoint flows (`client_credentials`, `refresh_token`) with automatic token refresh + Authorization header injection. The rendered `<token_type> <access_token>` passes `mcp/headers.py::illegal_header_value_reason` inside `OAuthTokenManager.get_authorization_header` — the one boundary the tool interceptor, the initial discovery headers and the durable task path all read their value from — so a token endpoint returning something the transport would refuse fails closed instead of letting h11 echo the token into a model-visible tool error. The rendered value is what gets checked, not the two fields separately, because that is what the transport sees. The operator's static `headers` get the same check in `mcp/client.py::build_server_params`, where `build_servers_config` already drops just that server and logs the reason.
|
||||
- **Per-user credentials (HTTP/SSE)**: `mcpServers.<server>.user_auth` maps DeerFlow user ids to credential header values (`$ENV_VAR` references supported). `mcp/user_scoped_auth.py::build_user_scoped_auth_interceptor` rewrites the configured header on every tool call from the authenticated runtime user (registered after OAuth in `mcp/interceptors.py`, so its per-user value wins the header for servers declaring both). Fail-closed: an unmapped user or an empty resolved credential raises a `ToolException` unless `on_missing: "passthrough"` is set; a resolved credential the transport would refuse as a header value (line break, surrounding whitespace, non-ASCII — `mcp/headers.py::illegal_header_value_reason`) is always denied without echoing the value, since h11 renders the full value into its exception message on the line break and whitespace cases and tool errors are model-visible (httpx catches the non-ASCII case earlier, naming only the offending character). The server's static `headers` serve startup tool discovery only. Gateway GET masks `user_auth.users` values; PUT round-trips masked values by preserving stored credentials.
|
||||
- **Per-request credentials (HTTP/SSE)**: `mcpServers.<server>.headers_from_context` maps HTTP header names to keys of the run request's `config.context.secrets` carrier, for credentials the caller chooses per request (multi-tenant gateways, per-run API keys) rather than per configured user. `mcp/context_headers.py::build_context_headers_interceptor` resolves the mapping on every tool call and rewrites those headers. Registered **after** `user_auth` in `mcp/interceptors.py`, so for a server declaring several sources the per-request value wins the final header: precedence is static `headers` < `oauth` < `user_auth` < `headers_from_context`. Fail-closed: a mapped key absent from the request secrets (or resolved empty) raises a `ToolException` naming only the missing key, unless `on_missing: "passthrough"` is set — a silent fallback would send one tenant's call under the discovery credential's authority. A resolved value the transport would refuse as a header value (line break, surrounding whitespace, non-ASCII) is always denied regardless of `on_missing`, without echoing the value: h11 renders the full value into its exception message on a line break or surrounding whitespace, and `ToolErrorHandlingMiddleware` copies tool errors into model-visible messages, so an unchecked bad credential would land the secret in the prompt, the checkpoint, and traces. `sse`/`http` only; a stdio server warns and is skipped, as with `user_auth`. The block stores names, never a credential, so the Gateway returns it unmasked and a `PUT` replaces the declared mapping verbatim; only `extra="allow"` keys inside it get sensitive-key masking, and those are restored from the stored block on a round-trip like every other masked extra. Durable `task_toolsets` calls split: submit is awaited inside the Agent run and carries the mapped headers (`McpTaskToolCaller.call_tool(request_scoped_headers=True)`, set only by `OrdinaryMcpTaskDriver.submit`), while status and cancel run after that run ended and keep the server-level credentials — so `on_missing: "deny"` covers submit but not those polls, which is what the startup warning is about.
|
||||
- **Header names are case-insensitive** (`mcp/headers.py`): every credential interceptor writes through `apply_header_overrides`, which drops a key differing only in case and emits the spelling the connection already uses. Without it a static `authorization` and an injected `Authorization` both reach httpx — the adapter merges connection and override headers with a plain `{**static, **override}` splat — and a server reading the field with a single-value accessor gets the static entry, silently inverting the precedence above. `headers_from_context.headers` also rejects two spellings of one header at config load.
|
||||
- **Reading the run context from an interceptor**: use `request.runtime` (LangGraph's tool node injects a `ToolRuntime` into any tool parameter named `runtime`, which covers both the pooled stdio wrapper and `langchain-mcp-adapters`' own HTTP/SSE tool), falling back to ambient `langgraph.runtime.get_runtime()`. Do **not** use `langgraph.config.get_config()["context"]`: the run context rides the runtime, not the `RunnableConfig` propagated to child runnables, so that key is `None` inside a tool call. `tests/test_mcp_context_headers.py::test_adapter_tool_receives_the_runtime_langgraph_injects` pins the injection rule against an upstream rename by disabling the ambient fallback and driving a real adapter tool through a real graph.
|
||||
- **Routing hints**: `extensions_config.json -> mcpServers.<server>.routing` and
|
||||
|
||||
@ -4,6 +4,7 @@ import logging
|
||||
from typing import Any
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig
|
||||
from deerflow.mcp.headers import illegal_header_value_reason
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@ -35,6 +36,17 @@ def build_server_params(server_name: str, config: McpServerConfig) -> dict[str,
|
||||
params["url"] = config.url
|
||||
# Add headers if present
|
||||
if config.headers:
|
||||
# A statically configured value the transport would refuse gets the
|
||||
# same treatment as a request-scoped one: h11 renders the full
|
||||
# value into its exception on a line break or surrounding
|
||||
# whitespace, which reaches the model through
|
||||
# ToolErrorHandlingMiddleware. These values are API keys often
|
||||
# enough to be worth refusing here, where build_servers_config
|
||||
# already drops just this server and logs the reason.
|
||||
for header_name, header_value in config.headers.items():
|
||||
reason = illegal_header_value_reason(header_value)
|
||||
if reason is not None:
|
||||
raise ValueError(f"MCP server '{server_name}' has a header '{header_name}' that cannot be sent as an HTTP header value: it {reason}")
|
||||
params["headers"] = config.headers
|
||||
else:
|
||||
raise ValueError(f"MCP server '{server_name}' has unsupported transport type: {transport_type}")
|
||||
|
||||
@ -27,6 +27,16 @@ Fail-closed by default: a mapped key that is absent from the request secrets
|
||||
falling back to the server's static discovery credential, which in a
|
||||
multi-tenant deployment would send one tenant's request under another
|
||||
tenant's authority. ``on_missing: "passthrough"`` is the explicit opt-out.
|
||||
|
||||
A resolved value the transport would refuse (line break, surrounding
|
||||
whitespace, non-ASCII — see ``mcp/headers.py``) is always denied, regardless
|
||||
of ``on_missing``. h11 renders the full value into its exception message when
|
||||
it refuses a line break or surrounding whitespace, and that message would
|
||||
otherwise travel into a model-visible tool error and the trace; the non-ASCII
|
||||
case fails inside httpx instead, which names only the offending character, so
|
||||
denying it here buys an actionable error rather than secrecy. Either way a
|
||||
passthrough fallback would run the call under the shared discovery credential
|
||||
even though the caller did supply a key.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@ -37,7 +47,11 @@ from typing import Any
|
||||
from langchain_core.tools import ToolException
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpContextHeadersConfig
|
||||
from deerflow.mcp.headers import apply_header_overrides, header_spellings
|
||||
from deerflow.mcp.headers import (
|
||||
apply_header_overrides,
|
||||
header_spellings,
|
||||
illegal_header_value_reason,
|
||||
)
|
||||
from deerflow.runtime.secret_context import extract_request_secrets
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@ -128,14 +142,44 @@ def build_context_headers_interceptor(extensions_config: ExtensionsConfig) -> An
|
||||
secrets = _request_secrets(request)
|
||||
resolved: dict[str, str] = {}
|
||||
missing: list[str] = []
|
||||
illegal: dict[str, str] = {}
|
||||
for header_name, secret_key in context_headers.headers.items():
|
||||
# Empty string covers a caller-side `$ENV_VAR` that was unset: an
|
||||
# empty credential must fail closed rather than send an empty header.
|
||||
value = secrets.get(secret_key, "")
|
||||
if value:
|
||||
resolved[header_name] = value
|
||||
else:
|
||||
if not value:
|
||||
missing.append(secret_key)
|
||||
continue
|
||||
# A value the transport would refuse (trailing newline from reading
|
||||
# a token file, CR/LF, non-ASCII) must be rejected *here*. h11
|
||||
# renders the full value into its exception message on the line
|
||||
# break and whitespace cases, and ToolErrorHandlingMiddleware
|
||||
# copies that message into a model-visible ToolMessage — putting
|
||||
# the secret in the prompt, the checkpoint, and traces, everywhere
|
||||
# this module promises it never goes. Always denied, regardless of
|
||||
# on_missing: the key is present, so falling back to the discovery
|
||||
# credential would silently run this tenant's call under the shared
|
||||
# authority.
|
||||
reason = illegal_header_value_reason(value)
|
||||
if reason is not None:
|
||||
illegal[secret_key] = reason
|
||||
continue
|
||||
resolved[header_name] = value
|
||||
|
||||
if illegal:
|
||||
illegal_keys = ", ".join(sorted(illegal))
|
||||
logger.warning(
|
||||
"Denied MCP tool call to server '%s': request-scoped secret(s) %s cannot be sent as an HTTP header value",
|
||||
request.server_name,
|
||||
illegal_keys,
|
||||
)
|
||||
details = "; ".join(f"'{key}' {reason}" for key, reason in sorted(illegal.items()))
|
||||
# Like the missing-key denial below, only the configured key names
|
||||
# (plus the reason) are surfaced — never the value.
|
||||
raise ToolException(
|
||||
f"MCP server '{request.server_name}' cannot send request-scoped credential(s) as HTTP header values: {details}. "
|
||||
"Fix the value passed in config.context.secrets; a stray newline picked up when reading a token from a file is the usual cause."
|
||||
)
|
||||
|
||||
if missing and context_headers.on_missing == "deny":
|
||||
missing_keys = ", ".join(sorted(missing))
|
||||
|
||||
@ -14,12 +14,54 @@ 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.
|
||||
|
||||
@ -10,7 +10,7 @@ from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpOAuthConfig
|
||||
from deerflow.mcp.headers import apply_header_overrides, header_spellings
|
||||
from deerflow.mcp.headers import apply_header_overrides, header_spellings, illegal_header_value_reason
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@ -63,7 +63,7 @@ class OAuthTokenManager:
|
||||
|
||||
token = self._tokens.get(server_name)
|
||||
if token and not self._is_expiring(token, oauth):
|
||||
return f"{token.token_type} {token.access_token}"
|
||||
return self._authorization_value(token, server_name)
|
||||
|
||||
lock = self._locks[server_name]
|
||||
# Acquire the OS-level lock off-thread so a blocking wait never blocks this
|
||||
@ -102,15 +102,45 @@ class OAuthTokenManager:
|
||||
try:
|
||||
token = self._tokens.get(server_name)
|
||||
if token and not self._is_expiring(token, oauth):
|
||||
return f"{token.token_type} {token.access_token}"
|
||||
return self._authorization_value(token, server_name)
|
||||
|
||||
fresh = await self._fetch_token(oauth)
|
||||
self._tokens[server_name] = fresh
|
||||
logger.info(f"Refreshed OAuth access token for MCP server: {server_name}")
|
||||
return f"{fresh.token_type} {fresh.access_token}"
|
||||
return self._authorization_value(fresh, server_name)
|
||||
finally:
|
||||
lock.release()
|
||||
|
||||
@staticmethod
|
||||
def _authorization_value(token: _OAuthToken, server_name: str) -> str:
|
||||
"""Render the Authorization value, refusing one the transport would echo.
|
||||
|
||||
The token endpoint's response is not this process's to control: an
|
||||
``access_token`` or ``token_type`` carrying a newline reaches h11, which
|
||||
raises with the full value in the message, and
|
||||
``ToolErrorHandlingMiddleware`` copies that message into a
|
||||
model-visible ToolMessage. Failing closed here keeps the token out of
|
||||
the prompt, the checkpoint, and traces, at the one boundary every caller
|
||||
goes through -- the tool interceptor, the initial discovery headers, and
|
||||
the durable task path all read their value from here. A token outside
|
||||
ASCII fails earlier, inside httpx, with only the offending character in
|
||||
the message; that one is refused for a deliverable error rather than for
|
||||
secrecy.
|
||||
|
||||
The rendered value is what gets checked, not the two fields separately,
|
||||
because the rendered value is what the transport sees. An
|
||||
``access_token`` of ``" abc"`` is legal once it sits after ``Bearer ``
|
||||
even though the field on its own carries leading whitespace, and
|
||||
rejecting it would deny a token the server would have accepted.
|
||||
"""
|
||||
value = f"{token.token_type} {token.access_token}"
|
||||
reason = illegal_header_value_reason(value)
|
||||
if reason is not None:
|
||||
# Names the server and the reason, never the token: this message
|
||||
# travels to the model on the interceptor path.
|
||||
raise ValueError(f"OAuth token for MCP server '{server_name}' cannot be sent as an HTTP header value: the Authorization value {reason}. Check what the token endpoint returned for this server.")
|
||||
return value
|
||||
|
||||
@staticmethod
|
||||
def _is_expiring(token: _OAuthToken, oauth: McpOAuthConfig) -> bool:
|
||||
now = datetime.now(UTC)
|
||||
|
||||
@ -27,7 +27,11 @@ from typing import Any
|
||||
from langchain_core.tools import ToolException
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpUserScopedAuthConfig
|
||||
from deerflow.mcp.headers import apply_header_overrides, header_spellings
|
||||
from deerflow.mcp.headers import (
|
||||
apply_header_overrides,
|
||||
header_spellings,
|
||||
illegal_header_value_reason,
|
||||
)
|
||||
from deerflow.runtime.user_context import resolve_runtime_user_id
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@ -116,6 +120,26 @@ def build_user_scoped_auth_interceptor(extensions_config: ExtensionsConfig) -> A
|
||||
f"No credential is configured for your account (user id '{user_id}') on MCP server '{request.server_name}'. Ask the operator to add this exact id to that server's user_auth.users map (or set its environment variable)."
|
||||
)
|
||||
|
||||
# A credential the transport would refuse (trailing newline from a
|
||||
# token file or a CRLF env-file, non-ASCII) must be rejected here. On
|
||||
# the line break and whitespace cases h11 renders the full value into
|
||||
# its exception message, which ToolErrorHandlingMiddleware copies into
|
||||
# a model-visible ToolMessage. Always denied, regardless of on_missing
|
||||
# — the user *is* mapped, so falling back to the discovery credential
|
||||
# would silently run the call under the shared authority.
|
||||
reason = illegal_header_value_reason(credential)
|
||||
if reason is not None:
|
||||
logger.warning(
|
||||
"Denied MCP tool call to server '%s': the user_auth credential for user '%s' cannot be sent as an HTTP header value (%s)",
|
||||
request.server_name,
|
||||
user_id,
|
||||
reason,
|
||||
)
|
||||
raise ToolException(
|
||||
f"The credential configured for your account (user id '{user_id}') on MCP server '{request.server_name}' {reason}, so it cannot be sent as an HTTP header. "
|
||||
"Ask the operator to fix that server's user_auth.users entry; a stray newline in the value or its environment variable is the usual cause."
|
||||
)
|
||||
|
||||
updated_headers = apply_header_overrides(
|
||||
request.headers,
|
||||
{user_auth.header: credential},
|
||||
|
||||
@ -1,5 +1,7 @@
|
||||
"""Core behavior tests for MCP client server config building."""
|
||||
|
||||
import logging
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig
|
||||
@ -83,6 +85,63 @@ def test_build_server_params_rejects_unsupported_transport():
|
||||
build_server_params("bad-transport", config)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("value", "reason"),
|
||||
[
|
||||
("Bearer static-secret-123\n", "line break"),
|
||||
("Bearer static-secret-caf\u00e9", "outside ASCII"),
|
||||
("Bearer static-secret-456 ", "whitespace"),
|
||||
],
|
||||
ids=["trailing-newline", "non-ascii", "trailing-space"],
|
||||
)
|
||||
def test_build_server_params_rejects_illegal_header_value(value: str, reason: str):
|
||||
"""A statically configured value the transport would refuse is denied here.
|
||||
|
||||
h11 renders the full value into its exception message on a line break or
|
||||
surrounding whitespace, which ToolErrorHandlingMiddleware turns into a
|
||||
model-visible ToolMessage. These values are API keys often enough that the
|
||||
denial names the header and the reason instead.
|
||||
"""
|
||||
config = McpServerConfig(type="http", url="https://example.com/mcp", headers={"Authorization": value})
|
||||
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
build_server_params("remote-server", config)
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert reason in message
|
||||
assert "static-secret" not in message
|
||||
assert "Authorization" in message
|
||||
|
||||
|
||||
def test_build_servers_config_drops_only_the_server_with_an_illegal_header(caplog):
|
||||
config = ExtensionsConfig.model_validate(
|
||||
{
|
||||
"mcpServers": {
|
||||
"broken": {
|
||||
"enabled": True,
|
||||
"type": "http",
|
||||
"url": "https://example.com/mcp",
|
||||
"headers": {"Authorization": "Bearer static-secret-123\n"},
|
||||
},
|
||||
"healthy": {
|
||||
"enabled": True,
|
||||
"type": "http",
|
||||
"url": "https://example.com/mcp",
|
||||
"headers": {"Authorization": "Bearer fine"},
|
||||
},
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
with caplog.at_level(logging.ERROR, logger="deerflow.mcp.client"):
|
||||
servers_config = build_servers_config(config)
|
||||
|
||||
# One bad server does not take the others down with it, and the log that
|
||||
# explains the drop does not carry the value either.
|
||||
assert set(servers_config) == {"healthy"}
|
||||
assert "static-secret" not in caplog.text
|
||||
|
||||
|
||||
@pytest.mark.parametrize("transport", ["sse", "http"])
|
||||
def test_mcp_server_config_accepts_transport_alias(transport: str):
|
||||
"""The MCP-spec ``transport`` field should be accepted as an alias for ``type``.
|
||||
|
||||
@ -343,6 +343,89 @@ def test_passthrough_still_injects_the_secrets_that_are_present():
|
||||
assert result.headers == {"X-Tenant-Id": "acme"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Illegal header values
|
||||
#
|
||||
# A secret that cannot travel as an HTTP header value (trailing newline from
|
||||
# reading a token file, CR/LF, characters outside ASCII) must be rejected
|
||||
# here, before it reaches the HTTP client. On the line break and whitespace
|
||||
# cases h11 renders the full value into its LocalProtocolError message,
|
||||
# ToolErrorHandlingMiddleware copies that message into a model-visible
|
||||
# ToolMessage, and the secret lands in the prompt, the checkpoint, and traces —
|
||||
# everywhere this module promises it never goes. Non-ASCII fails earlier,
|
||||
# inside httpx, with only the offending character in the message.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_secret_with_trailing_newline_is_denied_without_calling_handler():
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
handler = AsyncMock()
|
||||
with pytest.raises(ToolException, match="tenant_token"):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token=TENANT_TOKEN + "\n")), handler))
|
||||
handler.assert_not_awaited()
|
||||
|
||||
|
||||
def test_illegal_value_deny_message_does_not_contain_the_value():
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
with pytest.raises(ToolException) as excinfo:
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token="sk-secret-value\n")), AsyncMock()))
|
||||
assert "sk-secret-value" not in str(excinfo.value)
|
||||
|
||||
|
||||
def test_illegal_value_warning_log_does_not_contain_the_value(caplog):
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
with caplog.at_level(logging.WARNING, logger="deerflow.mcp.context_headers"), pytest.raises(ToolException):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token="sk-secret-value\n")), AsyncMock()))
|
||||
assert "sk-secret-value" not in caplog.text
|
||||
assert "tenant_token" in caplog.text
|
||||
|
||||
|
||||
def test_embedded_crlf_is_denied():
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
with pytest.raises(ToolException, match="tenant_token"):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token="a\r\nX-Injected: b")), AsyncMock()))
|
||||
|
||||
|
||||
def test_non_ascii_value_is_denied():
|
||||
"""httpx encodes str header values as ASCII and raises UnicodeEncodeError otherwise."""
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
for value in ("пароль", "Bearer caf\xe9"):
|
||||
with pytest.raises(ToolException, match="tenant_token"):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token=value)), AsyncMock()))
|
||||
|
||||
|
||||
def test_leading_or_trailing_whitespace_is_denied():
|
||||
"""h11 rejects field values with leading/trailing SP or HTAB."""
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
for value in ("Bearer x ", " Bearer x", "Bearer x\t"):
|
||||
with pytest.raises(ToolException, match="tenant_token"):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token=value)), AsyncMock()))
|
||||
|
||||
|
||||
def test_illegal_value_is_denied_even_with_on_missing_passthrough():
|
||||
"""passthrough covers an *absent* key; a present-but-broken value must not
|
||||
silently fall back to the shared discovery credential."""
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}, on_missing="passthrough"))
|
||||
with pytest.raises(ToolException, match="tenant_token"):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token=TENANT_TOKEN + "\n")), AsyncMock()))
|
||||
|
||||
|
||||
def test_values_with_embedded_spaces_and_tabs_are_not_rejected():
|
||||
"""h11 allows SP/HTAB between visible characters — 'Bearer <token>' must pass."""
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Token": "tenant_token"}))
|
||||
result = asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_token="Bearer abc\tdef ghi")), _echo_handler))
|
||||
assert result.headers["X-Tenant-Token"] == "Bearer abc\tdef ghi"
|
||||
|
||||
|
||||
def test_one_illegal_mapping_denies_the_whole_call():
|
||||
"""One broken credential denies the call; it must not partially inject."""
|
||||
interceptor = build_context_headers_interceptor(_config(headers={"X-Tenant-Id": "tenant_id", "X-Org": "org"}))
|
||||
handler = AsyncMock()
|
||||
with pytest.raises(ToolException, match="org"):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_with_secrets(tenant_id="acme", org="bad\n")), handler))
|
||||
handler.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Config model
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@ -15,7 +15,11 @@ from langchain_mcp_adapters.interceptors import MCPToolCallRequest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig, McpUserScopedAuthConfig
|
||||
from deerflow.mcp.headers import apply_header_overrides, header_spellings
|
||||
from deerflow.mcp.headers import (
|
||||
apply_header_overrides,
|
||||
header_spellings,
|
||||
illegal_header_value_reason,
|
||||
)
|
||||
from deerflow.mcp.oauth import build_oauth_tool_interceptor
|
||||
from deerflow.mcp.user_scoped_auth import build_user_scoped_auth_interceptor
|
||||
|
||||
@ -64,6 +68,81 @@ def test_override_does_not_mutate_the_base():
|
||||
assert base == {"Authorization": DISCOVERY}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# illegal_header_value_reason
|
||||
#
|
||||
# The boundary 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); h11's
|
||||
# field_vchar is ``[^\x00\s]`` with SP/HTAB allowed only between visible
|
||||
# characters. Values the transport accepts must not be rejected here.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value",
|
||||
[
|
||||
"Bearer sk-token\n",
|
||||
"Bearer sk-token\r",
|
||||
"a\r\nX-Injected: b",
|
||||
"a\x00b",
|
||||
"a\x0bb",
|
||||
"a\x0cb",
|
||||
" leading-space",
|
||||
"trailing-space ",
|
||||
"trailing-tab\t",
|
||||
"caf\xe9", # Latin-1 high byte: h11 would send it, but httpx encodes str values as ASCII first
|
||||
"\u043f\u0430\u0440\u043e\u043b\u044c",
|
||||
],
|
||||
)
|
||||
def test_transport_rejected_values_are_flagged(value):
|
||||
assert illegal_header_value_reason(value) is not None
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value",
|
||||
[
|
||||
"Bearer sk-token",
|
||||
"Bearer abc\tdef",
|
||||
"two words",
|
||||
"a\x7fb", # DEL: ASCII-encodable, and h11's field_vchar accepts it
|
||||
],
|
||||
)
|
||||
def test_transport_accepted_values_are_not_flagged(value):
|
||||
assert illegal_header_value_reason(value) is None
|
||||
|
||||
|
||||
def test_reason_never_repeats_the_value():
|
||||
reason = illegal_header_value_reason("sk-secret-value\n")
|
||||
assert reason is not None
|
||||
assert "sk-secret-value" not in reason
|
||||
|
||||
|
||||
def test_validator_mirrors_the_mcp_http_client_encoding_boundary():
|
||||
"""Pin the boundary against the real client the headers are handed to.
|
||||
|
||||
``build_server_params`` passes ``dict[str, str]`` headers through the MCP
|
||||
SDK's ``create_mcp_http_client`` into ``httpx.AsyncClient``, which encodes
|
||||
``str`` header values as ASCII at construction time. A value the validator
|
||||
accepts must construct that client; the canonical counter-example — a
|
||||
Latin-1 high byte h11 itself would happily send — must be flagged by the
|
||||
validator, because httpx raises ``UnicodeEncodeError`` before h11 runs.
|
||||
That exception names the offending character rather than the credential, so
|
||||
what the denial buys here is an actionable error instead of an encode
|
||||
failure raised from inside the client.
|
||||
"""
|
||||
from mcp.shared._httpx_utils import create_mcp_http_client
|
||||
|
||||
assert illegal_header_value_reason("Bearer caf\xe9") is not None
|
||||
with pytest.raises(UnicodeEncodeError):
|
||||
create_mcp_http_client(headers={"Authorization": "Bearer caf\xe9"})
|
||||
|
||||
for value in ("Bearer sk-token", "Bearer abc\tdef ghi", "a\x7fb"):
|
||||
assert illegal_header_value_reason(value) is None
|
||||
client = create_mcp_http_client(headers={"X-Tenant-Token": value})
|
||||
assert client.headers["X-Tenant-Token"] == value
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Static header spelling validation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import threading
|
||||
from typing import Any
|
||||
|
||||
@ -613,3 +614,127 @@ def test_get_authorization_header_cancelled_while_waiting_does_not_leak_lock(mon
|
||||
# Exactly one real token fetch: the cancelled waiter must never reach
|
||||
# _fetch_token, so the third call is the only one that performs it.
|
||||
assert len(post_calls) == 1
|
||||
|
||||
|
||||
# --- Illegal header values ---------------------------------------------------
|
||||
#
|
||||
# What the token endpoint returns is not this process's to control. An
|
||||
# access_token or token_type carrying a newline reaches h11, which raises with
|
||||
# the full value in the message, and ToolErrorHandlingMiddleware copies that
|
||||
# message into a model-visible ToolMessage. Every one of these asserts the
|
||||
# token never appears in what the caller sees.
|
||||
|
||||
|
||||
def _oauth_server_config() -> ExtensionsConfig:
|
||||
return ExtensionsConfig.model_validate(
|
||||
{
|
||||
"mcpServers": {
|
||||
"secure-http": {
|
||||
"enabled": True,
|
||||
"type": "http",
|
||||
"url": "https://api.example.com/mcp",
|
||||
"oauth": {
|
||||
"enabled": True,
|
||||
"token_url": "https://auth.example.com/oauth/token",
|
||||
"grant_type": "client_credentials",
|
||||
"client_id": "client-id",
|
||||
"client_secret": "client-secret",
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _token_endpoint_returns(monkeypatch, payload: dict[str, Any]) -> list[dict[str, Any]]:
|
||||
post_calls: list[dict[str, Any]] = []
|
||||
|
||||
def _client_factory(*args, **kwargs):
|
||||
return _MockAsyncClient(payload=payload, post_calls=post_calls, **kwargs)
|
||||
|
||||
monkeypatch.setattr("httpx.AsyncClient", _client_factory)
|
||||
return post_calls
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("payload", "secret"),
|
||||
[
|
||||
(
|
||||
{"access_token": "oauth-secret-123\n", "token_type": "Bearer", "expires_in": 3600},
|
||||
"oauth-secret-123",
|
||||
),
|
||||
(
|
||||
{"access_token": "oauth-secret-456", "token_type": "Bearer\r", "expires_in": 3600},
|
||||
"oauth-secret-456",
|
||||
),
|
||||
(
|
||||
{"access_token": "oauth-secret-caf\u00e9", "token_type": "Bearer", "expires_in": 3600},
|
||||
"oauth-secret-caf\u00e9",
|
||||
),
|
||||
(
|
||||
{"access_token": "oauth-secret-789 ", "token_type": "Bearer", "expires_in": 3600},
|
||||
"oauth-secret-789",
|
||||
),
|
||||
],
|
||||
ids=["trailing-newline", "cr-in-token-type", "non-ascii", "trailing-space"],
|
||||
)
|
||||
def test_illegal_oauth_token_is_denied_without_leaking(monkeypatch, payload, secret):
|
||||
_token_endpoint_returns(monkeypatch, payload)
|
||||
manager = OAuthTokenManager.from_extensions_config(_oauth_server_config())
|
||||
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
asyncio.run(manager.get_authorization_header("secure-http"))
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert secret not in message
|
||||
assert "secure-http" in message
|
||||
|
||||
|
||||
def test_oauth_token_kept_legal_by_the_space_after_token_type_is_accepted(monkeypatch):
|
||||
# The rendered value is the boundary, not the two fields on their own: this
|
||||
# access_token carries leading whitespace, which the transport tolerates
|
||||
# once it follows "Bearer ". Denying it would refuse a token the server
|
||||
# would have accepted.
|
||||
_token_endpoint_returns(monkeypatch, {"access_token": " leading-space-token", "token_type": "Bearer", "expires_in": 3600})
|
||||
manager = OAuthTokenManager.from_extensions_config(_oauth_server_config())
|
||||
|
||||
assert asyncio.run(manager.get_authorization_header("secure-http")) == "Bearer leading-space-token"
|
||||
|
||||
|
||||
def test_oauth_interceptor_denies_illegal_token_without_calling_the_handler(monkeypatch):
|
||||
_token_endpoint_returns(monkeypatch, {"access_token": "oauth-secret-abc\n", "token_type": "Bearer", "expires_in": 3600})
|
||||
config = _oauth_server_config()
|
||||
interceptor = build_oauth_tool_interceptor(config)
|
||||
assert interceptor is not None
|
||||
|
||||
class _Request:
|
||||
server_name = "secure-http"
|
||||
headers: dict[str, str] = {}
|
||||
|
||||
def override(self, **kwargs): # pragma: no cover - denied before reached
|
||||
raise AssertionError("the request must never be forwarded with an illegal token")
|
||||
|
||||
handler_calls: list[Any] = []
|
||||
|
||||
async def _handler(request): # pragma: no cover - denied before reached
|
||||
handler_calls.append(request)
|
||||
return "ok"
|
||||
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
asyncio.run(interceptor(_Request(), _handler))
|
||||
|
||||
assert "oauth-secret-abc" not in str(excinfo.value)
|
||||
assert handler_calls == []
|
||||
|
||||
|
||||
def test_initial_oauth_headers_skips_server_with_illegal_token(monkeypatch, caplog):
|
||||
_token_endpoint_returns(monkeypatch, {"access_token": "oauth-secret-xyz\n", "token_type": "Bearer", "expires_in": 3600})
|
||||
config = _oauth_server_config()
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="deerflow.mcp.oauth"):
|
||||
headers = asyncio.run(get_initial_oauth_headers(config))
|
||||
|
||||
# No header at all rather than a broken one: the connection then fails
|
||||
# authentication at the server, which says nothing about the token.
|
||||
assert headers == {}
|
||||
assert "oauth-secret-xyz" not in caplog.text
|
||||
|
||||
@ -118,6 +118,34 @@ def test_default_user_fallback_is_denied_when_unmapped():
|
||||
asyncio.run(interceptor(_request(runtime=None), AsyncMock()))
|
||||
|
||||
|
||||
def test_credential_with_trailing_newline_is_denied_without_leaking_it():
|
||||
"""A credential that cannot travel as a header value (docker env-file with
|
||||
CRLF line endings, `$ENV_VAR` set from a token file) must be rejected here:
|
||||
h11 renders the full value into its LocalProtocolError message, which
|
||||
ToolErrorHandlingMiddleware then copies into a model-visible ToolMessage."""
|
||||
interceptor = build_user_scoped_auth_interceptor(_config(users={"u1": "Bearer sk-secret-value\n"}))
|
||||
handler = AsyncMock()
|
||||
with pytest.raises(ToolException) as excinfo:
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_for_user("u1")), handler))
|
||||
handler.assert_not_awaited()
|
||||
assert "sk-secret-value" not in str(excinfo.value)
|
||||
assert "u1" in str(excinfo.value)
|
||||
|
||||
|
||||
def test_credential_with_carriage_return_is_denied_even_with_passthrough():
|
||||
"""passthrough covers an *unmapped user*; a mapped-but-broken credential
|
||||
must not silently fall back to the shared discovery credential."""
|
||||
interceptor = build_user_scoped_auth_interceptor(_config(users={"u1": "Bearer t1\r"}, on_missing="passthrough"))
|
||||
with pytest.raises(ToolException):
|
||||
asyncio.run(interceptor(_request(runtime=_runtime_for_user("u1")), AsyncMock()))
|
||||
|
||||
|
||||
def test_credential_with_embedded_space_is_not_rejected():
|
||||
interceptor = build_user_scoped_auth_interceptor(_config(users={"u1": "Bearer t1"}))
|
||||
result = asyncio.run(interceptor(_request(runtime=_runtime_for_user("u1")), _echo_handler))
|
||||
assert result.headers["Authorization"] == "Bearer t1"
|
||||
|
||||
|
||||
def test_env_var_reference_resolution(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("TEST_USER_CRED", "Bearer from-env")
|
||||
config_file = tmp_path / "extensions_config.json"
|
||||
|
||||
@ -140,7 +140,9 @@ discovery.
|
||||
- **Fail-closed by default**: a user with no mapped credential — or a mapped
|
||||
`$ENV_VAR` that is unset — gets a clear error instead of another user's
|
||||
credential. Set `"on_missing": "passthrough"` to instead forward such calls
|
||||
with the server's static headers.
|
||||
with the server's static headers. A mapped credential that cannot travel as
|
||||
an HTTP header (for example one with a trailing newline) is always rejected
|
||||
with an error that never repeats the value.
|
||||
- Combined with per-`(user, thread)` MCP session scoping, users cannot reach
|
||||
each other's authenticated sessions or credentials.
|
||||
- The Gateway API masks `user_auth.users` values in GET responses and preserves
|
||||
@ -208,6 +210,12 @@ conversation:
|
||||
call fails with an error naming the missing key rather than falling back to
|
||||
the discovery credential — which would send one tenant's request under
|
||||
another tenant's authority. Set `"on_missing": "passthrough"` to opt out.
|
||||
- A value that cannot travel as an HTTP header — a stray newline picked up when
|
||||
reading a token from a file, leading/trailing whitespace, characters outside
|
||||
ASCII — is always rejected, regardless of `on_missing`. The error names the
|
||||
offending key but never repeats the value; without this check a newline or
|
||||
stray whitespace would reach the HTTP client, whose rejection echoes the full
|
||||
credential into a model-visible tool error.
|
||||
- For a server declaring several sources, precedence is static `headers` <
|
||||
`oauth` < `user_auth` < `headers_from_context`: the value chosen for this one
|
||||
request is the most specific, so it wins.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user