Fix(dependency-firewall): implement full BypassSettings interface

Michael Eddingtonrequested to merge
mikeeddington/gitlab:fix/df-bypass-settings-schema-parity into master

Summary

Every git push to a project with an active Dependency Firewall policy returns:

remote: GitLab: 500 Internal Server Error
! [remote rejected] main -> main (pre-receive hook declined)

because Security::ScanResultPolicies::UserBypassChecker#roles_can_bypass? calls .default_roles on the policy's bypass_settings, and Security::DependencyFirewallPolicies::BypassSettings does not implement that method (nor service_account_ids, called by Security::ScanResultPolicies::BotBypassChecker).

This is a hard blocker for any git activity on a project with a DF policy attached (git push from CLI / SSH, MR creation, CI pipelines pushing built artifacts, imports).

Full backtrace and reproduction details on issue #606449.

Root cause

ee/lib/security/scan_result_policies/push_bypass_checker.rb:30 walks project.security_policies.with_bypass_settings without a type filter, so any DF policy attached to the project is passed through the approval-policy bypass-checker stack, which invokes members that only Security::ScanResultPolicies::BypassSettings implements.

The clean fix is to type-filter the query (add .type_approval_policy in push_bypass_checker.rb:30 and ee/app/models/ee/merge_request.rb:573), so DF policies never enter SCM push-bypass logic. That change requires product-level coordination with the Security Policies team about whether DF bypass_settings are ever intended to gate SCM operations.

What this MR does (interim additive fix)

Extends Security::DependencyFirewallPolicies::BypassSettings to implement the full public interface of its Security::ScanResultPolicies::BypassSettings sibling. The additions are pure schema parity — every added method mirrors the SRP-side implementation exactly.

Added members:

  • service_account_ids
  • branches
  • group_ids
  • default_roles
  • custom_role_ids
  • users_and_groups_empty?
  • bypass_actors_empty?

Behavioural impact:

  • git push: stops returning 500 for DF-policy projects (the push-bypass checker now completes normally against DF policies, returning false because DF bypass_settings contain no bypass actors by default).
  • DF enforcement (npm/pypi/maven/etc. downloads and uploads): unchanged. Security::DependencyFirewall::PolicyEvaluator#evaluate_policies only calls user_bypassed? and access_token_bypassed?, both of which were already implemented and are left untouched.
  • Semantic consequence: DF bypass_settings.users / bypass_settings.access_tokens now silently function as SCM push-bypass sources. If DF's product intent is that bypass_settings scope only to package-registry enforcement, the type-filter fix above is the correct long-term solution and this MR should be reverted then.

Verification

Verified live on gdk-in-a-box (GitLab 19.3.pre, Ultimate, dependency_firewall_phase1 FF enabled, root-namespace dependency_firewall_enabled set):

Scenario Before After
git push to DF-policy project (fires POST /api/v4/internal/allowed → PushBypassChecker) 500 (NoMethodError: undefined method 'default_roles' in user_bypass_checker.rb:42) 200, push accepted
glab dependency-firewall npm ci on @dfsmoke/df-block 403 (clean block, DF policy-violation message) 403 (unchanged)
Exceptions in production.log / exceptions_json.log during either operation many zero

Specs

Extends ee/spec/lib/security/dependency_firewall_policies/bypass_settings_spec.rb with coverage for every new method, mirroring the coverage in ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb.

Also adds an interface-parity spec that asserts every public instance method of Security::ScanResultPolicies::BypassSettings is implemented on Security::DependencyFirewallPolicies::BypassSettings. This catches future drift at CI time rather than at request time.

Related

  • Issue: gitlab-org/gitlab#606449 — full audit of security_policies polymorphic-association filter gaps (six other suspicious sites documented; not fixed here).

Reviewer notes

  • No DB migrations.
  • No public API changes.
  • Additive to a single class + its spec. No production code was modified outside ee/lib/security/dependency_firewall_policies/bypass_settings.rb.
  • Pipeline may exercise the interface-parity spec against the SRP class as it evolves; if SRP ever adds a new public method, the parity spec will fail here and prompt the parallel addition on the DF side.

Merge request reports