mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-06 02:54:26 +00:00
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:
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 %}
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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') }}"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
@@ -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",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
Reference in new issue
Block a user