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
21 changes: 21 additions & 0 deletions packages/mcp/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,27 @@ than a local `file_path`), enforce that in the tool body gated on
exclusion deserves a reason gets a plain code comment stating why; the exclusion
itself needs no annotation.

### Process-global configuration and the single-backend assumption

When the server runs in hosted (`remote`) mode, one process serves many users at the same time. Settings loaded from the environment are **shared by everyone** — there is one copy for the whole process, not one per user.

That's fine when a setting answers a question about the *deployment* ("which Pipefy backend do we talk to?", "how long before a lookup times out?"). It's a problem if a setting ever answers a question about a *user* ("who is this call acting as?", "which org does this caller belong to?") — because then one shared value would silently apply to every user. The one truly per-user thing, identity, never comes from settings: in hosted mode each request carries its own validated token (`RequestScopedIdentity`). `settings.auth` is read in both modes, but always for a per-deployment purpose: local mode resolves the one outbound startup credential from it, and remote mode reads it once at startup to derive the default inbound issuer URL (see "Resource-server profile" above) — safe under the same single-realm assumption.

Everything else that reads shared settings is safe because of one assumption, stated here on purpose so it isn't forgotten: **one hosted deployment serves a single integrator against a single backend.** In other words, every user of a deployment shares the same config because they genuinely share the same setup. The full list of shared-settings reads a tool call can hit, audited for #306:

- `PIPEFY_BASE_URL` — which backend the server talks to. Everyone hits the same one; this is the assumption itself.
- The GraphQL schema cache (`gql_reuse_fetched_graphql_schema`) — shared across users, which is fine because one backend means one schema.
- `permission_denied_enrichment_timeout_seconds` (`tools/graphql_error_helpers.py`) — a timeout knob. Same for everyone by design.
- `unified_envelope` (`core/tool_error_envelope.py`) — a flag that changes the shape of tool responses. Applies identically to every caller.
- The bind host and transport allowlist (`core/transport_security.py`, from `McpSettings`) — DNS-rebinding / bind-safety config resolved once at startup. A property of the deployment's front door, not of any caller.
- `default_webhook_name` and `allow_insecure_urls` (SDK webhook service, on create/update) — a cosmetic default name and an insecure-URL escape hatch; the webhook tools aren't exposed remotely anyway.

A related note: tools that call Pipefy's Internal API (like `delete_card_relation`) carry no special credential requirement. The SDK binds the session's one credential to all three API endpoints (public, Interfaces, Internal), so the Internal API simply receives whatever credential the caller already has — nothing in the client is service-account-specific. Those tools stay off the remote seed for the ordinary reason every tool starts off it: they are write mutations that haven't been reviewed for remote exposure yet.

**When this breaks (the rework trigger).** If one hosted server ever needs to serve *multiple* backends, or integrators with different config, the assumption is gone: "same for everyone" stops being true, and each of the reads above becomes a per-user question. The fix is the same one identity already went through in #302 — stop reading the value from shared settings and resolve it from the incoming request instead.

**How this is enforced.** An import-linter rule in `pyproject.toml` forbids tool code from *importing* `pipefy_mcp.settings`, even indirectly through a helper (that indirect path is how `unified_envelope` was caught). Every existing import is an explicitly listed, commented exception. If someone adds a new settings import to tool-reachable code, `uv run lint-imports` fails, and it only gets an exception after review confirms it reads a per-deployment value — never a per-user one. Known limit: the rule sees imports, not reads. Code that already holds a settings object — `McpRuntime.settings`, reachable from any tool through its request context — can read a value without adding an import, so that path stays a manual-review item. Per-user values must come from the request.

## Tool-call middleware

Cross-cutting concerns that wrap a tool invocation (logging, per-user quotas,
Expand Down
33 changes: 33 additions & 0 deletions packages/mcp/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,39 @@ layers = [
"pipefy_mcp.settings",
]

# Hosted per-user safety (#306): settings reachable from the tool layer are
# process-global — under the remote profile they are shared across concurrent
# callers, so such a read may only ever make a per-deployment decision, and each
# one is reviewed onto ignore_imports (direct or via a core helper; the contract
# checks transitive chains). The line and its rationale live in CLAUDE.md
# ("Process-global configuration and the single-backend assumption").
[[tool.importlinter.contracts]]
name = "Tools read no process-global settings"
type = "forbidden"
source_modules = ["pipefy_mcp.tools"]
forbidden_modules = ["pipefy_mcp.settings"]
ignore_imports = [
# Reviewed: permission_denied_enrichment_timeout_seconds, a deployment
# tuning knob, not a per-user decision.
"pipefy_mcp.tools.graphql_error_helpers -> pipefy_mcp.settings",
# Reviewed: unified_envelope, a per-deployment response-shape flag applied
# identically to every caller.
"pipefy_mcp.core.tool_error_envelope -> pipefy_mcp.settings",
# Reviewed: the composition root builds the runtime from settings at
# startup; tool_context only reads the already-built runtime off the
# lifespan context, never settings at call time.
"pipefy_mcp.core.runtime -> pipefy_mcp.settings",
# Reviewed: imports the Settings type for startup identity resolution.
# StartupIdentity receives settings from the composition root and
# RequestScopedIdentity resolves from the request bearer — no call-time
# global read. Reached transitively via core.runtime.
"pipefy_mcp.auth.session_identity -> pipefy_mcp.settings",
# Reviewed: reads McpSettings.host and the transport allowlist at startup
# for bind-safety / DNS-rebinding config — per-deployment, not a per-user
# decision. Reached transitively via core.runtime.
"pipefy_mcp.core.transport_security -> pipefy_mcp.settings",
]

# Next ratchet (disabled): lock the domain as framework-free once it has a home.
# The pure domain is still scattered: the error/success envelope lives in
# core/tool_error_envelope.py and the payload builders under tools/, with no single
Expand Down
8 changes: 4 additions & 4 deletions packages/mcp/src/pipefy_mcp/tools/pipe_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,15 +522,15 @@ async def delete_card_relation(
confirm: bool = False,
debug: bool = False,
) -> dict:
"""Remove a link between two related cards (requires service-account credentials).
"""Remove a link between two related cards.

``source_id`` is the **pipe relation** id from ``get_pipe_relations`` (same as
``create_card_relation``). Two-step flow: preview with ``confirm=False`` (default),
then execute with ``confirm=True`` after explicit approval.

Requires service-account credentials (PIPEFY_SERVICE_ACCOUNT_CLIENT_ID,
PIPEFY_SERVICE_ACCOUNT_CLIENT_SECRET) because the ``deleteCardRelation``
mutation is only available on the Internal API, not the public GraphQL schema.
The ``deleteCardRelation`` mutation is only available on the Internal API,
not the public GraphQL schema; it runs with the session's credential like
every other tool.

Args:
child_id: Child card ID in the relation.
Expand Down