Draft: Enforce Deny tool rules on the GitLab MCP server

Raounak Sharmarequested to merge
623329-enforce-deny-external-mcp into master

What does this MR do and why?

Tool rules set to Always Deny were enforced for Duo Agent Platform workflows, but not by the GitLab MCP server itself. POST /api/v4/mcp authenticated and then executed, with nothing consulting ai_tool_rules in between. An administrator who denied a tool in the AI Governance UI still saw Claude Code or Cursor run it, including manage_pipeline, the one governable tool that can delete a resource.

The check now happens on the server, so it holds regardless of how a client is configured. It sits behind the duo_mcp_server_governance_rule_enforcement flag, off by default. The server serves 41 tools; 22 carry a rule under their own name, and the rest are unlisted, in Registry::UNGOVERNED_TOOLS, or governed under a Duo Agent Platform catalog name instead.

Design decisions

  • The flag is checked per resolved container, not once up front. Feature.enabled? with no actor returns false for an actor-gated flag, so an early return would switch governance off for exactly the groups it was enabled for.
  • The verdict is looked up under the tool's canonical name. Aliases aren't settable in the UI, so without resolving them a rule could be routed around by spelling the tool differently, for example get_job_log instead of get_job.
  • A call that resolves no container is allowed. There's no namespace, so there are no rules to apply. Refusing every unscoped call would break tools that legitimately take no project.
  • The error class and the enforcement seam live in CE, because Grape freezes namespace handlers before prepend_mod runs and CE can't name an ee/ constant.

A tool served under one name and governed under another

Nine of the served tools carry their rule under a different name. For example, the server serves add_commit, but the rule an administrator sets in the governance UI is create_commit. GovernedMcpTools already computed that mapping, by intersecting each tool's tool_aliases with the Duo catalog, but nothing consulted it when resolving a verdict.

This was caught in manual testing, not by the specs: a Deny set in the UI silently did not apply, because permission_for read the served name, found no rule, and fell back to the tool's default.

Served as Rule is stored under
add_commit (destroy) create_commit
add_branch create_branch
save_merge_request create_merge_request, update_merge_request
save_work_item create_work_item, update_work_item
save_note create_merge_request_note, create_work_item_note
get_workitem_notes get_work_item_notes
get_merge_request_notes list_all_merge_request_notes
get_merge_request_diffs list_merge_request_diffs
list_merge_requests gitlab_merge_request_search

Resolution now consults every name a tool answers to and takes the most restrictive verdict, so either spelling reaches the rule. Alias resolution alone couldn't have fixed this: resolve_alias maps an alias to the served name, while the rule lives under the alias, pointing the other way. governed_tool_identity_spec walks all 12 mappings and asserts that a Deny on the governing name denies both spellings, so a tool added later can't reopen the hole silently.

Known limits

  • Only Deny stops a call. Ask has no prompt to raise on this path, so it executes.
  • A call must name a project or group. get_mcp_server_version takes no arguments and is declared ungovernable.
  • tools/list isn't filtered yet: !252995 (merged)
  • A call naming several containers answers to all of them, so a Deny can't be skipped by argument ordering.
  • Namespace resolution isn't user-scoped. A caller can name a project they can't access, and governance may answer 403 where another error would otherwise surface. The tool still enforces its own authorization, so this isn't a bypass, just a weak signal that a rule exists there.
  • fork_repository is governed by the source project (id), not the fork target (namespace_id), which may matter more for a write.

ee/lib/ee/api/mcp/handlers/call_tool.rb still has tool_call_namespace, which feeds analytics logging and reads only project_id / group_id. Governance stopped using it, so logging and governance can now disagree about which namespace a call belongs to, for the nine tools whose scope argument isn't project_id.

This MR also skips the issue's suggested reuse of tool_call_namespace, since it misses those same nine tools, which would leave those calls silently ungoverned. It changes check_tool_rule!'s signature from check_tool_rule!(tool_name, current_user) to check_tool_rule!(tool, tool_name, params), because the EE override needs the tool itself to look up its scope arguments, and nothing used current_user.

How to set up and validate locally

Verified on GDK.

  1. Create a PAT with the mcp scope (api alone isn't enough).

  2. Enable the flags and turn on Duo for a top-level group. The self-managed group Duo settings page is SaaS-gated, so the last line sets it directly:

    Feature.enable(:duo_mcp_tool_governance)       # registry
Feature.enable(:duo_mcp_server_governance_rule_enforcement)   # this MR
Group.find_by_full_path('your-group').namespace_settings.update!(duo_features_enabled: true)

  3. In that group, go to Settings > GitLab Duo > Governance > Tool management, and set get_work_item_types to Always Deny.

  4. Call the tool over MCP:

    curl -sS -X POST http://gdk.test:3000/api/v4/mcp \
  -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","id":1,"method":"tools/call",
       "params":{"name":"get_work_item_types","arguments":{"group_id":"your-group"}}}'

  5. On master, the denied tool still runs:

    {"jsonrpc":"2.0","result":{"content":[{"type":"text","text":"{\"workItemTypes\":[…]}"}],
 "isError":false},"id":1}

  6. On this branch, the same call is denied with HTTP 403:

    {"jsonrpc":"2.0","error":{"code":-32002,"message":"Tool denied",
 "data":{"tool":"get_work_item_types"}},"id":1}

    Set the tool back to Allow and the same call returns its result again. Note the Flipper cache: the running Puma process can take up to a minute to see a flag change made from the console.

Test matrix

Each row below is a bug found in review.

Case How to check Expected
REST-shaped tool Deny manage_pipeline, call with {"id":"group/project","pipeline_id":"1","name":"delete"} -32002
Alias Deny get_job, call it as get_job_log -32002, since an alias isn't settable in the UI and must resolve to the canonical rule
Catalog name Deny create_commit, call add_commit -32002, since the UI name and the wire name differ for 9 tools
Project overrides group Allow on the group, Deny on one project, call naming that project -32002
List spanning namespaces Deny on the second of two namespaces in project_ids -32002, regardless of order
Wrong-type Global ID Pass gid://gitlab/Group/1 as a project_id ungoverned, not resolved as project 1
Flag off Feature.disable(:duo_mcp_server_governance_rule_enforcement) call executes as before
Ask Set a tool to Always Ask executes; only Deny stops a call on this path 
bundle exec rspec spec/services/mcp spec/lib/api/mcp \
  ee/spec/lib/ai/tool_rules ee/spec/services/ai/tool_rules \
  ee/spec/lib/ee/api/mcp ee/spec/requests/api/mcp

Why there's no end-to-end test

This is an API-level policy decision with no UI surface of its own. ee/spec/requests/api/mcp/base_spec.rb drives the full stack through POST /api/v4/mcp and asserts the JSON-RPC error contract and status code, which is the boundary an E2E test would cover. The governance UI that sets these rules is unchanged.

References

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

Edited by Raounak Sharma

Merge request reports