Rob Lacey

Brighton, UK - contact@robl.me

Software Engineer specialising in Ruby on Rails. I do occasional freelance work, I love working for SARD JV Ltd, they are awesome and give me my pocket money every month. By all means dangle a carrot.


Mongoid OR queries are seriously dangerous

Upgrading to Mongoid 7.x and found a pretty frightening change in behaviour for OR queries. Any unclear OR declarations could have devastating side-effects.

5.4.1

irb(main):001:0> Train.where(id: 1).or(:expiry.gte => Date.today)
=> #<Mongoid::Criteria
  selector: {"_id"=>1, "$or"=>[{"expiry"=>{"$gte"=>2021-07-07 00:00:00 UTC}}]}
  options:  {:sort=>{"number"=>1}}
  class:    Train
  embedded: false>

irb(main):002:0> Train.or(:expiry.gte => Date.today).where(id: 1)
=> #<Mongoid::Criteria
  selector: {"$or"=>[{"expiry"=>{"$gte"=>2021-07-07 00:00:00 UTC}}], "_id"=>1}
  options:  {:sort=>{"number"=>1}}
  class:    Tenant
  embedded: false>

Previously, or queries would append the selector so where(id: 1) and some OR statement

7.3.0

irb(main):006:0> Train.where(id: 1).or(:expiry.gte => Date.today)
=> #<Mongoid::Criteria
  selector: {"$or"=>[{"_id"=>1}, {"expiry"=>{"$gte"=>2021-07-07 00:00:00 UTC}}]}
  options:  {:sort=>{"number"=>1}}
  class:    Train
  embedded: false>

irb(main):007:0> Train.or(:expiry.gte => Date.today).where(id: 1)
=> #<Mongoid::Criteria
  selector: {"$or"=>[{"expiry"=>{"$gte"=>2021-07-07 00:00:00 UTC}}], "_id"=>1}
  options:  {:sort=>{"number"=>1}}
  class:    Train
  embedded: false>

Now it combines them depending on the order they are declared. I am not entirely sure I like either way of declaring this. Having come from ActiveRecord land I would nearly always assume that you’re AND-ing everything you chain onto a scope.

So in this case we could look for our Tenant by ID, but actually find the first unexpired one instead. That’s one hell of a security hole.

I think the lesson here is to be extra cautious when using ‘OR’ in either ActiveRecord or Mongoid since the outcome is not always as obvious as you might think.