Critical bugs found in feature flags evaluation

[luismoyano]

Hello MixPanel! I'm Luis Moyano, maintainer of a json logic gem and also user of mixpanel-ruby in the company where I work.

In the JSONLogic org I've been working on documenting the scope and fitness of all the json logic gems. Because of this I found a couple of critical bugs and need to let you know about it.

Imagine a scenario setup where a product team wants to introduce a flow only to users who are NOT beta testers:

# Requires: gem install mixpanel-ruby webmock

require 'mixpanel-ruby'
require 'webmock/api'
include WebMock::API

WebMock.enable!

# Mock API response with a flag targeting is_beta_tester == false
flag_response = {
  'flags' => [{
    'key' => 'new_feature',
    'context' => 'distinct_id',
    'hash_salt' => 'salt',
    'ruleset' => {
      'rollout' => [{
        'rollout_percentage' => 100.0,
        'runtime_evaluation_rule' => {
          '==' => [{ 'var' => 'is_beta_tester' }, false]
        }
      }],
      'variants' => [{ 'key' => 'on', 'value' => 'enabled', 'split' => 100.0 }]
    }
  }]
}

stub_request(:get, /flags\/definitions/)
  .to_return(status: 200, body: flag_response.to_json)

tracker = Mixpanel::Tracker.new('token', nil, 
  local_flags_config: { enable_polling: false }) { |_,_| }
tracker.local_flags.start_polling_for_definitions!

# User with is_beta_tester = false SHOULD get the feature
context = {
  'distinct_id' => 'user123',
  'custom_properties' => { 'is_beta_tester' => false }
}

result = tracker.local_flags.get_variant_value('new_feature', 'fallback', context)

puts "Expected: 'enabled' (user matches is_beta_tester == false)"
puts "Actual:   '#{result}'"  # => 'fallback' (BUG! The user is incorrectly opted out of the flow)

This happens because json-logic-rb which was introduced in #129 evaluates falsy variables and loose equality incorrectly:

require 'json_logic'

data = { "value" => false }
JsonLogic.apply({ "var" => "value" }, data)
# Expected: false
# Actual: nil

JsonLogic.apply({ "==" => [nil, false] }, {})
# Expected per JSON Logic spec: true (both are falsy - coerced to 0)
# Actual: false

I ran a set of tests and found out almost all json logic implementations in ruby fail to it, the cases have been proposed already to the JSONLogic org for future inclusion and reference, and have also prepared a Ruby Script reproducing the bug directly with mixpanel's gem (attached at the bottom of this issue for you to review).

Because of this, I want to request you guys to consider switching from json-logic-rb to shiny_json_logic; During my years as a developer I've tried to contact the previous maintainers both privately and publicly with very little success and because of that I decided to just do it myself and developed a gem to deliver the right json logic standard to ruby applications.

I've also made it very easy to migrate as shiny_json_logic includes a JsonLogic alias, so JsonLogic.apply works without code changes. Just add it to your gemspec dependencies, change the requirement here for require 'shiny_json_logic' and it will work without any further ado.

If you guys want/need more info, I'll be more than happy to help with testing, answer questions.

Best regards!!

reproduce_mixpanel_bug_comparison.rb.zip