Require approval for GitLab Duo for Slack write actions

Igor Drozdovrequested to merge
feature/foundational-flow-pre-approved-privileges into master

What does this MR do and why?

Foundational flows can now pre-approve a narrower set of privileges than they are granted, and slack_assistant/v1 is the first flow to use it. Write actions in GitLab Duo for Slack are no longer pre-approved, so once the Duo Workflow Service side is in place they stop for approval instead of running unannounced.

A privilege that is granted but not pre-approved is what makes a running workflow pause for a human. Until now every code path that started a foundational flow copied agent_privileges into pre_approved_agent_privileges, so a foundational flow could never pause.

Commit 1: allow a foundational flow to narrow its pre-approved privileges

  • Ai::Catalog::FoundationalFlow gains a per-flow pre_approved_agent_privileges attribute, defaulting to nil. The reader falls back to agent_privileges when it is nil, so a flow that narrows nothing behaves exactly as before.
  • A validation rejects a definition that pre-approves a privilege it was never granted. It fails when the registry is first read, rather than later when a workflow fails to start.
  • Ai::DuoWorkflows::CreateAndStartWorkflowService#create_workflow_params and Ai::Messaging::Adapters::Base#start_workhorse_flow read the new attribute instead of reusing agent_privileges.
  • The other 14 flows leave the attribute nil and are unaffected.

Ai::Catalog::ExecuteWorkflowService is a third path that can start a foundational flow and still hardcodes its own full privilege list. Fixing it changes agent_privileges for every catalog-started flow at once, so it is tracked separately in #630334.

Commit 2: require approval for GitLab Duo for Slack write actions

slack_assistant/v1 pre-approves READ_ONLY_GITLAB only. It is still granted READ_WRITE_GITLAB and START_FLOWS, so those are no longer pre-approved.

In that flow's toolset, gitlab_api_get and gitlab_graphql remain pre-approved. These are not: create_issue, create_work_item, create_issue_note, create_work_item_note, create_merge_request_note, update_issue, update_work_item, update_merge_request.

This flow and not the others because Duo in Slack answers in a thread, so there is a person on the other end who can be asked. The other foundational flows run unattended.

Commit 3: let the Slack assistant pause to ask its user

allow_agent_to_request_user: true. The Duo Workflow Service reads this before it keeps an admin's ask rules and discards the whole list when a flow cannot reach a user (abstract_workflow.py:351-354). It then has no ask set to subtract from the pre-approved tools, so an admin who asks to be consulted about a tool this flow pre-approves is ignored. Deny is still enforced.

The registry default of false is right for the unattended flows, so this only changes the Slack one. That the two concerns share a single switch is tracked in #630336.

Nothing in Rails reads allow_agent_to_request_user; it is passed through to the Duo Workflow Service. On the service side the only other consumer is the legacy software_development workflow, not the flow registry path this flow uses, so there is no goal-disambiguation side effect.

Nothing prompts until the Duo Workflow Service config lands

The slack_assistant flow config does not set require_tool_approval, so AgentComponent defaults it to False and the approval nodes are skipped. It was added in feature/slack-assistant-human-input and then reverted on ai-assist main in 9d464680c. Only developer/2.0.0-interactive.yml, developer/2.1.0-interactive.yml and software_development/1.0.0.yml set it today.

So narrowing the pre-approved set changes what GitLab sends, and nothing more, until that config is restored. This MR is a prerequisite for the approval flow rather than the thing that switches it on. An earlier version of this description claimed the config was already in place, which was wrong.

Risk

The flow is behind the slack_duo_api_flow feature flag, which is default_enabled: false and type: wip, so there is no production impact.

The path that carries an approval request to Slack and applies the answer is also unfinished: !255537 (merged) posts the request and opens a review modal, and a follow-up will apply the submitted decision. With the flag on and the service config restored, a session that reaches a write action would sit in tool_call_approval_required until that lands.

Governance interaction

Ai::DuoWorkflows::CreateWorkflowService#resolve_agent_privileges does not touch this flow's lists. Slack passes agent_privileges and never privileges_from_client, and ambient is in Ai::ToolRule::WEB_SURFACES, so the method returns at return if web_surface? before either clamp. The narrowed list lands on the row intact.

That also means governance resolution never runs for this flow, so declaring [READ_ONLY_GITLAB] is not displacing a resolved list. Without the declaration what applies is the column default '{1,2}', which is READ_WRITE_FILES plus READ_ONLY_GITLAB, and pre-approved filesystem write makes no sense for a flow declaring coding_environment: "none". Letting an admin own this list instead requires the flow to go through resolution, which is a separate change.

Tests

  • Model spec: the nil fallback, integer coercion of declared values, that an explicitly empty list pre-approves nothing rather than falling back, a table-driven spec for the subset validation, and the Slack assistant's granted, pre-approved and ask-user attributes.
  • Adapter spec: the narrowed list reaches CreateWorkflowService, asserted as a literal rather than against the flow object, because this call site was reverted to flow.agent_privileges once already in !256183 (merged).

References

  • !254733 (merged) introduced the Slack assistant flow and the server-side flow execution path (merged).
  • !256183 (merged) moved the call site into Adapters::Base#start_workhorse_flow (merged); this MR is rebased on it.
  • !255537 (merged) posts the approval request to Slack and adds the review modal (in progress).
  • #630334 ExecuteWorkflowService ignores declared privileges.
  • #630336 admin ask rules dropped for flows that cannot prompt a user.

Screenshots or screen recordings

None. There is no user-visible change in GitLab itself. The visible effect is a Slack message asking for approval, added by !255537 (merged).

How to set up and validate locally

  1. Check out this branch.

  2. Run the specs:

    bundle exec rspec ee/spec/models/ai/catalog/foundational_flow_spec.rb ee/spec/services/ai/messaging/adapters/base_spec.rb ee/spec/services/ai/duo_workflows/create_and_start_workflow_service_spec.rb

  3. Open a Rails console with bundle exec rails console.

  4. Confirm the Slack assistant now pre-approves a strict subset, and can ask its user. This prints granted [2, 3, 7], pre-approved [2], and true:

    f = Ai::Catalog::FoundationalFlow['slack_assistant/v1']; [f.agent_privileges, f.pre_approved_agent_privileges, f.allow_agent_to_request_user]

  5. Confirm the other 14 flows are unchanged. Every other row's two lists match:

    Ai::Catalog::FoundationalFlow.all.map { |f| [f.foundational_flow_reference, f.agent_privileges, f.pre_approved_agent_privileges] }

  6. Confirm the validation rejects an ungranted pre-approval. This returns a non-empty error array:

    Ai::Catalog::FoundationalFlow.new(agent_privileges: [2], pre_approved_agent_privileges: [2, 3]).tap(&:valid?).errors[:pre_approved_agent_privileges]

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Igor Drozdov

Merge request reports