Guard governed-tool argument coverage and refuse unscoped governed calls

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Context

The GitLab MCP server enforces Always Deny AI tool-governance rules for external MCP clients such as Claude Code and Cursor. Before a tool runs, governance works out which project or group the call acts on and checks that namespace's rules.

This shipped in !258209 (merged) (GitLab 19.6) behind the feature flags duo_mcp_server_governance_rule_enforcement and duo_workflow_local_tool_governance. Both are disabled by default. It replaced !253147 (closed), which was closed. During review of that MR, AppSec asked for further hardening, and this issue tracks it.

Last checked against master at db8a3d7e.

Item 1: guard governed-tool argument coverage

Mostly done. One gap remains.

What shipped

A tool declares its container arguments with the container_arguments macro. The kinds are project, group, project_or_group and record. A record argument names a record whose project or group is looked up from it. A tool can also override governed_containers.

governable_tools_container_spec.rb runs in CI and checks every governable tool:

  • It declares at least one container argument that it accepts, including url when it takes one.
  • Every input property is declared, or is listed in the spec's not_a_container list. That list is the waiver list.
  • The not_a_container entries are not stale.
  • Every declared kind is one the resolver reads.
  • A record: argument without a lookup lambda comes with a governed_containers override.

The two gaps that prompted this item, and the record arguments listed in the original description, are now declared:

Argument Declared in
target_project_id (save_merge_request) save_merge_request_service.rb
pipeline_id (save_pipeline) save_pipeline_service.rb
parent_id (save_work_item) save_work_item_service.rb
work_items_ids (link_work_items) link_work_items_service.rb
epic_id (EE create_issue) ee/lib/ee/api/helpers/issues_helpers.rb

EXTRA_CONTAINER_ARGUMENTS is gone. REST tools declare their arguments through the :mcp route setting, which ApiTool#container_arguments merges with the route's boundary type.

What remains

The guard checks that an override exists. It never calls it, so an override that returns no container for a valid input still passes CI. Three cases were found by hand:

  • save_note with only note_id, fixed in !264854 (open)
  • save_note resolve_discussion with only discussion_id: #637282
  • save_merge_request_review resolve_discussion, the same pattern, noted in #637282

!261045 (draft) gives every governable tool its own governed_containers, built on the lookup its execute path uses, and treats a failed lookup as no container. After it lands, what each override returns is the thing to check.

The direction is under discussion in #628962 (comment 4151627456). One option is a CI spec that calls every override with representative inputs. The other is a runtime refusal, which is Item 2.

Item 2: refuse a governed call that names no container

Not started. A call whose containers all fail to resolve still runs ungoverned. check_tool_governance! keeps the resolved containers that have the flags on and that the caller can read, and returns before any rule check when none are left. The handler spec when an identifier resolves to no container expects the tool to answer for itself. The user documentation describes this: if the call names a project or group that does not exist or that you cannot access, its rules are not checked.

AppSec asked that such calls be refused, with only tools explicitly marked as having no target skipping the check. Tools fall into three classes:

  • Always-scoped tools, for example save_pipeline, accept_merge_request and add_commit. Refusing when no container resolves is correct, because it means something went wrong.
  • Never-scoped tools. These are marked ungovernable!: get_mcp_server_version, get_user and send_duo_session_input.
  • Optionally-scoped tools. list_projects, list_merge_requests and list_groups have required: [] and legitimately span everything the caller can see. Refusing them when no container is named would break working behavior.

The work starts with classifying every governable tool into these classes. There is no marker for always-scoped or optionally-scoped tools in the code yet.

Points to settle for the refusal:

  • Refuse on the unfiltered governed_containers result. The flag and readability filter runs afterwards, so refusing on the filtered list would also refuse calls into namespaces that don't have the flag.
  • With no container, there is no root namespace for McpGovernance.governs? to check the flags against. The refusal needs its own gate.
  • A lookup that fails because the record does not exist, or because the caller cannot see it, returns a refusal instead of the tool's own error. The refusal must not reveal whether the record exists.

For optionally-scoped tools, there are three options:

  1. Govern by the caller's default GitLab Duo namespace. tools/list already resolves rules against the caller's governing_namespace, which falls back to their default GitLab Duo namespace.
  2. Refuse the call and require the caller to name an explicit container.
  3. Keep allowing the call unscoped, accepting that these tools stay outside enforcement.

This is a product decision as much as a code one. The always-scoped refusal does not depend on it and can ship first.

Item 3: two further AppSec points

  • The flag-keying concern no longer applies. ContainerAccess#every_identifier_resolved? was removed. Governance checks the flags against each resolved container's root namespace. A url that matches no project or group resolves to no container, which is the Item 2 case.
  • The lookups are bounded. A list argument can name at most MAX_IDENTIFIERS_PER_ARGUMENT (100) identifiers, and a call can make at most MAX_LOOKUPS_PER_CALL (20) path or URL lookups. The per-call cap is behind mcp_container_lookup_cap (https://gitlab.com/gitlab-org/gitlab/-/work_items/635194). The hourly MCP endpoint rate limits are rolling out in https://gitlab.com/gitlab-org/gitlab/-/work_items/635267.

Blocks flag enablement

Item 2, and the remaining Item 1 gap, should land before duo_mcp_server_governance_rule_enforcement is enabled by default, per the AppSec request.

Edited by Raounak Sharma