fix(mcp): hide the path the application serves, not the sidecar's

f5272fa85f derived the 404 from mcp.endpoint.path, which for an
adapter-fronted provider names the sidecar. Mattermost declares /mcp
there and answers on /plugins/mattermost-ai/mcp-server/mcp, so the proxy
blocked a path the application never served while the native surface
stayed reachable. Baserow passed only because its two paths coincide -
one role agreeing proved nothing about the other four.

That surface is what the adapter exists to narrow: it serves four tools
out of fourteen, and the native path reaches all fourteen.

adapter.upstream_path joins upstream_network and upstream_transport in
meta, and the four roles whose upstream lives on their own vhost declare
it there. Gitea does not: its upstream is a separate sidecar on its own
network, so it has nothing to hide on the application's vhost, and the
absence says that.

The roles read the value back through lookup('config', ...) rather than
keeping a second copy, so the path is stated once.

The template now separates the two roles the old variable conflated:
endpoint.path is what gets served, and only where the surface is public
and enabled; every declared path that differs from it gets a 404. Three
tests pin the distinction rather than the outcome - mattermost's two
paths must differ, gitea must declare none, and a passthrough provider
must say where its upstream lives.
This commit is contained in:
kevinveenbirkenbach committed 2026-08-29 17:47:03 +02:00
1 parent 57ff37693c
commit 06c13e7f3e
11 files changed
+74 -11

No files matched your search

@@ -1,23 +1,30 @@
{% set mcp_path = lookup('config', application_id, 'mcp.endpoint.path', '') %}
{% if mcp_path %}
{% set mcp_endpoint_path = lookup('config', application_id, 'mcp.endpoint.path', '') %}
{% set mcp_upstream_path = lookup('config', application_id, 'mcp.adapter.upstream_path', '') %}
{% if mcp_endpoint_path or mcp_upstream_path %}
{% set mcp_public = lookup('config', application_id, 'mcp.exposure', '') == 'public'
and lookup('config', application_id, 'mcp.enabled', false) | bool %}
{% set mcp_acl_gated = lookup('sso', application_id, 'is_proxy_gated')
and (lookup('sso', application_id, 'oauth2_acl') | length) %}
{% if mcp_public and not mcp_acl_gated %}
{% if not mcp_acl_gated %}
{% set mcp_served = (mcp_endpoint_path | regex_replace('/+$', '')) if mcp_public else '' %}
{% if mcp_served %}
{% set mcp_limits = lookup('config', application_id, 'mcp.limits') %}
{% set mcp_zone = 'mcp_' ~ (application_id | replace('-', '_')) %}
{% set sso_proxy_enabled = lookup('sso', application_id, 'is_proxy_gated') %}
{% set location = mcp_path | regex_replace('/+$', '') %}
{% set location = mcp_served %}
{% set webserver_client_max_body_size = mcp_limits.request_bytes %}
{% set webserver_proxy_send_timeout = mcp_limits.timeout_seconds ~ 's' %}
{% set webserver_proxy_read_timeout = mcp_limits.stream_seconds ~ 's' %}
{% set webserver_send_timeout = mcp_limits.stream_seconds ~ 's' %}
{% set webserver_location_extra_configuration %}{% include 'roles/sys-svc-proxy/templates/mcp/directives.conf.j2' %}{% endset %}
{% include 'roles/sys-svc-proxy/templates/location/html.conf.j2' %}
{% elif not mcp_acl_gated %}
location {{ mcp_path | regex_replace('/+$', '') }} {
{% endif %}
{% for hidden in [mcp_upstream_path, mcp_endpoint_path] | map('regex_replace', '/+$', '') | unique | select | list %}
{% if hidden != mcp_served %}
location {{ hidden }} {
return 404;
}
{% endif %}
{% endfor %}
{% endif %}
{% endif %}
+1
View File
@@ -24,6 +24,7 @@ adapter:
upstream_transport: sse
specification_path: roles/web-app-baserow/files/mcp/tools.json
specification_sha256: sha256:716effccfedbb6e83cacd489bf0c2cf8f318853f9b20bf8fd733db4e637ec144
upstream_path: /mcp
endpoint:
service_key: baserowmcp
path: /mcp
+1 -1
View File
@@ -35,7 +35,7 @@ BASEROW_INTERNAL_URL: "http://{{ BASEROW_CONTAINER }}:{{ lookup('config', applic
BASEROW_MCP_ENABLED: "{{ lookup('config', application_id, 'mcp.enabled') }}"
BASEROW_MCP_ENDPOINT_KEY: "{{ lookup('config', application_id, 'secrets.credentials.mcp_endpoint_key') }}"
BASEROW_MCP_UPSTREAM_PATH: "/mcp"
BASEROW_MCP_UPSTREAM_PATH: "{{ lookup('config', application_id, 'mcp.adapter.upstream_path') }}"
BASEROW_MCP_ENDPOINT_PATH: "{{ lookup('config', application_id, 'mcp.endpoint.path') }}"
BASEROW_MCP_ENDPOINT_NAME: "Infinito.Nexus"
BASEROW_MCP_WORKSPACE_NAME: "Infinito.Nexus MCP"
+1
View File
@@ -23,6 +23,7 @@ adapter:
type: mcp_passthrough
specification_path: roles/web-app-gitlab/files/mcp/tools.json
specification_sha256: sha256:ae5ffe0f6c8eeffcc4fe7da8ad1cc978c5f49337a64e30a060b6ab7a5dfb4b24
upstream_path: /api/v4/mcp
endpoint:
service_key: gitlabmcp
path: /mcp
+1 -1
View File
@@ -84,7 +84,7 @@ GITLAB_MCP_CONTAINER: "{{ lookup('config', application_id, 'services.gitlabmcp.n
GITLAB_MCP_PORT: "{{ lookup('config', application_id, 'services.gitlabmcp.ports.internal.http') }}"
GITLAB_MCP_INSTANCE_DIR: "{{ [lookup('container', application_id, 'directories.instance'), 'mcp'] | path_join }}"
GITLAB_MCP_UPSTREAM_HOST: "{{ GITLAB_WORKHORSE_SERVICE_NAME }}"
GITLAB_MCP_UPSTREAM_PATH: "/api/v4/mcp"
GITLAB_MCP_UPSTREAM_PATH: "{{ lookup('config', application_id, 'mcp.adapter.upstream_path') }}"
GITLAB_MCP_UPSTREAM_URL: "http://{{ GITLAB_MCP_UPSTREAM_HOST }}:{{ GITLAB_WORKHORSE_PORT }}{{ GITLAB_MCP_UPSTREAM_PATH }}"
GITLAB_MCP_BEARER: "{{ lookup('config', application_id, 'secrets.credentials.mcp_bearer') }}"
GITLAB_MCP_CONTRACT:
+1
View File
@@ -21,6 +21,7 @@ adapter:
type: mcp_passthrough
specification_path: roles/web-app-jenkins/files/mcp/tools.json
specification_sha256: sha256:05d96a67cd799cdf2e74caaf4560c9282462ead555a6159fd2e0c74a705b6cf5
upstream_path: /mcp-server/mcp
endpoint:
service_key: jenkinsmcp
path: /mcp
+1 -1
View File
@@ -20,7 +20,7 @@ JENKINS_MCP_TOKEN_NAME: "infinito-mcp"
JENKINS_MCP_TOKEN_FILE: "/var/jenkins_home/secrets/infinito-mcp.token"
JENKINS_MCP_IDENTITY_URL: "{{ JENKINS_LOOPBACK_URL }}/whoAmI/api/json"
JENKINS_MCP_READY_URL: "{{ JENKINS_LOOPBACK_URL }}/login"
JENKINS_MCP_PLUGIN_PATH: "/mcp-server/mcp"
JENKINS_MCP_PLUGIN_PATH: "{{ lookup('config', application_id, 'mcp.adapter.upstream_path') }}"
JENKINS_MCP_URL: "{{ JENKINS_LOOPBACK_URL }}{{ JENKINS_MCP_PLUGIN_PATH }}"
JENKINS_MCP_CONTAINER: "{{ lookup('config', application_id, 'services.jenkinsmcp.name') }}"
JENKINS_MCP_PORT: "{{ lookup('config', application_id, 'services.jenkinsmcp.ports.internal.http') }}"
+1
View File
@@ -22,6 +22,7 @@ adapter:
type: mcp_passthrough
specification_path: roles/web-app-mattermost/files/mcp/tools.json
specification_sha256: sha256:b06c02b100fa8d959d3e4d9fca42682df943967f6bf2de2c66f7031d5a6b823f
upstream_path: /plugins/mattermost-ai/mcp-server/mcp
endpoint:
service_key: mattermostmcp
path: /mcp
+1 -1
View File
@@ -58,7 +58,7 @@ MATTERMOST_LDAP_CONTAINER_ADDRESS: "{{ lookup('container_address', 'svc-db-openl
MATTERMOST_MCP_CONTAINER: "{{ lookup('config', application_id, 'services.mattermostmcp.name') }}"
MATTERMOST_MCP_PORT: "{{ lookup('config', application_id, 'services.mattermostmcp.ports.internal.http') }}"
MATTERMOST_MCP_INSTANCE_DIR: "{{ [lookup('container', application_id, 'directories.instance'), 'mcp'] | path_join }}"
MATTERMOST_MCP_UPSTREAM_PATH: "/plugins/{{ MATTERMOST_MCP_PLUGIN_ID }}/mcp-server/mcp"
MATTERMOST_MCP_UPSTREAM_PATH: "{{ lookup('config', application_id, 'mcp.adapter.upstream_path') }}"
MATTERMOST_MCP_UPSTREAM_URL: "http://{{ MATTERMOST_CONTAINER }}:{{ container_port }}{{ MATTERMOST_MCP_UPSTREAM_PATH }}"
MATTERMOST_MCP_BEARER: "{{ lookup('config', application_id, 'secrets.credentials.mcp_bearer') }}"
MATTERMOST_MCP_CONTRACT:
@@ -198,5 +198,52 @@ class TestTheAppVhostServesOnlyPublicSurfaces(unittest.TestCase):
self.assertEqual(served, {r for r in PUBLIC_ROLES if r != "web-app-wordpress"})
class TestTheHiddenPathIsTheOneTheAppServes(unittest.TestCase):
"""What must be hidden is the upstream, not the sidecar's endpoint.
An adapter-fronted provider declares `endpoint.path` on its sidecar and
serves the real MCP surface somewhere else on its own vhost. Blocking the
sidecar's path there hides nothing: mattermost declares `/mcp` and answers
on `/plugins/mattermost-ai/mcp-server/mcp`, so the native surface stayed
reachable while the deploy looked correct. Baserow's two paths happen to
coincide, which is why one role passing proved nothing about the rest.
"""
def passthrough_roles(self) -> dict[str, dict]:
found = {}
for meta in (PROJECT_ROOT / "roles").glob(f"*/{ROLE_FILE_META_MCP}"):
block = load_yaml(meta)
if (block.get("adapter") or {}).get("type") == "mcp_passthrough":
found[meta.parent.parent.name] = block
return found
def test_a_passthrough_provider_says_where_its_upstream_lives(self):
"""Either on the application's own vhost, or on a separate network."""
for role, block in self.passthrough_roles().items():
with self.subTest(role=role):
adapter = block["adapter"]
self.assertTrue(
adapter.get("upstream_path") or adapter.get("upstream_network"),
f"{role} declares neither upstream_path nor upstream_network, "
f"so nothing states whether its native surface is exposed",
)
def test_the_role_that_made_this_visible_declares_its_real_path(self):
adapter = self.passthrough_roles()["web-app-mattermost"]["adapter"]
self.assertEqual(
"/plugins/mattermost-ai/mcp-server/mcp", adapter["upstream_path"]
)
self.assertNotEqual(
adapter["upstream_path"],
self.passthrough_roles()["web-app-mattermost"]["endpoint"]["path"],
)
def test_a_sidecar_upstream_declares_no_path_on_the_app_vhost(self):
"""Gitea's upstream is its own service, so it has nothing to hide."""
adapter = self.passthrough_roles()["web-app-gitea"]["adapter"]
self.assertNotIn("upstream_path", adapter)
self.assertTrue(adapter.get("upstream_network"))
if __name__ == "__main__":
unittest.main()
+6 -1
View File
@@ -29,7 +29,11 @@ Field vocabulary (see ``docs/contributing/design/role/services/mcp.md``):
``category_allowlist`` suits an upstream that switches whole categories and
offers no per-tool filter, where a tool allowlist would be unenforceable;
``mcp_passthrough`` fronts an upstream that already speaks MCP instead of
translating REST.
translating REST. ``upstream_path`` names that upstream where it lives on the
application's own vhost, so the proxy can hide it: reaching it directly walks
past the allowlist the adapter exists to enforce. A provider whose upstream is
a separate sidecar declares ``upstream_network`` instead and has nothing to
hide there.
* ``limits``: the request, response, timeout, concurrency, pagination
and stream ceilings the surface enforces.
* ``tools``: ``allowlist`` is what a reader may reach and
@@ -198,6 +202,7 @@ MCP_ADAPTER_KEYS = frozenset(
"specification_sha256",
"scope",
"upstream_transport",
"upstream_path",
}
)