Reject cross-organization compliance framework relink

What does this MR do?

ComplianceManagement::ComplianceFramework::SecurityPolicy.relink used insert_all to link a security policy to compliance frameworks, bypassing model validations with no organization boundary check. A policy could relink itself to a compliance framework belonging to a different organization.

This filters framework_policy_attrs down to frameworks in the same organization as the policy before calling insert_all.

Production impact today: none. GitLab.com is single-organization, so this closes a gap that only becomes exploitable once multi-organization/Cells ships.

References

Relates to https://gitlab.com/gitlab-com/gl-infra/tenant-scale/organizations/organizations-feature-parity/-/work_items/102 (finding "Compliance framework cross-linking has no org boundary")

Database review

New query, added in ComplianceManagement::Framework.ids_in_organization (ee/app/models/compliance_management/framework.rb), used to filter framework_policy_attrs down to frameworks belonging to the policy's organization. Exact SQL executed (captured via sql.active_record notification, not just #to_sql on the relation, since .pluck(:id) narrows the SELECT):

SELECT "compliance_management_frameworks"."id"
FROM "compliance_management_frameworks"
INNER JOIN "namespaces" ON "namespaces"."id" = "compliance_management_frameworks"."namespace_id"
WHERE "compliance_management_frameworks"."id" IN (1, 2, 3)
  AND "namespaces"."organization_id" = 1
LIMIT 3

framework_ids (the IN (...) list and the LIMIT) come from the caller's own framework_policy_attrs — a policy's policy_scope.compliance_frameworks list, hand-authored in the policy YAML, not unbounded input. The LIMIT is framework_ids.size, a true upper bound: id_in can't match more rows than the distinct ids requested.

EXPLAIN (ANALYZE, BUFFERS) (3 ids, local dev DB):

 Limit  (cost=0.85..18.75 rows=3 width=8) (actual time=6.831..10.255 rows=2 loops=1)
   Buffers: shared hit=5 read=8
   I/O Timings: read=10.114 write=0.000
   ->  Nested Loop  (cost=0.85..18.75 rows=3 width=8) (actual time=6.828..10.249 rows=2 loops=1)
         Buffers: shared hit=5 read=8
         I/O Timings: read=10.114 write=0.000
         ->  Index Scan using compliance_management_frameworks_pkey on public.compliance_management_frameworks  (cost=0.29..7.99 rows=3 width=12) (actual time=3.804..3.811 rows=2 loops=1)
               Index Cond: (compliance_management_frameworks.id = ANY ('{1,2,3}'::bigint[]))
               Buffers: shared read=3
               I/O Timings: read=3.770 write=0.000
         ->  Index Scan using namespaces_pkey on public.namespaces  (cost=0.57..3.59 rows=1 width=4) (actual time=3.213..3.213 rows=1 loops=2)
               Index Cond: (namespaces.id = compliance_management_frameworks.namespace_id)
               Filter: (namespaces.organization_id = 1)
               Buffers: shared hit=5 read=5
               I/O Timings: read=6.345 write=0.000
Settings: effective_cache_size = '472585MB', jit = 'off', random_page_cost = '1.5', work_mem = '230MB', seq_page_cost = '4'
Query ID: 6663165462740610988

https://postgres.ai/console/gitlab/gitlab-production-main/sessions/55792/commands/159628

Both sides of the join hit an index (compliance_management_frameworks primary key range, namespaces_pkey); no sequential scans. Row counts here are bounded by the number of frameworks referenced in one policy's scope, so this stays cheap regardless of table size.

How to test

bundle exec rspec ee/spec/models/compliance_management/compliance_framework/security_policy_spec.rb

Edited by Alan (Maciej) Paruszewski

Merge request reports

Loading
Loading