From c8ec83ee43f65dae16d9e0bfd23b09ad4a42813a Mon Sep 17 00:00:00 2001
From: michaelangeloio <arivera@gitlab.com>
Date: Thu, 11 Dec 2025 21:51:48 -0500
Subject: [PATCH 1/5] Authz Redaction Service for Knowledge Graph

Introduces a service to check Ability.allowed on various
resources to be used in redaction and client consumers.
---
 app/services/authz/redaction_service.rb       | 189 ++++++++++++
 ee/app/services/ee/authz/redaction_service.rb |  66 ++++
 .../ee/authz/redaction_service_spec.rb        | 181 +++++++++++
 spec/services/authz/redaction_service_spec.rb | 282 ++++++++++++++++++
 4 files changed, 718 insertions(+)
 create mode 100644 app/services/authz/redaction_service.rb
 create mode 100644 ee/app/services/ee/authz/redaction_service.rb
 create mode 100644 ee/spec/services/ee/authz/redaction_service_spec.rb
 create mode 100644 spec/services/authz/redaction_service_spec.rb

diff --git a/app/services/authz/redaction_service.rb b/app/services/authz/redaction_service.rb
new file mode 100644
index 00000000000000..9305dae8f2bc63
--- /dev/null
+++ b/app/services/authz/redaction_service.rb
@@ -0,0 +1,189 @@
+# frozen_string_literal: true
+
+module Authz
+  # Service to perform batch authorization checks for resources.
+  #
+  # This service is designed to check whether a user has read access to a batch
+  # of resources using GitLab's standard Ability system. It's used by the
+  # Knowledge Graph service for final redaction (Layer 3) but is generic enough
+  # to be used by any service requiring batch authorization checks.
+  #
+  # The service follows the same authorization pattern used by SearchService
+  # for redacting search results.
+  #
+  # IMPORTANT: This service assumes that the user has already been authenticated
+  # and authorized to make API requests. It does NOT perform user-level validation
+  # (e.g., checking if user is blocked or deactivated). The caller is responsible
+  # for ensuring the user is valid before invoking this service.
+  #
+  # @example
+  #   service = Authz::RedactionService.new(
+  #     user: current_user,
+  #     resources_by_type: {
+  #       'issues' => [123, 456],
+  #       'merge_requests' => [789]
+  #     }
+  #   )
+  #   result = service.execute
+  #   # => {
+  #   #      'issues' => { 123 => true, 456 => false },
+  #   #      'merge_requests' => { 789 => true }
+  #   #    }
+  #
+  # @see app/services/search_service.rb:136-139 for visible_result? pattern
+  # @see app/models/ability.rb:42-71 for batch authorization methods
+  class RedactionService
+    include Gitlab::Allowable
+
+    # Mapping of resource type keys (plural) to their corresponding model classes.
+    # All models must implement #to_ability_name for consistent ability naming.
+    #
+    # @see app/models/concerns/issuable.rb:495-497 for Issuable implementation
+    RESOURCE_CLASSES = {
+      'issues' => ::Issue,
+      'merge_requests' => ::MergeRequest,
+      'projects' => ::Project,
+      'milestones' => ::Milestone,
+      'snippets' => ::Snippet
+    }.freeze
+
+    # Preload associations needed for authorization checks to prevent N+1 queries.
+    # Each resource type needs associations accessed by its policy during evaluation.
+    #
+    # @see app/policies/issue_policy.rb - needs project for most conditions
+    # @see app/policies/merge_request_policy.rb - needs target_project
+    # @see app/policies/project_policy.rb - accesses namespace, project_feature, group
+    PRELOAD_ASSOCIATIONS = {
+      'issues' => [{ project: [:namespace, :project_feature, :group] }, :author, :work_item_type],
+      'merge_requests' => [{ target_project: [:namespace, :project_feature, :group] }, :author],
+      'projects' => [:namespace, :project_feature, :group],
+      'milestones' => [{ project: [:namespace, :project_feature] }, :group],
+      'snippets' => [{ project: [:namespace, :project_feature] }, :author]
+    }.freeze
+
+    # Returns the list of supported resource types.
+    #
+    # @return [Array<String>] List of valid resource type keys
+    def self.supported_types
+      RESOURCE_CLASSES.keys
+    end
+
+    # @param user [User] The user to check permissions for
+    # @param resources_by_type [Hash<String, Array<Integer>>] Resources grouped by type
+    #   e.g., { 'issues' => [1, 2, 3], 'merge_requests' => [4, 5] }
+    # @param logger [Logger, nil] Optional logger for redaction audit logging
+    def initialize(user:, resources_by_type:, logger: nil)
+      @user = user
+      @resources_by_type = resources_by_type
+      @logger = logger
+    end
+
+    # Executes the batch authorization check.
+    #
+    # Processes each resource type, batch loads resources with preloading,
+    # then checks each resource's read permission using the standard
+    # Ability.allowed? method.
+    #
+    # Uses DeclarativePolicy.user_scope to optimize policy evaluation when
+    # checking multiple resources for the same user, following the pattern
+    # from Ability.issues_readable_by_user.
+    #
+    # @return [Hash<String, Hash<Integer, Boolean>>] Authorization results grouped by type
+    #   e.g., { 'issues' => { 1 => true, 2 => false }, 'merge_requests' => { 4 => true } }
+    # @see app/models/ability.rb:42-48
+    def execute
+      return {} if resources_by_type.empty?
+
+      # Load all resources with preloading to prevent N+1 queries
+      loaded_resources_by_type = load_all_resources
+
+      # Check permissions using user_scope for optimization
+      # DeclarativePolicy.user_scope caches policy evaluations for the same user
+      # @see app/models/ability.rb:45-47
+      DeclarativePolicy.user_scope do
+        resources_by_type.each_with_object({}) do |(type, ids), results|
+          results[type] = authorize_resources_of_type(type, ids, loaded_resources_by_type[type] || {})
+        end
+      end
+    end
+
+    private
+
+    attr_reader :user, :resources_by_type, :logger
+
+    # Loads all resources for all types with appropriate preloading.
+    #
+    # @return [Hash<String, Hash<Integer, ActiveRecord::Base>>] Loaded resources by type
+    def load_all_resources
+      resources_by_type.each_with_object({}) do |(type, ids), loaded|
+        loaded[type] = load_resources_for_type(type, ids)
+      end
+    end
+
+    # Loads resources for a single type with preloading.
+    #
+    # @param type [String] The resource type
+    # @param ids [Array<Integer>] The resource IDs
+    # @return [Hash<Integer, ActiveRecord::Base>] Loaded resources indexed by ID
+    # rubocop:disable CodeReuse/ActiveRecord -- Batch loading with preloads for authorization checks
+    def load_resources_for_type(type, ids)
+      return {} if ids.blank?
+
+      klass = RESOURCE_CLASSES[type]
+      return {} unless klass
+
+      preloads = PRELOAD_ASSOCIATIONS[type] || []
+      klass.where(id: ids).includes(*preloads).index_by(&:id)
+    end
+    # rubocop:enable CodeReuse/ActiveRecord
+
+    # Authorizes all resources of a single type.
+    #
+    # Checks authorization for each resource, using pre-loaded data.
+    # Returns a hash mapping resource IDs to authorization results.
+    #
+    # @param type [String] The resource type (e.g., 'issues', 'merge_requests')
+    # @param ids [Array<Integer>] The resource IDs to authorize
+    # @param loaded_resources [Hash<Integer, ActiveRecord::Base>] Pre-loaded resources
+    # @return [Hash<Integer, Boolean>] Map of resource ID to authorization result
+    def authorize_resources_of_type(type, ids, loaded_resources)
+      return {} if ids.blank?
+
+      klass = RESOURCE_CLASSES[type]
+      return ids.index_with { false } unless klass
+
+      ids.index_with do |id|
+        resource = loaded_resources[id]
+
+        # Resource not found - deny access
+        next false if resource.nil?
+
+        # Check visibility using the same pattern as SearchService
+        visible_result?(resource)
+      end
+    end
+
+    # Checks if a resource is visible to the user.
+    #
+    # This method is intentionally identical to SearchService#visible_result?
+    # to ensure consistent authorization behavior across search, the Knowledge Graph,
+    # and any other consuming features.
+    #
+    # @param resource [ActiveRecord::Base] The resource to check
+    # @return [Boolean] Whether the user can read the resource
+    # @see app/services/search_service.rb:136-139
+    def visible_result?(resource)
+      # Resources without policies are considered visible
+      # This handles edge cases like plain Ruby objects
+      return true unless resource.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(resource)
+
+      # Use the resource's to_ability_name to construct the read ability
+      # For Issue: :read_issue
+      # For MergeRequest: :read_merge_request
+      # etc.
+      Ability.allowed?(user, :"read_#{resource.to_ability_name}", resource)
+    end
+  end
+end
+
+Authz::RedactionService.prepend_mod_with('Authz::RedactionService')
diff --git a/ee/app/services/ee/authz/redaction_service.rb b/ee/app/services/ee/authz/redaction_service.rb
new file mode 100644
index 00000000000000..a29192dd31bb43
--- /dev/null
+++ b/ee/app/services/ee/authz/redaction_service.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+module EE
+  module Authz
+    module RedactionService
+      extend ::Gitlab::Utils::Override
+
+      # EE-specific resource types added to the base CE resource classes.
+      EE_RESOURCE_CLASSES = {
+        'epics' => ::Epic,
+        'vulnerabilities' => ::Vulnerability
+      }.freeze
+
+      # EE-specific preload associations for authorization checks.
+      # These include nested associations needed by policies.
+      #
+      # @see ee/app/policies/epic_policy.rb - delegates to group
+      # @see ee/app/policies/vulnerability_policy.rb - delegates to project
+      EE_PRELOAD_ASSOCIATIONS = {
+        'epics' => [:group],
+        'vulnerabilities' => [{ project: [:namespace, :project_feature, :group] }]
+      }.freeze
+
+      module ClassMethods
+        extend ::Gitlab::Utils::Override
+
+        override :supported_types
+        def supported_types
+          super + EE::Authz::RedactionService::EE_RESOURCE_CLASSES.keys
+        end
+      end
+
+      def self.prepended(base)
+        base.singleton_class.prepend ClassMethods
+      end
+
+      private
+
+      override :load_resources_for_type
+      # rubocop:disable CodeReuse/ActiveRecord -- Batch loading with preloads for authorization checks
+      def load_resources_for_type(type, ids)
+        return super unless EE_RESOURCE_CLASSES.key?(type)
+        return {} if ids.blank?
+
+        klass = EE_RESOURCE_CLASSES[type]
+        preloads = EE_PRELOAD_ASSOCIATIONS[type] || []
+        klass.where(id: ids).includes(*preloads).index_by(&:id)
+      end
+      # rubocop:enable CodeReuse/ActiveRecord
+
+      override :authorize_resources_of_type
+      def authorize_resources_of_type(type, ids, loaded_resources)
+        return super unless EE_RESOURCE_CLASSES.key?(type)
+        return {} if ids.blank?
+
+        ids.index_with do |id|
+          resource = loaded_resources[id]
+
+          next false if resource.nil?
+
+          visible_result?(resource)
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/services/ee/authz/redaction_service_spec.rb b/ee/spec/services/ee/authz/redaction_service_spec.rb
new file mode 100644
index 00000000000000..78f19024e4cf93
--- /dev/null
+++ b/ee/spec/services/ee/authz/redaction_service_spec.rb
@@ -0,0 +1,181 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authz::RedactionService, feature_category: :permissions do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:private_group) { create(:group, :private) }
+  let_it_be(:private_group_with_access) { create(:group, :private) }
+  let_it_be(:project) { create(:project, :public, group: group) }
+  let_it_be(:private_project) { create(:project, :private, group: private_group) }
+  let_it_be(:private_project_with_access) { create(:project, :private, group: private_group_with_access) }
+
+  before_all do
+    # Developer access needed for read_security_resource (vulnerabilities)
+    # Reporter access is sufficient for epics
+    private_group_with_access.add_developer(user)
+  end
+
+  before do
+    stub_licensed_features(epics: true, security_dashboard: true)
+  end
+
+  describe '.supported_types' do
+    it 'includes EE resource types' do
+      expect(described_class.supported_types).to include('epics', 'vulnerabilities')
+    end
+
+    it 'includes CE resource types' do
+      expect(described_class.supported_types).to include(
+        'issues', 'merge_requests', 'projects', 'milestones', 'snippets'
+      )
+    end
+  end
+
+  describe '#execute' do
+    subject(:result) { service.execute }
+
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type) }
+
+    context 'with epics' do
+      let_it_be(:public_epic) { create(:epic, group: group) }
+      let_it_be(:private_epic) { create(:epic, group: private_group) }
+      let_it_be(:accessible_epic) { create(:epic, group: private_group_with_access) }
+      let_it_be(:confidential_epic) { create(:epic, :confidential, group: private_group_with_access) }
+
+      context 'when user can access public epic' do
+        let(:resources_by_type) { { 'epics' => [public_epic.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'epics' => { public_epic.id => true } })
+        end
+      end
+
+      context 'when user cannot access private epic' do
+        let(:resources_by_type) { { 'epics' => [private_epic.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'epics' => { private_epic.id => false } })
+        end
+      end
+
+      context 'when user has group access' do
+        let(:resources_by_type) { { 'epics' => [accessible_epic.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'epics' => { accessible_epic.id => true } })
+        end
+      end
+
+      context 'when user has group access to confidential epic' do
+        let(:resources_by_type) { { 'epics' => [confidential_epic.id] } }
+
+        it 'allows access for group member' do
+          expect(result).to eq({ 'epics' => { confidential_epic.id => true } })
+        end
+      end
+
+      context 'when checking multiple epics at once' do
+        let(:resources_by_type) do
+          { 'epics' => [public_epic.id, private_epic.id, accessible_epic.id] }
+        end
+
+        it 'returns correct authorization for each epic' do
+          expect(result).to eq({
+            'epics' => {
+              public_epic.id => true,
+              private_epic.id => false,
+              accessible_epic.id => true
+            }
+          })
+        end
+      end
+
+      context 'with non-existent epic' do
+        let(:resources_by_type) { { 'epics' => [non_existing_record_id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'epics' => { non_existing_record_id => false } })
+        end
+      end
+    end
+
+    context 'with vulnerabilities' do
+      let_it_be(:accessible_vulnerability) { create(:vulnerability, project: private_project_with_access) }
+      let_it_be(:inaccessible_vulnerability) { create(:vulnerability, project: private_project) }
+
+      context 'when user has project access' do
+        let(:resources_by_type) { { 'vulnerabilities' => [accessible_vulnerability.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'vulnerabilities' => { accessible_vulnerability.id => true } })
+        end
+      end
+
+      context 'when user does not have project access' do
+        let(:resources_by_type) { { 'vulnerabilities' => [inaccessible_vulnerability.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'vulnerabilities' => { inaccessible_vulnerability.id => false } })
+        end
+      end
+
+      context 'when checking multiple vulnerabilities at once' do
+        let(:resources_by_type) do
+          { 'vulnerabilities' => [accessible_vulnerability.id, inaccessible_vulnerability.id] }
+        end
+
+        it 'returns correct authorization for each vulnerability' do
+          expect(result).to eq({
+            'vulnerabilities' => {
+              accessible_vulnerability.id => true,
+              inaccessible_vulnerability.id => false
+            }
+          })
+        end
+      end
+
+      context 'with non-existent vulnerability' do
+        let(:resources_by_type) { { 'vulnerabilities' => [non_existing_record_id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'vulnerabilities' => { non_existing_record_id => false } })
+        end
+      end
+    end
+
+    context 'with mixed CE and EE resource types' do
+      let_it_be(:public_issue) { create(:issue, project: project) }
+      let_it_be(:public_epic) { create(:epic, group: group) }
+      let_it_be(:accessible_vulnerability) { create(:vulnerability, project: private_project_with_access) }
+      let_it_be(:private_mr) { create(:merge_request, source_project: private_project) }
+
+      let(:resources_by_type) do
+        {
+          'issues' => [public_issue.id],
+          'epics' => [public_epic.id],
+          'vulnerabilities' => [accessible_vulnerability.id],
+          'merge_requests' => [private_mr.id]
+        }
+      end
+
+      it 'handles both CE and EE resource types correctly' do
+        expect(result).to eq({
+          'issues' => { public_issue.id => true },
+          'epics' => { public_epic.id => true },
+          'vulnerabilities' => { accessible_vulnerability.id => true },
+          'merge_requests' => { private_mr.id => false }
+        })
+      end
+    end
+
+    context 'with empty arrays for EE types' do
+      let(:resources_by_type) { { 'epics' => [], 'vulnerabilities' => [] } }
+
+      it 'returns empty hashes for those types' do
+        expect(result).to eq({ 'epics' => {}, 'vulnerabilities' => {} })
+      end
+    end
+  end
+end
diff --git a/spec/services/authz/redaction_service_spec.rb b/spec/services/authz/redaction_service_spec.rb
new file mode 100644
index 00000000000000..9845dee81c3919
--- /dev/null
+++ b/spec/services/authz/redaction_service_spec.rb
@@ -0,0 +1,282 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Authz::RedactionService, feature_category: :permissions do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:public_project) { create(:project, :public) }
+  let_it_be(:private_project) { create(:project, :private) }
+  let_it_be(:private_project_with_access) { create(:project, :private) }
+
+  before_all do
+    private_project_with_access.add_reporter(user)
+  end
+
+  describe '.supported_types' do
+    it 'returns the list of supported resource types' do
+      expect(described_class.supported_types).to include(
+        'issues', 'merge_requests', 'projects', 'milestones', 'snippets'
+      )
+    end
+  end
+
+  describe '#execute' do
+    subject(:result) { service.execute }
+
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type) }
+
+    context 'with empty resources' do
+      let(:resources_by_type) { {} }
+
+      it 'returns an empty hash' do
+        expect(result).to eq({})
+      end
+    end
+
+    context 'with issues' do
+      let_it_be(:public_issue) { create(:issue, project: public_project) }
+      let_it_be(:private_issue) { create(:issue, project: private_project) }
+      let_it_be(:accessible_issue) { create(:issue, project: private_project_with_access) }
+      let_it_be(:confidential_issue) { create(:issue, :confidential, project: private_project_with_access) }
+
+      context 'when user has access to public issue' do
+        let(:resources_by_type) { { 'issues' => [public_issue.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'issues' => { public_issue.id => true } })
+        end
+      end
+
+      context 'when user does not have access to private issue' do
+        let(:resources_by_type) { { 'issues' => [private_issue.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'issues' => { private_issue.id => false } })
+        end
+      end
+
+      context 'when user has project access' do
+        let(:resources_by_type) { { 'issues' => [accessible_issue.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'issues' => { accessible_issue.id => true } })
+        end
+      end
+
+      context 'when user has project access to confidential issue' do
+        let(:resources_by_type) { { 'issues' => [confidential_issue.id] } }
+
+        it 'allows access for project member' do
+          expect(result).to eq({ 'issues' => { confidential_issue.id => true } })
+        end
+      end
+
+      context 'when checking multiple issues at once' do
+        let(:resources_by_type) do
+          { 'issues' => [public_issue.id, private_issue.id, accessible_issue.id] }
+        end
+
+        it 'returns correct authorization for each issue' do
+          expect(result).to eq({
+            'issues' => {
+              public_issue.id => true,
+              private_issue.id => false,
+              accessible_issue.id => true
+            }
+          })
+        end
+      end
+    end
+
+    context 'with merge_requests' do
+      let_it_be(:public_mr) { create(:merge_request, source_project: public_project) }
+      let_it_be(:private_mr) { create(:merge_request, source_project: private_project) }
+      let_it_be(:accessible_mr) { create(:merge_request, source_project: private_project_with_access) }
+
+      context 'when user has access to public MR' do
+        let(:resources_by_type) { { 'merge_requests' => [public_mr.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'merge_requests' => { public_mr.id => true } })
+        end
+      end
+
+      context 'when user does not have access to private MR' do
+        let(:resources_by_type) { { 'merge_requests' => [private_mr.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'merge_requests' => { private_mr.id => false } })
+        end
+      end
+
+      context 'when user has project access' do
+        let(:resources_by_type) { { 'merge_requests' => [accessible_mr.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'merge_requests' => { accessible_mr.id => true } })
+        end
+      end
+    end
+
+    context 'with projects' do
+      context 'when user can access public project' do
+        let(:resources_by_type) { { 'projects' => [public_project.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'projects' => { public_project.id => true } })
+        end
+      end
+
+      context 'when user cannot access private project' do
+        let(:resources_by_type) { { 'projects' => [private_project.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'projects' => { private_project.id => false } })
+        end
+      end
+
+      context 'when user has project access' do
+        let(:resources_by_type) { { 'projects' => [private_project_with_access.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'projects' => { private_project_with_access.id => true } })
+        end
+      end
+    end
+
+    context 'with non-existent resources' do
+      let(:resources_by_type) { { 'issues' => [non_existing_record_id] } }
+
+      it 'denies access to non-existent resources' do
+        expect(result).to eq({ 'issues' => { non_existing_record_id => false } })
+      end
+    end
+
+    context 'with mixed resource types' do
+      let_it_be(:public_issue) { create(:issue, project: public_project) }
+      let_it_be(:private_mr) { create(:merge_request, source_project: private_project) }
+
+      let(:resources_by_type) do
+        {
+          'issues' => [public_issue.id],
+          'merge_requests' => [private_mr.id],
+          'projects' => [public_project.id, private_project.id]
+        }
+      end
+
+      it 'handles multiple resource types correctly' do
+        expect(result).to eq({
+          'issues' => { public_issue.id => true },
+          'merge_requests' => { private_mr.id => false },
+          'projects' => {
+            public_project.id => true,
+            private_project.id => false
+          }
+        })
+      end
+    end
+
+    context 'with empty arrays for a type' do
+      let(:resources_by_type) { { 'issues' => [] } }
+
+      it 'returns empty hash for that type' do
+        expect(result).to eq({ 'issues' => {} })
+      end
+    end
+
+    context 'with milestones' do
+      let_it_be(:public_milestone) { create(:milestone, project: public_project) }
+      let_it_be(:private_milestone) { create(:milestone, project: private_project) }
+
+      context 'when user can access public milestone' do
+        let(:resources_by_type) { { 'milestones' => [public_milestone.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'milestones' => { public_milestone.id => true } })
+        end
+      end
+
+      context 'when user cannot access private milestone' do
+        let(:resources_by_type) { { 'milestones' => [private_milestone.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'milestones' => { private_milestone.id => false } })
+        end
+      end
+    end
+
+    context 'with snippets' do
+      let_it_be(:public_snippet) { create(:project_snippet, :public, project: public_project) }
+      let_it_be(:private_snippet) { create(:project_snippet, :private, project: private_project) }
+      let_it_be(:accessible_snippet) { create(:project_snippet, :private, project: private_project_with_access) }
+
+      context 'when user can access public snippet' do
+        let(:resources_by_type) { { 'snippets' => [public_snippet.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'snippets' => { public_snippet.id => true } })
+        end
+      end
+
+      context 'when user cannot access private snippet' do
+        let(:resources_by_type) { { 'snippets' => [private_snippet.id] } }
+
+        it 'denies access' do
+          expect(result).to eq({ 'snippets' => { private_snippet.id => false } })
+        end
+      end
+
+      context 'when user has project access' do
+        let(:resources_by_type) { { 'snippets' => [accessible_snippet.id] } }
+
+        it 'allows access' do
+          expect(result).to eq({ 'snippets' => { accessible_snippet.id => true } })
+        end
+      end
+    end
+
+    context 'with logger parameter' do
+      let(:logger) { instance_double(Logger) }
+      let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, logger: logger) }
+      let(:resources_by_type) { { 'issues' => [] } }
+
+      it 'accepts a logger parameter' do
+        expect { service }.not_to raise_error
+        expect(result).to eq({ 'issues' => {} })
+      end
+    end
+  end
+
+  describe 'performance optimization' do
+    let_it_be(:issues) { create_list(:issue, 3, project: public_project) }
+    let(:resources_by_type) { { 'issues' => issues.map(&:id) } }
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type) }
+
+    it 'uses DeclarativePolicy.user_scope for optimization' do
+      expect(DeclarativePolicy).to receive(:user_scope).and_call_original
+      service.execute
+    end
+
+    it 'batch loads resources to prevent N+1 queries' do
+      # First call to warm up
+      service.execute
+
+      # Recreate service to test fresh
+      new_service = described_class.new(user: user, resources_by_type: resources_by_type)
+
+      expect do
+        new_service.execute
+      end.not_to exceed_query_limit(10) # Reasonable limit for batch operation
+    end
+
+    it 'preloads nested associations to avoid N+1 in policies' do
+      # Verify that project associations are preloaded
+      service.execute
+
+      # The preloads should include nested project associations
+      expect(described_class::PRELOAD_ASSOCIATIONS['issues']).to include(
+        a_hash_including(project: array_including(:namespace, :project_feature))
+      )
+    end
+  end
+end
-- 
GitLab


From 89ac22789588e700c7e40eae599ec777f6016d14 Mon Sep 17 00:00:00 2001
From: michaelangeloio <arivera@gitlab.com>
Date: Fri, 12 Dec 2025 10:01:36 -0500
Subject: [PATCH 2/5] Add user validation, source tracking, and logging to
 RedactionService

  Address review feedback:

  - Raise ArgumentError when user is nil to fail fast on invalid input
  - Add mandatory 'source' parameter to track calling services
    (e.g., 'knowledge_graph', 'zoekt') for monitoring and debugging
  - Add logging for redacted results following SearchService pattern,
    logging only when resources are redacted

  The logging includes: source, user_id, total_requested,
  total_redacted, and redacted_by_type breakdown.
---
 app/services/authz/redaction_service.rb       | 56 ++++++++++++++---
 .../ee/authz/redaction_service_spec.rb        |  2 +-
 spec/services/authz/redaction_service_spec.rb | 61 +++++++++++++++++--
 3 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/app/services/authz/redaction_service.rb b/app/services/authz/redaction_service.rb
index 9305dae8f2bc63..b1f8c7d1d614dc 100644
--- a/app/services/authz/redaction_service.rb
+++ b/app/services/authz/redaction_service.rb
@@ -22,7 +22,8 @@ module Authz
   #     resources_by_type: {
   #       'issues' => [123, 456],
   #       'merge_requests' => [789]
-  #     }
+  #     },
+  #     source: 'knowledge_graph'
   #   )
   #   result = service.execute
   #   # => {
@@ -68,13 +69,19 @@ def self.supported_types
       RESOURCE_CLASSES.keys
     end
 
-    # @param user [User] The user to check permissions for
+    # @param user [User] The user to check permissions for (required)
     # @param resources_by_type [Hash<String, Array<Integer>>] Resources grouped by type
     #   e.g., { 'issues' => [1, 2, 3], 'merge_requests' => [4, 5] }
+    # @param source [String] Identifier for the calling service (e.g., 'knowledge_graph', 'zoekt')
+    #   Used for logging and monitoring to track which services are using redaction
     # @param logger [Logger, nil] Optional logger for redaction audit logging
-    def initialize(user:, resources_by_type:, logger: nil)
+    # @raise [ArgumentError] if user is nil
+    def initialize(user:, resources_by_type:, source:, logger: nil)
+      raise ArgumentError, 'user is required' if user.nil?
+
       @user = user
       @resources_by_type = resources_by_type
+      @source = source
       @logger = logger
     end
 
@@ -100,16 +107,20 @@ def execute
       # Check permissions using user_scope for optimization
       # DeclarativePolicy.user_scope caches policy evaluations for the same user
       # @see app/models/ability.rb:45-47
-      DeclarativePolicy.user_scope do
-        resources_by_type.each_with_object({}) do |(type, ids), results|
-          results[type] = authorize_resources_of_type(type, ids, loaded_resources_by_type[type] || {})
+      results = DeclarativePolicy.user_scope do
+        resources_by_type.each_with_object({}) do |(type, ids), authorization_results|
+          authorization_results[type] = authorize_resources_of_type(type, ids, loaded_resources_by_type[type] || {})
         end
       end
+
+      log_redacted_results(results)
+
+      results
     end
 
     private
 
-    attr_reader :user, :resources_by_type, :logger
+    attr_reader :user, :resources_by_type, :source, :logger
 
     # Loads all resources for all types with appropriate preloading.
     #
@@ -183,6 +194,37 @@ def visible_result?(resource)
       # etc.
       Ability.allowed?(user, :"read_#{resource.to_ability_name}", resource)
     end
+
+    # Logs redaction results for monitoring and debugging.
+    #
+    # Only logs when resources were redacted (authorized=false).
+    # Follows the SearchService#log_redacted_search_results pattern.
+    #
+    # @param results [Hash<String, Hash<Integer, Boolean>>] Authorization results by type
+    # @see app/services/search_service.rb:167-181
+    def log_redacted_results(results)
+      return unless logger
+
+      redacted_by_type = results.transform_values do |id_results|
+        id_results.count { |_id, authorized| !authorized }
+      end
+
+      # Only log if there were redacted resources
+      total_redacted = redacted_by_type.values.sum
+      return if total_redacted == 0
+
+      log_info = {
+        class: self.class.name,
+        message: 'redacted_authorization_results',
+        source: source,
+        user_id: user.id,
+        total_requested: results.values.sum(&:size),
+        total_redacted: total_redacted,
+        redacted_by_type: redacted_by_type
+      }
+
+      logger.error(log_info)
+    end
   end
 end
 
diff --git a/ee/spec/services/ee/authz/redaction_service_spec.rb b/ee/spec/services/ee/authz/redaction_service_spec.rb
index 78f19024e4cf93..8d7f9ef1fee64f 100644
--- a/ee/spec/services/ee/authz/redaction_service_spec.rb
+++ b/ee/spec/services/ee/authz/redaction_service_spec.rb
@@ -36,7 +36,7 @@
   describe '#execute' do
     subject(:result) { service.execute }
 
-    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type) }
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
 
     context 'with epics' do
       let_it_be(:public_epic) { create(:epic, group: group) }
diff --git a/spec/services/authz/redaction_service_spec.rb b/spec/services/authz/redaction_service_spec.rb
index 9845dee81c3919..8446578b37311a 100644
--- a/spec/services/authz/redaction_service_spec.rb
+++ b/spec/services/authz/redaction_service_spec.rb
@@ -20,10 +20,28 @@
     end
   end
 
+  describe '#initialize' do
+    context 'when user is nil' do
+      it 'raises ArgumentError' do
+        expect do
+          described_class.new(user: nil, resources_by_type: {}, source: 'test')
+        end.to raise_error(ArgumentError, 'user is required')
+      end
+    end
+
+    context 'when user is provided' do
+      it 'does not raise an error' do
+        expect do
+          described_class.new(user: user, resources_by_type: {}, source: 'test')
+        end.not_to raise_error
+      end
+    end
+  end
+
   describe '#execute' do
     subject(:result) { service.execute }
 
-    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type) }
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
 
     context 'with empty resources' do
       let(:resources_by_type) { {} }
@@ -237,20 +255,55 @@
 
     context 'with logger parameter' do
       let(:logger) { instance_double(Logger) }
-      let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, logger: logger) }
+      let(:service) do
+        described_class.new(user: user, resources_by_type: resources_by_type, source: 'knowledge_graph', logger: logger)
+      end
+
       let(:resources_by_type) { { 'issues' => [] } }
 
       it 'accepts a logger parameter' do
         expect { service }.not_to raise_error
         expect(result).to eq({ 'issues' => {} })
       end
+
+      context 'when resources are redacted' do
+        let_it_be(:private_issue) { create(:issue, project: private_project) }
+        let(:resources_by_type) { { 'issues' => [private_issue.id] } }
+
+        it 'logs redacted results' do
+          expect(logger).to receive(:error).with(
+            hash_including(
+              class: 'Authz::RedactionService',
+              message: 'redacted_authorization_results',
+              source: 'knowledge_graph',
+              user_id: user.id,
+              total_requested: 1,
+              total_redacted: 1,
+              redacted_by_type: { 'issues' => 1 }
+            )
+          )
+
+          result
+        end
+      end
+
+      context 'when no resources are redacted' do
+        let_it_be(:public_issue) { create(:issue, project: public_project) }
+        let(:resources_by_type) { { 'issues' => [public_issue.id] } }
+
+        it 'does not log' do
+          expect(logger).not_to receive(:error)
+
+          result
+        end
+      end
     end
   end
 
   describe 'performance optimization' do
     let_it_be(:issues) { create_list(:issue, 3, project: public_project) }
     let(:resources_by_type) { { 'issues' => issues.map(&:id) } }
-    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type) }
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
 
     it 'uses DeclarativePolicy.user_scope for optimization' do
       expect(DeclarativePolicy).to receive(:user_scope).and_call_original
@@ -262,7 +315,7 @@
       service.execute
 
       # Recreate service to test fresh
-      new_service = described_class.new(user: user, resources_by_type: resources_by_type)
+      new_service = described_class.new(user: user, resources_by_type: resources_by_type, source: 'test')
 
       expect do
         new_service.execute
-- 
GitLab


From f497718cb1383cf37c7190ab9691be51ece36c22 Mon Sep 17 00:00:00 2001
From: michaelangeloio <arivera@gitlab.com>
Date: Fri, 12 Dec 2025 13:22:48 -0500
Subject: [PATCH 3/5] Add tests for unsupported resource types

---
 spec/services/authz/redaction_service_spec.rb | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/spec/services/authz/redaction_service_spec.rb b/spec/services/authz/redaction_service_spec.rb
index 8446578b37311a..e232b05644fce8 100644
--- a/spec/services/authz/redaction_service_spec.rb
+++ b/spec/services/authz/redaction_service_spec.rb
@@ -202,6 +202,14 @@
       end
     end
 
+    context 'with unsupported resource type' do
+      let(:resources_by_type) { { 'unknown_type' => [1, 2, 3] } }
+
+      it 'denies access for all IDs of unsupported type' do
+        expect(result).to eq({ 'unknown_type' => { 1 => false, 2 => false, 3 => false } })
+      end
+    end
+
     context 'with milestones' do
       let_it_be(:public_milestone) { create(:milestone, project: public_project) }
       let_it_be(:private_milestone) { create(:milestone, project: private_project) }
@@ -300,6 +308,47 @@
     end
   end
 
+  describe 'visible_result? behavior' do
+    let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
+
+    context 'when resource does not respond to to_ability_name' do
+      let(:plain_object) { Struct.new(:id).new(999) }
+      let(:resources_by_type) { { 'issues' => [999] } }
+
+      before do
+        # Stub the resource loading to return a plain object without to_ability_name
+        allow(service).to receive(:load_all_resources).and_return({
+          'issues' => { 999 => plain_object }
+        })
+      end
+
+      it 'allows access for resources without to_ability_name' do
+        result = service.execute
+        expect(result).to eq({ 'issues' => { 999 => true } })
+      end
+    end
+
+    context 'when resource has no policy' do
+      let(:object_without_policy) do
+        Struct.new(:id, :to_ability_name).new(888, 'unknown_object')
+      end
+
+      let(:resources_by_type) { { 'issues' => [888] } }
+
+      before do
+        allow(service).to receive(:load_all_resources).and_return({
+          'issues' => { 888 => object_without_policy }
+        })
+        allow(DeclarativePolicy).to receive(:has_policy?).with(object_without_policy).and_return(false)
+      end
+
+      it 'allows access for resources without policy' do
+        result = service.execute
+        expect(result).to eq({ 'issues' => { 888 => true } })
+      end
+    end
+  end
+
   describe 'performance optimization' do
     let_it_be(:issues) { create_list(:issue, 3, project: public_project) }
     let(:resources_by_type) { { 'issues' => issues.map(&:id) } }
-- 
GitLab


From b59be5eb756c6102c3278fd1b6eef791259a77fd Mon Sep 17 00:00:00 2001
From: michaelangeloio <arivera@gitlab.com>
Date: Fri, 12 Dec 2025 16:25:11 -0500
Subject: [PATCH 4/5] Address feedback from Jay

---
 app/services/authz/redaction_service.rb       |  6 +++--
 ee/app/services/ee/authz/redaction_service.rb |  6 +++--
 .../ee/authz/redaction_service_spec.rb        | 25 +++++++++++++++++++
 spec/services/authz/redaction_service_spec.rb | 22 ++++++++++++++++
 4 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/app/services/authz/redaction_service.rb b/app/services/authz/redaction_service.rb
index b1f8c7d1d614dc..0b9555c87a8685 100644
--- a/app/services/authz/redaction_service.rb
+++ b/app/services/authz/redaction_service.rb
@@ -143,8 +143,10 @@ def load_resources_for_type(type, ids)
       klass = RESOURCE_CLASSES[type]
       return {} unless klass
 
-      preloads = PRELOAD_ASSOCIATIONS[type] || []
-      klass.where(id: ids).includes(*preloads).index_by(&:id)
+      preloads = PRELOAD_ASSOCIATIONS[type]
+      relation = klass.where(id: ids)
+      relation = relation.includes(*preloads) if preloads
+      relation.index_by(&:id)
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
diff --git a/ee/app/services/ee/authz/redaction_service.rb b/ee/app/services/ee/authz/redaction_service.rb
index a29192dd31bb43..693240855eaa6e 100644
--- a/ee/app/services/ee/authz/redaction_service.rb
+++ b/ee/app/services/ee/authz/redaction_service.rb
@@ -43,8 +43,10 @@ def load_resources_for_type(type, ids)
         return {} if ids.blank?
 
         klass = EE_RESOURCE_CLASSES[type]
-        preloads = EE_PRELOAD_ASSOCIATIONS[type] || []
-        klass.where(id: ids).includes(*preloads).index_by(&:id)
+        preloads = EE_PRELOAD_ASSOCIATIONS[type]
+        relation = klass.where(id: ids)
+        relation = relation.includes(*preloads) if preloads
+        relation.index_by(&:id)
       end
       # rubocop:enable CodeReuse/ActiveRecord
 
diff --git a/ee/spec/services/ee/authz/redaction_service_spec.rb b/ee/spec/services/ee/authz/redaction_service_spec.rb
index 8d7f9ef1fee64f..5241bf68fc9efe 100644
--- a/ee/spec/services/ee/authz/redaction_service_spec.rb
+++ b/ee/spec/services/ee/authz/redaction_service_spec.rb
@@ -178,4 +178,29 @@
       end
     end
   end
+
+  describe 'load_resources_for_type behavior' do
+    context 'when EE resource type has no preload associations defined' do
+      let_it_be(:public_epic) { create(:epic, group: group) }
+      let(:resources_by_type) { { 'epics' => [public_epic.id] } }
+      let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
+
+      before do
+        # Simulate an EE resource type that exists in EE_RESOURCE_CLASSES but has no EE_PRELOAD_ASSOCIATIONS entry
+        stub_const(
+          "EE::Authz::RedactionService::EE_PRELOAD_ASSOCIATIONS",
+          EE::Authz::RedactionService::EE_PRELOAD_ASSOCIATIONS.except('epics')
+        )
+      end
+
+      it 'does not raise an error when preloads are not defined' do
+        expect { service.execute }.not_to raise_error
+      end
+
+      it 'still performs authorization correctly' do
+        result = service.execute
+        expect(result).to eq({ 'epics' => { public_epic.id => true } })
+      end
+    end
+  end
 end
diff --git a/spec/services/authz/redaction_service_spec.rb b/spec/services/authz/redaction_service_spec.rb
index e232b05644fce8..d916b184ac37e4 100644
--- a/spec/services/authz/redaction_service_spec.rb
+++ b/spec/services/authz/redaction_service_spec.rb
@@ -349,6 +349,28 @@
     end
   end
 
+  describe 'load_resources_for_type behavior' do
+    context 'when resource type has no preload associations defined' do
+      let_it_be(:public_issue) { create(:issue, project: public_project) }
+      let(:resources_by_type) { { 'issues' => [public_issue.id] } }
+      let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
+
+      before do
+        # Simulate a resource type that exists in RESOURCE_CLASSES but has no PRELOAD_ASSOCIATIONS entry
+        stub_const("#{described_class}::PRELOAD_ASSOCIATIONS", described_class::PRELOAD_ASSOCIATIONS.except('issues'))
+      end
+
+      it 'does not raise an error when preloads are not defined' do
+        expect { service.execute }.not_to raise_error
+      end
+
+      it 'still performs authorization correctly' do
+        result = service.execute
+        expect(result).to eq({ 'issues' => { public_issue.id => true } })
+      end
+    end
+  end
+
   describe 'performance optimization' do
     let_it_be(:issues) { create_list(:issue, 3, project: public_project) }
     let(:resources_by_type) { { 'issues' => issues.map(&:id) } }
-- 
GitLab


From c143425f17bb9c1d078d357a91c97e40b885491b Mon Sep 17 00:00:00 2001
From: michaelangeloio <arivera@gitlab.com>
Date: Fri, 12 Dec 2025 17:49:38 -0500
Subject: [PATCH 5/5] remove redundant comments

---
 app/services/authz/redaction_service.rb       | 87 +------------------
 ee/app/services/ee/authz/redaction_service.rb |  6 --
 .../ee/authz/redaction_service_spec.rb        |  3 -
 spec/services/authz/redaction_service_spec.rb |  6 --
 4 files changed, 1 insertion(+), 101 deletions(-)

diff --git a/app/services/authz/redaction_service.rb b/app/services/authz/redaction_service.rb
index 0b9555c87a8685..562c612d866bfe 100644
--- a/app/services/authz/redaction_service.rb
+++ b/app/services/authz/redaction_service.rb
@@ -5,12 +5,9 @@ module Authz
   #
   # This service is designed to check whether a user has read access to a batch
   # of resources using GitLab's standard Ability system. It's used by the
-  # Knowledge Graph service for final redaction (Layer 3) but is generic enough
+  # Knowledge Graph service for final redaction but is generic enough
   # to be used by any service requiring batch authorization checks.
   #
-  # The service follows the same authorization pattern used by SearchService
-  # for redacting search results.
-  #
   # IMPORTANT: This service assumes that the user has already been authenticated
   # and authorized to make API requests. It does NOT perform user-level validation
   # (e.g., checking if user is blocked or deactivated). The caller is responsible
@@ -30,16 +27,9 @@ module Authz
   #   #      'issues' => { 123 => true, 456 => false },
   #   #      'merge_requests' => { 789 => true }
   #   #    }
-  #
-  # @see app/services/search_service.rb:136-139 for visible_result? pattern
-  # @see app/models/ability.rb:42-71 for batch authorization methods
   class RedactionService
     include Gitlab::Allowable
 
-    # Mapping of resource type keys (plural) to their corresponding model classes.
-    # All models must implement #to_ability_name for consistent ability naming.
-    #
-    # @see app/models/concerns/issuable.rb:495-497 for Issuable implementation
     RESOURCE_CLASSES = {
       'issues' => ::Issue,
       'merge_requests' => ::MergeRequest,
@@ -48,12 +38,6 @@ class RedactionService
       'snippets' => ::Snippet
     }.freeze
 
-    # Preload associations needed for authorization checks to prevent N+1 queries.
-    # Each resource type needs associations accessed by its policy during evaluation.
-    #
-    # @see app/policies/issue_policy.rb - needs project for most conditions
-    # @see app/policies/merge_request_policy.rb - needs target_project
-    # @see app/policies/project_policy.rb - accesses namespace, project_feature, group
     PRELOAD_ASSOCIATIONS = {
       'issues' => [{ project: [:namespace, :project_feature, :group] }, :author, :work_item_type],
       'merge_requests' => [{ target_project: [:namespace, :project_feature, :group] }, :author],
@@ -62,20 +46,10 @@ class RedactionService
       'snippets' => [{ project: [:namespace, :project_feature] }, :author]
     }.freeze
 
-    # Returns the list of supported resource types.
-    #
-    # @return [Array<String>] List of valid resource type keys
     def self.supported_types
       RESOURCE_CLASSES.keys
     end
 
-    # @param user [User] The user to check permissions for (required)
-    # @param resources_by_type [Hash<String, Array<Integer>>] Resources grouped by type
-    #   e.g., { 'issues' => [1, 2, 3], 'merge_requests' => [4, 5] }
-    # @param source [String] Identifier for the calling service (e.g., 'knowledge_graph', 'zoekt')
-    #   Used for logging and monitoring to track which services are using redaction
-    # @param logger [Logger, nil] Optional logger for redaction audit logging
-    # @raise [ArgumentError] if user is nil
     def initialize(user:, resources_by_type:, source:, logger: nil)
       raise ArgumentError, 'user is required' if user.nil?
 
@@ -85,28 +59,11 @@ def initialize(user:, resources_by_type:, source:, logger: nil)
       @logger = logger
     end
 
-    # Executes the batch authorization check.
-    #
-    # Processes each resource type, batch loads resources with preloading,
-    # then checks each resource's read permission using the standard
-    # Ability.allowed? method.
-    #
-    # Uses DeclarativePolicy.user_scope to optimize policy evaluation when
-    # checking multiple resources for the same user, following the pattern
-    # from Ability.issues_readable_by_user.
-    #
-    # @return [Hash<String, Hash<Integer, Boolean>>] Authorization results grouped by type
-    #   e.g., { 'issues' => { 1 => true, 2 => false }, 'merge_requests' => { 4 => true } }
-    # @see app/models/ability.rb:42-48
     def execute
       return {} if resources_by_type.empty?
 
-      # Load all resources with preloading to prevent N+1 queries
       loaded_resources_by_type = load_all_resources
 
-      # Check permissions using user_scope for optimization
-      # DeclarativePolicy.user_scope caches policy evaluations for the same user
-      # @see app/models/ability.rb:45-47
       results = DeclarativePolicy.user_scope do
         resources_by_type.each_with_object({}) do |(type, ids), authorization_results|
           authorization_results[type] = authorize_resources_of_type(type, ids, loaded_resources_by_type[type] || {})
@@ -122,20 +79,12 @@ def execute
 
     attr_reader :user, :resources_by_type, :source, :logger
 
-    # Loads all resources for all types with appropriate preloading.
-    #
-    # @return [Hash<String, Hash<Integer, ActiveRecord::Base>>] Loaded resources by type
     def load_all_resources
       resources_by_type.each_with_object({}) do |(type, ids), loaded|
         loaded[type] = load_resources_for_type(type, ids)
       end
     end
 
-    # Loads resources for a single type with preloading.
-    #
-    # @param type [String] The resource type
-    # @param ids [Array<Integer>] The resource IDs
-    # @return [Hash<Integer, ActiveRecord::Base>] Loaded resources indexed by ID
     # rubocop:disable CodeReuse/ActiveRecord -- Batch loading with preloads for authorization checks
     def load_resources_for_type(type, ids)
       return {} if ids.blank?
@@ -150,15 +99,6 @@ def load_resources_for_type(type, ids)
     end
     # rubocop:enable CodeReuse/ActiveRecord
 
-    # Authorizes all resources of a single type.
-    #
-    # Checks authorization for each resource, using pre-loaded data.
-    # Returns a hash mapping resource IDs to authorization results.
-    #
-    # @param type [String] The resource type (e.g., 'issues', 'merge_requests')
-    # @param ids [Array<Integer>] The resource IDs to authorize
-    # @param loaded_resources [Hash<Integer, ActiveRecord::Base>] Pre-loaded resources
-    # @return [Hash<Integer, Boolean>] Map of resource ID to authorization result
     def authorize_resources_of_type(type, ids, loaded_resources)
       return {} if ids.blank?
 
@@ -168,42 +108,18 @@ def authorize_resources_of_type(type, ids, loaded_resources)
       ids.index_with do |id|
         resource = loaded_resources[id]
 
-        # Resource not found - deny access
         next false if resource.nil?
 
-        # Check visibility using the same pattern as SearchService
         visible_result?(resource)
       end
     end
 
-    # Checks if a resource is visible to the user.
-    #
-    # This method is intentionally identical to SearchService#visible_result?
-    # to ensure consistent authorization behavior across search, the Knowledge Graph,
-    # and any other consuming features.
-    #
-    # @param resource [ActiveRecord::Base] The resource to check
-    # @return [Boolean] Whether the user can read the resource
-    # @see app/services/search_service.rb:136-139
     def visible_result?(resource)
-      # Resources without policies are considered visible
-      # This handles edge cases like plain Ruby objects
       return true unless resource.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(resource)
 
-      # Use the resource's to_ability_name to construct the read ability
-      # For Issue: :read_issue
-      # For MergeRequest: :read_merge_request
-      # etc.
       Ability.allowed?(user, :"read_#{resource.to_ability_name}", resource)
     end
 
-    # Logs redaction results for monitoring and debugging.
-    #
-    # Only logs when resources were redacted (authorized=false).
-    # Follows the SearchService#log_redacted_search_results pattern.
-    #
-    # @param results [Hash<String, Hash<Integer, Boolean>>] Authorization results by type
-    # @see app/services/search_service.rb:167-181
     def log_redacted_results(results)
       return unless logger
 
@@ -211,7 +127,6 @@ def log_redacted_results(results)
         id_results.count { |_id, authorized| !authorized }
       end
 
-      # Only log if there were redacted resources
       total_redacted = redacted_by_type.values.sum
       return if total_redacted == 0
 
diff --git a/ee/app/services/ee/authz/redaction_service.rb b/ee/app/services/ee/authz/redaction_service.rb
index 693240855eaa6e..6b247b37a11889 100644
--- a/ee/app/services/ee/authz/redaction_service.rb
+++ b/ee/app/services/ee/authz/redaction_service.rb
@@ -5,17 +5,11 @@ module Authz
     module RedactionService
       extend ::Gitlab::Utils::Override
 
-      # EE-specific resource types added to the base CE resource classes.
       EE_RESOURCE_CLASSES = {
         'epics' => ::Epic,
         'vulnerabilities' => ::Vulnerability
       }.freeze
 
-      # EE-specific preload associations for authorization checks.
-      # These include nested associations needed by policies.
-      #
-      # @see ee/app/policies/epic_policy.rb - delegates to group
-      # @see ee/app/policies/vulnerability_policy.rb - delegates to project
       EE_PRELOAD_ASSOCIATIONS = {
         'epics' => [:group],
         'vulnerabilities' => [{ project: [:namespace, :project_feature, :group] }]
diff --git a/ee/spec/services/ee/authz/redaction_service_spec.rb b/ee/spec/services/ee/authz/redaction_service_spec.rb
index 5241bf68fc9efe..4b8d9ab665b452 100644
--- a/ee/spec/services/ee/authz/redaction_service_spec.rb
+++ b/ee/spec/services/ee/authz/redaction_service_spec.rb
@@ -12,8 +12,6 @@
   let_it_be(:private_project_with_access) { create(:project, :private, group: private_group_with_access) }
 
   before_all do
-    # Developer access needed for read_security_resource (vulnerabilities)
-    # Reporter access is sufficient for epics
     private_group_with_access.add_developer(user)
   end
 
@@ -186,7 +184,6 @@
       let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
 
       before do
-        # Simulate an EE resource type that exists in EE_RESOURCE_CLASSES but has no EE_PRELOAD_ASSOCIATIONS entry
         stub_const(
           "EE::Authz::RedactionService::EE_PRELOAD_ASSOCIATIONS",
           EE::Authz::RedactionService::EE_PRELOAD_ASSOCIATIONS.except('epics')
diff --git a/spec/services/authz/redaction_service_spec.rb b/spec/services/authz/redaction_service_spec.rb
index d916b184ac37e4..09ede324ef4091 100644
--- a/spec/services/authz/redaction_service_spec.rb
+++ b/spec/services/authz/redaction_service_spec.rb
@@ -316,7 +316,6 @@
       let(:resources_by_type) { { 'issues' => [999] } }
 
       before do
-        # Stub the resource loading to return a plain object without to_ability_name
         allow(service).to receive(:load_all_resources).and_return({
           'issues' => { 999 => plain_object }
         })
@@ -356,7 +355,6 @@
       let(:service) { described_class.new(user: user, resources_by_type: resources_by_type, source: 'test') }
 
       before do
-        # Simulate a resource type that exists in RESOURCE_CLASSES but has no PRELOAD_ASSOCIATIONS entry
         stub_const("#{described_class}::PRELOAD_ASSOCIATIONS", described_class::PRELOAD_ASSOCIATIONS.except('issues'))
       end
 
@@ -382,10 +380,8 @@
     end
 
     it 'batch loads resources to prevent N+1 queries' do
-      # First call to warm up
       service.execute
 
-      # Recreate service to test fresh
       new_service = described_class.new(user: user, resources_by_type: resources_by_type, source: 'test')
 
       expect do
@@ -394,10 +390,8 @@
     end
 
     it 'preloads nested associations to avoid N+1 in policies' do
-      # Verify that project associations are preloaded
       service.execute
 
-      # The preloads should include nested project associations
       expect(described_class::PRELOAD_ASSOCIATIONS['issues']).to include(
         a_hash_including(project: array_including(:namespace, :project_feature))
       )
-- 
GitLab