Clamp caller-supplied agent privileges to governance

What does this MR do and why?

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/618863

aiDuoWorkflowCreate and POST /ai/duo_workflows/workflows accept agentPrivileges from the caller. clamp_client_privileges already intersected them with the governance resolution, but never ran: it returned early on web_surface?, and local surfaces additionally required duo_workflow_local_tool_governance, which is default-off.

Since web_surface? reads the caller's own environment param, defaulting to web when omitted, the caller chose whether the clamp applied to them.

Note the issue description is out of date. The early return it describes was already replaced by clamp_client_privileges in ac2861a1 / !249463 (merged)

Note

Rebased on !252169 (merged), which is now merged. That MR gave GovernanceSurface.for a distinct UNGOVERNED return, which changes the scope of what this MR closes. See Scope below, and the earlier claim that no environment value can skip the clamp is corrected there.

How the clamp applies

web_surface? is gone from the clamp, so a caller can no longer skip it by picking an environment value that reads as web, which is what omitting it did:

 def clamp_client_privileges
-  return if web_surface?
   return unless Feature.enabled?(:gitlab_duo_governance_settings, @container)
-  return unless Feature.enabled?(:duo_workflow_local_tool_governance, ...)
+  return unless known_privileges?(@params[:agent_privileges], @params[:pre_approved_agent_privileges])
+  return if ::Ai::ToolRules::GovernanceSurface.ungoverned?(resolved_surface)

Before, with an admin rule putting run_commands on Ask:

caller posts agentPrivileges [1, 2, 4], omits environment
  -> web_surface? is true (nil defaults to :web)
  -> return, clamp never runs
  -> row persists [1, 2, 4], RUN_COMMANDS pre-approved, no prompt

After:

caller posts agentPrivileges [1, 2, 4]
  -> privileges_from_client is true, so the clamp runs
  -> governance resolves [1, 2, 7]
  -> intersection persists [1, 2], RUN_COMMANDS gone

Scope

This closes the reported vector and everything that resolves as a web surface: an omitted environment, web, ambient and external. Those are always clamped now, regardless of what the caller declares.

ide, chat and chat_partial still skip the clamp while duo_workflow_local_tool_governance is off, which is the production default. GovernanceSurface.for returns UNGOVERNED for them, and the guard above bails out rather than clamping them against web rules, which is the defect !252169 (merged) fixed and the reason that MR had to land first.

That is the same set master already skips, so enforcement here is a strict superset and nothing regresses. But the caller-decides shape survives for those three values until #613544 rolls the flag out, so #618863 is closed for the web and omitted vectors specifically.

environment continues to select which rule column is read, via GovernanceSurface.for(...) || :web.

Not clamping what the caller never sent

Clamping on the sole basis of @params[:agent_privileges] would also have caught privileges the application sets itself, narrowing flows nobody influenced. Ai::Catalog::ExecuteWorkflowService pre-approves all seven, and agent_workflows and the foundational flows set their own defaults.

CreateWorkflowService therefore takes privileges_from_client, passed only by the GraphQL mutation and REST create. App-set privileges keep the behaviour they have on master, including the existing non-web clamp when the local flag is on.

The update path always clamps: its only caller is the update mutation, so privileges are always the caller's. Leaving it unclamped would let a session widen its own privileges on a later request, which is what #608968 covers.

Two smaller fixes

The update path omitted workflow_definition when resolving the surface, so :background was unreachable and it always fell back to :web. This is the one part of the diff that is not a tightening: on the background surface background_effective coerces an unset group default to allow, so developer/v1 resolves more permissively there than it did against :web.

Clamping an unknown privilege intersected it away and saved successfully, which suppressed the existing invalid-value validation. Invalid input now skips the clamp and reaches that validation. The check moved into Concerns::GovernanceResolution as known_privileges?, since both clamp sites need it.

Behaviour change

Callers of the two client endpoints have pre-approvals narrowed to the governance resolution. Under default web policy that is READ_ONLY_GITLAB only, since the other groups resolve to Ask.

That includes the GitLab-owned web actions built on duo_workflow_action.vue (fix pipeline, resolve dependency bump, resolve discussion, work item to MR). They send agent_privileges including READ_WRITE_FILES and no pre_approved_agent_privileges, so today they take the column default {1,2} and after this MR they store [READ_ONLY_GITLAB].

No prompt or stall follows, because those flows do not run tool approval. In DWS require_tool_approval defaults to false and the approval nodes are a no-op unless a flow config sets it, in both the v1 and experimental component. Of every flow config on ai-assist main, only developer/2.0.0-interactive, developer/2.1.0-interactive and software_development/1.0.0 enable it; all five fix_pipeline versions, resolve_dependency_bump/1.0.0 and developer/1.0.0 have it off, so pre_approved_agent_privileges is never read for them. The namespace-level tool_approval_for_session_enabled setting cannot change this: DWS never reads it, and its three Rails consumers only ever remove the approval capabilities when it is off.

This is worth re-checking if any of those flows ever turns require_tool_approval on, or developer moves to a 2.x-interactive config. That is the assumption the "no impact" reading rests on.

Web-surface creates that supply privileges now run one governance resolution they previously skipped, which is two queries (GovernedMcpTools.for, ToolRule.for_namespace) or three with a project.

Known gaps, deliberately not addressed

  • clamp_surface falls back to :web while build_resolution_service falls back to degraded_surface (the raw environment, read as local_access). So the same environment: "ide" workflow resolves different columns depending on whether the caller supplied privileges. @dbernardi flagged this on !249463 (merged) as part of the re-key follow-up, tracked in #624032.
  • A request carrying only pre_approved_agent_privileges falls through to server resolution, which overwrites rather than intersects. The gate condition is unchanged from master, so this is not a regression here. The mirror case, a request carrying only agent_privileges, was leaving the stored pre-approvals unclamped and is fixed in this MR.
  • On the :background surface the resolution pre-approves every group, because background_effective coerces an unset default to allow, and the clamp adopts that when the caller omits pre-approvals. That is wider than the column default this MR replaces. Reachable only with duo_workflow_background_tool_governance on, which is wip and default off, so it needs resolving before that flag rolls out rather than before this MR. Tracked in #627380.
  • An unknown privilege value skips the clamp rather than being rejected before it, so a caller-supplied value decides whether the control runs. Safe today, since the model's invalid-value validations are unconditional and reject the save, but the safety net is implicit. Tracked in #627357.
  • On the ungoverned path the update service returns before constrain_pre_approved_privileges, so a caller-supplied pre-approval is persisted without being reduced to a subset of its grants. The model validates that on create only. Master skips the same way for web and for flag-off local surfaces, so this MR narrows the exposure rather than adding to it.

How to set up and validate locally

  1. Set a namespace tool rule putting run_commands on Ask.
  2. Call aiDuoWorkflowCreate with agentPrivileges: [1, 2, 4] and no environment.
  3. Before: the workflow row keeps privilege 4. After: it is clamped out.

MR acceptance checklist

  • 845 examples across the affected service, request and GraphQL specs pass locally, 0 failures; RuboCop clean.
  • The two web-path examples stub duo_workflow_local_tool_governance off so they run under the shipped config. Verified by re-adding the flag check this MR deletes: without the stub the file stayed green, with it two examples fail.
  • No changelog entry: security-adjacent behaviour change on a confidential issue.
  • No migration, no documentation change.
Edited by Abhimanyu Singh

Merge request reports

Loading
Loading