Sure thing. Looks good for Merge Requests backend

Found some code not behind FF, but looks like all minor changes, so no need to worry too much.

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:

• UpdateApprovalsService → PolicyRuleEvaluationService#save → MergeRequest#reset_required_approvals → apply_report_approver_rules_to
• GroupProjectTransferWorker → LinkProjectService → SyncMergeRequestsService → apply_report_approver_rules_to
• MergeRequests::ReopenService → resync_policies → MergeRequest#reset_required_approvals → apply_report_approver_rules_to
• MergeRequests::UpdateService → schedule_policy_synchronization → MergeRequest#sync_project_approval_rules_for_* → apply_report_approver_rules_to
• SyncReportApproverApprovalRules → MergeRequest#synchronize_approval_rules_from_target_project → apply_report_approver_rules_to
• SyncMergeRequestsService → sync_merge_request → MergeRequest#sync_project_approval_rules_for_approval_policy_rules → apply_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

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

0. 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

---
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

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"

2. Navigate to project -> settings -> Protected Branches and add a protected branch feature/*

3. 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.

4. 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

5. 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.

6. 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

What does this MR do and why?

Add SAML signed out notification to gPAT page

Because groups that require SAML sign-in are filtered from view when not signed in, show a notification with a signing button on gPAT create page.

References

Issue: #593699

Screenshots or screen recordings

Before	After	After SAML Login

How to set up and validate locally

1. Feature.enable(:granular_personal_access_tokens) in Rails console
2. Setup SAML locally
3. Create an Ultimate licensed group with SAML enabled and "Enforce SSO-only authentication for web activity" checked
4. Invite user_1 to that group
5. Sign in as user_1 via SAML (go to the group, get redirected to SAML, authenticate with user1 / user1pass) — this creates the group_saml identity
6. Sign out, sign back in as admin and impersonate user_1
7. Visit https://gdk.test:3000/-/user_settings/personal_access_tokens/granular/new

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.

Cool thanks. Enabling FF fixed the issue 404. Looks good with my local testing. See the banner initially, and no group shows up before SSO authentication. After auth, see the group as below.

Approving now 🚀

Hi @marcogreg Could you pls do backend maintainer review pls?

Hi @alexbuijs 👋 The changes LGTM.

However, I am not able to reproduce the same behaviour locally 🤔 I should have HTTPs and SAML configured fine.

As admin, I could go to the group settings, and see below configuration.

When I login as the test user, if i go to the group, it would ask me to verify SAML SSO fine.

But this page https://gdk.test:3000/-/user_settings/personal_access_tokens/granular/new shows 404: Page not found for both admin and test user.

I think we are missing some steps somewhere? 🤔

zli (8e390341) at 17 Mar 00:35

... and 1624 more commits

zli (e23aa6ab) at 16 Mar 23:38

Hi @krasio I have updated the code as your comments, and also local testing succeed. Will run GitLab.com DB Testing job soon.

I was just trying to minimize the changes, so did not move the partitioned FK out of the swap block. 😥 Updated to standard approach now.

Also, I fail to see where we remove the original key, yet it's gone after the migrations. Can you please point me where to look?

Yeah, previously when we remove PK, it will also drop the dependent FK. Now I have updated to drop this FK explicitly like others.

Ahh, completely missed from my side. Wonder how do you captured this? Is it from the new schema file generated?

Updated with a new migration for adding this comment.

Nice catch! Sorry I just tried up and down works fine, but forgot that we should keep data consistent after each up and down. Updated now, and quite similar to the linked implementation.

zli (b141528d) at 16 Mar 09:44

Thanks for the MR, and database LGTM 👍

What does this MR do and why?

Converts the hard foreign key ai_instance_accessible_entity_rules.through_namespace_id → namespaces(id) to a loose foreign key, resolving the cross-schema reference (gitlab_main_cell_setting → gitlab_main_org) introduced when this table was reclassified.

The ON DELETE CASCADE behavior is replicated via the async cleanup mechanism. The namespaces table already has the LFK deletion tracking trigger installed.

Changes

• Added async_delete entry in config/gitlab_loose_foreign_keys.yml
• Post-deploy migration to remove the hard FK (fk_rails_43a6361a35)
• Removed entry from cross-DB FK allowlist in no_cross_db_foreign_keys_spec.rb
• Added 'cleanup by a loose foreign key' shared example spec

Closes #592220

References

• Issue: #592220
• Loose foreign keys documentation

Screenshots or screen recordings

Not applicable (no UI changes).

How to set up and validate locally

1. Run the migration:

   bin/rake db:migrate

2. Verify the FK is removed:

   SELECT * FROM pg_constraint WHERE conname = 'fk_rails_43a6361a35';
   -- Should return 0 rows

3. Run the specs:

   bin/rspec ee/spec/models/ai/feature_access_rule_spec.rb
   bin/rspec spec/lib/gitlab/database/no_cross_db_foreign_keys_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.

Thanks for addressing those nitpicks, LGTM and approved 🙏