Martin Cavoj activity https://gitlab.com/mcavoj 2026-03-17T19:27:13Z tag:gitlab.com,2026-03-17:5214451916 Martin Cavoj commented on merge request !222885 at GitLab.org / GitLab 2026-03-17T19:27:13Z mcavoj Martin Cavoj

@fabiopitino @furkanayhan I tried the approach above and gathered some thoughts:

  1. The mapping is passed to each Ci::CreatePipelineService for PEP evaluation.

The issue here is that the policies are evaluated before we process the project config, so we don't have the normalized_jobs yet that we could pass for the PEP evaluation.

  1. If mapping doesn't exist (typical project pipeline), lazily initialize an empty mapping.

I believe this is what we already do with normalized_jobs in Ci::Config here and this MR passes it along to the YamlProcessor::Result.

  1. If mapping exists use that to reference needs.

This is what this MR implements

  1. When jobs are added to a pipeline, they are also added to the mapping object. This allows other policies to reference an up-to-date mapping as the pipeline changes.

The jobs are added to a project pipeline towards the end, at the point when we can collect all the mappings.

We can combine all the mappings from the policy pipelines and pass them to JobInjector, so that the mappings are not dependent on the policy pipeline order.

I added tests to ensure that the needs can be evaluated also across policies.


The MR should resolve some issues with needs that customers have today, but maybe we could explore a different architectural approach of building policy pipelines where we break the EvaluatePolicies step into two:

      Gitlab::Ci::Pipeline::Chain::AssignPartition,
      Gitlab::Ci::Pipeline::Chain::PipelineExecutionPolicies::EvaluatePolicies, # Instead of building full policy pipelines at this point, maybe it's enough to build until `Config::Process` so that we can extract the declared stages for the project pipeline to merge
      Gitlab::Ci::Pipeline::Chain::Skip,
      Gitlab::Ci::Pipeline::Chain::Validate::Config,
      Gitlab::Ci::Pipeline::Chain::Config::Content,
      Gitlab::Ci::Pipeline::Chain::Config::Process, # Here we could extend the static validation and use the policies' `normalized_jobs` to support references from project to policies
      Gitlab::Ci::Pipeline::Chain::PipelineExecutionPolicies::BuildPolicies # Up until this point we stay in the "declared pipeline space". Here we would continue the policy pipeline build based on the processed configs from before, but also feeding the processed project config for extra context.
      Gitlab::Ci::Pipeline::Chain::StopLinting,
      ...
      Gitlab::Ci::Pipeline::Chain::PipelineExecutionPolicies::ApplyPolicies, # Same as today, pipelines get merged
      Gitlab::Ci::Pipeline::Chain::StopDryRun,

Or possibly using a slightly different SEQUENCE of steps for the policy pipeline.

WDYT?

tag:gitlab.com,2026-03-17:5214339073 Martin Cavoj pushed to project branch 577475-policy-job-ignores-needs-order-when-dependent-from-parallel-matrix at GitLab.org / GitLab 2026-03-17T18:51:04Z mcavoj Martin Cavoj

Martin Cavoj (c5f7a371) at 17 Mar 18:51

Combine all job mappings to support cross-policy needs references

... and 7578 more commits

tag:gitlab.com,2026-03-17:5213528000 Martin Cavoj commented on merge request !227096 at GitLab.org / GitLab 2026-03-17T15:27:32Z mcavoj Martin Cavoj

@mc_rocha could you please set to merge? I pushed a small change to the migration which removed your approval. Thanks!

tag:gitlab.com,2026-03-17:5213256580 Martin Cavoj commented on merge request !227114 at GitLab.org / GitLab 2026-03-17T14:35:30Z mcavoj Martin Cavoj

@arfedoro I've just noticed - this is not a user-facing change so we shouldn't add a changelog for it.

tag:gitlab.com,2026-03-17:5213237286 Martin Cavoj approved merge request !227114: [MR 1/5] Add prevent_editing_approval_rules to policy approval settings schema at GitLab.org / GitLab 2026-03-17T14:31:35Z mcavoj Martin Cavoj

What does this MR do and why?

Adds prevent_editing_approval_rules boolean to the policy approval settings JSON schemas and the ApprovalSettings value object.

This is MR 1 of 5 — a data-layer only change with no enforcement behavior. It unblocks all other MRs.

Changes

File Change
ee/app/validators/json_schemas/approval_policy_content.json Add prevent_editing_approval_rules: boolean to approval_settings.properties
app/validators/json_schemas/scan_result_policy_project_approval_settings.json Add prevent_editing_approval_rules: boolean to properties
ee/lib/security/scan_result_policies/approval_settings.rb Add prevent_editing_approval_rules accessor method

Verification steps

  1. Create a project

  2. Go to Secure Policies

  3. In the project, create an approval policy:

    approval_policy:
      - name: Prevent editing approval rules
        description: ''
        enabled: true
        actions:
          - type: require_approval
            approvals_required: 1
            role_approvers:
              - owner
              - maintainer
          - type: send_bot_message
            enabled: true
        rules:
          - type: any_merge_request
            branch_type: protected
            commits: any
        approval_settings:
          block_branch_modification: false
          prevent_pushing_and_force_pushing: false
          prevent_approval_by_author: false
          prevent_approval_by_commit_author: false
          remove_approvals_with_new_commit: false
          require_password_to_approve: false
          prevent_editing_approval_rules: true
        fallback_behavior:
          fail: closed
        enforcement_type: enforce
    
  4. In rails console, verify that prevent_editing_approval_rules is persisted correctly:

    Security::Policy.last.approval_policy.approval_settings.prevent_editing_approval_rules # => true

Part of #593362 Related to #588291

Changelog: added EE: true

tag:gitlab.com,2026-03-17:5213236534 Martin Cavoj commented on merge request !227114 at GitLab.org / GitLab 2026-03-17T14:31:26Z mcavoj Martin Cavoj

@arfedoro thanks, looks good! At this point, there's nothing much to control with a feature flag, but it would be useful in a follow-up MR for the enforcement.

Can we add verification steps in the MR description? Something like:

  1. Create a project
  2. In the project, create an approval policy:
    approval_policy:
      - name: Prevent editing approval rules
        description: ''
        enabled: true
        actions:
          - type: require_approval
            approvals_required: 1
            role_approvers:
              - owner
              - maintainer
          - type: send_bot_message
            enabled: true
        rules:
          - type: any_merge_request
            branch_type: protected
            commits: any
        approval_settings:
          block_branch_modification: false
          prevent_pushing_and_force_pushing: false
          prevent_approval_by_author: false
          prevent_approval_by_commit_author: false
          remove_approvals_with_new_commit: false
          require_password_to_approve: false
          prevent_editing_approval_rules: true
        fallback_behavior:
          fail: closed
        enforcement_type: enforce
    
  3. In rails console, verify that prevent_editing_approval_rules is persisted correctly:
    Security::Policy.last.approval_policy.approval_settings.prevent_editing_approval_rules # => true

@dblessing could you please take over the maintainer review?

tag:gitlab.com,2026-03-17:5213174255 Martin Cavoj commented on merge request !227096 at GitLab.org / GitLab 2026-03-17T14:19:56Z mcavoj Martin Cavoj

@bauerdominic done in !227096 (3b2b66b0).

tag:gitlab.com,2026-03-17:5213169537 Martin Cavoj pushed to project branch 592731-persist-per-project-schedules-for-sep-1 at GitLab.org / GitLab 2026-03-17T14:18:55Z mcavoj Martin Cavoj

Martin Cavoj (3b2b66b0) at 17 Mar 14:18

Add not null default 0

tag:gitlab.com,2026-03-17:5213064073 Martin Cavoj approved merge request !226138: Consider branch rules on merge request approval rule and policy violation registration at GitLab.org / GitLab 2026-03-17T13:58:57Z mcavoj Martin Cavoj

What does this MR do and why?

Security approval policies can be configured with branch rules to target specific branches (e.g., branches: ['feature/*']). Currently, when a merge request is created, approval rules and violations are registered for all policies, regardless of whether the policy's branch rules match the MR's target branch. Filtering only occurs later during evaluation in ApprovalState#wrapped_rules.

This causes several issues:

  • Policy Bot comments may show false positive violations for policies that do not apply to the target branch. ( fixed in #553183 )
  • Unnecessary violation records are created in the database.
  • UI inconsistencies, where non-applicable policies appear as unviolated (e.g., in the Approval Widget).
  • False positive audit events which are generated based on policy violations

This MR introduces pre-filtering based on branch rules before registering policy violations and creating merge request rules.

When the security_policy_approval_rule_branch_filtering feature flag is enabled:

  • For scan_result_policies, ApprovalProjectRule checks applies_to_branch? before creating the merge request approval rule and violation.
  • If a policy does not apply to the target branch, no approval rule or violation is created.
  • If an existing rule or violation becomes applicable or inapplicable (e.g., if the target branch changes), it is created or deleted accordingly.

Side effects

This is a backend change and the merge request approval rules as well as the security policies feature are expected to work as before. However, there is one side affect of this change.

Approval overrides are applied based on persisted violations. Previously, overrides were applied regardless of the target branch because violations were registered even when policies did not apply.

This MR corrects that behavior. Approval overrides are now enforced only when the policy applies to the merge request’s target branch according to its branch rules.

See the discussion here for more on this: #553189 (comment 2903710412)

Spec changes

The ApprovalProjectRule#apply_report_approver_rules_to method now checks whether the approval rule applies to the merge request's target branch before creating or updating the MR approval rule.

If the branch is not protected or the rule doesn't apply to that branch, the method deletes any existing rules/violations and returns nil instead of a rule object.

This change impacts all services that eventually call apply_report_approver_rules_to:

  • UpdateApprovalsServicePolicyRuleEvaluationService#saveMergeRequest#reset_required_approvalsapply_report_approver_rules_to
  • GroupProjectTransferWorkerLinkProjectServiceSyncMergeRequestsServiceapply_report_approver_rules_to
  • MergeRequests::ReopenServiceresync_policiesMergeRequest#reset_required_approvalsapply_report_approver_rules_to
  • MergeRequests::UpdateServiceschedule_policy_synchronizationMergeRequest#sync_project_approval_rules_for_*apply_report_approver_rules_to
  • SyncReportApproverApprovalRulesMergeRequest#synchronize_approval_rules_from_target_projectapply_report_approver_rules_to
  • SyncMergeRequestsServicesync_merge_requestMergeRequest#sync_project_approval_rules_for_approval_policy_rulesapply_report_approver_rules_to

All these services share a common downstream dependency on apply_report_approver_rules_to. To ensure tests pass, each affected spec was updated to:

  1. Create a protected_branch matching the merge request's target branch
  2. Add the :for_all_protected_branches trait to approval_project_rule factories

Screenshots or screen recordings

Before After
image image

Database Queries

New queries:

Check if any existing violations:

SELECT 1 AS one FROM "scan_result_policy_violations" INNER JOIN "scan_result_policies" ON "scan_result_policy_violations"."scan_result_policy_id" = "scan_result_policies"."id" WHERE "scan_result_policies"."id" = 23 AND "scan_result_policy_violations"."merge_request_id" = 140 LIMIT 1

explain: https://console.postgres.ai/gitlab/gitlab-production-main/sessions/49749/commands/148126

Deleting Violations:

DELETE FROM "scan_result_policy_violations"
WHERE "scan_result_policy_violations"."id" IN (
  SELECT "scan_result_policy_violations"."id"
  FROM "scan_result_policy_violations"
  INNER JOIN "scan_result_policies"
    ON "scan_result_policy_violations"."scan_result_policy_id" = "scan_result_policies"."id"
  WHERE "scan_result_policies"."id" = 23
    AND "scan_result_policy_violations"."merge_request_id" = 140
);

Explain: https://console.postgres.ai/gitlab/gitlab-production-main/sessions/49749/commands/148125 (It returns 0 rows, as we can't know the scan_result_policies.id since it is not exposed through APIs)

Delete Approval Rules

DELETE FROM "approval_merge_request_rules"
WHERE ("approval_merge_request_rules"."id")
    IN (
        SELECT
            "approval_merge_request_rules"."id"
        FROM
            "approval_merge_request_rules"
            INNER JOIN "approval_merge_request_rule_sources" ON "approval_merge_request_rule_sources"."approval_merge_request_rule_id" = "approval_merge_request_rules"."id"
        WHERE
            "approval_merge_request_rules"."merge_request_id" = 16248692
            AND "approval_merge_request_rules"."rule_type" = 3
            AND "approval_merge_request_rule_sources"."approval_project_rule_id" = 47696360)

Query plan: https://console.postgres.aiundefined/shared/322fbb4b-07b8-459e-a879-ce6536d7dea6

References

Spike issue #553189

#583612

How to set up and validate locally

  1. Enable the feature flag by running:
bin/rails runner "Feature.enable(:security_policy_approval_rule_branch_filtering)"
  1. Create a project with security policies
  • Secret detection - protected branch
  • SAST scan - feature branch
Policy YAML

.gitlab/security-policies/policy.yml

---
approval_policy:
- name: SAST scan - feature branch
  description: ''
  enabled: true
  rules:
  - type: scan_finding
    scanners:
    - sast
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
    branches:
    - feature/*
  actions:
  - type: require_approval
    approvals_required: 1
    role_approvers:
    - developer
    - maintainer
    - owner
  - type: send_bot_message
    enabled: true
  approval_settings:
    block_branch_modification: false
    prevent_pushing_and_force_pushing: false
    prevent_approval_by_author: true
    prevent_approval_by_commit_author: false
    remove_approvals_with_new_commit: false
    require_password_to_approve: false
  fallback_behavior:
    fail: closed
- name: Secret Detection - Protected Branch
  description: ''
  enabled: true
  enforcement_type: enforce
  rules:
  - type: scan_finding
    scanners:
    - type: secret_detection
      vulnerabilities_allowed: 0
      severity_levels:
      - critical
      - high
      vulnerability_states:
      - new_needs_triage
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
    branch_type: protected
  actions:
  - type: require_approval
    approvals_required: 1
    group_approvers_ids:
    - 24
  - type: send_bot_message
    enabled: true
  approval_settings:
    block_branch_modification: false
    prevent_pushing_and_force_pushing: false
    prevent_approval_by_author: false
    prevent_approval_by_commit_author: false
    remove_approvals_with_new_commit: false
    require_password_to_approve: false
  fallback_behavior:
    fail: closed

.gitlab-ci.yml

image: busybox:latest
include:
  - template: 'Jobs/Secret-Detection.gitlab-ci.yml'
  - template: 'Jobs/SAST.gitlab-ci.yml'
stages:
  - test

test_job:
  stage: test
  script:
    - echo "Running tests..."

variables:
  AST_ENABLE_MR_PIPELINES: "true"
  1. Navigate to project -> settings -> Protected Branches and add a protected branch feature/*

  2. Add the following 2 files in a branch and create MR targeting main branch

env.sample

AWS_TOKEN=AKIAZYONPI3G4JNCCWGQ

test.rb

job = params[:job]
eval(job)

Observation: only the Secret detection - protected branch is violated. Approval rules and policy bot comment is recorded accordingly.

  1. Create a feature branch (feature/test-branch) in the repository and Change the MR target branch to it

Observation: Both (Secret detection - protected branch and SAST scan - feature branch ) policy is violated. Approval rules and policy bot comment is updated accordingly

  1. To test, approval rule overwrites are working correctly
  • Navigate to project -\> settings -\> Merge requests -\> Merge request approvals : Uncheck all boxes
  • The MR is not allowed to be approved by merge request author when Secret detection - protected branch is violated since the policy has prevent_approval_by_author: true overwrite.
  • Change the target branch to main , now the MR is allowed to be approved by merge request author.
  1. Code coverage rules are working as expected.

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.

Related to #583612

tag:gitlab.com,2026-03-17:5213060908 Martin Cavoj approved merge request !224319: Add PEP/SEP mergeability check for security policy pipeline status at GitLab.org / GitLab 2026-03-17T13:58:25Z mcavoj Martin Cavoj

What does this MR do and why?

Adds a new PEP/SEP-scoped mergeability check (security_policy_pipeline_check) that ensures all pipelines for the latest commit must succeed before a merge request can be merged when security policies are enforced.

image

Problem

When Pipeline Execution Policies (PEP) or Scan Execution Policies (SEP) inject security scans into MR pipelines, projects with existing branch pipelines end up with duplicate pipelines. The existing mergeability check only evaluates the MR pipeline (which includes the security scans), completely ignoring the branch pipeline where functional tests may be failing. This allows broken code to be merged even when branch pipelines are red.

Solution

Introduces a new CheckSecurityPolicyPipelineStatusService mergeability check that:

  • Activates only when the project has security_orchestration_policies license, has PEP/SEP policy configurations (scoped to type_scan_execution_policy or type_pipeline_execution_policy to avoid activating for projects with only approval or vulnerability management policies), and at least one internal pipeline exists for the latest MR commit
  • Excludes external pipelines (created via the Commit Status API) to prevent bypassing the security policy check by setting a commit status to success (see #535437)
  • Evaluates both the latest MR pipeline (merge_request_event source) and the latest branch pipeline (all other sources) for the HEAD SHA
  • Supports merged results pipelines by using for_sha_or_source_sha to match pipelines where the SHA is a merge commit but source_sha is the branch HEAD
  • Blocks or warns based on the project's "Pipelines must succeed" setting:
    • Setting enabled: hard block (failure) if any evaluated pipeline fails
    • Setting disabled: soft warning if any evaluated pipeline fails, user can still merge
  • Respects the "Skipped pipelines are considered successful" setting (allow_merge_on_skipped_pipeline), consistent with CheckCiStatusService
  • Waits (returns checking) if any evaluated pipeline is still active
  • Is non-cacheable since pipeline statuses change dynamically
  • Is skipped during auto-merge flows via skip_security_policy_pipeline_check
  • Is gated behind a feature flag security_policy_pipeline_check (disabled by default)

Changes

  • New service: ee/app/services/merge_requests/mergeability/check_security_policy_pipeline_status_service.rb - core mergeability check logic
  • New scope: with_security_policy_jobs on Ci::Pipeline to efficiently query pipelines that have policy-injected builds
  • MR model: registers the new check in mergeable_state_checks and adds skip_security_policy_pipeline_check to auto-merge skip options
  • GraphQL: new SECURITY_POLICY_PIPELINE_CHECK enum value in DetailedMergeStatusEnum and MergeabilityCheckIdentifier
  • Feature flag: security_policy_pipeline_check (gitlab_com_derisk, disabled by default)
  • Docs & schema: updated GraphQL reference docs and introspection JSON

Note: Frontend changes (merge check component, contextual pipeline note, CSS, and frontend constants) have been removed from this MR and will be handled separately.

References

Closes #589650

How to set up and validate locally

  1. Enable the security_orchestration_policies license feature
  2. Enable the security_policy_pipeline_check feature flag
  3. Create a project with a PEP or SEP policy configuration
  4. Open a merge request and trigger a pipeline that has policy-injected jobs
  5. Observe the new SECURITY_POLICY_PIPELINE_CHECK mergeability check identifier via the GraphQL API
  6. Verify that a failing branch pipeline blocks (or warns on) the merge, even when the MR pipeline succeeds

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.

tag:gitlab.com,2026-03-17:5213044116 Martin Cavoj commented on merge request !227096 at GitLab.org / GitLab 2026-03-17T13:55:02Z mcavoj Martin Cavoj

@bauerdominic good call, I mirrored this from PEP where we couldn't do it, as we need to differentiate between self-healed records and the old ones that need to use the old scheduling, and I moved the NOT NULL DEFAULT 0 into a cleanup issue.

However, for SEP project schedules, we can add that right away 👍

tag:gitlab.com,2026-03-17:5213025573 Martin Cavoj approved merge request !225769: Track CI catalog component usage in pipeline execution policy pipelines at GitLab.org / GitLab 2026-03-17T13:51:18Z mcavoj Martin Cavoj

What does this MR do and why?

CI catalog component usage statistics were not being tracked when components were included via pipeline execution policy pipelines. Since policy pipelines run as separate child pipelines, their included_components were invisible to the ComponentUsage chain step running on the parent pipeline.

This MR fixes the gap by:

  • Adding an included_components attribute to Security::PipelineExecutionPolicy::Pipeline to store which catalog components were used by each policy pipeline
  • Capturing included_components from yaml_processor_result after each policy pipeline is created in PipelineContext#build_policy_pipelines!
  • Exposing policy_pipelines_included_components to aggregate and deduplicate components across all policy pipelines
  • Skipping ComponentUsage when running inside a policy pipeline (creating_policy_pipeline?) to avoid double-counting, since each policy pipeline runs its own full CreatePipelineService chain — the project pipeline's ComponentUsage step (via EE override) then merges in all policy pipeline components and tracks them in a single call to TrackComponentUsageWorker

Note: This change was proposed by GitLab Duo based on the observed undercounting problem. While I understand the approach taken here, I'm aware there may be alternative solutions — reviewer input on the overall approach is very welcome.

References

The CI/CD Catalog currently shows only 6 projects using the component, while the AI hackathon participants group has 100+ projects that used that CI component via a pipeline execution policy — none of which are being counted in the usage statistics. Note that this fix only corrects the undercounting for future pipeline runs — past runs are unaffected.

Screenshot 2026-03-03 at 21.22.14.png

Screenshots or screen recordings

N/A - backend only change

How to set up and validate locally

  1. Create a CI catalog component in a project
  2. Create a pipeline execution policy that includes that component
  3. Trigger a pipeline on a project with that policy applied
  4. Verify Ci::Catalog::Resources::TrackComponentUsageWorker is enqueued with both the project's own components and the policy pipeline's components

Open questions

  • Should we add an integration-level test in ee/spec/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb that exercises the full flow (policy enforces a CI file using a catalog component → TrackComponentUsageWorker is enqueued with the correct data)? I did not add one to keep the scope focused, but happy to if reviewers feel it is needed.

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.

tag:gitlab.com,2026-03-17:5213025273 Martin Cavoj commented on merge request !225769 at GitLab.org / GitLab 2026-03-17T13:51:15Z mcavoj Martin Cavoj

@furkanayhan looks great to me too, thanks @mmichaux-ext!

tag:gitlab.com,2026-03-17:5213016465 Martin Cavoj commented on merge request !224663 at GitLab.org / GitLab 2026-03-17T13:49:26Z mcavoj Martin Cavoj

@alan I found it - it was in the MR !220685 (comment 3057874752) and this is the issue: #589398.

tag:gitlab.com,2026-03-17:5213006715 Martin Cavoj commented on merge request !224663 at GitLab.org / GitLab 2026-03-17T13:47:32Z mcavoj Martin Cavoj

@alan I think that's a good approach 👍

tag:gitlab.com,2026-03-17:5212823500 Martin Cavoj commented on merge request !224324 at GitLab.org / GitLab 2026-03-17T13:12:08Z mcavoj Martin Cavoj

Thanks @alan for working on this and apologies for the delay! I left a few comments 🏓

tag:gitlab.com,2026-03-17:5212823463 Martin Cavoj commented on merge request !224324 at GitLab.org / GitLab 2026-03-17T13:12:07Z mcavoj Martin Cavoj

suggestion: we should add a test for two competing policies to cover the fact that the highest severity wins.

tag:gitlab.com,2026-03-17:5212823439 Martin Cavoj commented on merge request !224324 at GitLab.org / GitLab 2026-03-17T13:12:07Z mcavoj Martin Cavoj

suggestion: this doesn't appear to be used.

tag:gitlab.com,2026-03-17:5212823416 Martin Cavoj commented on merge request !224324 at GitLab.org / GitLab 2026-03-17T13:12:07Z mcavoj Martin Cavoj

question: we match this as "override already applied", although the applied override was for medium severity and our finding is high. Is that an issue? Should we match the severity? I don't know if finding's severity can change over time.

tag:gitlab.com,2026-03-17:5212823391 Martin Cavoj commented on merge request !224324 at GitLab.org / GitLab 2026-03-17T13:12:07Z mcavoj Martin Cavoj

suggestion(non-blocking): this is redundant, as it's the same as the content defined above