## Summary In `ee/lib/api/project_mirror.rb`, the `params` block for `PUT /projects/:id/mirror/pull` ("Update project pull mirroring settings") contains a `mutually_exclusive` declaration that names a parameter which does not exist: ```ruby params do optional :enabled, type: Grape::API::Boolean, desc: 'Enables pull mirroring in a project' # ... optional :only_mirror_protected_branches, type: Grape::API::Boolean, desc: 'Only mirror protected branches' optional :mirror_overwrites_diverged_branches, type: Grape::API::Boolean, desc: 'Pull mirror overwrites diverged branches' optional :mirror_branch_regex, type: String, desc: 'Only mirror branches with names that match this regex' mutually_exclusive :only_protected_branches, :mirror_branch_regex at_least_one_of :enabled, :url, :auth_user, :auth_password, :mirror_overwrites_diverged_branches, :mirror_trigger_builds, :only_mirror_protected_branches, :mirror_branch_regex end ``` The declared parameter is `only_mirror_protected_branches`, but the `mutually_exclusive` call references `only_protected_branches` — the `mirror_` segment is missing. The `at_least_one_of` declaration on the very next line uses the correct name, which is good evidence this is a typo rather than an intentional reference to some other attribute. The likely origin is a copy-paste. `ee/lib/ee/api/helpers/remote_mirrors_helpers.rb:13` contains the byte-identical line: ```ruby mutually_exclusive :only_protected_branches, :mirror_branch_regex ``` There it is correct, because the push-mirror (remote mirrors) params really do declare `only_protected_branches`, in `lib/api/helpers/remote_mirrors_helpers.rb:10`. The pull-mirror endpoint names its equivalent parameter `only_mirror_protected_branches` instead, and does not use those helpers. Grape's mutual-exclusion validators resolve their declared attribute names against the keys actually present in the request. `Grape::Validations::Validators::MultipleParamsBase#keys_in_common` intersects the validator's attribute list with `resource_params.keys`. Because `only_protected_branches` is never a key in any real request, that intersection can never reach two entries, so the validation never fires. The rule is dead code. ### Verified, not inferred Confirmed against Grape 2.4.0 with a self-contained script — two endpoints differing only in that one symbol, both sent both parameters: ``` as written (typo) -> HTTP 200 {"accepted":true} with the fix -> HTTP 400 {"error":"only_mirror_protected_branches, mirror_branch_regex are mutually exclusive"} ``` The script, runnable from a GitLab checkout with `bundle exec ruby <file>`: ```ruby require 'grape' require 'rack/test' B = Grape::API::Boolean class ProbeAPI < Grape::API format :json params do optional :only_mirror_protected_branches, type: B optional :mirror_branch_regex, type: String mutually_exclusive :only_protected_branches, :mirror_branch_regex # as written today end put('/as_written') { { accepted: true } } params do optional :only_mirror_protected_branches, type: B optional :mirror_branch_regex, type: String mutually_exclusive :only_mirror_protected_branches, :mirror_branch_regex # proposed fix end put('/fixed') { { accepted: true } } end include Rack::Test::Methods def app = ProbeAPI both = { only_mirror_protected_branches: true, mirror_branch_regex: 'main' } put '/as_written', both puts "as written (typo) -> HTTP #{last_response.status} #{last_response.body}" put '/fixed', both puts "with the fix -> HTTP #{last_response.status} #{last_response.body}" ``` ### Ruled out: nothing else declares this parameter The name is not supplied by a helper, a shared params block, or any other include: - The `params` block above declares all eight parameters inline. Neither that block nor `ee/lib/api/project_mirror.rb` as a whole contains a single `use :` statement, so no shared params block is mixed in. - The only place `only_protected_branches` is declared as a parameter anywhere is `lib/api/helpers/remote_mirrors_helpers.rb:10`, which belongs to the push-mirror endpoints and is never referenced from `ee/lib/api/project_mirror.rb`. - Grape's own route introspection confirms it. Comparing the two entries in `route.params` for this endpoint: ```ruby only_mirror_protected_branches {:required=>false, :desc=>"Only mirror protected branches", :type=>"Grape::API::Boolean"} only_protected_branches {:required=>false} ``` Every genuinely declared parameter carries `desc` and `type`. The bare `{:required=>false}` entry is what Grape records for a name referenced by a validator with no `requires`/`optional` declaration behind it. Since it is undeclared, `declared_params(include_missing: false)` in the endpoint body never returns it, so the value is discarded even if a client sends it. ## Impact 1: the documented constraint is not enforced `doc/api/project_pull_mirroring.md` documents this constraint twice — on the request attribute tables for both the update and the create/configure operations: > `mirror_branch_regex` — Contains a regular expression. Only branches with names matching the regex are mirrored. Requires `only_mirror_protected_branches` to be disabled. Today a request that sets both `only_mirror_protected_branches=true` and `mirror_branch_regex=<pattern>` is accepted with no `400`, contradicting that documentation, and both settings reach the service. Worth the owning team confirming the intended behaviour before fixing: whether the combination should be rejected outright, or whether one setting should take precedence and the documentation should change instead. ## Impact 2: a non-existent parameter leaks into the published OpenAPI spec The `gitlab-grape-openapi` gem generates `doc/api/openapi/openapi_v3.yaml` from these Grape definitions. From version 0.5.0 the generator reads `mutually_exclusive` declarations and appends a prose note to each named parameter's description, since OpenAPI 3.0 has no native keyword for cross-parameter exclusion. The generator trusts the names in the declaration, so it emitted a request-body property for `only_protected_branches`, which this endpoint does not accept: ```yaml only_protected_branches: type: string nullable: true description: Mutually exclusive with `mirror_branch_regex`. ``` Two details worth noting: - The description consists solely of the generated note, because no `desc:` exists for it anywhere — nothing declares this parameter. - Its `type` is `string`, not the `Grape::API::Boolean` used by the real parameter. That follows from the same root cause: Grape records no type for a validator-only name, so the generator falls back to `string`. So the published API reference now advertises a string parameter that the endpoint silently ignores. That has no behavioural effect by itself, but it is incorrect user-facing documentation. The gem is working as designed here — it faithfully reports what the source declares. The bump only made a pre-existing typo visible. ## Steps to reproduce ```shell curl --request PUT \ --header "PRIVATE-TOKEN: <your_access_token>" \ --url "https://gitlab.example.com/api/v4/projects/<project_id>/mirror/pull" \ --data "only_mirror_protected_branches=true" \ --data "mirror_branch_regex=^release/.*" ``` Expected: `400 Bad Request`, per the documented constraint. Actual: the request succeeds and both settings are persisted. ## Proposed fix ```diff - mutually_exclusive :only_protected_branches, :mirror_branch_regex + mutually_exclusive :only_mirror_protected_branches, :mirror_branch_regex ``` A one-word change with a real behaviour consequence. It needs: - A request spec asserting that sending both parameters returns `400`. No such coverage exists today, which is why this went unnoticed for eleven months. The nearest existing example, in `ee/spec/requests/api/project_mirror_spec.rb`, sets `only_mirror_protected_branches` without `mirror_branch_regex`, so it never exercises the constraint. - A `Changelog: fixed` trailer with `EE: true`, since it changes API behaviour. - Regeneration of `doc/api/openapi/openapi_v3.yaml` via `bin/rake gitlab:openapi:v3:generate`, which drops the phantom property. ### Risk This switches on a validation that has been inert for eleven months, so requests that currently succeed with both parameters set would begin returning `400`. That is a user-visible change for any existing caller sending both. Mitigating factor: the endpoint carries `route_setting :lifecycle, :experiment`, so it sits outside GA stability guarantees. The team should decide whether it warrants a deprecation note first. ## Scope and audit An audit of the wider API found this is an isolated case, not a pattern. Two independent checks agreed: - Scanning all 1,847 operations in the generated v3 spec for a parameter whose description consists only of a generated exclusion note — the signature of a name nothing declares — returned exactly one hit: this one. - A source-level audit of all 115 `mutually_exclusive`, `exactly_one_of`, `at_least_one_of`, and `all_or_none_of` declarations across `lib/api` and `ee/lib/api` flagged six candidates. Five were confirmed benign: - Two in `lib/api/helpers/snippets_helpers.rb`, where the referenced names are declared in the endpoint block that does `use :update_file_params`. - Two in `ee/lib/api/ldap_group_links.rb`, where the parameters are declared with string names rather than symbols. - One in `lib/api/pages_domains.rb`, where the referenced names are the `as:` aliases of real parameters — confirmed working by the passing spec "fails to create pages domain without key". Caveat: the generator skips `mutually_exclusive` constraints nested inside request-body object properties, so a typo in a nested block would not surface as a phantom property in the spec. The source-level audit does cover those nested blocks and found nothing further. ## History - Introduced in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167901 ("REST API: add an endpoint to configure pull mirrors"), commit `ce0aeec2ee7e`, 2024-10-18. - That MR contributed to https://gitlab.com/gitlab-org/gitlab/-/issues/494294. - Owning team per `.gitlab/CODEOWNERS`: `[Source Code Backend] @gitlab-com/create-team/source-code/backend`. The file has no explicit CODEOWNERS entry, but it declares `feature_category :source_code_management`, and the mirror services it calls sit in that section.