Fix comment Advanced Search for the Planner role

Ravi Kumarrequested to merge
538209-planner-notes-search into master

What does this MR do and why?

Advanced Search (Elasticsearch) hid comments from a Planner that ProjectPolicy says they can read. This MR fixes the comments (notes) scope so results match what a Planner can actually see.

A local probe compared Ability.allowed?(user, :read_note, note) against the notes search filters, across project visibility, feature access levels, and roles (guest, planner, reporter, non-member, group-level and external planner, external guest), for issue, internal, confidential-issue, MR, internal MR, commit, and snippet notes. It found three Planner gaps, nothing else:

  1. Internal comments and comments on confidential issues, any visibility: NoteClassProxy#confidentiality_filter hardcoded Reporter as the minimum, but Planner has read_internal_note and can read confidential issues.
  2. Comments on merge requests in private projects with the MR feature ENABLED: the filter required Reporter, but ProjectPolicy only requires Reporter when the feature is PRIVATE.
  3. Comments on commits in private projects, repository ENABLED or PRIVATE: the filter's repository minimum was Reporter, but Planner has read_code.

The fix, in ee/lib/elastic/latest/note_class_proxy.rb, builds a relaxed_feature_access hash for the membership filter by merging Search::Elastic::MergeRequestQueryBuilder::RELAXED_FEATURE_ACCESS (merge_requests: Planner, ENABLED only) with Elastic::Latest::GitClassProxy::COMMIT_RELAXED_FEATURE_ACCESS (repository: Planner, ENABLED or PRIVATE) — the rules merge request and commit search already use. It's built in a method, not a constant, to avoid a circular autoload between NoteClassProxy and GitClassProxy. Confidentiality minimums drop from Reporter to Planner.

Relaxed rules only apply to features present in the query, and each note only carries its own noteable's access level, so this can't leak into issue or snippet comments. Guest (level 10) stays below the Planner (15) confidentiality minimum, and PRIVATE merge requests still need Reporter, so nothing new reaches Guest, external guests, or non-members.

ee/lib/gitlab/elastic/search_results.rb drops the related_ids_for_notes override that set relaxed_feature_access only for MergeRequest sub-queries — the proxy now does this for every notes query, so the override was redundant.

Not done: converting notes search to the Query Builder pattern the issue suggested — kept minimal, matching the prior MRs below. Out of scope: two pre-existing mismatches unrelated to Planner, present on master too — a non-member can get a comment on a private project snippet in a public or internal project, and an external guest can get commit notes in an internal project. Both are stripped by SearchService redaction before display, affecting only counts.

Docs: doc/user/permissions.md no longer says Planner can't search comments on MRs in private projects or on confidential issues. The repository footnote now only says Planner can't use exact or advanced code search in private projects (blob search is tracked under the epic).

Tests

  • note_class_proxy_spec.rb asserts the confidentiality filter now uses the Planner minimum instead of Reporter.
  • Notes-on-merge-requests visibility specs (global, group, project) switch to permission_table_for_planner_feature_access; without the fix, the private/enabled Planner row fails.
  • Notes-on-commits visibility specs switch to permission_table_for_repository_access; without the fix, private/enabled and private/private Planner rows fail.
  • Notes-on-issues visibility specs use a new permission_table_for_planner_notes_feature_access (existing table plus Planner rows expecting the plain and internal note; the original table is untouched since wiki_page_policy_spec also uses it). Without the fix, every non-disabled Planner row fails.
  • project_search_results_spec.rb drops three examples pinning the old related_ids_for_notes override; the existing "matched only by one of its comments" example still covers the Planner MR-tab path end to end.
  • The notes-on-issues and notes-on-merge-requests visibility specs (global, group, project) now include Planner tables for non-confidential comments on confidential issues and for internal MR comments (permission_table_for_planner_confidential_issue_access, permission_table_for_planner_internal_note_access).
  • ip_restricted_snippet_notes_spec.rb drops the "accepted divergence: Planner loses confidential-note visibility on the Elasticsearch route" block. It pinned the old Reporter minimum, and this MR removes that divergence. note_class_proxy_spec.rb (the filter uses the Planner minimum) and the new Planner visibility specs now cover those checks.

References

Screenshots or screen recordings

Not applicable: backend search change, no UI.

How to set up and validate locally

With GDK and Elasticsearch enabled: create a private project, add a user as Planner, add an internal issue comment, an MR comment, and a commit comment, each with a distinctive word. As that user, search the word in the Comments tab at global, group, and project level. Before the fix, none show up; after, all three do. As a Guest on the same project, repeat the search: none should show up.

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.

🤖 Generated with Claude Code

Edited by Ravi Kumar

Merge request reports