Commit Graph
12 Commits
Author SHA1 Message Date
kevinveenbirkenbachandClaude Opus 5 a458ef2965 fix(mcp): reject a provider with no callable tool
Hermes and OpenClaw wrapped the tool allowlist in a guard that dropped the key when the callable set was empty. Both upstreams read an absent filter as "register every tool the server serves", so a provider resolving to no tools got the widest surface instead of none. Nextcloud keeps its contract in tools.categories with no allowlist and admits both clients, so it was the live case.

The templates now render the filter unconditionally, the way the n8n client node already pins include to selected; the two tests that asserted the omission are inverted. mcp_servers additionally rejects such a provider with the fatal code tools_unenforceable: a client filters by tool name, so a runtime-composed surface reaches it as no surface at all, and a server the agent can never call is worse wired in than refused with a reason.

A deployment pairing Nextcloud with either agent now aborts naming the provider, where it previously handed the agent every Context Agent tool unfiltered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 16:19:08 +02:00
kevinveenbirkenbachandClaude Opus 5 791eee7c9f refactor(mcp)!: prove the endpoint contract with a CLI test, not a deploy probe
The contract check ran as an Ansible task in the middle of every provider deploy, and the two client checks ran the same way. A test that is part of the deploy cannot be run without deploying, cannot be re-run on its own, and reports its verdict as a deploy failure. Twenty-seven task files lose 491 lines to this.

It is now what it always was. Each provider ships files/test/test.sh, templates/test.env.j2 and meta/tests.yml, and test-e2e-cli stages one shared harness beside it: run.sh drives contract.sh in a throwaway container on the provider's own network, then isolation.sh for adapter-fronted providers. The read probe each provider offers moved from the calling task into meta/mcp.yml next to the allowlist it must satisfy, and the schema now rejects a read probe naming a tool outside that allowlist.

The bearer the test presents is the one a client resolves through mcp.credential, via the new mcp_client_bearer lookup over resolve_credential, which moved to utils.roles.applications.mcp so it is the same function mcp_servers renders every client connection with. The probe presented whatever the calling role passed, which for the fifteen adapter-fronted providers is a different secret than any client holds.

Sidecar isolation is read from the engine rather than measured inside the container, so it no longer needs a delegation to the node holding the task: attachment is compared by network name, which drops the gwbridge special case, and a swarm worker says it cannot measure instead of skipping silently.

Retrying is bounded by wall clock against MCP_DEADLINE_SECONDS, which scales through the same lookup('timeout') as the harness budget it has to stay under. A flag that is neither true nor false aborts rather than reading as off.

mcp-provider-probe becomes mcp-provider-cli-test: an enabled provider must ship the test rather than include the task. mcp-probe-bearer becomes mcp-adapter-bearer, tying ADAPTER_BEARER to the declared credential key now that no probe include carries it. A new integration test renders the shared env template for every provider through Ansible's own templar, so a later change to it breaks visibly instead of poisoning twenty deploys quietly.

Hermes and OpenClaw keep their client-side proof as CLI tests of their own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 20:46:26 +02:00
kevinveenbirkenbachandClaude Opus 5 8e3fe04d72 fix(mcp): read a provider's bearer where provisioning writes it
Discovery resolved applications[role].credentials, the address from
before f713ba8350 moved every credential under secrets.credentials. The
run's own inventory shows which one is real: twelve applications carry
secrets.credentials, none carries the top-level key. So every provider
declaring credential.source: credentials - fifteen of the twenty - handed
back an empty mapping, and reconciliation aborted on a secret that had
been provisioned all along:

  mcp_servers: 1 authorized MCP server(s) cannot be rendered.
  web-app-prometheus: credential_missing: web-app-prometheus declares
  owner 'mcp-web-app-prometheus' but no secret resolved

The five providers on token_store read through users and were never
affected, which is why only some clients broke.

Provider and client were reading the same secret at two addresses:
web-app-prometheus/vars/main.yml:32 renders its bearer from
secrets.credentials.mcp_bearer. Without the strict assert this would have
surfaced as a 401 at runtime rather than a failed deploy.

The lookup was correct when 6881ac5132 wrote it - the rename landed on a
branch it was not an ancestor of, and arrived here through the main
merges of 08-15. Its own message notes that no regex over lookup('config')
could see an access path like this one; a .get("credentials") split across
two lines in a Python plugin is equally invisible to it.

The address now comes from credential_key's constants, which is where the
module already claims a credential is addressed by exactly one string.

resolve_credential was always handed a fabricated mapping directly, so the
function was covered and its caller's address was not. The new tests close
that seam, one of them end to end over the shape provisioning produces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 20:57:07 +02:00
kevinveenbirkenbach c04c49e753 refactor(mcp): drop the path-addressed endpoint from client discovery
Baserow was the only role declaring endpoint.key_credential, and putting
the adapter in front of it left the field with no consumer. Keeping it
would also contradict the decision that put the adapter there: a secret
in the URL path cannot be scoped per consumer and cannot be revoked on
its own, so a provider authenticating that way belongs behind an adapter
rather than in a client's rendered configuration. The segment now stays
between the adapter and its upstream.

Discovery loses a config lookup per provider per client along with the
branch. The test that proved the segment was appended becomes its
inverse: an endpoint still declaring the field gets a URL without one, so
the decision is held rather than merely unproven.
2026-08-29 10:55:34 +02:00
kevinveenbirkenbachandClaude Opus 5 ec736bd4d7 feat(mcp): offer no client a tool its provider calls mutating
Both agents registered every tool every provider served. Hermes documents that as its own default in tools/mcp_tool.py, "Neither set -> register all tools", and the role never set tools.include; OpenClaw's toolFilter was never rendered at all. A provider that grew a mutating tool would have reached both without anything declaring it.

A serving role now names its mutating tools, the discovery snapshot carries that set beside the allowlist, and every client renders the allowlist minus it in whatever form its upstream enforces. The subtraction sits at the client and not in discovery because Flowise compares the declared set against what its registry actually discovered and needs the full served list. Where a provider permits mutations there is nothing to withhold, so the set renders empty.

This is a client-side boundary, not a server-side one: whoever holds the endpoint key can still call a write tool directly. Baserow is the case that made it necessary, since its MCPEndpoint model carries no scope and its owner is a workspace administrator, so the declared mutating_tools_enabled bounds nothing there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 00:29:12 +02:00
kevinveenbirkenbachandClaude Opus 5 48d8289a37 fix(mcp): render a deferred consumer id before the registry matches it
sys-svc-mcp-reconcile sets application_id from a loop variable, so the
lookup received the literal {{ mcp_client.id }}. An unrendered id matches
no allowed_consumers, which rejected every provider as
consumer_not_allowed and emptied each client's registry without raising.

Pull the resolution into resolve_consumer_id() so the branch is reachable
from a test, and cover the three cases: a deferred id is rendered, a plain
id is kept, a missing id aborts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 11:42:38 +02:00
kevinveenbirkenbach 001aeda0b1 fix(mcp): narrow discovery to the providers the run deploys
A client rendered an endpoint for every provider the inventory knew,
including providers this run never deploys, so it carried connections
nothing was serving. Discovery now intersects with
application_closure(deployment.whitelist), the same set the service
loader preloads from. An unresolvable closure filters nothing, because
dropping every provider would look exactly like a deployment without
any.

credential_missing stays non-fatal in the app pass, where a provider may
still be waiting to write its secret, and becomes fatal at the
reconciliation stage, where every provider has already run.
2026-08-09 22:52:19 +02:00
kevinveenbirkenbach 06bc856c32 refactor(mcp): make MCP a role topic instead of a service entry
The block sat under services but never behaved like a service. A service has
exactly one providing role, declared with provides, so the registry can map the
key to that role and consumers bind their flag to its presence. No role declares
provides: mcp. There are seventeen providers and the pairing is done by
allowed_consumers inside the block itself.

Every rule that enforces the service model therefore needed an exemption for it:
a shared-provider suppression in fourteen roles, a playwright-service-flag
suppression, and two freshly added exemptions in the dynamic-flag and
variants-static-override checks. Four independent rules, four escape hatches,
one cause. They are all gone now, together with a bond key that only the
resource budgeter reads and only for entries under services, and a wordpress
addon bridge that pointed at a service block that no longer exists.

The block moves to meta/mcp.yml. The applications loader maps meta/<name>.yml to
applications.<app>.<name> generically, so the move needs no wiring; the file
takes the same shape as meta/rbac.yml. Every config path follows, the variant
overrides are lifted out of their services map, and the vocabulary module leaves
the services package because it no longer describes one. roles_with_service
gains an explicit topic parameter rather than an implicit fallback, so nothing
silently resolves against the wrong container.

Files that are otherwise unrelated to this move are included where the committed
tree would otherwise keep pointing at a path that no longer resolves: thirteen
vars files, eleven playwright env templates and one task file.
2026-08-06 20:36:00 +02:00
kevinveenbirkenbach 7c1d733a85 feat(mcp): add the service vocabulary, discovery and rendering layer
Defines what a services.mcp block may declare and enforces it: classification,
transport, auth, implementation and the provenance fields that make an upstream
claim checkable. The lint refuses an unknown key anywhere in the block and
refuses a provider whose allowed consumers cannot speak its transport or its
auth scheme.

Discovery resolves each provider's credential and endpoint per consumer and
per deployment, rejecting a server with a named reason. Transport, auth and
endpoint faults are fatal; a missing credential is not, because it also occurs
when the provider is simply not provisioned yet in the same play.

The filters render the Authorization header from the declared scheme, build
Open WebUI's tool-server connections with their group grants, and derive the
members of those groups from the RBAC assignments. Role scoping gains
application-scoped roles, so an mcp role only counts when it is granted on a
specific application.

roles_with_service gains a scope and a direction filter, and drops the canonical
domain requirement for direction-filtered entries: an MCP URL is built from the
service key and the internal port, so a provider without a public vhost stays
valid.
2026-08-06 14:13:26 +02:00
kevinveenbirkenbachandClaude Opus 5 19eaec2b6d fix(mcp): declare the subject each server actually authenticates as
Four roles declared auth_subject: user while handing the client an
account-bound credential. Nextcloud and Home Assistant mint against the
administrator account, Mattermost issues the administrator's personal access
token, and Baserow creates a dedicated mcp@<domain> account with its own
workspace. The declarations now name what the minting tasks do; Baserow's
README additionally claimed the administrator owns its endpoint, which it
never did.

The tools declarations follow one convention instead of two: the field records
the deployment's intent and the README names the missing enforcement, which is
what GitLab already did and what Baserow and Mattermost now do.

The discovery lookup carries auth_subject through so clients can see the
subject. No consumer branches on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-03 22:08:49 +02:00
kevinveenbirkenbachandClaude Opus 5 5583a2cf74 feat(mcp): express blocked surfaces and path-addressed endpoints
Add a blocker vocabulary so a role whose upstream MCP path this
deployment cannot use says why instead of reporting an absence. Without
it OpenProject reads as "upstream has no MCP", which is false: the server
ships, it is Enterprise-only. A blocker and a served surface are mutually
exclusive, so a role cannot claim both.

Add endpoint.key_credential and endpoint.suffix for servers that address
a session through the URL path. Baserow streams from
{base}/mcp/{key}/sse, and endpoint fields must stay literal, so the
composition belongs in the discovery layer. A server that declares a path
key without a resolvable value is dropped rather than handed to clients
half-built.

Carry the upstream provenance in the role block, so the audit derives it
instead of keeping a second copy.

Require an explicit mutating_tools_enabled: false for a privileged
auth_subject; the rule previously only fired when the key was present and
true, so omitting it slipped an administrator-scoped surface through.

Move the MCP filters into plugins/filter/mcp/ so they stop sharing a name
prefix with their siblings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 21:58:01 +02:00
kevinveenbirkenbachandClaude Opus 5 6881ac5132 feat(mcp): add basic_auth to the service contract and a discovery lookup
Add basic_auth to the MCP auth schema so Jenkins can declare its
HTTP-Basic endpoint through the contract instead of around it.

Introduce the client-side renderers: mcp_authorization builds the
Authorization header per scheme, mcp_tool_server_connections builds the
Open WebUI tool-server entries and drops servers whose scheme it cannot
present, and mcp_renderable_servers filters a server list, since Jinja
select() needs a test and the repo ships no test plugins.

Replace the inline MCP_DISCOVERED_SERVERS Jinja with a mcp_servers
lookup, and carry auth and username per entry so basic_auth consumers can
build their credential.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 00:29:42 +02:00