## Summary The Planner role can read code in a private repository, but `Project.filter_by_feature_visibility(:repository, user)` filters Planners out. The two checks disagree. No user-facing path has been found yet where a Planner is wrongly denied code. This issue proposes aligning the helper with the role and checking every consumer. ## Details The Planner role grants `read_code` and `download_code`. `BlobPolicy` enables `read_blob` on `read_code`. So a Planner can read code in a private project whose repository feature is set to private (members only). The role description says Planners "view and collaborate on code". `filter_by_feature_visibility` uses a different rule. It requires Reporter for the repository feature in a private project. Planner is access level 15 and Reporter is 20, so a Planner does not pass the filter. The same helper feeds other code paths. Advanced Search already works around the mismatch for Planner with `relaxed_feature_access` rules, so code and commit search return results for a Planner. Other consumers may not have a workaround. I have not found a user-facing path where a Planner is wrongly denied code. The mismatch shows up in a spec. The proposal below includes checking every consumer to see whether one exists. <details><summary>Why</summary> - `config/authz/roles/planner.yml` grants `read_code` and `download_code` to the Planner role. - `BlobPolicy` enables `read_blob` when the user has `read_code`. - `Project.filter_by_feature_visibility` calls `ProjectFeature.required_minimum_access_level_for_private_project` for the feature. - In `app/models/project_feature.rb`, that method returns `PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }`. The minimum for a private project is Reporter (20). Planner is 15, so a Planner is excluded. - The same helper feeds `Search::GroupsFinder#min_access_level` in `ee/app/finders/search/groups_finder.rb`. - It also feeds the Advanced Search authorization in `ee/lib/search/elastic/concerns/authorization_utils.rb`. That code uses `relaxed_feature_access` rules for Planner, which is why Advanced Search returns code and commit results for a Planner. - In `spec/models/project_spec.rb`, `filter_by_feature_visibility` with the code feature, a private project, a private repository and a Planner user expects 1 project and gets 0. - Because of this, `permission_table_for_guest_feature_access_and_non_private_project_only` has no Planner rows. </details> ## Why this has no user-facing effect today `ProjectFeature::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT` sets `repository: REPORTER`. That value predates the Planner role (access level 15). It exists so Guests can't read code in private projects. Every user-facing path already lets a Planner read private-repository code, so changing the constant has no visible effect. When the Planner role was added with `read_code`, each search engine that hit the mismatch added its own Planner exception. None of them changed the shared constant. The constant is only reached when `Project.filter_by_feature_visibility(:repository)` is called directly for a private project, and no user-facing path does that. | Consumer | Uses the Reporter floor? | Why a Planner still gets code | |---|---|---| | Zoekt code search, `Search::Zoekt::AccessBranchBuilder` (through `Search::AuthorizationContext#get_access_levels_for_feature`) | Yes, `access_levels[:private_project]` | For `features == 'repository'`, `private_min_access_level` returns `[access_levels[:private_project], PRIVATE_RELAXED_MIN_ACCESS_LEVEL].min`. `PRIVATE_RELAXED_MIN_ACCESS_LEVEL` is `::Gitlab::Access::PLANNER`. | | Advanced Search code and commit search, `ee/lib/search/elastic/concerns/authorization_utils.rb` | Yes, `private_project:` | `Elastic::Latest::GitClassProxy` passes `REPOSITORY_RELAXED_FEATURE_ACCESS`, a second `relaxed_feature_access` rule that covers Planner. | | Zoekt result filtering, `Project.filter_out_public_projects_with_unauthorized_private_repos` | Calls `filter_by_feature_visibility(:repository)` | It only applies to public projects, where the private-project floor doesn't apply. | | `Search::GroupsFinder` with `features:` | Yes | No production caller passes `features:`. Callers pass an explicit `min_access_level`. | Basic (Postgres) code search and repository pages check `Ability` (`read_code`, `download_code`). The Planner role grants both, so these paths never used this floor. Once the floor is Planner, Zoekt's `PRIVATE_RELAXED_MIN_ACCESS_LEVEL` clamp and Advanced Search's repository `relaxed_feature_access` rule are redundant. A later MR can remove them to leave one source of truth, and that work is not part of this change. ## Proposal 1. Change the private-project repository minimum to `Gitlab::Access::PLANNER`. Planner is the only role between Guest and Reporter, so only Planners gain access. 1. Compare each consumer of the helper against `Ability` for a Planner. Look for any place where the filter and the policy give different answers. Include `Search::GroupsFinder` and the Advanced Search authorization. Check whether the `relaxed_feature_access` workaround can then be removed for repository. 1. Add Planner rows to `permission_table_for_guest_feature_access_and_non_private_project_only`. ## Links - Fix: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/259718 - Found while adding Planner to the search visibility specs in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/259653 - Parent issue: https://gitlab.com/gitlab-org/gitlab/-/work_items/538212 - Epic: https://gitlab.com/groups/gitlab-org/-/work_items/17674