MCP exposure: role gate, catalog preview, merged default, tool definitions, writes switch, stateless default, health check - #77
Draft
nicdavidson wants to merge 22 commits into
Draft
nicdavidson wants to merge 22 commits into
nicdavidson wants to merge 22 commits into
Conversation
… endpoint
With the switch on, a non-admin identity must hold a role_service_access
grant on the MCP service itself before /mcp/{service} admits it (HTTP 403,
JSON-RPC -32003, audit status 'denied'). Admins always pass. The role's
backend grants still decide what it can do once connected.
Migration adds the column with default false so existing services keep the
open-door behaviour; new services start with the switch on via the model's
creating hook. Nothing ever writes role rows automatically.
Adds GET /_internal/ai/mcp-access (admin) listing granted roles plus roles
seen in the request log, flagged granted/denied, so an admin can see who
would lose access before turning the switch on.
Closes #63
Prefixed style emits every verb once per database, so five databases produce 80 near-identical tools that bloat client context and confuse tool selection. Merged style registers each verb once with a service argument. - McpServerConfig creating hook sets tool_style=merged when a new row has none; config-schema default and labels updated, prefixed marked legacy. - Existing rows untouched: the tool_style column stays nullable with no backfill, and null still reads as prefixed in the daemon. - README documents the setting and the default. - Source-wiring test covers the hook, schema default, and migration. Closes #65
4ae30fc added list_tools to the facade but left the four tools/list expectations in lazy.test.ts and scoping.test.ts on the old four-tool list, so npm test has been red on develop since.
…role denials allow_writes (mcp_server_config, boolean, default true) lets an admin make an MCP server read-only regardless of role or per-tool toggles. When off the daemon never registers the write verbs (create/update/delete_records, call_stored_procedure/function, create_file, create_folder, delete_file) in either tool style, so tools/list cannot advertise them and the lazy facade cannot reach them; the server instructions say the server is read-only. Hidden for system_mcp, whose daemon has no DB/file write verbs. handleError now classifies a DreamFactory 403, or the 401 'User is not authenticated' df-core returns for a key-only role with no access, as a Permission Error before the generic 401 branch, so the model stops retrying authentication when its role is simply denied. Closes #67
Flip MCP_STATELESS to default true. The daemon issues no Mcp-Session-Id and rebuilds the MCP server per request, so any node behind a load balancer can answer any request without stickiness. MCP_STATELESS=false (also 0/no/off) restores the warm, process-pinned sessions. Stateless servers never see the client's tools/list, so the lazy facade's "call before list is served as off" rule disabled shaping, paging, hot tools and the X-Mcp-Ledger header on every stateless call. Prime the lazy catalog per request in stateless mode so those behave as documented. Log the bound port on listen so tests can boot the daemon on port 0. Add a spawn-based test that drives the real daemon over HTTP in both modes: initialize -> initialized -> tools/list -> tools/call as separate requests, with and without Mcp-Session-Id, the no-handshake rpc-bridge shape, GET 405, and fetch_more across stateless requests. Document the mode table, the per-node fetch_more limitation, the lost clientInfo passthrough detection, and the upgrade note.
A custom tool can write too, so with allow_writes=false only API tools using GET are registered; non-GET API tools and function tools are skipped, logged, and counted in the server instructions.
…/ai/mcp-health) Probes the data and system daemons' /health (2s timeout, never throws) and checks APP_URL vs request origin, MCP_INTERNAL_BASE_URL, MCP_INTERNAL_KEY, node on PATH and the daemon session mode. Each check carries a plain-language message saying what to fix; overall status is the worst check. The data daemon's /health now reports its version.
In stateless mode the daemon only sees initialize.clientInfo on the initialize request, so codex/grok/hermes lost the full-catalog passthrough on tools/list. The PHP proxy now sends X-Mcp-Client-Name on every request, resolved server-side from the OAuth client's registered client_name (RequestLogger::resolveClientName, now public) or the API-key app name; the rpc bridge identifies as df-ai-chat. The value is reduced to printable ASCII and capped at 128 chars. The daemon uses the header for the passthrough decision when the per-request server never saw an initialize. Tests: spawned stateless daemon with X-Mcp-Client-Name: Codex CLI gets the full catalog while an anonymous request gets the facade; PHP unit test for header sanitisation plus source wiring.
…ession (#64) Daemon: POST /mcp/catalog/preview (internal-key gated) takes the same _mcpConfig / _mcpAvailableServices the proxy envelopes, plus clientName and lazyMode overrides, and runs the real registration path (createServer) in a throwaway in-memory server. Returns { tools[{name,title,description,category, write,service?}], count, bytes, lazy, facade }. No DreamFactory calls, no session, nothing executed. The envelope config parser is shared with the proxied request (parseMcpConfig) so the preview cannot drift from a session. PHP: admin-only GET /_internal/ai/mcp-catalog?service=<name|id>&role_id=<id> |app_id=<id>[&client=][&lazy_mode=]. Seeds the session as that role (RoleSession: no user, app id, Role::getCachedInfo — the key-auth composition), resolves the catalog through the shared AvailableServices::resolve(), reads the role's verb mask per backend, asks the daemon for the preview, and restores the admin's session. No audit row. Tests: daemon unit test for the preview (prefixed vs merged, lazy on/auto/off + override, passthrough client, disabled_tools, custom tools, empty exposure); PHP wiring test (route, admin gate, shared resolver, no proxy / rpc / audit) and a functional RoleSession test on the real df-core Session.
Compare host and non-default port first; a scheme-only difference becomes a separate app_url_scheme note (status ok) that mentions TLS termination and trusted proxies. When X-Forwarded-Proto / X-Forwarded-Host are present they drive the comparison (diagnostic only); details carry both raw and forwarded origins. Tests for forwarded https, scheme-only mismatch and a genuine host mismatch.
…eat-mcp-exposure-integration
…t-mcp-exposure-integration
…wn-key errors, tool annotations Canonical argument names on every generated DB and file tool are now snake_case (table_name, count_only, include_count, procedure_name, group_by, include_files, ...), matching the tool names and DreamFactory's REST parameters. A normaliser in front of the SDK's validation maps any casing of a known key (camelCase, PascalCase, kebab, UPPER) to the canonical name, so existing camelCase calls keep working; in merged mode the service argument also accepts a service label. A key that maps to nothing is a validation error naming the closest valid key instead of being silently stripped. The same path serves direct calls, custom tools and the lazy facade's call_tool. Every registered tool now carries MCP tool annotations derived from its verb (readOnlyHint for get/list/search/describe/aggregate/find/discover, destructiveHint for delete/update, openWorldHint=false; custom tools are open-world and read-only when GET; call_tool leaves the effect unknown). The X-Mcp-Ledger header moves to a small request-scoped ledger module that works in every lazy mode and gains arg_aliases / arg_errors counters; PHP persists them to mcp_request_log (migration + RequestLogger + model). Tests: normaliser units and end-to-end alias/error/annotation/ledger checks over InMemoryTransport, a golden tools/list snapshot for a prefixed two-DB server (UPDATE_SNAPSHOT=1 npm test to refresh), and a PHP wiring test for the ledger columns. Also fixes two pre-existing test issues: facade list expectations missing list_tools, and the custom function tool's 30s timeout timer pinning the test process after every run. Closes #66
…cp-exposure-integration # Conflicts: # src/Models/McpServerConfig.php
…p-exposure-integration # Conflicts: # daemon/src/server.ts # src/Models/McpServerConfig.php
…p-exposure-integration # Conflicts: # src/ServiceProvider.php
…mcp-exposure-integration # Conflicts: # README.md # daemon/src/server.ts # src/Client/McpDaemonClient.php # src/ServiceProvider.php
…-mcp-exposure-integration # Conflicts: # daemon/src/server.ts # daemon/src/services/tools.service.ts
The tool-definitions branch replaced lazy.service's isReadOnly() with per-tool annotations; the catalog preview now reads readOnlyHint from the listed tool and falls back to the verb-derived annotation.
…ckend Session::getServicePermissions(service) with no component only answers the service-wide rows, so a role granted purely at table level (GET on _table/orders/* and _table/customers/*) previewed as verbs=[] and the UI marked every tool denied. backends[].verbs is now the union of verb masks over every role row for that service, any component; component_scoped is true when no row is service-wide, and components lists the patterns so the UI can say "limited to orders, customers". Script-only rows are ignored, inactive roles report nothing, and a service with no rows still falls back to the service-wide lookup (df-core's all-services rows).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Team branch for the MCP exposure work (Tuesday 2026-09-22 comparison). Do not merge until the review; this PR exists so the whole entry can be read and diffed against develop in one place.
Everything here branched from develop and was reviewed per issue before integration:
GET /_internal/ai/mcp-accessX-Mcp-Client-Namekeeps lazy passthrough workingGET /_internal/ai/mcp-health(proxy-aware APP_URL check)GET /_internal/ai/mcp-catalog?service=&role_id=+ daemonPOST /mcp/catalog/previewIntegration-only commits resolve the overlaps:
allow_writesparsed in the sharedparseMcpConfig, the catalog preview reads read-only from tool annotations, one wiring assertion moved with the parse.Behaviour changes are listed per PR above and collected in the release change log; the short version: nothing changes on upgrade except two additive columns, one new ledger pair, tool argument names advertised in snake_case (camelCase still accepted), stateless daemon by default (opt-out), and every tool now carries annotations.
Tests on the integrated tree: daemon 36/36 +
tscclean; PHPUnit 227 tests / 893 assertions; live checks on df-dev: role-access 10/10, system-mcp 35/35, health, catalog preview, snake_case aliases + unknown-key error, writes switch 58 -> 43, merged label matching — all pass.The admin UI half lives in df-admin-interface
feat/mcp-exposure(#523).