From cb88559567c0f427c583266d9966f57ecb039e9c Mon Sep 17 00:00:00 2001 From: Matt Coles Date: Fri, 17 Jul 2026 12:51:00 +1000 Subject: [PATCH] feat: expose companion skill as MCP resources --- README.md | 21 +++++++------- openspec/specs/anchors/access-control.yml | 14 ++------- openspec/specs/anchors/auth.yml | 2 +- src/acme_mcp/access.py | 22 +++++++++----- src/acme_mcp/server.py | 10 +++++-- .../skills}/handle-downloads/SKILL.md | 1 + tests/test_skills.py | 29 +++++++++++++++++++ 7 files changed, 67 insertions(+), 32 deletions(-) rename {skills => src/acme_mcp/skills}/handle-downloads/SKILL.md (98%) create mode 100644 tests/test_skills.py diff --git a/README.md b/README.md index 9828a7a..f0c1099 100644 --- a/README.md +++ b/README.md @@ -13,13 +13,13 @@ can see how the pieces fit instead of stitching them together yourself. | Concern | Where | What it shows | | --- | --- | --- | | Auth | `src/acme_mcp/auth.py` | `JWTVerifier` in prod, `StaticTokenVerifier` for local dev; the group → tag map | -| Per-group access | `src/acme_mcp/access.py` | `GroupTagFilter` middleware: hides tools **and** blocks calls to hidden ones | +| Per-group access | `src/acme_mcp/access.py` | FastMCP `AuthMiddleware`: hides components **and** blocks direct use | | Audit trail | `src/acme_mcp/audit.py` | `AuditLog` middleware logging user / groups / tool / timing on every call | | Deterministic domains | `src/acme_mcp/domains/{orders,billing,admin}.py` | plain typed tools over a data backend | | Agent behind a tool | `src/acme_mcp/domains/support.py` + `agents.py` | an injectable, mockable inner agent on a tight leash | | File delivery | `src/acme_mcp/domains/reports.py` + `storage.py` | upload to S3, return a short-lived signed URL — never the bytes | | Composition | `src/acme_mcp/server.py` | mount in-process domains; proxy a separately-owned one | -| Companion skill | `skills/handle-downloads/SKILL.md` | how an agent should treat a returned `download_url` | +| Companion skill | `src/acme_mcp/skills/handle-downloads/SKILL.md` | a reports-scoped skill published by the MCP server | ## Install @@ -61,23 +61,24 @@ caller sees orders/billing/support/reports tools; `finance` sees billing/reports Every request is authenticated, then two middleware run: 1. `AuditLog` records who called what. -2. `GroupTagFilter` enforces access in **two** places — it hides tools the caller - isn't cleared for (`on_list_tools`) *and* blocks calls to them - (`on_call_tool`), so a guessed tool name still fails. Filtering only the list - would leave a named tool callable; filtering the call is what shuts the door. +2. FastMCP's `AuthMiddleware` hides components the caller isn't cleared for and + blocks direct use, so guessing a hidden tool or resource still fails. Tools are tagged by domain (`orders`, `billing`, `admin`, `support`, `reports`); `GROUP_TAGS` in `auth.py` maps each org group to the tags it may use. The identity tool `whoami` is tagged `public` so any authenticated caller can see it. +The `handle-downloads` skill is tagged `reports`, so the same groups that can +export a report can discover and read its handling instructions through +`skill://handle-downloads/SKILL.md`. The `admin` group is cleared for the wildcard tag (`ALL_TAGS`) rather than an explicit domain list, so it stays a true superset — including a later-composed domain such as the proxied `analytics` service — without anyone having to edit its tag list each time a domain is added. -> **Note on the post's `Transform`:** the draft post sketches this filter as a -> `Transform`/`add_transform`. This repo implements it as FastMCP **middleware** -> (`on_list_tools` + `on_call_tool`), which is the supported per-request -> mechanism in FastMCP 3 and delivers the same two-layer guarantee. +The companion skill lives in the server package and is exposed with FastMCP's +`SkillProvider`. The client reads it as an MCP resource when it needs the +instructions; the server remains responsible for authenticating and authorizing +that read. ## Test diff --git a/openspec/specs/anchors/access-control.yml b/openspec/specs/anchors/access-control.yml index ec679cc..35399c9 100644 --- a/openspec/specs/anchors/access-control.yml +++ b/openspec/specs/anchors/access-control.yml @@ -3,25 +3,17 @@ access.list-filter: rule: kind: function_definition - has: { field: name, regex: '^on_list_tools$' } - inside: - kind: class_definition - has: { field: name, regex: '^GroupTagFilter$' } - stopBy: end + has: { field: name, regex: '^build_access_middleware$' } files: [src/**/*.py] access.call-block: rule: kind: function_definition - has: { field: name, regex: '^on_call_tool$' } - inside: - kind: class_definition - has: { field: name, regex: '^GroupTagFilter$' } - stopBy: end + has: { field: name, regex: '^build_access_middleware$' } files: [src/**/*.py] access.cleared-for: rule: kind: function_definition - has: { field: name, regex: '^cleared_for$' } + has: { field: name, regex: '^group_access$' } files: [src/**/*.py] diff --git a/openspec/specs/anchors/auth.yml b/openspec/specs/anchors/auth.yml index 3b31227..ed61076 100644 --- a/openspec/specs/anchors/auth.yml +++ b/openspec/specs/anchors/auth.yml @@ -18,7 +18,7 @@ auth.group-tags: auth.default-deny: rule: kind: function_definition - has: { field: name, regex: '^allowed_tags$' } + has: { field: name, regex: '^tags_for_groups$' } files: [src/**/*.py] auth.fail-closed-groups: diff --git a/src/acme_mcp/access.py b/src/acme_mcp/access.py index b2a262a..7b3bb2f 100644 --- a/src/acme_mcp/access.py +++ b/src/acme_mcp/access.py @@ -9,12 +9,12 @@ FastMCP 3 ships callable-based authorization for exactly this. An auth check is a function that takes an :class:`AuthContext` (the caller's token plus the component being accessed) and returns ``True`` to allow or ``False`` to deny. -Wiring that check through :class:`AuthMiddleware` enforces it across every -component in two places at once: +Wiring that check through :class:`AuthMiddleware` enforces it across tools, +resources, and prompts when they are listed or used. -* it filters denied tools out of ``tools/list``, so they never clutter the - model's context; and -* it blocks a direct call to a denied tool even if the model guesses the name. +* it filters denied components out of the list responses, so they never clutter + the model's context; and +* it blocks direct use even if the model guesses a hidden name or URI. One tradeoff to know about: the built-in does not pretend a denied tool doesn't exist. A blocked call surfaces an "insufficient permissions" error that names @@ -37,7 +37,7 @@ def group_access(ctx: AuthContext) -> bool: ``True`` when the caller holds the wildcard (``ALL_TAGS``, e.g. admin) or when any of the component's tags is in the set the caller's groups map to. - ``public``-tagged tools pass for any authenticated caller; an unauthenticated + ``public``-tagged components pass for any authenticated caller; an unauthenticated caller (``ctx.token is None``) is denied everything, which is what hides every business tool from ``tools/list`` before any auth provider rejects the request outright. @@ -47,9 +47,15 @@ def group_access(ctx: AuthContext) -> bool: allowed = tags_for_groups(ctx.token.claims.get("groups")) if ALL_TAGS in allowed: return True - return bool(set(ctx.component.tags) & allowed) + + component_tags = set(ctx.component.tags) + if skill_info := getattr(ctx.component, "skill_info", None): + # ponytail: FastMCP 3.4 keeps skill frontmatter as metadata rather than + # component tags. Remove this fallback when SkillProvider projects tags. + component_tags.update(skill_info.frontmatter.get("tags", [])) + return bool(component_tags & allowed) def build_access_middleware() -> AuthMiddleware: - """Server-wide authorization: hide denied tools and block calls to them.""" + """Server-wide authorization for tools, resources, and prompts.""" return AuthMiddleware(auth=group_access) diff --git a/src/acme_mcp/server.py b/src/acme_mcp/server.py index 5353747..611be3c 100644 --- a/src/acme_mcp/server.py +++ b/src/acme_mcp/server.py @@ -5,8 +5,9 @@ 1. Authenticate every caller (:func:`acme_mcp.auth.build_auth`). 2. Mount each business domain as its own sub-server, so the codebase stays split by domain rather than one giant file. -3. Wrap every tool call in audit logging (:class:`acme_mcp.audit.AuditLog`). -4. Filter the tools each caller sees and can run by their group +3. Publish the companion skill as MCP resources. +4. Wrap every tool call in audit logging (:class:`acme_mcp.audit.AuditLog`). +5. Filter the components each caller sees and can use by their group (:func:`acme_mcp.access.build_access_middleware`). A local stdio server is a convenience; a remote HTTP server is production @@ -17,6 +18,7 @@ from __future__ import annotations import os +from pathlib import Path from fastmcp import FastMCP @@ -29,6 +31,7 @@ from acme_mcp.domains.reports import reports_server from acme_mcp.domains.support import support_server from fastmcp.server.dependencies import get_access_token +from fastmcp.server.providers.skills import SkillProvider # The separately-owned analytics domain runs as its own service. We proxy it # rather than holding it in-process; this is the closest thing to lazy loading, @@ -38,6 +41,7 @@ ANALYTICS_URL = os.environ.get( "ACME_MCP_ANALYTICS_URL", "https://analytics.acme.internal/mcp" ) +HANDLE_DOWNLOADS_SKILL = Path(__file__).parent / "skills" / "handle-downloads" def build_server(env: str | None = None) -> FastMCP: @@ -59,6 +63,8 @@ def whoami() -> dict: for sub in (orders_server, billing_server, admin_server, support_server, reports_server): mcp.mount(sub) + mcp.add_provider(SkillProvider(HANDLE_DOWNLOADS_SKILL)) + # Audit first so it wraps the outermost call; the access middleware sits # inside it and decides who may reach each tool. mcp.add_middleware(AuditLog()) diff --git a/skills/handle-downloads/SKILL.md b/src/acme_mcp/skills/handle-downloads/SKILL.md similarity index 98% rename from skills/handle-downloads/SKILL.md rename to src/acme_mcp/skills/handle-downloads/SKILL.md index 237bcf9..48ae99b 100644 --- a/skills/handle-downloads/SKILL.md +++ b/src/acme_mcp/skills/handle-downloads/SKILL.md @@ -1,6 +1,7 @@ --- name: handle-downloads description: How to handle a download_url returned by an acme-mcp tool. Use whenever a tool result contains a "download_url" field (for example from export_report) — present it to the user as a link, mention the expiry, and never read the file into context. +tags: ["reports"] --- # Handling file downloads from acme-mcp diff --git a/tests/test_skills.py b/tests/test_skills.py new file mode 100644 index 0000000..5a97c39 --- /dev/null +++ b/tests/test_skills.py @@ -0,0 +1,29 @@ +"""The server publishes its companion skill as reports-scoped MCP resources.""" + +import pytest +from fastmcp import Client + +from tests.conftest import as_caller + +SKILL_URI = "skill://handle-downloads/SKILL.md" +MANIFEST_URI = "skill://handle-downloads/_manifest" + + +@pytest.mark.parametrize("groups", [["support"], ["finance"], ["admin"]]) +async def test_reports_callers_can_discover_and_read_skill(server, groups): + with as_caller(groups=groups): + async with Client(server) as client: + uris = {str(resource.uri) for resource in await client.list_resources()} + result = await client.read_resource(SKILL_URI) + + assert {SKILL_URI, MANIFEST_URI} <= uris + assert "# Handling file downloads from acme-mcp" in result[0].text + + +async def test_unknown_group_cannot_discover_or_read_skill(server): + with as_caller(groups=["engineering"]): + async with Client(server) as client: + uris = {str(resource.uri) for resource in await client.list_resources()} + assert SKILL_URI not in uris + with pytest.raises(Exception): + await client.read_resource(SKILL_URI)