From 947d988cf7cf11056050cb42fe89a47da2a610e1 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Tue, 5 Mar 2019 11:57:20 -0800 Subject: [PATCH] fix(Brain): avoid retaining dangerous robot/brain references --- package.json | 1 + src/brain.js | 13 +++++++++---- test/brain_test.js | 5 +++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index cb53e6016..358e8b9c2 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "devDependencies": { "chai": "~2.1.0", "coveralls": "^3.0.2", + "is-circular": "^1.0.2", "mocha": "^5.2.0", "mockery": "^1.4.0", "nyc": "^13.0.0", diff --git a/src/brain.js b/src/brain.js index b0694a975..bc1e73060 100644 --- a/src/brain.js +++ b/src/brain.js @@ -22,8 +22,10 @@ let reconstructUserIfNecessary = function (user, robot) { // populating the new user with its values. // Also add the `robot` field so it gets a reference. user.robot = robot + let newUser = new User(id, user) + delete user.robot - return new User(id, user) + return newUser } else { return user } @@ -39,7 +41,9 @@ class Brain extends EventEmitter { users: {}, _private: {} } - this.robot = robot + this.getRobot = function () { + return robot + } this.autoSave = true @@ -146,7 +150,7 @@ class Brain extends EventEmitter { if (data && data.users) { for (let k in data.users) { let user = this.data.users[k] - this.data.users[k] = reconstructUserIfNecessary(user, this.robot) + this.data.users[k] = reconstructUserIfNecessary(user, this.getRobot()) } } @@ -168,7 +172,7 @@ class Brain extends EventEmitter { if (!options) { options = {} } - options.robot = this.robot + options.robot = this.getRobot() if (!user) { user = new User(id, options) @@ -179,6 +183,7 @@ class Brain extends EventEmitter { user = new User(id, options) this.data.users[id] = user } + delete options.robot return user } diff --git a/test/brain_test.js b/test/brain_test.js index 22c11f87b..207dcdd85 100644 --- a/test/brain_test.js +++ b/test/brain_test.js @@ -10,6 +10,8 @@ chai.use(require('sinon-chai')) const expect = chai.expect +const isCircular = require('is-circular') + // Hubot classes const Brain = require('../src/brain') const User = require('../src/user') @@ -65,6 +67,7 @@ describe('Brain', function () { expect(user.constructor.name).to.equal('User') expect(user.id).to.equal('4') expect(user.name).to.equal('new') + expect(isCircular(this.brain)).to.be.false }) }) @@ -326,6 +329,8 @@ describe('Brain', function () { for (let user of this.brain.usersForRawFuzzyName('Guy One')) { expect(user.constructor.name).to.equal('User') } + + expect(isCircular(this.brain)).to.be.false }) }) })