mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-06 10:59:59 +00:00
feat(proxy): serve an MCP path only where it is meant to be public
Baserow's image mounts the MCP app on the ASGI root unconditionally, so
mcp.enabled only decides whether an endpoint record exists. The
application's own vhost kept routing /mcp/*: switched off it answered
401 Endpoint not found., and a refused credential is still a surface.
Switched on it is worse - the adapter narrows fourteen upstream tools to
four, and the publicly routed native path reaches all fourteen for
anyone holding the endpoint key. web-app-homeassistant had the same
shape, an exposure: internal surface reachable publicly. Both now 404
there and stay reachable on the container network.
The public surfaces get the edge limits the contract already declared.
Zones are written per role by the role that owns them, in the shape
web-app-prometheus uses, because one round-scoped file would drop another
application's zones on the next deploy. The rate is concurrent_requests
per second rather than an eighth limit key: a client allowed N in flight,
each bounded by timeout_seconds, cannot sustain more unless every call
returns instantly. A refusal is 429, not nginx's 503, because a client
that should back off must not read it as an outage. stream_seconds
finally binds something - the adapter holds no stream, the proxy does.
The shared location snippet's three timeouts become parameters defaulting
to the 900s they hard-coded, so no existing vhost changes.
Found by the disabled-state assertion 7d8e7e87ff added, which had never
run: its helper was never staged, so the two defects hid each other.
This commit is contained in:
1 parent
85af82b956
commit
f5272fa85f
15 files changed
+267
-3
No files matched your search
@@ -35,9 +35,9 @@ location {{location}}
|
||||
proxy_set_header Connection $connection_upgrade;
|
||||
|
||||
proxy_connect_timeout 5s;
|
||||
proxy_send_timeout 900s;
|
||||
proxy_read_timeout 900s;
|
||||
send_timeout 900s;
|
||||
proxy_send_timeout {{ webserver_proxy_send_timeout | default('900s') }};
|
||||
proxy_read_timeout {{ webserver_proxy_read_timeout | default('900s') }};
|
||||
send_timeout {{ webserver_send_timeout | default('900s') }};
|
||||
|
||||
{% set proxy_lua_enabled = proxy_lua_enabled | default(true) | bool %}
|
||||
proxy_buffering {{ 'on' if proxy_lua_enabled else 'off' }};
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
limit_req zone={{ mcp_zone }}_req burst={{ mcp_limits.concurrent_requests }} nodelay;
|
||||
limit_conn {{ mcp_zone }}_conn {{ mcp_limits.concurrent_requests }};
|
||||
limit_req_status 429;
|
||||
limit_conn_status 429;
|
||||
@@ -0,0 +1,23 @@
|
||||
{% set mcp_path = lookup('config', application_id, 'mcp.endpoint.path', '') %}
|
||||
{% if mcp_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 %}
|
||||
{% 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 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('/+$', '') }} {
|
||||
return 404;
|
||||
}
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
@@ -0,0 +1,4 @@
|
||||
{% set mcp_limits = lookup('config', application_id, 'mcp.limits') %}
|
||||
{% set mcp_zone = 'mcp_' ~ (application_id | replace('-', '_')) %}
|
||||
limit_req_zone $binary_remote_addr zone={{ mcp_zone }}_req:10m rate={{ mcp_limits.concurrent_requests }}r/s;
|
||||
limit_conn_zone $binary_remote_addr zone={{ mcp_zone }}_conn:10m;
|
||||
@@ -56,4 +56,6 @@ server
|
||||
|
||||
{% include 'roles/web-app-prometheus/templates/nginx/locations.conf.j2' %}
|
||||
|
||||
{% include 'roles/sys-svc-proxy/templates/mcp/vhost.conf.j2' %}
|
||||
|
||||
}
|
||||
@@ -25,4 +25,6 @@ server {
|
||||
|
||||
error_page 500 501 502 503 504 /500.html;
|
||||
{% include 'roles/web-app-prometheus/templates/nginx/locations.conf.j2' %}
|
||||
|
||||
{% include 'roles/sys-svc-proxy/templates/mcp/vhost.conf.j2' %}
|
||||
}
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
- name: "🚦 Declare the MCP rate and connection zones of {{ application_id }}"
|
||||
stack_host_template:
|
||||
src: "roles/sys-svc-proxy/templates/mcp/zones.conf.j2"
|
||||
dest: "{{ [lookup('nginx', 'directories.configuration.global'), 'mcp-' ~ application_id ~ '.conf'] | path_join }}"
|
||||
notify: restart-openresty
|
||||
@@ -4,6 +4,10 @@
|
||||
- name: "🔑 Realign the persisted database credentials before the health gate"
|
||||
include_tasks: utils/patch_db_credentials.yml
|
||||
|
||||
- name: "🚦 Declare the MCP edge limits before the vhost references them"
|
||||
include_tasks: "{{ lookup('path_absolute', 'roles/sys-svc-webserver-core/tasks/utils/mcp_edge_limits.yml') }}"
|
||||
when: MOODLE_MCP_ENABLED | bool
|
||||
|
||||
- name: "🧩 load docker, db and proxy for {{ application_id }}"
|
||||
include_role:
|
||||
name: sys-stk-full
|
||||
|
||||
@@ -4,6 +4,10 @@
|
||||
- name: "🔑 Realign the persisted database credentials before the health gate"
|
||||
include_tasks: utils/patch_db_credentials.yml
|
||||
|
||||
- name: "🚦 Declare the MCP edge limits before the vhost references them"
|
||||
include_tasks: "{{ lookup('path_absolute', 'roles/sys-svc-webserver-core/tasks/utils/mcp_edge_limits.yml') }}"
|
||||
when: NEXTCLOUD_MCP_ENABLED | bool
|
||||
|
||||
- name: 🔧 Setup the full docker stack
|
||||
include_tasks: 01_fullstack.yml
|
||||
vars:
|
||||
|
||||
@@ -41,4 +41,6 @@ server
|
||||
|
||||
try_files $uri $uri/ =404;
|
||||
}
|
||||
|
||||
{% include 'roles/sys-svc-proxy/templates/mcp/vhost.conf.j2' %}
|
||||
}
|
||||
@@ -1,6 +1,10 @@
|
||||
---
|
||||
- include_tasks: utils/once/flag.yml
|
||||
|
||||
- name: "🚦 Declare the MCP edge limits before the vhost references them"
|
||||
include_tasks: "{{ lookup('path_absolute', 'roles/sys-svc-webserver-core/tasks/utils/mcp_edge_limits.yml') }}"
|
||||
when: WORDPRESS_MCP_ENABLED | bool
|
||||
|
||||
- name: "📂 Include role sys-stk-front-proxy for {{ application_id }}"
|
||||
include_role:
|
||||
name: sys-stk-front-proxy
|
||||
|
||||
@@ -20,6 +20,7 @@ WORDPRESS_ADDONS_REQUIRED: "{{ lookup('config', application_id, 'addons') | dict
|
||||
map(attribute='key') | list }}"
|
||||
WORDPRESS_USER: "www-data"
|
||||
WORDPRESS_LITELLM_ENV_ID: "{{ lookup('config', application_id, 'addons.ai-engine.config.environment.id') }}"
|
||||
WORDPRESS_MCP_ENABLED: "{{ lookup('config', application_id, 'mcp.enabled') }}"
|
||||
WORDPRESS_MCP_ADAPTER_VERSION: "{{ lookup('config', application_id, 'addons.mcp-adapter.version') }}"
|
||||
WORDPRESS_MCP_ACCOUNT: "{{ lookup('users', 'mcp-web-app-wordpress') }}"
|
||||
WORDPRESS_MCP_USERNAME: "{{ WORDPRESS_MCP_ACCOUNT.username }}"
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
from pathlib import Path
|
||||
|
||||
PROJECT_ROOT: Path = Path(__file__).resolve().parents[5]
|
||||
@@ -0,0 +1,3 @@
|
||||
from pathlib import Path
|
||||
|
||||
PROJECT_ROOT: Path = Path(__file__).resolve().parents[6]
|
||||
@@ -0,0 +1,202 @@
|
||||
"""The edge limits a public MCP surface is served behind.
|
||||
|
||||
Every role declares seven `mcp.limits`, and the adapter enforces five of them
|
||||
on its own request path. Two only ever bite at the reverse proxy:
|
||||
`concurrent_requests`, which no adapter sees until a connection already
|
||||
occupies a thread, and `stream_seconds`, which bounds a held stream the
|
||||
adapter never holds. These templates are where the declaration becomes an
|
||||
nginx directive, so the values are pinned here rather than merely rendered.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
|
||||
from jinja2 import Environment, FileSystemLoader, StrictUndefined
|
||||
|
||||
from utils.cache.yaml import load_yaml
|
||||
from utils.roles.mapping import ROLE_FILE_META_MCP
|
||||
|
||||
from . import PROJECT_ROOT
|
||||
|
||||
TEMPLATE_DIR = PROJECT_ROOT / "roles/sys-svc-proxy/templates/mcp"
|
||||
|
||||
PUBLIC_ROLES = ("web-app-moodle", "web-app-nextcloud", "web-app-wordpress")
|
||||
|
||||
|
||||
def declared_limits(role: str) -> dict:
|
||||
"""Return a role's own `mcp.limits`, as its meta declares them.
|
||||
|
||||
Args:
|
||||
role: the role directory name.
|
||||
"""
|
||||
return load_yaml(PROJECT_ROOT / "roles" / role / ROLE_FILE_META_MCP)["limits"]
|
||||
|
||||
|
||||
def render(name: str, **context) -> str:
|
||||
"""Return one template rendered with a stubbed `config` lookup.
|
||||
|
||||
Args:
|
||||
name: template file name inside the MCP template directory.
|
||||
context: variables the template reads.
|
||||
"""
|
||||
env = Environment(
|
||||
loader=FileSystemLoader(str(TEMPLATE_DIR)),
|
||||
undefined=StrictUndefined,
|
||||
autoescape=False, # noqa: S701 - nginx config, not markup; Ansible renders it the same way and HTML-escaping `$binary_remote_addr` would break it
|
||||
)
|
||||
return env.get_template(name).render(**context)
|
||||
|
||||
|
||||
class TestZones(unittest.TestCase):
|
||||
def test_the_rate_follows_the_declared_concurrency(self):
|
||||
limits = declared_limits("web-app-moodle")
|
||||
rendered = render(
|
||||
"zones.conf.j2",
|
||||
application_id="web-app-moodle",
|
||||
lookup=lambda *_a, **_k: limits,
|
||||
)
|
||||
self.assertIn(
|
||||
f"rate={limits['concurrent_requests']}r/s",
|
||||
rendered,
|
||||
)
|
||||
|
||||
def test_each_role_gets_a_zone_name_of_its_own(self):
|
||||
rendered = render(
|
||||
"zones.conf.j2",
|
||||
application_id="web-app-moodle",
|
||||
lookup=lambda *_a, **_k: declared_limits("web-app-moodle"),
|
||||
)
|
||||
self.assertIn("zone=mcp_web_app_moodle_req:10m", rendered)
|
||||
self.assertIn("zone=mcp_web_app_moodle_conn:10m", rendered)
|
||||
|
||||
def test_the_two_zone_kinds_never_share_a_name(self):
|
||||
"""nginx refuses a `limit_conn_zone` reusing a `limit_req_zone` name."""
|
||||
rendered = render(
|
||||
"zones.conf.j2",
|
||||
application_id="web-app-nextcloud",
|
||||
lookup=lambda *_a, **_k: declared_limits("web-app-nextcloud"),
|
||||
)
|
||||
names = [
|
||||
part.split(":", 1)[0]
|
||||
for line in rendered.splitlines()
|
||||
for part in line.split()
|
||||
if part.startswith("zone=")
|
||||
]
|
||||
self.assertEqual(len(names), len(set(names)))
|
||||
|
||||
def test_every_public_role_renders_a_zone_pair(self):
|
||||
for role in PUBLIC_ROLES:
|
||||
with self.subTest(role=role):
|
||||
rendered = render(
|
||||
"zones.conf.j2",
|
||||
application_id=role,
|
||||
lookup=lambda *_a, _r=role, **_k: declared_limits(_r),
|
||||
)
|
||||
self.assertIn("limit_req_zone", rendered)
|
||||
self.assertIn("limit_conn_zone", rendered)
|
||||
|
||||
|
||||
class TestDirectives(unittest.TestCase):
|
||||
def context(self, role: str) -> dict:
|
||||
return {
|
||||
"mcp_zone": "mcp_" + role.replace("-", "_"),
|
||||
"mcp_limits": declared_limits(role),
|
||||
}
|
||||
|
||||
def test_the_connection_ceiling_is_the_declared_concurrency(self):
|
||||
limits = declared_limits("web-app-nextcloud")
|
||||
rendered = render("directives.conf.j2", **self.context("web-app-nextcloud"))
|
||||
self.assertIn(
|
||||
f"limit_conn mcp_web_app_nextcloud_conn {limits['concurrent_requests']};",
|
||||
rendered,
|
||||
)
|
||||
|
||||
def test_the_burst_matches_the_ceiling_so_a_legal_client_is_never_delayed(self):
|
||||
limits = declared_limits("web-app-moodle")
|
||||
rendered = render("directives.conf.j2", **self.context("web-app-moodle"))
|
||||
self.assertIn(
|
||||
f"burst={limits['concurrent_requests']} nodelay;",
|
||||
rendered,
|
||||
)
|
||||
|
||||
def test_a_refusal_is_429_rather_than_the_nginx_default_503(self):
|
||||
"""503 reads as an outage; a client that backs off needs 429."""
|
||||
rendered = render("directives.conf.j2", **self.context("web-app-moodle"))
|
||||
self.assertIn("limit_req_status 429;", rendered)
|
||||
self.assertIn("limit_conn_status 429;", rendered)
|
||||
|
||||
def test_the_directives_reference_the_zones_the_zone_file_declares(self):
|
||||
for role in PUBLIC_ROLES:
|
||||
with self.subTest(role=role):
|
||||
zone = "mcp_" + role.replace("-", "_")
|
||||
zones = render(
|
||||
"zones.conf.j2",
|
||||
application_id=role,
|
||||
lookup=lambda *_a, _r=role, **_k: declared_limits(_r),
|
||||
)
|
||||
directives = render("directives.conf.j2", **self.context(role))
|
||||
self.assertIn(f"zone={zone}_req:", zones)
|
||||
self.assertIn(f"zone={zone}_req ", directives)
|
||||
self.assertIn(f"zone={zone}_conn:", zones)
|
||||
self.assertIn(f"{zone}_conn ", directives)
|
||||
|
||||
|
||||
class TestPublicSurfacesAreCovered(unittest.TestCase):
|
||||
def test_the_pinned_roles_are_exactly_the_public_ones(self):
|
||||
"""A new public surface must not reach the edge without these limits."""
|
||||
found = set()
|
||||
for meta in (PROJECT_ROOT / "roles").glob(f"*/{ROLE_FILE_META_MCP}"):
|
||||
if load_yaml(meta).get("exposure") == "public":
|
||||
found.add(meta.parent.parent.name)
|
||||
self.assertEqual(found, set(PUBLIC_ROLES))
|
||||
|
||||
|
||||
def endpoint_roles() -> dict[str, dict]:
|
||||
"""Return every role declaring an MCP endpoint path, by role name."""
|
||||
found = {}
|
||||
for meta in (PROJECT_ROOT / "roles").glob(f"*/{ROLE_FILE_META_MCP}"):
|
||||
block = load_yaml(meta)
|
||||
if (block.get("endpoint") or {}).get("path"):
|
||||
found[meta.parent.parent.name] = block
|
||||
return found
|
||||
|
||||
|
||||
class TestTheAppVhostServesOnlyPublicSurfaces(unittest.TestCase):
|
||||
"""`vhost.conf.j2` serves the MCP path, or returns 404 for it.
|
||||
|
||||
A surface reachable on the application's own public vhost while the
|
||||
service is off, or while its declared exposure is internal, is a surface
|
||||
nobody decided to publish. For an adapter-fronted provider it is worse
|
||||
than untidy: the adapter exists to narrow the upstream's tool set, and a
|
||||
publicly routed native path walks straight past it.
|
||||
"""
|
||||
|
||||
def served(self, block: dict) -> bool:
|
||||
return bool(block.get("enabled")) and block.get("exposure") == "public"
|
||||
|
||||
def test_an_internal_surface_is_not_served_on_the_public_vhost(self):
|
||||
for role, block in endpoint_roles().items():
|
||||
if block.get("exposure") != "public":
|
||||
with self.subTest(role=role):
|
||||
self.assertFalse(self.served(block))
|
||||
|
||||
def test_the_adapter_fronted_provider_hides_its_native_path(self):
|
||||
"""Reaching baserow's own endpoint would bypass the four-tool contract."""
|
||||
block = endpoint_roles()["web-app-baserow"]
|
||||
self.assertEqual("adapter_server", block["classification"])
|
||||
self.assertFalse(self.served(block))
|
||||
|
||||
def test_a_disabled_surface_is_never_served(self):
|
||||
for role, block in endpoint_roles().items():
|
||||
if not block.get("enabled"):
|
||||
with self.subTest(role=role):
|
||||
self.assertFalse(self.served(block))
|
||||
|
||||
def test_the_public_roles_are_the_only_ones_served(self):
|
||||
served = {r for r, b in endpoint_roles().items() if self.served(b)}
|
||||
self.assertEqual(served, {r for r in PUBLIC_ROLES if r != "web-app-wordpress"})
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user