Merge pull request #1638 from strongloop/exclude-hidden-properties-from-where

Report errors if where clause contains hidden properties

lib/dao.js

@@ -26,7 +26,7 @@ var geo = require('./geo');
 var Memory = require('./connectors/memory').Memory;
 var utils = require('./utils');
 var fieldsToArray = utils.fieldsToArray;
-var removeUndefined = utils.removeUndefined;
+var sanitizeQueryOrData = utils.sanitizeQuery;
 var setScopeValuesFromWhere = utils.setScopeValuesFromWhere;
 var idEquals = utils.idEquals;
 var mergeQuery = utils.mergeQuery;
@@ -406,7 +406,7 @@ DataAccessObject.create = function(data, options, cb) {
     obj.trigger('create', function(createDone) {
       obj.trigger('save', function(saveDone) {
         var _idName = idName(Model);
-        var val = removeUndefined(obj.toObject(true));
+        var val = Model._sanitize(obj.toObject(true));
         function createCallback(err, id, rev) {
           if (id) {
             obj.__data[_idName] = id;
@@ -635,7 +635,7 @@ DataAccessObject.upsert = function(data, options, cb) {
         }
 
         function callConnector() {
-          update = removeUndefined(update);
+          update = Model._sanitize(update);
           context = {
             Model: Model,
             where: ctx.where,
@@ -804,11 +804,11 @@ DataAccessObject.upsertWithWhere = function(where, data, options, cb) {
         function callConnector() {
           try {
             // Support an optional where object
-            var handleUndefined = Model._getSetting('normalizeUndefinedInQuery');
+            var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
-            // alter configuration of how removeUndefined handles undefined values
+            // alter configuration of how sanitizeQuery handles undefined values
-            ctx.where = removeUndefined(ctx.where, handleUndefined);
+            ctx.where = Model._sanitize(ctx.where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
             ctx.where = Model._coerce(ctx.where, options);
-            update = removeUndefined(update);
+            update = Model._sanitize(update);
             update = Model._coerce(update, options);
           } catch (err) {
             return process.nextTick(function() {
@@ -989,7 +989,7 @@ DataAccessObject.replaceOrCreate = function replaceOrCreate(data, options, cb) {
           }, update, options);
 
           function callConnector() {
-            update = removeUndefined(update);
+            update = Model._sanitize(update);
             context = {
               Model: Model,
               where: where,
@@ -1170,7 +1170,7 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
       });
     }
 
-    data = removeUndefined(data);
+    data = Model._sanitize(data);
     var context = {
       Model: Model,
       where: query.where,
@@ -1477,6 +1477,7 @@ DataAccessObject._getSetting = function(key) {
 };
 
 var operators = {
+  eq: '=',
   gt: '>',
   gte: '>=',
   lt: '<',
@@ -1578,9 +1579,9 @@ DataAccessObject._normalize = function(filter, options) {
       Object.keys(this.definition.properties), this.settings.strict);
   }
 
-  var handleUndefined = this._getSetting('normalizeUndefinedInQuery');
+  var normalizeUndefinedInQuery = this._getSetting('normalizeUndefinedInQuery');
-  // alter configuration of how removeUndefined handles undefined values
+  // alter configuration of how sanitizeQuery handles undefined values
-  filter = removeUndefined(filter, handleUndefined);
+  filter = this._sanitize(filter, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
   this._coerce(filter.where, options);
   return filter;
 };
@@ -1643,6 +1644,38 @@ function coerceArray(val) {
   return arrayVal;
 }
 
+DataAccessObject._getHiddenProperties = function() {
+  var settings = this.definition.settings || {};
+  var result = settings.hiddenProperties || settings.hidden;
+  if (typeof result === 'string') {
+    result = [result];
+  }
+  result = result || [];
+  return result;
+};
+
+DataAccessObject._getProtectedProperties = function() {
+  var settings = this.definition.settings || {};
+  var result = settings.protectedProperties || settings.protected;
+  if (typeof result === 'string') {
+    result = [result];
+  }
+  result = result || [];
+  return result;
+};
+
+DataAccessObject._sanitize = function(query, options) {
+  options = options || {};
+
+  // Check violation of keys
+  var prohibitedKeys = this._getHiddenProperties();
+  if (options.excludeProtectedProperties) {
+    prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties());
+  }
+  return sanitizeQueryOrData(query,
+    Object.assign({prohibitedKeys: prohibitedKeys}, options));
+};
+
 /*
  * Coerce values based the property types
  * @param {Object} where The where clause
@@ -1653,10 +1686,9 @@ function coerceArray(val) {
  */
 DataAccessObject._coerce = function(where, options) {
   var self = this;
-  if (!where) {
+  if (where == null) {
     return where;
   }
-
   options = options || {};
 
   var err;
@@ -1791,6 +1823,7 @@ DataAccessObject._coerce = function(where, options) {
       // NOOP when not coercable into an array.
     }
 
+    var allowExtendedOperators = self._allowExtendedOperators(options);
     // Coerce the array items
     if (Array.isArray(val)) {
       for (var i = 0; i < val.length; i++) {
@@ -1802,7 +1835,6 @@ DataAccessObject._coerce = function(where, options) {
       }
     } else {
       if (val != null) {
-        var allowExtendedOperators = self._allowExtendedOperators(options);
         if (operator === null && val instanceof RegExp) {
           // Normalize {name: /A/} to {name: {regexp: /A/}}
           operator = 'regexp';
@@ -1814,12 +1846,28 @@ DataAccessObject._coerce = function(where, options) {
         } else if (allowExtendedOperators && typeof val === 'object') {
           // Do not coerce object values when extended operators are allowed
         } else {
+          if (!allowExtendedOperators) {
+            var extendedOperators = Object.keys(val).filter(function(k) {
+              return k[0] === '$';
+            });
+            if (extendedOperators.length) {
+              const msg = g.f('Operators "' + extendedOperators.join(', ') + '" are not allowed in query');
+              const err = new Error(msg);
+              err.code = 'OPERATOR_NOT_ALLOWED_IN_QUERY';
+              err.statusCode = 400;
+              err.details = {
+                operators: extendedOperators,
+                where: where,
+              };
+              throw err;
+            }
+          }
           val = DataType(val);
         }
       }
     }
     // Rebuild {property: {operator: value}}
-    if (operator) {
+    if (operator && operator !== 'eq') {
       var value = {};
       value[operator] = val;
       if (exp.options) {
@@ -2277,9 +2325,9 @@ DataAccessObject.destroyAll = function destroyAll(where, options, cb) {
     } else {
       try {
         // Support an optional where object
-        var handleUndefined = Model._getSetting('normalizeUndefinedInQuery');
+        var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
-        // alter configuration of how removeUndefined handles undefined values
+        // alter configuration of how sanitizeQuery handles undefined values
-        where = removeUndefined(where, handleUndefined);
+        where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
         where = Model._coerce(where, options);
       } catch (err) {
         return process.nextTick(function() {
@@ -2432,9 +2480,9 @@ DataAccessObject.count = function(where, options, cb) {
   where = query.where;
 
   try {
-    var handleUndefined = Model._getSetting('normalizeUndefinedInQuery');
+    var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
-    // alter configuration of how removeUndefined handles undefined values
+    // alter configuration of how sanitizeQuery handles undefined values
-    where = removeUndefined(where, handleUndefined);
+    where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
     where = this._coerce(where, options);
   } catch (err) {
     process.nextTick(function() {
@@ -2540,7 +2588,7 @@ DataAccessObject.prototype.save = function(options, cb) {
     function save() {
       inst.trigger('save', function(saveDone) {
         inst.trigger('update', function(updateDone) {
-          data = removeUndefined(data);
+          data = Model._sanitize(data);
           function saveCallback(err, unusedData, result) {
             if (err) {
               return cb(err, inst);
@@ -2727,11 +2775,11 @@ DataAccessObject.updateAll = function(where, data, options, cb) {
   function doUpdate(where, data) {
     try {
       // Support an optional where object
-      var handleUndefined = Model._getSetting('normalizeUndefinedInQuery');
+      var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery');
-      // alter configuration of how removeUndefined handles undefined values
+      // alter configuration of how sanitizeQuery handles undefined values
-      where = removeUndefined(where, handleUndefined);
+      where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery});
       where = Model._coerce(where, options);
-      data = removeUndefined(data);
+      data = Model._sanitize(data);
       data = Model._coerce(data, options);
     } catch (err) {
       return process.nextTick(function() {
@@ -3064,7 +3112,7 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
 
     function validateAndCallConnector(err, data) {
       if (err) return cb(err);
-      data = removeUndefined(data);
+      data = Model._sanitize(data);
       // update instance's properties
       inst.setAttributes(data);
 
@@ -3216,7 +3264,7 @@ function(data, options, cb) {
   if (data instanceof Model) {
     data = data.toObject(false);
   }
-  data = removeUndefined(data);
+  data = Model._sanitize(data);
 
   // Make sure id(s) cannot be changed
   var idNames = Model.definition.idNames();
@@ -3289,7 +3337,7 @@ function(data, options, cb) {
           inst.trigger('update', function(done) {
             copyData(data, inst);
             var typedData = convertSubsetOfPropertiesByType(inst, data);
-            context.data = removeUndefined(typedData);
+            context.data = Model._sanitize(typedData);
 
             // Depending on the connector, the database response can
             // contain information about the updated record(s), but also

lib/include.js

@@ -204,6 +204,13 @@ Inclusion.include = function(objects, include, options, cb) {
    * @param cb
    */
   function findWithForeignKeysByPage(model, filter, fkName, pageSize, options, cb) {
+    try {
+      model._sanitize(filter.where, {excludeProtectedProperties: true});
+      model._coerce(filter.where);
+    } catch (e) {
+      return cb(e);
+    }
+
     var foreignKeys = [];
     if (filter.where[fkName]) {
       foreignKeys = filter.where[fkName].inq;

lib/include_utils.js

@@ -5,6 +5,8 @@
 
 'use strict';
 
+var g = require('strong-globalize')();
+
 module.exports.buildOneToOneIdentityMapWithOrigKeys = buildOneToOneIdentityMapWithOrigKeys;
 module.exports.buildOneToManyIdentityMapWithOrigKeys = buildOneToManyIdentityMapWithOrigKeys;
 module.exports.join = join;
@@ -15,7 +17,7 @@ const util = require('util');
 function getId(obj, idName) {
   var id = obj && obj[idName];
   if (id == null) {
-    const msg = util.format('ID property "%s" is missing for included item: %j. ' +
+    const msg = g.f('ID property "%s" is missing for included item: %j. ' +
       'Please make sure `fields` include "%s" if it\'s present in the `filter`',
     idName, obj, idName);
     const err = new Error(msg);

lib/utils.js

@@ -7,7 +7,7 @@
 exports.safeRequire = safeRequire;
 exports.fieldsToArray = fieldsToArray;
 exports.selectFields = selectFields;
-exports.removeUndefined = removeUndefined;
+exports.sanitizeQuery = sanitizeQuery;
 exports.parseSettings = parseSettings;
 exports.mergeSettings = exports.deepMerge = deepMerge;
 exports.deepMergeProperty = deepMergeProperty;
@@ -31,6 +31,7 @@ exports.idsHaveDuplicates = idsHaveDuplicates;
 var g = require('strong-globalize')();
 var traverse = require('traverse');
 var assert = require('assert');
+var debug = require('debug')('loopback:juggler:utils');
 
 function safeRequire(module) {
   try {
@@ -302,26 +303,60 @@ function selectFields(fields) {
 }
 
 /**
- * Remove undefined values from the queury object
+ * Sanitize the query object
- * @param query
+ * @param query {object} The query object
- * @param handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore")
+ * @param options
- * @returns {exports.map|*}
+ * @property normalizeUndefinedInQuery {String} either "nullify", "throw" or "ignore" (default: "ignore")
+ * @property prohibitedKeys {String[]} An array of prohibited keys to be removed
+ * @returns {*}
  */
-function removeUndefined(query, handleUndefined) {
+function sanitizeQuery(query, options) {
+  debug('Sanitizing query object: %j', query);
   if (typeof query !== 'object' || query === null) {
     return query;
   }
+  options = options || {};
+  if (typeof options === 'string') {
+    // Keep it backward compatible
+    options = {normalizeUndefinedInQuery: options};
+  }
+  const prohibitedKeys = options.prohibitedKeys;
+  const offendingKeys = [];
+  const normalizeUndefinedInQuery = options.normalizeUndefinedInQuery;
+  const maxDepth = options.maxDepth || 10;
   // WARNING: [rfeng] Use map() will cause mongodb to produce invalid BSON
   // as traverse doesn't transform the ObjectId correctly
-  return traverse(query).forEach(function(x) {
+  const result = traverse(query).forEach(function(x) {
+    /**
+     * Security risk if the client passes in a very deep where object
+     */
+    if (this.level > maxDepth || this.circular) {
+      const msg = g.f('The query object is too deep or circular');
+      const err = new Error(msg);
+      err.statusCode = 400;
+      err.code = 'WHERE_OBJECT_TOO_DEEP';
+      throw err;
+    }
+    /**
+     * Make sure prohibited keys are removed from the query to prevent
+     * sensitive values from being guessed
+     */
+    if (prohibitedKeys && prohibitedKeys.indexOf(this.key) !== -1) {
+      offendingKeys.push(this.key);
+      this.remove();
+      return;
+    }
+
+    /**
+     * Handle undefined values
+     */
     if (x === undefined) {
-      switch (handleUndefined) {
+      switch (normalizeUndefinedInQuery) {
         case 'nullify':
           this.update(null);
           break;
         case 'throw':
           throw new Error(g.f('Unexpected `undefined` in query'));
-          break;
         case 'ignore':
         default:
           this.remove();
@@ -337,10 +372,20 @@ function removeUndefined(query, handleUndefined) {
 
     return x;
   });
+
+  if (offendingKeys.length) {
+    console.error(
+      g.f(
+        'Potential security alert: hidden/protected properties %j are used in query.',
+        offendingKeys
+      )
+    );
+  }
+  return result;
 }
 
-var url = require('url');
+const url = require('url');
-var qs = require('qs');
+const qs = require('qs');
 
 /**
  * Parse a URL into a settings object
@@ -505,7 +550,7 @@ function defineCachedRelations(obj) {
 
 /**
  * Check if the argument is plain object
- * @param {*) obj The obj value
+ * @param {*} obj The obj value
  * @returns {boolean}
  */
 function isPlainObject(obj) {

test/allow-extended-operators.test.js

@@ -67,50 +67,59 @@ describe('allowExtendedOperators', () => {
     }
   }
 
+  function assertOperatorNotAllowed(err) {
+    should.exist(err);
+    err.message.should.match(/Operators "\$exists" are not allowed in query/);
+    err.code.should.equal('OPERATOR_NOT_ALLOWED_IN_QUERY');
+    err.statusCode.should.equal(400);
+    err.details.should.have.property('operators');
+    err.details.should.have.property('where');
+  }
+
   describe('dataSource.settings.allowExtendedOperators', () => {
     context('DAO.find()', () => {
-      it('converts extended operators to string value by default', () => {
+      it('reports invalid operator by default', () => {
         const TestModel = createTestModel();
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('preserves extended operators with allowExtendedOperators set', () => {
         const TestModel = createTestModel({allowExtendedOperators: true});
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
 
       it('`Model.settings.allowExtendedOperators` override data source settings - ' +
-        'converts extended operators', () => {
+        'reports invalid operator', () => {
         const TestModel = createTestModel({allowExtendedOperators: true}, {allowExtendedOperators: false});
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('`Model.settings.allowExtendedOperators` override data source settings - ' +
         'preserves extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: false}, {allowExtendedOperators: true});
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
 
       it('`options.allowExtendedOperators` override data source settings - ' +
-        'converts extended operators', () => {
+        'reports invalid operator', () => {
         const TestModel = createTestModel({allowExtendedOperators: true});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('`options.allowExtendedOperators` override data source settings - ' +
         'preserves extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: false});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
@@ -168,37 +177,37 @@ describe('allowExtendedOperators', () => {
     context('DAO.find()', () => {
       it('preserves extended operators with allowExtendedOperators set', () => {
         const TestModel = createTestModel({}, {allowExtendedOperators: true});
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
 
       it('`dataSource.settings.allowExtendedOperators` honor Model settings - ' +
-        'converts extended operators', () => {
+        'reports invalid operator', () => {
         const TestModel = createTestModel({allowExtendedOperators: true}, {allowExtendedOperators: false});
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('`dataSource.settings.allowExtendedOperators` honor Model settings - ' +
         'preserves extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: false}, {allowExtendedOperators: true});
-        return TestModel.find(extendedQuery()).then((results) => {
+        return TestModel.find(extendedQuery()).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
 
       it('`options.allowExtendedOperators` override Model settings - converts extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: true});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('`options.allowExtendedOperators` Model settings - preserves extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: false});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
@@ -255,7 +264,7 @@ describe('allowExtendedOperators', () => {
     context('DAO.find()', () => {
       it('preserves extended operators with allowExtendedOperators set', () => {
         const TestModel = createTestModel();
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
@@ -263,15 +272,15 @@ describe('allowExtendedOperators', () => {
       it('`dataSource.settings.allowExtendedOperators` honor options settings - ' +
         'converts extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: true});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('`dataSource.settings.allowExtendedOperators` honor options settings - ' +
         'preserves extended operators', () => {
         const TestModel = createTestModel({allowExtendedOperators: false});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });
@@ -279,15 +288,15 @@ describe('allowExtendedOperators', () => {
       it('`Model.settings.allowExtendedOperators` honor options settings - ' +
         'converts extended operators', () => {
         const TestModel = createTestModel({}, {allowExtendedOperators: true});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => {
-          should(results[0].value).eql('[object Object]');
+          assertOperatorNotAllowed(err);
         });
       });
 
       it('`Model.settings.allowExtendedOperators` honor options settings - ' +
         'preserves extended operators', () => {
         const TestModel = createTestModel({}, {allowExtendedOperators: false});
-        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => {
+        return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => {
           should(results[0].value).eql({$exists: true});
         });
       });

test/model-definition.test.js

@@ -258,7 +258,6 @@ describe('ModelDefinition class', function() {
   });
 
   it('should serialize protected properties into JSON', function() {
-    var modelBuilder = memory.modelBuilder;
     var ProtectedModel = memory.createModel('protected', {}, {
       protected: ['protectedProperty'],
     });
@@ -272,18 +271,20 @@ describe('ModelDefinition class', function() {
   });
 
   it('should not serialize protected properties of nested models into JSON', function(done) {
-    var modelBuilder = memory.modelBuilder;
     var Parent = memory.createModel('parent');
     var Child = memory.createModel('child', {}, {protected: ['protectedProperty']});
     Parent.hasMany(Child);
     Parent.create({
       name: 'parent',
     }, function(err, parent) {
+      if (err) return done(err);
       parent.children.create({
         name: 'child',
         protectedProperty: 'protectedValue',
       }, function(err, child) {
+        if (err) return done(err);
         Parent.find({include: 'children'}, function(err, parents) {
+          if (err) return done(err);
           var serialized = parents[0].toJSON();
           var child = serialized.children[0];
           assert.equal(child.name, 'child');
@@ -295,7 +296,6 @@ describe('ModelDefinition class', function() {
   });
 
   it('should not serialize hidden properties into JSON', function() {
-    var modelBuilder = memory.modelBuilder;
     var HiddenModel = memory.createModel('hidden', {}, {
       hidden: ['secret'],
     });
@@ -312,18 +312,20 @@ describe('ModelDefinition class', function() {
   });
 
   it('should not serialize hidden properties of nested models into JSON', function(done) {
-    var modelBuilder = memory.modelBuilder;
     var Parent = memory.createModel('parent');
     var Child = memory.createModel('child', {}, {hidden: ['secret']});
     Parent.hasMany(Child);
     Parent.create({
       name: 'parent',
     }, function(err, parent) {
+      if (err) return done(err);
       parent.children.create({
         name: 'child',
         secret: 'secret',
       }, function(err, child) {
+        if (err) return done(err);
         Parent.find({include: 'children'}, function(err, parents) {
+          if (err) return done(err);
           var serialized = parents[0].toJSON();
           var child = serialized.children[0];
           assert.equal(child.name, 'child');
@@ -334,6 +336,140 @@ describe('ModelDefinition class', function() {
     });
   });
 
+  describe('hidden properties', function() {
+    var Child;
+    beforeEach(givenChildren);
+
+    it('should be removed if used in where', function() {
+      return Child.find({
+        where: {secret: 'guess'},
+      }).then(assertHiddenPropertyIsIgnored);
+    });
+
+    it('should be removed if used in where.and', function() {
+      return Child.find({
+        where: {and: [{secret: 'guess'}]},
+      }).then(assertHiddenPropertyIsIgnored);
+    });
+
+    /**
+     * Create two children with a hidden property, one with a matching
+     * value, the other with a non-matching value
+     */
+    function givenChildren() {
+      Child = memory.createModel('child', {}, {hidden: ['secret']});
+      return Child.create([{
+        name: 'childA',
+        secret: 'secret',
+      }, {
+        name: 'childB',
+        secret: 'guess',
+      }]);
+    }
+
+    function assertHiddenPropertyIsIgnored(children) {
+      // All children are found whether the `secret` condition matches or not
+      // as the condition is removed because it's hidden
+      children.length.should.equal(2);
+    }
+  });
+
+  function assertParentIncludeChildren(parents) {
+    parents[0].toJSON().children.length.should.equal(1);
+  }
+
+  describe('protected properties', function() {
+    var Parent;
+    var Child;
+    beforeEach(givenParentAndChild);
+
+    it('should be removed if used in include scope', function() {
+      Parent.find({
+        include: {
+          relation: 'children',
+          scope: {
+            where: {
+              secret: 'x',
+            },
+          },
+        },
+      }).then(assertParentIncludeChildren);
+    });
+
+    it('should be rejected if used in include scope.where.and', function() {
+      return Parent.find({
+        include: {
+          relation: 'children',
+          scope: {
+            where: {
+              and: [{secret: 'x'}],
+            },
+          },
+        },
+      }).then(assertParentIncludeChildren);
+    });
+
+    it('should be removed if a hidden property is used in include scope', function() {
+      return Parent.find({
+        include: {
+          relation: 'children',
+          scope: {
+            where: {
+              secret: 'x',
+            },
+          },
+        },
+      }).then(assertParentIncludeChildren);
+    });
+
+    function givenParentAndChild() {
+      Parent = memory.createModel('parent');
+      Child = memory.createModel('child', {}, {protected: ['secret']});
+      Parent.hasMany(Child);
+      return Parent.create({
+        name: 'parent',
+      }).then(parent => {
+        return parent.children.create({
+          name: 'child',
+          secret: 'secret',
+        });
+      });
+    }
+  });
+
+  describe('hidden properties in include', function() {
+    var Parent;
+    var Child;
+    beforeEach(givenParentAndChildWithHiddenProperty);
+
+    it('should be rejected if used in scope', function() {
+      return Parent.find({
+        include: {
+          relation: 'children',
+          scope: {
+            where: {
+              secret: 'x',
+            },
+          },
+        },
+      }).then(assertParentIncludeChildren);
+    });
+
+    function givenParentAndChildWithHiddenProperty() {
+      Parent = memory.createModel('parent');
+      Child = memory.createModel('child', {}, {hidden: ['secret']});
+      Parent.hasMany(Child);
+      return Parent.create({
+        name: 'parent',
+      }).then(parent => {
+        return parent.children.create({
+          name: 'child',
+          secret: 'secret',
+        });
+      });
+    }
+  });
+
   it('should throw error for property names containing dot', function() {
     (function() { memory.createModel('Dotted', {'dot.name': String}); })
       .should

test/util.test.js

@@ -8,7 +8,7 @@ var should = require('./init.js');
 var utils = require('../lib/utils');
 var ObjectID = require('bson').ObjectID;
 var fieldsToArray = utils.fieldsToArray;
-var removeUndefined = utils.removeUndefined;
+var sanitizeQuery = utils.sanitizeQuery;
 var deepMerge = utils.deepMerge;
 var rankArrayElements = utils.rankArrayElements;
 var mergeIncludes = utils.mergeIncludes;
@@ -52,33 +52,65 @@ describe('util.fieldsToArray', function() {
   });
 });
 
-describe('util.removeUndefined', function() {
+describe('util.sanitizeQuery', function() {
   it('Remove undefined values from the query object', function() {
     var q1 = {where: {x: 1, y: undefined}};
-    should.deepEqual(removeUndefined(q1), {where: {x: 1}});
+    should.deepEqual(sanitizeQuery(q1), {where: {x: 1}});
 
     var q2 = {where: {x: 1, y: 2}};
-    should.deepEqual(removeUndefined(q2), {where: {x: 1, y: 2}});
+    should.deepEqual(sanitizeQuery(q2), {where: {x: 1, y: 2}});
 
     var q3 = {where: {x: 1, y: {in: [2, undefined]}}};
-    should.deepEqual(removeUndefined(q3), {where: {x: 1, y: {in: [2]}}});
+    should.deepEqual(sanitizeQuery(q3), {where: {x: 1, y: {in: [2]}}});
 
-    should.equal(removeUndefined(null), null);
+    should.equal(sanitizeQuery(null), null);
 
-    should.equal(removeUndefined(undefined), undefined);
+    should.equal(sanitizeQuery(undefined), undefined);
 
-    should.equal(removeUndefined('x'), 'x');
+    should.equal(sanitizeQuery('x'), 'x');
 
     var date = new Date();
     var q4 = {where: {x: 1, y: date}};
-    should.deepEqual(removeUndefined(q4), {where: {x: 1, y: date}});
+    should.deepEqual(sanitizeQuery(q4), {where: {x: 1, y: date}});
 
     // test handling of undefined
     var q5 = {where: {x: 1, y: undefined}};
-    should.deepEqual(removeUndefined(q5, 'nullify'), {where: {x: 1, y: null}});
+    should.deepEqual(sanitizeQuery(q5, 'nullify'), {where: {x: 1, y: null}});
+
+    q5 = {where: {x: 1, y: undefined}};
+    should.deepEqual(sanitizeQuery(q5, {normalizeUndefinedInQuery: 'nullify'}), {where: {x: 1, y: null}});
 
     var q6 = {where: {x: 1, y: undefined}};
-    (function() { removeUndefined(q6, 'throw'); }).should.throw(/`undefined` in query/);
+    (function() { sanitizeQuery(q6, 'throw'); }).should.throw(/`undefined` in query/);
+  });
+
+  it('Report errors for circular or deep query objects', function() {
+    var q7 = {where: {x: 1}};
+    q7.where.y = q7;
+    (function() { sanitizeQuery(q7); }).should.throw(
+      /The query object is too deep or circular/
+    );
+
+    var q8 = {where: {and: [{and: [{and: [{and: [{and: [{and:
+      [{and: [{and: [{and: [{x: 1}]}]}]}]}]}]}]}]}]}};
+    (function() { sanitizeQuery(q8); }).should.throw(
+      /The query object is too deep or circular/
+    );
+
+    var q9 = {where: {and: [{and: [{and: [{and: [{x: 1}]}]}]}]}};
+    (function() { sanitizeQuery(q8, {maxDepth: 4}); }).should.throw(
+      /The query object is too deep or circular/
+    );
+  });
+
+  it('Removed prohibited properties in query objects', function() {
+    var q1 = {where: {secret: 'guess'}};
+    sanitizeQuery(q1, {prohibitedKeys: ['secret']});
+    q1.where.should.eql({});
+
+    var q2 = {and: [{secret: 'guess'}, {x: 1}]};
+    sanitizeQuery(q2, {prohibitedKeys: ['secret']});
+    q2.should.eql({and: [{}, {x: 1}]});
   });
 });