Refuse a blank rules or actions element

What does this MR do and why?

API::Validations::Validators::NoBlankElements is now declared on the rules and actions arrays of both Policy Store v2 write endpoints in ee/lib/api/govern/policies.rb, and doc/api/policy_store.md gains a line stating that an entry cannot be blank.

A blank actions entry was accepted and stored on both create and update, answering 201 or 200. That covers actions: [""], actions: [{}], and actions: [[]]. Nothing else refused it, so the malformed value would have surfaced at evaluation, far from its cause. It now answers 400.

A blank rules entry already answered 400 on both routes, but not from one place. On create the parameter layer refused [""] and [{}], and only [[]] reached the store. On update all three reached the store, which refused them in its own terms: rule 0: expected an object with a type for [""] and [[]], and rule 0: unsupported rule type nil for [{}]. Both name the rule index but not the parameter. Every case now answers from the parameter layer, before compilation, in the shape the other parameter errors use: rules[0] is blank. Several blanks in one array are named together, as rules[0], rules[2] is blank.

Messages for a non-blank malformed entry are unchanged, for example rules: [{"type":"not_a_rule"}] still answers rules[0][type] does not have a valid value. A blank entry now carries rules[0] is blank ahead of whatever it already produced. null, [], and valid arrays behave as before, and PATCH with rules: [] still stores the empty array.

The linked issue named both write routes and flagged actions as very likely affected in the same way, asking for that to be confirmed rather than assumed. It was, on both routes.

Design decisions

• type: Array[JSON] instead: rejected, because it drops the element index from every message. rules[1][type] is missing would become rules[type] is missing, trading away precision that already worked to fix one case. Array[Hash] is not available at all, because Grape refuses it as a group type.
• Extracting the shared parameter declarations into one place, as the issue suggested: rejected. The guard sits on the outer array declaration, which the two routes declare differently, so sharing would not reduce the number of places it appears.
• Guarding all four arrays rather than only actions: the guard then does not depend on how each array happens to be declared, and rules gains the position in its message.
• The changed status code is permitted here: input that was accepted now answers 400, which doc/development/api_styleguide.md forbids in general and exempts for an experiment-stage endpoint behind an off-by-default flag. These routes carry route_setting :lifecycle, :experiment, hidden true, and security_policies_v2 disabled by default.
• No changelog entry: these routes sit behind the security_policies_v2 feature flag, disabled by default, and doc/development/feature_flags/_index.md says a change behind a disabled flag should not have one.

How to set up and validate locally

1. On gdk rails console, enable the flag and the instance setting:

Feature.enable(:security_policies_v2)
ApplicationSetting.current.update!(policy_store_experiment_enabled: true)

2. In the same console, confirm the gate the endpoint actually checks:

Feature.enabled?(:security_policies_v2, :instance)             # => true
Gitlab::CurrentSettings.policy_store_experiment_enabled?       # => true
License.feature_available?(:security_orchestration_policies)   # => true

The third needs an Ultimate licence. You also need a personal access token with the api scope, for a user who is an administrator or an owner of the organization. Organization id 1 is the default on a GDK.

3. Try to create a policy with a blank action:

curl --request POST \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store" \
  --header "PRIVATE-TOKEN: <token>" \
  --header "Content-Type: application/json" \
  --data '{"name":"Blank element probe","trigger_type":"deployment_requested","rules":[{"type":"custom","value":"package governance"}],"actions":[""]}' \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 400 with {"error":"actions[0] is blank"}, because on master the same request answers 201 and stores actions: [""]. Nothing is created, so the next step can reuse the name.

4. Create a valid policy:

curl --request POST \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store" \
  --header "PRIVATE-TOKEN: <token>" \
  --header "Content-Type: application/json" \
  --data '{"name":"Blank element probe","trigger_type":"deployment_requested","rules":[{"type":"custom","value":"package governance"}]}' \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 201, because the response body carries the id the later steps need.

5. Patch the policy from step 4 with a blank action:

curl --request PATCH \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store/<policy_id>" \
  --header "PRIVATE-TOKEN: <token>" \
  --data 'actions[]=' \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 400 with {"error":"actions[0] is blank"}, because on master the same request answers 200 and stores actions: [""].

6. Read the policy back:

curl --request GET \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store/<policy_id>" \
  --header "PRIVATE-TOKEN: <token>" \
  --write-out '\nHTTP %{http_code}\n'

Verify actions is still [] and version is still 1, because that proves the refused update from step 5 stored nothing.

7. Send a blank rule, which is where the message changes shape rather than merely appearing:

curl --request PATCH \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store/<policy_id>" \
  --header "PRIVATE-TOKEN: <token>" \
  --data 'rules[]=' \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 400 with {"error":"rules[0] is blank"}, because on master the same request answers 400 with {"error":"rule 0: expected an object with a type"}, which names no parameter.

8. Send several blank rules at once:

curl --request PATCH \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store/<policy_id>" \
  --header "PRIVATE-TOKEN: <token>" \
  --header "Content-Type: application/json" \
  --data '{"rules":["",{"type":"custom","value":"package governance"},{}]}' \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 400 with {"error":"rules[0], rules[2] is blank"}, because every blank position is named rather than only the first.

9. Close on the opposite path:

curl --request PATCH \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store/<policy_id>" \
  --header "PRIVATE-TOKEN: <token>" \
  --header "Content-Type: application/json" \
  --data '{"actions":[{"type":"block"}]}' \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 200 with the action present in the response, because a well-formed actions entry still saves.

10. Delete the policy, so the steps can be run again without tripping the name-uniqueness check:

curl --request DELETE \
  --url "https://gdk.test:3443/api/v4/organizations/1/security/policy_store/<policy_id>" \
  --header "PRIVATE-TOKEN: <token>" \
  --write-out '\nHTTP %{http_code}\n'

Verify this answers 204.

References

• Closes https://gitlab.com/gitlab-org/gitlab/-/issues/613680
• The update endpoint where this was found: !249174 (merged) (merged)
• The create endpoint, which carries the same declarations: !249155 (merged) (merged)
• The API documentation this MR adds to: !248603 (merged) (merged)
• Rule compilation, which is why a blank rules entry is already refused: !249000 (merged) (merged)
• Persists policies to govern_policies: !250282 (merged) (merged)
• A gap this MR does not close, filed separately: https://gitlab.com/gitlab-org/gitlab/-/work_items/618532
• Epic https://gitlab.com/groups/gitlab-org/-/epics/22937