Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ These variables load into `pipefy_mcp.McpSettings` (`settings.mcp`). TOML keys u
|----------|---------|--------|
| `PIPEFY_MCP_PROFILE` | `local` | Launch profile: `local` registers every tool; `remote` exposes only remote-safe tools and validates an inbound bearer per request. |
| `PIPEFY_MCP_TRANSPORT` | derived from profile | `stdio` or `http`. Unset: `local` → `stdio`, `remote` → `http`. |
| `PIPEFY_MCP_HOST` | `127.0.0.1` | HTTP bind host (loopback only until hosted OBO lands). |
| `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_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. |
30 changes: 30 additions & 0 deletions packages/infra/src/pipefy_infra/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
literal-IP check (callers must follow up with the DNS gate).
* :func:`assert_hostname_is_not_internal`: building block of the above;
exposed for callers that already have a parsed hostname.
* :func:`is_loopback_host`: predicate reporting whether a bind host keeps
a server reachable only from the local machine (``localhost`` or a
literal IP in ``127.0.0.0/8`` or ``::1``).
* :func:`assert_hostname_resolves_to_public_ips`: asynchronous DNS gate.
Resolves the hostname and rejects when any resolved IP is in a blocked
range. Counter to DNS-rebinding attacks that point a public name at an
Expand Down Expand Up @@ -101,6 +104,33 @@ def assert_hostname_is_not_internal(hostname: str, *, context: str) -> None:
)


def is_loopback_host(host: str) -> bool:
"""Report whether ``host`` binds a server to the local machine only.

Loopback means ``localhost`` or a literal IP in ``127.0.0.0/8`` or ``::1``.
``0.0.0.0`` (and ``::``) is unspecified, not loopback, so it is reported as
non-loopback. A hostname that is not a literal IP (anything other than
``localhost``) is reported as non-loopback: it is not resolved here, and
treating an unresolved name as reachable is the safe default for a
bind-safety gate.

Bracketed IPv6 literals (``[::1]``) are tolerated for callers passing a raw
URL host slot rather than ``urlparse(...).hostname``.
"""
candidate = (host or "").strip().lower()
if candidate == "localhost":
return True
candidate = (
candidate[1:-1]
if candidate.startswith("[") and candidate.endswith("]")
else candidate
)
try:
return ipaddress.ip_address(candidate).is_loopback
except ValueError:
return False


def validate_https_url(
url: str,
field_label: str,
Expand Down
18 changes: 18 additions & 0 deletions packages/infra/tests/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,21 @@ async def test_validate_and_assert_public_url_allow_insecure_skips_literal_ip():
allow_insecure=True,
)
assert hostname == "127.0.0.1"


@pytest.mark.unit
@pytest.mark.parametrize(
"host",
["127.0.0.1", "127.0.0.2", "localhost", "LOCALHOST", "::1", "[::1]", " ::1 "],
)
def test_is_loopback_host_accepts_loopback(host: str) -> None:
assert security.is_loopback_host(host) is True


@pytest.mark.unit
@pytest.mark.parametrize(
"host",
["0.0.0.0", "::", "203.0.113.5", "example.com", "", " "],
)
def test_is_loopback_host_rejects_non_loopback(host: str) -> None:
assert security.is_loopback_host(host) is False
33 changes: 22 additions & 11 deletions packages/mcp/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ marker is described below.
resource-server URL is configured, validates an inbound bearer per request.
- **`--transport {stdio|http}`** (env `PIPEFY_MCP_TRANSPORT`). Left unset it follows
the profile: `local` speaks stdio, `remote` serves over Streamable HTTP. Set it
explicitly to run `local` over loopback HTTP. `remote` over stdio is rejected: a
explicitly to run `local` over HTTP (loopback by default; see "Bind-safety
interlock"). `remote` over stdio is rejected: a
per-request bearer has no stdio equivalent. The pair is resolved (and validated)
once, at startup, by `resolve_mcp_settings`.

Expand All @@ -45,8 +46,9 @@ bearer per request and opens a per-request session carrying a snapshot of that
validated bearer, so concurrent callers each act as themselves rather than as a
single identity resolved at startup. All sessions share one process-scoped engine
(the GraphQL endpoints and their schema cache). `local` runs as the one credential
resolved at startup. The transport still binds loopback-only until the
DNS-rebinding host/Origin allowlist lands.
resolved at startup. The unauthenticated `local` profile binds loopback-only over
HTTP unless the `PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND` escape hatch is set; the
authenticated `remote` profile binds any host (see "Bind-safety interlock" below).

**Resource-server profile.** Config is split by domain. *Token validation* is an
auth concern and lives in `pipefy_auth.JwtValidationSettings` (`settings.jwt`,
Expand All @@ -71,14 +73,23 @@ 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.

**Loopback bind.** `_assert_safe_http_bind` restricts the HTTP transport to a
loopback bind, unconditionally for now. Per-request on-behalf-of identity (each
call runs as the validated caller, not a single startup identity) means inbound
identity is not the constraint; the constraint is DNS-rebinding protection, the
configurable host / Origin allowlist for a proxied deployment. Off-loopback binding
stays off until that lands (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).
**Bind-safety interlock.** The property protected is auth posture, not bind
interface: an unauthenticated profile must not be reachable by untrusted callers.
`McpSettings._enforce_bind_safety` (a `model_validator` at the settings boundary)
refuses a non-loopback HTTP bind under the unauthenticated `local` profile unless
`PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND` is set. Living at the settings boundary means
no serving path routes around it (the coverage argument, why every serving path
inherits the guarantee, lives on the `_enforce_bind_safety` docstring). The `remote` profile
validates a per-request bearer, so its bind host is irrelevant and is not checked
(a container binds `0.0.0.0` and is still private). Loopback detection is
`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).

## Tool registration

Expand Down
38 changes: 4 additions & 34 deletions packages/mcp/src/pipefy_mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@
You are connected to a Pipefy MCP server for managing Kanban-style workflow processes.
""").strip()

# Hosts that keep the server reachable only from the local machine. The HTTP
# transport refuses to bind anywhere else (a routable interface or 0.0.0.0)
# until the DNS-rebinding host/Origin allowlist lands; see
# _assert_safe_http_bind.
_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "::1"})


def _make_lifespan(
runtime: McpRuntime,
Expand Down Expand Up @@ -143,29 +137,6 @@ def build_pipefy_mcp_server(
return app


def _assert_safe_http_bind(*, host: str) -> None:
"""Refuse to bind the HTTP transport to a non-loopback host.

The HTTP transport is restricted to loopback for now. The resource-server
profile carries per-request on-behalf-of identity (each call runs as the
validated caller, not a single startup identity), so inbound identity is not
the constraint; the constraint is DNS-rebinding protection, the configurable
host / Origin allowlist. (The filesystem tools, e.g. the attachment uploads,
also only make sense on loopback, where the server shares the client's disk;
remote-safe file inputs are separate follow-up work.)

Off-loopback binding stays off until that allowlist lands; see
experiments/hosted-obo/RFC-OUTLINE.md.
"""
if host in _LOOPBACK_HOSTS:
return
raise RuntimeError(
f"Refusing to serve over HTTP on a non-loopback host ({host}). The HTTP "
f"transport is restricted to loopback (127.0.0.1/localhost/::1) until "
f"the hosted on-behalf-of profile lands."
)


def run_server(settings: Settings) -> None:
"""Run the Pipefy MCP server. The single serve entry point for both transports.

Expand All @@ -175,8 +146,7 @@ def run_server(settings: Settings) -> None:
pair) and passes the result, so this module reads no process globals.

``settings.mcp.transport == "stdio"`` speaks MCP over stdio; ``"http"`` serves
over Streamable HTTP on ``host``/``port``, restricted to a loopback bind until
the hosted on-behalf-of profile lands (see :func:`_assert_safe_http_bind`).
over Streamable HTTP on ``host``/``port``.

``settings.mcp.profile == "remote"`` selects the default-deny remote-safe tool
surface and validates an inbound bearer per request; it requires a configured
Expand All @@ -191,8 +161,7 @@ def run_server(settings: Settings) -> None:
Both transports build the same app through :func:`build_pipefy_mcp_server`,
which builds the app-scoped runtime via :meth:`McpRuntime.for_profile` (owning
the inbound resource-server pair for ``remote`` and failing fast when that
profile has no resource server). They differ only in the transport ``run`` and
HTTP's bind concerns.
profile has no resource server). They differ only in the transport ``run``.
"""
mcp = settings.mcp
configure_observability_logging(log_level=mcp.log_level)
Expand All @@ -214,6 +183,7 @@ def run_server(settings: Settings) -> None:
mcp.profile,
"active" if mcp.profile == "remote" else "inactive",
)
_assert_safe_http_bind(host=mcp.host)

# Bind safety is enforced at the settings boundary (McpSettings._enforce_bind_safety);
# host/port arrive already vetted, so there is nothing to re-check here.
build_pipefy_mcp_server(settings).run("streamable-http")
54 changes: 52 additions & 2 deletions packages/mcp/src/pipefy_mcp/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ def settings_customise_sources(
description=(
"Bind host for the Streamable HTTP transport (env: PIPEFY_MCP_HOST). "
"Only consulted when serving over HTTP (--transport http); the stdio "
"transport ignores it. Must stay loopback (the default): the HTTP "
"transport refuses a non-loopback bind while it is unauthenticated."
"transport ignores it. Under the unauthenticated 'local' profile a "
"non-loopback bind is refused unless PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND "
"is set; the authenticated 'remote' profile binds any host."
),
)

Expand All @@ -120,6 +121,18 @@ def settings_customise_sources(
),
)

allow_insecure_http_bind: bool = Field(
default=False,
description=(
"When true (env: PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND), the "
"unauthenticated 'local' profile may serve HTTP on a non-loopback "
"host. The escape hatch for exposing the full tool surface with no "
"inbound bearer to callers that are not on the local machine. The "
"authenticated 'remote' profile never needs it: it validates a "
"per-request bearer, so its bind host is unrestricted."
),
)

@field_validator("log_level", mode="before")
@classmethod
def _normalize_log_level(cls, value: object) -> object:
Expand All @@ -145,6 +158,43 @@ def _resolve_transport(self) -> Self:
)
return self

@model_validator(mode="after")
def _enforce_bind_safety(self) -> Self:
"""Refuse to serve the unauthenticated tool surface off loopback.

The property protected is auth posture, not bind interface: an
unauthenticated profile must not be reachable by untrusted callers. The
'local' profile registers every tool and wires no inbound bearer, so
over HTTP on a non-loopback host it exposes the full surface to whoever
can reach the port. The 'remote' profile validates a per-request bearer,
so its bind host is irrelevant and is not checked here (a container
binds 0.0.0.0 legitimately). Runs after :meth:`_resolve_transport`, so
``transport`` is concrete.

Enforcing at the settings boundary means every serving path inherits the
guarantee: :func:`build_pipefy_mcp_server` and a caller building the ASGI
app directly both take a resolved ``Settings``, so none re-checks the bind
or routes around it.

``allow_insecure_http_bind`` is the explicit escape hatch for operators
who accept an unauthenticated public bind.
"""
if (
self.transport == "http"
and self.profile == "local"
and not self.allow_insecure_http_bind
and not security.is_loopback_host(self.host)
):
raise ValueError(
"the 'local' profile serves every tool with no inbound bearer, "
f"so it refuses a non-loopback HTTP bind ({self.host!r}). Bind a "
"loopback host, switch to '--profile remote' with a resource "
"server to validate per-request callers, or set "
"PIPEFY_MCP_ALLOW_INSECURE_HTTP_BIND to accept an "
"unauthenticated public bind."
)
return self


class ResourceServerSettings(BaseSettings):
"""This MCP server's identity as an OAuth protected resource (HTTP profile).
Expand Down
2 changes: 1 addition & 1 deletion packages/mcp/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def test_rejects_an_empty_host(mocker, bad_host):

Without the guard, an empty value reaches ``resolve_mcp_settings`` as an
explicit init-kwarg, silently displacing the ``127.0.0.1`` default and later
failing the loopback check with a misleading non-loopback error.
tripping the bind-safety interlock with a misleading non-loopback error.
"""
server_mock = mocker.patch("pipefy_mcp.main.run_server")
argv = [bad_host] if bad_host.startswith("--host=") else ["--host", bad_host]
Expand Down
46 changes: 19 additions & 27 deletions packages/mcp/tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from pipefy_mcp.core.tool_middleware import ToolCallContext, short_circuit_error
from pipefy_mcp.observability.tool_log_middleware import tool_log_middleware
from pipefy_mcp.server import (
_assert_safe_http_bind,
_make_lifespan,
_register_pipefy_tools,
build_pipefy_mcp_server,
Expand Down Expand Up @@ -272,20 +271,6 @@ async def foreign_mcp_tool() -> str:
# --- HTTP (Streamable) transport profile ------------------------------------


@pytest.mark.unit
@pytest.mark.parametrize("host", ["127.0.0.1", "localhost", "::1"])
def test_loopback_http_bind_allows_loopback_hosts(host):
_assert_safe_http_bind(host=host)


@pytest.mark.unit
@pytest.mark.parametrize("host", ["0.0.0.0", "203.0.113.5"])
def test_loopback_http_bind_refuses_non_loopback_hosts(host):
"""A non-loopback bind is refused until the hosted on-behalf-of profile lands."""
with pytest.raises(RuntimeError, match="non-loopback host"):
_assert_safe_http_bind(host=host)


@pytest.mark.unit
def test_run_server_stdio_logs_the_argv_resolved_profile(monkeypatch):
"""The startup log reflects the argv-resolved profile, not the ambient env.
Expand Down Expand Up @@ -382,9 +367,7 @@ def test_run_server_remote_without_resource_server_fails_fast(monkeypatch):
The remote profile acts on behalf of the caller, so it needs a per-request
bearer to validate. Without a configured resource server there is no
per-request identity, and silently falling back to the single startup
credential would defeat the profile, so the runtime refuses to build. The
loopback bind guard runs first, so the host here is loopback to reach the
resource-server check.
credential would defeat the profile, so the runtime refuses to build.
"""
monkeypatch.delenv("PIPEFY_MCP_RS_RESOURCE_SERVER_URL", raising=False)
with pytest.raises(RuntimeError, match="requires a resource server"):
Expand Down Expand Up @@ -435,17 +418,26 @@ def test_run_server_http_respects_an_explicit_zero_port(remote_rs_env):


@pytest.mark.unit
def test_run_server_http_refuses_non_loopback_before_building(remote_rs_env):
"""The loopback guard fires before the app is built or served."""
with patch("pipefy_mcp.server.build_pipefy_mcp_server") as mock_build:
with pytest.raises(RuntimeError, match="Refusing to serve"):
run_server(
resolve_mcp_settings(
profile="remote", transport="http", host="0.0.0.0", port=9123
)
def test_run_server_remote_serves_off_loopback_without_a_bind_guard(remote_rs_env):
"""The resource-server profile binds a non-loopback host with no guard or bypass.

Auth posture, not bind interface, is the axis: ``remote`` validates a
per-request bearer, so ``0.0.0.0`` is a legitimate hosted bind and nothing
refuses it (a container binds ``0.0.0.0`` and is still private).
"""
fake_app = MagicMock()
with patch(
"pipefy_mcp.server.build_pipefy_mcp_server", return_value=fake_app
) as mock_build:
run_server(
resolve_mcp_settings(
profile="remote", transport="http", host="0.0.0.0", port=9123
)
)

mock_build.assert_not_called()
(built_settings,), _ = mock_build.call_args
assert built_settings.mcp.host == "0.0.0.0"
fake_app.run.assert_called_once_with("streamable-http")


# --- Resource-server role (OAuth 2.0 inbound bearer validation) --------------
Expand Down
Loading