POST /features/:name silently ignores an unrecognized key parameter

Summary

POST /api/v4/features/:name accepts an optional key parameter selecting a rollout strategy (percentage_of_actors or percentage_of_time). When the request enables or disables the flag via a boolean value, an unrecognized or misspelled key is silently ignored: the API returns 201, but the flag ends up fully enabled/disabled instead of rolled out at a percentage, with no indication that key was not applied.

The endpoint is admin-only (lib/api/features.rb:5, before { authenticated_as_admin! }) and declares feature_category :feature_flags (lib/api/features.rb:8).

Steps to reproduce

Send POST /api/v4/features/my_flag with:

• value=true
• key=percentage_of_actor (typo: missing trailing "s" from the documented percentage_of_actors)

Current behaviour

Returns HTTP 201. The flag is fully enabled. No percentage-of-actors rollout is configured, and no error or warning indicates that key was ignored or invalid.

Expected behaviour

Either the unrecognized key is rejected with an error, or the response indicates it was not applied.

Root cause

In app/services/admin/set_feature_flag_service.rb, update_global (around lines 58-77) evaluates conditions in order:

1. if enable? -> Feature.enable(name)
2. elsif disable? -> Feature.disable(name)
3. elsif percentage_of_actors? -> Feature.enable_percentage_of_actors(name, percentage)
4. elsif percentage_of_time? -> Feature.enable_percentage_of_time(name, percentage)
5. else raise UnknownOperationError

enable?/disable? are decided purely by value and never consult key. When value is boolean, one of the first two branches matches immediately and key is never read, whatever it contains.

The failure is value-dependent: with a non-boolean value (for example value=50), the first two branches don't match, so an unrecognized key falls through to else, raising UnknownOperationError (converted at line 31 to ServiceResponse.error(reason: :illegal_operation)). Only the enable/disable paths are silent.

A comment at set_feature_flag_service.rb:41 notes the branch ordering in update_targets/update_global is deliberately order-dependent, so a naive reorder is probably not a safe fix on its own.

The sibling update_targets method isn't affected: lib/api/features.rb:92-100 declares nine separate mutually_exclusive :key, ... rules (against feature_group, user, group, namespace, project, organization, repository, runner and endpoint), so key can never accompany a gate target.

There is also no test coverage: spec/requests/api/features_spec.rb only ever sends a correctly-spelled key (key: 'percentage_of_actors', line 262). Nothing documents whether the silent-ignore behaviour is intended.

Why fixing it is a breaking change

Any fix turns a request that currently returns 201 into an error. doc/development/api_styleguide.md:622 classifies a status-code change (other than away from 500) as breaking, and success-to-error is the most sensitive form. Mitigating factors: the endpoint is instance-admin-only, and the current behaviour already produces a materially wrong result (full enable/disable instead of a percentage rollout), arguably worse than an explicit rejection.

Possible fixes

1. Validate key inside set_feature_flag_service.rb when present, raising UnknownOperationError before the enable/disable branches short-circuit past it.
2. Declare values: %w[percentage_of_actors percentage_of_time] on the Grape key parameter in lib/api/features.rb, rejecting invalid values at the parameter layer with HTTP 400 before the service is reached.

Notes

Option 2 was implemented and then reverted while working on #612739 (closed) (declaring Grape parameter allowed-values/defaults that currently exist only in prose), specifically because it turns the 201 into a 400 for malformed key values, and that breaking-change decision needs to be made deliberately rather than riding along in a documentation-focused merge request. That analysis is recorded there, so it does not need to be redone.