Resolve MR branch and validate goals for business logic scans
A Business Logic Security Scan started from the API or Duo Chat with a merge request IID scans the default branch instead of that merge request.
Merge order: Targets
masterdirectly; it doesn't depend on any other BLSA MR's code. It can merge in any order.
What does this MR do and why?
- A Business Logic Security Scan started from the API or Duo Chat with a merge request IID now checks out that merge request's source branch. Before, it checked out the default branch.
- Duo Chat now runs the flow's goal validator. A fork or deletion-only merge request is refused with a 400, as it already is on the workflows API.
- A merge request IID that matches no merge request is now refused (
invalid_goal), from GitLab Duo's review on !257092 (merged). - A closed or merged merge request is refused too (
invalid_goal,The merge request is not open), and resolves no branch. - The branch comes from a new, optional
source_branch_resolveron foundational flows. Only this flow declares one.
Part of the BLSA split of !246889. Tracker: https://gitlab.com/gitlab-org/gitlab/-/work_items/630266
Behavior
| Start path | Goal | Before | After |
|---|---|---|---|
API, no source_branch |
MR IID | Checks out the default branch. Whole-repository scan | Checks out the MR's source branch. Scans the files the MR adds or modifies (with the full-scan fallbacks from !257465 (merged): BL_TARGET_FILES, more than 400 files, paths that can't be split) |
| Duo Chat | MR IID | Checks out the default branch. Whole-repository scan | Checks out the MR's source branch. Scans the files the MR adds or modifies (with the full-scan fallbacks from !257465 (merged): BL_TARGET_FILES, more than 400 files, paths that can't be split) |
| Duo Chat | Fork MR IID | Not refused. Whole-repository scan of the default branch | 400: Merge requests from forks are not scanned |
| API or Duo Chat | Closed, merged or unknown MR IID | Not refused. Whole-repository scan of the default branch | 400: The merge request is not open / Merge request not found |
| Duo Chat | Deletion-only MR IID | Not refused. Whole-repository scan of the default branch | 400: The merge request has no added or modified files to scan |
| Automatic trigger (pipeline) | MR IID or pipeline URL | Checks out the pipeline's ref | Unchanged. The pipeline's ref is passed explicitly and wins |
| Session restart | MR IID | Checks out the default branch. Whole-repository scan | Checks out the MR's source branch. Scans the files the MR adds or modifies (with the full-scan fallbacks from !257465 (merged): BL_TARGET_FILES, more than 400 files, paths that can't be split) |
Changed-files scoping comes from !257465 (merged). Until !257465 (merged) merges, a scan started with an MR IID checks out the MR's source branch and scans the whole repository. Once !257465 (merged) is in, it scans only the files the MR adds or modifies, as the table shows.
An explicit source_branch always wins. A goal that is not an MR IID (a pipeline URL) resolves no branch.
How to review
bl_security/definition.rb: the resolver. Open MR IID to source branch; nil for a fork MR, a non-numeric goal, an unknown IID, or a closed or merged MR. The validator refuses unknown and non-open MRs.attributes.rbandfoundational_flow.rb: the new attribute, its keyword validation, andresolve_source_branch_for.flows/execute_service.rbandduo_workflow_helpers.rb: thesource_branch.presence || resolverfallback. ExecuteService covers the consumer path, Duo Chat and restart. The helper covers the name-started path.agent_workflows.rb:validate_flow_goal!runs only for the business logic flow, and only after the execute-permission check on both paths.
Backward compatibility / impact
| Consumer | Change | Risk | Mitigation |
|---|---|---|---|
Duo Chat and agent callers of code_review/v1 on POST agent_workflows |
None | A new 400 for group containers, or a normalized goal, if the validator ran for every flow | Fix 3: the validator runs only for the business logic flow reference (bl_security/experimental). Other flows are byte-identical to the target branch, whatever the flag. Scoping by flow was the smaller change than scoping by flag |
Duo Chat and agent callers of bl_security on POST agent_workflows |
Unknown, closed, merged, fork and deletion-only MRs get a 400 | A caller could probe for merge requests it cannot run the flow on | Fix 2: validated after the execute-permission check, so an unauthorized caller gets the normal authorization error first. The validator itself returns nil when bl_security_analyzer is off |
Ai::Catalog::Flows::ExecuteService and the helper's start_workflow_params |
Fall back to the flow's resolved branch when source_branch is blank |
Another flow checks out an unexpected branch | Only this flow declares a source_branch_resolver; every other flow resolves nil. An explicit source_branch always wins |
New source_branch_resolver attribute |
Optional, keyword-validated like its siblings | None for existing flows | Nil by default |
validate_flow_goal! moved into API::Helpers::DuoWorkflowHelpers. Since bd13282c it returns Invalid goal to callers without read_merge_request, on workflows.rb too; otherwise workflows.rb behaves as before.
Design decisions
- A fork MR resolves no branch. Its branch is not in the target project. The validator refuses those runs anyway.
- The branch is resolved from the goal, not stored. Restart re-resolves it the same way, so no new column is needed.
Known limitations and follow-ups
workflows.rbvalidates the goal before authorization. Pre-existing; left unchanged here and filed as a follow-up.- Session restart or resume drops an explicit
source_branch. Pre-existing. Restart re-resolves the branch from the goal, so a changed source branch changes what it checks out. - An explicit
source_branchthat doesn't match the goal's MR still wins. It is not checked against the MR. - Possible follow-up: reuse
GOAL_RESOLVERfor the source branch and setnoteable_resolverto link the workflow to its MR.
Preflight fixes (this revision)
- Closed or merged MRs are refused (
invalid_goal) and resolve no branch, so nothing checks out a stale branch. - On
agent_workflows, the goal is validated after the execute-permission check on both paths, so it no longer leaks whether an MR exists. - On
agent_workflows, only the business logic flow is validated, so flag-off behaviour for other flows (e.g.code_review/v1on a group) is unchanged.
File inventory (11 files)
- Resolver:
ee/app/models/ai/catalog/foundational_flow/bl_security/definition.rb,foundational_flow.rb,foundational_flow/attributes.rb - Start paths:
ee/app/services/ai/catalog/flows/execute_service.rb,ee/lib/api/helpers/duo_workflow_helpers.rb - API:
ee/lib/api/ai/duo_workflows/agent_workflows.rb,ee/lib/api/ai/duo_workflows/workflows.rb(validator moved to the helper) - Specs:
duo_workflow_helpers_spec.rb,foundational_flow_spec.rb,agent_workflows_spec.rb,execute_service_spec.rb
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.