Fix FilterEvaluator dropping false values for string keys

Tiger Watsonrequested to merge
fix-filter-evaluator-false-values into master

What does this MR do and why?

Gitlab::FilterEvaluator.dig_value in lib/gitlab/filter_evaluator.rb:64 returns the wrong result when a path resolves to false on a string-keyed hash: it returns nil instead of false, so rules like eq false never match. This MR fixes the lookup so false is preserved for any key style.

Details

The method resolved each path segment with value[key] || value[key.to_sym]. When the hash uses string keys and the stored value is false, || falls through to the symbol lookup, which returns nil because the key does not exist as a symbol. The rule then compares against nil instead of false.

Before this fix, for object_attributes.default_branch eq false with the value false present: a string-keyed plain Hash returned false (wrong), while a symbol-keyed Hash, an ActiveSupport::HashWithIndifferentAccess, and a SimpleDelegator around a symbol-keyed Hash all returned true. Today's pipeline hook payload (Gitlab::DataBuilder::Pipeline, symbol keys) and webhook integration payloads (Gitlab::WebHooks.prepare_data, indifferent access) are not affected. The affected shape is a string-keyed plain Hash, which is what Ai::CloudEventsFlowTriggerWorker#execute_flow_triggers builds as filter_data (ee/app/workers/concerns/ai/cloud_events_flow_trigger_worker.rb:81-82). No boolean field is exposed there yet, so this is currently latent, not user-visible.

The evaluator is shared by flow triggers, webhook custom filters, and integration filters. object_attributes.default_branch is an allowed filter field with values [true, false] (ee/app/models/ai/flow_trigger.rb:46), so the evaluator should handle false correctly regardless of key style.

The fix replaces the || fallback with value.key?(key) ? value[key] : value[key.to_sym], with a one-line comment explaining why key? is used instead of ||. A missing key still resolves to nil as before.

References

Screenshots or screen recordings

Not applicable, this is a backend-only change.

How to set up and validate locally

In a Rails console:

filter = { 'rules' => [{ 'field' => 'object_attributes.default_branch', 'operator' => 'eq', 'value' => false }] }
Gitlab::FilterEvaluator.evaluate(filter, { 'object_attributes' => { 'default_branch' => false } })
# master: false. With this MR: true.

A new spec was added, written first and confirmed failing with "expected true, got false": context "when a string-keyed value is false" in spec/lib/gitlab/filter_evaluator_spec.rb, evaluating object_attributes.default_branch eq false against { 'object_attributes' => { 'default_branch' => false } }.

Test plan, run and passing locally:

  • spec/lib/gitlab/filter_evaluator_spec.rb (17 examples)
  • ee/spec/services/ai/flow_triggers/filter_evaluator_spec.rb, ee/spec/services/ee/ci/pipelines/hook_service_spec.rb, ee/spec/services/ai/flow_triggers/event_trigger_service_spec.rb, spec/models/projects/triggered_hooks_spec.rb, spec/workers/integrations/execute_worker_spec.rb (66 examples combined)
  • RuboCop on lib/gitlab/filter_evaluator.rb and spec/lib/gitlab/filter_evaluator_spec.rb, no offenses

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

Merge request reports