From b36232d87a645b5a338dbb3b7de91537fdc55e74 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Wed, 22 Jul 2026 11:29:33 +0100 Subject: [PATCH 1/4] Validate snapshot ownership against device group team --- forge/ee/db/controllers/DeviceGroup.js | 7 +++++ .../api/applicationDeviceGroups_spec.js | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/forge/ee/db/controllers/DeviceGroup.js b/forge/ee/db/controllers/DeviceGroup.js index 11bb899eeb..dc55c5b52e 100644 --- a/forge/ee/db/controllers/DeviceGroup.js +++ b/forge/ee/db/controllers/DeviceGroup.js @@ -104,6 +104,13 @@ module.exports = { if (!snapshot) { throw new ValidationError('Snapshot does not exist') } + // ensure the snapshot belongs to the same team as the device group's + // application - never trust a payload-supplied snapshot id to be in-team + const snapshotTeamId = await snapshot.getTeamId() + const application = await deviceGroup.getApplication() + if (!snapshotTeamId || !application || snapshotTeamId !== application.TeamId) { + throw new ValidationError('Snapshot does not belong to the same team') + } snapshotId = snapshot.id } diff --git a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js index ed0638f257..a8c1c787d2 100644 --- a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js +++ b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js @@ -529,6 +529,37 @@ describe('Application Device Groups API', function () { app.comms.devices.sendCommand.callCount.should.equal(0) // no devices should have been sent an update }) + // Bob (BTeam owner) should not be able to point his BTeam device group at an ATeam snapshot. + it('Rejects a targetSnapshot that belongs to another team (400)', async function () { + const { sid, application, deviceGroup, device1of2, device2of2, snapshot } = await prepare() + + // Create an application, instance and snapshot owned by ATeam (foreign to bob's BTeam group) + const foreignApplication = await factory.createApplication({ name: generateName('foreign-app') }, TestObjects.ATeam) + const foreignInstance = await factory.createInstance({ name: generateName('foreign-instance') }, foreignApplication, app.stack, app.template, app.projectType, { start: false }) + const foreignSnapshot = await factory.createSnapshot({ name: generateName('foreign-snapshot') }, foreignInstance, TestObjects.alice) + + // now call the API to point the group at the foreign snapshot + const response = await callUpdate(sid, application, deviceGroup, { + targetSnapshotId: foreignSnapshot.hashid + }) + + // should fail + response.statusCode.should.equal(400) + response.json().should.have.property('code', 'invalid_input') + + const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid) + const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid) + const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid) + + // should not have updated the group or devices - still the original in-team snapshot + updatedDeviceGroup.should.have.property('targetSnapshotId', snapshot.id) + updatedDevice1.should.have.property('targetSnapshotId', snapshot.id) + updatedDevice2.should.have.property('targetSnapshotId', snapshot.id) + + // check no devices got an update command + app.comms.devices.sendCommand.callCount.should.equal(0) + }) + it('Cannot update a device group with empty name', async function () { const sid = await login('bob', 'bbPassword') const application = await factory.createApplication({ name: generateName('app') }, TestObjects.BTeam) From 283c3acd032087af3b66b813e204b8b5b4aaa74e Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Fri, 24 Jul 2026 12:43:27 +0100 Subject: [PATCH 2/4] Ensure snapshot belongs to the same application as the device group --- forge/ee/db/controllers/DeviceGroup.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/forge/ee/db/controllers/DeviceGroup.js b/forge/ee/db/controllers/DeviceGroup.js index dc55c5b52e..1278c94af8 100644 --- a/forge/ee/db/controllers/DeviceGroup.js +++ b/forge/ee/db/controllers/DeviceGroup.js @@ -104,12 +104,22 @@ module.exports = { if (!snapshot) { throw new ValidationError('Snapshot does not exist') } - // ensure the snapshot belongs to the same team as the device group's - // application - never trust a payload-supplied snapshot id to be in-team - const snapshotTeamId = await snapshot.getTeamId() + // ensure the snapshot belongs to the same application as the device + // group - never trust a payload-supplied snapshot id to be in-scope. + // Scoped to application (not just team) because granular RBAC can + // authorize a caller for this application without authorizing them + // for another application in the same team. const application = await deviceGroup.getApplication() - if (!snapshotTeamId || !application || snapshotTeamId !== application.TeamId) { - throw new ValidationError('Snapshot does not belong to the same team') + let snapshotApplicationId = null + if (snapshot.ownerType === 'instance') { + const project = await snapshot.getProject() + snapshotApplicationId = project?.ApplicationId + } else if (snapshot.ownerType === 'device') { + const device = await snapshot.getDevice() + snapshotApplicationId = device?.ApplicationId || (device?.ProjectId ? (await device.getProject())?.ApplicationId : null) + } + if (!application || !snapshotApplicationId || snapshotApplicationId !== application.id) { + throw new ValidationError('Snapshot does not belong to the same application') } snapshotId = snapshot.id } From bc7fe87af3edbafd4c342ffbb142da21bc14fe9d Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Fri, 24 Jul 2026 12:43:57 +0100 Subject: [PATCH 3/4] fix unit test (use own snapshot) --- .../forge/ee/routes/api/applicationDeviceGroups_spec.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js index a8c1c787d2..64f8aab6f4 100644 --- a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js +++ b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js @@ -442,10 +442,13 @@ describe('Application Device Groups API', function () { // first lets verify the devices have the snapshot set as target device1of2.should.have.property('targetSnapshotId', snapshot.id) - device1of2.should.have.property('targetSnapshotId', snapshot.id) + device2of2.should.have.property('targetSnapshotId', snapshot.id) - // create a new snapshot - const newSnapshot = await factory.createSnapshot({ name: generateName('snapshot') }, TestObjects.instance, TestObjects.bob) + // create a new snapshot on an instance in the group's own application - + // TestObjects.instance lives in a different BTeam application and would + // now be rejected by the application-scope check + const ownInstance = await factory.createInstance({ name: generateName('instance') }, application, app.stack, app.template, app.projectType, { start: false }) + const newSnapshot = await factory.createSnapshot({ name: generateName('snapshot') }, ownInstance, TestObjects.bob) // now call the API to update targetSnapshot only const response = await callUpdate(sid, application, deviceGroup, { From fe45c70a6202d76e972263b2077e01a69865fed2 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Fri, 24 Jul 2026 12:45:20 +0100 Subject: [PATCH 4/4] Reject targetSnapshot belonging to a different application --- .../api/applicationDeviceGroups_spec.js | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js index 64f8aab6f4..832222ee71 100644 --- a/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js +++ b/test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js @@ -563,6 +563,53 @@ describe('Application Device Groups API', function () { app.comms.devices.sendCommand.callCount.should.equal(0) }) + // Bob (BTeam) should not be able to point his BTeam device group at a same-team + // snapshot that belongs to a different BTeam application. + it('Rejects a targetSnapshot that belongs to a different application in the same team (400)', async function () { + const { sid, application, deviceGroup, device1of2, device2of2, snapshot } = await prepare() + // Create a second BTeam application (same team, different application) with its own instance snapshot + const otherApplication = await factory.createApplication({ name: generateName('other-app') }, TestObjects.BTeam) + const otherInstance = await factory.createInstance({ name: generateName('other-instance') }, otherApplication, app.stack, app.template, app.projectType, { start: false }) + const otherInstanceSnapshot = await factory.createSnapshot({ name: generateName('other-instance-snapshot') }, otherInstance, TestObjects.bob) + const response = await callUpdate(sid, application, deviceGroup, { + targetSnapshotId: otherInstanceSnapshot.hashid + }) + response.statusCode.should.equal(400) + response.json().should.have.property('code', 'invalid_input') + const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid) + const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid) + const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid) + updatedDeviceGroup.should.have.property('targetSnapshotId', snapshot.id) + updatedDevice1.should.have.property('targetSnapshotId', snapshot.id) + updatedDevice2.should.have.property('targetSnapshotId', snapshot.id) + app.comms.devices.sendCommand.callCount.should.equal(0) + }) + // Same scenario, but the foreign snapshot belongs to an application-owned device + // rather than an instance - exercises the device-owned branch of the fix. + it('Rejects a device-owned targetSnapshot that belongs to a different application in the same team (400)', async function () { + const { sid, application, deviceGroup, device1of2, device2of2, snapshot } = await prepare() + const otherApplication = await factory.createApplication({ name: generateName('other-app') }, TestObjects.BTeam) + const otherDevice = await factory.createDevice({ name: generateName('other-device') }, TestObjects.BTeam, null, otherApplication) + const otherDeviceSnapshot = await app.db.models.ProjectSnapshot.create({ + name: generateName('other-device-snapshot'), + DeviceId: otherDevice.id, + settings: {}, + flows: {} + }) + const response = await callUpdate(sid, application, deviceGroup, { + targetSnapshotId: otherDeviceSnapshot.hashid + }) + response.statusCode.should.equal(400) + response.json().should.have.property('code', 'invalid_input') + const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid) + const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid) + const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid) + updatedDeviceGroup.should.have.property('targetSnapshotId', snapshot.id) + updatedDevice1.should.have.property('targetSnapshotId', snapshot.id) + updatedDevice2.should.have.property('targetSnapshotId', snapshot.id) + app.comms.devices.sendCommand.callCount.should.equal(0) + }) + it('Cannot update a device group with empty name', async function () { const sid = await login('bob', 'bbPassword') const application = await factory.createApplication({ name: generateName('app') }, TestObjects.BTeam)