Skip to content

Commit 947d988

Browse files
committed
fix(Brain): avoid retaining dangerous robot/brain references
1 parent d2b57bd commit 947d988

File tree

3 files changed

+15
-4
lines changed

3 files changed

+15
-4
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"devDependencies": {
2929
"chai": "~2.1.0",
3030
"coveralls": "^3.0.2",
31+
"is-circular": "^1.0.2",
3132
"mocha": "^5.2.0",
3233
"mockery": "^1.4.0",
3334
"nyc": "^13.0.0",

src/brain.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ let reconstructUserIfNecessary = function (user, robot) {
2222
// populating the new user with its values.
2323
// Also add the `robot` field so it gets a reference.
2424
user.robot = robot
25+
let newUser = new User(id, user)
26+
delete user.robot
2527

26-
return new User(id, user)
28+
return newUser
2729
} else {
2830
return user
2931
}
@@ -39,7 +41,9 @@ class Brain extends EventEmitter {
3941
users: {},
4042
_private: {}
4143
}
42-
this.robot = robot
44+
this.getRobot = function () {
45+
return robot
46+
}
4347

4448
this.autoSave = true
4549

@@ -146,7 +150,7 @@ class Brain extends EventEmitter {
146150
if (data && data.users) {
147151
for (let k in data.users) {
148152
let user = this.data.users[k]
149-
this.data.users[k] = reconstructUserIfNecessary(user, this.robot)
153+
this.data.users[k] = reconstructUserIfNecessary(user, this.getRobot())
150154
}
151155
}
152156

@@ -168,7 +172,7 @@ class Brain extends EventEmitter {
168172
if (!options) {
169173
options = {}
170174
}
171-
options.robot = this.robot
175+
options.robot = this.getRobot()
172176

173177
if (!user) {
174178
user = new User(id, options)
@@ -179,6 +183,7 @@ class Brain extends EventEmitter {
179183
user = new User(id, options)
180184
this.data.users[id] = user
181185
}
186+
delete options.robot
182187

183188
return user
184189
}

test/brain_test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ chai.use(require('sinon-chai'))
1010

1111
const expect = chai.expect
1212

13+
const isCircular = require('is-circular')
14+
1315
// Hubot classes
1416
const Brain = require('../src/brain')
1517
const User = require('../src/user')
@@ -65,6 +67,7 @@ describe('Brain', function () {
6567
expect(user.constructor.name).to.equal('User')
6668
expect(user.id).to.equal('4')
6769
expect(user.name).to.equal('new')
70+
expect(isCircular(this.brain)).to.be.false
6871
})
6972
})
7073

@@ -326,6 +329,8 @@ describe('Brain', function () {
326329
for (let user of this.brain.usersForRawFuzzyName('Guy One')) {
327330
expect(user.constructor.name).to.equal('User')
328331
}
332+
333+
expect(isCircular(this.brain)).to.be.false
329334
})
330335
})
331336
})

0 commit comments

Comments
 (0)