diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dbf8e52..9227c66e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **CLI**: `pipefy pipe start-form ` mirrors MCP `get_start_form_fields` (optional `--required-only`). - **CLI**: `pipefy card fill --phase --fields '{"…"}'` fills phase fields non-interactively; filters to editable phase field IDs before `update_card`. Optional `--required-only` limits the phase field lookup to required fields. JSON responses may include `skipped_field_ids` when `--fields` keys are dropped by the filter. - **SDK**: `pipefy_sdk.field_filters` exports `filter_editable_field_definitions`, `filter_fields_by_definitions`, and `skipped_field_ids` for shared MCP/CLI field filtering. +- **MCP**: the HTTP transport derives a configurable DNS-rebinding Host/Origin allowlist. FastMCP auto-enables a loopback-only allowlist on its `127.0.0.1` construction host, so behind a proxy that forwards the public Host the server answered `421 Misdirected Request` before any handler ran; setting `resource_server_url` now widens the allowlist to that public host (plus loopback) and keeps DNS-rebinding protection on. `PIPEFY_MCP_ALLOWED_HOSTS` and `PIPEFY_MCP_ALLOWED_ORIGINS` (JSON) extend it for extra hostnames or a stricter Origin posture; an empty `PIPEFY_MCP_ALLOWED_ORIGINS` is the strictest form, rejecting any request that sends an Origin. Unset (no resource-server URL and no override) leaves FastMCP's loopback-only default in force, so the installed local subprocess case is unchanged. Closes #303. ### Changed +- **MCP**: the HTTP bind-safety guard now keys on auth posture, not bind interface. The old `_assert_safe_http_bind` refused any non-loopback HTTP bind, which false-positived on the entire hosted deployment (a container binds `0.0.0.0` and is still private) and lived in the run path, so a caller building the ASGI app directly bypassed it. It is replaced by `McpSettings._enforce_bind_safety`, a cross-field validator at the settings boundary that every serving path inherits: the unauthenticated `local` profile still refuses a non-loopback HTTP bind unless `PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND` is set, while the `remote` profile (which always carries inbound auth) binds any host without a bind-host check. Loopback detection moves to `pipefy_infra.security.is_loopback_host`, covering all of `127.0.0.0/8` and `::1` rather than three hardcoded string literals. Closes #379. - **CLI / Plugin (Claude Code)**: the CLI install path now installs `pipefy-cli` from PyPI instead of a `git+…#subdirectory=packages/cli` reference with `--with` sibling pins. The `/pipefy:install` slash command runs `uv tool install --force pipefy-cli`, and the documented snippets (root `README.md`, `packages/cli/README.md`, `docs/MIGRATION.md`) use `uvx --from pipefy-cli pipefy …` / `uv tool install pipefy-cli`. PyPI resolves `pipefy` and `pipefy-auth` transitively, so the explicit `--with` flags are gone and installs no longer pay a git clone plus build. This matches the MCP server's PyPI install (#368); do not pass a global `--prerelease allow`, which would let transitive dependencies jump to their own pre-releases. `RELEASE.md` verification snippets move to PyPI accordingly, and the now-unused `latest` moving-tag release step is removed. Closes #234. ## [0.3.0-alpha.1] - 2026-07-04 diff --git a/docs/config.md b/docs/config.md index e5b20528..15d20348 100644 --- a/docs/config.md +++ b/docs/config.md @@ -43,6 +43,10 @@ remote_mode = false host = "127.0.0.1" port = 8000 log_level = "INFO" +# allowed_hosts / allowed_origins are optional; unset derives the allowlist from +# the resource-server URL. Set them only for extra proxied hostnames. +allowed_hosts = ["mcp.pipefy.com"] +allowed_origins = ["https://mcp.pipefy.com"] ``` Keys use **bare pydantic field names**, not the upper-case `PIPEFY_` environment variable names. The env-only aliases (`PIPEFY_TOKEN`, `PIPEFY_OAUTH_CLIENT`, ...) exist to refuse unprefixed environment leakage and do not double as TOML keys. @@ -128,5 +132,7 @@ These variables load into `pipefy_mcp.McpSettings` (`settings.mcp`). TOML keys u | `PIPEFY_MCP_HOST` | `127.0.0.1` | HTTP bind host. The unauthenticated `local` profile refuses a non-loopback bind unless `PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND` is set; the authenticated `remote` profile binds any host. | | `PIPEFY_MCP_PORT` | `8000` | HTTP bind port. | | `PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND` | `false` | Escape hatch: lets the unauthenticated `local` profile serve HTTP on a non-loopback host, exposing the full tool surface with no inbound bearer. The `remote` profile never needs it. | +| `PIPEFY_MCP_ALLOWED_HOSTS` | unset | JSON array of extra `Host` header values the HTTP transport accepts for DNS-rebinding protection, on top of loopback and the `PIPEFY_MCP_RS_RESOURCE_SERVER_URL` host. Unset derives the allowlist from the resource-server URL, so a proxied deployment usually needs none; set it only when a proxy forwards a public `Host` that differs. An entry matches an exact `Host` or, as `host:*`, any port. These are extra entries, so an empty array behaves like unset (the resource-server host is still derived). | +| `PIPEFY_MCP_ALLOWED_ORIGINS` | unset | JSON array overriding the derived `scheme://host` `Origin` allowlist. Unset derives `http`/`https` origins from the allowed hosts; a non-empty array replaces them with a custom set (e.g. `https`-only); an empty array is the strictest override, rejecting any request that sends an `Origin` header (a request with no `Origin` still passes). | | `PIPEFY_MCP_UNIFIED_ENVELOPE` | `true` | When true, migrated tools return `{success, data, ...}`. | | `PIPEFY_MCP_LOG_LEVEL` | `INFO` | Governs **both** hosted structured JSON lines on **stdout** and the FastMCP root logger (RichHandler text on **stderr**). Accepts `DEBUG`, `INFO`, `WARNING`, `ERROR`, or `CRITICAL` (case-insensitive). The server passes the resolved level to `FastMCP(log_level=...)` at construction, so this is the single operator knob for MCP logging. Some MCP tooling documents a separate `FASTMCP_LOG_LEVEL` env var; when using `pipefy-mcp-server`, prefer `PIPEFY_MCP_LOG_LEVEL` — it configures the root logger explicitly and keeps structured events on the same level. | diff --git a/packages/mcp/AGENTS.md b/packages/mcp/AGENTS.md index d8a35555..384131ed 100644 --- a/packages/mcp/AGENTS.md +++ b/packages/mcp/AGENTS.md @@ -69,9 +69,10 @@ The JWKS/RS256 validation lives in `pipefy_auth` (`JwtValidator`); the MCP adapt `auth/resource_server.py` (`JwtTokenVerifier`) maps validated claims onto the SDK's `AccessToken`. FastMCP serves the RFC 9728 protected-resource metadata and the `401` + `WWW-Authenticate` challenge; `build_resource_server_auth` (same -module) resolves the inbound issuer and pairs the verifier with `AuthSettings`. -The runtime (`McpRuntime.for_profile`) calls it for the `remote` profile and holds -the pair as `inbound_auth`, which `server.py` wires into the app. +module) pairs the verifier with `AuthSettings` from an already-resolved issuer. +The runtime (`McpRuntime.for_profile`) resolves the inbound issuer, gates on it, +and calls the builder for the `remote` profile, holding the pair as `inbound_auth`, +which `server.py` wires into the app. **Bind-safety interlock.** The property protected is auth posture, not bind interface: an unauthenticated profile must not be reachable by untrusted callers. @@ -85,11 +86,23 @@ validates a per-request bearer, so its bind host is irrelevant and is not checke `pipefy_infra.security.is_loopback_host`, which covers all of `127.0.0.0/8` and `::1`. This replaced an earlier bind-interface guard (`_assert_safe_http_bind`) that false-positived on the entire hosted profile and lived in the run path where the -ASGI-app path bypassed it. DNS-rebinding protection (the configurable host / Origin -allowlist for a proxied deployment) is separate follow-up work; see -`experiments/hosted-obo/RFC-OUTLINE.md`. The attachment tools' local `file_path` -inputs also still assume a loopback peer that shares the client's disk (remote-safe -file inputs are separate follow-up work). +ASGI-app path bypassed it. The attachment tools' local `file_path` inputs also +still assume a loopback peer that shares the client's disk (remote-safe file inputs +are separate follow-up work). + +**Transport allowlist.** DNS-rebinding protection is a separate axis from the +bind-safety interlock: it checks the inbound request's `Host` / `Origin`, not the +bind interface. FastMCP auto-enables a loopback-only allowlist on the `127.0.0.1` +construction host, so behind a proxy that forwards the public `Host` it answers +`421 Misdirected Request`. `core/transport_security.py:build_transport_security` +widens it by deriving the allowed host from `resource_server_url` (the public origin +the `remote` profile already declares) plus loopback, and `build_pipefy_mcp_server` +passes the result to FastMCP. `PIPEFY_MCP_ALLOWED_HOSTS` / `PIPEFY_MCP_ALLOWED_ORIGINS` +(JSON) extend it for extra hostnames or a stricter Origin posture. Unset (no +resource-server URL and no override) leaves FastMCP's loopback-only default in force, +so the local subprocess case is unaffected. Being configuration derived at +composition (mirroring `build_resource_server_auth`), it lives in the composition +tier, not in `settings.py`, which keeps the mcp SDK out of the config boundary. ## Tool registration diff --git a/packages/mcp/src/pipefy_mcp/auth/__init__.py b/packages/mcp/src/pipefy_mcp/auth/__init__.py index e440506e..7e75491e 100644 --- a/packages/mcp/src/pipefy_mcp/auth/__init__.py +++ b/packages/mcp/src/pipefy_mcp/auth/__init__.py @@ -1,15 +1,25 @@ -"""Resource-server auth for the HTTP transport: inbound bearer validation.""" +"""Identity for the HTTP transport: inbound bearer validation and session identity.""" from pipefy_mcp.auth.request_identity import require_request_bearer from pipefy_mcp.auth.resource_server import ( JwtTokenVerifier, + ResourceServer, ResourceServerAuth, build_resource_server_auth, ) +from pipefy_mcp.auth.session_identity import ( + AuthSource, + RequestScopedIdentity, + StartupIdentity, +) __all__ = [ + "AuthSource", "JwtTokenVerifier", + "RequestScopedIdentity", + "ResourceServer", "ResourceServerAuth", + "StartupIdentity", "build_resource_server_auth", "require_request_bearer", ] diff --git a/packages/mcp/src/pipefy_mcp/auth/request_identity.py b/packages/mcp/src/pipefy_mcp/auth/request_identity.py index 5570ac58..a8cc1c7e 100644 --- a/packages/mcp/src/pipefy_mcp/auth/request_identity.py +++ b/packages/mcp/src/pipefy_mcp/auth/request_identity.py @@ -5,7 +5,7 @@ that caller rather than as one identity resolved at startup. This module reads the caller's validated bearer off the request the tool handler received, so the runtime can snapshot it into a per-session credential (see -:meth:`pipefy_mcp.core.runtime.RequestScopedIdentity.resolve`). +:meth:`pipefy_mcp.auth.session_identity.RequestScopedIdentity.resolve`). The bearer comes from the request the handler passes in (``ctx.request_context.request``), not from ``AuthContextMiddleware``'s diff --git a/packages/mcp/src/pipefy_mcp/auth/resource_server.py b/packages/mcp/src/pipefy_mcp/auth/resource_server.py index 14c307ca..99356706 100644 --- a/packages/mcp/src/pipefy_mcp/auth/resource_server.py +++ b/packages/mcp/src/pipefy_mcp/auth/resource_server.py @@ -13,16 +13,20 @@ the SDK's ``AccessToken`` and turns a validation failure into the ``None`` the protocol reads as "reject". -:func:`build_resource_server_auth` is the composition root for this profile: it -resolves the inbound issuer, constructs the verifier over a ``JwtValidator``, -and pairs it with FastMCP's ``AuthSettings`` for the server to wire in. +:func:`build_resource_server_auth` constructs the verifier over a ``JwtValidator`` +and pairs it with FastMCP's ``AuthSettings`` for the server to wire in. It takes an +already-resolved issuer: the composition root +(:meth:`pipefy_mcp.core.runtime.McpRuntime.for_profile`) resolves the inbound issuer +and gates on it, so this builder only wires the resolved values in. """ from __future__ import annotations import asyncio import logging +from dataclasses import dataclass from typing import Any +from urllib.parse import urlparse from mcp.server.auth.provider import AccessToken, TokenVerifier from mcp.server.auth.settings import AuthSettings as FastMcpAuthSettings @@ -34,11 +38,42 @@ SkipAudience, ) -from pipefy_mcp.settings import ResourceServerSettings - logger = logging.getLogger(__name__) +@dataclass(frozen=True) +class ResourceServer: + """A validated resource-server URL paired with its parsed Host authorities. + + Built by :meth:`from_url` at composition, so holding one is proof its + ``host_authorities`` are resolved before any read. ``url`` is the verbatim RFC 9728 + resource identifier (kept exact, as clients compare against it); ``host_authorities`` + are the ``host[:port]`` values it presents in the ``Host`` header, which the + transport allowlist widens. + """ + + url: str + host_authorities: tuple[str, ...] + + @classmethod + def from_url(cls, url: str) -> ResourceServer: + """Parse an already-validated URL into its ``Host`` header authorities. + + An IPv6 literal is bracketed, as the wire Host carries it (``urlparse`` + reports it unbracketed); a URL that names a port also contributes the + ``host:port`` authority. + """ + parsed = urlparse(url) + hostname = parsed.hostname + authorities: list[str] = [] + if hostname: + host = f"[{hostname}]" if ":" in hostname else hostname + authorities.append(host) + if parsed.port: + authorities.append(f"{host}:{parsed.port}") + return cls(url=url, host_authorities=tuple(authorities)) + + class PipefyAccessToken(AccessToken): """AccessToken with the JWT ``sub`` claim preserved for request logging.""" @@ -134,34 +169,20 @@ def _to_access_token(self, token: str, claims: dict[str, Any]) -> AccessToken: def build_resource_server_auth( - rs: ResourceServerSettings, + resource: ResourceServer, jwt_validation: JwtValidationSettings, *, - default_issuer_url: str | None, -) -> ResourceServerAuth | None: - """Build the inbound bearer verifier and FastMCP auth config, or ``None``. - - The resource-server profile has no enable flag: it is active when this - server's ``resource_server_url`` is configured. Absent it, this returns - ``None`` and the unauthenticated foundation profile constructs ``FastMCP`` - exactly as before. - - The inbound issuer is ``jwt_validation.issuer_url`` if set, else - ``default_issuer_url`` (see :class:`JwtValidationSettings` for why the login - issuer is the fallback). With ``resource_server_url`` set but no issuer - resolvable (the stored-session login is disabled and no override is given), - validation is impossible, so this raises rather than serve an open endpoint. + issuer_url: str, + required_scopes: list[str] | None = None, +) -> ResourceServerAuth: + """Build the inbound bearer verifier and FastMCP auth config for ``resource``. + + Called only when the resource-server profile is active: the composition root + parses the configured ``resource_server_url`` into ``resource`` and resolves the + inbound ``issuer_url`` (gating on both, so an unresolvable issuer fails fast at + the root). This receives an already-parsed identity and a resolved issuer and just + wires them onto the validator and FastMCP's ``AuthSettings``. """ - if rs.resource_server_url is None: - return None - issuer_url = jwt_validation.resolve_issuer_url(default_issuer_url) - if issuer_url is None: - raise RuntimeError( - "The resource-server profile is active " - "(PIPEFY_MCP_RS_RESOURCE_SERVER_URL is set) but no inbound issuer is " - "resolvable: set PIPEFY_JWT_ISSUER_URL, or leave the stored-session " - "login enabled so its issuer can be reused." - ) # Fold the loose audience pair into the AudiencePolicy sum type. A # verify-without-audience is already rejected at JwtValidationSettings # construction, so `is not None` only narrows the Optional for the type @@ -177,11 +198,11 @@ def build_resource_server_auth( allow_insecure_urls=jwt_validation.allow_insecure_urls, jwks_uri=jwt_validation.jwks_uri, ), - resource=rs.resource_server_url, + resource=resource.url, ) auth = FastMcpAuthSettings( issuer_url=issuer_url, - resource_server_url=rs.resource_server_url, - required_scopes=rs.required_scopes, + resource_server_url=resource.url, + required_scopes=required_scopes, ) return verifier, auth diff --git a/packages/mcp/src/pipefy_mcp/auth/session_identity.py b/packages/mcp/src/pipefy_mcp/auth/session_identity.py new file mode 100644 index 00000000..21504d08 --- /dev/null +++ b/packages/mcp/src/pipefy_mcp/auth/session_identity.py @@ -0,0 +1,98 @@ +"""Outbound identity for a request's SDK session: who each session acts as. + +The counterpart to :mod:`pipefy_mcp.auth.request_identity` (which extracts the +*inbound* bearer a caller presents): these types resolve the *outbound* +``httpx.Auth`` the per-request SDK session binds. The two profiles pick a +different variant at the composition root, and both speak one ``resolve`` contract +so the runtime opens every session uniformly. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from httpx import Auth +from pipefy_auth import ( + StaticBearerAuth, + build_httpx_auth, + configure_keychain_backend, + missing_auth_message, + resolve_pipefy_auth, +) +from starlette.requests import Request + +from pipefy_mcp._docs import DOCS_SETUP_REF +from pipefy_mcp.auth.request_identity import require_request_bearer +from pipefy_mcp.settings import Settings + + +@dataclass(frozen=True) +class StartupIdentity: + """One credential resolved from settings at startup; every request runs as it. + + The stdio/local profile: with no inbound bearer, the composition root resolves + the highest-precedence configured credential once (via + :meth:`from_configured_credential`); :meth:`resolve` returns that same + credential for every session. + """ + + auth: Auth + + @classmethod + def from_configured_credential(cls, settings: Settings) -> StartupIdentity: + """Resolve the one startup credential from settings, or fail fast. + + Swaps the keyring backend (no-op when ``auto``), resolves the + highest-precedence configured credential (the keychain read behind + :func:`resolve_pipefy_auth`), and raises when none is configured so a + missing credential surfaces at startup rather than on the first tool call. + + The resolved auth refreshes lazily (a stored session wires + :class:`pipefy_auth.RefreshableBearerAuth`): the token is fetched and + refreshed on the first request that needs it, not eagerly here. + """ + configure_keychain_backend(settings.auth.keychain_backend) + resolved = resolve_pipefy_auth( + static_token=settings.auth.static_token, + service_account=settings.auth.to_service_account(), + oidc_client=settings.auth.to_oidc_client(), + ) + if resolved is None: + raise RuntimeError( + f"{missing_auth_message()} " + f"See {DOCS_SETUP_REF} for host-specific install steps." + ) + return cls(build_httpx_auth(resolved)) + + def resolve(self, request: Request | None) -> Auth: + # The startup credential is request-independent; the request the runtime + # threads through for the hosted profile has nothing to resolve here. + return self.auth + + +@dataclass(frozen=True) +class RequestScopedIdentity: + """The calling user's identity, resolved per request (hosted profile). + + :meth:`resolve` snapshots the validated bearer off the ``request`` the tool + handler passes in into a static credential for that one session, so concurrent + callers never share identity. Reading the request the handler received (rather + than ``auth_context_var``, which stateful Streamable HTTP freezes at the + session's first bearer) is what keeps the snapshot on the current caller. A + future auth transform (OBO exchange, a distinct downstream audience) is a change + to what this method returns, nothing else. + """ + + def resolve(self, request: Request | None) -> Auth: + return StaticBearerAuth(require_request_bearer(request)) + + +# The identity source for a request's session, chosen by profile at the +# composition root (:meth:`pipefy_mcp.core.runtime.McpRuntime.for_profile`): each +# variant's :meth:`resolve` takes the in-flight request and returns the +# ``httpx.Auth`` the per-request session binds. Both arms speak that one contract, +# so the runtime opens every session uniformly with no per-variant branching. +AuthSource = StartupIdentity | RequestScopedIdentity + + +__all__ = ["AuthSource", "RequestScopedIdentity", "StartupIdentity"] diff --git a/packages/mcp/src/pipefy_mcp/core/runtime.py b/packages/mcp/src/pipefy_mcp/core/runtime.py index bdb9d193..e90382e9 100644 --- a/packages/mcp/src/pipefy_mcp/core/runtime.py +++ b/packages/mcp/src/pipefy_mcp/core/runtime.py @@ -1,96 +1,21 @@ from __future__ import annotations -from dataclasses import dataclass - -from httpx import Auth -from pipefy_auth import ( - StaticBearerAuth, - build_httpx_auth, - configure_keychain_backend, - missing_auth_message, - resolve_pipefy_auth, -) +from mcp.server.transport_security import TransportSecuritySettings from pipefy_sdk import PipefyClient, PipefyEngine from starlette.requests import Request -from pipefy_mcp._docs import DOCS_SETUP_REF -from pipefy_mcp.auth.request_identity import require_request_bearer -from pipefy_mcp.auth.resource_server import ( +from pipefy_mcp.auth import ( + AuthSource, + RequestScopedIdentity, + ResourceServer, ResourceServerAuth, + StartupIdentity, build_resource_server_auth, ) +from pipefy_mcp.core.transport_security import build_transport_security from pipefy_mcp.settings import Settings -@dataclass(frozen=True) -class StartupIdentity: - """One credential resolved from settings at startup; every request runs as it. - - The stdio/local profile: with no inbound bearer, the composition root resolves - the highest-precedence configured credential once (via - :meth:`from_configured_credential`); :meth:`resolve` returns that same - credential for every session. - """ - - auth: Auth - - @classmethod - def from_configured_credential(cls, settings: Settings) -> StartupIdentity: - """Resolve the one startup credential from settings, or fail fast. - - Swaps the keyring backend (no-op when ``auto``), resolves the - highest-precedence configured credential (the keychain read behind - :func:`resolve_pipefy_auth`), and raises when none is configured so a - missing credential surfaces at startup rather than on the first tool call. - - The resolved auth refreshes lazily (a stored session wires - :class:`pipefy_auth.RefreshableBearerAuth`): the token is fetched and - refreshed on the first request that needs it, not eagerly here. - """ - configure_keychain_backend(settings.auth.keychain_backend) - resolved = resolve_pipefy_auth( - static_token=settings.auth.static_token, - service_account=settings.auth.to_service_account(), - oidc_client=settings.auth.to_oidc_client(), - ) - if resolved is None: - raise RuntimeError( - f"{missing_auth_message()} " - f"See {DOCS_SETUP_REF} for host-specific install steps." - ) - return cls(build_httpx_auth(resolved)) - - def resolve(self, request: Request | None) -> Auth: - # The startup credential is request-independent; the request the runtime - # threads through for the hosted profile has nothing to resolve here. - return self.auth - - -@dataclass(frozen=True) -class RequestScopedIdentity: - """The calling user's identity, resolved per request (hosted profile). - - :meth:`resolve` snapshots the validated bearer off the ``request`` the tool - handler passes in into a static credential for that one session, so concurrent - callers never share identity. Reading the request the handler received (rather - than ``auth_context_var``, which stateful Streamable HTTP freezes at the - session's first bearer) is what keeps the snapshot on the current caller. A - future auth transform (OBO exchange, a distinct downstream audience) is a change - to what this method returns, nothing else. - """ - - def resolve(self, request: Request | None) -> Auth: - return StaticBearerAuth(require_request_bearer(request)) - - -# The identity source for a request's session, chosen by profile at the -# composition root (:meth:`McpRuntime.for_profile`): each variant's :meth:`resolve` -# takes the in-flight request and returns the ``httpx.Auth`` the per-request session -# binds. Both arms speak that one contract, so the runtime opens every session -# uniformly with no per-variant branching. -AuthSource = StartupIdentity | RequestScopedIdentity - - def _login_issuer_url(settings: Settings) -> str | None: """The issuer this process logs into, the fallback for the inbound issuer. @@ -107,11 +32,14 @@ class McpRuntime: Built once at server startup via :meth:`for_profile`, which turns the resolved settings into wired resources: the outbound identity (whose :meth:`resolve` - backs each request's session) and, under the ``remote`` profile, the inbound - resource-server ``(verifier, auth)`` pair FastMCP uses to validate each caller's - bearer. It owns the process-scoped :class:`PipefyEngine` (the shared endpoints - and GraphQL schema cache, built auth-agnostic with no network I/O) and opens a - cheap per-request session bound to the caller's identity. + backs each request's session), the HTTP transport's DNS-rebinding allowlist, and, + under the ``remote`` profile, the inbound resource-server ``(verifier, auth)`` pair + FastMCP uses to validate each caller's bearer. It parses the ``resource_server_url`` + into one :class:`ResourceServer` and feeds it to both the inbound-auth and the + allowlist builders, so they cannot disagree on the resource host. It owns the + process-scoped :class:`PipefyEngine` (the shared endpoints and GraphQL schema cache, + built auth-agnostic with no network I/O) and opens a cheap per-request session bound + to the caller's identity. Building the engine here is safe off the event loop: it does no network I/O and binds nothing to a running loop (its endpoints open a fresh per-request @@ -119,7 +47,7 @@ class McpRuntime: later handles requests. This is a stepping stone toward a single per-app runtime; today it owns the - shared engine and the inbound-auth pair. + shared engine, the inbound-auth pair, and the transport allowlist. """ def __init__( @@ -128,44 +56,84 @@ def __init__( identity: AuthSource, *, inbound_auth: ResourceServerAuth | None = None, + transport_security: TransportSecuritySettings | None = None, ) -> None: self._settings = settings self._identity = identity self.inbound_auth = inbound_auth + self.transport_security = transport_security self._engine = PipefyEngine.build(settings.pipefy, surface="mcp") @classmethod def for_profile(cls, settings: Settings) -> McpRuntime: """Build the runtime for the resolved profile, wiring inbound and outbound auth. - The composition root's one build step: ``settings.mcp.profile`` selects the - outbound identity and, for ``remote``, the inbound resource-server pair. + The composition root's one build step: it parses the ``resource_server_url`` + once, builds the transport allowlist from that one parsed resource, and + selects the per-profile identity (and, for ``remote``, the inbound-auth pair). + Parsing here keeps the resource a single value both the allowlist and the + inbound-auth pair are derived from, so they cannot disagree on the host; the + one ``cls(...)`` call then wires the fields common to both profiles. + """ + url = settings.rs.resource_server_url + resource = ResourceServer.from_url(url) if url else None + transport_security = build_transport_security(settings.mcp, resource) + if settings.mcp.profile == "remote": + identity, inbound_auth = cls._remote_identity(settings, resource) + else: + identity, inbound_auth = cls._local_identity(settings), None + return cls( + settings, + identity, + inbound_auth=inbound_auth, + transport_security=transport_security, + ) + + @staticmethod + def _remote_identity( + settings: Settings, resource: ResourceServer | None + ) -> tuple[AuthSource, ResourceServerAuth]: + """The ``remote`` profile's identity: a per-request bearer and an inbound RS pair. ``remote`` acts on behalf of each caller: it validates a per-request bearer - (the inbound ``(verifier, auth)`` pair) and each session replays that - caller's snapshotted bearer (:class:`RequestScopedIdentity`). A configured - resource server is mandatory there, so this fails fast when none resolves, - rather than serve an open endpoint or silently fall back to a startup - credential. - - Every other profile (stdio, or ``local`` over loopback HTTP) has no inbound - identity: the one startup credential is resolved from settings (and fails - fast when none is configured) and every session acts as it. + (the inbound ``(verifier, auth)`` pair) and each session replays that caller's + snapshotted bearer (:class:`RequestScopedIdentity`). A resource server and a + resolvable inbound issuer are both mandatory here, so this gates on each and + fails fast rather than serve an open endpoint or silently fall back to a + startup credential. """ - if settings.mcp.profile == "remote": - inbound_auth = build_resource_server_auth( - settings.rs, - settings.jwt, - default_issuer_url=_login_issuer_url(settings), + if resource is None: + raise RuntimeError( + "the 'remote' profile requires a resource server: set " + "PIPEFY_MCP_RS_RESOURCE_SERVER_URL so the server validates a " + "per-request bearer and acts on behalf of the caller." + ) + # The inbound issuer is the explicit PIPEFY_JWT_ISSUER_URL override, else the + # login issuer this process authenticates against; with neither, the bearers + # cannot be validated, so refuse to build rather than serve an open endpoint. + issuer_url = settings.jwt.resolve_issuer_url(_login_issuer_url(settings)) + if issuer_url is None: + raise RuntimeError( + "the 'remote' profile requires an inbound issuer: set " + "PIPEFY_JWT_ISSUER_URL, or leave the stored-session login enabled so " + "its issuer can be reused." ) - if inbound_auth is None: - raise RuntimeError( - "the 'remote' profile requires a resource server: set " - "PIPEFY_MCP_RS_RESOURCE_SERVER_URL so the server validates a " - "per-request bearer and acts on behalf of the caller." - ) - return cls(settings, RequestScopedIdentity(), inbound_auth=inbound_auth) - return cls(settings, StartupIdentity.from_configured_credential(settings)) + inbound_auth = build_resource_server_auth( + resource, + settings.jwt, + issuer_url=issuer_url, + required_scopes=settings.rs.required_scopes, + ) + return RequestScopedIdentity(), inbound_auth + + @staticmethod + def _local_identity(settings: Settings) -> AuthSource: + """The ``local`` profile's identity (stdio, or loopback HTTP): one startup credential. + + No inbound identity: the one startup credential is resolved from settings (and + fails fast when none is configured) and every session acts as it. + """ + return StartupIdentity.from_configured_credential(settings) def session_for_request(self, request: Request | None) -> PipefyClient: """Open a session bound to the current request's identity. diff --git a/packages/mcp/src/pipefy_mcp/core/transport_security.py b/packages/mcp/src/pipefy_mcp/core/transport_security.py new file mode 100644 index 00000000..eb29b76d --- /dev/null +++ b/packages/mcp/src/pipefy_mcp/core/transport_security.py @@ -0,0 +1,91 @@ +"""Derive the HTTP transport's DNS-rebinding allowlist. + +FastMCP auto-enables a loopback-only Host/Origin allowlist whenever it is built +with a loopback ``host`` and no explicit ``transport_security``. The builder always +passes ``host=settings.mcp.host`` (default ``127.0.0.1``), so behind a proxy that +forwards the public Host the transport answers ``421 Misdirected Request`` before +any handler runs. This module turns the resolved settings into the +``TransportSecuritySettings`` that widens the allowlist to the public host while +keeping DNS-rebinding protection on. + +This is configuration resolution, not an MCP extension: it produces a static +config value the SDK reads, mirroring +:func:`pipefy_mcp.auth.resource_server.build_resource_server_auth` (settings to the +SDK's ``AuthSettings``). The mcp-SDK type is kept out of ``settings.py`` so the +config boundary stays framework-free. + +The public host comes from the :class:`pipefy_mcp.auth.ResourceServer` the runtime +parses once and feeds in (which the remote profile already requires), so the standard +fronted deployment needs no allowlist config; ``allowed_hosts`` / ``allowed_origins`` +extend it. When neither a resource nor an explicit allowlist is given the function +returns ``None``, leaving FastMCP's own loopback default in force. +""" + +from __future__ import annotations + +from mcp.server.transport_security import TransportSecuritySettings + +from pipefy_mcp.auth import ResourceServer +from pipefy_mcp.settings import McpSettings + +# Loopback entries kept in every explicit allowlist so widening for a proxy does +# not lock out local tooling on the box (a proxy on the same host still reaches the +# server over loopback). Mirrors FastMCP's own loopback auto-enable set. +_LOOPBACK_HOSTS = ("127.0.0.1", "localhost", "[::1]") + + +def build_transport_security( + mcp: McpSettings, resource: ResourceServer | None +) -> TransportSecuritySettings | None: + """Resolve the transport allowlist, or ``None`` to keep FastMCP's default. + + Returns ``None`` only when there is nothing to configure: no ``resource``, no + ``allowed_hosts``, and an unset (``None``) ``allowed_origins``, so FastMCP applies + its own loopback auto-enable and current behavior is preserved. Otherwise it + enables DNS-rebinding protection over loopback plus the resource's public host and + any explicit entries. An explicit ``allowed_origins`` (including an empty list, + which rejects any request that sends an Origin) is honored verbatim; only an unset + list falls back to the origins derived from the allowed hosts. + """ + public_hosts = list(resource.host_authorities) if resource else [] + public_hosts += mcp.allowed_hosts or [] + explicit_origins = mcp.allowed_origins + + # An explicit origin allowlist (including an empty one) is an override worth + # honoring, so it keeps protection on even with no host to add: ``allowed_origins=[]`` + # is the strictest posture, rejecting any request that sends an Origin. Only an + # unset origin list plus no host leaves nothing to configure. + if not public_hosts and explicit_origins is None: + return None + + # Each host contributes an exact form and a ``host:*`` any-port form: the SDK + # matches a Host header by exact string or trailing ``:*`` wildcard, and + # ``localhost:*`` does not match a portless ``localhost``, so both cover a Host + # sent with or without a port. ``dict.fromkeys`` dedupes while keeping order. + allowed_hosts = list( + dict.fromkeys( + form + for host in (*_LOOPBACK_HOSTS, *public_hosts) + for form in (host, f"{host}:*") + ) + ) + + if explicit_origins is not None: + allowed_origins = list(explicit_origins) + else: + # allowed_hosts is already deduped and (host, scheme) is injective, so no + # duplicate origin can arise; derive directly with no dedup pass. + allowed_origins = [ + f"{scheme}://{host}" + for host in allowed_hosts + for scheme in ("http", "https") + ] + + return TransportSecuritySettings( + enable_dns_rebinding_protection=True, + allowed_hosts=allowed_hosts, + allowed_origins=allowed_origins, + ) + + +__all__ = ["build_transport_security"] diff --git a/packages/mcp/src/pipefy_mcp/server.py b/packages/mcp/src/pipefy_mcp/server.py index 8f07fb50..a54442b7 100644 --- a/packages/mcp/src/pipefy_mcp/server.py +++ b/packages/mcp/src/pipefy_mcp/server.py @@ -100,6 +100,11 @@ def build_pipefy_mcp_server( remote-safe tool surface, and ``settings.mcp.host`` / ``settings.mcp.port`` give the HTTP bind (they matter only for the HTTP transport; stdio ignores them). + The DNS-rebinding allowlist for the HTTP transport is built by the runtime (from + the ``resource_server_url`` host plus any ``allowed_hosts`` / ``allowed_origins``) + and read off it here as ``runtime.transport_security``; it is ``None`` (FastMCP's + own loopback default) when nothing is configured, and irrelevant for stdio. + ``extra_tool_middlewares`` is the public registration seam for a consumer of this builder (a hosted serving layer that wants per-tool metrics, say): the chain installs once, so a consumer folds its middleware in here rather than reaching @@ -126,6 +131,7 @@ def build_pipefy_mcp_server( log_level=settings.mcp.log_level, token_verifier=verifier, auth=auth, + transport_security=runtime.transport_security, ) _register_pipefy_tools(app, remote_mode=settings.mcp.profile == "remote") # Wrap the tool-call handler with the built-in chain plus any consumer middleware. diff --git a/packages/mcp/src/pipefy_mcp/settings.py b/packages/mcp/src/pipefy_mcp/settings.py index 8d06af48..f8033a6b 100644 --- a/packages/mcp/src/pipefy_mcp/settings.py +++ b/packages/mcp/src/pipefy_mcp/settings.py @@ -10,6 +10,7 @@ AliasChoices, Field, ValidationError, + ValidationInfo, field_validator, model_validator, ) @@ -133,6 +134,30 @@ def settings_customise_sources( ), ) + allowed_hosts: list[str] | None = Field( + default=None, + description=( + "Extra Host header values the HTTP transport accepts, on top of " + "loopback and the resource_server_url host (env: " + "PIPEFY_MCP_ALLOWED_HOSTS as a JSON array). Needed only when a proxy " + "forwards a public Host that differs from the resource-server URL; the " + "standard fronted deployment derives its allowlist from " + "resource_server_url and sets none. An entry matches an exact Host or, " + "as 'host:*', any port on that host." + ), + ) + + allowed_origins: list[str] | None = Field( + default=None, + description=( + "Origin header values the HTTP transport accepts (env: " + "PIPEFY_MCP_ALLOWED_ORIGINS as a JSON array). Unset derives the " + "scheme://host origins from the allowed hosts; a non-empty array replaces " + "them with a custom set, and an empty array is the strictest override, " + "rejecting any request that sends an Origin header." + ), + ) + @field_validator("log_level", mode="before") @classmethod def _normalize_log_level(cls, value: object) -> object: @@ -195,6 +220,36 @@ def _enforce_bind_safety(self) -> Self: ) return self + @field_validator("allowed_hosts", "allowed_origins", mode="after") + @classmethod + def _normalize_allowlist( + cls, values: list[str] | None, info: ValidationInfo + ) -> list[str] | None: + """Trim entries and reject a blank one; ``None`` stays ``None``. + + Each entry is a Host or Origin the operator vouches for, so surrounding + whitespace is trimmed (a stray space would break the transport's exact + match), but a blank entry (an empty string, or an unfilled template value) + raises rather than being silently dropped: a dropped entry hides the typo, + and for ``allowed_origins`` it could collapse the list to the strict + reject-all-Origin posture. An explicitly empty list is a deliberate value + and is kept. This does not run the internal-host SSRF gate (``localhost`` + is a wanted loopback entry). + """ + if values is None: + return None + cleaned: list[str] = [] + for value in values: + entry = value.strip() + if not entry: + raise ValueError( + f"{info.field_name} contains a blank entry; remove it or give " + "a real Host/Origin value (an empty list is accepted, a blank " + "entry is not)." + ) + cleaned.append(entry) + return cleaned + class ResourceServerSettings(BaseSettings): """This MCP server's identity as an OAuth protected resource (HTTP profile). diff --git a/packages/mcp/tests/auth/test_resource_server.py b/packages/mcp/tests/auth/test_resource_server.py index 23ba1e23..78d416ae 100644 --- a/packages/mcp/tests/auth/test_resource_server.py +++ b/packages/mcp/tests/auth/test_resource_server.py @@ -4,8 +4,10 @@ pin :class:`JwtTokenVerifier`'s two jobs: mapping validated claims onto the SDK ``AccessToken`` and turning a validation failure into ``None`` (which FastMCP renders as a 401). The JWT/JWKS validation itself is covered in ``pipefy_auth``'s -``test_verification.py``. The builder tests pin -:func:`build_resource_server_auth`'s issuer resolution and active/inactive gating. +``test_verification.py``. The builder tests pin that +:func:`build_resource_server_auth` wires an already-resolved issuer onto the verifier +and ``AuthSettings``; the issuer resolution and active/inactive gating it used to own +now live at the composition root and are covered in ``core/test_runtime.py``. """ from __future__ import annotations @@ -15,8 +17,11 @@ import pytest from pipefy_auth import JwtValidationSettings, TokenValidationError -from pipefy_mcp.auth import JwtTokenVerifier, build_resource_server_auth -from pipefy_mcp.settings import ResourceServerSettings +from pipefy_mcp.auth import ( + JwtTokenVerifier, + ResourceServer, + build_resource_server_auth, +) _RESOURCE = "https://mcp.example.com/mcp" _ISSUER = "https://idp.example.com/realms/x" @@ -187,25 +192,36 @@ def test_build_stamps_resource_server_url_not_audience() -> None: token's stamped resource must match it, not the (often unset) audience. """ verifier, _ = build_resource_server_auth( - ResourceServerSettings(resource_server_url=_RESOURCE), + ResourceServer.from_url(_RESOURCE), # audience set and distinct from resource_server_url: the stamped resource # must follow resource_server_url, not audience. JwtValidationSettings( audience="urn:some-other-audience", jwks_uri="https://idp.example.com/jwks", ), - default_issuer_url=_ISSUER, + issuer_url=_ISSUER, ) assert verifier._resource == _RESOURCE +@pytest.mark.unit +def test_build_advertises_the_parsed_resource_url_in_the_metadata() -> None: + """AuthSettings' resource_server_url comes from the parsed ResourceServer carrier.""" + _, auth = build_resource_server_auth( + ResourceServer.from_url(_RESOURCE), + JwtValidationSettings(jwks_uri="https://idp.example.com/jwks"), + issuer_url=_ISSUER, + ) + assert str(auth.resource_server_url).rstrip("/") == _RESOURCE + + @pytest.mark.unit def test_build_skips_audience_by_default() -> None: """No audience config folds to SkipAudience: the validator does not check aud.""" verifier, _ = build_resource_server_auth( - ResourceServerSettings(resource_server_url=_RESOURCE), + ResourceServer.from_url(_RESOURCE), JwtValidationSettings(jwks_uri="https://idp.example.com/jwks"), - default_issuer_url=_ISSUER, + issuer_url=_ISSUER, ) assert verifier._validator._verify_aud is False assert verifier._validator._audience is None @@ -215,62 +231,51 @@ def test_build_skips_audience_by_default() -> None: def test_build_requires_audience_when_verifying() -> None: """verify_audience with an audience folds to RequireAudience(audience).""" verifier, _ = build_resource_server_auth( - ResourceServerSettings(resource_server_url=_RESOURCE), + ResourceServer.from_url(_RESOURCE), JwtValidationSettings( audience="api://x", verify_audience=True, jwks_uri="https://idp.example.com/jwks", ), - default_issuer_url=_ISSUER, + issuer_url=_ISSUER, ) assert verifier._validator._verify_aud is True assert verifier._validator._audience == "api://x" @pytest.mark.unit -def test_build_inactive_when_unconfigured() -> None: - """No resource_server_url means no auth: the profile is off, not an error.""" - assert ( - build_resource_server_auth( - ResourceServerSettings(), - JwtValidationSettings(), - default_issuer_url=_ISSUER, - ) - is None - ) - +def test_build_advertises_the_given_issuer() -> None: + """The resolved issuer the composition root passes is advertised verbatim. -@pytest.mark.unit -def test_build_issuer_defaults_to_login_issuer() -> None: - """With no inbound override, the inbound issuer is the login issuer.""" + Resolving the issuer (override vs login fallback, and the unresolvable fail-fast) + is the runtime's job; this pins only that the builder wires it onto AuthSettings. + """ _, auth = build_resource_server_auth( - ResourceServerSettings(resource_server_url=_RESOURCE), + ResourceServer.from_url(_RESOURCE), JwtValidationSettings(jwks_uri="https://idp.example.com/jwks"), - default_issuer_url=_ISSUER, + issuer_url=_ISSUER, ) assert str(auth.issuer_url).rstrip("/") == _ISSUER +# --- ResourceServer.from_url (host-authority parsing) ------------------------- + + @pytest.mark.unit -def test_build_inbound_issuer_overrides_login_issuer() -> None: - """An explicit inbound issuer wins over the login issuer.""" - override = "https://other-idp.example.com/realms/y" - _, auth = build_resource_server_auth( - ResourceServerSettings(resource_server_url=_RESOURCE), - JwtValidationSettings( - issuer_url=override, jwks_uri="https://other-idp.example.com/jwks" - ), - default_issuer_url=_ISSUER, - ) - assert str(auth.issuer_url).rstrip("/") == override +def test_resource_server_keeps_the_verbatim_url_and_derives_bare_host() -> None: + resource = ResourceServer.from_url("https://mcp.pipefy.com/mcp") + assert resource.url == "https://mcp.pipefy.com/mcp" + assert resource.host_authorities == ("mcp.pipefy.com",) @pytest.mark.unit -def test_build_without_resolvable_issuer_raises() -> None: - """resource_server_url set but no issuer (override or login) is a misconfiguration.""" - with pytest.raises(RuntimeError, match="no inbound issuer"): - build_resource_server_auth( - ResourceServerSettings(resource_server_url=_RESOURCE), - JwtValidationSettings(), - default_issuer_url=None, - ) +def test_resource_server_adds_host_port_when_url_names_a_port() -> None: + resource = ResourceServer.from_url("https://mcp.pipefy.com:8443/mcp") + assert resource.host_authorities == ("mcp.pipefy.com", "mcp.pipefy.com:8443") + + +@pytest.mark.unit +def test_resource_server_brackets_an_ipv6_literal() -> None: + """urlparse reports the host unbracketed; the wire Host is bracketed.""" + resource = ResourceServer.from_url("https://[2001:db8::1]:8443/mcp") + assert resource.host_authorities == ("[2001:db8::1]", "[2001:db8::1]:8443") diff --git a/packages/mcp/tests/core/test_runtime.py b/packages/mcp/tests/core/test_runtime.py index 045d9ac7..cac7a426 100644 --- a/packages/mcp/tests/core/test_runtime.py +++ b/packages/mcp/tests/core/test_runtime.py @@ -3,9 +3,16 @@ import httpx import pytest -from _rs_fixtures import authenticated_user, remote_rs_settings, request_with_user +from _rs_fixtures import ( + RS_JWKS_URI, + RS_RESOURCE, + authenticated_user, + remote_rs_settings, + request_with_user, +) from pipefy_auth import ( AuthSettings, + JwtValidationSettings, RefreshableBearerAuth, StaticBearerAuth, TokenResponse, @@ -14,11 +21,8 @@ from pipefy_sdk import PipefyClient, PipefySettings from pipefy_mcp._docs import DOCS_SETUP_REF -from pipefy_mcp.core.runtime import ( - McpRuntime, - RequestScopedIdentity, - StartupIdentity, -) +from pipefy_mcp.auth import RequestScopedIdentity, StartupIdentity +from pipefy_mcp.core.runtime import McpRuntime from pipefy_mcp.settings import McpSettings, Settings @@ -127,6 +131,38 @@ def test_remote_selects_request_scoped_identity_and_builds_inbound_auth(self): assert runtime.inbound_auth is not None assert isinstance(runtime._identity, RequestScopedIdentity) + @pytest.mark.unit + def test_remote_feeds_one_resource_to_the_allowlist_and_the_inbound_metadata(self): + """The parsed resource host reaches the transport allowlist and the RS metadata. + + The composition root parses ``resource_server_url`` once and feeds that one + :class:`ResourceServer` to both builders, so the allowlist's public host and + the advertised metadata resource cannot disagree. + """ + runtime = McpRuntime.for_profile(remote_rs_settings()) + + assert runtime.transport_security is not None + assert "mcp.example.com" in runtime.transport_security.allowed_hosts + _, auth = runtime.inbound_auth + assert str(auth.resource_server_url).rstrip("/") == RS_RESOURCE + + @pytest.mark.unit + def test_local_builds_the_transport_allowlist_from_explicit_hosts( + self, clear_auth_env + ): + """A local profile still builds the allowlist from PIPEFY_MCP_ALLOWED_HOSTS.""" + settings = Settings( + pipefy=PipefySettings(base_url="https://api.pipefy.com"), + auth=AuthSettings(static_token="env-bearer"), + mcp=McpSettings(allowed_hosts=["proxy.internal"]), + ) + + runtime = McpRuntime.for_profile(settings) + + assert runtime.inbound_auth is None + assert runtime.transport_security is not None + assert "proxy.internal" in runtime.transport_security.allowed_hosts + @pytest.mark.unit def test_remote_snapshots_the_callers_bearer_into_its_session(self): """A session opened under the remote profile carries the request's validated bearer.""" @@ -152,6 +188,49 @@ def test_remote_without_resource_server_fails_fast( with pytest.raises(RuntimeError, match="requires a resource server"): McpRuntime.for_profile(settings) + @pytest.mark.unit + def test_remote_without_resolvable_issuer_fails_fast(self, monkeypatch): + """Remote with a resource but no inbound issuer (override or login) refuses to build. + + The composition root resolves the inbound issuer and gates on it, so a + resource server with no issuer to validate its bearers fails fast here rather + than in the auth builder. Disabling the stored-session login drops the login + issuer and the delenv drops the explicit override, so neither source resolves. + """ + monkeypatch.delenv("PIPEFY_JWT_ISSUER_URL", raising=False) + settings = remote_rs_settings().model_copy( + update={ + "auth": AuthSettings(disable_stored_session=True), + "jwt": JwtValidationSettings(jwks_uri=RS_JWKS_URI), + } + ) + with pytest.raises(RuntimeError, match="requires an inbound issuer"): + McpRuntime.for_profile(settings) + + @pytest.mark.unit + def test_remote_inbound_issuer_defaults_to_the_login_issuer(self, monkeypatch): + """With no PIPEFY_JWT_ISSUER_URL override, the inbound issuer is the login issuer. + + A single-realm deployment reuses the IdP the server logs into as the issuer + that mints the inbound bearers it validates. ``remote_rs_settings`` pins an + explicit override, so drop it and configure only ``auth_url`` (the login + issuer) to pin the override-absent fallback the composition root owns + (``_login_issuer_url`` -> ``resolve_issuer_url``). + """ + monkeypatch.delenv("PIPEFY_JWT_ISSUER_URL", raising=False) + login_issuer = "https://signin.pipefy.com/realms/pipefy" + settings = remote_rs_settings().model_copy( + update={ + "auth": AuthSettings(auth_url=login_issuer), + "jwt": JwtValidationSettings(jwks_uri=RS_JWKS_URI), + } + ) + + runtime = McpRuntime.for_profile(settings) + + _, auth = runtime.inbound_auth + assert str(auth.issuer_url).rstrip("/") == login_issuer + @pytest.mark.unit def test_local_static_token_binds_the_static_bearer(self, clear_auth_env): """``PIPEFY_TOKEN`` resolves to a static-bearer startup identity, no inbound auth.""" diff --git a/packages/mcp/tests/core/test_transport_security.py b/packages/mcp/tests/core/test_transport_security.py new file mode 100644 index 00000000..a093c3a3 --- /dev/null +++ b/packages/mcp/tests/core/test_transport_security.py @@ -0,0 +1,138 @@ +"""Tests for ``build_transport_security`` (the DNS-rebinding allowlist resolver).""" + +import pytest + +from pipefy_mcp.auth import ResourceServer +from pipefy_mcp.core.transport_security import build_transport_security +from pipefy_mcp.settings import McpSettings + +_LOOPBACK_FORMS = { + "127.0.0.1", + "127.0.0.1:*", + "localhost", + "localhost:*", + "[::1]", + "[::1]:*", +} + + +def _build(*, resource_server_url=None, allowed_hosts=None, allowed_origins=None): + """Build the allowlist the way the runtime does: parse the URL once, feed it in.""" + resource = ( + ResourceServer.from_url(resource_server_url) if resource_server_url else None + ) + mcp = McpSettings(allowed_hosts=allowed_hosts, allowed_origins=allowed_origins) + return build_transport_security(mcp, resource) + + +@pytest.mark.unit +def test_unset_returns_none_to_keep_fastmcp_default(): + """No resource URL and no explicit allowlist -> FastMCP's own default stands.""" + assert _build() is None + + +@pytest.mark.unit +def test_resource_server_url_host_is_derived_and_protection_enabled(): + """The public host is added (bare and any-port) with loopback retained.""" + security = _build(resource_server_url="https://mcp.pipefy.com/mcp") + assert security is not None + assert security.enable_dns_rebinding_protection is True + assert "mcp.pipefy.com" in security.allowed_hosts + assert "mcp.pipefy.com:*" in security.allowed_hosts + assert _LOOPBACK_FORMS.issubset(set(security.allowed_hosts)) + + +@pytest.mark.unit +def test_resource_server_url_with_explicit_port_keeps_host_and_hostport(): + """A URL carrying a port contributes both the bare host and host:port.""" + security = _build(resource_server_url="https://mcp.pipefy.com:8443/mcp") + assert security is not None + assert "mcp.pipefy.com" in security.allowed_hosts + assert "mcp.pipefy.com:8443" in security.allowed_hosts + + +@pytest.mark.unit +def test_ipv6_literal_resource_url_is_bracketed_to_match_the_wire_host(): + """An IPv6-literal host is bracketed, as the Host header carries it. + + urlparse reports the hostname unbracketed ('2606:4700:4700::1111'), but a + client sends the bracketed form; without re-bracketing every request 421s. + """ + security = _build(resource_server_url="https://[2606:4700:4700::1111]/mcp") + assert security is not None + assert "[2606:4700:4700::1111]" in security.allowed_hosts + assert "[2606:4700:4700::1111]:*" in security.allowed_hosts + + +@pytest.mark.unit +def test_ipv6_literal_with_port_keeps_bracketed_host_and_hostport(): + """A ported IPv6 URL contributes both the bracketed host and host:port.""" + security = _build(resource_server_url="https://[2606:4700:4700::1111]:8443/mcp") + assert security is not None + assert "[2606:4700:4700::1111]" in security.allowed_hosts + assert "[2606:4700:4700::1111]:8443" in security.allowed_hosts + + +@pytest.mark.unit +def test_explicit_allowed_hosts_extend_the_derived_set(): + """PIPEFY_MCP_ALLOWED_HOSTS adds to loopback and the resource host.""" + security = _build( + resource_server_url="https://mcp.pipefy.com/mcp", + allowed_hosts=["proxy.internal"], + ) + assert security is not None + assert "mcp.pipefy.com" in security.allowed_hosts + assert "proxy.internal" in security.allowed_hosts + assert "proxy.internal:*" in security.allowed_hosts + + +@pytest.mark.unit +def test_explicit_allowed_hosts_alone_enable_protection(): + """An override with no resource URL still produces an enabled allowlist.""" + security = _build(allowed_hosts=["proxy.internal"]) + assert security is not None + assert security.enable_dns_rebinding_protection is True + assert "proxy.internal" in security.allowed_hosts + + +@pytest.mark.unit +def test_derived_origins_cover_each_host_in_both_schemes(): + """Without an override, origins are http+https for every allowed host.""" + security = _build(resource_server_url="https://mcp.pipefy.com/mcp") + assert security is not None + assert "http://mcp.pipefy.com" in security.allowed_origins + assert "https://mcp.pipefy.com" in security.allowed_origins + assert "http://localhost:*" in security.allowed_origins + + +@pytest.mark.unit +def test_explicit_allowed_origins_replace_the_derived_origins(): + """An explicit origin allowlist is used verbatim, not merged with derived.""" + security = _build( + resource_server_url="https://mcp.pipefy.com/mcp", + allowed_origins=["https://mcp.pipefy.com"], + ) + assert security is not None + assert security.allowed_origins == ["https://mcp.pipefy.com"] + + +@pytest.mark.unit +def test_explicit_empty_origins_are_honored_as_the_strictest_override(): + """An explicit empty origin list is kept verbatim (reject any Origin), not derived.""" + security = _build( + resource_server_url="https://mcp.pipefy.com/mcp", allowed_origins=[] + ) + assert security is not None + assert security.allowed_origins == [] + # The host allowlist is still derived; only origins are the strict override. + assert "mcp.pipefy.com" in security.allowed_hosts + + +@pytest.mark.unit +def test_explicit_empty_origins_alone_still_enable_protection(): + """Empty origins with no host still builds an allowlist (not FastMCP's default).""" + security = _build(allowed_origins=[]) + assert security is not None + assert security.enable_dns_rebinding_protection is True + assert security.allowed_origins == [] + assert _LOOPBACK_FORMS.issubset(set(security.allowed_hosts)) diff --git a/packages/mcp/tests/test_server.py b/packages/mcp/tests/test_server.py index bd0673cd..aeeb1c00 100644 --- a/packages/mcp/tests/test_server.py +++ b/packages/mcp/tests/test_server.py @@ -1,4 +1,5 @@ import json +from contextlib import asynccontextmanager from datetime import timedelta from unittest.mock import MagicMock, patch @@ -13,6 +14,7 @@ from pipefy_sdk import PipefySettings from pipefy_mcp.core.tool_middleware import ToolCallContext, short_circuit_error +from pipefy_mcp.core.transport_security import build_transport_security from pipefy_mcp.observability.tool_log_middleware import tool_log_middleware from pipefy_mcp.server import ( _make_lifespan, @@ -69,6 +71,7 @@ def mocked_runtime(): runtime = MagicMock() runtime.session_for_request.return_value = MagicMock() runtime.inbound_auth = None + runtime.transport_security = None with patch("pipefy_mcp.server.McpRuntime.for_profile", return_value=runtime): yield runtime @@ -463,6 +466,25 @@ def _asgi_client(app): return httpx.AsyncClient(transport=transport, base_url="http://testserver") +@asynccontextmanager +async def _serving_asgi_client(app): + """An ASGI client with the app's lifespan running. + + The transport's DNS-rebinding Host check sits behind the streamable-HTTP + session manager, whose task group is started by the ASGI lifespan. httpx's + ASGITransport does not run lifespan events, so drive Starlette's lifespan + context explicitly (the 401/metadata tests do not need this because they + short-circuit before the session manager). + """ + asgi = app.streamable_http_app() + async with asgi.router.lifespan_context(asgi): + transport = httpx.ASGITransport(app=asgi) + async with httpx.AsyncClient( + transport=transport, base_url="http://testserver" + ) as client: + yield client + + @pytest.mark.unit async def test_http_unauthenticated_request_gets_401_challenge(): """A request with no bearer is rejected with a 401 + WWW-Authenticate challenge. @@ -490,3 +512,63 @@ async def test_http_serves_protected_resource_metadata(): assert body["resource"].rstrip("/") == RS_RESOURCE advertised = [s.rstrip("/") for s in body["authorization_servers"]] assert RS_ISSUER in advertised + + +# --- transport allowlist (DNS-rebinding) reaches FastMCP and the transport ---- + +_PUBLIC_HOST_PING = { + "json": {"jsonrpc": "2.0", "method": "ping", "id": 1}, + "headers": {"host": "mcp.pipefy.com"}, +} + + +@pytest.mark.unit +def test_build_passes_the_runtimes_none_transport_security_to_fastmcp(mocked_runtime): + """When the runtime built no allowlist, FastMCP keeps its own loopback default.""" + with patch("pipefy_mcp.server.FastMCP") as mock_fastmcp: + build_pipefy_mcp_server(_MINIMAL_PIPEFY_SETTINGS) + assert mock_fastmcp.call_args.kwargs["transport_security"] is None + + +@pytest.mark.unit +def test_build_passes_the_runtimes_allowlist_to_fastmcp(mocked_runtime): + """The allowlist the runtime built reaches the FastMCP constructor verbatim.""" + mocked_runtime.transport_security = build_transport_security( + McpSettings(allowed_hosts=["mcp.pipefy.com"]), None + ) + with patch("pipefy_mcp.server.FastMCP") as mock_fastmcp: + build_pipefy_mcp_server(_MINIMAL_PIPEFY_SETTINGS) + assert ( + mock_fastmcp.call_args.kwargs["transport_security"] + is mocked_runtime.transport_security + ) + + +@pytest.mark.unit +async def test_http_loopback_default_rejects_a_public_host(mocked_runtime): + """The default (loopback-only) allowlist answers 421 to a public Host header. + + This is the behavior a fronted deployment hits before the allowlist is + configured: FastMCP auto-enables the loopback allowlist on the 127.0.0.1 + construction host, so a proxied public Host is a DNS-rebinding rejection. + """ + app = build_pipefy_mcp_server(_MINIMAL_PIPEFY_SETTINGS) + async with _serving_asgi_client(app) as client: + resp = await client.post("/mcp", **_PUBLIC_HOST_PING) + assert resp.status_code == 421 + + +@pytest.mark.unit +async def test_http_configured_allowlist_accepts_a_public_host(mocked_runtime): + """With the public host allowlisted, the same request clears the Host gate. + + It no longer 421s; it reaches the transport handler (which then fails on the + missing session, not on DNS-rebinding), proving the allowlist widened the gate. + """ + mocked_runtime.transport_security = build_transport_security( + McpSettings(allowed_hosts=["mcp.pipefy.com"]), None + ) + app = build_pipefy_mcp_server(_MINIMAL_PIPEFY_SETTINGS) + async with _serving_asgi_client(app) as client: + resp = await client.post("/mcp", **_PUBLIC_HOST_PING) + assert resp.status_code != 421 diff --git a/packages/mcp/tests/test_settings.py b/packages/mcp/tests/test_settings.py index ec3d410d..e3a75715 100644 --- a/packages/mcp/tests/test_settings.py +++ b/packages/mcp/tests/test_settings.py @@ -271,3 +271,58 @@ def test_mcp_remote_binds_any_host_without_the_escape_hatch(): settings = McpSettings(profile="remote", transport="http", host="0.0.0.0") assert settings.host == "0.0.0.0" assert settings.allow_insecure_http_bind is False + + +# --- transport allowlist fields (DNS-rebinding host/Origin allowlist) --------- + + +@pytest.mark.unit +def test_mcp_allowlists_default_to_none(): + """Unset allowlists stay None so the builder can preserve FastMCP's default.""" + mcp = McpSettings() + assert mcp.allowed_hosts is None + assert mcp.allowed_origins is None + + +@pytest.mark.unit +def test_mcp_allowed_hosts_from_env_parses_json(monkeypatch): + """PIPEFY_MCP_ALLOWED_HOSTS is a JSON array, like RS required_scopes.""" + monkeypatch.setenv("PIPEFY_MCP_ALLOWED_HOSTS", '["mcp.pipefy.com", "mcp:8000"]') + assert Settings().mcp.allowed_hosts == ["mcp.pipefy.com", "mcp:8000"] + + +@pytest.mark.unit +def test_mcp_allowed_origins_from_env_parses_json(monkeypatch): + monkeypatch.setenv("PIPEFY_MCP_ALLOWED_ORIGINS", '["https://mcp.pipefy.com"]') + assert Settings().mcp.allowed_origins == ["https://mcp.pipefy.com"] + + +@pytest.mark.unit +def test_mcp_allowlist_trims_surrounding_whitespace(): + """Each entry is trimmed; localhost is kept (no SSRF gate on operator entries).""" + mcp = McpSettings( + allowed_hosts=[" mcp.pipefy.com ", "localhost"], + allowed_origins=[" https://mcp.pipefy.com "], + ) + assert mcp.allowed_hosts == ["mcp.pipefy.com", "localhost"] + assert mcp.allowed_origins == ["https://mcp.pipefy.com"] + + +@pytest.mark.unit +@pytest.mark.parametrize("field", ["allowed_hosts", "allowed_origins"]) +def test_mcp_allowlist_rejects_a_blank_entry(field): + """A blank/whitespace entry is a config error, not a silently-dropped value. + + Dropping it would hide the typo, and for allowed_origins could collapse the + list to the strict reject-all-Origin posture, so the settings boundary refuses. + """ + with pytest.raises(ValidationError, match="blank entry"): + McpSettings(**{field: ["ok", " "]}) + + +@pytest.mark.unit +def test_mcp_explicit_empty_allowlist_is_preserved(): + """An explicit [] is a deliberate value (reject-all-Origin), not a blank entry.""" + mcp = McpSettings(allowed_hosts=[], allowed_origins=[]) + assert mcp.allowed_hosts == [] + assert mcp.allowed_origins == [] diff --git a/packages/mcp/tests/tools/conftest.py b/packages/mcp/tests/tools/conftest.py index 3ef5f791..e99f26f8 100644 --- a/packages/mcp/tests/tools/conftest.py +++ b/packages/mcp/tests/tools/conftest.py @@ -6,7 +6,8 @@ import pytest from mcp.server.fastmcp import FastMCP -from pipefy_mcp.core.runtime import McpRuntime, RequestScopedIdentity +from pipefy_mcp.auth import RequestScopedIdentity +from pipefy_mcp.core.runtime import McpRuntime from pipefy_mcp.settings import settings diff --git a/packages/mcp/tests/tools/test_pipe_tools.py b/packages/mcp/tests/tools/test_pipe_tools.py index 37728dfe..85d8c293 100644 --- a/packages/mcp/tests/tools/test_pipe_tools.py +++ b/packages/mcp/tests/tools/test_pipe_tools.py @@ -17,7 +17,8 @@ ) from pipefy_sdk import PipefyClient -from pipefy_mcp.core.runtime import McpRuntime, RequestScopedIdentity +from pipefy_mcp.auth import RequestScopedIdentity +from pipefy_mcp.core.runtime import McpRuntime from pipefy_mcp.core.tool_error_envelope import tool_error, tool_error_message from pipefy_mcp.settings import settings from pipefy_mcp.tools.pipe_tool_helpers import ( diff --git a/packages/mcp/tests/tools/test_tool_context.py b/packages/mcp/tests/tools/test_tool_context.py index bf4d07e1..769b74b8 100644 --- a/packages/mcp/tests/tools/test_tool_context.py +++ b/packages/mcp/tests/tools/test_tool_context.py @@ -5,7 +5,8 @@ import pytest -from pipefy_mcp.core.runtime import McpRuntime, RequestScopedIdentity +from pipefy_mcp.auth import RequestScopedIdentity +from pipefy_mcp.core.runtime import McpRuntime from pipefy_mcp.settings import settings from pipefy_mcp.tools.tool_context import get_pipefy_client